shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

&& in the end of script or function if set -e enabled

Open safinaskar opened this issue 9 years ago • 5 comments

Hi. I often use set -e in my scripts. In this mode foo && bar works exactly as I want: if foo fails, then bar will not be evaluated, but the script itself will not exit and will continue to evaluate. But, if this foo && bar is in the end of some function or script, then, unfortunately, exit code of this function or script will be non-zero, which will lead to very hard-to-catch bugs. Consider the following example:

#!/bin/sh

set -e

false && echo Not printed # "Not printed" really will not be printed,
# exactly as I want, this is good :)

echo Continued # But the script will continue evaluating, so
# "Continued" will be printed, again, exactly as I want, this is good,
# too :)

f(){
  false && echo Not printed # Again, "Not printed" really not printed,
  # all is OK :)
}

f
# Attention! Here "f" returned non-zero and "set -e" is enabled, so
# the script will exit here. And this is very annoying. This is very-very
# hard-to-catch bug :(

echo Not printed # Unfortunately, this will not be printed :(

# So, the right way to write such functions (if you use "set -e"):
f(){
  false && echo Not printed
  :
}

Also, && in the end of a script is bad, too. For example:

#!/bin/sh

set -e

false && echo Not printed

This script will return non-zero and this is very unexpected, too.

So, please, add to your checker warning "&& in the end of script or function and "set -e" is enabled, consider adding ":" after the command". Also, I think this should be added even if "set -e" is not enabled (because script in question may be sourced from "set -e"-script).

Also, I recommend you, koalaman, and also, I recommend all people reading this bug report to do the following: please check all your scripts and really consider adding ":" after "&&"-commands in the end of functions and scripts

safinaskar avatar Dec 11 '14 23:12 safinaskar

I thought the construction false && echo would always produce an error with set -e enabled (could it be only some of the shells behave like that?) and for that reason I try to always append it with ||: like false && echo "This is false" ||:.

I'd like to better have this check instead which should also be easier to implement.

timurb avatar Dec 13 '14 11:12 timurb

timurb,

16  2014-12-13 19:05:08  ~# bash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:16  ~# dash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:18  ~# ash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:20  ~# ksh -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:22  ~# zsh -c 'set -e; false && :; echo word'
word

Writing a && b || : is bad idea. Because the shell will not exit if b fails (and you probably want it to exit in such situation). So, one should write a && b; : in such situation (or just a && b if you are sure this is not end of function or script)

safinaskar avatar Dec 13 '14 16:12 safinaskar

@safinaskar Why do you use set -e at all in this example if you don't want to exit on non-zero command return statuses?

koalaman avatar Mar 20 '15 17:03 koalaman

I just always use set -e. And I want to exit on non-zero return, i. e. I want to exit on every non-successful command. But I consider constructs like "a && b" as successful if "a" failed and "b" didn't fail. So, I don't want to exit if "a" failed and "b" is successful. And if this construct is outside of any function, there is really no any exit, exactly as I want. But if this "a && b" stays in the end of function, then this function surprisingly returns non-zero (this contradicts to my intuition because I consider "a && b" as successful and so I consider the function as successful) and then (when the function is called) the script exits (I don't want this).

Remember: this is just simplified examples. This cases may be parts of some real big script. In such script I want to use set -e

safinaskar avatar Mar 20 '15 18:03 safinaskar

I was going to contribute this check, because I faced this problem in the following code:

f() {
  # ...
  [ "$failed" ] && echo 'FAILED'
}
f; echo $?
# 1

The function returns 1 while working fine. I think it's important to warn that the a && b statement returns 1 when b is not executed. It's just like https://www.shellcheck.net/wiki/SC2155

Himura2la avatar May 12 '22 07:05 Himura2la