Accept an escaped ! as negation in [ ] test expressions - #3517
Open
Eljees wants to merge 1 commit into
Open
Conversation
readCondGroup already reads its parentheses with
`readRegularOrEscaped (string "(")`, so `[ \( x \) ]` parses, and SC1028 tells
users to write them that way. readCondNot, however, matched a bare `char '!'`,
so the equally valid `[ \! x ]` was not recognised as a negation: on its own it
produced a bogus SC2057 "Unknown binary operator", and combined with escaped
parentheses it failed to parse at all (SC1072/SC1073).
Both bash and dash treat `!`, `\!` and `"!"` alike in `[ ]`, because test
simply receives the argument `!`. Inside `[[ ]]` they do not: bash rejects
`[[ \! -e foo ]]` with "conditional binary operator expected". The escaped form
is therefore accepted only when parsing `[ ]`, leaving `[[ ]]` as it was.
Fixes koalaman#3475
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.
Fixes #3475.
Symptom
The script is valid and runs correctly. A probe isolates the trigger to the escaped
!, not to the parentheses or the quoting:[ \! \( -x "$a" -a -e "$b" \) ][ ! \( -x "$a" -a -e "$b" \) ][ \( -x "$a" -a -e "$b" \) ][ \! -x "$a" ]The second row matters:
\(and\)are already accepted — and SC1028 actively tells users to write them that way — while\!is not.Cause
readCondGroupreads its parentheses through the escape-tolerant helper:readCondNotmatches a bare character instead:So an escaped
\!never becomesTC_Unary "!". On its own it degrades into a word, which then looks like the left operand of a binary test and yields the spurious SC2057; combined with a group it derails the parse entirely.Why the fix is conditional on
single[ ]and[[ ]]genuinely differ here, so I checked both against real shells rather than assuming:[ ! -x /nonexistent ][ \! -x /nonexistent ][[ ! -x /nonexistent ]][[ \! -x /nonexistent ]]conditional binary operator expectedIn
[ ]the backslash is only shell quoting andteststill receives the argument!. In[[ ]]bash rejects the escaped form outright, so ShellCheck's current complaint there is correct. The escaped form is therefore accepted only when parsing[ ];[[ ]]is untouched and still reports SC2057 for[[ \! -e foo ]].Tests
One per symptom: the parse failure and the spurious SC2057. Both fail on
masterwith the tests alone (*** Failed! Falsified (after 1 test)for exactly these two and nothing else) and pass with the change; the fullcabal testsuite is green on GHC 9.8.4.End-to-end, per
CLAUDE.md:-aremains;[ \! -x "$a" ]is now silent;[ ! -x "$a" ]is unchanged;[[ \! -x "$a" ]]still reports SC2057.One behaviour change worth flagging
[ \! ]— a one-argument test of the literal string!, which every shell evaluates as true — used to parse as a nullary test and now fails to parse. That is because it takes exactly the same path as the bare[ ! ], which already failed to parse before this change. I left that alone: making the negation parser backtrack would also change long-standing behaviour for the unescaped form, which seemed out of scope here. Happy to address it separately if you'd like the two of them fixed together.AI usage
I used Claude to help locate the parser path and draft the change and the tests, following this repository's
.claude/CLAUDE.md. I reviewed every line, established the cause with the probe above rather than by assumption, verified the[ ]versus[[ ]]difference against bash and dash before choosing the conditional form, ran the new tests against unpatchedmasterfirst to confirm they fail, and ran the full test suite and the end-to-end checks myself.