On Mon, Jul 22, 2024 at 02:59:13AM -0400, Eric Sunshine wrote:
> From: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
>
> Unlike "VAR=val cmd" one-shot environment variable assignments which
> exist only for the invocation of 'cmd', those assigned by "VAR=val
> shell-func" exist within the running shell and continue to do so until
> the process exits. check-non-portable-shell.pl warns when it detects
> such usage since, more often than not, the author who writes such an
> invocation is unaware of the undesirable behavior.
>
> However, a limitation of the check is that it only detects such
> invocations when variable assignment (i.e. `VAR=val`) is the first
> thing on the line. Thus, it can easily be fooled by an invocation such
> as:
>
> echo X | VAR=val shell-func
>
> Address this shortcoming by loosening the check so that the variable
> assignment can be recognized even when not at the beginning of the line.
>
> Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> ---
> t/check-non-portable-shell.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
> index b2b28c2ced..44b23d6ddd 100755
> --- a/t/check-non-portable-shell.pl
> +++ b/t/check-non-portable-shell.pl
> @@ -49,7 +49,7 @@ sub err {
> /\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable (use FOO=bar && export FOO)';
> /\blocal\s+[A-Za-z0-9_]*=\$([A-Za-z0-9_{]|[(][^(])/ and
> err q(quote "$val" in 'local var=$val');
> - /^\s*([A-Z0-9_]+=(\w*|(["']).*?\3)\s+)+(\w+)/ and exists($func{$4}) and
> + /\b([A-Z0-9_]+=(\w*|(["']).*?\3)\s+)+(\w+)/ and !/test_env.+=/ and exists($func{$4}) and
Losing "^\s*" means we'll cause false positives, such as:
# VAR=VAL shell-func
echo VAR=VAL shell-func
Regardless of that, the regex will continue to pose problems with:
VAR=$OTHER_VALUE shell-func
VAR=$(cmd) shell-func
VAR=VAL\ UE shell-func
VAR="\"val\" shell-func UE" non-shell-func
Which, of course, should be cases that should be written in a more
orthodox way.
But we will start to detect errors like the ones mentioned in the
message, which are more likely to happen.
I think this change is a good step forward, and I'm happy with it as it
is.
Thanks
> err '"FOO=bar shell_func" assignment extends beyond "shell_func"';
> $line = '';
> # this resets our $. for each file
> --
> 2.45.2
>
