1{- 2 Copyright 2012-2021 Vidar Holen 3 4 This file is part of ShellCheck. 5 https://www.shellcheck.net 6 7 ShellCheck is free software: you can redistribute it and/or modify 8 it under the terms of the GNU General Public License as published by 9 the Free Software Foundation, either version 3 of the License, or 10 (at your option) any later version. 11 12 ShellCheck is distributed in the hope that it will be useful, 13 but WITHOUT ANY WARRANTY; without even the implied warranty of 14 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 15 GNU General Public License for more details. 16 17 You should have received a copy of the GNU General Public License 18 along with this program. If not, see <https://www.gnu.org/licenses/>. 19-} 20{-# LANGUAGE TemplateHaskell #-} 21{-# LANGUAGE FlexibleContexts #-} 22module ShellCheck.Analytics (runAnalytics, optionalChecks, ShellCheck.Analytics.runTests) where 23 24import ShellCheck.AST 25import ShellCheck.ASTLib 26import ShellCheck.AnalyzerLib hiding (producesComments) 27import ShellCheck.Data 28import ShellCheck.Parser 29import ShellCheck.Interface 30import ShellCheck.Regex 31 32import Control.Arrow (first) 33import Control.Monad 34import Control.Monad.Identity 35import Control.Monad.State 36import Control.Monad.Writer hiding ((<>)) 37import Control.Monad.Reader 38import Data.Char 39import Data.Functor 40import Data.Function (on) 41import Data.List 42import Data.Maybe 43import Data.Ord 44import Data.Semigroup 45import Debug.Trace -- STRIP 46import qualified Data.Map.Strict as Map 47import Test.QuickCheck.All (forAllProperties) 48import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) 49 50-- Checks that are run on the AST root 51treeChecks :: [Parameters -> Token -> [TokenComment]] 52treeChecks = [ 53 nodeChecksToTreeCheck nodeChecks 54 ,subshellAssignmentCheck 55 ,checkSpacefulness 56 ,checkQuotesInLiterals 57 ,checkShebangParameters 58 ,checkFunctionsUsedExternally 59 ,checkUnusedAssignments 60 ,checkUnpassedInFunctions 61 ,checkArrayWithoutIndex 62 ,checkShebang 63 ,checkUnassignedReferences 64 ,checkUncheckedCdPushdPopd 65 ,checkArrayAssignmentIndices 66 ,checkUseBeforeDefinition 67 ,checkAliasUsedInSameParsingUnit 68 ,checkArrayValueUsedAsIndex 69 ] 70 71runAnalytics :: AnalysisSpec -> [TokenComment] 72runAnalytics options = 73 runList options treeChecks ++ runList options optionalChecks 74 where 75 root = asScript options 76 optionals = getEnableDirectives root ++ asOptionalChecks options 77 optionalChecks = 78 if "all" `elem` optionals 79 then map snd optionalTreeChecks 80 else mapMaybe (\c -> Map.lookup c optionalCheckMap) optionals 81 82runList :: AnalysisSpec -> [Parameters -> Token -> [TokenComment]] 83 -> [TokenComment] 84runList spec list = notes 85 where 86 root = asScript spec 87 params = makeParameters spec 88 notes = concatMap (\f -> f params root) list 89 90getEnableDirectives root = 91 case root of 92 T_Annotation _ list _ -> [s | EnableComment s <- list] 93 _ -> [] 94 95checkList l t = concatMap (\f -> f t) l 96 97-- Checks that are run on each node in the AST 98runNodeAnalysis f p t = execWriter (doAnalysis (f p) t) 99 100-- Perform multiple node checks in a single iteration over the tree 101nodeChecksToTreeCheck checkList = 102 runNodeAnalysis 103 (\p t -> (mapM_ ((\ f -> f t) . (\ f -> f p)) 104 checkList)) 105 106nodeChecks :: [Parameters -> Token -> Writer [TokenComment] ()] 107nodeChecks = [ 108 checkUuoc 109 ,checkPipePitfalls 110 ,checkForInQuoted 111 ,checkForInLs 112 ,checkShorthandIf 113 ,checkDollarStar 114 ,checkUnquotedDollarAt 115 ,checkStderrRedirect 116 ,checkUnquotedN 117 ,checkNumberComparisons 118 ,checkSingleBracketOperators 119 ,checkDoubleBracketOperators 120 ,checkLiteralBreakingTest 121 ,checkConstantNullary 122 ,checkDivBeforeMult 123 ,checkArithmeticDeref 124 ,checkArithmeticBadOctal 125 ,checkComparisonAgainstGlob 126 ,checkCaseAgainstGlob 127 ,checkCommarrays 128 ,checkOrNeq 129 ,checkEchoWc 130 ,checkConstantIfs 131 ,checkPipedAssignment 132 ,checkAssignAteCommand 133 ,checkUuoeVar 134 ,checkQuotedCondRegex 135 ,checkForInCat 136 ,checkFindExec 137 ,checkValidCondOps 138 ,checkGlobbedRegex 139 ,checkTestRedirects 140 ,checkBadParameterSubstitution 141 ,checkPS1Assignments 142 ,checkBackticks 143 ,checkInexplicablyUnquoted 144 ,checkTildeInQuotes 145 ,checkLonelyDotDash 146 ,checkSpuriousExec 147 ,checkSpuriousExpansion 148 ,checkDollarBrackets 149 ,checkSshHereDoc 150 ,checkGlobsAsOptions 151 ,checkWhileReadPitfalls 152 ,checkArithmeticOpCommand 153 ,checkCharRangeGlob 154 ,checkUnquotedExpansions 155 ,checkSingleQuotedVariables 156 ,checkRedirectToSame 157 ,checkPrefixAssignmentReference 158 ,checkLoopKeywordScope 159 ,checkCdAndBack 160 ,checkWrongArithmeticAssignment 161 ,checkConditionalAndOrs 162 ,checkFunctionDeclarations 163 ,checkStderrPipe 164 ,checkOverridingPath 165 ,checkArrayAsString 166 ,checkUnsupported 167 ,checkMultipleAppends 168 ,checkSuspiciousIFS 169 ,checkShouldUseGrepQ 170 ,checkTestArgumentSplitting 171 ,checkConcatenatedDollarAt 172 ,checkTildeInPath 173 ,checkReadWithoutR 174 ,checkLoopVariableReassignment 175 ,checkTrailingBracket 176 ,checkReturnAgainstZero 177 ,checkRedirectedNowhere 178 ,checkUnmatchableCases 179 ,checkSubshellAsTest 180 ,checkSplittingInArrays 181 ,checkRedirectionToNumber 182 ,checkGlobAsCommand 183 ,checkFlagAsCommand 184 ,checkEmptyCondition 185 ,checkPipeToNowhere 186 ,checkForLoopGlobVariables 187 ,checkSubshelledTests 188 ,checkInvertedStringTest 189 ,checkRedirectionToCommand 190 ,checkDollarQuoteParen 191 ,checkUselessBang 192 ,checkTranslatedStringVariable 193 ,checkModifiedArithmeticInRedirection 194 ,checkBlatantRecursion 195 ,checkBadTestAndOr 196 ,checkAssignToSelf 197 ,checkEqualsInCommand 198 ,checkSecondArgIsComparison 199 ,checkComparisonWithLeadingX 200 ,checkCommandWithTrailingSymbol 201 ,checkUnquotedParameterExpansionPattern 202 ] 203 204optionalChecks = map fst optionalTreeChecks 205 206 207prop_verifyOptionalExamples = all check optionalTreeChecks 208 where 209 check (desc, check) = 210 verifyTree check (cdPositive desc) 211 && verifyNotTree check (cdNegative desc) 212 213optionalTreeChecks :: [(CheckDescription, (Parameters -> Token -> [TokenComment]))] 214optionalTreeChecks = [ 215 (newCheckDescription { 216 cdName = "quote-safe-variables", 217 cdDescription = "Suggest quoting variables without metacharacters", 218 cdPositive = "var=hello; echo $var", 219 cdNegative = "var=hello; echo \"$var\"" 220 }, checkVerboseSpacefulness) 221 222 ,(newCheckDescription { 223 cdName = "avoid-nullary-conditions", 224 cdDescription = "Suggest explicitly using -n in `[ $var ]`", 225 cdPositive = "[ \"$var\" ]", 226 cdNegative = "[ -n \"$var\" ]" 227 }, nodeChecksToTreeCheck [checkNullaryExpansionTest]) 228 229 ,(newCheckDescription { 230 cdName = "add-default-case", 231 cdDescription = "Suggest adding a default case in `case` statements", 232 cdPositive = "case $? in 0) echo 'Success';; esac", 233 cdNegative = "case $? in 0) echo 'Success';; *) echo 'Fail' ;; esac" 234 }, nodeChecksToTreeCheck [checkDefaultCase]) 235 236 ,(newCheckDescription { 237 cdName = "require-variable-braces", 238 cdDescription = "Suggest putting braces around all variable references", 239 cdPositive = "var=hello; echo $var", 240 cdNegative = "var=hello; echo ${var}" 241 }, nodeChecksToTreeCheck [checkVariableBraces]) 242 243 ,(newCheckDescription { 244 cdName = "check-unassigned-uppercase", 245 cdDescription = "Warn when uppercase variables are unassigned", 246 cdPositive = "echo $VAR", 247 cdNegative = "VAR=hello; echo $VAR" 248 }, checkUnassignedReferences' True) 249 250 ,(newCheckDescription { 251 cdName = "require-double-brackets", 252 cdDescription = "Require [[ and warn about [ in Bash/Ksh", 253 cdPositive = "[ -e /etc/issue ]", 254 cdNegative = "[[ -e /etc/issue ]]" 255 }, checkRequireDoubleBracket) 256 257 ,(newCheckDescription { 258 cdName = "check-set-e-suppressed", 259 cdDescription = "Notify when set -e is suppressed during function invocation", 260 cdPositive = "set -e; func() { cp *.txt ~/backup; rm *.txt; }; func && echo ok", 261 cdNegative = "set -e; func() { cp *.txt ~/backup; rm *.txt; }; func; echo ok" 262 }, checkSetESuppressed) 263 264 ,(newCheckDescription { 265 cdName = "check-extra-masked-returns", 266 cdDescription = "Check for additional cases where exit codes are masked", 267 cdPositive = "rm -r \"$(get_chroot_dir)/home\"", 268 cdNegative = "set -e; dir=\"$(get_chroot_dir)\"; rm -r \"$dir/home\"" 269 }, checkExtraMaskedReturns) 270 ] 271 272optionalCheckMap :: Map.Map String (Parameters -> Token -> [TokenComment]) 273optionalCheckMap = Map.fromList $ map item optionalTreeChecks 274 where 275 item (desc, check) = (cdName desc, check) 276 277wouldHaveBeenGlob s = '*' `elem` s 278 279verify :: (Parameters -> Token -> Writer [TokenComment] ()) -> String -> Bool 280verify f s = checkNode f s == Just True 281 282verifyNot :: (Parameters -> Token -> Writer [TokenComment] ()) -> String -> Bool 283verifyNot f s = checkNode f s == Just False 284 285verifyTree :: (Parameters -> Token -> [TokenComment]) -> String -> Bool 286verifyTree f s = producesComments f s == Just True 287 288verifyNotTree :: (Parameters -> Token -> [TokenComment]) -> String -> Bool 289verifyNotTree f s = producesComments f s == Just False 290 291checkCommand str f t@(T_SimpleCommand id _ (cmd:rest)) 292 | t `isCommand` str = f cmd rest 293checkCommand _ _ _ = return () 294 295checkUnqualifiedCommand str f t@(T_SimpleCommand id _ (cmd:rest)) 296 | t `isUnqualifiedCommand` str = f cmd rest 297checkUnqualifiedCommand _ _ _ = return () 298 299verifyCodes :: (Parameters -> Token -> Writer [TokenComment] ()) -> [Code] -> String -> Bool 300verifyCodes f l s = codes == Just l 301 where 302 treeCheck = runNodeAnalysis f 303 comments = runAndGetComments treeCheck s 304 codes = map (cCode . tcComment) <$> comments 305 306checkNode f = producesComments (runNodeAnalysis f) 307producesComments :: (Parameters -> Token -> [TokenComment]) -> String -> Maybe Bool 308producesComments f s = not . null <$> runAndGetComments f s 309 310runAndGetComments f s = do 311 let pr = pScript s 312 prRoot pr 313 let spec = defaultSpec pr 314 let params = makeParameters spec 315 return $ 316 filterByAnnotation spec params $ 317 runList spec [f] 318 319-- Copied from https://wiki.haskell.org/Edit_distance 320dist :: Eq a => [a] -> [a] -> Int 321dist a b 322 = last (if lab == 0 then mainDiag 323 else if lab > 0 then lowers !! (lab - 1) 324 else{- < 0 -} uppers !! (-1 - lab)) 325 where mainDiag = oneDiag a b (head uppers) (-1 : head lowers) 326 uppers = eachDiag a b (mainDiag : uppers) -- upper diagonals 327 lowers = eachDiag b a (mainDiag : lowers) -- lower diagonals 328 eachDiag a [] diags = [] 329 eachDiag a (bch:bs) (lastDiag:diags) = oneDiag a bs nextDiag lastDiag : eachDiag a bs diags 330 where nextDiag = head (tail diags) 331 oneDiag a b diagAbove diagBelow = thisdiag 332 where doDiag [] b nw n w = [] 333 doDiag a [] nw n w = [] 334 doDiag (ach:as) (bch:bs) nw n w = me : doDiag as bs me (tail n) (tail w) 335 where me = if ach == bch then nw else 1 + min3 (head w) nw (head n) 336 firstelt = 1 + head diagBelow 337 thisdiag = firstelt : doDiag a b firstelt diagAbove (tail diagBelow) 338 lab = length a - length b 339 min3 x y z = if x < y then x else min y z 340 341hasFloatingPoint params = shellType params == Ksh 342 343-- Checks whether the current parent path is part of a condition 344isCondition [] = False 345isCondition [_] = False 346isCondition (child:parent:rest) = 347 case child of 348 T_BatsTest {} -> True -- count anything in a @test as conditional 349 _ -> getId child `elem` map getId (getConditionChildren parent) || isCondition (parent:rest) 350 where 351 getConditionChildren t = 352 case t of 353 T_AndIf _ left right -> [left] 354 T_OrIf id left right -> [left] 355 T_IfExpression id conditions elses -> concatMap (take 1 . reverse . fst) conditions 356 T_WhileExpression id c l -> take 1 . reverse $ c 357 T_UntilExpression id c l -> take 1 . reverse $ c 358 _ -> [] 359 360-- helpers to build replacements 361replaceStart id params n r = 362 let tp = tokenPositions params 363 (start, _) = tp Map.! id 364 new_end = start { 365 posColumn = posColumn start + n 366 } 367 depth = length $ getPath (parentMap params) (T_EOF id) 368 in 369 newReplacement { 370 repStartPos = start, 371 repEndPos = new_end, 372 repString = r, 373 repPrecedence = depth, 374 repInsertionPoint = InsertAfter 375 } 376replaceEnd id params n r = 377 let tp = tokenPositions params 378 (_, end) = tp Map.! id 379 new_start = end { 380 posColumn = posColumn end - n 381 } 382 new_end = end { 383 posColumn = posColumn end 384 } 385 depth = length $ getPath (parentMap params) (T_EOF id) 386 in 387 newReplacement { 388 repStartPos = new_start, 389 repEndPos = new_end, 390 repString = r, 391 repPrecedence = depth, 392 repInsertionPoint = InsertBefore 393 } 394replaceToken id params r = 395 let tp = tokenPositions params 396 (start, end) = tp Map.! id 397 depth = length $ getPath (parentMap params) (T_EOF id) 398 in 399 newReplacement { 400 repStartPos = start, 401 repEndPos = end, 402 repString = r, 403 repPrecedence = depth, 404 repInsertionPoint = InsertBefore 405 } 406 407surroundWith id params s = fixWith [replaceStart id params 0 s, replaceEnd id params 0 s] 408fixWith fixes = newFix { fixReplacements = fixes } 409 410analyse f t = execState (doAnalysis f t) [] 411 412-- Make a map from functions to definition IDs 413functions t = Map.fromList $ analyse findFunctions t 414findFunctions (T_Function id _ _ name _) 415 = modify ((name, id):) 416findFunctions _ = return () 417 418-- Make a map from aliases to definition IDs 419aliases t = Map.fromList $ analyse findAliases t 420findAliases t@(T_SimpleCommand _ _ (_:args)) 421 | t `isUnqualifiedCommand` "alias" = mapM_ getAlias args 422findAliases _ = return () 423getAlias arg = 424 let string = onlyLiteralString arg 425 in when ('=' `elem` string) $ 426 modify ((takeWhile (/= '=') string, getId arg):) 427 428prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)" 429checkEchoWc _ (T_Pipeline id _ [a, b]) = 430 when (acmd == ["echo", "${VAR}"]) $ 431 case bcmd of 432 ["wc", "-c"] -> countMsg 433 ["wc", "-m"] -> countMsg 434 _ -> return () 435 where 436 acmd = oversimplify a 437 bcmd = oversimplify b 438 countMsg = style id 2000 "See if you can use ${#variable} instead." 439checkEchoWc _ _ = return () 440 441prop_checkPipedAssignment1 = verify checkPipedAssignment "A=ls | grep foo" 442prop_checkPipedAssignment2 = verifyNot checkPipedAssignment "A=foo cmd | grep foo" 443prop_checkPipedAssignment3 = verifyNot checkPipedAssignment "A=foo" 444checkPipedAssignment _ (T_Pipeline _ _ (T_Redirecting _ _ (T_SimpleCommand id (_:_) []):_:_)) = 445 warn id 2036 "If you wanted to assign the output of the pipeline, use a=$(b | c) ." 446checkPipedAssignment _ _ = return () 447 448prop_checkAssignAteCommand1 = verify checkAssignAteCommand "A=ls -l" 449prop_checkAssignAteCommand2 = verify checkAssignAteCommand "A=ls --sort=$foo" 450prop_checkAssignAteCommand3 = verify checkAssignAteCommand "A=cat foo | grep bar" 451prop_checkAssignAteCommand4 = verifyNot checkAssignAteCommand "A=foo ls -l" 452prop_checkAssignAteCommand5 = verify checkAssignAteCommand "PAGER=cat grep bar" 453prop_checkAssignAteCommand6 = verifyNot checkAssignAteCommand "PAGER=\"cat\" grep bar" 454prop_checkAssignAteCommand7 = verify checkAssignAteCommand "here=pwd" 455checkAssignAteCommand _ (T_SimpleCommand id [T_Assignment _ _ _ _ assignmentTerm] list) = 456 -- Check if first word is intended as an argument (flag or glob). 457 if firstWordIsArg list 458 then 459 err id 2037 "To assign the output of a command, use var=$(cmd) ." 460 else 461 -- Check if it's a known, unquoted command name. 462 when (isCommonCommand $ getUnquotedLiteral assignmentTerm) $ 463 warn id 2209 "Use var=$(command) to assign output (or quote to assign string)." 464 where 465 isCommonCommand (Just s) = s `elem` commonCommands 466 isCommonCommand _ = False 467 firstWordIsArg list = fromMaybe False $ do 468 head <- list !!! 0 469 return $ isGlob head || isUnquotedFlag head 470 471checkAssignAteCommand _ _ = return () 472 473prop_checkArithmeticOpCommand1 = verify checkArithmeticOpCommand "i=i + 1" 474prop_checkArithmeticOpCommand2 = verify checkArithmeticOpCommand "foo=bar * 2" 475prop_checkArithmeticOpCommand3 = verifyNot checkArithmeticOpCommand "foo + opts" 476checkArithmeticOpCommand _ (T_SimpleCommand id [T_Assignment {}] (firstWord:_)) = 477 mapM_ check $ getGlobOrLiteralString firstWord 478 where 479 check op = 480 when (op `elem` ["+", "-", "*", "/"]) $ 481 warn (getId firstWord) 2099 $ 482 "Use $((..)) for arithmetics, e.g. i=$((i " ++ op ++ " 2))" 483checkArithmeticOpCommand _ _ = return () 484 485prop_checkWrongArit = verify checkWrongArithmeticAssignment "i=i+1" 486prop_checkWrongArit2 = verify checkWrongArithmeticAssignment "n=2; i=n*2" 487checkWrongArithmeticAssignment params (T_SimpleCommand id [T_Assignment _ _ _ _ val] []) = 488 sequence_ $ do 489 str <- getNormalString val 490 match <- matchRegex regex str 491 var <- match !!! 0 492 op <- match !!! 1 493 Map.lookup var references 494 return . warn (getId val) 2100 $ 495 "Use $((..)) for arithmetics, e.g. i=$((i " ++ op ++ " 2))" 496 where 497 regex = mkRegex "^([_a-zA-Z][_a-zA-Z0-9]*)([+*-]).+$" 498 references = foldl (flip ($)) Map.empty (map insertRef $ variableFlow params) 499 insertRef (Assignment (_, _, name, _)) = 500 Map.insert name () 501 insertRef _ = Prelude.id 502 503 getNormalString (T_NormalWord _ words) = do 504 parts <- mapM getLiterals words 505 return $ concat parts 506 getNormalString _ = Nothing 507 508 getLiterals (T_Literal _ s) = return s 509 getLiterals (T_Glob _ s) = return s 510 getLiterals _ = Nothing 511checkWrongArithmeticAssignment _ _ = return () 512 513 514prop_checkUuoc1 = verify checkUuoc "cat foo | grep bar" 515prop_checkUuoc2 = verifyNot checkUuoc "cat * | grep bar" 516prop_checkUuoc3 = verify checkUuoc "cat \"$var\" | grep bar" 517prop_checkUuoc3b = verifyNot checkUuoc "cat $var | grep bar" 518prop_checkUuoc3c = verifyNot checkUuoc "cat \"${!var}\" | grep bar" 519prop_checkUuoc4 = verifyNot checkUuoc "cat $var" 520prop_checkUuoc5 = verifyNot checkUuoc "cat \"$@\"" 521prop_checkUuoc6 = verifyNot checkUuoc "cat -n | grep bar" 522checkUuoc _ (T_Pipeline _ _ (T_Redirecting _ _ cmd:_:_)) = 523 checkCommand "cat" (const f) cmd 524 where 525 f [word] | not (mayBecomeMultipleArgs word || isOption word) = 526 style (getId word) 2002 "Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead." 527 f _ = return () 528 isOption word = "-" `isPrefixOf` onlyLiteralString word 529checkUuoc _ _ = return () 530 531prop_checkPipePitfalls3 = verify checkPipePitfalls "ls | grep -v mp3" 532prop_checkPipePitfalls4 = verifyNot checkPipePitfalls "find . -print0 | xargs -0 foo" 533prop_checkPipePitfalls5 = verifyNot checkPipePitfalls "ls -N | foo" 534prop_checkPipePitfalls6 = verify checkPipePitfalls "find . | xargs foo" 535prop_checkPipePitfalls7 = verifyNot checkPipePitfalls "find . -printf '%s\\n' | xargs foo" 536prop_checkPipePitfalls8 = verify checkPipePitfalls "foo | grep bar | wc -l" 537prop_checkPipePitfalls9 = verifyNot checkPipePitfalls "foo | grep -o bar | wc -l" 538prop_checkPipePitfalls10 = verifyNot checkPipePitfalls "foo | grep -o bar | wc" 539prop_checkPipePitfalls11 = verifyNot checkPipePitfalls "foo | grep bar | wc" 540prop_checkPipePitfalls12 = verifyNot checkPipePitfalls "foo | grep -o bar | wc -c" 541prop_checkPipePitfalls13 = verifyNot checkPipePitfalls "foo | grep bar | wc -c" 542prop_checkPipePitfalls14 = verifyNot checkPipePitfalls "foo | grep -o bar | wc -cmwL" 543prop_checkPipePitfalls15 = verifyNot checkPipePitfalls "foo | grep bar | wc -cmwL" 544prop_checkPipePitfalls16 = verifyNot checkPipePitfalls "foo | grep -r bar | wc -l" 545prop_checkPipePitfalls17 = verifyNot checkPipePitfalls "foo | grep -l bar | wc -l" 546prop_checkPipePitfalls18 = verifyNot checkPipePitfalls "foo | grep -L bar | wc -l" 547checkPipePitfalls _ (T_Pipeline id _ commands) = do 548 for ["find", "xargs"] $ 549 \(find:xargs:_) -> 550 let args = oversimplify xargs ++ oversimplify find 551 in 552 unless (any ($ args) [ 553 hasShortParameter '0', 554 hasParameter "null", 555 hasParameter "print0", 556 hasParameter "printf" 557 ]) $ warn (getId find) 2038 558 "Use -print0/-0 or -exec + to allow for non-alphanumeric filenames." 559 560 for' ["ps", "grep"] $ 561 \x -> info x 2009 "Consider using pgrep instead of grepping ps output." 562 563 for ["grep", "wc"] $ 564 \(grep:wc:_) -> 565 let flagsGrep = maybe [] (map snd . getAllFlags) $ getCommand grep 566 flagsWc = maybe [] (map snd . getAllFlags) $ getCommand wc 567 in 568 unless (any (`elem` ["l", "files-with-matches", "L", "files-without-matches", "o", "only-matching", "r", "R", "recursive"]) flagsGrep 569 || any (`elem` ["m", "chars", "w", "words", "c", "bytes", "L", "max-line-length"]) flagsWc 570 || null flagsWc) $ 571 style (getId grep) 2126 "Consider using grep -c instead of grep|wc -l." 572 573 didLs <- fmap or . sequence $ [ 574 for' ["ls", "grep"] $ 575 \x -> warn x 2010 "Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.", 576 for' ["ls", "xargs"] $ 577 \x -> warn x 2011 "Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames." 578 ] 579 unless didLs $ void $ 580 for ["ls", "?"] $ 581 \(ls:_) -> unless (hasShortParameter 'N' (oversimplify ls)) $ 582 info (getId ls) 2012 "Use find instead of ls to better handle non-alphanumeric filenames." 583 where 584 for l f = 585 let indices = indexOfSublists l (map (headOrDefault "" . oversimplify) commands) 586 in do 587 mapM_ (f . (\ n -> take (length l) $ drop n commands)) indices 588 return . not . null $ indices 589 for' l f = for l (first f) 590 first func (x:_) = func (getId $ getCommandTokenOrThis x) 591 first _ _ = return () 592 hasShortParameter char = any (\x -> "-" `isPrefixOf` x && char `elem` x) 593 hasParameter string = 594 any (isPrefixOf string . dropWhile (== '-')) 595checkPipePitfalls _ _ = return () 596 597indexOfSublists sub = f 0 598 where 599 f _ [] = [] 600 f n a@(r:rest) = 601 let others = f (n+1) rest in 602 if match sub a 603 then n:others 604 else others 605 match ("?":r1) (_:r2) = match r1 r2 606 match (x1:r1) (x2:r2) | x1 == x2 = match r1 r2 607 match [] _ = True 608 match _ _ = False 609 610 611prop_checkShebangParameters1 = verifyTree checkShebangParameters "#!/usr/bin/env bash -x\necho cow" 612prop_checkShebangParameters2 = verifyNotTree checkShebangParameters "#! /bin/sh -l " 613prop_checkShebangParameters3 = verifyNotTree checkShebangParameters "#!/usr/bin/env -S bash -x\necho cow" 614prop_checkShebangParameters4 = verifyNotTree checkShebangParameters "#!/usr/bin/env --split-string bash -x\necho cow" 615checkShebangParameters p (T_Annotation _ _ t) = checkShebangParameters p t 616checkShebangParameters _ (T_Script _ (T_Literal id sb) _) = 617 [makeComment ErrorC id 2096 "On most OS, shebangs can only specify a single parameter." | isMultiWord] 618 where 619 isMultiWord = length (words sb) > 2 && not (sb `matches` re) 620 re = mkRegex "env +(-S|--split-string)" 621 622prop_checkShebang1 = verifyNotTree checkShebang "#!/usr/bin/env bash -x\necho cow" 623prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh -l " 624prop_checkShebang3 = verifyTree checkShebang "ls -l" 625prop_checkShebang4 = verifyNotTree checkShebang "#shellcheck shell=sh\nfoo" 626prop_checkShebang5 = verifyTree checkShebang "#!/usr/bin/env ash" 627prop_checkShebang6 = verifyNotTree checkShebang "#!/usr/bin/env ash\n# shellcheck shell=dash\n" 628prop_checkShebang7 = verifyNotTree checkShebang "#!/usr/bin/env ash\n# shellcheck shell=sh\n" 629prop_checkShebang8 = verifyTree checkShebang "#!bin/sh\ntrue" 630prop_checkShebang9 = verifyNotTree checkShebang "# shellcheck shell=sh\ntrue" 631prop_checkShebang10= verifyNotTree checkShebang "#!foo\n# shellcheck shell=sh ignore=SC2239\ntrue" 632prop_checkShebang11= verifyTree checkShebang "#!/bin/sh/\ntrue" 633prop_checkShebang12= verifyTree checkShebang "#!/bin/sh/ -xe\ntrue" 634prop_checkShebang13= verifyTree checkShebang "#!/bin/busybox sh" 635prop_checkShebang14= verifyNotTree checkShebang "#!/bin/busybox sh\n# shellcheck shell=sh\n" 636prop_checkShebang15= verifyNotTree checkShebang "#!/bin/busybox sh\n# shellcheck shell=dash\n" 637prop_checkShebang16= verifyTree checkShebang "#!/bin/busybox ash" 638prop_checkShebang17= verifyNotTree checkShebang "#!/bin/busybox ash\n# shellcheck shell=dash\n" 639prop_checkShebang18= verifyNotTree checkShebang "#!/bin/busybox ash\n# shellcheck shell=sh\n" 640checkShebang params (T_Annotation _ list t) = 641 if any isOverride list then [] else checkShebang params t 642 where 643 isOverride (ShellOverride _) = True 644 isOverride _ = False 645checkShebang params (T_Script _ (T_Literal id sb) _) = execWriter $ do 646 unless (shellTypeSpecified params) $ do 647 when (null sb) $ 648 err id 2148 "Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive." 649 when (executableFromShebang sb == "ash") $ 650 warn id 2187 "Ash scripts will be checked as Dash. Add '# shellcheck shell=dash' to silence." 651 unless (null sb) $ do 652 unless ("/" `isPrefixOf` sb) $ 653 err id 2239 "Ensure the shebang uses an absolute path to the interpreter." 654 when ("/" `isSuffixOf` head (words sb)) $ 655 err id 2246 "This shebang specifies a directory. Ensure the interpreter is a file." 656 657 658prop_checkForInQuoted = verify checkForInQuoted "for f in \"$(ls)\"; do echo foo; done" 659prop_checkForInQuoted2 = verifyNot checkForInQuoted "for f in \"$@\"; do echo foo; done" 660prop_checkForInQuoted2a = verifyNot checkForInQuoted "for f in *.mp3; do echo foo; done" 661prop_checkForInQuoted2b = verify checkForInQuoted "for f in \"*.mp3\"; do echo foo; done" 662prop_checkForInQuoted3 = verify checkForInQuoted "for f in 'find /'; do true; done" 663prop_checkForInQuoted4 = verify checkForInQuoted "for f in 1,2,3; do true; done" 664prop_checkForInQuoted4a = verifyNot checkForInQuoted "for f in foo{1,2,3}; do true; done" 665prop_checkForInQuoted5 = verify checkForInQuoted "for f in ls; do true; done" 666prop_checkForInQuoted6 = verifyNot checkForInQuoted "for f in \"${!arr}\"; do true; done" 667prop_checkForInQuoted7 = verify checkForInQuoted "for f in ls, grep, mv; do true; done" 668prop_checkForInQuoted8 = verify checkForInQuoted "for f in 'ls', 'grep', 'mv'; do true; done" 669prop_checkForInQuoted9 = verifyNot checkForInQuoted "for f in 'ls,' 'grep,' 'mv'; do true; done" 670checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) 671 | any willSplit list && not (mayBecomeMultipleArgs word) 672 || maybe False wouldHaveBeenGlob (getLiteralString word) = 673 err id 2066 "Since you double quoted this, it will not word split, and the loop will only run once." 674checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_SingleQuoted id _]] _) = 675 warn id 2041 "This is a literal string. To run as a command, use $(..) instead of '..' . " 676checkForInQuoted _ (T_ForIn _ _ [single] _) 677 | maybe False (',' `elem`) $ getUnquotedLiteral single = 678 warn (getId single) 2042 "Use spaces, not commas, to separate loop elements." 679 | not (willSplit single || mayBecomeMultipleArgs single) = 680 warn (getId single) 2043 "This loop will only ever run once. Bad quoting or missing glob/expansion?" 681checkForInQuoted params (T_ForIn _ _ multiple _) = 682 forM_ multiple $ \arg -> sequence_ $ do 683 suffix <- getTrailingUnquotedLiteral arg 684 string <- getLiteralString suffix 685 guard $ "," `isSuffixOf` string 686 return $ 687 warnWithFix (getId arg) 2258 688 "The trailing comma is part of the value, not a separator. Delete or quote it." 689 (fixWith [replaceEnd (getId suffix) params 1 ""]) 690checkForInQuoted _ _ = return () 691 692prop_checkForInCat1 = verify checkForInCat "for f in $(cat foo); do stuff; done" 693prop_checkForInCat1a= verify checkForInCat "for f in `cat foo`; do stuff; done" 694prop_checkForInCat2 = verify checkForInCat "for f in $(cat foo | grep lol); do stuff; done" 695prop_checkForInCat2a= verify checkForInCat "for f in `cat foo | grep lol`; do stuff; done" 696prop_checkForInCat3 = verifyNot checkForInCat "for f in $(cat foo | grep bar | wc -l); do stuff; done" 697checkForInCat _ (T_ForIn _ f [T_NormalWord _ w] _) = mapM_ checkF w 698 where 699 checkF (T_DollarExpansion id [T_Pipeline _ _ r]) 700 | all isLineBased r = 701 info id 2013 "To read lines rather than words, pipe/redirect to a 'while read' loop." 702 checkF (T_Backticked id cmds) = checkF (T_DollarExpansion id cmds) 703 checkF _ = return () 704 isLineBased cmd = any (cmd `isCommand`) 705 ["grep", "fgrep", "egrep", "sed", "cat", "awk", "cut", "sort"] 706checkForInCat _ _ = return () 707 708prop_checkForInLs = verify checkForInLs "for f in $(ls *.mp3); do mplayer \"$f\"; done" 709prop_checkForInLs2 = verify checkForInLs "for f in `ls *.mp3`; do mplayer \"$f\"; done" 710prop_checkForInLs3 = verify checkForInLs "for f in `find / -name '*.mp3'`; do mplayer \"$f\"; done" 711checkForInLs _ = try 712 where 713 try (T_ForIn _ f [T_NormalWord _ [T_DollarExpansion id [x]]] _) = 714 check id f x 715 try (T_ForIn _ f [T_NormalWord _ [T_Backticked id [x]]] _) = 716 check id f x 717 try _ = return () 718 check id f x = 719 case oversimplify x of 720 ("ls":n) -> 721 let warntype = if any ("-" `isPrefixOf`) n then warn else err in 722 warntype id 2045 "Iterating over ls output is fragile. Use globs." 723 ("find":_) -> warn id 2044 "For loops over find output are fragile. Use find -exec or a while read loop." 724 _ -> return () 725 726 727prop_checkFindExec1 = verify checkFindExec "find / -name '*.php' -exec rm {};" 728prop_checkFindExec2 = verify checkFindExec "find / -exec touch {} && ls {} \\;" 729prop_checkFindExec3 = verify checkFindExec "find / -execdir cat {} | grep lol +" 730prop_checkFindExec4 = verifyNot checkFindExec "find / -name '*.php' -exec foo {} +" 731prop_checkFindExec5 = verifyNot checkFindExec "find / -execdir bash -c 'a && b' \\;" 732prop_checkFindExec6 = verify checkFindExec "find / -type d -execdir rm *.jpg \\;" 733checkFindExec _ cmd@(T_SimpleCommand _ _ t@(h:r)) | cmd `isCommand` "find" = do 734 c <- broken r False 735 when c $ 736 let wordId = getId $ last t in 737 err wordId 2067 "Missing ';' or + terminating -exec. You can't use |/||/&&, and ';' has to be a separate, quoted argument." 738 739 where 740 broken [] v = return v 741 broken (w:r) v = do 742 when v (mapM_ warnFor $ fromWord w) 743 case getLiteralString w of 744 Just "-exec" -> broken r True 745 Just "-execdir" -> broken r True 746 Just "+" -> broken r False 747 Just ";" -> broken r False 748 _ -> broken r v 749 750 shouldWarn x = 751 case x of 752 T_DollarExpansion _ _ -> True 753 T_Backticked _ _ -> True 754 T_Glob _ _ -> True 755 T_Extglob {} -> True 756 _ -> False 757 758 warnFor x = 759 when(shouldWarn x) $ 760 info (getId x) 2014 "This will expand once before find runs, not per file found." 761 762 fromWord (T_NormalWord _ l) = l 763 fromWord _ = [] 764checkFindExec _ _ = return () 765 766 767prop_checkUnquotedExpansions1 = verify checkUnquotedExpansions "rm $(ls)" 768prop_checkUnquotedExpansions1a= verify checkUnquotedExpansions "rm `ls`" 769prop_checkUnquotedExpansions2 = verify checkUnquotedExpansions "rm foo$(date)" 770prop_checkUnquotedExpansions3 = verify checkUnquotedExpansions "[ $(foo) == cow ]" 771prop_checkUnquotedExpansions3a= verify checkUnquotedExpansions "[ ! $(foo) ]" 772prop_checkUnquotedExpansions4 = verifyNot checkUnquotedExpansions "[[ $(foo) == cow ]]" 773prop_checkUnquotedExpansions5 = verifyNot checkUnquotedExpansions "for f in $(cmd); do echo $f; done" 774prop_checkUnquotedExpansions6 = verifyNot checkUnquotedExpansions "$(cmd)" 775prop_checkUnquotedExpansions7 = verifyNot checkUnquotedExpansions "cat << foo\n$(ls)\nfoo" 776prop_checkUnquotedExpansions8 = verifyNot checkUnquotedExpansions "set -- $(seq 1 4)" 777prop_checkUnquotedExpansions9 = verifyNot checkUnquotedExpansions "echo foo `# inline comment`" 778prop_checkUnquotedExpansions10 = verify checkUnquotedExpansions "#!/bin/sh\nexport var=$(val)" 779checkUnquotedExpansions params = 780 check 781 where 782 check t@(T_DollarExpansion _ c) = examine t c 783 check t@(T_Backticked _ c) = examine t c 784 check t@(T_DollarBraceCommandExpansion _ c) = examine t c 785 check _ = return () 786 tree = parentMap params 787 examine t contents = 788 unless (null contents || shouldBeSplit t || isQuoteFree (shellType params) tree t || usedAsCommandName tree t) $ 789 warn (getId t) 2046 "Quote this to prevent word splitting." 790 791 shouldBeSplit t = 792 getCommandNameFromExpansion t == Just "seq" 793 794 795prop_checkRedirectToSame = verify checkRedirectToSame "cat foo > foo" 796prop_checkRedirectToSame2 = verify checkRedirectToSame "cat lol | sed -e 's/a/b/g' > lol" 797prop_checkRedirectToSame3 = verifyNot checkRedirectToSame "cat lol | sed -e 's/a/b/g' > foo.bar && mv foo.bar lol" 798prop_checkRedirectToSame4 = verifyNot checkRedirectToSame "foo /dev/null > /dev/null" 799prop_checkRedirectToSame5 = verifyNot checkRedirectToSame "foo > bar 2> bar" 800prop_checkRedirectToSame6 = verifyNot checkRedirectToSame "echo foo > foo" 801prop_checkRedirectToSame7 = verifyNot checkRedirectToSame "sed 's/foo/bar/g' file | sponge file" 802prop_checkRedirectToSame8 = verifyNot checkRedirectToSame "while read -r line; do _=\"$fname\"; done <\"$fname\"" 803prop_checkRedirectToSame9 = verifyNot checkRedirectToSame "while read -r line; do cat < \"$fname\"; done <\"$fname\"" 804checkRedirectToSame params s@(T_Pipeline _ _ list) = 805 mapM_ (\l -> (mapM_ (\x -> doAnalysis (checkOccurrences x) l) (getAllRedirs list))) list 806 where 807 note x = makeComment InfoC x 2094 808 "Make sure not to read and write the same file in the same pipeline." 809 checkOccurrences t@(T_NormalWord exceptId x) u@(T_NormalWord newId y) | 810 exceptId /= newId 811 && x == y 812 && not (isInput t && isInput u) 813 && not (isOutput t && isOutput u) 814 && not (special t) 815 && not (any isHarmlessCommand [t,u]) 816 && not (any containsAssignment [u]) = do 817 addComment $ note newId 818 addComment $ note exceptId 819 checkOccurrences _ _ = return () 820 getAllRedirs = concatMap (\t -> 821 case t of 822 T_Redirecting _ ls _ -> concatMap getRedirs ls 823 _ -> []) 824 getRedirs (T_FdRedirect _ _ (T_IoFile _ op file)) = 825 case op of T_Greater _ -> [file] 826 T_Less _ -> [file] 827 T_DGREAT _ -> [file] 828 _ -> [] 829 getRedirs _ = [] 830 special x = "/dev/" `isPrefixOf` concat (oversimplify x) 831 isInput t = 832 case drop 1 $ getPath (parentMap params) t of 833 T_IoFile _ op _:_ -> 834 case op of 835 T_Less _ -> True 836 _ -> False 837 _ -> False 838 isOutput t = 839 case drop 1 $ getPath (parentMap params) t of 840 T_IoFile _ op _:_ -> 841 case op of 842 T_Greater _ -> True 843 T_DGREAT _ -> True 844 _ -> False 845 _ -> False 846 isHarmlessCommand arg = fromMaybe False $ do 847 cmd <- getClosestCommand (parentMap params) arg 848 name <- getCommandBasename cmd 849 return $ name `elem` ["echo", "printf", "sponge"] 850 containsAssignment arg = fromMaybe False $ do 851 cmd <- getClosestCommand (parentMap params) arg 852 return $ isAssignment cmd 853 854checkRedirectToSame _ _ = return () 855 856 857prop_checkShorthandIf = verify checkShorthandIf "[[ ! -z file ]] && scp file host || rm file" 858prop_checkShorthandIf2 = verifyNot checkShorthandIf "[[ ! -z file ]] && { scp file host || echo 'Eek'; }" 859prop_checkShorthandIf3 = verifyNot checkShorthandIf "foo && bar || echo baz" 860prop_checkShorthandIf4 = verifyNot checkShorthandIf "foo && a=b || a=c" 861prop_checkShorthandIf5 = verifyNot checkShorthandIf "foo && rm || printf b" 862prop_checkShorthandIf6 = verifyNot checkShorthandIf "if foo && bar || baz; then true; fi" 863prop_checkShorthandIf7 = verifyNot checkShorthandIf "while foo && bar || baz; do true; done" 864prop_checkShorthandIf8 = verify checkShorthandIf "if true; then foo && bar || baz; fi" 865checkShorthandIf params x@(T_AndIf id _ (T_OrIf _ _ (T_Pipeline _ _ t))) 866 | not (isOk t || inCondition) = 867 info id 2015 "Note that A && B || C is not if-then-else. C may run when A is true." 868 where 869 isOk [t] = isAssignment t || fromMaybe False (do 870 name <- getCommandBasename t 871 return $ name `elem` ["echo", "exit", "return", "printf"]) 872 isOk _ = False 873 inCondition = isCondition $ getPath (parentMap params) x 874checkShorthandIf _ _ = return () 875 876 877prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done" 878prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*" 879prop_checkDollarStar3 = verifyNot checkDollarStar "[[ $* = 'a b' ]]" 880prop_checkDollarStar4 = verify checkDollarStar "for f in ${var[*]}; do ..; done" 881prop_checkDollarStar5 = verify checkDollarStar "ls ${*//foo/bar}" 882prop_checkDollarStar6 = verify checkDollarStar "ls ${var[*]%%.*}" 883prop_checkDollarStar7 = verify checkDollarStar "ls ${*}" 884prop_checkDollarStar8 = verifyNot checkDollarStar "ls ${#*}" 885prop_checkDollarStar9 = verify checkDollarStar "ls ${arr[*]}" 886prop_checkDollarStar10 = verifyNot checkDollarStar "ls ${#arr[*]}" 887checkDollarStar p t@(T_NormalWord _ [T_DollarBraced id _ l]) 888 | not (isStrictlyQuoteFree (shellType p) (parentMap p) t) = do 889 let str = concat (oversimplify l) 890 when ("*" `isPrefixOf` str) $ 891 warn id 2048 "Use \"$@\" (with quotes) to prevent whitespace problems." 892 when ("[*]" `isPrefixOf` (getBracedModifier str) && isVariableChar (headOrDefault '!' str)) $ 893 warn id 2048 "Use \"${array[@]}\" (with quotes) to prevent whitespace problems." 894 895checkDollarStar _ _ = return () 896 897 898prop_checkUnquotedDollarAt = verify checkUnquotedDollarAt "ls $@" 899prop_checkUnquotedDollarAt1= verifyNot checkUnquotedDollarAt "ls ${#@}" 900prop_checkUnquotedDollarAt2 = verify checkUnquotedDollarAt "ls ${foo[@]}" 901prop_checkUnquotedDollarAt3 = verifyNot checkUnquotedDollarAt "ls ${#foo[@]}" 902prop_checkUnquotedDollarAt4 = verifyNot checkUnquotedDollarAt "ls \"$@\"" 903prop_checkUnquotedDollarAt5 = verifyNot checkUnquotedDollarAt "ls ${foo/@/ at }" 904prop_checkUnquotedDollarAt6 = verifyNot checkUnquotedDollarAt "a=$@" 905prop_checkUnquotedDollarAt7 = verify checkUnquotedDollarAt "for f in ${var[@]}; do true; done" 906prop_checkUnquotedDollarAt8 = verifyNot checkUnquotedDollarAt "echo \"${args[@]:+${args[@]}}\"" 907prop_checkUnquotedDollarAt9 = verifyNot checkUnquotedDollarAt "echo ${args[@]:+\"${args[@]}\"}" 908prop_checkUnquotedDollarAt10 = verifyNot checkUnquotedDollarAt "echo ${@+\"$@\"}" 909checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not $ isStrictlyQuoteFree (shellType p) (parentMap p) word = 910 forM_ (find isArrayExpansion parts) $ \x -> 911 unless (isQuotedAlternativeReference x) $ 912 err (getId x) 2068 913 "Double quote array expansions to avoid re-splitting elements." 914checkUnquotedDollarAt _ _ = return () 915 916prop_checkConcatenatedDollarAt1 = verify checkConcatenatedDollarAt "echo \"foo$@\"" 917prop_checkConcatenatedDollarAt2 = verify checkConcatenatedDollarAt "echo ${arr[@]}lol" 918prop_checkConcatenatedDollarAt3 = verify checkConcatenatedDollarAt "echo $a$@" 919prop_checkConcatenatedDollarAt4 = verifyNot checkConcatenatedDollarAt "echo $@" 920prop_checkConcatenatedDollarAt5 = verifyNot checkConcatenatedDollarAt "echo \"${arr[@]}\"" 921checkConcatenatedDollarAt p word@T_NormalWord {} 922 | not $ isQuoteFree (shellType p) (parentMap p) word 923 || null (drop 1 parts) = 924 mapM_ for array 925 where 926 parts = getWordParts word 927 array = find isArrayExpansion parts 928 for t = err (getId t) 2145 "Argument mixes string and array. Use * or separate argument." 929checkConcatenatedDollarAt _ _ = return () 930 931prop_checkArrayAsString1 = verify checkArrayAsString "a=$@" 932prop_checkArrayAsString2 = verify checkArrayAsString "a=\"${arr[@]}\"" 933prop_checkArrayAsString3 = verify checkArrayAsString "a=*.png" 934prop_checkArrayAsString4 = verify checkArrayAsString "a={1..10}" 935prop_checkArrayAsString5 = verifyNot checkArrayAsString "a='*.gif'" 936prop_checkArrayAsString6 = verifyNot checkArrayAsString "a=$*" 937prop_checkArrayAsString7 = verifyNot checkArrayAsString "a=( $@ )" 938checkArrayAsString _ (T_Assignment id _ _ _ word) = 939 if willConcatInAssignment word 940 then 941 warn (getId word) 2124 942 "Assigning an array to a string! Assign as array, or use * instead of @ to concatenate." 943 else 944 when (willBecomeMultipleArgs word) $ 945 warn (getId word) 2125 946 "Brace expansions and globs are literal in assignments. Quote it or use an array." 947checkArrayAsString _ _ = return () 948 949prop_checkArrayWithoutIndex1 = verifyTree checkArrayWithoutIndex "foo=(a b); echo $foo" 950prop_checkArrayWithoutIndex2 = verifyNotTree checkArrayWithoutIndex "foo='bar baz'; foo=($foo); echo ${foo[0]}" 951prop_checkArrayWithoutIndex3 = verifyTree checkArrayWithoutIndex "coproc foo while true; do echo cow; done; echo $foo" 952prop_checkArrayWithoutIndex4 = verifyTree checkArrayWithoutIndex "coproc tail -f log; echo $COPROC" 953prop_checkArrayWithoutIndex5 = verifyTree checkArrayWithoutIndex "a[0]=foo; echo $a" 954prop_checkArrayWithoutIndex6 = verifyTree checkArrayWithoutIndex "echo $PIPESTATUS" 955prop_checkArrayWithoutIndex7 = verifyTree checkArrayWithoutIndex "a=(a b); a+=c" 956prop_checkArrayWithoutIndex8 = verifyTree checkArrayWithoutIndex "declare -a foo; foo=bar;" 957prop_checkArrayWithoutIndex9 = verifyTree checkArrayWithoutIndex "read -r -a arr <<< 'foo bar'; echo \"$arr\"" 958prop_checkArrayWithoutIndex10 = verifyTree checkArrayWithoutIndex "read -ra arr <<< 'foo bar'; echo \"$arr\"" 959prop_checkArrayWithoutIndex11 = verifyNotTree checkArrayWithoutIndex "read -rpfoobar r; r=42" 960checkArrayWithoutIndex params _ = 961 doVariableFlowAnalysis readF writeF defaultMap (variableFlow params) 962 where 963 defaultMap = Map.fromList $ map (\x -> (x,())) arrayVariables 964 readF _ (T_DollarBraced id _ token) _ = do 965 map <- get 966 return . maybeToList $ do 967 name <- getLiteralString token 968 assigned <- Map.lookup name map 969 return $ makeComment WarningC id 2128 970 "Expanding an array without an index only gives the first element." 971 readF _ _ _ = return [] 972 973 writeF _ (T_Assignment id mode name [] _) _ (DataString _) = do 974 isArray <- gets (Map.member name) 975 return $ if not isArray then [] else 976 case mode of 977 Assign -> [makeComment WarningC id 2178 "Variable was used as an array but is now assigned a string."] 978 Append -> [makeComment WarningC id 2179 "Use array+=(\"item\") to append items to an array."] 979 980 writeF _ t name (DataArray _) = do 981 modify (Map.insert name ()) 982 return [] 983 writeF _ expr name _ = do 984 if isIndexed expr 985 then modify (Map.insert name ()) 986 else modify (Map.delete name) 987 return [] 988 989 isIndexed expr = 990 case expr of 991 T_Assignment _ _ _ (_:_) _ -> True 992 _ -> False 993 994prop_checkStderrRedirect = verify checkStderrRedirect "test 2>&1 > cow" 995prop_checkStderrRedirect2 = verifyNot checkStderrRedirect "test > cow 2>&1" 996prop_checkStderrRedirect3 = verifyNot checkStderrRedirect "test 2>&1 > file | grep stderr" 997prop_checkStderrRedirect4 = verifyNot checkStderrRedirect "errors=$(test 2>&1 > file)" 998prop_checkStderrRedirect5 = verifyNot checkStderrRedirect "read < <(test 2>&1 > file)" 999prop_checkStderrRedirect6 = verify checkStderrRedirect "foo | bar 2>&1 > /dev/null" 1000prop_checkStderrRedirect7 = verifyNot checkStderrRedirect "{ cmd > file; } 2>&1" 1001checkStderrRedirect params redir@(T_Redirecting _ [ 1002 T_FdRedirect id "2" (T_IoDuplicate _ (T_GREATAND _) "1"), 1003 T_FdRedirect _ _ (T_IoFile _ op _) 1004 ] _) = case op of 1005 T_Greater _ -> error 1006 T_DGREAT _ -> error 1007 _ -> return () 1008 where 1009 usesOutput t = 1010 case t of 1011 (T_Pipeline _ _ list) -> length list > 1 && not (isParentOf (parentMap params) (last list) redir) 1012 T_ProcSub {} -> True 1013 T_DollarExpansion {} -> True 1014 T_Backticked {} -> True 1015 _ -> False 1016 isCaptured = any usesOutput $ getPath (parentMap params) redir 1017 1018 error = unless isCaptured $ 1019 warn id 2069 "To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify)." 1020 1021checkStderrRedirect _ _ = return () 1022 1023lt x = trace ("Tracing " ++ show x) x -- STRIP 1024ltt t = trace ("Tracing " ++ show t) -- STRIP 1025 1026 1027prop_checkSingleQuotedVariables = verify checkSingleQuotedVariables "echo '$foo'" 1028prop_checkSingleQuotedVariables2 = verify checkSingleQuotedVariables "echo 'lol$1.jpg'" 1029prop_checkSingleQuotedVariables3 = verifyNot checkSingleQuotedVariables "sed 's/foo$/bar/'" 1030prop_checkSingleQuotedVariables3a= verify checkSingleQuotedVariables "sed 's/${foo}/bar/'" 1031prop_checkSingleQuotedVariables3b= verify checkSingleQuotedVariables "sed 's/$(echo cow)/bar/'" 1032prop_checkSingleQuotedVariables3c= verify checkSingleQuotedVariables "sed 's/$((1+foo))/bar/'" 1033prop_checkSingleQuotedVariables4 = verifyNot checkSingleQuotedVariables "awk '{print $1}'" 1034prop_checkSingleQuotedVariables5 = verifyNot checkSingleQuotedVariables "trap 'echo $SECONDS' EXIT" 1035prop_checkSingleQuotedVariables6 = verifyNot checkSingleQuotedVariables "sed -n '$p'" 1036prop_checkSingleQuotedVariables6a= verify checkSingleQuotedVariables "sed -n '$pattern'" 1037prop_checkSingleQuotedVariables7 = verifyNot checkSingleQuotedVariables "PS1='$PWD \\$ '" 1038prop_checkSingleQuotedVariables8 = verify checkSingleQuotedVariables "find . -exec echo '$1' {} +" 1039prop_checkSingleQuotedVariables9 = verifyNot checkSingleQuotedVariables "find . -exec awk '{print $1}' {} \\;" 1040prop_checkSingleQuotedVariables10= verify checkSingleQuotedVariables "echo '`pwd`'" 1041prop_checkSingleQuotedVariables11= verifyNot checkSingleQuotedVariables "sed '${/lol/d}'" 1042prop_checkSingleQuotedVariables12= verifyNot checkSingleQuotedVariables "eval 'echo $1'" 1043prop_checkSingleQuotedVariables13= verifyNot checkSingleQuotedVariables "busybox awk '{print $1}'" 1044prop_checkSingleQuotedVariables14= verifyNot checkSingleQuotedVariables "[ -v 'bar[$foo]' ]" 1045prop_checkSingleQuotedVariables15= verifyNot checkSingleQuotedVariables "git filter-branch 'test $GIT_COMMIT'" 1046prop_checkSingleQuotedVariables16= verify checkSingleQuotedVariables "git '$a'" 1047prop_checkSingleQuotedVariables17= verifyNot checkSingleQuotedVariables "rename 's/(.)a/$1/g' *" 1048prop_checkSingleQuotedVariables18= verifyNot checkSingleQuotedVariables "echo '``'" 1049prop_checkSingleQuotedVariables19= verifyNot checkSingleQuotedVariables "echo '```'" 1050prop_checkSingleQuotedVariables20= verifyNot checkSingleQuotedVariables "mumps -run %XCMD 'W $O(^GLOBAL(5))'" 1051prop_checkSingleQuotedVariables21= verifyNot checkSingleQuotedVariables "mumps -run LOOP%XCMD --xec 'W $O(^GLOBAL(6))'" 1052prop_checkSingleQuotedVariables22= verifyNot checkSingleQuotedVariables "jq '$__loc__'" 1053prop_checkSingleQuotedVariables23= verifyNot checkSingleQuotedVariables "command jq '$__loc__'" 1054prop_checkSingleQuotedVariables24= verifyNot checkSingleQuotedVariables "exec jq '$__loc__'" 1055prop_checkSingleQuotedVariables25= verifyNot checkSingleQuotedVariables "exec -c -a foo jq '$__loc__'" 1056 1057 1058checkSingleQuotedVariables params t@(T_SingleQuoted id s) = 1059 when (s `matches` re) $ 1060 if "sed" == commandName 1061 then unless (s `matches` sedContra) showMessage 1062 else unless isProbablyOk showMessage 1063 where 1064 parents = parentMap params 1065 showMessage = info id 2016 1066 "Expressions don't expand in single quotes, use double quotes for that." 1067 commandName = fromMaybe "" $ do 1068 cmd <- getClosestCommand parents t 1069 name <- getCommandBasename cmd 1070 return $ if name == "find" then getFindCommand cmd else if name == "git" then getGitCommand cmd else if name == "mumps" then getMumpsCommand cmd else name 1071 1072 isProbablyOk = 1073 any isOkAssignment (take 3 $ getPath parents t) 1074 || commandName `elem` [ 1075 "trap" 1076 ,"sh" 1077 ,"bash" 1078 ,"ksh" 1079 ,"zsh" 1080 ,"ssh" 1081 ,"eval" 1082 ,"xprop" 1083 ,"alias" 1084 ,"sudo" -- covering "sudo sh" and such 1085 ,"docker" -- like above 1086 ,"podman" 1087 ,"dpkg-query" 1088 ,"jq" -- could also check that user provides --arg 1089 ,"rename" 1090 ,"rg" 1091 ,"unset" 1092 ,"git filter-branch" 1093 ,"mumps -run %XCMD" 1094 ,"mumps -run LOOP%XCMD" 1095 ] 1096 || "awk" `isSuffixOf` commandName 1097 || "perl" `isPrefixOf` commandName 1098 1099 commonlyQuoted = ["PS1", "PS2", "PS3", "PS4", "PROMPT_COMMAND"] 1100 isOkAssignment t = 1101 case t of 1102 T_Assignment _ _ name _ _ -> name `elem` commonlyQuoted 1103 TC_Unary _ _ "-v" _ -> True 1104 _ -> False 1105 1106 re = mkRegex "\\$[{(0-9a-zA-Z_]|`[^`]+`" 1107 sedContra = mkRegex "\\$[{dpsaic]($|[^a-zA-Z])" 1108 1109 getFindCommand (T_SimpleCommand _ _ words) = 1110 let list = map getLiteralString words 1111 cmd = dropWhile (\x -> x /= Just "-exec" && x /= Just "-execdir") list 1112 in 1113 case cmd of 1114 (flag:cmd:rest) -> fromMaybe "find" cmd 1115 _ -> "find" 1116 getFindCommand (T_Redirecting _ _ cmd) = getFindCommand cmd 1117 getFindCommand _ = "find" 1118 getGitCommand (T_SimpleCommand _ _ words) = 1119 case map getLiteralString words of 1120 Just "git":Just "filter-branch":_ -> "git filter-branch" 1121 _ -> "git" 1122 getGitCommand (T_Redirecting _ _ cmd) = getGitCommand cmd 1123 getGitCommand _ = "git" 1124 getMumpsCommand (T_SimpleCommand _ _ words) = 1125 case map getLiteralString words of 1126 Just "mumps":Just "-run":Just "%XCMD":_ -> "mumps -run %XCMD" 1127 Just "mumps":Just "-run":Just "LOOP%XCMD":_ -> "mumps -run LOOP%XCMD" 1128 _ -> "mumps" 1129 getMumpsCommand (T_Redirecting _ _ cmd) = getMumpsCommand cmd 1130 getMumpsCommand _ = "mumps" 1131checkSingleQuotedVariables _ _ = return () 1132 1133 1134prop_checkUnquotedN = verify checkUnquotedN "if [ -n $foo ]; then echo cow; fi" 1135prop_checkUnquotedN2 = verify checkUnquotedN "[ -n $cow ]" 1136prop_checkUnquotedN3 = verifyNot checkUnquotedN "[[ -n $foo ]] && echo cow" 1137prop_checkUnquotedN4 = verify checkUnquotedN "[ -n $cow -o -t 1 ]" 1138prop_checkUnquotedN5 = verifyNot checkUnquotedN "[ -n \"$@\" ]" 1139checkUnquotedN _ (TC_Unary _ SingleBracket "-n" t) | willSplit t = 1140 unless (any isArrayExpansion $ getWordParts t) $ -- There's SC2198 for these 1141 err (getId t) 2070 "-n doesn't work with unquoted arguments. Quote or use [[ ]]." 1142checkUnquotedN _ _ = return () 1143 1144prop_checkNumberComparisons1 = verify checkNumberComparisons "[[ $foo < 3 ]]" 1145prop_checkNumberComparisons2 = verify checkNumberComparisons "[[ 0 >= $(cmd) ]]" 1146prop_checkNumberComparisons3 = verifyNot checkNumberComparisons "[[ $foo ]] > 3" 1147prop_checkNumberComparisons4 = verify checkNumberComparisons "[[ $foo > 2.72 ]]" 1148prop_checkNumberComparisons5 = verify checkNumberComparisons "[[ $foo -le 2.72 ]]" 1149prop_checkNumberComparisons6 = verify checkNumberComparisons "[[ 3.14 -eq $foo ]]" 1150prop_checkNumberComparisons7 = verifyNot checkNumberComparisons "[[ 3.14 == $foo ]]" 1151prop_checkNumberComparisons8 = verify checkNumberComparisons "[ foo <= bar ]" 1152prop_checkNumberComparisons9 = verify checkNumberComparisons "[ foo \\>= bar ]" 1153prop_checkNumberComparisons11 = verify checkNumberComparisons "[ $foo -eq 'N' ]" 1154prop_checkNumberComparisons12 = verify checkNumberComparisons "[ x$foo -gt x${N} ]" 1155prop_checkNumberComparisons13 = verify checkNumberComparisons "[ $foo > $bar ]" 1156prop_checkNumberComparisons14 = verifyNot checkNumberComparisons "[[ foo < bar ]]" 1157prop_checkNumberComparisons15 = verifyNot checkNumberComparisons "[ $foo '>' $bar ]" 1158prop_checkNumberComparisons16 = verify checkNumberComparisons "[ foo -eq 'y' ]" 1159prop_checkNumberComparisons17 = verify checkNumberComparisons "[[ 'foo' -eq 2 ]]" 1160prop_checkNumberComparisons18 = verify checkNumberComparisons "[[ foo -eq 2 ]]" 1161prop_checkNumberComparisons19 = verifyNot checkNumberComparisons "foo=1; [[ foo -eq 2 ]]" 1162prop_checkNumberComparisons20 = verify checkNumberComparisons "[[ 2 -eq / ]]" 1163prop_checkNumberComparisons21 = verify checkNumberComparisons "[[ foo -eq foo ]]" 1164 1165checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do 1166 if isNum lhs || isNum rhs 1167 then do 1168 when (isLtGt op) $ 1169 err id 2071 $ 1170 op ++ " is for string comparisons. Use " ++ eqv op ++ " instead." 1171 when (isLeGe op && hasStringComparison) $ 1172 err id 2071 $ op ++ " is not a valid operator. " ++ 1173 "Use " ++ eqv op ++ " ." 1174 else do 1175 when (isLeGe op || isLtGt op) $ 1176 mapM_ checkDecimals [lhs, rhs] 1177 1178 when (isLeGe op && hasStringComparison) $ 1179 err id 2122 $ op ++ " is not a valid operator. " ++ 1180 "Use '! a " ++ esc ++ invert op ++ " b' instead." 1181 1182 when (typ == SingleBracket && op `elem` ["<", ">"]) $ 1183 case shellType params of 1184 Sh -> return () -- These are unsupported and will be caught by bashism checks. 1185 Dash -> err id 2073 $ "Escape \\" ++ op ++ " to prevent it redirecting." 1186 _ -> err id 2073 $ "Escape \\" ++ op ++ " to prevent it redirecting (or switch to [[ .. ]])." 1187 1188 when (op `elem` arithmeticBinaryTestOps) $ do 1189 mapM_ checkDecimals [lhs, rhs] 1190 mapM_ checkString [lhs, rhs] 1191 1192 1193 where 1194 hasStringComparison = shellType params /= Sh 1195 isLtGt = flip elem ["<", "\\<", ">", "\\>"] 1196 isLeGe = flip elem ["<=", "\\<=", ">=", "\\>="] 1197 1198 checkDecimals hs = 1199 when (isFraction hs && not (hasFloatingPoint params)) $ 1200 err (getId hs) 2072 decimalError 1201 decimalError = "Decimals are not supported. " ++ 1202 "Either use integers only, or use bc or awk to compare." 1203 1204 checkString t = 1205 let 1206 asString = getLiteralStringDef "\0" t 1207 isVar = isVariableName asString 1208 kind = if isVar then "a variable" else "an arithmetic expression" 1209 fix = if isVar then "$var" else "$((expr))" 1210 in 1211 when (isNonNum t) $ 1212 if typ == SingleBracket 1213 then 1214 err (getId t) 2170 $ 1215 "Invalid number for " ++ op ++ ". Use " ++ seqv op ++ 1216 " to compare as string (or use " ++ fix ++ 1217 " to expand as " ++ kind ++ ")." 1218 else 1219 -- We should warn if any of the following holds: 1220 -- The string is not a variable name 1221 -- Any part of it is quoted 1222 -- It's not a recognized variable name 1223 when (not isVar || any isQuotes (getWordParts t) || asString `notElem` assignedVariables) $ 1224 warn (getId t) 2309 $ 1225 op ++ " treats this as " ++ kind ++ ". " ++ 1226 "Use " ++ seqv op ++ " to compare as string (or expand explicitly with " ++ fix ++ ")." 1227 1228 assignedVariables :: [String] 1229 assignedVariables = mapMaybe f (variableFlow params) 1230 where 1231 f t = do 1232 Assignment (_, _, name, _) <- return t 1233 return name 1234 1235 isNonNum t = not . all numChar $ onlyLiteralString t 1236 numChar x = isDigit x || x `elem` "+-. " 1237 1238 isNum t = 1239 case oversimplify t of 1240 [v] -> all isDigit v 1241 _ -> False 1242 1243 isFraction t = 1244 case oversimplify t of 1245 [v] -> isJust $ matchRegex floatRegex v 1246 _ -> False 1247 1248 eqv ('\\':s) = eqv s 1249 eqv "<" = "-lt" 1250 eqv ">" = "-gt" 1251 eqv "<=" = "-le" 1252 eqv ">=" = "-ge" 1253 eqv _ = "the numerical equivalent" 1254 1255 esc = if typ == SingleBracket then "\\" else "" 1256 seqv "-ge" = "! a " ++ esc ++ "< b" 1257 seqv "-gt" = esc ++ ">" 1258 seqv "-le" = "! a " ++ esc ++ "> b" 1259 seqv "-lt" = esc ++ "<" 1260 seqv "-eq" = "=" 1261 seqv "-ne" = "!=" 1262 seqv _ = "the string equivalent" 1263 1264 invert ('\\':s) = invert s 1265 invert "<=" = ">" 1266 invert ">=" = "<" 1267 1268 floatRegex = mkRegex "^[-+]?[0-9]+\\.[0-9]+$" 1269checkNumberComparisons _ _ = return () 1270 1271prop_checkSingleBracketOperators1 = verify checkSingleBracketOperators "[ test =~ foo ]" 1272checkSingleBracketOperators params (TC_Binary id SingleBracket "=~" lhs rhs) 1273 | shellType params `elem` [Bash, Ksh] = 1274 err id 2074 $ "Can't use =~ in [ ]. Use [[..]] instead." 1275checkSingleBracketOperators _ _ = return () 1276 1277prop_checkDoubleBracketOperators1 = verify checkDoubleBracketOperators "[[ 3 \\< 4 ]]" 1278prop_checkDoubleBracketOperators3 = verifyNot checkDoubleBracketOperators "[[ foo < bar ]]" 1279checkDoubleBracketOperators _ x@(TC_Binary id typ op lhs rhs) 1280 | typ == DoubleBracket && op `elem` ["\\<", "\\>"] = 1281 err id 2075 $ "Escaping " ++ op ++" is required in [..], but invalid in [[..]]" 1282checkDoubleBracketOperators _ _ = return () 1283 1284prop_checkConditionalAndOrs1 = verify checkConditionalAndOrs "[ foo && bar ]" 1285prop_checkConditionalAndOrs2 = verify checkConditionalAndOrs "[[ foo -o bar ]]" 1286prop_checkConditionalAndOrs3 = verifyNot checkConditionalAndOrs "[[ foo || bar ]]" 1287prop_checkConditionalAndOrs4 = verify checkConditionalAndOrs "[ foo -a bar ]" 1288prop_checkConditionalAndOrs5 = verify checkConditionalAndOrs "[ -z 3 -o a = b ]" 1289checkConditionalAndOrs _ t = 1290 case t of 1291 (TC_And id SingleBracket "&&" _ _) -> 1292 err id 2107 "Instead of [ a && b ], use [ a ] && [ b ]." 1293 (TC_And id DoubleBracket "-a" _ _) -> 1294 err id 2108 "In [[..]], use && instead of -a." 1295 (TC_Or id SingleBracket "||" _ _) -> 1296 err id 2109 "Instead of [ a || b ], use [ a ] || [ b ]." 1297 (TC_Or id DoubleBracket "-o" _ _) -> 1298 err id 2110 "In [[..]], use || instead of -o." 1299 1300 (TC_And id SingleBracket "-a" _ _) -> 1301 warn id 2166 "Prefer [ p ] && [ q ] as [ p -a q ] is not well defined." 1302 (TC_Or id SingleBracket "-o" _ _) -> 1303 warn id 2166 "Prefer [ p ] || [ q ] as [ p -o q ] is not well defined." 1304 1305 _ -> return () 1306 1307prop_checkQuotedCondRegex1 = verify checkQuotedCondRegex "[[ $foo =~ \"bar.*\" ]]" 1308prop_checkQuotedCondRegex2 = verify checkQuotedCondRegex "[[ $foo =~ '(cow|bar)' ]]" 1309prop_checkQuotedCondRegex3 = verifyNot checkQuotedCondRegex "[[ $foo =~ $foo ]]" 1310prop_checkQuotedCondRegex4 = verifyNot checkQuotedCondRegex "[[ $foo =~ \"bar\" ]]" 1311prop_checkQuotedCondRegex5 = verifyNot checkQuotedCondRegex "[[ $foo =~ 'cow bar' ]]" 1312prop_checkQuotedCondRegex6 = verify checkQuotedCondRegex "[[ $foo =~ 'cow|bar' ]]" 1313checkQuotedCondRegex _ (TC_Binary _ _ "=~" _ rhs) = 1314 case rhs of 1315 T_NormalWord id [T_DoubleQuoted _ _] -> error rhs 1316 T_NormalWord id [T_SingleQuoted _ _] -> error rhs 1317 _ -> return () 1318 where 1319 error t = 1320 unless (isConstantNonRe t) $ 1321 warn (getId t) 2076 1322 "Remove quotes from right-hand side of =~ to match as a regex rather than literally." 1323 re = mkRegex "[][*.+()|]" 1324 hasMetachars s = s `matches` re 1325 isConstantNonRe t = fromMaybe False $ do 1326 s <- getLiteralString t 1327 return . not $ hasMetachars s 1328checkQuotedCondRegex _ _ = return () 1329 1330prop_checkGlobbedRegex1 = verify checkGlobbedRegex "[[ $foo =~ *foo* ]]" 1331prop_checkGlobbedRegex2 = verify checkGlobbedRegex "[[ $foo =~ f* ]]" 1332prop_checkGlobbedRegex3 = verifyNot checkGlobbedRegex "[[ $foo =~ $foo ]]" 1333prop_checkGlobbedRegex4 = verifyNot checkGlobbedRegex "[[ $foo =~ ^c.* ]]" 1334prop_checkGlobbedRegex5 = verifyNot checkGlobbedRegex "[[ $foo =~ \\* ]]" 1335prop_checkGlobbedRegex6 = verifyNot checkGlobbedRegex "[[ $foo =~ (o*) ]]" 1336prop_checkGlobbedRegex7 = verifyNot checkGlobbedRegex "[[ $foo =~ \\*foo ]]" 1337prop_checkGlobbedRegex8 = verifyNot checkGlobbedRegex "[[ $foo =~ x\\* ]]" 1338checkGlobbedRegex _ (TC_Binary _ DoubleBracket "=~" _ rhs) 1339 | isConfusedGlobRegex s = 1340 warn (getId rhs) 2049 "=~ is for regex, but this looks like a glob. Use = instead." 1341 where s = concat $ oversimplify rhs 1342checkGlobbedRegex _ _ = return () 1343 1344 1345prop_checkConstantIfs1 = verify checkConstantIfs "[[ foo != bar ]]" 1346prop_checkConstantIfs2a= verify checkConstantIfs "[ n -le 4 ]" 1347prop_checkConstantIfs2b= verifyNot checkConstantIfs "[[ n -le 4 ]]" 1348prop_checkConstantIfs3 = verify checkConstantIfs "[[ $n -le 4 && n != 2 ]]" 1349prop_checkConstantIfs4 = verifyNot checkConstantIfs "[[ $n -le 3 ]]" 1350prop_checkConstantIfs5 = verifyNot checkConstantIfs "[[ $n -le $n ]]" 1351prop_checkConstantIfs6 = verifyNot checkConstantIfs "[[ a -ot b ]]" 1352prop_checkConstantIfs7 = verifyNot checkConstantIfs "[ a -nt b ]" 1353prop_checkConstantIfs8 = verifyNot checkConstantIfs "[[ ~foo == '~foo' ]]" 1354prop_checkConstantIfs9 = verify checkConstantIfs "[[ *.png == [a-z] ]]" 1355prop_checkConstantIfs10 = verifyNot checkConstantIfs "[[ ~me == ~+ ]]" 1356prop_checkConstantIfs11 = verifyNot checkConstantIfs "[[ ~ == ~+ ]]" 1357prop_checkConstantIfs12 = verify checkConstantIfs "[[ '~' == x ]]" 1358checkConstantIfs _ (TC_Binary id typ op lhs rhs) | not isDynamic = 1359 if isConstant lhs && isConstant rhs 1360 then warn id 2050 "This expression is constant. Did you forget the $ on a variable?" 1361 else checkUnmatchable id op lhs rhs 1362 where 1363 isDynamic = 1364 op `elem` arithmeticBinaryTestOps 1365 && typ == DoubleBracket 1366 || op `elem` [ "-nt", "-ot", "-ef"] 1367 1368 checkUnmatchable id op lhs rhs = 1369 when (op `elem` ["=", "==", "!="] && not (wordsCanBeEqual lhs rhs)) $ 1370 warn id 2193 "The arguments to this comparison can never be equal. Make sure your syntax is correct." 1371checkConstantIfs _ _ = return () 1372 1373prop_checkLiteralBreakingTest = verify checkLiteralBreakingTest "[[ a==$foo ]]" 1374prop_checkLiteralBreakingTest2 = verify checkLiteralBreakingTest "[ $foo=3 ]" 1375prop_checkLiteralBreakingTest3 = verify checkLiteralBreakingTest "[ $foo!=3 ]" 1376prop_checkLiteralBreakingTest4 = verify checkLiteralBreakingTest "[ \"$(ls) \" ]" 1377prop_checkLiteralBreakingTest5 = verify checkLiteralBreakingTest "[ -n \"$(true) \" ]" 1378prop_checkLiteralBreakingTest6 = verify checkLiteralBreakingTest "[ -z $(true)z ]" 1379prop_checkLiteralBreakingTest7 = verifyNot checkLiteralBreakingTest "[ -z $(true) ]" 1380prop_checkLiteralBreakingTest8 = verifyNot checkLiteralBreakingTest "[ $(true)$(true) ]" 1381prop_checkLiteralBreakingTest10 = verify checkLiteralBreakingTest "[ -z foo ]" 1382checkLiteralBreakingTest _ t = sequence_ $ 1383 case t of 1384 (TC_Nullary _ _ w@(T_NormalWord _ l)) -> do 1385 guard . not $ isConstant w -- Covered by SC2078 1386 comparisonWarning l `mplus` tautologyWarning w "Argument to implicit -n is always true due to literal strings." 1387 (TC_Unary _ _ op w@(T_NormalWord _ l)) -> 1388 case op of 1389 "-n" -> tautologyWarning w "Argument to -n is always true due to literal strings." 1390 "-z" -> tautologyWarning w "Argument to -z is always false due to literal strings." 1391 _ -> fail "not relevant" 1392 _ -> fail "not my problem" 1393 where 1394 hasEquals = matchToken ('=' `elem`) 1395 isNonEmpty = matchToken (not . null) 1396 matchToken m t = maybe False m (getLiteralString t) 1397 1398 comparisonWarning list = do 1399 token <- find hasEquals list 1400 return $ err (getId token) 2077 "You need spaces around the comparison operator." 1401 tautologyWarning t s = do 1402 token <- find isNonEmpty $ getWordParts t 1403 return $ err (getId token) 2157 s 1404 1405prop_checkConstantNullary = verify checkConstantNullary "[[ '$(foo)' ]]" 1406prop_checkConstantNullary2 = verify checkConstantNullary "[ \"-f lol\" ]" 1407prop_checkConstantNullary3 = verify checkConstantNullary "[[ cmd ]]" 1408prop_checkConstantNullary4 = verify checkConstantNullary "[[ ! cmd ]]" 1409prop_checkConstantNullary5 = verify checkConstantNullary "[[ true ]]" 1410prop_checkConstantNullary6 = verify checkConstantNullary "[ 1 ]" 1411prop_checkConstantNullary7 = verify checkConstantNullary "[ false ]" 1412checkConstantNullary _ (TC_Nullary _ _ t) | isConstant t = 1413 case fromMaybe "" $ getLiteralString t of 1414 "false" -> err (getId t) 2158 "[ false ] is true. Remove the brackets." 1415 "0" -> err (getId t) 2159 "[ 0 ] is true. Use 'false' instead." 1416 "true" -> style (getId t) 2160 "Instead of '[ true ]', just use 'true'." 1417 "1" -> style (getId t) 2161 "Instead of '[ 1 ]', use 'true'." 1418 _ -> err (getId t) 2078 "This expression is constant. Did you forget a $ somewhere?" 1419 where 1420 string = fromMaybe "" $ getLiteralString t 1421 1422checkConstantNullary _ _ = return () 1423 1424prop_checkForDecimals1 = verify checkForDecimals "((3.14*c))" 1425prop_checkForDecimals2 = verify checkForDecimals "foo[1.2]=bar" 1426prop_checkForDecimals3 = verifyNot checkForDecimals "declare -A foo; foo[1.2]=bar" 1427checkForDecimals params t@(TA_Expansion id _) = sequence_ $ do 1428 guard $ not (hasFloatingPoint params) 1429 str <- getLiteralString t 1430 first <- str !!! 0 1431 guard $ isDigit first && '.' `elem` str 1432 return $ err id 2079 "(( )) doesn't support decimals. Use bc or awk." 1433checkForDecimals _ _ = return () 1434 1435prop_checkDivBeforeMult = verify checkDivBeforeMult "echo $((c/n*100))" 1436prop_checkDivBeforeMult2 = verifyNot checkDivBeforeMult "echo $((c*100/n))" 1437prop_checkDivBeforeMult3 = verifyNot checkDivBeforeMult "echo $((c/10*10))" 1438checkDivBeforeMult params (TA_Binary _ "*" (TA_Binary id "/" _ x) y) 1439 | not (hasFloatingPoint params) && x /= y = 1440 info id 2017 "Increase precision by replacing a/b*c with a*c/b." 1441checkDivBeforeMult _ _ = return () 1442 1443prop_checkArithmeticDeref = verify checkArithmeticDeref "echo $((3+$foo))" 1444prop_checkArithmeticDeref2 = verify checkArithmeticDeref "cow=14; (( s+= $cow ))" 1445prop_checkArithmeticDeref3 = verifyNot checkArithmeticDeref "cow=1/40; (( s+= ${cow%%/*} ))" 1446prop_checkArithmeticDeref4 = verifyNot checkArithmeticDeref "(( ! $? ))" 1447prop_checkArithmeticDeref5 = verifyNot checkArithmeticDeref "(($1))" 1448prop_checkArithmeticDeref6 = verify checkArithmeticDeref "(( a[$i] ))" 1449prop_checkArithmeticDeref7 = verifyNot checkArithmeticDeref "(( 10#$n ))" 1450prop_checkArithmeticDeref8 = verifyNot checkArithmeticDeref "let i=$i+1" 1451prop_checkArithmeticDeref9 = verifyNot checkArithmeticDeref "(( a[foo] ))" 1452prop_checkArithmeticDeref10= verifyNot checkArithmeticDeref "(( a[\\$foo] ))" 1453prop_checkArithmeticDeref11= verifyNot checkArithmeticDeref "a[$foo]=wee" 1454prop_checkArithmeticDeref12= verify checkArithmeticDeref "for ((i=0; $i < 3; i)); do true; done" 1455prop_checkArithmeticDeref13= verifyNot checkArithmeticDeref "(( $$ ))" 1456prop_checkArithmeticDeref14= verifyNot checkArithmeticDeref "(( $! ))" 1457prop_checkArithmeticDeref15= verifyNot checkArithmeticDeref "(( ${!var} ))" 1458prop_checkArithmeticDeref16= verifyNot checkArithmeticDeref "(( ${x+1} + ${x=42} ))" 1459checkArithmeticDeref params t@(TA_Expansion _ [T_DollarBraced id _ l]) = 1460 unless (isException $ concat $ oversimplify l) getWarning 1461 where 1462 isException [] = True 1463 isException s@(h:_) = any (`elem` "/.:#%?*@$-!+=^,") s || isDigit h 1464 getWarning = fromMaybe noWarning . msum . map warningFor $ parents params t 1465 warningFor t = 1466 case t of 1467 T_Arithmetic {} -> return normalWarning 1468 T_DollarArithmetic {} -> return normalWarning 1469 T_ForArithmetic {} -> return normalWarning 1470 T_SimpleCommand {} -> return noWarning 1471 _ -> Nothing 1472 1473 normalWarning = style id 2004 "$/${} is unnecessary on arithmetic variables." 1474 noWarning = return () 1475checkArithmeticDeref _ _ = return () 1476 1477prop_checkArithmeticBadOctal1 = verify checkArithmeticBadOctal "(( 0192 ))" 1478prop_checkArithmeticBadOctal2 = verifyNot checkArithmeticBadOctal "(( 0x192 ))" 1479prop_checkArithmeticBadOctal3 = verifyNot checkArithmeticBadOctal "(( 1 ^ 0777 ))" 1480checkArithmeticBadOctal _ t@(TA_Expansion id _) = sequence_ $ do 1481 str <- getLiteralString t 1482 guard $ str `matches` octalRE 1483 return $ err id 2080 "Numbers with leading 0 are considered octal." 1484 where 1485 octalRE = mkRegex "^0[0-7]*[8-9]" 1486checkArithmeticBadOctal _ _ = return () 1487 1488prop_checkComparisonAgainstGlob = verify checkComparisonAgainstGlob "[[ $cow == $bar ]]" 1489prop_checkComparisonAgainstGlob2 = verifyNot checkComparisonAgainstGlob "[[ $cow == \"$bar\" ]]" 1490prop_checkComparisonAgainstGlob3 = verify checkComparisonAgainstGlob "[ $cow = *foo* ]" 1491prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow = foo ]" 1492prop_checkComparisonAgainstGlob5 = verify checkComparisonAgainstGlob "[[ $cow != $bar ]]" 1493prop_checkComparisonAgainstGlob6 = verify checkComparisonAgainstGlob "[ $f != /* ]" 1494checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _ _])) 1495 | op `elem` ["=", "==", "!="] = 1496 warn id 2053 $ "Quote the right-hand side of " ++ op ++ " in [[ ]] to prevent glob matching." 1497checkComparisonAgainstGlob params (TC_Binary _ SingleBracket op _ word) 1498 | op `elem` ["=", "==", "!="] && isGlob word = 1499 err (getId word) 2081 msg 1500 where 1501 msg = if isBashLike params 1502 then "[ .. ] can't match globs. Use [[ .. ]] or case statement." 1503 else "[ .. ] can't match globs. Use a case statement." 1504 1505checkComparisonAgainstGlob _ _ = return () 1506 1507prop_checkCaseAgainstGlob1 = verify checkCaseAgainstGlob "case foo in lol$n) foo;; esac" 1508prop_checkCaseAgainstGlob2 = verify checkCaseAgainstGlob "case foo in $(foo)) foo;; esac" 1509prop_checkCaseAgainstGlob3 = verifyNot checkCaseAgainstGlob "case foo in *$bar*) foo;; esac" 1510checkCaseAgainstGlob _ t = 1511 case t of 1512 (T_CaseExpression _ _ cases) -> mapM_ check cases 1513 _ -> return () 1514 where 1515 check (_, list, _) = mapM_ check' list 1516 check' expr@(T_NormalWord _ list) 1517 -- If it's already a glob, assume that's what the user wanted 1518 | not (isGlob expr) && any isQuoteableExpansion list = 1519 warn (getId expr) 2254 "Quote expansions in case patterns to match literally rather than as a glob." 1520 check' _ = return () 1521 1522prop_checkCommarrays1 = verify checkCommarrays "a=(1, 2)" 1523prop_checkCommarrays2 = verify checkCommarrays "a+=(1,2,3)" 1524prop_checkCommarrays3 = verifyNot checkCommarrays "cow=(1 \"foo,bar\" 3)" 1525prop_checkCommarrays4 = verifyNot checkCommarrays "cow=('one,' 'two')" 1526prop_checkCommarrays5 = verify checkCommarrays "a=([a]=b, [c]=d)" 1527prop_checkCommarrays6 = verify checkCommarrays "a=([a]=b,[c]=d,[e]=f)" 1528prop_checkCommarrays7 = verify checkCommarrays "a=(1,2)" 1529checkCommarrays _ (T_Array id l) = 1530 when (any (isCommaSeparated . literal) l) $ 1531 warn id 2054 "Use spaces, not commas, to separate array elements." 1532 where 1533 literal (T_IndexedElement _ _ l) = literal l 1534 literal (T_NormalWord _ l) = concatMap literal l 1535 literal (T_Literal _ str) = str 1536 literal _ = "" 1537 1538 isCommaSeparated = elem ',' 1539checkCommarrays _ _ = return () 1540 1541prop_checkOrNeq1 = verify checkOrNeq "if [[ $lol -ne cow || $lol -ne foo ]]; then echo foo; fi" 1542prop_checkOrNeq2 = verify checkOrNeq "(( a!=lol || a!=foo ))" 1543prop_checkOrNeq3 = verify checkOrNeq "[ \"$a\" != lol || \"$a\" != foo ]" 1544prop_checkOrNeq4 = verifyNot checkOrNeq "[ a != $cow || b != $foo ]" 1545prop_checkOrNeq5 = verifyNot checkOrNeq "[[ $a != /home || $a != */public_html/* ]]" 1546prop_checkOrNeq6 = verify checkOrNeq "[ $a != a ] || [ $a != b ]" 1547prop_checkOrNeq7 = verify checkOrNeq "[ $a != a ] || [ $a != b ] || true" 1548prop_checkOrNeq8 = verifyNot checkOrNeq "[[ $a != x || $a != x ]]" 1549prop_checkOrNeq9 = verifyNot checkOrNeq "[ 0 -ne $FOO ] || [ 0 -ne $BAR ]" 1550-- This only catches the most idiomatic cases. Fixme? 1551 1552-- For test-level "or": [ x != y -o x != z ] 1553checkOrNeq _ (TC_Or id typ op (TC_Binary _ _ op1 lhs1 rhs1 ) (TC_Binary _ _ op2 lhs2 rhs2)) 1554 | (op1 == op2 && (op1 == "-ne" || op1 == "!=")) && lhs1 == lhs2 && rhs1 /= rhs2 && not (any isGlob [rhs1,rhs2]) = 1555 warn id 2055 $ "You probably wanted " ++ (if typ == SingleBracket then "-a" else "&&") ++ " here, otherwise it's always true." 1556 1557-- For arithmetic context "or" 1558checkOrNeq _ (TA_Binary id "||" (TA_Binary _ "!=" word1 _) (TA_Binary _ "!=" word2 _)) 1559 | word1 == word2 = 1560 warn id 2056 "You probably wanted && here, otherwise it's always true." 1561 1562-- For command level "or": [ x != y ] || [ x != z ] 1563checkOrNeq _ (T_OrIf id lhs rhs) = sequence_ $ do 1564 (lhs1, op1, rhs1) <- getExpr lhs 1565 (lhs2, op2, rhs2) <- getExpr rhs 1566 guard $ op1 == op2 && op1 `elem` ["-ne", "!="] 1567 guard $ lhs1 == lhs2 && rhs1 /= rhs2 1568 guard . not $ any isGlob [rhs1, rhs2] 1569 return $ warn id 2252 "You probably wanted && here, otherwise it's always true." 1570 where 1571 getExpr x = 1572 case x of 1573 T_OrIf _ lhs _ -> getExpr lhs -- Fetches x and y in `T_OrIf x (T_OrIf y z)` 1574 T_Pipeline _ _ [x] -> getExpr x 1575 T_Redirecting _ _ c -> getExpr c 1576 T_Condition _ _ c -> getExpr c 1577 TC_Binary _ _ op lhs rhs -> orient (lhs, op, rhs) 1578 _ -> Nothing 1579 1580 -- Swap items so that the constant side is rhs (or Nothing if both/neither is constant) 1581 orient (lhs, op, rhs) = 1582 case (isConstant lhs, isConstant rhs) of 1583 (True, False) -> return (rhs, op, lhs) 1584 (False, True) -> return (lhs, op, rhs) 1585 _ -> Nothing 1586 1587 1588checkOrNeq _ _ = return () 1589 1590 1591prop_checkValidCondOps1 = verify checkValidCondOps "[[ a -xz b ]]" 1592prop_checkValidCondOps2 = verify checkValidCondOps "[ -M a ]" 1593prop_checkValidCondOps2a= verifyNot checkValidCondOps "[ 3 \\> 2 ]" 1594prop_checkValidCondOps3 = verifyNot checkValidCondOps "[ 1 = 2 -a 3 -ge 4 ]" 1595prop_checkValidCondOps4 = verifyNot checkValidCondOps "[[ ! -v foo ]]" 1596checkValidCondOps _ (TC_Binary id _ s _ _) 1597 | s `notElem` binaryTestOps = 1598 warn id 2057 "Unknown binary operator." 1599checkValidCondOps _ (TC_Unary id _ s _) 1600 | s `notElem` unaryTestOps = 1601 warn id 2058 "Unknown unary operator." 1602checkValidCondOps _ _ = return () 1603 1604prop_checkUuoeVar1 = verify checkUuoeVar "for f in $(echo $tmp); do echo lol; done" 1605prop_checkUuoeVar2 = verify checkUuoeVar "date +`echo \"$format\"`" 1606prop_checkUuoeVar3 = verifyNot checkUuoeVar "foo \"$(echo -e '\r')\"" 1607prop_checkUuoeVar4 = verifyNot checkUuoeVar "echo $tmp" 1608prop_checkUuoeVar5 = verify checkUuoeVar "foo \"$(echo \"$(date) value:\" $value)\"" 1609prop_checkUuoeVar6 = verifyNot checkUuoeVar "foo \"$(echo files: *.png)\"" 1610prop_checkUuoeVar7 = verifyNot checkUuoeVar "foo $(echo $(bar))" -- covered by 2005 1611prop_checkUuoeVar8 = verifyNot checkUuoeVar "#!/bin/sh\nz=$(echo)" 1612prop_checkUuoeVar9 = verify checkUuoeVar "foo $(echo $(<file))" 1613checkUuoeVar _ p = 1614 case p of 1615 T_Backticked id [cmd] -> check id cmd 1616 T_DollarExpansion id [cmd] -> check id cmd 1617 _ -> return () 1618 where 1619 couldBeOptimized f = case f of 1620 T_Glob {} -> False 1621 T_Extglob {} -> False 1622 T_BraceExpansion {} -> False 1623 T_NormalWord _ l -> all couldBeOptimized l 1624 T_DoubleQuoted _ l -> all couldBeOptimized l 1625 _ -> True 1626 1627 check id (T_Pipeline _ _ [T_Redirecting _ _ c]) = warnForEcho id c 1628 check _ _ = return () 1629 isCovered first rest = null rest && tokenIsJustCommandOutput first 1630 warnForEcho id = checkUnqualifiedCommand "echo" $ \_ vars -> 1631 case vars of 1632 (first:rest) -> 1633 unless (isCovered first rest || "-" `isPrefixOf` onlyLiteralString first) $ 1634 when (all couldBeOptimized vars) $ style id 2116 1635 "Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'." 1636 _ -> return () 1637 1638 1639prop_checkTestRedirects1 = verify checkTestRedirects "test 3 > 1" 1640prop_checkTestRedirects2 = verifyNot checkTestRedirects "test 3 \\> 1" 1641prop_checkTestRedirects3 = verify checkTestRedirects "/usr/bin/test $var > $foo" 1642prop_checkTestRedirects4 = verifyNot checkTestRedirects "test 1 -eq 2 2> file" 1643checkTestRedirects _ (T_Redirecting id redirs cmd) | cmd `isCommand` "test" = 1644 mapM_ check redirs 1645 where 1646 check t = 1647 when (suspicious t) $ 1648 warn (getId t) 2065 "This is interpreted as a shell file redirection, not a comparison." 1649 suspicious t = -- Ignore redirections of stderr because these are valid for squashing e.g. int errors, 1650 case t of -- and >> and similar redirections because these are probably not comparisons. 1651 T_FdRedirect _ fd (T_IoFile _ op _) -> fd /= "2" && isComparison op 1652 _ -> False 1653 isComparison t = 1654 case t of 1655 T_Greater _ -> True 1656 T_Less _ -> True 1657 _ -> False 1658checkTestRedirects _ _ = return () 1659 1660prop_checkPS11 = verify checkPS1Assignments "PS1='\\033[1;35m\\$ '" 1661prop_checkPS11a= verify checkPS1Assignments "export PS1='\\033[1;35m\\$ '" 1662prop_checkPSf2 = verify checkPS1Assignments "PS1='\\h \\e[0m\\$ '" 1663prop_checkPS13 = verify checkPS1Assignments "PS1=$'\\x1b[c '" 1664prop_checkPS14 = verify checkPS1Assignments "PS1=$'\\e[3m; '" 1665prop_checkPS14a= verify checkPS1Assignments "export PS1=$'\\e[3m; '" 1666prop_checkPS15 = verifyNot checkPS1Assignments "PS1='\\[\\033[1;35m\\]\\$ '" 1667prop_checkPS16 = verifyNot checkPS1Assignments "PS1='\\[\\e1m\\e[1m\\]\\$ '" 1668prop_checkPS17 = verifyNot checkPS1Assignments "PS1='e033x1B'" 1669prop_checkPS18 = verifyNot checkPS1Assignments "PS1='\\[\\e\\]'" 1670checkPS1Assignments _ (T_Assignment _ _ "PS1" _ word) = warnFor word 1671 where 1672 warnFor word = 1673 let contents = concat $ oversimplify word in 1674 when (containsUnescaped contents) $ 1675 info (getId word) 2025 "Make sure all escape sequences are enclosed in \\[..\\] to prevent line wrapping issues" 1676 containsUnescaped s = 1677 let unenclosed = subRegex enclosedRegex s "" in 1678 isJust $ matchRegex escapeRegex unenclosed 1679 enclosedRegex = mkRegex "\\\\\\[.*\\\\\\]" -- FIXME: shouldn't be eager 1680 escapeRegex = mkRegex "\\\\x1[Bb]|\\\\e|\x1B|\\\\033" 1681checkPS1Assignments _ _ = return () 1682 1683prop_checkBackticks1 = verify checkBackticks "echo `foo`" 1684prop_checkBackticks2 = verifyNot checkBackticks "echo $(foo)" 1685prop_checkBackticks3 = verifyNot checkBackticks "echo `#inlined comment` foo" 1686checkBackticks params (T_Backticked id list) | not (null list) = 1687 addComment $ 1688 makeCommentWithFix StyleC id 2006 "Use $(...) notation instead of legacy backticks `...`." 1689 (fixWith [replaceStart id params 1 "$(", replaceEnd id params 1 ")"]) 1690checkBackticks _ _ = return () 1691 1692 1693prop_checkBadParameterSubstitution1 = verify checkBadParameterSubstitution "${foo$n}" 1694prop_checkBadParameterSubstitution2 = verifyNot checkBadParameterSubstitution "${foo//$n/lol}" 1695prop_checkBadParameterSubstitution3 = verify checkBadParameterSubstitution "${$#}" 1696prop_checkBadParameterSubstitution4 = verify checkBadParameterSubstitution "${var${n}_$((i%2))}" 1697prop_checkBadParameterSubstitution5 = verifyNot checkBadParameterSubstitution "${bar}" 1698prop_checkBadParameterSubstitution6 = verify checkBadParameterSubstitution "${\"bar\"}" 1699prop_checkBadParameterSubstitution7 = verify checkBadParameterSubstitution "${{var}" 1700prop_checkBadParameterSubstitution8 = verify checkBadParameterSubstitution "${$(x)//x/y}" 1701prop_checkBadParameterSubstitution9 = verifyNot checkBadParameterSubstitution "$# ${#} $! ${!} ${!#} ${#!}" 1702prop_checkBadParameterSubstitution10 = verify checkBadParameterSubstitution "${'foo'}" 1703prop_checkBadParameterSubstitution11 = verify checkBadParameterSubstitution "${${x%.*}##*/}" 1704 1705checkBadParameterSubstitution _ t = 1706 case t of 1707 (T_DollarBraced i _ (T_NormalWord _ contents@(first:_))) -> 1708 if isIndirection contents 1709 then err i 2082 "To expand via indirection, use arrays, ${!name} or (for sh only) eval." 1710 else checkFirst first 1711 _ -> return () 1712 1713 where 1714 1715 isIndirection vars = 1716 let list = mapMaybe isIndirectionPart vars in 1717 not (null list) && and list 1718 1719 isIndirectionPart t = 1720 case t of T_DollarExpansion {} -> Just True 1721 T_Backticked {} -> Just True 1722 T_DollarBraced {} -> Just True 1723 T_DollarArithmetic {} -> Just True 1724 T_Literal _ s -> if all isVariableChar s 1725 then Nothing 1726 else Just False 1727 _ -> Just False 1728 1729 checkFirst t = 1730 case t of 1731 T_Literal id (c:_) -> 1732 if isVariableChar c || isSpecialVariableChar c 1733 then return () 1734 else err id 2296 $ "Parameter expansions can't start with " ++ e4m [c] ++ ". Double check syntax." 1735 1736 T_ParamSubSpecialChar {} -> return () 1737 1738 T_DoubleQuoted id [T_Literal _ s] | isVariable s -> 1739 err id 2297 "Double quotes must be outside ${}: ${\"invalid\"} vs \"${valid}\"." 1740 1741 T_DollarBraced id braces _ | isUnmodifiedParameterExpansion t -> 1742 err id 2298 $ 1743 (if braces then "${${x}}" else "${$x}") 1744 ++ " is invalid. For expansion, use ${x}. For indirection, use arrays, ${!x} or (for sh) eval." 1745 1746 T_DollarBraced {} -> 1747 err (getId t) 2299 "Parameter expansions can't be nested. Use temporary variables." 1748 1749 _ | isCommandSubstitution t -> 1750 err (getId t) 2300 "Parameter expansion can't be applied to command substitutions. Use temporary variables." 1751 1752 _ -> err (getId t) 2301 $ "Parameter expansion starts with unexpected " ++ name t ++ ". Double check syntax." 1753 1754 isVariable str = 1755 case str of 1756 [c] -> isVariableStartChar c || isSpecialVariableChar c || isDigit c 1757 x -> isVariableName x 1758 1759 name t = 1760 case t of 1761 T_SingleQuoted {} -> "quotes" 1762 T_DoubleQuoted {} -> "quotes" 1763 _ -> "syntax" 1764 1765 1766prop_checkInexplicablyUnquoted1 = verify checkInexplicablyUnquoted "echo 'var='value';'" 1767prop_checkInexplicablyUnquoted2 = verifyNot checkInexplicablyUnquoted "'foo'*" 1768prop_checkInexplicablyUnquoted3 = verifyNot checkInexplicablyUnquoted "wget --user-agent='something'" 1769prop_checkInexplicablyUnquoted4 = verify checkInexplicablyUnquoted "echo \"VALUES (\"id\")\"" 1770prop_checkInexplicablyUnquoted5 = verifyNot checkInexplicablyUnquoted "\"$dir\"/\"$file\"" 1771prop_checkInexplicablyUnquoted6 = verifyNot checkInexplicablyUnquoted "\"$dir\"some_stuff\"$file\"" 1772prop_checkInexplicablyUnquoted7 = verifyNot checkInexplicablyUnquoted "${dir/\"foo\"/\"bar\"}" 1773prop_checkInexplicablyUnquoted8 = verifyNot checkInexplicablyUnquoted " 'foo'\\\n 'bar'" 1774prop_checkInexplicablyUnquoted9 = verifyNot checkInexplicablyUnquoted "[[ $x =~ \"foo\"(\"bar\"|\"baz\") ]]" 1775prop_checkInexplicablyUnquoted10 = verifyNot checkInexplicablyUnquoted "cmd ${x+--name=\"$x\" --output=\"$x.out\"}" 1776prop_checkInexplicablyUnquoted11 = verifyNot checkInexplicablyUnquoted "echo \"foo\"/\"bar\"" 1777prop_checkInexplicablyUnquoted12 = verifyNot checkInexplicablyUnquoted "declare \"foo\"=\"bar\"" 1778checkInexplicablyUnquoted params (T_NormalWord id tokens) = mapM_ check (tails tokens) 1779 where 1780 check (T_SingleQuoted _ _:T_Literal id str:_) 1781 | not (null str) && all isAlphaNum str = 1782 info id 2026 "This word is outside of quotes. Did you intend to 'nest '\"'single quotes'\"' instead'? " 1783 1784 check (T_DoubleQuoted _ a:trapped:T_DoubleQuoted _ b:_) = 1785 case trapped of 1786 T_DollarExpansion id _ -> warnAboutExpansion id 1787 T_DollarBraced id _ _ -> warnAboutExpansion id 1788 T_Literal id s 1789 | not (quotesSingleThing a && quotesSingleThing b 1790 || s `elem` ["=", ":", "/"] 1791 || isSpecial (getPath (parentMap params) trapped) 1792 ) -> 1793 warnAboutLiteral id 1794 _ -> return () 1795 1796 check _ = return () 1797 1798 -- Regexes for [[ .. =~ re ]] are parsed with metacharacters like ()| as unquoted 1799 -- literals. The same is true for ${x+"foo" "bar"}. Avoid overtriggering on these. 1800 isSpecial t = 1801 case t of 1802 (T_Redirecting {} : _) -> False 1803 T_DollarBraced {} : _ -> True 1804 (a:(TC_Binary _ _ "=~" lhs rhs):rest) -> getId a == getId rhs 1805 _:rest -> isSpecial rest 1806 _ -> False 1807 1808 -- If the surrounding quotes quote single things, like "$foo"_and_then_some_"$stuff", 1809 -- the quotes were probably intentional and harmless. 1810 quotesSingleThing x = case x of 1811 [T_DollarExpansion _ _] -> True 1812 [T_DollarBraced _ _ _] -> True 1813 [T_Backticked _ _] -> True 1814 _ -> False 1815 1816 warnAboutExpansion id = 1817 warn id 2027 "The surrounding quotes actually unquote this. Remove or escape them." 1818 warnAboutLiteral id = 1819 warn id 2140 "Word is of the form \"A\"B\"C\" (B indicated). Did you mean \"ABC\" or \"A\\\"B\\\"C\"?" 1820checkInexplicablyUnquoted _ _ = return () 1821 1822prop_checkTildeInQuotes1 = verify checkTildeInQuotes "var=\"~/out.txt\"" 1823prop_checkTildeInQuotes2 = verify checkTildeInQuotes "foo > '~/dir'" 1824prop_checkTildeInQuotes4 = verifyNot checkTildeInQuotes "~/file" 1825prop_checkTildeInQuotes5 = verifyNot checkTildeInQuotes "echo '/~foo/cow'" 1826prop_checkTildeInQuotes6 = verifyNot checkTildeInQuotes "awk '$0 ~ /foo/'" 1827checkTildeInQuotes _ = check 1828 where 1829 verify id ('~':'/':_) = warn id 2088 "Tilde does not expand in quotes. Use $HOME." 1830 verify _ _ = return () 1831 check (T_NormalWord _ (T_SingleQuoted id str:_)) = 1832 verify id str 1833 check (T_NormalWord _ (T_DoubleQuoted _ (T_Literal id str:_):_)) = 1834 verify id str 1835 check _ = return () 1836 1837prop_checkLonelyDotDash1 = verify checkLonelyDotDash "./ file" 1838prop_checkLonelyDotDash2 = verifyNot checkLonelyDotDash "./file" 1839checkLonelyDotDash _ t@(T_Redirecting id _ _) 1840 | isUnqualifiedCommand t "./" = 1841 err id 2083 "Don't add spaces after the slash in './file'." 1842checkLonelyDotDash _ _ = return () 1843 1844 1845prop_checkSpuriousExec1 = verify checkSpuriousExec "exec foo; true" 1846prop_checkSpuriousExec2 = verify checkSpuriousExec "if a; then exec b; exec c; fi" 1847prop_checkSpuriousExec3 = verifyNot checkSpuriousExec "echo cow; exec foo" 1848prop_checkSpuriousExec4 = verifyNot checkSpuriousExec "if a; then exec b; fi" 1849prop_checkSpuriousExec5 = verifyNot checkSpuriousExec "exec > file; cmd" 1850prop_checkSpuriousExec6 = verify checkSpuriousExec "exec foo > file; cmd" 1851prop_checkSpuriousExec7 = verifyNot checkSpuriousExec "exec file; echo failed; exit 3" 1852prop_checkSpuriousExec8 = verifyNot checkSpuriousExec "exec {origout}>&1- >tmp.log 2>&1; bar" 1853prop_checkSpuriousExec9 = verify checkSpuriousExec "for file in rc.d/*; do exec \"$file\"; done" 1854prop_checkSpuriousExec10 = verifyNot checkSpuriousExec "exec file; r=$?; printf >&2 'failed\n'; return $r" 1855checkSpuriousExec _ = doLists 1856 where 1857 doLists (T_Script _ _ cmds) = doList cmds False 1858 doLists (T_BraceGroup _ cmds) = doList cmds False 1859 doLists (T_WhileExpression _ _ cmds) = doList cmds True 1860 doLists (T_UntilExpression _ _ cmds) = doList cmds True 1861 doLists (T_ForIn _ _ _ cmds) = doList cmds True 1862 doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds True 1863 doLists (T_IfExpression _ thens elses) = do 1864 mapM_ (\(_, l) -> doList l False) thens 1865 doList elses False 1866 doLists _ = return () 1867 1868 stripCleanup = reverse . dropWhile cleanup . reverse 1869 cleanup (T_Pipeline _ _ [cmd]) = 1870 isCommandMatch cmd (`elem` ["echo", "exit", "printf", "return"]) 1871 || isAssignment cmd 1872 cleanup _ = False 1873 1874 doList = doList' . stripCleanup 1875 -- The second parameter is True if we are in a loop 1876 -- In that case we should emit the warning also if `exec' is the last statement 1877 doList' (current:t@(following:_)) False = do 1878 commentIfExec current 1879 doList t False 1880 doList' (current:tail) True = do 1881 commentIfExec current 1882 doList tail True 1883 doList' _ _ = return () 1884 1885 commentIfExec (T_Pipeline id _ [c]) = commentIfExec c 1886 commentIfExec (T_Redirecting _ _ (T_SimpleCommand id _ (cmd:additionalArg:_))) | 1887 getLiteralString cmd == Just "exec" = 1888 warn id 2093 "Remove \"exec \" if script should continue after this command." 1889 commentIfExec _ = return () 1890 1891 1892prop_checkSpuriousExpansion1 = verify checkSpuriousExpansion "if $(true); then true; fi" 1893prop_checkSpuriousExpansion3 = verifyNot checkSpuriousExpansion "$(cmd) --flag1 --flag2" 1894prop_checkSpuriousExpansion4 = verify checkSpuriousExpansion "$((i++))" 1895checkSpuriousExpansion _ (T_SimpleCommand _ _ [T_NormalWord _ [word]]) = check word 1896 where 1897 check word = case word of 1898 T_DollarExpansion id _ -> 1899 warn id 2091 "Remove surrounding $() to avoid executing output (or use eval if intentional)." 1900 T_Backticked id _ -> 1901 warn id 2092 "Remove backticks to avoid executing output (or use eval if intentional)." 1902 T_DollarArithmetic id _ -> 1903 err id 2084 "Remove '$' or use '_=$((expr))' to avoid executing output." 1904 _ -> return () 1905checkSpuriousExpansion _ _ = return () 1906 1907 1908prop_checkDollarBrackets1 = verify checkDollarBrackets "echo $[1+2]" 1909prop_checkDollarBrackets2 = verifyNot checkDollarBrackets "echo $((1+2))" 1910checkDollarBrackets _ (T_DollarBracket id _) = 1911 style id 2007 "Use $((..)) instead of deprecated $[..]" 1912checkDollarBrackets _ _ = return () 1913 1914prop_checkSshHereDoc1 = verify checkSshHereDoc "ssh host << foo\necho $PATH\nfoo" 1915prop_checkSshHereDoc2 = verifyNot checkSshHereDoc "ssh host << 'foo'\necho $PATH\nfoo" 1916checkSshHereDoc _ (T_Redirecting _ redirs cmd) 1917 | cmd `isCommand` "ssh" = 1918 mapM_ checkHereDoc redirs 1919 where 1920 hasVariables = mkRegex "[`$]" 1921 checkHereDoc (T_FdRedirect _ _ (T_HereDoc id _ Unquoted token tokens)) 1922 | not (all isConstant tokens) = 1923 warn id 2087 $ "Quote '" ++ (e4m token) ++ "' to make here document expansions happen on the server side rather than on the client." 1924 checkHereDoc _ = return () 1925checkSshHereDoc _ _ = return () 1926 1927--- Subshell detection 1928prop_subshellAssignmentCheck = verifyTree subshellAssignmentCheck "cat foo | while read bar; do a=$bar; done; echo \"$a\"" 1929prop_subshellAssignmentCheck2 = verifyNotTree subshellAssignmentCheck "while read bar; do a=$bar; done < file; echo \"$a\"" 1930prop_subshellAssignmentCheck3 = verifyTree subshellAssignmentCheck "( A=foo; ); rm $A" 1931prop_subshellAssignmentCheck4 = verifyNotTree subshellAssignmentCheck "( A=foo; rm $A; )" 1932prop_subshellAssignmentCheck5 = verifyTree subshellAssignmentCheck "cat foo | while read cow; do true; done; echo $cow;" 1933prop_subshellAssignmentCheck6 = verifyTree subshellAssignmentCheck "( export lol=$(ls); ); echo $lol;" 1934prop_subshellAssignmentCheck6a= verifyTree subshellAssignmentCheck "( typeset -a lol=a; ); echo $lol;" 1935prop_subshellAssignmentCheck7 = verifyTree subshellAssignmentCheck "cmd | while read foo; do (( n++ )); done; echo \"$n lines\"" 1936prop_subshellAssignmentCheck8 = verifyTree subshellAssignmentCheck "n=3 & echo $((n++))" 1937prop_subshellAssignmentCheck9 = verifyTree subshellAssignmentCheck "read n & n=foo$n" 1938prop_subshellAssignmentCheck10 = verifyTree subshellAssignmentCheck "(( n <<= 3 )) & (( n |= 4 )) &" 1939prop_subshellAssignmentCheck11 = verifyTree subshellAssignmentCheck "cat /etc/passwd | while read line; do let n=n+1; done\necho $n" 1940prop_subshellAssignmentCheck12 = verifyTree subshellAssignmentCheck "cat /etc/passwd | while read line; do let ++n; done\necho $n" 1941prop_subshellAssignmentCheck13 = verifyTree subshellAssignmentCheck "#!/bin/bash\necho foo | read bar; echo $bar" 1942prop_subshellAssignmentCheck14 = verifyNotTree subshellAssignmentCheck "#!/bin/ksh93\necho foo | read bar; echo $bar" 1943prop_subshellAssignmentCheck15 = verifyNotTree subshellAssignmentCheck "#!/bin/ksh\ncat foo | while read bar; do a=$bar; done\necho \"$a\"" 1944prop_subshellAssignmentCheck16 = verifyNotTree subshellAssignmentCheck "(set -e); echo $@" 1945prop_subshellAssignmentCheck17 = verifyNotTree subshellAssignmentCheck "foo=${ { bar=$(baz); } 2>&1; }; echo $foo $bar" 1946prop_subshellAssignmentCheck18 = verifyTree subshellAssignmentCheck "( exec {n}>&2; ); echo $n" 1947prop_subshellAssignmentCheck19 = verifyNotTree subshellAssignmentCheck "#!/bin/bash\nshopt -s lastpipe; echo a | read -r b; echo \"$b\"" 1948prop_subshellAssignmentCheck20 = verifyTree subshellAssignmentCheck "@test 'foo' { a=1; }\n@test 'bar' { echo $a; }\n" 1949prop_subshellAssignmentCheck21 = verifyNotTree subshellAssignmentCheck "test1() { echo foo | if [[ $var ]]; then echo $var; fi; }; test2() { echo $var; }" 1950prop_subshellAssignmentCheck22 = verifyNotTree subshellAssignmentCheck "( [[ -n $foo || -z $bar ]] ); echo $foo $bar" 1951prop_subshellAssignmentCheck23 = verifyNotTree subshellAssignmentCheck "( export foo ); echo $foo" 1952subshellAssignmentCheck params t = 1953 let flow = variableFlow params 1954 check = findSubshelled flow [("oops",[])] Map.empty 1955 in execWriter check 1956 1957 1958findSubshelled [] _ _ = return () 1959findSubshelled (Assignment x@(_, _, str, data_):rest) scopes@((reason,scope):restscope) deadVars = 1960 if isTrueAssignmentSource data_ 1961 then findSubshelled rest ((reason, x:scope):restscope) $ Map.insert str Alive deadVars 1962 else findSubshelled rest scopes deadVars 1963 1964findSubshelled (Reference (_, readToken, str):rest) scopes deadVars = do 1965 unless (shouldIgnore str) $ case Map.findWithDefault Alive str deadVars of 1966 Alive -> return () 1967 Dead writeToken reason -> do 1968 info (getId writeToken) 2030 $ "Modification of " ++ str ++ " is local (to subshell caused by "++ reason ++")." 1969 info (getId readToken) 2031 $ str ++ " was modified in a subshell. That change might be lost." 1970 findSubshelled rest scopes deadVars 1971 where 1972 shouldIgnore str = 1973 str `elem` ["@", "*", "IFS"] 1974 1975findSubshelled (StackScope (SubshellScope reason):rest) scopes deadVars = 1976 findSubshelled rest ((reason,[]):scopes) deadVars 1977 1978findSubshelled (StackScopeEnd:rest) ((reason, scope):oldScopes) deadVars = 1979 findSubshelled rest oldScopes $ 1980 foldl (\m (_, token, var, _) -> 1981 Map.insert var (Dead token reason) m) deadVars scope 1982 1983 1984-- FIXME: This is a very strange way of doing it. 1985-- For each variable read/write, run a stateful function that emits 1986-- comments. The comments are collected and returned. 1987doVariableFlowAnalysis :: 1988 (Token -> Token -> String -> State t [v]) 1989 -> (Token -> Token -> String -> DataType -> State t [v]) 1990 -> t 1991 -> [StackData] 1992 -> [v] 1993 1994doVariableFlowAnalysis readFunc writeFunc empty flow = evalState ( 1995 foldM (\list x -> do { l <- doFlow x; return $ l ++ list; }) [] flow 1996 ) empty 1997 where 1998 doFlow (Reference (base, token, name)) = 1999 readFunc base token name 2000 doFlow (Assignment (base, token, name, values)) = 2001 writeFunc base token name values 2002 doFlow _ = return [] 2003 2004---- Check whether variables could have spaces/globs 2005prop_checkSpacefulness1 = verifyTree checkSpacefulness "a='cow moo'; echo $a" 2006prop_checkSpacefulness2 = verifyNotTree checkSpacefulness "a='cow moo'; [[ $a ]]" 2007prop_checkSpacefulness3 = verifyNotTree checkSpacefulness "a='cow*.mp3'; echo \"$a\"" 2008prop_checkSpacefulness4 = verifyTree checkSpacefulness "for f in *.mp3; do echo $f; done" 2009prop_checkSpacefulness4a= verifyNotTree checkSpacefulness "foo=3; foo=$(echo $foo)" 2010prop_checkSpacefulness5 = verifyTree checkSpacefulness "a='*'; b=$a; c=lol${b//foo/bar}; echo $c" 2011prop_checkSpacefulness6 = verifyTree checkSpacefulness "a=foo$(lol); echo $a" 2012prop_checkSpacefulness7 = verifyTree checkSpacefulness "a=foo\\ bar; rm $a" 2013prop_checkSpacefulness8 = verifyNotTree checkSpacefulness "a=foo\\ bar; a=foo; rm $a" 2014prop_checkSpacefulness10= verifyTree checkSpacefulness "rm $1" 2015prop_checkSpacefulness11= verifyTree checkSpacefulness "rm ${10//foo/bar}" 2016prop_checkSpacefulness12= verifyNotTree checkSpacefulness "(( $1 + 3 ))" 2017prop_checkSpacefulness13= verifyNotTree checkSpacefulness "if [[ $2 -gt 14 ]]; then true; fi" 2018prop_checkSpacefulness14= verifyNotTree checkSpacefulness "foo=$3 env" 2019prop_checkSpacefulness15= verifyNotTree checkSpacefulness "local foo=$1" 2020prop_checkSpacefulness16= verifyNotTree checkSpacefulness "declare foo=$1" 2021prop_checkSpacefulness17= verifyTree checkSpacefulness "echo foo=$1" 2022prop_checkSpacefulness18= verifyNotTree checkSpacefulness "$1 --flags" 2023prop_checkSpacefulness19= verifyTree checkSpacefulness "echo $PWD" 2024prop_checkSpacefulness20= verifyNotTree checkSpacefulness "n+='foo bar'" 2025prop_checkSpacefulness21= verifyNotTree checkSpacefulness "select foo in $bar; do true; done" 2026prop_checkSpacefulness22= verifyNotTree checkSpacefulness "echo $\"$1\"" 2027prop_checkSpacefulness23= verifyNotTree checkSpacefulness "a=(1); echo ${a[@]}" 2028prop_checkSpacefulness24= verifyTree checkSpacefulness "a='a b'; cat <<< $a" 2029prop_checkSpacefulness25= verifyTree checkSpacefulness "a='s/[0-9]//g'; sed $a" 2030prop_checkSpacefulness26= verifyTree checkSpacefulness "a='foo bar'; echo {1,2,$a}" 2031prop_checkSpacefulness27= verifyNotTree checkSpacefulness "echo ${a:+'foo'}" 2032prop_checkSpacefulness28= verifyNotTree checkSpacefulness "exec {n}>&1; echo $n" 2033prop_checkSpacefulness29= verifyNotTree checkSpacefulness "n=$(stuff); exec {n}>&-;" 2034prop_checkSpacefulness30= verifyTree checkSpacefulness "file='foo bar'; echo foo > $file;" 2035prop_checkSpacefulness31= verifyNotTree checkSpacefulness "echo \"`echo \\\"$1\\\"`\"" 2036prop_checkSpacefulness32= verifyNotTree checkSpacefulness "var=$1; [ -v var ]" 2037prop_checkSpacefulness33= verifyTree checkSpacefulness "for file; do echo $file; done" 2038prop_checkSpacefulness34= verifyTree checkSpacefulness "declare foo$n=$1" 2039prop_checkSpacefulness35= verifyNotTree checkSpacefulness "echo ${1+\"$1\"}" 2040prop_checkSpacefulness36= verifyNotTree checkSpacefulness "arg=$#; echo $arg" 2041prop_checkSpacefulness37= verifyNotTree checkSpacefulness "@test 'status' {\n [ $status -eq 0 ]\n}" 2042prop_checkSpacefulness37v = verifyTree checkVerboseSpacefulness "@test 'status' {\n [ $status -eq 0 ]\n}" 2043prop_checkSpacefulness38= verifyTree checkSpacefulness "a=; echo $a" 2044prop_checkSpacefulness39= verifyNotTree checkSpacefulness "a=''\"\"''; b=x$a; echo $b" 2045prop_checkSpacefulness40= verifyNotTree checkSpacefulness "a=$((x+1)); echo $a" 2046prop_checkSpacefulness41= verifyNotTree checkSpacefulness "exec $1 --flags" 2047prop_checkSpacefulness42= verifyNotTree checkSpacefulness "run $1 --flags" 2048prop_checkSpacefulness43= verifyNotTree checkSpacefulness "$foo=42" 2049prop_checkSpacefulness44= verifyTree checkSpacefulness "#!/bin/sh\nexport var=$value" 2050prop_checkSpacefulness45= verifyNotTree checkSpacefulness "wait -zzx -p foo; echo $foo" 2051prop_checkSpacefulness46= verifyNotTree checkSpacefulness "x=0; (( x += 1 )); echo $x" 2052 2053data SpaceStatus = SpaceSome | SpaceNone | SpaceEmpty deriving (Eq) 2054instance Semigroup SpaceStatus where 2055 SpaceNone <> SpaceNone = SpaceNone 2056 SpaceSome <> _ = SpaceSome 2057 _ <> SpaceSome = SpaceSome 2058 SpaceEmpty <> x = x 2059 x <> SpaceEmpty = x 2060instance Monoid SpaceStatus where 2061 mempty = SpaceEmpty 2062 mappend = (<>) 2063 2064-- This is slightly awkward because we want to support structured 2065-- optional checks based on nearly the same logic 2066checkSpacefulness params = checkSpacefulness' onFind params 2067 where 2068 emit x = tell [x] 2069 onFind spaces token _ = 2070 when (spaces /= SpaceNone) $ 2071 if isDefaultAssignment (parentMap params) token 2072 then 2073 emit $ makeComment InfoC (getId token) 2223 2074 "This default assignment may cause DoS due to globbing. Quote it." 2075 else 2076 unless (quotesMayConflictWithSC2281 params token) $ 2077 emit $ makeCommentWithFix InfoC (getId token) 2086 2078 "Double quote to prevent globbing and word splitting." 2079 (addDoubleQuotesAround params token) 2080 2081 isDefaultAssignment parents token = 2082 let modifier = getBracedModifier $ bracedString token in 2083 any (`isPrefixOf` modifier) ["=", ":="] 2084 && isParamTo parents ":" token 2085 2086 -- Given a T_DollarBraced, return a simplified version of the string contents. 2087 bracedString (T_DollarBraced _ _ l) = concat $ oversimplify l 2088 bracedString _ = error "Internal shellcheck error, please report! (bracedString on non-variable)" 2089 2090prop_checkSpacefulness4v= verifyTree checkVerboseSpacefulness "foo=3; foo=$(echo $foo)" 2091prop_checkSpacefulness8v= verifyTree checkVerboseSpacefulness "a=foo\\ bar; a=foo; rm $a" 2092prop_checkSpacefulness28v = verifyTree checkVerboseSpacefulness "exec {n}>&1; echo $n" 2093prop_checkSpacefulness36v = verifyTree checkVerboseSpacefulness "arg=$#; echo $arg" 2094prop_checkSpacefulness44v = verifyNotTree checkVerboseSpacefulness "foo=3; $foo=4" 2095checkVerboseSpacefulness params = checkSpacefulness' onFind params 2096 where 2097 onFind spaces token name = 2098 when (spaces == SpaceNone 2099 && name `notElem` specialVariablesWithoutSpaces 2100 && not (quotesMayConflictWithSC2281 params token)) $ 2101 tell [makeCommentWithFix StyleC (getId token) 2248 2102 "Prefer double quoting even when variables don't contain special characters." 2103 (addDoubleQuotesAround params token)] 2104 2105-- Don't suggest quotes if this will instead be autocorrected 2106-- from $foo=bar to foo=bar. This is not pretty but ok. 2107quotesMayConflictWithSC2281 params t = 2108 case getPath (parentMap params) t of 2109 _ : T_NormalWord parentId (me:T_Literal _ ('=':_):_) : T_SimpleCommand _ _ (cmd:_) : _ -> 2110 (getId t) == (getId me) && (parentId == getId cmd) 2111 _ -> False 2112 2113addDoubleQuotesAround params token = (surroundWith (getId token) params "\"") 2114checkSpacefulness' 2115 :: (SpaceStatus -> Token -> String -> Writer [TokenComment] ()) -> 2116 Parameters -> Token -> [TokenComment] 2117checkSpacefulness' onFind params t = 2118 doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params) 2119 where 2120 defaults = zip variablesWithoutSpaces (repeat SpaceNone) 2121 2122 hasSpaces name = gets (Map.findWithDefault SpaceSome name) 2123 2124 setSpaces name status = 2125 modify $ Map.insert name status 2126 2127 readF _ token name = do 2128 spaces <- hasSpaces name 2129 let needsQuoting = 2130 isExpansion token 2131 && not (isArrayExpansion token) -- There's another warning for this 2132 && not (isCountingReference token) 2133 && not (isQuoteFree (shellType params) parents token) 2134 && not (isQuotedAlternativeReference token) 2135 && not (usedAsCommandName parents token) 2136 2137 return . execWriter $ when needsQuoting $ onFind spaces token name 2138 2139 where 2140 emit x = tell [x] 2141 2142 writeF _ (TA_Assignment {}) name _ = setSpaces name SpaceNone >> return [] 2143 writeF _ _ name (DataString SourceExternal) = setSpaces name SpaceSome >> return [] 2144 writeF _ _ name (DataString SourceInteger) = setSpaces name SpaceNone >> return [] 2145 2146 writeF _ _ name (DataString (SourceFrom vals)) = do 2147 map <- get 2148 setSpaces name 2149 (isSpacefulWord (\x -> Map.findWithDefault SpaceSome x map) vals) 2150 return [] 2151 2152 writeF _ _ _ _ = return [] 2153 2154 parents = parentMap params 2155 2156 isExpansion t = 2157 case t of 2158 (T_DollarBraced _ _ _ ) -> True 2159 _ -> False 2160 2161 isSpacefulWord :: (String -> SpaceStatus) -> [Token] -> SpaceStatus 2162 isSpacefulWord f = mconcat . map (isSpaceful f) 2163 isSpaceful :: (String -> SpaceStatus) -> Token -> SpaceStatus 2164 isSpaceful spacefulF x = 2165 case x of 2166 T_DollarExpansion _ _ -> SpaceSome 2167 T_Backticked _ _ -> SpaceSome 2168 T_Glob _ _ -> SpaceSome 2169 T_Extglob {} -> SpaceSome 2170 T_DollarArithmetic _ _ -> SpaceNone 2171 T_Literal _ s -> fromLiteral s 2172 T_SingleQuoted _ s -> fromLiteral s 2173 T_DollarBraced _ _ l -> spacefulF $ getBracedReference $ concat $ oversimplify l 2174 T_NormalWord _ w -> isSpacefulWord spacefulF w 2175 T_DoubleQuoted _ w -> isSpacefulWord spacefulF w 2176 _ -> SpaceEmpty 2177 where 2178 globspace = "*?[] \t\n" 2179 containsAny s = any (`elem` s) 2180 fromLiteral "" = SpaceEmpty 2181 fromLiteral s | s `containsAny` globspace = SpaceSome 2182 fromLiteral _ = SpaceNone 2183 2184prop_CheckVariableBraces1 = verify checkVariableBraces "a='123'; echo $a" 2185prop_CheckVariableBraces2 = verifyNot checkVariableBraces "a='123'; echo ${a}" 2186prop_CheckVariableBraces3 = verifyNot checkVariableBraces "#shellcheck disable=SC2016\necho '$a'" 2187prop_CheckVariableBraces4 = verifyNot checkVariableBraces "echo $* $1" 2188prop_CheckVariableBraces5 = verifyNot checkVariableBraces "$foo=42" 2189checkVariableBraces params t@(T_DollarBraced id False l) 2190 | name `notElem` unbracedVariables && not (quotesMayConflictWithSC2281 params t) = 2191 styleWithFix id 2250 2192 "Prefer putting braces around variable references even when not strictly required." 2193 (fixFor t) 2194 where 2195 name = getBracedReference $ concat $ oversimplify l 2196 fixFor token = fixWith [replaceStart (getId token) params 1 "${" 2197 ,replaceEnd (getId token) params 0 "}"] 2198checkVariableBraces _ _ = return () 2199 2200prop_checkQuotesInLiterals1 = verifyTree checkQuotesInLiterals "param='--foo=\"bar\"'; app $param" 2201prop_checkQuotesInLiterals1a= verifyTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; app $param" 2202prop_checkQuotesInLiterals2 = verifyNotTree checkQuotesInLiterals "param='--foo=\"bar\"'; app \"$param\"" 2203prop_checkQuotesInLiterals3 =verifyNotTree checkQuotesInLiterals "param=('--foo='); app \"${param[@]}\"" 2204prop_checkQuotesInLiterals4 = verifyNotTree checkQuotesInLiterals "param=\"don't bother with this one\"; app $param" 2205prop_checkQuotesInLiterals5 = verifyNotTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; eval app $param" 2206prop_checkQuotesInLiterals6 = verifyTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm $param\"; $cmd" 2207prop_checkQuotesInLiterals6a= verifyNotTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm ${#param}\"; $cmd" 2208prop_checkQuotesInLiterals7 = verifyTree checkQuotesInLiterals "param='my\\ file'; rm $param" 2209prop_checkQuotesInLiterals8 = verifyTree checkQuotesInLiterals "param=\"/foo/'bar baz'/etc\"; rm $param" 2210prop_checkQuotesInLiterals9 = verifyNotTree checkQuotesInLiterals "param=\"/foo/'bar baz'/etc\"; rm ${#param}" 2211checkQuotesInLiterals params t = 2212 doVariableFlowAnalysis readF writeF Map.empty (variableFlow params) 2213 where 2214 getQuotes name = gets (Map.lookup name) 2215 setQuotes name ref = modify $ Map.insert name ref 2216 deleteQuotes = modify . Map.delete 2217 parents = parentMap params 2218 quoteRegex = mkRegex "\"|([/= ]|^)'|'( |$)|\\\\ " 2219 containsQuotes s = s `matches` quoteRegex 2220 2221 writeF _ _ name (DataString (SourceFrom values)) = do 2222 quoteMap <- get 2223 let quotedVars = msum $ map (forToken quoteMap) values 2224 case quotedVars of 2225 Nothing -> deleteQuotes name 2226 Just x -> setQuotes name x 2227 return [] 2228 writeF _ _ _ _ = return [] 2229 2230 forToken map (T_DollarBraced id _ t) = 2231 -- skip getBracedReference here to avoid false positives on PE 2232 Map.lookup (concat . oversimplify $ t) map 2233 forToken quoteMap (T_DoubleQuoted id tokens) = 2234 msum $ map (forToken quoteMap) tokens 2235 forToken quoteMap (T_NormalWord id tokens) = 2236 msum $ map (forToken quoteMap) tokens 2237 forToken _ t = 2238 if containsQuotes (concat $ oversimplify t) 2239 then return $ getId t 2240 else Nothing 2241 2242 squashesQuotes t = 2243 case t of 2244 T_DollarBraced id _ l -> "#" `isPrefixOf` concat (oversimplify l) 2245 _ -> False 2246 2247 readF _ expr name = do 2248 assignment <- getQuotes name 2249 return $ case assignment of 2250 Just j 2251 | not (isParamTo parents "eval" expr) 2252 && not (isQuoteFree (shellType params) parents expr) 2253 && not (squashesQuotes expr) 2254 -> [ 2255 makeComment WarningC j 2089 $ 2256 "Quotes/backslashes will be treated literally. " ++ suggestion, 2257 makeComment WarningC (getId expr) 2090 2258 "Quotes/backslashes in this variable will not be respected." 2259 ] 2260 _ -> [] 2261 suggestion = 2262 if supportsArrays (shellType params) 2263 then "Use an array." 2264 else "Rewrite using set/\"$@\" or functions." 2265 2266 2267prop_checkFunctionsUsedExternally1 = 2268 verifyTree checkFunctionsUsedExternally "foo() { :; }; sudo foo" 2269prop_checkFunctionsUsedExternally2 = 2270 verifyTree checkFunctionsUsedExternally "alias f='a'; xargs -0 f" 2271prop_checkFunctionsUsedExternally2b= 2272 verifyNotTree checkFunctionsUsedExternally "alias f='a'; find . -type f" 2273prop_checkFunctionsUsedExternally2c= 2274 verifyTree checkFunctionsUsedExternally "alias f='a'; find . -type f -exec f +" 2275prop_checkFunctionsUsedExternally3 = 2276 verifyNotTree checkFunctionsUsedExternally "f() { :; }; echo f" 2277prop_checkFunctionsUsedExternally4 = 2278 verifyNotTree checkFunctionsUsedExternally "foo() { :; }; sudo \"foo\"" 2279prop_checkFunctionsUsedExternally5 = 2280 verifyTree checkFunctionsUsedExternally "foo() { :; }; ssh host foo" 2281prop_checkFunctionsUsedExternally6 = 2282 verifyNotTree checkFunctionsUsedExternally "foo() { :; }; ssh host echo foo" 2283prop_checkFunctionsUsedExternally7 = 2284 verifyNotTree checkFunctionsUsedExternally "install() { :; }; sudo apt-get install foo" 2285prop_checkFunctionsUsedExternally8 = 2286 verifyTree checkFunctionsUsedExternally "foo() { :; }; command sudo foo" 2287prop_checkFunctionsUsedExternally9 = 2288 verifyTree checkFunctionsUsedExternally "foo() { :; }; exec -c sudo foo" 2289checkFunctionsUsedExternally params t = 2290 runNodeAnalysis checkCommand params t 2291 where 2292 checkCommand _ t@(T_SimpleCommand _ _ argv) = 2293 case getCommandNameAndToken False t of 2294 (Just str, t) -> do 2295 let name = basename str 2296 let args = skipOver t argv 2297 let argStrings = map (\x -> (fromMaybe "" $ getLiteralString x, x)) args 2298 let candidates = getPotentialCommands name argStrings 2299 mapM_ (checkArg name) candidates 2300 _ -> return () 2301 checkCommand _ _ = return () 2302 2303 skipOver t list = drop 1 $ dropWhile (\c -> getId c /= id) $ list 2304 where id = getId t 2305 2306 -- Try to pick out the argument[s] that may be commands 2307 getPotentialCommands name argAndString = 2308 case name of 2309 "chroot" -> firstNonFlag 2310 "screen" -> firstNonFlag 2311 "sudo" -> firstNonFlag 2312 "xargs" -> firstNonFlag 2313 "tmux" -> firstNonFlag 2314 "ssh" -> take 1 $ drop 1 $ dropFlags argAndString 2315 "find" -> take 1 $ drop 1 $ 2316 dropWhile (\x -> fst x `notElem` findExecFlags) argAndString 2317 _ -> [] 2318 where 2319 firstNonFlag = take 1 $ dropFlags argAndString 2320 findExecFlags = ["-exec", "-execdir", "-ok"] 2321 dropFlags = dropWhile (\x -> "-" `isPrefixOf` fst x) 2322 2323 functionsAndAliases = Map.union (functions t) (aliases t) 2324 2325 checkArg cmd (_, arg) = sequence_ $ do 2326 literalArg <- getUnquotedLiteral arg -- only consider unquoted literals 2327 definitionId <- Map.lookup literalArg functionsAndAliases 2328 return $ do 2329 warn (getId arg) 2033 2330 "Shell functions can't be passed to external commands." 2331 info definitionId 2032 $ 2332 "Use own script or sh -c '..' to run this from " ++ cmd ++ "." 2333 2334prop_checkUnused0 = verifyNotTree checkUnusedAssignments "var=foo; echo $var" 2335prop_checkUnused1 = verifyTree checkUnusedAssignments "var=foo; echo $bar" 2336prop_checkUnused2 = verifyNotTree checkUnusedAssignments "var=foo; export var;" 2337prop_checkUnused3 = verifyTree checkUnusedAssignments "for f in *; do echo '$f'; done" 2338prop_checkUnused4 = verifyTree checkUnusedAssignments "local i=0" 2339prop_checkUnused5 = verifyNotTree checkUnusedAssignments "read lol; echo $lol" 2340prop_checkUnused6 = verifyNotTree checkUnusedAssignments "var=4; (( var++ ))" 2341prop_checkUnused7 = verifyNotTree checkUnusedAssignments "var=2; $((var))" 2342prop_checkUnused8 = verifyTree checkUnusedAssignments "var=2; var=3;" 2343prop_checkUnused9 = verifyNotTree checkUnusedAssignments "read ''" 2344prop_checkUnused10= verifyNotTree checkUnusedAssignments "read -p 'test: '" 2345prop_checkUnused11= verifyNotTree checkUnusedAssignments "bar=5; export foo[$bar]=3" 2346prop_checkUnused12= verifyNotTree checkUnusedAssignments "read foo; echo ${!foo}" 2347prop_checkUnused13= verifyNotTree checkUnusedAssignments "x=(1); (( x[0] ))" 2348prop_checkUnused14= verifyNotTree checkUnusedAssignments "x=(1); n=0; echo ${x[n]}" 2349prop_checkUnused15= verifyNotTree checkUnusedAssignments "x=(1); n=0; (( x[n] ))" 2350prop_checkUnused16= verifyNotTree checkUnusedAssignments "foo=5; declare -x foo" 2351prop_checkUnused16b= verifyNotTree checkUnusedAssignments "f() { local -x foo; foo=42; bar; }; f" 2352prop_checkUnused17= verifyNotTree checkUnusedAssignments "read -i 'foo' -e -p 'Input: ' bar; $bar;" 2353prop_checkUnused18= verifyNotTree checkUnusedAssignments "a=1; arr=( [$a]=42 ); echo \"${arr[@]}\"" 2354prop_checkUnused19= verifyNotTree checkUnusedAssignments "a=1; let b=a+1; echo $b" 2355prop_checkUnused20= verifyNotTree checkUnusedAssignments "a=1; PS1='$a'" 2356prop_checkUnused21= verifyNotTree checkUnusedAssignments "a=1; trap 'echo $a' INT" 2357prop_checkUnused22= verifyNotTree checkUnusedAssignments "a=1; [ -v a ]" 2358prop_checkUnused23= verifyNotTree checkUnusedAssignments "a=1; [ -R a ]" 2359prop_checkUnused24= verifyNotTree checkUnusedAssignments "mapfile -C a b; echo ${b[@]}" 2360prop_checkUnused25= verifyNotTree checkUnusedAssignments "readarray foo; echo ${foo[@]}" 2361prop_checkUnused26= verifyNotTree checkUnusedAssignments "declare -F foo" 2362prop_checkUnused27= verifyTree checkUnusedAssignments "var=3; [ var -eq 3 ]" 2363prop_checkUnused28= verifyNotTree checkUnusedAssignments "var=3; [[ var -eq 3 ]]" 2364prop_checkUnused29= verifyNotTree checkUnusedAssignments "var=(a b); declare -p var" 2365prop_checkUnused30= verifyTree checkUnusedAssignments "let a=1" 2366prop_checkUnused31= verifyTree checkUnusedAssignments "let 'a=1'" 2367prop_checkUnused32= verifyTree checkUnusedAssignments "let a=b=c; echo $a" 2368prop_checkUnused33= verifyNotTree checkUnusedAssignments "a=foo; [[ foo =~ ^{$a}$ ]]" 2369prop_checkUnused34= verifyNotTree checkUnusedAssignments "foo=1; (( t = foo )); echo $t" 2370prop_checkUnused35= verifyNotTree checkUnusedAssignments "a=foo; b=2; echo ${a:b}" 2371prop_checkUnused36= verifyNotTree checkUnusedAssignments "if [[ -v foo ]]; then true; fi" 2372prop_checkUnused37= verifyNotTree checkUnusedAssignments "fd=2; exec {fd}>&-" 2373prop_checkUnused38= verifyTree checkUnusedAssignments "(( a=42 ))" 2374prop_checkUnused39= verifyNotTree checkUnusedAssignments "declare -x -f foo" 2375prop_checkUnused40= verifyNotTree checkUnusedAssignments "arr=(1 2); num=2; echo \"${arr[@]:num}\"" 2376prop_checkUnused41= verifyNotTree checkUnusedAssignments "@test 'foo' {\ntrue\n}\n" 2377prop_checkUnused42= verifyNotTree checkUnusedAssignments "DEFINE_string foo '' ''; echo \"${FLAGS_foo}\"" 2378prop_checkUnused43= verifyTree checkUnusedAssignments "DEFINE_string foo '' ''" 2379prop_checkUnused44= verifyNotTree checkUnusedAssignments "DEFINE_string \"foo$ibar\" x y" 2380prop_checkUnused45= verifyTree checkUnusedAssignments "readonly foo=bar" 2381prop_checkUnused46= verifyTree checkUnusedAssignments "readonly foo=(bar)" 2382prop_checkUnused47= verifyNotTree checkUnusedAssignments "a=1; alias hello='echo $a'" 2383prop_checkUnused48= verifyNotTree checkUnusedAssignments "_a=1" 2384prop_checkUnused49= verifyNotTree checkUnusedAssignments "declare -A array; key=a; [[ -v array[$key] ]]" 2385prop_checkUnused50= verifyNotTree checkUnusedAssignments "foofunc() { :; }; typeset -fx foofunc" 2386 2387checkUnusedAssignments params t = execWriter (mapM_ warnFor unused) 2388 where 2389 flow = variableFlow params 2390 references = foldl (flip ($)) defaultMap (map insertRef flow) 2391 insertRef (Reference (base, token, name)) = 2392 Map.insert (stripSuffix name) () 2393 insertRef _ = id 2394 2395 assignments = foldl (flip ($)) Map.empty (map insertAssignment flow) 2396 insertAssignment (Assignment (_, token, name, _)) | isVariableName name = 2397 Map.insert name token 2398 insertAssignment _ = id 2399 2400 unused = Map.assocs $ Map.difference assignments references 2401 2402 warnFor (name, token) = 2403 unless ("_" `isPrefixOf` name) $ 2404 warn (getId token) 2034 $ 2405 name ++ " appears unused. Verify use (or export if used externally)." 2406 2407 stripSuffix = takeWhile isVariableChar 2408 defaultMap = Map.fromList $ zip internalVariables $ repeat () 2409 2410prop_checkUnassignedReferences1 = verifyTree checkUnassignedReferences "echo $foo" 2411prop_checkUnassignedReferences2 = verifyNotTree checkUnassignedReferences "foo=hello; echo $foo" 2412prop_checkUnassignedReferences3 = verifyTree checkUnassignedReferences "MY_VALUE=3; echo $MYVALUE" 2413prop_checkUnassignedReferences4 = verifyNotTree checkUnassignedReferences "RANDOM2=foo; echo $RANDOM" 2414prop_checkUnassignedReferences5 = verifyNotTree checkUnassignedReferences "declare -A foo=([bar]=baz); echo ${foo[bar]}" 2415prop_checkUnassignedReferences6 = verifyNotTree checkUnassignedReferences "foo=..; echo ${foo-bar}" 2416prop_checkUnassignedReferences7 = verifyNotTree checkUnassignedReferences "getopts ':h' foo; echo $foo" 2417prop_checkUnassignedReferences8 = verifyNotTree checkUnassignedReferences "let 'foo = 1'; echo $foo" 2418prop_checkUnassignedReferences9 = verifyNotTree checkUnassignedReferences "echo ${foo-bar}" 2419prop_checkUnassignedReferences10= verifyNotTree checkUnassignedReferences "echo ${foo:?}" 2420prop_checkUnassignedReferences11= verifyNotTree checkUnassignedReferences "declare -A foo; echo \"${foo[@]}\"" 2421prop_checkUnassignedReferences12= verifyNotTree checkUnassignedReferences "typeset -a foo; echo \"${foo[@]}\"" 2422prop_checkUnassignedReferences13= verifyNotTree checkUnassignedReferences "f() { local foo; echo $foo; }" 2423prop_checkUnassignedReferences14= verifyNotTree checkUnassignedReferences "foo=; echo $foo" 2424prop_checkUnassignedReferences15= verifyNotTree checkUnassignedReferences "f() { true; }; export -f f" 2425prop_checkUnassignedReferences16= verifyNotTree checkUnassignedReferences "declare -A foo=( [a b]=bar ); echo ${foo[a b]}" 2426prop_checkUnassignedReferences17= verifyNotTree checkUnassignedReferences "USERS=foo; echo $USER" 2427prop_checkUnassignedReferences18= verifyNotTree checkUnassignedReferences "FOOBAR=42; export FOOBAR=" 2428prop_checkUnassignedReferences19= verifyNotTree checkUnassignedReferences "readonly foo=bar; echo $foo" 2429prop_checkUnassignedReferences20= verifyNotTree checkUnassignedReferences "printf -v foo bar; echo $foo" 2430prop_checkUnassignedReferences21= verifyTree checkUnassignedReferences "echo ${#foo}" 2431prop_checkUnassignedReferences22= verifyNotTree checkUnassignedReferences "echo ${!os*}" 2432prop_checkUnassignedReferences23= verifyTree checkUnassignedReferences "declare -a foo; foo[bar]=42;" 2433prop_checkUnassignedReferences24= verifyNotTree checkUnassignedReferences "declare -A foo; foo[bar]=42;" 2434prop_checkUnassignedReferences25= verifyNotTree checkUnassignedReferences "declare -A foo=(); foo[bar]=42;" 2435prop_checkUnassignedReferences26= verifyNotTree checkUnassignedReferences "a::b() { foo; }; readonly -f a::b" 2436prop_checkUnassignedReferences27= verifyNotTree checkUnassignedReferences ": ${foo:=bar}" 2437prop_checkUnassignedReferences28= verifyNotTree checkUnassignedReferences "#!/bin/ksh\necho \"${.sh.version}\"\n" 2438prop_checkUnassignedReferences29= verifyNotTree checkUnassignedReferences "if [[ -v foo ]]; then echo $foo; fi" 2439prop_checkUnassignedReferences30= verifyNotTree checkUnassignedReferences "if [[ -v foo[3] ]]; then echo ${foo[3]}; fi" 2440prop_checkUnassignedReferences31= verifyNotTree checkUnassignedReferences "X=1; if [[ -v foo[$X+42] ]]; then echo ${foo[$X+42]}; fi" 2441prop_checkUnassignedReferences32= verifyNotTree checkUnassignedReferences "if [[ -v \"foo[1]\" ]]; then echo ${foo[@]}; fi" 2442prop_checkUnassignedReferences33= verifyNotTree checkUnassignedReferences "f() { local -A foo; echo \"${foo[@]}\"; }" 2443prop_checkUnassignedReferences34= verifyNotTree checkUnassignedReferences "declare -A foo; (( foo[bar] ))" 2444prop_checkUnassignedReferences35= verifyNotTree checkUnassignedReferences "echo ${arr[foo-bar]:?fail}" 2445prop_checkUnassignedReferences36= verifyNotTree checkUnassignedReferences "read -a foo -r <<<\"foo bar\"; echo \"$foo\"" 2446prop_checkUnassignedReferences37= verifyNotTree checkUnassignedReferences "var=howdy; printf -v 'array[0]' %s \"$var\"; printf %s \"${array[0]}\";" 2447prop_checkUnassignedReferences38= verifyTree (checkUnassignedReferences' True) "echo $VAR" 2448prop_checkUnassignedReferences39= verifyNotTree checkUnassignedReferences "builtin export var=4; echo $var" 2449prop_checkUnassignedReferences40= verifyNotTree checkUnassignedReferences ": ${foo=bar}" 2450prop_checkUnassignedReferences41= verifyNotTree checkUnassignedReferences "mapfile -t files 123; echo \"${files[@]}\"" 2451prop_checkUnassignedReferences42= verifyNotTree checkUnassignedReferences "mapfile files -t; echo \"${files[@]}\"" 2452prop_checkUnassignedReferences43= verifyNotTree checkUnassignedReferences "mapfile --future files; echo \"${files[@]}\"" 2453prop_checkUnassignedReferences_minusNPlain = verifyNotTree checkUnassignedReferences "if [ -n \"$x\" ]; then echo $x; fi" 2454prop_checkUnassignedReferences_minusZPlain = verifyNotTree checkUnassignedReferences "if [ -z \"$x\" ]; then echo \"\"; fi" 2455prop_checkUnassignedReferences_minusNBraced = verifyNotTree checkUnassignedReferences "if [ -n \"${x}\" ]; then echo $x; fi" 2456prop_checkUnassignedReferences_minusZBraced = verifyNotTree checkUnassignedReferences "if [ -z \"${x}\" ]; then echo \"\"; fi" 2457prop_checkUnassignedReferences_minusNDefault = verifyNotTree checkUnassignedReferences "if [ -n \"${x:-}\" ]; then echo $x; fi" 2458prop_checkUnassignedReferences_minusZDefault = verifyNotTree checkUnassignedReferences "if [ -z \"${x:-}\" ]; then echo \"\"; fi" 2459prop_checkUnassignedReferences50 = verifyNotTree checkUnassignedReferences "echo ${foo:+bar}" 2460prop_checkUnassignedReferences51 = verifyNotTree checkUnassignedReferences "echo ${foo:+$foo}" 2461prop_checkUnassignedReferences52 = verifyNotTree checkUnassignedReferences "wait -p pid; echo $pid" 2462 2463checkUnassignedReferences = checkUnassignedReferences' False 2464checkUnassignedReferences' includeGlobals params t = warnings 2465 where 2466 (readMap, writeMap) = execState (mapM tally $ variableFlow params) (Map.empty, Map.empty) 2467 defaultAssigned = Map.fromList $ map (\a -> (a, ())) $ filter (not . null) internalVariables 2468 2469 tally (Assignment (_, _, name, _)) = 2470 modify (\(read, written) -> (read, Map.insert name () written)) 2471 tally (Reference (_, place, name)) = 2472 modify (\(read, written) -> (Map.insertWith (const id) name place read, written)) 2473 tally _ = return () 2474 2475 unassigned = Map.toList $ Map.difference (Map.difference readMap writeMap) defaultAssigned 2476 writtenVars = filter isVariableName $ Map.keys writeMap 2477 2478 getBestMatch var = do 2479 (match, score) <- listToMaybe best 2480 guard $ goodMatch var match score 2481 return match 2482 where 2483 matches = map (\x -> (x, match var x)) writtenVars 2484 best = sortBy (comparing snd) matches 2485 goodMatch var match score = 2486 let l = length match in 2487 l > 3 && score <= 1 2488 || l > 7 && score <= 2 2489 2490 isLocal = any isLower 2491 2492 warningForGlobals var place = do 2493 match <- getBestMatch var 2494 return $ info (getId place) 2153 $ 2495 "Possible misspelling: " ++ var ++ " may not be assigned. Did you mean " ++ match ++ "?" 2496 2497 warningForLocals var place = 2498 return $ warn (getId place) 2154 $ 2499 var ++ " is referenced but not assigned" ++ optionalTip ++ "." 2500 where 2501 optionalTip = 2502 if var `elem` commonCommands 2503 then " (for output from commands, use \"$(" ++ var ++ " ..." ++ ")\" )" 2504 else fromMaybe "" $ do 2505 match <- getBestMatch var 2506 return $ " (did you mean '" ++ match ++ "'?)" 2507 2508 warningFor (var, place) = do 2509 guard $ isVariableName var 2510 guard . not $ isException var place || isGuarded place 2511 (if includeGlobals || isLocal var 2512 then warningForLocals 2513 else warningForGlobals) var place 2514 2515 warnings = execWriter . sequence $ mapMaybe warningFor unassigned 2516 2517 -- Due to parsing, foo=( [bar]=baz ) parses 'bar' as a reference even for assoc arrays. 2518 -- Similarly, ${foo[bar baz]} may not be referencing bar/baz. Just skip these. 2519 -- We can also have ${foo:+$foo} should be treated like [[ -n $foo ]] && echo $foo 2520 isException var t = any shouldExclude $ getPath (parentMap params) t 2521 where 2522 shouldExclude t = 2523 case t of 2524 T_Array {} -> True 2525 (T_DollarBraced _ _ l) -> 2526 let str = concat $ oversimplify l 2527 ref = getBracedReference str 2528 mod = getBracedModifier str 2529 in 2530 -- Either we're used as an array index like ${arr[here]} 2531 ref /= var || 2532 -- or the reference is guarded by a parent, ${here:+foo$here} 2533 "+" `isPrefixOf` mod || ":+" `isPrefixOf` mod 2534 _ -> False 2535 2536 isGuarded (T_DollarBraced _ _ v) = 2537 rest `matches` guardRegex 2538 where 2539 name = concat $ oversimplify v 2540 rest = dropWhile isVariableChar $ dropWhile (`elem` "#!") name 2541 isGuarded _ = False 2542 -- :? or :- with optional array index and colon 2543 guardRegex = mkRegex "^(\\[.*\\])?:?[-?]" 2544 2545 match var candidate = 2546 if var /= candidate && map toLower var == map toLower candidate 2547 then 1 2548 else dist var candidate 2549 2550 2551prop_checkGlobsAsOptions1 = verify checkGlobsAsOptions "rm *.txt" 2552prop_checkGlobsAsOptions2 = verify checkGlobsAsOptions "ls ??.*" 2553prop_checkGlobsAsOptions3 = verifyNot checkGlobsAsOptions "rm -- *.txt" 2554prop_checkGlobsAsOptions4 = verifyNot checkGlobsAsOptions "*.txt" 2555prop_checkGlobsAsOptions5 = verifyNot checkGlobsAsOptions "echo 'Files:' *.txt" 2556prop_checkGlobsAsOptions6 = verifyNot checkGlobsAsOptions "printf '%s\\n' *" 2557checkGlobsAsOptions _ cmd@(T_SimpleCommand _ _ args) = 2558 unless ((fromMaybe "" $ getCommandBasename cmd) `elem` ["echo", "printf"]) $ 2559 mapM_ check $ takeWhile (not . isEndOfArgs) (drop 1 args) 2560 where 2561 check v@(T_NormalWord _ (T_Glob id s:_)) | s == "*" || s == "?" = 2562 info id 2035 "Use ./*glob* or -- *glob* so names with dashes won't become options." 2563 check _ = return () 2564 2565 isEndOfArgs t = 2566 case concat $ oversimplify t of 2567 "--" -> True 2568 ":::" -> True 2569 "::::" -> True 2570 _ -> False 2571 2572checkGlobsAsOptions _ _ = return () 2573 2574 2575prop_checkWhileReadPitfalls1 = verify checkWhileReadPitfalls "while read foo; do ssh $foo uptime; done < file" 2576prop_checkWhileReadPitfalls2 = verifyNot checkWhileReadPitfalls "while read -u 3 foo; do ssh $foo uptime; done 3< file" 2577prop_checkWhileReadPitfalls3 = verifyNot checkWhileReadPitfalls "while true; do ssh host uptime; done" 2578prop_checkWhileReadPitfalls4 = verifyNot checkWhileReadPitfalls "while read foo; do ssh $foo hostname < /dev/null; done" 2579prop_checkWhileReadPitfalls5 = verifyNot checkWhileReadPitfalls "while read foo; do echo ls | ssh $foo; done" 2580prop_checkWhileReadPitfalls6 = verifyNot checkWhileReadPitfalls "while read foo <&3; do ssh $foo; done 3< foo" 2581prop_checkWhileReadPitfalls7 = verify checkWhileReadPitfalls "while read foo; do if true; then ssh $foo uptime; fi; done < file" 2582prop_checkWhileReadPitfalls8 = verifyNot checkWhileReadPitfalls "while read foo; do ssh -n $foo uptime; done < file" 2583prop_checkWhileReadPitfalls9 = verify checkWhileReadPitfalls "while read foo; do ffmpeg -i foo.mkv bar.mkv -an; done" 2584prop_checkWhileReadPitfalls10 = verify checkWhileReadPitfalls "while read foo; do mplayer foo.ogv > file; done" 2585prop_checkWhileReadPitfalls11 = verifyNot checkWhileReadPitfalls "while read foo; do mplayer foo.ogv <<< q; done" 2586prop_checkWhileReadPitfalls12 = verifyNot checkWhileReadPitfalls "while read foo\ndo\nmplayer foo.ogv << EOF\nq\nEOF\ndone" 2587prop_checkWhileReadPitfalls13 = verify checkWhileReadPitfalls "while read foo; do x=$(ssh host cmd); done" 2588prop_checkWhileReadPitfalls14 = verify checkWhileReadPitfalls "while read foo; do echo $(ssh host cmd) < /dev/null; done" 2589prop_checkWhileReadPitfalls15 = verify checkWhileReadPitfalls "while read foo; do ssh $foo cmd & done" 2590 2591checkWhileReadPitfalls params (T_WhileExpression id [command] contents) 2592 | isStdinReadCommand command = 2593 mapM_ checkMuncher contents 2594 where 2595 -- Map of munching commands to a function that checks if the flags should exclude it 2596 munchers = Map.fromList [ 2597 ("ssh", (hasFlag, addFlag, "-n")), 2598 ("ffmpeg", (hasArgument, addFlag, "-nostdin")), 2599 ("mplayer", (hasArgument, addFlag, "-noconsolecontrols")), 2600 ("HandBrakeCLI", (\_ _ -> False, addRedirect, "< /dev/null")) 2601 ] 2602 -- Use flag parsing, e.g. "-an" -> "a", "n" 2603 hasFlag ('-':flag) = elem flag . map snd . getAllFlags 2604 -- Simple string match, e.g. "-an" -> "-an" 2605 hasArgument arg = elem arg . mapMaybe getLiteralString . fromJust . getCommandArgv 2606 addFlag string cmd = fixWith [replaceEnd (getId $ getCommandTokenOrThis cmd) params 0 (' ':string)] 2607 addRedirect string cmd = fixWith [replaceEnd (getId cmd) params 0 (' ':string)] 2608 2609 isStdinReadCommand (T_Pipeline _ _ [T_Redirecting id redirs cmd]) = 2610 let plaintext = oversimplify cmd 2611 in headOrDefault "" plaintext == "read" 2612 && ("-u" `notElem` plaintext) 2613 && not (any stdinRedirect redirs) 2614 isStdinReadCommand _ = False 2615 2616 checkMuncher :: Token -> Writer [TokenComment] () 2617 checkMuncher (T_Pipeline _ _ (T_Redirecting _ redirs cmd:_)) = do 2618 -- Check command substitutions regardless of the command 2619 case cmd of 2620 T_SimpleCommand _ vars args -> 2621 mapM_ checkMuncher $ concat $ concatMap getCommandSequences $ concatMap getWords $ vars ++ args 2622 _ -> return () 2623 2624 unless (any stdinRedirect redirs) $ do 2625 -- Recurse into ifs/loops/groups/etc if this doesn't redirect 2626 mapM_ checkMuncher $ concat $ getCommandSequences cmd 2627 2628 -- Check the actual command 2629 sequence_ $ do 2630 name <- getCommandBasename cmd 2631 (check, fix, flag) <- Map.lookup name munchers 2632 guard $ not (check flag cmd) 2633 2634 return $ do 2635 info id 2095 $ 2636 name ++ " may swallow stdin, preventing this loop from working properly." 2637 warnWithFix (getId cmd) 2095 2638 ("Use " ++ name ++ " " ++ flag ++ " to prevent " ++ name ++ " from swallowing stdin.") 2639 (fix flag cmd) 2640 checkMuncher (T_Backgrounded _ t) = checkMuncher t 2641 checkMuncher _ = return () 2642 2643 stdinRedirect (T_FdRedirect _ fd op) 2644 | fd == "0" = True 2645 | fd == "" = 2646 case op of 2647 T_IoFile _ (T_Less _) _ -> True 2648 T_IoDuplicate _ (T_LESSAND _) _ -> True 2649 T_HereString _ _ -> True 2650 T_HereDoc {} -> True 2651 _ -> False 2652 stdinRedirect _ = False 2653 2654 getWords t = 2655 case t of 2656 T_Assignment _ _ _ _ x -> getWordParts x 2657 _ -> getWordParts t 2658checkWhileReadPitfalls _ _ = return () 2659 2660 2661prop_checkPrefixAssign1 = verify checkPrefixAssignmentReference "var=foo echo $var" 2662prop_checkPrefixAssign2 = verifyNot checkPrefixAssignmentReference "var=$(echo $var) cmd" 2663checkPrefixAssignmentReference params t@(T_DollarBraced id _ value) = 2664 check path 2665 where 2666 name = getBracedReference $ concat $ oversimplify value 2667 path = getPath (parentMap params) t 2668 idPath = map getId path 2669 2670 check [] = return () 2671 check (t:rest) = 2672 case t of 2673 T_SimpleCommand _ vars (_:_) -> mapM_ checkVar vars 2674 _ -> check rest 2675 checkVar (T_Assignment aId mode aName [] value) | 2676 aName == name && (aId `notElem` idPath) = do 2677 warn aId 2097 "This assignment is only seen by the forked process." 2678 warn id 2098 "This expansion will not see the mentioned assignment." 2679 checkVar _ = return () 2680 2681checkPrefixAssignmentReference _ _ = return () 2682 2683prop_checkCharRangeGlob1 = verify checkCharRangeGlob "ls *[:digit:].jpg" 2684prop_checkCharRangeGlob2 = verifyNot checkCharRangeGlob "ls *[[:digit:]].jpg" 2685prop_checkCharRangeGlob3 = verify checkCharRangeGlob "ls [10-15]" 2686prop_checkCharRangeGlob4 = verifyNot checkCharRangeGlob "ls [a-zA-Z]" 2687prop_checkCharRangeGlob5 = verifyNot checkCharRangeGlob "tr -d [aa]" -- tr has 2060 2688prop_checkCharRangeGlob6 = verifyNot checkCharRangeGlob "[[ $x == [!!]* ]]" 2689prop_checkCharRangeGlob7 = verifyNot checkCharRangeGlob "[[ -v arr[keykey] ]]" 2690prop_checkCharRangeGlob8 = verifyNot checkCharRangeGlob "[[ arr[keykey] -gt 1 ]]" 2691prop_checkCharRangeGlob9 = verifyNot checkCharRangeGlob "read arr[keykey]" -- tr has 2313 2692checkCharRangeGlob p t@(T_Glob id str) | 2693 isCharClass str && not isIgnoredCommand && not (isDereferenced t) = 2694 if ":" `isPrefixOf` contents 2695 && ":" `isSuffixOf` contents 2696 && contents /= ":" 2697 then warn id 2101 "Named class needs outer [], e.g. [[:digit:]]." 2698 else 2699 when ('[' `notElem` contents && hasDupes) $ 2700 info id 2102 "Ranges can only match single chars (mentioned due to duplicates)." 2701 where 2702 isCharClass str = "[" `isPrefixOf` str && "]" `isSuffixOf` str 2703 contents = dropNegation . drop 1 . take (length str - 1) $ str 2704 hasDupes = any ((>1) . length) . group . sort . filter (/= '-') $ contents 2705 dropNegation s = 2706 case s of 2707 '!':rest -> rest 2708 '^':rest -> rest 2709 x -> x 2710 2711 isIgnoredCommand = fromMaybe False $ do 2712 cmd <- getClosestCommand (parentMap p) t 2713 return $ isCommandMatch cmd (`elem` ["tr", "read"]) 2714 2715 -- Check if this is a dereferencing context like [[ -v array[operandhere] ]] 2716 isDereferenced = fromMaybe False . msum . map isDereferencingOp . getPath (parentMap p) 2717 isDereferencingOp t = 2718 case t of 2719 TC_Binary _ DoubleBracket str _ _ -> return $ isDereferencingBinaryOp str 2720 TC_Unary _ _ str _ -> return $ str == "-v" 2721 T_SimpleCommand {} -> return False 2722 _ -> Nothing 2723checkCharRangeGlob _ _ = return () 2724 2725 2726 2727prop_checkCdAndBack1 = verify checkCdAndBack "for f in *; do cd $f; git pull; cd ..; done" 2728prop_checkCdAndBack2 = verifyNot checkCdAndBack "for f in *; do cd $f || continue; git pull; cd ..; done" 2729prop_checkCdAndBack3 = verifyNot checkCdAndBack "while [[ $PWD != / ]]; do cd ..; done" 2730prop_checkCdAndBack4 = verify checkCdAndBack "cd $tmp; foo; cd -" 2731prop_checkCdAndBack5 = verifyNot checkCdAndBack "cd ..; foo; cd .." 2732prop_checkCdAndBack6 = verify checkCdAndBack "for dir in */; do cd \"$dir\"; some_cmd; cd ..; done" 2733prop_checkCdAndBack7 = verifyNot checkCdAndBack "set -e; for dir in */; do cd \"$dir\"; some_cmd; cd ..; done" 2734prop_checkCdAndBack8 = verifyNot checkCdAndBack "cd tmp\nfoo\n# shellcheck disable=SC2103\ncd ..\n" 2735checkCdAndBack params t = 2736 unless (hasSetE params) $ mapM_ doList $ getCommandSequences t 2737 where 2738 isCdRevert t = 2739 case oversimplify t of 2740 [_, p] -> p `elem` ["..", "-"] 2741 _ -> False 2742 2743 getCandidate (T_Annotation _ _ x) = getCandidate x 2744 getCandidate (T_Pipeline id _ [x]) | x `isCommand` "cd" = return x 2745 getCandidate _ = Nothing 2746 2747 findCdPair list = 2748 case list of 2749 (a:b:rest) -> 2750 if isCdRevert b && not (isCdRevert a) 2751 then return $ getId b 2752 else findCdPair (b:rest) 2753 _ -> Nothing 2754 2755 doList list = sequence_ $ do 2756 cd <- findCdPair $ mapMaybe getCandidate list 2757 return $ info cd 2103 "Use a ( subshell ) to avoid having to cd back." 2758 2759prop_checkLoopKeywordScope1 = verify checkLoopKeywordScope "continue 2" 2760prop_checkLoopKeywordScope2 = verify checkLoopKeywordScope "for f; do ( break; ); done" 2761prop_checkLoopKeywordScope3 = verify checkLoopKeywordScope "if true; then continue; fi" 2762prop_checkLoopKeywordScope4 = verifyNot checkLoopKeywordScope "while true; do break; done" 2763prop_checkLoopKeywordScope5 = verify checkLoopKeywordScope "if true; then break; fi" 2764prop_checkLoopKeywordScope6 = verify checkLoopKeywordScope "while true; do true | { break; }; done" 2765prop_checkLoopKeywordScope7 = verifyNot checkLoopKeywordScope "#!/bin/ksh\nwhile true; do true | { break; }; done" 2766checkLoopKeywordScope params t | 2767 name `elem` map Just ["continue", "break"] = 2768 if not $ any isLoop path 2769 then if any isFunction $ take 1 path 2770 -- breaking at a source/function invocation is an abomination. Let's ignore it. 2771 then err (getId t) 2104 $ "In functions, use return instead of " ++ fromJust name ++ "." 2772 else err (getId t) 2105 $ fromJust name ++ " is only valid in loops." 2773 else case map subshellType $ filter (not . isFunction) path of 2774 Just str:_ -> warn (getId t) 2106 $ 2775 "This only exits the subshell caused by the " ++ str ++ "." 2776 _ -> return () 2777 where 2778 name = getCommandName t 2779 path = let p = getPath (parentMap params) t in filter relevant p 2780 subshellType t = case leadType params t of 2781 NoneScope -> Nothing 2782 SubshellScope str -> return str 2783 relevant t = isLoop t || isFunction t || isJust (subshellType t) 2784checkLoopKeywordScope _ _ = return () 2785 2786 2787prop_checkFunctionDeclarations1 = verify checkFunctionDeclarations "#!/bin/ksh\nfunction foo() { command foo --lol \"$@\"; }" 2788prop_checkFunctionDeclarations2 = verify checkFunctionDeclarations "#!/bin/dash\nfunction foo { lol; }" 2789prop_checkFunctionDeclarations3 = verifyNot checkFunctionDeclarations "foo() { echo bar; }" 2790checkFunctionDeclarations params 2791 (T_Function id (FunctionKeyword hasKeyword) (FunctionParentheses hasParens) _ _) = 2792 case shellType params of 2793 Bash -> return () 2794 Ksh -> 2795 when (hasKeyword && hasParens) $ 2796 err id 2111 "ksh does not allow 'function' keyword and '()' at the same time." 2797 Dash -> forSh 2798 Sh -> forSh 2799 2800 where 2801 forSh = do 2802 when (hasKeyword && hasParens) $ 2803 warn id 2112 "'function' keyword is non-standard. Delete it." 2804 when (hasKeyword && not hasParens) $ 2805 warn id 2113 "'function' keyword is non-standard. Use 'foo()' instead of 'function foo'." 2806checkFunctionDeclarations _ _ = return () 2807 2808 2809 2810prop_checkStderrPipe1 = verify checkStderrPipe "#!/bin/ksh\nfoo |& bar" 2811prop_checkStderrPipe2 = verifyNot checkStderrPipe "#!/bin/bash\nfoo |& bar" 2812checkStderrPipe params = 2813 case shellType params of 2814 Ksh -> match 2815 _ -> const $ return () 2816 where 2817 match (T_Pipe id "|&") = 2818 err id 2118 "Ksh does not support |&. Use 2>&1 |." 2819 match _ = return () 2820 2821prop_checkUnpassedInFunctions1 = verifyTree checkUnpassedInFunctions "foo() { echo $1; }; foo" 2822prop_checkUnpassedInFunctions2 = verifyNotTree checkUnpassedInFunctions "foo() { echo $1; };" 2823prop_checkUnpassedInFunctions3 = verifyNotTree checkUnpassedInFunctions "foo() { echo $lol; }; foo" 2824prop_checkUnpassedInFunctions4 = verifyNotTree checkUnpassedInFunctions "foo() { echo $0; }; foo" 2825prop_checkUnpassedInFunctions5 = verifyNotTree checkUnpassedInFunctions "foo() { echo $1; }; foo 'lol'; foo" 2826prop_checkUnpassedInFunctions6 = verifyNotTree checkUnpassedInFunctions "foo() { set -- *; echo $1; }; foo" 2827prop_checkUnpassedInFunctions7 = verifyTree checkUnpassedInFunctions "foo() { echo $1; }; foo; foo;" 2828prop_checkUnpassedInFunctions8 = verifyNotTree checkUnpassedInFunctions "foo() { echo $((1)); }; foo;" 2829prop_checkUnpassedInFunctions9 = verifyNotTree checkUnpassedInFunctions "foo() { echo $(($b)); }; foo;" 2830prop_checkUnpassedInFunctions10= verifyNotTree checkUnpassedInFunctions "foo() { echo $!; }; foo;" 2831prop_checkUnpassedInFunctions11= verifyNotTree checkUnpassedInFunctions "foo() { bar() { echo $1; }; bar baz; }; foo;" 2832prop_checkUnpassedInFunctions12= verifyNotTree checkUnpassedInFunctions "foo() { echo ${!var*}; }; foo;" 2833prop_checkUnpassedInFunctions13= verifyNotTree checkUnpassedInFunctions "# shellcheck disable=SC2120\nfoo() { echo $1; }\nfoo\n" 2834prop_checkUnpassedInFunctions14= verifyTree checkUnpassedInFunctions "foo() { echo $#; }; foo" 2835checkUnpassedInFunctions params root = 2836 execWriter $ mapM_ warnForGroup referenceGroups 2837 where 2838 functionMap :: Map.Map String Token 2839 functionMap = Map.fromList $ 2840 map (\t@(T_Function _ _ _ name _) -> (name,t)) functions 2841 functions = execWriter $ doAnalysis (tell . maybeToList . findFunction) root 2842 2843 findFunction t@(T_Function id _ _ name body) 2844 | any (isPositionalReference t) flow && not (any isPositionalAssignment flow) 2845 = return t 2846 where flow = getVariableFlow params body 2847 findFunction _ = Nothing 2848 2849 isPositionalAssignment x = 2850 case x of 2851 Assignment (_, _, str, _) -> isPositional str 2852 _ -> False 2853 isPositionalReference function x = 2854 case x of 2855 Reference (_, t, str) -> isPositional str && t `isDirectChildOf` function 2856 _ -> False 2857 2858 isDirectChildOf child parent = fromMaybe False $ do 2859 function <- find (\x -> case x of 2860 T_Function {} -> True 2861 T_Script {} -> True -- for sourced files 2862 _ -> False) $ 2863 getPath (parentMap params) child 2864 return $ getId parent == getId function 2865 2866 referenceList :: [(String, Bool, Token)] 2867 referenceList = execWriter $ 2868 doAnalysis (sequence_ . checkCommand) root 2869 checkCommand :: Token -> Maybe (Writer [(String, Bool, Token)] ()) 2870 checkCommand t@(T_SimpleCommand _ _ (cmd:args)) = do 2871 str <- getLiteralString cmd 2872 guard $ Map.member str functionMap 2873 return $ tell [(str, null args, t)] 2874 checkCommand _ = Nothing 2875 2876 isPositional str = str == "*" || str == "@" || str == "#" 2877 || (all isDigit str && str /= "0" && str /= "") 2878 2879 isArgumentless (_, b, _) = b 2880 referenceGroups = Map.elems $ foldr updateWith Map.empty referenceList 2881 updateWith x@(name, _, _) = Map.insertWith (++) name [x] 2882 2883 warnForGroup group = 2884 -- Allow ignoring SC2120 on the function to ignore all calls 2885 when (all isArgumentless group && not ignoring) $ do 2886 mapM_ suggestParams group 2887 warnForDeclaration func name 2888 where (name, func) = getFunction group 2889 ignoring = shouldIgnoreCode params 2120 func 2890 2891 suggestParams (name, _, thing) = 2892 info (getId thing) 2119 $ 2893 "Use " ++ (e4m name) ++ " \"$@\" if function's $1 should mean script's $1." 2894 warnForDeclaration func name = 2895 warn (getId func) 2120 $ 2896 name ++ " references arguments, but none are ever passed." 2897 2898 getFunction ((name, _, _):_) = 2899 (name, functionMap Map.! name) 2900 2901 2902prop_checkOverridingPath1 = verify checkOverridingPath "PATH=\"$var/$foo\"" 2903prop_checkOverridingPath2 = verify checkOverridingPath "PATH=\"mydir\"" 2904prop_checkOverridingPath3 = verify checkOverridingPath "PATH=/cow/foo" 2905prop_checkOverridingPath4 = verifyNot checkOverridingPath "PATH=/cow/foo/bin" 2906prop_checkOverridingPath5 = verifyNot checkOverridingPath "PATH='/bin:/sbin'" 2907prop_checkOverridingPath6 = verifyNot checkOverridingPath "PATH=\"$var/$foo\" cmd" 2908prop_checkOverridingPath7 = verifyNot checkOverridingPath "PATH=$OLDPATH" 2909prop_checkOverridingPath8 = verifyNot checkOverridingPath "PATH=$PATH:/stuff" 2910checkOverridingPath _ (T_SimpleCommand _ vars []) = 2911 mapM_ checkVar vars 2912 where 2913 checkVar (T_Assignment id Assign "PATH" [] word) 2914 | not $ any (`isInfixOf` string) ["/bin", "/sbin" ] = do 2915 when ('/' `elem` string && ':' `notElem` string) $ notify id 2916 when (isLiteral word && ':' `notElem` string && '/' `notElem` string) $ notify id 2917 where string = concat $ oversimplify word 2918 checkVar _ = return () 2919 notify id = warn id 2123 "PATH is the shell search path. Use another name." 2920checkOverridingPath _ _ = return () 2921 2922prop_checkTildeInPath1 = verify checkTildeInPath "PATH=\"$PATH:~/bin\"" 2923prop_checkTildeInPath2 = verify checkTildeInPath "PATH='~foo/bin'" 2924prop_checkTildeInPath3 = verifyNot checkTildeInPath "PATH=~/bin" 2925checkTildeInPath _ (T_SimpleCommand _ vars _) = 2926 mapM_ checkVar vars 2927 where 2928 checkVar (T_Assignment id Assign "PATH" [] (T_NormalWord _ parts)) 2929 | any (\x -> isQuoted x && hasTilde x) parts = 2930 warn id 2147 "Literal tilde in PATH works poorly across programs." 2931 checkVar _ = return () 2932 2933 hasTilde t = '~' `elem` onlyLiteralString t 2934 isQuoted T_DoubleQuoted {} = True 2935 isQuoted T_SingleQuoted {} = True 2936 isQuoted _ = False 2937checkTildeInPath _ _ = return () 2938 2939prop_checkUnsupported3 = verify checkUnsupported "#!/bin/sh\ncase foo in bar) baz ;& esac" 2940prop_checkUnsupported4 = verify checkUnsupported "#!/bin/ksh\ncase foo in bar) baz ;;& esac" 2941prop_checkUnsupported5 = verify checkUnsupported "#!/bin/bash\necho \"${ ls; }\"" 2942checkUnsupported params t = 2943 unless (null support || (shellType params `elem` support)) $ 2944 report name 2945 where 2946 (name, support) = shellSupport t 2947 report s = err (getId t) 2127 $ 2948 "To use " ++ s ++ ", specify #!/usr/bin/env " ++ 2949 (intercalate " or " . map (map toLower . show) $ support) 2950 2951-- TODO: Move more of these checks here 2952shellSupport t = 2953 case t of 2954 T_CaseExpression _ _ list -> forCase (map (\(a,_,_) -> a) list) 2955 T_DollarBraceCommandExpansion {} -> ("${ ..; } command expansion", [Ksh]) 2956 _ -> ("", []) 2957 where 2958 forCase seps | CaseContinue `elem` seps = ("cases with ;;&", [Bash]) 2959 forCase seps | CaseFallThrough `elem` seps = ("cases with ;&", [Bash, Ksh]) 2960 forCase _ = ("", []) 2961 2962 2963groupWith f = groupBy ((==) `on` f) 2964 2965prop_checkMultipleAppends1 = verify checkMultipleAppends "foo >> file; bar >> file; baz >> file;" 2966prop_checkMultipleAppends2 = verify checkMultipleAppends "foo >> file; bar | grep f >> file; baz >> file;" 2967prop_checkMultipleAppends3 = verifyNot checkMultipleAppends "foo < file; bar < file; baz < file;" 2968checkMultipleAppends params t = 2969 mapM_ checkList $ getCommandSequences t 2970 where 2971 checkList list = 2972 mapM_ checkGroup (groupWith (fmap fst) $ map getTarget list) 2973 checkGroup (Just (_,id):_:_:_) = 2974 style id 2129 2975 "Consider using { cmd1; cmd2; } >> file instead of individual redirects." 2976 checkGroup _ = return () 2977 getTarget (T_Annotation _ _ t) = getTarget t 2978 getTarget (T_Pipeline _ _ args@(_:_)) = getTarget (last args) 2979 getTarget (T_Redirecting id list _) = do 2980 file <- mapMaybe getAppend list !!! 0 2981 return (file, id) 2982 getTarget _ = Nothing 2983 getAppend (T_FdRedirect _ _ (T_IoFile _ T_DGREAT {} f)) = return f 2984 getAppend _ = Nothing 2985 2986 2987prop_checkSuspiciousIFS1 = verify checkSuspiciousIFS "IFS=\"\\n\"" 2988prop_checkSuspiciousIFS2 = verifyNot checkSuspiciousIFS "IFS=$'\\t'" 2989prop_checkSuspiciousIFS3 = verify checkSuspiciousIFS "IFS=' \\t\\n'" 2990checkSuspiciousIFS params (T_Assignment _ _ "IFS" [] value) = 2991 mapM_ check $ getLiteralString value 2992 where 2993 hasDollarSingle = shellType params == Bash || shellType params == Ksh 2994 n = if hasDollarSingle then "$'\\n'" else "'<literal linefeed here>'" 2995 t = if hasDollarSingle then "$'\\t'" else "\"$(printf '\\t')\"" 2996 check value = 2997 case value of 2998 "\\n" -> suggest n 2999 "\\t" -> suggest t 3000 x | '\\' `elem` x -> suggest2 "a literal backslash" 3001 x | 'n' `elem` x -> suggest2 "the literal letter 'n'" 3002 x | 't' `elem` x -> suggest2 "the literal letter 't'" 3003 _ -> return () 3004 suggest r = warn (getId value) 2141 $ "This backslash is literal. Did you mean IFS=" ++ r ++ " ?" 3005 suggest2 desc = warn (getId value) 2141 $ "This IFS value contains " ++ desc ++ ". For tabs/linefeeds/escapes, use $'..', literal, or printf." 3006checkSuspiciousIFS _ _ = return () 3007 3008 3009prop_checkGrepQ1= verify checkShouldUseGrepQ "[[ $(foo | grep bar) ]]" 3010prop_checkGrepQ2= verify checkShouldUseGrepQ "[ -z $(fgrep lol) ]" 3011prop_checkGrepQ3= verify checkShouldUseGrepQ "[ -n \"$(foo | zgrep lol)\" ]" 3012prop_checkGrepQ4= verifyNot checkShouldUseGrepQ "[ -z $(grep bar | cmd) ]" 3013prop_checkGrepQ5= verifyNot checkShouldUseGrepQ "rm $(ls | grep file)" 3014prop_checkGrepQ6= verifyNot checkShouldUseGrepQ "[[ -n $(pgrep foo) ]]" 3015checkShouldUseGrepQ params t = 3016 sequence_ $ case t of 3017 TC_Nullary id _ token -> check id True token 3018 TC_Unary id _ "-n" token -> check id True token 3019 TC_Unary id _ "-z" token -> check id False token 3020 _ -> fail "not check" 3021 where 3022 check id bool token = do 3023 name <- getFinalGrep token 3024 let op = if bool then "-n" else "-z" 3025 let flip = if bool then "" else "! " 3026 return . style id 2143 $ 3027 "Use " ++ flip ++ name ++ " -q instead of " ++ 3028 "comparing output with [ " ++ op ++ " .. ]." 3029 3030 getFinalGrep t = do 3031 cmds <- getPipeline t 3032 guard . not . null $ cmds 3033 name <- getCommandBasename $ last cmds 3034 guard . isGrep $ name 3035 return name 3036 getPipeline t = 3037 case t of 3038 T_NormalWord _ [x] -> getPipeline x 3039 T_DoubleQuoted _ [x] -> getPipeline x 3040 T_DollarExpansion _ [x] -> getPipeline x 3041 T_Pipeline _ _ cmds -> return cmds 3042 _ -> fail "unknown" 3043 isGrep = (`elem` ["grep", "egrep", "fgrep", "zgrep"]) 3044 3045prop_checkTestArgumentSplitting1 = verify checkTestArgumentSplitting "[ -e *.mp3 ]" 3046prop_checkTestArgumentSplitting2 = verifyNot checkTestArgumentSplitting "[[ $a == *b* ]]" 3047prop_checkTestArgumentSplitting3 = verify checkTestArgumentSplitting "[[ *.png == '' ]]" 3048prop_checkTestArgumentSplitting4 = verify checkTestArgumentSplitting "[[ foo == f{o,oo,ooo} ]]" 3049prop_checkTestArgumentSplitting5 = verify checkTestArgumentSplitting "[[ $@ ]]" 3050prop_checkTestArgumentSplitting6 = verify checkTestArgumentSplitting "[ -e $@ ]" 3051prop_checkTestArgumentSplitting7 = verify checkTestArgumentSplitting "[ $@ == $@ ]" 3052prop_checkTestArgumentSplitting8 = verify checkTestArgumentSplitting "[[ $@ = $@ ]]" 3053prop_checkTestArgumentSplitting9 = verifyNot checkTestArgumentSplitting "[[ foo =~ bar{1,2} ]]" 3054prop_checkTestArgumentSplitting10 = verifyNot checkTestArgumentSplitting "[ \"$@\" ]" 3055prop_checkTestArgumentSplitting11 = verify checkTestArgumentSplitting "[[ \"$@\" ]]" 3056prop_checkTestArgumentSplitting12 = verify checkTestArgumentSplitting "[ *.png ]" 3057prop_checkTestArgumentSplitting13 = verify checkTestArgumentSplitting "[ \"$@\" == \"\" ]" 3058prop_checkTestArgumentSplitting14 = verify checkTestArgumentSplitting "[[ \"$@\" == \"\" ]]" 3059prop_checkTestArgumentSplitting15 = verifyNot checkTestArgumentSplitting "[[ \"$*\" == \"\" ]]" 3060prop_checkTestArgumentSplitting16 = verifyNot checkTestArgumentSplitting "[[ -v foo[123] ]]" 3061prop_checkTestArgumentSplitting17 = verifyNot checkTestArgumentSplitting "#!/bin/ksh\n[ -e foo* ]" 3062prop_checkTestArgumentSplitting18 = verify checkTestArgumentSplitting "#!/bin/ksh\n[ -d foo* ]" 3063prop_checkTestArgumentSplitting19 = verifyNot checkTestArgumentSplitting "[[ var[x] -eq 2*3 ]]" 3064prop_checkTestArgumentSplitting20 = verify checkTestArgumentSplitting "[ var[x] -eq 2 ]" 3065prop_checkTestArgumentSplitting21 = verify checkTestArgumentSplitting "[ 6 -eq 2*3 ]" 3066checkTestArgumentSplitting :: Parameters -> Token -> Writer [TokenComment] () 3067checkTestArgumentSplitting params t = 3068 case t of 3069 (TC_Unary _ typ op token) | isGlob token -> 3070 if op == "-v" 3071 then 3072 when (typ == SingleBracket) $ 3073 err (getId token) 2208 $ 3074 "Use [[ ]] or quote arguments to -v to avoid glob expansion." 3075 else 3076 if (typ == SingleBracket && shellType params == Ksh) 3077 then 3078 -- Ksh appears to stop processing after unrecognized tokens, so operators 3079 -- will effectively work with globs, but only the first match. 3080 when (op `elem` [['-', c] | c <- "bcdfgkprsuwxLhNOGRS" ]) $ 3081 warn (getId token) 2245 $ 3082 op ++ " only applies to the first expansion of this glob. Use a loop to check any/all." 3083 else 3084 err (getId token) 2144 $ 3085 op ++ " doesn't work with globs. Use a for loop." 3086 3087 (TC_Nullary _ typ token) -> do 3088 checkBraces typ token 3089 checkGlobs typ token 3090 when (typ == DoubleBracket) $ 3091 checkArrays typ token 3092 3093 (TC_Unary _ typ op token) -> checkAll typ token 3094 3095 (TC_Binary _ typ op lhs rhs) | op `elem` arithmeticBinaryTestOps -> 3096 if typ == DoubleBracket 3097 then 3098 mapM_ (\c -> do 3099 checkArrays typ c 3100 checkBraces typ c) [lhs, rhs] 3101 else 3102 mapM_ (\c -> do 3103 checkNumericalGlob typ c 3104 checkArrays typ c 3105 checkBraces typ c) [lhs, rhs] 3106 3107 (TC_Binary _ typ op lhs rhs) -> 3108 if op `elem` ["=", "==", "!=", "=~"] 3109 then do 3110 checkAll typ lhs 3111 checkArrays typ rhs 3112 checkBraces typ rhs 3113 else mapM_ (checkAll typ) [lhs, rhs] 3114 _ -> return () 3115 where 3116 checkAll typ token = do 3117 checkArrays typ token 3118 checkBraces typ token 3119 checkGlobs typ token 3120 3121 checkArrays typ token = 3122 when (any isArrayExpansion $ getWordParts token) $ 3123 if typ == SingleBracket 3124 then warn (getId token) 2198 "Arrays don't work as operands in [ ]. Use a loop (or concatenate with * instead of @)." 3125 else err (getId token) 2199 "Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @)." 3126 3127 checkBraces typ token = 3128 when (any isBraceExpansion $ getWordParts token) $ 3129 if typ == SingleBracket 3130 then warn (getId token) 2200 "Brace expansions don't work as operands in [ ]. Use a loop." 3131 else err (getId token) 2201 "Brace expansion doesn't happen in [[ ]]. Use a loop." 3132 3133 checkGlobs typ token = 3134 when (isGlob token) $ 3135 if typ == SingleBracket 3136 then warn (getId token) 2202 "Globs don't work as operands in [ ]. Use a loop." 3137 else err (getId token) 2203 "Globs are ignored in [[ ]] except right of =/!=. Use a loop." 3138 3139 checkNumericalGlob SingleBracket token = 3140 -- var[x] and x*2 look like globs 3141 when (shellType params /= Ksh && isGlob token) $ 3142 err (getId token) 2255 "[ ] does not apply arithmetic evaluation. Evaluate with $((..)) for numbers, or use string comparator for strings." 3143 3144 3145prop_checkReadWithoutR1 = verify checkReadWithoutR "read -a foo" 3146prop_checkReadWithoutR2 = verifyNot checkReadWithoutR "read -ar foo" 3147prop_checkReadWithoutR3 = verifyNot checkReadWithoutR "read -t 0" 3148prop_checkReadWithoutR4 = verifyNot checkReadWithoutR "read -t 0 && read --d '' -r bar" 3149prop_checkReadWithoutR5 = verifyNot checkReadWithoutR "read -t 0 foo < file.txt" 3150prop_checkReadWithoutR6 = verifyNot checkReadWithoutR "read -u 3 -t 0" 3151checkReadWithoutR _ t@T_SimpleCommand {} | t `isUnqualifiedCommand` "read" 3152 && "r" `notElem` map snd flags && not has_t0 = 3153 info (getId $ getCommandTokenOrThis t) 2162 "read without -r will mangle backslashes." 3154 where 3155 flags = getAllFlags t 3156 has_t0 = Just "0" == do 3157 parsed <- getGnuOpts flagsForRead $ arguments t 3158 (_, t) <- lookup "t" parsed 3159 getLiteralString t 3160 3161checkReadWithoutR _ _ = return () 3162 3163prop_checkUncheckedCd1 = verifyTree checkUncheckedCdPushdPopd "cd ~/src; rm -r foo" 3164prop_checkUncheckedCd2 = verifyNotTree checkUncheckedCdPushdPopd "cd ~/src || exit; rm -r foo" 3165prop_checkUncheckedCd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; cd ~/src; rm -r foo" 3166prop_checkUncheckedCd4 = verifyNotTree checkUncheckedCdPushdPopd "if cd foo; then rm foo; fi" 3167prop_checkUncheckedCd5 = verifyTree checkUncheckedCdPushdPopd "if true; then cd foo; fi" 3168prop_checkUncheckedCd6 = verifyNotTree checkUncheckedCdPushdPopd "cd .." 3169prop_checkUncheckedCd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\ncd foo\nrm bar" 3170prop_checkUncheckedCd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; cd foo; rm bar" 3171prop_checkUncheckedCd9 = verifyTree checkUncheckedCdPushdPopd "builtin cd ~/src; rm -r foo" 3172prop_checkUncheckedPushd1 = verifyTree checkUncheckedCdPushdPopd "pushd ~/src; rm -r foo" 3173prop_checkUncheckedPushd2 = verifyNotTree checkUncheckedCdPushdPopd "pushd ~/src || exit; rm -r foo" 3174prop_checkUncheckedPushd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; pushd ~/src; rm -r foo" 3175prop_checkUncheckedPushd4 = verifyNotTree checkUncheckedCdPushdPopd "if pushd foo; then rm foo; fi" 3176prop_checkUncheckedPushd5 = verifyTree checkUncheckedCdPushdPopd "if true; then pushd foo; fi" 3177prop_checkUncheckedPushd6 = verifyNotTree checkUncheckedCdPushdPopd "pushd .." 3178prop_checkUncheckedPushd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\npushd foo\nrm bar" 3179prop_checkUncheckedPushd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; pushd foo; rm bar" 3180prop_checkUncheckedPushd9 = verifyNotTree checkUncheckedCdPushdPopd "pushd -n foo" 3181prop_checkUncheckedPopd1 = verifyTree checkUncheckedCdPushdPopd "popd; rm -r foo" 3182prop_checkUncheckedPopd2 = verifyNotTree checkUncheckedCdPushdPopd "popd || exit; rm -r foo" 3183prop_checkUncheckedPopd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; popd; rm -r foo" 3184prop_checkUncheckedPopd4 = verifyNotTree checkUncheckedCdPushdPopd "if popd; then rm foo; fi" 3185prop_checkUncheckedPopd5 = verifyTree checkUncheckedCdPushdPopd "if true; then popd; fi" 3186prop_checkUncheckedPopd6 = verifyTree checkUncheckedCdPushdPopd "popd" 3187prop_checkUncheckedPopd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\npopd\nrm bar" 3188prop_checkUncheckedPopd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; popd; rm bar" 3189prop_checkUncheckedPopd9 = verifyNotTree checkUncheckedCdPushdPopd "popd -n foo" 3190prop_checkUncheckedPopd10 = verifyNotTree checkUncheckedCdPushdPopd "cd ../.." 3191prop_checkUncheckedPopd11 = verifyNotTree checkUncheckedCdPushdPopd "cd ../.././.." 3192prop_checkUncheckedPopd12 = verifyNotTree checkUncheckedCdPushdPopd "cd /" 3193prop_checkUncheckedPopd13 = verifyTree checkUncheckedCdPushdPopd "cd ../../.../.." 3194 3195checkUncheckedCdPushdPopd params root = 3196 if hasSetE params then 3197 [] 3198 else execWriter $ doAnalysis checkElement root 3199 where 3200 checkElement t@T_SimpleCommand {} 3201 | name `elem` ["cd", "pushd", "popd"] 3202 && not (isSafeDir t) 3203 && not (name `elem` ["pushd", "popd"] && ("n" `elem` map snd (getAllFlags t))) 3204 && not (isCondition $ getPath (parentMap params) t) = 3205 warnWithFix (getId t) 2164 3206 ("Use '" ++ name ++ " ... || exit' or '" ++ name ++ " ... || return' in case " ++ name ++ " fails.") 3207 (fixWith [replaceEnd (getId t) params 0 " || exit"]) 3208 where name = getName t 3209 checkElement _ = return () 3210 getName t = fromMaybe "" $ getCommandName t 3211 isSafeDir t = case oversimplify t of 3212 [_, str] -> str `matches` regex 3213 _ -> False 3214 regex = mkRegex "^/*((\\.|\\.\\.)/+)*(\\.|\\.\\.)?$" 3215 3216prop_checkLoopVariableReassignment1 = verify checkLoopVariableReassignment "for i in *; do for i in *.bar; do true; done; done" 3217prop_checkLoopVariableReassignment2 = verify checkLoopVariableReassignment "for i in *; do for((i=0; i<3; i++)); do true; done; done" 3218prop_checkLoopVariableReassignment3 = verifyNot checkLoopVariableReassignment "for i in *; do for j in *.bar; do true; done; done" 3219prop_checkLoopVariableReassignment4 = verifyNot checkLoopVariableReassignment "for _ in *; do for _ in *.bar; do true; done; done" 3220checkLoopVariableReassignment params token = 3221 sequence_ $ case token of 3222 T_ForIn {} -> check 3223 T_ForArithmetic {} -> check 3224 _ -> Nothing 3225 where 3226 check = do 3227 str <- loopVariable token 3228 guard $ str /= "_" 3229 next <- find (\x -> loopVariable x == Just str) path 3230 return $ do 3231 warn (getId token) 2165 "This nested loop overrides the index variable of its parent." 3232 warn (getId next) 2167 "This parent loop has its index variable overridden." 3233 path = drop 1 $ getPath (parentMap params) token 3234 loopVariable :: Token -> Maybe String 3235 loopVariable t = 3236 case t of 3237 T_ForIn _ s _ _ -> return s 3238 T_ForArithmetic _ 3239 (TA_Sequence _ 3240 [TA_Assignment _ "=" 3241 (TA_Variable _ var _ ) _]) 3242 _ _ _ -> return var 3243 _ -> fail "not loop" 3244 3245prop_checkTrailingBracket1 = verify checkTrailingBracket "if -z n ]]; then true; fi " 3246prop_checkTrailingBracket2 = verifyNot checkTrailingBracket "if [[ -z n ]]; then true; fi " 3247prop_checkTrailingBracket3 = verify checkTrailingBracket "a || b ] && thing" 3248prop_checkTrailingBracket4 = verifyNot checkTrailingBracket "run [ foo ]" 3249prop_checkTrailingBracket5 = verifyNot checkTrailingBracket "run bar ']'" 3250checkTrailingBracket _ token = 3251 case token of 3252 T_SimpleCommand _ _ tokens@(_:_) -> check (last tokens) token 3253 _ -> return () 3254 where 3255 check (T_NormalWord id [T_Literal _ str]) command 3256 | str `elem` [ "]]", "]" ] 3257 && opposite `notElem` parameters 3258 = warn id 2171 $ 3259 "Found trailing " ++ str ++ " outside test. Add missing " ++ opposite ++ " or quote if intentional." 3260 where 3261 opposite = invert str 3262 parameters = oversimplify command 3263 check _ _ = return () 3264 invert s = 3265 case s of 3266 "]]" -> "[[" 3267 "]" -> "[" 3268 x -> x 3269 3270prop_checkReturnAgainstZero1 = verify checkReturnAgainstZero "[ $? -eq 0 ]" 3271prop_checkReturnAgainstZero2 = verify checkReturnAgainstZero "[[ \"$?\" -gt 0 ]]" 3272prop_checkReturnAgainstZero3 = verify checkReturnAgainstZero "[[ 0 -ne $? ]]" 3273prop_checkReturnAgainstZero4 = verifyNot checkReturnAgainstZero "[[ $? -eq 4 ]]" 3274prop_checkReturnAgainstZero5 = verify checkReturnAgainstZero "[[ 0 -eq $? ]]" 3275prop_checkReturnAgainstZero6 = verifyNot checkReturnAgainstZero "[[ $R -eq 0 ]]" 3276prop_checkReturnAgainstZero7 = verify checkReturnAgainstZero "(( $? == 0 ))" 3277prop_checkReturnAgainstZero8 = verify checkReturnAgainstZero "(( $? ))" 3278prop_checkReturnAgainstZero9 = verify checkReturnAgainstZero "(( ! $? ))" 3279prop_checkReturnAgainstZero10 = verifyNot checkReturnAgainstZero "x=$(( $? > 0 ))" 3280prop_checkReturnAgainstZero11 = verify checkReturnAgainstZero "(( ! ! ! $? ))" 3281prop_checkReturnAgainstZero12 = verify checkReturnAgainstZero "[ ! $? -eq 0 ]" 3282prop_checkReturnAgainstZero13 = verifyNot checkReturnAgainstZero "(( ! $? && $? > 42))" 3283prop_checkReturnAgainstZero14 = verifyNot checkReturnAgainstZero "[[ -e foo || $? -eq 0 ]]" 3284prop_checkReturnAgainstZero15 = verifyNot checkReturnAgainstZero "(( $?, n=1 ))" 3285prop_checkReturnAgainstZero16 = verifyNot checkReturnAgainstZero "(( $? || $? == 4 ))" 3286prop_checkReturnAgainstZero17 = verifyNot checkReturnAgainstZero "(( $? + 0 ))" 3287prop_checkReturnAgainstZero18 = verifyNot checkReturnAgainstZero "f() { if [ $? -eq 0 ]; then :; fi; }" 3288prop_checkReturnAgainstZero19 = verifyNot checkReturnAgainstZero "f() ( [ $? -eq 0 ] || exit 42; )" 3289prop_checkReturnAgainstZero20 = verify checkReturnAgainstZero "f() { if :; then x; [ $? -eq 0 ] && exit; fi; }" 3290prop_checkReturnAgainstZero21 = verify checkReturnAgainstZero "(( ( $? ) ))" 3291prop_checkReturnAgainstZero22 = verify checkReturnAgainstZero "[[ ( $? -eq 0 ) ]]" 3292checkReturnAgainstZero params token = 3293 case token of 3294 TC_Binary id _ op lhs rhs -> check op lhs rhs 3295 TA_Binary id op lhs rhs 3296 | op `elem` [">", "<", ">=", "<=", "==", "!="] -> check op lhs rhs 3297 TA_Unary id op@"!" exp 3298 | isExitCode exp -> message (checksSuccessLhs op) (getId exp) 3299 TA_Sequence _ [exp] 3300 | isExitCode exp -> message False (getId exp) 3301 _ -> return () 3302 where 3303 -- We don't want to warn about composite expressions like 3304 -- [[ $? -eq 0 || $? -eq 4 ]] since these can be annoying to rewrite. 3305 isOnlyTestInCommand t = 3306 case getPath (parentMap params) t of 3307 _:(T_Condition {}):_ -> True 3308 _:(T_Arithmetic {}):_ -> True 3309 _:(TA_Sequence _ [_]):(T_Arithmetic {}):_ -> True 3310 3311 -- Some negations and groupings are also fine 3312 _:next@(TC_Unary _ _ "!" _):_ -> isOnlyTestInCommand next 3313 _:next@(TA_Unary _ "!" _):_ -> isOnlyTestInCommand next 3314 _:next@(TC_Group {}):_ -> isOnlyTestInCommand next 3315 _:next@(TA_Sequence _ [_]):_ -> isOnlyTestInCommand next 3316 _ -> False 3317 3318 -- TODO: Do better $? tracking and filter on whether 3319 -- the target command is in the same function 3320 getFirstCommandInFunction = f 3321 where 3322 f t = case t of 3323 T_Function _ _ _ _ x -> f x 3324 T_BraceGroup _ (x:_) -> f x 3325 T_Subshell _ (x:_) -> f x 3326 T_Annotation _ _ x -> f x 3327 T_AndIf _ x _ -> f x 3328 T_OrIf _ x _ -> f x 3329 T_Pipeline _ _ (x:_) -> f x 3330 T_Redirecting _ _ (T_IfExpression _ (((x:_),_):_) _) -> f x 3331 x -> x 3332 3333 isFirstCommandInFunction = fromMaybe False $ do 3334 let path = getPath (parentMap params) token 3335 func <- listToMaybe $ filter isFunction path 3336 cmd <- getClosestCommand (parentMap params) token 3337 return $ getId cmd == getId (getFirstCommandInFunction func) 3338 3339 -- Is "$? op 0" trying to check if the command succeeded? 3340 checksSuccessLhs op = not $ op `elem` ["-gt", "-ne", "!=", "!"] 3341 -- Is "0 op $?" trying to check if the command succeeded? 3342 checksSuccessRhs op = op `notElem` ["-ne", "!="] 3343 3344 check op lhs rhs = 3345 if isZero rhs && isExitCode lhs 3346 then message (checksSuccessLhs op) (getId lhs) 3347 else when (isZero lhs && isExitCode rhs) $ message (checksSuccessRhs op) (getId rhs) 3348 isZero t = getLiteralString t == Just "0" 3349 isExitCode t = 3350 case getWordParts t of 3351 [T_DollarBraced _ _ l] -> concat (oversimplify l) == "?" 3352 _ -> False 3353 3354 message forSuccess id = when (isOnlyTestInCommand token && not isFirstCommandInFunction) $ style id 2181 $ 3355 "Check exit code directly with e.g. 'if " ++ (if forSuccess then "" else "! ") ++ "mycmd;', not indirectly with $?." 3356 3357 3358prop_checkRedirectedNowhere1 = verify checkRedirectedNowhere "> file" 3359prop_checkRedirectedNowhere2 = verify checkRedirectedNowhere "> file | grep foo" 3360prop_checkRedirectedNowhere3 = verify checkRedirectedNowhere "grep foo | > bar" 3361prop_checkRedirectedNowhere4 = verifyNot checkRedirectedNowhere "grep foo > bar" 3362prop_checkRedirectedNowhere5 = verifyNot checkRedirectedNowhere "foo | grep bar > baz" 3363prop_checkRedirectedNowhere6 = verifyNot checkRedirectedNowhere "var=$(value) 2> /dev/null" 3364prop_checkRedirectedNowhere7 = verifyNot checkRedirectedNowhere "var=$(< file)" 3365prop_checkRedirectedNowhere8 = verifyNot checkRedirectedNowhere "var=`< file`" 3366checkRedirectedNowhere params token = 3367 case token of 3368 T_Pipeline _ _ [single] -> sequence_ $ do 3369 redir <- getDanglingRedirect single 3370 guard . not $ isInExpansion token 3371 return $ warn (getId redir) 2188 "This redirection doesn't have a command. Move to its command (or use 'true' as no-op)." 3372 3373 T_Pipeline _ _ list -> forM_ list $ \x -> sequence_ $ do 3374 redir <- getDanglingRedirect x 3375 return $ err (getId redir) 2189 "You can't have | between this redirection and the command it should apply to." 3376 3377 _ -> return () 3378 where 3379 isInExpansion t = 3380 case drop 1 $ getPath (parentMap params) t of 3381 T_DollarExpansion _ [_] : _ -> True 3382 T_Backticked _ [_] : _ -> True 3383 t@T_Annotation {} : _ -> isInExpansion t 3384 _ -> False 3385 getDanglingRedirect token = 3386 case token of 3387 T_Redirecting _ (first:_) (T_SimpleCommand _ [] []) -> return first 3388 _ -> Nothing 3389 3390 3391prop_checkArrayAssignmentIndices1 = verifyTree checkArrayAssignmentIndices "declare -A foo; foo=(bar)" 3392prop_checkArrayAssignmentIndices2 = verifyNotTree checkArrayAssignmentIndices "declare -a foo; foo=(bar)" 3393prop_checkArrayAssignmentIndices3 = verifyNotTree checkArrayAssignmentIndices "declare -A foo; foo=([i]=bar)" 3394prop_checkArrayAssignmentIndices4 = verifyTree checkArrayAssignmentIndices "typeset -A foo; foo+=(bar)" 3395prop_checkArrayAssignmentIndices5 = verifyTree checkArrayAssignmentIndices "arr=( [foo]= bar )" 3396prop_checkArrayAssignmentIndices6 = verifyTree checkArrayAssignmentIndices "arr=( [foo] = bar )" 3397prop_checkArrayAssignmentIndices7 = verifyNotTree checkArrayAssignmentIndices "arr=( var=value )" 3398prop_checkArrayAssignmentIndices8 = verifyNotTree checkArrayAssignmentIndices "arr=( [foo]=bar )" 3399prop_checkArrayAssignmentIndices9 = verifyNotTree checkArrayAssignmentIndices "arr=( [foo]=\"\" )" 3400prop_checkArrayAssignmentIndices10 = verifyTree checkArrayAssignmentIndices "declare -A arr; arr=( var=value )" 3401prop_checkArrayAssignmentIndices11 = verifyTree checkArrayAssignmentIndices "arr=( 1=value )" 3402prop_checkArrayAssignmentIndices12 = verifyTree checkArrayAssignmentIndices "arr=( $a=value )" 3403prop_checkArrayAssignmentIndices13 = verifyTree checkArrayAssignmentIndices "arr=( $((1+1))=value )" 3404checkArrayAssignmentIndices params root = 3405 runNodeAnalysis check params root 3406 where 3407 assocs = getAssociativeArrays root 3408 check _ t = 3409 case t of 3410 T_Assignment _ _ name [] (T_Array _ list) -> 3411 let isAssoc = name `elem` assocs in 3412 mapM_ (checkElement isAssoc) list 3413 _ -> return () 3414 3415 checkElement isAssociative t = 3416 case t of 3417 T_IndexedElement _ _ (T_Literal id "") -> 3418 warn id 2192 "This array element has no value. Remove spaces after = or use \"\" for empty string." 3419 T_IndexedElement {} -> 3420 return () 3421 3422 T_NormalWord _ parts -> 3423 let literalEquals = do 3424 T_Literal id str <- parts 3425 let (before, after) = break ('=' ==) str 3426 guard $ all isDigit before && not (null after) 3427 return $ warnWithFix id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it." (surroundWith id params "\"") 3428 in 3429 if null literalEquals && isAssociative 3430 then warn (getId t) 2190 "Elements in associative arrays need index, e.g. array=( [index]=value ) ." 3431 else sequence_ literalEquals 3432 3433 _ -> return () 3434 3435 3436prop_checkUnmatchableCases1 = verify checkUnmatchableCases "case foo in bar) true; esac" 3437prop_checkUnmatchableCases2 = verify checkUnmatchableCases "case foo-$bar in ??|*) true; esac" 3438prop_checkUnmatchableCases3 = verify checkUnmatchableCases "case foo in foo) true; esac" 3439prop_checkUnmatchableCases4 = verifyNot checkUnmatchableCases "case foo-$bar in foo*|*bar|*baz*) true; esac" 3440prop_checkUnmatchableCases5 = verify checkUnmatchableCases "case $f in *.txt) true;; f??.txt) false;; esac" 3441prop_checkUnmatchableCases6 = verifyNot checkUnmatchableCases "case $f in ?*) true;; *) false;; esac" 3442prop_checkUnmatchableCases7 = verifyNot checkUnmatchableCases "case $f in $(x)) true;; asdf) false;; esac" 3443prop_checkUnmatchableCases8 = verify checkUnmatchableCases "case $f in cow) true;; bar|cow) false;; esac" 3444prop_checkUnmatchableCases9 = verifyNot checkUnmatchableCases "case $f in x) true;;& x) false;; esac" 3445checkUnmatchableCases params t = 3446 case t of 3447 T_CaseExpression _ word list -> do 3448 -- Check all patterns for whether they can ever match 3449 let allpatterns = concatMap snd3 list 3450 -- Check only the non-fallthrough branches for shadowing 3451 let breakpatterns = concatMap snd3 $ filter (\x -> fst3 x == CaseBreak) list 3452 3453 if isConstant word 3454 then warn (getId word) 2194 3455 "This word is constant. Did you forget the $ on a variable?" 3456 else mapM_ (check $ wordToPseudoGlob word) allpatterns 3457 3458 let exactGlobs = tupMap wordToExactPseudoGlob breakpatterns 3459 let fuzzyGlobs = tupMap wordToPseudoGlob breakpatterns 3460 let dominators = zip exactGlobs (tails $ drop 1 fuzzyGlobs) 3461 3462 mapM_ checkDoms dominators 3463 3464 _ -> return () 3465 where 3466 fst3 (x,_,_) = x 3467 snd3 (_,x,_) = x 3468 tp = tokenPositions params 3469 check target candidate = unless (pseudoGlobsCanOverlap target $ wordToPseudoGlob candidate) $ 3470 warn (getId candidate) 2195 3471 "This pattern will never match the case statement's word. Double check them." 3472 3473 tupMap f l = map (\x -> (x, f x)) l 3474 checkDoms ((glob, Just x), rest) = 3475 forM_ (find (\(_, p) -> x `pseudoGlobIsSuperSetof` p) rest) $ 3476 \(first,_) -> do 3477 warn (getId glob) 2221 $ "This pattern always overrides a later one" <> patternContext (getId first) 3478 warn (getId first) 2222 $ "This pattern never matches because of a previous pattern" <> patternContext (getId glob) 3479 where 3480 patternContext :: Id -> String 3481 patternContext id = 3482 case posLine . fst <$> Map.lookup id tp of 3483 Just l -> " on line " <> show l <> "." 3484 _ -> "." 3485 checkDoms _ = return () 3486 3487 3488prop_checkSubshellAsTest1 = verify checkSubshellAsTest "( -e file )" 3489prop_checkSubshellAsTest2 = verify checkSubshellAsTest "( 1 -gt 2 )" 3490prop_checkSubshellAsTest3 = verifyNot checkSubshellAsTest "( grep -c foo bar )" 3491prop_checkSubshellAsTest4 = verifyNot checkSubshellAsTest "[ 1 -gt 2 ]" 3492prop_checkSubshellAsTest5 = verify checkSubshellAsTest "( -e file && -x file )" 3493prop_checkSubshellAsTest6 = verify checkSubshellAsTest "( -e file || -x file && -t 1 )" 3494prop_checkSubshellAsTest7 = verify checkSubshellAsTest "( ! -d file )" 3495checkSubshellAsTest _ t = 3496 case t of 3497 T_Subshell id [w] -> check id w 3498 _ -> return () 3499 where 3500 check id t = case t of 3501 (T_Banged _ w) -> check id w 3502 (T_AndIf _ w _) -> check id w 3503 (T_OrIf _ w _) -> check id w 3504 (T_Pipeline _ _ [T_Redirecting _ _ (T_SimpleCommand _ [] (first:second:_))]) -> 3505 checkParams id first second 3506 _ -> return () 3507 3508 3509 checkParams id first second = do 3510 when (maybe False (`elem` unaryTestOps) $ getLiteralString first) $ 3511 err id 2204 "(..) is a subshell. Did you mean [ .. ], a test expression?" 3512 when (maybe False (`elem` binaryTestOps) $ getLiteralString second) $ 3513 warn id 2205 "(..) is a subshell. Did you mean [ .. ], a test expression?" 3514 3515 3516prop_checkSplittingInArrays1 = verify checkSplittingInArrays "a=( $var )" 3517prop_checkSplittingInArrays2 = verify checkSplittingInArrays "a=( $(cmd) )" 3518prop_checkSplittingInArrays3 = verifyNot checkSplittingInArrays "a=( \"$var\" )" 3519prop_checkSplittingInArrays4 = verifyNot checkSplittingInArrays "a=( \"$(cmd)\" )" 3520prop_checkSplittingInArrays5 = verifyNot checkSplittingInArrays "a=( $! $$ $# )" 3521prop_checkSplittingInArrays6 = verifyNot checkSplittingInArrays "a=( ${#arr[@]} )" 3522prop_checkSplittingInArrays7 = verifyNot checkSplittingInArrays "a=( foo{1,2} )" 3523prop_checkSplittingInArrays8 = verifyNot checkSplittingInArrays "a=( * )" 3524checkSplittingInArrays params t = 3525 case t of 3526 T_Array _ elements -> mapM_ check elements 3527 _ -> return () 3528 where 3529 check word = case word of 3530 T_NormalWord _ parts -> mapM_ checkPart parts 3531 _ -> return () 3532 checkPart part = case part of 3533 T_DollarExpansion id _ -> forCommand id 3534 T_DollarBraceCommandExpansion id _ -> forCommand id 3535 T_Backticked id _ -> forCommand id 3536 T_DollarBraced id _ str | 3537 not (isCountingReference part) 3538 && not (isQuotedAlternativeReference part) 3539 && getBracedReference (concat $ oversimplify str) `notElem` variablesWithoutSpaces 3540 -> warn id 2206 $ 3541 if shellType params == Ksh 3542 then "Quote to prevent word splitting/globbing, or split robustly with read -A or while read." 3543 else "Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a." 3544 _ -> return () 3545 3546 forCommand id = 3547 warn id 2207 $ 3548 if shellType params == Ksh 3549 then "Prefer read -A or while read to split command output (or quote to avoid splitting)." 3550 else "Prefer mapfile or read -a to split command output (or quote to avoid splitting)." 3551 3552 3553prop_checkRedirectionToNumber1 = verify checkRedirectionToNumber "( 1 > 2 )" 3554prop_checkRedirectionToNumber2 = verify checkRedirectionToNumber "foo 1>2" 3555prop_checkRedirectionToNumber3 = verifyNot checkRedirectionToNumber "echo foo > '2'" 3556prop_checkRedirectionToNumber4 = verifyNot checkRedirectionToNumber "foo 1>&2" 3557checkRedirectionToNumber _ t = case t of 3558 T_IoFile id _ word -> sequence_ $ do 3559 file <- getUnquotedLiteral word 3560 guard $ all isDigit file 3561 return $ warn id 2210 "This is a file redirection. Was it supposed to be a comparison or fd operation?" 3562 _ -> return () 3563 3564prop_checkGlobAsCommand1 = verify checkGlobAsCommand "foo*" 3565prop_checkGlobAsCommand2 = verify checkGlobAsCommand "$(var[i])" 3566prop_checkGlobAsCommand3 = verifyNot checkGlobAsCommand "echo foo*" 3567checkGlobAsCommand _ t = case t of 3568 T_SimpleCommand _ _ (first:_) 3569 | isGlob first -> 3570 warn (getId first) 2211 "This is a glob used as a command name. Was it supposed to be in ${..}, array, or is it missing quoting?" 3571 _ -> return () 3572 3573 3574prop_checkFlagAsCommand1 = verify checkFlagAsCommand "-e file" 3575prop_checkFlagAsCommand2 = verify checkFlagAsCommand "foo\n --bar=baz" 3576prop_checkFlagAsCommand3 = verifyNot checkFlagAsCommand "'--myexec--' args" 3577prop_checkFlagAsCommand4 = verifyNot checkFlagAsCommand "var=cmd --arg" -- Handled by SC2037 3578checkFlagAsCommand _ t = case t of 3579 T_SimpleCommand _ [] (first:_) 3580 | isUnquotedFlag first -> 3581 warn (getId first) 2215 "This flag is used as a command name. Bad line break or missing [ .. ]?" 3582 _ -> return () 3583 3584 3585prop_checkEmptyCondition1 = verify checkEmptyCondition "if [ ]; then ..; fi" 3586prop_checkEmptyCondition2 = verifyNot checkEmptyCondition "[ foo -o bar ]" 3587checkEmptyCondition _ t = case t of 3588 TC_Empty id _ -> style id 2212 "Use 'false' instead of empty [/[[ conditionals." 3589 _ -> return () 3590 3591prop_checkPipeToNowhere1 = verify checkPipeToNowhere "foo | echo bar" 3592prop_checkPipeToNowhere2 = verify checkPipeToNowhere "basename < file.txt" 3593prop_checkPipeToNowhere3 = verify checkPipeToNowhere "printf 'Lol' <<< str" 3594prop_checkPipeToNowhere4 = verify checkPipeToNowhere "printf 'Lol' << eof\nlol\neof\n" 3595prop_checkPipeToNowhere5 = verifyNot checkPipeToNowhere "echo foo | xargs du" 3596prop_checkPipeToNowhere6 = verifyNot checkPipeToNowhere "ls | echo $(cat)" 3597prop_checkPipeToNowhere7 = verifyNot checkPipeToNowhere "echo foo | var=$(cat) ls" 3598prop_checkPipeToNowhere8 = verify checkPipeToNowhere "foo | true" 3599prop_checkPipeToNowhere9 = verifyNot checkPipeToNowhere "mv -i f . < /dev/stdin" 3600prop_checkPipeToNowhere10 = verify checkPipeToNowhere "ls > file | grep foo" 3601prop_checkPipeToNowhere11 = verify checkPipeToNowhere "ls | grep foo < file" 3602prop_checkPipeToNowhere12 = verify checkPipeToNowhere "ls > foo > bar" 3603prop_checkPipeToNowhere13 = verify checkPipeToNowhere "ls > foo 2> bar > baz" 3604prop_checkPipeToNowhere14 = verify checkPipeToNowhere "ls > foo &> bar" 3605prop_checkPipeToNowhere15 = verifyNot checkPipeToNowhere "ls > foo 2> bar |& grep 'No space left'" 3606prop_checkPipeToNowhere16 = verifyNot checkPipeToNowhere "echo World | cat << EOF\nhello $(cat)\nEOF\n" 3607prop_checkPipeToNowhere17 = verify checkPipeToNowhere "echo World | cat << 'EOF'\nhello $(cat)\nEOF\n" 3608prop_checkPipeToNowhere18 = verifyNot checkPipeToNowhere "ls 1>&3 3>&1 3>&- | wc -l" 3609prop_checkPipeToNowhere19 = verifyNot checkPipeToNowhere "find . -print0 | du --files0-from=/dev/stdin" 3610prop_checkPipeToNowhere20 = verifyNot checkPipeToNowhere "find . | du --exclude-from=/dev/fd/0" 3611 3612data PipeType = StdoutPipe | StdoutStderrPipe | NoPipe deriving (Eq) 3613checkPipeToNowhere :: Parameters -> Token -> WriterT [TokenComment] Identity () 3614checkPipeToNowhere params t = 3615 case t of 3616 T_Pipeline _ pipes cmds -> 3617 mapM_ checkPipe $ commandsWithContext pipes cmds 3618 T_Redirecting _ redirects cmd | any redirectsStdin redirects -> checkRedir cmd 3619 _ -> return () 3620 where 3621 checkPipe (input, stage, output) = do 3622 let hasConsumers = hasAdditionalConsumers stage 3623 let hasProducers = hasAdditionalProducers stage 3624 3625 sequence_ $ do 3626 cmd <- getCommand stage 3627 name <- getCommandBasename cmd 3628 guard $ name `elem` nonReadingCommands 3629 guard $ not hasConsumers && input /= NoPipe 3630 guard . not $ commandSpecificException name cmd 3631 3632 -- Confusing echo for cat is so common that it's worth a special case 3633 let suggestion = 3634 if name == "echo" 3635 then "Did you want 'cat' instead?" 3636 else "Wrong command or missing xargs?" 3637 return $ warn (getId cmd) 2216 $ 3638 "Piping to '" ++ name ++ "', a command that doesn't read stdin. " ++ suggestion 3639 3640 sequence_ $ do 3641 T_Redirecting _ redirs cmd <- return stage 3642 fds <- mapM getRedirectionFds redirs 3643 3644 let fdAndToken :: [(Integer, Token)] 3645 fdAndToken = 3646 concatMap (\(list, redir) -> map (\n -> (n, redir)) list) $ 3647 zip fds redirs 3648 3649 let fdMap = 3650 Map.fromListWith (++) $ 3651 map (\(a,b) -> (a,[b])) fdAndToken 3652 3653 let inputWarning = sequence_ $ do 3654 guard $ input /= NoPipe && not hasConsumers 3655 (override:_) <- Map.lookup 0 fdMap 3656 return $ err (getOpId override) 2259 $ 3657 "This redirection overrides piped input. To use both, merge or pass filenames." 3658 3659 -- Only produce output warnings for regular pipes, since these are 3660 -- way more common, and `foo > out 2> err |& foo` can still write 3661 -- to stderr if the files fail to open 3662 let outputWarning = sequence_ $ do 3663 guard $ output == StdoutPipe && not hasProducers 3664 (override:_) <- Map.lookup 1 fdMap 3665 return $ err (getOpId override) 2260 $ 3666 "This redirection overrides the output pipe. Use 'tee' to output to both." 3667 3668 return $ do 3669 inputWarning 3670 outputWarning 3671 mapM_ warnAboutDupes $ Map.assocs fdMap 3672 3673 commandSpecificException name cmd = 3674 case name of 3675 "du" -> any ((`elem` ["exclude-from", "files0-from"]) . snd) $ getAllFlags cmd 3676 _ -> False 3677 3678 warnAboutDupes (n, list@(_:_:_)) = 3679 forM_ list $ \c -> err (getOpId c) 2261 $ 3680 "Multiple redirections compete for " ++ str n ++ ". Use cat, tee, or pass filenames instead." 3681 warnAboutDupes _ = return () 3682 3683 alternative = 3684 if shellType params `elem` [Bash, Ksh] 3685 then "process substitutions or temp files" 3686 else "temporary files" 3687 3688 str n = 3689 case n of 3690 0 -> "stdin" 3691 1 -> "stdout" 3692 2 -> "stderr" 3693 _ -> "FD " ++ show n 3694 3695 checkRedir cmd = sequence_ $ do 3696 name <- getCommandBasename cmd 3697 guard $ name `elem` nonReadingCommands 3698 guard . not $ hasAdditionalConsumers cmd 3699 guard . not $ name `elem` ["cp", "mv", "rm"] && cmd `hasFlag` "i" 3700 let suggestion = 3701 if name == "echo" 3702 then "Did you want 'cat' instead?" 3703 else "Bad quoting, wrong command or missing xargs?" 3704 return $ warn (getId cmd) 2217 $ 3705 "Redirecting to '" ++ name ++ "', a command that doesn't read stdin. " ++ suggestion 3706 3707 -- Could any words in a SimpleCommand consume stdin (e.g. echo "$(cat)")? 3708 hasAdditionalConsumers = treeContains mayConsume 3709 -- Could any words in a SimpleCommand produce stdout? E.g. >(tee foo) 3710 hasAdditionalProducers = treeContains mayProduce 3711 treeContains pred t = isNothing $ 3712 doAnalysis (guard . not . pred) t 3713 3714 mayConsume t = 3715 case t of 3716 T_ProcSub _ "<" _ -> True 3717 T_Backticked {} -> True 3718 T_DollarExpansion {} -> True 3719 _ -> False 3720 3721 mayProduce t = 3722 case t of 3723 T_ProcSub _ ">" _ -> True 3724 _ -> False 3725 3726 getOpId t = 3727 case t of 3728 T_FdRedirect _ _ x -> getOpId x 3729 T_IoFile _ op _ -> getId op 3730 _ -> getId t 3731 3732 getRedirectionFds t = 3733 case t of 3734 T_FdRedirect _ "" x -> getDefaultFds x 3735 T_FdRedirect _ "&" _ -> return [1, 2] 3736 T_FdRedirect _ num x | all isDigit num -> 3737 -- Don't report the number unless we know what it is. 3738 -- This avoids triggering on 3>&1 1>&3 3739 getDefaultFds x *> return [read num] 3740 -- Don't bother with {fd}>42 and such 3741 _ -> Nothing 3742 3743 getDefaultFds redir = 3744 case redir of 3745 T_HereDoc {} -> return [0] 3746 T_HereString {} -> return [0] 3747 T_IoFile _ op _ -> 3748 case op of 3749 T_Less {} -> return [0] 3750 T_Greater {} -> return [1] 3751 T_DGREAT {} -> return [1] 3752 T_GREATAND {} -> return [1, 2] 3753 T_CLOBBER {} -> return [1] 3754 T_IoDuplicate _ op "-" -> getDefaultFds op 3755 _ -> Nothing 3756 _ -> Nothing 3757 3758 redirectsStdin t = 3759 fromMaybe False $ do 3760 fds <- getRedirectionFds t 3761 return $ 0 `elem` fds 3762 3763 pipeType t = 3764 case t of 3765 T_Pipe _ "|" -> StdoutPipe 3766 T_Pipe _ "|&" -> StdoutStderrPipe 3767 _ -> NoPipe 3768 3769 commandsWithContext pipes cmds = 3770 let pipeTypes = map pipeType pipes 3771 inputs = NoPipe : pipeTypes 3772 outputs = pipeTypes ++ [NoPipe] 3773 in 3774 zip3 inputs cmds outputs 3775 3776prop_checkUseBeforeDefinition1 = verifyTree checkUseBeforeDefinition "f; f() { true; }" 3777prop_checkUseBeforeDefinition2 = verifyNotTree checkUseBeforeDefinition "f() { true; }; f" 3778prop_checkUseBeforeDefinition3 = verifyNotTree checkUseBeforeDefinition "if ! mycmd --version; then mycmd() { true; }; fi" 3779prop_checkUseBeforeDefinition4 = verifyNotTree checkUseBeforeDefinition "mycmd || mycmd() { f; }" 3780checkUseBeforeDefinition _ t = 3781 execWriter $ evalStateT (mapM_ examine $ revCommands) Map.empty 3782 where 3783 examine t = case t of 3784 T_Pipeline _ _ [T_Redirecting _ _ (T_Function _ _ _ name _)] -> 3785 modify $ Map.insert name t 3786 T_Annotation _ _ w -> examine w 3787 T_Pipeline _ _ cmds -> do 3788 m <- get 3789 unless (Map.null m) $ 3790 mapM_ (checkUsage m) $ concatMap recursiveSequences cmds 3791 _ -> return () 3792 3793 checkUsage map cmd = sequence_ $ do 3794 name <- getCommandName cmd 3795 def <- Map.lookup name map 3796 return $ 3797 err (getId cmd) 2218 3798 "This function is only defined later. Move the definition up." 3799 3800 revCommands = reverse $ concat $ getCommandSequences t 3801 recursiveSequences x = 3802 let list = concat $ getCommandSequences x in 3803 if null list 3804 then [x] 3805 else concatMap recursiveSequences list 3806 3807prop_checkForLoopGlobVariables1 = verify checkForLoopGlobVariables "for i in $var/*.txt; do true; done" 3808prop_checkForLoopGlobVariables2 = verifyNot checkForLoopGlobVariables "for i in \"$var\"/*.txt; do true; done" 3809prop_checkForLoopGlobVariables3 = verifyNot checkForLoopGlobVariables "for i in $var; do true; done" 3810checkForLoopGlobVariables _ t = 3811 case t of 3812 T_ForIn _ _ words _ -> mapM_ check words 3813 _ -> return () 3814 where 3815 check (T_NormalWord _ parts) = 3816 when (any isGlob parts) $ 3817 mapM_ suggest $ filter isQuoteableExpansion parts 3818 suggest t = info (getId t) 2231 3819 "Quote expansions in this for loop glob to prevent wordsplitting, e.g. \"$dir\"/*.txt ." 3820 3821 3822prop_checkSubshelledTests1 = verify checkSubshelledTests "a && ( [ b ] || ! [ c ] )" 3823prop_checkSubshelledTests2 = verify checkSubshelledTests "( [ a ] )" 3824prop_checkSubshelledTests3 = verify checkSubshelledTests "( [ a ] && [ b ] || test c )" 3825prop_checkSubshelledTests4 = verify checkSubshelledTests "( [ a ] && { [ b ] && [ c ]; } )" 3826prop_checkSubshelledTests5 = verifyNot checkSubshelledTests "( [[ ${var:=x} = y ]] )" 3827prop_checkSubshelledTests6 = verifyNot checkSubshelledTests "( [[ $((i++)) = 10 ]] )" 3828prop_checkSubshelledTests7 = verifyNot checkSubshelledTests "( [[ $((i+=1)) = 10 ]] )" 3829prop_checkSubshelledTests8 = verify checkSubshelledTests "# shellcheck disable=SC2234\nf() ( [[ x ]] )" 3830 3831checkSubshelledTests params t = 3832 case t of 3833 T_Subshell id list | all isTestStructure list && (not (hasAssignment t)) -> 3834 case () of 3835 -- Special case for if (test) and while (test) 3836 _ | isCompoundCondition (getPath (parentMap params) t) -> 3837 style id 2233 "Remove superfluous (..) around condition to avoid subshell overhead." 3838 3839 -- Special case for ([ x ]), except for func() ( [ x ] ) 3840 _ | isSingleTest list && (not $ isFunctionBody (getPath (parentMap params) t)) -> 3841 style id 2234 "Remove superfluous (..) around test command to avoid subshell overhead." 3842 3843 -- General case for ([ x ] || [ y ] && etc) 3844 _ -> style id 2235 "Use { ..; } instead of (..) to avoid subshell overhead." 3845 _ -> return () 3846 where 3847 3848 isSingleTest cmds = 3849 case cmds of 3850 [c] | isTestCommand c -> True 3851 _ -> False 3852 3853 isFunctionBody path = 3854 case path of 3855 (_:f:_) -> isFunction f 3856 _ -> False 3857 3858 isTestStructure t = 3859 case t of 3860 T_Banged _ t -> isTestStructure t 3861 T_AndIf _ a b -> isTestStructure a && isTestStructure b 3862 T_OrIf _ a b -> isTestStructure a && isTestStructure b 3863 T_Pipeline _ [] [T_Redirecting _ _ cmd] -> 3864 case cmd of 3865 T_BraceGroup _ ts -> all isTestStructure ts 3866 T_Subshell _ ts -> all isTestStructure ts 3867 _ -> isTestCommand t 3868 _ -> isTestCommand t 3869 3870 isTestCommand t = 3871 case t of 3872 T_Pipeline _ [] [T_Redirecting _ _ cmd] -> 3873 case cmd of 3874 T_Condition {} -> True 3875 _ -> cmd `isCommand` "test" 3876 _ -> False 3877 3878 -- Check if a T_Subshell is used as a condition, e.g. if ( test ) 3879 -- This technically also triggers for `if true; then ( test ); fi` 3880 -- but it's still a valid suggestion. 3881 isCompoundCondition chain = 3882 case dropWhile skippable (drop 1 chain) of 3883 T_IfExpression {} : _ -> True 3884 T_WhileExpression {} : _ -> True 3885 T_UntilExpression {} : _ -> True 3886 _ -> False 3887 3888 hasAssignment t = isNothing $ doAnalysis guardNotAssignment t 3889 guardNotAssignment t = 3890 case t of 3891 TA_Assignment {} -> Nothing 3892 TA_Unary _ s _ -> guard . not $ "++" `isInfixOf` s || "--" `isInfixOf` s 3893 T_DollarBraced _ _ l -> 3894 let str = concat $ oversimplify l 3895 modifier = getBracedModifier str 3896 in 3897 guard . not $ "=" `isPrefixOf` modifier || ":=" `isPrefixOf` modifier 3898 T_DollarBraceCommandExpansion {} -> Nothing 3899 _ -> Just () 3900 3901 -- Skip any parent of a T_Subshell until we reach something interesting 3902 skippable t = 3903 case t of 3904 T_Redirecting _ [] _ -> True 3905 T_Pipeline _ [] _ -> True 3906 T_Annotation {} -> True 3907 _ -> False 3908 3909prop_checkInvertedStringTest1 = verify checkInvertedStringTest "[ ! -z $var ]" 3910prop_checkInvertedStringTest2 = verify checkInvertedStringTest "! [[ -n $var ]]" 3911prop_checkInvertedStringTest3 = verifyNot checkInvertedStringTest "! [ -x $var ]" 3912prop_checkInvertedStringTest4 = verifyNot checkInvertedStringTest "[[ ! -w $var ]]" 3913prop_checkInvertedStringTest5 = verifyNot checkInvertedStringTest "[ -z $var ]" 3914checkInvertedStringTest _ t = 3915 case t of 3916 TC_Unary _ _ "!" (TC_Unary _ _ op _) -> 3917 case op of 3918 "-n" -> style (getId t) 2236 "Use -z instead of ! -n." 3919 "-z" -> style (getId t) 2236 "Use -n instead of ! -z." 3920 _ -> return () 3921 T_Banged _ (T_Pipeline _ _ 3922 [T_Redirecting _ _ (T_Condition _ _ (TC_Unary _ _ op _))]) -> 3923 case op of 3924 "-n" -> style (getId t) 2237 "Use [ -z .. ] instead of ! [ -n .. ]." 3925 "-z" -> style (getId t) 2237 "Use [ -n .. ] instead of ! [ -z .. ]." 3926 _ -> return () 3927 _ -> return () 3928 3929prop_checkRedirectionToCommand1 = verify checkRedirectionToCommand "ls > rm" 3930prop_checkRedirectionToCommand2 = verifyNot checkRedirectionToCommand "ls > 'rm'" 3931prop_checkRedirectionToCommand3 = verifyNot checkRedirectionToCommand "ls > myfile" 3932checkRedirectionToCommand _ t = 3933 case t of 3934 T_IoFile _ _ (T_NormalWord id [T_Literal _ str]) | str `elem` commonCommands 3935 && str /= "file" -> -- This would be confusing 3936 warn id 2238 "Redirecting to/from command name instead of file. Did you want pipes/xargs (or quote to ignore)?" 3937 _ -> return () 3938 3939prop_checkNullaryExpansionTest1 = verify checkNullaryExpansionTest "[[ $(a) ]]" 3940prop_checkNullaryExpansionTest2 = verify checkNullaryExpansionTest "[[ $a ]]" 3941prop_checkNullaryExpansionTest3 = verifyNot checkNullaryExpansionTest "[[ $a=1 ]]" 3942prop_checkNullaryExpansionTest4 = verifyNot checkNullaryExpansionTest "[[ -n $(a) ]]" 3943prop_checkNullaryExpansionTest5 = verify checkNullaryExpansionTest "[[ \"$a$b\" ]]" 3944prop_checkNullaryExpansionTest6 = verify checkNullaryExpansionTest "[[ `x` ]]" 3945checkNullaryExpansionTest params t = 3946 case t of 3947 TC_Nullary _ _ word -> 3948 case getWordParts word of 3949 [t] | isCommandSubstitution t -> 3950 styleWithFix id 2243 "Prefer explicit -n to check for output (or run command without [/[[ to check for success)." fix 3951 3952 -- If they're constant, you get SC2157 &co 3953 x | all (not . isConstant) x -> 3954 styleWithFix id 2244 "Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer)." fix 3955 _ -> return () 3956 where 3957 id = getId word 3958 fix = fixWith [replaceStart id params 0 "-n "] 3959 _ -> return () 3960 3961 3962prop_checkDollarQuoteParen1 = verify checkDollarQuoteParen "$\"(foo)\"" 3963prop_checkDollarQuoteParen2 = verify checkDollarQuoteParen "$\"{foo}\"" 3964prop_checkDollarQuoteParen3 = verifyNot checkDollarQuoteParen "\"$(foo)\"" 3965prop_checkDollarQuoteParen4 = verifyNot checkDollarQuoteParen "$\"..\"" 3966checkDollarQuoteParen params t = 3967 case t of 3968 T_DollarDoubleQuoted id ((T_Literal _ (c:_)):_) | c `elem` "({" -> 3969 warnWithFix id 2247 "Flip leading $ and \" if this should be a quoted substitution." (fix id) 3970 _ -> return () 3971 where 3972 fix id = fixWith [replaceStart id params 2 "\"$"] 3973 3974prop_checkTranslatedStringVariable1 = verify checkTranslatedStringVariable "foo_bar2=val; $\"foo_bar2\"" 3975prop_checkTranslatedStringVariable2 = verifyNot checkTranslatedStringVariable "$\"foo_bar2\"" 3976prop_checkTranslatedStringVariable3 = verifyNot checkTranslatedStringVariable "$\"..\"" 3977prop_checkTranslatedStringVariable4 = verifyNot checkTranslatedStringVariable "var=val; $\"$var\"" 3978prop_checkTranslatedStringVariable5 = verifyNot checkTranslatedStringVariable "foo=var; bar=val2; $\"foo bar\"" 3979checkTranslatedStringVariable params (T_DollarDoubleQuoted id [T_Literal _ s]) 3980 | all isVariableChar s 3981 && Map.member s assignments 3982 = warnWithFix id 2256 "This translated string is the name of a variable. Flip leading $ and \" if this should be a quoted substitution." (fix id) 3983 where 3984 assignments = foldl (flip ($)) Map.empty (map insertAssignment $ variableFlow params) 3985 insertAssignment (Assignment (_, token, name, _)) | isVariableName name = 3986 Map.insert name token 3987 insertAssignment _ = Prelude.id 3988 fix id = fixWith [replaceStart id params 2 "\"$"] 3989checkTranslatedStringVariable _ _ = return () 3990 3991prop_checkDefaultCase1 = verify checkDefaultCase "case $1 in a) true ;; esac" 3992prop_checkDefaultCase2 = verify checkDefaultCase "case $1 in ?*?) true ;; *? ) true ;; esac" 3993prop_checkDefaultCase3 = verifyNot checkDefaultCase "case $1 in x|*) true ;; esac" 3994prop_checkDefaultCase4 = verifyNot checkDefaultCase "case $1 in **) true ;; esac" 3995checkDefaultCase _ t = 3996 case t of 3997 T_CaseExpression id _ list -> 3998 unless (any canMatchAny list) $ 3999 info id 2249 "Consider adding a default *) case, even if it just exits with error." 4000 _ -> return () 4001 where 4002 canMatchAny (_, list, _) = any canMatchAny' list 4003 -- hlint objects to 'pattern' as a variable name 4004 canMatchAny' pat = fromMaybe False $ do 4005 pg <- wordToExactPseudoGlob pat 4006 return $ pseudoGlobIsSuperSetof pg [PGMany] 4007 4008prop_checkUselessBang1 = verify checkUselessBang "set -e; ! true; rest" 4009prop_checkUselessBang2 = verifyNot checkUselessBang "! true; rest" 4010prop_checkUselessBang3 = verify checkUselessBang "set -e; while true; do ! true; done" 4011prop_checkUselessBang4 = verifyNot checkUselessBang "set -e; if ! true; then true; fi" 4012prop_checkUselessBang5 = verifyNot checkUselessBang "set -e; ( ! true )" 4013prop_checkUselessBang6 = verify checkUselessBang "set -e; { ! true; }" 4014prop_checkUselessBang7 = verifyNot checkUselessBang "set -e; x() { ! [ x ]; }" 4015prop_checkUselessBang8 = verifyNot checkUselessBang "set -e; if { ! true; }; then true; fi" 4016prop_checkUselessBang9 = verifyNot checkUselessBang "set -e; while ! true; do true; done" 4017checkUselessBang params t = when (hasSetE params) $ mapM_ check (getNonReturningCommands t) 4018 where 4019 check t = 4020 case t of 4021 T_Banged id cmd | not $ isCondition (getPath (parentMap params) t) -> 4022 addComment $ makeCommentWithFix InfoC id 2251 4023 "This ! is not on a condition and skips errexit. Use `&& exit 1` instead, or make sure $? is checked." 4024 (fixWith [replaceStart id params 1 "", replaceEnd (getId cmd) params 0 " && exit 1"]) 4025 _ -> return () 4026 4027 -- Get all the subcommands that aren't likely to be the return value 4028 getNonReturningCommands :: Token -> [Token] 4029 getNonReturningCommands t = 4030 case t of 4031 T_Script _ _ list -> dropLast list 4032 T_BraceGroup _ list -> if isFunctionBody t then dropLast list else list 4033 T_Subshell _ list -> dropLast list 4034 T_WhileExpression _ conds cmds -> dropLast conds ++ cmds 4035 T_UntilExpression _ conds cmds -> dropLast conds ++ cmds 4036 T_ForIn _ _ _ list -> list 4037 T_ForArithmetic _ _ _ _ list -> list 4038 T_Annotation _ _ t -> getNonReturningCommands t 4039 T_IfExpression _ conds elses -> 4040 concatMap (dropLast . fst) conds ++ concatMap snd conds ++ elses 4041 _ -> [] 4042 4043 isFunctionBody t = 4044 case getPath (parentMap params) t of 4045 _:T_Function {}:_-> True 4046 _ -> False 4047 4048 dropLast t = 4049 case t of 4050 [_] -> [] 4051 x:rest -> x : dropLast rest 4052 _ -> [] 4053 4054prop_checkModifiedArithmeticInRedirection1 = verify checkModifiedArithmeticInRedirection "ls > $((i++))" 4055prop_checkModifiedArithmeticInRedirection2 = verify checkModifiedArithmeticInRedirection "cat < \"foo$((i++)).txt\"" 4056prop_checkModifiedArithmeticInRedirection3 = verifyNot checkModifiedArithmeticInRedirection "while true; do true; done > $((i++))" 4057prop_checkModifiedArithmeticInRedirection4 = verify checkModifiedArithmeticInRedirection "cat <<< $((i++))" 4058prop_checkModifiedArithmeticInRedirection5 = verify checkModifiedArithmeticInRedirection "cat << foo\n$((i++))\nfoo\n" 4059prop_checkModifiedArithmeticInRedirection6 = verifyNot checkModifiedArithmeticInRedirection "#!/bin/dash\nls > $((i=i+1))" 4060checkModifiedArithmeticInRedirection params t = unless (shellType params == Dash) $ 4061 case t of 4062 T_Redirecting _ redirs (T_SimpleCommand _ _ (_:_)) -> mapM_ checkRedirs redirs 4063 _ -> return () 4064 where 4065 checkRedirs t = 4066 case t of 4067 T_FdRedirect _ _ (T_IoFile _ _ word) -> 4068 mapM_ checkArithmetic $ getWordParts word 4069 T_FdRedirect _ _ (T_HereString _ word) -> 4070 mapM_ checkArithmetic $ getWordParts word 4071 T_FdRedirect _ _ (T_HereDoc _ _ _ _ list) -> 4072 mapM_ checkArithmetic list 4073 _ -> return () 4074 checkArithmetic t = 4075 case t of 4076 T_DollarArithmetic _ x -> checkModifying x 4077 _ -> return () 4078 checkModifying t = 4079 case t of 4080 TA_Sequence _ list -> mapM_ checkModifying list 4081 TA_Unary id s _ | s `elem` ["|++", "++|", "|--", "--|"] -> warnFor id 4082 TA_Assignment id _ _ _ -> warnFor id 4083 TA_Binary _ _ x y -> mapM_ checkModifying [x ,y] 4084 TA_Trinary _ x y z -> mapM_ checkModifying [x, y, z] 4085 _ -> return () 4086 warnFor id = 4087 warn id 2257 "Arithmetic modifications in command redirections may be discarded. Do them separately." 4088 4089prop_checkAliasUsedInSameParsingUnit1 = verifyTree checkAliasUsedInSameParsingUnit "alias x=y; x" 4090prop_checkAliasUsedInSameParsingUnit2 = verifyNotTree checkAliasUsedInSameParsingUnit "alias x=y\nx" 4091prop_checkAliasUsedInSameParsingUnit3 = verifyTree checkAliasUsedInSameParsingUnit "{ alias x=y\nx\n}" 4092prop_checkAliasUsedInSameParsingUnit4 = verifyNotTree checkAliasUsedInSameParsingUnit "alias x=y; 'x';" 4093prop_checkAliasUsedInSameParsingUnit5 = verifyNotTree checkAliasUsedInSameParsingUnit ":\n{\n#shellcheck disable=SC2262\nalias x=y\nx\n}" 4094prop_checkAliasUsedInSameParsingUnit6 = verifyNotTree checkAliasUsedInSameParsingUnit ":\n{\n#shellcheck disable=SC2262\nalias x=y\nalias x=z\nx\n}" -- Only consider the first instance 4095checkAliasUsedInSameParsingUnit :: Parameters -> Token -> [TokenComment] 4096checkAliasUsedInSameParsingUnit params root = 4097 let 4098 -- Get all root commands 4099 commands = concat $ getCommandSequences root 4100 -- Group them by whether they start on the same line where the previous one ended 4101 units = groupByLink followsOnLine commands 4102 in 4103 execWriter $ mapM_ checkUnit units 4104 where 4105 lineSpan t = 4106 let m = tokenPositions params in do 4107 (start, end) <- Map.lookup t m 4108 return $ (posLine start, posLine end) 4109 4110 followsOnLine a b = fromMaybe False $ do 4111 (_, end) <- lineSpan (getId a) 4112 (start, _) <- lineSpan (getId b) 4113 return $ end == start 4114 4115 checkUnit :: [Token] -> Writer [TokenComment] () 4116 checkUnit unit = evalStateT (mapM_ (doAnalysis findCommands) unit) (Map.empty) 4117 4118 isSourced t = 4119 let 4120 f (T_SourceCommand {}) = True 4121 f _ = False 4122 in 4123 any f $ getPath (parentMap params) t 4124 4125 findCommands :: Token -> StateT (Map.Map String Token) (Writer [TokenComment]) () 4126 findCommands t = case t of 4127 T_SimpleCommand _ _ (cmd:args) -> 4128 case getUnquotedLiteral cmd of 4129 Just "alias" -> 4130 mapM_ addAlias args 4131 Just name | '/' `notElem` name -> do 4132 cmd <- gets (Map.lookup name) 4133 case cmd of 4134 Just alias -> 4135 unless (isSourced t || shouldIgnoreCode params 2262 alias) $ do 4136 warn (getId alias) 2262 "This alias can't be defined and used in the same parsing unit. Use a function instead." 4137 info (getId t) 2263 "Since they're in the same parsing unit, this command will not refer to the previously mentioned alias." 4138 _ -> return () 4139 _ -> return () 4140 _ -> return () 4141 addAlias arg = do 4142 let (name, value) = break (== '=') $ getLiteralStringDef "-" arg 4143 when (isVariableName name && not (null value)) $ 4144 modify (Map.insertWith (\new old -> old) name arg) 4145 4146-- Like groupBy, but compares pairs of adjacent elements, rather than against the first of the span 4147prop_groupByLink1 = groupByLink (\a b -> a+1 == b) [1,2,3,2,3,7,8,9] == [[1,2,3], [2,3], [7,8,9]] 4148prop_groupByLink2 = groupByLink (==) ([] :: [()]) == [] 4149groupByLink :: (a -> a -> Bool) -> [a] -> [[a]] 4150groupByLink f list = 4151 case list of 4152 [] -> [] 4153 (x:xs) -> foldr c n xs x [] 4154 where 4155 c next rest current span = 4156 if f current next 4157 then rest next (current:span) 4158 else (reverse $ current:span) : rest next [] 4159 n current span = [reverse (current:span)] 4160 4161 4162prop_checkBlatantRecursion1 = verify checkBlatantRecursion ":(){ :|:& };:" 4163prop_checkBlatantRecursion2 = verify checkBlatantRecursion "f() { f; }" 4164prop_checkBlatantRecursion3 = verifyNot checkBlatantRecursion "f() { command f; }" 4165prop_checkBlatantRecursion4 = verify checkBlatantRecursion "cd() { cd \"$lol/$1\" || exit; }" 4166prop_checkBlatantRecursion5 = verifyNot checkBlatantRecursion "cd() { [ -z \"$1\" ] || cd \"$1\"; }" 4167prop_checkBlatantRecursion6 = verifyNot checkBlatantRecursion "cd() { something; cd $1; }" 4168prop_checkBlatantRecursion7 = verifyNot checkBlatantRecursion "cd() { builtin cd $1; }" 4169checkBlatantRecursion :: Parameters -> Token -> Writer [TokenComment] () 4170checkBlatantRecursion params t = 4171 case t of 4172 T_Function _ _ _ name body -> 4173 case getCommandSequences body of 4174 [first : _] -> checkList name first 4175 _ -> return () 4176 _ -> return () 4177 where 4178 checkList :: String -> Token -> Writer [TokenComment] () 4179 checkList name t = 4180 case t of 4181 T_Backgrounded _ t -> checkList name t 4182 T_AndIf _ lhs _ -> checkList name lhs 4183 T_OrIf _ lhs _ -> checkList name lhs 4184 T_Pipeline _ _ cmds -> mapM_ (checkCommand name) cmds 4185 _ -> return () 4186 4187 checkCommand :: String -> Token -> Writer [TokenComment] () 4188 checkCommand name cmd = sequence_ $ do 4189 let (invokedM, t) = getCommandNameAndToken True cmd 4190 invoked <- invokedM 4191 guard $ name == invoked 4192 return $ 4193 errWithFix (getId t) 2264 4194 ("This function unconditionally re-invokes itself. Missing 'command'?") 4195 (fixWith [replaceStart (getId t) params 0 $ "command "]) 4196 4197 4198prop_checkBadTestAndOr1 = verify checkBadTestAndOr "[ x ] & [ y ]" 4199prop_checkBadTestAndOr2 = verify checkBadTestAndOr "test -e foo & [ y ]" 4200prop_checkBadTestAndOr3 = verify checkBadTestAndOr "[ x ] | [ y ]" 4201checkBadTestAndOr params t = 4202 case t of 4203 T_Pipeline _ seps cmds@(_:_:_) -> checkOrs seps cmds 4204 T_Backgrounded id cmd -> checkAnds id cmd 4205 _ -> return () 4206 where 4207 checkOrs seps cmds = 4208 let maybeSeps = map Just seps 4209 commandWithSeps = zip3 (Nothing:maybeSeps) cmds (maybeSeps ++ [Nothing]) 4210 in 4211 mapM_ checkTest commandWithSeps 4212 checkTest (before, cmd, after) = 4213 when (isTest cmd) $ do 4214 checkPipe before 4215 checkPipe after 4216 4217 checkPipe t = 4218 case t of 4219 Just (T_Pipe id "|") -> 4220 warnWithFix id 2266 "Use || for logical OR. Single | will pipe." $ 4221 fixWith [replaceEnd id params 0 "|"] 4222 _ -> return () 4223 4224 checkAnds id t = 4225 case t of 4226 T_AndIf _ _ rhs -> checkAnds id rhs 4227 T_OrIf _ _ rhs -> checkAnds id rhs 4228 T_Pipeline _ _ list | not (null list) -> checkAnds id (last list) 4229 cmd -> when (isTest cmd) $ 4230 errWithFix id 2265 "Use && for logical AND. Single & will background and return true." $ 4231 (fixWith [replaceEnd id params 0 "&"]) 4232 4233 isTest t = 4234 case t of 4235 T_Condition {} -> True 4236 T_SimpleCommand {} -> t `isCommand` "test" 4237 T_Redirecting _ _ t -> isTest t 4238 T_Annotation _ _ t -> isTest t 4239 _ -> False 4240 4241prop_checkComparisonWithLeadingX1 = verify checkComparisonWithLeadingX "[ x$foo = xlol ]" 4242prop_checkComparisonWithLeadingX2 = verify checkComparisonWithLeadingX "test x$foo = xlol" 4243prop_checkComparisonWithLeadingX3 = verifyNot checkComparisonWithLeadingX "[ $foo = xbar ]" 4244prop_checkComparisonWithLeadingX4 = verifyNot checkComparisonWithLeadingX "test $foo = xbar" 4245prop_checkComparisonWithLeadingX5 = verify checkComparisonWithLeadingX "[ \"x$foo\" = 'xlol' ]" 4246prop_checkComparisonWithLeadingX6 = verify checkComparisonWithLeadingX "[ x\"$foo\" = x'lol' ]" 4247checkComparisonWithLeadingX params t = 4248 case t of 4249 TC_Binary id typ op lhs rhs | op == "=" || op == "==" -> 4250 check lhs rhs 4251 T_SimpleCommand _ _ [cmd, lhs, op, rhs] | 4252 getLiteralString cmd == Just "test" && 4253 getLiteralString op `elem` [Just "=", Just "=="] -> 4254 check lhs rhs 4255 _ -> return () 4256 where 4257 msg = "Avoid x-prefix in comparisons as it no longer serves a purpose." 4258 check lhs rhs = sequence_ $ do 4259 l <- fixLeadingX lhs 4260 r <- fixLeadingX rhs 4261 return $ styleWithFix (getId lhs) 2268 msg $ fixWith [l, r] 4262 4263 fixLeadingX token = 4264 case getWordParts token of 4265 T_Literal id ('x':_):_ -> 4266 case token of 4267 -- The side is a single, unquoted x, so we have to quote 4268 T_NormalWord _ [T_Literal id "x"] -> 4269 return $ replaceStart id params 1 "\"\"" 4270 -- Otherwise we can just delete it 4271 _ -> return $ replaceStart id params 1 "" 4272 T_SingleQuoted id ('x':_):_ -> 4273 -- Replace the single quote and x 4274 return $ replaceStart id params 2 "'" 4275 _ -> Nothing 4276 4277prop_checkAssignToSelf1 = verify checkAssignToSelf "x=$x" 4278prop_checkAssignToSelf2 = verify checkAssignToSelf "x=${x}" 4279prop_checkAssignToSelf3 = verify checkAssignToSelf "x=\"$x\"" 4280prop_checkAssignToSelf4 = verifyNot checkAssignToSelf "x=$x mycmd" 4281checkAssignToSelf _ t = 4282 case t of 4283 T_SimpleCommand _ vars [] -> mapM_ check vars 4284 _ -> return () 4285 where 4286 check t = 4287 case t of 4288 T_Assignment id Assign name [] t -> 4289 case getWordParts t of 4290 [T_DollarBraced _ _ b] -> do 4291 when (Just name == getLiteralString b) $ 4292 msg id 4293 _ -> return () 4294 _ -> return () 4295 msg id = info id 2269 "This variable is assigned to itself, so the assignment does nothing." 4296 4297 4298prop_checkEqualsInCommand1a = verifyCodes checkEqualsInCommand [2277] "#!/bin/bash\n0='foo'" 4299prop_checkEqualsInCommand2a = verifyCodes checkEqualsInCommand [2278] "#!/bin/ksh \n$0='foo'" 4300prop_checkEqualsInCommand3a = verifyCodes checkEqualsInCommand [2279] "#!/bin/dash\n${0}='foo'" 4301prop_checkEqualsInCommand4a = verifyCodes checkEqualsInCommand [2280] "#!/bin/sh \n0='foo'" 4302 4303prop_checkEqualsInCommand1b = verifyCodes checkEqualsInCommand [2270] "1='foo'" 4304prop_checkEqualsInCommand2b = verifyCodes checkEqualsInCommand [2270] "${2}='foo'" 4305 4306prop_checkEqualsInCommand1c = verifyCodes checkEqualsInCommand [2271] "var$((n+1))=value" 4307prop_checkEqualsInCommand2c = verifyCodes checkEqualsInCommand [2271] "var${x}=value" 4308prop_checkEqualsInCommand3c = verifyCodes checkEqualsInCommand [2271] "var$((cmd))x='foo'" 4309prop_checkEqualsInCommand4c = verifyCodes checkEqualsInCommand [2271] "$(cmd)='foo'" 4310 4311prop_checkEqualsInCommand1d = verifyCodes checkEqualsInCommand [2273] "=======" 4312prop_checkEqualsInCommand2d = verifyCodes checkEqualsInCommand [2274] "======= Here =======" 4313prop_checkEqualsInCommand3d = verifyCodes checkEqualsInCommand [2275] "foo\n=42" 4314 4315prop_checkEqualsInCommand1e = verifyCodes checkEqualsInCommand [] "--foo=bar" 4316prop_checkEqualsInCommand2e = verifyCodes checkEqualsInCommand [] "$(cmd)'=foo'" 4317prop_checkEqualsInCommand3e = verifyCodes checkEqualsInCommand [2276] "var${x}/=value" 4318prop_checkEqualsInCommand4e = verifyCodes checkEqualsInCommand [2276] "${}=value" 4319prop_checkEqualsInCommand5e = verifyCodes checkEqualsInCommand [2276] "${#x}=value" 4320 4321prop_checkEqualsInCommand1f = verifyCodes checkEqualsInCommand [2281] "$var=foo" 4322prop_checkEqualsInCommand2f = verifyCodes checkEqualsInCommand [2281] "$a=$b" 4323prop_checkEqualsInCommand3f = verifyCodes checkEqualsInCommand [2281] "${var}=foo" 4324prop_checkEqualsInCommand4f = verifyCodes checkEqualsInCommand [2281] "${var[42]}=foo" 4325prop_checkEqualsInCommand5f = verifyCodes checkEqualsInCommand [2281] "$var+=foo" 4326 4327prop_checkEqualsInCommand1g = verifyCodes checkEqualsInCommand [2282] "411toppm=true" 4328 4329checkEqualsInCommand params originalToken = 4330 case originalToken of 4331 T_SimpleCommand _ _ (word:_) -> check word 4332 _ -> return () 4333 where 4334 hasEquals t = 4335 case t of 4336 T_Literal _ s -> '=' `elem` s 4337 _ -> False 4338 4339 check t@(T_NormalWord _ list) | any hasEquals list = 4340 case break hasEquals list of 4341 (leading, (eq:_)) -> msg t (stripSinglePlus leading) eq 4342 _ -> return () 4343 check _ = return () 4344 4345 -- This is a workaround for the parser adding + and = as separate literals 4346 stripSinglePlus l = 4347 case reverse l of 4348 (T_Literal _ "+"):rest -> reverse rest 4349 _ -> l 4350 4351 positionalAssignmentRe = mkRegex "^[0-9][0-9]?=" 4352 positionalMsg id = 4353 err id 2270 "To assign positional parameters, use 'set -- first second ..' (or use [ ] to compare)." 4354 indirectionMsg id = 4355 err id 2271 "For indirection, use arrays, declare \"var$n=value\", or (for sh) read/eval." 4356 badComparisonMsg id = 4357 err id 2272 "Command name contains ==. For comparison, use [ \"$var\" = value ]." 4358 conflictMarkerMsg id = 4359 err id 2273 "Sequence of ===s found. Merge conflict or intended as a commented border?" 4360 borderMsg id = 4361 err id 2274 "Command name starts with ===. Intended as a commented border?" 4362 prefixMsg id = 4363 err id 2275 "Command name starts with =. Bad line break?" 4364 genericMsg id = 4365 err id 2276 "This is interpreted as a command name containing '='. Bad assignment or comparison?" 4366 assign0Msg id bashfix = 4367 case shellType params of 4368 Bash -> errWithFix id 2277 "Use BASH_ARGV0 to assign to $0 in bash (or use [ ] to compare)." bashfix 4369 Ksh -> err id 2278 "$0 can't be assigned in Ksh (but it does reflect the current function)." 4370 Dash -> err id 2279 "$0 can't be assigned in Dash. This becomes a command name." 4371 _ -> err id 2280 "$0 can't be assigned this way, and there is no portable alternative." 4372 leadingNumberMsg id = 4373 err id 2282 "Variable names can't start with numbers, so this is interpreted as a command." 4374 4375 isExpansion t = 4376 case t of 4377 T_Arithmetic {} -> True 4378 _ -> isQuoteableExpansion t 4379 4380 isConflictMarker cmd = fromMaybe False $ do 4381 str <- getUnquotedLiteral cmd 4382 guard $ all (== '=') str 4383 guard $ length str >= 4 && length str <= 12 -- Git uses 7 but who knows 4384 return True 4385 4386 mayBeVariableName l = fromMaybe False $ do 4387 guard . not $ any isQuotes l 4388 guard . not $ any willBecomeMultipleArgs l 4389 str <- getLiteralStringExt (\_ -> Just "x") (T_NormalWord (Id 0) l) 4390 return $ isVariableName str 4391 4392 isLeadingNumberVar s = 4393 let lead = takeWhile (/= '=') s 4394 in not (null lead) && isDigit (head lead) 4395 && all isVariableChar lead && not (all isDigit lead) 4396 4397 msg cmd leading (T_Literal litId s) = do 4398 -- There are many different cases, and the order of the branches matter. 4399 case leading of 4400 -- --foo=42 4401 [] | "-" `isPrefixOf` s -> -- There's SC2215 for these 4402 return () 4403 4404 -- ======Hello====== 4405 [] | "=" `isPrefixOf` s -> 4406 case originalToken of 4407 T_SimpleCommand _ [] [word] | isConflictMarker word -> 4408 conflictMarkerMsg (getId originalToken) 4409 _ | "===" `isPrefixOf` s -> borderMsg (getId originalToken) 4410 _ -> prefixMsg (getId cmd) 4411 4412 -- '$var==42' 4413 _ | "==" `isInfixOf` s -> 4414 badComparisonMsg (getId cmd) 4415 4416 -- '${foo[x]}=42' and '$foo=42' 4417 [T_DollarBraced id braced l] | "=" `isPrefixOf` s -> do 4418 let variableStr = concat $ oversimplify l 4419 let variableReference = getBracedReference variableStr 4420 let variableModifier = getBracedModifier variableStr 4421 let isPlain = isVariableName variableStr 4422 let isPositional = all isDigit variableStr 4423 4424 let isArray = variableReference /= "" 4425 && "[" `isPrefixOf` variableModifier 4426 && "]" `isSuffixOf` variableModifier 4427 4428 case () of 4429 -- '$foo=bar' should already have caused a parse-time SC1066 4430 -- _ | not braced && isPlain -> 4431 -- return () 4432 4433 _ | variableStr == "" -> -- Don't try to fix ${}=foo 4434 genericMsg (getId cmd) 4435 4436 -- '$#=42' or '${#var}=42' 4437 _ | "#" `isPrefixOf` variableStr -> 4438 genericMsg (getId cmd) 4439 4440 -- '${0}=42' 4441 _ | variableStr == "0" -> 4442 assign0Msg id $ fixWith [replaceToken id params "BASH_ARGV0"] 4443 4444 -- '$2=2' 4445 _ | isPositional -> 4446 positionalMsg id 4447 4448 _ | isArray || isPlain -> 4449 errWithFix id 2281 4450 ("Don't use " ++ (if braced then "${}" else "$") ++ " on the left side of assignments.") $ 4451 fixWith $ 4452 if braced 4453 then [ replaceStart id params 2 "", replaceEnd id params 1 "" ] 4454 else [ replaceStart id params 1 "" ] 4455 4456 _ -> indirectionMsg id 4457 4458 -- 2=42 4459 [] | s `matches` positionalAssignmentRe -> 4460 if "0=" `isPrefixOf` s 4461 then 4462 assign0Msg litId $ fixWith [replaceStart litId params 1 "BASH_ARGV0"] 4463 else 4464 positionalMsg litId 4465 4466 -- 9foo=42 4467 [] | isLeadingNumberVar s -> 4468 leadingNumberMsg (getId cmd) 4469 4470 -- var${foo}x=42 4471 (_:_) | mayBeVariableName leading && (all isVariableChar $ takeWhile (/= '=') s) -> 4472 indirectionMsg (getId cmd) 4473 4474 _ -> genericMsg (getId cmd) 4475 4476 4477prop_checkSecondArgIsComparison1 = verify checkSecondArgIsComparison "foo = $bar" 4478prop_checkSecondArgIsComparison2 = verify checkSecondArgIsComparison "$foo = $bar" 4479prop_checkSecondArgIsComparison3 = verify checkSecondArgIsComparison "2f == $bar" 4480prop_checkSecondArgIsComparison4 = verify checkSecondArgIsComparison "'var' =$bar" 4481prop_checkSecondArgIsComparison5 = verify checkSecondArgIsComparison "foo ='$bar'" 4482prop_checkSecondArgIsComparison6 = verify checkSecondArgIsComparison "$foo =$bar" 4483prop_checkSecondArgIsComparison7 = verify checkSecondArgIsComparison "2f ==$bar" 4484prop_checkSecondArgIsComparison8 = verify checkSecondArgIsComparison "'var' =$bar" 4485prop_checkSecondArgIsComparison9 = verify checkSecondArgIsComparison "var += $(foo)" 4486prop_checkSecondArgIsComparison10 = verify checkSecondArgIsComparison "var +=$(foo)" 4487checkSecondArgIsComparison _ t = 4488 case t of 4489 T_SimpleCommand _ _ (lhs:arg:_) -> sequence_ $ do 4490 argString <- getLeadingUnquotedString arg 4491 case argString of 4492 '=':'=':'=':'=':_ -> Nothing -- Don't warn about `echo ======` and such 4493 '+':'=':_ -> 4494 return $ err (headId t) 2285 $ 4495 "Remove spaces around += to assign (or quote '+=' if literal)." 4496 '=':'=':_ -> 4497 return $ err (getId t) 2284 $ 4498 "Use [ x = y ] to compare values (or quote '==' if literal)." 4499 '=':_ -> 4500 return $ err (headId arg) 2283 $ 4501 "Remove spaces around = to assign (or use [ ] to compare, or quote '=' if literal)." 4502 _ -> Nothing 4503 _ -> return () 4504 where 4505 -- We don't pinpoint exactly, but this helps cases like foo =$bar 4506 headId t = 4507 case t of 4508 T_NormalWord _ (x:_) -> getId x 4509 _ -> getId t 4510 4511 4512prop_checkCommandWithTrailingSymbol1 = verify checkCommandWithTrailingSymbol "/" 4513prop_checkCommandWithTrailingSymbol2 = verify checkCommandWithTrailingSymbol "/foo/ bar/baz" 4514prop_checkCommandWithTrailingSymbol3 = verify checkCommandWithTrailingSymbol "/" 4515prop_checkCommandWithTrailingSymbol4 = verifyNot checkCommandWithTrailingSymbol "/*" 4516prop_checkCommandWithTrailingSymbol5 = verifyNot checkCommandWithTrailingSymbol "$foo/$bar" 4517prop_checkCommandWithTrailingSymbol6 = verify checkCommandWithTrailingSymbol "foo, bar" 4518prop_checkCommandWithTrailingSymbol7 = verifyNot checkCommandWithTrailingSymbol ". foo.sh" 4519prop_checkCommandWithTrailingSymbol8 = verifyNot checkCommandWithTrailingSymbol ": foo" 4520prop_checkCommandWithTrailingSymbol9 = verifyNot checkCommandWithTrailingSymbol "/usr/bin/python[23] file.py" 4521 4522checkCommandWithTrailingSymbol _ t = 4523 case t of 4524 T_SimpleCommand _ _ (cmd:_) -> 4525 let str = fromJust $ getLiteralStringExt (\_ -> Just "x") cmd 4526 last = lastOrDefault 'x' str 4527 in 4528 case str of 4529 "." -> return () -- The . command 4530 ":" -> return () -- The : command 4531 " " -> return () -- Probably caught by SC1101 4532 "//" -> return () -- Probably caught by SC1127 4533 "" -> err (getId cmd) 2286 "This empty string is interpreted as a command name. Double check syntax (or use 'true' as a no-op)." 4534 _ | last == '/' -> err (getId cmd) 2287 "This is interpreted as a command name ending with '/'. Double check syntax." 4535 _ | last `elem` "\\.,([{<>}])#\"\'% " -> warn (getId cmd) 2288 ("This is interpreted as a command name ending with " ++ (format last) ++ ". Double check syntax.") 4536 _ | '\t' `elem` str -> err (getId cmd) 2289 "This is interpreted as a command name containing a tab. Double check syntax." 4537 _ | '\n' `elem` str -> err (getId cmd) 2289 "This is interpreted as a command name containing a linefeed. Double check syntax." 4538 _ -> return () 4539 _ -> return () 4540 where 4541 format x = 4542 case x of 4543 ' ' -> "space" 4544 '\'' -> "apostrophe" 4545 '\"' -> "doublequote" 4546 x -> '\'' : x : "\'" 4547 4548 4549prop_checkRequireDoubleBracket1 = verifyTree checkRequireDoubleBracket "[ -x foo ]" 4550prop_checkRequireDoubleBracket2 = verifyTree checkRequireDoubleBracket "[ foo -o bar ]" 4551prop_checkRequireDoubleBracket3 = verifyNotTree checkRequireDoubleBracket "#!/bin/sh\n[ -x foo ]" 4552prop_checkRequireDoubleBracket4 = verifyNotTree checkRequireDoubleBracket "[[ -x foo ]]" 4553checkRequireDoubleBracket params = 4554 if isBashLike params 4555 then nodeChecksToTreeCheck [check] params 4556 else const [] 4557 where 4558 check _ t = case t of 4559 T_Condition id SingleBracket _ -> 4560 styleWithFix id 2292 "Prefer [[ ]] over [ ] for tests in Bash/Ksh." (fixFor t) 4561 _ -> return () 4562 4563 fixFor t = fixWith $ 4564 if isSimple t 4565 then 4566 [ 4567 replaceStart (getId t) params 0 "[", 4568 replaceEnd (getId t) params 0 "]" 4569 ] 4570 else [] 4571 4572 -- We don't tag operators like < and -o well enough to replace them, 4573 -- so just handle the simple cases. 4574 isSimple t = case t of 4575 T_Condition _ _ s -> isSimple s 4576 TC_Binary _ _ op _ _ -> not $ any (\x -> x `elem` op) "<>" 4577 TC_Unary {} -> True 4578 TC_Nullary {} -> True 4579 _ -> False 4580 4581 4582prop_checkUnquotedParameterExpansionPattern1 = verify checkUnquotedParameterExpansionPattern "echo \"${var#$x}\"" 4583prop_checkUnquotedParameterExpansionPattern2 = verify checkUnquotedParameterExpansionPattern "echo \"${var%%$(x)}\"" 4584prop_checkUnquotedParameterExpansionPattern3 = verifyNot checkUnquotedParameterExpansionPattern "echo \"${var[#$x]}\"" 4585prop_checkUnquotedParameterExpansionPattern4 = verifyNot checkUnquotedParameterExpansionPattern "echo \"${var%\"$x\"}\"" 4586 4587checkUnquotedParameterExpansionPattern params x = 4588 case x of 4589 T_DollarBraced _ True word@(T_NormalWord _ (T_Literal _ s : rest@(_:_))) -> do 4590 let modifier = getBracedModifier $ concat $ oversimplify word 4591 when ("%" `isPrefixOf` modifier || "#" `isPrefixOf` modifier) $ 4592 mapM_ check rest 4593 _ -> return () 4594 where 4595 check t = 4596 case t of 4597 T_DollarBraced {} -> inform t 4598 T_DollarExpansion {} -> inform t 4599 T_Backticked {} -> inform t 4600 _ -> return () 4601 4602 inform t = 4603 infoWithFix (getId t) 2295 4604 "Expansions inside ${..} need to be quoted separately, otherwise they match as patterns." $ 4605 surroundWith (getId t) params "\"" 4606 4607 4608prop_checkArrayValueUsedAsIndex1 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo ${arr[i]}; done" 4609prop_checkArrayValueUsedAsIndex2 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo ${arr[$i]}; done" 4610prop_checkArrayValueUsedAsIndex3 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo $((arr[i])); done" 4611prop_checkArrayValueUsedAsIndex4 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr1[@]} ${arr2[@]}; do echo ${arr1[$i]}; done" 4612prop_checkArrayValueUsedAsIndex5 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr1[@]} ${arr2[@]}; do echo ${arr2[$i]}; done" 4613prop_checkArrayValueUsedAsIndex7 = verifyNotTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo ${arr[K]}; done" 4614prop_checkArrayValueUsedAsIndex8 = verifyNotTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do i=42; echo ${arr[i]}; done" 4615prop_checkArrayValueUsedAsIndex9 = verifyNotTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo ${arr2[i]}; done" 4616 4617checkArrayValueUsedAsIndex params _ = 4618 doVariableFlowAnalysis read write Map.empty (variableFlow params) 4619 where 4620 write loop@T_ForIn {} _ name (DataString (SourceFrom words)) = do 4621 modify $ Map.insert name (loop, mapMaybe f words) 4622 return [] 4623 where 4624 f x = do 4625 name <- getArrayName x 4626 return (x, name) 4627 4628 write _ _ name _ = do 4629 modify $ Map.delete name 4630 return [] 4631 4632 read _ t name = do 4633 varMap <- get 4634 return $ fromMaybe [] $ do 4635 (loop, arrays) <- Map.lookup name varMap 4636 (arrayRef, arrayName) <- getArrayIfUsedAsIndex name t 4637 -- Is this one of the 'for' arrays? 4638 (loopWord, _) <- find ((==arrayName) . snd) arrays 4639 -- Are we still in this loop? 4640 guard $ getId loop `elem` map getId (getPath parents t) 4641 return [ 4642 makeComment WarningC (getId loopWord) 2302 "This loops over values. To loop over keys, use \"${!array[@]}\".", 4643 makeComment WarningC (getId arrayRef) 2303 $ (e4m name) ++ " is an array value, not a key. Use directly or loop over keys instead." 4644 ] 4645 4646 parents = parentMap params 4647 4648 getArrayName :: Token -> Maybe String 4649 getArrayName t = do 4650 [T_DollarBraced _ _ l] <- return $ getWordParts t 4651 let str = concat $ oversimplify l 4652 guard $ getBracedModifier str == "[@]" && not ("!" `isPrefixOf` str) 4653 return $ getBracedReference str 4654 4655 -- This is much uglier than it should be 4656 getArrayIfUsedAsIndex :: String -> Token -> Maybe (Token, String) 4657 getArrayIfUsedAsIndex name t = 4658 case t of 4659 T_DollarBraced _ _ list -> do 4660 let ref = getBracedReference $ concat $ oversimplify list 4661 guard $ ref == name 4662 -- We found a $name. Look up the chain to see if it's ${arr[$name]} 4663 list@T_NormalWord {} <- Map.lookup (getId t) parents 4664 (T_DollarBraced _ _ parentList) <- Map.lookup (getId list) parents 4665 (T_Literal _ head : index : T_Literal _ tail : _) <- return $ getWordParts parentList 4666 let str = concat $ oversimplify list 4667 let modifier = getBracedModifier str 4668 guard $ getId index == getId t 4669 guard $ "[${VAR}]" `isPrefixOf` modifier 4670 return (t, getBracedReference str) 4671 4672 T_NormalWord wordId list -> do 4673 -- We found just name. Check if it's part of ${something[name]} 4674 parent@(T_DollarBraced _ _ parentList) <- Map.lookup wordId parents 4675 let str = concat $ oversimplify t 4676 let modifier = getBracedModifier str 4677 guard $ ("[" ++ name ++ "]") `isPrefixOf` modifier 4678 return (parent, getBracedReference str) 4679 4680 TA_Variable indexId ref [] -> do 4681 -- We found arithmetic name. See if it's part of arithmetic arr[name] 4682 guard $ ref == name 4683 (TA_Sequence seqId [element]) <- Map.lookup indexId parents 4684 guard $ getId element == indexId 4685 parent@(TA_Variable arrayId arrayName [element]) <- Map.lookup seqId parents 4686 guard $ getId element == seqId 4687 return (parent, arrayName) 4688 4689 _ -> Nothing 4690 4691prop_checkSetESuppressed1 = verifyTree checkSetESuppressed "set -e; f(){ :; }; x=$(f)" 4692prop_checkSetESuppressed2 = verifyNotTree checkSetESuppressed "f(){ :; }; x=$(f)" 4693prop_checkSetESuppressed3 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; x=$(set -e; f)" 4694prop_checkSetESuppressed4 = verifyTree checkSetESuppressed "set -e; f(){ :; }; baz=$(set -e; f) || :" 4695prop_checkSetESuppressed5 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; baz=$(echo \"\") || :" 4696prop_checkSetESuppressed6 = verifyTree checkSetESuppressed "set -e; f(){ :; }; f && echo" 4697prop_checkSetESuppressed7 = verifyTree checkSetESuppressed "set -e; f(){ :; }; f || echo" 4698prop_checkSetESuppressed8 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; echo && f" 4699prop_checkSetESuppressed9 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; echo || f" 4700prop_checkSetESuppressed10 = verifyTree checkSetESuppressed "set -e; f(){ :; }; ! f" 4701prop_checkSetESuppressed11 = verifyTree checkSetESuppressed "set -e; f(){ :; }; if f; then :; fi" 4702prop_checkSetESuppressed12 = verifyTree checkSetESuppressed "set -e; f(){ :; }; if set -e; f; then :; fi" 4703prop_checkSetESuppressed13 = verifyTree checkSetESuppressed "set -e; f(){ :; }; while f; do :; done" 4704prop_checkSetESuppressed14 = verifyTree checkSetESuppressed "set -e; f(){ :; }; while set -e; f; do :; done" 4705prop_checkSetESuppressed15 = verifyTree checkSetESuppressed "set -e; f(){ :; }; until f; do :; done" 4706prop_checkSetESuppressed16 = verifyTree checkSetESuppressed "set -e; f(){ :; }; until set -e; f; do :; done" 4707prop_checkSetESuppressed17 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; g(){ :; }; g f" 4708prop_checkSetESuppressed18 = verifyNotTree checkSetESuppressed "set -e; shopt -s inherit_errexit; f(){ :; }; x=$(f)" 4709checkSetESuppressed params t = 4710 if hasSetE params then runNodeAnalysis checkNode params t else [] 4711 where 4712 checkNode _ (T_SimpleCommand _ _ (cmd:_)) = when (isFunction cmd) (checkCmd cmd) 4713 checkNode _ _ = return () 4714 4715 functions_ = functions t 4716 4717 isFunction cmd = isJust $ do 4718 literalArg <- getUnquotedLiteral cmd 4719 Map.lookup literalArg functions_ 4720 4721 checkCmd cmd = go $ getPath (parentMap params) cmd 4722 where 4723 go (child:parent:rest) = do 4724 case parent of 4725 T_Banged _ condition | child `isIn` [condition] -> informConditional "a ! condition" cmd 4726 T_AndIf _ condition _ | child `isIn` [condition] -> informConditional "an && condition" cmd 4727 T_OrIf _ condition _ | child `isIn` [condition] -> informConditional "an || condition" cmd 4728 T_IfExpression _ condition _ | child `isIn` concatMap fst condition -> informConditional "an 'if' condition" cmd 4729 T_UntilExpression _ condition _ | child `isIn` condition -> informConditional "an 'until' condition" cmd 4730 T_WhileExpression _ condition _ | child `isIn` condition -> informConditional "a 'while' condition" cmd 4731 T_DollarExpansion {} | not $ errExitEnabled parent -> informUninherited cmd 4732 T_Backticked {} | not $ errExitEnabled parent -> informUninherited cmd 4733 _ -> return () 4734 go (parent:rest) 4735 go _ = return () 4736 4737 informConditional condType t = 4738 info (getId t) 2310 ( 4739 "This function is invoked in " ++ condType ++ " so set -e " ++ 4740 "will be disabled. Invoke separately if failures should " ++ 4741 "cause the script to exit.") 4742 informUninherited t = 4743 info (getId t) 2311 ( 4744 "Bash implicitly disabled set -e for this function " ++ 4745 "invocation because it's inside a command substitution. " ++ 4746 "Add set -e; before it or enable inherit_errexit.") 4747 errExitEnabled t = hasInheritErrexit params || containsSetE t 4748 isIn t cmds = getId t `elem` map getId cmds 4749 4750 4751prop_checkExtraMaskedReturns1 = verifyTree checkExtraMaskedReturns "cat < <(ls)" 4752prop_checkExtraMaskedReturns2 = verifyTree checkExtraMaskedReturns "read -ra arr <(ls)" 4753prop_checkExtraMaskedReturns3 = verifyTree checkExtraMaskedReturns "ls >(cat)" 4754prop_checkExtraMaskedReturns4 = verifyTree checkExtraMaskedReturns "false | true" 4755prop_checkExtraMaskedReturns5 = verifyNotTree checkExtraMaskedReturns "set -o pipefail; false | true" 4756prop_checkExtraMaskedReturns6 = verifyNotTree checkExtraMaskedReturns "false | true || true" 4757prop_checkExtraMaskedReturns7 = verifyTree checkExtraMaskedReturns "true $(false)" 4758prop_checkExtraMaskedReturns8 = verifyTree checkExtraMaskedReturns "x=$(false)$(true)" 4759prop_checkExtraMaskedReturns9 = verifyNotTree checkExtraMaskedReturns "x=$(false)true" 4760prop_checkExtraMaskedReturns10 = verifyTree checkExtraMaskedReturns "x=`false``false`" 4761prop_checkExtraMaskedReturns11 = verifyTree checkExtraMaskedReturns "x=\"$(false)$(true)\"" 4762prop_checkExtraMaskedReturns12 = verifyTree checkExtraMaskedReturns "x=\"$(false)\"\"$(true)\"" 4763prop_checkExtraMaskedReturns13 = verifyTree checkExtraMaskedReturns "true <<<$(false)" 4764prop_checkExtraMaskedReturns14 = verifyNotTree checkExtraMaskedReturns "echo asdf | false" 4765prop_checkExtraMaskedReturns15 = verifyNotTree checkExtraMaskedReturns "readonly x=$(false)" 4766prop_checkExtraMaskedReturns16 = verifyTree checkExtraMaskedReturns "readarray -t files < <(ls)" 4767prop_checkExtraMaskedReturns17 = verifyNotTree checkExtraMaskedReturns "x=( $(false) false )" 4768prop_checkExtraMaskedReturns18 = verifyTree checkExtraMaskedReturns "x=( $(false) $(false) )" 4769prop_checkExtraMaskedReturns19 = verifyNotTree checkExtraMaskedReturns "x=( $(false) [4]=false )" 4770prop_checkExtraMaskedReturns20 = verifyTree checkExtraMaskedReturns "x=( $(false) [4]=$(false) )" 4771prop_checkExtraMaskedReturns21 = verifyTree checkExtraMaskedReturns "cat << foo\n $(false)\nfoo" 4772prop_checkExtraMaskedReturns22 = verifyTree checkExtraMaskedReturns "[[ $(false) ]]" 4773prop_checkExtraMaskedReturns23 = verifyNotTree checkExtraMaskedReturns "x=$(false) y=z" 4774prop_checkExtraMaskedReturns24 = verifyNotTree checkExtraMaskedReturns "x=$(( $(date +%s) ))" 4775prop_checkExtraMaskedReturns25 = verifyTree checkExtraMaskedReturns "echo $(( $(date +%s) ))" 4776prop_checkExtraMaskedReturns26 = verifyNotTree checkExtraMaskedReturns "x=( $(false) )" 4777prop_checkExtraMaskedReturns27 = verifyTree checkExtraMaskedReturns "x=$(false) false" 4778prop_checkExtraMaskedReturns28 = verifyTree checkExtraMaskedReturns "x=$(false) y=$(false)" 4779prop_checkExtraMaskedReturns29 = verifyNotTree checkExtraMaskedReturns "false < <(set -e)" 4780prop_checkExtraMaskedReturns30 = verifyNotTree checkExtraMaskedReturns "false < <(shopt -s cdspell)" 4781prop_checkExtraMaskedReturns31 = verifyNotTree checkExtraMaskedReturns "false < <(dirname \"${BASH_SOURCE[0]}\")" 4782prop_checkExtraMaskedReturns32 = verifyNotTree checkExtraMaskedReturns "false < <(basename \"${BASH_SOURCE[0]}\")" 4783prop_checkExtraMaskedReturns33 = verifyNotTree checkExtraMaskedReturns "{ false || true; } | true" 4784prop_checkExtraMaskedReturns34 = verifyNotTree checkExtraMaskedReturns "{ false || :; } | true" 4785prop_checkExtraMaskedReturns35 = verifyTree checkExtraMaskedReturns "f() { local -r x=$(false); }" 4786 4787checkExtraMaskedReturns params t = runNodeAnalysis findMaskingNodes params t 4788 where 4789 findMaskingNodes _ (T_Arithmetic _ list) = findMaskedNodesInList [list] 4790 findMaskingNodes _ (T_Array _ list) = findMaskedNodesInList $ allButLastSimpleCommands list 4791 findMaskingNodes _ (T_Condition _ _ condition) = findMaskedNodesInList [condition] 4792 findMaskingNodes _ (T_DoubleQuoted _ list) = findMaskedNodesInList $ allButLastSimpleCommands list 4793 findMaskingNodes _ (T_HereDoc _ _ _ _ list) = findMaskedNodesInList list 4794 findMaskingNodes _ (T_HereString _ word) = findMaskedNodesInList [word] 4795 findMaskingNodes _ (T_NormalWord _ parts) = findMaskedNodesInList $ allButLastSimpleCommands parts 4796 findMaskingNodes _ (T_Pipeline _ _ cmds) | not (hasPipefail params) = findMaskedNodesInList $ allButLastSimpleCommands cmds 4797 findMaskingNodes _ (T_ProcSub _ _ list) = findMaskedNodesInList list 4798 findMaskingNodes _ (T_SimpleCommand _ assigns (_:args)) = findMaskedNodesInList $ assigns ++ args 4799 findMaskingNodes _ (T_SimpleCommand _ assigns []) = findMaskedNodesInList $ allButLastSimpleCommands assigns 4800 findMaskingNodes _ _ = return () 4801 4802 findMaskedNodesInList = mapM_ (doAnalysis findMaskedNodes) 4803 4804 isMaskedNode t = not (isHarmlessCommand t || isCheckedElsewhere t || isMaskDeliberate t) 4805 findMaskedNodes t@(T_SimpleCommand _ _ (_:_)) = when (isMaskedNode t) $ inform t 4806 findMaskedNodes t@T_Condition {} = when (isMaskedNode t) $ inform t 4807 findMaskedNodes _ = return () 4808 4809 containsSimpleCommand t = isNothing $ doAnalysis go t 4810 where 4811 go t = case t of 4812 T_SimpleCommand {} -> fail "" 4813 _ -> return () 4814 4815 allButLastSimpleCommands cmds = 4816 if null simpleCommands then [] else init simpleCommands 4817 where 4818 simpleCommands = filter containsSimpleCommand cmds 4819 4820 inform t = info (getId t) 2312 ("Consider invoking this command " 4821 ++ "separately to avoid masking its return value (or use '|| true' " 4822 ++ "to ignore).") 4823 4824 isMaskDeliberate t = hasParent isOrIf t 4825 where 4826 isOrIf _ (T_OrIf _ _ (T_Pipeline _ _ [T_Redirecting _ _ cmd])) 4827 = getCommandBasename cmd `elem` [Just "true", Just ":"] 4828 isOrIf _ _ = False 4829 4830 isCheckedElsewhere t = hasParent isDeclaringCommand t 4831 where 4832 isDeclaringCommand t _ = fromMaybe False $ do 4833 cmd <- getCommand t 4834 basename <- getCommandBasename cmd 4835 return $ 4836 case basename of 4837 -- local -r x=$(false) is intentionally ignored for SC2155 4838 "local" | "r" `elem` (map snd $ getAllFlags cmd) -> False 4839 _ -> basename `elem` declaringCommands 4840 4841 isHarmlessCommand t = fromMaybe False $ do 4842 basename <- getCommandBasename t 4843 return $ basename `elem` [ 4844 "echo" 4845 ,"basename" 4846 ,"dirname" 4847 ,"printf" 4848 ,"set" 4849 ,"shopt" 4850 ] 4851 4852 parentChildPairs t = go $ parents params t 4853 where 4854 go (child:parent:rest) = (parent, child):go (parent:rest) 4855 go _ = [] 4856 4857 hasParent pred t = any (uncurry pred) (parentChildPairs t) 4858 4859 4860return [] 4861runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) 4862