Chris's Wiki :: blog/programming/BourneIfCanSetVars

Today I was writing some Bourne shell code where I wanted to run a command, gather its output (if any), and see whether or not it succeeded. My standard form for this is:

res="$(... whatever ...)"
if [ "$?" -eq 0 ]; then ...
  ...
fi

(I could potentially use a pipeline to process the command's output, but there can be reasons to capture the output. Here I deliberately don't want to process any output the command may produce if it reports that it failed.)

When I ran my script through shellcheck (as I always do these days), it reported SC2181, which is, to quote:

SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.

This isn't the first time I've seen SC2181 and as always, I rolled my eyes at it because it seemed obviously wrong, because of course you can't merge these two lines together. But this time I went off to the Shellcheck repository to report it as an issue, and before I reported it as an issue I did a search, and that was when I discovered that Shellcheck was not wrong.

To my surprise, the Bourne shell allows you to perform command substitutions and capture the output in variables in if expressions. You really can write my two lines in a single one as:

if res="$(...)"; then
  ...
fi

You can even set variables to ordinary values if you want to, but 'var=$value' normally or always has an exit status of 0 so it's not very useful to put it in an if expression.

Despite this transformation being possible, I opted not to do it in my case because my command substitution was for a rather long command. In my opinion, it's just as wrong for readability to write:

if res="$( ... giant long command line ...)" ; then

as it would be to write a similar thing in C:

if (res = ptr->someLongFunction(with, many, arguments, that, sprawl)) {

In both cases things are much more readable and clear if you put things on two lines instead of trying to jam everything on one line. The Bourne shell should be written the way I initially did, and the C should be:

res = ptr->someLongFunction(with, many, arguments, that, sprawl);
if (res) {

Just because you can jam the two things together on one line doesn't mean that you should. Sometimes it's clearer on one line and sometimes it isn't.

(Of course ideally you wouldn't have so many arguments to a C function call. Unfortunately long command lines can be unavoidable in shell scripts, because some commands have very verbose options.)

(This elaborates on a tweet of mine, and also on the Fediverse.)

PS: This isn't the first Shellcheck warning that I routinely ignore.