diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e2662d..d7fb5e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Latest - ??? ### Added +- SC2231: Suggest quoting expansions in for loop globs - SC2229: Warn about 'read $var' - SC2227: Warn about redirections in the middle of 'find' commands - SC2224,SC2225,SC2226: Warn when using mv/cp/ln without a destination diff --git a/src/ShellCheck/ASTLib.hs b/src/ShellCheck/ASTLib.hs index dc58a12..7bb356d 100644 --- a/src/ShellCheck/ASTLib.hs +++ b/src/ShellCheck/ASTLib.hs @@ -435,3 +435,12 @@ pseudoGlobIsSuperSetof = matchable wordsCanBeEqual x y = fromMaybe True $ liftM2 pseudoGlobsCanOverlap (wordToPseudoGlob x) (wordToPseudoGlob y) + +-- Is this an expansion that can be quoted, +-- e.g. $(foo) `foo` $foo (but not {foo,})? +isQuoteableExpansion t = case t of + T_DollarExpansion {} -> True + T_DollarBraceCommandExpansion {} -> True + T_Backticked {} -> True + T_DollarBraced {} -> True + _ -> False diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index b094acf..1e7576e 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -166,6 +166,7 @@ nodeChecks = [ ,checkFlagAsCommand ,checkEmptyCondition ,checkPipeToNowhere + ,checkForLoopGlobVariables ] @@ -2908,5 +2909,19 @@ checkUseBeforeDefinition _ t = then [x] else concatMap recursiveSequences list +prop_checkForLoopGlobVariables1 = verify checkForLoopGlobVariables "for i in $var/*.txt; do true; done" +prop_checkForLoopGlobVariables2 = verifyNot checkForLoopGlobVariables "for i in \"$var\"/*.txt; do true; done" +prop_checkForLoopGlobVariables3 = verifyNot checkForLoopGlobVariables "for i in $var; do true; done" +checkForLoopGlobVariables _ t = + case t of + T_ForIn _ _ words _ -> mapM_ check words + _ -> return () + where + check (T_NormalWord _ parts) = + when (any isGlob parts) $ + mapM_ suggest $ filter isQuoteableExpansion parts + suggest t = info (getId t) 2231 + "Quote expansions in this for loop glob to prevent wordsplitting, e.g. \"$dir\"/*.txt ." + return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])