Don't trigger SC2154 (unassigned var) in -n/-z expressions

[trespasserw]

For bugs

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

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

#!/bin/sh

if [ -n "$__wtf__" ]; then
  echo "there is some $__wtf__"
fi

Here's what shellcheck currently says:

Line 3:
if [ -n "$__wtf__" ]; then
         ^-- SC2154: __wtf__ is referenced but not assigned.

Here's what I wanted or expected to see:

No warnings.

Rationale:

The -n and -z checks are idiomatic ways of probing a global variable before it's use.

[leagris]

If you use this, ShellCheck does not complain because it has a default empty value

if [ -n "${__wtf__:-}" ]; then
  echo "there is some $__wtf__"
fi

[kenahoo]

@leagris - would that be considered a workaround for a bug/misfeature in shellcheck, or is your suggestion a "best practice" for how to write these kinds of things in scripts?

[matthewpersico]

I’ve never heard of it as a best practice it I could see it as one. If you get the 2154, then you have to decide: global var or real bug. Using the :- syntax tells you six months from now that you made that decision. Putting an exclusion directive above it may not mean that. Or, either way, get into the habit of commenting such constructs with # gv?

On Mon, Aug 26, 2019 at 5:53 PM Ken Williams ***@***.***> wrote: @leagris <https://github.com/leagris> - would that be considered a workaround for a bug/misfeature in shellcheck, or is your suggestion a "best practice" for how to write these kinds of things in scripts? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1583?email_source=notifications&email_token=AAX365XL76RXCLNR5N2DNGDQGRGEVA5CNFSM4HNTXTEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5FZSFI#issuecomment-525048085>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAX365TOWZYCH7JCH3L7B53QGRGEVANCNFSM4HNTXTEA> .

-- Matthew O. Persico

[leagris]

I agree with @matthewpersico that the default empty value makes it explicit. When you review the code 6 months later, you know that $__wtf__ may come from an external source, and if it is undefined you want to consider it as an empty string.

Now I also think that shellcheck is a bit too zealously complaining on "$__wtf__", because it is also clear this will result in an empty string when variable is not defined.

This is probably a check to move to some strict switch when/if this is implemented in the future like the "\\" backslash escaping in double quote strings.

[kenahoo]

$__wtf__ may come from an external source, and if it is undefined you want to consider it as an empty string.

In most cases I don't actually want to consider it as an empty string, I want to take different action based on the presence or absence of a variable. I'm not sure how defaulting it to an empty string helps either clarity or logic - I feel like it would actually make the reader start wondering "wait, what does the :- logic do again?", and then go down the rabbit hole of https://stackoverflow.com/questions/3601515/how-to-check-if-a-variable-is-set-in-bash again...