Stable version 0.3.5

This release is dedicated to Maru, internet celebrity cat.
Where would the web be without you? (Runner-up: Tim Berners-Lee)

README.md

@@ -16,7 +16,7 @@ The goals of ShellCheck are:
   - To point out subtle caveats, corner cases and pitfalls, that may cause an
     advanced user's otherwise working script to fail under future circumstances.
 
-ShellCheck is written in Haskell, and requires at least 1 GB of RAM to compile.
+ShellCheck is written in Haskell, and requires 2 GB of memory to compile.
 
 ## Installing
 
@@ -25,9 +25,9 @@ On systems with Cabal:
     cabal update
     cabal install shellcheck
 
-On Arch Linux with community packages enabled:
+On Debian based distros:
 
-    pacman -S shellcheck
+    apt-get install shellcheck
 
 On OS X with homebrew:
 

ShellCheck.cabal

@@ -1,7 +1,7 @@
 Name:             ShellCheck
-Version:          0.3.4
+Version:          0.3.5
 Synopsis:         Shell script analysis tool
-License:          OtherLicense
+License:          AGPL-3
 License-file:     LICENSE
 Category:         Static Analysis
 Author:           Vidar Holen
@@ -42,11 +42,12 @@ library
       mtl,
       parsec,
       regex-compat,
-      QuickCheck >= 2.2
+      QuickCheck >= 2.7.4
     exposed-modules:
       ShellCheck.Analytics
       ShellCheck.AST
       ShellCheck.Data
+      ShellCheck.Options
       ShellCheck.Parser
       ShellCheck.Simple
     other-modules:
@@ -62,7 +63,8 @@ executable shellcheck
       mtl,
       parsec,
       regex-compat,
-      QuickCheck >= 2.2
+      transformers,
+      QuickCheck >= 2.7.4
     main-is: shellcheck.hs
 
 test-suite test-shellcheck
@@ -76,6 +78,7 @@ test-suite test-shellcheck
       mtl,
       parsec,
       regex-compat,
-      QuickCheck >= 2.2
+      transformers,
+      QuickCheck >= 2.7.4
     main-is: test/shellcheck.hs
 

ShellCheck/AST.hs

@@ -34,6 +34,7 @@ data CaseType = CaseBreak | CaseFallThrough | CaseContinue deriving (Show, Eq)
 data Token =
     TA_Binary Id String Token Token
     | TA_Expansion Id [Token]
+    | TA_Index Id Token
     | TA_Sequence Id [Token]
     | TA_Trinary Id Token Token Token
     | TA_Unary Id String Token
@@ -245,6 +246,7 @@ analyze f g i =
         c <- round t3
         return $ TA_Trinary id a b c
     delve (TA_Expansion id t) = dl t $ TA_Expansion id
+    delve (TA_Index id t) = d1 t $ TA_Index id
     delve (T_Annotation id anns t) = d1 t $ T_Annotation id anns
     delve t = return t
 
@@ -330,6 +332,7 @@ getId t = case t of
         TA_Sequence id _  -> id
         TA_Trinary id _ _ _  -> id
         TA_Expansion id _  -> id
+        TA_Index id _  -> id
         T_ProcSub id _ _ -> id
         T_Glob id _ -> id
         T_ForArithmetic id _ _ _ _ -> id

ShellCheck/Analytics.hs

@@ -16,7 +16,7 @@
     along with this program.  If not, see <http://www.gnu.org/licenses/>.
 -}
 {-# LANGUAGE TemplateHaskell #-}
-module ShellCheck.Analytics (AnalysisOption(..), filterByAnnotation, runAnalytics, shellForExecutable, runTests) where
+module ShellCheck.Analytics (AnalysisOptions(..), defaultAnalysisOptions, filterByAnnotation, runAnalytics, shellForExecutable, runTests) where
 
 import Control.Arrow (first)
 import Control.Monad
@@ -29,23 +29,20 @@ import Data.List
 import Data.Maybe
 import Debug.Trace
 import ShellCheck.AST
+import ShellCheck.Options
 import ShellCheck.Data
 import ShellCheck.Parser hiding (runTests)
 import Text.Regex
 import qualified Data.Map as Map
 import Test.QuickCheck.All (quickCheckAll)
 
-data Shell = Ksh | Zsh | Sh | Bash
-    deriving (Show, Eq)
-
 data Parameters = Parameters {
     variableFlow :: [StackData],
     parentMap :: Map.Map Id Token,
-    shellType :: Shell
+    shellType :: Shell,
+    shellTypeSpecified :: Bool
     }
 
-data AnalysisOption = ForceShell Shell
-
 -- Checks that are run on the AST root
 treeChecks :: [Parameters -> Token -> [Note]]
 treeChecks = [
@@ -55,11 +52,12 @@ treeChecks = [
     ,subshellAssignmentCheck
     ,checkSpacefulness
     ,checkQuotesInLiterals
-    ,checkShebang
+    ,checkShebangParameters
     ,checkFunctionsUsedExternally
     ,checkUnusedAssignments
     ,checkUnpassedInFunctions
     ,checkArrayWithoutIndex
+    ,checkShebang
     ]
 
 checksFor Sh = [
@@ -81,24 +79,20 @@ checksFor Bash = [
     ,checkForDecimals
     ]
 
-runAnalytics :: [AnalysisOption] -> Token -> [Note]
+runAnalytics :: AnalysisOptions -> Token -> [Note]
 runAnalytics options root = runList options root treeChecks
 
 runList options root list = notes
     where
         params = Parameters {
-                shellType = getShellOption,
+                shellType = fromMaybe (determineShell root) $ optionShellType options,
+                shellTypeSpecified = isJust $ optionShellType options,
                 parentMap = getParentTree root,
                 variableFlow = getVariableFlow (shellType params) (parentMap params) root
         }
-        notes = concatMap (\f -> f params root) list
+        notes = filter (\c -> getCode c `notElem` optionExcludes options) $ concatMap (\f -> f params root) list
+        getCode (Note _ _ c _) = c
 
-        getShellOption =
-            fromMaybe (determineShell root) . msum $
-                map (\option ->
-                    case option of
-                        ForceShell x -> return x
-                        ) options
 
 checkList l t = concatMap (\f -> f t) l
 
@@ -207,6 +201,7 @@ nodeChecks = [
     ,checkTestGlobs
     ,checkConcatenatedDollarAt
     ,checkFindActionPrecedence
+    ,checkTildeInPath
     ]
 
 
@@ -289,6 +284,15 @@ matches string regex = isJust $ matchRegex regex string
 headOrDefault _ (a:_) = a
 headOrDefault def _ = def
 
+getAllMatches :: Regex -> String -> [[String]]
+getAllMatches regex str = fromJust $ f str
+  where
+    f str = do
+        (_, _, rest, groups) <- matchRegexAll regex str
+        more <- f rest
+        return $ groups : more
+      `mappend` return []
+
 isConstant token =
     case token of
         T_NormalWord _ l   -> all isConstant l
@@ -343,21 +347,21 @@ getFlags _ = []
         [] -> Nothing
         (r:_) -> Just r
 
-verify :: (Parameters -> Token -> Writer [a] ()) -> String -> Bool
+verify :: (Parameters -> Token -> Writer [Note] ()) -> String -> Bool
 verify f s = checkNode f s == Just True
 
-verifyNot :: (Parameters -> Token -> Writer [a] ()) -> String -> Bool
+verifyNot :: (Parameters -> Token -> Writer [Note] ()) -> String -> Bool
 verifyNot f s = checkNode f s == Just False
 
-verifyTree :: (Parameters -> Token -> [a]) -> String -> Bool
+verifyTree :: (Parameters -> Token -> [Note]) -> String -> Bool
 verifyTree f s = checkTree f s == Just True
 
-verifyNotTree :: (Parameters -> Token -> [a]) -> String -> Bool
+verifyNotTree :: (Parameters -> Token -> [Note]) -> String -> Bool
 verifyNotTree f s = checkTree f s == Just False
 
 checkNode f = checkTree (runNodeAnalysis f)
 checkTree f s = case parseShell "-" s of
-        (ParseResult (Just (t, m)) _) -> Just . not . null $ runList [] t [f]
+        (ParseResult (Just (t, m)) _) -> Just . not . null $ runList defaultAnalysisOptions t [f]
         _ -> Nothing
 
 
@@ -460,15 +464,13 @@ prop_checkUuoc1 = verify checkUuoc "cat foo | grep bar"
 prop_checkUuoc2 = verifyNot checkUuoc "cat * | grep bar"
 prop_checkUuoc3 = verify checkUuoc "cat $var | grep bar"
 prop_checkUuoc4 = verifyNot checkUuoc "cat $var"
+prop_checkUuoc5 = verifyNot checkUuoc "cat \"$@\""
 checkUuoc _ (T_Pipeline _ _ (T_Redirecting _ _ cmd:_:_)) =
     checkCommand "cat" (const f) cmd
   where
-    f [word] = when (isSimple word) $
+    f [word] = unless (mayBecomeMultipleArgs word) $
         style (getId word) 2002 "Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead."
     f _ = return ()
-    isSimple (T_NormalWord _ parts) = all isSimple parts
-    isSimple (T_DollarBraced _ _) = True
-    isSimple x = not $ willSplit x
 checkUuoc _ _ = return ()
 
 prop_checkNeedlessCommands = verify checkNeedlessCommands "foo=$(expr 3 + 2)"
@@ -494,7 +496,7 @@ checkPipePitfalls _ (T_Pipeline id _ commands) = do
         \(find:xargs:_) ->
           let args = deadSimple xargs ++ deadSimple find
           in
-            unless (or $ map ($ args) [
+            unless (any ($ args) [
                 hasShortParameter '0',
                 hasParameter "null",
                 hasParameter "print0",
@@ -531,9 +533,9 @@ checkPipePitfalls _ (T_Pipeline id _ commands) = do
     for' l f = for l (first f)
     first func (x:_) = func (getId x)
     first _ _ = return ()
-    hasShortParameter char list = any (\x -> "-" `isPrefixOf` x && char `elem` x) list
+    hasShortParameter char = any (\x -> "-" `isPrefixOf` x && char `elem` x)
-    hasParameter string list =
+    hasParameter string =
-        any (isPrefixOf string . dropWhile (== '-')) list
+        any (isPrefixOf string . dropWhile (== '-'))
 checkPipePitfalls _ _ = return ()
 
 indexOfSublists sub = f 0
@@ -583,11 +585,18 @@ mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f t
     f (T_NormalWord _ parts) = any f parts
     f _ = False
 
-prop_checkShebang1 = verifyTree checkShebang "#!/usr/bin/env bash -x\necho cow"
+prop_checkShebangParameters1 = verifyTree checkShebangParameters "#!/usr/bin/env bash -x\necho cow"
-prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh  -l "
+prop_checkShebangParameters2 = verifyNotTree checkShebangParameters "#! /bin/sh  -l "
-checkShebang _ (T_Script id sb _) =
+checkShebangParameters _ (T_Script id sb _) =
     [Note id ErrorC 2096 "On most OS, shebangs can only specify a single parameter." | length (words sb) > 2]
 
+prop_checkShebang1 = verifyNotTree checkShebang "#!/usr/bin/env bash -x\necho cow"
+prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh  -l "
+prop_checkShebang3 = verifyTree checkShebang "ls -l"
+checkShebang params (T_Script id sb _) =
+    [Note id InfoC 2148 $ "Shebang (#!) missing. Assuming " ++ (show $ shellType params) ++ "."
+        | not (shellTypeSpecified params) && sb == "" ]
+
 prop_checkBashisms = verify checkBashisms "while read a; do :; done < <(a)"
 prop_checkBashisms2 = verify checkBashisms "[ foo -nt bar ]"
 prop_checkBashisms3 = verify checkBashisms "echo $((i++))"
@@ -608,8 +617,8 @@ prop_checkBashisms17= verify checkBashisms "echo $((RANDOM%6+1))"
 prop_checkBashisms18= verify checkBashisms "foo &> /dev/null"
 checkBashisms _ = bashism
   where
-    errMsg id s = err id 2040 $ "#!/bin/sh was specified, so " ++ s ++ " not supported, even when sh is actually bash."
+    errMsg id s = err id 2040 $ "In sh, " ++ s ++ " not supported, even when sh is actually bash."
-    warnMsg id s = warn id 2039 $ "#!/bin/sh was specified, but " ++ s ++ " not standard."
+    warnMsg id s = warn id 2039 $ "In POSIX sh, " ++ s ++ " not supported."
     bashism (T_ProcSub id _ _) = errMsg id "process substitution is"
     bashism (T_Extglob id _ _) = warnMsg id "extglob is"
     bashism (T_DollarSingleQuoted id _) = warnMsg id "$'..' is"
@@ -793,18 +802,18 @@ prop_checkRedirectToSame3 = verifyNot checkRedirectToSame "cat lol | sed -e 's/a
 prop_checkRedirectToSame4 = verifyNot checkRedirectToSame "foo /dev/null > /dev/null"
 prop_checkRedirectToSame5 = verifyNot checkRedirectToSame "foo > bar 2> bar"
 checkRedirectToSame params s@(T_Pipeline _ _ list) =
-    mapM_ (\l -> (mapM_ (\x -> doAnalysis (checkOccurences x) l) (getAllRedirs list))) list
+    mapM_ (\l -> (mapM_ (\x -> doAnalysis (checkOccurrences x) l) (getAllRedirs list))) list
   where
     note x = Note x InfoC 2094
                 "Make sure not to read and write the same file in the same pipeline."
-    checkOccurences t@(T_NormalWord exceptId x) u@(T_NormalWord newId y) =
+    checkOccurrences t@(T_NormalWord exceptId x) u@(T_NormalWord newId y) =
         when (exceptId /= newId
                 && x == y
                 && not (isOutput t && isOutput u)
                 && not (special t)) $ do
             addNote $ note newId
             addNote $ note exceptId
-    checkOccurences _ _ = return ()
+    checkOccurrences _ _ = return ()
     getAllRedirs = concatMap (\(T_Redirecting _ ls _) -> concatMap getRedirs ls)
     getRedirs (T_FdRedirect _ _ (T_IoFile _ op file)) =
             case op of T_Greater _ -> [file]
@@ -858,13 +867,12 @@ prop_checkUnquotedDollarAt3 = verifyNot checkUnquotedDollarAt "ls ${#foo[@]}"
 prop_checkUnquotedDollarAt4 = verifyNot checkUnquotedDollarAt "ls \"$@\""
 prop_checkUnquotedDollarAt5 = verifyNot checkUnquotedDollarAt "ls ${foo/@/ at }"
 prop_checkUnquotedDollarAt6 = verifyNot checkUnquotedDollarAt "a=$@"
-checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not isAssigned =
+prop_checkUnquotedDollarAt7 = verify checkUnquotedDollarAt "for f in ${var[@]}; do true; done"
+prop_checkUnquotedDollarAt8 = verifyNot checkUnquotedDollarAt "echo \"${args[@]:+${args[@]}}\""
+checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not $ isStrictlyQuoteFree (parentMap p) word =
     forM_ (take 1 $ filter isArrayExpansion parts) $ \x ->
         err (getId x) 2068
             "Double quote array expansions, otherwise they're like $* and break on spaces."
-  where
-    path = getPath (parentMap p) word
-    isAssigned = any isAssignment . take 2 $ path
 checkUnquotedDollarAt _ _ = return ()
 
 prop_checkConcatenatedDollarAt1 = verify checkConcatenatedDollarAt "echo \"foo$@\""
@@ -948,6 +956,9 @@ prop_checkSingleQuotedVariables5 = verifyNot checkSingleQuotedVariables "trap 'e
 prop_checkSingleQuotedVariables6 = verifyNot checkSingleQuotedVariables "sed -n '$p'"
 prop_checkSingleQuotedVariables6a= verify checkSingleQuotedVariables "sed -n '$pattern'"
 prop_checkSingleQuotedVariables7 = verifyNot checkSingleQuotedVariables "PS1='$PWD \\$ '"
+prop_checkSingleQuotedVariables8 = verify checkSingleQuotedVariables "find . -exec echo '$1' {} +"
+prop_checkSingleQuotedVariables9 = verifyNot checkSingleQuotedVariables "find . -exec awk '{print $1}' {} \\;"
+prop_checkSingleQuotedVariables10= verify checkSingleQuotedVariables "echo '`pwd`'"
 checkSingleQuotedVariables params t@(T_SingleQuoted id s) =
     when (s `matches` re) $
         if "sed" == commandName
@@ -959,7 +970,10 @@ checkSingleQuotedVariables params t@(T_SingleQuoted id s) =
         "Expressions don't expand in single quotes, use double quotes for that."
     commandName = fromMaybe "" $ do
         cmd <- getClosestCommand parents t
-        getCommandBasename cmd
+        name <- getCommandBasename cmd
+        if name == "find"
+            then return $ getFindCommand cmd
+            else return name
 
     isProbablyOk =
             any isOkAssignment (take 3 $ getPath parents t)
@@ -982,8 +996,18 @@ checkSingleQuotedVariables params t@(T_SingleQuoted id s) =
             T_Assignment _ _ name _ _ -> name `elem` commonlyQuoted
             otherwise -> False
 
-    re = mkRegex "\\$[{(0-9a-zA-Z_]"
+    re = mkRegex "\\$[{(0-9a-zA-Z_]|`.*`"
     sedContra = mkRegex "\\$[dpsaic]($|[^a-zA-Z])"
+
+    getFindCommand (T_SimpleCommand _ _ words) =
+        let list = map getLiteralString words
+            cmd  = dropWhile (\x -> x /= Just "-exec" && x /= Just "-execdir") list
+        in
+          case cmd of
+            (flag:cmd:rest) -> fromMaybe "find" cmd
+            _ -> "find"
+    getFindCommand (T_Redirecting _ _ cmd) = getFindCommand cmd
+    getFindCommand _ = "find"
 checkSingleQuotedVariables _ _ = return ()
 
 
@@ -1197,19 +1221,29 @@ prop_checkArithmeticDeref5 = verifyNot checkArithmeticDeref "(($1))"
 prop_checkArithmeticDeref6 = verify checkArithmeticDeref "(( a[$i] ))"
 prop_checkArithmeticDeref7 = verifyNot checkArithmeticDeref "(( 10#$n ))"
 prop_checkArithmeticDeref8 = verifyNot checkArithmeticDeref "let i=$i+1"
+prop_checkArithmeticDeref9 = verifyNot checkArithmeticDeref "(( a[foo] ))"
+prop_checkArithmeticDeref10= verifyNot checkArithmeticDeref "(( a[\\$foo] ))"
+prop_checkArithmeticDeref11= verifyNot checkArithmeticDeref "a[$foo]=wee"
+prop_checkArithmeticDeref12= verify checkArithmeticDeref "for ((i=0; $i < 3; i)); do true; done"
+prop_checkArithmeticDeref13= verifyNot checkArithmeticDeref "(( $$ ))"
 checkArithmeticDeref params t@(TA_Expansion _ [T_DollarBraced id l]) =
-    unless ((isException $ bracedString l) || (not isNormal)) $
+    unless (isException $ bracedString l) getWarning
-        style id 2004 "$ on variables in (( )) is unnecessary."
   where
     isException [] = True
-    isException s = any (`elem` "/.:#%?*@") s || isDigit (head s)
+    isException s = any (`elem` "/.:#%?*@$") s || isDigit (head s)
-    isNormal = fromMaybe True $ msum $ map isNormalContext $ (parents params t)
+    getWarning = fromMaybe noWarning . msum . map warningFor $ parents params t
-    isNormalContext t =
+    warningFor t =
         case t of
-            T_Arithmetic {} -> return True
+            T_Arithmetic {} -> return normalWarning
-            T_DollarArithmetic {} -> return True
+            T_DollarArithmetic {} -> return normalWarning
-            T_SimpleCommand {} -> return False
+            T_ForArithmetic {} -> return normalWarning
-            _ -> fail "Irrelevant"
+            TA_Index {} -> return indexWarning
+            T_SimpleCommand {} -> return noWarning
+            _ -> Nothing
+
+    normalWarning = style id 2004 "$/${} is unnecessary on arithmetic variables."
+    indexWarning = style id 2149 "Remove $/${} for numeric index, or escape it for string."
+    noWarning = return ()
 checkArithmeticDeref _ _ = return ()
 
 prop_checkArithmeticBadOctal1 = verify checkArithmeticBadOctal "(( 0192 ))"
@@ -1296,8 +1330,14 @@ getTokenMap t =
     f t = modify (Map.insert (getId t) t)
 
 
--- Is this node self quoting?
+-- Is this node self quoting for a regular element?
-isQuoteFree tree t =
+isQuoteFree = isQuoteFreeNode False
+
+-- Is this node striclty self quoting, for array expansions
+isStrictlyQuoteFree = isQuoteFreeNode True
+
+
+isQuoteFreeNode strict tree t =
     (isQuoteFreeElement t == Just True) ||
         head (mapMaybe isQuoteFreeContext (drop 1 $ getPath tree t) ++ [False])
   where
@@ -1317,14 +1357,17 @@ isQuoteFree tree t =
             T_Arithmetic {} -> return True
             T_Assignment {} -> return True
             T_Redirecting {} -> return $
-                any (isCommand t) ["local", "declare", "typeset", "export"]
+                if strict then False else
+                    -- Not true, just a hack to prevent warning about non-expansion refs
+                    any (isCommand t) ["local", "declare", "typeset", "export", "trap"]
             T_DoubleQuoted _ _ -> return True
+            T_DollarDoubleQuoted _ _ -> return True
             T_CaseExpression {} -> return True
             T_HereDoc {} -> return True
             T_DollarBraced {} -> return True
-            -- Pragmatically assume it's desirable to split here
+            -- When non-strict, pragmatically assume it's desirable to split here
-            T_ForIn {} -> return True
+            T_ForIn {} -> return (not strict)
-            T_SelectIn {} -> return True
+            T_SelectIn {} -> return (not strict)
             _ -> Nothing
 
 isParamTo tree cmd =
@@ -1385,7 +1428,7 @@ getGlobOrLiteralString = getLiteralStringExt f
 
 getLiteralStringExt more = g
   where
-    allInList = liftM concat . sequence . map g
+    allInList = liftM concat . mapM g
     g (T_DoubleQuoted _ l) = allInList l
     g (T_DollarDoubleQuoted _ l) = allInList l
     g (T_NormalWord _ l) = allInList l
@@ -1396,12 +1439,23 @@ getLiteralStringExt more = g
 
 isLiteral t = isJust $ getLiteralString t
 
--- turn a NormalWord like foo="bar $baz" into a series of constituent elements like [foo=,bar ,$baz]
+-- Get a literal string ignoring all non-literals
+onlyLiteralString :: Token -> String
+onlyLiteralString = fromJust . getLiteralStringExt (const $ return "")
+
+-- Turn a NormalWord like foo="bar $baz" into a series of constituent elements like [foo=,bar ,$baz]
 getWordParts (T_NormalWord _ l)   = concatMap getWordParts l
 getWordParts (T_DoubleQuoted _ l) = l
 getWordParts other                = [other]
 
-isCommand token str = isCommandMatch token (\cmd -> cmd  == str || ("/" ++ str) `isSuffixOf` cmd)
+getUnquotedLiteral (T_NormalWord _ list) =
+    liftM concat $ mapM str list
+  where
+    str (T_Literal _ s) = return s
+    str _ = Nothing
+getUnquotedLiteral _ = Nothing
+
+isCommand token str = isCommandMatch token (\cmd -> cmd  == str || ('/' : str) `isSuffixOf` cmd)
 isUnqualifiedCommand token str = isCommandMatch token (== str)
 
 isCommandMatch token matcher = fromMaybe False $ do
@@ -1444,18 +1498,24 @@ prop_checkUuoeCmd4 = verify checkUuoeCmd "echo \"`date`\""
 prop_checkUuoeCmd5 = verifyNot checkUuoeCmd "echo \"The time is $(date)\""
 checkUuoeCmd _ = checkUnqualifiedCommand "echo" (const f) where
     msg id = style id 2005 "Useless echo? Instead of 'echo $(cmd)', just use 'cmd'."
-    f [T_NormalWord id [T_DollarExpansion _ _]] = msg id
+    f [token] = when (tokenIsJustCommandOutput token) $ msg (getId token)
-    f [T_NormalWord id [T_DoubleQuoted _ [T_DollarExpansion _ _]]] = msg id
-    f [T_NormalWord id [T_Backticked _ _]] = msg id
-    f [T_NormalWord id [T_DoubleQuoted _ [T_Backticked _ _]]] = msg id
     f _ = return ()
 
+-- Check whether a word is entirely output from a single command
+tokenIsJustCommandOutput t = case t of
+    T_NormalWord id [T_DollarExpansion _ _] -> True
+    T_NormalWord id [T_DoubleQuoted _ [T_DollarExpansion _ _]] -> True
+    T_NormalWord id [T_Backticked _ _] -> True
+    T_NormalWord id [T_DoubleQuoted _ [T_Backticked _ _]] -> True
+    _ -> False
+
 prop_checkUuoeVar1 = verify checkUuoeVar "for f in $(echo $tmp); do echo lol; done"
 prop_checkUuoeVar2 = verify checkUuoeVar "date +`echo \"$format\"`"
 prop_checkUuoeVar3 = verifyNot checkUuoeVar "foo \"$(echo -e '\r')\""
 prop_checkUuoeVar4 = verifyNot checkUuoeVar "echo $tmp"
 prop_checkUuoeVar5 = verify checkUuoeVar "foo \"$(echo \"$(date) value:\" $value)\""
 prop_checkUuoeVar6 = verifyNot checkUuoeVar "foo \"$(echo files: *.png)\""
+prop_checkUuoeVar7 = verifyNot checkUuoeVar "foo $(echo $(bar))" -- covered by 2005
 checkUuoeVar _ p =
     case p of
         T_Backticked id [cmd] -> check id cmd
@@ -1472,8 +1532,9 @@ checkUuoeVar _ p =
 
     check id (T_Pipeline _ _ [T_Redirecting _ _ c]) = warnForEcho id c
     check _ _ = return ()
-    warnForEcho id = checkUnqualifiedCommand "echo" $ \_ vars ->
+    isCovered first rest = null rest && tokenIsJustCommandOutput first
-        unless ("-" `isPrefixOf` concat (concatMap deadSimple vars)) $
+    warnForEcho id = checkUnqualifiedCommand "echo" $ \_ vars@(first:rest) ->
+        unless (isCovered first rest || "-" `isPrefixOf` onlyLiteralString first) $
             when (all couldBeOptimized vars) $ style id 2116
                 "Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'."
 
@@ -1688,7 +1749,7 @@ checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens
         case trapped of
             T_DollarExpansion id _ -> warnAboutExpansion id
             T_DollarBraced id _ -> warnAboutExpansion id
-            T_Literal id s -> unless (s == "/") $ warnAboutLiteral id
+            T_Literal id s -> unless (s == "/" || s == "=") $ warnAboutLiteral id
             _ -> return ()
 
     check _ = return ()
@@ -1949,6 +2010,10 @@ getReferencedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Litera
         "declare" -> if "x" `elem` getFlags base
             then concatMap getReference rest
             else []
+        "trap" ->
+            case rest of
+                head:_ -> map (\x -> (head, head, x)) $ getVariablesFromLiteralToken head
+                _ -> []
         _ -> []
   where
     getReference t@(T_Assignment _ _ name _ value) = [(t, t, name)]
@@ -1969,6 +2034,7 @@ getModifiedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal
         "declare" -> concatMap getModifierParam rest
         "typeset" -> concatMap getModifierParam rest
         "local" -> concatMap getModifierParam rest
+        "set" -> [(base, base, "@", DataFrom rest)]
 
         _ -> []
   where
@@ -2010,10 +2076,39 @@ getReferencedVariables t =
             (t, t, getBracedReference str) :
                 map (\x -> (l, l, x)) (getIndexReferences str)
         TA_Expansion id _ -> maybeToList $ do
-            str <- getLiteralStringExt (const $ return "#") t
+            str <- getLiteralStringExt literalizer t
+            guard . not $ null str
+            when (isDigit $ head str) $ fail "is a number"
             return (t, t, getBracedReference str)
-        T_Assignment id Append str _ _ -> [(t, t, str)]
+        T_Assignment id mode str _ word ->
+            [(t, t, str) | mode == Append] ++ specialReferences str t word
         x -> getReferencedVariableCommand x
+  where
+    -- Try to reduce false positives for unused vars only referenced from evaluated vars
+    specialReferences name base word =
+        if name `elem` [
+            "PS1", "PS2", "PS3", "PS4",
+            "PROMPT_COMMAND"
+          ]
+        then
+            map (\x -> (base, base, x)) $
+                getVariablesFromLiteralToken word
+        else []
+
+    literalizer (TA_Index {}) = return ""  -- x[0] becomes a reference of x
+    literalizer _ = Nothing
+
+-- Try to get referenced variables from a literal string like "$foo"
+-- Ignores tons of cases like arithmetic evaluation and array indices.
+prop_getVariablesFromLiteral1 =
+    getVariablesFromLiteral "$foo${bar//a/b}$BAZ" == ["foo", "bar", "BAZ"]
+getVariablesFromLiteral string =
+    map (!! 0) $ getAllMatches variableRegex string
+  where
+    variableRegex = mkRegex "\\$\\{?([A-Za-z0-9_]+)"
+
+getVariablesFromLiteralToken token =
+    getVariablesFromLiteral (fromJust $ getLiteralStringExt (const $ return " ") token)
 
 getVariableFlow shell parents t =
     let (_, stack) = runState (doStackAnalysis startScope endScope t) []
@@ -2095,6 +2190,7 @@ 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"
+prop_checkSpacefulnessM = verifyNotTree checkSpacefulness "echo $\"$1\""
 
 checkSpacefulness params t =
     doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params)
@@ -2166,6 +2262,7 @@ prop_checkQuotesInLiterals5 = verifyNotTree checkQuotesInLiterals "param=\"--foo
 prop_checkQuotesInLiterals6 = verifyTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm $param\"; $cmd"
 prop_checkQuotesInLiterals6a= verifyNotTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm ${#param}\"; $cmd"
 prop_checkQuotesInLiterals7 = verifyTree checkQuotesInLiterals "param='my\\ file'; rm $param"
+prop_checkQuotesInLiterals8 = verifyTree checkQuotesInLiterals "param=\"/foo/'bar baz'/etc\"; rm $param"
 checkQuotesInLiterals params t =
     doVariableFlowAnalysis readF writeF Map.empty (variableFlow params)
   where
@@ -2173,7 +2270,7 @@ checkQuotesInLiterals params t =
     setQuotes name ref = modify $ Map.insert name ref
     deleteQuotes = modify . Map.delete
     parents = parentMap params
-    quoteRegex = mkRegex "\"|([= ]|^)'|'( |$)|\\\\ "
+    quoteRegex = mkRegex "\"|([/= ]|^)'|'( |$)|\\\\ "
     containsQuotes s = s `matches` quoteRegex
 
     writeF _ _ name (DataFrom values) = do
@@ -2218,6 +2315,8 @@ prop_checkFunctionsUsedExternally2 =
   verifyTree checkFunctionsUsedExternally "alias f='a'; xargs -n 1 f"
 prop_checkFunctionsUsedExternally3 =
   verifyNotTree checkFunctionsUsedExternally "f() { :; }; echo f"
+prop_checkFunctionsUsedExternally4 =
+  verifyNotTree checkFunctionsUsedExternally "foo() { :; }; sudo \"foo\""
 checkFunctionsUsedExternally params t =
     runNodeAnalysis checkCommand params t
   where
@@ -2246,13 +2345,13 @@ checkFunctionsUsedExternally params t =
         let string = concat $ deadSimple arg
         in when ('=' `elem` string) $
             modify ((takeWhile (/= '=') string, getId arg):)
-    checkArg cmd arg =
+    checkArg cmd arg = potentially $ do
-        case Map.lookup (concat $ deadSimple arg) functions of
+        literalArg <- getUnquotedLiteral arg  -- only consider unquoted literals
-            Nothing -> return ()
+        definitionId <- Map.lookup literalArg functions
-            Just id -> do
+        return $ do
             warn (getId arg) 2033
               "Shell functions can't be passed to external commands."
-              info id 2032 $
+            info definitionId 2032 $
               "Use own script or sh -c '..' to run this from " ++ cmd ++ "."
 
 prop_checkUnused0 = verifyNotTree checkUnusedAssignments "var=foo; echo $var"
@@ -2275,7 +2374,9 @@ prop_checkUnused16= verifyNotTree checkUnusedAssignments "foo=5; declare -x foo"
 prop_checkUnused17= verifyNotTree checkUnusedAssignments "read -i 'foo' -e -p 'Input: ' bar; $bar;"
 prop_checkUnused18= verifyNotTree checkUnusedAssignments "a=1; arr=( [$a]=42 ); echo \"${arr[@]}\""
 prop_checkUnused19= verifyNotTree checkUnusedAssignments "a=1; let b=a+1; echo $b"
-checkUnusedAssignments params t = snd $ runWriter (mapM_ checkAssignment flow)
+prop_checkUnused20= verifyNotTree checkUnusedAssignments "a=1; PS1='$a'"
+prop_checkUnused21= verifyNotTree checkUnusedAssignments "a=1; trap 'echo $a' INT"
+checkUnusedAssignments params t = execWriter (mapM_ warnFor unused)
   where
     flow = variableFlow params
     references = foldl (flip ($)) defaultMap (map insertRef flow)
@@ -2283,13 +2384,16 @@ checkUnusedAssignments params t = snd $ runWriter (mapM_ checkAssignment flow)
         Map.insert (stripSuffix name) ()
     insertRef _ = id
 
-    checkAssignment (Assignment (_, token, name, _)) | isVariableName name =
+    assignments = foldl (flip ($)) Map.empty (map insertAssignment flow)
-        case Map.lookup name references of
+    insertAssignment (Assignment (_, token, name, _)) | isVariableName name =
-            Just _ -> return ()
+        Map.insert name token
-            Nothing ->
+    insertAssignment _ = id
+
+    unused = Map.assocs $ Map.difference assignments references
+
+    warnFor (name, token) =
         info (getId token) 2034 $
             name ++ " appears unused. Verify it or export it."
-    checkAssignment _ = return ()
 
     stripSuffix = takeWhile isVariableChar
     defaultMap = Map.fromList $ zip internalVariables $ repeat ()
@@ -2619,6 +2723,10 @@ prop_checkUnpassedInFunctions2 = verifyNotTree checkUnpassedInFunctions "foo() {
 prop_checkUnpassedInFunctions3 = verifyNotTree checkUnpassedInFunctions "foo() { echo $lol; }; foo"
 prop_checkUnpassedInFunctions4 = verifyNotTree checkUnpassedInFunctions "foo() { echo $0; }; foo"
 prop_checkUnpassedInFunctions5 = verifyNotTree checkUnpassedInFunctions "foo() { echo $1; }; foo 'lol'; foo"
+prop_checkUnpassedInFunctions6 = verifyNotTree checkUnpassedInFunctions "foo() { set -- *; echo $1; }; foo"
+prop_checkUnpassedInFunctions7 = verifyTree checkUnpassedInFunctions "foo() { echo $1; }; foo; foo;"
+prop_checkUnpassedInFunctions8 = verifyNotTree checkUnpassedInFunctions "foo() { echo $((1)); }; foo;"
+prop_checkUnpassedInFunctions9 = verifyNotTree checkUnpassedInFunctions "foo() { echo $(($b)); }; foo;"
 checkUnpassedInFunctions params root =
     execWriter $ mapM_ warnForGroup referenceGroups
   where
@@ -2626,15 +2734,23 @@ checkUnpassedInFunctions params root =
     functionMap = Map.fromList $
         map (\t@(T_Function _ _ _ name _) -> (name,t)) functions
     functions = execWriter $ doAnalysis (tell . maybeToList . findFunction) root
-    findFunction t@(T_DollarBraced id token) = do
+
-        str <- getLiteralString token
+    findFunction t@(T_Function id _ _ name body) =
-        unless (isPositional str) $ fail "Not positional"
+        let flow = getVariableFlow (shellType params) (parentMap params) body
-        let path = getPath (parentMap params) t
+        in
-        find isFunction path
+          if any isPositionalReference flow && not (any isPositionalAssignment flow)
+            then return t
+            else Nothing
     findFunction _ = Nothing
 
-    isFunction (T_Function {}) = True
+    isPositionalAssignment x =
-    isFunction _ = False
+        case x of
+            Assignment (_, _, str, _) -> isPositional str
+            _ -> False
+    isPositionalReference x =
+        case x of
+            Reference (_, _, str) -> isPositional str
+            _ -> False
 
     referenceList :: [(String, Bool, Token)]
     referenceList = execWriter $
@@ -2711,6 +2827,22 @@ checkOverridingPath _ (T_SimpleCommand _ vars []) =
     notify id = warn id 2123 "PATH is the shell search path. Use another name."
 checkOverridingPath _ _ = return ()
 
+prop_checkTildeInPath1 = verify checkTildeInPath "PATH=\"$PATH:~/bin\""
+prop_checkTildeInPath2 = verify checkTildeInPath "PATH='~foo/bin'"
+prop_checkTildeInPath3 = verifyNot checkTildeInPath "PATH=~/bin"
+checkTildeInPath _ (T_SimpleCommand _ vars _) =
+    mapM_ checkVar vars
+  where
+    checkVar (T_Assignment id Assign "PATH" Nothing (T_NormalWord _ parts)) =
+        when (any (\x -> isQuoted x && hasTilde x) parts) $
+            warn id 2147 "Literal tilde in PATH works poorly across programs."
+    checkVar _ = return ()
+
+    hasTilde t = fromMaybe False (liftM2 elem (return '~') (getLiteralStringExt (const $ return "") t))
+    isQuoted (T_DoubleQuoted {}) = True
+    isQuoted (T_SingleQuoted {}) = True
+    isQuoted _ = False
+checkTildeInPath _ _ = return ()
 
 prop_checkUnsupported1 = verifyNot checkUnsupported "#!/bin/zsh\nfunction { echo cow; }"
 prop_checkUnsupported2 = verify checkUnsupported "#!/bin/sh\nfunction { echo cow; }"
@@ -2867,14 +2999,14 @@ prop_checkFindActionPrecedence2 = verifyNot checkFindActionPrecedence "find . -n
 prop_checkFindActionPrecedence3 = verifyNot checkFindActionPrecedence "find . -name '*.wav' -o -name '*.au'"
 checkFindActionPrecedence params = checkCommand "find" (const f)
   where
-    pattern = [isMatch, const True, isParam ["-o"], isMatch, const True, isAction]
+    pattern = [isMatch, const True, isParam ["-o", "-or"], isMatch, const True, isAction]
     f list | length list < length pattern = return ()
     f list@(_:rest) =
         if all id (zipWith ($) pattern list)
         then warnFor (list !! ((length pattern)-1))
         else f rest
-    isMatch = isParam [ "-name", "-regex", "-iname", "-iregex" ]
+    isMatch = isParam [ "-name", "-regex", "-iname", "-iregex", "-wholename", "-iwholename" ]
-    isAction = isParam [ "-exec", "-execdir", "-delete", "-print" ]
+    isAction = isParam [ "-exec", "-execdir", "-delete", "-print", "-print0" ]
     isParam strs t = fromMaybe False $ do
         param <- getLiteralString t
         return $ param `elem` strs

ShellCheck/Options.hs

@@ -0,0 +1,14 @@
+module ShellCheck.Options where
+
+data Shell = Ksh | Zsh | Sh | Bash
+    deriving (Show, Eq)
+
+data AnalysisOptions = AnalysisOptions {
+    optionShellType :: Maybe Shell,
+    optionExcludes :: [Integer]
+}
+
+defaultAnalysisOptions = AnalysisOptions {
+    optionShellType = Nothing,
+    optionExcludes = []
+}

ShellCheck/Parser.hs

@@ -16,7 +16,7 @@
     along with this program.  If not, see <http://www.gnu.org/licenses/>.
 -}
 {-# LANGUAGE NoMonomorphismRestriction, TemplateHaskell #-}
-module ShellCheck.Parser (Note(..), Severity(..), parseShell, ParseResult(..), ParseNote(..), sortNotes, noteToParseNote, runTests) where
+module ShellCheck.Parser (Note(..), Severity(..), parseShell, ParseResult(..), ParseNote(..), sortNotes, noteToParseNote, runTests, readScript) where
 
 import ShellCheck.AST
 import ShellCheck.Data
@@ -61,13 +61,13 @@ unicodeDoubleQuoteChars = "\x201C\x201D\x2033\x2036"
 
 prop_spacing = isOk spacing "  \\\n # Comment"
 spacing = do
-    x <- many (many1 linewhitespace <|> try (string "\\\n"))
+    x <- many (many1 linewhitespace <|> try (string "\\\n" >> return ""))
     optional readComment
     return $ concat x
 
 spacing1 = do
     spacing <- spacing
-    when (null spacing) $ fail "no spacing"
+    when (null spacing) $ fail "Expected whitespace"
     return spacing
 
 prop_allspacing = isOk allspacing "#foo"
@@ -84,7 +84,7 @@ allspacing = do
 
 allspacingOrFail = do
     s <- allspacing
-    when (null s) $ fail "Expected spaces"
+    when (null s) $ fail "Expected whitespace"
 
 unicodeDoubleQuote = do
     pos <- getPosition
@@ -183,9 +183,9 @@ popContext = do
         then do
             let (a:r) = v
             setCurrentContexts r
-            return [a]
+            return $ Just a
         else
-            return []
+            return Nothing
 
 pushContext c = do
     v <- getCurrentContexts
@@ -233,8 +233,8 @@ reluctantlyTill1 p end = do
 attempting rest branch =
     (try branch >> rest) <|> rest
 
-orFail parser stuff =
+orFail parser errorAction =
-    try (disregard parser) <|> (disregard stuff >> fail "nope")
+    try parser <|> (errorAction >>= fail)
 
 wasIncluded p = option False (p >> return True)
 
@@ -252,7 +252,7 @@ withContext entry p = do
         popContext
         return v
      <|> do -- p failed without consuming input, abort context
-        popContext
+        v <- popContext
         fail ""
 
 called s p = do
@@ -270,12 +270,22 @@ readConditionContents single =
                                     parseProblemAt pos WarningC 1009 "Use 'if cmd; then ..' to check exit code, or 'if [[ $(cmd) == .. ]]' to check output.")
 
   where
+    spacingOrLf = condSpacing True
+    condSpacing required = do
+        pos <- getPosition
+        space <- allspacing
+        when (required && null space) $
+            parseProblemAt pos ErrorC 1035 "You are missing a required space here."
+        when (single && '\n' `elem` space) $
+            parseProblemAt pos ErrorC 1080 "When breaking lines in [ ], you need \\ before the linefeed."
+        return space
+
     typ = if single then SingleBracket else DoubleBracket
     readCondBinaryOp = try $ do
         optional guardArithmetic
         id <- getNextId
         op <- choice (map tryOp ["==", "!=", "<=", ">=", "=~", ">", "<", "=", "\\<=", "\\>=", "\\<", "\\>"]) <|> otherOp
-        hardCondSpacing
+        spacingOrLf
         return op
       where
         tryOp s = try $ do
@@ -285,7 +295,7 @@ readConditionContents single =
         otherOp = try $ do
             id <- getNextId
             s <- readOp
-            when (s == "-a" || s == "-o") $ fail "Wrong operator"
+            when (s == "-a" || s == "-o") $ fail "Unexpected operator"
             return $ TC_Binary id typ s
 
     guardArithmetic = do
@@ -298,17 +308,14 @@ readConditionContents single =
     readCondUnaryExp = do
       op <- readCondUnaryOp
       pos <- getPosition
-      (do
+      (readCondWord >>= return . op) `orFail` do
-          arg <- readCondWord
-          return $ op arg)
-        <|> (do
           parseProblemAt pos ErrorC 1019 "Expected this to be an argument to the unary condition."
-              fail "oops")
+          return "Expected an argument for the unary operator"
 
     readCondUnaryOp = try $ do
         id <- getNextId
         s <- readOp
-        hardCondSpacing
+        spacingOrLf
         return $ TC_Unary id typ s
 
     readOp = try $ do
@@ -337,19 +344,20 @@ readConditionContents single =
 
     readCondAndOp = do
         id <- getNextId
-        x <- try (string "&&" <|> string "-a")
+        x <- try (readAndOrOp "&&" False <|> readAndOrOp "-a" True)
-        softCondSpacing
-        skipLineFeeds
         return $ TC_And id typ x
 
     readCondOrOp = do
         optional guardArithmetic
         id <- getNextId
-        x <- try (string "||" <|> string "-o")
+        x <- try (readAndOrOp "||" False <|> readAndOrOp "-o" True)
-        softCondSpacing
-        skipLineFeeds
         return $ TC_Or id typ x
 
+    readAndOrOp op requiresSpacing = do
+        x <- string op
+        condSpacing requiresSpacing
+        return x
+
     readCondNoaryOrBinary = do
       id <- getNextId
       x <- readCondWord `attempting` (do
@@ -373,16 +381,21 @@ readConditionContents single =
           id <- getNextId
           pos <- getPosition
           lparen <- try $ string "(" <|> string "\\("
-          when (single && lparen == "(") $ parseProblemAt pos ErrorC 1028 "In [..] you have to escape (). Use [[..]] instead."
+          when (single && lparen == "(") $
-          when (not single && lparen == "\\(") $ parseProblemAt pos ErrorC 1029 "In [[..]] you shouldn't escape ()."
+              parseProblemAt pos ErrorC 1028 "In [..] you have to escape (). Use [[..]] instead."
-          if single then hardCondSpacing else disregard spacing
+          when (not single && lparen == "\\(") $
+              parseProblemAt pos ErrorC 1029 "In [[..]] you shouldn't escape ()."
+          condSpacing single
           x <- readCondContents
           cpos <- getPosition
           rparen <- string ")" <|> string "\\)"
-          if single then hardCondSpacing else disregard spacing
+          condSpacing single
-          when (single && rparen == ")") $ parseProblemAt cpos ErrorC 1030 "In [..] you have to escape (). Use [[..]] instead."
+          when (single && rparen == ")") $
-          when (not single && rparen == "\\)") $ parseProblemAt cpos ErrorC 1031 "In [[..]] you shouldn't escape ()."
+              parseProblemAt cpos ErrorC 1030 "In [..] you have to escape (). Use [[..]] instead."
-          when (isEscaped lparen `xor` isEscaped rparen) $ parseProblemAt pos ErrorC 1032 "Did you just escape one half of () but not the other?"
+          when (not single && rparen == "\\)") $
+              parseProblemAt cpos ErrorC 1031 "In [[..]] you shouldn't escape ()."
+          when (isEscaped lparen `xor` isEscaped rparen) $
+              parseProblemAt pos ErrorC 1032 "Did you just escape one half of () but not the other?"
           return $ TC_Group id typ x
       where
         isEscaped ('\\':_) = True
@@ -426,21 +439,15 @@ readConditionContents single =
             str <- string "|"
             return $ T_Literal id str
 
-    skipLineFeeds = do
-        pos <- getPosition
-        spacing <- allspacing
-        when (single  && '\n' `elem` spacing) $
-            parseProblemAt pos ErrorC 1080 "In [ ] you need \\ before line feeds."
-
     readCondTerm = do
         term <- readCondNot <|> readCondExpr
-        skipLineFeeds
+        condSpacing False
         return term
 
     readCondNot = do
         id <- getNextId
         char '!'
-        softCondSpacing
+        spacingOrLf
         expr <- readCondExpr
         return $ TC_Unary id typ "!" expr
 
@@ -452,7 +459,6 @@ readConditionContents single =
     readCondContents = readCondOr
 
 
-
 prop_a1 = isOk readArithmeticContents " n++ + ++c"
 prop_a2 = isOk readArithmeticContents "$N*4-(3,2)"
 prop_a3 = isOk readArithmeticContents "n|=2<<1"
@@ -489,10 +495,10 @@ readArithmeticContents =
 
     readArrayIndex = do
         id <- getNextId
-        start <- literal "["
+        char '['
         middle <- readArithmeticContents
-        end <- literal "]"
+        char ']'
-        return $ T_NormalWord id [start, middle, end]
+        return $ TA_Index id middle
 
     literal s = do
         id <- getNextId
@@ -596,7 +602,7 @@ readArithmeticContents =
             id <- getNextId
             op <- try $ string "++" <|> string "--"
             spacing
-            return $ TA_Unary id ("|" ++ op) x
+            return $ TA_Unary id ('|':op) x
          <|>
             return x
 
@@ -613,8 +619,10 @@ prop_readCondition6 = isOk readCondition "[[ $c =~ ^[yY]$ ]]"
 prop_readCondition7 = isOk readCondition "[[ ${line} =~ ^[[:space:]]*# ]]"
 prop_readCondition8 = isOk readCondition "[[ $l =~ ogg|flac ]]"
 prop_readCondition9 = isOk readCondition "[ foo -a -f bar ]"
-prop_readCondition10= isOk readCondition "[[ a == b \n || c == d ]]"
+prop_readCondition10= isOk readCondition "[[\na == b\n||\nc == d ]]"
-prop_readCondition11= isOk readCondition "[[ a == b || \n c == d ]]"
+prop_readCondition10a= isOk readCondition "[[\na == b  ||\nc == d ]]"
+prop_readCondition10b= isOk readCondition "[[ a == b\n||\nc == 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
@@ -622,9 +630,17 @@ readCondition = called "test expression" $ do
     id <- getNextId
     open <- try (string "[[") <|> string "["
     let single = open == "["
-    condSpacingMsg False $ if single
+
-          then "You need spaces after the opening [ and before the closing ]."
+    pos <- getPosition
-          else "You need spaces after the opening [[ and before the closing ]]."
+    space <- allspacing
+    when (null space) $
+        parseProblemAt pos ErrorC 1035 $ "You need a space after the " ++
+            if single
+                then "[ and before the ]."
+                else "[[ and before the ]]."
+    when (single && '\n' `elem` space) $
+        parseProblemAt pos ErrorC 1080 "You need \\ before line feeds to break lines in [ ]."
+
     condition <- readConditionContents single
 
     cpos <- getPosition
@@ -635,14 +651,6 @@ readCondition = called "test expression" $ do
     many readCmdWord -- Read and throw away remainders to get then/do warnings. Fixme?
     return $ T_Condition id (if single then SingleBracket else DoubleBracket) condition
 
-
-hardCondSpacing = condSpacingMsg False "You need a space here."
-softCondSpacing = condSpacingMsg True "You need a space here."
-condSpacingMsg soft msg = do
-  pos <- getPosition
-  space <- spacing
-  when (null space) $ (if soft then parseNoteAt else parseProblemAt) pos ErrorC 1035 msg
-
 readAnnotationPrefix = do
     char '#'
     many linewhitespace
@@ -811,6 +819,8 @@ prop_readBackTicked3 = isWarning readBackTicked "´grep \"\\\"\"´"
 prop_readBackTicked4 = isOk readBackTicked "`echo foo\necho bar`"
 prop_readBackTicked5 = isOk readSimpleCommand "echo `foo`bar"
 prop_readBackTicked6 = isWarning readSimpleCommand "echo `foo\necho `bar"
+prop_readBackTicked7 = isOk readSimpleCommand "`#inline comment`"
+prop_readBackTicked8 = isOk readSimpleCommand "echo `#comment` \\\nbar baz"
 readBackTicked = called "backtick expansion" $ do
     id <- getNextId
     startPos <- getPosition
@@ -826,7 +836,7 @@ readBackTicked = called "backtick expansion" $ do
             suggestForgotClosingQuote startPos endPos "backtick expansion"
 
     -- Result positions may be off due to escapes
-    result <- subParse subStart readCompoundList (unEscape subString)
+    result <- subParse subStart readTermOrNone (unEscape subString)
     return $ T_Backticked id result
   where
     unEscape [] = []
@@ -1390,6 +1400,12 @@ readAndOr = do
                 then andOr
                 else T_Annotation aid annotations andOr
 
+readTermOrNone = do
+    allspacing
+    readTerm <|> do
+        eof
+        return []
+
 readTerm = do
     allspacing
     m <- readAndOr
@@ -1460,6 +1476,7 @@ readIfClause = called "if expression" $ do
     g_Fi `orFail` do
         parseProblemAt pos ErrorC 1046 "Couldn't find 'fi' for this 'if'."
         parseProblem ErrorC 1047 "Expected 'fi' matching previously mentioned 'if'."
+        return "Expected 'fi'."
 
     return $ T_IfExpression id ((condition, action):elifs) elses
 
@@ -1475,12 +1492,13 @@ readIfPart = do
     allspacing
     condition <- readTerm
 
-    optional (do
+    ifNextToken (g_Fi <|> g_Elif) $
-        try . lookAhead $ g_Fi
+        parseProblemAt pos ErrorC 1049 "Did you forget the 'then' for this 'if'?"
-        parseProblemAt pos ErrorC 1049 "Did you forget the 'then' for this 'if'?")
 
     called "then clause" $ do
-        g_Then `orFail` parseProblem ErrorC 1050 "Expected 'then'."
+        g_Then `orFail` do
+            parseProblem ErrorC 1050 "Expected 'then'."
+            return "Expected 'then'."
 
         acceptButWarn g_Semi ErrorC 1051 "No semicolons directly after 'then'."
         allspacing
@@ -1496,6 +1514,10 @@ readElifPart = called "elif clause" $ do
         parseProblemAt pos ErrorC 1075 "Use 'elif' instead of 'else if'."
     allspacing
     condition <- readTerm
+
+    ifNextToken (g_Fi <|> g_Elif) $
+        parseProblemAt pos ErrorC 1049 "Did you forget the 'then' for this 'elif'?"
+
     g_Then
     acceptButWarn g_Semi ErrorC 1052 "No semicolons directly after 'then'."
     allspacing
@@ -1514,6 +1536,11 @@ readElsePart = called "else clause" $ do
     verifyNotEmptyIf "else"
     readTerm
 
+ifNextToken parser action =
+    optional $ do
+        try . lookAhead $ parser
+        action
+
 prop_readSubshell = isOk readSubshell "( cd /foo; tar cf stuff.tar * )"
 readSubshell = called "explicit subshell" $ do
     id <- getNextId
@@ -1537,7 +1564,7 @@ readBraceGroup = called "brace group" $ do
     list <- readTerm
     char '}' <|> do
         parseProblem ErrorC 1056 "Expected a '}'. If you have one, try a ; or \\n in front of it."
-        fail "Unable to parse"
+        fail "Missing '}'"
     return $ T_BraceGroup id list
 
 prop_readWhileClause = isOk readWhileClause "while [[ -e foo ]]; do sleep 1; done"
@@ -1562,7 +1589,9 @@ readDoGroup loopPos = do
                 try . lookAhead $ g_Done
                 parseProblemAt loopPos ErrorC 1057 "Did you forget the 'do' for this loop?")
 
-    g_Do `orFail` parseProblem ErrorC 1058 "Expected 'do'."
+    g_Do `orFail` do
+        parseProblem ErrorC 1058 "Expected 'do'."
+        return "Expected 'do'."
 
     acceptButWarn g_Semi ErrorC 1059 "No semicolons directly after 'do'."
     allspacing
@@ -1575,6 +1604,7 @@ readDoGroup loopPos = do
     g_Done `orFail` do
             parseProblemAt pos ErrorC 1061 "Couldn't find 'done' for this 'do'."
             parseProblem ErrorC 1062 "Expected 'done' matching previously mentioned 'do'."
+            return "Expected 'done'."
     return commands
 
 
@@ -1588,11 +1618,12 @@ 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"
+prop_readForClause12= isWarning readForClause "for $a in *; do echo \"$a\"; done"
 readForClause = called "for loop" $ do
     pos <- getPosition
     (T_For id) <- g_For
     spacing
-    readRegular id pos <|> readArithmetic id pos
+    readArithmetic id pos <|> readRegular id pos
   where
     readArithmetic id pos = called "arithmetic for condition" $ do
         try $ string "(("
@@ -1613,6 +1644,8 @@ readForClause = called "for loop" $ do
         return list
 
     readRegular id pos = do
+        acceptButWarn (char '$') ErrorC 1086
+            "Don't use $ on the iterator name in for loops."
         names <- readNames
         readShort names <|> readLong names
       where
@@ -1687,7 +1720,10 @@ readCaseItem = called "case item" $ do
     optional g_Lparen
     spacing
     pattern <- readPattern
-    g_Rparen
+    void g_Rparen <|> do
+        parseProblem ErrorC 1085
+            "Did you forget to move the ;; after extending this case item?"
+        fail "Expected ) to open a new case item"
     readLineBreak
     list <- (lookAhead readCaseSeparator >> return []) <|> readCompoundList
     separator <- readCaseSeparator `attempting` do
@@ -1793,8 +1829,7 @@ readLetSuffix = many1 (readIoRedirect <|> try readLetExpression <|> readCmdWord)
 -- Get whatever a parser would parse as a string
 readStringForParser parser = do
     pos <- lookAhead (parser >> getPosition)
-    s <- readUntil pos
+    readUntil pos
-    return s
   where
     readUntil endPos = anyChar `reluctantlyTill` (getPosition >>= guard . (== endPos))
 
@@ -1809,6 +1844,7 @@ prop_readAssignmentWord8 = isOk readAssignmentWord "a[4''$(cat foo)]=42"
 prop_readAssignmentWord9 = isOk readAssignmentWord "IFS= "
 prop_readAssignmentWord10= isWarning readAssignmentWord "foo$n=42"
 prop_readAssignmentWord11= isOk readAssignmentWord "foo=([a]=b [c] [d]= [e f )"
+prop_readAssignmentWord12= isOk readAssignmentWord "a[b <<= 3 + c]='thing'"
 readAssignmentWord = try $ do
     id <- getNextId
     pos <- getPosition
@@ -1844,14 +1880,10 @@ readAssignmentWord = try $ do
         id <- getNextId
         return $ T_Literal id ""
 
--- This is only approximate. Fixme?
--- * Bash allows foo[' ' "" $(true) 2 ``]=var
--- * foo[bar] dereferences bar
 readArrayIndex = do
     char '['
     optional space
-    x <- readNormalishWord "]"
+    x <- readArithmeticContents
-    optional space
     char ']'
     return x
 
@@ -1903,8 +1935,8 @@ tryParseWordToken keyword t = try $ do
             "Scripts are case sensitive. Use '" ++ keyword ++ "', not '" ++ str ++ "'."
     return $ t id
 
-anycaseString =
+anycaseString str =
-    mapM anycaseChar
+    mapM anycaseChar str <?> str
   where
     anycaseChar c = char (toLower c) <|> char (toUpper c)
 
@@ -1942,10 +1974,12 @@ g_Rparen = tryToken ")" T_Rparen
 g_Bang = do
     id <- getNextId
     char '!'
-    softCondSpacing
+    void spacing1 <|> do
+        pos <- getPosition
+        parseProblemAt pos ErrorC 1035
+            "You are missing a required space after the !."
     return $ T_Bang id
 
-
 g_Semi = do
     notFollowedBy2 g_DSEMI
     tryToken ";" T_Semi
@@ -1958,12 +1992,21 @@ readKeyword = choice [ g_Then, g_Else, g_Elif, g_Fi, g_Do, g_Done, g_Esac, g_Rbr
 ifParse p t f =
     (lookAhead (try p) >> t) <|> f
 
+prop_readShebang1 = isOk readShebang "#!/bin/sh\n"
+prop_readShebang2 = isWarning readShebang "!# /bin/sh\n"
 readShebang = do
-    try $ string "#!"
+    try readCorrect <|> try readSwapped
     str <- many $ noneOf "\r\n"
     optional carriageReturn
     optional linefeed
     return str
+  where
+    readCorrect = void $ string "#!"
+    readSwapped = do
+        pos <- getPosition
+        string "!#"
+        parseProblemAt pos ErrorC 1084
+            "Use #!, not !#, for the shebang."
 
 prop_readScript1 = isOk readScript "#!/bin/bash\necho hello world\n"
 prop_readScript2 = isWarning readScript "#!/bin/bash\r\necho hello world\n"
@@ -1987,11 +2030,11 @@ readScript = do
             return $ T_Script id sb commands;
         } <|> do {
             parseProblem WarningC 1014 "Couldn't read any commands.";
-            return $ T_Script id sb [T_EOF id];
+            return $ T_Script id sb []
         }
       else do
         many anyChar
-        return $ T_Script id sb [T_EOF id];
+        return $ T_Script id sb [];
 
   where
     basename s = reverse . takeWhile (/= '/') . reverse $ s
@@ -2060,20 +2103,21 @@ sortNotes = sortBy compareNotes
 data ParseResult = ParseResult { parseResult :: Maybe (Token, Map.Map Id SourcePos), parseNotes :: [ParseNote] } deriving (Show)
 
 makeErrorFor parsecError =
-    ParseNote (errorPos parsecError) ErrorC 1072 $ getStringFromParsec $ errorMessages parsecError
+    ParseNote (errorPos parsecError) ErrorC 1072 $
+        getStringFromParsec $ errorMessages parsecError
 
 getStringFromParsec errors =
-        case map snd $ sortWith fst $ map f errors of
+        case map f errors of
-            r -> unwords (take 1 $ nub r)  ++ " Fix any mentioned problems and try again."
+            r -> unwords (take 1 $ catMaybes $ reverse r)  ++
-    where f err =
+                " Fix any mentioned problems and try again."
+    where
+        f err =
             case err of
-                UnExpect s    -> (1, unexpected s)
+                UnExpect s    ->  return $ unexpected s
-                SysUnExpect s -> (2, unexpected s)
+                SysUnExpect s ->  return $ unexpected s
-                Expect s      -> (3, "Expected " ++ s ++ ".")
+                Expect s      ->  return $ "Expected " ++ s ++ "."
-                Message s     -> (4, s ++ ".")
+                Message s     ->  if null s then Nothing else return $ s ++ "."
-          wut "" = "eof"
+        unexpected s = "Unexpected " ++ (if null s then "eof" else s) ++ "."
-          wut x = x
-          unexpected s = "Unexpected " ++ wut s ++ "."
 
 parseShell filename contents =
     case rp (parseWithNotes readScript) filename contents of

ShellCheck/Simple.hs

@@ -18,15 +18,16 @@
 {-# LANGUAGE TemplateHaskell #-}
 module ShellCheck.Simple (shellCheck, ShellCheckComment, scLine, scColumn, scSeverity, scCode, scMessage, runTests) where
 
-import ShellCheck.Parser hiding (runTests)
-import ShellCheck.Analytics hiding (runTests)
-import Data.Maybe
-import Text.Parsec.Pos
 import Data.List
+import Data.Maybe
+import ShellCheck.Analytics hiding (runTests)
+import ShellCheck.Options
+import ShellCheck.Parser hiding (runTests)
 import Test.QuickCheck.All (quickCheckAll)
+import Text.Parsec.Pos
 
-shellCheck :: String -> [AnalysisOption] -> [ShellCheckComment]
+shellCheck :: AnalysisOptions -> String -> [ShellCheckComment]
-shellCheck script options =
+shellCheck options script =
     let (ParseResult result notes) = parseShell "-" script in
         let allNotes = notes ++ concat (maybeToList $ do
             (tree, posMap) <- result
@@ -51,21 +52,25 @@ severityToString s =
 formatNote (ParseNote pos severity code text) =
     ShellCheckComment (sourceLine pos) (sourceColumn pos) (severityToString severity) (fromIntegral code) text
 
+testCheck = shellCheck defaultAnalysisOptions { optionExcludes = [2148] } -- Ignore #! warnings
 prop_findsParseIssue =
-    let comments = shellCheck "echo \"$12\"" [] in
+    let comments = testCheck "echo \"$12\"" in
         length comments == 1 && scCode (head comments) == 1037
 prop_commentDisablesParseIssue1 =
-    null $ shellCheck "#shellcheck disable=SC1037\necho \"$12\"" []
+    null $ testCheck "#shellcheck disable=SC1037\necho \"$12\""
 prop_commentDisablesParseIssue2 =
-    null $ shellCheck "#shellcheck disable=SC1037\n#lol\necho \"$12\"" []
+    null $ testCheck "#shellcheck disable=SC1037\n#lol\necho \"$12\""
 
 prop_findsAnalysisIssue =
-    let comments = shellCheck "echo $1" [] in
+    let comments = testCheck "echo $1" in
         length comments == 1 && scCode (head comments) == 2086
 prop_commentDisablesAnalysisIssue1 =
-    null $ shellCheck "#shellcheck disable=SC2086\necho $1" []
+    null $ testCheck "#shellcheck disable=SC2086\necho $1"
 prop_commentDisablesAnalysisIssue2 =
-    null $ shellCheck "#shellcheck disable=SC2086\n#lol\necho $1" []
+    null $ testCheck "#shellcheck disable=SC2086\n#lol\necho $1"
+
+prop_optionDisablesIssue1 =
+    null $ shellCheck (defaultAnalysisOptions { optionExcludes = [2086, 2148] }) "echo $1"
 
 return []
 runTests = $quickCheckAll

shellcheck.1.md

@@ -18,24 +18,28 @@ corner cases can cause delayed failures.
 
 # OPTIONS
 
-**-f** *FORMAT*, **--format=***FORMAT*
-
-:   Specify the output format of shellcheck, which prints its results in the
-    standard output. Subsequent **-f** options are ignored, see **FORMATS**
-    below for more information.
-
 **-e**\ *CODE1*[,*CODE2*...],\ **--exclude=***CODE1*[,*CODE2*...]
 
 :   Explicitly exclude the specified codes from the report. Subsequent **-e**
     options are cumulative, but all the codes can be specified at once,
     comma-separated as a single argument.
 
+**-f** *FORMAT*, **--format=***FORMAT*
+
+:   Specify the output format of shellcheck, which prints its results in the
+    standard output. Subsequent **-f** options are ignored, see **FORMATS**
+    below for more information.
+
 **-s**\ *shell*,\ **--shell=***shell*
 
 :   Specify Bourne shell dialect. Valid values are *sh*, *bash*, *ksh* and
     *zsh*. The default is to use the file's shebang, or *bash* if the target
     shell can't be determined.
 
+**-V**\ *version*,\ **--version**
+
+:   Print version and exit.
+
 # FORMATS
 
 **tty**

shellcheck.hs

@@ -17,12 +17,16 @@
 -}
 import Control.Exception
 import Control.Monad
+import Control.Monad.Trans
+import Control.Monad.Trans.Error
 import Data.Char
 import Data.Maybe
+import Data.Monoid
 import GHC.Exts
 import GHC.IO.Device
 import Prelude hiding (catch)
 import ShellCheck.Data
+import ShellCheck.Options
 import ShellCheck.Simple
 import ShellCheck.Analytics
 import System.Console.GetOpt
@@ -34,23 +38,29 @@ import Text.JSON
 import qualified Data.Map as Map
 
 data Flag = Flag String String
+data Status = NoProblems | SomeProblems | BadInput | SupportFailure | SyntaxFailure | RuntimeException deriving (Ord, Eq)
+
+instance Error Status where
+    noMsg = RuntimeException
+
+instance Monoid Status where
+    mempty = NoProblems
+    mappend = max
 
 header = "Usage: shellcheck [OPTIONS...] FILES..."
 options = [
-    Option ['f'] ["format"]
+    Option "e" ["exclude"]
-        (ReqArg (Flag "format") "FORMAT") "output format",
-    Option ['e'] ["exclude"]
         (ReqArg (Flag "exclude") "CODE1,CODE2..") "exclude types of warnings",
-    Option ['s'] ["shell"]
+    Option "f" ["format"]
+        (ReqArg (Flag "format") "FORMAT") "output format",
+    Option "s" ["shell"]
         (ReqArg (Flag "shell") "SHELLNAME") "Specify dialect (bash,sh,ksh,zsh)",
-    Option ['V'] ["version"]
+    Option "V" ["version"]
         (NoArg $ Flag "version" "true") "Print version information"
     ]
 
 printErr = hPutStrLn stderr
 
-syntaxFailure = ExitFailure 3
-supportFailure = ExitFailure 4
 
 instance JSON ShellCheckComment where
   showJSON c = makeObj [
@@ -62,16 +72,15 @@ instance JSON ShellCheckComment where
       ]
   readJSON = undefined
 
+parseArguments :: [String] -> ErrorT Status IO ([Flag], [FilePath])
 parseArguments argv =
     case getOpt Permute options argv of
-        (opts, files, []) -> do
+        (opts, files, []) -> return (opts, files)
-            verifyOptions opts files
-            return $ Just (opts, files)
-
         (_, _, errors) -> do
-            printErr $ concat errors ++ "\n" ++ usageInfo header options
+            liftIO . printErr $ concat errors ++ "\n" ++ usageInfo header options
-            exitWith syntaxFailure
+            throwError SyntaxFailure
 
+formats :: Map.Map String (AnalysisOptions -> [FilePath] -> IO Status)
 formats = Map.fromList [
     ("json", forJson),
     ("gcc", forGcc),
@@ -79,9 +88,21 @@ formats = Map.fromList [
     ("tty", forTty)
     ]
 
+toStatus = liftM (either id (const NoProblems)) . runErrorT
+
+catchExceptions :: IO Status -> IO Status
+catchExceptions action = action -- action `catch` handler
+  where
+    handler err = do
+        printErr $ show (err :: SomeException)
+        return RuntimeException
+
+checkComments comments = if null comments then NoProblems else SomeProblems
+
+forTty :: AnalysisOptions -> [FilePath] -> IO Status
 forTty options files = do
     output <- mapM doFile files
-    return $ and output
+    return $ mconcat output
   where
     clear = ansi 0
     ansi n = "\x1B[" ++ show n ++ "m"
@@ -97,7 +118,7 @@ forTty options files = do
     colorComment level comment =
         ansi (colorForLevel level) ++ comment ++ clear
 
-    doFile path = do
+    doFile path = catchExceptions $ do
         contents <- readContents path
         doInput path contents
 
@@ -119,34 +140,36 @@ forTty options files = do
             mapM_ (\c -> putStrLn (colorFunc (scSeverity c) $ cuteIndent c)) x
             putStrLn ""
           ) groups
-        return $ null comments
+        return . checkComments $ comments
 
     cuteIndent comment =
         replicate (scColumn comment - 1) ' ' ++
             "^-- " ++ code (scCode comment) ++ ": " ++ scMessage comment
 
-    code code = "SC" ++ (show code)
+    code code = "SC" ++ show code
 
     getColorFunc = do
         term <- hIsTerminalDevice stdout
         return $ if term then colorComment else const id
 
 -- This totally ignores the filenames. Fixme?
-forJson options files = do
+forJson :: AnalysisOptions -> [FilePath] -> IO Status
+forJson options files = catchExceptions $ do
     comments <- liftM concat $ mapM (commentsFor options) files
     putStrLn $ encodeStrict comments
-    return . null $ comments
+    return $ checkComments comments
 
 -- Mimic GCC "file:line:col: (error|warning|note): message" format
+forGcc :: AnalysisOptions -> [FilePath] -> IO Status
 forGcc options files = do
     files <- mapM process files
-    return $ and files
+    return $ mconcat files
   where
-    process file = do
+    process file = catchExceptions $ do
         contents <- readContents file
         let comments = makeNonVirtual (getComments options contents) contents
         mapM_ (putStrLn . format file) comments
-        return $ null comments
+        return $ checkComments comments
 
     format filename c = concat [
             filename, ":",
@@ -162,20 +185,18 @@ forGcc options files = do
       ]
 
 -- Checkstyle compatible output. A bit of a hack to avoid XML dependencies
+forCheckstyle :: AnalysisOptions -> [FilePath] -> IO Status
 forCheckstyle options files = do
     putStrLn "<?xml version='1.0' encoding='UTF-8'?>"
     putStrLn "<checkstyle version='4.3'>"
-    statuses <- mapM (\x -> process x `catch` report) files
+    statuses <- mapM process files
     putStrLn "</checkstyle>"
-    return $ and statuses
+    return $ mconcat statuses
   where
-    process file = do
+    process file = catchExceptions $ do
         comments <- commentsFor options file
         putStrLn (formatFile file comments)
-        return $ null comments
+        return $ checkComments comments
-    report error = do
-        printErr $ show (error :: SomeException)
-        return False
 
     severity "error" = "error"
     severity "warning" = "warning"
@@ -197,31 +218,31 @@ forCheckstyle options files = do
         attr "column" $ show . scColumn $ c,
         attr "severity" $ severity . scSeverity $ c,
         attr "message" $ scMessage c,
-        attr "source" $ "ShellCheck.SC" ++ (show $ scCode c),
+        attr "source" $ "ShellCheck.SC" ++ show (scCode c),
         "/>\n"
         ]
 
-commentsFor options file =
+commentsFor options file = liftM (getComments options) $ readContents file
-    liftM (getComments options) $ readContents file
 
-getComments options contents =
+getComments = shellCheck
-    excludeCodes (getExclusions options) $ shellCheck contents analysisOptions
-  where
-    analysisOptions = catMaybes [ shellOption ]
-    shellOption = do
-        option <- getOption options "shell"
-        sh <- shellForExecutable option
-        return $ ForceShell sh
 
-
+readContents :: FilePath -> IO String
-readContents file = if file == "-" then getContents else readFile file
+readContents file =
+    if file == "-"
+    then getContents
+    else readFile file
 
 -- Realign comments from a tabstop of 8 to 1
 makeNonVirtual comments contents =
     map fix comments
   where
     ls = lines contents
-    fix c = c { scColumn = real (ls !! (scLine c - 1)) 0 0 (scColumn c) }
+    fix c = c {
+        scColumn =
+            if scLine c > 0 && scLine c <= length ls
+            then real (ls !! (scLine c - 1)) 0 0 (scColumn c)
+            else scColumn c
+    }
     real _ r v target | target <= v = r
     real [] r v _ = r -- should never happen
     real ('\t':rest) r v target =
@@ -240,7 +261,7 @@ split char str =
   where
     split' (a:rest) element =
         if a == char
-        then (reverse element) : split' rest []
+        then reverse element : split' rest []
         else split' rest (a:element)
     split' [] element = [reverse element]
 
@@ -257,45 +278,71 @@ excludeCodes codes =
 
 main = do
     args <- getArgs
-    parsedArgs <- parseArguments args
+    status <- toStatus $ do
-    code <- do
+        (flags, files) <- parseArguments args
-        status <- process parsedArgs
+        process flags files
-        return $ if status then ExitSuccess else ExitFailure 1
+    exitWith $ statusToCode status
-     `catch` return
-     `catch` \err -> do
-        printErr $ show (err :: SomeException)
-        return $ ExitFailure 2
-    exitWith code
 
-process Nothing = return False
+statusToCode status =
-process (Just (options, files)) =
+    case status of
-  let format = fromMaybe "tty" $ getOption options "format" in
+        NoProblems -> ExitSuccess
+        SomeProblems -> ExitFailure 1
+        BadInput -> ExitFailure 5
+        SyntaxFailure -> ExitFailure 3
+        SupportFailure -> ExitFailure 4
+        RuntimeException -> ExitFailure 2
+
+process :: [Flag] -> [FilePath] -> ErrorT Status IO ()
+process flags files = do
+    options <- foldM (flip parseOption) defaultAnalysisOptions flags
+    verifyFiles files
+    let format = fromMaybe "tty" $ getOption flags "format"
     case Map.lookup format formats of
         Nothing -> do
+            liftIO $ do
                 printErr $ "Unknown format " ++ format
-            printErr $ "Supported formats:"
+                printErr "Supported formats:"
                 mapM_ (printErr . write) $ Map.keys formats
-            exitWith supportFailure
+            throwError SupportFailure
           where write s = "  " ++ s
-        Just f -> do
+        Just f -> ErrorT $ liftM Left $ f options files
-            f options files
 
-verifyOptions opts files = do
+parseOption flag options =
-    when (isJust $ getOption opts "version") printVersionAndExit
+    case flag of
+        Flag "shell" str ->
+                fromMaybe (die $ "Unknown shell: " ++ str) $ do
+                    shell <- shellForExecutable str
+                    return $ return options { optionShellType = Just shell }
 
-    let shell = getOption opts "shell" in
+        Flag "exclude" str -> do
-        when (isJust shell && isNothing (shell >>= shellForExecutable)) $ do
+            new <- mapM parseNum $ split ',' str
-            printErr $ "Unknown shell: " ++ (fromJust shell)
+            let old = optionExcludes options
-            exitWith supportFailure
+            return options { optionExcludes = new ++ old }
 
+        Flag "version" _ -> do
+            liftIO printVersion
+            throwError NoProblems
+
+        _ -> return options
+  where
+    die s = do
+        liftIO $ printErr s
+        throwError SupportFailure
+    parseNum ('S':'C':str) = parseNum str
+    parseNum num = do
+        unless (all isDigit num) $ do
+            liftIO . printErr $ "Bad exclusion: " ++ num
+            throwError SyntaxFailure
+        return (Prelude.read num :: Integer)
+
+verifyFiles files =
     when (null files) $ do
-        printErr "No files specified.\n"
+        liftIO $ printErr "No files specified.\n"
-        printErr $ usageInfo header options
+        liftIO $ printErr $ usageInfo header options
-        exitWith syntaxFailure
+        throwError SyntaxFailure
 
-printVersionAndExit = do
+printVersion = do
-    putStrLn $ "ShellCheck - shell script analysis tool"
+    putStrLn   "ShellCheck - shell script analysis tool"
     putStrLn $ "version: " ++ shellcheckVersion
-    putStrLn $ "license: GNU Affero General Public License, version 3"
+    putStrLn   "license: GNU Affero General Public License, version 3"
-    putStrLn $ "website: http://www.shellcheck.net"
+    putStrLn   "website: http://www.shellcheck.net"
-    exitWith ExitSuccess