koalaman / shellcheck

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

SC2003 should not warn about `expr index` usage #1402

Open vapier opened 5 years ago

vapier commented 5 years ago

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

#!/bin/sh
expr index 'abc' b

Here's what shellcheck currently says:

Line 2:
expr index 'abc' b
^-- SC2003: expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]].

Here's what I wanted or expected to see:

It shouldn't warn at all. There is no builtin shell equivalent for expr index.

vapier commented 5 years ago

hrm, handling of length, substr, index, or match is a bit dubious. POSIX marks those as undefined. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/expr.html

But it's not a shell builtin, so we can't bind to the dialect (POSIX/bash/etc...) as a signal whether they would be available in the script environment.

matthewpersico commented 5 years ago

@vapier, I think the point is that expr is not antiquated in Bourne shell, that $((..)), ${}, [[ ]] are not available in Bourne shell to do arithmetic stuff. And even if #!/bin/sh happens to be a link to bash at the time when shellcheck is run, you cannot guarantee that going forward; think running shellcheck in dev and then deploying somewhere else.

vapier commented 5 years ago

i think you missed the point i'm highlighting. i'm not talking about expr's arithmetic subcommands here (which the builtin $((...)) arithmetic syntax handles fine), i'm talking about the string subcommands. there is no equiv in the shell.

koalaman commented 5 years ago

SC2003 already has a few exceptions for things that are hard to reproduce, like :, and index could definitely be added to that list.

POSIX says that "The use of string arguments length, substr, index, or match produces unspecified results.", so are there any workarounds that also work while also being completely POSIX?

Maybe var=abc; prefix=${var%%b*}; echo "${#prefix}" but you'd have to handle the case of no matching substring.

vapier commented 5 years ago

expr length <str> can be replaced with var="<str>"; echo ${#var} or echo "<str>" | wc -c.

expr substr <string> <pos> <length> could be replaced with two cuts: echo "<string>" | cut -b<pos>- | cut -b-<length>.

expr match <string> <regex> (and expr <string> : <regex>) could be replaced with echo "<string>" | grep -q "<regex>".

i don't have an answer for expr index. but we should be able to improve SC2003 for the other subcommands in the meantime.

chuhn commented 3 years ago

It's not only about return codes.

expr match … returns matched subexpressions:

$ expr match 'ASDF1234qwer' '[[:upper:]]\+\([[:digit:]]\+\)[[:lower:]]\+'
1234

The same results may also be achieved with bash built-ins (or echo '…' | sed -e 's/…/\1/') but those approaches are IMHO not more favourable than expr in general.

patsevanton commented 1 year ago

How correct write bash code?

echo Execution time was $(expr "$end_time" - "$start_time")  s.
brother commented 1 year ago

How correct write bash code?

echo Execution time was $(expr "$end_time" - "$start_time")  s.

either using printf or echo and with bash as destination shell (the thread is about portability though).

echo "Execution time was $(( end_time - start_time )) s."
printf "Execution time was %d s." $(( end_time - start_time ))