"SC2004 (style): $/${} is unnecessary on arithmetic variables" is too strict

laurenthuberdeau commented 2 months ago

For bugs

Rule Id: SC2004
My shellcheck version (shellcheck --version or "online"): 0.10.0
[x] The rule's wiki page does not already cover this (e.g. https://shellcheck.net/wiki/SC2086)
[x] I tried on https://www.shellcheck.net/ and verified that this is still a problem on the latest commit

#!/bin/sh

foo=1
var1="foo"

echo $(($var1))

Here's what shellcheck currently says:

[Line 3:](javascript:setPosition(3, 1))
foo=1
^-- [SC2034](https://www.shellcheck.net/wiki/SC2034) (warning): foo appears unused. Verify use (or export if used externally).

[Line 6:](javascript:setPosition(6, 9))
echo $(($var1))
        ^-- [SC2004](https://www.shellcheck.net/wiki/SC2004) (style): $/${} is unnecessary on arithmetic variables.

Here's what I wanted or expected to see:

In POSIX shell, indirect expansion of numeric variables can be done using variable expansion in arithmetic expansion. $((var1)) is not equivalent to $(($var1)), the first returns 0 and the second 12.

The https://www.shellcheck.net/wiki/SC2004 page should mention this exception.

ale5000-git commented 2 months ago

I'm not sure where you have tried but with this:

foo=1
var1="foo"
echo $(($var1))
echo $((var1))

you get:

1
1

I have tried both in bash and busybox ash.

laurenthuberdeau commented 2 months ago

I use dash which outputs:

1
test.sh: 4: Illegal number: foo

Curiously, ash, bash, ksh and zsh all seem to do the same thing where variables are recursively expanded until variables (except for variables that appear left in assignment operators) no longer appear in the expression. Dash seems to only expand each variables once and so it errors on foo as it's not a valid integer literal.

Take this code for example:

baz=12
bar="baz"
foo="bar"
var1="foo"
echo $(($var1))
echo $((var1))

It produces on bash, ksh, zsh:

12
12

while dash produces:

test.sh: 5: Illegal number: bar

ale5000-git commented 2 months ago

@koalaman I think that SC2004 should be skipped for dash and there should be a new rule specific to dash that say to avoid multiple levels of "variable variables".

koalaman / shellcheck