Wandering Thoughts archives

2017-07-06

My current views on Shellcheck

As part of the reaction to my last entry, I've been asked how I feel about shellcheck (also). As it happens, I have some opinions here.

Shellcheck is what I'd classify as a linter. I've dealt with a lot of linters and I've wound up with a general approach to looking at them, which I summarize as a linter is interesting to the extent that it gives me useful new information. To do this, a linter needs to do two things; it must find some genuine issues that matter (or at least that I care about), and it can't complain too much about stuff I don't care about (or where it's wrong). Or in other words a useful linter must have some signal (the more the better) and not too much noise.

What is signal is somewhat in the eye of the beholder. It is not enough for a linter to be accurate; it has to be interesting as well. For an example of this, I will turn to Go. The Go community has a whole bunch of high-signal low-noise linters, starting with 'go vet' itself. If gosimple or unparam report something, it is real and you should probably care about it; if you fix something reported by either tool, a project maintainer would probably take your patch. Then there is errcheck. Errcheck is accurate, but it turns out that silently ignoring errors is extremely common in production Go code and probably very few maintainers would accept patches to litter their code with various ways to make errcheck shut up. In practice errcheck is too persnickety and so I never bother running it against my Go code, unlike other Go linters.

(The Go world has settled on a style of linters where there are a bunch, each of which is narrowly focused on a single area. This has spawned the obvious meta-linter project.)

Unfortunately, when I gave Shellcheck a spin it fell much more towards the errcheck end of the linter spectrum than the go vet end. Here is an example that shows one issue I have with it:

; cat testit
#!/bin/sh
echo Error: $1
; shellcheck testit
[...]
echo Error: $1
            ^-- SC2086: Double quote to prevent globbing and word splitting.

This is technically correct yet almost useless in practice. It's not an actual problem; I can't think of a situation where the lack of double quotes will cause an issue here, given that we are just running echo (and it has an initial argument). Perhaps it would be good style to write '"$1"' instead, but then it might be good style to litter your Go code with markers that you're explicitly ignoring errors (and thus make errcheck shut up). Neither are going to happen in this reality.

Linting shell scripts is a hard problem and I applaud Shellcheck for trying. If I was writing very high assurance shell scripts that absolute had to work, even in a hostile environment, I might use Shellcheck and make sure all my scripts passed all its checks. But for finding real issues in my casual-usage shell scripts, it is too much noise and too little signal in its current state.

(Also, according to a test with the version available online through shellcheck.net, it doesn't currently spot my variable reuse bug.)

Another issue or hazard with shellcheck is illustrated by the following warning it spat out when I tried it on a random administration script we have:

In errmail line 22:
  trap "rm -rf $TMPDIR; exit" 0 1 2 13 15
       ^-- SC2064: Use single quotes, otherwise this expands now rather than when signalled.

Actually, having this expand immediately is deliberate. But shellcheck can't tell that because it requires contextual knowledge, and shellcheck has decided to tilt strongly towards having opinions on your style instead of just reporting on things that are almost certainly errors or problems. A linter having style opinions is a completely fair thing and can be valuable, but it basically always raises the linter's noise level (and only really works if you agree with the linter's opinions).

(This also shows the limitations of Shellcheck's abilities, because $TMPDIR never changes in the rest of the script so the actual behavior of the trap would be the same either way. But Shellcheck either can't see that or doesn't care, so it emits this warning for an issue that doesn't actually exist in the real script.)

PS: To be absolutely clear, I don't think this makes Shellcheck a bad idea or a bad implementation. It's just not a tool that I'm likely to find very useful in practice, and thus not one I'm likely to use (despite initial hopes for it, because of course I'd love a good way to find lurking errors in my Bourne shell scripts).

Sidebar: The signal to noise ratio in practice

Some people will say that a linter that produces some signal makes it worth wading through whatever noise in order to get said signal. This might be correct in theory, for extremely high-assurance programming, but it is wrong in practice. Besides other issues, it's generally very hard for humans to find that small amount of signal in a lot of noise.

For less critical programming, it is unquestionably a bad ratio of work to reward. There are almost always going to be more productive things you can do with X amount of time than to pick through lots of linter noise in search of some signal that may or may not be there for your current code.

ShellcheckNoiseVsSignal written at 02:04:04; Add Comment

By day for July 2017: 5 6 7; before July; after July.

Page tools: See As Normal.
Search:
Login: Password:
Atom Syndication: Recent Pages, Recent Comments.

This dinky wiki is brought to you by the Insane Hackers Guild, Python sub-branch.