SC2250: New optional check for braces around variable references

Always using braces makes it harder to accidentally change a variable by
pasting other text next to it, but the warning is off by default because
it's definitely a style preference.  Omit special and positional
variables from the check because appending additional characters to them
already doesn't change parsing.

CHANGELOG.md

@@ -8,12 +8,13 @@
 - Source paths: Use `-P dir1:dir2` or a `source-path=dir1` directive
                 to specify search paths for sourced files.
 - json1 format like --format=json but treats tabs as single characters
-- SC2249: Warn about `case` with missing default case (verbose)
+- SC2250: Warn about variable references without braces (optional)
-- SC2248: Warn about unquoted variables without special chars (verbose)
+- SC2249: Warn about `case` with missing default case (optional)
+- SC2248: Warn about unquoted variables without special chars (optional)
 - SC2247: Warn about $"(cmd)" and $"{var}"
 - SC2246: Warn if a shebang's interpreter ends with /
 - SC2245: Warn that Ksh ignores all but the first glob result in `[`
-- SC2243/SC2244: Suggest using explicit -n for `[ $foo ]` (verbose)
+- SC2243/SC2244: Suggest using explicit -n for `[ $foo ]` (optional)
 
 ### Changed
 - If a directive or shebang is not specified, a `.bash/.bats/.dash/.ksh`

src/ShellCheck/Analytics.hs

@@ -223,6 +223,13 @@ optionalTreeChecks = [
         cdPositive = "case $? in 0) echo 'Success';; esac",
         cdNegative = "case $? in 0) echo 'Success';; *) echo 'Fail' ;; esac"
     }, nodeChecksToTreeCheck [checkDefaultCase])
+
+    ,(newCheckDescription {
+        cdName = "require-braces",
+        cdDescription = "Suggest putting braces around all variable references",
+        cdPositive = "var=hello; echo $var",
+        cdNegative = "var=hello; echo ${var}"
+    }, nodeChecksToTreeCheck [checkVariableBraces])
     ]
 
 optionalCheckMap :: Map.Map String (Parameters -> Token -> [TokenComment])
@@ -1843,6 +1850,24 @@ checkSpacefulness' onFind params t =
         globspace = "*?[] \t\n"
         containsAny s = any (`elem` s)
 
+prop_CheckVariableBraces1 = verify checkVariableBraces "a='123'; echo $a"
+prop_CheckVariableBraces2 = verifyNot checkVariableBraces "a='123'; echo ${a}"
+prop_CheckVariableBraces3 = verifyNot checkVariableBraces "#shellcheck disable=SC2016\necho '$a'"
+prop_CheckVariableBraces4 = verifyNot checkVariableBraces "echo $* $1"
+checkVariableBraces params t =
+    case t of
+        T_DollarBraced id False _ ->
+            unless (name `elem` unbracedVariables) $
+                styleWithFix id 2250
+                    "Prefer putting braces around variable references even when not strictly required."
+                    (fixFor t)
+
+        _ -> return ()
+  where
+    name = getBracedReference $ bracedString t
+    fixFor token = fixWith [replaceStart (getId token) params 1 "${"
+                           ,replaceEnd (getId token) params 0 "}"]
+
 prop_checkQuotesInLiterals1 = verifyTree checkQuotesInLiterals "param='--foo=\"bar\"'; app $param"
 prop_checkQuotesInLiterals1a= verifyTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; app $param"
 prop_checkQuotesInLiterals2 = verifyNotTree checkQuotesInLiterals "param='--foo=\"bar\"'; app \"$param\""

src/ShellCheck/Data.hs

@@ -49,6 +49,10 @@ variablesWithoutSpaces = specialVariablesWithoutSpaces ++ [
 
 specialVariables = specialVariablesWithoutSpaces ++ ["@", "*"]
 
+unbracedVariables = specialVariables ++ [
+    "0", "1", "2", "3", "4", "5", "6", "7", "8", "9"
+  ]
+
 arrayVariables = [
     "BASH_ALIASES", "BASH_ARGC", "BASH_ARGV", "BASH_CMDS", "BASH_LINENO",
     "BASH_REMATCH", "BASH_SOURCE", "BASH_VERSINFO", "COMP_WORDS", "COPROC",