Add SC2317 warning about unreachable commands

This commit is contained in:
Vidar Holen 2022-07-19 14:33:00 -07:00
parent f77a545282
commit 642ad86125
2 changed files with 25 additions and 8 deletions

View File

@ -1,6 +1,7 @@
## Git ## Git
### Added ### Added
- SC2316: Warn about 'local readonly foo' and similar (thanks, patrickxia!) - SC2316: Warn about 'local readonly foo' and similar (thanks, patrickxia!)
- SC2317: Warn about unreachable commands
### Fixed ### Fixed

View File

@ -202,6 +202,7 @@ nodeChecks = [
,checkCommandWithTrailingSymbol ,checkCommandWithTrailingSymbol
,checkUnquotedParameterExpansionPattern ,checkUnquotedParameterExpansionPattern
,checkBatsTestDoesNotUseNegation ,checkBatsTestDoesNotUseNegation
,checkCommandIsUnreachable
] ]
optionalChecks = map fst optionalTreeChecks optionalChecks = map fst optionalTreeChecks
@ -4129,13 +4130,6 @@ checkAliasUsedInSameParsingUnit params root =
checkUnit :: [Token] -> Writer [TokenComment] () checkUnit :: [Token] -> Writer [TokenComment] ()
checkUnit unit = evalStateT (mapM_ (doAnalysis findCommands) unit) (Map.empty) checkUnit unit = evalStateT (mapM_ (doAnalysis findCommands) unit) (Map.empty)
isSourced t =
let
f (T_SourceCommand {}) = True
f _ = False
in
any f $ getPath (parentMap params) t
findCommands :: Token -> StateT (Map.Map String Token) (Writer [TokenComment]) () findCommands :: Token -> StateT (Map.Map String Token) (Writer [TokenComment]) ()
findCommands t = case t of findCommands t = case t of
T_SimpleCommand _ _ (cmd:args) -> T_SimpleCommand _ _ (cmd:args) ->
@ -4146,7 +4140,7 @@ checkAliasUsedInSameParsingUnit params root =
cmd <- gets (Map.lookup name) cmd <- gets (Map.lookup name)
case cmd of case cmd of
Just alias -> Just alias ->
unless (isSourced t || shouldIgnoreCode params 2262 alias) $ do unless (isSourced params t || shouldIgnoreCode params 2262 alias) $ do
warn (getId alias) 2262 "This alias can't be defined and used in the same parsing unit. Use a function instead." warn (getId alias) 2262 "This alias can't be defined and used in the same parsing unit. Use a function instead."
info (getId t) 2263 "Since they're in the same parsing unit, this command will not refer to the previously mentioned alias." info (getId t) 2263 "Since they're in the same parsing unit, this command will not refer to the previously mentioned alias."
_ -> return () _ -> return ()
@ -4157,6 +4151,14 @@ checkAliasUsedInSameParsingUnit params root =
when (isVariableName name && not (null value)) $ when (isVariableName name && not (null value)) $
modify (Map.insertWith (\new old -> old) name arg) modify (Map.insertWith (\new old -> old) name arg)
isSourced params t =
let
f (T_SourceCommand {}) = True
f _ = False
in
any f $ getPath (parentMap params) t
-- Like groupBy, but compares pairs of adjacent elements, rather than against the first of the span -- Like groupBy, but compares pairs of adjacent elements, rather than against the first of the span
prop_groupByLink1 = groupByLink (\a b -> a+1 == b) [1,2,3,2,3,7,8,9] == [[1,2,3], [2,3], [7,8,9]] prop_groupByLink1 = groupByLink (\a b -> a+1 == b) [1,2,3,2,3,7,8,9] == [[1,2,3], [2,3], [7,8,9]]
prop_groupByLink2 = groupByLink (==) ([] :: [()]) == [] prop_groupByLink2 = groupByLink (==) ([] :: [()]) == []
@ -4910,5 +4912,19 @@ checkBatsTestDoesNotUseNegation params t =
x:rest -> isLastOf t rest x:rest -> isLastOf t rest
[] -> False [] -> False
prop_checkCommandIsUnreachable1 = verify checkCommandIsUnreachable "foo; bar; exit; baz"
prop_checkCommandIsUnreachable2 = verify checkCommandIsUnreachable "die() { exit; }; foo; bar; die; baz"
prop_checkCommandIsUnreachable3 = verifyNot checkCommandIsUnreachable "foo; bar || exit; baz"
checkCommandIsUnreachable params t =
case t of
T_Pipeline {} -> sequence_ $ do
state <- CF.getIncomingState (cfgAnalysis params) id
guard . not $ CF.stateIsReachable state
guard . not $ isSourced params t
return $ info id 2317 "Command appears to be unreachable. Check usage (or ignore if invoked indirectly)."
_ -> return ()
where id = getId t
return [] return []
runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])