shellcheck
shellcheck copied to clipboard
Option to make `set -e` and friends mandatory in bash
This is a feature request to enforce "set -e" and similar safeguard options when linting bash. The motivation is to actively guide developers to write saner scripts with fewer unexpected behaviour. This post sums up the reasoning behind this: http://kvz.io/blog/2013/11/21/bash-best-practices/
More precisely this would be
-
set -u
(set -o nounset
) -
set -e
(set -o errexit
) -
set -o pipefail
This enforcement would be rather opinionated and possibly needed to be made optional.
Either way, I'd be happy to know what your thoughts are on having this added. Or am I just missing something obvious and I can already enforce such a behaviour?
Definitely optional and almost certainly off by default. Some of us actually check our return codes to do appropriate actions and expecting, e.g., a global set -e does more harm than good.
On Wed, Jul 08, 2015 at 06:01:24AM -0700, Christoph Burgmer wrote:
This is a feature request to enforce "set -e" and similar safeguard options when linting bash. The motivation is to actively guide developers to write saner scripts with fewer unexpected behaviour. This post sums up the reasoning behind this: http://kvz.io/blog/2013/11/21/bash-best-practices/
More precisely this would be
set -u
(set -o nounset
)set -e
(set -o errexit
)set -o pipefail
This enforcement would be rather opinionated and possibly needed to be made optional.
Either way, I'd be happy to know what your thoughts are on having this added. Or am I just missing something obvious and I can already enforce such a behaviour?
Because using 'set -e' and 'set -u' in general is considered a bad practice:
- http://mywiki.wooledge.org/BashFAQ/105
- http://wiki.bash-hackers.org/scripting/obsolete (search for "error handling")
Now, you could bring up thousands of online references that say otherwise and actually recommend using 'set -e' and 'set -u', but :-) these are usually by people that have no actual clue of the problems with 'set -e', probably because they just stumbled across that option and decided to write a blog post about it (that's how the internet works, right?).
It's easy to tell, because this guy's (Kevin van Zonneveld) bash "best practices" are not really best practices.
Using the 'env' trick in the hash bang is NOT a best practice. It leads to inconvenient behavior if the user is allowed to modify PATH.
The 11th advice doesn't actually work:
dualbus@hp ~ % bash -euc 'if [ "${NAME}" = x ]; then :; fi'
bash: NAME: unbound variable
The double quotes have nothing to do with 'set -u'. If you want to avoid this, then either don't use 'set -u', define the variable before hand, or use:
bash -euc 'if [ "${NAME+var is not set}" = x ]; then :; fi'
Which will cause NAME to expand to 'var is not set' if the variable isn't set.
The magic variables advice is dumb. Not all scripts exist on disk, why does he assume so?
Surrounding variables with {} needlessly is just dumb and gives a false sense of correctness. No, just use {} when needed to disambiguate.
The part about using long options makes sense if you don't care about portability, which is ok, since it's a 'bash best practices'.
Anyways, if you're looking for resources written by people actually involved in bash, look at Greg Wooledge's wiki and the bash hackers wiki.
Eduardo Bustamante https://dualbus.me/
Thanks for the pointers.
It looks like there are different opinions and no satisfiable answer that "just works": If you use "set -e" some commands will quit your script even for some valid cases, while if you stay away from "set -e" you'll have to live with uncaught failures.
I don't think discussing wether the pros outweigh the cons in this thread will lead to much, so I will stay out of the discussion.
I still believe that those in favour of making use of these 3 options might want to use shellcheck to enforce them being used.
On Sat, Jul 11, 2015 at 05:19:09AM -0700, Christoph Burgmer wrote:
I still believe that those in favour of making use of these 3 options might want to use shellcheck to enforce them being used.
I can't agree more. I'm definitely in favor of having shellcheck give detailed advise if it detects that you're using these options. For example, warning in case of potential pitfalls.
Eduardo Bustamante https://dualbus.me/
It seems we are talking about two things though.
- Add an option to recommend
set -e
and others. - Warn about pitfalls when using those options.
I'd be happy to have both.
An option to enable a pseudo-"strict" mode (with appropriate warning about caveats, etc.) is certainly something that would be reasonable (though I can't speak to the technical difficulties involved).
Like requiring every statement to be followed by || true
or || exit/return
to simulate a robust set -e
?
demands of such set -e
s will be super awesome by option.
presence of || true
or grep ....| cat
is separate problem that easy to catch during code review and some time it is he only way out.
Also define a clear IFS variable:
http://redsymbol.net/articles/unofficial-bash-strict-mode/
And make sure that -e, -u, -o pipefail, IFS are set ONLY for the particular shell implementations that support these features. E.g., bash/ksh/zsh tend to support all of these, while dash/posh/ash/sh tend to support very few of these.
+1 would like to an option to enable this rule
It seems we are talking about two things though.
- Add an option to recommend
set -e
and others.- Warn about pitfalls when using those options.
I'd be happy to have both.
I think having both would be very valuable. If we're using this crazy language, we need all the help we can to not footgun ourselves.