diff -pruN 0.10.0-1/CHANGELOG.md 0.11.0-1/CHANGELOG.md --- 0.10.0-1/CHANGELOG.md 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/CHANGELOG.md 2001-09-09 01:46:40.000000000 +0000 @@ -1,3 +1,34 @@ +## v0.11.0 - 2025-08-03 +### Added +- SC2327/SC2328: Warn about capturing the output of redirected commands. +- SC2329: Warn when (non-escaping) functions are never invoked. +- SC2330: Warn about unsupported glob matches with [[ .. ]] in BusyBox. +- SC2331: Suggest using standard -e instead of unary -a in tests. +- SC2332: Warn about `[ ! -o opt ]` being unconditionally true in Bash. +- SC3062: Warn about bashism `[ -o opt ]`. +- Optional `avoid-negated-conditions`: suggest replacing `[ ! a -eq b ]` + with `[ a -ne b ]`, and similar for -ge/-lt/=/!=/etc (SC2335). +- Precompiled binaries for Linux riscv64 (linux.riscv64) + +### Changed +- SC2002 about Useless Use Of Cat is now disabled by default. It can be + re-enabled with `--enable=useless-use-of-cat` or equivalent directive. +- SC2236/SC2237 about replacing `[ ! -n .. ]` with `[ -z ]` and vice versa + is now optional under `avoid-negated-conditions`. +- SC2015 about `A && B || C` no longer triggers when B is a test command. +- SC3012: Do not warn about `\<` and `\>` in test/[] as specified in POSIX.1-2024 +- Diff output now uses / as path separator on Windows + +### Fixed +- SC2218 about function use-before-define is now more accurate. +- SC2317 about unreachable commands is now less spammy for nested ones. +- SC2292, optional suggestion for [[ ]], now triggers for Busybox. +- Updates for Bash 5.3, including `${| cmd; }` and `source -p` + +### Removed +- SC3013: removed since the operators `-ot/-nt/-ef` are specified in POSIX.1-2024 + + ## v0.10.0 - 2024-03-07 ### Added - Precompiled binaries for macOS ARM64 (darwin.aarch64) diff -pruN 0.10.0-1/README.md 0.11.0-1/README.md --- 0.10.0-1/README.md 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/README.md 2001-09-09 01:46:40.000000000 +0000 @@ -110,9 +110,11 @@ Services and platforms that have ShellCh * [Codacy](https://www.codacy.com/) * [Code Climate](https://codeclimate.com/) * [Code Factor](https://www.codefactor.io/) +* [Codety](https://www.codety.io/) via the [Codety Scanner](https://github.com/codetyio/codety-scanner) * [CircleCI](https://circleci.com) via the [ShellCheck Orb](https://circleci.com/orbs/registry/orb/circleci/shellcheck) * [Github](https://github.com/features/actions) (only Linux) -* [Trunk Check](https://trunk.io/products/check) (universal linter; [allows you to explicitly version your shellcheck install](https://github.com/trunk-io/plugins/blob/bcbb361dcdbe4619af51ea7db474d7fb87540d20/.trunk/trunk.yaml#L32)) via the [shellcheck plugin](https://github.com/trunk-io/plugins/blob/main/linters/shellcheck/plugin.yaml) +* [Trunk Code Quality](https://trunk.io/code-quality) (universal linter; [allows you to explicitly version your shellcheck install](https://github.com/trunk-io/plugins/blob/bcbb361dcdbe4619af51ea7db474d7fb87540d20/.trunk/trunk.yaml#L32)) via the [shellcheck plugin](https://github.com/trunk-io/plugins/blob/main/linters/shellcheck/plugin.yaml) +* [CodeRabbit](https://coderabbit.ai/) Most other services, including [GitLab](https://about.gitlab.com/), let you install ShellCheck yourself, either through the system's package manager (see [Installing](#installing)), @@ -228,11 +230,17 @@ Using the [nix package manager](https:// nix-env -iA nixpkgs.shellcheck ``` +Using the [Flox package manager](https://flox.dev/) +```sh +flox install shellcheck +``` + Alternatively, you can download pre-compiled binaries for the latest release here: * [Linux, x86_64](https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.linux.x86_64.tar.xz) (statically linked) * [Linux, armv6hf](https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.linux.armv6hf.tar.xz), i.e. Raspberry Pi (statically linked) * [Linux, aarch64](https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.linux.aarch64.tar.xz) aka ARM64 (statically linked) +* [macOS, aarch64](https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.darwin.aarch64.tar.xz) * [macOS, x86_64](https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.darwin.x86_64.tar.xz) * [Windows, x86](https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.zip) diff -pruN 0.10.0-1/ShellCheck.cabal 0.11.0-1/ShellCheck.cabal --- 0.10.0-1/ShellCheck.cabal 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/ShellCheck.cabal 2001-09-09 01:46:40.000000000 +0000 @@ -1,5 +1,5 @@ Name: ShellCheck -Version: 0.10.0 +Version: 0.11.0 Synopsis: Shell script analysis tool License: GPL-3 License-file: LICENSE @@ -46,19 +46,19 @@ library semigroups build-depends: -- The lower bounds are based on GHC 7.10.3 - -- The upper bounds are based on GHC 9.8.1 + -- The upper bounds are based on GHC 9.12.1 aeson >= 1.4.0 && < 2.3, array >= 0.5.1 && < 0.6, base >= 4.8.0.0 && < 5, bytestring >= 0.10.6 && < 0.13, - containers >= 0.5.6 && < 0.8, + containers >= 0.5.6 && < 0.9, deepseq >= 1.4.1 && < 1.6, - Diff >= 0.4.0 && < 0.6, + Diff >= 0.4.0 && < 1.1, fgl (>= 5.7.0 && < 5.8.1.0) || (>= 5.8.1.1 && < 5.9), - filepath >= 1.4.0 && < 1.5, + filepath >= 1.4.0 && < 1.6, mtl >= 2.2.2 && < 2.4, parsec >= 3.1.14 && < 3.2, - QuickCheck >= 2.14.2 && < 2.15, + QuickCheck >= 2.14.2 && < 2.17, regex-tdfa >= 1.2.0 && < 1.4, transformers >= 0.4.2 && < 0.7, diff -pruN 0.10.0-1/debian/changelog 0.11.0-1/debian/changelog --- 0.10.0-1/debian/changelog 2024-03-17 09:09:13.000000000 +0000 +++ 0.11.0-1/debian/changelog 2025-08-12 19:58:53.000000000 +0000 @@ -1,3 +1,13 @@ +shellcheck (0.11.0-1) unstable; urgency=medium + + [ Ilias Tsitsimpis ] + * Declare compliance with Debian policy 4.7.0 + + [ Samuel Henrique ] + * New upstream release + + -- Samuel Henrique Tue, 12 Aug 2025 20:58:53 +0100 + shellcheck (0.10.0-1) unstable; urgency=medium [ Ilias Tsitsimpis ] diff -pruN 0.10.0-1/debian/control 0.11.0-1/debian/control --- 0.10.0-1/debian/control 2024-03-17 09:09:13.000000000 +0000 +++ 0.11.0-1/debian/control 2025-08-12 19:58:53.000000000 +0000 @@ -11,10 +11,10 @@ Build-Depends: debhelper (>= 10), ghc (>= 8.4.3), ghc-prof, libghc-diff-dev (>= 0.4.0), - libghc-diff-dev (<< 0.6), + libghc-diff-dev (<< 1.1), libghc-diff-prof, libghc-quickcheck2-dev (>= 2.14.2), - libghc-quickcheck2-dev (<< 2.15), + libghc-quickcheck2-dev (<< 2.17), libghc-quickcheck2-prof, libghc-aeson-dev (>= 1.4.0), libghc-aeson-dev (<< 2.3), @@ -26,7 +26,7 @@ Build-Depends: debhelper (>= 10), libghc-regex-tdfa-dev (<< 1.4), libghc-regex-tdfa-prof, pandoc, -Standards-Version: 4.6.2 +Standards-Version: 4.7.0 Homepage: https://www.shellcheck.net/ Vcs-Browser: https://salsa.debian.org/haskell-team/DHG_packages/tree/master/p/shellcheck Vcs-Git: https://salsa.debian.org/haskell-team/DHG_packages.git [p/shellcheck] diff -pruN 0.10.0-1/shellcheck.1.md 0.11.0-1/shellcheck.1.md --- 0.10.0-1/shellcheck.1.md 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/shellcheck.1.md 2001-09-09 01:46:40.000000000 +0000 @@ -78,7 +78,7 @@ not warn at all, as `ksh` supports decim : Don't try to look for .shellcheckrc configuration files. ---rcfile\ RCFILE +**--rcfile** *RCFILE* : Prefer the specified configuration file over searching for one in the default locations. @@ -317,7 +317,7 @@ Here is an example `.shellcheckrc`: disable=SC2236 If no `.shellcheckrc` is found in any of the parent directories, ShellCheck -will look in `~/.shellcheckrc` followed by the XDG config directory +will look in `~/.shellcheckrc` followed by the `$XDG_CONFIG_HOME` (usually `~/.config/shellcheckrc`) on Unix, or `%APPDATA%/shellcheckrc` on Windows. Only the first file found will be used. @@ -397,10 +397,10 @@ long list of wonderful contributors. # COPYRIGHT -Copyright 2012-2024, Vidar Holen and contributors. +Copyright 2012-2025, Vidar Holen and contributors. Licensed under the GNU General Public License version 3 or later, see https://gnu.org/licenses/gpl.html # SEE ALSO -sh(1) bash(1) +sh(1), bash(1), dash(1), ksh(1) diff -pruN 0.10.0-1/src/ShellCheck/AST.hs 0.11.0-1/src/ShellCheck/AST.hs --- 0.10.0-1/src/ShellCheck/AST.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/AST.hs 2001-09-09 01:46:40.000000000 +0000 @@ -31,6 +31,7 @@ newtype Id = Id Int deriving (Show, Eq, data Quoted = Quoted | Unquoted deriving (Show, Eq) data Dashed = Dashed | Undashed deriving (Show, Eq) +data Piped = Piped | Unpiped deriving (Show, Eq) data AssignmentMode = Assign | Append deriving (Show, Eq) newtype FunctionKeyword = FunctionKeyword Bool deriving (Show, Eq) newtype FunctionParentheses = FunctionParentheses Bool deriving (Show, Eq) @@ -84,7 +85,7 @@ data InnerToken t = | Inner_T_DollarDoubleQuoted [t] | Inner_T_DollarExpansion [t] | Inner_T_DollarSingleQuoted String - | Inner_T_DollarBraceCommandExpansion [t] + | Inner_T_DollarBraceCommandExpansion Piped [t] | Inner_T_Done | Inner_T_DoubleQuoted [t] | Inner_T_EOF @@ -138,7 +139,7 @@ data InnerToken t = | Inner_T_WhileExpression [t] [t] | Inner_T_Annotation [Annotation] t | Inner_T_Pipe String - | Inner_T_CoProc (Maybe String) t + | Inner_T_CoProc (Maybe Token) t | Inner_T_CoProcBody t | Inner_T_Include t | Inner_T_SourceCommand t t @@ -206,7 +207,7 @@ pattern T_Annotation id anns t = OuterTo pattern T_Arithmetic id c = OuterToken id (Inner_T_Arithmetic c) pattern T_Array id t = OuterToken id (Inner_T_Array t) pattern TA_Sequence id l = OuterToken id (Inner_TA_Sequence l) -pattern TA_Parentesis id t = OuterToken id (Inner_TA_Parenthesis t) +pattern TA_Parenthesis id t = OuterToken id (Inner_TA_Parenthesis t) pattern T_Assignment id mode var indices value = OuterToken id (Inner_T_Assignment mode var indices value) pattern TA_Trinary id t1 t2 t3 = OuterToken id (Inner_TA_Trinary t1 t2 t3) pattern TA_Unary id op t1 = OuterToken id (Inner_TA_Unary op t1) @@ -228,7 +229,7 @@ pattern T_CoProc id var body = OuterToke pattern TC_Or id typ str t1 t2 = OuterToken id (Inner_TC_Or typ str t1 t2) pattern TC_Unary id typ op token = OuterToken id (Inner_TC_Unary typ op token) pattern T_DollarArithmetic id c = OuterToken id (Inner_T_DollarArithmetic c) -pattern T_DollarBraceCommandExpansion id list = OuterToken id (Inner_T_DollarBraceCommandExpansion list) +pattern T_DollarBraceCommandExpansion id pipe list = OuterToken id (Inner_T_DollarBraceCommandExpansion pipe list) pattern T_DollarBraced id braced op = OuterToken id (Inner_T_DollarBraced braced op) pattern T_DollarBracket id c = OuterToken id (Inner_T_DollarBracket c) pattern T_DollarDoubleQuoted id list = OuterToken id (Inner_T_DollarDoubleQuoted list) @@ -259,7 +260,7 @@ pattern T_Subshell id l = OuterToken id pattern T_UntilExpression id c l = OuterToken id (Inner_T_UntilExpression c l) pattern T_WhileExpression id c l = OuterToken id (Inner_T_WhileExpression c l) -{-# COMPLETE T_AND_IF, T_Bang, T_Case, TC_Empty, T_CLOBBER, T_DGREAT, T_DLESS, T_DLESSDASH, T_Do, T_DollarSingleQuoted, T_Done, T_DSEMI, T_Elif, T_Else, T_EOF, T_Esac, T_Fi, T_For, T_Glob, T_GREATAND, T_Greater, T_If, T_In, T_Lbrace, T_Less, T_LESSAND, T_LESSGREAT, T_Literal, T_Lparen, T_NEWLINE, T_OR_IF, T_ParamSubSpecialChar, T_Pipe, T_Rbrace, T_Rparen, T_Select, T_Semi, T_SingleQuoted, T_Then, T_UnparsedIndex, T_Until, T_While, TA_Assignment, TA_Binary, TA_Expansion, T_AndIf, T_Annotation, T_Arithmetic, T_Array, TA_Sequence, TA_Parentesis, T_Assignment, TA_Trinary, TA_Unary, TA_Variable, T_Backgrounded, T_Backticked, T_Banged, T_BatsTest, T_BraceExpansion, T_BraceGroup, TC_And, T_CaseExpression, TC_Binary, TC_Group, TC_Nullary, T_Condition, T_CoProcBody, T_CoProc, TC_Or, TC_Unary, T_DollarArithmetic, T_DollarBraceCommandExpansion, T_DollarBraced, T_DollarBracket, T_DollarDoubleQuoted, T_DollarExpansion, T_DoubleQuoted, T_Extglob, T_FdRedirect, T_ForArithmetic, T_ForIn, T_Function, T_HereDoc, T_HereString, T_IfExpression, T_Include, T_IndexedElement, T_IoDuplicate, T_IoFile, T_NormalWord, T_OrIf, T_Pipeline, T_ProcSub, T_Redirecting, T_Script, T_SelectIn, T_SimpleCommand, T_SourceCommand, T_Subshell, T_UntilExpression, T_WhileExpression #-} +{-# COMPLETE T_AND_IF, T_Bang, T_Case, TC_Empty, T_CLOBBER, T_DGREAT, T_DLESS, T_DLESSDASH, T_Do, T_DollarSingleQuoted, T_Done, T_DSEMI, T_Elif, T_Else, T_EOF, T_Esac, T_Fi, T_For, T_Glob, T_GREATAND, T_Greater, T_If, T_In, T_Lbrace, T_Less, T_LESSAND, T_LESSGREAT, T_Literal, T_Lparen, T_NEWLINE, T_OR_IF, T_ParamSubSpecialChar, T_Pipe, T_Rbrace, T_Rparen, T_Select, T_Semi, T_SingleQuoted, T_Then, T_UnparsedIndex, T_Until, T_While, TA_Assignment, TA_Binary, TA_Expansion, T_AndIf, T_Annotation, T_Arithmetic, T_Array, TA_Sequence, TA_Parenthesis, T_Assignment, TA_Trinary, TA_Unary, TA_Variable, T_Backgrounded, T_Backticked, T_Banged, T_BatsTest, T_BraceExpansion, T_BraceGroup, TC_And, T_CaseExpression, TC_Binary, TC_Group, TC_Nullary, T_Condition, T_CoProcBody, T_CoProc, TC_Or, TC_Unary, T_DollarArithmetic, T_DollarBraceCommandExpansion, T_DollarBraced, T_DollarBracket, T_DollarDoubleQuoted, T_DollarExpansion, T_DoubleQuoted, T_Extglob, T_FdRedirect, T_ForArithmetic, T_ForIn, T_Function, T_HereDoc, T_HereString, T_IfExpression, T_Include, T_IndexedElement, T_IoDuplicate, T_IoFile, T_NormalWord, T_OrIf, T_Pipeline, T_ProcSub, T_Redirecting, T_Script, T_SelectIn, T_SimpleCommand, T_SourceCommand, T_Subshell, T_UntilExpression, T_WhileExpression #-} instance Eq Token where OuterToken _ a == OuterToken _ b = a == b diff -pruN 0.10.0-1/src/ShellCheck/ASTLib.hs 0.11.0-1/src/ShellCheck/ASTLib.hs --- 0.10.0-1/src/ShellCheck/ASTLib.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/ASTLib.hs 2001-09-09 01:46:40.000000000 +0000 @@ -446,6 +446,12 @@ getLiteralStringExt more = g -- Is this token a string literal? isLiteral t = isJust $ getLiteralString t +-- Is this token a string literal number? +isLiteralNumber t = fromMaybe False $ do + s <- getLiteralString t + guard $ all isDigit s + return True + -- Escape user data for messages. -- Messages generally avoid repeating user data, but sometimes it's helpful. e4m = escapeForMessage @@ -555,7 +561,7 @@ getCommandNameFromExpansion t = case t of T_DollarExpansion _ [c] -> extract c T_Backticked _ [c] -> extract c - T_DollarBraceCommandExpansion _ [c] -> extract c + T_DollarBraceCommandExpansion _ _ [c] -> extract c _ -> Nothing where extract (T_Pipeline _ _ [cmd]) = getCommandName cmd @@ -610,7 +616,7 @@ getCommandSequences t = T_Annotation _ _ t -> getCommandSequences t T_DollarExpansion _ cmds -> [cmds] - T_DollarBraceCommandExpansion _ cmds -> [cmds] + T_DollarBraceCommandExpansion _ _ cmds -> [cmds] T_Backticked _ cmds -> [cmds] _ -> [] diff -pruN 0.10.0-1/src/ShellCheck/Analytics.hs 0.11.0-1/src/ShellCheck/Analytics.hs --- 0.10.0-1/src/ShellCheck/Analytics.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Analytics.hs 2001-09-09 01:46:40.000000000 +0000 @@ -103,8 +103,7 @@ nodeChecksToTreeCheck checkList = nodeChecks :: [Parameters -> Token -> Writer [TokenComment] ()] nodeChecks = [ - checkUuoc - ,checkPipePitfalls + checkPipePitfalls ,checkForInQuoted ,checkForInLs ,checkShorthandIf @@ -124,6 +123,7 @@ nodeChecks = [ ,checkCaseAgainstGlob ,checkCommarrays ,checkOrNeq + ,checkAndEq ,checkEchoWc ,checkConstantIfs ,checkPipedAssignment @@ -183,7 +183,6 @@ nodeChecks = [ ,checkPipeToNowhere ,checkForLoopGlobVariables ,checkSubshelledTests - ,checkInvertedStringTest ,checkRedirectionToCommand ,checkDollarQuoteParen ,checkUselessBang @@ -204,6 +203,8 @@ nodeChecks = [ ,checkUnnecessaryArithmeticExpansionIndex ,checkUnnecessaryParens ,checkPlusEqualsNumber + ,checkExpansionWithRedirection + ,checkUnaryTestA ] optionalChecks = map fst optionalTreeChecks @@ -232,6 +233,13 @@ optionalTreeChecks = [ }, nodeChecksToTreeCheck [checkNullaryExpansionTest]) ,(newCheckDescription { + cdName = "avoid-negated-conditions", + cdDescription = "Suggest removing unnecessary comparison negations", + cdPositive = "[ ! \"$var\" -eq 1 ]", + cdNegative = "[ \"$var\" -ne 1 ]" + }, nodeChecksToTreeCheck [checkUnnecessarilyInvertedTest]) + + ,(newCheckDescription { cdName = "add-default-case", cdDescription = "Suggest adding a default case in `case` statements", cdPositive = "case $? in 0) echo 'Success';; esac", @@ -272,6 +280,13 @@ optionalTreeChecks = [ cdPositive = "rm -r \"$(get_chroot_dir)/home\"", cdNegative = "set -e; dir=\"$(get_chroot_dir)\"; rm -r \"$dir/home\"" }, checkExtraMaskedReturns) + + ,(newCheckDescription { + cdName = "useless-use-of-cat", + cdDescription = "Check for Useless Use Of Cat (UUOC)", + cdPositive = "cat foo | grep bar", + cdNegative = "grep bar foo" + }, nodeChecksToTreeCheck [checkUuoc]) ] optionalCheckMap :: Map.Map String (Parameters -> Token -> [TokenComment]) @@ -490,15 +505,12 @@ checkWrongArithmeticAssignment params (T sequence_ $ do str <- getNormalString val var:op:_ <- matchRegex regex str - Map.lookup var references + guard $ S.member var references return . warn (getId val) 2100 $ "Use $((..)) for arithmetics, e.g. i=$((i " ++ op ++ " 2))" where regex = mkRegex "^([_a-zA-Z][_a-zA-Z0-9]*)([+*-]).+$" - references = foldl (flip ($)) Map.empty (map insertRef $ variableFlow params) - insertRef (Assignment (_, _, name, _)) = - Map.insert name () - insertRef _ = Prelude.id + references = S.fromList [name | Assignment (_, _, name, _) <- variableFlow params] getNormalString (T_NormalWord _ words) = do parts <- mapM getLiterals words @@ -794,7 +806,7 @@ checkUnquotedExpansions params = where check t@(T_DollarExpansion _ c) = examine t c check t@(T_Backticked _ c) = examine t c - check t@(T_DollarBraceCommandExpansion _ c) = examine t c + check t@(T_DollarBraceCommandExpansion _ _ c) = examine t c check _ = return () tree = parentMap params examine t contents = @@ -876,13 +888,16 @@ prop_checkShorthandIf5 = verifyNot check prop_checkShorthandIf6 = verifyNot checkShorthandIf "if foo && bar || baz; then true; fi" prop_checkShorthandIf7 = verifyNot checkShorthandIf "while foo && bar || baz; do true; done" prop_checkShorthandIf8 = verify checkShorthandIf "if true; then foo && bar || baz; fi" -checkShorthandIf params x@(T_OrIf _ (T_AndIf id _ _) (T_Pipeline _ _ t)) - | not (isOk t || inCondition) = +prop_checkShorthandIf9 = verifyNot checkShorthandIf "foo && [ -x /file ] || bar" +prop_checkShorthandIf10 = verifyNot checkShorthandIf "foo && bar || true" +prop_checkShorthandIf11 = verify checkShorthandIf "foo && bar || false" +checkShorthandIf params x@(T_OrIf _ (T_AndIf id _ b) (T_Pipeline _ _ t)) + | not (isOk t || inCondition) && not (isTestCommand b) = info id 2015 "Note that A && B || C is not if-then-else. C may run when A is true." where isOk [t] = isAssignment t || fromMaybe False (do name <- getCommandBasename t - return $ name `elem` ["echo", "exit", "return", "printf"]) + return $ name `elem` ["echo", "exit", "return", "printf", "true", ":"]) isOk _ = False inCondition = isCondition $ getPath (parentMap params) x checkShorthandIf _ _ = return () @@ -972,32 +987,32 @@ prop_checkArrayWithoutIndex9 = verifyTre prop_checkArrayWithoutIndex10 = verifyTree checkArrayWithoutIndex "read -ra arr <<< 'foo bar'; echo \"$arr\"" prop_checkArrayWithoutIndex11 = verifyNotTree checkArrayWithoutIndex "read -rpfoobar r; r=42" checkArrayWithoutIndex params _ = - doVariableFlowAnalysis readF writeF defaultMap (variableFlow params) + doVariableFlowAnalysis readF writeF defaultSet (variableFlow params) where - defaultMap = Map.fromList $ map (\x -> (x,())) arrayVariables + defaultSet = S.fromList arrayVariables readF _ (T_DollarBraced id _ token) _ = do - map <- get + s <- get return . maybeToList $ do name <- getLiteralString token - assigned <- Map.lookup name map + guard $ S.member name s return $ makeComment WarningC id 2128 "Expanding an array without an index only gives the first element." readF _ _ _ = return [] writeF _ (T_Assignment id mode name [] _) _ (DataString _) = do - isArray <- gets (Map.member name) + isArray <- gets (S.member name) return $ if not isArray then [] else case mode of Assign -> [makeComment WarningC id 2178 "Variable was used as an array but is now assigned a string."] Append -> [makeComment WarningC id 2179 "Use array+=(\"item\") to append items to an array."] writeF _ t name (DataArray _) = do - modify (Map.insert name ()) + modify (S.insert name) return [] writeF _ expr name _ = do if isIndexed expr - then modify (Map.insert name ()) - else modify (Map.delete name) + then modify (S.insert name) + else modify (S.delete name) return [] isIndexed expr = @@ -1096,8 +1111,11 @@ checkSingleQuotedVariables params t@(T_S ,"xprop" ,"alias" ,"sudo" -- covering "sudo sh" and such + ,"doas" -- same as sudo + ,"run0" -- same as sudo ,"docker" -- like above ,"podman" + ,"oc" ,"dpkg-query" ,"jq" -- could also check that user provides --arg ,"rename" @@ -1523,6 +1541,7 @@ prop_checkComparisonAgainstGlob3 = verif prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow = foo ]" prop_checkComparisonAgainstGlob5 = verify checkComparisonAgainstGlob "[[ $cow != $bar ]]" prop_checkComparisonAgainstGlob6 = verify checkComparisonAgainstGlob "[ $f != /* ]" +prop_checkComparisonAgainstGlob7 = verify checkComparisonAgainstGlob "#!/bin/busybox sh\n[[ $f == *foo* ]]" checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _ _])) | op `elem` ["=", "==", "!="] = warn id 2053 $ "Quote the right-hand side of " ++ op ++ " in [[ ]] to prevent glob matching." @@ -1530,10 +1549,14 @@ checkComparisonAgainstGlob params (TC_Bi | op `elem` ["=", "==", "!="] && isGlob word = err (getId word) 2081 msg where - msg = if isBashLike params + msg = if (shellType params) `elem` [Bash, Ksh] -- Busybox does not support glob matching then "[ .. ] can't match globs. Use [[ .. ]] or case statement." else "[ .. ] can't match globs. Use a case statement." +checkComparisonAgainstGlob params (TC_Binary _ DoubleBracket op _ word) + | shellType params == BusyboxSh && op `elem` ["=", "==", "!="] && isGlob word = + err (getId word) 2330 "BusyBox [[ .. ]] does not support glob matching. Use a case statement." + checkComparisonAgainstGlob _ _ = return () prop_checkCaseAgainstGlob1 = verify checkCaseAgainstGlob "case foo in lol$n) foo;; esac" @@ -1620,6 +1643,64 @@ checkOrNeq _ (T_OrIf id lhs rhs) = seque checkOrNeq _ _ = return () +prop_checkAndEq1 = verifyNot checkAndEq "cow=0; foo=0; if [[ $lol -eq cow && $lol -eq foo ]]; then echo foo; fi" +prop_checkAndEq2 = verifyNot checkAndEq "lol=0 foo=0; (( a==lol && a==foo ))" +prop_checkAndEq3 = verify checkAndEq "[ \"$a\" = lol && \"$a\" = foo ]" +prop_checkAndEq4 = verifyNot checkAndEq "[ a = $cow && b = $foo ]" +prop_checkAndEq5 = verifyNot checkAndEq "[[ $a = /home && $a = */public_html/* ]]" +prop_checkAndEq6 = verify checkAndEq "[ $a = a ] && [ $a = b ]" +prop_checkAndEq7 = verify checkAndEq "[ $a = a ] && [ $a = b ] || true" +prop_checkAndEq8 = verifyNot checkAndEq "[[ $a == x && $a == x ]]" +prop_checkAndEq9 = verifyNot checkAndEq "[ 0 -eq $FOO ] && [ 0 -eq $BAR ]" +prop_checkAndEq10 = verify checkAndEq "(( a == 1 && a == 2 ))" +prop_checkAndEq11 = verify checkAndEq "[ $x -eq 1 ] && [ $x -eq 2 ]" +prop_checkAndEq12 = verify checkAndEq "[ 1 -eq $x ] && [ $x -eq 2 ]" +prop_checkAndEq13 = verifyNot checkAndEq "[ 1 -eq $x ] && [ $x -eq 1 ]" +prop_checkAndEq14 = verifyNot checkAndEq "[ $a = $b ] && [ $a = $c ]" + +checkAndEqOperands "-eq" rhs1 rhs2 = isLiteralNumber rhs1 && isLiteralNumber rhs2 +checkAndEqOperands op rhs1 rhs2 | op == "=" || op == "==" = isLiteral rhs1 && isLiteral rhs2 +checkAndEqOperands _ _ _ = False + +-- For test-level "and": [ x = y -a x = z ] +checkAndEq _ (TC_And id typ op (TC_Binary _ _ op1 lhs1 rhs1 ) (TC_Binary _ _ op2 lhs2 rhs2)) + | op1 == op2 && lhs1 == lhs2 && rhs1 /= rhs2 && checkAndEqOperands op1 rhs1 rhs2 = + warn id 2333 $ "You probably wanted " ++ (if typ == SingleBracket then "-o" else "||") ++ " here, otherwise it's always false." + +-- For arithmetic context "and" +checkAndEq _ (TA_Binary id "&&" (TA_Binary _ "==" lhs1 rhs1) (TA_Binary _ "==" lhs2 rhs2)) + | lhs1 == lhs2 && isLiteralNumber rhs1 && isLiteralNumber rhs2 = + warn id 2334 "You probably wanted || here, otherwise it's always false." + +-- For command level "and": [ x = y ] && [ x = z ] +checkAndEq _ (T_AndIf id lhs rhs) = sequence_ $ do + (lhs1, op1, rhs1) <- getExpr lhs + (lhs2, op2, rhs2) <- getExpr rhs + guard $ op1 == op2 + guard $ lhs1 == lhs2 && rhs1 /= rhs2 + guard $ checkAndEqOperands op1 rhs1 rhs2 + return $ warn id 2333 "You probably wanted || here, otherwise it's always false." + where + getExpr x = + case x of + T_AndIf _ lhs _ -> getExpr lhs -- Fetches x and y in `T_AndIf x (T_AndIf y z)` + T_Pipeline _ _ [x] -> getExpr x + T_Redirecting _ _ c -> getExpr c + T_Condition _ _ c -> getExpr c + TC_Binary _ _ op lhs rhs -> orient (lhs, op, rhs) + _ -> Nothing + + -- Swap items so that the constant side is rhs (or Nothing if both/neither is constant) + orient (lhs, op, rhs) = + case (isConstant lhs, isConstant rhs) of + (True, False) -> return (rhs, op, lhs) + (False, True) -> return (lhs, op, rhs) + _ -> Nothing + + +checkAndEq _ _ = return () + + prop_checkValidCondOps1 = verify checkValidCondOps "[[ a -xz b ]]" prop_checkValidCondOps2 = verify checkValidCondOps "[ -M a ]" prop_checkValidCondOps2a = verifyNot checkValidCondOps "[ 3 \\> 2 ]" @@ -1885,7 +1966,9 @@ prop_checkSpuriousExec8 = verifyNot chec prop_checkSpuriousExec9 = verify checkSpuriousExec "for file in rc.d/*; do exec \"$file\"; done" prop_checkSpuriousExec10 = verifyNot checkSpuriousExec "exec file; r=$?; printf >&2 'failed\n'; return $r" prop_checkSpuriousExec11 = verifyNot checkSpuriousExec "exec file; :" -checkSpuriousExec _ = doLists +prop_checkSpuriousExec12 = verifyNot checkSpuriousExec "#!/bin/bash\nshopt -s execfail; exec foo; exec bar; echo 'Error'; exit 1;" +prop_checkSpuriousExec13 = verify checkSpuriousExec "#!/bin/dash\nshopt -s execfail; exec foo; exec bar; echo 'Error'; exit 1;" +checkSpuriousExec params t = when (not $ hasExecfail params) $ doLists t where doLists (T_Script _ _ cmds) = doList cmds False doLists (T_BraceGroup _ cmds) = doList cmds False @@ -2255,7 +2338,7 @@ prop_checkFunctionsUsedExternally2c = prop_checkFunctionsUsedExternally3 = verifyNotTree checkFunctionsUsedExternally "f() { :; }; echo f" prop_checkFunctionsUsedExternally4 = - verifyNotTree checkFunctionsUsedExternally "foo() { :; }; sudo \"foo\"" + verifyNotTree checkFunctionsUsedExternally "foo() { :; }; run0 \"foo\"" prop_checkFunctionsUsedExternally5 = verifyTree checkFunctionsUsedExternally "foo() { :; }; ssh host foo" prop_checkFunctionsUsedExternally6 = @@ -2265,7 +2348,7 @@ prop_checkFunctionsUsedExternally7 = prop_checkFunctionsUsedExternally8 = verifyTree checkFunctionsUsedExternally "foo() { :; }; command sudo foo" prop_checkFunctionsUsedExternally9 = - verifyTree checkFunctionsUsedExternally "foo() { :; }; exec -c sudo foo" + verifyTree checkFunctionsUsedExternally "foo() { :; }; exec -c doas foo" checkFunctionsUsedExternally params t = runNodeAnalysis checkCommand params t where @@ -2289,6 +2372,8 @@ checkFunctionsUsedExternally params t = "chroot" -> firstNonFlag "screen" -> firstNonFlag "sudo" -> firstNonFlag + "doas" -> firstNonFlag + "run0" -> firstNonFlag "xargs" -> firstNonFlag "tmux" -> firstNonFlag "ssh" -> take 1 $ drop 1 $ dropFlags argAndString @@ -2373,15 +2458,9 @@ prop_checkUnused51 = verifyTree checkUnu checkUnusedAssignments params t = execWriter (mapM_ warnFor unused) where flow = variableFlow params - references = foldl (flip ($)) defaultMap (map insertRef flow) - insertRef (Reference (base, token, name)) = - Map.insert (stripSuffix name) () - insertRef _ = id - - assignments = foldl (flip ($)) Map.empty (map insertAssignment flow) - insertAssignment (Assignment (_, token, name, _)) | isVariableName name = - Map.insert name token - insertAssignment _ = id + references = Map.union (Map.fromList [(stripSuffix name, ()) | Reference (base, token, name) <- flow]) defaultMap + + assignments = Map.fromList [(name, token) | Assignment (_, token, name, _) <- flow, isVariableName name] unused = Map.assocs $ Map.difference assignments references @@ -2445,6 +2524,7 @@ prop_checkUnassignedReferences_minusZDef prop_checkUnassignedReferences50 = verifyNotTree checkUnassignedReferences "echo ${foo:+bar}" prop_checkUnassignedReferences51 = verifyNotTree checkUnassignedReferences "echo ${foo:+$foo}" prop_checkUnassignedReferences52 = verifyNotTree checkUnassignedReferences "wait -p pid; echo $pid" +prop_checkUnassignedReferences53 = verifyTree checkUnassignedReferences "x=($foo)" checkUnassignedReferences = checkUnassignedReferences' False checkUnassignedReferences' includeGlobals params t = warnings @@ -2500,14 +2580,12 @@ checkUnassignedReferences' includeGlobal warnings = execWriter . sequence $ mapMaybe warningFor unassigned - -- Due to parsing, foo=( [bar]=baz ) parses 'bar' as a reference even for assoc arrays. - -- Similarly, ${foo[bar baz]} may not be referencing bar/baz. Just skip these. + -- ${foo[bar baz]} may not be referencing bar/baz. Just skip these. -- We can also have ${foo:+$foo} should be treated like [[ -n $foo ]] && echo $foo isException var t = any shouldExclude $ getPath (parentMap params) t where shouldExclude t = case t of - T_Array {} -> True (T_DollarBraced _ _ l) -> let str = concat $ oversimplify l ref = getBracedReference str @@ -2818,6 +2896,10 @@ prop_checkUnpassedInFunctions11 = verify prop_checkUnpassedInFunctions12 = verifyNotTree checkUnpassedInFunctions "foo() { echo ${!var*}; }; foo;" prop_checkUnpassedInFunctions13 = verifyNotTree checkUnpassedInFunctions "# shellcheck disable=SC2120\nfoo() { echo $1; }\nfoo\n" prop_checkUnpassedInFunctions14 = verifyTree checkUnpassedInFunctions "foo() { echo $#; }; foo" +prop_checkUnpassedInFunctions15 = verifyNotTree checkUnpassedInFunctions "foo() { echo ${1-x}; }; foo" +prop_checkUnpassedInFunctions16 = verifyNotTree checkUnpassedInFunctions "foo() { echo ${1:-x}; }; foo" +prop_checkUnpassedInFunctions17 = verifyNotTree checkUnpassedInFunctions "foo() { mycommand ${1+--verbose}; }; foo" +prop_checkUnpassedInFunctions18 = verifyNotTree checkUnpassedInFunctions "foo() { if mycheck; then foo ${1?Missing}; fi; }; foo" checkUnpassedInFunctions params root = execWriter $ mapM_ warnForGroup referenceGroups where @@ -2834,9 +2916,10 @@ checkUnpassedInFunctions params root = case x of Assignment (_, _, str, _) -> isPositional str _ -> False + isPositionalReference function x = case x of - Reference (_, t, str) -> isPositional str && t `isDirectChildOf` function + Reference (_, t, str) -> isPositional str && t `isDirectChildOf` function && not (hasDefaultValue t) _ -> False isDirectChildOf child parent = fromMaybe False $ do @@ -2850,6 +2933,7 @@ checkUnpassedInFunctions params root = referenceList :: [(String, Bool, Token)] referenceList = execWriter $ doAnalysis (sequence_ . checkCommand) root + checkCommand :: Token -> Maybe (Writer [(String, Bool, Token)] ()) checkCommand t@(T_SimpleCommand _ _ (cmd:args)) = do str <- getLiteralString cmd @@ -2860,6 +2944,22 @@ checkUnpassedInFunctions params root = isPositional str = str == "*" || str == "@" || str == "#" || (all isDigit str && str /= "0" && str /= "") + -- True if t is a variable that specifies a default value, + -- such as ${1-x} or ${1:-x}. + hasDefaultValue t = + case t of + T_DollarBraced _ True l -> + let str = concat $ oversimplify l + in isDefaultValueModifier $ getBracedModifier str + _ -> False + + isDefaultValueModifier str = + case str of + ':':c:_ -> c `elem` handlesDefault + c:_ -> c `elem` handlesDefault + _ -> False + where handlesDefault = "-+?" + isArgumentless (_, b, _) = b referenceGroups = Map.elems $ foldr updateWith Map.empty referenceList updateWith x@(name, _, _) = Map.insertWith (++) name [x] @@ -2922,7 +3022,8 @@ checkTildeInPath _ _ = return () prop_checkUnsupported3 = verify checkUnsupported "#!/bin/sh\ncase foo in bar) baz ;& esac" prop_checkUnsupported4 = verify checkUnsupported "#!/bin/ksh\ncase foo in bar) baz ;;& esac" -prop_checkUnsupported5 = verify checkUnsupported "#!/bin/bash\necho \"${ ls; }\"" +prop_checkUnsupported5 = verifyNot checkUnsupported "#!/bin/bash\necho \"${ ls; }\"" +prop_checkUnsupported6 = verify checkUnsupported "#!/bin/ash\necho \"${ ls; }\"" checkUnsupported params t = unless (null support || (shellType params `elem` support)) $ report name @@ -2936,7 +3037,7 @@ checkUnsupported params t = shellSupport t = case t of T_CaseExpression _ _ list -> forCase (map (\(a,_,_) -> a) list) - T_DollarBraceCommandExpansion {} -> ("${ ..; } command expansion", [Ksh]) + T_DollarBraceCommandExpansion {} -> ("${ ..; } command expansion", [Bash, Ksh]) _ -> ("", []) where forCase seps | CaseContinue `elem` seps = ("cases with ;;&", [Bash]) @@ -3024,7 +3125,7 @@ checkShouldUseGrepQ params t = T_DollarExpansion _ [x] -> getPipeline x T_Pipeline _ _ cmds -> return cmds _ -> fail "unknown" - isGrep = (`elem` ["grep", "egrep", "fgrep", "zgrep"]) + isGrep = (`elem` ["grep", "egrep", "fgrep", "bz3grep", "bzgrep", "xzgrep", "zgrep", "zipgrep", "zstdgrep"]) prop_checkTestArgumentSplitting1 = verify checkTestArgumentSplitting "[ -e *.mp3 ]" prop_checkTestArgumentSplitting2 = verifyNot checkTestArgumentSplitting "[[ $a == *b* ]]" @@ -3297,7 +3398,7 @@ checkReturnAgainstZero params token = next@(TA_Unary _ "!" _):_ -> isOnlyTestInCommand next next@(TC_Group {}):_ -> isOnlyTestInCommand next next@(TA_Sequence _ [_]):_ -> isOnlyTestInCommand next - next@(TA_Parentesis _ _):_ -> isOnlyTestInCommand next + next@(TA_Parenthesis _ _):_ -> isOnlyTestInCommand next _ -> False -- TODO: Do better $? tracking and filter on whether @@ -3516,7 +3617,7 @@ checkSplittingInArrays params t = _ -> return () checkPart part = case part of T_DollarExpansion id _ -> forCommand id - T_DollarBraceCommandExpansion id _ -> forCommand id + T_DollarBraceCommandExpansion id _ _ -> forCommand id T_Backticked id _ -> forCommand id T_DollarBraced id _ str | not (isCountingReference part) @@ -3592,6 +3693,8 @@ prop_checkPipeToNowhere17 = verify check prop_checkPipeToNowhere18 = verifyNot checkPipeToNowhere "ls 1>&3 3>&1 3>&- | wc -l" prop_checkPipeToNowhere19 = verifyNot checkPipeToNowhere "find . -print0 | du --files0-from=/dev/stdin" prop_checkPipeToNowhere20 = verifyNot checkPipeToNowhere "find . | du --exclude-from=/dev/fd/0" +prop_checkPipeToNowhere21 = verifyNot checkPipeToNowhere "yes | cp -ri foo/* bar" +prop_checkPipeToNowhere22 = verifyNot checkPipeToNowhere "yes | rm --interactive *" data PipeType = StdoutPipe | StdoutStderrPipe | NoPipe deriving (Eq) checkPipeToNowhere :: Parameters -> Token -> WriterT [TokenComment] Identity () @@ -3657,6 +3760,7 @@ checkPipeToNowhere params t = commandSpecificException name cmd = case name of "du" -> any ((`elem` ["exclude-from", "files0-from"]) . snd) $ getAllFlags cmd + _ | name `elem` interactiveFlagCmds -> hasInteractiveFlag cmd _ -> False warnAboutDupes (n, list@(_:_:_)) = @@ -3680,7 +3784,7 @@ checkPipeToNowhere params t = name <- getCommandBasename cmd guard $ name `elem` nonReadingCommands guard . not $ hasAdditionalConsumers cmd - guard . not $ name `elem` ["cp", "mv", "rm"] && cmd `hasFlag` "i" + guard . not $ name `elem` interactiveFlagCmds && hasInteractiveFlag cmd let suggestion = if name == "echo" then "Did you want 'cat' instead?" @@ -3695,6 +3799,9 @@ checkPipeToNowhere params t = treeContains pred t = isNothing $ doAnalysis (guard . not . pred) t + interactiveFlagCmds = [ "cp", "mv", "rm" ] + hasInteractiveFlag cmd = cmd `hasFlag` "i" || cmd `hasFlag` "interactive" + mayConsume t = case t of T_ProcSub _ "<" _ -> True @@ -3761,32 +3868,32 @@ prop_checkUseBeforeDefinition1 = verifyT prop_checkUseBeforeDefinition2 = verifyNotTree checkUseBeforeDefinition "f() { true; }; f" prop_checkUseBeforeDefinition3 = verifyNotTree checkUseBeforeDefinition "if ! mycmd --version; then mycmd() { true; }; fi" prop_checkUseBeforeDefinition4 = verifyNotTree checkUseBeforeDefinition "mycmd || mycmd() { f; }" -checkUseBeforeDefinition _ t = - execWriter $ evalStateT (mapM_ examine $ revCommands) Map.empty +prop_checkUseBeforeDefinition5 = verifyTree checkUseBeforeDefinition "false || mycmd; mycmd() { f; }" +prop_checkUseBeforeDefinition6 = verifyNotTree checkUseBeforeDefinition "f() { one; }; f; f() { two; }; f" +checkUseBeforeDefinition :: Parameters -> Token -> [TokenComment] +checkUseBeforeDefinition params t = fromMaybe [] $ do + cfga <- cfgAnalysis params + let funcs = execState (doAnalysis findFunction t) Map.empty + -- Green cut: no point enumerating commands if there are no functions. + guard . not $ Map.null funcs + return $ execWriter $ doAnalysis (findInvocation cfga funcs) t where - examine t = case t of - T_Pipeline _ _ [T_Redirecting _ _ (T_Function _ _ _ name _)] -> - modify $ Map.insert name t - T_Annotation _ _ w -> examine w - T_Pipeline _ _ cmds -> do - m <- get - unless (Map.null m) $ - mapM_ (checkUsage m) $ concatMap recursiveSequences cmds - _ -> return () - - checkUsage map cmd = sequence_ $ do - name <- getCommandName cmd - def <- Map.lookup name map - return $ - err (getId cmd) 2218 - "This function is only defined later. Move the definition up." + findFunction t = + case t of + T_Function id _ _ name _ -> modify (Map.insertWith (++) name [id]) + _ -> return () - revCommands = reverse $ concat $ getCommandSequences t - recursiveSequences x = - let list = concat $ getCommandSequences x in - if null list - then [x] - else concatMap recursiveSequences list + findInvocation cfga funcs t = + case t of + T_SimpleCommand id _ (cmd:_) -> sequence_ $ do + name <- getLiteralString cmd + invocations <- Map.lookup name funcs + -- Is the function definitely being defined later? + guard $ any (\c -> CF.doesPostDominate cfga c id) invocations + -- Was one already defined, so it's actually a re-definition? + guard . not $ any (\c -> CF.doesPostDominate cfga id c) invocations + return $ err id 2218 "This function is only defined later. Move the definition up." + _ -> return () prop_checkForLoopGlobVariables1 = verify checkForLoopGlobVariables "for i in $var/*.txt; do true; done" prop_checkForLoopGlobVariables2 = verifyNot checkForLoopGlobVariables "for i in \"$var\"/*.txt; do true; done" @@ -3890,12 +3997,17 @@ checkSubshelledTests params t = T_Annotation {} -> True _ -> False -prop_checkInvertedStringTest1 = verify checkInvertedStringTest "[ ! -z $var ]" -prop_checkInvertedStringTest2 = verify checkInvertedStringTest "! [[ -n $var ]]" -prop_checkInvertedStringTest3 = verifyNot checkInvertedStringTest "! [ -x $var ]" -prop_checkInvertedStringTest4 = verifyNot checkInvertedStringTest "[[ ! -w $var ]]" -prop_checkInvertedStringTest5 = verifyNot checkInvertedStringTest "[ -z $var ]" -checkInvertedStringTest _ t = +prop_checkUnnecessarilyInvertedTest1 = verify checkUnnecessarilyInvertedTest "[ ! -z $var ]" +prop_checkUnnecessarilyInvertedTest2 = verify checkUnnecessarilyInvertedTest "! [[ -n $var ]]" +prop_checkUnnecessarilyInvertedTest3 = verifyNot checkUnnecessarilyInvertedTest "! [ -x $var ]" +prop_checkUnnecessarilyInvertedTest4 = verifyNot checkUnnecessarilyInvertedTest "[[ ! -w $var ]]" +prop_checkUnnecessarilyInvertedTest5 = verifyNot checkUnnecessarilyInvertedTest "[ -z $var ]" +prop_checkUnnecessarilyInvertedTest6 = verify checkUnnecessarilyInvertedTest "! [ $var != foo ]" +prop_checkUnnecessarilyInvertedTest7 = verify checkUnnecessarilyInvertedTest "[[ ! $var == foo ]]" +prop_checkUnnecessarilyInvertedTest8 = verifyNot checkUnnecessarilyInvertedTest "! [[ $var =~ .* ]]" +prop_checkUnnecessarilyInvertedTest9 = verify checkUnnecessarilyInvertedTest "[ ! $var -eq 0 ]" +prop_checkUnnecessarilyInvertedTest10 = verify checkUnnecessarilyInvertedTest "! [[ $var -gt 3 ]]" +checkUnnecessarilyInvertedTest _ t = case t of TC_Unary _ _ "!" (TC_Unary _ _ op _) -> case op of @@ -3908,7 +4020,34 @@ checkInvertedStringTest _ t = "-n" -> style (getId t) 2237 "Use [ -z .. ] instead of ! [ -n .. ]." "-z" -> style (getId t) 2237 "Use [ -n .. ] instead of ! [ -z .. ]." _ -> return () + TC_Unary _ _ "!" (TC_Binary _ bracketStyle op _ _) -> + maybeSuggestRewrite True bracketStyle (getId t) op + T_Banged _ (T_Pipeline _ _ + [T_Redirecting _ _ (T_Condition _ _ (TC_Binary _ bracketStyle op _ _))]) -> + maybeSuggestRewrite False bracketStyle (getId t) op _ -> return () + where + inversionMap = Map.fromList [ + ("=", "!="), + ("==", "!="), + ("!=", "="), + ("-eq", "-ne"), + ("-ne", "-eq"), + ("-le", "-gt"), + ("-gt", "-le"), + ("-ge", "-lt"), + ("-lt", "-ge") + ] + maybeSuggestRewrite bangInside bracketStyle id op = sequence_ $ do + newOp <- Map.lookup op inversionMap + let oldExpr = "a " ++ op ++ " b" + let newExpr = "a " ++ newOp ++ " b" + let bracket s = if bracketStyle == SingleBracket then "[ " ++ s ++ " ]" else "[[ " ++ s ++ " ]]" + return $ + if bangInside + then style id 2335 $ "Use " ++ newExpr ++ " instead of ! " ++ oldExpr ++ "." + else style id 2335 $ "Use " ++ (bracket newExpr) ++ " instead of ! " ++ (bracket oldExpr) ++ "." + prop_checkRedirectionToCommand1 = verify checkRedirectionToCommand "ls > rm" prop_checkRedirectionToCommand2 = verifyNot checkRedirectionToCommand "ls > 'rm'" @@ -3962,13 +4101,10 @@ prop_checkTranslatedStringVariable4 = ve prop_checkTranslatedStringVariable5 = verifyNot checkTranslatedStringVariable "foo=var; bar=val2; $\"foo bar\"" checkTranslatedStringVariable params (T_DollarDoubleQuoted id [T_Literal _ s]) | all isVariableChar s - && Map.member s assignments + && S.member s assignments = warnWithFix id 2256 "This translated string is the name of a variable. Flip leading $ and \" if this should be a quoted substitution." (fix id) where - assignments = foldl (flip ($)) Map.empty (map insertAssignment $ variableFlow params) - insertAssignment (Assignment (_, token, name, _)) | isVariableName name = - Map.insert name token - insertAssignment _ = Prelude.id + assignments = S.fromList [name | Assignment (_, _, name, _) <- variableFlow params, isVariableName name] fix id = fixWith [replaceStart id params 2 "\"$"] checkTranslatedStringVariable _ _ = return () @@ -3998,6 +4134,7 @@ prop_checkUselessBang6 = verify checkUse prop_checkUselessBang7 = verifyNot checkUselessBang "set -e; x() { ! [ x ]; }" prop_checkUselessBang8 = verifyNot checkUselessBang "set -e; if { ! true; }; then true; fi" prop_checkUselessBang9 = verifyNot checkUselessBang "set -e; while ! true; do true; done" +prop_checkUselessBang10 = verify checkUselessBang "set -e\nshellcheck disable=SC0000\n! true\nrest" checkUselessBang params t = when (hasSetE params) $ mapM_ check (getNonReturningCommands t) where check t = @@ -4006,6 +4143,7 @@ checkUselessBang params t = when (hasSet addComment $ makeCommentWithFix InfoC id 2251 "This ! is not on a condition and skips errexit. Use `&& exit 1` instead, or make sure $? is checked." (fixWith [replaceStart id params 1 "", replaceEnd (getId cmd) params 0 " && exit 1"]) + T_Annotation _ _ t -> check t _ -> return () -- Get all the subcommands that aren't likely to be the return value @@ -4196,7 +4334,7 @@ checkBadTestAndOr params t = in mapM_ checkTest commandWithSeps checkTest (before, cmd, after) = - when (isTest cmd) $ do + when (isTestCommand cmd) $ do checkPipe before checkPipe after @@ -4212,17 +4350,10 @@ checkBadTestAndOr params t = T_AndIf _ _ rhs -> checkAnds id rhs T_OrIf _ _ rhs -> checkAnds id rhs T_Pipeline _ _ list | not (null list) -> checkAnds id (last list) - cmd -> when (isTest cmd) $ + cmd -> when (isTestCommand cmd) $ errWithFix id 2265 "Use && for logical AND. Single & will background and return true." $ (fixWith [replaceEnd id params 0 "&"]) - isTest t = - case t of - T_Condition {} -> True - T_SimpleCommand {} -> t `isCommand` "test" - T_Redirecting _ _ t -> isTest t - T_Annotation _ _ t -> isTest t - _ -> False prop_checkComparisonWithLeadingX1 = verify checkComparisonWithLeadingX "[ x$foo = xlol ]" prop_checkComparisonWithLeadingX2 = verify checkComparisonWithLeadingX "test x$foo = xlol" @@ -4230,14 +4361,16 @@ prop_checkComparisonWithLeadingX3 = veri prop_checkComparisonWithLeadingX4 = verifyNot checkComparisonWithLeadingX "test $foo = xbar" prop_checkComparisonWithLeadingX5 = verify checkComparisonWithLeadingX "[ \"x$foo\" = 'xlol' ]" prop_checkComparisonWithLeadingX6 = verify checkComparisonWithLeadingX "[ x\"$foo\" = x'lol' ]" +prop_checkComparisonWithLeadingX7 = verify checkComparisonWithLeadingX "[ X$foo != Xbar ]" checkComparisonWithLeadingX params t = case t of - TC_Binary id typ op lhs rhs | op == "=" || op == "==" -> - check lhs rhs - T_SimpleCommand _ _ [cmd, lhs, op, rhs] | - getLiteralString cmd == Just "test" && - getLiteralString op `elem` [Just "=", Just "=="] -> - check lhs rhs + TC_Binary id typ op lhs rhs + | op `elem` ["=", "==", "!="] -> + check lhs rhs + T_SimpleCommand _ _ [cmd, lhs, op, rhs] + | getLiteralString cmd == Just "test" && + getLiteralString op `elem` [Just "=", Just "==", Just "!="] -> + check lhs rhs _ -> return () where msg = "Avoid x-prefix in comparisons as it no longer serves a purpose." @@ -4247,19 +4380,20 @@ checkComparisonWithLeadingX params t = return $ styleWithFix (getId lhs) 2268 msg $ fixWith [l, r] fixLeadingX token = - case getWordParts token of - T_Literal id ('x':_):_ -> + case getWordParts token of + T_Literal id (c:_):_ | toLower c == 'x' -> case token of - -- The side is a single, unquoted x, so we have to quote - T_NormalWord _ [T_Literal id "x"] -> + -- The side is a single, unquoted x or X, so we have to quote + T_NormalWord _ [T_Literal id [c]] -> return $ replaceStart id params 1 "\"\"" -- Otherwise we can just delete it _ -> return $ replaceStart id params 1 "" - T_SingleQuoted id ('x':_):_ -> - -- Replace the single quote and x - return $ replaceStart id params 2 "'" + T_SingleQuoted id (c:rest):_ | toLower c == 'x' -> + -- Replace the single quote and the character x or X + return $ replaceStart id params 2 "'" _ -> Nothing + prop_checkAssignToSelf1 = verify checkAssignToSelf "x=$x" prop_checkAssignToSelf2 = verify checkAssignToSelf "x=${x}" prop_checkAssignToSelf3 = verify checkAssignToSelf "x=\"$x\"" @@ -4538,13 +4672,13 @@ prop_checkRequireDoubleBracket2 = verify prop_checkRequireDoubleBracket3 = verifyNotTree checkRequireDoubleBracket "#!/bin/sh\n[ -x foo ]" prop_checkRequireDoubleBracket4 = verifyNotTree checkRequireDoubleBracket "[[ -x foo ]]" checkRequireDoubleBracket params = - if isBashLike params + if (shellType params) `elem` [Bash, Ksh, BusyboxSh] then nodeChecksToTreeCheck [check] params else const [] where check _ t = case t of T_Condition id SingleBracket _ -> - styleWithFix id 2292 "Prefer [[ ]] over [ ] for tests in Bash/Ksh." (fixFor t) + styleWithFix id 2292 "Prefer [[ ]] over [ ] for tests in Bash/Ksh/Busybox." (fixFor t) _ -> return () fixFor t = fixWith $ @@ -4895,16 +5029,33 @@ checkBatsTestDoesNotUseNegation params t prop_checkCommandIsUnreachable1 = verify checkCommandIsUnreachable "foo; bar; exit; baz" prop_checkCommandIsUnreachable2 = verify checkCommandIsUnreachable "die() { exit; }; foo; bar; die; baz" prop_checkCommandIsUnreachable3 = verifyNot checkCommandIsUnreachable "foo; bar || exit; baz" +prop_checkCommandIsUnreachable4 = verifyNot checkCommandIsUnreachable "f() { foo; }; # Maybe sourced" +prop_checkCommandIsUnreachable5 = verify checkCommandIsUnreachable "f() { foo; }; exit # Not sourced" checkCommandIsUnreachable params t = case t of T_Pipeline {} -> sequence_ $ do cfga <- cfgAnalysis params - state <- CF.getIncomingState cfga id + state <- CF.getIncomingState cfga (getId t) guard . not $ CF.stateIsReachable state guard . not $ isSourced params t - return $ info id 2317 "Command appears to be unreachable. Check usage (or ignore if invoked indirectly)." + guard . not $ any (\t -> isUnreachable t || isUnreachableFunction t) $ NE.drop 1 $ getPath (parentMap params) t + return $ info (getId t) 2317 "Command appears to be unreachable. Check usage (or ignore if invoked indirectly)." + T_Function id _ _ _ _ -> + when (isUnreachableFunction t + && (not . any isUnreachableFunction . NE.drop 1 $ getPath (parentMap params) t) + && (not $ isSourced params t)) $ + info id 2329 "This function is never invoked. Check usage (or ignored if invoked indirectly)." _ -> return () - where id = getId t + where + isUnreachableFunction :: Token -> Bool + isUnreachableFunction f = + case f of + T_Function id _ _ _ t -> isUnreachable t + _ -> False + isUnreachable t = fromMaybe False $ do + cfga <- cfgAnalysis params + state <- CF.getIncomingState cfga (getId t) + return . not $ CF.stateIsReachable state prop_checkOverwrittenExitCode1 = verify checkOverwrittenExitCode "x; [ $? -eq 1 ] || [ $? -eq 2 ]" @@ -4984,14 +5135,14 @@ checkUnnecessaryParens params t = T_ForArithmetic _ x y z _ -> mapM_ (checkLeading "for (((x); (y); (z))) is the same as for ((x; y; z))") [x,y,z] T_Assignment _ _ _ [t] _ -> checkLeading "a[(x)] is the same as a[x]" t T_Arithmetic _ t -> checkLeading "(( (x) )) is the same as (( x ))" t - TA_Parentesis _ (TA_Sequence _ [ TA_Parentesis id _ ]) -> + TA_Parenthesis _ (TA_Sequence _ [ TA_Parenthesis id _ ]) -> styleWithFix id 2322 "In arithmetic contexts, ((x)) is the same as (x). Prefer only one layer of parentheses." $ fix id _ -> return () where checkLeading str t = case t of - TA_Sequence _ [TA_Parentesis id _ ] -> styleWithFix id 2323 (str ++ ". Prefer not wrapping in additional parentheses.") $ fix id + TA_Sequence _ [TA_Parenthesis id _ ] -> styleWithFix id 2323 (str ++ ". Prefer not wrapping in additional parentheses.") $ fix id _ -> return () fix id = @@ -5017,7 +5168,8 @@ checkPlusEqualsNumber params t = state <- CF.getIncomingState cfga id guard $ isNumber state word guard . not $ fromMaybe False $ CF.variableMayBeDeclaredInteger state var - return $ warn id 2324 "var+=1 will append, not increment. Use (( var += 1 )), declare -i var, or quote number to silence." + -- Recommend "typeset" because ksh does not have "declare". + return $ warn id 2324 "var+=1 will append, not increment. Use (( var += 1 )), typeset -i var, or quote number to silence." _ -> return () where @@ -5039,5 +5191,52 @@ checkPlusEqualsNumber params t = isUnquotedNumber || isNumericalVariableName || isNumericalVariableExpansion + +prop_checkExpansionWithRedirection1 = verify checkExpansionWithRedirection "var=$(foo > bar)" +prop_checkExpansionWithRedirection2 = verify checkExpansionWithRedirection "var=`foo 1> bar`" +prop_checkExpansionWithRedirection3 = verify checkExpansionWithRedirection "var=${ foo >> bar; }" +prop_checkExpansionWithRedirection4 = verify checkExpansionWithRedirection "var=$(foo | bar > baz)" +prop_checkExpansionWithRedirection5 = verifyNot checkExpansionWithRedirection "stderr=$(foo 2>&1 > /dev/null)" +prop_checkExpansionWithRedirection6 = verifyNot checkExpansionWithRedirection "var=$(foo; bar > baz)" +prop_checkExpansionWithRedirection7 = verifyNot checkExpansionWithRedirection "var=$(foo > bar; baz)" +prop_checkExpansionWithRedirection8 = verifyNot checkExpansionWithRedirection "var=$(cat <&3)" +checkExpansionWithRedirection params t = + case t of + T_DollarExpansion id [cmd] -> check id cmd + T_Backticked id [cmd] -> check id cmd + T_DollarBraceCommandExpansion id _ [cmd] -> check id cmd + _ -> return () + where + check id pipe = + case pipe of + (T_Pipeline _ _ t@(_:_)) -> checkCmd id (last t) + _ -> return () + + checkCmd captureId (T_Redirecting _ redirs _) = foldr (walk captureId) (return ()) redirs + + walk captureId t acc = + case t of + T_FdRedirect _ _ (T_IoDuplicate _ _ "1") -> return () + T_FdRedirect id "1" (T_IoDuplicate _ _ _) -> return () + T_FdRedirect id "" (T_IoDuplicate _ op _) | op `elem` [T_GREATAND (Id 0), T_Greater (Id 0)] -> emit id captureId True + T_FdRedirect id str (T_IoFile _ op file) | str `elem` ["", "1"] && op `elem` [ T_DGREAT (Id 0), T_Greater (Id 0) ] -> + emit id captureId $ getLiteralString file /= Just "/dev/null" + _ -> acc + + emit redirectId captureId suggestTee = do + warn captureId 2327 "This command substitution will be empty because the command's output gets redirected away." + err redirectId 2328 $ "This redirection takes output away from the command substitution" ++ if suggestTee then " (use tee to duplicate)." else "." + + +prop_checkUnaryTestA1 = verify checkUnaryTestA "[ -a foo ]" +prop_checkUnaryTestA2 = verify checkUnaryTestA "[ ! -a foo ]" +prop_checkUnaryTestA3 = verifyNot checkUnaryTestA "[ foo -a bar ]" +checkUnaryTestA params t = + case t of + TC_Unary id _ "-a" _ -> + styleWithFix id 2331 "For file existence, prefer standard -e over legacy -a." $ + fixWith [replaceStart id params 2 "-e"] + _ -> return () + return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff -pruN 0.10.0-1/src/ShellCheck/AnalyzerLib.hs 0.11.0-1/src/ShellCheck/AnalyzerLib.hs --- 0.10.0-1/src/ShellCheck/AnalyzerLib.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/AnalyzerLib.hs 2001-09-09 01:46:40.000000000 +0000 @@ -89,6 +89,8 @@ data Parameters = Parameters { hasSetE :: Bool, -- Whether this script has 'set -o pipefail' anywhere. hasPipefail :: Bool, + -- Whether this script has 'shopt -s execfail' anywhere. + hasExecfail :: Bool, -- A linear (bad) analysis of data flow variableFlow :: [StackData], -- A map from Id to Token @@ -226,6 +228,10 @@ makeParameters spec = params BusyboxSh -> isOptionSet "pipefail" root Sh -> True Ksh -> isOptionSet "pipefail" root, + hasExecfail = + case shellType params of + Bash -> isOptionSet "execfail" root + _ -> False, shellTypeSpecified = isJust (asShellType spec) || isJust (asFallbackShell spec), idMap = getTokenMap root, parentMap = getParentTree root, @@ -535,7 +541,9 @@ getModifiedVariables t = T_BatsTest {} -> [ (t, t, "lines", DataArray SourceExternal), (t, t, "status", DataString SourceInteger), - (t, t, "output", DataString SourceExternal) + (t, t, "output", DataString SourceExternal), + (t, t, "stderr", DataString SourceExternal), + (t, t, "stderr_lines", DataArray SourceExternal) ] -- Count [[ -v foo ]] as an "assignment". @@ -557,8 +565,12 @@ getModifiedVariables t = T_FdRedirect _ ('{':var) op -> -- {foo}>&2 modifies foo [(t, t, takeWhile (/= '}') var, DataString SourceInteger) | not $ isClosingFileOp op] - T_CoProc _ name _ -> - [(t, t, fromMaybe "COPROC" name, DataArray SourceInteger)] + T_CoProc _ Nothing _ -> + [(t, t, "COPROC", DataArray SourceInteger)] + + T_CoProc _ (Just token) _ -> do + name <- maybeToList $ getLiteralString token + [(t, t, name, DataArray SourceInteger)] --Points to 'for' rather than variable T_ForIn id str [] _ -> [(t, t, str, DataString SourceExternal)] @@ -902,16 +914,6 @@ supportsArrays Bash = True supportsArrays Ksh = True supportsArrays _ = False --- Returns true if the shell is Bash or Ksh (sorry for the name, Ksh) -isBashLike :: Parameters -> Bool -isBashLike params = - case shellType params of - Bash -> True - Ksh -> True - Dash -> False - BusyboxSh -> False - Sh -> False - isTrueAssignmentSource c = case c of DataString SourceChecked -> False @@ -929,6 +931,14 @@ modifiesVariable params token name = Assignment (_, _, n, source) -> isTrueAssignmentSource source && n == name _ -> False +isTestCommand t = + case t of + T_Condition {} -> True + T_SimpleCommand {} -> t `isCommand` "test" + T_Redirecting _ _ t -> isTestCommand t + T_Annotation _ _ t -> isTestCommand t + T_Pipeline _ _ [t] -> isTestCommand t + _ -> False return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff -pruN 0.10.0-1/src/ShellCheck/CFG.hs 0.11.0-1/src/ShellCheck/CFG.hs --- 0.10.0-1/src/ShellCheck/CFG.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/CFG.hs 2001-09-09 01:46:40.000000000 +0000 @@ -295,19 +295,19 @@ removeUnnecessaryStructuralNodes (nodes, regularEdges = filter isRegularEdge edges inDegree = counter $ map (\(from,to,_) -> from) regularEdges outDegree = counter $ map (\(from,to,_) -> to) regularEdges - structuralNodes = S.fromList $ map fst $ filter isStructural nodes + structuralNodes = S.fromList [node | (node, CFStructuralNode) <- nodes] candidateNodes = S.filter isLinear structuralNodes edgesToCollapse = S.fromList $ filter filterEdges regularEdges remapping :: M.Map Node Node - remapping = foldl' (\m (new, old) -> M.insert old new m) M.empty $ map orderEdge $ S.toList edgesToCollapse - recursiveRemapping = M.fromList $ map (\c -> (c, recursiveLookup remapping c)) $ M.keys remapping + remapping = M.fromList $ map orderEdge $ S.toList edgesToCollapse + recursiveRemapping = M.mapWithKey (\c _ -> recursiveLookup remapping c) remapping filterEdges (a,b,_) = a `S.member` candidateNodes && b `S.member` candidateNodes - orderEdge (a,b,_) = if a < b then (a,b) else (b,a) - counter = foldl' (\map key -> M.insertWith (+) key 1 map) M.empty + orderEdge (a,b,_) = if a < b then (b,a) else (a,b) + counter = M.fromListWith (+) . map (\key -> (key, 1)) isRegularEdge (_, _, CFEFlow) = True isRegularEdge _ = False @@ -317,11 +317,6 @@ removeUnnecessaryStructuralNodes (nodes, Nothing -> node Just x -> recursiveLookup map x - isStructural (node, label) = - case label of - CFStructuralNode -> True - _ -> False - isLinear node = M.findWithDefault 0 node inDegree == 1 && M.findWithDefault 0 node outDegree == 1 @@ -495,7 +490,7 @@ build t = do TA_Binary _ _ a b -> sequentially [a,b] TA_Expansion _ list -> sequentially list TA_Sequence _ list -> sequentially list - TA_Parentesis _ t -> build t + TA_Parenthesis _ t -> build t TA_Trinary _ cond a b -> do condition <- build cond @@ -673,10 +668,18 @@ build t = do status <- newNodeRange $ CFSetExitCode id linkRange cond status - T_CoProc id maybeName t -> do - let name = fromMaybe "COPROC" maybeName + T_CoProc id maybeNameToken t -> do + -- If unspecified, "COPROC". If not a constant string, Nothing. + let maybeName = case maybeNameToken of + Just x -> getLiteralString x + Nothing -> Just "COPROC" + + let parentNode = case maybeName of + Just str -> applySingle $ IdTagged id $ CFWriteVariable str CFValueArray + Nothing -> CFStructuralNode + start <- newStructuralNode - parent <- newNodeRange $ applySingle $ IdTagged id $ CFWriteVariable name CFValueArray + parent <- newNodeRange parentNode child <- subshell id "coproc" $ build t end <- newNodeRange $ CFSetExitCode id @@ -713,6 +716,9 @@ build t = do linkRange totalRead result else return totalRead + T_DollarBraceCommandExpansion id _ body -> + sequentially body + T_DoubleQuoted _ list -> sequentially list T_DollarExpansion id body -> diff -pruN 0.10.0-1/src/ShellCheck/CFGAnalysis.hs 0.11.0-1/src/ShellCheck/CFGAnalysis.hs --- 0.10.0-1/src/ShellCheck/CFGAnalysis.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/CFGAnalysis.hs 2001-09-09 01:46:40.000000000 +0000 @@ -133,7 +133,7 @@ internalToExternal s = literalValue = Nothing } } - flatVars = M.unionsWith (\_ last -> last) $ map mapStorage [sGlobalValues s, sLocalValues s, sPrefixValues s] + flatVars = M.unions $ map mapStorage [sPrefixValues s, sLocalValues s, sGlobalValues s] -- Conveniently get the state before a token id getIncomingState :: CFGAnalysis -> Id -> Maybe ProgramState @@ -672,7 +672,7 @@ vmPatch base diff = _ | vmIsQuickEqual base diff -> diff _ -> VersionedMap { mapVersion = -1, - mapStorage = M.unionWith (flip const) (mapStorage base) (mapStorage diff) + mapStorage = M.union (mapStorage diff) (mapStorage base) } -- Set a variable. This includes properties. Applies it to the appropriate scope. @@ -1286,7 +1286,7 @@ dataflow ctx entry = do else do let (next, rest) = S.deleteFindMin ps nexts <- process states next - writeSTRef pending $ foldl (flip S.insert) rest nexts + writeSTRef pending $ S.union (S.fromList nexts) rest f (n-1) pending states process states node = do @@ -1350,7 +1350,7 @@ analyzeControlFlow params t = -- All nodes we've touched invocations <- readSTRef $ cInvocations ctx - let invokedNodes = M.fromDistinctAscList $ map (\c -> (c, ())) $ S.toList $ M.keysSet $ groupByNode $ M.map snd invocations + let invokedNodes = M.fromSet (const ()) $ S.unions $ map (M.keysSet . snd) $ M.elems invocations -- Invoke all functions that were declared but not invoked -- This is so that we still get warnings for dead code @@ -1373,7 +1373,7 @@ analyzeControlFlow params t = -- Fill in the map with unreachable states for anything we didn't get to let baseStates = M.fromDistinctAscList $ map (\c -> (c, (unreachableState, unreachableState))) $ uncurry enumFromTo $ nodeRange $ cfGraph cfg - let allStates = M.unionWith (flip const) baseStates invokedStates + let allStates = M.union invokedStates baseStates -- Convert to external states let nodeToData = M.map (\(a,b) -> (internalToExternal a, internalToExternal b)) allStates diff -pruN 0.10.0-1/src/ShellCheck/Checker.hs 0.11.0-1/src/ShellCheck/Checker.hs --- 0.10.0-1/src/ShellCheck/Checker.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Checker.hs 2001-09-09 01:46:40.000000000 +0000 @@ -221,6 +221,9 @@ prop_worksWhenSourcing = prop_worksWhenSourcingWithDashDash = null $ checkWithIncludes [("lib", "bar=1")] "source -- lib; echo \"$bar\"" +prop_worksWhenSourcingWithDashP = + null $ checkWithIncludes [("lib", "bar=1")] "source -p \"$MYPATH\" lib; echo \"$bar\"" + prop_worksWhenDotting = null $ checkWithIncludes [("lib", "bar=1")] ". lib; echo \"$bar\"" diff -pruN 0.10.0-1/src/ShellCheck/Checks/Commands.hs 0.11.0-1/src/ShellCheck/Checks/Commands.hs --- 0.10.0-1/src/ShellCheck/Checks/Commands.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Checks/Commands.hs 2001-09-09 01:46:40.000000000 +0000 @@ -725,6 +725,9 @@ prop_checkGetPrintfFormats4 = getPrintfF prop_checkGetPrintfFormats5 = getPrintfFormats "%bPassed: %d, %bFailed: %d%b, Skipped: %d, %bErrored: %d%b\\n" == "bdbdbdbdb" prop_checkGetPrintfFormats6 = getPrintfFormats "%s%s" == "ss" prop_checkGetPrintfFormats7 = getPrintfFormats "%s\n%s" == "ss" +prop_checkGetPrintfFormats8 = getPrintfFormats "%ld" == "d" +prop_checkGetPrintfFormats9 = getPrintfFormats "%lld" == "d" +prop_checkGetPrintfFormats10 = getPrintfFormats "%Q" == "Q" getPrintfFormats = getFormats where -- Get the arguments in the string as a string of type characters, @@ -743,17 +746,17 @@ getPrintfFormats = getFormats regexBasedGetFormats rest = case matchRegex re rest of - Just [width, precision, typ, rest, _] -> + Just [width, precision, len, typ, rest, _] -> (if width == "*" then "*" else "") ++ (if precision == "*" then "*" else "") ++ typ ++ getFormats rest Nothing -> take 1 rest ++ getFormats rest where -- constructed based on specifications in "man printf" - re = mkRegex "#?-?\\+? ?0?(\\*|\\d*)\\.?(\\d*|\\*)([diouxXfFeEgGaAcsbq])((\n|.)*)" - -- \____ _____/\___ ____/ \____ ____/\_________ _________/ \______ / - -- V V V V V - -- flags field width precision format character rest + re = mkRegex "^#?-?\\+? ?0?(\\*|\\d*)\\.?(\\d*|\\*)(hh|h|l|ll|q|L|j|z|Z|t)?([diouxXfFeEgGaAcsbqQSC])((\n|.)*)" + -- \____ _____/\___ ____/ \____ ____/\__________ ___________/\___________ ___________/\___ ___/ + -- V V V V V V + -- flags field width precision length modifier format character rest -- field width and precision can be specified with an '*' instead of a digit, -- in which case printf will accept one more argument for each '*' used @@ -1241,7 +1244,7 @@ checkSudoArgs = CommandCheck (Basename " command <- getLiteralString commandArg guard $ command `elem` builtins return $ warn (getId t) 2232 $ "Can't use sudo with builtins like " ++ command ++ ". Did you want sudo sh -c .. instead?" - builtins = [ "cd", "eval", "export", "history", "read", "source", "wait" ] + builtins = [ "cd", "command", "declare", "eval", "exec", "exit", "export", "hash", "history", "local", "popd", "pushd", "read", "readonly", "return", "set", "source", "trap", "type", "typeset", "ulimit", "umask", "unset", "wait" ] -- This mess is why ShellCheck prefers not to know. parseOpts = getBsdOpts "vAknSbEHPa:g:h:p:u:c:T:r:" @@ -1431,9 +1434,8 @@ prop_checkBackreferencingDeclaration7 = checkBackreferencingDeclaration cmd = CommandCheck (Exactly cmd) check where check t = do - cfga <- asks cfgAnalysis - when (isJust cfga) $ - foldM_ (perArg $ fromJust cfga) M.empty $ arguments t + maybeCfga <- asks cfgAnalysis + mapM_ (\cfga -> foldM_ (perArg cfga) M.empty $ arguments t) maybeCfga perArg cfga leftArgs t = case t of diff -pruN 0.10.0-1/src/ShellCheck/Checks/ControlFlow.hs 0.11.0-1/src/ShellCheck/Checks/ControlFlow.hs --- 0.10.0-1/src/ShellCheck/Checks/ControlFlow.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Checks/ControlFlow.hs 2001-09-09 01:46:40.000000000 +0000 @@ -78,7 +78,7 @@ controlFlowEffectChecks = [ runNodeChecks :: [ControlFlowNodeCheck] -> ControlFlowCheck runNodeChecks perNode = do cfg <- asks cfgAnalysis - sequence_ $ runOnAll <$> cfg + mapM_ runOnAll cfg where getData datas n@(node, label) = do (pre, post) <- M.lookup node datas diff -pruN 0.10.0-1/src/ShellCheck/Checks/Custom.hs 0.11.0-1/src/ShellCheck/Checks/Custom.hs --- 0.10.0-1/src/ShellCheck/Checks/Custom.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Checks/Custom.hs 2001-09-09 01:46:40.000000000 +0000 @@ -1,7 +1,7 @@ {- This empty file is provided for ease of patching in site specific checks. However, there are no guarantees regarding compatibility between versions. --} +-} {-# LANGUAGE TemplateHaskell #-} module ShellCheck.Checks.Custom (checker, ShellCheck.Checks.Custom.runTests) where diff -pruN 0.10.0-1/src/ShellCheck/Checks/ShellSupport.hs 0.11.0-1/src/ShellCheck/Checks/ShellSupport.hs --- 0.10.0-1/src/ShellCheck/Checks/ShellSupport.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Checks/ShellSupport.hs 2001-09-09 01:46:40.000000000 +0000 @@ -63,6 +63,7 @@ checks = [ ,checkPS1Assignments ,checkMultipleBangs ,checkBangAfterPipe + ,checkNegatedUnaryOps ] testChecker (ForShell _ t) = @@ -86,7 +87,7 @@ checkForDecimals = ForShell [Sh, Dash, B prop_checkBashisms = verify checkBashisms "while read a; do :; done < <(a)" -prop_checkBashisms2 = verify checkBashisms "[ foo -nt bar ]" +prop_checkBashisms2 = verifyNot checkBashisms "[ foo -nt bar ]" prop_checkBashisms3 = verify checkBashisms "echo $((i++))" prop_checkBashisms4 = verify checkBashisms "rm !(*.hs)" prop_checkBashisms5 = verify checkBashisms "source file" @@ -212,6 +213,16 @@ prop_checkBashisms118 = verify checkBash prop_checkBashisms119 = verify checkBashisms "#!/bin/busybox sh\nx='test'\n${x^^[t]}" -- SC3059 prop_checkBashisms120 = verify checkBashisms "#!/bin/sh\n[ x == y ]" prop_checkBashisms121 = verifyNot checkBashisms "#!/bin/sh\n# shellcheck shell=busybox\n[ x == y ]" +prop_checkBashisms122 = verify checkBashisms "#!/bin/dash\n$'a'" +prop_checkBashisms123 = verifyNot checkBashisms "#!/bin/busybox sh\n$'a'" +prop_checkBashisms124 = verify checkBashisms "#!/bin/dash\ntype -p test" +prop_checkBashisms125 = verifyNot checkBashisms "#!/bin/busybox sh\ntype -p test" +prop_checkBashisms126 = verifyNot checkBashisms "#!/bin/busybox sh\nread -p foo -r bar" +prop_checkBashisms127 = verifyNot checkBashisms "#!/bin/busybox sh\necho -ne foo" +prop_checkBashisms128 = verify checkBashisms "#!/bin/dash\ntype -p test" +prop_checkBashisms129 = verify checkBashisms "#!/bin/sh\n[ -k /tmp ]" +prop_checkBashisms130 = verifyNot checkBashisms "#!/bin/dash\ntest -k /tmp" +prop_checkBashisms131 = verify checkBashisms "#!/bin/sh\n[ -o errexit ]" checkBashisms = ForShell [Sh, Dash, BusyboxSh] $ \t -> do params <- ask kludge params t @@ -229,7 +240,8 @@ checkBashisms = ForShell [Sh, Dash, Busy bashism (T_ProcSub id _ _) = warnMsg id 3001 "process substitution is" bashism (T_Extglob id _ _) = warnMsg id 3002 "extglob is" - bashism (T_DollarSingleQuoted id _) = warnMsg id 3003 "$'..' is" + bashism (T_DollarSingleQuoted id _) = + unless isBusyboxSh $ warnMsg id 3003 "$'..' is" bashism (T_DollarDoubleQuoted id _) = warnMsg id 3004 "$\"..\" is" bashism (T_ForArithmetic id _ _ _ _) = warnMsg id 3005 "arithmetic for loops are" bashism (T_Arithmetic id _) = warnMsg id 3006 "standalone ((..)) is" @@ -239,34 +251,16 @@ checkBashisms = ForShell [Sh, Dash, Busy bashism (T_Condition id DoubleBracket _) = unless isBusyboxSh $ warnMsg id 3010 "[[ ]] is" bashism (T_HereString id _) = warnMsg id 3011 "here-strings are" - bashism (TC_Binary id SingleBracket op _ _) - | op `elem` [ "<", ">", "\\<", "\\>", "<=", ">=", "\\<=", "\\>="] = - unless isDash $ warnMsg id 3012 $ "lexicographical " ++ op ++ " is" - bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just op, rhs]) - | op `elem` [ "<", ">", "\\<", "\\>", "<=", ">=", "\\<=", "\\>="] = - unless isDash $ warnMsg id 3012 $ "lexicographical " ++ op ++ " is" - bashism (TC_Binary id SingleBracket op _ _) - | op `elem` [ "-ot", "-nt", "-ef" ] = - unless isDash $ warnMsg id 3013 $ op ++ " is" - bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just op, rhs]) - | op `elem` [ "-ot", "-nt", "-ef" ] = - unless isDash $ warnMsg id 3013 $ op ++ " is" - bashism (TC_Binary id SingleBracket "==" _ _) = - unless isBusyboxSh $ warnMsg id 3014 "== in place of = is" - bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just "==", rhs]) = - unless isBusyboxSh $ warnMsg id 3014 "== in place of = is" - bashism (TC_Binary id SingleBracket "=~" _ _) = - warnMsg id 3015 "=~ regex matching is" - bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just "=~", rhs]) = - warnMsg id 3015 "=~ regex matching is" - bashism (TC_Unary id SingleBracket "-v" _) = - warnMsg id 3016 "unary -v (in place of [ -n \"${var+x}\" ]) is" - bashism (T_SimpleCommand id _ [asStr -> Just "test", asStr -> Just "-v", _]) = - warnMsg id 3016 "unary -v (in place of [ -n \"${var+x}\" ]) is" - bashism (TC_Unary id _ "-a" _) = - warnMsg id 3017 "unary -a in place of -e is" - bashism (T_SimpleCommand id _ [asStr -> Just "test", asStr -> Just "-a", _]) = - warnMsg id 3017 "unary -a in place of -e is" + + bashism (TC_Binary id _ op _ _) = + checkTestOp bashismBinaryTestFlags op id + bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just op, rhs]) = + checkTestOp bashismBinaryTestFlags op id + bashism (TC_Unary id _ op _) = + checkTestOp bashismUnaryTestFlags op id + bashism (T_SimpleCommand id _ [asStr -> Just "test", asStr -> Just op, _]) = + checkTestOp bashismUnaryTestFlags op id + bashism (TA_Unary id op _) | op `elem` [ "|++", "|--", "++|", "--|"] = warnMsg id 3018 $ filter (/= '|') op ++ " is" @@ -321,7 +315,11 @@ checkBashisms = ForShell [Sh, Dash, Busy bashism t@(T_SimpleCommand _ _ (cmd:arg:_)) | t `isCommand` "echo" && argString `matches` flagRegex = - if isDash + if isBusyboxSh + then + unless (argString `matches` busyboxFlagRegex) $ + warnMsg (getId arg) 3036 "echo flags besides -n and -e" + else if isDash then when (argString /= "-n") $ warnMsg (getId arg) 3036 "echo flags besides -n" @@ -330,6 +328,7 @@ checkBashisms = ForShell [Sh, Dash, Busy where argString = concat $ oversimplify arg flagRegex = mkRegex "^-[eEsn]+$" + busyboxFlagRegex = mkRegex "^-[en]+$" bashism t@(T_SimpleCommand _ _ (cmd:arg:_)) | getLiteralString cmd == Just "exec" && "-" `isPrefixOf` concat (oversimplify arg) = @@ -443,10 +442,10 @@ checkBashisms = ForShell [Sh, Dash, Busy ("hash", Just $ if isDash then ["r", "v"] else ["r"]), ("jobs", Just ["l", "p"]), ("printf", Just []), - ("read", Just $ if isDash then ["r", "p"] else ["r"]), + ("read", Just $ if isDash || isBusyboxSh then ["r", "p"] else ["r"]), ("readonly", Just ["p"]), ("trap", Just []), - ("type", Just []), + ("type", Just $ if isBusyboxSh then ["p"] else []), ("ulimit", if isDash then Nothing else Just ["f"]), ("umask", Just ["S"]), ("unset", Just ["f", "v"]), @@ -498,6 +497,50 @@ checkBashisms = ForShell [Sh, Dash, Busy Assignment (_, _, name, _) -> name == var _ -> False + checkTestOp table op id = sequence_ $ do + (code, shells, msg) <- Map.lookup op table + guard . not $ shellType params `elem` shells + return $ warnMsg id code (msg op) + + +buildTestFlagMap list = Map.fromList $ concatMap (\(x,y) -> map (\c -> (c,y)) x) list +bashismBinaryTestFlags = buildTestFlagMap [ + -- ([list of applicable flags], + -- (error code, exempt shells, message builder :: String -> String)), + -- + -- Distinct error codes allow the wiki to give more helpful, targeted + -- information. + (["<", ">", "\\<", "\\>", "<=", ">=", "\\<=", "\\>="], + (3012, [Dash, BusyboxSh], \op -> "lexicographical " ++ op ++ " is")), + (["=="], + (3014, [BusyboxSh], \op -> op ++ " in place of = is")), + (["=~"], + (3015, [], \op -> op ++ " regex matching is")), + + ([], (0,[],const "")) + ] +bashismUnaryTestFlags = buildTestFlagMap [ + (["-v"], + (3016, [], \op -> "test " ++ op ++ " (in place of [ -n \"${var+x}\" ]) is")), + (["-a"], + (3017, [], \op -> "unary " ++ op ++ " in place of -e is")), + (["-o"], + (3062, [], \op -> "test " ++ op ++ " to check options is")), + (["-R"], + (3063, [], \op -> "test " ++ op ++ " and namerefs in general are")), + (["-N"], + (3064, [], \op -> "test " ++ op ++ " is")), + (["-k"], + (3065, [Dash, BusyboxSh], \op -> "test " ++ op ++ " is")), + (["-G"], + (3066, [Dash, BusyboxSh], \op -> "test " ++ op ++ " is")), + (["-O"], + (3067, [Dash, BusyboxSh], \op -> "test " ++ op ++ " is")), + + ([], (0,[],const "")) + ] + + prop_checkEchoSed1 = verify checkEchoSed "FOO=$(echo \"$cow\" | sed 's/foo/bar/g')" prop_checkEchoSed1b = verify checkEchoSed "FOO=$(sed 's/foo/bar/g' <<< \"$cow\")" prop_checkEchoSed2 = verify checkEchoSed "rm $(echo $cow | sed -e 's,foo,bar,')" @@ -637,5 +680,22 @@ checkBangAfterPipe = ForShell [Dash, Bus err id 2326 "! is not allowed in the middle of pipelines. Use command group as in cmd | { ! cmd; } if necessary." _ -> return () + +prop_checkNegatedUnaryOps1 = verify checkNegatedUnaryOps "[ ! -o braceexpand ]" +prop_checkNegatedUnaryOps2 = verifyNot checkNegatedUnaryOps "[ -o braceexpand ]" +prop_checkNegatedUnaryOps3 = verifyNot checkNegatedUnaryOps "[[ ! -o braceexpand ]]" +prop_checkNegatedUnaryOps4 = verifyNot checkNegatedUnaryOps "! [ -o braceexpand ]" +prop_checkNegatedUnaryOps5 = verify checkNegatedUnaryOps "[ ! -a file ]" +checkNegatedUnaryOps = ForShell [Bash] f + where + f token = case token of + TC_Unary id SingleBracket "!" (TC_Unary _ _ op _) | op `elem` ["-a", "-o"] -> + err id 2332 $ msg op + _ -> return () + + msg "-o" = "[ ! -o opt ] is always true because -o becomes logical OR. Use [[ ]] or ! [ -o opt ]." + msg "-a" = "[ ! -a file ] is always true because -a becomes logical AND. Use -e instead." + msg _ = pleaseReport "unhandled negated unary message" + return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff -pruN 0.10.0-1/src/ShellCheck/Data.hs 0.11.0-1/src/ShellCheck/Data.hs --- 0.10.0-1/src/ShellCheck/Data.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Data.hs 2001-09-09 01:46:40.000000000 +0000 @@ -49,6 +49,7 @@ internalVariables = [ "LINES", "MAIL", "MAILCHECK", "MAILPATH", "OPTERR", "PATH", "POSIXLY_CORRECT", "PROMPT_COMMAND", "PROMPT_DIRTRIM", "PS0", "PS1", "PS2", "PS3", "PS4", "SHELL", "TIMEFORMAT", "TMOUT", "TMPDIR", + "BASH_MONOSECONDS", "BASH_TRAPSIG", "GLOBSORT", "auto_resume", "histchars", -- Other @@ -62,6 +63,9 @@ internalVariables = [ , "FLAGS_ARGC", "FLAGS_ARGV", "FLAGS_ERROR", "FLAGS_FALSE", "FLAGS_HELP", "FLAGS_PARENT", "FLAGS_RESERVED", "FLAGS_TRUE", "FLAGS_VERSION", "flags_error", "flags_return" + + -- Bats + ,"stderr", "stderr_lines" ] specialIntegerVariables = [ @@ -75,7 +79,7 @@ variablesWithoutSpaces = specialVariable "EPOCHREALTIME", "EPOCHSECONDS", "LINENO", "OPTIND", "PPID", "RANDOM", "READLINE_ARGUMENT", "READLINE_MARK", "READLINE_POINT", "SECONDS", "SHELLOPTS", "SHLVL", "SRANDOM", "UID", "COLUMNS", "HISTFILESIZE", - "HISTSIZE", "LINES" + "HISTSIZE", "LINES", "BASH_MONOSECONDS", "BASH_TRAPSIG" -- shflags , "FLAGS_ERROR", "FLAGS_FALSE", "FLAGS_TRUE" @@ -164,6 +168,7 @@ shellForExecutable name = "ksh" -> return Ksh "ksh88" -> return Ksh "ksh93" -> return Ksh + "oksh" -> return Ksh _ -> Nothing flagsForRead = "sreu:n:N:i:p:a:t:" diff -pruN 0.10.0-1/src/ShellCheck/Formatter/Diff.hs 0.11.0-1/src/ShellCheck/Formatter/Diff.hs --- 0.10.0-1/src/ShellCheck/Formatter/Diff.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Formatter/Diff.hs 2001-09-09 01:46:40.000000000 +0000 @@ -191,11 +191,17 @@ splitLast x = let (last, rest) = splitAt 1 $ reverse x in (reverse rest, last) +-- git patch does not like `\` on Windows +normalizePath path = + case path of + c:rest -> (if c == pathSeparator then '/' else c) : normalizePath rest + [] -> [] + formatDoc color (DiffDoc name lf regions) = let (most, last) = splitLast regions in - (color bold $ "--- " ++ ("a" name)) ++ "\n" ++ - (color bold $ "+++ " ++ ("b" name)) ++ "\n" ++ + (color bold $ "--- " ++ (normalizePath $ "a" name)) ++ "\n" ++ + (color bold $ "+++ " ++ (normalizePath $ "b" name)) ++ "\n" ++ concatMap (formatRegion color LinefeedOk) most ++ concatMap (formatRegion color lf) last diff -pruN 0.10.0-1/src/ShellCheck/Formatter/TTY.hs 0.11.0-1/src/ShellCheck/Formatter/TTY.hs --- 0.10.0-1/src/ShellCheck/Formatter/TTY.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Formatter/TTY.hs 2001-09-09 01:46:40.000000000 +0000 @@ -169,7 +169,7 @@ showFixedString color comments lineNum f -- and/or other unrelated lines. let (excerptFix, excerpt) = sliceFile mergedFix fileLines -- in the spirit of error prone - putStrLn $ color "message" "Did you mean: " + putStrLn $ color "message" "Did you mean:" putStrLn $ unlines $ applyFix excerptFix excerpt cuteIndent :: PositionedComment -> String diff -pruN 0.10.0-1/src/ShellCheck/Parser.hs 0.11.0-1/src/ShellCheck/Parser.hs --- 0.10.0-1/src/ShellCheck/Parser.hs 2001-09-09 01:46:40.000000000 +0000 +++ 0.11.0-1/src/ShellCheck/Parser.hs 2001-09-09 01:46:40.000000000 +0000 @@ -48,6 +48,7 @@ import qualified Control.Monad.Reader as import qualified Control.Monad.State as Ms import qualified Data.List.NonEmpty as NE import qualified Data.Map.Strict as Map +import Debug.Trace import Test.QuickCheck.All (quickCheckAll) @@ -66,10 +67,14 @@ singleQuote = char '\'' doubleQuote = char '"' variableStart = upper <|> lower <|> oneOf "_" variableChars = upper <|> lower <|> digit <|> oneOf "_" --- Chars to allow in function names -functionChars = variableChars <|> oneOf ":+?-./^@," +-- Chars to allow function names to start with +functionStartChars = variableChars <|> oneOf ":+?-./^@," +-- Chars to allow inside function names +functionChars = variableChars <|> oneOf "#:+?-./^@," +-- Chars to allow function names to start with, using the 'function' keyword +extendedFunctionStartChars = functionStartChars <|> oneOf "[]*=!" -- Chars to allow in functions using the 'function' keyword -extendedFunctionChars = functionChars <|> oneOf "[]*=!" +extendedFunctionChars = extendedFunctionStartChars <|> oneOf "[]*=!" specialVariable = oneOf (concat specialVariables) paramSubSpecialChars = oneOf "/:+-=%" quotableChars = "|&;<>()\\ '\t\n\r\xA0" ++ doubleQuotableChars @@ -141,15 +146,9 @@ carriageReturn = do parseProblemAt pos ErrorC 1017 "Literal carriage return. Run script through tr -d '\\r' ." return '\r' -almostSpace = - choice [ - check '\xA0' "unicode non-breaking space", - check '\x200B' "unicode zerowidth space" - ] - where - check c name = do - parseNote ErrorC 1018 $ "This is a " ++ name ++ ". Delete and retype it." - char c +almostSpace = do + parseNote ErrorC 1018 $ "This is a unicode space. Delete and retype it." + oneOf "\xA0\x2002\x2003\x2004\x2005\x2006\x2007\x2008\x2009\x200B\x202F" return ' ' --------- Message/position annotation on top of user state @@ -827,7 +826,7 @@ readArithmeticContents = char ')' id <- endSpan start spacing - return $ TA_Parentesis id s + return $ TA_Parenthesis id s readArithTerm = readGroup <|> readVariable <|> readExpansion @@ -1701,16 +1700,17 @@ readAmbiguous prefix expected alternativ prop_readDollarBraceCommandExpansion1 = isOk readDollarBraceCommandExpansion "${ ls; }" prop_readDollarBraceCommandExpansion2 = isOk readDollarBraceCommandExpansion "${\nls\n}" -readDollarBraceCommandExpansion = called "ksh ${ ..; } command expansion" $ do +prop_readDollarBraceCommandExpansion3 = isOk readDollarBraceCommandExpansion "${| REPLY=42; }" +readDollarBraceCommandExpansion = called "ksh-style ${ ..; } command expansion" $ do start <- startSpan - try $ do - string "${" - whitespace + c <- try $ do + string "${" + char '|' <|> whitespace allspacing term <- readTerm - char '}' <|> fail "Expected } to end the ksh ${ ..; } command expansion" + char '}' <|> fail "Expected } to end the ksh-style ${ ..; } command expansion" id <- endSpan start - return $ T_DollarBraceCommandExpansion id term + return $ T_DollarBraceCommandExpansion id (if c == '|' then Piped else Unpiped) term prop_readDollarBraced1 = isOk readDollarBraced "${foo//bar/baz}" prop_readDollarBraced2 = isOk readDollarBraced "${foo/'{cow}'}" @@ -2211,17 +2211,18 @@ readSimpleCommand = called "simple comma readSource :: Monad m => Token -> SCParser m Token -readSource t@(T_Redirecting _ _ (T_SimpleCommand cmdId _ (cmd:file':rest'))) = do - let file = getFile file' rest' +readSource t@(T_Redirecting _ _ (T_SimpleCommand cmdId _ (cmd:args'))) = do + let file = getFile args' override <- getSourceOverride let literalFile = do - name <- override `mplus` getLiteralString file `mplus` stripDynamicPrefix file + name <- override `mplus` (getLiteralString =<< file) `mplus` (stripDynamicPrefix =<< file) -- Hack to avoid 'source ~/foo' trying to read from literal tilde guard . not $ "~/" `isPrefixOf` name return name + let fileId = fromMaybe (getId cmd) (getId <$> file) case literalFile of Nothing -> do - parseNoteAtId (getId file) WarningC 1090 + parseNoteAtId fileId WarningC 1090 "ShellCheck can't follow non-constant source. Use a directive to specify location." return t Just filename -> do @@ -2229,7 +2230,7 @@ readSource t@(T_Redirecting _ _ (T_Simpl if not proceed then do -- FIXME: This actually gets squashed without -a - parseNoteAtId (getId file) InfoC 1093 + parseNoteAtId fileId InfoC 1093 "This file appears to be recursively sourced. Ignoring." return t else do @@ -2247,7 +2248,7 @@ readSource t@(T_Redirecting _ _ (T_Simpl return (contents, resolved) case input of Left err -> do - parseNoteAtId (getId file) InfoC 1091 $ + parseNoteAtId fileId InfoC 1091 $ "Not following: " ++ err return t Right script -> do @@ -2259,18 +2260,19 @@ readSource t@(T_Redirecting _ _ (T_Simpl return $ T_SourceCommand id1 t (T_Include id2 src) let failed = do - parseNoteAtId (getId file) WarningC 1094 + parseNoteAtId fileId WarningC 1094 "Parsing of sourced file failed. Ignoring it." return t included <|> failed where - getFile :: Token -> [Token] -> Token - getFile file (next:rest) = - case getLiteralString file of - Just "--" -> next - x -> file - getFile file _ = file + getFile :: [Token] -> Maybe Token + getFile (first:rest) = + case getLiteralString first of + Just "--" -> rest !!! 0 + Just "-p" -> rest !!! 1 + _ -> return first + getFile _ = Nothing getSourcePath t = case t of @@ -2757,6 +2759,8 @@ prop_readFunctionDefinition10 = isOk rea prop_readFunctionDefinition11 = isWarning readFunctionDefinition "function foo{\ntrue\n}" prop_readFunctionDefinition12 = isOk readFunctionDefinition "function []!() { true; }" prop_readFunctionDefinition13 = isOk readFunctionDefinition "@require(){ true; }" +prop_readFunctionDefinition14 = isOk readFunctionDefinition "foo#bar(){ :; }" +prop_readFunctionDefinition15 = isNotOk readFunctionDefinition "#bar(){ :; }" readFunctionDefinition = called "function" $ do start <- startSpan functionSignature <- try readFunctionSignature @@ -2774,7 +2778,7 @@ readFunctionDefinition = called "functio string "function" whitespace spacing - name <- many1 extendedFunctionChars + name <- (:) <$> extendedFunctionStartChars <*> many extendedFunctionChars spaces <- spacing hasParens <- wasIncluded readParens when (not hasParens && null spaces) $ @@ -2783,7 +2787,7 @@ readFunctionDefinition = called "functio return $ \id -> T_Function id (FunctionKeyword True) (FunctionParentheses hasParens) name readWithoutFunction = try $ do - name <- many1 functionChars + name <- (:) <$> functionStartChars <*> many functionChars guard $ name /= "time" -- Interferes with time ( foo ) spacing readParens @@ -2801,17 +2805,29 @@ readFunctionDefinition = called "functio prop_readCoProc1 = isOk readCoProc "coproc foo { echo bar; }" prop_readCoProc2 = isOk readCoProc "coproc { echo bar; }" prop_readCoProc3 = isOk readCoProc "coproc echo bar" +prop_readCoProc4 = isOk readCoProc "coproc a=b echo bar" +prop_readCoProc5 = isOk readCoProc "coproc 'foo' { echo bar; }" +prop_readCoProc6 = isOk readCoProc "coproc \"foo$$\" { echo bar; }" +prop_readCoProc7 = isOk readCoProc "coproc 'foo' ( echo bar )" +prop_readCoProc8 = isOk readCoProc "coproc \"foo$$\" while true; do true; done" readCoProc = called "coproc" $ do start <- startSpan try $ do string "coproc" - whitespace + spacing1 choice [ try $ readCompoundCoProc start, readSimpleCoProc start ] where readCompoundCoProc start = do - var <- optionMaybe $ - readVariableName `thenSkip` whitespace - body <- readBody readCompoundCommand + notFollowedBy2 readAssignmentWord + (var, body) <- choice [ + try $ do + body <- readBody readCompoundCommand + return (Nothing, body), + try $ do + var <- readNormalWord `thenSkip` spacing + body <- readBody readCompoundCommand + return (Just var, body) + ] id <- endSpan start return $ T_CoProc id var body readSimpleCoProc start = do @@ -3381,7 +3397,8 @@ readScriptFile sourced = do "busybox sh", "bash", "bats", - "ksh" + "ksh", + "oksh" ] badShells = [ "awk", @@ -3390,6 +3407,7 @@ readScriptFile sourced = do "fish", "perl", "python", + "python3", "ruby", "tcsh", "zsh" @@ -3442,13 +3460,22 @@ isOk p s = parsesCleanly p s == Jus isWarning p s = parsesCleanly p s == Just False -- The string parses with warnings isNotOk p s = parsesCleanly p s == Nothing -- The string does not parse -parsesCleanly parser string = runIdentity $ do - (res, sys) <- runParser testEnvironment - (parser >> eof >> getState) "-" string - case (res, sys) of - (Right userState, systemState) -> - return $ Just . null $ parseNotes userState ++ parseProblems systemState - (Left _, _) -> return Nothing +-- If the parser matches the string, return Right [ParseNotes+ParseProblems] +-- If it does not match the string, return Left [ParseProblems] +getParseOutput parser string = runIdentity $ do + (res, systemState) <- runParser testEnvironment + (parser >> eof >> getState) "-" string + return $ case res of + Right userState -> + Right $ parseNotes userState ++ parseProblems systemState + Left _ -> Left $ parseProblems systemState + +-- If the parser matches the string, return Just whether it was clean (without emitting suggestions) +-- Otherwise, Nothing +parsesCleanly parser string = + case getParseOutput parser string of + Right list -> Just $ null list + Left _ -> Nothing parseWithNotes parser = do item <- parser