shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

[SC2120] False positive when a function is called by `trap` with arguments

Open Frederick888 opened this issue 1 year ago • 2 comments

For bugs

  • Rule Id (if any, e.g. SC1000): SC2120
  • My shellcheck version (shellcheck --version or "online"): online
  • [x] The rule's wiki page does not already cover this (e.g. https://shellcheck.net/wiki/SC2086)
  • [x] I tried on https://www.shellcheck.net/ and verified that this is still a problem on the latest commit

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

clean_up() {
        local code=$?
        printf 'Cleaning up!\n'
        if [[ -n "$1" ]]; then
                printf 'Invoked due to error %d on line %d\n' "$code" "$1"
                exit "$code"
        fi
        exit 0
}

trap 'clean_up $LINENO' ERR SIGINT SIGTERM SIGQUIT

if [[ -z "$INPUT" ]]; then
        printf 'Missing input!\n'
        clean_up
fi

# do something

Here's what shellcheck currently says:

Line 3:
clean_up() {
^-- SC2120 (warning): clean_up references arguments, but none are ever passed.
 
Line 17:
        clean_up
        ^-- SC2119 (info): Use clean_up "$@" if function's $1 should mean script's $1.

Here's what I wanted or expected to see:

No SC2120 error as trap 'clean_up $LINENO' ERR SIGINT SIGTERM SIGQUIT can call clean_up with an argument.

Frederick888 avatar Sep 03 '24 10:09 Frederick888

What you seem to be trying to do is trigger the trap from within the if-fi structure if the length of INPUT is zero (which could include a nul character). To do so, you'd have to put a command there, rather than clean_up after printf, which can send the script process a signal which you've already defined as caught using the trap builtin.

For instance, kill -s INT would trigger the trap, or a similar command using /bin/kill. As an aside, both the exit builtin and (assuming "Halt" is undefined) echo ${Halt:?} will send SIGEXIT to the script's process.

greycat's page on traps can be helpful.

https://mywiki.wooledge.org/SignalTrap

Wiley

On Tue, Sep 3, 2024, 3:30 AM Frederick Zhang @.***> wrote:

For bugs

  • Rule Id (if any, e.g. SC1000): SC2119, SC2120
  • My shellcheck version (shellcheck --version or "online"): online
  • The rule's wiki page does not already cover this (e.g. https://shellcheck.net/wiki/SC2086)
  • I tried on https://www.shellcheck.net/ and verified that this is still a problem on the latest commit

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 clean_up() { local code=$? printf 'Cleaning up!\n' if [[ -n "$1" ]]; then printf 'Invoked due to error %d on line %d\n' "$code" "$1" exit "$code" fi exit 0 } trap 'clean_up $LINENO' ERR SIGINT SIGTERM SIGQUIT if [[ -z "$INPUT" ]]; then printf 'Missing input!\n' clean_upfi

do something

Here's what shellcheck currently says:

Line 3: clean_up() { ^-- SC2120 (warning): clean_up references arguments, but none are ever passed.

Line 17: clean_up ^-- SC2119 (info): Use clean_up "$@" if function's $1 should mean script's $1.

Here's what I wanted or expected to see:

No errors as trap 'clean_up $LINENO' ERR SIGINT SIGTERM SIGQUIT can call clean_up with an argument.

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

wileyhy avatar Sep 13 '24 12:09 wileyhy

@wileyhy No, in my example clean_up is something that can be called directly when an expected/handled issue arises to clean up and exit, or called when an unexpected error / interruption happens (during # do something). I did not want to trigger the trap from within if-fi.

Anyway, I fail to see how that's related to this issue report. clean_up can be called with an argument nevertheless.

Frederick888 avatar Sep 13 '24 13:09 Frederick888