From 9e35aa7ce8c62aaffe87ecc05a653d3bcbb17b56 Mon Sep 17 00:00:00 2001 From: "mr.Shu" Date: Sun, 14 May 2017 14:00:10 +0200 Subject: [PATCH 1/2] SC2164: Make SC2164 apply to `pushd` and `popd` * Since `pushd` and `popd` have the same failure cases, make the check for SC2164 apply to them as well. * This commit also refactors the code a bit as `hasSetE` is now used in multiple places. * Fixes #863. Signed-off-by: mr.Shu --- ShellCheck/Analytics.hs | 46 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs index 33d81fb..cbb47d9 100644 --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -62,6 +62,8 @@ treeChecks = [ ,checkShebang ,checkUnassignedReferences ,checkUncheckedCd + ,checkUncheckedPushd + ,checkUncheckedPopd ,checkArrayAssignmentIndices ] @@ -2539,7 +2541,7 @@ prop_checkUncheckedCd6 = verifyNotTree checkUncheckedCd "cd .." prop_checkUncheckedCd7 = verifyNotTree checkUncheckedCd "#!/bin/bash -e\ncd foo\nrm bar" prop_checkUncheckedCd8 = verifyNotTree checkUncheckedCd "set -o errexit; cd foo; rm bar" checkUncheckedCd params root = - if hasSetE then [] else execWriter $ doAnalysis checkElement root + if hasSetE root then [] else execWriter $ doAnalysis checkElement root where checkElement t@T_SimpleCommand {} = when(t `isUnqualifiedCommand` "cd" @@ -2548,7 +2550,47 @@ checkUncheckedCd params root = warn (getId t) 2164 "Use 'cd ... || exit' or 'cd ... || return' in case cd fails." checkElement _ = return () isCdDotDot t = oversimplify t == ["cd", ".."] - hasSetE = isNothing $ doAnalysis (guard . not . isSetE) root + +prop_checkUncheckedPushd1 = verifyTree checkUncheckedPushd "pushd ~/src; rm -r foo" +prop_checkUncheckedPushd2 = verifyNotTree checkUncheckedPushd "pushd ~/src || exit; rm -r foo" +prop_checkUncheckedPushd3 = verifyNotTree checkUncheckedPushd "set -e; pushd ~/src; rm -r foo" +prop_checkUncheckedPushd4 = verifyNotTree checkUncheckedPushd "if pushd foo; then rm foo; fi" +prop_checkUncheckedPushd5 = verifyTree checkUncheckedPushd "if true; then pushd foo; fi" +prop_checkUncheckedPushd6 = verifyNotTree checkUncheckedPushd "pushd .." +prop_checkUncheckedPushd7 = verifyNotTree checkUncheckedPushd "#!/bin/bash -e\npushd foo\nrm bar" +prop_checkUncheckedPushd8 = verifyNotTree checkUncheckedPushd "set -o errexit; pushd foo; rm bar" +prop_checkUncheckedPushd9 = verifyNotTree checkUncheckedPushd "pushd -n foo" +checkUncheckedPushd params root = + if hasSetE root then [] else execWriter $ doAnalysis checkElement root + where + checkElement t@T_SimpleCommand {} = + when(t `isUnqualifiedCommand` "pushd" + && not (isPushdDotDot t) + && not ("n" `elem` map snd (getAllFlags t)) + && not (isCondition $ getPath (parentMap params) t)) $ + warn (getId t) 2164 "Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails." + checkElement _ = return () + isPushdDotDot t = oversimplify t == ["pushd", ".."] + +prop_checkUncheckedPopd1 = verifyTree checkUncheckedPopd "popd; rm -r foo" +prop_checkUncheckedPopd2 = verifyNotTree checkUncheckedPopd "popd || exit; rm -r foo" +prop_checkUncheckedPopd3 = verifyNotTree checkUncheckedPopd "set -e; popd; rm -r foo" +prop_checkUncheckedPopd4 = verifyNotTree checkUncheckedPopd "if popd; then rm foo; fi" +prop_checkUncheckedPopd5 = verifyTree checkUncheckedPopd "if true; then popd; fi" +prop_checkUncheckedPopd6 = verifyTree checkUncheckedPopd "popd" +prop_checkUncheckedPopd7 = verifyNotTree checkUncheckedPopd "#!/bin/bash -e\npopd\nrm bar" +prop_checkUncheckedPopd8 = verifyNotTree checkUncheckedPopd "set -o errexit; popd; rm bar" +checkUncheckedPopd params root = + if hasSetE root then [] else execWriter $ doAnalysis checkElement root + where + checkElement t@T_SimpleCommand {} = + when(t `isUnqualifiedCommand` "popd" + && not (isCondition $ getPath (parentMap params) t)) $ + warn (getId t) 2164 "Use 'popd || exit' or 'popd || return' in case popd fails." + checkElement _ = return () + +hasSetE root = isNothing $ doAnalysis (guard . not . isSetE) root + where isSetE t = case t of T_Script _ str _ -> str `matches` re From 954aa99b113cce396ed03e21d556de1bf0530680 Mon Sep 17 00:00:00 2001 From: "mr.Shu" Date: Mon, 12 Jun 2017 12:13:27 +0200 Subject: [PATCH 2/2] Analytics.hs: Refactor cd, popd and pushd checks * Refactor the check of unchecked `cd`, `pushd` and `popd` into one function. Signed-off-by: mr.Shu --- ShellCheck/Analytics.hs | 103 +++++++++++++++------------------------- 1 file changed, 37 insertions(+), 66 deletions(-) diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs index 0409e68..67abb7b 100644 --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -61,9 +61,7 @@ treeChecks = [ ,checkArrayWithoutIndex ,checkShebang ,checkUnassignedReferences - ,checkUncheckedCd - ,checkUncheckedPushd - ,checkUncheckedPopd + ,checkUncheckedCdPushdPopd ,checkArrayAssignmentIndices ] @@ -2465,75 +2463,48 @@ checkReadWithoutR _ t@T_SimpleCommand {} | t `isUnqualifiedCommand` "read" = info (getId t) 2162 "read without -r will mangle backslashes." checkReadWithoutR _ _ = return () -prop_checkUncheckedCd1 = verifyTree checkUncheckedCd "cd ~/src; rm -r foo" -prop_checkUncheckedCd2 = verifyNotTree checkUncheckedCd "cd ~/src || exit; rm -r foo" -prop_checkUncheckedCd3 = verifyNotTree checkUncheckedCd "set -e; cd ~/src; rm -r foo" -prop_checkUncheckedCd4 = verifyNotTree checkUncheckedCd "if cd foo; then rm foo; fi" -prop_checkUncheckedCd5 = verifyTree checkUncheckedCd "if true; then cd foo; fi" -prop_checkUncheckedCd6 = verifyNotTree checkUncheckedCd "cd .." -prop_checkUncheckedCd7 = verifyNotTree checkUncheckedCd "#!/bin/bash -e\ncd foo\nrm bar" -prop_checkUncheckedCd8 = verifyNotTree checkUncheckedCd "set -o errexit; cd foo; rm bar" -checkUncheckedCd params root = - if hasSetE params - then [] +prop_checkUncheckedCd1 = verifyTree checkUncheckedCdPushdPopd "cd ~/src; rm -r foo" +prop_checkUncheckedCd2 = verifyNotTree checkUncheckedCdPushdPopd "cd ~/src || exit; rm -r foo" +prop_checkUncheckedCd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; cd ~/src; rm -r foo" +prop_checkUncheckedCd4 = verifyNotTree checkUncheckedCdPushdPopd "if cd foo; then rm foo; fi" +prop_checkUncheckedCd5 = verifyTree checkUncheckedCdPushdPopd "if true; then cd foo; fi" +prop_checkUncheckedCd6 = verifyNotTree checkUncheckedCdPushdPopd "cd .." +prop_checkUncheckedCd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\ncd foo\nrm bar" +prop_checkUncheckedCd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; cd foo; rm bar" +prop_checkUncheckedPushd1 = verifyTree checkUncheckedCdPushdPopd "pushd ~/src; rm -r foo" +prop_checkUncheckedPushd2 = verifyNotTree checkUncheckedCdPushdPopd "pushd ~/src || exit; rm -r foo" +prop_checkUncheckedPushd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; pushd ~/src; rm -r foo" +prop_checkUncheckedPushd4 = verifyNotTree checkUncheckedCdPushdPopd "if pushd foo; then rm foo; fi" +prop_checkUncheckedPushd5 = verifyTree checkUncheckedCdPushdPopd "if true; then pushd foo; fi" +prop_checkUncheckedPushd6 = verifyNotTree checkUncheckedCdPushdPopd "pushd .." +prop_checkUncheckedPushd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\npushd foo\nrm bar" +prop_checkUncheckedPushd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; pushd foo; rm bar" +prop_checkUncheckedPushd9 = verifyNotTree checkUncheckedCdPushdPopd "pushd -n foo" +prop_checkUncheckedPopd1 = verifyTree checkUncheckedCdPushdPopd "popd; rm -r foo" +prop_checkUncheckedPopd2 = verifyNotTree checkUncheckedCdPushdPopd "popd || exit; rm -r foo" +prop_checkUncheckedPopd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; popd; rm -r foo" +prop_checkUncheckedPopd4 = verifyNotTree checkUncheckedCdPushdPopd "if popd; then rm foo; fi" +prop_checkUncheckedPopd5 = verifyTree checkUncheckedCdPushdPopd "if true; then popd; fi" +prop_checkUncheckedPopd6 = verifyTree checkUncheckedCdPushdPopd "popd" +prop_checkUncheckedPopd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\npopd\nrm bar" +prop_checkUncheckedPopd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; popd; rm bar" + +checkUncheckedCdPushdPopd params root = + if hasSetE params then + [] else execWriter $ doAnalysis checkElement root where checkElement t@T_SimpleCommand {} = - when(t `isUnqualifiedCommand` "cd" - && not (isCdDotDot t) + when(name t `elem` ["cd", "pushd", "popd"] + && not (isSafeDir t) + && not (name t == "pushd" && ("n" `elem` map snd (getAllFlags t))) && not (isCondition $ getPath (parentMap params) t)) $ warn (getId t) 2164 "Use 'cd ... || exit' or 'cd ... || return' in case cd fails." checkElement _ = return () - isCdDotDot t = oversimplify t == ["cd", ".."] - -prop_checkUncheckedPushd1 = verifyTree checkUncheckedPushd "pushd ~/src; rm -r foo" -prop_checkUncheckedPushd2 = verifyNotTree checkUncheckedPushd "pushd ~/src || exit; rm -r foo" -prop_checkUncheckedPushd3 = verifyNotTree checkUncheckedPushd "set -e; pushd ~/src; rm -r foo" -prop_checkUncheckedPushd4 = verifyNotTree checkUncheckedPushd "if pushd foo; then rm foo; fi" -prop_checkUncheckedPushd5 = verifyTree checkUncheckedPushd "if true; then pushd foo; fi" -prop_checkUncheckedPushd6 = verifyNotTree checkUncheckedPushd "pushd .." -prop_checkUncheckedPushd7 = verifyNotTree checkUncheckedPushd "#!/bin/bash -e\npushd foo\nrm bar" -prop_checkUncheckedPushd8 = verifyNotTree checkUncheckedPushd "set -o errexit; pushd foo; rm bar" -prop_checkUncheckedPushd9 = verifyNotTree checkUncheckedPushd "pushd -n foo" -checkUncheckedPushd params root = - if hasSetE root then [] else execWriter $ doAnalysis checkElement root - where - checkElement t@T_SimpleCommand {} = - when(t `isUnqualifiedCommand` "pushd" - && not (isPushdDotDot t) - && not ("n" `elem` map snd (getAllFlags t)) - && not (isCondition $ getPath (parentMap params) t)) $ - warn (getId t) 2164 "Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails." - checkElement _ = return () - isPushdDotDot t = oversimplify t == ["pushd", ".."] - -prop_checkUncheckedPopd1 = verifyTree checkUncheckedPopd "popd; rm -r foo" -prop_checkUncheckedPopd2 = verifyNotTree checkUncheckedPopd "popd || exit; rm -r foo" -prop_checkUncheckedPopd3 = verifyNotTree checkUncheckedPopd "set -e; popd; rm -r foo" -prop_checkUncheckedPopd4 = verifyNotTree checkUncheckedPopd "if popd; then rm foo; fi" -prop_checkUncheckedPopd5 = verifyTree checkUncheckedPopd "if true; then popd; fi" -prop_checkUncheckedPopd6 = verifyTree checkUncheckedPopd "popd" -prop_checkUncheckedPopd7 = verifyNotTree checkUncheckedPopd "#!/bin/bash -e\npopd\nrm bar" -prop_checkUncheckedPopd8 = verifyNotTree checkUncheckedPopd "set -o errexit; popd; rm bar" -checkUncheckedPopd params root = - if hasSetE root then [] else execWriter $ doAnalysis checkElement root - where - checkElement t@T_SimpleCommand {} = - when(t `isUnqualifiedCommand` "popd" - && not (isCondition $ getPath (parentMap params) t)) $ - warn (getId t) 2164 "Use 'popd || exit' or 'popd || return' in case popd fails." - checkElement _ = return () - -hasSetE root = isNothing $ doAnalysis (guard . not . isSetE) root - where - isSetE t = - case t of - T_Script _ str _ -> str `matches` re - T_SimpleCommand {} -> - t `isUnqualifiedCommand` "set" && - ("errexit" `elem` oversimplify t || "e" `elem` map snd (getAllFlags t)) - _ -> False - re = mkRegex "[[:space:]]-[^-]*e" + name t = fromMaybe "" $ getCommandName t + isSafeDir t = case oversimplify t of + [_, ".."] -> True; + _ -> False prop_checkLoopVariableReassignment1 = verify checkLoopVariableReassignment "for i in *; do for i in *.bar; do true; done; done" prop_checkLoopVariableReassignment2 = verify checkLoopVariableReassignment "for i in *; do for((i=0; i<3; i++)); do true; done; done"