koalaman / shellcheck

ShellCheck, a static analysis tool for shell scripts
https://www.shellcheck.net
GNU General Public License v3.0
36.12k stars 1.77k forks source link

SC2317: false-positive with EXIT trap in subshell? #2660

Open pschyska opened 1 year ago

pschyska commented 1 year ago

For bugs

For new checks and feature suggestions

Here's a snippet or screenshot that shows the problem:

#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

(
    tmp=$(mktemp -d)
    cleanup() {
        if [[ -d $tmp ]]; then
            rm -rf "$tmp"
        fi
        printf "removed tmp: %s\n" "$tmp"
    }
    trap cleanup EXIT

    printf "tmp is %s\n" "$tmp"
)
printf "After subshell\n"

Here's what shellcheck currently says:

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

Here's what I wanted or expected to see:

This looks like a false positive, because running that script gives the expected outcome:

tmp is /tmp/tmp.coTHszefaD
removed tmp: /tmp/tmp.coTHszefaD
After subshell

I don't think that case is covered in the other bug reports.

The following, in this case equivalent, script doesn't trigger the warning:

#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

tmp=$(mktemp -d)
cleanup() {
    if [[ -d $tmp ]]; then
        rm -rf "$tmp"
    fi
    printf "removed tmp: %s\n" "$tmp"
}
(
    trap cleanup EXIT
    printf "tmp is %s\n" "$tmp"
)
printf "After subshell\n"
rmartine-ias commented 1 year ago

Also encountering this. Minimal example:

#!/bin/bash

function sayHi() {
    echo hi
}
trap 'sayHi' EXIT
exit 0
$ shellcheck test.sh

In test.sh line 4:
    echo hi
    ^-----^ SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).

For more information:
  https://www.shellcheck.net/wiki/SC2317 -- Command appears to be unreachable...

Since most of the work for my script is done in a trap, this causes false positives for almost half of the lines in the file.

$ shellcheck --version
ShellCheck - shell script analysis tool
version: 0.9.0
license: GNU General Public License, version 3
website: https://www.shellcheck.net

This is reproducible on shellcheck.net.

robert914 commented 1 year ago

Also encountering the same issue. Any functions I define that are called from any signal trap specification (not just EXIT) are getting many if not all lines within flagged as unreachable. I don't feel like it's the right approach to add in-line waivers on these functions for that check as it will then not check cases that are actually unreachable within those functions.

oliv3r commented 1 year ago

Can we rename this subject to include 'trap'? As there's several issues with SC2317, the trap call being one of them (as mentioned earlier).

#!/bin/sh

cleanup()
{
        trap - EXIT
}

trap cleanup EXIT

exit 0

yields

$ shellcheck myscript

        trap - EXIT
        ^-- [SC2317](https://www.shellcheck.net/wiki/SC2317) (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).

        echo "Need to clean this up"
        ^-- [SC2317](https://www.shellcheck.net/wiki/SC2317) (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).

$
e-kwsm commented 2 months ago

dup of #2542?