shellcheck
shellcheck copied to clipboard
Feature: When using "set -u" and testing if a variable is set, don't forget "${:-}"
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/bash
set -u
if [ -z "$FOOBAR" ]; then
echo "FOOBAR is empty"
fi
#!/bin/bash
set -u
if [ -n "$FOOBAR" ]; then
echo "FOOBAR is not empty"
fi
If "-n" or "-z" is used, the developer probably expects that the variable may be unset. But because "set -u" will abort the program, the test is ineffective.
Here's what shellcheck currently says:
Nothing
Here's what I wanted or expected to see:
SC....: If $FOOBAR is unset, this test will abort the script. Use "${FOOBAR:-}" instead.
Caveat: it is possible that the programmer knows that the variable is set, but still wants to know if it is empty. In this case, using "${FOOBAR:-}" doesn't hurt, but forfeits the error checking of "set -u"
If this change were implemented, I'd have to add a bunch more # shellcheck disable lines in my scripts.
I think it's relatively common to have a variable that should be assigned to something, but you want to test whether or not it's the empty string - for example, if the variable was previously assigned to the output of some command which may or may not have returned any output. In this case, as you mention, using ${foo:-} gives up the error checking.
The whole point of set -u is to abort the script if a variable is undefined, so it seems strange to warn the programmer that this command will work as intended. Given the possibility of variables being defined but empty, the -z and -n tests are no different from any other place in the script where a variable could appear.
If you want to know if a variable is empty, then test for it explicitly.
#!/bin/sh
set -u
case "${foo:-}" in
'') echo "Empty variable." ;;
*) echo "Set variable." ;;
esac
My understanding set -u should exit when it encounters an unset variable so the script's maintainer can fix it with the goal of there being no unset variables.
Old issue, but just became current for me.
A script that was working in a Home Assistant add-on for users not defining "DISPLAY" just broke for them, probably because the Home Assistant Team (member) decided to add more security by requiring that all variables are defined.
So before, the "set +u" was the default and "echo "DISPLAY:'$DISPLAY'" worked even if DISPLAY was not set, now this fails the script.
DISPLAY is conditionnaly set depending on a configuration parameter set by the user.
I checked how to enable checking this in shellscript. I even tried adding 'set -u' to the script explicitally.
So I think that this would be a good check to (optionnaly) add - and automatically add or disable when set -u/set +u is detected.