Optionally suggest [[ over [ in Bash scripts (-o require-double-brackets) (fixes koalaman#887)

koalaman · koalaman · commit fe81dc1c275f · 2021-07-27T18:53:30.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 - SC2286-SC2288: Warn when command name ends in a symbol like `/.)'"`
 - SC2289: Warn when command name contains tabs or linefeeds
 - SC2291: Warn about repeated unquoted spaces between words in echo
+- SC2292: Suggest [[ over [ in Bash/Ksh scripts (optional)
 
 ### Fixed
 - SC2102 about repetitions in ranges no longer triggers on [[ -v arr[xx] ]]
diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
@@ -244,6 +244,13 @@ optionalTreeChecks = [
         cdPositive = "echo $VAR",
         cdNegative = "VAR=hello; echo $VAR"
     }, checkUnassignedReferences' True)
+
+    ,(newCheckDescription {
+        cdName = "require-double-brackets",
+        cdDescription = "Require [[ and warn about [ in Bash/Ksh",
+        cdPositive = "[ -e /etc/issue ]",
+        cdNegative = "[[ -e /etc/issue ]]"
+    }, checkRequireDoubleBracket)
     ]
 
 optionalCheckMap :: Map.Map String (Parameters -> Token -> [TokenComment])
@@ -4311,5 +4318,39 @@ checkCommandWithTrailingSymbol _ t =
             '\"' -> "doublequote"
             x -> '\'' : x : "\'"
 
+
+prop_checkRequireDoubleBracket1 = verifyTree checkRequireDoubleBracket "[ -x foo ]"
+prop_checkRequireDoubleBracket2 = verifyTree checkRequireDoubleBracket "[ foo -o bar ]"
+prop_checkRequireDoubleBracket3 = verifyNotTree checkRequireDoubleBracket "#!/bin/sh\n[ -x foo ]"
+prop_checkRequireDoubleBracket4 = verifyNotTree checkRequireDoubleBracket "[[ -x foo ]]"
+checkRequireDoubleBracket params =
+    if isBashLike params
+    then nodeChecksToTreeCheck [check] params
+    else const []
+  where
+    check _ t = case t of
+        T_Condition id SingleBracket _ ->
+            styleWithFix id 2292 "Prefer [[ ]] over [ ] for tests in Bash/Ksh." (fixFor t)
+        _ -> return ()
+
+    fixFor t = fixWith $
+        if isSimple t
+        then
+            [
+                replaceStart (getId t) params 0 "[",
+                replaceEnd (getId t) params 0 "]"
+            ]
+        else []
+
+    -- We don't tag operators like < and -o well enough to replace them,
+    -- so just handle the simple cases.
+    isSimple t = case t of
+        T_Condition _ _ s -> isSimple s
+        TC_Binary _ _ op _ _ -> not $ any (\x -> x `elem` op) "<>"
+        TC_Unary {} -> True
+        TC_Nullary {} -> True
+        _ -> False
+
+
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])
diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs
@@ -178,7 +178,8 @@ makeCommentWithFix :: Severity -> Id -> Code -> String -> Fix -> TokenComment
 makeCommentWithFix severity id code str fix =
     let comment = makeComment severity id code str
         withFix = comment {
-            tcFix = Just fix
+            -- If fix is empty, pretend it wasn't there.
+            tcFix = if null (fixReplacements fix) then Nothing else Just fix
         }
     in force withFix
 

Original file line number	Diff line number	Diff line change
`@@ -178,7 +178,8 @@ makeCommentWithFix :: Severity -> Id -> Code -> String -> Fix -> TokenComment`
`178`	`178`	`makeCommentWithFix severity id code str fix =`
`179`	`179`	`let comment = makeComment severity id code str`
`180`	`180`	`withFix = comment {`
`181`		`- tcFix = Just fix`
	`181`	`+ -- If fix is empty, pretend it wasn't there.`
	`182`	`+ tcFix = if null (fixReplacements fix) then Nothing else Just fix`
`182`	`183`	`}`
`183`	`184`	`in force withFix`
`184`	`185`