((expression)) at the start of a $(expression) is miss-parsed

[dimo414]

For example:

$((( $exit_code == 0 )) && echo GREEN || echo RED)

This reports a "SC1073 Couldn't parse this $((..)) expression." error, but this evaluates just fine in Bash:

$ exit_code=0
$ echo $((( $exit_code == 0 )) && echo GREEN || echo RED)
GREEN
$ exit_code=1
$ echo $((( $exit_code == 0 )) && echo GREEN || echo RED)
RED

[kurahaupo]

If that works then it's a bug in bash; it should use "longest match wins"
token matching, which in this case should yield $(( and (, rather than $(
and ((.

[kurahaupo]

OK, apparently it's not that simple.

Later versions of Bash have some "smart matching" logic where it counts the
open brackets in the whole structure and checks for a matching closing ))
before deciding whether it's an expression or sub-shell.

Personally I don't think it's a good idea to rely on that;

• it's not portable to older versions of Bash
• it's tantamount to experimental, as there are still corner cases that may break

[dimo414]

I reported it as a bug, and it sounds like POSIX allows this behavior, though it's undefined. It might not be good syntax, but it seems like it shouldn't fail the ShellCheck parser.

[deryni]

Yeah, this is an unfortunate ugliness inherent to shell syntax but it is valid by spec (bottom of section 2.6.3 Command Substitution though the specific quoted text is new to the 2013 edition of the spec the ambiguity is noted in the 2004 edition as well).

That said counting on it working for you is a bit of a gamble since you are playing with what the shell thinks it can parse as an arithmetic expansion versus what it can't. So this should probably be a warning.

[koalaman]

Would the right thing be to warn about $((( and ((( in general, suggesting it should be rewritten as $( (( or $(( ( so that shells and people can more easily decode it?

Or should there be matching logic similar to bash's where only $((( that is intended as $( (( triggers it?

[deryni]

Ideally I think the answer would be both. That said I also think that's problematic.
I think the warning should definitely be added (as explanation for the current parse error if nothing else).

Fixing the parse error would also be nice but runs into the "gamble" bit of my previous comment I think. You'd have to pick a specific sh/bash parsing behavior to emulate and if that parsing changes in sh/bash then shellcheck stops catching/matching the same cases.

(Given that this is a parsing error that prevents shellcheck analysis entirely perhaps a shellcheck directive to instruct shellcheck to skip parsing of this line and pick up again afterwards would be in order? Or a pair of directives (e.g. # SC-end-parse and # SC-start-parse which could also share a single line) to let shellcheck parse around it for people who can't/won't change it? And actually this would even potentially allow for shellcheck to parse scripts embedded in other contexts (rpm spec files, etc.) if the regions were allowed to be defined from start to end as well as just implicitly starting at the top of the file... but maybe this deserves its own filed ticket.)

[koalaman]

Any ((( was a bit noisy and most shells would probably choose to interpret it as (( (, so 825c1b5 adds a warning when it figures that it was supposed to be ( ((.

Optimally this should also have handled e.g. $((foo) ), but frequent missing )s caused it to mistrigger.