fix: parse command-position assignments correctly#1427
Merged
Conversation
A `name=value` after `&&` / `||` or in a pipeline is an assignment, but the single-command parser captured it as a command whose name was the variable, with `=value` as an argument. A variable named for a builtin was then mis-flagged as that builtin: `[[ cond ]] && test=$(x)` drew the test-command katas, `timeout=-t$2` drew the timeout kata, and `seq=$mac` drew the seq katas. Assignment katas also missed these, so a real `fpath=($fpath $dir)` append after `&&` went unreported. reshapeCommandAssignment converts the fully-parsed command node into an `=` InfixExpression when it is a lone glued `=value` argument. It runs after the command is parsed, so it never changes token consumption -- the caller's cursor is untouched, which is what makes it safe where an early re-parse desynced the dispatch loop. The env-prefixed form (`FOO=bar cmd`) and the spaced form (`cd =$x`) stay commands. Corpus effect: 10 false positives removed across 8 lines (variables named test/seq/timeout/GEOMETRY_PROMPT assigned in a logical chain); one real array append now reported. Parser-error sweep stays 402/0. Signed-off-by: afadesigns <afadesign.official@gmail.com>
github-actions
Bot
added
ci
A fuzzed input can leave a nil expression in a concatenated argument's part slice. reshapeCommandAssignment dereferenced `Parts[1]` for a token when building a multi-part assignment RHS, panicking on such input. Use the concatenation's own token for position and bail out of the reshape entirely when any part is nil, so a malformed command stays the SimpleCommand it parsed as. Seed the parser fuzzer with command-position assignment forms, including a truncated one. Signed-off-by: afadesigns <afadesign.official@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What: a
name=valueafter&&/||or in a pipeline is now parsed as an assignment, not a command.Why: the single-command parser captured it as a command whose name was the variable, with
=valueas an argument. A variable named for a builtin was mis-flagged as that builtin --[[ cond ]] && test=$(x)drew the test-command katas,timeout=-t$2drew the timeout kata,seq=$macdrew the seq katas. Assignment katas also missed these, so a realfpath=($fpath $dir)append after&&went unreported.How:
reshapeCommandAssignmentconverts the fully-parsed command node into an=InfixExpression when it is a lone glued=valueargument. It runs after the command is parsed, so it never changes token consumption -- the caller's cursor is untouched, which is what makes it safe where an early re-parse desynced the dispatch loop. The env-prefixed form (FOO=bar cmd) and the spaced form (cd =$x) stay commands.Corpus effect: 10 false positives removed across 8 lines (variables named test/seq/timeout/GEOMETRY_PROMPT assigned in a logical chain); one real array append now reported. Parser-error sweep stays 402/0; fix-corpus and full suite green.
Tests:
TestParseCommandPositionAssignment(parser),TestCommandPositionAssignmentNotMisflaggedandTestCommandPositionAssignmentKeepsRealCommands(katas).Severity: false positives removed, one false negative closed.