Merge branch 'ngzhian-1416-encourage-n'

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## Since previous release
 ### Added
 - Preliminary support for fix suggestions
+- SC2243/SC2244: Suggest using explicit -n for `[ $foo ]`
 
 ## v0.6.0 - 2018-12-02
 ### Added

src/ShellCheck/ASTLib.hs

@@ -485,8 +485,11 @@ wordsCanBeEqual x y = fromMaybe True $
 -- Is this an expansion that can be quoted,
 -- e.g. $(foo) `foo` $foo (but not {foo,})?
 isQuoteableExpansion t = case t of
+    T_DollarBraced {} -> True
+    _ -> isCommandSubstitution t
+
+isCommandSubstitution t = case t of
     T_DollarExpansion {} -> True
     T_DollarBraceCommandExpansion {} -> True
     T_Backticked {} -> True
-    T_DollarBraced {} -> True
     _ -> False

src/ShellCheck/Analytics.hs

@@ -170,6 +170,7 @@ nodeChecks = [
     ,checkSubshelledTests
     ,checkInvertedStringTest
     ,checkRedirectionToCommand
+    ,checkNullaryExpansionTest
     ]
 
 
@@ -3096,5 +3097,27 @@ checkRedirectionToCommand _ t =
                 warn id 2238 "Redirecting to/from command name instead of file. Did you want pipes/xargs (or quote to ignore)?"
         _ -> return ()
 
+prop_checkNullaryExpansionTest1 = verify checkNullaryExpansionTest "[[ $(a) ]]"
+prop_checkNullaryExpansionTest2 = verify checkNullaryExpansionTest "[[ $a ]]"
+prop_checkNullaryExpansionTest3 = verifyNot checkNullaryExpansionTest "[[ $a=1 ]]"
+prop_checkNullaryExpansionTest4 = verifyNot checkNullaryExpansionTest "[[ -n $(a) ]]"
+prop_checkNullaryExpansionTest5 = verify checkNullaryExpansionTest "[[ \"$a$b\" ]]"
+prop_checkNullaryExpansionTest6 = verify checkNullaryExpansionTest "[[ `x` ]]"
+checkNullaryExpansionTest params t =
+    case t of
+        TC_Nullary _ _ word ->
+            case getWordParts word of
+                [t] | isCommandSubstitution t ->
+                    styleWithFix id 2243 "Prefer explicit -n to check for output (or run command without [/[[ to check for success)." fix
+
+                -- If they're constant, you get SC2157 &co
+                x | all (not . isConstant) x ->
+                    styleWithFix id 2244 "Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer)." fix
+                _ -> return ()
+            where
+                id = getId word
+                fix = fixWith [replaceStart id params 0 "-n "]
+        _ -> return ()
+
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])

src/ShellCheck/AnalyzerLib.hs

@@ -155,11 +155,14 @@ err   id code str = addComment $ makeComment ErrorC id code str
 info  id code str = addComment $ makeComment InfoC id code str
 style id code str = addComment $ makeComment StyleC id code str
 
-warnWithFix id code str fix = addComment $
-    let comment = makeComment WarningC id code str in
-    comment {
-        tcFix = Just fix
-    }
+warnWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m ()
+warnWithFix  = addCommentWithFix WarningC
+styleWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m ()
+styleWithFix = addCommentWithFix StyleC
+
+addCommentWithFix :: MonadWriter [TokenComment] m => Severity -> Id -> Code -> String -> Fix -> m ()
+addCommentWithFix severity id code str fix =
+    addComment $ makeCommentWithFix severity id code str fix
 
 makeCommentWithFix :: Severity -> Id -> Code -> String -> Fix -> TokenComment
 makeCommentWithFix severity id code str fix =