Improve Fix memory usage

Fix build with ghc 8.6.1

.github/ISSUE_TEMPLATE.md

@@ -1,8 +1,8 @@
 #### For bugs
 - Rule Id (if any, e.g. SC1000): 
 - My shellcheck version (`shellcheck --version` or "online"): 
+- [ ] I read the issue's wiki page, e.g. https://github.com/koalaman/shellcheck/wiki/SC2086
 - [ ] I tried on shellcheck.net and verified that this is still a problem on the latest commit
-- [ ] It's not reproducible on shellcheck.net, but I think that's because it's an OS, configuration or encoding issue
 
 #### For new checks and feature suggestions
 - [ ] shellcheck.net (i.e. the latest commit) currently gives no useful warnings about this

.prepare_deploy

@@ -27,7 +27,7 @@ do
   zip "${file%.*}.zip" README.txt LICENSE.txt "$file"
 done
 
-for file in *.linux
+for file in *.linux-x86_64
 do
   base="${file%.*}"
   cp "$file" "shellcheck"
@@ -35,6 +35,14 @@ do
   rm "shellcheck"
 done
 
+for file in *.linux-armv6hf
+do
+  base="${file%.*}"
+  cp "$file" "shellcheck"
+  tar -cJf "$base.linux.armv6hf.tar.xz" --transform="s:^:$base/:" README.txt LICENSE.txt shellcheck
+  rm "shellcheck"
+done
+
 for file in ./*
 do
   sha512sum "$file" > "$file.sha512sum"

.travis.yml

@@ -11,6 +11,7 @@ before_install:
   - TAGS=""
   - test "$TRAVIS_BRANCH" = master && TAGS="$TAGS latest" || true
   - test -n "$TRAVIS_TAG"          && TAGS="$TAGS stable $TRAVIS_TAG" || true
+  - echo "Tags are $TAGS"
 
 script:
   - mkdir deploy
@@ -29,18 +30,21 @@ script:
   - docker rm "$id"
   - ls -l shellcheck
   - ./shellcheck myscript
-  - for tag in $TAGS; do cp "shellcheck" "deploy/shellcheck-$tag.linux"; done
+  - for tag in $TAGS; do cp "shellcheck" "deploy/shellcheck-$tag.linux-x86_64"; done
   # Linux Alpine based Docker image
   - name="$DOCKER_BASE-alpine"
   - DOCKER_BUILDS="$DOCKER_BUILDS $name"
   - sed -e '/DELETE-MARKER/,$d' Dockerfile > Dockerfile.alpine
   - docker build -f Dockerfile.alpine -t "$name:current" .
   - docker run "$name:current" sh -c 'shellcheck --version'
+  # Linux armv6hf static executable
+  - docker run -v "$PWD:/mnt" koalaman/armv6hf-builder -c 'compile-shellcheck'
+  - for tag in $TAGS; do cp "shellcheck" "deploy/shellcheck-$tag.linux-armv6hf"; done
+  - rm -f shellcheck || true
   # Windows .exe
-  - docker pull koalaman/winghc
+  - docker run --user="$UID" -v "$PWD:/appdata" koalaman/winghc cuib
-  - docker run --user="$UID" --rm -v "$PWD:/appdata" koalaman/winghc cuib
   - for tag in $TAGS; do cp "dist/build/ShellCheck/shellcheck.exe" "deploy/shellcheck-$tag.exe"; done
-  - rm -rf dist || true
+  - rm -rf dist shellcheck || true
   # Misc packaging
   - ./.prepare_deploy
 

CHANGELOG.md

@@ -1,3 +1,18 @@
+## ???
+### Added
+- Command line option --severity/-S for filtering by minimum severity
+- Command line option --wiki-link-count/-W for showing wiki links
+- SC2236/SC2237: Suggest -n/-z instead of ! -z/-n
+- SC2238: Warn when redirecting to a known command name, e.g. ls > rm
+- SC2239: Warn if the shebang is not an absolute path, e.g. #!bin/sh
+- SC1133: Better diagnostics when starting a line with |/||/&&
+### Changed
+- Most warnings now have useful end positions
+- SC1117 about unknown double-quoted escape sequences has been retired
+### Fixed
+- SC2021 no longer triggers for equivalence classes like '[=e=]'
+- SC2221/SC2222 no longer mistriggers on fall-through case branches
+
 ## v0.5.0 - 2018-05-31
 ### Added
 - SC2233/SC2234/SC2235: Suggest removing or replacing (..) around tests

Dockerfile

@@ -9,13 +9,13 @@ RUN apt-get update && apt-get install -y ghc cabal-install
 # Install Haskell deps
 # (This is a separate copy/run so that source changes don't require rebuilding)
 COPY ShellCheck.cabal ./
-RUN cabal update && cabal install --dependencies-only
+RUN cabal update && cabal install --dependencies-only --ghc-options="-optlo-Os -split-sections"
 
 # Copy source and build it
 COPY LICENSE Setup.hs shellcheck.hs ./
 COPY src src
 RUN cabal build Paths_ShellCheck && \
-  ghc -optl-static -optl-pthread -isrc -idist/build/autogen --make shellcheck && \
+  ghc -optl-static -optl-pthread -isrc -idist/build/autogen --make shellcheck -split-sections -optc-Wl,--gc-sections -optlo-Os && \
   strip --strip-all shellcheck
 
 RUN mkdir -p /out/bin && \

LICENSE

@@ -1,3 +1,13 @@
+Employer mandated disclaimer:
+
+  I am providing code in the repository to you under an open source license.
+  Because this is my personal repository, the license you receive to my code is
+  from me and other individual contributors, and not my employer (Facebook).
+
+  - Vidar "koala_man" Holen
+
+----
+
                     GNU GENERAL PUBLIC LICENSE
                        Version 3, 29 June 2007
 

README.md

@@ -4,7 +4,7 @@
 
 ShellCheck is a GPLv3 tool that gives warnings and suggestions for bash/sh shell scripts:
 
-![Screenshot of a terminal showing problematic shell script lines highlighted](doc/terminal.png).
+![Screenshot of a terminal showing problematic shell script lines highlighted](doc/terminal.png)
 
 The goals of ShellCheck are
 
@@ -70,7 +70,7 @@ You can see ShellCheck suggestions directly in a variety of editors.
 
 ![Screenshot of Vim showing inlined shellcheck feedback](doc/vim-syntastic.png).
 
-* Emacs, through [Flycheck](https://github.com/flycheck/flycheck):
+* Emacs, through [Flycheck](https://github.com/flycheck/flycheck) or [Flymake](https://github.com/federicotdn/flymake-shellcheck):
 
 ![Screenshot of emacs showing inlined shellcheck feedback](doc/emacs-flycheck.png).
 
@@ -163,6 +163,7 @@ or use `koalaman/shellcheck-alpine` if you want a larger Alpine Linux based imag
 Alternatively, you can download pre-compiled binaries for the latest release here:
 
 * [Linux, x86_64](https://storage.googleapis.com/shellcheck/shellcheck-stable.linux.x86_64.tar.xz) (statically linked)
+* [Linux, armv6hf](https://storage.googleapis.com/shellcheck/shellcheck-stable.linux.armv6hf.tar.xz), i.e. Raspberry Pi (statically linked)
 * [Windows, x86](https://storage.googleapis.com/shellcheck/shellcheck-stable.zip)
 
 or see the [storage bucket listing](https://shellcheck.storage.googleapis.com/index.html) for checksums, older versions and the latest daily builds.
@@ -171,19 +172,21 @@ or see the [storage bucket listing](https://shellcheck.storage.googleapis.com/in
 
 Travis CI has now integrated ShellCheck by default, so you don't need to manually install it.
 
-If you still want to do so in order to upgrade at your leisure or ensure the latest release:
+If you still want to do so in order to upgrade at your leisure or ensure the latest release, follow the steps to install the shellcheck binary, bellow.
 
-    install:
+## Installing the shellcheck binary
 
-      # Install a custom version of shellcheck instead of Travis CI's default
+*Pre-requisite*: the program 'xz' needs to be installed on the system.  
-      - scversion="stable" # or "v0.4.7", or "latest"
+To install it on debian/ubuntu/linux mint, run `apt install xz-utils`.  
-      - wget "https://storage.googleapis.com/shellcheck/shellcheck-$scversion.linux.x86_64.tar.xz"
+To install it on Redhat/Fedora/CentOS, run `yum -y install xz`.  
-      - tar --xz -xvf "shellcheck-$scversion.linux.x86_64.tar.xz"
-      - shellcheck() { "shellcheck-$scversion/shellcheck" "$@"; }
-      - shellcheck --version
 
-    script:
+```bash
-      - shellcheck *.sh
+export scversion="stable" # or "v0.4.7", or "latest"
+wget "https://storage.googleapis.com/shellcheck/shellcheck-${scversion}.linux.x86_64.tar.xz"
+tar --xz -xvf shellcheck-"${scversion}".linux.x86_64.tar.xz
+cp shellcheck-"${scversion}"/shellcheck /usr/bin/
+shellcheck --version
+```
 
 ## Compiling from source
 
@@ -442,3 +445,8 @@ ShellCheck is licensed under the GNU General Public License, v3. A copy of this
 Copyright 2012-2018, Vidar 'koala_man' Holen and contributors.
 
 Happy ShellChecking!
+
+
+## Other Resources                                                                          
+* The wiki has [long form descriptions](https://github.com/koalaman/shellcheck/wiki/Checks) for each warning, e.g. [SC2221](https://github.com/koalaman/shellcheck/wiki/SC2221).
+* ShellCheck does not attempt to enforce any kind of formatting or indenting style, so also check out [shfmt](https://github.com/mvdan/sh)!

ShellCheck.cabal

@@ -35,7 +35,7 @@ custom-setup
   setup-depends:
     base    >= 4    && <5,
     process >= 1.0  && <1.7,
-    Cabal   >= 1.10 && <2.3
+    Cabal   >= 1.10 && <2.5
 
 source-repository head
     type: git
@@ -53,6 +53,7 @@ library
       base > 4.6.0.1 && < 5,
       bytestring,
       containers >= 0.5,
+      deepseq >= 1.4.0.0,
       directory,
       mtl >= 2.2.1,
       parsec,
@@ -89,6 +90,7 @@ executable shellcheck
       aeson,
       base >= 4 && < 5,
       bytestring,
+      deepseq >= 1.4.0.0,
       ShellCheck,
       containers,
       directory,
@@ -104,6 +106,7 @@ test-suite test-shellcheck
       aeson,
       base >= 4 && < 5,
       bytestring,
+      deepseq >= 1.4.0.0,
       ShellCheck,
       containers,
       directory,

quickrun

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # quickrun runs ShellCheck in an interpreted mode.
 # This allows testing changes without recompiling.
 

quicktest

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # quicktest runs the ShellCheck unit tests in an interpreted mode.
 # This allows running tests without compiling, which can be faster.
 # 'cabal test' remains the source of truth.

shellcheck.1.md

@@ -56,6 +56,11 @@ not warn at all, as `ksh` supports decimals in arithmetic contexts.
     standard output. Subsequent **-f** options are ignored, see **FORMATS**
     below for more information.
 
+**-S**\ *SEVERITY*,\ **--severity=***severity*
+
+:   Specify minimum severity of errors to consider. Valid values are *error*,
+    *warning*, *info* and *style*.    The default is *style*.
+
 **-s**\ *shell*,\ **--shell=***shell*
 
 :   Specify Bourne shell dialect. Valid values are *sh*, *bash*, *dash* and *ksh*.
@@ -66,6 +71,11 @@ not warn at all, as `ksh` supports decimals in arithmetic contexts.
 
 :   Print version information and exit.
 
+**-W** *NUM*,\ **--wiki-link-count=NUM**
+
+:   For TTY output, show *NUM* wiki links to more information about mentioned
+    warnings. Set to 0 to disable them entirely.
+
 **-x**,\ **--external-sources**
 
 :   Follow 'source' statements even when the file is not specified as input.

shellcheck.hs

@@ -67,15 +67,17 @@ instance Monoid Status where
 data Options = Options {
     checkSpec        :: CheckSpec,
     externalSources  :: Bool,
-    formatterOptions :: FormatterOptions
+    formatterOptions :: FormatterOptions,
+    minSeverity      :: Severity
 }
 
 defaultOptions = Options {
     checkSpec = emptyCheckSpec,
     externalSources = False,
-    formatterOptions = FormatterOptions {
+    formatterOptions = newFormatterOptions {
         foColorOption = ColorAuto
-    }
+    },
+    minSeverity = StyleC
 }
 
 usageHeader = "Usage: shellcheck [OPTIONS...] FILES..."
@@ -93,8 +95,14 @@ options = [
     Option "s" ["shell"]
         (ReqArg (Flag "shell") "SHELLNAME")
         "Specify dialect (sh, bash, dash, ksh)",
+    Option "S" ["severity"]
+        (ReqArg (Flag "severity") "SEVERITY")
+        "Minimum severity of errors to consider (error, warning, info, style)",
     Option "V" ["version"]
         (NoArg $ Flag "version" "true") "Print version information",
+    Option "W" ["wiki-link-count"]
+        (ReqArg (Flag "wiki-link-count") "NUM")
+        "The number of wiki links to show, when applicable.",
     Option "x" ["external-sources"]
         (NoArg $ Flag "externals" "true") "Allow 'source' outside of FILES"
     ]
@@ -137,12 +145,6 @@ split char str =
         else split' rest (a:element)
     split' [] element = [reverse element]
 
-getExclusions options =
-    let elements = concatMap (split ',') $ getOptions options "exclude"
-        clean = dropWhile (not . isDigit)
-    in
-        map (Prelude.read . clean) elements :: [Int]
-
 toStatus = fmap (either id id) . runExceptT
 
 getEnvArgs = do
@@ -222,12 +224,28 @@ runFormatter sys format options files = do
                 then NoProblems
                 else SomeProblems
 
-parseColorOption colorOption =
+parseEnum name value list =
-    case colorOption of
+    case filter ((== value) . fst) list of
-        "auto"   -> ColorAuto
+        [(name, value)] -> return value
-        "always" -> ColorAlways
+        [] -> do
-        "never"  -> ColorNever
+            printErr $ "Unknown value for --" ++ name ++ ". " ++
-        _        -> error $ "Bad value for --color `" ++ colorOption ++ "'"
+                       "Valid options are: " ++ (intercalate ", " $ map fst list)
+            throwError SupportFailure
+
+parseColorOption value =
+    parseEnum "color" value [
+        ("auto",   ColorAuto),
+        ("always", ColorAlways),
+        ("never",  ColorNever)
+        ]
+
+parseSeverityOption value =
+    parseEnum "severity" value [
+        ("error",   ErrorC),
+        ("warning", WarningC),
+        ("info",    InfoC),
+        ("style",   StyleC)
+        ]
 
 parseOption flag options =
     case flag of
@@ -241,7 +259,7 @@ parseOption flag options =
                         }
 
         Flag "exclude" str -> do
-            new <- mapM parseNum $ split ',' str
+            new <- mapM parseNum $ filter (not . null) $ split ',' str
             let old = csExcludedWarnings . checkSpec $ options
             return options {
                 checkSpec = (checkSpec options) {
@@ -258,10 +276,11 @@ parseOption flag options =
                 externalSources = True
             }
 
-        Flag "color" color ->
+        Flag "color" color -> do
+            option <- parseColorOption color
             return options {
                 formatterOptions = (formatterOptions options) {
-                    foColorOption = parseColorOption color
+                    foColorOption = option
                 }
             }
 
@@ -272,6 +291,22 @@ parseOption flag options =
                 }
             }
 
+        Flag "severity" severity -> do
+            option <- parseSeverityOption severity
+            return options {
+                checkSpec = (checkSpec options) {
+                    csMinSeverity = option
+                }
+            }
+
+        Flag "wiki-link-count" countString -> do
+            count <- parseNum countString
+            return options {
+                formatterOptions = (formatterOptions options) {
+                    foWikiLinkCount = count
+                }
+            }
+
         _ -> return options
   where
     die s = do
@@ -280,7 +315,7 @@ parseOption flag options =
     parseNum ('S':'C':str) = parseNum str
     parseNum num = do
         unless (all isDigit num) $ do
-            printErr $ "Bad exclusion: " ++ num
+            printErr $ "Invalid number: " ++ num
             throwError SyntaxFailure
         return (Prelude.read num :: Integer)
 

src/ShellCheck/AST.hs

@@ -17,14 +17,17 @@
     You should have received a copy of the GNU General Public License
     along with this program.  If not, see <https://www.gnu.org/licenses/>.
 -}
+{-# LANGUAGE DeriveGeneric, DeriveAnyClass #-}
 module ShellCheck.AST where
 
+import GHC.Generics (Generic)
 import Control.Monad.Identity
+import Control.DeepSeq
 import Text.Parsec
 import qualified ShellCheck.Regex as Re
 import Prelude hiding (id)
 
-newtype Id = Id Int deriving (Show, Eq, Ord)
+newtype Id = Id Int deriving (Show, Eq, Ord, Generic, NFData)
 
 data Quoted = Quoted | Unquoted deriving (Show, Eq)
 data Dashed = Dashed | Undashed deriving (Show, Eq)

src/ShellCheck/ASTLib.hs

@@ -23,6 +23,7 @@ import ShellCheck.AST
 
 import Control.Monad.Writer
 import Control.Monad
+import Data.Char
 import Data.Functor
 import Data.List
 import Data.Maybe
@@ -226,8 +227,43 @@ getLiteralStringExt more = g
     g (T_SingleQuoted _ s) = return s
     g (T_Literal _ s) = return s
     g (T_ParamSubSpecialChar _ s) = return s
+    g (T_DollarSingleQuoted _ s) = return $ decodeEscapes s
     g x = more x
 
+    -- Bash style $'..' decoding
+    decodeEscapes ('\\':c:cs) =
+        case c of
+            'a' -> '\a' : rest
+            'b' -> '\b' : rest
+            'e' -> '\x1B' : rest
+            'f' -> '\f' : rest
+            'n' -> '\n' : rest
+            'r' -> '\r' : rest
+            't' -> '\t' : rest
+            'v' -> '\v' : rest
+            '\'' -> '\'' : rest
+            '"' -> '"' : rest
+            '\\' -> '\\' : rest
+            'x' ->
+                case cs of
+                    (x:y:more) ->
+                        if isHexDigit x && isHexDigit y
+                        then chr (16*(digitToInt x) + (digitToInt y)) : rest
+                        else '\\':c:rest
+            _ | isOctDigit c ->
+                let digits = take 3 $ takeWhile isOctDigit (c:cs)
+                    num = parseOct digits
+                in (if num < 256 then chr num else '?') : rest
+            _ -> '\\' : c : rest
+      where
+        rest = decodeEscapes cs
+        parseOct = f 0
+          where
+            f n "" = n
+            f n (c:rest) = f (n * 8 + digitToInt c) rest
+    decodeEscapes (c:cs) = c : decodeEscapes cs
+    decodeEscapes [] = []
+
 -- Is this token a string literal?
 isLiteral t = isJust $ getLiteralString t
 
@@ -257,17 +293,27 @@ getCommand t =
         T_Annotation _ _ t -> getCommand t
         _ -> Nothing
 
--- Maybe get the command name of a token representing a command
+-- Maybe get the command name string of a token representing a command
-getCommandName t = do
+getCommandName :: Token -> Maybe String
+getCommandName = fst . getCommandNameAndToken
+
+-- Get the command name token from a command, i.e.
+-- the token representing 'ls' in 'ls -la 2> foo'.
+-- If it can't be determined, return the original token.
+getCommandTokenOrThis = snd . getCommandNameAndToken
+
+getCommandNameAndToken :: Token -> (Maybe String, Token)
+getCommandNameAndToken t = fromMaybe (Nothing, t) $ do
     (T_SimpleCommand _ _ (w:rest)) <- getCommand t
     s <- getLiteralString w
     if "busybox" `isSuffixOf` s || "builtin" == s
         then
             case rest of
-                (applet:_) -> getLiteralString applet
+                (applet:_) -> return (getLiteralString applet, applet)
-                _ -> return s
+                _ -> return (Just s, w)
         else
-            return s
+            return (Just s, w)
+
 
 -- If a command substitution is a single command, get its name.
 --  $(date +%s) = Just "date"

src/ShellCheck/Analytics.hs

@@ -168,6 +168,8 @@ nodeChecks = [
     ,checkPipeToNowhere
     ,checkForLoopGlobVariables
     ,checkSubshelledTests
+    ,checkInvertedStringTest
+    ,checkRedirectionToCommand
     ]
 
 
@@ -239,6 +241,39 @@ isCondition (child:parent:rest) =
             T_UntilExpression id c l -> take 1 . reverse $ c
             _ -> []
 
+-- helpers to build replacements
+replaceStart id params n r =
+    let tp = tokenPositions params
+        (start, _) = tp Map.! id
+        new_end = start {
+            posColumn = posColumn start + n
+        }
+    in
+    newReplacement {
+        repStartPos = start,
+        repEndPos = new_end,
+        repString = r
+    }
+replaceEnd id params n r =
+    -- because of the way we count columns 1-based
+    -- we have to offset end columns by 1
+    let tp = tokenPositions params
+        (_, end) = tp Map.! id
+        new_start = end {
+            posColumn = posColumn end - n + 1
+        }
+        new_end = end {
+            posColumn = posColumn end + 1
+        }
+    in
+    newReplacement {
+        repStartPos = new_start,
+        repEndPos = new_end,
+        repString = r
+    }
+surroundWidth id params s = fixWith [replaceStart id params 0 s, replaceEnd id params 0 s]
+fixWith fixes = newFix { fixReplacements = fixes }
+
 prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)"
 checkEchoWc _ (T_Pipeline id _ [a, b]) =
     when (acmd == ["echo", "${VAR}"]) $
@@ -396,7 +431,7 @@ checkPipePitfalls _ (T_Pipeline id _ commands) = do
             mapM_ (f . (\ n -> take (length l) $ drop n commands)) indices
             return . not . null $ indices
     for' l f = for l (first f)
-    first func (x:_) = func (getId x)
+    first func (x:_) = func (getId $ getCommandTokenOrThis x)
     first _ _ = return ()
     hasShortParameter char = any (\x -> "-" `isPrefixOf` x && char `elem` x)
     hasParameter string =
@@ -430,17 +465,22 @@ prop_checkShebang4 = verifyNotTree checkShebang "#shellcheck shell=sh\nfoo"
 prop_checkShebang5 = verifyTree checkShebang "#!/usr/bin/env ash"
 prop_checkShebang6 = verifyNotTree checkShebang "#!/usr/bin/env ash\n# shellcheck shell=dash\n"
 prop_checkShebang7 = verifyNotTree checkShebang "#!/usr/bin/env ash\n# shellcheck shell=sh\n"
+prop_checkShebang8 = verifyTree checkShebang "#!bin/sh\ntrue"
+prop_checkShebang9 = verifyNotTree checkShebang "# shellcheck shell=sh\ntrue"
+prop_checkShebang10= verifyNotTree checkShebang "#!foo\n# shellcheck shell=sh ignore=SC2239\ntrue"
 checkShebang params (T_Annotation _ list t) =
     if any isOverride list then [] else checkShebang params t
   where
     isOverride (ShellOverride _) = True
     isOverride _ = False
-checkShebang params (T_Script id sb _) = execWriter $
+checkShebang params (T_Script id sb _) = execWriter $ do
     unless (shellTypeSpecified params) $ do
         when (sb == "") $
             err id 2148 "Tips depend on target shell and yours is unknown. Add a shebang."
         when (executableFromShebang sb == "ash") $
             warn id 2187 "Ash scripts will be checked as Dash. Add '# shellcheck shell=dash' to silence."
+    unless (null sb || "/" `isPrefixOf` sb) $
+        err id 2239 "Ensure the shebang uses an absolute path to the interpreter."
 
 
 prop_checkForInQuoted = verify checkForInQuoted "for f in \"$(ls)\"; do echo foo; done"
@@ -709,6 +749,7 @@ prop_checkArrayWithoutIndex5 = verifyTree checkArrayWithoutIndex "a[0]=foo; echo
 prop_checkArrayWithoutIndex6 = verifyTree checkArrayWithoutIndex "echo $PIPESTATUS"
 prop_checkArrayWithoutIndex7 = verifyTree checkArrayWithoutIndex "a=(a b); a+=c"
 prop_checkArrayWithoutIndex8 = verifyTree checkArrayWithoutIndex "declare -a foo; foo=bar;"
+prop_checkArrayWithoutIndex9 = verifyTree checkArrayWithoutIndex "read -r -a arr <<< 'foo bar'; echo \"$arr\""
 checkArrayWithoutIndex params _ =
     doVariableFlowAnalysis readF writeF defaultMap (variableFlow params)
   where
@@ -1019,7 +1060,7 @@ checkQuotedCondRegex _ (TC_Binary _ _ "=~" _ rhs) =
     error t =
         unless (isConstantNonRe t) $
             err (getId t) 2076
-                "Don't quote rhs of =~, it'll match literally rather than as a regex."
+                "Don't quote right-hand side of =~, it'll match literally rather than as a regex."
     re = mkRegex "[][*.+()|]"
     hasMetachars s = s `matches` re
     isConstantNonRe t = fromMaybe False $ do
@@ -1029,13 +1070,16 @@ checkQuotedCondRegex _ _ = return ()
 
 prop_checkGlobbedRegex1 = verify checkGlobbedRegex "[[ $foo =~ *foo* ]]"
 prop_checkGlobbedRegex2 = verify checkGlobbedRegex "[[ $foo =~ f* ]]"
-prop_checkGlobbedRegex2a = verify checkGlobbedRegex "[[ $foo =~ \\#* ]]"
 prop_checkGlobbedRegex3 = verifyNot checkGlobbedRegex "[[ $foo =~ $foo ]]"
 prop_checkGlobbedRegex4 = verifyNot checkGlobbedRegex "[[ $foo =~ ^c.* ]]"
+prop_checkGlobbedRegex5 = verifyNot checkGlobbedRegex "[[ $foo =~ \\* ]]"
+prop_checkGlobbedRegex6 = verifyNot checkGlobbedRegex "[[ $foo =~ (o*) ]]"
+prop_checkGlobbedRegex7 = verifyNot checkGlobbedRegex "[[ $foo =~ \\*foo ]]"
+prop_checkGlobbedRegex8 = verifyNot checkGlobbedRegex "[[ $foo =~ x\\* ]]"
 checkGlobbedRegex _ (TC_Binary _ DoubleBracket "=~" _ rhs) =
     let s = concat $ oversimplify rhs in
         when (isConfusedGlobRegex s) $
-            warn (getId rhs) 2049 "=~ is for regex. Use == for globs."
+            warn (getId rhs) 2049 "=~ is for regex, but this looks like a glob. Use = instead."
 checkGlobbedRegex _ _ = return ()
 
 
@@ -1188,7 +1232,7 @@ prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow
 prop_checkComparisonAgainstGlob5 = verify checkComparisonAgainstGlob "[[ $cow != $bar ]]"
 checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _]))
     | op `elem` ["=", "==", "!="] =
-        warn id 2053 $ "Quote the rhs of " ++ op ++ " in [[ ]] to prevent glob matching."
+        warn id 2053 $ "Quote the right-hand side of " ++ op ++ " in [[ ]] to prevent glob matching."
 checkComparisonAgainstGlob _ (TC_Binary _ SingleBracket op _ word)
         | (op == "=" || op == "==") && isGlob word =
     err (getId word) 2081 "[ .. ] can't match globs. Use [[ .. ]] or case statement."
@@ -1323,8 +1367,10 @@ checkPS1Assignments _ _ = return ()
 prop_checkBackticks1 = verify checkBackticks "echo `foo`"
 prop_checkBackticks2 = verifyNot checkBackticks "echo $(foo)"
 prop_checkBackticks3 = verifyNot checkBackticks "echo `#inlined comment` foo"
-checkBackticks _ (T_Backticked id list) | not (null list) =
+checkBackticks params (T_Backticked id list) | not (null list) =
-    style id 2006 "Use $(..) instead of legacy `..`."
+    addComment $
+        makeCommentWithFix StyleC id 2006  "Use $(...) notation instead of legacy backticked `...`."
+            (fixWith [replaceStart id params 1 "$(", replaceEnd id params 1 ")"])
 checkBackticks _ _ = return ()
 
 prop_checkIndirectExpansion1 = verify checkIndirectExpansion "${foo$n}"
@@ -1598,6 +1644,7 @@ prop_checkSpacefulness32= verifyNotTree checkSpacefulness "var=$1; [ -v var ]"
 prop_checkSpacefulness33= verifyTree checkSpacefulness "for file; do echo $file; done"
 prop_checkSpacefulness34= verifyTree checkSpacefulness "declare foo$n=$1"
 prop_checkSpacefulness35= verifyNotTree checkSpacefulness "echo ${1+\"$1\"}"
+prop_checkSpacefulness36= verifyNotTree checkSpacefulness "arg=$#; echo $arg"
 
 checkSpacefulness params t =
     doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params)
@@ -1627,8 +1674,10 @@ checkSpacefulness params t =
                 makeComment InfoC (getId token) 2223
                     "This default assignment may cause DoS due to globbing. Quote it."
             else
-                makeComment InfoC (getId token) 2086
+                makeCommentWithFix InfoC (getId token) 2086
-                    "Double quote to prevent globbing and word splitting."
+                    "Double quote to prevent globbing and word splitting." (surroundWidth (getId token) params "\"")
+                -- makeComment InfoC (getId token) 2086
+                --     "Double quote to prevent globbing and word splitting."
 
     writeF _ _ name (DataString SourceExternal) = setSpaces name True >> return []
     writeF _ _ name (DataString SourceInteger) = setSpaces name False >> return []
@@ -1879,6 +1928,7 @@ prop_checkUnassignedReferences32= verifyNotTree checkUnassignedReferences "if [[
 prop_checkUnassignedReferences33= verifyNotTree checkUnassignedReferences "f() { local -A foo; echo \"${foo[@]}\"; }"
 prop_checkUnassignedReferences34= verifyNotTree checkUnassignedReferences "declare -A foo; (( foo[bar] ))"
 prop_checkUnassignedReferences35= verifyNotTree checkUnassignedReferences "echo ${arr[foo-bar]:?fail}"
+prop_checkUnassignedReferences36= verifyNotTree checkUnassignedReferences "read -a foo -r <<<\"foo bar\"; echo \"$foo\""
 checkUnassignedReferences params t = warnings
   where
     (readMap, writeMap) = execState (mapM tally $ variableFlow params) (Map.empty, Map.empty)
@@ -2069,6 +2119,7 @@ prop_checkCdAndBack1 = verify checkCdAndBack "for f in *; do cd $f; git pull; cd
 prop_checkCdAndBack2 = verifyNot checkCdAndBack "for f in *; do cd $f || continue; git pull; cd ..; done"
 prop_checkCdAndBack3 = verifyNot checkCdAndBack "while [[ $PWD != / ]]; do cd ..; done"
 prop_checkCdAndBack4 = verify checkCdAndBack "cd $tmp; foo; cd -"
+prop_checkCdAndBack5 = verifyNot checkCdAndBack "cd ..; foo; cd .."
 checkCdAndBack params = doLists
   where
     shell = shellType params
@@ -2091,10 +2142,20 @@ checkCdAndBack params = doLists
     getCmd (T_Pipeline id _ [x]) = getCommandName x
     getCmd _ = Nothing
 
+    findCdPair list =
+        case list of
+            (a:b:rest) ->
+                if isCdRevert b && not (isCdRevert a)
+                then return $ getId b
+                else findCdPair (b:rest)
+            _ -> Nothing
+
+
     doList list =
         let cds = filter ((== Just "cd") . getCmd) list in
-            when (length cds >= 2 && isCdRevert (last cds)) $
+            potentially $ do
-               info (getId $ last cds) 2103 message
+                cd <- findCdPair cds
+                return $ info cd 2103 message
 
     message = "Use a ( subshell ) to avoid having to cd back."
 
@@ -2472,7 +2533,7 @@ prop_checkReadWithoutR1 = verify checkReadWithoutR "read -a foo"
 prop_checkReadWithoutR2 = verifyNot checkReadWithoutR "read -ar foo"
 checkReadWithoutR _ t@T_SimpleCommand {} | t `isUnqualifiedCommand` "read" =
     unless ("r" `elem` map snd (getAllFlags t)) $
-        info (getId t) 2162 "read without -r will mangle backslashes."
+        info (getId $ getCommandTokenOrThis t) 2162 "read without -r will mangle backslashes."
 checkReadWithoutR _ _ = return ()
 
 prop_checkUncheckedCd1 = verifyTree checkUncheckedCdPushdPopd "cd ~/src; rm -r foo"
@@ -2501,6 +2562,7 @@ prop_checkUncheckedPopd5 = verifyTree checkUncheckedCdPushdPopd "if true; then p
 prop_checkUncheckedPopd6 = verifyTree checkUncheckedCdPushdPopd "popd"
 prop_checkUncheckedPopd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\npopd\nrm bar"
 prop_checkUncheckedPopd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; popd; rm bar"
+prop_checkUncheckedPopd9 = verifyNotTree checkUncheckedCdPushdPopd "popd -n foo"
 
 checkUncheckedCdPushdPopd params root =
     if hasSetE params then
@@ -2510,9 +2572,10 @@ checkUncheckedCdPushdPopd params root =
     checkElement t@T_SimpleCommand {} =
         when(name t `elem` ["cd", "pushd", "popd"]
             && not (isSafeDir t)
-            && not (name t == "pushd" && ("n" `elem` map snd (getAllFlags t)))
+            && not (name t `elem` ["pushd", "popd"] && ("n" `elem` map snd (getAllFlags t)))
             && not (isCondition $ getPath (parentMap params) t)) $
-                warn (getId t) 2164 "Use 'cd ... || exit' or 'cd ... || return' in case cd fails."
+                warnWithFix (getId t) 2164 "Use 'cd ... || exit' or 'cd ... || return' in case cd fails."
+                    (fixWith [replaceEnd (getId t) params 0 " || exit"])
     checkElement _ = return ()
     name t = fromMaybe "" $ getCommandName t
     isSafeDir t = case oversimplify t of
@@ -2669,7 +2732,7 @@ checkArrayAssignmentIndices params root =
                         T_Literal id str -> [(id,str)]
                         _ -> []
                     guard $ '=' `elem` str
-                    return $ warn id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it."
+                    return $ warnWithFix id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it." (surroundWidth id params "\"")
                 in
                     if null literalEquals && isAssociative
                     then warn (getId t) 2190 "Elements in associative arrays need index, e.g. array=( [index]=value ) ."
@@ -2685,26 +2748,31 @@ prop_checkUnmatchableCases5 = verify checkUnmatchableCases "case $f in *.txt) tr
 prop_checkUnmatchableCases6 = verifyNot checkUnmatchableCases "case $f in ?*) true;; *) false;; esac"
 prop_checkUnmatchableCases7 = verifyNot checkUnmatchableCases "case $f in $(x)) true;; asdf) false;; esac"
 prop_checkUnmatchableCases8 = verify checkUnmatchableCases "case $f in cow) true;; bar|cow) false;; esac"
+prop_checkUnmatchableCases9 = verifyNot checkUnmatchableCases "case $f in x) true;;& x) false;; esac"
 checkUnmatchableCases _ t =
     case t of
         T_CaseExpression _ word list -> do
-            let patterns = concatMap snd3 list
+            -- Check all patterns for whether they can ever match
+            let allpatterns  = concatMap snd3 list
+            -- Check only the non-fallthrough branches for shadowing
+            let breakpatterns = concatMap snd3 $ filter (\x -> fst3 x == CaseBreak) list
 
             if isConstant word
                 then warn (getId word) 2194
                         "This word is constant. Did you forget the $ on a variable?"
                 else  potentially $ do
                     pg <- wordToPseudoGlob word
-                    return $ mapM_ (check pg) patterns
+                    return $ mapM_ (check pg) allpatterns
 
-            let exactGlobs = tupMap wordToExactPseudoGlob patterns
+            let exactGlobs = tupMap wordToExactPseudoGlob breakpatterns
-            let fuzzyGlobs = tupMap wordToPseudoGlob patterns
+            let fuzzyGlobs = tupMap wordToPseudoGlob breakpatterns
             let dominators = zip exactGlobs (tails $ drop 1 fuzzyGlobs)
 
             mapM_ checkDoms dominators
 
         _ -> return ()
   where
+    fst3 (x,_,_) = x
     snd3 (_,x,_) = x
     check target candidate = potentially $ do
         candidateGlob <- wordToPseudoGlob candidate
@@ -2837,6 +2905,7 @@ prop_checkPipeToNowhere5 = verifyNot checkPipeToNowhere "echo foo | xargs du"
 prop_checkPipeToNowhere6 = verifyNot checkPipeToNowhere "ls | echo $(cat)"
 prop_checkPipeToNowhere7 = verifyNot checkPipeToNowhere "echo foo | var=$(cat) ls"
 prop_checkPipeToNowhere8 = verify checkPipeToNowhere "foo | true"
+prop_checkPipeToNowhere9 = verifyNot checkPipeToNowhere "mv -i f . < /dev/stdin"
 checkPipeToNowhere :: Parameters -> Token -> WriterT [TokenComment] Identity ()
 checkPipeToNowhere _ t =
     case t of
@@ -2849,15 +2918,25 @@ checkPipeToNowhere _ t =
         name <- getCommandBasename cmd
         guard $ name `elem` nonReadingCommands
         guard . not $ hasAdditionalConsumers cmd
+        -- Confusing echo for cat is so common that it's worth a special case
+        let suggestion =
+                if name == "echo"
+                then "Did you want 'cat' instead?"
+                else "Wrong command or missing xargs?"
         return $ warn (getId cmd) 2216 $
-            "Piping to '" ++ name ++ "', a command that doesn't read stdin. Wrong command or missing xargs?"
+            "Piping to '" ++ name ++ "', a command that doesn't read stdin. " ++ suggestion
 
     checkRedir cmd = potentially $ do
         name <- getCommandBasename cmd
         guard $ name `elem` nonReadingCommands
         guard . not $ hasAdditionalConsumers cmd
+        guard . not $ name `elem` ["cp", "mv", "rm"] && cmd `hasFlag` "i"
+        let suggestion =
+                if name == "echo"
+                then "Did you want 'cat' instead?"
+                else "Bad quoting, wrong command or missing xargs?"
         return $ warn (getId cmd) 2217 $
-            "Redirecting to '" ++ name ++ "', a command that doesn't read stdin. Bad quoting or missing xargs?"
+            "Redirecting to '" ++ name ++ "', a command that doesn't read stdin. " ++ suggestion
 
     -- Could any words in a SimpleCommand consume stdin (e.g. echo "$(cat)")?
     hasAdditionalConsumers t = fromMaybe True $ do
@@ -2926,9 +3005,10 @@ checkForLoopGlobVariables _ t =
 prop_checkSubshelledTests1 = verify checkSubshelledTests "a && ( [ b ] || ! [ c ] )"
 prop_checkSubshelledTests2 = verify checkSubshelledTests "( [ a ] )"
 prop_checkSubshelledTests3 = verify checkSubshelledTests "( [ a ] && [ b ] || test c )"
+prop_checkSubshelledTests4 = verify checkSubshelledTests "( [ a ] && { [ b ] && [ c ]; } )"
 checkSubshelledTests params t =
     case t of
-        T_Subshell id list | isSubshelledTest t ->
+        T_Subshell id list | all isTestStructure list ->
             case () of
                 -- Special case for if (test) and while (test)
                 _ | isCompoundCondition (getPath (parentMap params) t) ->
@@ -2948,19 +3028,24 @@ checkSubshelledTests params t =
             [c] | isTestCommand c -> True
             _ -> False
 
-    isSubshelledTest t =
+    isTestStructure t =
         case t of
-            T_Subshell _ list -> all isSubshelledTest list
+            T_Banged _ t -> isTestStructure t
-            T_AndIf _ a b -> isSubshelledTest a && isSubshelledTest b
+            T_AndIf _ a b -> isTestStructure a && isTestStructure b
-            T_OrIf  _ a b -> isSubshelledTest a && isSubshelledTest b
+            T_OrIf  _ a b -> isTestStructure a && isTestStructure b
-            T_Annotation _ _ t -> isSubshelledTest t
+            T_Pipeline _ [] [T_Redirecting _ _ cmd] ->
+                case cmd of
+                    T_BraceGroup _ ts -> all isTestStructure ts
+                    T_Subshell   _ ts -> all isTestStructure ts
+                    _ -> isTestCommand t
             _ -> isTestCommand t
 
     isTestCommand t =
         case t of
-            T_Banged _ t -> isTestCommand t
+            T_Pipeline _ [] [T_Redirecting _ _ cmd] ->
-            T_Pipeline _ [] [T_Redirecting _ _ T_Condition {}] -> True
+                case cmd of
-            T_Pipeline _ [] [T_Redirecting _ _ cmd] -> cmd `isCommand` "test"
+                    T_Condition {} -> True
+                    _ -> cmd `isCommand` "test"
             _ -> False
 
     -- Check if a T_Subshell is used as a condition, e.g. if ( test )
@@ -2981,5 +3066,35 @@ checkSubshelledTests params t =
             T_Annotation {} -> True
             _ -> False
 
+prop_checkInvertedStringTest1 = verify checkInvertedStringTest "[ ! -z $var ]"
+prop_checkInvertedStringTest2 = verify checkInvertedStringTest "! [[ -n $var ]]"
+prop_checkInvertedStringTest3 = verifyNot checkInvertedStringTest "! [ -x $var ]"
+prop_checkInvertedStringTest4 = verifyNot checkInvertedStringTest "[[ ! -w $var ]]"
+prop_checkInvertedStringTest5 = verifyNot checkInvertedStringTest "[ -z $var ]"
+checkInvertedStringTest _ t =
+    case t of
+        TC_Unary _ _ "!" (TC_Unary _ _ op _) ->
+            case op of
+                "-n" -> style (getId t) 2236 "Use -z instead of ! -n."
+                "-z" -> style (getId t) 2236 "Use -n instead of ! -z."
+                _ -> return ()
+        T_Banged _ (T_Pipeline _ _
+          [T_Redirecting _ _ (T_Condition _ _ (TC_Unary _ _ op _))]) ->
+            case op of
+                "-n" -> style (getId t) 2237 "Use [ -z .. ] instead of ! [ -n .. ]."
+                "-z" -> style (getId t) 2237 "Use [ -n .. ] instead of ! [ -z .. ]."
+                _ -> return ()
+        _ -> return ()
+
+prop_checkRedirectionToCommand1 = verify checkRedirectionToCommand "ls > rm"
+prop_checkRedirectionToCommand2 = verifyNot checkRedirectionToCommand "ls > 'rm'"
+prop_checkRedirectionToCommand3 = verifyNot checkRedirectionToCommand "ls > myfile"
+checkRedirectionToCommand _ t =
+    case t of
+        T_IoFile _ _ (T_NormalWord id [T_Literal _ str]) | str `elem` commonCommands ->
+            unless (str == "file") $ -- This would be confusing
+                warn id 2238 "Redirecting to/from command name instead of file. Did you want pipes/xargs (or quote to ignore)?"
+        _ -> return ()
+
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])

src/ShellCheck/Analyzer.hs

@@ -30,7 +30,7 @@ import qualified ShellCheck.Checks.ShellSupport
 
 -- TODO: Clean up the cruft this is layered on
 analyzeScript :: AnalysisSpec -> AnalysisResult
-analyzeScript spec = AnalysisResult {
+analyzeScript spec = newAnalysisResult {
     arComments =
         filterByAnnotation spec params . nub $
             runAnalytics spec

src/ShellCheck/AnalyzerLib.hs

@@ -20,26 +20,28 @@
 {-# LANGUAGE FlexibleContexts #-}
 {-# LANGUAGE TemplateHaskell  #-}
 module ShellCheck.AnalyzerLib where
-import           ShellCheck.AST
-import           ShellCheck.ASTLib
-import           ShellCheck.Data
-import           ShellCheck.Interface
-import           ShellCheck.Parser
-import           ShellCheck.Regex
 
-import           Control.Arrow          (first)
+import ShellCheck.AST
-import           Control.Monad.Identity
+import ShellCheck.ASTLib
-import           Control.Monad.RWS
+import ShellCheck.Data
-import           Control.Monad.State
+import ShellCheck.Interface
-import           Control.Monad.Writer
+import ShellCheck.Parser
-import           Data.Char
+import ShellCheck.Regex
-import           Data.List
-import qualified Data.Map               as Map
-import           Data.Maybe
-import           Data.Semigroup
 
-import           Test.QuickCheck.All    (forAllProperties)
+import Control.Arrow (first)
-import           Test.QuickCheck.Test   (maxSuccess, quickCheckWithResult, stdArgs)
+import Control.DeepSeq
+import Control.Monad.Identity
+import Control.Monad.RWS
+import Control.Monad.State
+import Control.Monad.Writer
+import Data.Char
+import Data.List
+import Data.Maybe
+import Data.Semigroup
+import qualified Data.Map as Map
+
+import Test.QuickCheck.All (forAllProperties)
+import Test.QuickCheck.Test (maxSuccess, quickCheckWithResult, stdArgs)
 
 type Analysis = AnalyzerM ()
 type AnalyzerM a = RWS Parameters [TokenComment] Cache a
@@ -81,7 +83,8 @@ data Parameters = Parameters {
     parentMap          :: Map.Map Id Token, -- A map from Id to parent Token
     shellType          :: Shell,            -- The shell type, such as Bash or Ksh
     shellTypeSpecified :: Bool,    -- True if shell type was forced via flags
-    rootNode           :: Token              -- The root node of the AST
+    rootNode           :: Token,              -- The root node of the AST
+    tokenPositions     :: Map.Map Id (Position, Position) -- map from token id to start and end position
     }
 
 -- TODO: Cache results of common AST ops here
@@ -109,19 +112,17 @@ data DataSource =
 
 data VariableState = Dead Token String | Alive deriving (Show)
 
-defaultSpec root = AnalysisSpec {
+defaultSpec root = spec {
-    asScript = root,
     asShellType = Nothing,
     asCheckSourced = False,
     asExecutionMode = Executed
-}
+} where spec = newAnalysisSpec root
 
 pScript s =
   let
-    pSpec = ParseSpec {
+    pSpec = newParseSpec {
         psFilename = "script",
-        psScript = s,
+        psScript = s
-        psCheckSourced = False
     }
   in prRoot . runIdentity $ parseScript (mockedSystemInterface []) pSpec
 
@@ -135,9 +136,16 @@ producesComments c s = do
 
 makeComment :: Severity -> Id -> Code -> String -> TokenComment
 makeComment severity id code note =
-    TokenComment id $ Comment severity code note
+    newTokenComment {
+        tcId = id,
+        tcComment = newComment {
+            cSeverity = severity,
+            cCode = code,
+            cMessage = note
+        }
+    }
 
-addComment note = tell [note]
+addComment note = note `deepseq` tell [note]
 
 warn :: MonadWriter [TokenComment] m => Id -> Code -> String -> m ()
 warn  id code str = addComment $ makeComment WarningC id code str
@@ -145,6 +153,20 @@ err   id code str = addComment $ makeComment ErrorC id code str
 info  id code str = addComment $ makeComment InfoC id code str
 style id code str = addComment $ makeComment StyleC id code str
 
+warnWithFix id code str fix = addComment $
+    let comment = makeComment WarningC id code str in
+    comment {
+        tcFix = Just fix
+    }
+
+makeCommentWithFix :: Severity -> Id -> Code -> String -> Fix -> TokenComment
+makeCommentWithFix severity id code str fix =
+    let comment = makeComment severity id code str
+        withFix = comment {
+            tcFix = Just fix
+        }
+    in withFix `deepseq` withFix
+
 makeParameters spec =
     let params = Parameters {
         rootNode = root,
@@ -159,7 +181,8 @@ makeParameters spec =
 
         shellTypeSpecified = isJust $ asShellType spec,
         parentMap = getParentTree root,
-        variableFlow = getVariableFlow params root
+        variableFlow = getVariableFlow params root,
+        tokenPositions = asTokenPositions spec
     } in params
   where root = asScript spec
 
@@ -235,9 +258,10 @@ getParentTree t =
   where
     pre t = modify (first ((:) t))
     post t = do
-        (_:rest, map) <- get
+        (x, map) <- get
-        case rest of []    -> put (rest, map)
+        case x of
-                     (x:_) -> put (rest, Map.insert (getId t) x map)
+          _:rest -> case rest of []    -> put (rest, map)
+                                 (x:_) -> put (rest, Map.insert (getId t) x map)
 
 -- Given a root node, make a map from Id to Token
 getTokenMap :: Token -> Map.Map Id Token
@@ -520,12 +544,22 @@ getReferencedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Litera
 
 getReferencedVariableCommand _ = []
 
+-- The function returns a tuple consisting of four items describing an assignment.
+-- Given e.g. declare foo=bar
+-- (
+--   BaseCommand :: Token,     -- The command/structure assigning the variable, i.e. declare foo=bar
+--   AssignmentToken :: Token, -- The specific part that assigns this variable, i.e. foo=bar
+--   VariableName :: String,   -- The variable name, i.e. foo
+--   VariableValue :: DataType -- A description of the value being assigned, i.e. "Literal string with value foo"
+-- )
 getModifiedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal _ x:_):rest)) =
    filter (\(_,_,s,_) -> not ("-" `isPrefixOf` s)) $
     case x of
         "read" ->
-            let params = map getLiteral rest in
+            let params = map getLiteral rest
-                catMaybes . takeWhile isJust . reverse $ params
+                readArrayVars = getReadArrayVariables rest
+            in
+                catMaybes . (++ readArrayVars) . takeWhile isJust . reverse $ params
         "getopts" ->
             case rest of
                 opts:var:_ -> maybeToList $ getLiteral var
@@ -568,10 +602,14 @@ getModifiedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal
       where
         defaultType = if any (`elem` flags) ["a", "A"] then DataArray else DataString
 
-    getLiteral t = do
+    getLiteralOfDataType t d = do
         s <- getLiteralString t
         when ("-" `isPrefixOf` s) $ fail "argument"
-        return (base, t, s, DataString SourceExternal)
+        return (base, t, s, d)
+
+    getLiteral t = getLiteralOfDataType t (DataString SourceExternal)
+
+    getLiteralArray t = getLiteralOfDataType t (DataArray SourceExternal)
 
     getModifierParamString = getModifierParam DataString
 
@@ -613,6 +651,11 @@ getModifiedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal
         guard $ isVariableName name
         return (base, lastArg, name, DataArray SourceExternal)
 
+    -- get all the array variables used in read, e.g. read -a arr
+    getReadArrayVariables args = do
+        map (getLiteralArray . snd)
+            (filter (\(x,_) -> getLiteralString x == Just "-a") (zip (args) (tail args)))
+
 getModifiedVariableCommand _ = []
 
 getIndexReferences s = fromMaybe [] $ do
@@ -812,10 +855,9 @@ filterByAnnotation asSpec params =
     filter (not . shouldIgnore)
   where
     token = asScript asSpec
-    idFor (TokenComment id _) = id
     shouldIgnore note =
         any (shouldIgnoreFor (getCode note)) $
-            getPath parents (T_Bang $ idFor note)
+            getPath parents (T_Bang $ tcId note)
     shouldIgnoreFor num (T_Annotation _ anns _) =
         any hasNum anns
       where
@@ -824,7 +866,7 @@ filterByAnnotation asSpec params =
     shouldIgnoreFor _ T_Include {} = not $ asCheckSourced asSpec
     shouldIgnoreFor _ _ = False
     parents = parentMap params
-    getCode (TokenComment _ (Comment _ c _)) = c
+    getCode = cCode . tcComment
 
 -- Is this a ${#anything}, to get string length or array count?
 isCountingReference (T_DollarBraced id token) =

src/ShellCheck/Checker.hs

@@ -37,56 +37,75 @@ import Control.Monad
 
 import Test.QuickCheck.All
 
-tokenToPosition map (TokenComment id c) = fromMaybe fail $ do
+tokenToPosition startMap t = fromMaybe fail $ do
-    position <- Map.lookup id map
+    span <- Map.lookup (tcId t) startMap
-    return $ PositionedComment position position c
+    return $ newPositionedComment {
+        pcStartPos = fst span,
+        pcEndPos = snd span,
+        pcComment = tcComment t,
+        pcFix = tcFix t
+    }
   where
     fail = error "Internal shellcheck error: id doesn't exist. Please report!"
 
 checkScript :: Monad m => SystemInterface m -> CheckSpec -> m CheckResult
 checkScript sys spec = do
     results <- checkScript (csScript spec)
-    return CheckResult {
+    return emptyCheckResult {
         crFilename = csFilename spec,
         crComments = results
     }
   where
     checkScript contents = do
-        result <- parseScript sys ParseSpec {
+        result <- parseScript sys newParseSpec {
             psFilename = csFilename spec,
             psScript = contents,
-            psCheckSourced = csCheckSourced spec
+            psCheckSourced = csCheckSourced spec,
+            psShellTypeOverride = csShellTypeOverride spec
         }
         let parseMessages = prComments result
+        let tokenPositions = prTokenPositions result
+        let analysisSpec root =
+                as {
+                    asScript = root,
+                    asShellType = csShellTypeOverride spec,
+                    asCheckSourced = csCheckSourced spec,
+                    asExecutionMode = Executed,
+                    asTokenPositions = tokenPositions
+                } where as = newAnalysisSpec root
         let analysisMessages =
                 fromMaybe [] $
                     (arComments . analyzeScript . analysisSpec)
                         <$> prRoot result
-        let translator = tokenToPosition (prTokenPositions result)
+        let translator = tokenToPosition tokenPositions
         return . nub . sortMessages . filter shouldInclude $
             (parseMessages ++ map translator analysisMessages)
 
-    shouldInclude (PositionedComment _ _ (Comment _ code _)) =
+    shouldInclude pc =
-        code `notElem` csExcludedWarnings spec
+        let code     = cCode (pcComment pc)
+            severity = cSeverity (pcComment pc)
+        in
+            code `notElem` csExcludedWarnings spec &&
+            severity <= csMinSeverity spec
 
     sortMessages = sortBy (comparing order)
-    order (PositionedComment pos _ (Comment severity code message)) =
+    order pc =
-        (posFile pos, posLine pos, posColumn pos, severity, code, message)
+        let pos = pcStartPos pc
-    getPosition (PositionedComment pos _ _) = pos
+            comment = pcComment pc in
+        (posFile pos,
+         posLine pos,
+         posColumn pos,
+         cSeverity comment,
+         cCode comment,
+         cMessage comment)
+    getPosition = pcStartPos
 
-    analysisSpec root =
-        AnalysisSpec {
-            asScript = root,
-            asShellType = csShellTypeOverride spec,
-            asCheckSourced = csCheckSourced spec,
-            asExecutionMode = Executed
-         }
 
 getErrors sys spec =
     sort . map getCode . crComments $
         runIdentity (checkScript sys spec)
   where
-    getCode (PositionedComment _ _ (Comment _ code _)) = code
+    getCode = cCode . pcComment
 
 check = checkWithIncludes []
 
@@ -136,6 +155,21 @@ prop_optionDisablesIssue2 =
                     csExcludedWarnings = [2148, 1037]
                 }
 
+prop_wontParseBadShell =
+    [1071] == check "#!/usr/bin/python\ntrue $1\n"
+
+prop_optionDisablesBadShebang =
+    null $ getErrors
+                (mockedSystemInterface [])
+                emptyCheckSpec {
+                    csScript = "#!/usr/bin/python\ntrue\n",
+                    csShellTypeOverride = Just Sh
+                }
+
+prop_annotationDisablesBadShebang =
+    [] == check "#!/usr/bin/python\n# shellcheck shell=sh\ntrue\n"
+
+
 prop_canParseDevNull =
     [] == check "source /dev/null"
 
@@ -180,7 +214,7 @@ prop_filewideAnnotation1 = null $
 prop_filewideAnnotation2 = null $
     check "#!/bin/sh\n# shellcheck disable=2086\ntrue\necho $1"
 prop_filewideAnnotation3 = null $
-    check "#!/bin/sh\n#unerlated\n# shellcheck disable=2086\ntrue\necho $1"
+    check "#!/bin/sh\n#unrelated\n# shellcheck disable=2086\ntrue\necho $1"
 prop_filewideAnnotation4 = null $
     check "#!/bin/sh\n# shellcheck disable=2086\n#unrelated\ntrue\necho $1"
 prop_filewideAnnotation5 = null $
@@ -197,6 +231,5 @@ prop_filewideAnnotation8 = null $
 prop_sourcePartOfOriginalScript = -- #1181: -x disabled posix warning for 'source'
     2039 `elem` checkWithIncludes [("./saywhat.sh", "echo foo")] "#!/bin/sh\nsource ./saywhat.sh"
 
-
 return []
 runTests = $quickCheckAll

src/ShellCheck/Checks/Commands.hs

@@ -141,6 +141,7 @@ prop_checkTr8 = verifyNot checkTr "tr aeiou _____"
 prop_checkTr9 = verifyNot checkTr "a-z n-za-m"
 prop_checkTr10= verifyNot checkTr "tr --squeeze-repeats rl lr"
 prop_checkTr11= verifyNot checkTr "tr abc '[d*]'"
+prop_checkTr12= verifyNot checkTr "tr '[=e=]' 'e'"
 checkTr = CommandCheck (Basename "tr") (mapM_ f . arguments)
   where
     f w | isGlob w = -- The user will go [ab] -> '[ab]' -> 'ab'. Fixme?
@@ -152,7 +153,7 @@ checkTr = CommandCheck (Basename "tr") (mapM_ f . arguments)
         Just s -> do  -- Eliminate false positives by only looking for dupes in SET2?
           when (not ("-" `isPrefixOf` s || "[:" `isInfixOf` s) && duplicated s) $
             info (getId word) 2020 "tr replaces sets of chars, not words (mentioned due to duplicates)."
-          unless ("[:" `isPrefixOf` s) $
+          unless ("[:" `isPrefixOf` s || "[=" `isPrefixOf` s) $
             when ("[" `isPrefixOf` s && "]" `isSuffixOf` s && (length s > 2) && ('*' `notElem` s)) $
               info (getId word) 2021 "Don't use [] around classes in tr, it replaces literal square brackets."
         Nothing -> return ()
@@ -183,7 +184,7 @@ prop_checkNeedlessExpr4 = verifyNot checkNeedlessExpr "foo=$(expr foo \\< regex)
 checkNeedlessExpr = CommandCheck (Basename "expr") f where
     f t =
         when (all (`notElem` exceptions) (words $ arguments t)) $
-            style (getId t) 2003
+            style (getId $ getCommandTokenOrThis t) 2003
                 "expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]]."
     -- These operators are hard to replicate in POSIX
     exceptions = [ ":", "<", ">", "<=", ">=" ]
@@ -324,26 +325,18 @@ prop_checkUnusedEchoEscapes2 = verifyNot checkUnusedEchoEscapes "echo -e 'foi\\n
 prop_checkUnusedEchoEscapes3 = verify checkUnusedEchoEscapes "echo \"n:\\t42\""
 prop_checkUnusedEchoEscapes4 = verifyNot checkUnusedEchoEscapes "echo lol"
 prop_checkUnusedEchoEscapes5 = verifyNot checkUnusedEchoEscapes "echo -n -e '\n'"
-checkUnusedEchoEscapes = CommandCheck (Basename "echo") (f . arguments)
+checkUnusedEchoEscapes = CommandCheck (Basename "echo") f
   where
-    isDashE = mkRegex "^-.*e"
     hasEscapes = mkRegex "\\\\[rnt]"
-    f args | concat (concatMap oversimplify allButLast) `matches` isDashE =
+    f cmd =
-        return ()
+        whenShell [Sh, Bash, Ksh] $
-      where allButLast = reverse . drop 1 . reverse $ args
+            unless (cmd `hasFlag` "e") $
-    f args = mapM_ checkEscapes args
+                mapM_ examine $ arguments cmd
 
-    checkEscapes (T_NormalWord _ args) =
+    examine token = do
-        mapM_ checkEscapes args
+        let str = onlyLiteralString token
-    checkEscapes (T_DoubleQuoted id args) =
-        mapM_ checkEscapes args
-    checkEscapes (T_Literal id str) = examine id str
-    checkEscapes (T_SingleQuoted id str) = examine id str
-    checkEscapes _ = return ()
-
-    examine id str =
         when (str `matches` hasEscapes) $
-            info id 2028 "echo won't expand escape sequences. Consider printf."
+            info (getId token) 2028 "echo may not expand escape sequences. Use printf."
 
 
 prop_checkInjectableFindSh1 = verify checkInjectableFindSh "find . -exec sh -c 'echo {}' \\;"
@@ -515,6 +508,9 @@ prop_checkPrintfVar12= verify checkPrintfVar "printf '%s %s\\n' 1 2 3"
 prop_checkPrintfVar13= verifyNot checkPrintfVar "printf '%s %s\\n' 1 2 3 4"
 prop_checkPrintfVar14= verify checkPrintfVar "printf '%*s\\n' 1"
 prop_checkPrintfVar15= verifyNot checkPrintfVar "printf '%*s\\n' 1 2"
+prop_checkPrintfVar16= verifyNot checkPrintfVar "printf $'string'"
+prop_checkPrintfVar17= verify checkPrintfVar "printf '%-*s\\n' 1"
+prop_checkPrintfVar18= verifyNot checkPrintfVar "printf '%-*s\\n' 1 2"
 checkPrintfVar = CommandCheck (Exactly "printf") (f . arguments) where
     f (doubledash:rest) | getLiteralString doubledash == Just "--" = f rest
     f (dashv:var:rest) | getLiteralString dashv == Just "-v" = f rest
@@ -525,11 +521,20 @@ checkPrintfVar = CommandCheck (Exactly "printf") (f . arguments) where
         case string of
             '%':'%':rest -> countFormats rest
             '%':'(':rest -> 1 + countFormats (dropWhile (/= ')') rest)
-            '%':'*':rest -> 2 + countFormats rest -- width is specified as an argument
+            '%':rest -> regexBasedCountFormats rest + countFormats (dropWhile (/= '%') rest)
-            '%':rest -> 1 + countFormats rest
             _:rest -> countFormats rest
             [] -> 0
 
+    regexBasedCountFormats rest =
+        maybe 1 (foldl (\acc group -> acc + (if group == "*" then 1 else 0)) 1) (matchRegex re rest)
+      where
+        -- constructed based on specifications in "man printf"
+        re = mkRegex "#?-?\\+? ?0?(\\*|\\d*).?(\\d*|\\*)[diouxXfFeEgGaAcsb]"
+        --            \____ _____/\___ ____/ \____ ____/\________ ________/
+        --                 V          V           V               V
+        --               flags    field width  precision   format character
+        -- field width and precision can be specified with a '*' instead of a digit,
+        -- in which case printf will accept one more argument for each '*' used
     check format more = do
         fromMaybe (return ()) $ do
             string <- getLiteralString format
@@ -678,20 +683,24 @@ prop_checkFindWithoutPath3 = verifyNot checkFindWithoutPath "find . -type f"
 prop_checkFindWithoutPath4 = verifyNot checkFindWithoutPath "find -H -L \"$path\" -print"
 prop_checkFindWithoutPath5 = verifyNot checkFindWithoutPath "find -O3 ."
 prop_checkFindWithoutPath6 = verifyNot checkFindWithoutPath "find -D exec ."
+prop_checkFindWithoutPath7 = verifyNot checkFindWithoutPath "find --help"
+prop_checkFindWithoutPath8 = verifyNot checkFindWithoutPath "find -Hx . -print"
 checkFindWithoutPath = CommandCheck (Basename "find") f
   where
-    f (T_SimpleCommand _ _ (cmd:args)) =
+    f t@(T_SimpleCommand _ _ (cmd:args)) =
-        unless (hasPath args) $
+        unless (t `hasFlag` "help" || hasPath args) $
             info (getId cmd) 2185 "Some finds don't have a default path. Specify '.' explicitly."
 
-    -- This is a bit of a kludge. find supports flag arguments both before and after the path,
+    -- This is a bit of a kludge. find supports flag arguments both before and
-    -- as well as multiple non-flag arguments that are not the path. We assume that all the
+    -- after the path, as well as multiple non-flag arguments that are not the
-    -- pre-path flags are single characters, which is generally the case except for -O3.
+    -- path. We assume that all the pre-path flags are single characters from a
+    -- list of GNU and macOS flags.
     hasPath (first:rest) =
         let flag = fromJust $ getLiteralStringExt (const $ return "___") first in
             not ("-" `isPrefixOf` flag) || isLeadingFlag flag && hasPath rest
     hasPath [] = False
-    isLeadingFlag flag = length flag <= 2 || "-O" `isPrefixOf` flag
+    isLeadingFlag flag = length flag <= 2 || all (`elem` leadingFlagChars) flag
+    leadingFlagChars="-EHLPXdfsxO0123456789"
 
 
 prop_checkTimeParameters1 = verify checkTimeParameters "time -f lol sleep 10"
@@ -740,20 +749,20 @@ checkLocalScope = CommandCheck (Exactly "local") $ \t ->
     whenShell [Bash, Dash] $ do -- Ksh allows it, Sh doesn't support local
         path <- getPathM t
         unless (any isFunction path) $
-            err (getId t) 2168 "'local' is only valid in functions."
+            err (getId $ getCommandTokenOrThis t) 2168 "'local' is only valid in functions."
 
 prop_checkDeprecatedTempfile1 = verify checkDeprecatedTempfile "var=$(tempfile)"
 prop_checkDeprecatedTempfile2 = verifyNot checkDeprecatedTempfile "tempfile=$(mktemp)"
 checkDeprecatedTempfile = CommandCheck (Basename "tempfile") $
-    \t -> warn (getId t) 2186 "tempfile is deprecated. Use mktemp instead."
+    \t -> warn (getId $ getCommandTokenOrThis t) 2186 "tempfile is deprecated. Use mktemp instead."
 
 prop_checkDeprecatedEgrep = verify checkDeprecatedEgrep "egrep '.+'"
 checkDeprecatedEgrep = CommandCheck (Basename "egrep") $
-    \t -> info (getId t) 2196 "egrep is non-standard and deprecated. Use grep -E instead."
+    \t -> info (getId $ getCommandTokenOrThis t) 2196 "egrep is non-standard and deprecated. Use grep -E instead."
 
 prop_checkDeprecatedFgrep = verify checkDeprecatedFgrep "fgrep '*' files"
 checkDeprecatedFgrep = CommandCheck (Basename "fgrep") $
-    \t -> info (getId t) 2197 "fgrep is non-standard and deprecated. Use grep -F instead."
+    \t -> info (getId $ getCommandTokenOrThis t) 2197 "fgrep is non-standard and deprecated. Use grep -F instead."
 
 prop_checkWhileGetoptsCase1 = verify checkWhileGetoptsCase "while getopts 'a:b' x; do case $x in a) foo;; esac; done"
 prop_checkWhileGetoptsCase2 = verify checkWhileGetoptsCase "while getopts 'a:' x; do case $x in a) foo;; b) bar;; esac; done"
@@ -947,7 +956,7 @@ checkFindRedirections = CommandCheck (Basename "find") f
 
 prop_checkWhich = verify checkWhich "which '.+'"
 checkWhich = CommandCheck (Basename "which") $
-    \t -> info (getId t) 2230 "which is non-standard. Use builtin 'command -v' instead."
+    \t -> info (getId $ getCommandTokenOrThis t) 2230 "which is non-standard. Use builtin 'command -v' instead."
 
 prop_checkSudoRedirect1 = verify checkSudoRedirect "sudo echo 3 > /proc/file"
 prop_checkSudoRedirect2 = verify checkSudoRedirect "sudo cmd < input"

src/ShellCheck/Checks/ShellSupport.hs

@@ -127,7 +127,7 @@ prop_checkBashisms44= verifyNot checkBashisms "#!/bin/dash\ntrap foo int"
 prop_checkBashisms45= verifyNot checkBashisms "#!/bin/dash\ntrap foo INT"
 prop_checkBashisms46= verify checkBashisms "#!/bin/dash\ntrap foo SIGINT"
 prop_checkBashisms47= verify checkBashisms "#!/bin/dash\necho foo 42>/dev/null"
-prop_checkBashisms48= verifyNot checkBashisms "#!/bin/dash\necho $LINENO"
+prop_checkBashisms48= verifyNot checkBashisms "#!/bin/sh\necho $LINENO"
 prop_checkBashisms49= verify checkBashisms "#!/bin/dash\necho $MACHTYPE"
 prop_checkBashisms50= verify checkBashisms "#!/bin/sh\ncmd >& file"
 prop_checkBashisms51= verifyNot checkBashisms "#!/bin/sh\ncmd 2>&1"
@@ -302,11 +302,11 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do
         (re $ "^[" ++ varChars ++ "*@]+(\\[.*\\])?/", "string replacement is")
         ]
     bashVars = [
-        "LINENO", "OSTYPE", "MACHTYPE", "HOSTTYPE", "HOSTNAME",
+        "OSTYPE", "MACHTYPE", "HOSTTYPE", "HOSTNAME",
         "DIRSTACK", "EUID", "UID", "SHLVL", "PIPESTATUS", "SHELLOPTS"
         ]
     bashDynamicVars = [ "RANDOM", "SECONDS" ]
-    dashVars = [ "LINENO" ]
+    dashVars = [ ]
     isBashVariable var =
         (var `elem` bashDynamicVars
             || var `elem` bashVars && not (isAssigned var))

src/ShellCheck/Data.hs

@@ -39,7 +39,7 @@ internalVariables = [
   ]
 
 variablesWithoutSpaces = [
-    "$", "-", "?", "!",
+    "$", "-", "?", "!", "#",
     "BASHPID", "BASH_ARGC", "BASH_LINENO", "BASH_SUBSHELL", "EUID", "LINENO",
     "OPTIND", "PPID", "RANDOM", "SECONDS", "SHELLOPTS", "SHLVL", "UID",
     "COLUMNS", "HISTFILESIZE", "HISTSIZE", "LINES"

src/ShellCheck/Formatter/Format.hs

@@ -30,17 +30,17 @@ data Formatter = Formatter {
     footer :: IO ()
 }
 
-sourceFile (PositionedComment pos _ _) = posFile pos
+sourceFile = posFile . pcStartPos
-lineNo (PositionedComment pos _ _) = posLine pos
+lineNo = posLine . pcStartPos
-endLineNo (PositionedComment _ end _) = posLine end
+endLineNo = posLine . pcEndPos
-colNo  (PositionedComment pos _ _) = posColumn pos
+colNo  = posColumn . pcStartPos
-endColNo  (PositionedComment _ end _) = posColumn end
+endColNo = posColumn . pcEndPos
-codeNo (PositionedComment _ _ (Comment _ code _)) = code
+codeNo = cCode . pcComment
-messageText (PositionedComment _ _ (Comment _ _ t)) = t
+messageText = cMessage . pcComment
 
 severityText :: PositionedComment -> String
-severityText (PositionedComment _ _ (Comment c _ _)) =
+severityText pc =
-    case c of
+    case cSeverity (pcComment pc) of
         ErrorC   -> "error"
         WarningC -> "warning"
         InfoC    -> "info"
@@ -51,11 +51,14 @@ makeNonVirtual comments contents =
     map fix comments
   where
     ls = lines contents
-    fix c@(PositionedComment start end comment) = PositionedComment start {
+    fix c = c {
-        posColumn = realignColumn lineNo colNo c
+        pcStartPos = (pcStartPos c) {
-    } end {
+            posColumn = realignColumn lineNo colNo c
-        posColumn = realignColumn endLineNo endColNo c
+        }
-    } comment
+      , pcEndPos = (pcEndPos c) {
+            posColumn = realignColumn endLineNo endColNo c
+        }
+    }
     realignColumn lineNo colNo c =
       if lineNo c > 0 && lineNo c <= fromIntegral (length ls)
       then real (ls !! fromIntegral (lineNo c - 1)) 0 0 (colNo c)

src/ShellCheck/Formatter/JSON.hs

@@ -39,8 +39,24 @@ format = do
         footer = finish ref
     }
 
-instance ToJSON (PositionedComment) where
+instance ToJSON Replacement where
-  toJSON comment@(PositionedComment start end (Comment level code string)) =
+    toJSON replacement =
+        let start = repStartPos replacement
+            end = repEndPos replacement
+            str = repString replacement in
+        object [
+          "line" .= posLine start,
+          "endLine" .= posLine end,
+          "column" .= posColumn start,
+          "endColumn" .= posColumn end,
+          "replaceWith" .= str
+        ]
+
+instance ToJSON PositionedComment where
+  toJSON comment =
+    let start = pcStartPos comment
+        end = pcEndPos comment
+        c = pcComment comment in
     object [
       "file" .= posFile start,
       "line" .= posLine start,
@@ -48,11 +64,15 @@ instance ToJSON (PositionedComment) where
       "column" .= posColumn start,
       "endColumn" .= posColumn end,
       "level" .= severityText comment,
-      "code" .= code,
+      "code" .= cCode c,
-      "message" .= string
+      "message" .= cMessage c,
+      "fix" .= pcFix comment
     ]
 
-  toEncoding comment@(PositionedComment start end (Comment level code string)) =
+  toEncoding comment =
+    let start = pcStartPos comment
+        end = pcEndPos comment
+        c = pcComment comment in
     pairs (
          "file" .= posFile start
       <> "line" .= posLine start
@@ -60,10 +80,16 @@ instance ToJSON (PositionedComment) where
       <> "column" .= posColumn start
       <> "endColumn" .= posColumn end
       <> "level" .= severityText comment
-      <> "code" .= code
+      <> "code" .= cCode c
-      <> "message" .= string
+      <> "message" .= cMessage c
+      <> "fix" .= pcFix comment
     )
 
+instance ToJSON Fix where
+    toJSON fix = object [
+        "replacements" .= fixReplacements fix
+        ]
+
 outputError file msg = hPutStrLn stderr $ file ++ ": " ++ msg
 collectResult ref result _ =
     modifyIORef ref (\x -> crComments result ++ x)
@@ -71,4 +97,3 @@ collectResult ref result _ =
 finish ref = do
     list <- readIORef ref
     BL.putStrLn $ encode list
-

src/ShellCheck/Formatter/TTY.hs

@@ -22,18 +22,28 @@ module ShellCheck.Formatter.TTY (format) where
 import ShellCheck.Interface
 import ShellCheck.Formatter.Format
 
+import Control.Monad
+import Data.IORef
 import Data.List
+import Data.Maybe
 import GHC.Exts
-import System.Info
 import System.IO
+import System.Info
+
+wikiLink = "https://www.shellcheck.net/wiki/"
+
+-- An arbitrary Ord thing to order warnings
+type Ranking = (Char, Severity, Integer)
 
 format :: FormatterOptions -> IO Formatter
-format options = return Formatter {
+format options = do
-    header = return (),
+    topErrorRef <- newIORef []
-    footer = return (),
+    return Formatter {
-    onFailure = outputError options,
+        header = return (),
-    onResult = outputResult options
+        footer = outputWiki topErrorRef,
-}
+        onFailure = outputError options,
+        onResult = outputResult options topErrorRef
+    }
 
 colorForLevel level =
     case level of
@@ -45,13 +55,60 @@ colorForLevel level =
         "source"  -> 0 -- none
         _ -> 0         -- none
 
+rankError :: PositionedComment -> Ranking
+rankError err = (ranking, cSeverity $ pcComment err, cCode $ pcComment err)
+  where
+    ranking =
+        if cCode (pcComment err) `elem` uninteresting
+        then 'Z'
+        else 'A'
+
+    -- A list of the most generic, least directly helpful
+    -- error codes to downrank.
+    uninteresting = [
+        1009, -- Mentioned parser error was..
+        1019, -- Expected this to be an argument
+        1036, -- ( is invalid here
+        1047, -- Expected 'fi'
+        1062, -- Expected 'done'
+        1070, -- Parsing stopped here (generic)
+        1072, -- Missing/unexpected ..
+        1073, -- Couldn't parse this ..
+        1088, -- Parsing stopped here (paren)
+        1089  -- Parsing stopped here (keyword)
+        ]
+
+appendComments errRef comments max = do
+    previous <- readIORef errRef
+    let current = map (\x -> (rankError x, cCode $ pcComment x, cMessage $ pcComment x)) comments
+    writeIORef errRef . take max . nubBy equal . sort $ previous ++ current
+  where
+    fst3 (x,_,_) = x
+    equal x y = fst3 x == fst3 y
+
+outputWiki :: IORef [(Ranking, Integer, String)] -> IO ()
+outputWiki errRef = do
+    issues <- readIORef errRef
+    unless (null issues) $ do
+        putStrLn "For more information:"
+        mapM_ showErr issues
+  where
+    showErr (_, code, msg) =
+        putStrLn $ "  " ++ wikiLink ++ "SC" ++ show code ++ " -- " ++ shorten msg
+    limit = 40
+    shorten msg =
+        if length msg < limit
+        then msg
+        else (take (limit-3) msg) ++ "..."
+
 outputError options file error = do
     color <- getColorFunc $ foColorOption options
     hPutStrLn stderr $ color "error" $ file ++ ": " ++ error
 
-outputResult options result sys = do
+outputResult options ref result sys = do
     color <- getColorFunc $ foColorOption options
     let comments = crComments result
+    appendComments ref comments (fromIntegral $ foWikiLinkCount options)
     let fileGroups = groupWith sourceFile comments
     mapM_ (outputForFile color sys) fileGroups
 
@@ -62,8 +119,8 @@ outputForFile color sys comments = do
     let fileLines = lines contents
     let lineCount = fromIntegral $ length fileLines
     let groups = groupWith lineNo comments
-    mapM_ (\x -> do
+    mapM_ (\commentsForLine -> do
-        let lineNum = lineNo (head x)
+        let lineNum = lineNo (head commentsForLine)
         let line = if lineNum < 1 || lineNum > lineCount
                         then ""
                         else fileLines !! fromIntegral (lineNum - 1)
@@ -71,16 +128,75 @@ outputForFile color sys comments = do
         putStrLn $ color "message" $
            "In " ++ fileName ++" line " ++ show lineNum ++ ":"
         putStrLn (color "source" line)
-        mapM_ (\c -> putStrLn (color (severityText c) $ cuteIndent c)) x
+        mapM_ (\c -> putStrLn (color (severityText c) $ cuteIndent c)) commentsForLine
         putStrLn ""
+        -- FIXME: Enable when reasonably stable
+        -- showFixedString color comments lineNum line
       ) groups
 
+hasApplicableFix lineNum comment = fromMaybe False $ do
+    replacements <- fixReplacements <$> pcFix comment
+    guard $ all (\c -> onSameLine (repStartPos c) && onSameLine (repEndPos c)) replacements
+    return True
+  where
+    onSameLine pos = posLine pos == lineNum
+
+-- FIXME: Work correctly with multiple replacements
+showFixedString color comments lineNum line =
+    case filter (hasApplicableFix lineNum) comments of
+        (first:_) -> do
+            -- in the spirit of error prone
+            putStrLn $ color "message" "Did you mean: "
+            putStrLn $ fixedString first line
+            putStrLn ""
+        _ -> return ()
+
+-- need to do something smart about sorting by end index
+fixedString :: PositionedComment -> String -> String
+fixedString comment line =
+    case (pcFix comment) of
+    Nothing -> ""
+    Just rs ->
+        applyReplacement (fixReplacements rs) line 0
+        where
+            applyReplacement [] s _ = s
+            applyReplacement (rep:xs) s offset =
+                let replacementString = repString rep
+                    start = (posColumn . repStartPos) rep
+                    end = (posColumn . repEndPos) rep
+                    z = doReplace start end s replacementString
+                    len_r = (fromIntegral . length) replacementString in
+                applyReplacement xs z (offset + (end - start) + len_r)
+
+-- FIXME: Work correctly with tabs
+-- start and end comes from pos, which is 1 based
+-- doReplace 0 0 "1234" "A" -> "A1234" -- technically not valid
+-- doReplace 1 1 "1234" "A" -> "A1234"
+-- doReplace 1 2 "1234" "A" -> "A234"
+-- doReplace 3 3 "1234" "A" -> "12A34"
+-- doReplace 4 4 "1234" "A" -> "123A4"
+-- doReplace 5 5 "1234" "A" -> "1234A"
+doReplace start end o r =
+    let si = fromIntegral (start-1)
+        ei = fromIntegral (end-1)
+        (x, xs) = splitAt si o
+        (y, z) = splitAt (ei - si) xs
+    in
+    x ++ r ++ z
+
 cuteIndent :: PositionedComment -> String
 cuteIndent comment =
     replicate (fromIntegral $ colNo comment - 1) ' ' ++
-        "^-- " ++ code (codeNo comment) ++ ": " ++ messageText comment
+        makeArrow ++ " " ++ code (codeNo comment) ++ ": " ++ messageText comment
+  where
+    arrow n = '^' : replicate (fromIntegral $ n-2) '-' ++ "^"
+    makeArrow =
+        let sameLine = lineNo comment == endLineNo comment
+            delta = endColNo comment - colNo comment
+        in
+            if sameLine && delta > 2 && delta < 32 then arrow delta else "^--"
 
-code code = "SC" ++ show code
+code num = "SC" ++ show num
 
 getColorFunc colorOption = do
     term <- hIsTerminalDevice stdout

src/ShellCheck/Interface.hs

@@ -17,10 +17,51 @@
     You should have received a copy of the GNU General Public License
     along with this program.  If not, see <https://www.gnu.org/licenses/>.
 -}
-module ShellCheck.Interface where
+{-# LANGUAGE DeriveGeneric, DeriveAnyClass #-}
+module ShellCheck.Interface
+    (
+    SystemInterface(..)
+    , CheckSpec(csFilename, csScript, csCheckSourced, csExcludedWarnings, csShellTypeOverride, csMinSeverity)
+    , CheckResult(crFilename, crComments)
+    , ParseSpec(psFilename, psScript, psCheckSourced, psShellTypeOverride)
+    , ParseResult(prComments, prTokenPositions, prRoot)
+    , AnalysisSpec(asScript, asShellType, asExecutionMode, asCheckSourced, asTokenPositions)
+    , AnalysisResult(arComments)
+    , FormatterOptions(foColorOption, foWikiLinkCount)
+    , Shell(Ksh, Sh, Bash, Dash)
+    , ExecutionMode(Executed, Sourced)
+    , ErrorMessage
+    , Code
+    , Severity(ErrorC, WarningC, InfoC, StyleC)
+    , Position(posFile, posLine, posColumn)
+    , Comment(cSeverity, cCode, cMessage)
+    , PositionedComment(pcStartPos , pcEndPos , pcComment, pcFix)
+    , ColorOption(ColorAuto, ColorAlways, ColorNever)
+    , TokenComment(tcId, tcComment, tcFix)
+    , emptyCheckResult
+    , newParseResult
+    , newAnalysisSpec
+    , newAnalysisResult
+    , newFormatterOptions
+    , newPosition
+    , newTokenComment
+    , mockedSystemInterface
+    , newParseSpec
+    , emptyCheckSpec
+    , newPositionedComment
+    , newComment
+    , Fix(fixReplacements)
+    , newFix
+    , Replacement(repStartPos, repEndPos, repString)
+    , newReplacement
+    ) where
 
 import ShellCheck.AST
+
+import Control.DeepSeq
 import Control.Monad.Identity
+import Data.Monoid
+import GHC.Generics (Generic)
 import qualified Data.Map as Map
 
 
@@ -35,7 +76,8 @@ data CheckSpec = CheckSpec {
     csScript :: String,
     csCheckSourced :: Bool,
     csExcludedWarnings :: [Integer],
-    csShellTypeOverride :: Maybe Shell
+    csShellTypeOverride :: Maybe Shell,
+    csMinSeverity :: Severity
 } deriving (Show, Eq)
 
 data CheckResult = CheckResult {
@@ -43,44 +85,85 @@ data CheckResult = CheckResult {
     crComments :: [PositionedComment]
 } deriving (Show, Eq)
 
+emptyCheckResult :: CheckResult
+emptyCheckResult = CheckResult {
+    crFilename = "",
+    crComments = []
+}
+
 emptyCheckSpec :: CheckSpec
 emptyCheckSpec = CheckSpec {
     csFilename = "",
     csScript = "",
     csCheckSourced = False,
     csExcludedWarnings = [],
-    csShellTypeOverride = Nothing
+    csShellTypeOverride = Nothing,
+    csMinSeverity = StyleC
+}
+
+newParseSpec :: ParseSpec
+newParseSpec = ParseSpec {
+    psFilename = "",
+    psScript = "",
+    psCheckSourced = False,
+    psShellTypeOverride = Nothing
 }
 
 -- Parser input and output
 data ParseSpec = ParseSpec {
     psFilename :: String,
     psScript :: String,
-    psCheckSourced :: Bool
+    psCheckSourced :: Bool,
+    psShellTypeOverride :: Maybe Shell
 } deriving (Show, Eq)
 
 data ParseResult = ParseResult {
     prComments :: [PositionedComment],
-    prTokenPositions :: Map.Map Id Position,
+    prTokenPositions :: Map.Map Id (Position, Position),
     prRoot :: Maybe Token
 } deriving (Show, Eq)
 
+newParseResult :: ParseResult
+newParseResult = ParseResult {
+    prComments = [],
+    prTokenPositions = Map.empty,
+    prRoot = Nothing
+}
+
 -- Analyzer input and output
 data AnalysisSpec = AnalysisSpec {
     asScript :: Token,
     asShellType :: Maybe Shell,
     asExecutionMode :: ExecutionMode,
-    asCheckSourced :: Bool
+    asCheckSourced :: Bool,
+    asTokenPositions :: Map.Map Id (Position, Position)
+}
+
+newAnalysisSpec token = AnalysisSpec {
+    asScript = token,
+    asShellType = Nothing,
+    asExecutionMode = Executed,
+    asCheckSourced = False,
+    asTokenPositions = Map.empty
 }
 
 newtype AnalysisResult = AnalysisResult {
     arComments :: [TokenComment]
 }
 
+newAnalysisResult = AnalysisResult {
+    arComments = []
+}
 
 -- Formatter options
-newtype FormatterOptions = FormatterOptions {
+data FormatterOptions = FormatterOptions {
-    foColorOption :: ColorOption
+    foColorOption :: ColorOption,
+    foWikiLinkCount :: Integer
+}
+
+newFormatterOptions = FormatterOptions {
+    foColorOption = ColorAuto,
+    foWikiLinkCount = 3
 }
 
 
@@ -91,16 +174,81 @@ data ExecutionMode = Executed | Sourced deriving (Show, Eq)
 type ErrorMessage = String
 type Code = Integer
 
-data Severity = ErrorC | WarningC | InfoC | StyleC deriving (Show, Eq, Ord)
+data Severity = ErrorC | WarningC | InfoC | StyleC
+    deriving (Show, Eq, Ord, Generic, NFData)
 data Position = Position {
     posFile :: String,    -- Filename
     posLine :: Integer,   -- 1 based source line
     posColumn :: Integer  -- 1 based source column, where tabs are 8
-} deriving (Show, Eq)
+} deriving (Show, Eq, Generic, NFData)
 
-data Comment = Comment Severity Code String deriving (Show, Eq)
+newPosition :: Position
-data PositionedComment = PositionedComment Position Position Comment deriving (Show, Eq)
+newPosition = Position {
-data TokenComment = TokenComment Id Comment deriving (Show, Eq)
+    posFile   = "",
+    posLine   = 1,
+    posColumn = 1
+}
+
+data Comment = Comment {
+    cSeverity :: Severity,
+    cCode     :: Code,
+    cMessage  :: String
+} deriving (Show, Eq, Generic, NFData)
+
+newComment :: Comment
+newComment = Comment {
+    cSeverity = StyleC,
+    cCode     = 0,
+    cMessage  = ""
+}
+
+-- only support single line for now
+data Replacement = Replacement {
+    repStartPos :: Position,
+    repEndPos :: Position,
+    repString :: String
+} deriving (Show, Eq, Generic, NFData)
+
+newReplacement = Replacement {
+    repStartPos = newPosition,
+    repEndPos = newPosition,
+    repString = ""
+}
+
+data Fix = Fix {
+    fixReplacements :: [Replacement]
+} deriving (Show, Eq, Generic, NFData)
+
+newFix = Fix {
+    fixReplacements = []
+}
+
+data PositionedComment = PositionedComment {
+    pcStartPos :: Position,
+    pcEndPos   :: Position,
+    pcComment  :: Comment,
+    pcFix      :: Maybe Fix
+} deriving (Show, Eq, Generic, NFData)
+
+newPositionedComment :: PositionedComment
+newPositionedComment = PositionedComment {
+    pcStartPos = newPosition,
+    pcEndPos   = newPosition,
+    pcComment  = newComment,
+    pcFix      = Nothing
+}
+
+data TokenComment = TokenComment {
+    tcId :: Id,
+    tcComment :: Comment,
+    tcFix :: Maybe Fix
+} deriving (Show, Eq, Generic, NFData)
+
+newTokenComment = TokenComment {
+    tcId = Id 0,
+    tcComment = newComment,
+    tcFix = Nothing
+}
 
 data ColorOption =
     ColorAuto

striptests

@@ -5,6 +5,7 @@
 set -o pipefail
 
 sponge() {
+  local data
   data="$(cat)"
   printf '%s\n' "$data" > "$1"
 }
@@ -22,7 +23,7 @@ modify() {
 }
 
 detestify() {
-  echo "-- AUTOGENERATED from ShellCheck by striptests. Do not modify."
+  printf '%s\n' '-- AUTOGENERATED from ShellCheck by striptests. Do not modify.'
   awk '
     BEGIN {
       state = 0;
@@ -52,7 +53,7 @@ detestify() {
 
 
 
-if [[ ! -e ShellCheck.cabal ]]
+if [[ ! -e 'ShellCheck.cabal' ]]
 then
   echo "Run me from the ShellCheck directory." >&2
   exit 1
@@ -64,7 +65,7 @@ then
   exit 2
 fi
 
-modify ShellCheck.cabal sed -e '
+modify 'ShellCheck.cabal' sed -e '
   /QuickCheck/d
   /^test-suite/{ s/.*//; q; }
   '

test/buildtest

@@ -0,0 +1,35 @@
+#!/bin/bash
+# This script configures, builds and runs tests.
+# It's meant for automatic cross-distro testing.
+
+die() { echo "$*" >&2; exit 1; }
+
+[ -e "ShellCheck.cabal" ] ||
+  die "ShellCheck.cabal not in current dir"
+command -v cabal ||
+  die "cabal is missing"
+
+cabal update ||
+  die "can't update"
+cabal install --dependencies-only --enable-tests ||
+  die "can't install dependencies"
+cabal configure --enable-tests ||
+  die "configure failed"
+cabal build ||
+  die "build failed"
+cabal test ||
+  die "test failed"
+
+dist/build/shellcheck/shellcheck - << 'EOF' || die "execution failed"
+#!/bin/sh
+echo "Hello World"
+EOF
+
+dist/build/shellcheck/shellcheck - << 'EOF' && die "negative execution failed"
+#!/bin/sh
+echo $1
+EOF
+
+
+echo "Success"
+exit 0

test/distrotest

@@ -0,0 +1,75 @@
+#!/bin/bash
+# This script runs 'buildtest' on each of several distros
+# via Docker.
+set -o pipefail
+
+exec 3>&1 4>&2
+die() { echo "$*" >&4; exit 1; }
+
+[ -e "ShellCheck.cabal" ] || die "ShellCheck.cabal not in this dir"
+
+[ "$1" = "--run" ] || {
+cat << EOF
+This script pulls multiple distros via Docker and compiles
+ShellCheck and dependencies for each one. It takes hours,
+and is still highly experimental.
+
+Make sure you're plugged in and have screen/tmux in place,
+then re-run with $0 --run to continue.
+EOF
+exit 0
+}
+
+
+log=$(mktemp) || die "Can't create temp file"
+date >> "$log" || die "Can't write to log"
+
+echo "Logging to $log" >&3
+exec >> "$log" 2>&1
+
+final=0
+while read -r distro setup
+do
+  [[ "$distro" = "#"* || -z "$distro" ]] && continue
+  printf '%s ' "$distro" >&3
+  docker pull "$distro" || die "Can't pull $distro"
+  printf 'pulled. ' >&3
+
+  tmp=$(mktemp -d) || die "Can't make temp dir"
+  cp -r . "$tmp/" || die "Can't populate test dir"
+  printf 'Result: ' >&3
+  < /dev/null docker run -v "$tmp:/mnt" "$distro" sh -c "
+    $setup
+    cd /mnt || exit 1
+    test/buildtest
+    "
+  ret=$?
+  if [ "$ret" = 0 ]
+  then
+    echo "OK" >&3
+  else
+    echo "FAIL with $ret. See $log" >&3
+    final=1
+  fi
+  rm -rf "$tmp"
+done << EOF
+# Docker tag          Setup command
+debian:stable         apt-get update && apt-get install -y cabal-install
+debian:testing        apt-get update && apt-get install -y cabal-install
+ubuntu:latest         apt-get update && apt-get install -y cabal-install
+opensuse:latest       zypper install -y cabal-install ghc
+
+# Older Ubuntu versions we want to support
+ubuntu:18.04          apt-get update && apt-get install -y cabal-install
+ubuntu:17.10          apt-get update && apt-get install -y cabal-install
+
+# Misc
+haskell:latest        true
+
+# Known to currently fail
+centos:latest         yum install -y epel-release && yum install -y cabal-install
+fedora:latest         dnf install -y cabal-install
+base/archlinux:latest pacman -S -y --noconfirm cabal-install ghc-static base-devel
+EOF
+
+exit "$final"