Skip to content

Commit 47d358c

Browse files
committed
Tighten SC2333/SC2334 to only trigger against literals.
1 parent ad58768 commit 47d358c

2 files changed

Lines changed: 25 additions & 10 deletions

File tree

src/ShellCheck/ASTLib.hs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,12 @@ getLiteralStringExt more = g
446446
-- Is this token a string literal?
447447
isLiteral t = isJust $ getLiteralString t
448448

449+
-- Is this token a string literal number?
450+
isLiteralNumber t = fromMaybe False $ do
451+
s <- getLiteralString t
452+
guard $ all isDigit s
453+
return True
454+
449455
-- Escape user data for messages.
450456
-- Messages generally avoid repeating user data, but sometimes it's helpful.
451457
e4m = escapeForMessage

src/ShellCheck/Analytics.hs

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1635,34 +1635,43 @@ checkOrNeq _ (T_OrIf id lhs rhs) = sequence_ $ do
16351635
checkOrNeq _ _ = return ()
16361636

16371637

1638-
prop_checkAndEq1 = verify checkAndEq "if [[ $lol -eq cow && $lol -eq foo ]]; then echo foo; fi"
1639-
prop_checkAndEq2 = verify checkAndEq "(( a==lol && a==foo ))"
1638+
prop_checkAndEq1 = verifyNot checkAndEq "cow=0; foo=0; if [[ $lol -eq cow && $lol -eq foo ]]; then echo foo; fi"
1639+
prop_checkAndEq2 = verifyNot checkAndEq "lol=0 foo=0; (( a==lol && a==foo ))"
16401640
prop_checkAndEq3 = verify checkAndEq "[ \"$a\" = lol && \"$a\" = foo ]"
16411641
prop_checkAndEq4 = verifyNot checkAndEq "[ a = $cow && b = $foo ]"
16421642
prop_checkAndEq5 = verifyNot checkAndEq "[[ $a = /home && $a = */public_html/* ]]"
16431643
prop_checkAndEq6 = verify checkAndEq "[ $a = a ] && [ $a = b ]"
16441644
prop_checkAndEq7 = verify checkAndEq "[ $a = a ] && [ $a = b ] || true"
16451645
prop_checkAndEq8 = verifyNot checkAndEq "[[ $a == x && $a == x ]]"
16461646
prop_checkAndEq9 = verifyNot checkAndEq "[ 0 -eq $FOO ] && [ 0 -eq $BAR ]"
1647+
prop_checkAndEq10 = verify checkAndEq "(( a == 1 && a == 2 ))"
1648+
prop_checkAndEq11 = verify checkAndEq "[ $x -eq 1 ] && [ $x -eq 2 ]"
1649+
prop_checkAndEq12 = verify checkAndEq "[ 1 -eq $x ] && [ $x -eq 2 ]"
1650+
prop_checkAndEq13 = verifyNot checkAndEq "[ 1 -eq $x ] && [ $x -eq 1 ]"
1651+
prop_checkAndEq14 = verifyNot checkAndEq "[ $a = $b ] && [ $a = $c ]"
1652+
1653+
checkAndEqOperands "-eq" rhs1 rhs2 = isLiteralNumber rhs1 && isLiteralNumber rhs2
1654+
checkAndEqOperands op rhs1 rhs2 | op == "=" || op == "==" = isLiteral rhs1 && isLiteral rhs2
1655+
checkAndEqOperands _ _ _ = False
16471656

16481657
-- For test-level "and": [ x = y -a x = z ]
16491658
checkAndEq _ (TC_And id typ op (TC_Binary _ _ op1 lhs1 rhs1 ) (TC_Binary _ _ op2 lhs2 rhs2))
1650-
| (op1 == op2 && (op1 == "-eq" || op1 == "=" || op1 == "==")) && lhs1 == lhs2 && rhs1 /= rhs2 && not (any isGlob [rhs1,rhs2]) =
1651-
warn id 2055 $ "You probably wanted " ++ (if typ == SingleBracket then "-o" else "||") ++ " here, otherwise it's always false."
1659+
| op1 == op2 && lhs1 == lhs2 && rhs1 /= rhs2 && checkAndEqOperands op1 rhs1 rhs2 =
1660+
warn id 2333 $ "You probably wanted " ++ (if typ == SingleBracket then "-o" else "||") ++ " here, otherwise it's always false."
16521661

16531662
-- For arithmetic context "and"
1654-
checkAndEq _ (TA_Binary id "&&" (TA_Binary _ "==" word1 _) (TA_Binary _ "==" word2 _))
1655-
| word1 == word2 =
1656-
warn id 2056 "You probably wanted || here, otherwise it's always false."
1663+
checkAndEq _ (TA_Binary id "&&" (TA_Binary _ "==" lhs1 rhs1) (TA_Binary _ "==" lhs2 rhs2))
1664+
| lhs1 == lhs2 && isLiteralNumber rhs1 && isLiteralNumber rhs2 =
1665+
warn id 2334 "You probably wanted || here, otherwise it's always false."
16571666

16581667
-- For command level "and": [ x = y ] && [ x = z ]
16591668
checkAndEq _ (T_AndIf id lhs rhs) = sequence_ $ do
16601669
(lhs1, op1, rhs1) <- getExpr lhs
16611670
(lhs2, op2, rhs2) <- getExpr rhs
1662-
guard $ op1 == op2 && op1 `elem` ["-eq", "=", "=="]
1671+
guard $ op1 == op2
16631672
guard $ lhs1 == lhs2 && rhs1 /= rhs2
1664-
guard . not $ any isGlob [rhs1, rhs2]
1665-
return $ warn id 2252 "You probably wanted || here, otherwise it's always false."
1673+
guard $ checkAndEqOperands op1 rhs1 rhs2
1674+
return $ warn id 2333 "You probably wanted || here, otherwise it's always false."
16661675
where
16671676
getExpr x =
16681677
case x of

0 commit comments

Comments
 (0)