SC2154 should optionally strict check to avoid errors when using "set -u"

[kkmuffme]

For bugs

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

For new checks and feature suggestions

• https://www.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
set -u
if [[ -n "$foo" ]]
then
	echo "$foo"
fi

Here's what shellcheck currently says:

No error

Here's what I wanted or expected to see:

foo is referenced but not assigned
with code SC2154

Since this is an error.

Correct/no error code e.g.

#!/bin/bash
set -u
if [[ -n "${foo+x}" ]] && [[ -n "$foo" ]]
then
	echo "$foo"
fi

While usually this is pointless, in some cases the "set -u" is outside of your control (= project requirements, set by a parent file that calls your script,...) and of course these issues always happen when you're on vacation :-)
It would be helpful if there were a strict/pedantic mode for SC2154/check-unassigned-uppercase to check them as if "set -u" were set and report errors accordingly

[wileyhy]

`set -u` is such a test, and it's specified by POSIX. Why risk getting it wrong while reimplementing it when the availability of nounset is already required? Wiley ~ $ head -v test.sh ==> test.sh <== #!/bin/bash -x set -u if [[ -n ${foo} ]] then echo "$foo" fi ~ $ ./test.sh + set -u ./test.sh: line 3: foo: unbound variable ~ $

…

On Sun, Nov 10, 2024, 20:23 kkmuffme ***@***.***> wrote: For bugs - Rule Id 2154: - My shellcheck version: 0.10.0 - The rule's wiki page does not already cover this (e.g. https://shellcheck.net/wiki/SC2086) - I tried on https://www.shellcheck.net/ and verified that this is still a problem on the latest commit For new checks and feature suggestions - https://www.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/bashset -uif [[ -n "$foo" ]]then echo "$foo"fi Here's what shellcheck currently says: No error Here's what I wanted or expected to see: foo is referenced but not assigned with code SC2154 Since this is an error. Correct/no error code e.g. #!/bin/bashset -uif [[ -n "${foo+x}" ]] && [[ -n "$foo" ]]then echo "$foo"fi While usually this is pointless, in some cases the "set -u" is outside of your control (= project requirements, set by a parent file that calls your script,...) and of course these issues always happen when you're on vacation :-) It would be helpful if there were a strict/pedantic mode for SC2154/check-unassigned-uppercase to check them as if "set -u" were set and report errors accordingly — Reply to this email directly, view it on GitHub <#3081>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F25WKTVCDUO3PCLQQFD2AAWK3AVCNFSM6AAAAABRQ7NRESVHI2DSMVQWIX3LMV43ASLTON2WKOZSGY2DQMBZGUZDENI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[kkmuffme]

availability of nounset is already required?

What do you mean?

The problem is that currently there is no way to check a script statically, whether it will fail when "set -u" is added or not.
You'd have to try out all possible scenarios.
In a very simplistic case:

set -u

if [[ "$1" -eq 2 ]]
then
    if [[ "$foo" == "bar" ]]
    then
        echo "hello"
    fi
fi

This would only fail if called with the first arg set to a value of 2.

It would be helpful if shellcheck could report an error for this case just like it normally does with SC2154 already

[wileyhy]

"You'd have to try out all possible scenarios." And isn't that why people say 'limit your scripts to 100 lines?' How else could anyone find all the bugs? But then of course, if the legends are to be believed, some corporations still maintain 20,000 line shell programs on (pick your favorite obscure Unix flavor here). Bourne shell really wasn't written with error handling in mind. I wish it was. You could rewrite it in csh, though, right? ;-)

…

On Mon, Nov 18, 2024, 16:45 kkmuffme ***@***.***> wrote: availability of nounset is already required? What do you mean? The problem is that currently there is no way to check a script statically, whether it will fail when "set -u" is added or not. You'd have to try out all possible scenarios. In a very simplistic case: set -u if [[ "$1" -eq 2 ]] then if [[ "$foo" == "bar" ]] then echo "hello" fi fi This would only fail if called with the first arg set to a value of 2. It would be helpful if shellcheck could report an error for this case just like it normally does with SC2154 already — Reply to this email directly, view it on GitHub <#3081 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F22IVAZQDWDXICAFW3T2BKC2BAVCNFSM6AAAAABRQ7NRESVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBUGQ3TGMBXGA> . You are receiving this because you commented.Message ID: ***@***.***>

[wileyhy]

I'm sorry. There might be a solution. Someone else more knowledgeable than me should take it from here. Wiley

…

On Wed, Nov 20, 2024, 03:32 Wiley Young ***@***.***> wrote: "You'd have to try out all possible scenarios." And isn't that why people say 'limit your scripts to 100 lines?' How else could anyone find all the bugs? But then of course, if the legends are to be believed, some corporations still maintain 20,000 line shell programs on (pick your favorite obscure Unix flavor here). Bourne shell really wasn't written with error handling in mind. I wish it was. You could rewrite it in csh, though, right? ;-) On Mon, Nov 18, 2024, 16:45 kkmuffme ***@***.***> wrote: > availability of nounset is already required? > > What do you mean? > > The problem is that currently there is no way to check a script > statically, whether it will fail when "set -u" is added or not. > You'd have to try out all possible scenarios. > In a very simplistic case: > > set -u > > if [[ "$1" -eq 2 ]] > then > if [[ "$foo" == "bar" ]] > then > echo "hello" > fi > fi > > This would only fail if called with the first arg set to a value of 2. > > It would be helpful if shellcheck could report an error for this case > just like it normally does with SC2154 already > > — > Reply to this email directly, view it on GitHub > <#3081 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AUF2F22IVAZQDWDXICAFW3T2BKC2BAVCNFSM6AAAAABRQ7NRESVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBUGQ3TGMBXGA> > . > You are receiving this because you commented.Message ID: > ***@***.***> >

[kkmuffme]

And isn't that why people say 'limit your scripts to 100 lines?'

Even in 100 lines, there's plenty possible cases. Isn't that why shellcheck is used in the first place?

I'm sorry. There might be a solution. Someone else more knowledgeable than me should take it from here.

I think the solution is already implemented, it just needs a minor tweak for this case to work:
if set -u is set in the script or optionally a new shellcheck config flag is set, SC2154 should report an error for variables used in -z and -n (e.g. if [[ -n "$foo ]]) too, except if its used like -z ${foo+x} (or -n or whatever other checks won't trigger errors in "set -u") (where x might be any non-empty string)

Basically the way how variables get marked as defined needs to be slightly changed