Analytics.hs 142.5 KB
Newer Older
V
Vidar Holen 已提交
1
{-
2 3
    Copyright 2012-2015 Vidar Holen

V
Vidar Holen 已提交
4
    This file is part of ShellCheck.
M
Mike Frysinger 已提交
5
    https://www.shellcheck.net
V
Vidar Holen 已提交
6 7

    ShellCheck is free software: you can redistribute it and/or modify
V
Vidar Holen 已提交
8
    it under the terms of the GNU General Public License as published by
V
Vidar Holen 已提交
9 10 11 12 13 14
    the Free Software Foundation, either version 3 of the License, or
    (at your option) any later version.

    ShellCheck is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
V
Vidar Holen 已提交
15
    GNU General Public License for more details.
V
Vidar Holen 已提交
16

V
Vidar Holen 已提交
17
    You should have received a copy of the GNU General Public License
M
Mike Frysinger 已提交
18
    along with this program.  If not, see <https://www.gnu.org/licenses/>.
V
Vidar Holen 已提交
19
-}
V
Vidar Holen 已提交
20 21
{-# LANGUAGE TemplateHaskell #-}
{-# LANGUAGE FlexibleContexts #-}
22 23 24
module ShellCheck.Analytics (runAnalytics, ShellCheck.Analytics.runTests) where

import ShellCheck.AST
25
import ShellCheck.ASTLib
V
Vidar Holen 已提交
26
import ShellCheck.AnalyzerLib hiding (producesComments)
27 28 29 30
import ShellCheck.Data
import ShellCheck.Parser
import ShellCheck.Interface
import ShellCheck.Regex
31

R
Rodrigo Setti 已提交
32
import Control.Arrow (first)
33
import Control.Monad
34
import Control.Monad.Identity
V
Vidar Holen 已提交
35
import Control.Monad.State
V
Vidar Holen 已提交
36
import Control.Monad.Writer
37
import Control.Monad.Reader
38
import Data.Char
V
Vidar Holen 已提交
39
import Data.Functor
R
Rodrigo Setti 已提交
40
import Data.Function (on)
41
import Data.List
V
Vidar Holen 已提交
42
import Data.Maybe
43
import Data.Ord
44
import Debug.Trace
45
import qualified Data.Map.Strict as Map
46 47
import Test.QuickCheck.All (forAllProperties)
import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess)
48

V
Vidar Holen 已提交
49
-- Checks that are run on the AST root
50
treeChecks :: [Parameters -> Token -> [TokenComment]]
V
Vidar Holen 已提交
51 52
treeChecks = [
    runNodeAnalysis
R
Rodrigo Setti 已提交
53
        (\p t -> (mapM_ ((\ f -> f t) . (\ f -> f p))
V
Vidar Holen 已提交
54
            nodeChecks))
55 56 57
    ,subshellAssignmentCheck
    ,checkSpacefulness
    ,checkQuotesInLiterals
V
Vidar Holen 已提交
58
    ,checkShebangParameters
59
    ,checkFunctionsUsedExternally
60
    ,checkUnusedAssignments
61
    ,checkUnpassedInFunctions
62
    ,checkArrayWithoutIndex
V
Vidar Holen 已提交
63
    ,checkShebang
64
    ,checkUnassignedReferences
65
    ,checkUncheckedCdPushdPopd
66
    ,checkArrayAssignmentIndices
67
    ,checkUseBeforeDefinition
68 69
    ]

70 71 72
runAnalytics :: AnalysisSpec -> [TokenComment]
runAnalytics options =
        runList options treeChecks
V
Vidar Holen 已提交
73

74 75 76
runList :: AnalysisSpec -> [Parameters -> Token -> [TokenComment]]
    -> [TokenComment]
runList spec list = notes
V
Vidar Holen 已提交
77
    where
78
        root = asScript spec
79
        params = makeParameters spec
80 81
        notes = concatMap (\f -> f params root) list

82

83
checkList l t = concatMap (\f -> f t) l
84

85

V
Vidar Holen 已提交
86 87
-- Checks that are run on each node in the AST
runNodeAnalysis f p t = execWriter (doAnalysis (f p) t)
88

89
nodeChecks :: [Parameters -> Token -> Writer [TokenComment] ()]
V
Vidar Holen 已提交
90
nodeChecks = [
91
    checkUuoc
92
    ,checkPipePitfalls
93 94 95
    ,checkForInQuoted
    ,checkForInLs
    ,checkShorthandIf
96
    ,checkDollarStar
97 98
    ,checkUnquotedDollarAt
    ,checkStderrRedirect
99
    ,checkUnquotedN
V
Vidar Holen 已提交
100
    ,checkNumberComparisons
101
    ,checkSingleBracketOperators
102
    ,checkDoubleBracketOperators
103
    ,checkLiteralBreakingTest
104
    ,checkConstantNullary
105
    ,checkDivBeforeMult
V
Vidar Holen 已提交
106
    ,checkArithmeticDeref
107
    ,checkArithmeticBadOctal
V
Vidar Holen 已提交
108
    ,checkComparisonAgainstGlob
V
Vidar Holen 已提交
109
    ,checkCommarrays
110
    ,checkOrNeq
111
    ,checkEchoWc
112
    ,checkConstantIfs
V
Vidar Holen 已提交
113
    ,checkPipedAssignment
V
Vidar Holen 已提交
114
    ,checkAssignAteCommand
115
    ,checkUuoeVar
116
    ,checkQuotedCondRegex
V
Vidar Holen 已提交
117
    ,checkForInCat
118
    ,checkFindExec
119
    ,checkValidCondOps
120
    ,checkGlobbedRegex
V
Vidar Holen 已提交
121
    ,checkTestRedirects
V
Vidar Holen 已提交
122
    ,checkIndirectExpansion
123
    ,checkPS1Assignments
124
    ,checkBackticks
125
    ,checkInexplicablyUnquoted
V
Vidar Holen 已提交
126
    ,checkTildeInQuotes
127
    ,checkLonelyDotDash
V
Vidar Holen 已提交
128
    ,checkSpuriousExec
129
    ,checkSpuriousExpansion
130
    ,checkDollarBrackets
131
    ,checkSshHereDoc
132
    ,checkGlobsAsOptions
133
    ,checkWhileReadPitfalls
V
Vidar Holen 已提交
134
    ,checkArithmeticOpCommand
135
    ,checkCharRangeGlob
V
Vidar Holen 已提交
136
    ,checkUnquotedExpansions
V
Vidar Holen 已提交
137
    ,checkSingleQuotedVariables
138
    ,checkRedirectToSame
V
Vidar Holen 已提交
139
    ,checkPrefixAssignmentReference
140
    ,checkLoopKeywordScope
V
Vidar Holen 已提交
141 142 143
    ,checkCdAndBack
    ,checkWrongArithmeticAssignment
    ,checkConditionalAndOrs
144
    ,checkFunctionDeclarations
V
Vidar Holen 已提交
145
    ,checkStderrPipe
146
    ,checkOverridingPath
147
    ,checkArrayAsString
148
    ,checkUnsupported
149
    ,checkMultipleAppends
150
    ,checkSuspiciousIFS
151
    ,checkShouldUseGrepQ
152
    ,checkTestArgumentSplitting
153
    ,checkConcatenatedDollarAt
V
Vidar Holen 已提交
154
    ,checkTildeInPath
155
    ,checkMaskedReturns
V
Vidar Holen 已提交
156
    ,checkReadWithoutR
157
    ,checkLoopVariableReassignment
158
    ,checkTrailingBracket
159
    ,checkReturnAgainstZero
160
    ,checkRedirectedNowhere
161
    ,checkUnmatchableCases
V
Vidar Holen 已提交
162
    ,checkSubshellAsTest
163
    ,checkSplittingInArrays
164
    ,checkRedirectionToNumber
165
    ,checkGlobAsCommand
166
    ,checkFlagAsCommand
V
Vidar Holen 已提交
167
    ,checkEmptyCondition
168
    ,checkPipeToNowhere
169
    ,checkForLoopGlobVariables
170
    ,checkSubshelledTests
V
Vidar Holen 已提交
171 172
    ]

173

174
wouldHaveBeenGlob s = '*' `elem` s
175

176
verify :: (Parameters -> Token -> Writer [TokenComment] ()) -> String -> Bool
V
Vidar Holen 已提交
177
verify f s = checkNode f s == Just True
178

179
verifyNot :: (Parameters -> Token -> Writer [TokenComment] ()) -> String -> Bool
V
Vidar Holen 已提交
180
verifyNot f s = checkNode f s == Just False
181

182 183
verifyTree :: (Parameters -> Token -> [TokenComment]) -> String -> Bool
verifyTree f s = producesComments f s == Just True
184

185 186
verifyNotTree :: (Parameters -> Token -> [TokenComment]) -> String -> Bool
verifyNotTree f s = producesComments f s == Just False
187

188 189 190 191 192 193 194
checkCommand str f t@(T_SimpleCommand id _ (cmd:rest)) =
    when (t `isCommand` str) $ f cmd rest
checkCommand _ _ _ = return ()

checkUnqualifiedCommand str f t@(T_SimpleCommand id _ (cmd:rest)) =
    when (t `isUnqualifiedCommand` str) $ f cmd rest
checkUnqualifiedCommand _ _ _ = return ()
195

196 197 198 199

checkNode f = producesComments (runNodeAnalysis f)
producesComments :: (Parameters -> Token -> [TokenComment]) -> String -> Maybe Bool
producesComments f s = do
200
        root <- pScript s
201
        return . not . null $ runList (defaultSpec root) [f]
202

203 204 205 206 207 208 209 210 211 212 213 214 215 216 217
-- Copied from https://wiki.haskell.org/Edit_distance
dist :: Eq a => [a] -> [a] -> Int
dist a b
    = last (if lab == 0 then mainDiag
            else if lab > 0 then lowers !! (lab - 1)
                 else{- < 0 -}   uppers !! (-1 - lab))
    where mainDiag = oneDiag a b (head uppers) (-1 : head lowers)
          uppers = eachDiag a b (mainDiag : uppers) -- upper diagonals
          lowers = eachDiag b a (mainDiag : lowers) -- lower diagonals
          eachDiag a [] diags = []
          eachDiag a (bch:bs) (lastDiag:diags) = oneDiag a bs nextDiag lastDiag : eachDiag a bs diags
              where nextDiag = head (tail diags)
          oneDiag a b diagAbove diagBelow = thisdiag
              where doDiag [] b nw n w = []
                    doDiag a [] nw n w = []
C
cleanup  
Chad Brewbaker 已提交
218
                    doDiag (ach:as) (bch:bs) nw n w = me : doDiag as bs me (tail n) (tail w)
219 220 221 222 223 224
                        where me = if ach == bch then nw else 1 + min3 (head w) nw (head n)
                    firstelt = 1 + head diagBelow
                    thisdiag = firstelt : doDiag a b firstelt diagAbove (tail diagBelow)
          lab = length a - length b
          min3 x y z = if x < y then x else min y z

225
hasFloatingPoint params = shellType params == Ksh
226

227 228 229 230 231 232 233 234 235 236 237 238 239 240 241
-- Checks whether the current parent path is part of a condition
isCondition [] = False
isCondition [_] = False
isCondition (child:parent:rest) =
    getId child `elem` map getId (getConditionChildren parent) || isCondition (parent:rest)
  where
    getConditionChildren t =
        case t of
            T_AndIf _ left right -> [left]
            T_OrIf id left right -> [left]
            T_IfExpression id conditions elses -> concatMap (take 1 . reverse . fst) conditions
            T_WhileExpression id c l -> take 1 . reverse $ c
            T_UntilExpression id c l -> take 1 . reverse $ c
            _ -> []

242
prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)"
V
Vidar Holen 已提交
243
checkEchoWc _ (T_Pipeline id _ [a, b]) =
V
Vidar Holen 已提交
244 245
    when (acmd == ["echo", "${VAR}"]) $
        case bcmd of
V
Vidar Holen 已提交
246 247
            ["wc", "-c"] -> countMsg
            ["wc", "-m"] -> countMsg
248 249
            _ -> return ()
  where
250 251
    acmd = oversimplify a
    bcmd = oversimplify b
V
Vidar Holen 已提交
252
    countMsg = style id 2000 "See if you can use ${#variable} instead."
V
Vidar Holen 已提交
253
checkEchoWc _ _ = return ()
254

V
Vidar Holen 已提交
255 256 257
prop_checkPipedAssignment1 = verify checkPipedAssignment "A=ls | grep foo"
prop_checkPipedAssignment2 = verifyNot checkPipedAssignment "A=foo cmd | grep foo"
prop_checkPipedAssignment3 = verifyNot checkPipedAssignment "A=foo"
V
Vidar Holen 已提交
258
checkPipedAssignment _ (T_Pipeline _ _ (T_Redirecting _ _ (T_SimpleCommand id (_:_) []):_:_)) =
V
Vidar Holen 已提交
259
    warn id 2036 "If you wanted to assign the output of the pipeline, use a=$(b | c) ."
V
Vidar Holen 已提交
260
checkPipedAssignment _ _ = return ()
V
Vidar Holen 已提交
261

V
Vidar Holen 已提交
262 263
prop_checkAssignAteCommand1 = verify checkAssignAteCommand "A=ls -l"
prop_checkAssignAteCommand2 = verify checkAssignAteCommand "A=ls --sort=$foo"
264 265
prop_checkAssignAteCommand3 = verify checkAssignAteCommand "A=cat foo | grep bar"
prop_checkAssignAteCommand4 = verifyNot checkAssignAteCommand "A=foo ls -l"
266 267 268 269 270 271 272 273 274 275 276 277
prop_checkAssignAteCommand5 = verify checkAssignAteCommand "PAGER=cat grep bar"
prop_checkAssignAteCommand6 = verifyNot checkAssignAteCommand "PAGER=\"cat\" grep bar"
prop_checkAssignAteCommand7 = verify checkAssignAteCommand "here=pwd"
checkAssignAteCommand _ (T_SimpleCommand id (T_Assignment _ _ _ _ assignmentTerm:[]) list) =
    -- Check if first word is intended as an argument (flag or glob).
    if firstWordIsArg list
    then
        err id 2037 "To assign the output of a command, use var=$(cmd) ."
    else
        -- Check if it's a known, unquoted command name.
        when (isCommonCommand $ getUnquotedLiteral assignmentTerm) $
            warn id 2209 "Use var=$(command) to assign output (or quote to assign string)."
278 279 280
  where
    isCommonCommand (Just s) = s `elem` commonCommands
    isCommonCommand _ = False
281 282
    firstWordIsArg list = fromMaybe False $ do
        head <- list !!! 0
283
        return $ isGlob head || isUnquotedFlag head
284

V
Vidar Holen 已提交
285
checkAssignAteCommand _ _ = return ()
V
Vidar Holen 已提交
286

V
Vidar Holen 已提交
287 288 289
prop_checkArithmeticOpCommand1 = verify checkArithmeticOpCommand "i=i + 1"
prop_checkArithmeticOpCommand2 = verify checkArithmeticOpCommand "foo=bar * 2"
prop_checkArithmeticOpCommand3 = verifyNot checkArithmeticOpCommand "foo + opts"
V
Vidar Holen 已提交
290
checkArithmeticOpCommand _ (T_SimpleCommand id [T_Assignment {}] (firstWord:_)) =
V
Vidar Holen 已提交
291 292 293 294 295 296
    fromMaybe (return ()) $ check <$> getGlobOrLiteralString firstWord
  where
    check op =
        when (op `elem` ["+", "-", "*", "/"]) $
            warn (getId firstWord) 2099 $
                "Use $((..)) for arithmetics, e.g. i=$((i " ++ op ++ " 2))"
V
Vidar Holen 已提交
297 298 299 300
checkArithmeticOpCommand _ _ = return ()

prop_checkWrongArit = verify checkWrongArithmeticAssignment "i=i+1"
prop_checkWrongArit2 = verify checkWrongArithmeticAssignment "n=2; i=n*2"
R
Rodrigo Setti 已提交
301
checkWrongArithmeticAssignment params (T_SimpleCommand id (T_Assignment _ _ _ _ val:[]) []) =
V
Vidar Holen 已提交
302 303 304 305 306 307
  fromMaybe (return ()) $ do
    str <- getNormalString val
    match <- matchRegex regex str
    var <- match !!! 0
    op <- match !!! 1
    Map.lookup var references
V
Vidar Holen 已提交
308 309
    return . warn (getId val) 2100 $
        "Use $((..)) for arithmetics, e.g. i=$((i " ++ op ++ " 2))"
V
Vidar Holen 已提交
310 311
  where
    regex = mkRegex "^([_a-zA-Z][_a-zA-Z0-9]*)([+*-]).+$"
V
Vidar Holen 已提交
312
    references = foldl (flip ($)) Map.empty (map insertRef $ variableFlow params)
V
Vidar Holen 已提交
313 314
    insertRef (Assignment (_, _, name, _)) =
        Map.insert name ()
V
Vidar Holen 已提交
315
    insertRef _ = Prelude.id
V
Vidar Holen 已提交
316 317 318 319 320 321 322 323 324

    getNormalString (T_NormalWord _ words) = do
        parts <- foldl (liftM2 (\x y -> x ++ [y])) (Just []) $ map getLiterals words
        return $ concat parts
    getNormalString _ = Nothing

    getLiterals (T_Literal _ s) = return s
    getLiterals (T_Glob _ s) = return s
    getLiterals _ = Nothing
V
Vidar Holen 已提交
325
checkWrongArithmeticAssignment _ _ = return ()
V
Vidar Holen 已提交
326

V
Vidar Holen 已提交
327

328 329 330
prop_checkUuoc1 = verify checkUuoc "cat foo | grep bar"
prop_checkUuoc2 = verifyNot checkUuoc "cat * | grep bar"
prop_checkUuoc3 = verify checkUuoc "cat $var | grep bar"
331
prop_checkUuoc4 = verifyNot checkUuoc "cat $var"
V
Vidar Holen 已提交
332
prop_checkUuoc5 = verifyNot checkUuoc "cat \"$@\""
333
prop_checkUuoc6 = verifyNot checkUuoc "cat -n | grep bar"
R
Rodrigo Setti 已提交
334
checkUuoc _ (T_Pipeline _ _ (T_Redirecting _ _ cmd:_:_)) =
335
    checkCommand "cat" (const f) cmd
336
  where
337
    f [word] = unless (mayBecomeMultipleArgs word || isOption word) $
V
Vidar Holen 已提交
338
        style (getId word) 2002 "Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead."
339
    f _ = return ()
340
    isOption word = "-" `isPrefixOf` onlyLiteralString word
V
Vidar Holen 已提交
341
checkUuoc _ _ = return ()
342

343
prop_checkPipePitfalls3 = verify checkPipePitfalls "ls | grep -v mp3"
344 345 346
prop_checkPipePitfalls4 = verifyNot checkPipePitfalls "find . -print0 | xargs -0 foo"
prop_checkPipePitfalls5 = verifyNot checkPipePitfalls "ls -N | foo"
prop_checkPipePitfalls6 = verify checkPipePitfalls "find . | xargs foo"
347
prop_checkPipePitfalls7 = verifyNot checkPipePitfalls "find . -printf '%s\\n' | xargs foo"
348 349
prop_checkPipePitfalls8 = verify checkPipePitfalls "foo | grep bar | wc -l"
prop_checkPipePitfalls9 = verifyNot checkPipePitfalls "foo | grep -o bar | wc -l"
S
Scorpiokat 已提交
350 351 352 353 354 355
prop_checkPipePitfalls10 = verifyNot checkPipePitfalls "foo | grep -o bar | wc"
prop_checkPipePitfalls11 = verifyNot checkPipePitfalls "foo | grep bar | wc"
prop_checkPipePitfalls12 = verifyNot checkPipePitfalls "foo | grep -o bar | wc -c"
prop_checkPipePitfalls13 = verifyNot checkPipePitfalls "foo | grep bar | wc -c"
prop_checkPipePitfalls14 = verifyNot checkPipePitfalls "foo | grep -o bar | wc -cmwL"
prop_checkPipePitfalls15 = verifyNot checkPipePitfalls "foo | grep bar | wc -cmwL"
E
Ekaterina Efimova 已提交
356
prop_checkPipePitfalls16 = verifyNot checkPipePitfalls "foo | grep -r bar | wc -l"
V
Vidar Holen 已提交
357
checkPipePitfalls _ (T_Pipeline id _ commands) = do
358
    for ["find", "xargs"] $
359
        \(find:xargs:_) ->
360
          let args = oversimplify xargs ++ oversimplify find
361
          in
V
Vidar Holen 已提交
362
            unless (any ($ args) [
363 364 365 366 367 368
                hasShortParameter '0',
                hasParameter "null",
                hasParameter "print0",
                hasParameter "printf"
              ]) $ warn (getId find) 2038
                      "Use -print0/-0 or -exec + to allow for non-alphanumeric filenames."
369

V
Vidar Holen 已提交
370
    for' ["ps", "grep"] $
V
Vidar Holen 已提交
371
        \x -> info x 2009 "Consider using pgrep instead of grepping ps output."
V
Vidar Holen 已提交
372

373 374
    for ["grep", "wc"] $
        \(grep:wc:_) ->
375 376
            let flagsGrep = fromMaybe [] $ map snd . getAllFlags <$> getCommand grep
                flagsWc = fromMaybe [] $ map snd . getAllFlags <$> getCommand wc
377
            in
378
                unless (any (`elem` ["o", "only-matching", "r", "R", "recursive"]) flagsGrep || any (`elem` ["m", "chars", "w", "words", "c", "bytes", "L", "max-line-length"]) flagsWc || null flagsWc) $
S
Scorpiokat 已提交
379
                    style (getId grep) 2126 "Consider using grep -c instead of grep|wc -l."
V
Vidar Holen 已提交
380

381
    didLs <- fmap or . sequence $ [
382
        for' ["ls", "grep"] $
V
Vidar Holen 已提交
383
            \x -> warn x 2010 "Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.",
384
        for' ["ls", "xargs"] $
V
Vidar Holen 已提交
385
            \x -> warn x 2011 "Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames."
386
        ]
V
Vidar Holen 已提交
387
    unless didLs $ do
388
        for ["ls", "?"] $
389
            \(ls:_) -> unless (hasShortParameter 'N' (oversimplify ls)) $
V
Vidar Holen 已提交
390
                info (getId ls) 2012 "Use find instead of ls to better handle non-alphanumeric filenames."
391
        return ()
392 393
  where
    for l f =
394
        let indices = indexOfSublists l (map (headOrDefault "" . oversimplify) commands)
395
        in do
R
Rodrigo Setti 已提交
396
            mapM_ (f . (\ n -> take (length l) $ drop n commands)) indices
397 398
            return . not . null $ indices
    for' l f = for l (first f)
V
Vidar Holen 已提交
399
    first func (x:_) = func (getId $ getCommandTokenOrThis x)
400
    first _ _ = return ()
V
Vidar Holen 已提交
401 402 403
    hasShortParameter char = any (\x -> "-" `isPrefixOf` x && char `elem` x)
    hasParameter string =
        any (isPrefixOf string . dropWhile (== '-'))
V
Vidar Holen 已提交
404
checkPipePitfalls _ _ = return ()
405

R
Rodrigo Setti 已提交
406
indexOfSublists sub = f 0
407 408 409 410
  where
    f _ [] = []
    f n a@(r:rest) =
        let others = f (n+1) rest in
411
            if match sub a
412 413
              then n:others
              else others
414
    match ("?":r1) (_:r2) = match r1 r2
415
    match (x1:r1) (x2:r2) | x1 == x2 = match r1 r2
416
    match [] _ = True
417 418
    match _ _ = False

419

V
Vidar Holen 已提交
420 421
prop_checkShebangParameters1 = verifyTree checkShebangParameters "#!/usr/bin/env bash -x\necho cow"
prop_checkShebangParameters2 = verifyNotTree checkShebangParameters "#! /bin/sh  -l "
422
checkShebangParameters p (T_Annotation _ _ t) = checkShebangParameters p t
V
Vidar Holen 已提交
423
checkShebangParameters _ (T_Script id sb _) =
424
    [makeComment ErrorC id 2096 "On most OS, shebangs can only specify a single parameter." | length (words sb) > 2]
425

V
Vidar Holen 已提交
426 427 428
prop_checkShebang1 = verifyNotTree checkShebang "#!/usr/bin/env bash -x\necho cow"
prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh  -l "
prop_checkShebang3 = verifyTree checkShebang "ls -l"
429
prop_checkShebang4 = verifyNotTree checkShebang "#shellcheck shell=sh\nfoo"
V
Vidar Holen 已提交
430 431 432
prop_checkShebang5 = verifyTree checkShebang "#!/usr/bin/env ash"
prop_checkShebang6 = verifyNotTree checkShebang "#!/usr/bin/env ash\n# shellcheck shell=dash\n"
prop_checkShebang7 = verifyNotTree checkShebang "#!/usr/bin/env ash\n# shellcheck shell=sh\n"
433 434 435 436 437
checkShebang params (T_Annotation _ list t) =
    if any isOverride list then [] else checkShebang params t
  where
    isOverride (ShellOverride _) = True
    isOverride _ = False
V
Vidar Holen 已提交
438 439 440 441 442 443
checkShebang params (T_Script id sb _) = execWriter $
    unless (shellTypeSpecified params) $ do
        when (sb == "") $
            err id 2148 "Tips depend on target shell and yours is unknown. Add a shebang."
        when (executableFromShebang sb == "ash") $
            warn id 2187 "Ash scripts will be checked as Dash. Add '# shellcheck shell=dash' to silence."
V
Vidar Holen 已提交
444

445

446
prop_checkForInQuoted = verify checkForInQuoted "for f in \"$(ls)\"; do echo foo; done"
447
prop_checkForInQuoted2 = verifyNot checkForInQuoted "for f in \"$@\"; do echo foo; done"
448
prop_checkForInQuoted2a = verifyNot checkForInQuoted "for f in *.mp3; do echo foo; done"
449
prop_checkForInQuoted2b = verify checkForInQuoted "for f in \"*.mp3\"; do echo foo; done"
450
prop_checkForInQuoted3 = verify checkForInQuoted "for f in 'find /'; do true; done"
451
prop_checkForInQuoted4 = verify checkForInQuoted "for f in 1,2,3; do true; done"
452
prop_checkForInQuoted4a = verifyNot checkForInQuoted "for f in foo{1,2,3}; do true; done"
453
prop_checkForInQuoted5 = verify checkForInQuoted "for f in ls; do true; done"
454
prop_checkForInQuoted6 = verifyNot checkForInQuoted "for f in \"${!arr}\"; do true; done"
V
Vidar Holen 已提交
455
checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) =
456
    when (any (\x -> willSplit x && not (mayBecomeMultipleArgs x)) list
457
            || (fmap wouldHaveBeenGlob (getLiteralString word) == Just True)) $
R
Rodrigo Setti 已提交
458
        err id 2066 "Since you double quoted this, it will not word split, and the loop will only run once."
V
Vidar Holen 已提交
459 460
checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_SingleQuoted id _]] _) =
    warn id 2041 "This is a literal string. To run as a command, use $(..) instead of '..' . "
V
Vidar Holen 已提交
461
checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_Literal id s]] _) =
462
    if ',' `elem` s
V
Vidar Holen 已提交
463
      then unless ('{' `elem` s) $
R
Rodrigo Setti 已提交
464
            warn id 2042 "Use spaces, not commas, to separate loop elements."
V
Vidar Holen 已提交
465
      else warn id 2043 "This loop will only ever run once for a constant value. Did you perhaps mean to loop over dir/*, $var or $(cmd)?"
V
Vidar Holen 已提交
466
checkForInQuoted _ _ = return ()
467

V
Vidar Holen 已提交
468
prop_checkForInCat1 = verify checkForInCat "for f in $(cat foo); do stuff; done"
469
prop_checkForInCat1a= verify checkForInCat "for f in `cat foo`; do stuff; done"
V
Vidar Holen 已提交
470
prop_checkForInCat2 = verify checkForInCat "for f in $(cat foo | grep lol); do stuff; done"
471
prop_checkForInCat2a= verify checkForInCat "for f in `cat foo | grep lol`; do stuff; done"
V
Vidar Holen 已提交
472
prop_checkForInCat3 = verifyNot checkForInCat "for f in $(cat foo | grep bar | wc -l); do stuff; done"
V
Vidar Holen 已提交
473
checkForInCat _ (T_ForIn _ f [T_NormalWord _ w] _) = mapM_ checkF w
V
Vidar Holen 已提交
474
  where
V
Vidar Holen 已提交
475
    checkF (T_DollarExpansion id [T_Pipeline _ _ r])
V
Vidar Holen 已提交
476
        | all isLineBased r =
V
Vidar Holen 已提交
477
            info id 2013 "To read lines rather than words, pipe/redirect to a 'while read' loop."
478
    checkF (T_Backticked id cmds) = checkF (T_DollarExpansion id cmds)
V
Vidar Holen 已提交
479
    checkF _ = return ()
480
    isLineBased cmd = any (cmd `isCommand`)
481
                        ["grep", "fgrep", "egrep", "sed", "cat", "awk", "cut", "sort"]
V
Vidar Holen 已提交
482
checkForInCat _ _ = return ()
483 484

prop_checkForInLs = verify checkForInLs "for f in $(ls *.mp3); do mplayer \"$f\"; done"
485
prop_checkForInLs2 = verify checkForInLs "for f in `ls *.mp3`; do mplayer \"$f\"; done"
486
prop_checkForInLs3 = verify checkForInLs "for f in `find / -name '*.mp3'`; do mplayer \"$f\"; done"
R
Rodrigo Setti 已提交
487
checkForInLs _ = try
488
  where
V
Vidar Holen 已提交
489
   try (T_ForIn _ f [T_NormalWord _ [T_DollarExpansion id [x]]] _) =
490
        check id f x
V
Vidar Holen 已提交
491
   try (T_ForIn _ f [T_NormalWord _ [T_Backticked id [x]]] _) =
492 493 494
        check id f x
   try _ = return ()
   check id f x =
495
    case oversimplify x of
496
      ("ls":n) ->
497
        let warntype = if any ("-" `isPrefixOf`) n then warn else err in
498 499
          warntype id 2045 "Iterating over ls output is fragile. Use globs."
      ("find":_) -> warn id 2044 "For loops over find output are fragile. Use find -exec or a while read loop."
500
      _ -> return ()
501 502


503 504 505 506 507
prop_checkFindExec1 = verify checkFindExec "find / -name '*.php' -exec rm {};"
prop_checkFindExec2 = verify checkFindExec "find / -exec touch {} && ls {} \\;"
prop_checkFindExec3 = verify checkFindExec "find / -execdir cat {} | grep lol +"
prop_checkFindExec4 = verifyNot checkFindExec "find / -name '*.php' -exec foo {} +"
prop_checkFindExec5 = verifyNot checkFindExec "find / -execdir bash -c 'a && b' \\;"
508
prop_checkFindExec6 = verify checkFindExec "find / -type d -execdir rm *.jpg \\;"
V
Vidar Holen 已提交
509
checkFindExec _ cmd@(T_SimpleCommand _ _ t@(h:r)) | cmd `isCommand` "find" = do
510
    c <- broken r False
R
Rodrigo Setti 已提交
511
    when c $
512
        let wordId = getId $ last t in
V
Vidar Holen 已提交
513
            err wordId 2067 "Missing ';' or + terminating -exec. You can't use |/||/&&, and ';' has to be a separate, quoted argument."
514 515

  where
516 517
    broken [] v = return v
    broken (w:r) v = do
R
Rodrigo Setti 已提交
518
        when v (mapM_ warnFor $ fromWord w)
519 520 521 522 523 524 525 526 527 528 529 530
        case getLiteralString w of
            Just "-exec" -> broken r True
            Just "-execdir" -> broken r True
            Just "+" -> broken r False
            Just ";" -> broken r False
            _ -> broken r v

    shouldWarn x =
      case x of
        T_DollarExpansion _ _ -> True
        T_Backticked _ _ -> True
        T_Glob _ _ -> True
R
Rodrigo Setti 已提交
531
        T_Extglob {} -> True
532 533 534
        _ -> False

    warnFor x =
V
Vidar Holen 已提交
535 536
        when(shouldWarn x) $
            info (getId x) 2014 "This will expand once before find runs, not per file found."
537 538 539

    fromWord (T_NormalWord _ l) = l
    fromWord _ = []
V
Vidar Holen 已提交
540 541 542 543 544 545 546 547 548 549 550 551
checkFindExec _ _ = return ()


prop_checkUnquotedExpansions1 = verify checkUnquotedExpansions "rm $(ls)"
prop_checkUnquotedExpansions1a= verify checkUnquotedExpansions "rm `ls`"
prop_checkUnquotedExpansions2 = verify checkUnquotedExpansions "rm foo$(date)"
prop_checkUnquotedExpansions3 = verify checkUnquotedExpansions "[ $(foo) == cow ]"
prop_checkUnquotedExpansions3a= verify checkUnquotedExpansions "[ ! $(foo) ]"
prop_checkUnquotedExpansions4 = verifyNot checkUnquotedExpansions "[[ $(foo) == cow ]]"
prop_checkUnquotedExpansions5 = verifyNot checkUnquotedExpansions "for f in $(cmd); do echo $f; done"
prop_checkUnquotedExpansions6 = verifyNot checkUnquotedExpansions "$(cmd)"
prop_checkUnquotedExpansions7 = verifyNot checkUnquotedExpansions "cat << foo\n$(ls)\nfoo"
552
prop_checkUnquotedExpansions8 = verifyNot checkUnquotedExpansions "set -- $(seq 1 4)"
553
prop_checkUnquotedExpansions9 = verifyNot checkUnquotedExpansions "echo foo `# inline comment`"
R
Rodrigo Setti 已提交
554 555
checkUnquotedExpansions params =
    check
556
  where
557 558 559
    check t@(T_DollarExpansion _ c) = examine t c
    check t@(T_Backticked _ c) = examine t c
    check t@(T_DollarBraceCommandExpansion _ c) = examine t c
560
    check _ = return ()
V
Vidar Holen 已提交
561
    tree = parentMap params
562 563
    examine t contents =
        unless (null contents || shouldBeSplit t || isQuoteFree tree t || usedAsCommandName tree t) $
V
Vidar Holen 已提交
564
            warn (getId t) 2046 "Quote this to prevent word splitting."
565

566 567 568
    shouldBeSplit t =
        getCommandNameFromExpansion t == Just "seq"

569

V
Vidar Holen 已提交
570 571 572 573 574
prop_checkRedirectToSame = verify checkRedirectToSame "cat foo > foo"
prop_checkRedirectToSame2 = verify checkRedirectToSame "cat lol | sed -e 's/a/b/g' > lol"
prop_checkRedirectToSame3 = verifyNot checkRedirectToSame "cat lol | sed -e 's/a/b/g' > foo.bar && mv foo.bar lol"
prop_checkRedirectToSame4 = verifyNot checkRedirectToSame "foo /dev/null > /dev/null"
prop_checkRedirectToSame5 = verifyNot checkRedirectToSame "foo > bar 2> bar"
575 576
prop_checkRedirectToSame6 = verifyNot checkRedirectToSame "echo foo > foo"
prop_checkRedirectToSame7 = verifyNot checkRedirectToSame "sed 's/foo/bar/g' file | sponge file"
577
prop_checkRedirectToSame8 = verifyNot checkRedirectToSame "while read -r line; do _=\"$fname\"; done <\"$fname\""
V
Vidar Holen 已提交
578
checkRedirectToSame params s@(T_Pipeline _ _ list) =
579
    mapM_ (\l -> (mapM_ (\x -> doAnalysis (checkOccurrences x) l) (getAllRedirs list))) list
V
Vidar Holen 已提交
580
  where
581
    note x = makeComment InfoC x 2094
V
Vidar Holen 已提交
582
                "Make sure not to read and write the same file in the same pipeline."
583
    checkOccurrences t@(T_NormalWord exceptId x) u@(T_NormalWord newId y) =
V
Vidar Holen 已提交
584 585 586
        when (exceptId /= newId
                && x == y
                && not (isOutput t && isOutput u)
587
                && not (special t)
588 589
                && not (any isHarmlessCommand [t,u])
                && not (any containsAssignment [u])) $ do
590 591
            addComment $ note newId
            addComment $ note exceptId
592
    checkOccurrences _ _ = return ()
V
Vidar Holen 已提交
593 594 595 596
    getAllRedirs = concatMap (\t ->
        case t of
            T_Redirecting _ ls _ -> concatMap getRedirs ls
            _ -> [])
V
Vidar Holen 已提交
597 598 599 600 601 602
    getRedirs (T_FdRedirect _ _ (T_IoFile _ op file)) =
            case op of T_Greater _ -> [file]
                       T_Less _    -> [file]
                       T_DGREAT _  -> [file]
                       _ -> []
    getRedirs _ = []
603
    special x = "/dev/" `isPrefixOf` concat (oversimplify x)
V
Vidar Holen 已提交
604 605
    isOutput t =
        case drop 1 $ getPath (parentMap params) t of
R
Rodrigo Setti 已提交
606
            T_IoFile _ op _:_ ->
V
Vidar Holen 已提交
607 608 609 610 611
                case op of
                    T_Greater _  -> True
                    T_DGREAT _ -> True
                    _ -> False
            _ -> False
612 613 614 615
    isHarmlessCommand arg = fromMaybe False $ do
        cmd <- getClosestCommand (parentMap params) arg
        name <- getCommandBasename cmd
        return $ name `elem` ["echo", "printf", "sponge"]
616 617 618
    containsAssignment arg = fromMaybe False $ do
        cmd <- getClosestCommand (parentMap params) arg
        return $ isAssignment cmd
619

620
checkRedirectToSame _ _ = return ()
621 622


V
Vidar Holen 已提交
623 624
prop_checkShorthandIf  = verify checkShorthandIf "[[ ! -z file ]] && scp file host || rm file"
prop_checkShorthandIf2 = verifyNot checkShorthandIf "[[ ! -z file ]] && { scp file host || echo 'Eek'; }"
625 626
prop_checkShorthandIf3 = verifyNot checkShorthandIf "foo && bar || echo baz"
prop_checkShorthandIf4 = verifyNot checkShorthandIf "foo && a=b || a=c"
627
prop_checkShorthandIf5 = verifyNot checkShorthandIf "foo && rm || printf b"
628 629 630 631 632
prop_checkShorthandIf6 = verifyNot checkShorthandIf "if foo && bar || baz; then true; fi"
prop_checkShorthandIf7 = verifyNot checkShorthandIf "while foo && bar || baz; do true; done"
prop_checkShorthandIf8 = verify checkShorthandIf "if true; then foo && bar || baz; fi"
checkShorthandIf params x@(T_AndIf id _ (T_OrIf _ _ (T_Pipeline _ _ t)))
        | not (isOk t || inCondition) =
V
Vidar Holen 已提交
633
    info id 2015 "Note that A && B || C is not if-then-else. C may run when A is true."
634
  where
R
Rodrigo Setti 已提交
635
    isOk [t] = isAssignment t || fromMaybe False (do
636
        name <- getCommandBasename t
637
        return $ name `elem` ["echo", "exit", "return", "printf"])
638
    isOk _ = False
639
    inCondition = isCondition $ getPath (parentMap params) x
V
Vidar Holen 已提交
640
checkShorthandIf _ _ = return ()
V
Vidar Holen 已提交
641

642

643
prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done"
644
prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*"
645
prop_checkDollarStar3 = verifyNot checkDollarStar "[[ $* = 'a b' ]]"
646 647
checkDollarStar p t@(T_NormalWord _ [b@(T_DollarBraced id _)])
      | bracedString b == "*"  =
648
    unless (isStrictlyQuoteFree (parentMap p) t) $
R
Rodrigo Setti 已提交
649
        warn id 2048 "Use \"$@\" (with quotes) to prevent whitespace problems."
V
Vidar Holen 已提交
650
checkDollarStar _ _ = return ()
651 652 653


prop_checkUnquotedDollarAt = verify checkUnquotedDollarAt "ls $@"
V
Vidar Holen 已提交
654 655 656 657 658
prop_checkUnquotedDollarAt1= verifyNot checkUnquotedDollarAt "ls ${#@}"
prop_checkUnquotedDollarAt2 = verify checkUnquotedDollarAt "ls ${foo[@]}"
prop_checkUnquotedDollarAt3 = verifyNot checkUnquotedDollarAt "ls ${#foo[@]}"
prop_checkUnquotedDollarAt4 = verifyNot checkUnquotedDollarAt "ls \"$@\""
prop_checkUnquotedDollarAt5 = verifyNot checkUnquotedDollarAt "ls ${foo/@/ at }"
659
prop_checkUnquotedDollarAt6 = verifyNot checkUnquotedDollarAt "a=$@"
660 661
prop_checkUnquotedDollarAt7 = verify checkUnquotedDollarAt "for f in ${var[@]}; do true; done"
prop_checkUnquotedDollarAt8 = verifyNot checkUnquotedDollarAt "echo \"${args[@]:+${args[@]}}\""
662
prop_checkUnquotedDollarAt9 = verifyNot checkUnquotedDollarAt "echo ${args[@]:+\"${args[@]}\"}"
V
Vidar Holen 已提交
663
prop_checkUnquotedDollarAt10 = verifyNot checkUnquotedDollarAt "echo ${@+\"$@\"}"
664
checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not $ isStrictlyQuoteFree (parentMap p) word =
R
Rodrigo Setti 已提交
665
    forM_ (take 1 $ filter isArrayExpansion parts) $ \x ->
V
Vidar Holen 已提交
666
        unless (isQuotedAlternativeReference x) $
667 668
            err (getId x) 2068
                "Double quote array expansions to avoid re-splitting elements."
V
Vidar Holen 已提交
669
checkUnquotedDollarAt _ _ = return ()
670

671 672 673 674 675
prop_checkConcatenatedDollarAt1 = verify checkConcatenatedDollarAt "echo \"foo$@\""
prop_checkConcatenatedDollarAt2 = verify checkConcatenatedDollarAt "echo ${arr[@]}lol"
prop_checkConcatenatedDollarAt3 = verify checkConcatenatedDollarAt "echo $a$@"
prop_checkConcatenatedDollarAt4 = verifyNot checkConcatenatedDollarAt "echo $@"
prop_checkConcatenatedDollarAt5 = verifyNot checkConcatenatedDollarAt "echo \"${arr[@]}\""
676
checkConcatenatedDollarAt p word@T_NormalWord {}
677 678 679 680 681 682 683 684
    | not $ isQuoteFree (parentMap p) word =
        unless (null $ drop 1 parts) $
            mapM_ for array
  where
    parts = getWordParts word
    array = take 1 $ filter isArrayExpansion parts
    for t = err (getId t) 2145 "Argument mixes string and array. Use * or separate argument."
checkConcatenatedDollarAt _ _ = return ()
685 686 687 688 689 690 691 692 693 694 695 696 697 698 699 700 701 702 703

prop_checkArrayAsString1 = verify checkArrayAsString "a=$@"
prop_checkArrayAsString2 = verify checkArrayAsString "a=\"${arr[@]}\""
prop_checkArrayAsString3 = verify checkArrayAsString "a=*.png"
prop_checkArrayAsString4 = verify checkArrayAsString "a={1..10}"
prop_checkArrayAsString5 = verifyNot checkArrayAsString "a='*.gif'"
prop_checkArrayAsString6 = verifyNot checkArrayAsString "a=$*"
prop_checkArrayAsString7 = verifyNot checkArrayAsString "a=( $@ )"
checkArrayAsString _ (T_Assignment id _ _ _ word) =
    if willConcatInAssignment word
    then
      warn (getId word) 2124
        "Assigning an array to a string! Assign as array, or use * instead of @ to concatenate."
    else
      when (willBecomeMultipleArgs word) $
        warn (getId word) 2125
          "Brace expansions and globs are literal in assignments. Quote it or use an array."
checkArrayAsString _ _ = return ()

704 705
prop_checkArrayWithoutIndex1 = verifyTree checkArrayWithoutIndex "foo=(a b); echo $foo"
prop_checkArrayWithoutIndex2 = verifyNotTree checkArrayWithoutIndex "foo='bar baz'; foo=($foo); echo ${foo[0]}"
706 707
prop_checkArrayWithoutIndex3 = verifyTree checkArrayWithoutIndex "coproc foo while true; do echo cow; done; echo $foo"
prop_checkArrayWithoutIndex4 = verifyTree checkArrayWithoutIndex "coproc tail -f log; echo $COPROC"
708
prop_checkArrayWithoutIndex5 = verifyTree checkArrayWithoutIndex "a[0]=foo; echo $a"
709
prop_checkArrayWithoutIndex6 = verifyTree checkArrayWithoutIndex "echo $PIPESTATUS"
710 711
prop_checkArrayWithoutIndex7 = verifyTree checkArrayWithoutIndex "a=(a b); a+=c"
prop_checkArrayWithoutIndex8 = verifyTree checkArrayWithoutIndex "declare -a foo; foo=bar;"
712
checkArrayWithoutIndex params _ =
713
    doVariableFlowAnalysis readF writeF defaultMap (variableFlow params)
714
  where
715
    defaultMap = Map.fromList $ map (\x -> (x,())) arrayVariables
716 717 718 719
    readF _ (T_DollarBraced id token) _ = do
        map <- get
        return . maybeToList $ do
            name <- getLiteralString token
720
            assigned <- Map.lookup name map
721 722
            return $ makeComment WarningC id 2128
                    "Expanding an array without an index only gives the first element."
723 724
    readF _ _ _ = return []

725
    writeF _ (T_Assignment id mode name [] _) _ (DataString _) = do
726 727 728 729 730 731
        isArray <- gets (isJust . Map.lookup name)
        return $ if not isArray then [] else
            case mode of
                Assign -> [makeComment WarningC id 2178 "Variable was used as an array but is now assigned a string."]
                Append -> [makeComment WarningC id 2179 "Use array+=(\"item\") to append items to an array."]

732
    writeF _ t name (DataArray _) = do
733
        modify (Map.insert name ())
734
        return []
735 736
    writeF _ expr name _ = do
        if isIndexed expr
737
          then modify (Map.insert name ())
738
          else modify (Map.delete name)
739 740
        return []

741 742
    isIndexed expr =
        case expr of
743
            T_Assignment _ _ _ (_:_) _ -> True
744 745
            _ -> False

V
Vidar Holen 已提交
746 747
prop_checkStderrRedirect = verify checkStderrRedirect "test 2>&1 > cow"
prop_checkStderrRedirect2 = verifyNot checkStderrRedirect "test > cow 2>&1"
748 749 750 751
prop_checkStderrRedirect3 = verifyNot checkStderrRedirect "test 2>&1 > file | grep stderr"
prop_checkStderrRedirect4 = verifyNot checkStderrRedirect "errors=$(test 2>&1 > file)"
prop_checkStderrRedirect5 = verifyNot checkStderrRedirect "read < <(test 2>&1 > file)"
prop_checkStderrRedirect6 = verify checkStderrRedirect "foo | bar 2>&1 > /dev/null"
752
prop_checkStderrRedirect7 = verifyNot checkStderrRedirect "{ cmd > file; } 2>&1"
753
checkStderrRedirect params redir@(T_Redirecting _ [
754
    T_FdRedirect id "2" (T_IoDuplicate _ (T_GREATAND _) "1"),
V
Vidar Holen 已提交
755 756 757 758 759
    T_FdRedirect _ _ (T_IoFile _ op _)
    ] _) = case op of
            T_Greater _ -> error
            T_DGREAT _ -> error
            _ -> return ()
760 761 762 763
  where
    usesOutput t =
        case t of
            (T_Pipeline _ _ list) -> length list > 1 && not (isParentOf (parentMap params) (last list) redir)
764 765 766
            T_ProcSub {} -> True
            T_DollarExpansion {} -> True
            T_Backticked {} -> True
767 768 769 770
            _ -> False
    isCaptured = any usesOutput $ getPath (parentMap params) redir

    error = unless isCaptured $
771
        warn id 2069 "To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify)."
772

V
Vidar Holen 已提交
773
checkStderrRedirect _ _ = return ()
V
Vidar Holen 已提交
774

775 776
lt x = trace ("Tracing " ++ show x) x
ltt t = trace ("Tracing " ++ show t)
V
Vidar Holen 已提交
777 778


V
Vidar Holen 已提交
779 780 781 782 783 784 785 786 787 788
prop_checkSingleQuotedVariables  = verify checkSingleQuotedVariables "echo '$foo'"
prop_checkSingleQuotedVariables2 = verify checkSingleQuotedVariables "echo 'lol$1.jpg'"
prop_checkSingleQuotedVariables3 = verifyNot checkSingleQuotedVariables "sed 's/foo$/bar/'"
prop_checkSingleQuotedVariables3a= verify checkSingleQuotedVariables "sed 's/${foo}/bar/'"
prop_checkSingleQuotedVariables3b= verify checkSingleQuotedVariables "sed 's/$(echo cow)/bar/'"
prop_checkSingleQuotedVariables3c= verify checkSingleQuotedVariables "sed 's/$((1+foo))/bar/'"
prop_checkSingleQuotedVariables4 = verifyNot checkSingleQuotedVariables "awk '{print $1}'"
prop_checkSingleQuotedVariables5 = verifyNot checkSingleQuotedVariables "trap 'echo $SECONDS' EXIT"
prop_checkSingleQuotedVariables6 = verifyNot checkSingleQuotedVariables "sed -n '$p'"
prop_checkSingleQuotedVariables6a= verify checkSingleQuotedVariables "sed -n '$pattern'"
789
prop_checkSingleQuotedVariables7 = verifyNot checkSingleQuotedVariables "PS1='$PWD \\$ '"
790 791
prop_checkSingleQuotedVariables8 = verify checkSingleQuotedVariables "find . -exec echo '$1' {} +"
prop_checkSingleQuotedVariables9 = verifyNot checkSingleQuotedVariables "find . -exec awk '{print $1}' {} \\;"
V
Vidar Holen 已提交
792
prop_checkSingleQuotedVariables10= verify checkSingleQuotedVariables "echo '`pwd`'"
793 794
prop_checkSingleQuotedVariables11= verifyNot checkSingleQuotedVariables "sed '${/lol/d}'"
prop_checkSingleQuotedVariables12= verifyNot checkSingleQuotedVariables "eval 'echo $1'"
795
prop_checkSingleQuotedVariables13= verifyNot checkSingleQuotedVariables "busybox awk '{print $1}'"
796
prop_checkSingleQuotedVariables14= verifyNot checkSingleQuotedVariables "[ -v 'bar[$foo]' ]"
797 798
prop_checkSingleQuotedVariables15= verifyNot checkSingleQuotedVariables "git filter-branch 'test $GIT_COMMIT'"
prop_checkSingleQuotedVariables16= verify checkSingleQuotedVariables "git '$a'"
V
Vidar Holen 已提交
799 800
prop_checkSingleQuotedVariables17= verifyNot checkSingleQuotedVariables "rename 's/(.)a/$1/g' *"

V
Vidar Holen 已提交
801
checkSingleQuotedVariables params t@(T_SingleQuoted id s) =
V
Vidar Holen 已提交
802 803 804 805
    when (s `matches` re) $
        if "sed" == commandName
        then unless (s `matches` sedContra) showMessage
        else unless isProbablyOk showMessage
V
Vidar Holen 已提交
806
  where
V
Vidar Holen 已提交
807
    parents = parentMap params
R
Rodrigo Setti 已提交
808
    showMessage = info id 2016
V
Vidar Holen 已提交
809 810
        "Expressions don't expand in single quotes, use double quotes for that."
    commandName = fromMaybe "" $ do
V
Vidar Holen 已提交
811
        cmd <- getClosestCommand parents t
812
        name <- getCommandBasename cmd
813
        return $ if name == "find" then getFindCommand cmd else if name == "git" then getGitCommand cmd else name
V
Vidar Holen 已提交
814

815
    isProbablyOk =
R
Rodrigo Setti 已提交
816
            any isOkAssignment (take 3 $ getPath parents t)
817
            || commandName `elem` [
V
Vidar Holen 已提交
818 819 820 821 822
                "trap"
                ,"sh"
                ,"bash"
                ,"ksh"
                ,"zsh"
823
                ,"ssh"
824
                ,"eval"
825
                ,"xprop"
826
                ,"alias"
827
                ,"sudo" -- covering "sudo sh" and such
828
                ,"docker" -- like above
829
                ,"dpkg-query"
830
                ,"jq"  -- could also check that user provides --arg
831
                ,"rename"
832
                ,"unset"
833
                ,"git filter-branch"
V
Vidar Holen 已提交
834
                ]
V
Vidar Holen 已提交
835 836
            || "awk" `isSuffixOf` commandName
            || "perl" `isPrefixOf` commandName
V
Vidar Holen 已提交
837

838 839 840 841
    commonlyQuoted = ["PS1", "PS2", "PS3", "PS4", "PROMPT_COMMAND"]
    isOkAssignment t =
        case t of
            T_Assignment _ _ name _ _ -> name `elem` commonlyQuoted
842
            TC_Unary _ _ "-v" _ -> True
843
            _ -> False
844

V
Vidar Holen 已提交
845
    re = mkRegex "\\$[{(0-9a-zA-Z_]|`.*`"
846
    sedContra = mkRegex "\\$[{dpsaic]($|[^a-zA-Z])"
847 848 849 850 851 852 853 854 855 856

    getFindCommand (T_SimpleCommand _ _ words) =
        let list = map getLiteralString words
            cmd  = dropWhile (\x -> x /= Just "-exec" && x /= Just "-execdir") list
        in
          case cmd of
            (flag:cmd:rest) -> fromMaybe "find" cmd
            _ -> "find"
    getFindCommand (T_Redirecting _ _ cmd) = getFindCommand cmd
    getFindCommand _ = "find"
857 858 859 860 861 862
    getGitCommand (T_SimpleCommand _ _ words) =
        case map getLiteralString words of
            Just "git":Just "filter-branch":_ -> "git filter-branch"
            _ -> "git"
    getGitCommand (T_Redirecting _ _ cmd) = getGitCommand cmd
    getGitCommand _ = "git"
V
Vidar Holen 已提交
863
checkSingleQuotedVariables _ _ = return ()
864 865


866 867 868
prop_checkUnquotedN = verify checkUnquotedN "if [ -n $foo ]; then echo cow; fi"
prop_checkUnquotedN2 = verify checkUnquotedN "[ -n $cow ]"
prop_checkUnquotedN3 = verifyNot checkUnquotedN "[[ -n $foo ]] && echo cow"
V
Vidar Holen 已提交
869
prop_checkUnquotedN4 = verify checkUnquotedN "[ -n $cow -o -t 1 ]"
870
prop_checkUnquotedN5 = verifyNot checkUnquotedN "[ -n \"$@\" ]"
V
Vidar Holen 已提交
871 872
checkUnquotedN _ (TC_Unary _ SingleBracket "-n" (T_NormalWord id [t])) | willSplit t =
       err id 2070 "-n doesn't work with unquoted arguments. Quote or use [[ ]]."
V
Vidar Holen 已提交
873
checkUnquotedN _ _ = return ()
V
Vidar Holen 已提交
874 875 876 877

prop_checkNumberComparisons1 = verify checkNumberComparisons "[[ $foo < 3 ]]"
prop_checkNumberComparisons2 = verify checkNumberComparisons "[[ 0 >= $(cmd) ]]"
prop_checkNumberComparisons3 = verifyNot checkNumberComparisons "[[ $foo ]] > 3"
V
Vidar Holen 已提交
878 879
prop_checkNumberComparisons4 = verify checkNumberComparisons "[[ $foo > 2.72 ]]"
prop_checkNumberComparisons5 = verify checkNumberComparisons "[[ $foo -le 2.72 ]]"
880 881
prop_checkNumberComparisons6 = verify checkNumberComparisons "[[ 3.14 -eq $foo ]]"
prop_checkNumberComparisons7 = verifyNot checkNumberComparisons "[[ 3.14 == $foo ]]"
882
prop_checkNumberComparisons8 = verify checkNumberComparisons "[ foo <= bar ]"
883
prop_checkNumberComparisons9 = verify checkNumberComparisons "[ foo \\>= bar ]"
884 885 886 887 888
prop_checkNumberComparisons11 = verify checkNumberComparisons "[ $foo -eq 'N' ]"
prop_checkNumberComparisons12 = verify checkNumberComparisons "[ x$foo -gt x${N} ]"
prop_checkNumberComparisons13 = verify checkNumberComparisons "[ $foo > $bar ]"
prop_checkNumberComparisons14 = verifyNot checkNumberComparisons "[[ foo < bar ]]"
prop_checkNumberComparisons15 = verifyNot checkNumberComparisons "[ $foo '>' $bar ]"
889
checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do
890
    if isNum lhs || isNum rhs
891 892 893
      then do
        when (isLtGt op) $
          err id 2071 $
R
Rodrigo Setti 已提交
894
            op ++ " is for string comparisons. Use " ++ eqv op ++ " instead."
895
        when (isLeGe op && hasStringComparison) $
896
            err id 2071 $ op ++ " is not a valid operator. " ++
R
Rodrigo Setti 已提交
897
              "Use " ++ eqv op ++ " ."
898 899 900 901
      else do
        when (isLeGe op || isLtGt op) $
            mapM_ checkDecimals [lhs, rhs]

902
        when (isLeGe op && hasStringComparison) $
903
            err id 2122 $ op ++ " is not a valid operator. " ++
904 905 906 907 908 909 910
                "Use '! a " ++ esc ++ invert op ++ " b' instead."

        when (typ == SingleBracket && op `elem` ["<", ">"]) $
            case shellType params of
                Sh -> return ()  -- These are unsupported and will be caught by bashism checks.
                Dash -> err id 2073 $ "Escape \\" ++ op ++ " to prevent it redirecting."
                _ -> err id 2073 $ "Escape \\" ++ op ++ " to prevent it redirecting (or switch to [[ .. ]])."
V
Vidar Holen 已提交
911

912
    when (op `elem` ["-lt", "-gt", "-le", "-ge", "-eq"]) $ do
V
Vidar Holen 已提交
913
        mapM_ checkDecimals [lhs, rhs]
914 915
        when (typ == SingleBracket) $
            checkStrings [lhs, rhs]
V
Vidar Holen 已提交
916 917

  where
918
      hasStringComparison = shellType params /= Sh
919 920 921 922
      isLtGt = flip elem ["<", "\\<", ">", "\\>"]
      isLeGe = flip elem ["<=", "\\<=", ">=", "\\>="]

      checkDecimals hs =
923
        when (isFraction hs && not (hasFloatingPoint params)) $
924 925 926 927
            err (getId hs) 2072 decimalError
      decimalError = "Decimals are not supported. " ++
        "Either use integers only, or use bc or awk to compare."

928 929
      checkStrings =
        mapM_ stringError . take 1 . filter isNonNum
930 931 932 933 934 935

      isNonNum t = fromMaybe False $ do
        s <- getLiteralStringExt (const $ return "") t
        return . not . all numChar $ s
      numChar x = isDigit x || x `elem` "+-. "

936 937
      stringError t = err (getId t) 2170 $
          "Numerical " ++ op ++ " does not dereference in [..]. Expand or use string operator."
938

939
      isNum t =
940
        case oversimplify t of
941 942 943
            [v] -> all isDigit v
            _ -> False
      isFraction t =
944
        case oversimplify t of
945 946 947
            [v] -> isJust $ matchRegex floatRegex v
            _ -> False

948
      eqv ('\\':s) = eqv s
V
Vidar Holen 已提交
949 950 951 952 953
      eqv "<" = "-lt"
      eqv ">" = "-gt"
      eqv "<=" = "-le"
      eqv ">=" = "-ge"
      eqv _ = "the numerical equivalent"
954

955 956 957 958 959 960 961 962 963
      esc = if typ == SingleBracket then "\\" else ""
      seqv "-ge" = "! a " ++ esc ++ "< b"
      seqv "-gt" = esc ++ ">"
      seqv "-le" = "! a " ++ esc ++ "> b"
      seqv "-lt" = esc ++ "<"
      seqv "-eq" = "="
      seqv "-ne" = "!="
      seqv _ = "the string equivalent"

964 965 966 967
      invert ('\\':s) = invert s
      invert "<=" = ">"
      invert ">=" = "<"

968
      floatRegex = mkRegex "^[-+]?[0-9]+\\.[0-9]+$"
V
Vidar Holen 已提交
969
checkNumberComparisons _ _ = return ()
V
Vidar Holen 已提交
970

971
prop_checkSingleBracketOperators1 = verify checkSingleBracketOperators "[ test =~ foo ]"
972 973 974
checkSingleBracketOperators params (TC_Binary id SingleBracket "=~" lhs rhs) =
    when (shellType params `elem` [Bash, Ksh]) $
        err id 2074 $ "Can't use =~ in [ ]. Use [[..]] instead."
V
Vidar Holen 已提交
975
checkSingleBracketOperators _ _ = return ()
976

977 978
prop_checkDoubleBracketOperators1 = verify checkDoubleBracketOperators "[[ 3 \\< 4 ]]"
prop_checkDoubleBracketOperators3 = verifyNot checkDoubleBracketOperators "[[ foo < bar ]]"
V
Vidar Holen 已提交
979
checkDoubleBracketOperators _ x@(TC_Binary id typ op lhs rhs)
980
    | typ == DoubleBracket && op `elem` ["\\<", "\\>"] =
V
Vidar Holen 已提交
981
        err id 2075 $ "Escaping " ++ op ++" is required in [..], but invalid in [[..]]"
V
Vidar Holen 已提交
982 983 984 985 986
checkDoubleBracketOperators _ _ = return ()

prop_checkConditionalAndOrs1 = verify checkConditionalAndOrs "[ foo && bar ]"
prop_checkConditionalAndOrs2 = verify checkConditionalAndOrs "[[ foo -o bar ]]"
prop_checkConditionalAndOrs3 = verifyNot checkConditionalAndOrs "[[ foo || bar ]]"
987 988 989 990 991 992 993 994 995 996 997 998 999 1000 1001 1002 1003 1004
prop_checkConditionalAndOrs4 = verify checkConditionalAndOrs "[ foo -a bar ]"
prop_checkConditionalAndOrs5 = verify checkConditionalAndOrs "[ -z 3 -o a = b ]"
checkConditionalAndOrs _ t =
    case t of
        (TC_And id SingleBracket "&&" _ _) ->
            err id 2107 "Instead of [ a && b ], use [ a ] && [ b ]."
        (TC_And id DoubleBracket "-a" _ _) ->
            err id 2108 "In [[..]], use && instead of -a."
        (TC_Or id SingleBracket "||" _ _) ->
            err id 2109 "Instead of [ a || b ], use [ a ] || [ b ]."
        (TC_Or id DoubleBracket "-o" _ _) ->
            err id 2110 "In [[..]], use || instead of -o."

        (TC_And id SingleBracket "-a" _ _) ->
            warn id 2166 "Prefer [ p ] && [ q ] as [ p -a q ] is not well defined."
        (TC_Or id SingleBracket "-o" _ _) ->
            warn id 2166 "Prefer [ p ] || [ q ] as [ p -o q ] is not well defined."

V
Vidar Holen 已提交
1005
        _ -> return ()
1006

1007 1008
prop_checkQuotedCondRegex1 = verify checkQuotedCondRegex "[[ $foo =~ \"bar.*\" ]]"
prop_checkQuotedCondRegex2 = verify checkQuotedCondRegex "[[ $foo =~ '(cow|bar)' ]]"
V
Vidar Holen 已提交
1009
prop_checkQuotedCondRegex3 = verifyNot checkQuotedCondRegex "[[ $foo =~ $foo ]]"
1010 1011
prop_checkQuotedCondRegex4 = verifyNot checkQuotedCondRegex "[[ $foo =~ \"bar\" ]]"
prop_checkQuotedCondRegex5 = verifyNot checkQuotedCondRegex "[[ $foo =~ 'cow bar' ]]"
1012
prop_checkQuotedCondRegex6 = verify checkQuotedCondRegex "[[ $foo =~ 'cow|bar' ]]"
V
Vidar Holen 已提交
1013
checkQuotedCondRegex _ (TC_Binary _ _ "=~" _ rhs) =
1014
    case rhs of
1015 1016
        T_NormalWord id [T_DoubleQuoted _ _] -> error rhs
        T_NormalWord id [T_SingleQuoted _ _] -> error rhs
V
Vidar Holen 已提交
1017 1018
        _ -> return ()
  where
1019 1020 1021 1022
    error t =
        unless (isConstantNonRe t) $
            err (getId t) 2076
                "Don't quote rhs of =~, it'll match literally rather than as a regex."
1023
    re = mkRegex "[][*.+()|]"
1024 1025 1026 1027
    hasMetachars s = s `matches` re
    isConstantNonRe t = fromMaybe False $ do
        s <- getLiteralString t
        return . not $ hasMetachars s
V
Vidar Holen 已提交
1028
checkQuotedCondRegex _ _ = return ()
V
Vidar Holen 已提交
1029

1030 1031 1032 1033 1034
prop_checkGlobbedRegex1 = verify checkGlobbedRegex "[[ $foo =~ *foo* ]]"
prop_checkGlobbedRegex2 = verify checkGlobbedRegex "[[ $foo =~ f* ]]"
prop_checkGlobbedRegex2a = verify checkGlobbedRegex "[[ $foo =~ \\#* ]]"
prop_checkGlobbedRegex3 = verifyNot checkGlobbedRegex "[[ $foo =~ $foo ]]"
prop_checkGlobbedRegex4 = verifyNot checkGlobbedRegex "[[ $foo =~ ^c.* ]]"
V
Vidar Holen 已提交
1035
checkGlobbedRegex _ (TC_Binary _ DoubleBracket "=~" _ rhs) =
1036
    let s = concat $ oversimplify rhs in
R
Rodrigo Setti 已提交
1037 1038
        when (isConfusedGlobRegex s) $
            warn (getId rhs) 2049 "=~ is for regex. Use == for globs."
V
Vidar Holen 已提交
1039
checkGlobbedRegex _ _ = return ()
1040

V
Vidar Holen 已提交
1041

1042
prop_checkConstantIfs1 = verify checkConstantIfs "[[ foo != bar ]]"
1043 1044 1045
prop_checkConstantIfs2a= verify checkConstantIfs "[ n -le 4 ]"
prop_checkConstantIfs2b= verifyNot checkConstantIfs "[[ n -le 4 ]]"
prop_checkConstantIfs3 = verify checkConstantIfs "[[ $n -le 4 && n != 2 ]]"
1046 1047
prop_checkConstantIfs4 = verifyNot checkConstantIfs "[[ $n -le 3 ]]"
prop_checkConstantIfs5 = verifyNot checkConstantIfs "[[ $n -le $n ]]"
1048 1049
prop_checkConstantIfs6 = verifyNot checkConstantIfs "[[ a -ot b ]]"
prop_checkConstantIfs7 = verifyNot checkConstantIfs "[ a -nt b ]"
V
Vidar Holen 已提交
1050
prop_checkConstantIfs8 = verifyNot checkConstantIfs "[[ ~foo == '~foo' ]]"
1051
prop_checkConstantIfs9 = verify checkConstantIfs "[[ *.png == [a-z] ]]"
1052
checkConstantIfs _ (TC_Binary id typ op lhs rhs) | not isDynamic =
1053 1054 1055
    if isConstant lhs && isConstant rhs
        then  warn id 2050 "This expression is constant. Did you forget the $ on a variable?"
        else checkUnmatchable id op lhs rhs
1056
  where
1057 1058 1059 1060
    isDynamic =
        op `elem` [ "-lt", "-gt", "-le", "-ge", "-eq", "-ne" ]
            && typ == DoubleBracket
        || op `elem` [ "-nt", "-ot", "-ef"]
1061 1062 1063 1064

    checkUnmatchable id op lhs rhs =
        when (op `elem` ["=", "==", "!="] && not (wordsCanBeEqual lhs rhs)) $
            warn id 2193 "The arguments to this comparison can never be equal. Make sure your syntax is correct."
V
Vidar Holen 已提交
1065
checkConstantIfs _ _ = return ()
1066

1067 1068 1069 1070 1071 1072 1073 1074
prop_checkLiteralBreakingTest = verify checkLiteralBreakingTest "[[ a==$foo ]]"
prop_checkLiteralBreakingTest2 = verify checkLiteralBreakingTest "[ $foo=3 ]"
prop_checkLiteralBreakingTest3 = verify checkLiteralBreakingTest "[ $foo!=3 ]"
prop_checkLiteralBreakingTest4 = verify checkLiteralBreakingTest "[ \"$(ls) \" ]"
prop_checkLiteralBreakingTest5 = verify checkLiteralBreakingTest "[ -n \"$(true) \" ]"
prop_checkLiteralBreakingTest6 = verify checkLiteralBreakingTest "[ -z $(true)z ]"
prop_checkLiteralBreakingTest7 = verifyNot checkLiteralBreakingTest "[ -z $(true) ]"
prop_checkLiteralBreakingTest8 = verifyNot checkLiteralBreakingTest "[ $(true)$(true) ]"
1075
prop_checkLiteralBreakingTest10 = verify checkLiteralBreakingTest "[ -z foo ]"
1076 1077
checkLiteralBreakingTest _ t = potentially $
        case t of
1078
            (TC_Nullary _ _ w@(T_NormalWord _ l)) -> do
1079
                guard . not $ isConstant w -- Covered by SC2078
1080
                comparisonWarning l `mplus` tautologyWarning w "Argument to implicit -n is always true due to literal strings."
1081
            (TC_Unary _ _ op w@(T_NormalWord _ l)) ->
1082 1083 1084 1085 1086
                case op of
                    "-n" -> tautologyWarning w "Argument to -n is always true due to literal strings."
                    "-z" -> tautologyWarning w "Argument to -z is always false due to literal strings."
                    _ -> fail "not relevant"
            _ -> fail "not my problem"
1087
  where
1088 1089 1090
    hasEquals = matchToken ('=' `elem`)
    isNonEmpty = matchToken (not . null)
    matchToken m t = isJust $ do
1091
        str <- getLiteralString t
1092
        guard $ m str
1093
        return ()
1094

1095 1096
    comparisonWarning list = do
        token <- listToMaybe $ filter hasEquals list
1097
        return $ err (getId token) 2077 "You need spaces around the comparison operator."
1098
    tautologyWarning t s = do
1099
        token <- listToMaybe $ filter isNonEmpty $ getWordParts t
1100
        return $ err (getId token) 2157 s
V
Vidar Holen 已提交
1101

1102 1103 1104 1105 1106 1107 1108 1109
prop_checkConstantNullary = verify checkConstantNullary "[[ '$(foo)' ]]"
prop_checkConstantNullary2 = verify checkConstantNullary "[ \"-f lol\" ]"
prop_checkConstantNullary3 = verify checkConstantNullary "[[ cmd ]]"
prop_checkConstantNullary4 = verify checkConstantNullary "[[ ! cmd ]]"
prop_checkConstantNullary5 = verify checkConstantNullary "[[ true ]]"
prop_checkConstantNullary6 = verify checkConstantNullary "[ 1 ]"
prop_checkConstantNullary7 = verify checkConstantNullary "[ false ]"
checkConstantNullary _ (TC_Nullary _ _ t) | isConstant t =
1110 1111 1112 1113 1114 1115 1116 1117 1118
    case fromMaybe "" $ getLiteralString t of
        "false" -> err (getId t) 2158 "[ false ] is true. Remove the brackets."
        "0" -> err (getId t) 2159 "[ 0 ] is true. Use 'false' instead."
        "true" -> style (getId t) 2160 "Instead of '[ true ]', just use 'true'."
        "1" -> style (getId t) 2161 "Instead of '[ 1 ]', use 'true'."
        _ -> err (getId t) 2078 "This expression is constant. Did you forget a $ somewhere?"
  where
    string = fromMaybe "" $ getLiteralString t

1119
checkConstantNullary _ _ = return ()
V
Vidar Holen 已提交
1120

1121 1122 1123
prop_checkForDecimals1 = verify checkForDecimals "((3.14*c))"
prop_checkForDecimals2 = verify checkForDecimals "foo[1.2]=bar"
prop_checkForDecimals3 = verifyNot checkForDecimals "declare -A foo; foo[1.2]=bar"
1124 1125
checkForDecimals params t@(TA_Expansion id _) = potentially $ do
    guard $ not (hasFloatingPoint params)
1126 1127 1128 1129
    str <- getLiteralString t
    first <- str !!! 0
    guard $ isDigit first && '.' `elem` str
    return $ err id 2079 "(( )) doesn't support decimals. Use bc or awk."
V
Vidar Holen 已提交
1130
checkForDecimals _ _ = return ()
1131 1132 1133

prop_checkDivBeforeMult = verify checkDivBeforeMult "echo $((c/n*100))"
prop_checkDivBeforeMult2 = verifyNot checkDivBeforeMult "echo $((c*100/n))"
1134 1135 1136 1137
prop_checkDivBeforeMult3 = verifyNot checkDivBeforeMult "echo $((c/10*10))"
checkDivBeforeMult params (TA_Binary _ "*" (TA_Binary id "/" _ x) y)
    | not (hasFloatingPoint params) && x /= y =
        info id 2017 "Increase precision by replacing a/b*c with a*c/b."
V
Vidar Holen 已提交
1138
checkDivBeforeMult _ _ = return ()
1139

V
Vidar Holen 已提交
1140 1141 1142
prop_checkArithmeticDeref = verify checkArithmeticDeref "echo $((3+$foo))"
prop_checkArithmeticDeref2 = verify checkArithmeticDeref "cow=14; (( s+= $cow ))"
prop_checkArithmeticDeref3 = verifyNot checkArithmeticDeref "cow=1/40; (( s+= ${cow%%/*} ))"
1143
prop_checkArithmeticDeref4 = verifyNot checkArithmeticDeref "(( ! $? ))"
V
Vidar Holen 已提交
1144
prop_checkArithmeticDeref5 = verifyNot checkArithmeticDeref "(($1))"
1145
prop_checkArithmeticDeref6 = verify checkArithmeticDeref "(( a[$i] ))"
1146
prop_checkArithmeticDeref7 = verifyNot checkArithmeticDeref "(( 10#$n ))"
1147
prop_checkArithmeticDeref8 = verifyNot checkArithmeticDeref "let i=$i+1"
1148 1149 1150 1151
prop_checkArithmeticDeref9 = verifyNot checkArithmeticDeref "(( a[foo] ))"
prop_checkArithmeticDeref10= verifyNot checkArithmeticDeref "(( a[\\$foo] ))"
prop_checkArithmeticDeref11= verifyNot checkArithmeticDeref "a[$foo]=wee"
prop_checkArithmeticDeref12= verify checkArithmeticDeref "for ((i=0; $i < 3; i)); do true; done"
V
Vidar Holen 已提交
1152
prop_checkArithmeticDeref13= verifyNot checkArithmeticDeref "(( $$ ))"
1153 1154
prop_checkArithmeticDeref14= verifyNot checkArithmeticDeref "(( $! ))"
prop_checkArithmeticDeref15= verifyNot checkArithmeticDeref "(( ${!var} ))"
1155 1156
checkArithmeticDeref params t@(TA_Expansion _ [b@(T_DollarBraced id _)]) =
    unless (isException $ bracedString b) getWarning
V
Vidar Holen 已提交
1157
  where
1158
    isException [] = True
1159
    isException s = any (`elem` "/.:#%?*@$-!") s || isDigit (head s)
1160 1161
    getWarning = fromMaybe noWarning . msum . map warningFor $ parents params t
    warningFor t =
1162
        case t of
1163 1164 1165 1166 1167 1168 1169 1170
            T_Arithmetic {} -> return normalWarning
            T_DollarArithmetic {} -> return normalWarning
            T_ForArithmetic {} -> return normalWarning
            T_SimpleCommand {} -> return noWarning
            _ -> Nothing

    normalWarning = style id 2004 "$/${} is unnecessary on arithmetic variables."
    noWarning = return ()
V
Vidar Holen 已提交
1171
checkArithmeticDeref _ _ = return ()
V
Vidar Holen 已提交
1172

1173 1174 1175
prop_checkArithmeticBadOctal1 = verify checkArithmeticBadOctal "(( 0192 ))"
prop_checkArithmeticBadOctal2 = verifyNot checkArithmeticBadOctal "(( 0x192 ))"
prop_checkArithmeticBadOctal3 = verifyNot checkArithmeticBadOctal "(( 1 ^ 0777 ))"
1176 1177 1178 1179 1180 1181
checkArithmeticBadOctal _ t@(TA_Expansion id _) = potentially $ do
    str <- getLiteralString t
    guard $ str `matches` octalRE
    return $ err id 2080 "Numbers with leading 0 are considered octal."
  where
    octalRE = mkRegex "^0[0-7]*[8-9]"
V
Vidar Holen 已提交
1182
checkArithmeticBadOctal _ _ = return ()
V
Vidar Holen 已提交
1183 1184 1185

prop_checkComparisonAgainstGlob = verify checkComparisonAgainstGlob "[[ $cow == $bar ]]"
prop_checkComparisonAgainstGlob2 = verifyNot checkComparisonAgainstGlob "[[ $cow == \"$bar\" ]]"
V
Vidar Holen 已提交
1186 1187
prop_checkComparisonAgainstGlob3 = verify checkComparisonAgainstGlob "[ $cow = *foo* ]"
prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow = foo ]"
1188 1189 1190 1191
prop_checkComparisonAgainstGlob5 = verify checkComparisonAgainstGlob "[[ $cow != $bar ]]"
checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _]))
    | op `elem` ["=", "==", "!="] =
        warn id 2053 $ "Quote the rhs of " ++ op ++ " in [[ ]] to prevent glob matching."
V
Vidar Holen 已提交
1192
checkComparisonAgainstGlob _ (TC_Binary _ SingleBracket op _ word)
V
Vidar Holen 已提交
1193
        | (op == "=" || op == "==") && isGlob word =
1194
    err (getId word) 2081 "[ .. ] can't match globs. Use [[ .. ]] or case statement."
V
Vidar Holen 已提交
1195
checkComparisonAgainstGlob _ _ = return ()
V
Vidar Holen 已提交
1196

V
Vidar Holen 已提交
1197 1198 1199
prop_checkCommarrays1 = verify checkCommarrays "a=(1, 2)"
prop_checkCommarrays2 = verify checkCommarrays "a+=(1,2,3)"
prop_checkCommarrays3 = verifyNot checkCommarrays "cow=(1 \"foo,bar\" 3)"
1200
prop_checkCommarrays4 = verifyNot checkCommarrays "cow=('one,' 'two')"
1201 1202
prop_checkCommarrays5 = verify checkCommarrays "a=([a]=b, [c]=d)"
prop_checkCommarrays6 = verify checkCommarrays "a=([a]=b,[c]=d,[e]=f)"
V
Vidar Holen 已提交
1203
checkCommarrays _ (T_Array id l) =
1204 1205 1206
    when (any (isCommaSeparated . literal) l) $
        warn id 2054 "Use spaces, not commas, to separate array elements."
  where
1207
    literal (T_IndexedElement _ _ l) = literal l
1208 1209 1210 1211
    literal (T_NormalWord _ l) = concatMap literal l
    literal (T_Literal _ str) = str
    literal _ = "str"

R
Rodrigo Setti 已提交
1212
    isCommaSeparated str = "," `isSuffixOf` str || length (filter (== ',') str) > 1
V
Vidar Holen 已提交
1213
checkCommarrays _ _ = return ()
V
Vidar Holen 已提交
1214

1215 1216 1217 1218
prop_checkOrNeq1 = verify checkOrNeq "if [[ $lol -ne cow || $lol -ne foo ]]; then echo foo; fi"
prop_checkOrNeq2 = verify checkOrNeq "(( a!=lol || a!=foo ))"
prop_checkOrNeq3 = verify checkOrNeq "[ \"$a\" != lol || \"$a\" != foo ]"
prop_checkOrNeq4 = verifyNot checkOrNeq "[ a != $cow || b != $foo ]"
1219
prop_checkOrNeq5 = verifyNot checkOrNeq "[[ $a != /home || $a != */public_html/* ]]"
1220
-- This only catches the most idiomatic cases. Fixme?
1221 1222
checkOrNeq _ (TC_Or id typ op (TC_Binary _ _ op1 lhs1 rhs1 ) (TC_Binary _ _ op2 lhs2 rhs2))
    | lhs1 == lhs2 && (op1 == op2 && (op1 == "-ne" || op1 == "!=")) && not (any isGlob [rhs1,rhs2]) =
V
Vidar Holen 已提交
1223
        warn id 2055 $ "You probably wanted " ++ (if typ == SingleBracket then "-a" else "&&") ++ " here."
1224

V
Vidar Holen 已提交
1225
checkOrNeq _ (TA_Binary id "||" (TA_Binary _ "!=" word1 _) (TA_Binary _ "!=" word2 _))
1226
    | word1 == word2 =
V
Vidar Holen 已提交
1227
        warn id 2056 "You probably wanted && here."
V
Vidar Holen 已提交
1228
checkOrNeq _ _ = return ()
V
Vidar Holen 已提交
1229

1230

1231 1232
prop_checkValidCondOps1 = verify checkValidCondOps "[[ a -xz b ]]"
prop_checkValidCondOps2 = verify checkValidCondOps "[ -M a ]"
1233
prop_checkValidCondOps2a= verifyNot checkValidCondOps "[ 3 \\> 2 ]"
1234 1235
prop_checkValidCondOps3 = verifyNot checkValidCondOps "[ 1 = 2 -a 3 -ge 4 ]"
prop_checkValidCondOps4 = verifyNot checkValidCondOps "[[ ! -v foo ]]"
V
Vidar Holen 已提交
1236
checkValidCondOps _ (TC_Binary id _ s _ _)
1237
    | s `notElem` binaryTestOps =
V
Vidar Holen 已提交
1238
        warn id 2057 "Unknown binary operator."
V
Vidar Holen 已提交
1239
checkValidCondOps _ (TC_Unary id _ s _)
1240
    | s `notElem`  unaryTestOps =
V
Vidar Holen 已提交
1241
        warn id 2058 "Unknown unary operator."
V
Vidar Holen 已提交
1242
checkValidCondOps _ _ = return ()
1243

1244 1245 1246 1247
prop_checkUuoeVar1 = verify checkUuoeVar "for f in $(echo $tmp); do echo lol; done"
prop_checkUuoeVar2 = verify checkUuoeVar "date +`echo \"$format\"`"
prop_checkUuoeVar3 = verifyNot checkUuoeVar "foo \"$(echo -e '\r')\""
prop_checkUuoeVar4 = verifyNot checkUuoeVar "echo $tmp"
V
Vidar Holen 已提交
1248 1249
prop_checkUuoeVar5 = verify checkUuoeVar "foo \"$(echo \"$(date) value:\" $value)\""
prop_checkUuoeVar6 = verifyNot checkUuoeVar "foo \"$(echo files: *.png)\""
1250
prop_checkUuoeVar7 = verifyNot checkUuoeVar "foo $(echo $(bar))" -- covered by 2005
1251
prop_checkUuoeVar8 = verifyNot checkUuoeVar "#!/bin/sh\nz=$(echo)"
1252
prop_checkUuoeVar9 = verify checkUuoeVar "foo $(echo $(<file))"
1253 1254 1255 1256 1257 1258
checkUuoeVar _ p =
    case p of
        T_Backticked id [cmd] -> check id cmd
        T_DollarExpansion id [cmd] -> check id cmd
        _ -> return ()
  where
V
Vidar Holen 已提交
1259 1260 1261 1262 1263 1264 1265
    couldBeOptimized f = case f of
        T_Glob {} -> False
        T_Extglob {} -> False
        T_BraceExpansion {} -> False
        T_NormalWord _ l -> all couldBeOptimized l
        T_DoubleQuoted _ l -> all couldBeOptimized l
        _ -> True
1266

V
Vidar Holen 已提交
1267
    check id (T_Pipeline _ _ [T_Redirecting _ _ c]) = warnForEcho id c
1268
    check _ _ = return ()
1269
    isCovered first rest = null rest && tokenIsJustCommandOutput first
1270 1271 1272 1273 1274 1275
    warnForEcho id = checkUnqualifiedCommand "echo" $ \_ vars ->
        case vars of
          (first:rest) ->
            unless (isCovered first rest || "-" `isPrefixOf` onlyLiteralString first) $
                when (all couldBeOptimized vars) $ style id 2116
                    "Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'."
V
Vidar Holen 已提交
1276
          _ -> return ()
1277

1278

V
Vidar Holen 已提交
1279 1280 1281
prop_checkTestRedirects1 = verify checkTestRedirects "test 3 > 1"
prop_checkTestRedirects2 = verifyNot checkTestRedirects "test 3 \\> 1"
prop_checkTestRedirects3 = verify checkTestRedirects "/usr/bin/test $var > $foo"
1282 1283 1284 1285 1286 1287
prop_checkTestRedirects4 = verifyNot checkTestRedirects "test 1 -eq 2 2> file"
checkTestRedirects _ (T_Redirecting id redirs cmd) | cmd `isCommand` "test" =
    mapM_ check redirs
  where
    check t =
        when (suspicious t) $
V
Vidar Holen 已提交
1288
            warn (getId t) 2065 "This is interpreted as a shell file redirection, not a comparison."
1289 1290 1291
    suspicious t = -- Ignore redirections of stderr because these are valid for squashing e.g. int errors,
        case t of  -- and >> and similar redirections because these are probably not comparisons.
            T_FdRedirect _ fd (T_IoFile _ op _) -> fd /= "2" && isComparison op
V
Vidar Holen 已提交
1292
            _ -> False
1293 1294 1295 1296
    isComparison t =
        case t of
            T_Greater _ -> True
            T_Less _ -> True
V
Vidar Holen 已提交
1297
            _ -> False
V
Vidar Holen 已提交
1298
checkTestRedirects _ _ = return ()
1299

1300 1301 1302 1303 1304 1305 1306 1307
prop_checkPS11 = verify checkPS1Assignments "PS1='\\033[1;35m\\$ '"
prop_checkPS11a= verify checkPS1Assignments "export PS1='\\033[1;35m\\$ '"
prop_checkPSf2 = verify checkPS1Assignments "PS1='\\h \\e[0m\\$ '"
prop_checkPS13 = verify checkPS1Assignments "PS1=$'\\x1b[c '"
prop_checkPS14 = verify checkPS1Assignments "PS1=$'\\e[3m; '"
prop_checkPS14a= verify checkPS1Assignments "export PS1=$'\\e[3m; '"
prop_checkPS15 = verifyNot checkPS1Assignments "PS1='\\[\\033[1;35m\\]\\$ '"
prop_checkPS16 = verifyNot checkPS1Assignments "PS1='\\[\\e1m\\e[1m\\]\\$ '"
1308 1309
prop_checkPS17 = verifyNot checkPS1Assignments "PS1='e033x1B'"
prop_checkPS18 = verifyNot checkPS1Assignments "PS1='\\[\\e\\]'"
V
Vidar Holen 已提交
1310
checkPS1Assignments _ (T_Assignment _ _ "PS1" _ word) = warnFor word
1311
  where
V
Vidar Holen 已提交
1312
    warnFor word =
1313
        let contents = concat $ oversimplify word in
V
Vidar Holen 已提交
1314
            when (containsUnescaped contents) $
V
Vidar Holen 已提交
1315
                info (getId word) 2025 "Make sure all escape sequences are enclosed in \\[..\\] to prevent line wrapping issues"
1316 1317 1318 1319
    containsUnescaped s =
        let unenclosed = subRegex enclosedRegex s "" in
           isJust $ matchRegex escapeRegex unenclosed
    enclosedRegex = mkRegex "\\\\\\[.*\\\\\\]" -- FIXME: shouldn't be eager
1320
    escapeRegex = mkRegex "\\\\x1[Bb]|\\\\e|\x1B|\\\\033"
V
Vidar Holen 已提交
1321
checkPS1Assignments _ _ = return ()
1322

1323 1324
prop_checkBackticks1 = verify checkBackticks "echo `foo`"
prop_checkBackticks2 = verifyNot checkBackticks "echo $(foo)"
1325 1326
prop_checkBackticks3 = verifyNot checkBackticks "echo `#inlined comment` foo"
checkBackticks _ (T_Backticked id list) | not (null list) =
1327
    style id 2006 "Use $(...) notation instead of legacy backticked `...`."
V
Vidar Holen 已提交
1328
checkBackticks _ _ = return ()
1329

V
Vidar Holen 已提交
1330 1331
prop_checkIndirectExpansion1 = verify checkIndirectExpansion "${foo$n}"
prop_checkIndirectExpansion2 = verifyNot checkIndirectExpansion "${foo//$n/lol}"
V
Vidar Holen 已提交
1332 1333 1334
prop_checkIndirectExpansion3 = verify checkIndirectExpansion "${$#}"
prop_checkIndirectExpansion4 = verify checkIndirectExpansion "${var${n}_$((i%2))}"
prop_checkIndirectExpansion5 = verifyNot checkIndirectExpansion "${bar}"
V
Vidar Holen 已提交
1335
checkIndirectExpansion _ (T_DollarBraced i (T_NormalWord _ contents)) =
V
Vidar Holen 已提交
1336
    when (isIndirection contents) $
1337
        err i 2082 "To expand via indirection, use arrays, ${!name} or (for sh only) eval."
V
Vidar Holen 已提交
1338
  where
V
Vidar Holen 已提交
1339
    isIndirection vars =
R
Rodrigo Setti 已提交
1340 1341
        let list = mapMaybe isIndirectionPart vars in
            not (null list) && and list
V
Vidar Holen 已提交
1342 1343 1344 1345 1346 1347 1348 1349 1350 1351
    isIndirectionPart t =
        case t of T_DollarExpansion _ _ ->  Just True
                  T_Backticked _ _ ->       Just True
                  T_DollarBraced _ _ ->     Just True
                  T_DollarArithmetic _ _ -> Just True
                  T_Literal _ s -> if all isVariableChar s
                                    then Nothing
                                    else Just False
                  _ -> Just False

V
Vidar Holen 已提交
1352
checkIndirectExpansion _ _ = return ()
V
Vidar Holen 已提交
1353

1354 1355 1356
prop_checkInexplicablyUnquoted1 = verify checkInexplicablyUnquoted "echo 'var='value';'"
prop_checkInexplicablyUnquoted2 = verifyNot checkInexplicablyUnquoted "'foo'*"
prop_checkInexplicablyUnquoted3 = verifyNot checkInexplicablyUnquoted "wget --user-agent='something'"
1357 1358
prop_checkInexplicablyUnquoted4 = verify checkInexplicablyUnquoted "echo \"VALUES (\"id\")\""
prop_checkInexplicablyUnquoted5 = verifyNot checkInexplicablyUnquoted "\"$dir\"/\"$file\""
1359
prop_checkInexplicablyUnquoted6 = verifyNot checkInexplicablyUnquoted "\"$dir\"some_stuff\"$file\""
1360
prop_checkInexplicablyUnquoted7 = verifyNot checkInexplicablyUnquoted "${dir/\"foo\"/\"bar\"}"
1361
prop_checkInexplicablyUnquoted8 = verifyNot checkInexplicablyUnquoted "  'foo'\\\n  'bar'"
V
Vidar Holen 已提交
1362
checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens)
1363
  where
R
Rodrigo Setti 已提交
1364
    check (T_SingleQuoted _ _:T_Literal id str:_)
1365
        | not (null str) && all isAlphaNum str =
R
Rodrigo Setti 已提交
1366
        info id 2026 "This word is outside of quotes. Did you intend to 'nest '\"'single quotes'\"' instead'? "
1367

1368
    check (T_DoubleQuoted _ a:trapped:T_DoubleQuoted _ b:_) =
1369
        case trapped of
1370 1371
            T_DollarExpansion id _ -> warnAboutExpansion id
            T_DollarBraced id _ -> warnAboutExpansion id
1372 1373 1374
            T_Literal id s ->
                unless (quotesSingleThing a && quotesSingleThing b) $
                    warnAboutLiteral id
1375 1376
            _ -> return ()

1377
    check _ = return ()
1378 1379 1380 1381 1382 1383 1384 1385 1386

    -- If the surrounding quotes quote single things, like "$foo"_and_then_some_"$stuff",
    -- the quotes were probably intentional and harmless.
    quotesSingleThing x = case x of
        [T_DollarExpansion _ _] -> True
        [T_DollarBraced _ _] -> True
        [T_Backticked _ _] -> True
        _ -> False

1387
    warnAboutExpansion id =
R
Rodrigo Setti 已提交
1388
        warn id 2027 "The surrounding quotes actually unquote this. Remove or escape them."
1389
    warnAboutLiteral id =
1390
        warn id 2140 "Word is of the form \"A\"B\"C\" (B indicated). Did you mean \"ABC\" or \"A\\\"B\\\"C\"?"
V
Vidar Holen 已提交
1391
checkInexplicablyUnquoted _ _ = return ()
1392

V
Vidar Holen 已提交
1393 1394 1395 1396
prop_checkTildeInQuotes1 = verify checkTildeInQuotes "var=\"~/out.txt\""
prop_checkTildeInQuotes2 = verify checkTildeInQuotes "foo > '~/dir'"
prop_checkTildeInQuotes4 = verifyNot checkTildeInQuotes "~/file"
prop_checkTildeInQuotes5 = verifyNot checkTildeInQuotes "echo '/~foo/cow'"
1397
prop_checkTildeInQuotes6 = verifyNot checkTildeInQuotes "awk '$0 ~ /foo/'"
V
Vidar Holen 已提交
1398
checkTildeInQuotes _ = check
V
Vidar Holen 已提交
1399
  where
1400
    verify id ('~':'/':_) = warn id 2088 "Tilde does not expand in quotes. Use $HOME."
1401
    verify _ _ = return ()
R
Rodrigo Setti 已提交
1402
    check (T_NormalWord _ (T_SingleQuoted id str:_)) =
V
Vidar Holen 已提交
1403
        verify id str
R
Rodrigo Setti 已提交
1404
    check (T_NormalWord _ (T_DoubleQuoted _ (T_Literal id str:_):_)) =
V
Vidar Holen 已提交
1405 1406 1407
        verify id str
    check _ = return ()

1408 1409
prop_checkLonelyDotDash1 = verify checkLonelyDotDash "./ file"
prop_checkLonelyDotDash2 = verifyNot checkLonelyDotDash "./file"
V
Vidar Holen 已提交
1410
checkLonelyDotDash _ t@(T_Redirecting id _ _)
1411
    | isUnqualifiedCommand t "./" =
V
Vidar Holen 已提交
1412
        err id 2083 "Don't add spaces after the slash in './file'."
V
Vidar Holen 已提交
1413
checkLonelyDotDash _ _ = return ()
1414

V
Vidar Holen 已提交
1415 1416 1417 1418 1419 1420 1421

prop_checkSpuriousExec1 = verify checkSpuriousExec "exec foo; true"
prop_checkSpuriousExec2 = verify checkSpuriousExec "if a; then exec b; exec c; fi"
prop_checkSpuriousExec3 = verifyNot checkSpuriousExec "echo cow; exec foo"
prop_checkSpuriousExec4 = verifyNot checkSpuriousExec "if a; then exec b; fi"
prop_checkSpuriousExec5 = verifyNot checkSpuriousExec "exec > file; cmd"
prop_checkSpuriousExec6 = verify checkSpuriousExec "exec foo > file; cmd"
V
Vidar Holen 已提交
1422
prop_checkSpuriousExec7 = verifyNot checkSpuriousExec "exec file; echo failed; exit 3"
1423
prop_checkSpuriousExec8 = verifyNot checkSpuriousExec "exec {origout}>&1- >tmp.log 2>&1; bar"
V
Vidar Holen 已提交
1424
checkSpuriousExec _ = doLists
V
Vidar Holen 已提交
1425 1426 1427 1428 1429
  where
    doLists (T_Script _ _ cmds) = doList cmds
    doLists (T_BraceGroup _ cmds) = doList cmds
    doLists (T_WhileExpression _ _ cmds) = doList cmds
    doLists (T_UntilExpression _ _ cmds) = doList cmds
V
Vidar Holen 已提交
1430
    doLists (T_ForIn _ _ _ cmds) = doList cmds
V
Vidar Holen 已提交
1431
    doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds
V
Vidar Holen 已提交
1432 1433 1434 1435 1436
    doLists (T_IfExpression _ thens elses) = do
        mapM_ (\(_, l) -> doList l) thens
        doList elses
    doLists _ = return ()

V
Vidar Holen 已提交
1437
    stripCleanup = reverse . dropWhile cleanup . reverse
V
Vidar Holen 已提交
1438
    cleanup (T_Pipeline _ _ [cmd]) =
V
Vidar Holen 已提交
1439 1440 1441 1442 1443
        isCommandMatch cmd (`elem` ["echo", "exit"])
    cleanup _ = False

    doList = doList' . stripCleanup
    doList' t@(current:following:_) = do
V
Vidar Holen 已提交
1444 1445
        commentIfExec current
        doList (tail t)
V
Vidar Holen 已提交
1446
    doList' _ = return ()
V
Vidar Holen 已提交
1447

V
Vidar Holen 已提交
1448
    commentIfExec (T_Pipeline id _ list) =
V
Vidar Holen 已提交
1449
      mapM_ commentIfExec $ take 1 list
V
Vidar Holen 已提交
1450 1451 1452
    commentIfExec (T_Redirecting _ _ f@(
      T_SimpleCommand id _ (cmd:arg:_))) =
        when (f `isUnqualifiedCommand` "exec") $
R
Rodrigo Setti 已提交
1453
          warn id 2093
V
Vidar Holen 已提交
1454 1455 1456 1457
            "Remove \"exec \" if script should continue after this command."
    commentIfExec _ = return ()


1458 1459 1460 1461
prop_checkSpuriousExpansion1 = verify checkSpuriousExpansion "if $(true); then true; fi"
prop_checkSpuriousExpansion2 = verify checkSpuriousExpansion "while \"$(cmd)\"; do :; done"
prop_checkSpuriousExpansion3 = verifyNot checkSpuriousExpansion "$(cmd) --flag1 --flag2"
prop_checkSpuriousExpansion4 = verify checkSpuriousExpansion "$((i++))"
V
Vidar Holen 已提交
1462
checkSpuriousExpansion _ (T_SimpleCommand _ _ [T_NormalWord _ [word]]) = check word
1463 1464 1465
  where
    check word = case word of
        T_DollarExpansion id _ ->
V
Vidar Holen 已提交
1466
            warn id 2091 "Remove surrounding $() to avoid executing output."
1467
        T_Backticked id _ ->
V
Vidar Holen 已提交
1468
            warn id 2092 "Remove backticks to avoid executing output."
1469
        T_DollarArithmetic id _ ->
V
Vidar Holen 已提交
1470
            err id 2084 "Remove '$' or use '_=$((expr))' to avoid executing output."
1471 1472
        T_DoubleQuoted id [subword] -> check subword
        _ -> return ()
V
Vidar Holen 已提交
1473
checkSpuriousExpansion _ _ = return ()
1474 1475


1476 1477
prop_checkDollarBrackets1 = verify checkDollarBrackets "echo $[1+2]"
prop_checkDollarBrackets2 = verifyNot checkDollarBrackets "echo $((1+2))"
V
Vidar Holen 已提交
1478
checkDollarBrackets _ (T_DollarBracket id _) =
V
Vidar Holen 已提交
1479
    style id 2007 "Use $((..)) instead of deprecated $[..]"
V
Vidar Holen 已提交
1480
checkDollarBrackets _ _ = return ()
1481

1482 1483
prop_checkSshHereDoc1 = verify checkSshHereDoc "ssh host << foo\necho $PATH\nfoo"
prop_checkSshHereDoc2 = verifyNot checkSshHereDoc "ssh host << 'foo'\necho $PATH\nfoo"
V
Vidar Holen 已提交
1484
checkSshHereDoc _ (T_Redirecting _ redirs cmd)
1485 1486 1487 1488
        | cmd `isCommand` "ssh" =
    mapM_ checkHereDoc redirs
  where
    hasVariables = mkRegex "[`$]"
1489 1490
    checkHereDoc (T_FdRedirect _ _ (T_HereDoc id _ Unquoted token tokens))
        | not (all isConstant tokens) =
V
Vidar Holen 已提交
1491
        warn id 2087 $ "Quote '" ++ token ++ "' to make here document expansions happen on the server side rather than on the client."
1492
    checkHereDoc _ = return ()
V
Vidar Holen 已提交
1493
checkSshHereDoc _ _ = return ()
1494

1495
--- Subshell detection
V
Vidar Holen 已提交
1496 1497 1498 1499 1500 1501 1502 1503 1504 1505 1506 1507 1508
prop_subshellAssignmentCheck = verifyTree     subshellAssignmentCheck "cat foo | while read bar; do a=$bar; done; echo \"$a\""
prop_subshellAssignmentCheck2 = verifyNotTree subshellAssignmentCheck "while read bar; do a=$bar; done < file; echo \"$a\""
prop_subshellAssignmentCheck3 = verifyTree    subshellAssignmentCheck "( A=foo; ); rm $A"
prop_subshellAssignmentCheck4 = verifyNotTree subshellAssignmentCheck "( A=foo; rm $A; )"
prop_subshellAssignmentCheck5 = verifyTree    subshellAssignmentCheck "cat foo | while read cow; do true; done; echo $cow;"
prop_subshellAssignmentCheck6 = verifyTree    subshellAssignmentCheck "( export lol=$(ls); ); echo $lol;"
prop_subshellAssignmentCheck6a= verifyTree    subshellAssignmentCheck "( typeset -a lol=a; ); echo $lol;"
prop_subshellAssignmentCheck7 = verifyTree    subshellAssignmentCheck "cmd | while read foo; do (( n++ )); done; echo \"$n lines\""
prop_subshellAssignmentCheck8 = verifyTree    subshellAssignmentCheck "n=3 & echo $((n++))"
prop_subshellAssignmentCheck9 = verifyTree    subshellAssignmentCheck "read n & n=foo$n"
prop_subshellAssignmentCheck10 = verifyTree    subshellAssignmentCheck "(( n <<= 3 )) & (( n |= 4 )) &"
prop_subshellAssignmentCheck11 = verifyTree subshellAssignmentCheck "cat /etc/passwd | while read line; do let n=n+1; done\necho $n"
prop_subshellAssignmentCheck12 = verifyTree subshellAssignmentCheck "cat /etc/passwd | while read line; do let ++n; done\necho $n"
1509 1510
prop_subshellAssignmentCheck13 = verifyTree subshellAssignmentCheck "#!/bin/bash\necho foo | read bar; echo $bar"
prop_subshellAssignmentCheck14 = verifyNotTree subshellAssignmentCheck "#!/bin/ksh93\necho foo | read bar; echo $bar"
V
Vidar Holen 已提交
1511
prop_subshellAssignmentCheck15 = verifyNotTree subshellAssignmentCheck "#!/bin/ksh\ncat foo | while read bar; do a=$bar; done\necho \"$a\""
1512
prop_subshellAssignmentCheck16 = verifyNotTree subshellAssignmentCheck "(set -e); echo $@"
V
Vidar Holen 已提交
1513
prop_subshellAssignmentCheck17 = verifyNotTree subshellAssignmentCheck "foo=${ { bar=$(baz); } 2>&1; }; echo $foo $bar"
1514
prop_subshellAssignmentCheck18 = verifyTree subshellAssignmentCheck "( exec {n}>&2; ); echo $n"
1515
prop_subshellAssignmentCheck19 = verifyNotTree subshellAssignmentCheck "#!/bin/bash\nshopt -s lastpipe; echo a | read -r b; echo \"$b\""
V
Vidar Holen 已提交
1516 1517
subshellAssignmentCheck params t =
    let flow = variableFlow params
1518
        check = findSubshelled flow [("oops",[])] Map.empty
1519
    in execWriter check
V
Vidar Holen 已提交
1520 1521 1522


findSubshelled [] _ _ = return ()
R
Rodrigo Setti 已提交
1523
findSubshelled (Assignment x@(_, _, str, _):rest) ((reason,scope):lol) deadVars =
1524
    findSubshelled rest ((reason, x:scope):lol) $ Map.insert str Alive deadVars
R
Rodrigo Setti 已提交
1525
findSubshelled (Reference (_, readToken, str):rest) scopes deadVars = do
1526
    unless (shouldIgnore str) $ case Map.findWithDefault Alive str deadVars of
V
Vidar Holen 已提交
1527
        Alive -> return ()
1528
        Dead writeToken reason -> do
V
Vidar Holen 已提交
1529 1530
                    info (getId writeToken) 2030 $ "Modification of " ++ str ++ " is local (to subshell caused by "++ reason ++")."
                    info (getId readToken) 2031 $ str ++ " was modified in a subshell. That change might be lost."
V
Vidar Holen 已提交
1531
    findSubshelled rest scopes deadVars
1532 1533 1534
  where
    shouldIgnore str =
        str `elem` ["@", "*", "IFS"]
V
Vidar Holen 已提交
1535

R
Rodrigo Setti 已提交
1536
findSubshelled (StackScope (SubshellScope reason):rest) scopes deadVars =
1537
    findSubshelled rest ((reason,[]):scopes) deadVars
V
Vidar Holen 已提交
1538

R
Rodrigo Setti 已提交
1539
findSubshelled (StackScopeEnd:rest) ((reason, scope):oldScopes) deadVars =
1540 1541 1542
    findSubshelled rest oldScopes $
        foldl (\m (_, token, var, _) ->
            Map.insert var (Dead token reason) m) deadVars scope
V
Vidar Holen 已提交
1543

1544 1545 1546 1547 1548 1549 1550 1551 1552 1553 1554

-- FIXME: This is a very strange way of doing it.
-- For each variable read/write, run a stateful function that emits
-- comments. The comments are collected and returned.
doVariableFlowAnalysis ::
    (Token -> Token -> String -> State t [v])
    -> (Token -> Token -> String -> DataType -> State t [v])
    -> t
    -> [StackData]
    -> [v]

R
Rodrigo Setti 已提交
1555
doVariableFlowAnalysis readFunc writeFunc empty flow = evalState (
1556 1557 1558 1559 1560 1561 1562 1563
    foldM (\list x -> do { l <- doFlow x;  return $ l ++ list; }) [] flow
    ) empty
  where
    doFlow (Reference (base, token, name)) =
        readFunc base token name
    doFlow (Assignment (base, token, name, values)) =
        writeFunc base token name values
    doFlow _ = return []
V
Vidar Holen 已提交
1564

1565
---- Check whether variables could have spaces/globs
V
Vidar Holen 已提交
1566 1567 1568 1569 1570 1571 1572 1573 1574
prop_checkSpacefulness1 = verifyTree checkSpacefulness "a='cow moo'; echo $a"
prop_checkSpacefulness2 = verifyNotTree checkSpacefulness "a='cow moo'; [[ $a ]]"
prop_checkSpacefulness3 = verifyNotTree checkSpacefulness "a='cow*.mp3'; echo \"$a\""
prop_checkSpacefulness4 = verifyTree checkSpacefulness "for f in *.mp3; do echo $f; done"
prop_checkSpacefulness4a= verifyNotTree checkSpacefulness "foo=3; foo=$(echo $foo)"
prop_checkSpacefulness5 = verifyTree checkSpacefulness "a='*'; b=$a; c=lol${b//foo/bar}; echo $c"
prop_checkSpacefulness6 = verifyTree checkSpacefulness "a=foo$(lol); echo $a"
prop_checkSpacefulness7 = verifyTree checkSpacefulness "a=foo\\ bar; rm $a"
prop_checkSpacefulness8 = verifyNotTree checkSpacefulness "a=foo\\ bar; a=foo; rm $a"
1575 1576 1577 1578 1579 1580 1581 1582 1583 1584 1585 1586 1587 1588
prop_checkSpacefulness10= verifyTree checkSpacefulness "rm $1"
prop_checkSpacefulness11= verifyTree checkSpacefulness "rm ${10//foo/bar}"
prop_checkSpacefulness12= verifyNotTree checkSpacefulness "(( $1 + 3 ))"
prop_checkSpacefulness13= verifyNotTree checkSpacefulness "if [[ $2 -gt 14 ]]; then true; fi"
prop_checkSpacefulness14= verifyNotTree checkSpacefulness "foo=$3 env"
prop_checkSpacefulness15= verifyNotTree checkSpacefulness "local foo=$1"
prop_checkSpacefulness16= verifyNotTree checkSpacefulness "declare foo=$1"
prop_checkSpacefulness17= verifyTree checkSpacefulness "echo foo=$1"
prop_checkSpacefulness18= verifyNotTree checkSpacefulness "$1 --flags"
prop_checkSpacefulness19= verifyTree checkSpacefulness "echo $PWD"
prop_checkSpacefulness20= verifyNotTree checkSpacefulness "n+='foo bar'"
prop_checkSpacefulness21= verifyNotTree checkSpacefulness "select foo in $bar; do true; done"
prop_checkSpacefulness22= verifyNotTree checkSpacefulness "echo $\"$1\""
prop_checkSpacefulness23= verifyNotTree checkSpacefulness "a=(1); echo ${a[@]}"
1589
prop_checkSpacefulness24= verifyTree checkSpacefulness "a='a    b'; cat <<< $a"
1590
prop_checkSpacefulness25= verifyTree checkSpacefulness "a='s/[0-9]//g'; sed $a"
1591
prop_checkSpacefulness26= verifyTree checkSpacefulness "a='foo bar'; echo {1,2,$a}"
1592
prop_checkSpacefulness27= verifyNotTree checkSpacefulness "echo ${a:+'foo'}"
1593 1594 1595
prop_checkSpacefulness28= verifyNotTree checkSpacefulness "exec {n}>&1; echo $n"
prop_checkSpacefulness29= verifyNotTree checkSpacefulness "n=$(stuff); exec {n}>&-;"
prop_checkSpacefulness30= verifyTree checkSpacefulness "file='foo bar'; echo foo > $file;"
1596
prop_checkSpacefulness31= verifyNotTree checkSpacefulness "echo \"`echo \\\"$1\\\"`\""
1597
prop_checkSpacefulness32= verifyNotTree checkSpacefulness "var=$1; [ -v var ]"
1598
prop_checkSpacefulness33= verifyTree checkSpacefulness "for file; do echo $file; done"
1599
prop_checkSpacefulness34= verifyTree checkSpacefulness "declare foo$n=$1"
V
Vidar Holen 已提交
1600
prop_checkSpacefulness35= verifyNotTree checkSpacefulness "echo ${1+\"$1\"}"
V
Vidar Holen 已提交
1601 1602 1603

checkSpacefulness params t =
    doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params)
V
Vidar Holen 已提交
1604
  where
1605
    defaults = zip variablesWithoutSpaces (repeat False)
V
Vidar Holen 已提交
1606

1607 1608
    hasSpaces name = do
        map <- get
1609
        return $ Map.findWithDefault True name map
V
Vidar Holen 已提交
1610

R
Rodrigo Setti 已提交
1611
    setSpaces name bool =
1612
        modify $ Map.insert name bool
V
Vidar Holen 已提交
1613

1614
    readF _ token name = do
1615 1616 1617
        spaces <- hasSpaces name
        return [warning |
                  isExpansion token && spaces
1618
                  && not (isArrayExpansion token) -- There's another warning for this
1619
                  && not (isCountingReference token)
R
Rodrigo Setti 已提交
1620
                  && not (isQuoteFree parents token)
1621
                  && not (isQuotedAlternativeReference token)
R
Rodrigo Setti 已提交
1622
                  && not (usedAsCommandName parents token)]
1623
      where
1624 1625 1626 1627 1628 1629 1630 1631
        warning =
            if isDefaultAssignment (parentMap params) token
            then
                makeComment InfoC (getId token) 2223
                    "This default assignment may cause DoS due to globbing. Quote it."
            else
                makeComment InfoC (getId token) 2086
                    "Double quote to prevent globbing and word splitting."
1632

1633 1634
    writeF _ _ name (DataString SourceExternal) = setSpaces name True >> return []
    writeF _ _ name (DataString SourceInteger) = setSpaces name False >> return []
1635

1636
    writeF _ _ name (DataString (SourceFrom vals)) = do
1637 1638
        map <- get
        setSpaces name
1639
            (isSpacefulWord (\x -> Map.findWithDefault True x map) vals)
1640 1641
        return []

1642 1643
    writeF _ _ _ _ = return []

V
Vidar Holen 已提交
1644
    parents = parentMap params
1645

1646 1647 1648 1649 1650
    isExpansion t =
        case t of
            (T_DollarBraced _ _ ) -> True
            _ -> False

1651
    isSpacefulWord :: (String -> Bool) -> [Token] -> Bool
R
Rodrigo Setti 已提交
1652
    isSpacefulWord f = any (isSpaceful f)
1653 1654 1655
    isSpaceful :: (String -> Bool) -> Token -> Bool
    isSpaceful spacefulF x =
        case x of
1656 1657
          T_DollarExpansion _ _ -> True
          T_Backticked _ _ -> True
1658
          T_Glob _ _         -> True
R
Rodrigo Setti 已提交
1659
          T_Extglob {}       -> True
1660 1661
          T_Literal _ s      -> s `containsAny` globspace
          T_SingleQuoted _ s -> s `containsAny` globspace
1662
          T_DollarBraced _ _ -> spacefulF $ getBracedReference $ bracedString x
1663 1664 1665 1666
          T_NormalWord _ w   -> isSpacefulWord spacefulF w
          T_DoubleQuoted _ w -> isSpacefulWord spacefulF w
          _ -> False
      where
1667
        globspace = "*?[] \t\n"
R
Rodrigo Setti 已提交
1668
        containsAny s = any (`elem` s)
V
Vidar Holen 已提交
1669

1670 1671 1672 1673 1674 1675
    isDefaultAssignment parents token =
        let modifier = getBracedModifier $ bracedString token in
            isExpansion token
            && any (`isPrefixOf` modifier) ["=", ":="]
            && isParamTo parents ":" token

V
Vidar Holen 已提交
1676 1677 1678 1679 1680 1681 1682 1683 1684
prop_checkQuotesInLiterals1 = verifyTree checkQuotesInLiterals "param='--foo=\"bar\"'; app $param"
prop_checkQuotesInLiterals1a= verifyTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; app $param"
prop_checkQuotesInLiterals2 = verifyNotTree checkQuotesInLiterals "param='--foo=\"bar\"'; app \"$param\""
prop_checkQuotesInLiterals3 =verifyNotTree checkQuotesInLiterals "param=('--foo='); app \"${param[@]}\""
prop_checkQuotesInLiterals4 = verifyNotTree checkQuotesInLiterals "param=\"don't bother with this one\"; app $param"
prop_checkQuotesInLiterals5 = verifyNotTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; eval app $param"
prop_checkQuotesInLiterals6 = verifyTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm $param\"; $cmd"
prop_checkQuotesInLiterals6a= verifyNotTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm ${#param}\"; $cmd"
prop_checkQuotesInLiterals7 = verifyTree checkQuotesInLiterals "param='my\\ file'; rm $param"
1685
prop_checkQuotesInLiterals8 = verifyTree checkQuotesInLiterals "param=\"/foo/'bar baz'/etc\"; rm $param"
1686
prop_checkQuotesInLiterals9 = verifyNotTree checkQuotesInLiterals "param=\"/foo/'bar baz'/etc\"; rm ${#param}"
V
Vidar Holen 已提交
1687 1688
checkQuotesInLiterals params t =
    doVariableFlowAnalysis readF writeF Map.empty (variableFlow params)
1689
  where
1690
    getQuotes name = fmap (Map.lookup name) get
1691 1692
    setQuotes name ref = modify $ Map.insert name ref
    deleteQuotes = modify . Map.delete
V
Vidar Holen 已提交
1693
    parents = parentMap params
1694
    quoteRegex = mkRegex "\"|([/= ]|^)'|'( |$)|\\\\ "
1695
    containsQuotes s = s `matches` quoteRegex
1696

1697
    writeF _ _ name (DataString (SourceFrom values)) = do
1698 1699
        quoteMap <- get
        let quotedVars = msum $ map (forToken quoteMap) values
1700
        case quotedVars of
1701 1702
            Nothing -> deleteQuotes name
            Just x -> setQuotes name x
1703 1704 1705
        return []
    writeF _ _ _ _ = return []

1706 1707
    forToken map (T_DollarBraced id t) =
        -- skip getBracedReference here to avoid false positives on PE
1708
        Map.lookup (concat . oversimplify $ t) map
1709 1710 1711 1712 1713
    forToken quoteMap (T_DoubleQuoted id tokens) =
        msum $ map (forToken quoteMap) tokens
    forToken quoteMap (T_NormalWord id tokens) =
        msum $ map (forToken quoteMap) tokens
    forToken _ t =
1714
        if containsQuotes (concat $ oversimplify t)
1715 1716 1717
        then return $ getId t
        else Nothing

1718 1719 1720
    squashesQuotes t =
        case t of
            T_DollarBraced id _ -> "#" `isPrefixOf` bracedString t
V
Vidar Holen 已提交
1721
            _ -> False
1722

1723 1724
    readF _ expr name = do
        assignment <- getQuotes name
R
Rodrigo Setti 已提交
1725 1726 1727 1728
        return
          (if isJust assignment
              && not (isParamTo parents "eval" expr)
              && not (isQuoteFree parents expr)
1729
              && not (squashesQuotes expr)
R
Rodrigo Setti 已提交
1730
              then [
1731
                  makeComment WarningC (fromJust assignment) 2089
R
Rodrigo Setti 已提交
1732
                      "Quotes/backslashes will be treated literally. Use an array.",
1733
                  makeComment WarningC (getId expr) 2090
R
Rodrigo Setti 已提交
1734 1735 1736
                      "Quotes/backslashes in this variable will not be respected."
                ]
              else [])
1737 1738 1739


prop_checkFunctionsUsedExternally1 =
V
Vidar Holen 已提交
1740
  verifyTree checkFunctionsUsedExternally "foo() { :; }; sudo foo"
1741
prop_checkFunctionsUsedExternally2 =
V
Vidar Holen 已提交
1742
  verifyTree checkFunctionsUsedExternally "alias f='a'; xargs -n 1 f"
1743
prop_checkFunctionsUsedExternally3 =
V
Vidar Holen 已提交
1744
  verifyNotTree checkFunctionsUsedExternally "f() { :; }; echo f"
1745 1746
prop_checkFunctionsUsedExternally4 =
  verifyNotTree checkFunctionsUsedExternally "foo() { :; }; sudo \"foo\""
V
Vidar Holen 已提交
1747 1748
checkFunctionsUsedExternally params t =
    runNodeAnalysis checkCommand params t
1749 1750 1751 1752 1753 1754 1755 1756 1757 1758
  where
    invokingCmds = [
        "chroot",
        "find",
        "screen",
        "ssh",
        "su",
        "sudo",
        "xargs"
        ]
V
Vidar Holen 已提交
1759
    checkCommand _ t@(T_SimpleCommand _ _ (cmd:args)) =
V
Vidar Holen 已提交
1760
        let name = fromMaybe "" $ getCommandBasename t in
1761 1762
          when (name `elem` invokingCmds) $
            mapM_ (checkArg name) args
V
Vidar Holen 已提交
1763
    checkCommand _ _ = return ()
1764

R
Rodrigo Setti 已提交
1765
    analyse f t = execState (doAnalysis f t) []
V
Vidar Holen 已提交
1766
    functions = Map.fromList $ analyse findFunctions t
1767
    findFunctions (T_Function id _ _ name _) = modify ((name, id):)
1768 1769 1770 1771
    findFunctions t@(T_SimpleCommand id _ (_:args))
        | t `isUnqualifiedCommand` "alias" = mapM_ getAlias args
    findFunctions _ = return ()
    getAlias arg =
1772
        let string = concat $ oversimplify arg
1773 1774
        in when ('=' `elem` string) $
            modify ((takeWhile (/= '=') string, getId arg):)
1775 1776 1777 1778 1779 1780 1781 1782
    checkArg cmd arg = potentially $ do
        literalArg <- getUnquotedLiteral arg  -- only consider unquoted literals
        definitionId <- Map.lookup literalArg functions
        return $ do
            warn (getId arg) 2033
              "Shell functions can't be passed to external commands."
            info definitionId 2032 $
              "Use own script or sh -c '..' to run this from " ++ cmd ++ "."
1783

V
Vidar Holen 已提交
1784 1785 1786 1787 1788 1789 1790 1791 1792 1793 1794 1795 1796
prop_checkUnused0 = verifyNotTree checkUnusedAssignments "var=foo; echo $var"
prop_checkUnused1 = verifyTree checkUnusedAssignments "var=foo; echo $bar"
prop_checkUnused2 = verifyNotTree checkUnusedAssignments "var=foo; export var;"
prop_checkUnused3 = verifyTree checkUnusedAssignments "for f in *; do echo '$f'; done"
prop_checkUnused4 = verifyTree checkUnusedAssignments "local i=0"
prop_checkUnused5 = verifyNotTree checkUnusedAssignments "read lol; echo $lol"
prop_checkUnused6 = verifyNotTree checkUnusedAssignments "var=4; (( var++ ))"
prop_checkUnused7 = verifyNotTree checkUnusedAssignments "var=2; $((var))"
prop_checkUnused8 = verifyTree checkUnusedAssignments "var=2; var=3;"
prop_checkUnused9 = verifyNotTree checkUnusedAssignments "read ''"
prop_checkUnused10= verifyNotTree checkUnusedAssignments "read -p 'test: '"
prop_checkUnused11= verifyNotTree checkUnusedAssignments "bar=5; export foo[$bar]=3"
prop_checkUnused12= verifyNotTree checkUnusedAssignments "read foo; echo ${!foo}"
1797
prop_checkUnused13= verifyNotTree checkUnusedAssignments "x=(1); (( x[0] ))"
1798 1799
prop_checkUnused14= verifyNotTree checkUnusedAssignments "x=(1); n=0; echo ${x[n]}"
prop_checkUnused15= verifyNotTree checkUnusedAssignments "x=(1); n=0; (( x[n] ))"
1800
prop_checkUnused16= verifyNotTree checkUnusedAssignments "foo=5; declare -x foo"
1801
prop_checkUnused17= verifyNotTree checkUnusedAssignments "read -i 'foo' -e -p 'Input: ' bar; $bar;"
1802
prop_checkUnused18= verifyNotTree checkUnusedAssignments "a=1; arr=( [$a]=42 ); echo \"${arr[@]}\""
1803
prop_checkUnused19= verifyNotTree checkUnusedAssignments "a=1; let b=a+1; echo $b"
1804 1805
prop_checkUnused20= verifyNotTree checkUnusedAssignments "a=1; PS1='$a'"
prop_checkUnused21= verifyNotTree checkUnusedAssignments "a=1; trap 'echo $a' INT"
1806 1807
prop_checkUnused22= verifyNotTree checkUnusedAssignments "a=1; [ -v a ]"
prop_checkUnused23= verifyNotTree checkUnusedAssignments "a=1; [ -R a ]"
1808 1809
prop_checkUnused24= verifyNotTree checkUnusedAssignments "mapfile -C a b; echo ${b[@]}"
prop_checkUnused25= verifyNotTree checkUnusedAssignments "readarray foo; echo ${foo[@]}"
1810
prop_checkUnused26= verifyNotTree checkUnusedAssignments "declare -F foo"
1811 1812
prop_checkUnused27= verifyTree checkUnusedAssignments "var=3; [ var -eq 3 ]"
prop_checkUnused28= verifyNotTree checkUnusedAssignments "var=3; [[ var -eq 3 ]]"
1813
prop_checkUnused29= verifyNotTree checkUnusedAssignments "var=(a b); declare -p var"
1814 1815 1816
prop_checkUnused30= verifyTree checkUnusedAssignments "let a=1"
prop_checkUnused31= verifyTree checkUnusedAssignments "let 'a=1'"
prop_checkUnused32= verifyTree checkUnusedAssignments "let a=b=c; echo $a"
V
Vidar Holen 已提交
1817
prop_checkUnused33= verifyNotTree checkUnusedAssignments "a=foo; [[ foo =~ ^{$a}$ ]]"
1818
prop_checkUnused34= verifyNotTree checkUnusedAssignments "foo=1; (( t = foo )); echo $t"
V
Vidar Holen 已提交
1819
prop_checkUnused35= verifyNotTree checkUnusedAssignments "a=foo; b=2; echo ${a:b}"
1820
prop_checkUnused36= verifyNotTree checkUnusedAssignments "if [[ -v foo ]]; then true; fi"
1821
prop_checkUnused37= verifyNotTree checkUnusedAssignments "fd=2; exec {fd}>&-"
1822
prop_checkUnused38= verifyTree checkUnusedAssignments "(( a=42 ))"
1823
prop_checkUnused39= verifyNotTree checkUnusedAssignments "declare -x -f foo"
1824
prop_checkUnused40= verifyNotTree checkUnusedAssignments "arr=(1 2); num=2; echo \"${arr[@]:num}\""
1825
checkUnusedAssignments params t = execWriter (mapM_ warnFor unused)
1826
  where
V
Vidar Holen 已提交
1827
    flow = variableFlow params
1828 1829
    references = foldl (flip ($)) defaultMap (map insertRef flow)
    insertRef (Reference (base, token, name)) =
1830
        Map.insert (stripSuffix name) ()
1831 1832
    insertRef _ = id

1833 1834 1835 1836 1837 1838 1839 1840
    assignments = foldl (flip ($)) Map.empty (map insertAssignment flow)
    insertAssignment (Assignment (_, token, name, _)) | isVariableName name =
        Map.insert name token
    insertAssignment _ = id

    unused = Map.assocs $ Map.difference assignments references

    warnFor (name, token) =
1841
        warn (getId token) 2034 $
1842
            name ++ " appears unused. Verify use (or export if used externally)."
1843

R
Rodrigo Setti 已提交
1844
    stripSuffix = takeWhile isVariableChar
1845 1846
    defaultMap = Map.fromList $ zip internalVariables $ repeat ()

1847 1848 1849 1850 1851 1852 1853 1854 1855 1856
prop_checkUnassignedReferences1 = verifyTree checkUnassignedReferences "echo $foo"
prop_checkUnassignedReferences2 = verifyNotTree checkUnassignedReferences "foo=hello; echo $foo"
prop_checkUnassignedReferences3 = verifyTree checkUnassignedReferences "MY_VALUE=3; echo $MYVALUE"
prop_checkUnassignedReferences4 = verifyNotTree checkUnassignedReferences "RANDOM2=foo; echo $RANDOM"
prop_checkUnassignedReferences5 = verifyNotTree checkUnassignedReferences "declare -A foo=([bar]=baz); echo ${foo[bar]}"
prop_checkUnassignedReferences6 = verifyNotTree checkUnassignedReferences "foo=..; echo ${foo-bar}"
prop_checkUnassignedReferences7 = verifyNotTree checkUnassignedReferences "getopts ':h' foo; echo $foo"
prop_checkUnassignedReferences8 = verifyNotTree checkUnassignedReferences "let 'foo = 1'; echo $foo"
prop_checkUnassignedReferences9 = verifyNotTree checkUnassignedReferences "echo ${foo-bar}"
prop_checkUnassignedReferences10= verifyNotTree checkUnassignedReferences "echo ${foo:?}"
1857 1858 1859 1860 1861 1862 1863 1864 1865 1866
prop_checkUnassignedReferences11= verifyNotTree checkUnassignedReferences "declare -A foo; echo \"${foo[@]}\""
prop_checkUnassignedReferences12= verifyNotTree checkUnassignedReferences "typeset -a foo; echo \"${foo[@]}\""
prop_checkUnassignedReferences13= verifyNotTree checkUnassignedReferences "f() { local foo; echo $foo; }"
prop_checkUnassignedReferences14= verifyNotTree checkUnassignedReferences "foo=; echo $foo"
prop_checkUnassignedReferences15= verifyNotTree checkUnassignedReferences "f() { true; }; export -f f"
prop_checkUnassignedReferences16= verifyNotTree checkUnassignedReferences "declare -A foo=( [a b]=bar ); echo ${foo[a b]}"
prop_checkUnassignedReferences17= verifyNotTree checkUnassignedReferences "USERS=foo; echo $USER"
prop_checkUnassignedReferences18= verifyNotTree checkUnassignedReferences "FOOBAR=42; export FOOBAR="
prop_checkUnassignedReferences19= verifyNotTree checkUnassignedReferences "readonly foo=bar; echo $foo"
prop_checkUnassignedReferences20= verifyNotTree checkUnassignedReferences "printf -v foo bar; echo $foo"
1867
prop_checkUnassignedReferences21= verifyTree checkUnassignedReferences "echo ${#foo}"
1868
prop_checkUnassignedReferences22= verifyNotTree checkUnassignedReferences "echo ${!os*}"
1869 1870
prop_checkUnassignedReferences23= verifyTree checkUnassignedReferences "declare -a foo; foo[bar]=42;"
prop_checkUnassignedReferences24= verifyNotTree checkUnassignedReferences "declare -A foo; foo[bar]=42;"
1871
prop_checkUnassignedReferences25= verifyNotTree checkUnassignedReferences "declare -A foo=(); foo[bar]=42;"
1872
prop_checkUnassignedReferences26= verifyNotTree checkUnassignedReferences "a::b() { foo; }; readonly -f a::b"
1873
prop_checkUnassignedReferences27= verifyNotTree checkUnassignedReferences ": ${foo:=bar}"
1874
prop_checkUnassignedReferences28= verifyNotTree checkUnassignedReferences "#!/bin/ksh\necho \"${.sh.version}\"\n"
1875 1876 1877 1878
prop_checkUnassignedReferences29= verifyNotTree checkUnassignedReferences "if [[ -v foo ]]; then echo $foo; fi"
prop_checkUnassignedReferences30= verifyNotTree checkUnassignedReferences "if [[ -v foo[3] ]]; then echo ${foo[3]}; fi"
prop_checkUnassignedReferences31= verifyNotTree checkUnassignedReferences "X=1; if [[ -v foo[$X+42] ]]; then echo ${foo[$X+42]}; fi"
prop_checkUnassignedReferences32= verifyNotTree checkUnassignedReferences "if [[ -v \"foo[1]\" ]]; then echo ${foo[@]}; fi"
1879
prop_checkUnassignedReferences33= verifyNotTree checkUnassignedReferences "f() { local -A foo; echo \"${foo[@]}\"; }"
1880
prop_checkUnassignedReferences34= verifyNotTree checkUnassignedReferences "declare -A foo; (( foo[bar] ))"
1881
prop_checkUnassignedReferences35= verifyNotTree checkUnassignedReferences "echo ${arr[foo-bar]:?fail}"
1882 1883 1884 1885 1886 1887 1888 1889
checkUnassignedReferences params t = warnings
  where
    (readMap, writeMap) = execState (mapM tally $ variableFlow params) (Map.empty, Map.empty)
    defaultAssigned = Map.fromList $ map (\a -> (a, ())) $ filter (not . null) internalVariables

    tally (Assignment (_, _, name, _))  =
        modify (\(read, written) -> (read, Map.insert name () written))
    tally (Reference (_, place, name)) =
1890
        modify (\(read, written) -> (Map.insertWith (const id) name place read, written))
1891 1892 1893 1894 1895 1896 1897 1898 1899 1900
    tally _ = return ()

    unassigned = Map.toList $ Map.difference (Map.difference readMap writeMap) defaultAssigned
    writtenVars = filter isVariableName $ Map.keys writeMap

    getBestMatch var = do
        (match, score) <- listToMaybe best
        guard $ goodMatch var match score
        return match
      where
1901
        matches = map (\x -> (x, match var x)) writtenVars
1902 1903 1904 1905 1906 1907 1908 1909 1910 1911 1912 1913 1914 1915 1916
        best = sortBy (comparing snd) matches
        goodMatch var match score =
            let l = length match in
                l > 3 && score <= 1
                || l > 7 && score <= 2

    isLocal = any isLower

    warningForGlobals var place = do
        match <- getBestMatch var
        return $ warn (getId place) 2153 $
            "Possible misspelling: " ++ var ++ " may not be assigned, but " ++ match ++ " is."

    warningForLocals var place =
        return $ warn (getId place) 2154 $
1917
            var ++ " is referenced but not assigned" ++ optionalTip ++ "."
1918
      where
1919 1920 1921 1922 1923 1924
        optionalTip =
            if var `elem` commonCommands
            then " (for output from commands, use \"$(" ++ var ++ " ..." ++ ")\" )"
            else fromMaybe "" $ do
                    match <- getBestMatch var
                    return $ " (did you mean '" ++ match ++ "'?)"
1925 1926

    warningFor var place = do
1927
        guard . not $ isInArray var place || isGuarded place
1928 1929 1930 1931 1932
        (if isLocal var then warningForLocals else warningForGlobals) var place

    warnings = execWriter . sequence $ mapMaybe (uncurry warningFor) unassigned

    -- Due to parsing, foo=( [bar]=baz ) parses 'bar' as a reference even for assoc arrays.
1933 1934
    -- Similarly, ${foo[bar baz]} may not be referencing bar/baz. Just skip these.
    isInArray var t = any isArray $ getPath (parentMap params) t
1935
      where
1936
        isArray T_Array {} = True
1937
        isArray b@(T_DollarBraced _ _) | var /= getBracedReference (bracedString b) = True
1938 1939 1940
        isArray _ = False

    isGuarded (T_DollarBraced _ v) =
1941
        rest `matches` guardRegex
1942
      where
1943
        name = concat $ oversimplify v
C
cleanup  
Chad Brewbaker 已提交
1944
        rest = dropWhile isVariableChar $ dropWhile (`elem` "#!") name
1945
    isGuarded _ = False
1946 1947
    --  :? or :- with optional array index and colon
    guardRegex = mkRegex "^(\\[.*\\])?:?[-?]"
1948

1949
    match var candidate =
1950
        if var /= candidate && map toLower var == map toLower candidate
1951 1952 1953
        then 1
        else dist var candidate

1954

1955 1956 1957
prop_checkGlobsAsOptions1 = verify checkGlobsAsOptions "rm *.txt"
prop_checkGlobsAsOptions2 = verify checkGlobsAsOptions "ls ??.*"
prop_checkGlobsAsOptions3 = verifyNot checkGlobsAsOptions "rm -- *.txt"
1958
prop_checkGlobsAsOptions4 = verifyNot checkGlobsAsOptions "*.txt"
V
Vidar Holen 已提交
1959
checkGlobsAsOptions _ (T_SimpleCommand _ _ args) =
1960
    mapM_ check $ takeWhile (not . isEndOfArgs) (drop 1 args)
1961
  where
R
Rodrigo Setti 已提交
1962
    check v@(T_NormalWord _ (T_Glob id s:_)) | s == "*" || s == "?" =
1963
        info id 2035 "Use ./*glob* or -- *glob* so names with dashes won't become options."
1964 1965 1966
    check _ = return ()

    isEndOfArgs t =
1967
        case concat $ oversimplify t of
1968 1969 1970 1971 1972
            "--" -> True
            ":::" -> True
            "::::" -> True
            _ -> False

V
Vidar Holen 已提交
1973
checkGlobsAsOptions _ _ = return ()
1974 1975 1976 1977 1978 1979 1980 1981


prop_checkWhileReadPitfalls1 = verify checkWhileReadPitfalls "while read foo; do ssh $foo uptime; done < file"
prop_checkWhileReadPitfalls2 = verifyNot checkWhileReadPitfalls "while read -u 3 foo; do ssh $foo uptime; done 3< file"
prop_checkWhileReadPitfalls3 = verifyNot checkWhileReadPitfalls "while true; do ssh host uptime; done"
prop_checkWhileReadPitfalls4 = verifyNot checkWhileReadPitfalls "while read foo; do ssh $foo hostname < /dev/null; done"
prop_checkWhileReadPitfalls5 = verifyNot checkWhileReadPitfalls "while read foo; do echo ls | ssh $foo; done"
prop_checkWhileReadPitfalls6 = verifyNot checkWhileReadPitfalls "while read foo <&3; do ssh $foo; done 3< foo"
1982
prop_checkWhileReadPitfalls7 = verify checkWhileReadPitfalls "while read foo; do if true; then ssh $foo uptime; fi; done < file"
1983
prop_checkWhileReadPitfalls8 = verifyNot checkWhileReadPitfalls "while read foo; do ssh -n $foo uptime; done < file"
1984

V
Vidar Holen 已提交
1985
checkWhileReadPitfalls _ (T_WhileExpression id [command] contents)
R
Rodrigo Setti 已提交
1986
        | isStdinReadCommand command =
1987 1988
    mapM_ checkMuncher contents
  where
1989
    munchers = [ "ssh", "ffmpeg", "mplayer", "HandBrakeCLI" ]
1990
    preventionFlags = ["n", "noconsolecontrols" ]
1991

V
Vidar Holen 已提交
1992
    isStdinReadCommand (T_Pipeline _ _ [T_Redirecting id redirs cmd]) =
1993
        let plaintext = oversimplify cmd
1994
        in head (plaintext ++ [""]) == "read"
V
Vidar Holen 已提交
1995
            && ("-u" `notElem` plaintext)
1996 1997 1998
            && all (not . stdinRedirect) redirs
    isStdinReadCommand _ = False

1999
    checkMuncher (T_Pipeline _ _ (T_Redirecting _ redirs cmd:_)) | not $ any stdinRedirect redirs =
2000 2001
        case cmd of
            (T_IfExpression _ thens elses) ->
R
Rodrigo Setti 已提交
2002
              mapM_ checkMuncher . concat $ map fst thens ++ map snd thens ++ [elses]
2003 2004 2005 2006

            _ -> potentially $ do
                name <- getCommandBasename cmd
                guard $ name `elem` munchers
2007

2008 2009 2010
                -- Sloppily check if the command has a flag to prevent eating stdin.
                let flags = getAllFlags cmd
                guard . not $ any (`elem` preventionFlags) $ map snd flags
2011 2012 2013 2014 2015
                return $ do
                    info id 2095 $
                        name ++ " may swallow stdin, preventing this loop from working properly."
                    warn (getId cmd) 2095 $
                        "Add < /dev/null to prevent " ++ name ++ " from swallowing stdin."
2016 2017 2018 2019 2020
    checkMuncher _ = return ()

    stdinRedirect (T_FdRedirect _ fd _)
        | fd == "" || fd == "0" = True
    stdinRedirect _ = False
V
Vidar Holen 已提交
2021
checkWhileReadPitfalls _ _ = return ()
V
Vidar Holen 已提交
2022 2023


V
Vidar Holen 已提交
2024 2025 2026
prop_checkPrefixAssign1 = verify checkPrefixAssignmentReference "var=foo echo $var"
prop_checkPrefixAssign2 = verifyNot checkPrefixAssignmentReference "var=$(echo $var) cmd"
checkPrefixAssignmentReference params t@(T_DollarBraced id value) =
V
Vidar Holen 已提交
2027 2028
    check path
  where
2029
    name = getBracedReference $ bracedString t
V
Vidar Holen 已提交
2030
    path = getPath (parentMap params) t
V
Vidar Holen 已提交
2031 2032 2033 2034 2035 2036
    idPath = map getId path

    check [] = return ()
    check (t:rest) =
        case t of
            T_SimpleCommand _ vars (_:_) -> mapM_ checkVar vars
V
Vidar Holen 已提交
2037
            _ -> check rest
2038
    checkVar (T_Assignment aId mode aName [] value) |
V
Vidar Holen 已提交
2039
            aName == name && (aId `notElem` idPath) = do
V
Vidar Holen 已提交
2040 2041 2042 2043 2044 2045
        warn aId 2097 "This assignment is only seen by the forked process."
        warn id 2098 "This expansion will not see the mentioned assignment."
    checkVar _ = return ()

checkPrefixAssignmentReference _ _ = return ()

2046 2047 2048 2049
prop_checkCharRangeGlob1 = verify checkCharRangeGlob "ls *[:digit:].jpg"
prop_checkCharRangeGlob2 = verifyNot checkCharRangeGlob "ls *[[:digit:]].jpg"
prop_checkCharRangeGlob3 = verify checkCharRangeGlob "ls [10-15]"
prop_checkCharRangeGlob4 = verifyNot checkCharRangeGlob "ls [a-zA-Z]"
2050 2051 2052
prop_checkCharRangeGlob5 = verifyNot checkCharRangeGlob "tr -d [a-zA-Z]" -- tr has 2060
checkCharRangeGlob p t@(T_Glob id str) |
  isCharClass str && not (isParamTo (parentMap p) "tr" t) =
2053 2054 2055 2056 2057
    if ":" `isPrefixOf` contents
        && ":" `isSuffixOf` contents
        && contents /= ":"
    then warn id 2101 "Named class needs outer [], e.g. [[:digit:]]."
    else
R
Rodrigo Setti 已提交
2058 2059
        when ('[' `notElem` contents && hasDupes) $
            info id 2102 "Ranges can only match single chars (mentioned due to duplicates)."
2060 2061
  where
    isCharClass str = "[" `isPrefixOf` str && "]" `isSuffixOf` str
R
Rodrigo Setti 已提交
2062
    contents = drop 1 . take (length str - 1) $ str
2063
    hasDupes = any (>1) . map length . group . sort . filter (/= '-') $ contents
V
Vidar Holen 已提交
2064
checkCharRangeGlob _ _ = return ()
2065 2066 2067



V
Vidar Holen 已提交
2068 2069 2070
prop_checkCdAndBack1 = verify checkCdAndBack "for f in *; do cd $f; git pull; cd ..; done"
prop_checkCdAndBack2 = verifyNot checkCdAndBack "for f in *; do cd $f || continue; git pull; cd ..; done"
prop_checkCdAndBack3 = verifyNot checkCdAndBack "while [[ $PWD != / ]]; do cd ..; done"
2071
prop_checkCdAndBack4 = verify checkCdAndBack "cd $tmp; foo; cd -"
V
Vidar Holen 已提交
2072
checkCdAndBack params = doLists
2073
  where
V
Vidar Holen 已提交
2074
    shell = shellType params
V
Vidar Holen 已提交
2075
    doLists (T_ForIn _ _ _ cmds) = doList cmds
V
Vidar Holen 已提交
2076
    doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds
2077 2078
    doLists (T_WhileExpression _ _ cmds) = doList cmds
    doLists (T_UntilExpression _ _ cmds) = doList cmds
2079
    doLists (T_Script _ _ cmds) = doList cmds
2080 2081 2082 2083 2084 2085
    doLists (T_IfExpression _ thens elses) = do
        mapM_ (\(_, l) -> doList l) thens
        doList elses
    doLists _ = return ()

    isCdRevert t =
2086
        case oversimplify t of
2087 2088 2089 2090
            ["cd", p] -> p `elem` ["..", "-"]
            _ -> False

    getCmd (T_Annotation id _ x) = getCmd x
V
Vidar Holen 已提交
2091
    getCmd (T_Pipeline id _ [x]) = getCommandName x
2092 2093 2094 2095 2096
    getCmd _ = Nothing

    doList list =
        let cds = filter ((== Just "cd") . getCmd) list in
            when (length cds >= 2 && isCdRevert (last cds)) $
2097
               info (getId $ last cds) 2103 message
2098

2099
    message = "Use a ( subshell ) to avoid having to cd back."
2100

V
Vidar Holen 已提交
2101 2102 2103 2104 2105
prop_checkLoopKeywordScope1 = verify checkLoopKeywordScope "continue 2"
prop_checkLoopKeywordScope2 = verify checkLoopKeywordScope "for f; do ( break; ); done"
prop_checkLoopKeywordScope3 = verify checkLoopKeywordScope "if true; then continue; fi"
prop_checkLoopKeywordScope4 = verifyNot checkLoopKeywordScope "while true; do break; done"
prop_checkLoopKeywordScope5 = verify checkLoopKeywordScope "if true; then break; fi"
2106 2107
prop_checkLoopKeywordScope6 = verify checkLoopKeywordScope "while true; do true | { break; }; done"
prop_checkLoopKeywordScope7 = verifyNot checkLoopKeywordScope "#!/bin/ksh\nwhile true; do true | { break; }; done"
V
Vidar Holen 已提交
2108
checkLoopKeywordScope params t |
2109 2110 2111 2112
        name `elem` map Just ["continue", "break"] =
    if not $ any isLoop path
    then if any isFunction $ take 1 path
        -- breaking at a source/function invocation is an abomination. Let's ignore it.
R
Rodrigo Setti 已提交
2113
        then err (getId t) 2104 $ "In functions, use return instead of " ++ fromJust name ++ "."
R
Rodrigo Setti 已提交
2114
        else err (getId t) 2105 $ fromJust name ++ " is only valid in loops."
2115
    else case map subshellType $ filter (not . isFunction) path of
R
Rodrigo Setti 已提交
2116
        Just str:_ -> warn (getId t) 2106 $
2117 2118 2119 2120
            "This only exits the subshell caused by the " ++ str ++ "."
        _ -> return ()
  where
    name = getCommandName t
V
Vidar Holen 已提交
2121
    path = let p = getPath (parentMap params) t in filter relevant p
2122
    subshellType t = case leadType params t of
2123 2124 2125 2126
        NoneScope -> Nothing
        SubshellScope str -> return str
    relevant t = isLoop t || isFunction t || isJust (subshellType t)
checkLoopKeywordScope _ _ = return ()
2127 2128 2129 2130 2131 2132 2133


prop_checkFunctionDeclarations1 = verify checkFunctionDeclarations "#!/bin/ksh\nfunction foo() { command foo --lol \"$@\"; }"
prop_checkFunctionDeclarations2 = verify checkFunctionDeclarations "#!/bin/dash\nfunction foo { lol; }"
prop_checkFunctionDeclarations3 = verifyNot checkFunctionDeclarations "foo() { echo bar; }"
checkFunctionDeclarations params
        (T_Function id (FunctionKeyword hasKeyword) (FunctionParentheses hasParens) _ _) =
R
Rodrigo Setti 已提交
2134
    case shellType params of
2135
        Bash -> return ()
R
Rodrigo Setti 已提交
2136
        Ksh ->
2137 2138
            when (hasKeyword && hasParens) $
                err id 2111 "ksh does not allow 'function' keyword and '()' at the same time."
2139 2140 2141 2142 2143
        Dash -> forSh
        Sh   -> forSh

    where
        forSh = do
2144 2145 2146 2147 2148
            when (hasKeyword && hasParens) $
                warn id 2112 "'function' keyword is non-standard. Delete it."
            when (hasKeyword && not hasParens) $
                warn id 2113 "'function' keyword is non-standard. Use 'foo()' instead of 'function foo'."
checkFunctionDeclarations _ _ = return ()
V
Vidar Holen 已提交
2149 2150


2151

V
Vidar Holen 已提交
2152
prop_checkStderrPipe1 = verify checkStderrPipe "#!/bin/ksh\nfoo |& bar"
V
Vidar Holen 已提交
2153
prop_checkStderrPipe2 = verifyNot checkStderrPipe "#!/bin/bash\nfoo |& bar"
V
Vidar Holen 已提交
2154 2155 2156 2157 2158 2159 2160 2161
checkStderrPipe params =
    case shellType params of
        Ksh -> match
        _ -> const $ return ()
  where
    match (T_Pipe id "|&") =
        err id 2118 "Ksh does not support |&. Use 2>&1 |."
    match _ = return ()
2162 2163 2164 2165 2166 2167

prop_checkUnpassedInFunctions1 = verifyTree checkUnpassedInFunctions "foo() { echo $1; }; foo"
prop_checkUnpassedInFunctions2 = verifyNotTree checkUnpassedInFunctions "foo() { echo $1; };"
prop_checkUnpassedInFunctions3 = verifyNotTree checkUnpassedInFunctions "foo() { echo $lol; }; foo"
prop_checkUnpassedInFunctions4 = verifyNotTree checkUnpassedInFunctions "foo() { echo $0; }; foo"
prop_checkUnpassedInFunctions5 = verifyNotTree checkUnpassedInFunctions "foo() { echo $1; }; foo 'lol'; foo"
2168 2169 2170
prop_checkUnpassedInFunctions6 = verifyNotTree checkUnpassedInFunctions "foo() { set -- *; echo $1; }; foo"
prop_checkUnpassedInFunctions7 = verifyTree checkUnpassedInFunctions "foo() { echo $1; }; foo; foo;"
prop_checkUnpassedInFunctions8 = verifyNotTree checkUnpassedInFunctions "foo() { echo $((1)); }; foo;"
2171
prop_checkUnpassedInFunctions9 = verifyNotTree checkUnpassedInFunctions "foo() { echo $(($b)); }; foo;"
2172
prop_checkUnpassedInFunctions10= verifyNotTree checkUnpassedInFunctions "foo() { echo $!; }; foo;"
2173
prop_checkUnpassedInFunctions11= verifyNotTree checkUnpassedInFunctions "foo() { bar() { echo $1; }; bar baz; }; foo;"
2174
prop_checkUnpassedInFunctions12= verifyNotTree checkUnpassedInFunctions "foo() { echo ${!var*}; }; foo;"
2175 2176 2177 2178 2179 2180 2181
checkUnpassedInFunctions params root =
    execWriter $ mapM_ warnForGroup referenceGroups
  where
    functionMap :: Map.Map String Token
    functionMap = Map.fromList $
        map (\t@(T_Function _ _ _ name _) -> (name,t)) functions
    functions = execWriter $ doAnalysis (tell . maybeToList . findFunction) root
2182 2183

    findFunction t@(T_Function id _ _ name body) =
2184
        let flow = getVariableFlow params body
2185
        in
2186
          if any (isPositionalReference t) flow && not (any isPositionalAssignment flow)
2187 2188
            then return t
            else Nothing
2189 2190
    findFunction _ = Nothing

2191 2192 2193 2194
    isPositionalAssignment x =
        case x of
            Assignment (_, _, str, _) -> isPositional str
            _ -> False
2195
    isPositionalReference function x =
2196
        case x of
V
Vidar Holen 已提交
2197
            Reference (_, t, str) -> isPositional str && t `isDirectChildOf` function
2198
            _ -> False
2199

2200
    isDirectChildOf child parent = fromMaybe False $ do
2201 2202 2203 2204 2205
        function <- find (\x -> case x of
            T_Function {} -> True
            T_Script {} -> True  -- for sourced files
            _ -> False) $
                getPath (parentMap params) child
2206 2207
        return $ getId parent == getId function

2208 2209 2210 2211 2212 2213
    referenceList :: [(String, Bool, Token)]
    referenceList = execWriter $
        doAnalysis (fromMaybe (return ()) . checkCommand) root
    checkCommand :: Token -> Maybe (Writer [(String, Bool, Token)] ())
    checkCommand t@(T_SimpleCommand _ _ (cmd:args)) = do
        str <- getLiteralString cmd
2214
        guard $ Map.member str functionMap
2215 2216 2217 2218
        return $ tell [(str, null args, t)]
    checkCommand _ = Nothing

    isPositional str = str == "*" || str == "@"
2219
        || (all isDigit str && str /= "0" && str /= "")
2220 2221 2222 2223 2224 2225 2226 2227 2228 2229 2230 2231 2232 2233 2234 2235

    isArgumentless (_, b, _) = b
    referenceGroups = Map.elems $ foldr updateWith Map.empty referenceList
    updateWith x@(name, _, _) = Map.insertWith (++) name [x]

    warnForGroup group =
        when (all isArgumentless group) $ do
            mapM_ suggestParams group
            warnForDeclaration group

    suggestParams (name, _, thing) =
        info (getId thing) 2119 $
            "Use " ++ name ++ " \"$@\" if function's $1 should mean script's $1."
    warnForDeclaration ((name, _, _):_) =
        warn (getId . fromJust $ Map.lookup name functionMap) 2120 $
            name ++ " references arguments, but none are ever passed."
2236 2237


2238 2239 2240 2241 2242 2243 2244 2245 2246 2247 2248
prop_checkOverridingPath1 = verify checkOverridingPath "PATH=\"$var/$foo\""
prop_checkOverridingPath2 = verify checkOverridingPath "PATH=\"mydir\""
prop_checkOverridingPath3 = verify checkOverridingPath "PATH=/cow/foo"
prop_checkOverridingPath4 = verifyNot checkOverridingPath "PATH=/cow/foo/bin"
prop_checkOverridingPath5 = verifyNot checkOverridingPath "PATH='/bin:/sbin'"
prop_checkOverridingPath6 = verifyNot checkOverridingPath "PATH=\"$var/$foo\" cmd"
prop_checkOverridingPath7 = verifyNot checkOverridingPath "PATH=$OLDPATH"
prop_checkOverridingPath8 = verifyNot checkOverridingPath "PATH=$PATH:/stuff"
checkOverridingPath _ (T_SimpleCommand _ vars []) =
    mapM_ checkVar vars
  where
2249
    checkVar (T_Assignment id Assign "PATH" [] word) =
2250
        let string = concat $ oversimplify word
2251 2252 2253 2254 2255 2256
        in unless (any (`isInfixOf` string) ["/bin", "/sbin" ]) $ do
            when ('/' `elem` string && ':' `notElem` string) $ notify id
            when (isLiteral word && ':' `notElem` string && '/' `notElem` string) $ notify id
    checkVar _ = return ()
    notify id = warn id 2123 "PATH is the shell search path. Use another name."
checkOverridingPath _ _ = return ()
2257

V
Vidar Holen 已提交
2258 2259 2260 2261 2262 2263
prop_checkTildeInPath1 = verify checkTildeInPath "PATH=\"$PATH:~/bin\""
prop_checkTildeInPath2 = verify checkTildeInPath "PATH='~foo/bin'"
prop_checkTildeInPath3 = verifyNot checkTildeInPath "PATH=~/bin"
checkTildeInPath _ (T_SimpleCommand _ vars _) =
    mapM_ checkVar vars
  where
2264
    checkVar (T_Assignment id Assign "PATH" [] (T_NormalWord _ parts)) =
V
Vidar Holen 已提交
2265 2266 2267 2268 2269
        when (any (\x -> isQuoted x && hasTilde x) parts) $
            warn id 2147 "Literal tilde in PATH works poorly across programs."
    checkVar _ = return ()

    hasTilde t = fromMaybe False (liftM2 elem (return '~') (getLiteralStringExt (const $ return "") t))
2270 2271
    isQuoted T_DoubleQuoted {} = True
    isQuoted T_SingleQuoted {} = True
V
Vidar Holen 已提交
2272 2273
    isQuoted _ = False
checkTildeInPath _ _ = return ()
2274

2275 2276
prop_checkUnsupported3 = verify checkUnsupported "#!/bin/sh\ncase foo in bar) baz ;& esac"
prop_checkUnsupported4 = verify checkUnsupported "#!/bin/ksh\ncase foo in bar) baz ;;& esac"
V
Vidar Holen 已提交
2277
prop_checkUnsupported5 = verify checkUnsupported "#!/bin/bash\necho \"${ ls; }\""
2278
checkUnsupported params t =
2279
    when (not (null support) && (shellType params `notElem` support)) $
2280 2281 2282 2283 2284 2285 2286 2287 2288 2289
        report name
 where
    (name, support) = shellSupport t
    report s = err (getId t) 2127 $
        "To use " ++ s ++ ", specify #!/usr/bin/env " ++
            (map toLower . intercalate " or " . map show $ support)

-- TODO: Move more of these checks here
shellSupport t =
  case t of
2290
    T_CaseExpression _ _ list -> forCase (map (\(a,_,_) -> a) list)
V
Vidar Holen 已提交
2291
    T_DollarBraceCommandExpansion {} -> ("${ ..; } command expansion", [Ksh])
V
Vidar Holen 已提交
2292
    _ -> ("", [])
2293
  where
2294
    forCase seps | CaseContinue `elem` seps = ("cases with ;;&", [Bash])
V
Vidar Holen 已提交
2295
    forCase seps | CaseFallThrough `elem` seps = ("cases with ;&", [Bash, Ksh])
2296 2297
    forCase _ = ("", [])

2298

R
Rodrigo Setti 已提交
2299
groupWith f = groupBy ((==) `on` f)
2300 2301 2302 2303 2304 2305 2306 2307

prop_checkMultipleAppends1 = verify checkMultipleAppends "foo >> file; bar >> file; baz >> file;"
prop_checkMultipleAppends2 = verify checkMultipleAppends "foo >> file; bar | grep f >> file; baz >> file;"
prop_checkMultipleAppends3 = verifyNot checkMultipleAppends "foo < file; bar < file; baz < file;"
checkMultipleAppends params t =
    mapM_ checkList $ getCommandSequences t
  where
    checkList list =
2308
        mapM_ checkGroup (groupWith (fmap fst) $ map getTarget list)
2309 2310 2311 2312
    checkGroup (f:_:_:_) | isJust f =
        style (snd $ fromJust f) 2129
            "Consider using { cmd1; cmd2; } >> file instead of individual redirects."
    checkGroup _ = return ()
2313
    getTarget (T_Annotation _ _ t) = getTarget t
2314 2315
    getTarget (T_Pipeline _ _ args@(_:_)) = getTarget (last args)
    getTarget (T_Redirecting id list _) = do
R
Rodrigo Setti 已提交
2316
        file <- mapMaybe getAppend list !!! 0
2317 2318
        return (file, id)
    getTarget _ = Nothing
2319
    getAppend (T_FdRedirect _ _ (T_IoFile _ T_DGREAT {} f)) = return f
2320
    getAppend _ = Nothing
2321 2322


2323 2324
prop_checkSuspiciousIFS1 = verify checkSuspiciousIFS "IFS=\"\\n\""
prop_checkSuspiciousIFS2 = verifyNot checkSuspiciousIFS "IFS=$'\\t'"
2325
checkSuspiciousIFS params (T_Assignment id Assign "IFS" [] value) =
2326 2327 2328 2329
    potentially $ do
        str <- getLiteralString value
        return $ check str
  where
R
Rodrigo Setti 已提交
2330 2331
    n = if shellType params == Sh then "'<literal linefeed here>'" else "$'\\n'"
    t = if shellType params == Sh then "\"$(printf '\\t')\"" else "$'\\t'"
2332 2333 2334 2335 2336 2337 2338 2339 2340
    check value =
        case value of
            "\\n" -> suggest n
            "/n" -> suggest n
            "\\t" -> suggest t
            "/t" -> suggest t
            _ -> return ()
    suggest r = warn id 2141 $ "Did you mean IFS=" ++ r ++ " ?"
checkSuspiciousIFS _ _ = return ()
2341 2342


2343 2344 2345 2346 2347
prop_checkGrepQ1= verify checkShouldUseGrepQ "[[ $(foo | grep bar) ]]"
prop_checkGrepQ2= verify checkShouldUseGrepQ "[ -z $(fgrep lol) ]"
prop_checkGrepQ3= verify checkShouldUseGrepQ "[ -n \"$(foo | zgrep lol)\" ]"
prop_checkGrepQ4= verifyNot checkShouldUseGrepQ "[ -z $(grep bar | cmd) ]"
prop_checkGrepQ5= verifyNot checkShouldUseGrepQ "rm $(ls | grep file)"
2348
prop_checkGrepQ6= verifyNot checkShouldUseGrepQ "[[ -n $(pgrep foo) ]]"
2349 2350
checkShouldUseGrepQ params t =
    potentially $ case t of
2351
        TC_Nullary id _ token -> check id True token
2352 2353 2354 2355 2356 2357 2358 2359 2360
        TC_Unary id _ "-n" token -> check id True token
        TC_Unary id _ "-z" token -> check id False token
        _ -> fail "not check"
  where
    check id bool token = do
        name <- getFinalGrep token
        let op = if bool then "-n" else "-z"
        let flip = if bool then "" else "! "
        return . style id 2143 $
V
Vidar Holen 已提交
2361 2362
            "Use " ++ flip ++ name ++ " -q instead of " ++
                "comparing output with [ " ++ op ++ " .. ]."
2363 2364 2365 2366 2367 2368 2369 2370 2371 2372 2373 2374 2375 2376

    getFinalGrep t = do
        cmds <- getPipeline t
        guard . not . null $ cmds
        name <- getCommandBasename $ last cmds
        guard . isGrep $ name
        return name
    getPipeline t =
        case t of
            T_NormalWord _ [x] -> getPipeline x
            T_DoubleQuoted _ [x] -> getPipeline x
            T_DollarExpansion _ [x] -> getPipeline x
            T_Pipeline _ _ cmds -> return cmds
            _ -> fail "unknown"
2377
    isGrep = (`elem` ["grep", "egrep", "fgrep", "zgrep"])
2378

2379 2380 2381 2382 2383 2384 2385 2386 2387 2388 2389 2390 2391 2392 2393
prop_checkTestArgumentSplitting1 = verify checkTestArgumentSplitting "[ -e *.mp3 ]"
prop_checkTestArgumentSplitting2 = verifyNot checkTestArgumentSplitting "[[ $a == *b* ]]"
prop_checkTestArgumentSplitting3 = verify checkTestArgumentSplitting "[[ *.png == '' ]]"
prop_checkTestArgumentSplitting4 = verify checkTestArgumentSplitting "[[ foo == f{o,oo,ooo} ]]"
prop_checkTestArgumentSplitting5 = verify checkTestArgumentSplitting "[[ $@ ]]"
prop_checkTestArgumentSplitting6 = verify checkTestArgumentSplitting "[ -e $@ ]"
prop_checkTestArgumentSplitting7 = verify checkTestArgumentSplitting "[ $@ == $@ ]"
prop_checkTestArgumentSplitting8 = verify checkTestArgumentSplitting "[[ $@ = $@ ]]"
prop_checkTestArgumentSplitting9 = verifyNot checkTestArgumentSplitting "[[ foo =~ bar{1,2} ]]"
prop_checkTestArgumentSplitting10 = verifyNot checkTestArgumentSplitting "[ \"$@\" ]"
prop_checkTestArgumentSplitting11 = verify checkTestArgumentSplitting "[[ \"$@\" ]]"
prop_checkTestArgumentSplitting12 = verify checkTestArgumentSplitting "[ *.png ]"
prop_checkTestArgumentSplitting13 = verify checkTestArgumentSplitting "[ \"$@\" == \"\" ]"
prop_checkTestArgumentSplitting14 = verify checkTestArgumentSplitting "[[ \"$@\" == \"\" ]]"
prop_checkTestArgumentSplitting15 = verifyNot checkTestArgumentSplitting "[[ \"$*\" == \"\" ]]"
2394
prop_checkTestArgumentSplitting16 = verifyNot checkTestArgumentSplitting "[[ -v foo[123] ]]"
2395 2396 2397
checkTestArgumentSplitting :: Parameters -> Token -> Writer [TokenComment] ()
checkTestArgumentSplitting _ t =
    case t of
2398 2399 2400 2401 2402 2403 2404 2405 2406
        (TC_Unary _ typ op token) | isGlob token ->
            if op == "-v"
            then
                when (typ == SingleBracket) $
                    err (getId token) 2208 $
                      "Use [[ ]] or quote arguments to -v to avoid glob expansion."
            else
                err (getId token) 2144 $
                   op ++ " doesn't work with globs. Use a for loop."
2407 2408 2409 2410 2411 2412 2413 2414 2415 2416 2417 2418 2419 2420 2421 2422 2423 2424 2425 2426 2427 2428 2429 2430 2431 2432 2433 2434 2435 2436 2437 2438 2439 2440 2441 2442 2443 2444 2445 2446

        (TC_Nullary _ typ token) -> do
            checkBraces typ token
            checkGlobs typ token
            when (typ == DoubleBracket) $
                checkArrays typ token

        (TC_Unary _ typ op token) -> checkAll typ token

        (TC_Binary _ typ op lhs rhs) ->
            if op `elem` ["=", "==", "!=", "=~"]
            then do
                checkAll typ lhs
                checkArrays typ rhs
                checkBraces typ rhs
            else mapM_ (checkAll typ) [lhs, rhs]
        _ -> return ()
  where
    checkAll typ token = do
        checkArrays typ token
        checkBraces typ token
        checkGlobs typ token

    checkArrays typ token =
        when (any isArrayExpansion $ getWordParts token) $
            if typ == SingleBracket
            then warn (getId token) 2198 "Arrays don't work as operands in [ ]. Use a loop (or concatenate with * instead of @)."
            else err (getId token) 2199 "Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @)."

    checkBraces typ token =
        when (any isBraceExpansion $ getWordParts token) $
            if typ == SingleBracket
            then warn (getId token) 2200 "Brace expansions don't work as operands in [ ]. Use a loop."
            else err (getId token) 2201 "Brace expansion doesn't happen in [[ ]]. Use a loop."

    checkGlobs typ token =
        when (isGlob token) $
            if typ == SingleBracket
            then warn (getId token) 2202 "Globs don't work as operands in [ ]. Use a loop."
            else err (getId token) 2203 "Globs are ignored in [[ ]] except right of =/!=. Use a loop."
R
Rodrigo Setti 已提交
2447

2448

2449 2450 2451 2452 2453 2454 2455 2456
prop_checkMaskedReturns1 = verify checkMaskedReturns "f() { local a=$(false); }"
prop_checkMaskedReturns2 = verify checkMaskedReturns "declare a=$(false)"
prop_checkMaskedReturns3 = verify checkMaskedReturns "declare a=\"`false`\""
prop_checkMaskedReturns4 = verifyNot checkMaskedReturns "declare a; a=$(false)"
prop_checkMaskedReturns5 = verifyNot checkMaskedReturns "f() { local -r a=$(false); }"
checkMaskedReturns _ t@(T_SimpleCommand id _ (cmd:rest)) = potentially $ do
    name <- getCommandName t
    guard $ name `elem` ["declare", "export"]
2457
        || name == "local" && "r" `notElem` map snd (getAllFlags t)
2458 2459 2460 2461 2462 2463 2464 2465 2466 2467 2468 2469
    return $ mapM_ checkArgs rest
  where
    checkArgs (T_Assignment id _ _ _ word) | any hasReturn $ getWordParts word =
        warn id 2155 "Declare and assign separately to avoid masking return values."
    checkArgs _ = return ()

    hasReturn t = case t of
        T_Backticked {} -> True
        T_DollarExpansion {} -> True
        _ -> False
checkMaskedReturns _ _ = return ()

2470

V
Vidar Holen 已提交
2471 2472
prop_checkReadWithoutR1 = verify checkReadWithoutR "read -a foo"
prop_checkReadWithoutR2 = verifyNot checkReadWithoutR "read -ar foo"
2473
checkReadWithoutR _ t@T_SimpleCommand {} | t `isUnqualifiedCommand` "read" =
V
Vidar Holen 已提交
2474
    unless ("r" `elem` map snd (getAllFlags t)) $
V
Vidar Holen 已提交
2475
        info (getId $ getCommandTokenOrThis t) 2162 "read without -r will mangle backslashes."
V
Vidar Holen 已提交
2476 2477
checkReadWithoutR _ _ = return ()

2478 2479 2480 2481 2482 2483 2484 2485
prop_checkUncheckedCd1 = verifyTree checkUncheckedCdPushdPopd "cd ~/src; rm -r foo"
prop_checkUncheckedCd2 = verifyNotTree checkUncheckedCdPushdPopd "cd ~/src || exit; rm -r foo"
prop_checkUncheckedCd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; cd ~/src; rm -r foo"
prop_checkUncheckedCd4 = verifyNotTree checkUncheckedCdPushdPopd "if cd foo; then rm foo; fi"
prop_checkUncheckedCd5 = verifyTree checkUncheckedCdPushdPopd "if true; then cd foo; fi"
prop_checkUncheckedCd6 = verifyNotTree checkUncheckedCdPushdPopd "cd .."
prop_checkUncheckedCd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\ncd foo\nrm bar"
prop_checkUncheckedCd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; cd foo; rm bar"
N
Ng Zhi An 已提交
2486
prop_checkUncheckedCd9 = verifyTree checkUncheckedCdPushdPopd "builtin cd ~/src; rm -r foo"
2487 2488 2489 2490 2491 2492 2493 2494 2495 2496 2497 2498 2499 2500 2501 2502 2503
prop_checkUncheckedPushd1 = verifyTree checkUncheckedCdPushdPopd "pushd ~/src; rm -r foo"
prop_checkUncheckedPushd2 = verifyNotTree checkUncheckedCdPushdPopd "pushd ~/src || exit; rm -r foo"
prop_checkUncheckedPushd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; pushd ~/src; rm -r foo"
prop_checkUncheckedPushd4 = verifyNotTree checkUncheckedCdPushdPopd "if pushd foo; then rm foo; fi"
prop_checkUncheckedPushd5 = verifyTree checkUncheckedCdPushdPopd "if true; then pushd foo; fi"
prop_checkUncheckedPushd6 = verifyNotTree checkUncheckedCdPushdPopd "pushd .."
prop_checkUncheckedPushd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\npushd foo\nrm bar"
prop_checkUncheckedPushd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; pushd foo; rm bar"
prop_checkUncheckedPushd9 = verifyNotTree checkUncheckedCdPushdPopd "pushd -n foo"
prop_checkUncheckedPopd1 = verifyTree checkUncheckedCdPushdPopd "popd; rm -r foo"
prop_checkUncheckedPopd2 = verifyNotTree checkUncheckedCdPushdPopd "popd || exit; rm -r foo"
prop_checkUncheckedPopd3 = verifyNotTree checkUncheckedCdPushdPopd "set -e; popd; rm -r foo"
prop_checkUncheckedPopd4 = verifyNotTree checkUncheckedCdPushdPopd "if popd; then rm foo; fi"
prop_checkUncheckedPopd5 = verifyTree checkUncheckedCdPushdPopd "if true; then popd; fi"
prop_checkUncheckedPopd6 = verifyTree checkUncheckedCdPushdPopd "popd"
prop_checkUncheckedPopd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\npopd\nrm bar"
prop_checkUncheckedPopd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; popd; rm bar"
2504
prop_checkUncheckedPopd9 = verifyNotTree checkUncheckedCdPushdPopd "popd -n foo"
2505 2506 2507 2508

checkUncheckedCdPushdPopd params root =
    if hasSetE params then
        []
2509
    else execWriter $ doAnalysis checkElement root
2510
  where
2511
    checkElement t@T_SimpleCommand {} =
2512 2513
        when(name t `elem` ["cd", "pushd", "popd"]
            && not (isSafeDir t)
2514
            && not (name t `elem` ["pushd", "popd"] && ("n" `elem` map snd (getAllFlags t)))
2515
            && not (isCondition $ getPath (parentMap params) t)) $
2516
                warn (getId t) 2164 "Use 'cd ... || exit' or 'cd ... || return' in case cd fails."
2517
    checkElement _ = return ()
2518 2519 2520 2521
    name t = fromMaybe "" $ getCommandName t
    isSafeDir t = case oversimplify t of
          [_, ".."] -> True;
          _ -> False
2522

2523 2524 2525 2526 2527 2528 2529 2530 2531 2532 2533 2534 2535 2536
prop_checkLoopVariableReassignment1 = verify checkLoopVariableReassignment "for i in *; do for i in *.bar; do true; done; done"
prop_checkLoopVariableReassignment2 = verify checkLoopVariableReassignment "for i in *; do for((i=0; i<3; i++)); do true; done; done"
prop_checkLoopVariableReassignment3 = verifyNot checkLoopVariableReassignment "for i in *; do for j in *.bar; do true; done; done"
checkLoopVariableReassignment params token =
    potentially $ case token of
        T_ForIn {} -> check
        T_ForArithmetic {} -> check
        _ -> Nothing
  where
    check = do
        str <- loopVariable token
        next <- listToMaybe $ filter (\x -> loopVariable x == Just str) path
        return $ do
            warn (getId token) 2165 "This nested loop overrides the index variable of its parent."
2537
            warn (getId next)  2167 "This parent loop has its index variable overridden."
2538 2539 2540 2541 2542 2543 2544
    path = drop 1 $ getPath (parentMap params) token
    loopVariable :: Token -> Maybe String
    loopVariable t =
        case t of
            T_ForIn _ s _ _ -> return s
            T_ForArithmetic _
                (TA_Sequence _
2545
                    [TA_Assignment _ "="
2546
                        (TA_Variable _ var _ ) _])
2547 2548 2549
                            _ _ _ -> return var
            _ -> fail "not loop"

2550 2551 2552 2553 2554 2555 2556 2557
prop_checkTrailingBracket1 = verify checkTrailingBracket "if -z n ]]; then true; fi "
prop_checkTrailingBracket2 = verifyNot checkTrailingBracket "if [[ -z n ]]; then true; fi "
prop_checkTrailingBracket3 = verify checkTrailingBracket "a || b ] && thing"
prop_checkTrailingBracket4 = verifyNot checkTrailingBracket "run [ foo ]"
prop_checkTrailingBracket5 = verifyNot checkTrailingBracket "run bar ']'"
checkTrailingBracket _ token =
    case token of
        T_SimpleCommand _ _ tokens@(_:_) -> check (last tokens) token
V
Vidar Holen 已提交
2558
        _ -> return ()
2559 2560 2561 2562 2563 2564 2565 2566 2567 2568
  where
    check t command =
        case t of
            T_NormalWord id [T_Literal _ str] -> potentially $ do
                guard $ str `elem` [ "]]", "]" ]
                let opposite = invert str
                    parameters = oversimplify command
                guard $ opposite `notElem` parameters
                return $ warn id 2171 $
                    "Found trailing " ++ str ++ " outside test. Missing " ++ opposite ++ "?"
V
Vidar Holen 已提交
2569
            _ -> return ()
2570 2571 2572 2573 2574 2575
    invert s =
        case s of
            "]]" -> "[["
            "]" -> "["
            x -> x

2576 2577 2578 2579 2580 2581 2582 2583 2584 2585 2586 2587 2588 2589 2590 2591 2592
prop_checkReturnAgainstZero1 = verify checkReturnAgainstZero "[ $? -eq 0 ]"
prop_checkReturnAgainstZero2 = verify checkReturnAgainstZero "[[ \"$?\" -gt 0 ]]"
prop_checkReturnAgainstZero3 = verify checkReturnAgainstZero "[[ 0 -ne $? ]]"
prop_checkReturnAgainstZero4 = verifyNot checkReturnAgainstZero "[[ $? -eq 4 ]]"
prop_checkReturnAgainstZero5 = verify checkReturnAgainstZero "[[ 0 -eq $? ]]"
prop_checkReturnAgainstZero6 = verifyNot checkReturnAgainstZero "[[ $R -eq 0 ]]"
prop_checkReturnAgainstZero7 = verify checkReturnAgainstZero "(( $? == 0 ))"
prop_checkReturnAgainstZero8 = verify checkReturnAgainstZero "(( $? ))"
prop_checkReturnAgainstZero9 = verify checkReturnAgainstZero "(( ! $? ))"
checkReturnAgainstZero _ token =
    case token of
        TC_Binary id _ _ lhs rhs -> check lhs rhs
        TA_Binary id _ lhs rhs -> check lhs rhs
        TA_Unary id _ exp ->
            when (isExitCode exp) $ message (getId exp)
        TA_Sequence _ [exp] ->
            when (isExitCode exp) $ message (getId exp)
V
Vidar Holen 已提交
2593
        _ -> return ()
2594 2595 2596 2597 2598 2599 2600 2601
  where
    check lhs rhs =
        if isZero rhs && isExitCode lhs
        then message (getId lhs)
        else when (isZero lhs && isExitCode rhs) $ message (getId rhs)
    isZero t = getLiteralString t == Just "0"
    isExitCode t =
        case getWordParts t of
2602
            [exp@T_DollarBraced {}] -> bracedString exp == "?"
V
Vidar Holen 已提交
2603
            _ -> False
2604 2605
    message id = style id 2181 "Check exit code directly with e.g. 'if mycmd;', not indirectly with $?."

2606 2607 2608 2609 2610 2611 2612 2613 2614 2615 2616 2617 2618 2619 2620 2621 2622 2623 2624 2625 2626 2627 2628 2629 2630 2631 2632 2633 2634 2635 2636 2637 2638
prop_checkRedirectedNowhere1 = verify checkRedirectedNowhere "> file"
prop_checkRedirectedNowhere2 = verify checkRedirectedNowhere "> file | grep foo"
prop_checkRedirectedNowhere3 = verify checkRedirectedNowhere "grep foo | > bar"
prop_checkRedirectedNowhere4 = verifyNot checkRedirectedNowhere "grep foo > bar"
prop_checkRedirectedNowhere5 = verifyNot checkRedirectedNowhere "foo | grep bar > baz"
prop_checkRedirectedNowhere6 = verifyNot checkRedirectedNowhere "var=$(value) 2> /dev/null"
prop_checkRedirectedNowhere7 = verifyNot checkRedirectedNowhere "var=$(< file)"
prop_checkRedirectedNowhere8 = verifyNot checkRedirectedNowhere "var=`< file`"
checkRedirectedNowhere params token =
    case token of
        T_Pipeline _ _ [single] -> potentially $ do
            redir <- getDanglingRedirect single
            guard . not $ isInExpansion token
            return $ warn (getId redir) 2188 "This redirection doesn't have a command. Move to its command (or use 'true' as no-op)."

        T_Pipeline _ _ list -> forM_ list $ \x -> potentially $ do
            redir <- getDanglingRedirect x
            return $ err (getId redir) 2189 "You can't have | between this redirection and the command it should apply to."

        _ -> return ()
  where
    isInExpansion t =
        case drop 1 $ getPath (parentMap params) t of
            T_DollarExpansion _ [_] : _ -> True
            T_Backticked _ [_] : _ -> True
            T_Annotation _ _ u : _ -> isInExpansion u
            _ -> False
    getDanglingRedirect token =
        case token of
            T_Redirecting _ (first:_) (T_SimpleCommand _ [] []) -> return first
            _ -> Nothing


2639 2640 2641 2642 2643 2644 2645 2646 2647 2648 2649 2650 2651 2652 2653 2654 2655 2656 2657 2658 2659 2660 2661 2662 2663 2664 2665 2666 2667 2668 2669 2670 2671 2672 2673 2674
prop_checkArrayAssignmentIndices1 = verifyTree checkArrayAssignmentIndices "declare -A foo; foo=(bar)"
prop_checkArrayAssignmentIndices2 = verifyNotTree checkArrayAssignmentIndices "declare -a foo; foo=(bar)"
prop_checkArrayAssignmentIndices3 = verifyNotTree checkArrayAssignmentIndices "declare -A foo; foo=([i]=bar)"
prop_checkArrayAssignmentIndices4 = verifyTree checkArrayAssignmentIndices "typeset -A foo; foo+=(bar)"
prop_checkArrayAssignmentIndices5 = verifyTree checkArrayAssignmentIndices "arr=( [foo]= bar )"
prop_checkArrayAssignmentIndices6 = verifyTree checkArrayAssignmentIndices "arr=( [foo] = bar )"
prop_checkArrayAssignmentIndices7 = verifyTree checkArrayAssignmentIndices "arr=( var=value )"
prop_checkArrayAssignmentIndices8 = verifyNotTree checkArrayAssignmentIndices "arr=( [foo]=bar )"
prop_checkArrayAssignmentIndices9 = verifyNotTree checkArrayAssignmentIndices "arr=( [foo]=\"\" )"
checkArrayAssignmentIndices params root =
    runNodeAnalysis check params root
  where
    assocs = getAssociativeArrays root
    check _ t =
        case t of
            T_Assignment _ _ name [] (T_Array _ list) ->
                let isAssoc = name `elem` assocs in
                    mapM_ (checkElement isAssoc) list
            _ -> return ()

    checkElement isAssociative t =
        case t of
            T_IndexedElement _ _ (T_Literal id "") ->
                warn id 2192 "This array element has no value. Remove spaces after = or use \"\" for empty string."
            T_IndexedElement {} ->
                return ()

            T_NormalWord _ parts ->
                let literalEquals = do
                    part <- parts
                    (id, str) <- case part of
                        T_Literal id str -> [(id,str)]
                        _ -> []
                    guard $ '=' `elem` str
                    return $ warn id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it."
                in
2675
                    if null literalEquals && isAssociative
2676 2677 2678 2679 2680
                    then warn (getId t) 2190 "Elements in associative arrays need index, e.g. array=( [index]=value ) ."
                    else sequence_ literalEquals

            _ -> return ()

2681 2682 2683 2684
prop_checkUnmatchableCases1 = verify checkUnmatchableCases "case foo in bar) true; esac"
prop_checkUnmatchableCases2 = verify checkUnmatchableCases "case foo-$bar in ??|*) true; esac"
prop_checkUnmatchableCases3 = verify checkUnmatchableCases "case foo in foo) true; esac"
prop_checkUnmatchableCases4 = verifyNot checkUnmatchableCases "case foo-$bar in foo*|*bar|*baz*) true; esac"
2685 2686 2687 2688
prop_checkUnmatchableCases5 = verify checkUnmatchableCases "case $f in *.txt) true;; f??.txt) false;; esac"
prop_checkUnmatchableCases6 = verifyNot checkUnmatchableCases "case $f in ?*) true;; *) false;; esac"
prop_checkUnmatchableCases7 = verifyNot checkUnmatchableCases "case $f in $(x)) true;; asdf) false;; esac"
prop_checkUnmatchableCases8 = verify checkUnmatchableCases "case $f in cow) true;; bar|cow) false;; esac"
2689
prop_checkUnmatchableCases9 = verifyNot checkUnmatchableCases "case $f in x) true;;& x) false;; esac"
2690 2691
checkUnmatchableCases _ t =
    case t of
2692
        T_CaseExpression _ word list -> do
2693 2694 2695 2696
            -- Check all patterns for whether they can ever match
            let allpatterns  = concatMap snd3 list
            -- Check only the non-fallthrough branches for shadowing
            let breakpatterns = concatMap snd3 $ filter (\x -> fst3 x == CaseBreak) list
2697

2698
            if isConstant word
2699 2700 2701 2702
                then warn (getId word) 2194
                        "This word is constant. Did you forget the $ on a variable?"
                else  potentially $ do
                    pg <- wordToPseudoGlob word
2703
                    return $ mapM_ (check pg) allpatterns
2704

2705 2706
            let exactGlobs = tupMap wordToExactPseudoGlob breakpatterns
            let fuzzyGlobs = tupMap wordToPseudoGlob breakpatterns
2707 2708 2709 2710
            let dominators = zip exactGlobs (tails $ drop 1 fuzzyGlobs)

            mapM_ checkDoms dominators

2711 2712
        _ -> return ()
  where
2713
    fst3 (x,_,_) = x
2714
    snd3 (_,x,_) = x
2715 2716 2717 2718 2719
    check target candidate = potentially $ do
        candidateGlob <- wordToPseudoGlob candidate
        guard . not $ pseudoGlobsCanOverlap target candidateGlob
        return $ warn (getId candidate) 2195
                    "This pattern will never match the case statement's word. Double check them."
2720

2721 2722 2723 2724 2725 2726 2727 2728 2729 2730 2731 2732 2733 2734
    tupMap f l = zip l (map f l)
    checkDoms ((glob, Just x), rest) =
        case filter (\(_, p) -> x `pseudoGlobIsSuperSetof` p) valids of
            ((first,_):_) -> do
                warn (getId glob) 2221 "This pattern always overrides a later one."
                warn (getId first) 2222 "This pattern never matches because of a previous pattern."
            _ -> return ()
      where
        valids = concatMap f rest
        f (x, Just y) = [(x,y)]
        f _ = []
    checkDoms _ = return ()


V
Vidar Holen 已提交
2735 2736 2737 2738 2739 2740 2741 2742 2743 2744 2745 2746 2747 2748 2749 2750 2751 2752 2753 2754 2755 2756 2757 2758 2759 2760 2761 2762
prop_checkSubshellAsTest1 = verify checkSubshellAsTest "( -e file )"
prop_checkSubshellAsTest2 = verify checkSubshellAsTest "( 1 -gt 2 )"
prop_checkSubshellAsTest3 = verifyNot checkSubshellAsTest "( grep -c foo bar )"
prop_checkSubshellAsTest4 = verifyNot checkSubshellAsTest "[ 1 -gt 2 ]"
prop_checkSubshellAsTest5 = verify checkSubshellAsTest "( -e file && -x file )"
prop_checkSubshellAsTest6 = verify checkSubshellAsTest "( -e file || -x file && -t 1 )"
prop_checkSubshellAsTest7 = verify checkSubshellAsTest "( ! -d file )"
checkSubshellAsTest _ t =
    case t of
        T_Subshell id [w] -> check id w
        _ -> return ()
  where
    check id t = case t of
        (T_Banged _ w) -> check id w
        (T_AndIf _ w _) -> check id w
        (T_OrIf _ w _) -> check id w
        (T_Pipeline _ _ [T_Redirecting _ _ (T_SimpleCommand _ [] (first:second:_))]) ->
            checkParams id first second
        _ -> return ()


    checkParams id first second = do
        when (fromMaybe False $ (`elem` unaryTestOps) <$> getLiteralString first) $
            err id 2204 "(..) is a subshell. Did you mean [ .. ], a test expression?"
        when (fromMaybe False $ (`elem` binaryTestOps) <$> getLiteralString second) $
            warn id 2205 "(..) is a subshell. Did you mean [ .. ], a test expression?"


2763 2764 2765 2766 2767 2768 2769 2770 2771 2772 2773 2774 2775 2776 2777 2778 2779 2780 2781 2782 2783 2784 2785 2786 2787
prop_checkSplittingInArrays1 = verify checkSplittingInArrays "a=( $var )"
prop_checkSplittingInArrays2 = verify checkSplittingInArrays "a=( $(cmd) )"
prop_checkSplittingInArrays3 = verifyNot checkSplittingInArrays "a=( \"$var\" )"
prop_checkSplittingInArrays4 = verifyNot checkSplittingInArrays "a=( \"$(cmd)\" )"
prop_checkSplittingInArrays5 = verifyNot checkSplittingInArrays "a=( $! $$ $# )"
prop_checkSplittingInArrays6 = verifyNot checkSplittingInArrays "a=( ${#arr[@]} )"
prop_checkSplittingInArrays7 = verifyNot checkSplittingInArrays "a=( foo{1,2} )"
prop_checkSplittingInArrays8 = verifyNot checkSplittingInArrays "a=( * )"
checkSplittingInArrays params t =
    case t of
        T_Array _ elements -> mapM_ check elements
        _ -> return ()
  where
    check word = case word of
        T_NormalWord _ parts -> mapM_ checkPart parts
        _ -> return ()
    checkPart part = case part of
        T_DollarExpansion id _ -> forCommand id
        T_DollarBraceCommandExpansion id _ -> forCommand id
        T_Backticked id _ -> forCommand id
        T_DollarBraced id str |
            not (isCountingReference part)
            && not (isQuotedAlternativeReference part)
            && not (getBracedReference (bracedString part) `elem` variablesWithoutSpaces)
            -> warn id 2206 $
2788
                if shellType params == Ksh
2789 2790 2791 2792 2793 2794
                then "Quote to prevent word splitting, or split robustly with read -A or while read."
                else "Quote to prevent word splitting, or split robustly with mapfile or read -a."
        _ -> return ()

    forCommand id =
        warn id 2207 $
2795
            if shellType params == Ksh
2796 2797 2798 2799
            then "Prefer read -A or while read to split command output (or quote to avoid splitting)."
            else "Prefer mapfile or read -a to split command output (or quote to avoid splitting)."


2800 2801 2802 2803 2804 2805 2806 2807 2808 2809 2810
prop_checkRedirectionToNumber1 = verify checkRedirectionToNumber "( 1 > 2 )"
prop_checkRedirectionToNumber2 = verify checkRedirectionToNumber "foo 1>2"
prop_checkRedirectionToNumber3 = verifyNot checkRedirectionToNumber "echo foo > '2'"
prop_checkRedirectionToNumber4 = verifyNot checkRedirectionToNumber "foo 1>&2"
checkRedirectionToNumber _ t = case t of
    T_IoFile id _ word -> potentially $ do
        file <- getUnquotedLiteral word
        guard $ all isDigit file
        return $ warn id 2210 "This is a file redirection. Was it supposed to be a comparison or fd operation?"
    _ -> return ()

2811 2812 2813 2814 2815 2816 2817 2818 2819
prop_checkGlobAsCommand1 = verify checkGlobAsCommand "foo*"
prop_checkGlobAsCommand2 = verify checkGlobAsCommand "$(var[i])"
prop_checkGlobAsCommand3 = verifyNot checkGlobAsCommand "echo foo*"
checkGlobAsCommand _ t = case t of
    T_SimpleCommand _ _ (first:_) ->
        when (isGlob first) $
            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?"
    _ -> return ()

2820 2821 2822 2823 2824 2825 2826 2827 2828 2829 2830 2831

prop_checkFlagAsCommand1 = verify checkFlagAsCommand "-e file"
prop_checkFlagAsCommand2 = verify checkFlagAsCommand "foo\n  --bar=baz"
prop_checkFlagAsCommand3 = verifyNot checkFlagAsCommand "'--myexec--' args"
prop_checkFlagAsCommand4 = verifyNot checkFlagAsCommand "var=cmd --arg"  -- Handled by SC2037
checkFlagAsCommand _ t = case t of
    T_SimpleCommand _ [] (first:_) ->
        when (isUnquotedFlag first) $
            warn (getId first) 2215 "This flag is used as a command name. Bad line break or missing [ .. ]?"
    _ -> return ()


V
Vidar Holen 已提交
2832 2833 2834 2835 2836 2837
prop_checkEmptyCondition1 = verify checkEmptyCondition "if [ ]; then ..; fi"
prop_checkEmptyCondition2 = verifyNot checkEmptyCondition "[ foo -o bar ]"
checkEmptyCondition _ t = case t of
    TC_Empty id _ -> style id 2212 "Use 'false' instead of empty [/[[ conditionals."
    _ -> return ()

2838 2839 2840 2841 2842 2843 2844
prop_checkPipeToNowhere1 = verify checkPipeToNowhere "foo | echo bar"
prop_checkPipeToNowhere2 = verify checkPipeToNowhere "basename < file.txt"
prop_checkPipeToNowhere3 = verify checkPipeToNowhere "printf 'Lol' <<< str"
prop_checkPipeToNowhere4 = verify checkPipeToNowhere "printf 'Lol' << eof\nlol\neof\n"
prop_checkPipeToNowhere5 = verifyNot checkPipeToNowhere "echo foo | xargs du"
prop_checkPipeToNowhere6 = verifyNot checkPipeToNowhere "ls | echo $(cat)"
prop_checkPipeToNowhere7 = verifyNot checkPipeToNowhere "echo foo | var=$(cat) ls"
2845
prop_checkPipeToNowhere8 = verify checkPipeToNowhere "foo | true"
N
Ng Zhi An 已提交
2846
prop_checkPipeToNowhere9 = verifyNot checkPipeToNowhere "mv -i f . < /dev/stdin"
2847 2848 2849 2850 2851 2852 2853 2854 2855 2856 2857 2858 2859 2860 2861 2862 2863 2864 2865
checkPipeToNowhere :: Parameters -> Token -> WriterT [TokenComment] Identity ()
checkPipeToNowhere _ t =
    case t of
        T_Pipeline _ _ (first:rest) -> mapM_ checkPipe rest
        T_Redirecting _ redirects cmd -> when (any redirectsStdin redirects) $ checkRedir cmd
        _ -> return ()
  where
    checkPipe redir = potentially $ do
        cmd <- getCommand redir
        name <- getCommandBasename cmd
        guard $ name `elem` nonReadingCommands
        guard . not $ hasAdditionalConsumers cmd
        return $ warn (getId cmd) 2216 $
            "Piping to '" ++ name ++ "', a command that doesn't read stdin. Wrong command or missing xargs?"

    checkRedir cmd = potentially $ do
        name <- getCommandBasename cmd
        guard $ name `elem` nonReadingCommands
        guard . not $ hasAdditionalConsumers cmd
N
Ng Zhi An 已提交
2866
        guard . not $ name `elem` ["cp", "mv", "rm"] && cmd `hasFlag` "i"
2867 2868 2869 2870 2871 2872 2873 2874 2875 2876 2877 2878 2879 2880 2881 2882 2883 2884 2885 2886 2887 2888
        return $ warn (getId cmd) 2217 $
            "Redirecting to '" ++ name ++ "', a command that doesn't read stdin. Bad quoting or missing xargs?"

    -- Could any words in a SimpleCommand consume stdin (e.g. echo "$(cat)")?
    hasAdditionalConsumers t = fromMaybe True $ do
        doAnalysis (guard . not . mayConsume) t
        return False

    mayConsume t =
        case t of
            T_ProcSub {} -> True
            T_Backticked {} -> True
            T_DollarExpansion {} -> True
            _ -> False

    redirectsStdin t =
        case t of
            T_FdRedirect _ _ (T_IoFile _ T_Less {} _) -> True
            T_FdRedirect _ _ T_HereDoc {} -> True
            T_FdRedirect _ _ T_HereString {} -> True
            _ -> False

2889 2890 2891 2892 2893 2894 2895 2896 2897 2898 2899 2900 2901 2902 2903 2904 2905 2906 2907 2908 2909 2910 2911 2912 2913 2914 2915 2916 2917 2918 2919
prop_checkUseBeforeDefinition1 = verifyTree checkUseBeforeDefinition "f; f() { true; }"
prop_checkUseBeforeDefinition2 = verifyNotTree checkUseBeforeDefinition "f() { true; }; f"
prop_checkUseBeforeDefinition3 = verifyNotTree checkUseBeforeDefinition "if ! mycmd --version; then mycmd() { true; }; fi"
prop_checkUseBeforeDefinition4 = verifyNotTree checkUseBeforeDefinition "mycmd || mycmd() { f; }"
checkUseBeforeDefinition _ t =
    execWriter $ evalStateT (mapM_ examine $ revCommands) Map.empty
  where
    examine t = case t of
        T_Pipeline _ _ [T_Redirecting _ _ (T_Function _ _ _ name _)] ->
            modify $ Map.insert name t
        T_Annotation _ _ w -> examine w
        T_Pipeline _ _ cmds -> do
            m <- get
            unless (Map.null m) $
                mapM_ (checkUsage m) $ concatMap recursiveSequences cmds
        _ -> return ()

    checkUsage map cmd = potentially $ do
        name <- getCommandName cmd
        def <- Map.lookup name map
        return $
            err (getId cmd) 2218
                "This function is only defined later. Move the definition up."

    revCommands = reverse $ concat $ getCommandSequences t
    recursiveSequences x =
        let list = concat $ getCommandSequences x in
            if null list
            then [x]
            else concatMap recursiveSequences list

2920 2921 2922 2923 2924 2925 2926 2927 2928 2929 2930 2931 2932 2933
prop_checkForLoopGlobVariables1 = verify checkForLoopGlobVariables "for i in $var/*.txt; do true; done"
prop_checkForLoopGlobVariables2 = verifyNot checkForLoopGlobVariables "for i in \"$var\"/*.txt; do true; done"
prop_checkForLoopGlobVariables3 = verifyNot checkForLoopGlobVariables "for i in $var; do true; done"
checkForLoopGlobVariables _ t =
    case t of
        T_ForIn _ _ words _ -> mapM_ check words
        _ -> return ()
  where
    check (T_NormalWord _ parts) =
        when (any isGlob parts) $
            mapM_ suggest $ filter isQuoteableExpansion parts
    suggest t = info (getId t) 2231
        "Quote expansions in this for loop glob to prevent wordsplitting, e.g. \"$dir\"/*.txt ."

2934 2935 2936 2937 2938 2939 2940 2941 2942 2943 2944 2945 2946 2947 2948 2949 2950 2951 2952 2953 2954 2955 2956 2957 2958 2959 2960 2961 2962 2963 2964 2965 2966 2967 2968 2969 2970 2971 2972 2973 2974 2975 2976 2977 2978 2979 2980 2981 2982 2983 2984 2985 2986 2987 2988 2989 2990 2991
prop_checkSubshelledTests1 = verify checkSubshelledTests "a && ( [ b ] || ! [ c ] )"
prop_checkSubshelledTests2 = verify checkSubshelledTests "( [ a ] )"
prop_checkSubshelledTests3 = verify checkSubshelledTests "( [ a ] && [ b ] || test c )"
checkSubshelledTests params t =
    case t of
        T_Subshell id list | isSubshelledTest t ->
            case () of
                -- Special case for if (test) and while (test)
                _ | isCompoundCondition (getPath (parentMap params) t) ->
                    style id 2233 "Remove superfluous (..) around condition."

                -- Special case for ([ x ])
                _ | isSingleTest list ->
                    style id 2234 "Remove superfluous (..) around test command."

                -- General case for ([ x ] || [ y ] && etc)
                _ -> style id 2235 "Use { ..; } instead of (..) to avoid subshell overhead."
        _ -> return ()
  where

    isSingleTest cmds =
        case cmds of
            [c] | isTestCommand c -> True
            _ -> False

    isSubshelledTest t =
        case t of
            T_Subshell _ list -> all isSubshelledTest list
            T_AndIf _ a b -> isSubshelledTest a && isSubshelledTest b
            T_OrIf  _ a b -> isSubshelledTest a && isSubshelledTest b
            T_Annotation _ _ t -> isSubshelledTest t
            _ -> isTestCommand t

    isTestCommand t =
        case t of
            T_Banged _ t -> isTestCommand t
            T_Pipeline _ [] [T_Redirecting _ _ T_Condition {}] -> True
            T_Pipeline _ [] [T_Redirecting _ _ cmd] -> cmd `isCommand` "test"
            _ -> False

    -- Check if a T_Subshell is used as a condition, e.g. if ( test )
    -- This technically also triggers for `if true; then ( test ); fi`
    -- but it's still a valid suggestion.
    isCompoundCondition chain =
        case dropWhile skippable (drop 1 chain) of
            T_IfExpression {}    : _ -> True
            T_WhileExpression {} : _ -> True
            T_UntilExpression {} : _ -> True
            _ -> False

    -- Skip any parent of a T_Subshell until we reach something interesting
    skippable t =
        case t of
            T_Redirecting _ [] _ -> True
            T_Pipeline _ [] _ -> True
            T_Annotation {} -> True
            _ -> False

2992 2993
return []
runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])