From 30e94ea7ab20b2677f1559325e7392fd2cbe9071 Mon Sep 17 00:00:00 2001
From: Vidar Holen <spam@vidarholen.net>
Date: Thu, 29 Dec 2016 14:04:49 -0800
Subject: [PATCH] Warn about comparisons and cases that can never match.

---
 ShellCheck/ASTLib.hs    | 62 +++++++++++++++++++++++++++++++++++++++++
 ShellCheck/Analytics.hs | 31 +++++++++++++++++++--
 2 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/ShellCheck/ASTLib.hs b/ShellCheck/ASTLib.hs
index 904fa85..a83018d 100644
--- a/ShellCheck/ASTLib.hs
+++ b/ShellCheck/ASTLib.hs
@@ -309,3 +309,65 @@ getAssociativeArrays t =
         case t of
             T_Assignment _ _ name _ _ -> return name
             otherwise -> Nothing
+
+-- A Pseudoglob is a wildcard pattern used for checking if a match can succeed.
+-- For example, [[ $(cmd).jpg == [a-z] ]] will give the patterns *.jpg and ?, which
+-- can be proven never to match.
+data PseudoGlob = PGAny | PGMany | PGChar Char
+    deriving (Eq, Show)
+
+-- Turn a word into a PG pattern, replacing all unknown/runtime values with
+-- PGMany.
+wordToPseudoGlob :: Token -> Maybe [PseudoGlob]
+wordToPseudoGlob word =
+    simplifyPseudoGlob <$> concat <$> mapM f (getWordParts word)
+  where
+    f x = case x of
+        T_Literal _ s -> return $ map PGChar s
+        T_SingleQuoted _ s -> return $ map PGChar s
+
+        T_DollarBraced {} -> return [PGMany]
+        T_DollarExpansion {} -> return [PGMany]
+        T_Backticked {} -> return [PGMany]
+
+        T_Glob _ "?" -> return [PGAny]
+        T_Glob _ ('[':_)  -> return [PGAny]
+        T_Glob {} -> return [PGMany]
+
+        T_Extglob {} -> return [PGMany]
+
+        _ -> return [PGMany]
+
+-- Reorder a PseudoGlob for more efficient matching, e.g.
+-- f?*?**g -> f??*g
+simplifyPseudoGlob :: [PseudoGlob] -> [PseudoGlob]
+simplifyPseudoGlob = f
+  where
+    f [] = []
+    f (x@(PGChar _) : rest ) = x : f rest
+    f list =
+        let (anys, rest) = span (\x -> x == PGMany || x == PGAny) list in
+            order anys ++ f rest
+
+    order s = let (any, many) = partition (== PGAny) s in
+        any ++ take 1 many
+
+-- Check whether the two patterns can ever overlap.
+pseudoGlobsCanOverlap :: [PseudoGlob] -> [PseudoGlob] -> Bool
+pseudoGlobsCanOverlap = matchable
+  where
+    matchable x@(xf:xs) y@(yf:ys) =
+        case (xf, yf) of
+            (PGMany, _) -> matchable x ys || matchable xs y
+            (_, PGMany) -> matchable x ys || matchable xs y
+            (PGAny, _) -> matchable xs ys
+            (_, PGAny) -> matchable xs ys
+            (_, _) -> xf == yf && matchable xs ys
+
+    matchable [] [] = True
+    matchable (PGMany : rest) [] = matchable rest []
+    matchable (_:_) [] = False
+    matchable [] r = matchable r []
+
+wordsCanBeEqual x y = fromMaybe True $
+    liftM2 pseudoGlobsCanOverlap (wordToPseudoGlob x) (wordToPseudoGlob y)
diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs
index 32282c6..07abd6a 100644
--- a/ShellCheck/Analytics.hs
+++ b/ShellCheck/Analytics.hs
@@ -158,6 +158,7 @@ nodeChecks = [
     ,checkTrailingBracket
     ,checkReturnAgainstZero
     ,checkRedirectedNowhere
+    ,checkUnmatchableCases
     ]
 
 
@@ -1001,14 +1002,20 @@ prop_checkConstantIfs5 = verifyNot checkConstantIfs "[[ $n -le $n ]]"
 prop_checkConstantIfs6 = verifyNot checkConstantIfs "[[ a -ot b ]]"
 prop_checkConstantIfs7 = verifyNot checkConstantIfs "[ a -nt b ]"
 prop_checkConstantIfs8 = verifyNot checkConstantIfs "[[ ~foo == '~foo' ]]"
+prop_checkConstantIfs9 = verify checkConstantIfs "[[ *.png == [a-z] ]]"
 checkConstantIfs _ (TC_Binary id typ op lhs rhs) | not isDynamic =
-    when (isConstant lhs && isConstant rhs) $
-        warn id 2050 "This expression is constant. Did you forget the $ on a variable?"
+    if isConstant lhs && isConstant rhs
+        then  warn id 2050 "This expression is constant. Did you forget the $ on a variable?"
+        else checkUnmatchable id op lhs rhs
   where
     isDynamic =
         op `elem` [ "-lt", "-gt", "-le", "-ge", "-eq", "-ne" ]
             && typ == DoubleBracket
         || op `elem` [ "-nt", "-ot", "-ef"]
+
+    checkUnmatchable id op lhs rhs =
+        when (op `elem` ["=", "==", "!="] && not (wordsCanBeEqual lhs rhs)) $
+            warn id 2193 "The arguments to this comparison can never be equal. Make sure your syntax is correct."
 checkConstantIfs _ _ = return ()
 
 prop_checkLiteralBreakingTest = verify checkLiteralBreakingTest "[[ a==$foo ]]"
@@ -2614,6 +2621,26 @@ checkArrayAssignmentIndices params root =
 
             _ -> return ()
 
+prop_checkUnmatchableCases1 = verify checkUnmatchableCases "case foo in bar) true; esac"
+prop_checkUnmatchableCases2 = verify checkUnmatchableCases "case foo-$bar in ??|*) true; esac"
+prop_checkUnmatchableCases3 = verify checkUnmatchableCases "case foo in foo) true; esac"
+prop_checkUnmatchableCases4 = verifyNot checkUnmatchableCases "case foo-$bar in foo*|*bar|*baz*) true; esac"
+checkUnmatchableCases _ t =
+    case t of
+        T_CaseExpression _ word list ->
+            if isConstant word
+            then warn (getId word) 2194
+                    "This word is constant. Did you forget the $ on a variable?"
+            else  potentially $ do
+                pg <- wordToPseudoGlob word
+                return $ mapM_ (check pg) (concatMap (\(_,x,_) -> x) list)
+        _ -> return ()
+  where
+    check target candidate = potentially $ do
+        candidateGlob <- wordToPseudoGlob candidate
+        guard . not $ pseudoGlobsCanOverlap target candidateGlob
+        return $ warn (getId candidate) 2195
+                    "This pattern will never match the case statement's word. Double check them."
 
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])