print a warning when the return value of a function is checked when errexit is enabled

[johnatswoopsrch]

I would like shellcheck to print a warning when the return value of a function is checked when errexit is enabled.

For details on this problem, see:

• https://lists.gnu.org/archive/html/bug-bash/2012-12/msg00094.html
• https://stackoverflow.com/questions/19789102/why-is-bash-errexit-not-behaving-as-expected-in-function-calls/19789651#19789651

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:

#!/usr/bin/env bash
set -eu -o pipefail

main() {
    if func ; then
        echo func passed
    else
        echo func failed
    fi
}

func() {
    false
    echo false did not fail
}

main "$@"

Showing the problem:

aMBP:~> /tmp/try
false did not fail
func passed
aMBP:~>

Here's what shellcheck currently says:

It says nothing.

aMBP:~> shellcheck /tmp/try
aMBP:~>

Here's what I wanted or expected to see:

WARNING: checking the return value of a function disables errexit inside the function

[wileyhy]

In the code you provided, errexit was being ignored in the test following the 'if' command, so errrexit's "exit immediately" functionality was disabled when `func` was called. If a second call to `func` is appended to the end of the script, that call will occur in a context where `set -e`s "exit immediately" functionality is active, and "false did not fail" will fail to print. Wiley

…

On Tue, Dec 3, 2024, 08:55 John ***@***.***> wrote: I would like shellcheck to print a warning when the return value of a function is checked when errexit is enabled. For details on this problem, see: - https://lists.gnu.org/archive/html/bug-bash/2012-12/msg00094.html - https://stackoverflow.com/questions/19789102/why-is-bash-errexit-not-behaving-as-expected-in-function-calls/19789651#19789651 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: #!/usr/bin/env bashset -eu -o pipefail main() { if func ; then echo func passed else echo func failed fi } func() { false echo false did not fail } main "$@" Showing the problem: aMBP:~> /tmp/try false did not fail func passed aMBP:~> Here's what shellcheck currently says: It says nothing. aMBP:~> shellcheck /tmp/try aMBP:~> Here's what I wanted or expected to see: WARNING: checking the return value of a function disables errexit inside the function — Reply to this email directly, view it on GitHub <#3095>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F22QPTMOLA4EA4ACWGT2DXO77AVCNFSM6AAAAABS6HTDVOVHI2DSMVQWIX3LMV43ASLTON2WKOZSG4YTKNJTGY2DEMA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[wileyhy]

s/command/reserved word

…

On Sun, Dec 8, 2024, 21:21 Wiley Young ***@***.***> wrote: In the code you provided, errexit was being ignored in the test following the 'if' command, so errrexit's "exit immediately" functionality was disabled when `func` was called. If a second call to `func` is appended to the end of the script, that call will occur in a context where `set -e`s "exit immediately" functionality is active, and "false did not fail" will fail to print. Wiley On Tue, Dec 3, 2024, 08:55 John ***@***.***> wrote: > I would like shellcheck to print a warning when the return value of a > function is checked when errexit is enabled. > > For details on this problem, see: > > - https://lists.gnu.org/archive/html/bug-bash/2012-12/msg00094.html > - > https://stackoverflow.com/questions/19789102/why-is-bash-errexit-not-behaving-as-expected-in-function-calls/19789651#19789651 > > 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: > > #!/usr/bin/env bashset -eu -o pipefail > main() { > if func ; then > echo func passed > else > echo func failed > fi > } > func() { > false > echo false did not fail > } > > main "$@" > > Showing the problem: > > aMBP:~> /tmp/try > false did not fail > func passed > aMBP:~> > > Here's what shellcheck currently says: > > It says nothing. > > aMBP:~> shellcheck /tmp/try > aMBP:~> > > Here's what I wanted or expected to see: > > WARNING: checking the return value of a function disables errexit inside the function > > — > Reply to this email directly, view it on GitHub > <#3095>, or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AUF2F22QPTMOLA4EA4ACWGT2DXO77AVCNFSM6AAAAABS6HTDVOVHI2DSMVQWIX3LMV43ASLTON2WKOZSG4YTKNJTGY2DEMA> > . > You are receiving this because you are subscribed to this thread.Message > ID: ***@***.***> >

[johnatswoopsrch]

@wileyhy Yes, that would be the case. The links I provided explain the unexpected behavior.

[wileyhy]

The warning wanted or expected to see is, "checking the return value of a function disables errexit inside the function." Okay, I think I see what you're saying. ~ $ vim fn-erx.sh ~ $ chmod 0700 fn-erx.sh ~ $ ./fn-erx.sh + fn + false ~ $ cat ./fn-erx.sh #!/bin/bash set -xe fn() { false echo foo } fn echo $? # "checking the return value of a function" exit 00 In this syntactical form of checking a return value, above, errexit remains in effect within the function body. The question is how the return value is "checked." In the example you provided, the "test list" of an if-fi construct looked at whether or not said list had an exit status of zero. From the man page: "If a compound command or shell function executes in a context where -e is being ignored, none of the commands executed within the compound command or function body will be affected by the -e setting, even if -e is set and a command returns a failure status." "The shell does not exit if the command that fails is part of the command list immediately following a while or until keyword, part of the test following the if or elif reserved words, part of any command executed in a && or || list except the command following the final && or ||, any command in a pipeline but the last, or if the command's return value is being inverted with !." As it is, whether or not the test list of an if-fi ends with a function, as opposed to any other kind of command -like entity, isn't the reason why errexit was ignored. Errexit was ignored because the function occurred between an if and a then. So that warning message you've suggested is somewhat inaccurate. Errexit's "ignore list" are mostly structures that test or effect exit codes. How could a script ask of an OS any boolean questions if all non-zero responses caused a script to halt? If shellcheck did emit a warning whenever a compound command occurred between an if and a then, what could a script writer do to mitigate the risk to which the warning would refer other than to stop using if-fi constructs? Wiley

…

On Tue, Dec 10, 2024, 15:13 John ***@***.***> wrote: @wileyhy <https://github.com/wileyhy> Yes, that would be the case. The links I provided explain the unexpected behavior. — Reply to this email directly, view it on GitHub <#3095 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F2YY4N33WEVEECAN3U32E5YQTAVCNFSM6AAAAABS6HTDVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZTGE4DSMBSGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[johnatswoopsrch]

I would not expand this to compound commands in general, although maybe there are patterns that could be caught. That seems like a bigger and more complicated issue.

I'm not sure what accurate warning text would be. That might depend on exactly what you're able to detect.

I know functions are a problem in these cases, although I don't know if you're able to detect it or not. Not having errexit active in functions within a script that has it active is not intuitive, and a source of errors. I've wasted considerable time chasing a few of these problems, before getting it burned into my brain to watch for it. I'd like to save others that pain if possible.

It seems remediation is not very straight forward. My research and testing seems to indicate there are multiple alternatives to getting information out of functions without running them in compound commands, but none of them ideal. As well, some very simple cases work OK if you're careful.

1. Only check the return value from functions if you're sure there are no chances of unexpected errors that errexit would catch. Maybe single-command functions could work?
2. Otherwise, do not use return values from functions. If you're checking the return value, you've disabled errexit. If you have a real error, that justifies exiting the script, just use exit. The only case where checking the return value of a function is safe is if you've manually made every line safe from possible errors.
3. If you need to pass any value back from the function, you can
   2a) use a global variable (and of course don't recurse)
   2b) use a unique output file
   2c) save the output to a variable, by setting errexit in the function, and call the function in a subshell, ie output=$(func).

#!/usr/bin/env bash
set -eu -o pipefail

funcgood() {
    set -e
    echo yes
}

funcbad() {
    set -e
    false
    echo yes
}

set -x
value=$(funcgood)
echo value for funcgood is $value

value2=$(funcbad)
echo value for funcbad is $value2

bash-5.2$ /tmp/try2
++ funcgood
++ set -e
++ echo yes
+ value=yes
+ echo value for funcgood is yes
value for funcgood is yes
++ funcbad
++ false
++ echo yes
+ value=yes
+ echo value for funcbad is yes
value for funcbad is yes
bash-5.2$ /tmp/try2
++ funcgood
++ set -e
++ echo yes
+ value=yes
+ echo value for funcgood is yes
value for funcgood is yes
++ funcbad
++ set -e
++ false
+ value2=
bash-5.2$

If you are able to detect the issue, then what additional information to reference is tricky too, as there is no simple answer it seems.