From 3e7c2bfec04ed6a0479a5f0e35d129c8089f3e7e Mon Sep 17 00:00:00 2001 From: Vidar Holen Date: Sun, 2 Jun 2019 09:24:53 -0700 Subject: [PATCH] Warn about [ $a != x ] || [ $a != y ] --- CHANGELOG.md | 1 + src/ShellCheck/Analytics.hs | 29 +++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 262849f..93f83c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - 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 +- SC2252: Warn about `[ $a != x ] || [ $a != y ]`, similar to SC2055 - SC2251: Inform about ineffectual ! in front of commands - SC2250: Warn about variable references without braces (optional) - SC2249: Warn about `case` with missing default case (optional) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 36b941e..3ee454c 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -1348,14 +1348,39 @@ prop_checkOrNeq2 = verify checkOrNeq "(( a!=lol || a!=foo ))" prop_checkOrNeq3 = verify checkOrNeq "[ \"$a\" != lol || \"$a\" != foo ]" prop_checkOrNeq4 = verifyNot checkOrNeq "[ a != $cow || b != $foo ]" prop_checkOrNeq5 = verifyNot checkOrNeq "[[ $a != /home || $a != */public_html/* ]]" +prop_checkOrNeq6 = verify checkOrNeq "[ $a != a ] || [ $a != b ]" +prop_checkOrNeq7 = verify checkOrNeq "[ $a != a ] || [ $a != b ] || true" +prop_checkOrNeq8 = verifyNot checkOrNeq "[[ $a != x || $a != x ]]" -- This only catches the most idiomatic cases. Fixme? + +-- For test-level "or": [ x != y -o x != z ] checkOrNeq _ (TC_Or id typ op (TC_Binary _ _ op1 lhs1 rhs1 ) (TC_Binary _ _ op2 lhs2 rhs2)) - | lhs1 == lhs2 && (op1 == op2 && (op1 == "-ne" || op1 == "!=")) && not (any isGlob [rhs1,rhs2]) = + | (op1 == op2 && (op1 == "-ne" || op1 == "!=")) && lhs1 == lhs2 && rhs1 /= rhs2 && not (any isGlob [rhs1,rhs2]) = warn id 2055 $ "You probably wanted " ++ (if typ == SingleBracket then "-a" else "&&") ++ " here." +-- For arithmetic context "or" checkOrNeq _ (TA_Binary id "||" (TA_Binary _ "!=" word1 _) (TA_Binary _ "!=" word2 _)) | word1 == word2 = - warn id 2056 "You probably wanted && here." + warn id 2056 "You probably wanted && here, otherwise it's always true." + +-- For command level "or": [ x != y ] || [ x != z ] +checkOrNeq _ (T_OrIf id lhs rhs) = potentially $ do + (lhs1, op1, rhs1) <- getExpr lhs + (lhs2, op2, rhs2) <- getExpr rhs + guard $ op1 == op2 && op1 `elem` ["-ne", "!="] + guard $ lhs1 == lhs2 && rhs1 /= rhs2 + guard . not $ any isGlob [rhs1, rhs2] + return $ warn id 2252 "You probably wanted && here, otherwise it's always true." + where + getExpr x = + case x of + T_OrIf _ lhs _ -> getExpr lhs -- Fetches x and y in `T_OrIf x (T_OrIf y z)` + T_Pipeline _ _ [x] -> getExpr x + T_Redirecting _ _ c -> getExpr c + T_Condition _ _ c -> getExpr c + TC_Binary _ _ op lhs rhs -> return (lhs, op, rhs) + _ -> fail "" + checkOrNeq _ _ = return ()