Skip to content

Commit

Permalink
Skip SC2214 if variable is modified in loop (fixes koalaman#2351)
Browse files Browse the repository at this point in the history
  • Loading branch information
koalaman committed Oct 9, 2021
1 parent 3aedda7 commit c3aaa27
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 9 deletions.
10 changes: 1 addition & 9 deletions src/ShellCheck/Analytics.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1957,17 +1957,9 @@ subshellAssignmentCheck params t =

findSubshelled [] _ _ = return ()
findSubshelled (Assignment x@(_, _, str, data_):rest) scopes@((reason,scope):restscope) deadVars =
if isTrueAssignment data_
if isTrueAssignmentSource data_
then findSubshelled rest ((reason, x:scope):restscope) $ Map.insert str Alive deadVars
else findSubshelled rest scopes deadVars
where
isTrueAssignment c =
case c of
DataString SourceChecked -> False
DataString SourceDeclaration -> False
DataArray SourceChecked -> False
DataArray SourceDeclaration -> False
_ -> True

findSubshelled (Reference (_, readToken, str):rest) scopes deadVars = do
unless (shouldIgnore str) $ case Map.findWithDefault Alive str deadVars of
Expand Down
17 changes: 17 additions & 0 deletions src/ShellCheck/AnalyzerLib.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,23 @@ isUnmodifiedParameterExpansion t =
in getBracedReference str == str
_ -> False

isTrueAssignmentSource c =
case c of
DataString SourceChecked -> False
DataString SourceDeclaration -> False
DataArray SourceChecked -> False
DataArray SourceDeclaration -> False
_ -> True

modifiesVariable params token name =
or $ map check flow
where
flow = getVariableFlow params token
check t =
case t of
Assignment (_, _, n, source) -> isTrueAssignmentSource source && n == name
_ -> False


return []
runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])
5 changes: 5 additions & 0 deletions src/ShellCheck/Checks/Commands.hs
Original file line number Diff line number Diff line change
Expand Up @@ -961,11 +961,13 @@ prop_checkWhileGetoptsCase4 = verifyNot checkWhileGetoptsCase "while getopts 'a:
prop_checkWhileGetoptsCase5 = verifyNot checkWhileGetoptsCase "while getopts 'a:' x; do case $x in a) foo;; \\?) bar;; *) baz;; esac; done"
prop_checkWhileGetoptsCase6 = verifyNot checkWhileGetoptsCase "while getopts 'a:b' x; do case $y in a) foo;; esac; done"
prop_checkWhileGetoptsCase7 = verifyNot checkWhileGetoptsCase "while getopts 'a:b' x; do case x$x in xa) foo;; xb) foo;; esac; done"
prop_checkWhileGetoptsCase8 = verifyNot checkWhileGetoptsCase "while getopts 'a:b' x; do x=a; case $x in a) foo;; esac; done"
checkWhileGetoptsCase = CommandCheck (Exactly "getopts") f
where
f :: Token -> Analysis
f t@(T_SimpleCommand _ _ (cmd:arg1:name:_)) = do
path <- getPathM t
params <- ask
sequence_ $ do
options <- getLiteralString arg1
getoptsVar <- getLiteralString name
Expand All @@ -977,6 +979,9 @@ checkWhileGetoptsCase = CommandCheck (Exactly "getopts") f
[T_Literal _ caseVar] <- return $ getWordParts bracedWord
guard $ caseVar == getoptsVar

-- Make sure the variable isn't modified
guard . not $ modifiesVariable params (T_BraceGroup (Id 0) body) getoptsVar

return $ check (getId arg1) (map (:[]) $ filter (/= ':') options) caseCmd
f _ = return ()

Expand Down

0 comments on commit c3aaa27

Please sign in to comment.