diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 8499d8d0e..42d5a9eb4 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 dc0a4b17a..ff888109a 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