shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

SC2006 suggests wrong code if quotes were escaped

Open LiviaMedeiros opened this issue 3 years ago • 0 comments

For bugs

  • Rule Id (if any, e.g. SC1000): SC2006
  • My shellcheck version (shellcheck --version or "online"): 0.8.0
  • [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/sh
BASEDIR="/tmp/sc2006"
FOO="bar baz"
mkdir -p "$BASEDIR/$FOO"
cd "$BASEDIR" || exit
DIRNAME="` (cd \"$FOO\" && pwd) `"     # bad but works
#DIRNAME="$( (cd \"$FOO\" && pwd) )"   # SC: first suggestion
#DIRNAME="$( (cd \""$FOO"\" && pwd) )" # SC: second suggestion
#DIRNAME="$(cd "$FOO" && pwd)"         # good
echo "$DIRNAME"
pwd # make sure that directory wasn't changed for real
#rm -rf "$BASEDIR"

Here's what shellcheck currently says:

First run:

In test.sh line 6:
DIRNAME="` (cd \"$FOO\" && pwd) `" # bad but works
         ^----------------------^ SC2006 (style): Use $(...) notation instead of legacy backticks `...`.

Did you mean:
DIRNAME="$( (cd \"$FOO\" && pwd) )" # bad but works

For more information:
  https://www.shellcheck.net/wiki/SC2006 -- Use $(...) notation instead of le...

-f diff:

-DIRNAME="` (cd \"$FOO\" && pwd) `"
+DIRNAME="$( (cd \"$FOO\" && pwd) )"

Second run:

In test.sh line 7:
DIRNAME="$( (cd \"$FOO\" && pwd) )"   # first suggestion
                  ^--^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
DIRNAME="$( (cd \""$FOO"\" && pwd) )"   # first suggestion

For more information:
  https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...

-f diff:

-DIRNAME="$( (cd \"$FOO\" && pwd) )"
+DIRNAME="$( (cd \""$FOO"\" && pwd) )"

Third run: all "good", exit code 0

Here's what I wanted or expected to see:

-DIRNAME="` (cd \"$FOO\" && pwd) `"
+DIRNAME="$( (cd "$FOO" && pwd) )"

Or even better:

-DIRNAME="` (cd \"$FOO\" && pwd) `"
+DIRNAME="$(cd "$FOO" && pwd)"

Further details

Shell:

$ /bin/sh --version
GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)

Expected output of test snipped (backticks and desired versions)

/tmp/sc2006/bar baz
/tmp/sc2006

Output with suggested versions:

test.sh: line 6: cd: too many arguments

/tmp/sc2006

So, there are two problems:

  1. ShellCheck doesn't explicitly warn or fix redundant escaping between backticks. Suggested code doesn't work.
  2. ShellCheck suggests code that still has autofixable lines. If this is by design, maybe there should be an option to try recursively?

Also maybe I'm missing something, but $( (foo) ) should be reduced to $(foo). This mistake might easily occur due to excessive caution, i.e. "I'm unsure if $() guarantees a subshell, but () definitely does so let's combine them".

LiviaMedeiros avatar Apr 12 '22 19:04 LiviaMedeiros