Stable version 0.3.3

This release is dedicated to Jarkko Oikarinen, creator of IRC,
the fabric of the author's existence for so many years.

Makefile

@@ -1,6 +1,7 @@
 # TODO: Phase out Makefile in favor of Cabal
 
 GHCFLAGS=-O9
+GHCFLAGS_STATIC=$(GHCFLAGS) -optl-static -optl-pthread
 
 all: shellcheck .tests shellcheck.1
 	: Done
@@ -14,6 +15,7 @@ shellcheck: regardless
 	./test/runQuack && touch .tests
 
 shellcheck.1: shellcheck.1.md
+	: Formatting man page
 	pandoc -s -t man $< -o $@
 
 clean:
@@ -21,4 +23,8 @@ clean:
 	rm -f *.hi *.o ShellCheck/*.hi ShellCheck/*.o
 	rm -rf dist
 
+shellcheck-static: regardless
+	: Conditionally compiling a statically linked shellcheck-static
+	ghc $(GHCFLAGS_STATIC) --make shellcheck -o shellcheck-static
+
 regardless:

README.md

@@ -37,14 +37,26 @@ On Mac OS X with MacPorts (http://www.macports.org/):
 
     port install hs-cabal-install
 
-With cabal installed, cd to the shellcheck source directory and:
+Let cabal update itself, in case your distro version is outdated:
+
+    $ cabal update
+    $ cabal install cabal-install
+
+With cabal installed, cd to the ShellCheck source directory and:
 
     $ cabal install
-    ...
+    
+This will install ShellCheck to your ~/.cabal/bin directory.
+
+Add the directory to your PATH (for bash, add this to your ~/.bashrc file):
+
+    export PATH=$HOME/.cabal/bin:$PATH
+
+Verify that your PATH is set up correctly:    
+
     $ which shellcheck
     ~/.cabal/bin/shellcheck
 
-
 ## Building with Make
 
 ShellCheck is written in Haskell, and requires GHC, Parsec3, JSON and
@@ -53,16 +65,19 @@ Text.Regex. To run the unit tests, it also requires QuickCheck2.
 On Fedora, these can be installed with:
 
     yum install ghc ghc-parsec-devel ghc-QuickCheck-devel \
-      ghc-json-devel ghc-regex-compat-devel
+      ghc-json-devel ghc-regex-compat-devel pandoc
 
 On Ubuntu and similar, use:
 
     apt-get install ghc libghc-parsec3-dev libghc-json-dev \
-      libghc-regex-compat-dev libghc-quickcheck2-dev
+      libghc-regex-compat-dev libghc-quickcheck2-dev pandoc
 
 To build and run the tests, cd to the shellcheck source directory and:
 
     $ make
 
+If you want to distribute the binary and/or run it on other distros, you
+can `make shellcheck-static` to build a statically linked executable without
+library dependencies.
 
 Happy ShellChecking!

ShellCheck.cabal

@@ -1,6 +1,6 @@
 Name:             ShellCheck
 -- Must also be updated in ShellCheck/Data.hs :
-Version:          0.3.2
+Version:          0.3.3
 Synopsis:         Shell script analysis tool
 License:          OtherLicense
 License-file:     LICENSE

ShellCheck/AST.hs

@@ -28,6 +28,7 @@ data Dashed = Dashed | Undashed deriving (Show, Eq)
 data AssignmentMode = Assign | Append deriving (Show, Eq)
 data FunctionKeyword = FunctionKeyword Bool deriving (Show, Eq)
 data FunctionParentheses = FunctionParentheses Bool deriving (Show, Eq)
+data ForInType = NormalForIn | ShortForIn deriving (Show, Eq)
 
 data Token =
     TA_Base Id String Token
@@ -81,7 +82,7 @@ data Token =
     | T_Fi Id
     | T_For Id
     | T_ForArithmetic Id Token Token Token [Token]
-    | T_ForIn Id String [Token] [Token]
+    | T_ForIn Id ForInType [String] [Token] [Token]
     | T_Function Id FunctionKeyword FunctionParentheses String Token
     | T_GREATAND Id
     | T_Glob Id String
@@ -202,7 +203,7 @@ analyze f g i =
     delve (T_BraceGroup id l) = dl l $ T_BraceGroup id
     delve (T_WhileExpression id c l) = dll c l $ T_WhileExpression id
     delve (T_UntilExpression id c l) = dll c l $ T_UntilExpression id
-    delve (T_ForIn id v w l) = dll w l $ T_ForIn id v
+    delve (T_ForIn id t v w l) = dll w l $ T_ForIn id t v
     delve (T_SelectIn id v w l) = dll w l $ T_SelectIn id v
     delve (T_CaseExpression id word cases) = do
         newWord <- round word
@@ -308,7 +309,7 @@ getId t = case t of
         T_BraceGroup id _  -> id
         T_WhileExpression id _ _  -> id
         T_UntilExpression id _ _  -> id
-        T_ForIn id _ _ _  -> id
+        T_ForIn id _ _ _ _  -> id
         T_SelectIn id _ _ _  -> id
         T_CaseExpression id _ _ -> id
         T_Function id _ _ _ _  -> id

ShellCheck/Analytics.hs

@@ -55,6 +55,7 @@ treeChecks = [
     ,checkFunctionsUsedExternally
     ,checkUnusedAssignments
     ,checkUnpassedInFunctions
+    ,checkArrayWithoutIndex
     ]
 
 checksFor Sh = [
@@ -120,6 +121,8 @@ shellForExecutable _ = Nothing
 
 -- Checks that are run on each node in the AST
 runNodeAnalysis f p t = execWriter (doAnalysis (f p) t)
+
+nodeChecks :: [Parameters -> Token -> Writer [Note] ()]
 nodeChecks = [
     checkUuoc
     ,checkPipePitfalls
@@ -189,6 +192,15 @@ nodeChecks = [
     ,checkInteractiveSu
     ,checkStderrPipe
     ,checkSetAssignment
+    ,checkOverridingPath
+    ,checkArrayAsString
+    ,checkUnsupported
+    ,checkMultipleAppends
+    ,checkAliasesExpandEarly
+    ,checkSuspiciousIFS
+    ,checkAliasesUsesArgs
+    ,checkShouldUseGrepQ
+    ,checkTestGlobs
     ]
 
 
@@ -224,6 +236,8 @@ prop_isVariableName3 = not $ isVariableName "test: "
 isVariableName (x:r) = isVariableStartChar x && all isVariableChar r
 isVariableName _ = False
 
+potentially = fromMaybe (return ())
+
 matchAll re = unfoldr f
   where
     f str = do
@@ -252,9 +266,17 @@ isConfusedGlobRegex ('*':_) = True
 isConfusedGlobRegex [x,'*'] | x /= '\\' = True
 isConfusedGlobRegex _ = False
 
-isPotentiallyConfusedGlobRegex =
+getSuspiciousRegexWildcard str =
-    let re = mkRegex "[a-z1-9]\\*" in
+    if (not $ str `matches` contra)
-        isJust . matchRegex re
+    then do
+        match <- matchRegex suspicious str
+        str <- match !!! 0
+        str !!! 0
+    else
+        fail "looks good"
+  where
+    suspicious = mkRegex "([A-Za-z1-9])\\*"
+    contra = mkRegex "[^a-zA-Z1-9]\\*|[][^$+\\\\]"
 
 matches string regex = isJust $ matchRegex regex string
 
@@ -315,9 +337,16 @@ getFlags _ = []
         [] -> Nothing
         (r:_) -> Just r
 
+verify :: (Parameters -> Token -> Writer [a] ()) -> String -> Bool
 verify f s = checkNode f s == Just True
+
+verifyNot :: (Parameters -> Token -> Writer [a] ()) -> String -> Bool
 verifyNot f s = checkNode f s == Just False
+
+verifyTree :: (Parameters -> Token -> [a]) -> String -> Bool
 verifyTree f s = checkTree f s == Just True
+
+verifyNotTree :: (Parameters -> Token -> [a]) -> String -> Bool
 verifyNotTree f s = checkTree f s == Just False
 
 checkNode f = checkTree (runNodeAnalysis f)
@@ -439,9 +468,14 @@ checkUuoc _ _ = return ()
 prop_checkNeedlessCommands = verify checkNeedlessCommands "foo=$(expr 3 + 2)"
 prop_checkNeedlessCommands2 = verify checkNeedlessCommands "foo=`echo \\`expr 3 + 2\\``"
 prop_checkNeedlessCommands3 = verifyNot checkNeedlessCommands "foo=$(expr foo : regex)"
-checkNeedlessCommands _ cmd@(T_SimpleCommand id _ _) |
+prop_checkNeedlessCommands4 = verifyNot checkNeedlessCommands "foo=$(expr foo \\< regex)"
-        cmd `isCommand` "expr" && (not $ ":" `elem` deadSimple cmd) =
+checkNeedlessCommands _ cmd@(T_SimpleCommand id _ args) |
+        cmd `isCommand` "expr" && (not $ any (`elem` words) exceptions) =
     style id 2003 "expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]]."
+  where
+    -- These operators are hard to replicate in POSIX
+    exceptions = [ ":", "<", ">", "<=", ">=" ]
+    words = mapMaybe getLiteralString args
 checkNeedlessCommands _ _ = return ()
 
 prop_checkPipePitfalls3 = verify checkPipePitfalls "ls | grep -v mp3"
@@ -460,6 +494,9 @@ checkPipePitfalls _ (T_Pipeline id _ commands) = do
     for' ["ps", "grep"] $
         \x -> info x 2009 "Consider using pgrep instead of grepping ps output."
 
+    for' ["grep", "wc"] $
+        \x -> style x 2126 "Consider using grep -c instead of grep|wc."
+
     didLs <- liftM or . sequence $ [
         for' ["ls", "grep"] $
             \x -> warn x 2010 "Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.",
@@ -498,10 +535,37 @@ indexOfSublists sub all = f 0 all
 
 
 bracedString l = concat $ deadSimple l
-isMagicInQuotes (T_DollarBraced _ l) =
+
+isArrayExpansion (T_DollarBraced _ l) =
     let string = bracedString l in
-        '@' `elem` string || "!" `isPrefixOf` string
+        "@" `isPrefixOf` string ||
-isMagicInQuotes _ = False
+            not ("#" `isPrefixOf` string) && "[@]" `isInfixOf` string
+isArrayExpansion _ = False
+
+-- Is it certain that this arg will becomes multiple args?
+willBecomeMultipleArgs t = willConcatInAssignment t || f t
+  where
+    f (T_Extglob {}) = True
+    f (T_Glob {}) = True
+    f (T_BraceExpansion {}) = True
+    f (T_DoubleQuoted _ parts) = any f parts
+    f (T_NormalWord _ parts) = any f parts
+    f _ = False
+
+willConcatInAssignment t@(T_DollarBraced {}) = isArrayExpansion t
+willConcatInAssignment (T_DoubleQuoted _ parts) = any willConcatInAssignment parts
+willConcatInAssignment (T_NormalWord _ parts) = any willConcatInAssignment parts
+willConcatInAssignment _ = False
+
+-- Is it possible that this arg becomes multiple args?
+mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f t
+  where
+    f (T_DollarBraced _ l) =
+        let string = bracedString l in
+            "!" `isPrefixOf` string
+    f (T_DoubleQuoted _ parts) = any f parts
+    f (T_NormalWord _ parts) = any f parts
+    f _ = False
 
 prop_checkShebang1 = verifyTree checkShebang "#!/usr/bin/env bash -x\necho cow"
 prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh  -l "
@@ -598,17 +662,17 @@ prop_checkForInQuoted4 = verify checkForInQuoted "for f in 1,2,3; do true; done"
 prop_checkForInQuoted4a = verifyNot checkForInQuoted "for f in foo{1,2,3}; do true; done"
 prop_checkForInQuoted5 = verify checkForInQuoted "for f in ls; do true; done"
 prop_checkForInQuoted6 = verifyNot checkForInQuoted "for f in \"${!arr}\"; do true; done"
-checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) =
+checkForInQuoted _ (T_ForIn _ _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) =
-    when (any (\x -> willSplit x && not (isMagicInQuotes x)) list
+    when (any (\x -> willSplit x && not (mayBecomeMultipleArgs x)) list
             || (liftM wouldHaveBeenGlob (getLiteralString word) == Just True)) $
         err id 2066 $ "Since you double quoted this, it will not word split, and the loop will only run once."
-checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_SingleQuoted id s]] _) =
+checkForInQuoted _ (T_ForIn _ _ f [T_NormalWord _ [T_SingleQuoted id s]] _) =
     warn id 2041 $ "This is a literal string. To run as a command, use $(" ++ s ++ ")."
-checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_Literal id s]] _) =
+checkForInQuoted _ (T_ForIn _ _ f [T_NormalWord _ [T_Literal id s]] _) =
     if ',' `elem` s
       then unless ('{' `elem` s) $
             warn id 2042 $ "Use spaces, not commas, to separate loop elements."
-      else warn id 2043 $ "This loop will only run once, with " ++ f ++ "='" ++ s ++ "'."
+      else warn id 2043 $ "This loop will only run once, with " ++ (head f) ++ "='" ++ s ++ "'."
 checkForInQuoted _ _ = return ()
 
 prop_checkForInCat1 = verify checkForInCat "for f in $(cat foo); do stuff; done"
@@ -616,7 +680,7 @@ prop_checkForInCat1a= verify checkForInCat "for f in `cat foo`; do stuff; done"
 prop_checkForInCat2 = verify checkForInCat "for f in $(cat foo | grep lol); do stuff; done"
 prop_checkForInCat2a= verify checkForInCat "for f in `cat foo | grep lol`; do stuff; done"
 prop_checkForInCat3 = verifyNot checkForInCat "for f in $(cat foo | grep bar | wc -l); do stuff; done"
-checkForInCat _ (T_ForIn _ f [T_NormalWord _ w] _) = mapM_ checkF w
+checkForInCat _ (T_ForIn _ _ f [T_NormalWord _ w] _) = mapM_ checkF w
   where
     checkF (T_DollarExpansion id [T_Pipeline _ _ r])
         | all isLineBased r =
@@ -632,17 +696,17 @@ prop_checkForInLs2 = verify checkForInLs "for f in `ls *.mp3`; do mplayer \"$f\"
 prop_checkForInLs3 = verify checkForInLs "for f in `find / -name '*.mp3'`; do mplayer \"$f\"; done"
 checkForInLs _ t = try t
   where
-   try (T_ForIn _ f [T_NormalWord _ [T_DollarExpansion id [x]]] _) =
+   try (T_ForIn _ _ f [T_NormalWord _ [T_DollarExpansion id [x]]] _) =
         check id f x
-   try (T_ForIn _ f [T_NormalWord _ [T_Backticked id [x]]] _) =
+   try (T_ForIn _ _ f [T_NormalWord _ [T_Backticked id [x]]] _) =
         check id f x
    try _ = return ()
    check id f x =
     case deadSimple x of
       ("ls":n) ->
         let warntype = if any ("-" `isPrefixOf`) n then warn else err in
-          warntype id 2045 $ "Iterate over globs whenever possible (e.g. 'for f in */*.wav'), as for loops over ls will fail for filenames like 'my file*.txt'."
+          warntype id 2045 "Iterating over ls output is fragile. Use globs."
-      ("find":_) -> warn id 2044 $ "Use find -exec or a while read loop instead, as for loops over find will fail for filenames like 'my file*.txt'."
+      ("find":_) -> warn id 2044 "For loops over find output are fragile. Use find -exec or a while read loop."
       _ -> return ()
 
 
@@ -754,14 +818,20 @@ checkShorthandIf _ (T_AndIf id _ (T_OrIf _ _ (T_Pipeline _ _ t)))
   where
     isOk [t] = isAssignment t || (fromMaybe False $ do
         name <- getCommandBasename t
-        return $ name `elem` ["echo", "exit"])
+        return $ name `elem` ["echo", "exit", "return"])
     isOk _ = False
 checkShorthandIf _ _ = return ()
 
 
 prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done"
-checkDollarStar _ (T_NormalWord _ [(T_DollarBraced id l)]) | (bracedString l) == "*"  =
+prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*"
+checkDollarStar p t@(T_NormalWord _ [(T_DollarBraced id l)])
+      | (bracedString l) == "*"  =
+    unless isAssigned $
         warn id 2048 $ "Use \"$@\" (with quotes) to prevent whitespace problems."
+  where
+    path = getPath (parentMap p) t
+    isAssigned = any isAssignment . take 2 $ path
 checkDollarStar _ _ = return ()
 
 
@@ -771,14 +841,56 @@ prop_checkUnquotedDollarAt2 = verify checkUnquotedDollarAt "ls ${foo[@]}"
 prop_checkUnquotedDollarAt3 = verifyNot checkUnquotedDollarAt "ls ${#foo[@]}"
 prop_checkUnquotedDollarAt4 = verifyNot checkUnquotedDollarAt "ls \"$@\""
 prop_checkUnquotedDollarAt5 = verifyNot checkUnquotedDollarAt "ls ${foo/@/ at }"
-checkUnquotedDollarAt _ (T_NormalWord _ [T_DollarBraced id l]) =
+prop_checkUnquotedDollarAt6 = verifyNot checkUnquotedDollarAt "a=$@"
-    let string = bracedString l
+checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not isAssigned =
-        failing = err id 2068 $ "Add double quotes around ${" ++ string ++ "}, otherwise it's just like $* and breaks on spaces."
+    flip mapM_ (take 1 $ filter isArrayExpansion parts) $ \x -> do
-        in do
+        err (getId x) 2068 $
-            when ("@" `isPrefixOf` string) failing
+            "Double quote array expansions, otherwise they're like $* and break on spaces."
-            when (not ("#" `isPrefixOf` string) && "[@]" `isInfixOf` string) failing
+  where
+    path = getPath (parentMap p) word
+    isAssigned = any isAssignment . take 2 $ path
 checkUnquotedDollarAt _ _ = return ()
 
+
+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 ()
+
+prop_checkArrayWithoutIndex1 = verifyTree checkArrayWithoutIndex "foo=(a b); echo $foo"
+prop_checkArrayWithoutIndex2 = verifyNotTree checkArrayWithoutIndex "foo='bar baz'; foo=($foo); echo ${foo[0]}"
+checkArrayWithoutIndex params _ =
+    concat $ doVariableFlowAnalysis readF writeF Map.empty (variableFlow params)
+  where
+    readF _ (T_DollarBraced id token) _ = do
+        map <- get
+        return . maybeToList $ do
+            name <- getLiteralString token
+            assignment <- Map.lookup name map
+            return [(Note id WarningC 2128
+                "Expanding an array without an index only gives the first element.")]
+    readF _ _ _ = return []
+
+    writeF _ t name (DataFrom [T_Array {}]) = do
+        modify (Map.insert name t)
+        return []
+    writeF _ _ name _ = do
+        modify (Map.delete name)
+        return []
+
 prop_checkStderrRedirect = verify checkStderrRedirect "test 2>&1 > cow"
 prop_checkStderrRedirect2 = verifyNot checkStderrRedirect "test > cow 2>&1"
 checkStderrRedirect _ (T_Redirecting _ [
@@ -805,6 +917,7 @@ prop_checkSingleQuotedVariables4 = verifyNot checkSingleQuotedVariables "awk '{p
 prop_checkSingleQuotedVariables5 = verifyNot checkSingleQuotedVariables "trap 'echo $SECONDS' EXIT"
 prop_checkSingleQuotedVariables6 = verifyNot checkSingleQuotedVariables "sed -n '$p'"
 prop_checkSingleQuotedVariables6a= verify checkSingleQuotedVariables "sed -n '$pattern'"
+prop_checkSingleQuotedVariables7 = verifyNot checkSingleQuotedVariables "PS1='$PWD \\$ '"
 checkSingleQuotedVariables params t@(T_SingleQuoted id s) =
     when (s `matches` re) $
         if "sed" == commandName
@@ -819,19 +932,29 @@ checkSingleQuotedVariables params t@(T_SingleQuoted id s) =
         name <- getCommandBasename cmd
         return name
 
-    isProbablyOk = commandName `elem` [
+    isProbablyOk =
+            (any isOkAssignment $ take 3 $ getPath parents t)
+            || commandName `elem` [
                 "trap"
                 ,"sh"
                 ,"bash"
                 ,"ksh"
                 ,"zsh"
                 ,"ssh"
+                ,"xprop"
+                ,"alias"
                 ]
             || "awk" `isSuffixOf` commandName
             || "perl" `isPrefixOf` commandName
 
+    commonlyQuoted = ["PS1", "PS2", "PS3", "PS4", "PROMPT_COMMAND"]
+    isOkAssignment t =
+        case t of
+            T_Assignment _ _ name _ _ -> name `elem` commonlyQuoted
+            otherwise -> False
+
     re = mkRegex "\\$[{(0-9a-zA-Z_]"
-    sedContra = mkRegex "\\$[dp]($|[^a-zA-Z])"
+    sedContra = mkRegex "\\$[dpsaic]($|[^a-zA-Z])"
 checkSingleQuotedVariables _ _ = return ()
 
 
@@ -852,8 +975,11 @@ prop_checkNumberComparisons7 = verifyNot checkNumberComparisons "[[ 3.14 == $foo
 prop_checkNumberComparisons8 = verify checkNumberComparisons "[[ foo <= bar ]]"
 prop_checkNumberComparisons9 = verify checkNumberComparisons "[ foo \\>= bar ]"
 prop_checkNumberComparisons10= verify checkNumberComparisons "#!/bin/zsh -x\n[ foo >= bar ]]"
+prop_checkNumberComparisons11= verify checkNumberComparisons "[[ $foo -eq 'N' ]]"
+prop_checkNumberComparisons12= verify checkNumberComparisons "[ x$foo -gt x${N} ]"
 checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do
-    if (isNum lhs || isNum rhs)
+    if (isNum lhs && (not $ isNonNum rhs)
+            || isNum rhs && (not $ isNonNum lhs))
       then do
         when (isLtGt op) $
           err id 2071 $
@@ -871,6 +997,7 @@ checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do
 
     when (op `elem` ["-lt", "-gt", "-le", "-ge", "-eq"]) $ do
         mapM_ checkDecimals [lhs, rhs]
+        checkStrings [lhs, rhs]
 
   where
       isLtGt = flip elem ["<", "\\<", ">", "\\>"]
@@ -885,6 +1012,17 @@ checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do
       decimalError = "Decimals are not supported. " ++
         "Either use integers only, or use bc or awk to compare."
 
+      checkStrings hs =
+        mapM_ stringError . take 1 . filter isNonNum $ hs
+
+      isNonNum t = fromMaybe False $ do
+        s <- getLiteralStringExt (const $ return "") t
+        return . not . all numChar $ s
+      numChar x = isDigit x || x `elem` "+-. "
+
+      stringError t = err (getId t) 2130 $
+        op ++ " is for integer comparisons. Use " ++ (seqv op) ++ " instead."
+
       isNum t =
         case deadSimple t of
             [v] -> all isDigit v
@@ -901,6 +1039,15 @@ checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do
       eqv ">=" = "-ge"
       eqv _ = "the numerical equivalent"
 
+      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"
+
       invert ('\\':s) = invert s
       invert "<=" = ">"
       invert ">=" = "<"
@@ -994,9 +1141,10 @@ checkConstantNoary _ (TC_Noary _ _ t@(T_NormalWord id _)) | isConstant t = do
     err id 2078 $ "This expression is constant. Did you forget a $ somewhere?"
 checkConstantNoary _ _ = return ()
 
-prop_checkBraceExpansionVars = verify checkBraceExpansionVars "echo {1..$n}"
+prop_checkBraceExpansionVars1 = verify checkBraceExpansionVars "echo {1..$n}"
-checkBraceExpansionVars _ (T_BraceExpansion id s) | '$' `elem` s =
+prop_checkBraceExpansionVars2 = verifyNot checkBraceExpansionVars "echo {1,3,$n}"
-    warn id 2051 $ "Bash doesn't support variables in brace expansions."
+checkBraceExpansionVars _ (T_BraceExpansion id s) | "..$" `isInfixOf` s =
+    warn id 2051 $ "Bash doesn't support variables in brace range expansions."
 checkBraceExpansionVars _ _ = return ()
 
 prop_checkForDecimals = verify checkForDecimals "((3.14*c))"
@@ -1133,7 +1281,9 @@ isQuoteFree tree t =
             T_CaseExpression _ _ _ -> return True
             T_HereDoc _ _ _ _ _ -> return True
             T_DollarBraced {} -> return True
-            T_ForIn _ _ _ _ -> return True -- Pragmatically assume it's desirable here
+            -- Pragmatically assume it's desirable to split here
+            T_ForIn {} -> return True
+            T_SelectIn {} -> return True
             _ -> Nothing
 
 isParamTo tree cmd t =
@@ -1204,6 +1354,13 @@ getLiteralStringExt more t = g t
 
 isLiteral t = isJust $ getLiteralString t
 
+-- turn a NormalWord like foo="bar $baz" into a series of constituent elements like [foo=,bar ,$baz]
+getWordParts t = g t
+  where
+    g (T_NormalWord _ l) = concatMap g l
+    g (T_DoubleQuoted _ l) = l
+    g other = [other]
+
 isCommand token str = isCommandMatch token (\cmd -> cmd  == str || ("/" ++ str) `isSuffixOf` cmd)
 isUnqualifiedCommand token str = isCommandMatch token (\cmd -> cmd  == str)
 
@@ -1224,6 +1381,7 @@ basename = reverse . (takeWhile (/= '/')) . reverse
 isAssignment (T_Annotation _ _ w) = isAssignment w
 isAssignment (T_Redirecting _ _ w) = isAssignment w
 isAssignment (T_SimpleCommand _ (w:_) []) = True
+isAssignment (T_Assignment {}) = True
 isAssignment _ = False
 
 prop_checkPrintfVar1 = verify checkPrintfVar "printf \"Lol: $s\""
@@ -1338,24 +1496,31 @@ prop_checkGrepRe6 = verifyNot checkGrepRe "grep foo \\*.mp3"
 prop_checkGrepRe7 = verify checkGrepRe "grep *foo* file"
 prop_checkGrepRe8 = verify checkGrepRe "ls | grep foo*.jpg"
 prop_checkGrepRe9 = verifyNot checkGrepRe "grep '[0-9]*' file"
+prop_checkGrepRe10= verifyNot checkGrepRe "grep '^aa*' file"
+prop_checkGrepRe11= verifyNot checkGrepRe "grep --include=*.png foo"
 
 checkGrepRe _ = checkCommand "grep" (const f) where
     -- --regex=*(extglob) doesn't work. Fixme?
     skippable (Just s) = not ("--regex=" `isPrefixOf` s) && "-" `isPrefixOf` s
     skippable _ = False
     f [] = return ()
-    f (x:r) | skippable (getLiteralString x) = f r
+    f (x:r) | skippable (getLiteralStringExt (const $ return "_") x) = f r
     f (re:_) = do
         when (isGlob re) $ do
             warn (getId re) 2062 $ "Quote the grep pattern so the shell won't interpret it."
         let string = concat $ deadSimple re
         if isConfusedGlobRegex string then
             warn (getId re) 2063 $ "Grep uses regex, but this looks like a glob."
-          else
+          else potentially $ do
-            if (isPotentiallyConfusedGlobRegex string)
+            char <- getSuspiciousRegexWildcard string
-            then info (getId re) 2022 "Note that c* does not mean \"c followed by anything\" in regex."
+            return $ info (getId re) 2022 $
-            else return ()
+                "Note that unlike globs, " ++ [char] ++ "* here matches '" ++ [char, char, char] ++ "' but not '" ++ (wordStartingWith char) ++ "'."
 
+wordStartingWith c =
+    head . filter ([c] `isPrefixOf`) $ candidates
+  where
+    candidates =
+        sampleWords ++ (map (\(x:r) -> (toUpper x) : r) sampleWords) ++ [c:"test"]
 
 prop_checkTrapQuotes1 = verify checkTrapQuotes "trap \"echo $num\" INT"
 prop_checkTrapQuotes1a= verify checkTrapQuotes "trap \"echo `ls`\" INT"
@@ -1473,6 +1638,8 @@ checkIndirectExpansion _ _ = return ()
 prop_checkInexplicablyUnquoted1 = verify checkInexplicablyUnquoted "echo 'var='value';'"
 prop_checkInexplicablyUnquoted2 = verifyNot checkInexplicablyUnquoted "'foo'*"
 prop_checkInexplicablyUnquoted3 = verifyNot checkInexplicablyUnquoted "wget --user-agent='something'"
+prop_checkInexplicablyUnquoted4 = verify checkInexplicablyUnquoted "echo \"VALUES (\"id\")\""
+prop_checkInexplicablyUnquoted5 = verifyNot checkInexplicablyUnquoted "\"$dir\"/\"$file\""
 checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens)
   where
     check ((T_SingleQuoted _ _):(T_Literal id str):_)
@@ -1481,13 +1648,16 @@ checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens
 
     check ((T_DoubleQuoted _ _):trapped:(T_DoubleQuoted _ _):_) =
         case trapped of
-            T_DollarExpansion id _ -> warnAbout id
+            T_DollarExpansion id _ -> warnAboutExpansion id
-            T_DollarBraced id _ -> warnAbout id
+            T_DollarBraced id _ -> warnAboutExpansion id
+            T_Literal id s -> unless (s == "/") $ warnAboutLiteral id
             _ -> return ()
 
     check _ = return ()
-    warnAbout id =
+    warnAboutExpansion id =
-        info id 2027 $ "Surrounding quotes actually unquotes this (\"inside\"$outside\"inside\"). Did you forget your quote level?"
+        warn id 2027 $ "The surrounding quotes actually unquote this. Remove or escape them."
+    warnAboutLiteral id =
+        warn id 2140 $ "The double quotes around this do nothing. Remove or escape them."
 checkInexplicablyUnquoted _ _ = return ()
 
 prop_checkTildeInQuotes1 = verify checkTildeInQuotes "var=\"~/out.txt\""
@@ -1526,7 +1696,7 @@ checkSpuriousExec _ = doLists
     doLists (T_BraceGroup _ cmds) = doList cmds
     doLists (T_WhileExpression _ _ cmds) = doList cmds
     doLists (T_UntilExpression _ _ cmds) = doList cmds
-    doLists (T_ForIn _ _ _ cmds) = doList cmds
+    doLists (T_ForIn _ _ _ _ cmds) = doList cmds
     doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds
     doLists (T_IfExpression _ thens elses) = do
         mapM_ (\(_, l) -> doList l) thens
@@ -1576,11 +1746,14 @@ prop_checkUnusedEchoEscapes1 = verify checkUnusedEchoEscapes "echo 'foo\\nbar\\n
 prop_checkUnusedEchoEscapes2 = verifyNot checkUnusedEchoEscapes "echo -e 'foi\\nbar'"
 prop_checkUnusedEchoEscapes3 = verify checkUnusedEchoEscapes "echo \"n:\\t42\""
 prop_checkUnusedEchoEscapes4 = verifyNot checkUnusedEchoEscapes "echo lol"
+prop_checkUnusedEchoEscapes5 = verifyNot checkUnusedEchoEscapes "echo -n -e '\n'"
 checkUnusedEchoEscapes _ = checkCommand "echo" (const f)
   where
     isDashE = mkRegex "^-.*e"
     hasEscapes = mkRegex "\\\\[rnt]"
-    f (arg:_) | (concat $ deadSimple arg) `matches` isDashE = return ()
+    f args | (concat $ concatMap deadSimple allButLast) `matches` isDashE =
+        return ()
+      where allButLast = reverse . drop 1 . reverse $ args
     f args = mapM_ checkEscapes args
 
     checkEscapes (T_NormalWord _ args) =
@@ -1723,7 +1896,7 @@ getModifiedVariables t =
                 else []
 
         --Points to 'for' rather than variable
-        T_ForIn id str words _ -> [(t, t, str, DataFrom words)]
+        T_ForIn id _ strs words _ -> map (\str -> (t, t, str, DataFrom words)) strs
         T_SelectIn id str words _ -> [(t, t, str, DataFrom words)]
         _ -> []
 
@@ -1745,7 +1918,9 @@ getReferencedVariableCommand _ = []
 getModifiedVariableCommand base@(T_SimpleCommand _ _ ((T_NormalWord _ ((T_Literal _ x):_)):rest)) =
    filter (\(_,_,s,_) -> not ("-" `isPrefixOf` s)) $
     case x of
-        "read" ->  concatMap getLiteral rest
+        "read" ->
+            let params = map getLiteral rest in
+                catMaybes . takeWhile isJust . reverse $ params
         "let" -> concatMap letParamToLiteral rest
 
         "export" -> concatMap getModifierParam rest
@@ -1763,13 +1938,10 @@ getModifiedVariableCommand base@(T_SimpleCommand _ _ ((T_NormalWord _ ((T_Litera
         (T_NormalWord id1 [T_DoubleQuoted id2 [T_Literal id3 (stripEquals s)]])
     stripEqualsFrom t = t
 
-    getLiteral t@(T_NormalWord _ [T_Literal _ s]) =
+    getLiteral t = do
-        [(base, t, s, DataExternal)]
+        s <- getLiteralString t
-    getLiteral t@(T_NormalWord _ [T_SingleQuoted _ s]) =
+        when ("-" `isPrefixOf` s) $ fail "argument"
-        [(base, t, s, DataExternal)]
+        return (base, t, s, DataExternal)
-    getLiteral t@(T_NormalWord _ [T_DoubleQuoted _ [T_Literal id s]]) =
-        [(base, t, s, DataExternal)]
-    getLiteral x = []
 
     getModifierParam t@(T_Assignment _ _ name _ value) =
         [(base, t, name, DataFrom [value])]
@@ -1817,8 +1989,8 @@ getVariableFlow shell parents t =
             if assignFirst t then return () else setWritten t
             when (scopeType /= NoneScope) $ modify ((StackScopeEnd):)
 
-    assignFirst (T_ForIn _ _ _ _) = True
+    assignFirst (T_ForIn {}) = True
-    assignFirst (T_SelectIn _ _ _ _) = True
+    assignFirst (T_SelectIn {}) = True
     assignFirst _ = False
 
     setRead t =
@@ -1879,6 +2051,7 @@ prop_checkSpacefulnessH = verifyTree checkSpacefulness "echo foo=$1"
 prop_checkSpacefulnessI = verifyNotTree checkSpacefulness "$1 --flags"
 prop_checkSpacefulnessJ = verifyTree checkSpacefulness "echo $PWD"
 prop_checkSpacefulnessK = verifyNotTree checkSpacefulness "n+='foo bar'"
+prop_checkSpacefulnessL = verifyNotTree checkSpacefulness "select foo in $bar; do true; done"
 
 checkSpacefulness params t =
     doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params)
@@ -2056,6 +2229,7 @@ prop_checkUnused13= verifyNotTree checkUnusedAssignments "x=(1); (( x[0] ))"
 prop_checkUnused14= verifyNotTree checkUnusedAssignments "x=(1); n=0; echo ${x[n]}"
 prop_checkUnused15= verifyNotTree checkUnusedAssignments "x=(1); n=0; (( x[n] ))"
 prop_checkUnused16= verifyNotTree checkUnusedAssignments "foo=5; declare -x foo"
+prop_checkUnused17= verifyNotTree checkUnusedAssignments "read -i 'foo' -e -p 'Input: ' bar; $bar;"
 checkUnusedAssignments params t = snd $ runWriter (mapM_ checkAssignment flow)
   where
     flow = variableFlow params
@@ -2103,6 +2277,7 @@ prop_checkWhileReadPitfalls3 = verifyNot checkWhileReadPitfalls "while true; do
 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"
+prop_checkWhileReadPitfalls7 = verify checkWhileReadPitfalls "while read foo; do if true; then ssh $foo uptime; fi; done < file"
 
 checkWhileReadPitfalls _ (T_WhileExpression id [command] contents)
         | isStdinReadCommand command = do
@@ -2117,9 +2292,15 @@ checkWhileReadPitfalls _ (T_WhileExpression id [command] contents)
             && all (not . stdinRedirect) redirs
     isStdinReadCommand _ = False
 
-    checkMuncher (T_Pipeline _ _ ((T_Redirecting _ redirs cmd):_)) = do
+    checkMuncher (T_Pipeline _ _ ((T_Redirecting _ redirs cmd):_)) | not $ any stdinRedirect redirs = do
-        let name = fromMaybe "" $ getCommandBasename cmd
+        case cmd of
-        when ((not . any stdinRedirect $ redirs) && (name `elem` munchers)) $ do
+            (T_IfExpression _ thens elses) ->
+              mapM_ checkMuncher . concat $ (map fst thens) ++ (map snd thens) ++ [elses]
+
+            _ -> potentially $ do
+                name <- getCommandBasename cmd
+                guard $ name `elem` munchers
+                return $ do
                     info id 2095 $
                         name ++ " may swallow stdin, preventing this loop from working properly."
                     warn (getId cmd) 2095 $
@@ -2158,7 +2339,9 @@ 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]"
-checkCharRangeGlob _ (T_Glob id str) | isCharClass str =
+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) =
     if ":" `isPrefixOf` contents
         && ":" `isSuffixOf` contents
         && contents /= ":"
@@ -2181,7 +2364,7 @@ prop_checkCdAndBack3 = verifyNot checkCdAndBack "while [[ $PWD != / ]]; do cd ..
 checkCdAndBack params = doLists
   where
     shell = shellType params
-    doLists (T_ForIn _ _ _ cmds) = doList cmds
+    doLists (T_ForIn _ _ _ _ cmds) = doList cmds
     doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds
     doLists (T_WhileExpression _ _ cmds) = doList cmds
     doLists (T_UntilExpression _ _ cmds) = doList cmds
@@ -2465,3 +2648,170 @@ checkSetAssignment params = checkUnqualifiedCommand "set" f
     literal (T_NormalWord _ l) = concatMap literal l
     literal (T_Literal _ str) = str
     literal _ = "*"
+
+
+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
+    checkVar (T_Assignment id Assign "PATH" Nothing word) =
+        let string = concat $ deadSimple word
+        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 ()
+
+
+prop_checkUnsupported1 = verifyNot checkUnsupported "#!/bin/zsh\nfunction { echo cow; }"
+prop_checkUnsupported2 = verify checkUnsupported "#!/bin/sh\nfunction { echo cow; }"
+checkUnsupported params t =
+    when  (shellType params `notElem` support) $
+        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
+    T_Function _ _ _ "" _ -> ("anonymous functions", [Zsh])
+    T_ForIn _ _ (_:_:_) _ _ -> ("multi-index for loops", [Zsh])
+    T_ForIn _ ShortForIn _ _ _ -> ("short form for loops", [Zsh])
+    T_ProcSub _ "=" _ -> ("=(..) process substitution", [Zsh])
+    otherwise -> ("", [Bash, Ksh, Sh, Zsh])
+
+getCommandSequences t =
+    f t
+  where
+    f (T_Script _ _ cmds) = [cmds]
+    f (T_BraceGroup _ cmds) = [cmds]
+    f (T_Subshell _ cmds) = [cmds]
+    f (T_WhileExpression _ _ cmds) = [cmds]
+    f (T_UntilExpression _ _ cmds) = [cmds]
+    f (T_ForIn _ _ _ _ cmds) = [cmds]
+    f (T_ForArithmetic _ _ _ _ cmds) = [cmds]
+    f (T_IfExpression _ thens elses) = (map snd thens) ++ [elses]
+    f _ = []
+
+groupWith f l = groupBy (\x y -> f x == f y) l
+
+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 =
+        mapM_ checkGroup groups
+      where
+        groups = groupWith (liftM fst) $ map getTarget list
+    checkGroup (f:_:_:_) | isJust f =
+        style (snd $ fromJust f) 2129
+            "Consider using { cmd1; cmd2; } >> file instead of individual redirects."
+    checkGroup _ = return ()
+    getTarget (T_Pipeline _ _ args@(_:_)) = getTarget (last args)
+    getTarget (T_Redirecting id list _) = do
+        file <- (mapMaybe getAppend list) !!! 0
+        return (file, id)
+    getTarget _ = Nothing
+    getAppend (T_FdRedirect _ _ (T_IoFile _ (T_DGREAT {}) f)) = return f
+    getAppend _ = Nothing
+
+
+prop_checkAliasesExpandEarly1 = verify checkAliasesExpandEarly "alias foo=\"echo $PWD\""
+prop_checkAliasesExpandEarly2 = verifyNot checkAliasesExpandEarly "alias -p"
+prop_checkAliasesExpandEarly3 = verifyNot checkAliasesExpandEarly "alias foo='echo {1..10}'"
+checkAliasesExpandEarly params =
+    checkUnqualifiedCommand "alias" (const f)
+  where
+    f = mapM_ checkArg
+    checkArg arg | '=' `elem` (concat $ deadSimple arg) =
+        flip mapM_ (take 1 $ filter (not . isLiteral) $ getWordParts arg) $
+            \x -> warn (getId x) 2139 "This expands when defined, not when used. Consider escaping."
+    checkArg _ = return ()
+
+prop_checkSuspiciousIFS1 = verify checkSuspiciousIFS "IFS=\"\\n\""
+prop_checkSuspiciousIFS2 = verifyNot checkSuspiciousIFS "IFS=$'\\t'"
+checkSuspiciousIFS params (T_Assignment id Assign "IFS" Nothing value) =
+    potentially $ do
+        str <- getLiteralString value
+        return $ check str
+  where
+    n = if (shellType params == Sh) then "'<literal linefeed here>'" else "$'\\n'"
+    t = if (shellType params == Sh) then "\"$(printf '\\t')\"" else "$'\\t'"
+    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 ()
+
+prop_checkAliasesUsesArgs1 = verify checkAliasesUsesArgs "alias a='cp $1 /a'"
+prop_checkAliasesUsesArgs2 = verifyNot checkAliasesUsesArgs "alias $1='foo'"
+prop_checkAliasesUsesArgs3 = verify checkAliasesUsesArgs "alias a=\"echo \\${@}\""
+checkAliasesUsesArgs params =
+    checkUnqualifiedCommand "alias" (const f)
+  where
+    re = mkRegex "\\$\\{?[0-9*@]"
+    f = mapM_ checkArg
+    checkArg arg =
+        let string = fromJust $ getLiteralStringExt (const $ return "_") arg in
+            when ('=' `elem` string && string `matches` re) $
+                err (getId arg) 2142
+                    "Aliases can't use positional parameters. Use a function."
+
+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)"
+checkShouldUseGrepQ params t =
+    potentially $ case t of
+        TC_Noary id _ token -> check id True token
+        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 $
+            "Instead of [ " ++ op ++ " $(foo | " ++ name ++ " bar) ], " ++
+                "use " ++ flip ++ "foo | " ++ name ++ " -q bar ."
+
+    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"
+    isGrep = isSuffixOf "grep"
+
+prop_checkTestGlobs1 = verify checkTestGlobs "[ -e *.mp3 ]"
+prop_checkTestGlobs2 = verifyNot checkTestGlobs "[[ $a == *b* ]]"
+checkTestGlobs params (TC_Unary _ _ op token) | isGlob token =
+    err (getId token) 2144 $
+       op ++ " doesn't work with globs. Use a for loop."
+checkTestGlobs _ _ = return ()

ShellCheck/Data.hs

@@ -1,6 +1,6 @@
 module ShellCheck.Data where
 
-shellcheckVersion = "0.3.2"  -- Must also be updated in ShellCheck.cabal
+shellcheckVersion = "0.3.3"  -- Must also be updated in ShellCheck.cabal
 
 internalVariables = [
     -- Generic
@@ -74,3 +74,11 @@ commonCommands = [
     "val", "vi", "wait", "wc", "what", "who", "write", "xargs", "yacc",
     "zcat"
   ]
+
+sampleWords = [
+    "alpha", "bravo", "charlie", "delta", "echo", "foxtrot",
+    "golf", "hotel", "india", "juliett", "kilo", "lima", "mike",
+    "november", "oscar", "papa", "quebec", "romeo", "sierra",
+    "tango", "uniform", "victor", "whiskey", "xray", "yankee",
+    "zulu"
+  ]

ShellCheck/Parser.hs

@@ -64,6 +64,11 @@ spacing = do
     optional readComment
     return $ concat x
 
+spacing1 = do
+    spacing <- spacing
+    when (null spacing) $ fail "no spacing"
+    return spacing
+
 prop_allspacing = isOk allspacing "#foo"
 prop_allspacing2 = isOk allspacing " #foo\n # bar\n#baz\n"
 prop_allspacing3 = isOk allspacing "#foo\n#bar\n#baz\n"
@@ -403,7 +408,7 @@ readConditionContents single = do
       where
         readGlobLiteral = do
             id <- getNextId
-            s <- many1 (extglobStart <|> oneOf "[]$")
+            s <- many1 (extglobStart <|> oneOf "{}[]$")
             return $ T_Literal id s
         readGroup = called "regex grouping" $ do
             id <- getNextId
@@ -632,6 +637,7 @@ prop_readCondition9 = isOk readCondition "[ foo -a -f bar ]"
 prop_readCondition10= isOk readCondition "[[ a == b \n || c == d ]]"
 prop_readCondition11= isOk readCondition "[[ a == b || \n c == d ]]"
 prop_readCondition12= isWarning readCondition "[ a == b \n -o c == d ]"
+prop_readCondition13= isOk readCondition "[[ foo =~ ^fo{1,3}$ ]]"
 readCondition = called "test expression" $ do
     opos <- getPosition
     id <- getNextId
@@ -700,6 +706,8 @@ prop_readNormalWord = isOk readNormalWord "'foo'\"bar\"{1..3}baz$(lol)"
 prop_readNormalWord2 = isOk readNormalWord "foo**(foo)!!!(@@(bar))"
 prop_readNormalWord3 = isOk readNormalWord "foo#"
 prop_readNormalWord4 = isOk readNormalWord "$\"foo\"$'foo\nbar'"
+prop_readNormalWord5 = isWarning readNormalWord "${foo}}"
+prop_readNormalWord6 = isOk readNormalWord "foo/{}"
 readNormalWord = readNormalishWord ""
 
 readNormalishWord end = do
@@ -710,14 +718,24 @@ readNormalishWord end = do
     return $ T_NormalWord id x
 
 checkPossibleTermination pos [T_Literal _ x] =
-    if x `elem` ["do", "done", "then", "fi", "esac", "}"]
+    if x `elem` ["do", "done", "then", "fi", "esac"]
         then parseProblemAt pos WarningC 1010 $ "Use semicolon or linefeed before '" ++ x ++ "' (or quote to make it literal)."
         else return ()
 checkPossibleTermination _ _ = return ()
 
 readNormalWordPart end = do
     checkForParenthesis
-    readSingleQuoted <|> readDoubleQuoted <|> readGlob <|> readNormalDollar <|> readBraced <|> readBackTicked <|> readProcSub <|> (readNormalLiteral end)
+    choice [
+        readSingleQuoted,
+        readDoubleQuoted,
+        readGlob,
+        readNormalDollar,
+        readBraced,
+        readBackTicked,
+        readProcSub,
+        readNormalLiteral end,
+        readLiteralCurlyBraces
+      ]
   where
     checkForParenthesis = do
         return () `attempting` do
@@ -725,6 +743,19 @@ readNormalWordPart end = do
             lookAhead $ char '('
             parseProblemAt pos ErrorC 1036 "'(' is invalid here. Did you forget to escape it?"
 
+    readLiteralCurlyBraces = do
+        id <- getNextId
+        str <- findParam <|> literalBraces
+        return $ T_Literal id str
+
+    findParam = try $ string "{}"
+    literalBraces = do
+        pos <- getPosition
+        c <- oneOf "{}"
+        parseProblemAt pos WarningC 1083 $
+            "This " ++ [c] ++ " is literal. Check expression (missing ;/\\n?) or quote it."
+        return [c]
+
 
 readSpacePart = do
     id <- getNextId
@@ -745,10 +776,11 @@ readDollarBracedLiteral = do
 
 prop_readProcSub1 = isOk readProcSub "<(echo test | wc -l)"
 prop_readProcSub2 = isOk readProcSub "<(  if true; then true; fi )"
+prop_readProcSub3 = isOk readProcSub "=(ls)"
 readProcSub = called "process substitution" $ do
     id <- getNextId
     dir <- try $ do
-                    x <- oneOf "<>"
+                    x <- oneOf "<>="
                     char '('
                     return [x]
     allspacing
@@ -922,7 +954,7 @@ readGlob = readExtglob <|> readSimple <|> readClass <|> readGlobbyLiteral
             return $ T_Literal id [c]
 
 readNormalLiteralPart end = do
-    readNormalEscaped <|> (many1 $ noneOf (end ++ quotableChars ++ extglobStartChars ++ "["))
+    readNormalEscaped <|> (many1 $ noneOf (end ++ quotableChars ++ extglobStartChars ++ "[{}"))
 
 readNormalEscaped = called "escaped char" $ do
     pos <- getPosition
@@ -934,10 +966,12 @@ readNormalEscaped = called "escaped char" $ do
         do
             next <- anyChar
             case escapedChar next of
-                Just name -> parseNoteAt pos WarningC 1012 $ "\\" ++ [next] ++ " is just literal '" ++ [next] ++ "' here. For " ++ name ++ ", use \"$(printf \"\\" ++ [next] ++ "\")\"."
+                Just name -> parseNoteAt pos WarningC 1012 $ "\\" ++ [next] ++ " is just literal '" ++ [next] ++ "' here. For " ++ name ++ ", use " ++ (alternative next) ++ " instead."
                 Nothing -> parseNoteAt pos InfoC 1001 $ "This \\" ++ [next] ++ " will be a regular '" ++ [next] ++ "' in this context."
             return [next]
   where
+    alternative 'n' = "a quoted, literal line feed"
+    alternative t = "\"$(printf \"\\" ++ [t] ++ "\")\""
     escapedChar 'n' = Just "line feed"
     escapedChar 't' = Just "tab"
     escapedChar 'r' = Just "carriage return"
@@ -1027,7 +1061,10 @@ readBraced = try $ do
     char '{'
     str <- many1 ((readDoubleQuotedLiteral >>= (strip)) <|> readGenericLiteral1 (oneOf "}\"" <|> whitespace))
     char '}'
-    return $ T_BraceExpansion id $ concat str
+    let result = concat str
+    unless (',' `elem` result || ".." `isInfixOf` result) $
+        fail "Not a brace expression"
+    return $ T_BraceExpansion id $ result
 
 readNormalDollar = readDollarExpression <|> readDollarDoubleQuote <|> readDollarSingleQuote <|> readDollarLonely
 readDoubleQuotedDollar = readDollarExpression <|> readDollarLonely
@@ -1077,7 +1114,7 @@ readArithmeticExpression = called "((..)) command" $ do
 
 prop_readDollarBraced1 = isOk readDollarBraced "${foo//bar/baz}"
 prop_readDollarBraced2 = isOk readDollarBraced "${foo/'{cow}'}"
-prop_readDollarBraced3 = isOk readDollarBraced "${foo%%$(echo cow})}"
+prop_readDollarBraced3 = isOk readDollarBraced "${foo%%$(echo cow\\})}"
 prop_readDollarBraced4 = isOk readDollarBraced "${foo#\\}}"
 readDollarBraced = called "parameter expansion" $ do
     id <- getNextId
@@ -1131,7 +1168,6 @@ readDollarLonely = do
     pos <- getPosition
     char '$'
     n <- lookAhead (anyChar <|> (eof >> return '_'))
-    when (n /= '\'') $ parseNoteAt pos StyleC 1000 "$ is not used specially and should therefore be escaped."
     return $ T_Literal id "$"
 
 prop_readHereDoc = isOk readHereDoc "<< foo\nlol\ncow\nfoo"
@@ -1266,8 +1302,8 @@ readHereString = called "here string" $ do
 readNewlineList = many1 ((newline <|> carriageReturn) `thenSkip` spacing)
 readLineBreak = optional readNewlineList
 
-prop_roflol = isWarning readScript "a &; b"
+prop_readSeparator1 = isWarning readScript "a &; b"
-prop_roflol2 = isOk readScript "a & b"
+prop_readSeparator2 = isOk readScript "a & b"
 readSeparatorOp = do
     notFollowedBy2 (g_AND_IF <|> g_DSEMI)
     notFollowedBy2 (string "&>")
@@ -1276,6 +1312,8 @@ readSeparatorOp = do
                     spacing
                     pos <- getPosition
                     char ';'
+                    -- In case statements we might have foo & ;;
+                    notFollowedBy2 $ char ';'
                     parseProblemAt pos ErrorC 1045 "It's not 'foo &; bar', just 'foo & bar'."
                     return '&'
             ) <|> char ';' <|> char '&'
@@ -1317,6 +1355,7 @@ prop_readSimpleCommand3 = isOk readSimpleCommand "export foo=(bar baz)"
 prop_readSimpleCommand4 = isOk readSimpleCommand "typeset -a foo=(lol)"
 prop_readSimpleCommand5 = isOk readSimpleCommand "time if true; then echo foo; fi"
 prop_readSimpleCommand6 = isOk readSimpleCommand "time -p ( ls -l; )"
+prop_readSimpleCommand7 = isOk readSimpleCommand "cat =(ls)"
 readSimpleCommand = called "simple command" $ do
     id1 <- getNextId
     id2 <- getNextId
@@ -1568,6 +1607,7 @@ prop_readForClause7 = isOk readForClause "for ((;;)) do echo $i\ndone"
 prop_readForClause8 = isOk readForClause "for ((;;)) ; do echo $i\ndone"
 prop_readForClause9 = isOk readForClause "for i do true; done"
 prop_readForClause10= isOk readForClause "for ((;;)) { true; }"
+prop_readForClause11= isOk readForClause "for a b in *; do echo $a $b; done"
 readForClause = called "for loop" $ do
     pos <- getPosition
     (T_For id) <- g_For
@@ -1593,11 +1633,25 @@ readForClause = called "for loop" $ do
         return list
 
     readRegular id pos = do
-        name <- readVariableName
+        names <- readNames
-        spacing
+        readShort names <|> readLong names
+      where
+        readLong names = do
             values <- readInClause <|> (optional readSequentialSep >> return [])
             group <- readDoGroup pos
-        return $ T_ForIn id name values group
+            return $ T_ForIn id NormalForIn names values group
+        readShort names = do
+            char '('
+            allspacing
+            words <- many (readNormalWord `thenSkip` allspacing)
+            char ')'
+            allspacing
+            command <- readAndOr
+            return $ T_ForIn id ShortForIn names words [command]
+
+    readNames =
+        reluctantlyTill1 (readVariableName `thenSkip` spacing) $
+            disregard g_Do <|> disregard readInClause <|> disregard readSequentialSep
 
 prop_readSelectClause1 = isOk readSelectClause "select foo in *; do echo $foo; done"
 prop_readSelectClause2 = isOk readSelectClause "select foo; do echo $foo; done"
@@ -1632,6 +1686,7 @@ readInClause = do
 
 prop_readCaseClause = isOk readCaseClause "case foo in a ) lol; cow;; b|d) fooo; esac"
 prop_readCaseClause2 = isOk readCaseClause "case foo\n in * ) echo bar;; esac"
+prop_readCaseClause3 = isOk readCaseClause "case foo\n in * ) echo bar & ;; esac"
 readCaseClause = called "case expression" $ do
     id <- getNextId
     g_Case
@@ -1705,7 +1760,7 @@ readFunctionDefinition = called "function" $ do
                 g_Rparen
             return ()
 
-        readFunctionName = many1 functionChars
+        readFunctionName = many functionChars
 
 readPattern = (readNormalWord `thenSkip` spacing) `sepBy1` (char '|' `thenSkip` spacing)
 
@@ -1753,6 +1808,9 @@ readAssignmentWord = try $ do
     pos <- getPosition
     optional (char '$' >> parseNote ErrorC 1066 "Don't use $ on the left side of assignments.")
     variable <- readVariableName
+    notFollowedBy2 $ do -- Special case for zsh =(..) syntax
+        spacing1
+        string "=("
     optional (readNormalDollar >> parseNoteAt pos ErrorC
                                 1067 "For indirection, use (associative) arrays or 'read \"var$n\" <<< \"value\"'")
     index <- optionMaybe readArrayIndex
@@ -1893,6 +1951,10 @@ prop_readScript4 = isWarning readScript "#!/usr/bin/perl\nfoo=("
 readScript = do
     id <- getNextId
     pos <- getPosition
+    optional $ do
+        readUtf8Bom
+        parseProblem ErrorC 1082 $
+            "This file has a UTF-8 BOM. Remove it with: LC_CTYPE=C sed '1s/^...//' < yourscript ."
     sb <- option "" readShebang
     verifyShell pos (getShell sb)
     if (isValidShell $ getShell sb) /= Just False
@@ -1952,6 +2014,8 @@ readScript = do
         "tcsh"
         ]
 
+    readUtf8Bom = called "Byte Order Mark" $ string "\xFEFF"
+
 rp p filename contents = Ms.runState (runParserT p initialState filename contents) ([], [])
 
 isWarning p s = (fst cs) && (not . null . snd $ cs) where cs = checkString p s