Shellcheck: SC2230 - command -v is not a direct replacement for which

[arth1]

For bugs

• Rule Id (if any, e.g. SC1000): SC2230
• My shellcheck version (shellcheck --version or "online"): 4.4.7 (latest from git)
• [X] I tried on 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/sh

which "$0"
/usr/bin/which "$0"
command -v "$0"

Here's what shellcheck currently says:

In test.sh line 3:
which "$0"
^-- SC2230: which is non-standard. Use builtin 'command -v' instead.

In test.sh line 4:
/usr/bin/which "$0"
^-- SC2230: which is non-standard. Use builtin 'command -v' instead.

Here's what I wanted or expected to see:

Because "command -v" does not do the same as which, I would have liked to see nothing.
When running the above (from /tmp), I get:
/tmp/test.sh
/tmp/test.sh
./test.sh

While "which" may not be standard, it is commonly used to get the full path.
At the very least I would like to not get warned when I specifically use the path as in the third command - then it's fairly certain what the user wants, which won't be the common bash alias.

[Dmole]

You could use 'pwd'.

'which' is not posix
http://pubs.opengroup.org/onlinepubs/9699919799/idx/utilities.html
and not gnucore
https://en.m.wikipedia.org/wiki/List_of_GNU_Core_Utilities_commands

but it sure seems common to me.. where is it not?
also shellcheck is not warning about far less common things like Java... seems like a false positive to me.

[brodygov]

command -v also has slightly different behavior since it will return success for shell built-ins and functions that do not exist as standalone commands, whereas which only searches the PATH.

For example:

$ which if
# exits nonzero

$ command -v if
if
# exits zero

$ foo() { true ; }
$ which foo
# exits nonzero
$ command -v foo
foo
# exits zero

[pedroarthur]

Besides that, which a b c exits zero if __all__ of a, b, and c exist. Meanwhile, command -v a b c exits zero if __any__ of a, b, or c exists.

[pdietl]

So do we think that this check should be removed?

[brodygov]

:+1: I would favor removal.

[Light-Wizzard]

which can be replaced with type -p,
but it still needs the -x, I do not get any warnings using this:
[[ -x "$(type -p dir 2>/dev/null)" ]]

[leagris]

An alternative to which command, when you need to know the path of a non built-in command:

type -P ls
/bin/ls

[oliv3r]

While which can certainly be used if a user decides to (they can easily ignore the shellcheck warning, the argument 'but it is so common and always available' is of course moot and opinionated (yes, even busybox features it these days).

The point remains, if you use /bin/sh you kind of imply POSIX compliance, and even if not, some of us would still like this warning to be triggered. Just as which not being standard (again, even with being so ever common).

As with everything, there are exceptions of course. But I think this is not one of them, the exception should be in your code or your parser, where you globally or locally ignore this test as that would work for you. (You being not the individual personal you).

A fair improvement of course would be to the wikipage, for users who run into this, get noted of this.

While surely there are a few valid usecases, I was thinking a bit more about this, and from a users point of view, I can imagine the difference "will this command work in my script (command -v) vs what is the path of this utility (which)". Sometimes a user may be interested only where the binary is, rather then whether it will work. My earlier thoughts still remain the same, but the wiki surly needs clarification of this exceptional case.

~Not sure of the complexity, but shellcheck could of course try to figure out the intend. Is the result stored in a variable and that variable only passed along/used as a string, or invoked as a program, it should be a valid use case? Detecting the other uses however is a bit harder I suspect however.~ but then also relate this to the 'is which being used as a standard tool' or not.

I suppose a decent workaround would be:
WHICH="$(command -v which)"
which would fail if which is not available :) and users of which can use WHICH instead :)

[pedroarthur]

This is a example where which and command diverge. Using which, I check if all the tools I need exist with a single statement. If I use, command -V, it will return 0 if any of the tools exist.

#!/bin/bash
tooling=( bash no_such_command )

# shellcheck disable=2230
which      "${tooling[@]}" &> /dev/null || echo some tools are missing
command -v "${tooling[@]}" &> /dev/null || echo some tools are missing

Hence, command -v is not a replacement for which.

[oliv3r]

is this even supposed to work?
I use it as such:

check_requirements()
{
    for cmd in ${CMDS}; do
        if ! test_result="$(command -V "${cmd}")"; then
            test_result_fail="${test_result_fail:-}${test_result}\n"
        else
            test_result_pass="${test_result_pass:-}${test_result}\n"
        fi
    done

    echo "Passed tests:"
    # As the results contain \n, we expect these to be interpreted.
    # shellcheck disable=SC2059
    printf "${test_result_pass:-none\n}"
    echo
    echo "Failed tests:"
    # shellcheck disable=SC2059
    printf "${test_result_fail:-none\n}"
    echo

    if [ -n "${test_result_fail:-}" ]; then
        echo "Self-test failed, missing dependencies."
        exit 1
    fi
}

which yes, is a lot longer, but also is a _lot_ friendlier to the user imo :)

so yeah, which can do this in one line, but just because you can, doesn't mean you should imo :)

[pedroarthur]

is this even supposed to work?

Yes, which is supposed to work this way since day 0. Have you read its man page?

so yeah, which can do this in one line, but just because you can, doesn't mean you should imo :)

So, you advocate to maintain 15+ LoC that triggers far more concerning checks instead of a single one that just has a "non-standard tool" warning? Thank you, but no.

However, the main issue here is saying that things are interchangeable when they aren't. Following the advice in this check can break perfectly working code.

[oliv3r]

is this even supposed to work?

Yes, which is supposed to work this way since day 0. Have you read its man page?
Nope :)

so yeah, which can do this in one line, but just because you can, doesn't mean you should imo :)

So, you advocate to maintain 15+ LoC that triggers far more concerning checks instead of a single one that just has a "non-standard tool" warning? Thank you, but no.
more concerning checks? I personally prefer to have an output like:

Fair point, which does the same in one go, which is indeed (no pun intended) admittedly quite nice. So at the least, the wording could be improved absolutely. But the fact remains, and the warning, that it is a non-standard tool. So a green warning should be sufficient.

However, the main issue here is saying that things are interchangeable when they aren't. Following the advice in this check can break perfectly working code.

nod

[HalisCz]

I've found out another difference between which and command -v:

$ alias ls
alias ls='ls -al'
$ which ls
/usr/bin/ls
$ command -v ls
alias ls='ls -al'
$ type -P ls
/usr/bin/ls

Therefore I think the suggestion should be _replace which with type -P_.

[ghost]

The problem is that type -P doesn't work in some shells that are used as /bin/sh, such as dash. info

For a which (function) that works on bash(/sh), dash and zsh, by using type(optionally with -P if supported) see this gist

PR that added the command -v suggestion: https://github.com/koalaman/shellcheck/pull/1123
(linking here so that PR will have a link back to this issue)

---

one more reason (for me) to not use command -v:

$ sudo touch /usr/bin/mytestfile
[sudo] password for user: 

$ command -v mytestfile
/usr/bin/mytestfile

$ which mytestfile
which: no mytestfile in (/usr/local/sbin:/usr/local/bin:/usr/bin:....blabla)

$ l /usr/bin/mytestfile
-rw-r--r-- 1 root root 0 04.08.2019 22:09 /usr/bin/mytestfile

$ /usr/bin/mytestfile
-bash: /usr/bin/mytestfile: Permission denied

$ command /usr/bin/mytestfile
-bash: /usr/bin/mytestfile: Permission denied

$ bash --version
GNU bash, version 5.0.7(1)-maint (x86_64-pc-linux-gnu)
blah

source: https://unix.stackexchange.com/questions/85249/why-not-use-which-what-to-use-then/85250#comment666687_85250

[leagris]

Feel free to use, integrate in your projects, adapt, expand my Bash Implementation of the which command.
It behaves as the native implementation from the debianutils package.

This is the minified version which-min.sh, that is 455 bytes versus 946 bytes for the native version:

#!/usr/bin/env bash
IFS=:;if (($#>0))&&[ "${1:0:1}" == - ];then if [ "$1" != -a ];then printf >&2 'Illegal option %s\nUsage: which [-a] args\n' "$1";exit 2;else b=1;fi;shift;fi;read -rd '' -a g < <(printf %s:\\0 "$PATH");while (($# > 0));do f=0;for d in "${g[@]}";do e="$d/$1";while [[ -L $e && "$(ls -l "$e")" =~ -\>\ (.*) ]];do e="${BASH_REMATCH[1]}";done;if [[ -f $e && -x $e ]];then f=1;echo "$d/$1";((b))||break;fi;done;((f))||c=1;shift;done;exit $c

The human readable non-minified version is here: which.sh.

[abathur]

I just lost about 40 minutes debugging this one, assuming I'd introduced the error alongside quoting fixes around some alias commands that I made at the same time.

I'm still vaguely glad the issue came up--I realized I could at least swap which for type -P and shave a little execution time off--but it would be nice if the check language directly acknowledged that these aren't fungible replacements.

The current language is quite confident, and I naively took it at face value.

[tripleee]

The fundamental problem with which is that it's not reliable. There is a plethora of incompatible implementations, none of which are well-defined or portable. The recommendation should certainly be tweaked to not imply that command -v is always what you want to replace which with; in so many words, we could say "try command -v or type (or perhaps type -p if you are on Bash)".

Here's an excellent background on the topic by the always virtuose @stephane-chazelas:
https://unix.stackexchange.com/a/85250/19240

[Dmole]

Shellcheck requires the shell to be specified with #! so non-standard shells can be treated independently of sh/bash.

[AdamDanischewski]

Shellcheck's web page, states:

ShellCheck
finds bugs in your shell scripts.

It says that it finds _bugs_ it doesn't say anything about portability.

If finding bugs is the goal, then shellcheck should keep quiet about people on Bash using which, type -P, etc - since it is not an error.

Shell check probably needs to beef up its logic to handle differences between major shells - for instance type -P exists reliably in Bash but not in Dash.

Also, I didn't see anyway to specify the shell version - I suppose we should presume a recent version but it would be nice if shellcheck displayed what version of a shell it was running against.

[beltaurus]

I use shellcheck mainly in my Visual Studio Coder extension, where shellcheck issues are neatly listed in the "Problems" console.
So I agree that maybe "Problems" or "Issues" would be a semantically correct term, but IMHO it undervalues the type of feedback we get from shellcheck...

However:

1. Make the website description more accurate (should be easy).
2. It might be nice to have:
   "Errors" (bugs)
   "Warnings" (portability issues)
   "Info" (coding style).

[Dmole]

1. It might be nice to have:

In vim shellcheck lists Errors, and Warnings.
info things use the warning tag but start with "Note that".
examples;

error| Don't use $ on the left side of assignments. [SC1066]
warning| X appears unused. Verify it or export it. [SC2034]
warning| Note that, unescaped, this expands on the client side. [SC2029]

[beltaurus]

Indeed! I completely blanked here...
They are even color-coded in Visual Studio Coder:

That's what I get for commenting before my morning coffee...

[AdamDanischewski]

Maybe performance should also be considered:

[abathur]

Apparently this check defaults to off since https://github.com/koalaman/shellcheck/commit/0f15fa49ba8cc427aaad338140f82707b6865eb5 was released in 0.7.1.

[EricLuehrsen]

2. It might be nice to have:
   "Errors" (bugs)
   "Warnings" (portability issues)
   "Info" (coding style).

I tried suggesting a little more lint-like categories a while ago. It would be nice if the numbering scheme followed something like error (1000), warning (2000), info (3000), portability (4000), and best-practice/stye (5000). Deploying shell check as quality tool could be done as many lint-like tools. Over time it would easier to ratchet QA checks on system scripts and supported tools. Both of these ideas were not accepted.

Also /usr/bin/which and unspecific which should have different enable options. The former enabled for "pedantic POSIX" option, and the later normally enabled for naive system administrators. They could even be the same message number but one is "/usr/bin/which _may not be_ portable ..." and the other "which _is not_ portable ..." This is done for a number of GCC diagnostics.