shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

Shellcheck says that a function "references arguments" when it only checks if any argument has been passed

Open Ganton opened this issue 2 years ago • 4 comments

For bugs

  • Rule Id (if any, e.g. SC1000): SC2119 and SC2120
  • My shellcheck version (shellcheck --version or "online"): 0.8.0 and 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

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

#!/bin/bash

save() {
    if [[ $# != 0 ]]; then
        echo "ERROR: A wrong quantity of arguments was used." >&2
    fi
    
    echo "Actions would be done there."
}

save || exit 1

Here's what shellcheck currently says:

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

Line 11: save || exit 1 ^-- SC2119 (info): Use save "$@" if function's $1 should mean script's $1.

Here's what I wanted or expected to see:

Shellcheck shouldn't say anything because no arguments have to be passed, it's useful to check if any argument is passed.


Thanks for Shellcheck, Vidar et al!

Ganton avatar May 03 '22 07:05 Ganton

Just coming in to say that I'm encountering this as well.

  • Rule ID: SC2120 and SC2119
  • online

Example code:

#!/usr/bin/bash
fn() {
        [[ "$#" -lt 1 ]] || return 1
}

fn2() {
        [[ "$#" -eq 0 ]] || return 1
}

fn3() {
        [[ "$#" -ne 0 ]] && return 1
}

fn4() {
        if [[ "$#" -gt 0 ]]; then
                return 1
        fi
        return 0
}

fn
fn2
fn3
fn4

same sort of results as @Ganton , but I thought I'd just test a couple of different variations "just in case".

Only way around it is to include # shellcheck disable=SC2120 just before the function definition (and not where the if statement is as I expected)

rbairwell avatar Aug 05 '22 11:08 rbairwell

There is some logic in excluding $#, but empirically it appears to be pretty rare to check it without then using $@ or $1 based on the result. Is this part of a larger example?

koalaman avatar Aug 12 '22 04:08 koalaman

There is some logic in excluding $#, but empirically it appears to be pretty rare to check it without then using $@ or $1 based on the result. Is this part of a larger example?

If I have understood you correctly, yes, it is part of a bigger code, with several functions. In some of those functions: some arguments are used, in other functions: no argument is used and (as a kind of safety measure) it's checked if ─however─ an argument is passed. In those functions: $# is used (as seen in the examples), but neither $@ nor $1 are used.

Ganton avatar Aug 12 '22 19:08 Ganton

There is some logic in excluding $#, but empirically it appears to be pretty rare to check it without then using $@ or $1 based on the result. Is this part of a larger example?

Yes it is in my case as well. Basically, I've got functions which check that they have NOT been passed any arguments (for confirmation purposes) and will throw errors etc if $# is not zero/greater than 1 (i.e. I don't trust people using that bit of code to read documentation and/or I expect it to be mis-called) - seems to be identical to @Ganton's use case as well.

rbairwell avatar Aug 15 '22 09:08 rbairwell

Here's my usecase:

#!/usr/bin/env sh
update_opkg_package_list(){
    if test "${#}" -ne 0; then
        printf_fatal \
            'update_opkg_package_list: FATAL: This function requires no arguments, %s is given.\n' \
            "${#}"
        exit 99
    fi

    if ! test -e /var/opkg-lists/openwrt_base.sig; then
        printf_info -- \
            'Updating Opkg software list...\n'
        if ! opkg update; then
            return 1
        fi
    fi
}

if ! update_opkg_package_list; then
    printf_error \
        'Error: Unable to update Opkg software list.\n'
    exit 2
fi

The part that triggers the false positive is merely a pedantic check for function consumers that called the function wrongly without checking the documentation, positional arguments are not referenced at all.

brlin-tw avatar Mar 09 '23 09:03 brlin-tw