Fix bad advice for SC2251 (fixes #1588)

This commit is contained in:
Vidar Holen 2019-07-04 19:10:14 -07:00
parent 78b8e76066
commit 788cf17076
1 changed files with 20 additions and 10 deletions

View File

@ -3395,18 +3395,23 @@ checkDefaultCase _ t =
pg <- wordToExactPseudoGlob pat pg <- wordToExactPseudoGlob pat
return $ pseudoGlobIsSuperSetof pg [PGMany] return $ pseudoGlobIsSuperSetof pg [PGMany]
prop_checkUselessBang1 = verify checkUselessBang "! true; rest" prop_checkUselessBang1 = verify checkUselessBang "set -e; ! true; rest"
prop_checkUselessBang2 = verify checkUselessBang "while true; do ! true; done" prop_checkUselessBang2 = verifyNot checkUselessBang "! true; rest"
prop_checkUselessBang3 = verifyNot checkUselessBang "if ! true; then true; fi" prop_checkUselessBang3 = verify checkUselessBang "set -e; while true; do ! true; done"
prop_checkUselessBang4 = verifyNot checkUselessBang "( ! true )" prop_checkUselessBang4 = verifyNot checkUselessBang "set -e; if ! true; then true; fi"
prop_checkUselessBang5 = verifyNot checkUselessBang "{ ! true; }" prop_checkUselessBang5 = verifyNot checkUselessBang "set -e; ( ! true )"
prop_checkUselessBang6 = verifyNot checkUselessBang "x() { ! [ x ]; }" prop_checkUselessBang6 = verify checkUselessBang "set -e; { ! true; }"
checkUselessBang params t = mapM_ check (getNonReturningCommands t) prop_checkUselessBang7 = verifyNot checkUselessBang "set -e; x() { ! [ x ]; }"
prop_checkUselessBang8 = verifyNot checkUselessBang "set -e; if { ! true; }; then true; fi"
prop_checkUselessBang9 = verifyNot checkUselessBang "set -e; while ! true; do true; done"
checkUselessBang params t = when (hasSetE params) $ mapM_ check (getNonReturningCommands t)
where where
check t = check t =
case t of case t of
T_Banged id _ -> T_Banged id cmd | not $ isCondition (getPath (parentMap params) t) ->
info id 2251 "This ! is not on a condition and skips errexit. Use { ! ...; } to errexit, or verify usage." addComment $ makeCommentWithFix InfoC id 2251
"This ! is not on a condition and skips errexit. Use `&& exit 1` instead, or make sure $? is checked."
(fixWith [replaceStart id params 1 "", replaceEnd (getId cmd) params 0 " && exit 1"])
_ -> return () _ -> return ()
-- Get all the subcommands that aren't likely to be the return value -- Get all the subcommands that aren't likely to be the return value
@ -3414,7 +3419,7 @@ checkUselessBang params t = mapM_ check (getNonReturningCommands t)
getNonReturningCommands t = getNonReturningCommands t =
case t of case t of
T_Script _ _ list -> dropLast list T_Script _ _ list -> dropLast list
T_BraceGroup _ list -> dropLast list T_BraceGroup _ list -> if isFunctionBody t then dropLast list else list
T_Subshell _ list -> dropLast list T_Subshell _ list -> dropLast list
T_WhileExpression _ conds cmds -> dropLast conds ++ cmds T_WhileExpression _ conds cmds -> dropLast conds ++ cmds
T_UntilExpression _ conds cmds -> dropLast conds ++ cmds T_UntilExpression _ conds cmds -> dropLast conds ++ cmds
@ -3425,6 +3430,11 @@ checkUselessBang params t = mapM_ check (getNonReturningCommands t)
concatMap (dropLast . fst) conds ++ concatMap snd conds ++ elses concatMap (dropLast . fst) conds ++ concatMap snd conds ++ elses
_ -> [] _ -> []
isFunctionBody t =
case getPath (parentMap params) t of
_:T_Function {}:_-> True
_ -> False
dropLast t = dropLast t =
case t of case t of
[_] -> [] [_] -> []