SC2317 false positive warning

[orbea]

For bugs

• Rule Id (if any, e.g. SC1000): SC2317, SC2317
• My shellcheck version (shellcheck --version or 'online'): online
• I tried on shellcheck.net and verified that this is still a problem on the latest commit
• It's not reproducible on shellcheck.net, but I think that's because it's an OS, configuration or encoding issue

For new checks and feature suggestions

• 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/sh

set -e

quit () { err=$?; bar; }

trap 'quit; trap - EXIT; exit $err' EXIT INT

foo

exit 0

Here's what shellcheck currently says:

Line 3	SC2317: Command appears to be unreachable. Check usage (or ignore if invoked indirectly).
Line 3	SC2317: Command appears to be unreachable. Check usage (or ignore if invoked indirectly).

Here's what I wanted or expected to see:

This should not be a warning, the exit 0 at the end of a script is very common and the quit function is indeed reachable due to the trap. In this case the exit 0 may never be reached if the foo command fails for whatever reason and even if the exit 0 is reached the quit function which will have any cleanup is still activated due to the trap.

[dimo414]

It's the exit 0 at the end of the script that appears to be triggering this finding. Removing that line clears the "unreachable" finding. Here's a smaller example:

#!/bin/sh
foo() { bar; }
exit 0  # comment this out to do away with the finding

I disagree with your assertion that it's very common" to end a script with exit 0 - it's basically a no-op, and should rarely be needed, especially when using set -e. So a simple fix would be to remove that line from your script. The wiki mentions this:

Defined functions are assumed to be reachable when the script ends (not exits) since another file may source and invoke them.

That said, I agree this finding is confusing and at a minimum it should point to the exit line and explain how it's coming to this reachability conclusion.

[orbea]

So a simple fix would be to remove that line from your script.

You'll need to remove it from thousands of scripts.In other words that is not a reasonable solution.

[jjakob]

@dimo414 a trap on EXIT will be invoked even with exit 0, so there's no reason to say that function will not be reached.

[dimo414]

I agree that the finding is wrong, I'm just saying exit 0 isn't needed and removing that line appears to make the finding go away.

[orbea]

I'm just saying exit 0 isn't needed and removing that line appears to make the finding go away.

As already explained this is clearly not the solution.

[oliv3r]

I disagree with your assertion that it's very common" to end a script with exit 0 - it's basically a no-op, and should rarely be needed, especially when using set -e.

I diagree with your assertion too :)

The final exit 0 at the end, is your canary. E.g. if your script walks its normal life, and exists normally via that exit, it will prevent any attacks such as:

echo "rm -rf /" >> script.sh

so its simply a best practice to always do it.

Having said that, you are correct that technically it is not needed.

[qneill-sifive]

As others have stated, explicit exit VALUE is a useful programming idiom. My script hit this problem with this stanza:

if has_some_error_condition; then
    print_error_report
    exit 1
else
    print_success_report
    exit 0
fi

Also, it is not always the case that 'exit 0' is technically not needed as there can can be a reason to override $?

    some_command_that_might_fail
    exit 0

So I am heartily agreeing that "removing your exit 0" is not a good solution and that shellcheck should be fixed to handle this case

[foresto]

Aliases confuse it, too.

#!/bin/sh
foo() { echo "message"; }
alias bar=foo
true  # or some other command
RESULT=$?
if [ "$RESULT" -eq 0 ]; then bar; fi
exit "$RESULT"

My actual code is more complex, of course, with conditional aliases for multiple functions. Shellcheck 0.9.0 bombards me with "unreachable" messages for every line of every function. I don't remember this happening when I used 0.7.1 (though it's possible I wasn't using shellcheck on these scripts back then).

Also please note that the exit at the end is not a no-op.

[wileyhy]

I am experiencing this issue as well. I've found `exit 0` to be particularly useful during debugging, as is `exit 00`, `exit 101` and `exit "$LINENO"`. It is valid syntax. Thanks, Wiley

…

On Sun, Jul 9, 2023, 6:51 PM Forest ***@***.***> wrote: Aliases confuse it, too. #!/bin/sh foo() { echo "message"; } alias bar=foo true # or some other command RESULT=$? if [ "$RESULT" -eq 0 ]; then bar; fi exit "$RESULT" My actual code is more complex, of course, with conditional aliases for multiple functions. Shellcheck 0.9.0 bombards me with "unreachable" messages for every line of every function. (I don't remember this happening when I used 0.7.1.) Also please note that the exit at the end is not a no-op. — Reply to this email directly, view it on GitHub <#2542 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F22OWRGCYYRS7GDYBPLXPNN23ANCNFSM54OGXFDQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[ringerc]

It can't seem to cope with traps like

function premature_exit() {
    echo 1>&2 "Unexpected premature exit from entrypoint script at $0:$line"
}

trap 'line=${BASH_LINENO:-};premature_exit' EXIT

# ... do work ...
errcode=$?

# clear trap
trap EXIT

# exit with errcode propagated from inner workload
exit $errcode