Jeff King <peff@xxxxxxxx> writes:
> On Fri, Apr 05, 2024 at 05:09:02PM -0700, Junio C Hamano wrote:
>
>> Teach t/check-non-portable-shell.pl that right hand side of the
>> assignment done with "local VAR=VAL" need to be quoted. We
>> deliberately target only VAL that begins with $ so that we can catch
>>
>> - $variable_reference and positional parameter reference like $4
>> - $(command substitution)
>> - ${variable_reference-with_magic}
>>
>> while excluding
>>
>> - $'\n' that is a bash-ism freely usable in t990[23]
>> - $(( arithmetic )) whose result should be $IFS safe.
>> - $? that also is $IFS safe
>
> Hmm. Just porting over my comment from the other thread (before I
> realized you'd written this series), this misses:
>
> local foo=bar/$1
>
> etc. Should we look for the "$" anywhere on the line? I doubt we can get
> things foolproof, but requiring somebody to quote:
>
> local foo=$((1+2))
>
> does not seem like the worst outcome. I dunno.
Looking at the output from
$ git grep -E -e 'local [a-zA-Z0-9_]+=[^"]*[$]' t/
the listed ones in the proposed commit log message are the false
positives. Luckily we didn't have anything that tries to
concatenate parameter reference to something else.
But with the pattern we do miss
local var=$*
and possibly many others. So I am not sure. The false positives
do look moderately bad, so I'd rather start with the simplest one
proposed in the patch.
