koalaman / shellcheck

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

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

Open nom3ad opened 2 years ago

nom3ad commented 2 years ago

For new checks and feature suggestions

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.


kurahaupo commented 2 years ago

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:

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

kurahaupo commented 2 years ago

Duplicate of #2561