Idea: Encourage more local variables

[mcandre]

For new checks and feature suggestions

• shellcheck.net (i.e. the latest commit) currently gives no useful warnings about this
• I searched through https://github.com/koalaman/shellcheck/issues and didn't find anything related

Here's a snippet or screenshot that shows the problem:

#!/bin/bash
foo() {
    x=42
    echo "$x"
}

Here's what shellcheck currently says:

(nothing)

Here's what I wanted or expected to see:

We noticed that you appear to be using a modern bash descendant shell with an unexported, non-command-prefix, function lifetime variable assignment in lowercase outside of the conventional bindings http_proxy, HTTP_PROXY, https_proxy, HTTPS_PROXY, IFS, PATH, LIBS, CC, USER, HOME, BASH_VERSION, etc. etc. and would think it prudent to mark this variable local to reduce the risk of collision with variables in other scripts. If, on the other hand, the variable is intended for to be sourced into other scripts, note that convention is UPPER_SNAKE_CASE with an accompanying export statement.

Notes

I realize it's late in the game to attempt such a feat, but I think it's still practically doable. The lack of an export, lack of a reference to the same variable above or below the function declaration, and casing all point to the intent for the variable to be function scoped. ShellCheck could help notify the programmer that such a variable actually leaks into the rest of the script without a local designator.

The number of false positives is something to consider. A modest whitelist of common exceptions http_proxy, https_proxy, and so on could reduce the need for the programmer to create exceptions to the rule. Hopefully we can leverage casing for most of this.

Some kind of naming convention for not-exported-but-not-function-lifetimed variables could further help the programmer and the linter to establish variable scope more clearly.

[christophgil]

I strongly vote for this.
I keep forgetting the local.

I suggest that global scope variables could be recognized by upper case characters
or maybe by leading underscore as in python

Any other variable in functions should be assumed to be local.

Many thanks. Really cool code checker!
Cheers
C

[brother]

Upper case variables indicate that the scope is environment global, they are set outside the script. Not sure if I have invented this myself or inherited from somewhere.

[dgutson]

I think that this could be a good rule: suggest the usage of local
CC @itaranto

[mirekfranc]

I must say I was surprised that shellcheck doesn't already do this. Were there any attempts at implementing it?

[mirekfranc]

This is not only problem of explicit assignments. For example this...

#!/usr/bin/bash
f()
{
        for i in hello
        do
                echo $i
        done
}

f
echo $i

will print "hello" twice.

$ ./hello
hello
hello

[hramrach]

There is the additional problem that 'local' is not part of POSIX and some shells refuse to implement it.

Consequently, scoping your variables is non-portable.

It is nice thing to have for shells that implement it but probably should not be the default.

[dgutson]

There is the additional problem that 'local' is not part of POSIX and some shells refuse to implement it.

Consequently, scoping your variables is non-portable.

shellcheck is already capable of distinguishing the shell implementation, by reading the shebang, thus emitting specific warnings.

[mirekfranc]

It is nice thing to have for shells that implement it but probably should not be the default.

I think that shellcheck has some notion of what is checking anyway. Either with -s option, shebang or file extention.

Consequently, scoping your variables is non-portable.

Not only because of local or not. For example some shells are using typeset instead of local like ksh93, and for some, I think bash, these two are aliases. On top of that, typeset in ksh93 uses static scoping and works only in function function {} style definition, not in function () {} style.

Consider the following:

x=7
function foo { echo "in foo: $x"; }
function bar { typeset x=42; foo; }
bar
foo

It prints...

in foo: 42
in foo: 7

... in bash, but in ksh93, it prints...

in foo: 7
in foo: 7

... because of static scoping.

Now consider, the same program, but the other style of function definition...

x=7
foo() { echo "in foo: $x"; }
bar() { typeset x=42; foo; }
bar
foo

The bash gives the same output, but ksh93 now prints...

in foo: 42
in foo: 42

... because typeset is ignored.

I'm not sure how other ksh implementations behave and I do realize, it's a minefield, but that deoesn't detract from potential usefulness of some form of warning. My only regret is that my haskell is extremely rusty after some 19 years, so I'm unlikely to contribute myself to this.