Shellcheck: Arithmetic operations may exit with code 1; conflicts with errexit

[b0o]

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

When errexit is enabled, it can be dangerous to use "naked" arithmetic operations due to the behavior when the result is 0. According to bash(1):

((expression))
    The  expression is evaluated according to the rules described below under ARITHMETIC EVALUATION.
    If the value of the expression is non-zero, the return status is 0; otherwise the return status is 1.
    This is exactly equivalent to let "expression".

...

let arg [arg ...]
    Each arg is an arithmetic expression to be evaluated (see ARITHMETIC EVALUATION above). 
    If the last arg evaluates to 0, let returns 1; 0 is returned otherwise.

This can manifest in many ways, for example:

#!/bin/bash
set -e
declare -i j=-5
while [[ $j -lt 5 ]]; do
    echo "$j"
    ((j++))
done

The programmer would reasonably expect the result to be the numbers -5 through 4 echoed, but this is the actual result:

-4
-3
-2
-1
0

It may be worth suggesting not to use "naked" arithmetic operations. Safer alternatives are:

j=$((j + 1))
((j++)) || true
_=$((j++))

See also: https://wiki.bash-hackers.org/syntax/arith_expr#arithmetic_expressions_and_return_codes

Here's what shellcheck currently says:

Nothing

[matthewpersico]

More explicit example:

#!/usr/bin/env bash
if [[ $1 == 'barf' ]]
then
    set -e
fi
declare -i j=-5
while [[ $j -lt 5 ]]; do
    echo "before ++ j=$j"
    ((j++))
    echo "after ++ j=$j"
    echo
done

$ ./zero_vs_one
before ++ j=-5
after ++ j=-4

before ++ j=-4
after ++ j=-3

before ++ j=-3
after ++ j=-2

before ++ j=-2
after ++ j=-1

before ++ j=-1
after ++ j=0

before ++ j=0
after ++ j=1

before ++ j=1
after ++ j=2

before ++ j=2
after ++ j=3

before ++ j=3
after ++ j=4

before ++ j=4
after ++ j=5

$ ./zero_vs_one barf
before ++ j=-5
after ++ j=-4

before ++ j=-4
after ++ j=-3

before ++ j=-3
after ++ j=-2

before ++ j=-2
after ++ j=-1

before ++ j=-1
after ++ j=0

before ++ j=0

[leagris]

Another safe alternative with errexit is to follow stand-alone arithmetic with || true or ||:

((j++))||:

Or suffixing the arithmetic expression with |1:

((j++|1))

The same issue will arise when evaluating stand-alone RegEx:

#!/usr/bin/env/bash
set -o errexit
for s in "foo1" "foo2" "foo3" "fooFour" "foo5" "foo6"; do
  [[ "$s" =~ foo([[:digit:]]) ]]
  echo "${BASH_REMATCH[1]}"
done

Outputs:

1
2
3

[Nightfirecat]

Most scripts I've read expect standalone regex to operate that way though. A match would exit 0 and a non-match would exit 1, meaning you can gate code execution using set -x and a simple regex statement, or make pseudo-one line if statements ala [[ "$var" =~ regex ]] && if-statement-body-command. I wouldn't have expected the behavior described in this issue where a non-zero arithmetic result would, on its own, exit that number.

[leagris]

@Nightfirecat I see scripts where (($#)) || return is used as a short-hand of if [ $# -eq 0 ]; then return; fi in functions where arguments are mandatory. The use of stand-alone arithmetic return-code is quite popular.

[b0o]

@Nightfirecat

I wouldn't have expected the behavior described in this issue where a non-zero arithmetic result would, on its own, exit that number.

Probably a typo, but just to clarify: it's actually the inverse - a zero arithmetic result exits with code 1, non-zero results exit with code 0.

Regarding @leagris's regex example, that is more of an example of using a "naked" conditional operation ([[ ]]) than anything to do with the regex test, but I do think it's in the same vein as a "naked" arithmetic operation. However, I think that the behavior of [[ ]] is more intuitive, since it's well-understood what conditions will result in a zero or non-zero exit code.

Still, I think that instances of these "naked" compound commands ((( )), [ ], [[ ]]) without explicit handling of the return code (|| true, || return 0, || echo "foobar") is a code smell; if the programmer wants to cause the script/function to exit/return, they should explicitly do so with an ||-guard. It's more readable, less error-prone, more maintainable, and requires just a few extra characters.

I suppose this leads to a slippery-slope argument: that any command which might exit with a non-zero code (e.g. any command at all) should be guarded with an explicit ||. This leads me to backtrack a bit on the previous statement, and suggest that a ShellCheck warning should only be issued in cases where the non-zero exit code can be counter-intuitive and bug-prone, as with (( )), where such side-effects are not generally expected in the standalone case.

[Pitterling]

see also SC2084 - Remove '$' or use '_=$((expr))' to avoid executing output