shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

feature request: warn discarding exit code of `test`, `[`, and `[[`

Open e-kwsm opened this issue 1 year ago • 2 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 the problem:

#!/bin/bash
(
  test $# -gt 0
# ^-----------^ SCXXXX (level): Do not ignore exit status of test.
  if test $# -gt 0; then return; fi  # OK
)

(
  [ $# -gt 0 ]
# ^----------^ SCXXXX (level): Do not ignore exit status of [.
  [ $# -gt 0 ] || exit  # OK
  [ $# -gt 0 ] && exit  # OK
)

(
  [[ $# -gt 0 ]]
# ^------------^  SCXXXX (level): Do not ignore exit status of [[.
  set -e
  [[ $# -gt 0 ]]  # OK
)

Here's what shellcheck currently says:

Nothing.

Here's what I wanted or expected to see:

SCXXXX (level): Do not ignore exit status of test/[/[[.

e-kwsm avatar Jul 27 '24 19:07 e-kwsm

Hm... The presence of set -e in a script could indicate that it may have been intended for execution to have stopped whenever an exit status of test/[/[[ had been ignored. Wiley

On Sat, Jul 27, 2024, 13:00 Eisuke Kawashima @.***> wrote:

For new checks and feature suggestions

  • https://www.shellcheck.net/ (i.e. the latest commit) currently gives no useful warnings about this
  • 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 ( test $# -gt 0# ^-----------^ SCXXXX (level): Do not ignore exit status of test. if test $# -gt 0; then return; fi # OK )

( [ $# -gt 0 ]# ^----------^ SCXXXX (level): Do not ignore exit status of [. [ $# -gt 0 ] || exit # OK [ $# -gt 0 ] && exit # OK )

( [[ $# -gt 0 ]]# ^------------^ SCXXXX (level): Do not ignore exit status of [[. set -e [[ $# -gt 0 ]] # OK )

Here's what shellcheck currently says:

Nothing. Here's what I wanted or expected to see:

SCXXXX (level): Do not ignore exit status of test/[/[[.

— Reply to this email directly, view it on GitHub https://github.com/koalaman/shellcheck/issues/3031, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUF2F24EGC6BBLS4XLFX7FTZOP36BAVCNFSM6AAAAABLSEDDNOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGQZTGNRQHA3DKOI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

wileyhy avatar Aug 02 '24 10:08 wileyhy

Yes, shellcheck is aware of and tracks set -e's status, which is why in the third example @e-kwsm doesn't show an error message after the set -e.

plambert avatar Aug 22 '24 23:08 plambert

If I may add something here, the return code of (( <expr> )) should be checked too. Consider this:

set -e
I=0
(( I++ ))
echo 'This is not printed.'

Shellcheck recommends this approach instead of let <expr>.

jsmucr avatar Dec 10 '24 08:12 jsmucr