diff --git a/LICENSE b/LICENSE --- a/LICENSE +++ b/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 diff --git a/README.md b/README.md --- a/README.md +++ b/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 @@ ![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). @@ -133,6 +133,10 @@ brew install shellcheck +On OpenBSD: + + pkg_add shellcheck + On openSUSE zypper in ShellCheck @@ -163,27 +167,35 @@ 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. +Distro packages already come with a `man` page. If you are building from source, it can be installed with: + + pandoc -s -t man shellcheck.1.md -o shellcheck.1 + sudo mv shellcheck.1 /usr/share/man/man1 + ## Travis CI 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 - - 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" - - shellcheck() { "shellcheck-$scversion/shellcheck" "$@"; } - - shellcheck --version +*Pre-requisite*: the program 'xz' needs to be installed on the system. +To install it on debian/ubuntu/linux mint, run `apt install xz-utils`. +To install it on Redhat/Fedora/CentOS, run `yum -y install xz`. - script: - - shellcheck *.sh +```bash +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 +454,8 @@ 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)! diff --git a/Setup.hs b/Setup.hs --- a/Setup.hs +++ b/Setup.hs @@ -33,4 +33,4 @@ putStrLn $ "pandoc exited with " ++ show result return emptyHookedBuildInfo where - pandoc_cmd = "pandoc -s -t man shellcheck.1.md -o shellcheck.1" + pandoc_cmd = "pandoc -s -f markdown-smart -t man shellcheck.1.md -o shellcheck.1" diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,5 +1,5 @@ Name: ShellCheck -Version: 0.5.0 +Version: 0.6.0 Synopsis: Shell script analysis tool License: GPL-3 License-file: LICENSE @@ -28,6 +28,8 @@ shellcheck.1.md -- built with a cabal sdist hook shellcheck.1 + -- convenience script for stripping tests + striptests -- tests test/shellcheck.hs @@ -35,7 +37,7 @@ 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 diff --git a/shellcheck.1 b/shellcheck.1 --- a/shellcheck.1 +++ b/shellcheck.1 @@ -1,4 +1,4 @@ -.\" Automatically generated by Pandoc 1.19.2.4 +.\" Automatically generated by Pandoc 2.2.1 .\" .TH "SHELLCHECK" "1" "" "Shell script analysis tool" "" .hy @@ -71,6 +71,14 @@ .RS .RE .TP +.B \f[B]\-S\f[]\ \f[I]SEVERITY\f[],\ \f[B]\-\-severity=\f[]\f[I]severity\f[] +Specify minimum severity of errors to consider. +Valid values are \f[I]error\f[], \f[I]warning\f[], \f[I]info\f[] and +\f[I]style\f[]. +The default is \f[I]style\f[]. +.RS +.RE +.TP .B \f[B]\-s\f[]\ \f[I]shell\f[],\ \f[B]\-\-shell=\f[]\f[I]shell\f[] Specify Bourne shell dialect. Valid values are \f[I]sh\f[], \f[I]bash\f[], \f[I]dash\f[] and @@ -82,6 +90,13 @@ .TP .B \f[B]\-V\f[],\ \f[B]\-\-version\f[] Print version information and exit. +.RS +.RE +.TP +.B \f[B]\-W\f[] \f[I]NUM\f[],\ \f[B]\-\-wiki\-link\-count=NUM\f[] +For TTY output, show \f[I]NUM\f[] wiki links to more information about +mentioned warnings. +Set to 0 to disable them entirely. .RS .RE .TP diff --git a/shellcheck.1.md b/shellcheck.1.md --- a/shellcheck.1.md +++ b/shellcheck.1.md @@ -56,6 +56,11 @@ 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*. @@ -65,6 +70,11 @@ **-V**,\ **--version** : 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** diff --git a/shellcheck.hs b/shellcheck.hs --- a/shellcheck.hs +++ b/shellcheck.hs @@ -67,15 +67,17 @@ 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 @@ 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 @@ 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,13 +224,29 @@ then NoProblems else SomeProblems -parseColorOption colorOption = - case colorOption of - "auto" -> ColorAuto - "always" -> ColorAlways - "never" -> ColorNever - _ -> error $ "Bad value for --color `" ++ colorOption ++ "'" +parseEnum name value list = + case filter ((== value) . fst) list of + [(name, value)] -> return value + [] -> do + printErr $ "Unknown value for --" ++ name ++ ". " ++ + "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 Flag "shell" str -> @@ -241,7 +259,7 @@ } 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 @@ 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 @@ } } + 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 @@ 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) diff --git a/src/ShellCheck/ASTLib.hs b/src/ShellCheck/ASTLib.hs --- a/src/ShellCheck/ASTLib.hs +++ b/src/ShellCheck/ASTLib.hs @@ -23,6 +23,7 @@ import Control.Monad.Writer import Control.Monad +import Data.Char import Data.Functor import Data.List import Data.Maybe @@ -226,8 +227,43 @@ 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 @@ T_Annotation _ _ t -> getCommand t _ -> Nothing --- Maybe get the command name of a token representing a command -getCommandName t = do +-- Maybe get the command name string of a token representing a command +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 - _ -> return s + (applet:_) -> return (getLiteralString applet, applet) + _ -> 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" diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -168,6 +168,8 @@ ,checkPipeToNowhere ,checkForLoopGlobVariables ,checkSubshelledTests + ,checkInvertedStringTest + ,checkRedirectionToCommand ] @@ -396,7 +398,7 @@ 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 +432,22 @@ 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 +716,7 @@ 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 +1027,7 @@ 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 +1037,16 @@ 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 () @@ -1186,11 +1197,12 @@ prop_checkComparisonAgainstGlob3 = verify checkComparisonAgainstGlob "[ $cow = *foo* ]" prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow = foo ]" prop_checkComparisonAgainstGlob5 = verify checkComparisonAgainstGlob "[[ $cow != $bar ]]" +prop_checkComparisonAgainstGlob6 = verify checkComparisonAgainstGlob "[ $f != /* ]" 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 = + | op `elem` ["=", "==", "!="] && isGlob word = err (getId word) 2081 "[ .. ] can't match globs. Use [[ .. ]] or case statement." checkComparisonAgainstGlob _ _ = return () @@ -1324,7 +1336,7 @@ prop_checkBackticks2 = verifyNot checkBackticks "echo $(foo)" prop_checkBackticks3 = verifyNot checkBackticks "echo `#inlined comment` foo" checkBackticks _ (T_Backticked id list) | not (null list) = - style id 2006 "Use $(..) instead of legacy `..`." + style id 2006 "Use $(...) notation instead of legacy backticked `...`." checkBackticks _ _ = return () prop_checkIndirectExpansion1 = verify checkIndirectExpansion "${foo$n}" @@ -1598,6 +1610,7 @@ 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) @@ -1879,6 +1892,7 @@ 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 +2083,7 @@ 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 +2106,20 @@ 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)) $ - info (getId $ last cds) 2103 message + potentially $ do + cd <- findCdPair cds + return $ info cd 2103 message message = "Use a ( subshell ) to avoid having to cd back." @@ -2472,7 +2497,7 @@ 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 +2526,7 @@ 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,7 +2536,7 @@ 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." checkElement _ = return () @@ -2685,26 +2711,31 @@ 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 fuzzyGlobs = tupMap wordToPseudoGlob patterns + let exactGlobs = tupMap wordToExactPseudoGlob breakpatterns + 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 +2868,7 @@ 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 +2881,25 @@ 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 +2968,10 @@ 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 +2991,24 @@ [c] | isTestCommand c -> True _ -> False - isSubshelledTest t = + isTestStructure t = case t of - T_Subshell _ list -> all isSubshelledTest list - T_AndIf _ a b -> isSubshelledTest a && isSubshelledTest b - T_OrIf _ a b -> isSubshelledTest a && isSubshelledTest b - T_Annotation _ _ t -> isSubshelledTest t + T_Banged _ t -> isTestStructure t + T_AndIf _ a b -> isTestStructure a && isTestStructure b + T_OrIf _ a b -> isTestStructure a && isTestStructure b + 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 _ _ T_Condition {}] -> True - T_Pipeline _ [] [T_Redirecting _ _ cmd] -> cmd `isCommand` "test" + T_Pipeline _ [] [T_Redirecting _ _ cmd] -> + case cmd of + T_Condition {} -> True + _ -> cmd `isCommand` "test" _ -> False -- Check if a T_Subshell is used as a condition, e.g. if ( test ) @@ -2980,6 +3028,36 @@ T_Pipeline _ [] _ -> True 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 }) ) |]) diff --git a/src/ShellCheck/Analyzer.hs b/src/ShellCheck/Analyzer.hs --- a/src/ShellCheck/Analyzer.hs +++ b/src/ShellCheck/Analyzer.hs @@ -30,7 +30,7 @@ -- 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 diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -109,19 +109,17 @@ data VariableState = Dead Token String | Alive deriving (Show) -defaultSpec root = AnalysisSpec { - asScript = root, +defaultSpec root = spec { asShellType = Nothing, asCheckSourced = False, asExecutionMode = Executed -} +} where spec = newAnalysisSpec root pScript s = let - pSpec = ParseSpec { + pSpec = newParseSpec { psFilename = "script", - psScript = s, - psCheckSourced = False + psScript = s } in prRoot . runIdentity $ parseScript (mockedSystemInterface []) pSpec @@ -135,7 +133,14 @@ 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] @@ -235,9 +240,10 @@ where pre t = modify (first ((:) t)) post t = do - (_:rest, map) <- get - case rest of [] -> put (rest, map) - (x:_) -> put (rest, Map.insert (getId t) x map) + (x, map) <- get + case x of + _: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 +526,22 @@ 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 - catMaybes . takeWhile isJust . reverse $ params + let params = map getLiteral rest + readArrayVars = getReadArrayVariables rest + in + catMaybes . (++ readArrayVars) . takeWhile isJust . reverse $ params "getopts" -> case rest of opts:var:_ -> maybeToList $ getLiteral var @@ -568,11 +584,15 @@ 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 getModifierParam def t@(T_Assignment _ _ name _ value) = @@ -613,6 +633,11 @@ 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 +837,9 @@ 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 +848,7 @@ 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) = diff --git a/src/ShellCheck/Checker.hs b/src/ShellCheck/Checker.hs --- a/src/ShellCheck/Checker.hs +++ b/src/ShellCheck/Checker.hs @@ -37,25 +37,30 @@ import Test.QuickCheck.All -tokenToPosition map (TokenComment id c) = fromMaybe fail $ do - position <- Map.lookup id map - return $ PositionedComment position position c +tokenToPosition startMap t = fromMaybe fail $ do + span <- Map.lookup (tcId t) startMap + return $ newPositionedComment { + pcStartPos = fst span, + pcEndPos = snd span, + pcComment = tcComment 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 analysisMessages = @@ -66,27 +71,38 @@ return . nub . sortMessages . filter shouldInclude $ (parseMessages ++ map translator analysisMessages) - shouldInclude (PositionedComment _ _ (Comment _ code _)) = - code `notElem` csExcludedWarnings spec + shouldInclude pc = + 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)) = - (posFile pos, posLine pos, posColumn pos, severity, code, message) - getPosition (PositionedComment pos _ _) = pos + order pc = + let pos = pcStartPos pc + comment = pcComment pc in + (posFile pos, + posLine pos, + posColumn pos, + cSeverity comment, + cCode comment, + cMessage comment) + getPosition = pcStartPos analysisSpec root = - AnalysisSpec { + as { asScript = root, asShellType = csShellTypeOverride spec, asCheckSourced = csCheckSourced spec, asExecutionMode = Executed - } + } where as = newAnalysisSpec root getErrors sys spec = sort . map getCode . crComments $ runIdentity (checkScript sys spec) where - getCode (PositionedComment _ _ (Comment _ code _)) = code + getCode = cCode . pcComment check = checkWithIncludes [] @@ -136,6 +152,21 @@ 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 +211,7 @@ 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 $ @@ -196,7 +227,6 @@ prop_sourcePartOfOriginalScript = -- #1181: -x disabled posix warning for 'source' 2039 `elem` checkWithIncludes [("./saywhat.sh", "echo foo")] "#!/bin/sh\nsource ./saywhat.sh" - return [] runTests = $quickCheckAll diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -61,6 +61,7 @@ ,checkGrepRe ,checkTrapQuotes ,checkReturn + ,checkExit ,checkFindExecWithSingleArgument ,checkUnusedEchoEscapes ,checkInjectableFindSh @@ -92,6 +93,7 @@ ,checkWhich ,checkSudoRedirect ,checkSudoArgs + ,checkSourceArgs ] buildCommandMap :: [CommandCheck] -> Map.Map CommandName (Token -> Analysis) @@ -141,6 +143,7 @@ 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 +155,7 @@ 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 +186,7 @@ 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 = [ ":", "<", ">", "<=", ">=" ] @@ -279,15 +282,28 @@ prop_checkReturn5 = verify checkReturn "return -1" prop_checkReturn6 = verify checkReturn "return 1000" prop_checkReturn7 = verify checkReturn "return 'hello world'" -checkReturn = CommandCheck (Exactly "return") (f . arguments) +checkReturn = CommandCheck (Exactly "return") (returnOrExit + (\c -> err c 2151 "Only one integer 0-255 can be returned. Use stdout for other data.") + (\c -> err c 2152 "Can only return 0-255. Other data should be written to stdout.")) + +prop_checkExit1 = verifyNot checkExit "exit" +prop_checkExit2 = verifyNot checkExit "exit 1" +prop_checkExit3 = verifyNot checkExit "exit $var" +prop_checkExit4 = verifyNot checkExit "exit $((a|b))" +prop_checkExit5 = verify checkExit "exit -1" +prop_checkExit6 = verify checkExit "exit 1000" +prop_checkExit7 = verify checkExit "exit 'hello world'" +checkExit = CommandCheck (Exactly "exit") (returnOrExit + (\c -> err c 2241 "The exit status can only be one integer 0-255. Use stdout for other data.") + (\c -> err c 2242 "Can only exit with status 0-255. Other data should be written to stdout/stderr.")) + +returnOrExit multi invalid = (f . arguments) where f (first:second:_) = - err (getId second) 2151 - "Only one integer 0-255 can be returned. Use stdout for other data." + multi (getId first) f [value] = when (isInvalid $ literal value) $ - err (getId value) 2152 - "Can only return 0-255. Other data should be written to stdout." + invalid (getId value) f _ = return () isInvalid s = s == "" || any (not . isDigit) s || length s > 5 @@ -324,26 +340,18 @@ 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 = - return () - where allButLast = reverse . drop 1 . reverse $ args - f args = mapM_ checkEscapes args - - checkEscapes (T_NormalWord _ args) = - mapM_ checkEscapes args - 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 () + f cmd = + whenShell [Sh, Bash, Ksh] $ + unless (cmd `hasFlag` "e") $ + mapM_ examine $ arguments cmd - examine id str = + examine token = do + let str = onlyLiteralString token 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 +523,9 @@ 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 +536,20 @@ case string of '%':'%':rest -> countFormats rest '%':'(':rest -> 1 + countFormats (dropWhile (/= ')') rest) - '%':'*':rest -> 2 + countFormats rest -- width is specified as an argument - '%':rest -> 1 + countFormats rest + '%':rest -> regexBasedCountFormats rest + countFormats (dropWhile (/= '%') 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 +698,24 @@ 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)) = - unless (hasPath args) $ + f t@(T_SimpleCommand _ _ (cmd: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, - -- as well as multiple non-flag arguments that are not the path. We assume that all the - -- pre-path flags are single characters, which is generally the case except for -O3. + -- This is a bit of a kludge. find supports flag arguments both before and + -- after the path, as well as multiple non-flag arguments that are not the + -- 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 +764,20 @@ 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 +971,7 @@ 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" @@ -998,6 +1022,17 @@ builtins = [ "cd", "eval", "export", "history", "read", "source", "wait" ] -- This mess is why ShellCheck prefers not to know. parseOpts = getBsdOpts "vAknSbEHPa:g:h:p:u:c:T:r:" + +prop_checkSourceArgs1 = verify checkSourceArgs "#!/bin/sh\n. script arg" +prop_checkSourceArgs2 = verifyNot checkSourceArgs "#!/bin/sh\n. script" +prop_checkSourceArgs3 = verifyNot checkSourceArgs "#!/bin/bash\n. script arg" +checkSourceArgs = CommandCheck (Exactly ".") f + where + f t = whenShell [Sh, Dash] $ + case arguments t of + (file:arg1:_) -> warn (getId arg1) 2240 $ + "The dot command does not support arguments in sh/dash. Set them as variables." + _ -> return () return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs --- a/src/ShellCheck/Checks/ShellSupport.hs +++ b/src/ShellCheck/Checks/ShellSupport.hs @@ -127,7 +127,7 @@ 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 @@ (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)) diff --git a/src/ShellCheck/Data.hs b/src/ShellCheck/Data.hs --- a/src/ShellCheck/Data.hs +++ b/src/ShellCheck/Data.hs @@ -39,7 +39,7 @@ ] variablesWithoutSpaces = [ - "$", "-", "?", "!", + "$", "-", "?", "!", "#", "BASHPID", "BASH_ARGC", "BASH_LINENO", "BASH_SUBSHELL", "EUID", "LINENO", "OPTIND", "PPID", "RANDOM", "SECONDS", "SHELLOPTS", "SHLVL", "UID", "COLUMNS", "HISTFILESIZE", "HISTSIZE", "LINES" diff --git a/src/ShellCheck/Formatter/Format.hs b/src/ShellCheck/Formatter/Format.hs --- a/src/ShellCheck/Formatter/Format.hs +++ b/src/ShellCheck/Formatter/Format.hs @@ -30,17 +30,17 @@ footer :: IO () } -sourceFile (PositionedComment pos _ _) = posFile pos -lineNo (PositionedComment pos _ _) = posLine pos -endLineNo (PositionedComment _ end _) = posLine end -colNo (PositionedComment pos _ _) = posColumn pos -endColNo (PositionedComment _ end _) = posColumn end -codeNo (PositionedComment _ _ (Comment _ code _)) = code -messageText (PositionedComment _ _ (Comment _ _ t)) = t +sourceFile = posFile . pcStartPos +lineNo = posLine . pcStartPos +endLineNo = posLine . pcEndPos +colNo = posColumn . pcStartPos +endColNo = posColumn . pcEndPos +codeNo = cCode . pcComment +messageText = cMessage . pcComment severityText :: PositionedComment -> String -severityText (PositionedComment _ _ (Comment c _ _)) = - case c of +severityText pc = + case cSeverity (pcComment pc) of ErrorC -> "error" WarningC -> "warning" InfoC -> "info" @@ -51,11 +51,14 @@ map fix comments where ls = lines contents - fix c@(PositionedComment start end comment) = PositionedComment start { - posColumn = realignColumn lineNo colNo c - } end { - posColumn = realignColumn endLineNo endColNo c - } comment + fix c = c { + pcStartPos = (pcStartPos c) { + posColumn = realignColumn lineNo colNo c + } + , 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) diff --git a/src/ShellCheck/Formatter/JSON.hs b/src/ShellCheck/Formatter/JSON.hs --- a/src/ShellCheck/Formatter/JSON.hs +++ b/src/ShellCheck/Formatter/JSON.hs @@ -40,7 +40,10 @@ } instance ToJSON (PositionedComment) where - toJSON comment@(PositionedComment start end (Comment level code string)) = + toJSON comment = + let start = pcStartPos comment + end = pcEndPos comment + c = pcComment comment in object [ "file" .= posFile start, "line" .= posLine start, @@ -48,11 +51,14 @@ "column" .= posColumn start, "endColumn" .= posColumn end, "level" .= severityText comment, - "code" .= code, - "message" .= string + "code" .= cCode c, + "message" .= cMessage c ] - 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,8 +66,8 @@ <> "column" .= posColumn start <> "endColumn" .= posColumn end <> "level" .= severityText comment - <> "code" .= code - <> "message" .= string + <> "code" .= cCode c + <> "message" .= cMessage c ) outputError file msg = hPutStrLn stderr $ file ++ ": " ++ msg diff --git a/src/ShellCheck/Formatter/TTY.hs b/src/ShellCheck/Formatter/TTY.hs --- a/src/ShellCheck/Formatter/TTY.hs +++ b/src/ShellCheck/Formatter/TTY.hs @@ -22,18 +22,27 @@ import ShellCheck.Interface import ShellCheck.Formatter.Format +import Control.Monad +import Data.IORef import Data.List 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 { - header = return (), - footer = return (), - onFailure = outputError options, - onResult = outputResult options -} +format options = do + topErrorRef <- newIORef [] + return Formatter { + header = return (), + footer = outputWiki topErrorRef, + onFailure = outputError options, + onResult = outputResult options topErrorRef + } colorForLevel level = case level of @@ -45,13 +54,60 @@ "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 = 36 + 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 @@ -78,9 +134,16 @@ 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 diff --git a/src/ShellCheck/Interface.hs b/src/ShellCheck/Interface.hs --- a/src/ShellCheck/Interface.hs +++ b/src/ShellCheck/Interface.hs @@ -17,7 +17,39 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . -} -module ShellCheck.Interface where +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) + , 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) + , ColorOption(ColorAuto, ColorAlways, ColorNever) + , TokenComment(tcId, tcComment) + , emptyCheckResult + , newParseResult + , newAnalysisSpec + , newAnalysisResult + , newFormatterOptions + , newPosition + , newTokenComment + , mockedSystemInterface + , newParseSpec + , emptyCheckSpec + , newPositionedComment + , newComment + ) where import ShellCheck.AST import Control.Monad.Identity @@ -35,7 +67,8 @@ csScript :: String, csCheckSourced :: Bool, csExcludedWarnings :: [Integer], - csShellTypeOverride :: Maybe Shell + csShellTypeOverride :: Maybe Shell, + csMinSeverity :: Severity } deriving (Show, Eq) data CheckResult = CheckResult { @@ -43,28 +76,51 @@ 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, @@ -73,17 +129,33 @@ asCheckSourced :: Bool } +newAnalysisSpec token = AnalysisSpec { + asScript = token, + asShellType = Nothing, + asExecutionMode = Executed, + asCheckSourced = False +} + newtype AnalysisResult = AnalysisResult { arComments :: [TokenComment] } +newAnalysisResult = AnalysisResult { + arComments = [] +} -- Formatter options -newtype FormatterOptions = FormatterOptions { - foColorOption :: ColorOption +data FormatterOptions = FormatterOptions { + foColorOption :: ColorOption, + foWikiLinkCount :: Integer } +newFormatterOptions = FormatterOptions { + foColorOption = ColorAuto, + foWikiLinkCount = 3 +} + -- Supporting data types data Shell = Ksh | Sh | Bash | Dash deriving (Show, Eq) data ExecutionMode = Executed | Sourced deriving (Show, Eq) @@ -98,9 +170,48 @@ posColumn :: Integer -- 1 based source column, where tabs are 8 } deriving (Show, Eq) -data Comment = Comment Severity Code String deriving (Show, Eq) -data PositionedComment = PositionedComment Position Position Comment deriving (Show, Eq) -data TokenComment = TokenComment Id Comment deriving (Show, Eq) +newPosition :: Position +newPosition = Position { + posFile = "", + posLine = 1, + posColumn = 1 +} + +data Comment = Comment { + cSeverity :: Severity, + cCode :: Code, + cMessage :: String +} deriving (Show, Eq) + +newComment :: Comment +newComment = Comment { + cSeverity = StyleC, + cCode = 0, + cMessage = "" +} + +data PositionedComment = PositionedComment { + pcStartPos :: Position, + pcEndPos :: Position, + pcComment :: Comment +} deriving (Show, Eq) + +newPositionedComment :: PositionedComment +newPositionedComment = PositionedComment { + pcStartPos = newPosition, + pcEndPos = newPosition, + pcComment = newComment +} + +data TokenComment = TokenComment { + tcId :: Id, + tcComment :: Comment +} deriving (Show, Eq) + +newTokenComment = TokenComment { + tcId = Id 0, + tcComment = newComment +} data ColorOption = ColorAuto diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs --- a/src/ShellCheck/Parser.hs +++ b/src/ShellCheck/Parser.hs @@ -66,7 +66,7 @@ variableStart = upper <|> lower <|> oneOf "_" variableChars = upper <|> lower <|> digit <|> oneOf "_" -- Chars to allow in function names -functionChars = variableChars <|> oneOf ":+?-./^" +functionChars = variableChars <|> oneOf ":+?-./^@" -- Chars to allow in functions using the 'function' keyword extendedFunctionChars = functionChars <|> oneOf "[]*=!" specialVariable = oneOf "@*#?-$!" @@ -115,10 +115,10 @@ when (null s) $ fail "Expected whitespace" readUnicodeQuote = do - pos <- getPosition + start <- startSpan c <- oneOf (unicodeSingleQuotes ++ unicodeDoubleQuotes) - parseProblemAt pos WarningC 1110 "This is a unicode quote. Delete and retype it (or quote to make literal)." - id <- getNextIdAt pos + id <- endSpan start + parseProblemAtId id WarningC 1110 "This is a unicode quote. Delete and retype it (or quote to make literal)." return $ T_Literal id [c] carriageReturn = do @@ -151,7 +151,7 @@ data UserState = UserState { lastId :: Id, - positionMap :: Map.Map Id SourcePos, + positionMap :: Map.Map Id (SourcePos, SourcePos), parseNotes :: [ParseNote], hereDocMap :: Map.Map Id [Token], pendingHereDocs :: [HereDocContext] @@ -172,10 +172,10 @@ getLastId = lastId <$> getState -getNextIdAt sourcepos = do +getNextIdBetween startPos endPos = do state <- getState let newId = incId (lastId state) - let newMap = Map.insert newId sourcepos (positionMap state) + let newMap = Map.insert newId (startPos, endPos) (positionMap state) putState $ state { lastId = newId, positionMap = newMap @@ -183,11 +183,39 @@ return newId where incId (Id n) = Id $ n+1 -getNextId :: Monad m => SCParser m Id -getNextId = do - pos <- getPosition - getNextIdAt pos +getNextIdSpanningTokens startTok endTok = do + (start, _) <- getSpanForId (getId startTok) + (_, end) <- getSpanForId (getId endTok) + getNextIdBetween start end +-- Get an ID starting from the first token of the list, and ending after the last +getNextIdSpanningTokenList list = + if null list + then do + pos <- getPosition + getNextIdBetween pos pos + else + getNextIdSpanningTokens (head list) (last list) + +-- Get the span covered by an id +getSpanForId :: Monad m => Id -> SCParser m (SourcePos, SourcePos) +getSpanForId id = + Map.findWithDefault (error "Internal error: no position for id. Please report!") id <$> + getMap + +-- Create a new id with the same span as an existing one +getNewIdFor :: Monad m => Id -> SCParser m Id +getNewIdFor id = getSpanForId id >>= uncurry getNextIdBetween + +data IncompleteInterval = IncompleteInterval SourcePos + +startSpan = IncompleteInterval <$> getPosition + +endSpan (IncompleteInterval start) = do + endPos <- getPosition + id <- getNextIdBetween start endPos + return id + addToHereDocMap id list = do state <- getState let map = hereDocMap state @@ -277,7 +305,8 @@ data Environment m = Environment { systemInterface :: SystemInterface m, - checkSourced :: Bool + checkSourced :: Bool, + shellTypeOverride :: Maybe Shell } parseProblem level code msg = do @@ -317,10 +346,8 @@ parseProblemAtId :: Monad m => Id -> Severity -> Integer -> String -> SCParser m () parseProblemAtId id level code msg = do - map <- getMap - let pos = Map.findWithDefault - (error "Internal error (no position for id). Please report.") id map - parseProblemAt pos level code msg + (start, end) <- getSpanForId id + parseProblemAtWithEnd start end level code msg -- Store non-parse problems inside @@ -329,6 +356,9 @@ parseNoteAt pos c l a parseNoteAt pos c l a = addParseNote $ ParseNote pos pos c l a +parseNoteAtId id c l a = do + (start, end) <- getSpanForId id + addParseNote $ ParseNote start end c l a parseNoteAtWithEnd start end c l a = addParseNote $ ParseNote start end c l a @@ -364,8 +394,9 @@ -- Construct a node with a parser, e.g. T_Literal `withParser` (readGenericLiteral ",") withParser node parser = do - id <- getNextId + start <- startSpan contents <- parser + id <- endSpan start return $ node id contents wasIncluded p = option False (p >> return True) @@ -416,7 +447,6 @@ typ = if single then SingleBracket else DoubleBracket readCondBinaryOp = try $ do optional guardArithmetic - id <- getNextId op <- getOp spacingOrLf return op @@ -424,8 +454,9 @@ flaglessOps = [ "==", "!=", "<=", ">=", "=~", ">", "<", "=" ] getOp = do - id <- getNextId + start <- startSpan op <- readRegularOrEscaped anyOp + id <- endSpan start return $ TC_Binary id typ op anyOp = flagOp <|> flaglessOp <|> fail @@ -468,8 +499,9 @@ return "Expected an argument for the unary operator" readCondUnaryOp = try $ do - id <- getNextId + start <- startSpan s <- readOp + id <- endSpan start spacingOrLf return $ TC_Unary id typ s @@ -489,7 +521,7 @@ notFollowedBy2 (try (spacing >> string "]")) x <- readNormalWord pos <- getPosition - when (endedWith "]" x) $ do + when (endedWith "]" x && notArrayIndex x) $ do parseProblemAt pos ErrorC 1020 $ "You need a space before the " ++ (if single then "]" else "]]") ++ "." fail "Missing space before ]" @@ -503,26 +535,25 @@ case last s of T_Literal id s -> str `isSuffixOf` s _ -> False endedWith _ _ = False + notArrayIndex (T_NormalWord id s@(_:T_Literal _ t:_)) = t /= "[" + notArrayIndex _ = True - readCondAndOp = do - id <- getNextId - x <- try (readAndOrOp "&&" False <|> readAndOrOp "-a" True) - return $ TC_And id typ x + readCondAndOp = readAndOrOp TC_And "&&" False <|> readAndOrOp TC_And "-a" True readCondOrOp = do optional guardArithmetic - id <- getNextId - x <- try (readAndOrOp "||" False <|> readAndOrOp "-o" True) - return $ TC_Or id typ x + readAndOrOp TC_Or "||" False <|> readAndOrOp TC_Or "-o" True - readAndOrOp op requiresSpacing = do + readAndOrOp node op requiresSpacing = do optional $ lookAhead weirdDash - x <- string op + start <- startSpan + x <- try $ string op + id <- endSpan start condSpacing requiresSpacing - return x + return $ node id typ x readCondNullaryOrBinary = do - id <- getNextId + start <- startSpan x <- readCondWord `attempting` (do pos <- getPosition lookAhead (char '[') @@ -530,6 +561,7 @@ then "If grouping expressions inside [..], use \\( ..\\)." else "If grouping expressions inside [[..]], use ( .. )." ) + id <- endSpan start (do pos <- getPosition isRegex <- regexOperatorAhead @@ -550,7 +582,7 @@ "You need a space before and after the " ++ trailingOp ++ " ." readCondGroup = do - id <- getNextId + start <- startSpan pos <- getPosition lparen <- try $ readRegularOrEscaped (string "(") when (single && lparen == "(") $ @@ -561,6 +593,7 @@ x <- readCondContents cpos <- getPosition rparen <- readRegularOrEscaped (string ")") + id <- endSpan start condSpacing single when (single && rparen == ")") $ singleWarning cpos @@ -581,8 +614,9 @@ return True) <|> return False readRegex = called "regex" $ do - id <- getNextId + start <- startSpan parts <- many1 readPart + id <- endSpan start void spacing return $ T_NormalWord id parts where @@ -591,27 +625,31 @@ readSingleQuoted, readDoubleQuoted, readDollarExpression, - readNormalLiteral "( ", - readPipeLiteral, + readLiteralForParser $ readNormalLiteral "( ", + readLiteralString "|", readGlobLiteral ] readGlobLiteral = do - id <- getNextId + start <- startSpan s <- extglobStart <|> oneOf "{}[]$" + id <- endSpan start return $ T_Literal id [s] readGroup = called "regex grouping" $ do - id <- getNextId - char '(' + start <- startSpan + p1 <- readLiteralString "(" parts <- many (readPart <|> readRegexLiteral) - char ')' - return $ T_NormalWord id parts + p2 <- readLiteralString ")" + id <- endSpan start + return $ T_NormalWord id (p1:(parts ++ [p2])) readRegexLiteral = do - id <- getNextId + start <- startSpan str <- readGenericLiteral1 (singleQuote <|> doubleQuotable <|> oneOf "()") + id <- endSpan start return $ T_Literal id str - readPipeLiteral = do - id <- getNextId - str <- string "|" + readLiteralString s = do + start <- startSpan + str <- string s + id <- endSpan start return $ T_Literal id str readCondTerm = do @@ -620,8 +658,9 @@ return term readCondNot = do - id <- getNextId + start <- startSpan char '!' + id <- endSpan start spacingOrLf expr <- readCondExpr return $ TC_Unary id typ "!" expr @@ -656,6 +695,7 @@ prop_a20= isOk readArithmeticContents "a ? b ? c : d : e" prop_a21= isOk readArithmeticContents "a ? b : c ? d : e" prop_a22= isOk readArithmeticContents "!!a" +prop_a23= isOk readArithmeticContents "~0" readArithmeticContents :: Monad m => SCParser m Token readArithmeticContents = readSequence @@ -667,12 +707,13 @@ splitBy x ops = chainl1 x (readBinary ops) readBinary ops = readComboOp ops TA_Binary readComboOp op token = do - id <- getNextId + start <- startSpan op <- choice (map (\x -> try $ do s <- string x failIfIncompleteOp return s ) op) + id <- endSpan start spacing return $ token id op @@ -680,7 +721,7 @@ -- Read binary minus, but also check for -lt, -gt and friends: readMinusOp = do - id <- getNextId + start <- startSpan pos <- getPosition try $ do char '-' @@ -695,6 +736,7 @@ ("ne", "!=") ] parseProblemAt pos ErrorC 1106 $ "In arithmetic contexts, use " ++ alt ++ " instead of -" ++ str + id <- endSpan start spacing return $ TA_Binary id "-" where @@ -704,27 +746,30 @@ return (str, alt) readArrayIndex = do - id <- getNextId + start <- startSpan char '[' pos <- getPosition middle <- readStringForParser readArithmeticContents char ']' + id <- endSpan start return $ T_UnparsedIndex id pos middle literal s = do - id <- getNextId + start <- startSpan string s + id <- endSpan start return $ T_Literal id s readVariable = do - id <- getNextId + start <- startSpan name <- readVariableName indices <- many readArrayIndex + id <- endSpan start spacing return $ TA_Variable id name indices readExpansion = do - id <- getNextId + start <- startSpan pieces <- many1 $ choice [ readSingleQuoted, readDoubleQuoted, @@ -734,6 +779,7 @@ literal "#", readNormalLiteral "+-*/=%^,]?:" ] + id <- endSpan start spacing return $ TA_Expansion id pieces @@ -748,8 +794,9 @@ readSequence = do spacing - id <- getNextId + start <- startSpan l <- readAssignment `sepBy` (char ',' >> spacing) + id <- endSpan start return $ TA_Sequence id l readAssignment = chainr1 readTrinary readAssignmentOp @@ -758,13 +805,14 @@ readTrinary = do x <- readLogicalOr do - id <- getNextId + start <- startSpan string "?" spacing y <- readTrinary string ":" spacing z <- readTrinary + id <- endSpan start return $ TA_Trinary id x y z <|> return x @@ -783,16 +831,18 @@ readAnyNegated = readNegated <|> readAnySigned readNegated = do - id <- getNextId + start <- startSpan op <- oneOf "!~" + id <- endSpan start spacing x <- readAnyNegated return $ TA_Unary id [op] x readAnySigned = readSigned <|> readAnycremented readSigned = do - id <- getNextId + start <- startSpan op <- choice (map readSignOp "+-") + id <- endSpan start spacing x <- readAnycremented return $ TA_Unary id [op] x @@ -805,8 +855,9 @@ readAnycremented = readNormalOrPostfixIncremented <|> readPrefixIncremented readPrefixIncremented = do - id <- getNextId + start <- startSpan op <- try $ string "++" <|> string "--" + id <- endSpan start spacing x <- readArithTerm return $ TA_Unary id (op ++ "|") x @@ -815,8 +866,9 @@ x <- readArithTerm spacing do - id <- getNextId + start <- startSpan op <- try $ string "++" <|> string "--" + id <- endSpan start spacing return $ TA_Unary id ('|':op) x <|> @@ -824,35 +876,36 @@ -prop_readCondition = isOk readCondition "[ \\( a = b \\) -a \\( c = d \\) ]" -prop_readCondition2 = isOk readCondition "[[ (a = b) || (c = d) ]]" -prop_readCondition3 = isOk readCondition "[[ $c = [[:alpha:].~-] ]]" -prop_readCondition4 = isOk readCondition "[[ $c =~ *foo* ]]" -prop_readCondition5 = isOk readCondition "[[ $c =~ f( ]] )* ]]" -prop_readCondition5a= isOk readCondition "[[ $c =~ a(b) ]]" -prop_readCondition5b= isOk readCondition "[[ $c =~ f( ($var ]]) )* ]]" -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 "[[\na == b\n||\nc == d ]]" +prop_readCondition = isOk readCondition "[ \\( a = b \\) -a \\( c = d \\) ]" +prop_readCondition2 = isOk readCondition "[[ (a = b) || (c = d) ]]" +prop_readCondition3 = isOk readCondition "[[ $c = [[:alpha:].~-] ]]" +prop_readCondition4 = isOk readCondition "[[ $c =~ *foo* ]]" +prop_readCondition5 = isOk readCondition "[[ $c =~ f( ]] )* ]]" +prop_readCondition5a = isOk readCondition "[[ $c =~ a(b) ]]" +prop_readCondition5b = isOk readCondition "[[ $c =~ f( ($var ]]) )* ]]" +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 "[[\na == b\n||\nc == 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}$ ]]" -prop_readCondition14= isOk readCondition "[ foo '>' bar ]" -prop_readCondition15= isOk readCondition "[ foo \">=\" bar ]" -prop_readCondition16= isOk readCondition "[ foo \\< bar ]" -prop_readCondition17= isOk readCondition "[[ ${file::1} = [-.\\|/\\\\] ]]" -prop_readCondition18= isOk readCondition "[ ]" -prop_readCondition19= isOk readCondition "[ '(' x \")\" ]" -prop_readCondition20= isOk readCondition "[[ echo_rc -eq 0 ]]" -prop_readCondition21= isOk readCondition "[[ $1 =~ ^(a\\ b)$ ]]" -prop_readCondition22= isOk readCondition "[[ $1 =~ \\.a\\.(\\.b\\.)\\.c\\. ]]" +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}$ ]]" +prop_readCondition14 = isOk readCondition "[ foo '>' bar ]" +prop_readCondition15 = isOk readCondition "[ foo \">=\" bar ]" +prop_readCondition16 = isOk readCondition "[ foo \\< bar ]" +prop_readCondition17 = isOk readCondition "[[ ${file::1} = [-.\\|/\\\\] ]]" +prop_readCondition18 = isOk readCondition "[ ]" +prop_readCondition19 = isOk readCondition "[ '(' x \")\" ]" +prop_readCondition20 = isOk readCondition "[[ echo_rc -eq 0 ]]" +prop_readCondition21 = isOk readCondition "[[ $1 =~ ^(a\\ b)$ ]]" +prop_readCondition22 = isOk readCondition "[[ $1 =~ \\.a\\.(\\.b\\.)\\.c\\. ]]" +prop_readCondition23 = isOk readCondition "[[ -v arr[$var] ]]" readCondition = called "test expression" $ do opos <- getPosition - id <- getNextId + start <- startSpan open <- try (string "[[") <|> string "[" let single = open == "[" let typ = if single then SingleBracket else DoubleBracket @@ -870,11 +923,12 @@ condition <- readConditionContents single <|> do guard . not . null $ space lookAhead $ string "]" - id <- getNextIdAt pos + id <- endSpan start return $ TC_Empty id typ cpos <- getPosition close <- try (string "]]") <|> string "]" <|> fail "Expected test to end here (don't wrap commands in []/[[]])" + id <- endSpan start when (open == "[[" && close /= "]]") $ parseProblemAt cpos ErrorC 1033 "Did you mean ]] ?" when (open == "[" && close /= "]" ) $ parseProblemAt opos ErrorC 1034 "Did you mean [[ ?" spacing @@ -963,24 +1017,28 @@ readNormalWord = readNormalishWord "" readNormalishWord end = do - id <- getNextId + start <- startSpan pos <- getPosition x <- many1 (readNormalWordPart end) + id <- endSpan start checkPossibleTermination pos x return $ T_NormalWord id x readIndexSpan = do - id <- getNextId + start <- startSpan x <- many (readNormalWordPart "]" <|> someSpace <|> otherLiteral) + id <- endSpan start return $ T_NormalWord id x where someSpace = do - id <- getNextId + start <- startSpan str <- spacing1 + id <- endSpan start return $ T_Literal id str otherLiteral = do - id <- getNextId + start <- startSpan str <- many1 $ oneOf quotableChars + id <- endSpan start return $ T_Literal id str checkPossibleTermination pos [T_Literal _ x] = @@ -1011,8 +1069,9 @@ parseProblemAt pos ErrorC 1036 "'(' is invalid here. Did you forget to escape it?" readLiteralCurlyBraces = do - id <- getNextId + start <- startSpan str <- findParam <|> literalBraces + id <- endSpan start return $ T_Literal id str findParam = try $ string "{}" @@ -1025,13 +1084,15 @@ readSpacePart = do - id <- getNextId + start <- startSpan x <- many1 whitespace + id <- endSpan start return $ T_Literal id x readDollarBracedWord = do - id <- getNextId + start <- startSpan list <- many readDollarBracedPart + id <- endSpan start return $ T_NormalWord id list readDollarBracedPart = readSingleQuoted <|> readDoubleQuoted <|> @@ -1039,19 +1100,22 @@ readUnquotedBackTicked <|> readDollarBracedLiteral readDollarBracedLiteral = do - id <- getNextId + start <- startSpan vars <- (readBraceEscaped <|> (anyChar >>= \x -> return [x])) `reluctantlyTill1` bracedQuotable + id <- endSpan start return $ T_Literal id $ concat vars readParamSubSpecialChar = do - id <- getNextId - T_ParamSubSpecialChar id <$> many1 paramSubSpecialChars + start <- startSpan + x <- many1 paramSubSpecialChars + id <- endSpan start + return $ T_ParamSubSpecialChar id x prop_readProcSub1 = isOk readProcSub "<(echo test | wc -l)" prop_readProcSub2 = isOk readProcSub "<( if true; then true; fi )" prop_readProcSub3 = isOk readProcSub "<( # nothing here \n)" readProcSub = called "process substitution" $ do - id <- getNextId + start <- startSpan dir <- try $ do x <- oneOf "<>" char '(' @@ -1059,6 +1123,7 @@ list <- readCompoundListOrEmpty allspacing char ')' + id <- endSpan start return $ T_ProcSub id dir list prop_readSingleQuoted = isOk readSingleQuoted "'foo bar'" @@ -1069,7 +1134,7 @@ prop_readSingleQuoted7 = isOk readSingleQuoted "'foo\x201C\&bar'" prop_readSingleQuoted8 = isWarning readSingleQuoted "'foo\x2018\&bar'" readSingleQuoted = called "single quoted string" $ do - id <- getNextId + start <- startSpan startPos <- getPosition singleQuote s <- many readSingleQuotedPart @@ -1087,6 +1152,7 @@ when ('\n' `elem` string && not ("\n" `isPrefixOf` string)) $ suggestForgotClosingQuote startPos endPos "single quoted string" + id <- endSpan start return (T_SingleQuoted id string) readSingleQuotedLiteral = do @@ -1119,13 +1185,14 @@ readQuotedBackTicked = readBackTicked True readUnquotedBackTicked = readBackTicked False readBackTicked quoted = called "backtick expansion" $ do - id <- getNextId + start <- startSpan startPos <- getPosition backtick subStart <- getPosition subString <- readGenericLiteral "`ยด" endPos <- getPosition backtick + id <- endSpan start optional $ do c <- try . lookAhead $ suspectCharAfterQuotes @@ -1188,15 +1255,15 @@ prop_readDoubleQuoted6 = isOk readSimpleCommand "echo \"${ ls; }\"" prop_readDoubleQuoted7 = isOk readSimpleCommand "echo \"${ ls;}bar\"" prop_readDoubleQuoted8 = isWarning readDoubleQuoted "\"\x201Chello\x201D\"" -prop_readDoubleQuoted9 = isWarning readDoubleQuoted "\"foo\\n\"" prop_readDoubleQuoted10 = isOk readDoubleQuoted "\"foo\\\\n\"" readDoubleQuoted = called "double quoted string" $ do - id <- getNextId + start <- startSpan startPos <- getPosition doubleQuote x <- many doubleQuotedPart endPos <- getPosition doubleQuote <|> fail "Expected end of double quoted string" + id <- endSpan start optional $ do try . lookAhead $ suspectCharAfterQuotes <|> oneOf "$\"" when (any hasLineFeed x && not (startsWithLineFeed x)) $ @@ -1218,15 +1285,17 @@ where readUnicodeQuote = do pos <- getPosition - id <- getNextId + start <- startSpan c <- oneOf unicodeDoubleQuotes + id <- endSpan start parseProblemAt pos WarningC 1111 "This is a unicode quote. Delete and retype it (or ignore/singlequote for literal)." return $ T_Literal id [c] readDoubleLiteral = do - id <- getNextId + start <- startSpan s <- many1 readDoubleLiteralPart + id <- endSpan start return $ T_Literal id (concat s) readDoubleLiteralPart = do @@ -1234,8 +1303,9 @@ return $ concat x readNormalLiteral end = do - id <- getNextId + start <- startSpan s <- many1 (readNormalLiteralPart end) + id <- endSpan start return $ T_Literal id (concat s) prop_readGlob1 = isOk readGlob "*" @@ -1249,18 +1319,20 @@ readGlob = readExtglob <|> readSimple <|> readClass <|> readGlobbyLiteral where readSimple = do - id <- getNextId + start <- startSpan c <- oneOf "*?" + id <- endSpan start return $ T_Glob id [c] -- Doesn't handle weird things like [^]a] and [$foo]. fixme? readClass = try $ do - id <- getNextId + start <- startSpan char '[' s <- many1 (predefined <|> readNormalLiteralPart "]" <|> globchars) char ']' + id <- endSpan start return $ T_Glob id $ "[" ++ concat s ++ "]" where - globchars = liftM return . oneOf $ "!$[" ++ extglobStartChars + globchars = fmap return . oneOf $ "!$[" ++ extglobStartChars predefined = do try $ string "[:" s <- many1 letter @@ -1268,8 +1340,9 @@ return $ "[:" ++ s ++ ":]" readGlobbyLiteral = do - id <- getNextId + start <- startSpan c <- extglobStart <|> char '[' + id <- endSpan start return $ T_Literal id [c] readNormalLiteralPart customEnd = @@ -1298,7 +1371,7 @@ return [next] where alternative 'n' = "a quoted, literal line feed" - alternative t = "\"$(printf \"\\" ++ [t] ++ "\")\"" + alternative t = "\"$(printf '\\" ++ [t] ++ "')\"" escapedChar 'n' = Just "line feed" escapedChar 't' = Just "tab" escapedChar 'r' = Just "carriage return" @@ -1318,29 +1391,33 @@ prop_readExtglob7 = isOk readExtglob "*(<>)" prop_readExtglob8 = isOk readExtglob "@(|*())" readExtglob = called "extglob" $ do - id <- getNextId + start <- startSpan c <- try $ do f <- extglobStart char '(' return f contents <- readExtglobPart `sepBy` char '|' char ')' + id <- endSpan start return $ T_Extglob id [c] contents readExtglobPart = do - id <- getNextId + start <- startSpan x <- many (readExtglobGroup <|> readNormalWordPart "" <|> readSpacePart <|> readExtglobLiteral) + id <- endSpan start return $ T_NormalWord id x where readExtglobGroup = do - id <- getNextId char '(' + start <- startSpan contents <- readExtglobPart `sepBy` char '|' + id <- endSpan start char ')' return $ T_Extglob id "" contents readExtglobLiteral = do - id <- getNextId + start <- startSpan str <- many1 (oneOf "<>#;&") + id <- endSpan start return $ T_Literal id str @@ -1360,18 +1437,19 @@ pos <- getPosition bs <- backslash (linefeed >> return "") - <|> liftM return doubleQuotable + <|> fmap return doubleQuotable <|> do c <- anyChar - parseNoteAt pos StyleC 1117 $ - "Backslash is literal in \"\\" ++ [c] ++ "\". Prefer explicit escaping: \"\\\\" ++ [c] ++ "\"." + -- This is an invalid escape sequence where the \ is literal. + -- Previously this caused a SC1117, which may be re-enabled as + -- as a pedantic warning. return [bs, c] readBraceEscaped = do bs <- backslash (linefeed >> return "") - <|> liftM return bracedQuotable - <|> liftM (\ x -> [bs, x]) anyChar + <|> fmap return bracedQuotable + <|> fmap (\ x -> [bs, x]) anyChar readGenericLiteral endChars = do @@ -1449,46 +1527,51 @@ prop_readDollarSingleQuote = isOk readDollarSingleQuote "$'foo\\\'lol'" readDollarSingleQuote = called "$'..' expression" $ do - id <- getNextId + start <- startSpan try $ string "$'" str <- readGenericLiteral "'" char '\'' + id <- endSpan start return $ T_DollarSingleQuoted id str prop_readDollarDoubleQuote = isOk readDollarDoubleQuote "$\"hello\"" readDollarDoubleQuote = do lookAhead . try $ string "$\"" - id <- getNextId + start <- startSpan char '$' doubleQuote x <- many doubleQuotedPart doubleQuote <|> fail "Expected end of translated double quoted string" + id <- endSpan start return $ T_DollarDoubleQuoted id x prop_readDollarArithmetic = isOk readDollarArithmetic "$(( 3 * 4 +5))" prop_readDollarArithmetic2 = isOk readDollarArithmetic "$(((3*4)+(1*2+(3-1))))" readDollarArithmetic = called "$((..)) expression" $ do - id <- getNextId + start <- startSpan try (string "$((") c <- readArithmeticContents pos <- getPosition char ')' char ')' <|> fail "Expected a double )) to end the $((..))" + id <- endSpan start return (T_DollarArithmetic id c) readDollarBracket = called "$[..] expression" $ do - id <- getNextId + start <- startSpan try (string "$[") c <- readArithmeticContents string "]" + id <- endSpan start return (T_DollarBracket id c) prop_readArithmeticExpression = isOk readArithmeticExpression "((a?b:c))" readArithmeticExpression = called "((..)) command" $ do - id <- getNextId + start <- startSpan try (string "((") c <- readArithmeticContents string "))" + id <- endSpan start return (T_Arithmetic id c) -- If the next characters match prefix, try two different parsers and warn if the alternate parser had to be used @@ -1508,13 +1591,14 @@ prop_readDollarBraceCommandExpansion1 = isOk readDollarBraceCommandExpansion "${ ls; }" prop_readDollarBraceCommandExpansion2 = isOk readDollarBraceCommandExpansion "${\nls\n}" readDollarBraceCommandExpansion = called "ksh ${ ..; } command expansion" $ do - id <- getNextId + start <- startSpan try $ do string "${" whitespace allspacing term <- readTerm char '}' <|> fail "Expected } to end the ksh ${ ..; } command expansion" + id <- endSpan start return $ T_DollarBraceCommandExpansion id term prop_readDollarBraced1 = isOk readDollarBraced "${foo//bar/baz}" @@ -1522,20 +1606,22 @@ prop_readDollarBraced3 = isOk readDollarBraced "${foo%%$(echo cow\\})}" prop_readDollarBraced4 = isOk readDollarBraced "${foo#\\}}" readDollarBraced = called "parameter expansion" $ do - id <- getNextId + start <- startSpan try (string "${") word <- readDollarBracedWord char '}' + id <- endSpan start return $ T_DollarBraced id word prop_readDollarExpansion1= isOk readDollarExpansion "$(echo foo; ls\n)" prop_readDollarExpansion2= isOk readDollarExpansion "$( )" prop_readDollarExpansion3= isOk readDollarExpansion "$( command \n#comment \n)" readDollarExpansion = called "command expansion" $ do - id <- getNextId + start <- startSpan try (string "$(") cmds <- readCompoundListOrEmpty char ')' <|> fail "Expected end of $(..) expression" + id <- endSpan start return $ T_DollarExpansion id cmds prop_readDollarVariable = isOk readDollarVariable "$@" @@ -1544,14 +1630,15 @@ prop_readDollarVariable4 = isWarning (readDollarVariable >> string "[@]") "$arr[@]" prop_readDollarVariable5 = isWarning (readDollarVariable >> string "[f") "$arr[f" +readDollarVariable :: Monad m => SCParser m Token readDollarVariable = do - id <- getNextId + start <- startSpan pos <- getPosition let singleCharred p = do - n <- p - value <- wrap [n] - return (T_DollarBraced id value) + value <- wrapString ((:[]) <$> p) + id <- endSpan start + return $ (T_DollarBraced id value) let positional = do value <- singleCharred digit @@ -1562,8 +1649,8 @@ let special = singleCharred specialVariable let regular = do - name <- readVariableName - value <- wrap name + value <- wrapString readVariableName + id <- endSpan start return (T_DollarBraced id value) `attempting` do lookAhead $ char '[' parseNoteAt pos ErrorC 1087 "Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet)." @@ -1571,10 +1658,13 @@ try $ char '$' >> (positional <|> special <|> regular) where - wrap s = do - x <- getNextId - y <- getNextId - return $ T_NormalWord x [T_Literal y s] + wrapString p = do + start <- getPosition + s <- p + end <- getPosition + id1 <- getNextIdBetween start end + id2 <- getNextIdBetween start end + return $ T_NormalWord id1 [T_Literal id2 s] readVariableName = do f <- variableStart @@ -1582,9 +1672,9 @@ return (f:rest) readDollarLonely = do - id <- getNextId - pos <- getPosition + start <- startSpan char '$' + id <- endSpan start n <- lookAhead (anyChar <|> (eof >> return '_')) return $ T_Literal id "$" @@ -1605,12 +1695,9 @@ prop_readHereDoc15= isWarning readScript "cat <> return Dashed) <|> return Undashed @@ -1619,8 +1706,9 @@ try . lookAhead $ char '(' let message = "Shells are space sensitive. Use '< <(cmd)', not '<<" ++ sp ++ "(cmd)'." parseProblemAt pos ErrorC 1038 message - hid <- getNextId + start <- startSpan (quoted, endToken) <- readToken + hid <- endSpan start -- add empty tokens for now, read the rest in readPendingHereDocs let doc = T_HereDoc hid dashed quoted endToken [] @@ -1643,15 +1731,15 @@ readDoc (HereDocPending (T_HereDoc id dashed quoted endToken _) ctx) = swapContext ctx $ do - docPos <- getPosition - tokenPos <- Map.findWithDefault (error "Missing ID") id <$> getMap + docStartPos <- getPosition (terminated, wasWarned, lines) <- readDocLines dashed endToken + docEndPos <- getPosition let hereData = unlines lines unless terminated $ do unless wasWarned $ - debugHereDoc tokenPos endToken hereData + debugHereDoc id endToken hereData fail "Here document was not correctly terminated" - list <- parseHereData quoted docPos hereData + list <- parseHereData quoted (docStartPos, docEndPos) hereData addToHereDocMap id list -- Read the lines making up the here doc. Returns (IsTerminated, Lines) @@ -1731,40 +1819,42 @@ void (char '\n') <|> eof return c - parseHereData Quoted startPos hereData = do - id <- getNextIdAt startPos + parseHereData Quoted (start,end) hereData = do + id <- getNextIdBetween start end return [T_Literal id hereData] - parseHereData Unquoted startPos hereData = + parseHereData Unquoted (startPos, _) hereData = subParse startPos readHereData hereData readHereData = many $ doubleQuotedPart <|> readHereLiteral readHereLiteral = do - id <- getNextId + start <- startSpan chars <- many1 $ noneOf "`$\\" + id <- endSpan start return $ T_Literal id chars - debugHereDoc pos endToken doc + debugHereDoc tokenId endToken doc | endToken `isInfixOf` doc = let lookAt line = when (endToken `isInfixOf` line) $ - parseProblemAt pos ErrorC 1042 ("Close matches include '" ++ line ++ "' (!= '" ++ endToken ++ "').") + parseProblemAtId tokenId ErrorC 1042 ("Close matches include '" ++ line ++ "' (!= '" ++ endToken ++ "').") in do - parseProblemAt pos ErrorC 1041 ("Found '" ++ endToken ++ "' further down, but not on a separate line.") + parseProblemAtId tokenId ErrorC 1041 ("Found '" ++ endToken ++ "' further down, but not on a separate line.") mapM_ lookAt (lines doc) | map toLower endToken `isInfixOf` map toLower doc = - parseProblemAt pos ErrorC 1043 ("Found " ++ endToken ++ " further down, but with wrong casing.") + parseProblemAtId tokenId ErrorC 1043 ("Found " ++ endToken ++ " further down, but with wrong casing.") | otherwise = - parseProblemAt pos ErrorC 1044 ("Couldn't find end token `" ++ endToken ++ "' in the here document.") + parseProblemAtId tokenId ErrorC 1044 ("Couldn't find end token `" ++ endToken ++ "' in the here document.") readFilename = readNormalWord readIoFileOp = choice [g_DGREAT, g_LESSGREAT, g_GREATAND, g_LESSAND, g_CLOBBER, redirToken '<' T_Less, redirToken '>' T_Greater ] readIoDuplicate = try $ do - id <- getNextId + start <- startSpan op <- g_GREATAND <|> g_LESSAND target <- readIoVariable <|> digitsAndOrDash + id <- endSpan start return $ T_IoDuplicate id op target where -- either digits with optional dash, or a required dash @@ -1776,10 +1866,11 @@ prop_readIoFile = isOk readIoFile ">> \"$(date +%YYmmDD)\"" readIoFile = called "redirection" $ do - id <- getNextId + start <- startSpan op <- readIoFileOp spacing file <- readFilename + id <- endSpan start return $ T_IoFile id op file readIoVariable = try $ do @@ -1801,9 +1892,10 @@ prop_readIoRedirect6 = isOk readIoRedirect "{foo}<&-" prop_readIoRedirect7 = isOk readIoRedirect "{foo}>&1-" readIoRedirect = do - id <- getNextId + start <- startSpan n <- readIoSource redir <- readHereString <|> readHereDoc <|> readIoDuplicate <|> readIoFile + id <- endSpan start skipAnnotationAndWarn spacing return $ T_FdRedirect id n redir @@ -1812,31 +1904,46 @@ prop_readHereString = isOk readHereString "<<< \"Hello $world\"" readHereString = called "here string" $ do - id <- getNextId + start <- startSpan try $ string "<<<" + id <- endSpan start spacing - id2 <- getNextId word <- readNormalWord - return $ T_HereString id2 word + return $ T_HereString id word -readNewlineList = many1 ((linefeed <|> carriageReturn) `thenSkip` spacing) +readNewlineList = + many1 ((linefeed <|> carriageReturn) `thenSkip` spacing) <* checkBadBreak + where + checkBadBreak = optional $ do + pos <- getPosition + try $ lookAhead (oneOf "|&") -- See if the next thing could be |, || or && + parseProblemAt pos ErrorC 1133 + "Unexpected start of line. If breaking lines, |/||/&& should be at the end of the previous one." readLineBreak = optional readNewlineList prop_readSeparator1 = isWarning readScript "a &; b" prop_readSeparator2 = isOk readScript "a & b" prop_readSeparator3 = isWarning readScript "a & b" prop_readSeparator4 = isWarning readScript "a > file; b" +prop_readSeparator5 = isWarning readScript "curl https://example.com/?foo=moo&bar=cow" readSeparatorOp = do notFollowedBy2 (void g_AND_IF <|> void readCaseSeparator) notFollowedBy2 (string "&>") + start <- getPosition f <- try (do pos <- getPosition char '&' - optional $ do - s <- lookAhead . choice . map (try . string) $ - ["amp;", "gt;", "lt;"] - parseProblemAt pos ErrorC 1109 "This is an unquoted HTML entity. Replace with corresponding character." + optional $ choice [ + do + s <- lookAhead . choice . map (try . string) $ + ["amp;", "gt;", "lt;"] + parseProblemAt pos ErrorC 1109 "This is an unquoted HTML entity. Replace with corresponding character.", + do + try . lookAhead $ variableStart + parseProblemAt pos WarningC 1132 "This & terminates the command. Escape it or add space after & to silence." + ] + spacing pos <- getPosition char ';' @@ -1845,8 +1952,9 @@ parseProblemAt pos ErrorC 1045 "It's not 'foo &; bar', just 'foo & bar'." return '&' ) <|> char ';' <|> char '&' + end <- getPosition spacing - return f + return (f, (start, end)) readSequentialSep = void (g_Semi >> readLineBreak) <|> void readNewlineList readSeparator = @@ -1856,25 +1964,10 @@ return separator <|> do + start <- getPosition readNewlineList - return '\n' - -makeSimpleCommand id1 id2 prefix cmd suffix = - let - (preAssigned, preRest) = partition assignment prefix - (preRedirected, preRest2) = partition redirection preRest - (postRedirected, postRest) = partition redirection suffix - - redirs = preRedirected ++ postRedirected - assigns = preAssigned - args = cmd ++ preRest2 ++ postRest - in - T_Redirecting id1 redirs $ T_SimpleCommand id2 assigns args - where - assignment (T_Assignment {}) = True - assignment _ = False - redirection (T_FdRedirect {}) = True - redirection _ = False + end <- getPosition + return ('\n', (start, end)) prop_readSimpleCommand = isOk readSimpleCommand "echo test > file" prop_readSimpleCommand2 = isOk readSimpleCommand "cmd &> file" @@ -1891,18 +1984,19 @@ prop_readSimpleCommand13 = isWarning readSimpleCommand "ElseIf foo" prop_readSimpleCommand14 = isWarning readSimpleCommand "elseif[$i==2]" readSimpleCommand = called "simple command" $ do - id1 <- getNextId - id2 <- getNextId prefix <- option [] readCmdPrefix skipAnnotationAndWarn - pos <- getPosition cmd <- option Nothing $ Just <$> readCmdName when (null prefix && isNothing cmd) $ fail "Expected a command" case cmd of - Nothing -> return $ makeSimpleCommand id1 id2 prefix [] [] + Nothing -> do + id1 <- getNextIdSpanningTokenList prefix + id2 <- getNewIdFor id1 + return $ makeSimpleCommand id1 id2 prefix [] [] + Just cmd -> do - validateCommand pos cmd + validateCommand cmd suffix <- option [] $ getParser readCmdSuffix cmd [ (["declare", "export", "local", "readonly", "typeset"], readModifierSuffix), (["time"], readTimeSuffix), @@ -1910,9 +2004,12 @@ (["eval"], readEvalSuffix) ] + id1 <- getNextIdSpanningTokenList (prefix ++ (cmd:suffix)) + id2 <- getNewIdFor id1 + let result = makeSimpleCommand id1 id2 prefix [cmd] suffix if isCommand ["source", "."] cmd - then readSource pos result + then readSource result else return result where isCommand strings (T_NormalWord _ [T_Literal _ s]) = s `elem` strings @@ -1927,21 +2024,39 @@ case cmd of _ -> False - validateCommand pos cmd = + validateCommand cmd = case cmd of - (T_NormalWord _ [T_Literal _ "//"]) -> commentWarning pos - (T_NormalWord _ (T_Literal _ "/" : T_Glob _ "*" :_)) -> commentWarning pos + (T_NormalWord _ [T_Literal _ "//"]) -> commentWarning (getId cmd) + (T_NormalWord _ (T_Literal _ "/" : T_Glob _ "*" :_)) -> commentWarning (getId cmd) (T_NormalWord _ (T_Literal _ str:_)) -> do - let cmd = map toLower $ takeWhile isAlpha str - when (cmd `elem` ["elsif", "elseif"]) $ - parseProblemAt pos ErrorC 1131 "Use 'elif' to start another branch." + let cmdString = map toLower $ takeWhile isAlpha str + when (cmdString `elem` ["elsif", "elseif"]) $ + parseProblemAtId (getId cmd) ErrorC 1131 "Use 'elif' to start another branch." _ -> return () - commentWarning pos = - parseProblemAt pos ErrorC 1127 "Was this intended as a comment? Use # in sh." + commentWarning id = + parseProblemAtId id ErrorC 1127 "Was this intended as a comment? Use # in sh." -readSource :: Monad m => SourcePos -> Token -> SCParser m Token -readSource pos t@(T_Redirecting _ _ (T_SimpleCommand _ _ (cmd:file:_))) = do + makeSimpleCommand id1 id2 prefix cmd suffix = + let + (preAssigned, preRest) = partition assignment prefix + (preRedirected, preRest2) = partition redirection preRest + (postRedirected, postRest) = partition redirection suffix + + redirs = preRedirected ++ postRedirected + assigns = preAssigned + args = cmd ++ preRest2 ++ postRest + in + T_Redirecting id1 redirs $ T_SimpleCommand id2 assigns args + where + assignment (T_Assignment {}) = True + assignment _ = False + redirection (T_FdRedirect {}) = True + redirection _ = False + + +readSource :: Monad m => Token -> SCParser m Token +readSource t@(T_Redirecting _ _ (T_SimpleCommand cmdId _ (cmd:file:_))) = do override <- getSourceOverride let literalFile = do name <- override `mplus` getLiteralString file @@ -1950,14 +2065,14 @@ return name case literalFile of Nothing -> do - parseNoteAt pos WarningC 1090 + parseNoteAtId (getId file) WarningC 1090 "Can't follow non-constant source. Use a directive to specify location." return t Just filename -> do proceed <- shouldFollow filename if not proceed then do - parseNoteAt pos InfoC 1093 + parseNoteAtId (getId file) InfoC 1093 "This file appears to be recursively sourced. Ignoring." return t else do @@ -1968,19 +2083,19 @@ else system $ siReadFile sys filename case input of Left err -> do - parseNoteAt pos InfoC 1091 $ + parseNoteAtId (getId file) InfoC 1091 $ "Not following: " ++ err return t Right script -> do - id1 <- getNextIdAt pos - id2 <- getNextIdAt pos + id1 <- getNewIdFor cmdId + id2 <- getNewIdFor cmdId let included = do src <- subRead filename script return $ T_SourceCommand id1 t (T_Include id2 src) let failed = do - parseNoteAt pos WarningC 1094 + parseNoteAtId (getId file) WarningC 1094 "Parsing of sourced file failed. Ignoring it." return t @@ -1990,7 +2105,7 @@ withContext (ContextSource name) $ inSeparateContext $ subParse (initialPos name) readScript script -readSource _ t = return t +readSource t = return t prop_readPipeline = isOk readPipeline "! cat /etc/issue | grep -i ubuntu" @@ -2009,9 +2124,10 @@ prop_readAndOr1 = isOk readAndOr "# shellcheck disable=1\nfoo" prop_readAndOr2 = isOk readAndOr "# shellcheck disable=1\n# lol\n# shellcheck disable=3\nfoo" readAndOr = do - aid <- getNextId + start <- startSpan apos <- getPosition annotations <- readAnnotations + aid <- endSpan start unless (null annotations) $ optional $ do try . lookAhead $ readKeyword @@ -2039,27 +2155,28 @@ allspacing m <- readAndOr readTerm' m - -readTerm' current = - do - id <- getNextId - sep <- readSeparator - more <- option (T_EOF id) readAndOr - case more of (T_EOF _) -> return [transformWithSeparator id sep current] - _ -> do - list <- readTerm' more - return (transformWithSeparator id sep current : list) - <|> - return [current] - -transformWithSeparator i '&' = T_Backgrounded i -transformWithSeparator i _ = id + where + readTerm' current = + do + (sep, (start, end)) <- readSeparator + id <- getNextIdBetween start end + more <- option (T_EOF id) readAndOr + case more of (T_EOF _) -> return [transformWithSeparator id sep current] + _ -> do + list <- readTerm' more + return (transformWithSeparator id sep current : list) + <|> + return [current] + where + transformWithSeparator i '&' = T_Backgrounded i + transformWithSeparator i _ = id readPipeSequence = do - id <- getNextId + start <- startSpan (cmds, pipes) <- sepBy1WithSeparators readCommand (readPipe `thenSkip` (spacing >> readLineBreak)) + id <- endSpan start spacing return $ T_Pipeline id pipes cmds where @@ -2072,9 +2189,10 @@ readPipe = do notFollowedBy2 g_OR_IF - id <- getNextId + start <- startSpan char '|' qualifier <- string "&" <|> return "" + id <- endSpan start spacing return $ T_Pipe id ('|':qualifier) @@ -2109,7 +2227,7 @@ prop_readIfClause4 = isWarning readIfClause "if false; then true; else if true; then echo lol; fi; fi" prop_readIfClause5 = isOk readIfClause "if false; then true; else\nif true; then echo lol; fi; fi" readIfClause = called "if expression" $ do - id <- getNextId + start <- startSpan pos <- getPosition (condition, action) <- readIfPart elifs <- many readElifPart @@ -2119,6 +2237,7 @@ parseProblemAt pos ErrorC 1046 "Couldn't find 'fi' for this 'if'." parseProblem ErrorC 1047 "Expected 'fi' matching previously mentioned 'if'." return "Expected 'fi'" + id <- endSpan start return $ T_IfExpression id ((condition, action):elifs) elses @@ -2183,19 +2302,20 @@ prop_readSubshell = isOk readSubshell "( cd /foo; tar cf stuff.tar * )" readSubshell = called "explicit subshell" $ do - id <- getNextId + start <- startSpan char '(' allspacing list <- readCompoundList allspacing - char ')' <|> fail ") closing the subshell" + char ')' <|> fail "Expected ) closing the subshell" + id <- endSpan start return $ T_Subshell id list prop_readBraceGroup = isOk readBraceGroup "{ a; b | c | d; e; }" prop_readBraceGroup2 = isWarning readBraceGroup "{foo;}" prop_readBraceGroup3 = isOk readBraceGroup "{(foo)}" readBraceGroup = called "brace group" $ do - id <- getNextId + start <- startSpan char '{' void allspacingOrFail <|> optional (do lookAhead $ noneOf "(" -- {( is legal @@ -2208,31 +2328,33 @@ char '}' <|> do parseProblem ErrorC 1056 "Expected a '}'. If you have one, try a ; or \\n in front of it." fail "Missing '}'" + id <- endSpan start return $ T_BraceGroup id list prop_readWhileClause = isOk readWhileClause "while [[ -e foo ]]; do sleep 1; done" readWhileClause = called "while loop" $ do - pos <- getPosition - (T_While id) <- g_While + start <- startSpan + kwId <- getId <$> g_While condition <- readTerm - statements <- readDoGroup pos + statements <- readDoGroup kwId + id <- endSpan start return $ T_WhileExpression id condition statements prop_readUntilClause = isOk readUntilClause "until kill -0 $PID; do sleep 1; done" readUntilClause = called "until loop" $ do - pos <- getPosition - (T_Until id) <- g_Until + start <- startSpan + kwId <- getId <$> g_Until condition <- readTerm - statements <- readDoGroup pos + statements <- readDoGroup kwId + id <- endSpan start return $ T_UntilExpression id condition statements -readDoGroup loopPos = do - pos <- getPosition +readDoGroup kwId = do optional (do try . lookAhead $ g_Done - parseProblemAt loopPos ErrorC 1057 "Did you forget the 'do' for this loop?") + parseProblemAtId kwId ErrorC 1057 "Did you forget the 'do' for this loop?") - g_Do `orFail` do + doKw <- g_Do `orFail` do parseProblem ErrorC 1058 "Expected 'do'." return "Expected 'do'" @@ -2241,11 +2363,11 @@ optional (do try . lookAhead $ g_Done - parseProblemAt loopPos ErrorC 1060 "Can't have empty do clauses (use 'true' as a no-op).") + parseProblemAtId (getId doKw) ErrorC 1060 "Can't have empty do clauses (use 'true' as a no-op).") commands <- readCompoundList g_Done `orFail` do - parseProblemAt pos ErrorC 1061 "Couldn't find 'done' for this 'do'." + parseProblemAtId (getId doKw) ErrorC 1061 "Couldn't find 'done' for this 'do'." parseProblem ErrorC 1062 "Expected 'done' matching previously mentioned 'do'." return "Expected 'done'" return commands @@ -2266,9 +2388,9 @@ pos <- getPosition (T_For id) <- g_For spacing - readArithmetic id pos <|> readRegular id pos + readArithmetic id <|> readRegular id where - readArithmetic id pos = called "arithmetic for condition" $ do + readArithmetic id = called "arithmetic for condition" $ do try $ string "((" x <- readArithmeticContents char ';' >> spacing @@ -2279,29 +2401,28 @@ string "))" spacing optional $ readSequentialSep >> spacing - group <- readBraced <|> readDoGroup pos + group <- readBraced <|> readDoGroup id return $ T_ForArithmetic id x y z group readBraced = do (T_BraceGroup _ list) <- readBraceGroup return list - readRegular id pos = do + readRegular id = do acceptButWarn (char '$') ErrorC 1086 "Don't use $ on the iterator name in for loops." name <- readVariableName `thenSkip` allspacing values <- readInClause <|> (optional readSequentialSep >> return []) - group <- readDoGroup pos + group <- readDoGroup id return $ T_ForIn id name values group prop_readSelectClause1 = isOk readSelectClause "select foo in *; do echo $foo; done" prop_readSelectClause2 = isOk readSelectClause "select foo; do echo $foo; done" readSelectClause = called "select loop" $ do - pos <- getPosition (T_Select id) <- g_Select spacing typ <- readRegular - group <- readDoGroup pos + group <- readDoGroup id typ id group where readRegular = do @@ -2331,7 +2452,7 @@ prop_readCaseClause4 = isOk readCaseClause "case foo\n in *) echo bar ;& bar) foo; esac" prop_readCaseClause5 = isOk readCaseClause "case foo\n in *) echo bar;;& foo) baz;; esac" readCaseClause = called "case expression" $ do - id <- getNextId + start <- startSpan g_Case word <- readNormalWord allspacing @@ -2339,6 +2460,7 @@ readLineBreak list <- readCaseList g_Esac <|> fail "Expected 'esac' to close the case statement" + id <- endSpan start return $ T_CaseExpression id word list readCaseList = many readCaseItem @@ -2383,18 +2505,20 @@ prop_readFunctionDefinition10= isOk readFunctionDefinition "function foo () { true; }" prop_readFunctionDefinition11= isWarning readFunctionDefinition "function foo{\ntrue\n}" prop_readFunctionDefinition12= isOk readFunctionDefinition "function []!() { true; }" +prop_readFunctionDefinition13= isOk readFunctionDefinition "@require(){ true; }" readFunctionDefinition = called "function" $ do + start <- startSpan functionSignature <- try readFunctionSignature allspacing void (lookAhead $ oneOf "{(") <|> parseProblem ErrorC 1064 "Expected a { to open the function definition." group <- readBraceGroup <|> readSubshell - return $ functionSignature group + id <- endSpan start + return $ functionSignature id group where readFunctionSignature = readWithFunction <|> readWithoutFunction where readWithFunction = do - id <- getNextId try $ do string "function" whitespace @@ -2405,15 +2529,14 @@ when (not hasParens && null spaces) $ acceptButWarn (lookAhead (oneOf "{(")) ErrorC 1095 "You need a space or linefeed between the function name and body." - return $ T_Function id (FunctionKeyword True) (FunctionParentheses hasParens) name + return $ \id -> T_Function id (FunctionKeyword True) (FunctionParentheses hasParens) name readWithoutFunction = try $ do - id <- getNextId name <- many1 functionChars guard $ name /= "time" -- Interfers with time ( foo ) spacing readParens - return $ T_Function id (FunctionKeyword False) (FunctionParentheses True) name + return $ \id -> T_Function id (FunctionKeyword False) (FunctionParentheses True) name readParens = do g_Lparen @@ -2428,23 +2551,26 @@ prop_readCoProc2 = isOk readCoProc "coproc { echo bar; }" prop_readCoProc3 = isOk readCoProc "coproc echo bar" readCoProc = called "coproc" $ do - id <- getNextId + start <- startSpan try $ do string "coproc" whitespace - choice [ try $ readCompoundCoProc id, readSimpleCoProc id ] + choice [ try $ readCompoundCoProc start, readSimpleCoProc start ] where - readCompoundCoProc id = do + readCompoundCoProc start = do var <- optionMaybe $ readVariableName `thenSkip` whitespace body <- readBody readCompoundCommand + id <- endSpan start return $ T_CoProc id var body - readSimpleCoProc id = do + readSimpleCoProc start = do body <- readBody readSimpleCommand + id <- endSpan start return $ T_CoProc id Nothing body readBody parser = do - id <- getNextId + start <- startSpan body <- parser + id <- endSpan start return $ T_CoProcBody id body @@ -2452,7 +2578,6 @@ prop_readCompoundCommand = isOk readCompoundCommand "{ echo foo; }>/dev/null" readCompoundCommand = do - id <- getNextId cmd <- choice [ readBraceGroup, readAmbiguous "((" readArithmeticExpression readSubshell (\pos -> @@ -2469,6 +2594,7 @@ ] spacing redirs <- many readIoRedirect + id <- getNextIdSpanningTokenList (cmd:redirs) unless (null redirs) $ optional $ do lookAhead $ try (spacing >> needsSeparator) parseProblem WarningC 1013 "Bash requires ; or \\n here, after redirecting nested compound commands." @@ -2535,6 +2661,13 @@ where readUntil endPos = anyChar `reluctantlyTill` (getPosition >>= guard . (== endPos)) +-- Like readStringForParser, returning the span as a T_Literal +readLiteralForParser parser = do + start <- startSpan + str <- readStringForParser parser + id <- endSpan start + return $ T_Literal id str + prop_readAssignmentWord = isOk readAssignmentWord "a=42" prop_readAssignmentWord2 = isOk readAssignmentWord "b=(1 2 3)" prop_readAssignmentWord3 = isWarning readAssignmentWord "$b = 13" @@ -2556,7 +2689,7 @@ readAssignmentWord = readAssignmentWordExt True readWellFormedAssignment = readAssignmentWordExt False readAssignmentWordExt lenient = try $ do - id <- getNextId + start <- startSpan pos <- getPosition when lenient $ optional (char '$' >> parseNote ErrorC 1066 "Don't use $ on the left side of assignments.") @@ -2565,11 +2698,12 @@ optional (readNormalDollar >> parseNoteAt pos ErrorC 1067 "For indirection, use (associative) arrays or 'read \"var$n\" <<< \"value\"'") indices <- many readArrayIndex - hasLeftSpace <- liftM (not . null) spacing + hasLeftSpace <- fmap (not . null) spacing pos <- getPosition + id <- endSpan start op <- readAssignmentOp - hasRightSpace <- liftM (not . null) spacing - isEndOfCommand <- liftM isJust $ optionMaybe (try . lookAhead $ (void (oneOf "\r\n;&|)") <|> eof)) + hasRightSpace <- fmap (not . null) spacing + isEndOfCommand <- fmap isJust $ optionMaybe (try . lookAhead $ (void (oneOf "\r\n;&|)") <|> eof)) if not hasLeftSpace && (hasRightSpace || isEndOfCommand) then do when (variable /= "IFS" && hasRightSpace && not isEndOfCommand) $ @@ -2602,20 +2736,22 @@ string "=" >> return Assign ] readEmptyLiteral = do - id <- getNextId + start <- startSpan + id <- endSpan start return $ T_Literal id "" readArrayIndex = do - id <- getNextId + start <- startSpan char '[' pos <- getPosition str <- readStringForParser readIndexSpan char ']' + id <- endSpan start return $ T_UnparsedIndex id pos str readArray :: Monad m => SCParser m Token readArray = called "array assignment" $ do - id <- getNextId + start <- startSpan opening <- getPosition char '(' optional $ do @@ -2624,39 +2760,45 @@ allspacing words <- readElement `reluctantlyTill` char ')' char ')' <|> fail "Expected ) to close array assignment" + id <- endSpan start return $ T_Array id words where readElement = (readIndexed <|> readRegular) `thenSkip` allspacing readIndexed = do - id <- getNextId + start <- startSpan index <- try $ do x <- many1 readArrayIndex char '=' return x value <- readRegular <|> nothing + id <- endSpan start return $ T_IndexedElement id index value readRegular = readArray <|> readNormalWord nothing = do - id <- getNextId + start <- startSpan + id <- endSpan start return $ T_Literal id "" tryToken s t = try $ do - id <- getNextId + start <- startSpan string s + id <- endSpan start spacing return $ t id redirToken c t = try $ do - id <- getNextId + start <- startSpan char c + id <- endSpan start notFollowedBy2 $ char '(' return $ t id tryWordToken s t = tryParseWordToken s t `thenSkip` spacing tryParseWordToken keyword t = try $ do - id <- getNextId + start <- startSpan str <- anycaseString keyword + id <- endSpan start optional $ do c <- try . lookAhead $ anyChar @@ -2707,15 +2849,17 @@ g_In = tryWordToken "in" T_In <* skipAnnotationAndWarn g_Lbrace = tryWordToken "{" T_Lbrace g_Rbrace = do -- handled specially due to ksh echo "${ foo; }bar" - id <- getNextId + start <- startSpan char '}' + id <- endSpan start return $ T_Rbrace id g_Lparen = tryToken "(" T_Lparen g_Rparen = tryToken ")" T_Rparen g_Bang = do - id <- getNextId + start <- startSpan char '!' + id <- endSpan start void spacing1 <|> do pos <- getPosition parseProblemAt pos ErrorC 1035 @@ -2758,9 +2902,10 @@ readCorrect = void $ string "#!" readSwapped = do - pos <- getPosition + start <- startSpan string "!#" - parseProblemAt pos ErrorC 1084 + id <- endSpan start + parseProblemAtId id ErrorC 1084 "Use #!, not !#, for the shebang." skipSpaces = fmap (not . null) $ many linewhitespace @@ -2828,25 +2973,35 @@ prop_readScript4 = isWarning readScriptFile "#!/usr/bin/perl\nfoo=(" prop_readScript5 = isOk readScriptFile "#!/bin/bash\n#This is an empty script\n\n" readScriptFile = do - id <- getNextId + start <- startSpan pos <- getPosition optional $ do readUtf8Bom parseProblem ErrorC 1082 "This file has a UTF-8 BOM. Remove it with: LC_CTYPE=C sed '1s/^...//' < yourscript ." sb <- option "" readShebang - verifyShell pos (getShell sb) - if isValidShell (getShell sb) /= Just False + allspacing + annotationStart <- startSpan + annotations <- readAnnotations + annotationId <- endSpan annotationStart + let shellAnnotationSpecified = + any (\x -> case x of ShellOverride {} -> True; _ -> False) annotations + shellFlagSpecified <- isJust <$> Mr.asks shellTypeOverride + let ignoreShebang = shellAnnotationSpecified || shellFlagSpecified + + unless ignoreShebang $ + verifyShebang pos (getShell sb) + if ignoreShebang || isValidShell (getShell sb) /= Just False then do - allspacing - annotationId <- getNextId - annotations <- readAnnotations commands <- withAnnotations annotations readCompoundListOrEmpty + id <- endSpan start verifyEof - let script = T_Annotation annotationId annotations $ T_Script id sb commands + let script = T_Annotation annotationId annotations $ + T_Script id sb commands reparseIndices script else do many anyChar + id <- endSpan start return $ T_Script id sb [] where @@ -2860,7 +3015,7 @@ then second else basename first - verifyShell pos s = + verifyShebang pos s = do case isValidShell s of Just True -> return () Just False -> parseProblemAt pos ErrorC 1071 "ShellCheck only supports sh/bash/dash/ksh scripts. Sorry!" @@ -2910,24 +3065,28 @@ result { -- Remove the noisiest parts prTokenPositions = Map.fromList [ - (Id 0, Position { + (Id 0, (newPosition { posFile = "removed for clarity", posLine = -1, posColumn = -1 - })] + }, newPosition { + posFile = "removed for clarity", + posLine = -1, + posColumn = -1 + }))] } where result = runIdentity $ - parseScript (mockedSystemInterface []) $ ParseSpec { + parseScript (mockedSystemInterface []) $ newParseSpec { psFilename = "debug", - psScript = string, - psCheckSourced = False + psScript = string } testEnvironment = Environment { systemInterface = (mockedSystemInterface []), - checkSourced = False + checkSourced = False, + shellTypeOverride = Nothing } @@ -2999,14 +3158,14 @@ (result, state) <- runParser env (parseWithNotes readScript) name contents case result of Right (script, userstate) -> - return ParseResult { + return newParseResult { prComments = map toPositionedComment $ nub $ parseNotes userstate ++ parseProblems state, - prTokenPositions = Map.map posToPos (positionMap userstate), + prTokenPositions = Map.map startEndPosToPos (positionMap userstate), prRoot = Just $ reattachHereDocs script (hereDocMap userstate) } Left err -> - return ParseResult { + return newParseResult { prComments = map toPositionedComment $ notesForContext (contextStack state) @@ -3073,15 +3232,26 @@ toPositionedComment :: ParseNote -> PositionedComment toPositionedComment (ParseNote start end severity code message) = - PositionedComment (posToPos start) (posToPos end) $ Comment severity code message + newPositionedComment { + pcStartPos = (posToPos start) + , pcEndPos = (posToPos end) + , pcComment = newComment { + cSeverity = severity + , cCode = code + , cMessage = message + } + } posToPos :: SourcePos -> Position -posToPos sp = Position { +posToPos sp = newPosition { posFile = sourceName sp, posLine = fromIntegral $ sourceLine sp, posColumn = fromIntegral $ sourceColumn sp } +startEndPosToPos :: (SourcePos, SourcePos) -> (Position, Position) +startEndPosToPos (s, e) = (posToPos s, posToPos e) + -- TODO: Clean up crusty old code that this is layered on top of parseScript :: Monad m => SystemInterface m -> ParseSpec -> m ParseResult @@ -3090,7 +3260,8 @@ where env = Environment { systemInterface = sys, - checkSourced = psCheckSourced spec + checkSourced = psCheckSourced spec, + shellTypeOverride = psShellTypeOverride spec } -- Same as 'try' but emit syntax errors if the parse fails. diff --git a/striptests b/striptests new file mode 100644 --- /dev/null +++ b/striptests @@ -0,0 +1,78 @@ +#!/usr/bin/env bash +# This file strips all unit tests from ShellCheck, removing +# the dependency on QuickCheck and Template Haskell and +# reduces the binary size considerably. +set -o pipefail + +sponge() { + local data + data="$(cat)" + printf '%s\n' "$data" > "$1" +} + +modify() { + if ! "${@:2}" < "$1" | sponge "$1" + then + { + printf 'Failed to modify %s: ' "$1" + printf '%q ' "${@:2}" + printf '\n' + } >&2 + exit 1 + fi +} + +detestify() { + printf '%s\n' '-- AUTOGENERATED from ShellCheck by striptests. Do not modify.' + awk ' + BEGIN { + state = 0; + } + + /LANGUAGE TemplateHaskell/ { next; } + /^import.*Test\./ { next; } + + /^module/ { + sub(/,[^,)]*runTests/, ""); + } + + # Delete tests + /^prop_/ { state = 1; next; } + + # ..and any blank lines following them. + state == 1 && /^ / { next; } + + # Template Haskell marker + /^return / { + exit; + } + + { state = 0; print; } + ' +} + + + +if [[ ! -e 'ShellCheck.cabal' ]] +then + echo "Run me from the ShellCheck directory." >&2 + exit 1 +fi + +if [[ -d '.git' ]] && ! git diff --exit-code > /dev/null 2>&1 +then + echo "You have local changes! These may be overwritten." >&2 + exit 2 +fi + +modify 'ShellCheck.cabal' sed -e ' + /QuickCheck/d + /^test-suite/{ s/.*//; q; } + ' + +find . -name '.git' -prune -o -type f -name '*.hs' -print | + while IFS= read -r file + do + modify "$file" detestify + done +