Shellcheck: SC2181 suggests to check exit code directly when I check for several possible values

[AleXoundOS]

• Rule Id: SC2181
• My shellcheck version: 0.4.7
• [x] I tried on shellcheck.net and verified that this is still a problem on the latest commit
• [ ] It's not reproducible on shellcheck.net, but I think that's because it's an OS, configuration or encoding issue

Here's a snippet that shows the problem:

#!/usr/bin/env bash

gzip --test "$1"
if [[ $? -eq 0 || $? -eq 2 ]]
then
    echo ok
else
    echo bad
fi

Here's what shellcheck currently says:

In shellcheck-issue.sh line 4:
if [[ $? -eq 0 || $? -eq 2 ]]
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.

Here's what I wanted or expected to see:

Nothing, because the successful operation is not the only exit code I'm testing against. gzip possible exit statuses:

Exit status is normally 0; if an error occurs, exit status is 1. If a warning occurs, exit status is 2.

I want to test for both normal and warning, i.e. any of 0 and 2. And the remaining exit status - 1 falls into the else conditional path.

Moreover, shellcheck is silent for

if $? || [[ $? -eq 2 ]]

that is completely wrong. Since testing the first expression $? actually gets executed and rewrites the gzip exit code value. So that the second [[ $? -eq 2 ]] never matches.

P.S.
Of course, I could test just for exit status 1 in my case, but there could be new exit statues beside 0, 1, 2 in general.

[Nightfirecat]

Why not if gzip --test "$1" || [[ $? -eq 2 ]] ?

[AleXoundOS]

Ok, this works for checking for 2 different values. What about 3?

if gzip --test "$1" || [[ $? -eq 2 ]] || [[ $? -eq 3 ]] will not work as needed. This way we can test $? only once.

[Nightfirecat]

if gzip --test "$1" || [[ $? -eq 2 || $? -eq 3 ]], like in your original snippet?

[AleXoundOS]

@Nightfirecat, thank you, this works. I missed this variant somehow.

[koalaman]

This should still be fixed though.

[dschrempf]

May I add, that whenever a function checks for the last exit code, spellcheck complains about SC2181. E.g., I want to check if the last command succeeded and act accordingly in $PROMP_COMMAND, ike this

function prompt_command() {
if [ $? != 0 ]
then
  ...
fi

PROMPT_COMMAND=prompt_command

I don't think SC2181 can be avoided then.

[qneill]

Also what about subshells with possibly multiple commands?
( set -e; cmd1; cmd2; if [...]; then cmd3; else cmd4; fi; cmd5 ); [ $? -ne 0 ] && exit 1

[dteirney]

I ran into this problem when checking the exit code in a bash EXIT trap function. Pretty sure there isn't any other way to do the check than using $? in that case.

finish() {
    # shellcheck disable=SC2181
    if [ $? -ne 0 ]; then
        echo "ERROR: failed to create or initialize the JSPM package."
    fi
}

# bash ERR traps with unofficial-bash-strict-mode are a complicated beast.
# Easier to trap EXIT and then check the exit code.
trap finish EXIT

[stevecheckoway]

@dschrempf and @dteirney, the way I worked around this is to put

local ret=$?

as the first line of the function and then test against ${ret} instead. It's not ideal, but it works.

[arendtio]

FWIW, I am not even sure SC2181 is a wise suggestion at all. I mean there is the hint towards set -e but if I am not mistaken, there is a difference in execution of a function depending on if it was started as a normal command or from within an condition/if clause.

I think when it is being started as a normal command it does inherit the set -e but when being run as part of an if clause it does not inherit the set set -e:

#!/bin/sh

fail() {                                                                                                                                                                                                                                                                      
    return 1
}   

mySub() {
    printf 'mySub: '
    printf 'Foo'
    fail
    printf 'Bar'
}   

(   
    set -e
    mySub
)
printf '\n'

(   
    set -e
    if mySub; then
        printf ''
    fi
)
printf '\n'

The output is as follows:

$ ./demo.sh 
mySub: Foo
mySub: FooBar

So yes, $? isn't easy to handle due to its variable nature, but suggesting one way is better than the other when they are not resulting in the same execution isn't any better. I mean, when you are using set -e you probably want it to work within your functions too.

Furthermore, I think it is a valid use-case to check the exact return code to know what has gone wrong instead of just having one if handling all error cases without knowing which one occurred.

[koalaman]

@arendtio If anything, this suggestion seems more relevant when set -e is in effect, since it would make any sequence like foo; if [ $? -eq 0 ] pointless

You also only get this suggestion in comparisons against 0, so e.g. foo; if [ $? -eq 42 ] won't trigger it

[arendtio]

@koalaman I think I wasn't precise enough about what the issue is: The problem I am having is that when set -e is active, the two alternatives foo; if [ $? -eq 0 ] and if foo; do not result in the same execution. So they are no alternatives, they are different programs.

Yet SC2181 suggests you can easily replace one with the other, but in reality, it depends on what foo does (and if it requires set -e to be active).

[arendtio]

Maybe this demo2.sh illustrates the problem a bit better:

#!/bin/sh

process() {
        # $1 = Step name
        # $2 = log file

        # write the step to the log
        printf 'Step %s\n' "$1" >>"$2"

        #fail on step B
        if [ "$1" = "B" ]; then
                return 1
        fi
}

mySub() {
        # $1 = name of the test

        # initialize log file
        log="debug_log_$1.txt"
        printf '' >"$log"

        # do the work
        process A "$log"
        process B "$log"
        process C "$log"

        # relying on set -e to remove the debug log if no error occurred
        rm "$log"
}

printf 'Test One: '
(
        set -e
        mySub "One"
        if [ $? -eq 0 ]; then
                printf 'Finished without errors! Not possible... '
        fi
)
printf 'return code %s\n' "$?"

printf 'Test Two: '
(
        set -e
        if mySub "Two"; then
                printf 'Finished without errors! Not possible... '
        fi
)
printf 'return code %s\n' "$?"

Output:

$ ./demo2.sh 
Test One: return code 1
Test Two: Finished without errors! Not possible... return code 0

In addition, the first test leaves the debug_log_One.txt as it is supposed to.

[Akianonymus]

how do i tackle this ??

OPTS=$(getopt -q --options $SHORTOPTS --longoptions $LONGOPTS --name "$PROGNAME" -- "$@")

if [ $? -ne 0 ]; then
short_help
exit 1
fi
eval set -- "$OPTS"

[stevecheckoway]

@Akianonymus, you can put the assignment in the if.

if ! OPTS=$(getopt ...); then
  short_help
  exit 1
fi

[Akianonymus]

@Akianonymus, you can put the assignment in the if.

if ! OPTS=$(getopt ...); then
  short_help
  exit 1
fi

I did try it, it outputs this

if​ ​!​ OPTS= --: command not found 

[stevecheckoway]

@Akianonymus, here's a complete example using GNU's getopt.

#!/bin/sh

shortopts=hv
longopts=help,verbose

help() {
  cat <<HELP_EOF
Usage: $0 [OPTIONS]

Options:
  -h, --help         show this help
  -v, --verbose      be verbose
HELP_EOF
}

if ! opts=$(getopt --options "${shortopts}" --longoptions "${longopts}" -- "$@"); then
  help >&2
  exit 1
fi

eval set -- "${opts}"

verbose=0
while [ $# -gt 0 ]; do
  opt=$1
  shift
  case ${opt} in
    -h|--help)
      help
      exit 0
      ;;
    -v|--verbose)
      verbose=$((verbose + 1))
      ;;
    --)
      break
      ;;
  esac
done
if [ "${verbose}" -gt 0 ]; then
  echo "Verbosity: ${verbose}"
fi

[Akianonymus]

So, it worked, turns out i was missing '" on the variables, thanks for helping out.