From e9784fa9a77f6f018997f5d164148422f7da0b33 Mon Sep 17 00:00:00 2001
From: Vidar Holen <vidar@vidarholen.net>
Date: Mon, 25 Jul 2022 11:57:04 -0700
Subject: [PATCH] Refine #2544 to not warn when $? postdominates [ ] (fixes
#2544)
---
src/ShellCheck/Analytics.hs | 13 +++++++++++--
src/ShellCheck/CFGAnalysis.hs | 10 ++++++++++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
index 8499d8d..42d5a9e 100644
--- a/src/ShellCheck/Analytics.hs
+++ b/src/ShellCheck/Analytics.hs
@@ -4884,7 +4884,11 @@ checkCommandIsUnreachable params t =
prop_checkOverwrittenExitCode1 = verify checkOverwrittenExitCode "x; [ $? -eq 1 ] || [ $? -eq 2 ]"
prop_checkOverwrittenExitCode2 = verifyNot checkOverwrittenExitCode "x; [ $? -eq 1 ]"
prop_checkOverwrittenExitCode3 = verify checkOverwrittenExitCode "x; echo \"Exit is $?\"; [ $? -eq 0 ]"
-prop_checkOverwrittenExitCode4 = verifyNot checkOverwrittenExitCode "x; [ $? -eq 0 ]"
+prop_checkOverwrittenExitCode4 = verifyNot checkOverwrittenExitCode "x; [ $? -eq 0 ] && echo Success"
+prop_checkOverwrittenExitCode5 = verify checkOverwrittenExitCode "x; if [ $? -eq 0 ]; then var=$?; fi"
+prop_checkOverwrittenExitCode6 = verify checkOverwrittenExitCode "x; [ $? -gt 0 ] && fail=$?"
+prop_checkOverwrittenExitCode7 = verifyNot checkOverwrittenExitCode "[ 1 -eq 2 ]; status=$?"
+prop_checkOverwrittenExitCode8 = verifyNot checkOverwrittenExitCode "[ 1 -eq 2 ]; exit $?"
checkOverwrittenExitCode params t =
case t of
T_DollarBraced id _ val | getLiteralString val == Just "?" -> check id
@@ -4898,7 +4902,7 @@ checkOverwrittenExitCode params t =
let idToToken = idMap params
exitCodeTokens <- sequence $ map (\k -> Map.lookup k idToToken) $ S.toList exitCodeIds
return $ do
- when (all isCondition exitCodeTokens) $
+ when (all isCondition exitCodeTokens && not (usedUnconditionally t exitCodeIds)) $
warn id 2319 "This $? refers to a condition, not a command. Assign to a variable to avoid it being overwritten."
when (all isPrinting exitCodeTokens) $
warn id 2320 "This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten."
@@ -4909,6 +4913,11 @@ checkOverwrittenExitCode params t =
T_SimpleCommand {} -> getCommandName t == Just "test"
_ -> False
+ -- If we don't do anything based on the condition, assume we wanted the condition itself
+ -- This helps differentiate `x; [ $? -gt 0 ] && exit $?` vs `[ cond ]; exit $?`
+ usedUnconditionally t testIds =
+ all (\c -> CF.doesPostDominate (cfgAnalysis params) (getId t) c) testIds
+
isPrinting t =
case getCommandBasename t of
Just "echo" -> True
diff --git a/src/ShellCheck/CFGAnalysis.hs b/src/ShellCheck/CFGAnalysis.hs
index dc0a4b1..ff88810 100644
--- a/src/ShellCheck/CFGAnalysis.hs
+++ b/src/ShellCheck/CFGAnalysis.hs
@@ -56,6 +56,7 @@ module ShellCheck.CFGAnalysis (
,SpaceStatus (..)
,getIncomingState
,getOutgoingState
+ ,doesPostDominate
,ShellCheck.CFGAnalysis.runTests -- STRIP
) where
@@ -140,6 +141,15 @@ getOutgoingState analysis id = do
(start,end) <- M.lookup id $ tokenToRange analysis
snd <$> M.lookup end (nodeToData analysis)
+-- Conveniently determine whether one node postdominates another,
+-- i.e. whether 'target' always unconditionally runs after 'base'.
+doesPostDominate :: CFGAnalysis -> Id -> Id -> Bool
+doesPostDominate analysis target base = fromMaybe False $ do
+ (_, baseEnd) <- M.lookup base $ tokenToRange analysis
+ (targetStart, _) <- M.lookup target $ tokenToRange analysis
+ postDoms <- M.lookup baseEnd $ postDominators analysis
+ return $ S.member targetStart postDoms
+
getDataForNode analysis node = M.lookup node $ nodeToData analysis
-- The current state of data flow at a point in the program, potentially as a diff