shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

[Feature request] Catch ambiguity of using single quote in default value substitution

Open nom3ad opened this issue 1 year ago • 2 comments

For new checks and feature suggestions

  • [x] https://www.shellcheck.net/ (i.e. the latest commit) currently gives no useful warnings about this
  • [x] 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:

Let's say user want to assign variable A to 100 if it is not set already. Bash default value substitution methods using :- and := currently have an ambiguity when quotes are used in the expression.

Assume that for each of the following statements, A is unset.

#!/bin/bash

# passing cases -  A will be set as 100
A=${A:-100}
echo ${A:=100}   # already throws (SC2086) - "Double quote to prevent globbing and word splitting.""
A=${A:-"100"}
echo ${A:="100"}

A="${A:-100}"
echo "${A:=100}"
A="${A:-"100"}"
echo "${A:="100"}"

# Note: this is also a passing case, hence the  cause of ambiguity.
A=${A:-'100'}
echo ${A:='100'}

## bad cases  -  A will be set as '100' (with single quotes in the value)
A="${A:-'100'}"
echo "${A:='100'}"

Here's what shellcheck currently says:

Nothing

Here's what I wanted or expected to see:

shellcheck should warn about usages of single quote in the default value substitutions similar to above-mentioned bad cases.


nom3ad avatar Aug 29 '22 10:08 nom3ad

There's a surprising difference in the behaviour of quotes between ${var:-default} and ${var/pattern/replacement}.

The ${var:-default} case is defined by POSIX such the quoting state propagates inwards, so that (when the outside is quoted) that inner quotes are considered part of the default value. An exception is made where double quotes are used, because the alternative would be to consider "${var:-"default"}" to mean the concatenation of "${var:-" then default then "}" with the first part being a quoted but invalid expansion. In that case it makes sense to define it in a way that makes it useful.

In the ${var/pattern/replacement} case, the behaviour actually changed in Bash version 4.3/

Previously it treated quotes in the replacement as part of the value; subsequently it treats them as quoting the replacement (even when the whole expansion occurs within quotes).

As always, instituting a warning on quotes around the default will draw criticism from folk who intentionally use it.

If either warning is added then both should be, but with separate warning messages (since "${var:-"default"}" has unnecessary quotes (that will be removed) while "${var:-'default'}" has quotes that will be included in the expansion).

Perhaps they could recommend the unambiguous forms:

  • ^--- SCxxxx: double quotes inside double-quoted expansion will be ignored; use "${var:-\"default\"}" or "${var:-default}" to clarify intent.
  • ^--- SCxxxx: single quotes inside double-quoted expansion do not suppress inner expansions and will be included in the result; "${var:-\'default\'}" or "${var:-default_with_\$dollar_sign}" to clarify intent.

All bets are off when these are inside backticks, as that doubles the number of \ needed.

kurahaupo avatar Sep 03 '22 03:09 kurahaupo

Duplicate of #2561

kurahaupo avatar Sep 03 '22 04:09 kurahaupo