Merge branch 'master' of https://github.com/koalaman/shellcheck into mrshu/pushd-popd-like-cd

Signed-off-by: mr.Shu <mr@shu.io>

.prepare_deploy

@@ -0,0 +1,34 @@
+#!/bin/bash
+# This script packages up Travis compiled binaries
+set -ex
+shopt -s nullglob
+cd deploy
+
+cp ../LICENSE LICENSE.txt
+sed -e $'s/$/\r/' > README.txt << END
+This is a precompiled ShellCheck binary.
+      http://www.shellcheck.net/
+
+ShellCheck is a static analysis tool for shell scripts.
+It's licensed under the GNU General Public License v3.0.
+Information and source code is available on the website.
+
+This binary was compiled on $(date -u).
+
+
+
+      ====== Latest commits ======
+
+$(git log -n 3)
+END
+
+for file in ./*.exe
+do
+  zip "${file%.*}.zip" README.txt LICENSE.txt "$file"
+done
+
+for file in ./*
+do
+  sha512sum "$file" > "$file.sha512sum"
+done
+

.travis.yml

@@ -11,11 +11,35 @@ before_install:
     export TAG=$([ "$TRAVIS_BRANCH" == "master" ] && echo "latest" || ([ -n "$TRAVIS_TAG" ] && echo "$TRAVIS_TAG") || echo "$TRAVIS_BRANCH")
 
 script:
+  - mkdir deploy
+  # Windows .exe
+  - docker pull koalaman/winghc
+  - docker run -v "$PWD:/appdata" koalaman/winghc cuib
+  - cp "dist/build/ShellCheck/shellcheck.exe" "deploy/shellcheck-$TAG.exe"
+  # Linux Docker
   - docker build -t builder -f Dockerfile_builder .
-  - docker run --rm -it -v $(pwd):/mnt builder
+  - docker run --rm -it -v "$(pwd):/mnt" builder
-  - docker build -t $DOCKER_REPO:$TAG .
+  - docker build -t "$DOCKER_REPO:$TAG" .
 
 after_success:
+  - ./.prepare_deploy
   - docker login -e="$DOCKER_EMAIL" -u="$DOCKER_USERNAME" -p="$DOCKER_PASSWORD"
   - |-
     ([ "$TRAVIS_BRANCH" == "master" ] || [ -n "$TRAVIS_TAG" ]) && docker push "$DOCKER_REPO:$TAG"
+
+after_failure:
+  - pwd
+  - df -h
+  - find . -name '*.log' -type f -exec grep "" /dev/null {} +
+  - find .
+
+deploy:
+  provider: gcs
+  skip_cleanup: true
+  access_key_id: GOOG7MDN7WEH6IIGBDCA
+  secret_access_key:
+    secure: Bcx2cT0/E2ikj7sdamVq52xlLZF9dz9ojGPtoKfPyQhkkZa+McVI4xgUSuyyoSxyKj77sofx2y8m6PJYYumT4g5hREV1tfeUkl0J2DQFMbGDYEt7kxVkXCxojNvhHwTzLFv0ezstrxWWxQm81BfQQ4U9lggRXtndAP4czZnOeHPINPSiue1QNwRAEw05r5UoIUJXy/5xyUrjIxn381pAs+gJqP2COeN9kTKYH53nS/AAws29RprfZFnPlo7xxWmcjRcdS5KPdGXI/c6tQp5zl2iTh510VC1PN2w1Wvnn/oNWhiNdqPyVDsojIX5+sS3nejzJA+KFMxXSBlyXIY3wPpS/MdscU79X6Q5f9ivsFfsm7gNBmxHUPNn0HAvU4ROT/CCE9j6jSbs5PC7QBo3CK4++jxAwE/pd9HUc2rs3k0ofx3rgveJ7txpy5yPKfwIIBi98kVKlC4w7dLvNTOfjW1Imt2yH87XTfsE0UIG9st1WII6s4l/WgBx2GuwKdt6+3QUYiAlCFckkxWi+fAvpHZUEL43Qxub5fN+ZV7Zib1n7opchH4QKGBb6/y0WaDCmtCfu0lppoe/TH6saOTjDFj67NJSElK6ZDxGZ3uw4R+ret2gm6WRKT2Oeub8J33VzSa7VkmFpMPrAAfPa9N1Z4ewBLoTmvxSg2A0dDrCdJio=
+  bucket: shellcheck
+  local-dir: deploy
+  on:
+    repo: koalaman/shellcheck

README.md

@@ -25,7 +25,7 @@ There are a variety of ways to use ShellCheck!
 #### On the web
 Paste a shell script on http://www.shellcheck.net for instant feedback.
 
-[ShellCheck.net](http://www.shellcheck.net) is always synchronized to the latest git commit, and is the simplest way to give ShellCheck a go. Tell your friends!
+[ShellCheck.net](http://www.shellcheck.net) is always synchronized to the latest git commit, and is the easiest way to give ShellCheck a go. Tell your friends!
 
 
 #### From your terminal
@@ -126,13 +126,9 @@ or use OneClickInstall - https://software.opensuse.org/package/ShellCheck
 From Docker Hub:
 
     docker pull koalaman/shellcheck
+    docker run -v "$PWD:/mnt" koalaman/shellcheck myscript
 
-Using the Docker image can be done like so:
+For Windows, you can download [precompiled Windows executables](https://storage.googleapis.com/shellcheck/shellcheck-latest.zip).              
-
-    docker run -v $(pwd):/scripts koalaman/shellcheck /scripts/myscript.sh
-
-Here the local directory ( $(pwd) ) is mounted into the containers directory "/scripts". The script "myscript.sh" is checked.
-
 ## Compiling from source
 
 This section describes how to build ShellCheck from a source directory. ShellCheck is written in Haskell and requires 2GB of RAM to compile.
@@ -154,6 +150,10 @@ Verify that `cabal` is installed and update its dependency list with
 
     $ cabal install
 
+Or if you intend to run the tests:
+
+    $ cabal install --enable-tests
+
 This will compile ShellCheck and install it to your `~/.cabal/bin` directory.
 
 Add this directory to your `PATH` (for bash, add this to your `~/.bashrc`):
@@ -349,7 +349,9 @@ https://github.com/koalaman/shellcheck/issues
 
 ## Contributing
 
-Please submit patches to code or documentation as GitHub pull requests!
+Please submit patches to code or documentation as GitHub pull requests! Check
+out the [DevGuide](https://github.com/koalaman/shellcheck/wiki/DevGuide) on the
+ShellCheck Wiki.
 
 Contributions must be licensed under the GNU GPLv3.
 The contributor retains the copyright.

ShellCheck/Analytics.hs

@@ -143,7 +143,6 @@ nodeChecks = [
     ,checkWrongArithmeticAssignment
     ,checkConditionalAndOrs
     ,checkFunctionDeclarations
-    ,checkCatastrophicRm
     ,checkStderrPipe
     ,checkOverridingPath
     ,checkArrayAsString
@@ -661,15 +660,12 @@ prop_checkUnquotedDollarAt6 = verifyNot checkUnquotedDollarAt "a=$@"
 prop_checkUnquotedDollarAt7 = verify checkUnquotedDollarAt "for f in ${var[@]}; do true; done"
 prop_checkUnquotedDollarAt8 = verifyNot checkUnquotedDollarAt "echo \"${args[@]:+${args[@]}}\""
 prop_checkUnquotedDollarAt9 = verifyNot checkUnquotedDollarAt "echo ${args[@]:+\"${args[@]}\"}"
+prop_checkUnquotedDollarAt10 = verifyNot checkUnquotedDollarAt "echo ${@+\"$@\"}"
 checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not $ isStrictlyQuoteFree (parentMap p) word =
     forM_ (take 1 $ filter isArrayExpansion parts) $ \x ->
-        unless (isAlternative x) $
+        unless (isQuotedAlternativeReference x) $
             err (getId x) 2068
                 "Double quote array expansions to avoid re-splitting elements."
-  where
-    -- Fixme: should detect whether the alternative is quoted
-    isAlternative b@(T_DollarBraced _ t) = ":+" `isInfixOf` bracedString b
-    isAlternative _ = False
 checkUnquotedDollarAt _ _ = return ()
 
 prop_checkConcatenatedDollarAt1 = verify checkConcatenatedDollarAt "echo \"foo$@\""
@@ -1375,10 +1371,11 @@ prop_checkInexplicablyUnquoted4 = verify checkInexplicablyUnquoted "echo \"VALUE
 prop_checkInexplicablyUnquoted5 = verifyNot checkInexplicablyUnquoted "\"$dir\"/\"$file\""
 prop_checkInexplicablyUnquoted6 = verifyNot checkInexplicablyUnquoted "\"$dir\"some_stuff\"$file\""
 prop_checkInexplicablyUnquoted7 = verifyNot checkInexplicablyUnquoted "${dir/\"foo\"/\"bar\"}"
+prop_checkInexplicablyUnquoted8 = verifyNot checkInexplicablyUnquoted "  'foo'\\\n  'bar'"
 checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens)
   where
     check (T_SingleQuoted _ _:T_Literal id str:_)
-        | all isAlphaNum str =
+        | not (null str) && all isAlphaNum str =
         info id 2026 "This word is outside of quotes. Did you intend to 'nest '\"'single quotes'\"' instead'? "
 
     check (T_DoubleQuoted _ a:trapped:T_DoubleQuoted _ b:_) =
@@ -1527,6 +1524,7 @@ prop_subshellAssignmentCheck15 = verifyNotTree subshellAssignmentCheck "#!/bin/k
 prop_subshellAssignmentCheck16 = verifyNotTree subshellAssignmentCheck "(set -e); echo $@"
 prop_subshellAssignmentCheck17 = verifyNotTree subshellAssignmentCheck "foo=${ { bar=$(baz); } 2>&1; }; echo $foo $bar"
 prop_subshellAssignmentCheck18 = verifyTree subshellAssignmentCheck "( exec {n}>&2; ); echo $n"
+prop_subshellAssignmentCheck19 = verifyNotTree subshellAssignmentCheck "#!/bin/bash\nshopt -s lastpipe; echo a | read -r b; echo \"$b\""
 subshellAssignmentCheck params t =
     let flow = variableFlow params
         check = findSubshelled flow [("oops",[])] Map.empty
@@ -1611,6 +1609,7 @@ prop_checkSpacefulness31= verifyNotTree checkSpacefulness "echo \"`echo \\\"$1\\
 prop_checkSpacefulness32= verifyNotTree checkSpacefulness "var=$1; [ -v var ]"
 prop_checkSpacefulness33= verifyTree checkSpacefulness "for file; do echo $file; done"
 prop_checkSpacefulness34= verifyTree checkSpacefulness "declare foo$n=$1"
+prop_checkSpacefulness35= verifyNotTree checkSpacefulness "echo ${1+\"$1\"}"
 
 checkSpacefulness params t =
     doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params)
@@ -2110,7 +2109,7 @@ checkLoopKeywordScope params t |
   where
     name = getCommandName t
     path = let p = getPath (parentMap params) t in filter relevant p
-    subshellType t = case leadType (shellType params) (parentMap params) t of
+    subshellType t = case leadType params t of
         NoneScope -> Nothing
         SubshellScope str -> return str
     relevant t = isLoop t || isFunction t || isJust (subshellType t)
@@ -2139,72 +2138,6 @@ checkFunctionDeclarations params
 checkFunctionDeclarations _ _ = return ()
 
 
-prop_checkCatastrophicRm1 = verify checkCatastrophicRm "rm -r $1/$2"
-prop_checkCatastrophicRm2 = verify checkCatastrophicRm "rm -r /home/$foo"
-prop_checkCatastrophicRm3 = verifyNot checkCatastrophicRm "rm -r /home/${USER:?}/*"
-prop_checkCatastrophicRm4 = verify checkCatastrophicRm "rm -fr /home/$(whoami)/*"
-prop_checkCatastrophicRm5 = verifyNot checkCatastrophicRm "rm -r /home/${USER:-thing}/*"
-prop_checkCatastrophicRm6 = verify checkCatastrophicRm "rm --recursive /etc/*$config*"
-prop_checkCatastrophicRm8 = verify checkCatastrophicRm "rm -rf /home"
-prop_checkCatastrophicRm9 = verifyNot checkCatastrophicRm "rm -rf -- /home"
-prop_checkCatastrophicRm10= verifyNot checkCatastrophicRm "rm -r \"${DIR}\"/{.gitignore,.gitattributes,ci}"
-prop_checkCatastrophicRm11= verify checkCatastrophicRm "rm -r /{bin,sbin}/$exec"
-prop_checkCatastrophicRm12= verify checkCatastrophicRm "rm -r /{{usr,},{bin,sbin}}/$exec"
-prop_checkCatastrophicRm13= verifyNot checkCatastrophicRm "rm -r /{{a,b},{c,d}}/$exec"
-prop_checkCatastrophicRmA = verify checkCatastrophicRm "rm -rf /usr /lib/nvidia-current/xorg/xorg"
-prop_checkCatastrophicRmB = verify checkCatastrophicRm "rm -rf \"$STEAMROOT/\"*"
-checkCatastrophicRm params t@(T_SimpleCommand id _ tokens) | t `isCommand` "rm" =
-    when (any isRecursiveFlag simpleArgs) $
-        mapM_ (mapM_ checkWord . braceExpand) tokens
-  where
-    simpleArgs = oversimplify t
-
-    checkWord token =
-        case getLiteralString token of
-            Just str ->
-                when (notElem "--" simpleArgs && (fixPath str `elem` importantPaths)) $
-                    warn (getId token) 2114 "Warning: deletes a system directory. Use 'rm --' to disable this message."
-            Nothing ->
-                checkWord' token
-
-    checkWord' token = fromMaybe (return ()) $ do
-        filename <- getPotentialPath token
-        let path = fixPath filename
-        return . when (path `elem` importantPaths) $
-            warn (getId token) 2115 $ "Use \"${var:?}\" to ensure this never expands to " ++ path ++ " ."
-
-    fixPath filename =
-        let normalized = skipRepeating '/' . skipRepeating '*' $ filename in
-            if normalized == "/" then normalized else stripTrailing '/' normalized
-
-    getPotentialPath = getLiteralStringExt f
-      where
-        f (T_Glob _ str) = return str
-        f (T_DollarBraced _ word) =
-            let var = onlyLiteralString word in
-                if any (`isInfixOf` var) [":?", ":-", ":="]
-                then Nothing
-                else return ""
-        f _ = return ""
-
-    isRecursiveFlag "--recursive" = True
-    isRecursiveFlag ('-':'-':_) = False
-    isRecursiveFlag ('-':str) = 'r' `elem` str || 'R' `elem` str
-    isRecursiveFlag _ = False
-
-    stripTrailing c = reverse . dropWhile (== c) . reverse
-    skipRepeating c (a:b:rest) | a == b && b == c = skipRepeating c (b:rest)
-    skipRepeating c (a:r) = a:skipRepeating c r
-    skipRepeating _ [] = []
-
-    paths = [
-        "", "/bin", "/etc", "/home", "/mnt", "/usr", "/usr/share", "/usr/local",
-        "/var", "/lib"
-        ]
-    importantPaths = filter (not . null) $
-        ["", "/", "/*", "/*/*"] >>= (\x -> map (++x) paths)
-checkCatastrophicRm _ _ = return ()
-
 
 prop_checkStderrPipe1 = verify checkStderrPipe "#!/bin/ksh\nfoo |& bar"
 prop_checkStderrPipe2 = verifyNot checkStderrPipe "#!/bin/bash\nfoo |& bar"
@@ -2238,7 +2171,7 @@ checkUnpassedInFunctions params root =
     functions = execWriter $ doAnalysis (tell . maybeToList . findFunction) root
 
     findFunction t@(T_Function id _ _ name body) =
-        let flow = getVariableFlow (shellType params) (parentMap params) body
+        let flow = getVariableFlow params body
         in
           if any (isPositionalReference t) flow && not (any isPositionalAssignment flow)
             then return t
@@ -2541,7 +2474,9 @@ prop_checkUncheckedCd6 = verifyNotTree checkUncheckedCd "cd .."
 prop_checkUncheckedCd7 = verifyNotTree checkUncheckedCd "#!/bin/bash -e\ncd foo\nrm bar"
 prop_checkUncheckedCd8 = verifyNotTree checkUncheckedCd "set -o errexit; cd foo; rm bar"
 checkUncheckedCd params root =
-    if hasSetE root then [] else execWriter $ doAnalysis checkElement root
+    if hasSetE params
+    then []
+    else execWriter $ doAnalysis checkElement root
   where
     checkElement t@T_SimpleCommand {} =
         when(t `isUnqualifiedCommand` "cd"

ShellCheck/AnalyzerLib.hs

@@ -72,11 +72,13 @@ composeAnalyzers :: (a -> Analysis) -> (a -> Analysis) -> a -> Analysis
 composeAnalyzers f g x = f x >> g x
 
 data Parameters = Parameters {
-    variableFlow :: [StackData],
+    hasLastpipe :: Bool,           -- Whether this script has the 'lastpipe' option set/default.
-    parentMap :: Map.Map Id Token,
+    hasSetE     :: Bool,           -- Whether this script has 'set -e' anywhere.
-    shellType :: Shell,
+    variableFlow :: [StackData],   -- A linear (bad) analysis of data flow
-    shellTypeSpecified :: Bool,
+    parentMap :: Map.Map Id Token, -- A map from Id to parent Token
-    rootNode :: Token
+    shellType :: Shell,            -- The shell type, such as Bash or Ksh
+    shellTypeSpecified :: Bool,    -- True if shell type was forced via flags
+    rootNode :: Token              -- The root node of the AST
     }
 
 -- TODO: Cache results of common AST ops here
@@ -142,13 +144,48 @@ makeParameters spec =
     let params = Parameters {
         rootNode = root,
         shellType = fromMaybe (determineShell root) $ asShellType spec,
+        hasSetE = containsSetE root,
+        hasLastpipe =
+            case shellType params of
+                Bash -> containsLastpipe root
+                Dash -> False
+                Sh -> False
+                Ksh -> True,
+
         shellTypeSpecified = isJust $ asShellType spec,
         parentMap = getParentTree root,
-        variableFlow =
+        variableFlow = getVariableFlow params root
-            getVariableFlow (shellType params) (parentMap params) root
     } in params
   where root = asScript spec
 
+
+-- Does this script mention 'set -e' anywhere?
+-- Used as a hack to disable certain warnings.
+containsSetE root = isNothing $ doAnalysis (guard . not . isSetE) root
+  where
+    isSetE t =
+        case t of
+            T_Script _ str _ -> str `matches` re
+            T_SimpleCommand {}  ->
+                t `isUnqualifiedCommand` "set" &&
+                    ("errexit" `elem` oversimplify t ||
+                        "e" `elem` map snd (getAllFlags t))
+            _ -> False
+    re = mkRegex "[[:space:]]-[^-]*e"
+
+-- Does this script mention 'shopt -s lastpipe' anywhere?
+-- Also used as a hack.
+containsLastpipe root =
+        isNothing $ doAnalysis (guard . not . isShoptLastPipe) root
+    where
+        isShoptLastPipe t =
+            case t of
+                T_SimpleCommand {}  ->
+                    t `isUnqualifiedCommand` "shopt" &&
+                        ("lastpipe" `elem` oversimplify t)
+                _ -> False
+
+
 prop_determineShell0 = determineShell (fromJust $ pScript "#!/bin/sh") == Sh
 prop_determineShell1 = determineShell (fromJust $ pScript "#!/usr/bin/env ksh") == Ksh
 prop_determineShell2 = determineShell (fromJust $ pScript "") == Bash
@@ -184,8 +221,10 @@ executableFromShebang = shellFor
     shellFor s = reverse . takeWhile (/= '/') . reverse $ s
 
 
---- Context seeking
 
+-- Given a root node, make a map from Id to parent Token.
+-- This is used to populate parentMap in Parameters
+getParentTree :: Token -> Map.Map Id Token
 getParentTree t =
     snd . snd $ runState (doStackAnalysis pre post t) ([], Map.empty)
   where
@@ -195,18 +234,24 @@ getParentTree t =
         case rest of [] -> put (rest, map)
                      (x:_) -> put (rest, Map.insert (getId t) x map)
 
+-- Given a root node, make a map from Id to Token
+getTokenMap :: Token -> Map.Map Id Token
 getTokenMap t =
     execState (doAnalysis f t) Map.empty
   where
     f t = modify (Map.insert (getId t) t)
 
 
--- Is this node self quoting for a regular element?
+-- Is this token in a quoting free context? (i.e. would variable expansion split)
-isQuoteFree = isQuoteFreeNode False
+-- True:  Assignments, [[ .. ]], here docs, already in double quotes
-
+-- False: Regular words
--- Is this node striclty self quoting, for array expansions
 isStrictlyQuoteFree = isQuoteFreeNode True
 
+-- Like above, but also allow some cases where splitting may be desired.
+-- True:  Like above + for loops
+-- False: Like above
+isQuoteFree = isQuoteFreeNode False
+
 
 isQuoteFreeNode strict tree t =
     (isQuoteFreeElement t == Just True) ||
@@ -239,6 +284,9 @@ isQuoteFreeNode strict tree t =
             T_SelectIn {} -> return (not strict)
             _ -> Nothing
 
+-- Check if a token is a parameter to a certain command by name:
+-- Example: isParamTo (parentMap params) "sed" t
+isParamTo :: Map.Map Id Token -> String -> Token -> Bool
 isParamTo tree cmd =
     go
   where
@@ -254,16 +302,23 @@ isParamTo tree cmd =
             T_Redirecting {} -> isCommand t cmd
             _ -> False
 
+-- Get the parent command (T_Redirecting) of a Token, if any.
+getClosestCommand :: Map.Map Id Token -> Token -> Maybe Token
 getClosestCommand tree t =
-    msum . map getCommand $ getPath tree t
+    findFirst findCommand $ getPath tree t
   where
-    getCommand t@T_Redirecting {} = return t
+    findCommand t =
-    getCommand _ = Nothing
+        case t of
+            T_Redirecting {} -> return True
+            T_Script {} -> return False
+            _ -> Nothing
 
+-- Like above, if koala_man knew Haskell when starting this project.
 getClosestCommandM t = do
     tree <- asks parentMap
     return $ getClosestCommand tree t
 
+-- Is the token used as a command name (the first word in a T_SimpleCommand)?
 usedAsCommandName tree token = go (getId token) (tail $ getPath tree token)
   where
     go currentId (T_NormalWord id [word]:rest)
@@ -274,7 +329,7 @@ usedAsCommandName tree token = go (getId token) (tail $ getPath tree token)
         | currentId == getId word = True
     go _ _ = False
 
--- A list of the element and all its parents
+-- A list of the element and all its parents up to the root node.
 getPath tree t = t :
     case Map.lookup (getId t) tree of
         Nothing -> []
@@ -295,6 +350,18 @@ pathTo t = do
     parents <- reader parentMap
     return $ getPath parents t
 
+-- Find the first match in a list where the predicate is Just True.
+-- Stops if it's Just False and ignores Nothing.
+findFirst :: (a -> Maybe Bool) -> [a] -> Maybe a
+findFirst p l =
+    case l of
+        [] -> Nothing
+        (x:xs) ->
+            case p x of
+                Just True -> return x
+                Just False -> Nothing
+                Nothing -> findFirst p xs
+
 -- Check whether a word is entirely output from a single command
 tokenIsJustCommandOutput t = case t of
     T_NormalWord id [T_DollarExpansion _ cmds] -> check cmds
@@ -307,18 +374,18 @@ tokenIsJustCommandOutput t = case t of
     check _ = False
 
 -- TODO: Replace this with a proper Control Flow Graph
-getVariableFlow shell parents t =
+getVariableFlow params t =
     let (_, stack) = runState (doStackAnalysis startScope endScope t) []
     in reverse stack
   where
     startScope t =
-        let scopeType = leadType shell parents t
+        let scopeType = leadType params t
         in do
             when (scopeType /= NoneScope) $ modify (StackScope scopeType:)
             when (assignFirst t) $ setWritten t
 
     endScope t =
-        let scopeType = leadType shell parents t
+        let scopeType = leadType params t
         in do
             setRead t
             unless (assignFirst t) $ setWritten t
@@ -329,7 +396,7 @@ getVariableFlow shell parents t =
     assignFirst _ = False
 
     setRead t =
-        let read    = getReferencedVariables parents t
+        let read    = getReferencedVariables (parentMap params) t
         in mapM_ (\v -> modify (Reference v:)) read
 
     setWritten t =
@@ -337,7 +404,7 @@ getVariableFlow shell parents t =
         in mapM_ (\v -> modify (Assignment v:)) written
 
 
-leadType shell parents t =
+leadType params t =
     case t of
         T_DollarExpansion _ _  -> SubshellScope "$(..) expansion"
         T_Backticked _ _  -> SubshellScope "`..` expansion"
@@ -351,7 +418,7 @@ leadType shell parents t =
         _ -> NoneScope
   where
     parentPipeline = do
-        parent <- Map.lookup (getId t) parents
+        parent <- Map.lookup (getId t) (parentMap params)
         case parent of
             T_Pipeline {} -> return parent
             _ -> Nothing
@@ -360,17 +427,10 @@ leadType shell parents t =
         (T_Pipeline _ _ list) <- parentPipeline
         if length list <= 1
             then return False
-            else if lastCreatesSubshell
+            else if not $ hasLastpipe params
                 then return True
                 else return . not $ (getId . head $ reverse list) == getId t
 
-    lastCreatesSubshell =
-        case shell of
-            Bash -> True
-            Dash -> True
-            Sh -> True
-            Ksh -> False
-
 getModifiedVariables t =
     case t of
         T_SimpleCommand _ vars [] ->
@@ -623,13 +683,20 @@ dataTypeFrom defaultType v = (case v of T_Array {} -> DataArray; _ -> defaultTyp
 
 --- Command specific checks
 
+-- Compare a command to a string: t `isCommand` "sed" (also matches /usr/bin/sed)
 isCommand token str = isCommandMatch token (\cmd -> cmd  == str || ('/' : str) `isSuffixOf` cmd)
+
+-- Compare a command to a literal. Like above, but checks full path.
 isUnqualifiedCommand token str = isCommandMatch token (== str)
 
 isCommandMatch token matcher = fromMaybe False $ do
     cmd <- getCommandName token
     return $ matcher cmd
 
+-- Does this regex look like it was intended as a glob?
+-- True:  *foo*
+-- False: .*foo.*
+isConfusedGlobRegex :: String -> Bool
 isConfusedGlobRegex ('*':_) = True
 isConfusedGlobRegex [x,'*'] | x /= '\\' = True
 isConfusedGlobRegex _ = False
@@ -656,6 +723,7 @@ getVariablesFromLiteral string =
   where
     variableRegex = mkRegex "\\$\\{?([A-Za-z0-9_]+)"
 
+-- Get the variable name from an expansion like ${var:-foo}
 prop_getBracedReference1 = getBracedReference "foo" == "foo"
 prop_getBracedReference2 = getBracedReference "#foo" == "foo"
 prop_getBracedReference3 = getBracedReference "#" == "#"
@@ -706,13 +774,22 @@ getBracedModifier s = fromMaybe "" . listToMaybe $ do
     dropModifier (c:rest) | c `elem` "#!" = [rest, c:rest]
     dropModifier x = [x]
 
--- Useful generic functions
+-- Useful generic functions.
+
+-- Run an action in a Maybe (or do nothing).
+-- Example:
+-- potentially $ do
+--   s <- getLiteralString cmd
+--   guard $ s `elem` ["--recursive", "-r"]
+--   return $ warn .. "Something something recursive"
 potentially :: Monad m => Maybe (m ()) -> m ()
 potentially = fromMaybe (return ())
 
+-- Get element 0 or a default. Like `head` but safe.
 headOrDefault _ (a:_) = a
 headOrDefault def _ = def
 
+--- Get element n of a list, or Nothing. Like `!!` but safe.
 (!!!) list i =
     case drop i list of
         [] -> Nothing
@@ -752,8 +829,10 @@ isCountingReference _ = False
 isQuotedAlternativeReference t =
     case t of
         T_DollarBraced _ _ ->
-            ":+" `isInfixOf` bracedString t
+            getBracedModifier (bracedString t) `matches` re
         _ -> False
+  where
+    re = mkRegex "(^|\\]):?\\+"
 
 
 

ShellCheck/Checks/Commands.hs

@@ -38,7 +38,7 @@ import Control.Monad.RWS
 import Data.Char
 import Data.List
 import Data.Maybe
-import qualified Data.Map as Map
+import qualified Data.Map.Strict as Map
 import Test.QuickCheck.All (forAllProperties)
 import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess)
 
@@ -85,13 +85,15 @@ commandChecks = [
     ,checkDeprecatedTempfile
     ,checkDeprecatedEgrep
     ,checkDeprecatedFgrep
+    ,checkWhileGetoptsCase
+    ,checkCatastrophicRm
     ]
 
 buildCommandMap :: [CommandCheck] -> Map.Map CommandName (Token -> Analysis)
 buildCommandMap = foldl' addCheck Map.empty
   where
     addCheck map (CommandCheck name function) =
-        Map.insertWith' composeAnalyzers name function map
+        Map.insertWith composeAnalyzers name function map
 
 
 checkCommand :: Map.Map CommandName (Token -> Analysis) -> Token -> Analysis
@@ -385,17 +387,26 @@ prop_checkMkdirDashPM11 = verifyNot checkMkdirDashPM "mkdir --parents a/b"
 prop_checkMkdirDashPM12 = verifyNot checkMkdirDashPM "mkdir --mode=0755 a/b"
 prop_checkMkdirDashPM13 = verifyNot checkMkdirDashPM "mkdir_func -pm 0755 a/b"
 prop_checkMkdirDashPM14 = verifyNot checkMkdirDashPM "mkdir -p -m 0755 singlelevel"
+prop_checkMkdirDashPM15 = verifyNot checkMkdirDashPM "mkdir -p -m 0755 ../bin"
+prop_checkMkdirDashPM16 = verify checkMkdirDashPM "mkdir -p -m 0755 ../bin/laden"
+prop_checkMkdirDashPM17 = verifyNot checkMkdirDashPM "mkdir -p -m 0755 ./bin"
+prop_checkMkdirDashPM18 = verify checkMkdirDashPM "mkdir -p -m 0755 ./bin/laden"
+prop_checkMkdirDashPM19 = verifyNot checkMkdirDashPM "mkdir -p -m 0755 ./../bin"
+prop_checkMkdirDashPM20 = verifyNot checkMkdirDashPM "mkdir -p -m 0755 .././bin"
+prop_checkMkdirDashPM21 = verifyNot checkMkdirDashPM "mkdir -p -m 0755 ../../bin"
 checkMkdirDashPM = CommandCheck (Basename "mkdir") check
   where
     check t = potentially $ do
         let flags = getAllFlags t
         dashP <- find ((\f -> f == "p" || f == "parents") . snd) flags
         dashM <- find ((\f -> f == "m" || f == "mode") . snd) flags
-        guard $ any couldHaveSubdirs (drop 1 $ arguments t) -- mkdir -pm 0700 dir  is fine, but dir/subdir is not.
+        -- mkdir -pm 0700 dir  is fine, so is ../dir, but dir/subdir is not.
+        guard $ any couldHaveSubdirs (drop 1 $ arguments t)
         return $ warn (getId $ fst dashM) 2174 "When used with -p, -m only applies to the deepest directory."
     couldHaveSubdirs t = fromMaybe True $ do
         name <- getLiteralString t
-        return $ '/' `elem` name
+        return $ '/' `elem` name && not (name `matches` re)
+    re = mkRegex "^(\\.\\.?\\/)+[^/]+$"
 
 
 prop_checkNonportableSignals1 = verify checkNonportableSignals "trap f 8"
@@ -681,5 +692,130 @@ prop_checkDeprecatedFgrep = verify checkDeprecatedFgrep "fgrep '*' files"
 checkDeprecatedFgrep = CommandCheck (Basename "fgrep") $
     \t -> info (getId t) 2197 "fgrep is non-standard and deprecated. Use grep -F instead."
 
+prop_checkWhileGetoptsCase1 = verify checkWhileGetoptsCase "while getopts 'a:b' x; do case $x in a) foo;; esac; done"
+prop_checkWhileGetoptsCase2 = verify checkWhileGetoptsCase "while getopts 'a:' x; do case $x in a) foo;; b) bar;; esac; done"
+prop_checkWhileGetoptsCase3 = verifyNot checkWhileGetoptsCase "while getopts 'a:b' x; do case $x in a) foo;; b) bar;; esac; done"
+prop_checkWhileGetoptsCase4 = verifyNot checkWhileGetoptsCase "while getopts 'a:123' x; do case $x in a) foo;; [0-9]) bar;; esac; done"
+prop_checkWhileGetoptsCase5 = verifyNot checkWhileGetoptsCase "while getopts 'a:' x; do case $x in a) foo;; \\?) bar;; *) baz;; esac; done"
+checkWhileGetoptsCase = CommandCheck (Exactly "getopts") f
+  where
+    f :: Token -> Analysis
+    f t@(T_SimpleCommand _ _ (cmd:arg1:_))  = do
+        path <- getPathM t
+        potentially $ do
+            options <- getLiteralString arg1
+            (T_WhileExpression _ _ body) <- findFirst whileLoop path
+            caseCmd <- mapMaybe findCase body !!! 0
+            return $ check (getId arg1) (map (:[]) $ filter (/= ':') options) caseCmd
+    f _ = return ()
+
+    check :: Id -> [String] -> Token -> Analysis
+    check optId opts (T_CaseExpression id _ list) = do
+            unless (Nothing `Map.member` handledMap) $
+                mapM_ (warnUnhandled optId id) $ catMaybes $ Map.keys notHandled
+
+            mapM_ warnRedundant $ Map.toList notRequested
+
+        where
+            handledMap = Map.fromList (concatMap getHandledStrings list)
+            requestedMap = Map.fromList $ map (\x -> (Just x, ())) opts
+
+            notHandled = Map.difference requestedMap handledMap
+            notRequested = Map.difference handledMap requestedMap
+
+    warnUnhandled optId caseId str =
+        warn caseId 2213 $ "getopts specified -" ++ str ++ ", but it's not handled by this 'case'."
+
+    warnRedundant (key, expr) = potentially $ do
+        str <- key
+        guard $ str `notElem` ["*", ":", "?"]
+        return $ warn (getId expr) 2214 "This case is not specified by getopts."
+
+    getHandledStrings (_, globs, _) =
+        map (\x -> (literal x, x)) globs
+
+    literal :: Token -> Maybe String
+    literal t = do
+        getLiteralString t <> fromGlob t
+
+    fromGlob t =
+        case t of
+            T_Glob _ ('[':c:']':[]) -> return [c]
+            T_Glob _ "*" -> return "*"
+            _ -> Nothing
+
+    whileLoop t =
+        case t of
+            T_WhileExpression {} -> return True
+            T_Script {} -> return False
+            _ -> Nothing
+
+    findCase t =
+        case t of
+            T_Annotation _ _ x -> findCase x
+            T_Pipeline _ _ [x] -> findCase x
+            T_Redirecting _ _ x@(T_CaseExpression {}) -> return x
+            _ -> Nothing
+
+prop_checkCatastrophicRm1 = verify checkCatastrophicRm "rm -r $1/$2"
+prop_checkCatastrophicRm2 = verify checkCatastrophicRm "rm -r /home/$foo"
+prop_checkCatastrophicRm3 = verifyNot checkCatastrophicRm "rm -r /home/${USER:?}/*"
+prop_checkCatastrophicRm4 = verify checkCatastrophicRm "rm -fr /home/$(whoami)/*"
+prop_checkCatastrophicRm5 = verifyNot checkCatastrophicRm "rm -r /home/${USER:-thing}/*"
+prop_checkCatastrophicRm6 = verify checkCatastrophicRm "rm --recursive /etc/*$config*"
+prop_checkCatastrophicRm8 = verify checkCatastrophicRm "rm -rf /home"
+prop_checkCatastrophicRm10= verifyNot checkCatastrophicRm "rm -r \"${DIR}\"/{.gitignore,.gitattributes,ci}"
+prop_checkCatastrophicRm11= verify checkCatastrophicRm "rm -r /{bin,sbin}/$exec"
+prop_checkCatastrophicRm12= verify checkCatastrophicRm "rm -r /{{usr,},{bin,sbin}}/$exec"
+prop_checkCatastrophicRm13= verifyNot checkCatastrophicRm "rm -r /{{a,b},{c,d}}/$exec"
+prop_checkCatastrophicRmA = verify checkCatastrophicRm "rm -rf /usr /lib/nvidia-current/xorg/xorg"
+prop_checkCatastrophicRmB = verify checkCatastrophicRm "rm -rf \"$STEAMROOT/\"*"
+checkCatastrophicRm = CommandCheck (Basename "rm") $ \t ->
+    when (isRecursive t) $
+        mapM_ (mapM_ checkWord . braceExpand) $ arguments t
+  where
+    isRecursive = any (`elem` ["r", "R", "recursive"]) . map snd . getAllFlags
+
+    checkWord token =
+        case getLiteralString token of
+            Just str ->
+                when (fixPath str `elem` importantPaths) $
+                    warn (getId token) 2114 "Warning: deletes a system directory."
+            Nothing ->
+                checkWord' token
+
+    checkWord' token = fromMaybe (return ()) $ do
+        filename <- getPotentialPath token
+        let path = fixPath filename
+        return . when (path `elem` importantPaths) $
+            warn (getId token) 2115 $ "Use \"${var:?}\" to ensure this never expands to " ++ path ++ " ."
+
+    fixPath filename =
+        let normalized = skipRepeating '/' . skipRepeating '*' $ filename in
+            if normalized == "/" then normalized else stripTrailing '/' normalized
+
+    getPotentialPath = getLiteralStringExt f
+      where
+        f (T_Glob _ str) = return str
+        f (T_DollarBraced _ word) =
+            let var = onlyLiteralString word in
+                -- This shouldn't handle non-colon cases.
+                if any (`isInfixOf` var) [":?", ":-", ":="]
+                then Nothing
+                else return ""
+        f _ = return ""
+
+    stripTrailing c = reverse . dropWhile (== c) . reverse
+    skipRepeating c (a:b:rest) | a == b && b == c = skipRepeating c (b:rest)
+    skipRepeating c (a:r) = a:skipRepeating c r
+    skipRepeating _ [] = []
+
+    paths = [
+        "", "/bin", "/etc", "/home", "/mnt", "/usr", "/usr/share", "/usr/local",
+        "/var", "/lib", "/dev", "/media", "/boot", "/lib64", "/usr/bin"
+        ]
+    importantPaths = filter (not . null) $
+        ["", "/", "/*", "/*/*"] >>= (\x -> map (++x) paths)
+
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])

ShellCheck/Parser.hs

@@ -440,19 +440,9 @@ readConditionContents single =
 
         getOp = do
             id <- getNextId
-            op <- anyQuotedOp <|> anyEscapedOp <|> anyOp
+            op <- readRegularOrEscaped anyOp
             return $ TC_Binary id typ op
 
-        -- hacks to read quoted operators without having to read a shell word
-        anyEscapedOp = try $ do
-            char '\\'
-            escaped <$> anyOp
-        anyQuotedOp = try $ do
-            c <- oneOf "'\""
-            s <- anyOp
-            char c
-            return $ escaped s
-
         anyOp = flagOp <|> flaglessOp <|> fail
                     "Expected comparison operator (don't wrap commands in []/[[]])"
         flagOp = try $ do
@@ -461,7 +451,22 @@ readConditionContents single =
             return s
         flaglessOp =
             choice $ map (try . string) flaglessOps
-        escaped s = if any (`elem` s) "<>" then '\\':s else s
+
+        -- hacks to read quoted operators without having to read a shell word
+    readEscaped p = try $ withEscape <|> withQuotes
+      where
+        withEscape = do
+            char '\\'
+            escaped <$> p
+        withQuotes = do
+            c <- oneOf "'\""
+            s <- p
+            char c
+            return $ escaped s
+        escaped s = if any (`elem` s) "<>()" then '\\':s else s
+
+    readRegularOrEscaped p = readEscaped p <|> p
+
 
     guardArithmetic = do
         try . lookAhead $ disregard (oneOf "+*/%") <|> disregard (string "- ")
@@ -560,29 +565,30 @@ readConditionContents single =
             "You need a space before and after the " ++ trailingOp ++ " ."
 
     readCondGroup = do
-          id <- getNextId
+        id <- getNextId
-          pos <- getPosition
+        pos <- getPosition
-          lparen <- try $ string "(" <|> string "\\("
+        lparen <- try $ readRegularOrEscaped (string "(")
-          when (single && lparen == "(") $
+        when (single && lparen == "(") $
-              parseProblemAt pos ErrorC 1028 "In [..] you have to escape (). Use [[..]] instead."
+            singleWarning pos
-          when (not single && lparen == "\\(") $
+        when (not single && lparen == "\\(") $
-              parseProblemAt pos ErrorC 1029 "In [[..]] you shouldn't escape ()."
+            doubleWarning pos
-          condSpacing single
+        condSpacing single
-          x <- readCondContents
+        x <- readCondContents
-          cpos <- getPosition
+        cpos <- getPosition
-          rparen <- string ")" <|> string "\\)"
+        rparen <- readRegularOrEscaped (string ")")
-          condSpacing single
+        condSpacing single
-          when (single && rparen == ")") $
+        when (single && rparen == ")") $
-              parseProblemAt cpos ErrorC 1030 "In [..] you have to escape (). Use [[..]] instead."
+            singleWarning cpos
-          when (not single && rparen == "\\)") $
+        when (not single && rparen == "\\)") $
-              parseProblemAt cpos ErrorC 1031 "In [[..]] you shouldn't escape ()."
+            doubleWarning cpos
-          when (isEscaped lparen `xor` isEscaped rparen) $
+        return $ TC_Group id typ x
-              parseProblemAt pos ErrorC 1032 "Did you just escape one half of () but not the other?"
+
-          return $ TC_Group id typ x
       where
-        isEscaped ('\\':_) = True
+        singleWarning pos =
-        isEscaped _ = False
+            parseProblemAt pos ErrorC 1028 "In [..] you have to escape \\( \\) or preferably combine [..] expressions."
-        xor x y = x && not y || not x && y
+        doubleWarning pos =
+            parseProblemAt pos ErrorC 1029 "In [[..]] you shouldn't escape ( or )."
+
 
     -- Currently a bit of a hack since parsing rules are obscure
     regexOperatorAhead = lookAhead (do
@@ -849,6 +855,7 @@ prop_readCondition15= isOk readCondition "[ foo \">=\" bar ]"
 prop_readCondition16= isOk readCondition "[ foo \\< bar ]"
 prop_readCondition17= isOk readCondition "[[ ${file::1} = [-.\\|/\\\\] ]]"
 prop_readCondition18= isOk readCondition "[ ]"
+prop_readCondition19= isOk readCondition "[ '(' x \")\" ]"
 readCondition = called "test expression" $ do
     opos <- getPosition
     id <- getNextId
@@ -889,10 +896,13 @@ prop_readAnnotation1 = isOk readAnnotation "# shellcheck disable=1234,5678\n"
 prop_readAnnotation2 = isOk readAnnotation "# shellcheck disable=SC1234 disable=SC5678\n"
 prop_readAnnotation3 = isOk readAnnotation "# shellcheck disable=SC1234 source=/dev/null disable=SC5678\n"
 prop_readAnnotation4 = isWarning readAnnotation "# shellcheck cats=dogs disable=SC1234\n"
+prop_readAnnotation5 = isOk readAnnotation "# shellcheck disable=SC2002 # All cats are precious\n"
+prop_readAnnotation6 = isOk readAnnotation "# shellcheck disable=SC1234 # shellcheck foo=bar\n"
 readAnnotation = called "shellcheck annotation" $ do
     try readAnnotationPrefix
     many1 linewhitespace
     values <- many1 (readDisable <|> readSourceOverride <|> readShellOverride <|> anyKey)
+    optional readAnyComment
     linefeed
     many linewhitespace
     return $ concat values
@@ -926,7 +936,8 @@ readAnnotation = called "shellcheck annotation" $ do
 
     anyKey = do
         pos <- getPosition
-        anyChar `reluctantlyTill1` whitespace
+        noneOf "#\r\n"
+        anyChar `reluctantlyTill` whitespace
         many linewhitespace
         parseNoteAt pos WarningC 1107 "This directive is unknown. It will be ignored."
         return []
@@ -937,6 +948,9 @@ readAnnotations = do
 
 readComment = do
     unexpecting "shellcheck annotation" readAnnotationPrefix
+    readAnyComment
+
+readAnyComment = do
     char '#'
     many $ noneOf "\r\n"
 
@@ -2729,14 +2743,18 @@ readScript = do
     script <- readScriptFile
     reparseIndices script
 
-isWarning p s = parsesCleanly p s == Just False
-isOk p s =      parsesCleanly p s == Just True
-isNotOk p s =   parsesCleanly p s == Nothing
 
-testParse string = runIdentity $ do
+-- Interactively run a parser in ghci:
-    (res, _) <- runParser (mockedSystemInterface []) readScript "-" string
+-- debugParse readScript "echo 'hello world'"
+debugParse p string = runIdentity $ do
+    (res, _) <- runParser (mockedSystemInterface []) p "-" string
     return res
 
+
+isOk p s =      parsesCleanly p s == Just True   -- The string parses with no warnings
+isWarning p s = parsesCleanly p s == Just False  -- The string parses with warnings
+isNotOk p s =   parsesCleanly p s == Nothing     -- The string does not parse
+
 parsesCleanly parser string = runIdentity $ do
     (res, sys) <- runParser (mockedSystemInterface [])
                     (parser >> eof >> getState) "-" string
@@ -2745,6 +2763,16 @@ parsesCleanly parser string = runIdentity $ do
             return $ Just . null $ parseNotes userState ++ parseProblems systemState
         (Left _, _) -> return Nothing
 
+-- For printf debugging: print the value of an expression
+-- Example: return $ dump $ T_Literal id [c]
+dump :: Show a => a -> a
+dump x = trace (show x) x
+
+-- Like above, but print a specific expression:
+-- Example: return $ dumps ("Returning: " ++ [c])  $ T_Literal id [c]
+dumps :: Show x => x -> a -> a
+dumps t = trace (show t)
+
 parseWithNotes parser = do
     item <- parser
     state <- getState
@@ -2877,9 +2905,6 @@ parseScript sys spec =
     parseShell sys (psFilename spec) (psScript spec)
 
 
-lt x = trace (show x) x
-ltt t = trace (show t)
-
 return []
 runTests = $quickCheckAll