Shellcheck: Suggestion: case-statement should always have a default handler.

[nihilus]

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

Here's a snippet or screenshot that shows the problem:

#!/bin/sh
case $space in
[1-6]*)
  Message="All is quiet."
  ;;
[7-8]*)
  Message="Start thinking about cleaning out some stuff.  There's a partition that is $space % full."
  ;;
9[1-8])
  Message="Better hurry with that new disk...  One partition is $space % full."
  ;;
99)
  Message="I'm drowning here!  There's a partition at $space %!"
  ;;
esac

Here's what shellcheck currently says:

Here's what I wanted or expected to see:

Warning: no default case ('*)') in switch-statement!

[orbea]

You should consider examples where you are switching over a predetermined list and are handling all possible cases.

[koalaman]

Huh. I assumed that this would be as draconian as requiring all if statements to have an else, but looking at a corpus of shell scripts, a majority of cases actually appear to benefit from adding one.

To be sure, there are many cases that intentionally and correctly skip it. But there are also many that incorrectly assume they cover everything -- such as getopts loops that assume there will never be a bad option, or $? checks that assume the process will never die from a signal.

As an example, this is a perfectly valid command on Debian:

/etc/init.d/screen-cleanup fasdfasd

It correctly handles start, stop, restart etc, but without a default case it also allows other weird values.

I'm so surprised by this. It might actually warrant an info message.

[koalaman]

371dcdda adds SC2220 for warning about missing default branches specifically when handling getopts arguments, because this is almost always a problem.

Generally warning about all case statements should probably wait until there's a verbose mode.

[nihilus]

@koalaman neat, the rationale behind the suggestion is that usually one of the existing cases can be made the default one.

[orbea]

Maybe not relevant for shellcheck, but I'd like to point out some legacy shells like pdksh or the Slackware version of ash (They switched to dash just recently in the developmental tree) will exit silently on some instances when there is no default case.

[nihilus]

@orbea at least for pdksh it should be relevant unless it is a bug in pdksh itself.

[orbea]

Sorry, I was wrong about pdksh, it had a different bug with a similar result that was not connected to case.

[orbea]

Here is a example which currently does not warn and I do not see how it would gain from a default case.

#!/bin/sh

set -eu

foo=0
case "${FOO:-0}" in
  1) foo=1 ;;
  2) foo=2 ;;
  3) foo=3 ;;
esac

echo $foo

It could always be rewritten like this, but here are certain problems with that...

#!/bin/sh

case "${FOO:-0}" in
  1) foo=1 ;;
  2) foo=2 ;;
  3) foo=3 ;;
  *) foo=0 ;;
esac

echo $foo

For example in this more elaborate case there is extra effort making sure $foo is not undefined.

#!/bin/sh

set -eu

if [ "${BAR:-0}" = 1 ]; then
  case "${FOO:-0}" in
    1) foo=1 ;;
    2) foo=2 ;;
    3) foo=3 ;;
    *) foo=0 ;;
  esac
else
  foo=0
fi

echo $foo

This could be made increasingly more elaborate...

[nihilus]

@orbea: I see you never have encountered bitflipping and weird memory errors on a SoC; if you have a bitflip occuring (it might be part of some low-level hack to bypass security) then catching a default is'nt a bad thing. Like you point out youself, it can easily be rewritten and mitigate bitflipping attacks.

[koalaman]

ShellCheck just got a verbose mode (-S verbose), and if enabled you will now get a SC2249 warning about this. Thanks!

[nihilus]

@koalaman Finally! Thank you very much, maybe you should re-add SC1117? I will check it out at work today.

[koalaman]

I did like the idea of SC1117 but it very rarely resulted in any real benefits, so I'm not sure about reintroducing it even in verbose mode.