If/Boolean as a potential beginner error

[rugk]

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:

Many beginners may be familiar with other languages where you can just do if ( false ) { #code here not executed } and thus e.g. use a variable if ( $featureIsEnabled ) { #execute awesome feature } to conditionally do or not do things. These users may expect that the code is not executed when $featureIsEnabled is e.g. set to false or (in case of variable casting) to 0.

However Linux/POSIX shells seem to do it differently and threat everything as true (even a boolean "false") and only evaluate to false when nothing (an empty string) is given. The example below shows this in code form.

#!/bin/sh
#

vars="0 1 true false"

for var in $vars; do
    # echo "testing $var"
    if [ "$var" ]; then
        echo "var $var is true"
    fi
    if [ ! "$var" ]; then
        echo "var $var is false"
    fi
done

# test boolean: true
myBoolOption=true
if [ "$myBoolOption" ]; then
    echo "var <true> is true"
fi
if [ ! "$myBoolOption" ]; then
    echo "var <true> is false"
fi

# test boolean: false
myBoolOption=false
if [ "$myBoolOption" ]; then
    echo "var <true> is true"
fi
if [ ! "$myBoolOption" ]; then
    echo "var <true> is false"
fi

# test empty string
emptyString=""
if [ "$emptyString" ]; then
    echo "var <> is true"
fi
if [ ! "$emptyString" ]; then
    echo "var <> is false"
fi

Output:

$ ./test.sh
var 0 is true
var 1 is true
var true is true
var false is true
var <true> is true
var <true> is true
var <> is false

Shellcheck does not complain about this code at all.

So for beginners things such as var false is true can be a horrible error cause.

That's why I'd like to see a message from shellcheck in this cases as I think it could help many devs to prevent unnecessary debugging of such an issue, which is not apparent.

I don't know exactly when/how it should complain, but maybe it could show a warning for if [ "$myBoolOption" ]; then and e.g. suggest to use if [ "$myBoolOption"=true ]; then if it detects that $myBoolOption is a boolean or a 0/1 value or so.

[kurahaupo]

Variables don't have "types" so detecting when a variable should be considered a "boolean" is problematic; "false" is a perfectly good string value that has other uses.

Of course, the real problem is the inappropriate pervasive use of [ ... ] (the string-test command) in conjunction with if ... then when they are actually separate constructs and do not have to be used together.

As for the appropriate idiom, just writing

var=false
if $var ; then ...

works fine. As does

readonly true=1 false=  # necessary preamble, just once
...
var=false
if ((var)) ; then ...

And just for the record, your suggested fix would always result in "true", because "=true" is still a non-empty string.

On 5 January 2017 at 08:09, rugk [email protected] wrote:

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:

Many beginners may be familiar with other languages where you can just do if ( false ) { #code here not executed } and thus e.g. use a variable if ( $featureIsEnabled ) { #execute awesome feature } to conditionally do or not do things. These users may expect that the code is not executed when $featureIsEnabled is e.g. set to false or (in case of variable casting) to 0.

However Linux/POSIX shells seem to do it differently and threat everything as true (even a boolean "false") and only evaluate to false when nothing (an empty string) is given. The example below shows this in code form.

#!/bin/sh#

vars="0 1 true false" for var in $vars; do # echo "testing $var" if [ "$var" ]; then echo "var $var is true" fi if [ ! "$var" ]; then echo "var $var is false" fidone

test boolean: true

myBoolOption=trueif [ "$myBoolOption" ]; then echo "var is true"fiif [ ! "$myBoolOption" ]; then echo "var is false"fi

test boolean: false

myBoolOption=falseif [ "$myBoolOption" ]; then echo "var is true"fiif [ ! "$myBoolOption" ]; then echo "var is false"fi

test empty string

emptyString=""if [ "$emptyString" ]; then echo "var <> is true"fiif [ ! "$emptyString" ]; then echo "var <> is false"fi

Output:

$ ./test.shvar 0 is truevar 1 is truevar true is truevar false is truevar is truevar is truevar <> is false

Shellcheck does not complain about this code at all.

So for beginners things such as var false is true can be a horrible error cause.

That's why I'd like to see a message from shellcheck in this cases as I think it could help many devs to prevent unnecessary debugging of such an issue, which is not apparent.

I don't know exactly when/how it should complain, but maybe it could show a warning for if [ "$myBoolOption" ]; then and e.g. suggest to use if [ "$myBoolOption"=true ]; then if it detects that $myBoolOption is a boolean or a 0/1 value or so.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/koalaman/shellcheck/issues/816, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMChXl61-Ryg6KzTK_hCUDrnXczw-xaks5rPAqJgaJpZM4LbEdH .

[rugk]

Nice, indeed a good idea to just use if $var ; then. Maybe suggest this instead.

[cdavie-artium]

I would also like some sort of shellcheck rule that would point this potential gotcha out. Suggesting if $var ; then would be good, or if [ "$var" = "true" ] ; then (or if [[ "$var" == "true" ]] ; then. (Depending on the context, sometimes I prefer the shorter version; other times I prefer to use the strings "true" and "false" but make it more obvious that they are just strings.

[kurahaupo]

@rugk Please don't write:

if $var ; then

It's not totally "wrong", but it has problems.

1. it's not quoted, so
   • if the value of var contains space, tab, or newline, then it will be subject to word splitting;
   • after that, if it contains *, ?, or [...], then it will subject to filename globbing. The speed of the latter will depend on how many files are in the current directory; if you're in a network filesystem, that can take many seconds.
2. It interprets the content of var as a command and runs it; this would be bad if it contained, say, rm -rf /*
3. If you don't set a value, it will inherit whatever is in the environment, which could provided by an attacker (see point 3).

Please use this instead:

   if (( var )) ; then
            

(It's not perfect, but it's much faster, and (in newer shells) more secure; it's certainly no worse, even in older shells.)

[rugk]

Is that POSIX compatible though? Anyway one needs to come up with a proper suggestion of what is actually the best practise/recommend way and then a shellcheck rule could be implemented.