From 9092080a84af694f1a1fbc1d59f89d296f362069 Mon Sep 17 00:00:00 2001 From: Martin Schulze Date: Mon, 15 Nov 2021 11:49:36 +0100 Subject: [PATCH 1/2] bats: Add check for useless negation (SC2314/15) --- src/ShellCheck/Analytics.hs | 43 +++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 4a7f364..7c31137 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -199,6 +199,7 @@ nodeChecks = [ ,checkComparisonWithLeadingX ,checkCommandWithTrailingSymbol ,checkUnquotedParameterExpansionPattern + ,checkBatsTestDoesNotUseNegation ] optionalChecks = map fst optionalTreeChecks @@ -4863,5 +4864,47 @@ checkExtraMaskedReturns params t = runNodeAnalysis findMaskingNodes params t hasParent pred t = any (uncurry pred) (parentChildPairs t) +-- hard error on negated command that is not last +prop_checkBatsTestDoesNotUseNegation1 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! true; false; }" +prop_checkBatsTestDoesNotUseNegation2 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [[ -e test ]]; false; }" +prop_checkBatsTestDoesNotUseNegation3 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [ -e test ]; false; }" +-- acceptable formats: +-- using run +prop_checkBatsTestDoesNotUseNegation4 = verifyNot checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { run ! true; }" +-- using || false +prop_checkBatsTestDoesNotUseNegation5 = verifyNot checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [[ -e test ]] || false; }" +prop_checkBatsTestDoesNotUseNegation6 = verifyNot checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [ -e test ] || false; }" +-- only style warning when last command +prop_checkBatsTestDoesNotUseNegation7 = verifyCodes checkBatsTestDoesNotUseNegation [2314] "#!/usr/bin/env/bats\n@test \"name\" { ! true; }" +prop_checkBatsTestDoesNotUseNegation8 = verifyCodes checkBatsTestDoesNotUseNegation [2315] "#!/usr/bin/env/bats\n@test \"name\" { ! [[ -e test ]]; }" +prop_checkBatsTestDoesNotUseNegation9 = verifyCodes checkBatsTestDoesNotUseNegation [2315] "#!/usr/bin/env/bats\n@test \"name\" { ! [ -e test ]; }" + +checkBatsTestDoesNotUseNegation params t = + case t of + T_BatsTest _ _ (T_BraceGroup _ commands) -> mapM_ (check commands) commands + _ -> return () + where + check commands t = + case t of + T_Banged id (T_Pipeline _ _ [T_Redirecting _ _ (T_Condition idCondition _ _)]) -> + if t `isLastOf` commands + then style id 2315 "In Bats, ! will not fail the test if it is not the last command anymore. Fold the `!` into the conditional!" + else err id 2315 + "In Bats, ! does not cause a test failure. Fold the `!` into the conditional!" + + T_Banged id cmd -> if t `isLastOf` commands + then styleWithFix id 2314 + "In Bats, ! will not fail the test if it is not the last command anymore. Use `run ! ` (on Bats >= 1.5.0) instead." + (fixWith [replaceStart id params 0 "run "]) + else errWithFix id 2314 + "In Bats, ! does not cause a test failure. Use 'run ! ' (on Bats >= 1.5.0) instead." + (fixWith [replaceStart id params 0 "run "]) + _ -> return () + isLastOf t commands = + case commands of + [x] -> x == t + x:rest -> isLastOf t rest + [] -> False + return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) From d7971dafd1539de37bb29cb554d5fd4f4b95553b Mon Sep 17 00:00:00 2001 From: Vidar Holen Date: Sat, 4 Dec 2021 17:37:12 -0800 Subject: [PATCH 2/2] Minor formatting fixes --- src/ShellCheck/Analytics.hs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 7c31137..d48104e 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -4868,7 +4868,7 @@ checkExtraMaskedReturns params t = runNodeAnalysis findMaskingNodes params t prop_checkBatsTestDoesNotUseNegation1 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! true; false; }" prop_checkBatsTestDoesNotUseNegation2 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [[ -e test ]]; false; }" prop_checkBatsTestDoesNotUseNegation3 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [ -e test ]; false; }" --- acceptable formats: +-- acceptable formats: -- using run prop_checkBatsTestDoesNotUseNegation4 = verifyNot checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { run ! true; }" -- using || false @@ -4886,21 +4886,18 @@ checkBatsTestDoesNotUseNegation params t = where check commands t = case t of - T_Banged id (T_Pipeline _ _ [T_Redirecting _ _ (T_Condition idCondition _ _)]) -> + T_Banged id (T_Pipeline _ _ [T_Redirecting _ _ (T_Condition idCondition _ _)]) -> if t `isLastOf` commands then style id 2315 "In Bats, ! will not fail the test if it is not the last command anymore. Fold the `!` into the conditional!" - else err id 2315 - "In Bats, ! does not cause a test failure. Fold the `!` into the conditional!" - + else err id 2315 "In Bats, ! does not cause a test failure. Fold the `!` into the conditional!" + T_Banged id cmd -> if t `isLastOf` commands - then styleWithFix id 2314 - "In Bats, ! will not fail the test if it is not the last command anymore. Use `run ! ` (on Bats >= 1.5.0) instead." + then styleWithFix id 2314 "In Bats, ! will not fail the test if it is not the last command anymore. Use `run ! ` (on Bats >= 1.5.0) instead." (fixWith [replaceStart id params 0 "run "]) - else errWithFix id 2314 - "In Bats, ! does not cause a test failure. Use 'run ! ' (on Bats >= 1.5.0) instead." + else errWithFix id 2314 "In Bats, ! does not cause a test failure. Use 'run ! ' (on Bats >= 1.5.0) instead." (fixWith [replaceStart id params 0 "run "]) _ -> return () - isLastOf t commands = + isLastOf t commands = case commands of [x] -> x == t x:rest -> isLastOf t rest