shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

Recommend set -eufo pipefail and discourage POSIX traps

Open mcandre opened this issue 4 months ago • 4 comments

For new checks and feature suggestions

  • [x] https://www.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 a potential problem:

seach-java:

#!/bin/sh
trap ./clean ERR EXIT SIGINT

(false)

ls test/*

find "$JAVA_HOME" -type f -name -print0 |
    xargs -0 echo
$ mkdir -p test/fixtures
$ unset JAVA_HOME
$ ./search-java
...

Here's what shellcheck currently says:

Nothing

Here's what I wanted to see:

  • Prefer function traps to list traps for safety
  • Enable set -e to safeguard against system corruption
  • Enable set -u to safeguard against potentially undefined variables
  • Enable set -f to safeguard against glob portability issues
  • Enable set -o pipefail to safeguard against failures during piped aggregate commands

mcandre avatar Sep 02 '25 00:09 mcandre

Seems shellcheck implements some internal policy to ignore a variable totally if the last one is named with all caps and hasn't been declared in the script implicitly, probably assuming such a variable to be an external global one

For example, in my huge set of scripts implemented to provide flexible customised automated maintenance of remote server routines each script starts with the following header

#!/bin/sh
############ *posix* #############
# shellcheck shell=sh enable=all #
##################################

[ -z "${SHDEBUG}" ] || set -vx

And shellcheck with all its rules enabled (enforced by directive on line 3 of the header) had never produced no warning kinda SC2154: var is referenced but not assigned, it just silently ignores $SHDEBUG

juliyvchirkov avatar Sep 18 '25 07:09 juliyvchirkov

Seems shellcheck implements some internal policy to ignore a variable totally if the last one is named with all caps and hasn't been declared in the script implicitly, probably assuming such a variable to be an external global one

indeed. all caps are treated as globals and ignored.

A optional check exists in this space.

name: check-unassigned-uppercase desc: Warn when uppercase variables are unassigned example: echo $VAR fix: VAR=hello; echo $VAR

brother avatar Sep 18 '25 07:09 brother

@brother As it follows from this document, the whole list of optional checks, including check-unassigned-uppercase, should turn active when enable=all directive is set

And I can confirm linting my scripts with shellcheck I've faced a number of times a lions' share of optional checks listed by shellcheck --list-optional - like add-default-case, check-extra-masked-returns, quote-safe-variables, useless-use-of-cat and deprecate-which

Since I prefer to enable all checks by default and then to suppress the concrete one once per case if any, the header of each my script includes the line # shellcheck shell=sh enable=all #

#!/bin/sh
############ *posix* #############
# shellcheck shell=sh enable=all #
##################################

[ -z "${SHDEBUG}" ] || set -vx

Thus, as I understand, check-unassigned-uppercase should fire for $SHDEBUG, but for some reason it never does

Shellcheck online ignores $SHDEBUG as well

Please advice

juliyvchirkov avatar Sep 18 '25 12:09 juliyvchirkov

The test for -z var is special and might not be the best example for demonstrating that the enable=all is broken. If you change to a echo of the variable the output indicates it to work as expected.

I guess you could wish for a more comprehensive check and warning, beats me what path to choose there.

Image

brother avatar Sep 18 '25 13:09 brother