parser: a case pattern may be produced by a substitution (#744) - #746
Merged
Conversation
A case pattern is a word, so a substitution is a legal part of one. None of
the three spellings parsed:
case a in $(printf a)) echo match;; esac
error[E1001]: expected pattern in case statement
The same for the backtick form and for `$((1+2))`. bash, zsh and dash accept
all three.
The case-pattern parser carries a hand-written list of accepted token types.
It included TOK_VARIABLE -- which is why `$var` and `${p}` worked and made the
gap look narrower than it was -- but none of TOK_COMMAND_SUB, TOK_BACKQUOTE or
TOK_ARITH_EXP. The two canonical predicates next door in tokenizer.c,
token_is_argument_word_token and token_is_assignment_value_token, both already
list all three; only this site missed them. Added here rather than by adopting
a predicate, because this list is genuinely its own: it also accepts `*`, `[[`
and `]]`, and excludes keywords.
Not the #494 shape. That issue was a `)` inside a substitution being mistaken
for the pattern terminator; that work is intact, and once these tokens are
accepted such a `)` correctly does not end the pattern:
case ")" in $(printf ')')) echo match;; esac -> match
case ")" in `printf ')'`) echo match;; esac -> match
Both are pinned in the test so the two defects cannot be confused again -- the
issue as filed guessed at the scanner, and the scanner was never involved.
The issue also claimed the `$( )` spelling already worked, which was a control
asserted without being run: every substitution form failed identically. The
scope is corrected on the issue.
tests/integration/test_case_pattern_substitution.c: 21 checks -- all three
spellings as a whole pattern and as part of one, alternation, nesting, a
non-matching pattern that must still fall through, glob metacharacters on the
produced pattern, the two #494 boundary cases, and the eight forms that
already worked and must be untouched. 13 fail against the parent build.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
Closes #744.
The defect
A case pattern is a word, so a substitution is a legal part of one. None of the three spellings parsed:
Same for the backtick form and
$((1+2)). bash, zsh and dash accept all three.Cause
The case-pattern parser carries a hand-written list of accepted token types. It included
TOK_VARIABLE— which is why$varand${p}worked, and what made the gap look narrower than it was — but none ofTOK_COMMAND_SUB,TOK_BACKQUOTE,TOK_ARITH_EXP.The two canonical predicates next door in
tokenizer.c,token_is_argument_word_tokenandtoken_is_assignment_value_token, both already list all three. Only this site missed them. Added here rather than by adopting a predicate, because this list is genuinely its own: it also accepts*,[[,]]and excludes keywords.Not the #494 shape, and the issue guessed wrong
I filed this pointing at the pattern scanner, reasoning it failed to treat a backtick body as opaque. The scanner was never involved. #494's
)-boundary work is intact — once the tokens are accepted, a)inside a substitution correctly does not end the pattern:Both are pinned in the test so the two defects cannot be confused again.
The issue also claimed the
$( )spelling "parses and matches" — a control I asserted without running. Every substitution form failed identically. Scope corrected on the issue.Verification
tests/integration/test_case_pattern_substitution.c— 21 checks: all three spellings as a whole pattern and as part of one, alternation, nesting, a non-matching pattern that must still fall through (so the fix is not "accept and always match"), glob metacharacters on the produced pattern, the two Command substitution mis-terminates at a case-pattern ) #494 boundary cases, and the eight forms that already worked.