diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f834aa..c3e4940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,14 @@ - SC2331: Suggest using standard -e instead of unary -a in tests. - SC2332: Warn about `[ ! -o opt ]` being unconditionally true in Bash. - SC3062: Warn about bashism `[ -o opt ]`. +- Optional `avoid-negated-conditions`: suggest replacing `[ ! a -eq b ]` + with `[ a -ne b ]`, and similar for -ge/-lt/=/!=/etc (SC2335). - Precompiled binaries for Linux riscv64 (linux.riscv64) ### Changed - SC2002 about Useless Use Of Cat is now disabled by default. It can be re-enabled with `--enable=useless-use-of-cat` or equivalent directive. +- SC2236/SC2237 about replacing `[ ! -n .. ]` with `[ -z ]` and vice versa + is now optional under `avoid-negated-conditions`. - SC2015 about `A && B || C` no longer triggers when B is a test command. - SC3012: Do not warn about `\<` and `\>` in test/[] as specified in POSIX.1-2024 - Diff output now uses / as path separator on Windows diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index a13995a..373d495 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -183,7 +183,6 @@ nodeChecks = [ ,checkPipeToNowhere ,checkForLoopGlobVariables ,checkSubshelledTests - ,checkInvertedStringTest ,checkRedirectionToCommand ,checkDollarQuoteParen ,checkUselessBang @@ -233,6 +232,13 @@ optionalTreeChecks = [ cdNegative = "[ -n \"$var\" ]" }, nodeChecksToTreeCheck [checkNullaryExpansionTest]) + ,(newCheckDescription { + cdName = "avoid-negated-conditions", + cdDescription = "Suggest removing unnecessary comparison negations", + cdPositive = "[ ! \"$var\" -eq 1 ]", + cdNegative = "[ \"$var\" -ne 1 ]" + }, nodeChecksToTreeCheck [checkUnnecessarilyInvertedTest]) + ,(newCheckDescription { cdName = "add-default-case", cdDescription = "Suggest adding a default case in `case` statements", @@ -3991,12 +3997,17 @@ checkSubshelledTests params t = T_Annotation {} -> True _ -> False -prop_checkInvertedStringTest1 = verify checkInvertedStringTest "[ ! -z $var ]" -prop_checkInvertedStringTest2 = verify checkInvertedStringTest "! [[ -n $var ]]" -prop_checkInvertedStringTest3 = verifyNot checkInvertedStringTest "! [ -x $var ]" -prop_checkInvertedStringTest4 = verifyNot checkInvertedStringTest "[[ ! -w $var ]]" -prop_checkInvertedStringTest5 = verifyNot checkInvertedStringTest "[ -z $var ]" -checkInvertedStringTest _ t = +prop_checkUnnecessarilyInvertedTest1 = verify checkUnnecessarilyInvertedTest "[ ! -z $var ]" +prop_checkUnnecessarilyInvertedTest2 = verify checkUnnecessarilyInvertedTest "! [[ -n $var ]]" +prop_checkUnnecessarilyInvertedTest3 = verifyNot checkUnnecessarilyInvertedTest "! [ -x $var ]" +prop_checkUnnecessarilyInvertedTest4 = verifyNot checkUnnecessarilyInvertedTest "[[ ! -w $var ]]" +prop_checkUnnecessarilyInvertedTest5 = verifyNot checkUnnecessarilyInvertedTest "[ -z $var ]" +prop_checkUnnecessarilyInvertedTest6 = verify checkUnnecessarilyInvertedTest "! [ $var != foo ]" +prop_checkUnnecessarilyInvertedTest7 = verify checkUnnecessarilyInvertedTest "[[ ! $var == foo ]]" +prop_checkUnnecessarilyInvertedTest8 = verifyNot checkUnnecessarilyInvertedTest "! [[ $var =~ .* ]]" +prop_checkUnnecessarilyInvertedTest9 = verify checkUnnecessarilyInvertedTest "[ ! $var -eq 0 ]" +prop_checkUnnecessarilyInvertedTest10 = verify checkUnnecessarilyInvertedTest "! [[ $var -gt 3 ]]" +checkUnnecessarilyInvertedTest _ t = case t of TC_Unary _ _ "!" (TC_Unary _ _ op _) -> case op of @@ -4009,7 +4020,34 @@ checkInvertedStringTest _ t = "-n" -> style (getId t) 2237 "Use [ -z .. ] instead of ! [ -n .. ]." "-z" -> style (getId t) 2237 "Use [ -n .. ] instead of ! [ -z .. ]." _ -> return () + TC_Unary _ _ "!" (TC_Binary _ bracketStyle op _ _) -> + maybeSuggestRewrite True bracketStyle (getId t) op + T_Banged _ (T_Pipeline _ _ + [T_Redirecting _ _ (T_Condition _ _ (TC_Binary _ bracketStyle op _ _))]) -> + maybeSuggestRewrite False bracketStyle (getId t) op _ -> return () + where + inversionMap = Map.fromList [ + ("=", "!="), + ("==", "!="), + ("!=", "="), + ("-eq", "-ne"), + ("-ne", "-eq"), + ("-le", "-gt"), + ("-gt", "-le"), + ("-ge", "-lt"), + ("-lt", "-ge") + ] + maybeSuggestRewrite bangInside bracketStyle id op = sequence_ $ do + newOp <- Map.lookup op inversionMap + let oldExpr = "a " ++ op ++ " b" + let newExpr = "a " ++ newOp ++ " b" + let bracket s = if bracketStyle == SingleBracket then "[ " ++ s ++ " ]" else "[[ " ++ s ++ " ]]" + return $ + if bangInside + then style id 2335 $ "Use " ++ newExpr ++ " instead of ! " ++ oldExpr ++ "." + else style id 2335 $ "Use " ++ (bracket newExpr) ++ " instead of ! " ++ (bracket oldExpr) ++ "." + prop_checkRedirectionToCommand1 = verify checkRedirectionToCommand "ls > rm" prop_checkRedirectionToCommand2 = verifyNot checkRedirectionToCommand "ls > 'rm'"