Shellcheck: Suggest lowercase for variables

[tripleee]

For new checks and feature suggestions

• [x] shellcheck.net (i.e. the latest commit) currently gives no useful warnings about this
• [x] 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/sh
FOO=$(somecommand)
BAR=random_string
echo "$FOO is the new $BAR"

Here's what shellcheck currently says:

No issues detected!

Here's what I wanted or expected to see:

Uppercase variable names should be avoided (especially when they _do_ clash with actual system variables) but this is a slightly tricky heuristic to specify properly.

Basically, if all the variables in a script are upper case, display a gentle warning.

Maybe keep a whitelist of known system variables and adapt the previous heuristic accordingly.

[arth1]

Uppercase variable names are often used for good purpose (especially when they _do_ clash with actual system variables - that's likely the entire intent!). Many style guides recommend them, especially to make clear that it's a variable. Some recommend them for all non-local variables, or anything that might be similar to a command name or function.
You can have my $VERBOSE when you pry it out of my cold dead script...

[orbea]

This idea would make shellcheck rather useless for slackbuilds.org that correctly makes use of upper case variables, for example $PRGNAM, $VERSION, and etc. Changing this now would be not only extremely disruptive, it would be pointless and unrealistic.

[koalaman]

This is one of the things that I've wanted but haven't found a good way to do. I like the idea of some kind of style message when there's not a single lower or mixed case variable in the script. If it ignores all variables that comes from the environment or that gets exported, it should be relatively safe.

It definitely shouldn't be complaining for normal use involving necessary uppercase variables like CLASSPATH, GTK2_RC_FILES, RADEON_NO_VTXFMT, and other known and unknown variables though.

It probably needs a proper data flow analyzer and a pedantic/verbose mode. Or maybe it's a job for shfmt.

[Dmole]

I'd be in favor of warning that lower/mixed case variables should be upper case.

Path, path, PATh should all be PATH.

[matthewpersico]

But limit it to the variables that are set by the shell according to the man page. Let's not forget, shellcheck is called shell"check", not shell"programming-nanny".

[tripleee]

Suggesting that users change the private variable $path to $PATH is completely the wrong thing to do. It is a relatively common beginner bug to carelessly choose the name $PATH for a private variable, and then be surprised and stymied when you get ls: command not found for every external command. This is precisely what we want to avoid, and warn against.

[orbea]

While shellcheck does not currently support zsh, it is important to not mix $path and $PATH there as its a special variable which always share each others value.

$ zsh
% PATH=$(getconf PATH)
% echo $PATH
/bin:/usr/bin
% echo $path
/bin /usr/bin
% PATH=$PATH:/foo
% echo $path
/bin /usr/bin /foo

[matthewpersico]

@tripleee - I see what you're saying because of how @Dmole phrased his comment. I think the warning should be limited to the variables that the shell itself provides/depends on, and should be worded something like:

SCXXXX: Your variable "Foo" is a non-case match for the system provided environment variable "FOO." Please make sure that you want to use this variable "Foo" and not "FOO".

where "Foo" could be foo and foO and fOo, etc...

[mvdan]

Or maybe it's a job for shfmt.

Just weighing in now that I've seen this - definitely not a job for shfmt, as it's just an automated formatter. Suggesting changes to variable names would likely be based on a heuristic and personal preferences, so it's more suited for a linter that one would run manually from time to time instead.

It probably needs a proper data flow analyzer

I really hope noone is thinking of implementing this for shell :)