The "Use && exit 1" for SC2251 can result in a wrong exit code

[bdrung]

The "Correct code" from https://github.com/koalaman/shellcheck/wiki/SC2251 can result in a wrong exit code. I stumbled over this issue while working on https://salsa.debian.org/kernel-team/initramfs-tools/-/merge_requests/144

Here's a snippet that shows the problem:

#!/bin/sh
set -e
! test "${1-}" = "fail"

Shellcheck will correctly complain about SC2251, but this script works correctly (as long as the ! check is the last command):

$ ./example; echo $?
0
$ ./example fail; echo $?
1

Here's what shellcheck recommends:

https://github.com/koalaman/shellcheck/wiki/SC2251 recommends to change the code to:

#!/bin/sh
set -e
test "${1-}" = "fail" && exit 1

This causes the script to always exit with error code 1:

$ ./example; echo $?
1
$ ./example fail; echo $?
1

Here's what I suggest:

Adding || exit 1 would work for this case and probably for all other cases:

#!/bin/sh
set -e
! test "${1-}" = "fail" || exit 1

Then the exit code will be 0 again for the "successful" case:

$ ./example; echo $?
0
$ ./example fail; echo $?
1

[brother]

Reasonable. The wiki is open for edits.

[bdrung]

The error message of shellcheck needs to be changes as well. From

This ! is not on a condition and skips errexit. Use && exit 1 instead, or make sure $? is checked.

to something like:

This ! is not on a condition and skips errexit. Add || exit 1 or make sure $? is checked.

[bdrung]

I updated the wiki.