diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,25 @@ +## v0.9.0 - 2022-12-12 +### Added +- SC2316: Warn about 'local readonly foo' and similar (thanks, patrickxia!) +- SC2317: Warn about unreachable commands +- SC2318: Warn about backreferences in 'declare x=1 y=$x' +- SC2319/SC2320: Warn when $? refers to echo/printf/[ ]/[[ ]]/test +- SC2321: Suggest removing $((..)) in array[$((idx))]=val +- SC2322: Suggest collapsing double parentheses in arithmetic contexts +- SC2323: Suggest removing wrapping parentheses in a[(x+1)]=val + +### Fixed +- SC2086: Now uses DFA to make more accurate predictions about values +- SC2086: No longer warns about values declared as integer with declare -i + +### Changed +- ShellCheck now has a Data Flow Analysis engine to make smarter decisions + based on control flow rather than just syntax. Existing checks will + gradually start using it, which may cause them to trigger differently + (but more accurately). +- Values in directives/shellcheckrc can now be quoted with '' or "" + + ## v0.8.0 - 2021-11-06 ### Added - `disable=all` now conveniently disables all warnings diff --git a/LICENSE b/LICENSE --- a/LICENSE +++ b/LICENSE @@ -1,13 +1,3 @@ -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/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,5 +1,5 @@ Name: ShellCheck -Version: 0.8.0 +Version: 0.9.0 Synopsis: Shell script analysis tool License: GPL-3 License-file: LICENSE @@ -45,19 +45,26 @@ build-depends: semigroups build-depends: - aeson, - array, - base >= 4.8.0.0 && < 5, - bytestring, - containers >= 0.5, - deepseq >= 1.4.0.0, - Diff >= 0.2.0, - directory >= 1.2.3.0, - mtl >= 2.2.1, - filepath, - parsec, - regex-tdfa, - QuickCheck >= 2.7.4, + -- The lower bounds are based on GHC 7.10.3 + -- The upper bounds are based on GHC 9.4.3 + aeson >= 1.4.0 && < 2.2, + array >= 0.5.1 && < 0.6, + base >= 4.8.0.0 && < 5, + bytestring >= 0.10.6 && < 0.12, + containers >= 0.5.6 && < 0.7, + deepseq >= 1.4.1 && < 1.5, + Diff >= 0.4.0 && < 0.5, + fgl >= 5.7.0 && < 5.9, + filepath >= 1.4.0 && < 1.5, + mtl >= 2.2.2 && < 2.3, + parsec >= 3.1.14 && < 3.2, + QuickCheck >= 2.14.2 && < 2.15, + regex-tdfa >= 1.2.0 && < 1.4, + transformers >= 0.4.2 && < 0.6, + + -- getXdgDirectory from 1.2.3.0 + directory >= 1.2.3 && < 1.4, + -- When cabal supports it, move this to setup-depends: process exposed-modules: @@ -66,11 +73,15 @@ ShellCheck.Analytics ShellCheck.Analyzer ShellCheck.AnalyzerLib + ShellCheck.CFG + ShellCheck.CFGAnalysis ShellCheck.Checker ShellCheck.Checks.Commands + ShellCheck.Checks.ControlFlow ShellCheck.Checks.Custom ShellCheck.Checks.ShellSupport ShellCheck.Data + ShellCheck.Debug ShellCheck.Fixer ShellCheck.Formatter.Format ShellCheck.Formatter.CheckStyle @@ -82,6 +93,7 @@ ShellCheck.Formatter.Quiet ShellCheck.Interface ShellCheck.Parser + ShellCheck.Prelude ShellCheck.Regex other-modules: Paths_ShellCheck @@ -94,17 +106,19 @@ build-depends: aeson, array, - base >= 4 && < 5, + base, bytestring, containers, - deepseq >= 1.4.0.0, - Diff >= 0.2.0, - directory >= 1.2.3.0, - mtl >= 2.2.1, + deepseq, + Diff, + directory, + fgl, + mtl, filepath, - parsec >= 3.0, - QuickCheck >= 2.7.4, + parsec, + QuickCheck, regex-tdfa, + transformers, ShellCheck default-language: Haskell98 main-is: shellcheck.hs @@ -114,17 +128,19 @@ build-depends: aeson, array, - base >= 4 && < 5, + base, bytestring, containers, - deepseq >= 1.4.0.0, - Diff >= 0.2.0, - directory >= 1.2.3.0, - mtl >= 2.2.1, + deepseq, + Diff, + directory, + fgl, filepath, + mtl, parsec, - QuickCheck >= 2.7.4, + QuickCheck, regex-tdfa, + transformers, ShellCheck default-language: Haskell98 main-is: test/shellcheck.hs diff --git a/shellcheck.1.md b/shellcheck.1.md --- a/shellcheck.1.md +++ b/shellcheck.1.md @@ -282,6 +282,9 @@ source-path=SCRIPTDIR source-path=/mnt/chroot + # Since 0.9.0, values can be quoted with '' or "" to allow spaces + source-path="My Documents/scripts" + # Allow opening any 'source'd file, even if not specified as input external-sources=true @@ -375,7 +378,7 @@ # COPYRIGHT -Copyright 2012-2021, Vidar Holen and contributors. +Copyright 2012-2022, Vidar Holen and contributors. Licensed under the GNU General Public License version 3 or later, see https://gnu.org/licenses/gpl.html diff --git a/shellcheck.hs b/shellcheck.hs --- a/shellcheck.hs +++ b/shellcheck.hs @@ -34,6 +34,8 @@ import Control.Exception import Control.Monad +import Control.Monad.IO.Class +import Control.Monad.Trans.Class import Control.Monad.Except import Data.Bits import Data.Char @@ -225,7 +227,7 @@ f :: Status -> FilePath -> IO Status f status file = do newStatus <- process file `catch` handler file - return $ status `mappend` newStatus + return $! status `mappend` newStatus handler :: FilePath -> IOException -> IO Status handler file e = reportFailure file (show e) reportFailure file str = do diff --git a/src/ShellCheck/AST.hs b/src/ShellCheck/AST.hs --- a/src/ShellCheck/AST.hs +++ b/src/ShellCheck/AST.hs @@ -45,6 +45,7 @@ | Inner_TA_Variable String [t] | Inner_TA_Expansion [t] | Inner_TA_Sequence [t] + | Inner_TA_Parenthesis t | Inner_TA_Trinary t t t | Inner_TA_Unary String t | Inner_TC_And ConditionType String t t @@ -141,7 +142,7 @@ | Inner_T_CoProcBody t | Inner_T_Include t | Inner_T_SourceCommand t t - | Inner_T_BatsTest t t + | Inner_T_BatsTest String t deriving (Show, Eq, Functor, Foldable, Traversable) data Annotation = @@ -204,6 +205,7 @@ 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 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) @@ -256,7 +258,7 @@ 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, 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_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 #-} instance Eq Token where OuterToken _ a == OuterToken _ b = a == b diff --git a/src/ShellCheck/ASTLib.hs b/src/ShellCheck/ASTLib.hs --- a/src/ShellCheck/ASTLib.hs +++ b/src/ShellCheck/ASTLib.hs @@ -21,6 +21,7 @@ module ShellCheck.ASTLib where import ShellCheck.AST +import ShellCheck.Prelude import ShellCheck.Regex import Control.Monad.Writer @@ -138,7 +139,7 @@ flag (x, '-':'-':arg) = [ (x, takeWhile (/= '=') arg) ] flag (x, '-':args) = map (\v -> (x, [v])) args flag (x, _) = [ (x, "") ] -getFlagsUntil _ _ = error "Internal shellcheck error, please report! (getFlags on non-command)" +getFlagsUntil _ _ = error $ pleaseReport "getFlags on non-command" -- Get all flags in a GNU way, up until -- getAllFlags :: Token -> [(Token, String)] @@ -369,6 +370,21 @@ f (T_Glob _ str) = return str f _ = Nothing + +prop_getLiteralString1 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\x01") == Just "\1" +prop_getLiteralString2 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\xyz") == Just "\\xyz" +prop_getLiteralString3 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\x1") == Just "\x1" +prop_getLiteralString4 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\x1y") == Just "\x1y" +prop_getLiteralString5 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\xy") == Just "\\xy" +prop_getLiteralString6 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\x") == Just "\\x" +prop_getLiteralString7 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\1x") == Just "\1x" +prop_getLiteralString8 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\12x") == Just "\o12x" +prop_getLiteralString9 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\123x") == Just "\o123x" +prop_getLiteralString10 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\1234") == Just "\o123\&4" +prop_getLiteralString11 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\1") == Just "\1" +prop_getLiteralString12 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\12") == Just "\o12" +prop_getLiteralString13 = getLiteralString (T_DollarSingleQuoted (Id 0) "\\123") == Just "\o123" + -- Maybe get the literal value of a token, using a custom function -- to map unrecognized Tokens into strings. getLiteralStringExt :: Monad m => (Token -> m String) -> Token -> m String @@ -401,14 +417,15 @@ '\\' -> '\\' : rest 'x' -> case cs of - (x:y:more) -> - if isHexDigit x && isHexDigit y - then chr (16*(digitToInt x) + (digitToInt y)) : rest - else '\\':c:rest + (x:y:more) | isHexDigit x && isHexDigit y -> + chr (16*(digitToInt x) + (digitToInt y)) : decodeEscapes more + (x:more) | isHexDigit x -> + chr (digitToInt x) : decodeEscapes more + more -> '\\' : 'x' : decodeEscapes more _ | isOctDigit c -> - let digits = take 3 $ takeWhile isOctDigit (c:cs) - num = parseOct digits - in (if num < 256 then chr num else '?') : rest + let (digits, more) = spanMax isOctDigit 3 (c:cs) + num = (parseOct digits) `mod` 256 + in (chr num) : decodeEscapes more _ -> '\\' : c : rest where rest = decodeEscapes cs @@ -416,6 +433,11 @@ where f n "" = n f n (c:rest) = f (n * 8 + digitToInt c) rest + spanMax f n list = + let (first, second) = span f list + (prefix, suffix) = splitAt n first + in + (prefix, suffix ++ second) decodeEscapes (c:cs) = c : decodeEscapes cs decodeEscapes [] = [] @@ -763,6 +785,123 @@ fromEnvArgs args = fromMaybe "" $ find (notElem '=') $ skipFlags args basename s = reverse . takeWhile (/= '/') . reverse $ s skipFlags = dropWhile ("-" `isPrefixOf`) + + +-- Determining if a name is a variable +isVariableStartChar x = x == '_' || isAsciiLower x || isAsciiUpper x +isVariableChar x = isVariableStartChar x || isDigit x +isSpecialVariableChar = (`elem` "*@#?-$!") +variableNameRegex = mkRegex "[_a-zA-Z][_a-zA-Z0-9]*" + +prop_isVariableName1 = isVariableName "_fo123" +prop_isVariableName2 = not $ isVariableName "4" +prop_isVariableName3 = not $ isVariableName "test: " +isVariableName (x:r) = isVariableStartChar x && all isVariableChar r +isVariableName _ = False + + +-- Get the variable name from an expansion like ${var:-foo} +prop_getBracedReference1 = getBracedReference "foo" == "foo" +prop_getBracedReference2 = getBracedReference "#foo" == "foo" +prop_getBracedReference3 = getBracedReference "#" == "#" +prop_getBracedReference4 = getBracedReference "##" == "#" +prop_getBracedReference5 = getBracedReference "#!" == "!" +prop_getBracedReference6 = getBracedReference "!#" == "#" +prop_getBracedReference7 = getBracedReference "!foo#?" == "foo" +prop_getBracedReference8 = getBracedReference "foo-bar" == "foo" +prop_getBracedReference9 = getBracedReference "foo:-bar" == "foo" +prop_getBracedReference10 = getBracedReference "foo: -1" == "foo" +prop_getBracedReference11 = getBracedReference "!os*" == "" +prop_getBracedReference11b = getBracedReference "!os@" == "" +prop_getBracedReference12 = getBracedReference "!os?bar**" == "" +prop_getBracedReference13 = getBracedReference "foo[bar]" == "foo" +getBracedReference s = fromMaybe s $ + nameExpansion s `mplus` takeName noPrefix `mplus` getSpecial noPrefix `mplus` getSpecial s + where + noPrefix = dropPrefix s + dropPrefix (c:rest) | c `elem` "!#" = rest + dropPrefix cs = cs + takeName s = do + let name = takeWhile isVariableChar s + guard . not $ null name + return name + getSpecial (c:_) | isSpecialVariableChar c = return [c] + getSpecial _ = fail "empty or not special" + + nameExpansion ('!':next:rest) = do -- e.g. ${!foo*bar*} + guard $ isVariableChar next -- e.g. ${!@} + first <- find (not . isVariableChar) rest + guard $ first `elem` "*?@" + return "" + nameExpansion _ = Nothing + +-- Get the variable modifier like /a/b in ${var/a/b} +prop_getBracedModifier1 = getBracedModifier "foo:bar:baz" == ":bar:baz" +prop_getBracedModifier2 = getBracedModifier "!var:-foo" == ":-foo" +prop_getBracedModifier3 = getBracedModifier "foo[bar]" == "[bar]" +prop_getBracedModifier4 = getBracedModifier "foo[@]@Q" == "[@]@Q" +prop_getBracedModifier5 = getBracedModifier "@@Q" == "@Q" +getBracedModifier s = headOrDefault "" $ do + let var = getBracedReference s + a <- dropModifier s + dropPrefix var a + where + dropPrefix [] t = return t + dropPrefix (a:b) (c:d) | a == c = dropPrefix b d + dropPrefix _ _ = [] + + dropModifier (c:rest) | c `elem` "#!" = [rest, c:rest] + dropModifier x = [x] + +-- Get the variables from indices like ["x", "y"] in ${var[x+y+1]} +prop_getIndexReferences1 = getIndexReferences "var[x+y+1]" == ["x", "y"] +getIndexReferences s = fromMaybe [] $ do + match <- matchRegex re s + index <- match !!! 0 + return $ matchAllStrings variableNameRegex index + where + re = mkRegex "(\\[.*\\])" + +prop_getOffsetReferences1 = getOffsetReferences ":bar" == ["bar"] +prop_getOffsetReferences2 = getOffsetReferences ":bar:baz" == ["bar", "baz"] +prop_getOffsetReferences3 = getOffsetReferences "[foo]:bar" == ["bar"] +prop_getOffsetReferences4 = getOffsetReferences "[foo]:bar:baz" == ["bar", "baz"] +getOffsetReferences mods = fromMaybe [] $ do +-- if mods start with [, then drop until ] + match <- matchRegex re mods + offsets <- match !!! 1 + return $ matchAllStrings variableNameRegex offsets + where + re = mkRegex "^(\\[.+\\])? *:([^-=?+].*)" + + +-- Returns whether a token is a parameter expansion without any modifiers. +-- True for $var ${var} $1 $# +-- False for ${#var} ${var[x]} ${var:-0} +isUnmodifiedParameterExpansion t = + case t of + T_DollarBraced _ False _ -> True + T_DollarBraced _ _ list -> + let str = concat $ oversimplify list + in getBracedReference str == str + _ -> False + +--- A list of the element and all its parents up to the root node. +getPath tree t = t : + case Map.lookup (getId t) tree of + Nothing -> [] + Just parent -> getPath tree parent + +isClosingFileOp op = + case op of + T_IoDuplicate _ (T_GREATAND _) "-" -> True + T_IoDuplicate _ (T_LESSAND _) "-" -> True + _ -> False + +getEnableDirectives root = + case root of + T_Annotation _ list _ -> [s | EnableComment s <- list] + _ -> [] return [] runTests = $quickCheckAll diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2021 Vidar Holen + Copyright 2012-2022 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -19,13 +19,16 @@ -} {-# LANGUAGE TemplateHaskell #-} {-# LANGUAGE FlexibleContexts #-} -module ShellCheck.Analytics (runAnalytics, optionalChecks, ShellCheck.Analytics.runTests) where +module ShellCheck.Analytics (checker, optionalChecks, ShellCheck.Analytics.runTests) where import ShellCheck.AST import ShellCheck.ASTLib import ShellCheck.AnalyzerLib hiding (producesComments) +import ShellCheck.CFG +import qualified ShellCheck.CFGAnalysis as CF import ShellCheck.Data import ShellCheck.Parser +import ShellCheck.Prelude import ShellCheck.Interface import ShellCheck.Regex @@ -44,6 +47,7 @@ import Data.Semigroup import Debug.Trace -- STRIP import qualified Data.Map.Strict as Map +import qualified Data.Set as S import Test.QuickCheck.All (forAllProperties) import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) @@ -52,7 +56,6 @@ treeChecks = [ nodeChecksToTreeCheck nodeChecks ,subshellAssignmentCheck - ,checkSpacefulness ,checkQuotesInLiterals ,checkShebangParameters ,checkFunctionsUsedExternally @@ -68,30 +71,23 @@ ,checkArrayValueUsedAsIndex ] -runAnalytics :: AnalysisSpec -> [TokenComment] -runAnalytics options = - runList options treeChecks ++ runList options optionalChecks +checker spec params = mkChecker spec params treeChecks + +mkChecker spec params checks = + Checker { + perScript = \(Root root) -> do + tell $ concatMap (\f -> f params root) all, + perToken = const $ return () + } where - root = asScript options - optionals = getEnableDirectives root ++ asOptionalChecks options - optionalChecks = - if "all" `elem` optionals + all = checks ++ optionals + optionalKeys = asOptionalChecks spec + optionals = + if "all" `elem` optionalKeys then map snd optionalTreeChecks - else mapMaybe (\c -> Map.lookup c optionalCheckMap) optionals + else mapMaybe (\c -> Map.lookup c optionalCheckMap) optionalKeys -runList :: AnalysisSpec -> [Parameters -> Token -> [TokenComment]] - -> [TokenComment] -runList spec list = notes - where - root = asScript spec - params = makeParameters spec - notes = concatMap (\f -> f params root) list -getEnableDirectives root = - case root of - T_Annotation _ list _ -> [s | EnableComment s <- list] - _ -> [] - checkList l t = concatMap (\f -> f t) l -- Checks that are run on each node in the AST @@ -199,6 +195,12 @@ ,checkComparisonWithLeadingX ,checkCommandWithTrailingSymbol ,checkUnquotedParameterExpansionPattern + ,checkBatsTestDoesNotUseNegation + ,checkCommandIsUnreachable + ,checkSpacefulnessCfg + ,checkOverwrittenExitCode + ,checkUnnecessaryArithmeticExpansionIndex + ,checkUnnecessaryParens ] optionalChecks = map fst optionalTreeChecks @@ -217,7 +219,7 @@ cdDescription = "Suggest quoting variables without metacharacters", cdPositive = "var=hello; echo $var", cdNegative = "var=hello; echo \"$var\"" - }, checkVerboseSpacefulness) + }, nodeChecksToTreeCheck [checkVerboseSpacefulnessCfg]) ,(newCheckDescription { cdName = "avoid-nullary-conditions", @@ -309,12 +311,12 @@ runAndGetComments f s = do let pr = pScript s - prRoot pr + root <- prRoot pr let spec = defaultSpec pr let params = makeParameters spec return $ filterByAnnotation spec params $ - runList spec [f] + f params root -- Copied from https://wiki.haskell.org/Edit_distance dist :: Eq a => [a] -> [a] -> Int @@ -544,6 +546,11 @@ prop_checkPipePitfalls16 = verifyNot checkPipePitfalls "foo | grep -r bar | wc -l" prop_checkPipePitfalls17 = verifyNot checkPipePitfalls "foo | grep -l bar | wc -l" prop_checkPipePitfalls18 = verifyNot checkPipePitfalls "foo | grep -L bar | wc -l" +prop_checkPipePitfalls19 = verifyNot checkPipePitfalls "foo | grep -A2 bar | wc -l" +prop_checkPipePitfalls20 = verifyNot checkPipePitfalls "foo | grep -B999 bar | wc -l" +prop_checkPipePitfalls21 = verifyNot checkPipePitfalls "foo | grep --after-context 999 bar | wc -l" +prop_checkPipePitfalls22 = verifyNot checkPipePitfalls "foo | grep -B 1 --after-context 999 bar | wc -l" +prop_checkPipePitfalls23 = verifyNot checkPipePitfalls "ps -o pid,args -p $(pgrep java) | grep -F net.shellcheck.Test" checkPipePitfalls _ (T_Pipeline id _ commands) = do for ["find", "xargs"] $ \(find:xargs:_) -> @@ -557,18 +564,25 @@ ]) $ warn (getId find) 2038 "Use -print0/-0 or -exec + to allow for non-alphanumeric filenames." - for' ["ps", "grep"] $ - \x -> info x 2009 "Consider using pgrep instead of grepping ps output." + for ["ps", "grep"] $ + \(ps:grep:_) -> + let + psFlags = maybe [] (map snd . getAllFlags) $ getCommand ps + in + -- There are many ways to specify a pid: 1, -1, p 1, wup 1, -q 1, -p 1, --pid 1. + -- For simplicity we only deal with the most canonical looking flags: + unless (any (`elem` ["p", "pid", "q", "quick-pid"]) psFlags) $ + info (getId ps) 2009 "Consider using pgrep instead of grepping ps output." for ["grep", "wc"] $ \(grep:wc:_) -> let flagsGrep = maybe [] (map snd . getAllFlags) $ getCommand grep flagsWc = maybe [] (map snd . getAllFlags) $ getCommand wc in - unless (any (`elem` ["l", "files-with-matches", "L", "files-without-matches", "o", "only-matching", "r", "R", "recursive"]) flagsGrep + unless (any (`elem` ["l", "files-with-matches", "L", "files-without-matches", "o", "only-matching", "r", "R", "recursive", "A", "after-context", "B", "before-context"]) flagsGrep || any (`elem` ["m", "chars", "w", "words", "c", "bytes", "L", "max-line-length"]) flagsWc || null flagsWc) $ - style (getId grep) 2126 "Consider using grep -c instead of grep|wc -l." + style (getId grep) 2126 "Consider using 'grep -c' instead of 'grep|wc -l'." didLs <- fmap or . sequence $ [ for' ["ls", "grep"] $ @@ -628,15 +642,15 @@ 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" -prop_checkShebang11= verifyTree checkShebang "#!/bin/sh/\ntrue" -prop_checkShebang12= verifyTree checkShebang "#!/bin/sh/ -xe\ntrue" -prop_checkShebang13= verifyTree checkShebang "#!/bin/busybox sh" -prop_checkShebang14= verifyNotTree checkShebang "#!/bin/busybox sh\n# shellcheck shell=sh\n" -prop_checkShebang15= verifyNotTree checkShebang "#!/bin/busybox sh\n# shellcheck shell=dash\n" -prop_checkShebang16= verifyTree checkShebang "#!/bin/busybox ash" -prop_checkShebang17= verifyNotTree checkShebang "#!/bin/busybox ash\n# shellcheck shell=dash\n" -prop_checkShebang18= verifyNotTree checkShebang "#!/bin/busybox ash\n# shellcheck shell=sh\n" +prop_checkShebang10 = verifyNotTree checkShebang "#!foo\n# shellcheck shell=sh ignore=SC2239\ntrue" +prop_checkShebang11 = verifyTree checkShebang "#!/bin/sh/\ntrue" +prop_checkShebang12 = verifyTree checkShebang "#!/bin/sh/ -xe\ntrue" +prop_checkShebang13 = verifyTree checkShebang "#!/bin/busybox sh" +prop_checkShebang14 = verifyNotTree checkShebang "#!/bin/busybox sh\n# shellcheck shell=sh\n" +prop_checkShebang15 = verifyNotTree checkShebang "#!/bin/busybox sh\n# shellcheck shell=dash\n" +prop_checkShebang16 = verifyTree checkShebang "#!/bin/busybox ash" +prop_checkShebang17 = verifyNotTree checkShebang "#!/bin/busybox ash\n# shellcheck shell=dash\n" +prop_checkShebang18 = verifyNotTree checkShebang "#!/bin/busybox ash\n# shellcheck shell=sh\n" checkShebang params (T_Annotation _ list t) = if any isOverride list then [] else checkShebang params t where @@ -690,9 +704,9 @@ checkForInQuoted _ _ = return () prop_checkForInCat1 = verify checkForInCat "for f in $(cat foo); do stuff; done" -prop_checkForInCat1a= verify checkForInCat "for f in `cat foo`; do stuff; done" +prop_checkForInCat1a = verify checkForInCat "for f in `cat foo`; do stuff; done" prop_checkForInCat2 = verify checkForInCat "for f in $(cat foo | grep lol); do stuff; done" -prop_checkForInCat2a= verify checkForInCat "for f in `cat foo | grep lol`; do stuff; done" +prop_checkForInCat2a = verify checkForInCat "for f in `cat foo | grep lol`; do stuff; done" prop_checkForInCat3 = verifyNot checkForInCat "for f in $(cat foo | grep bar | wc -l); do stuff; done" checkForInCat _ (T_ForIn _ f [T_NormalWord _ w] _) = mapM_ checkF w where @@ -765,10 +779,10 @@ prop_checkUnquotedExpansions1 = verify checkUnquotedExpansions "rm $(ls)" -prop_checkUnquotedExpansions1a= verify checkUnquotedExpansions "rm `ls`" +prop_checkUnquotedExpansions1a = verify checkUnquotedExpansions "rm `ls`" prop_checkUnquotedExpansions2 = verify checkUnquotedExpansions "rm foo$(date)" prop_checkUnquotedExpansions3 = verify checkUnquotedExpansions "[ $(foo) == cow ]" -prop_checkUnquotedExpansions3a= verify checkUnquotedExpansions "[ ! $(foo) ]" +prop_checkUnquotedExpansions3a = verify checkUnquotedExpansions "[ ! $(foo) ]" prop_checkUnquotedExpansions4 = verifyNot checkUnquotedExpansions "[[ $(foo) == cow ]]" prop_checkUnquotedExpansions5 = verifyNot checkUnquotedExpansions "for f in $(cmd); do echo $f; done" prop_checkUnquotedExpansions6 = verifyNot checkUnquotedExpansions "$(cmd)" @@ -776,6 +790,7 @@ prop_checkUnquotedExpansions8 = verifyNot checkUnquotedExpansions "set -- $(seq 1 4)" prop_checkUnquotedExpansions9 = verifyNot checkUnquotedExpansions "echo foo `# inline comment`" prop_checkUnquotedExpansions10 = verify checkUnquotedExpansions "#!/bin/sh\nexport var=$(val)" +prop_checkUnquotedExpansions11 = verifyNot checkUnquotedExpansions "ps -p $(pgrep foo)" checkUnquotedExpansions params = check where @@ -789,7 +804,7 @@ warn (getId t) 2046 "Quote this to prevent word splitting." shouldBeSplit t = - getCommandNameFromExpansion t == Just "seq" + getCommandNameFromExpansion t `elem` [Just "seq", Just "pgrep"] prop_checkRedirectToSame = verify checkRedirectToSame "cat foo > foo" @@ -801,6 +816,7 @@ prop_checkRedirectToSame7 = verifyNot checkRedirectToSame "sed 's/foo/bar/g' file | sponge file" prop_checkRedirectToSame8 = verifyNot checkRedirectToSame "while read -r line; do _=\"$fname\"; done <\"$fname\"" prop_checkRedirectToSame9 = verifyNot checkRedirectToSame "while read -r line; do cat < \"$fname\"; done <\"$fname\"" +prop_checkRedirectToSame10 = verifyNot checkRedirectToSame "mapfile -t foo (mapM_ (\x -> doAnalysis (checkOccurrences x) l) (getAllRedirs list))) list where @@ -846,7 +862,7 @@ isHarmlessCommand arg = fromMaybe False $ do cmd <- getClosestCommand (parentMap params) arg name <- getCommandBasename cmd - return $ name `elem` ["echo", "printf", "sponge"] + return $ name `elem` ["echo", "mapfile", "printf", "sponge"] containsAssignment arg = fromMaybe False $ do cmd <- getClosestCommand (parentMap params) arg return $ isAssignment cmd @@ -862,7 +878,7 @@ 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_AndIf id _ (T_OrIf _ _ (T_Pipeline _ _ t))) +checkShorthandIf params x@(T_OrIf _ (T_AndIf id _ _) (T_Pipeline _ _ t)) | not (isOk t || inCondition) = info id 2015 "Note that A && B || C is not if-then-else. C may run when A is true." where @@ -896,7 +912,7 @@ prop_checkUnquotedDollarAt = verify checkUnquotedDollarAt "ls $@" -prop_checkUnquotedDollarAt1= verifyNot checkUnquotedDollarAt "ls ${#@}" +prop_checkUnquotedDollarAt1 = verifyNot checkUnquotedDollarAt "ls ${#@}" prop_checkUnquotedDollarAt2 = verify checkUnquotedDollarAt "ls ${foo[@]}" prop_checkUnquotedDollarAt3 = verifyNot checkUnquotedDollarAt "ls ${#foo[@]}" prop_checkUnquotedDollarAt4 = verifyNot checkUnquotedDollarAt "ls \"$@\"" @@ -1027,32 +1043,32 @@ prop_checkSingleQuotedVariables = verify checkSingleQuotedVariables "echo '$foo'" prop_checkSingleQuotedVariables2 = verify checkSingleQuotedVariables "echo 'lol$1.jpg'" prop_checkSingleQuotedVariables3 = verifyNot checkSingleQuotedVariables "sed 's/foo$/bar/'" -prop_checkSingleQuotedVariables3a= verify checkSingleQuotedVariables "sed 's/${foo}/bar/'" -prop_checkSingleQuotedVariables3b= verify checkSingleQuotedVariables "sed 's/$(echo cow)/bar/'" -prop_checkSingleQuotedVariables3c= verify checkSingleQuotedVariables "sed 's/$((1+foo))/bar/'" +prop_checkSingleQuotedVariables3a = verify checkSingleQuotedVariables "sed 's/${foo}/bar/'" +prop_checkSingleQuotedVariables3b = verify checkSingleQuotedVariables "sed 's/$(echo cow)/bar/'" +prop_checkSingleQuotedVariables3c = verify checkSingleQuotedVariables "sed 's/$((1+foo))/bar/'" prop_checkSingleQuotedVariables4 = verifyNot checkSingleQuotedVariables "awk '{print $1}'" prop_checkSingleQuotedVariables5 = verifyNot checkSingleQuotedVariables "trap 'echo $SECONDS' EXIT" prop_checkSingleQuotedVariables6 = verifyNot checkSingleQuotedVariables "sed -n '$p'" -prop_checkSingleQuotedVariables6a= verify checkSingleQuotedVariables "sed -n '$pattern'" +prop_checkSingleQuotedVariables6a = verify checkSingleQuotedVariables "sed -n '$pattern'" prop_checkSingleQuotedVariables7 = verifyNot checkSingleQuotedVariables "PS1='$PWD \\$ '" prop_checkSingleQuotedVariables8 = verify checkSingleQuotedVariables "find . -exec echo '$1' {} +" prop_checkSingleQuotedVariables9 = verifyNot checkSingleQuotedVariables "find . -exec awk '{print $1}' {} \\;" -prop_checkSingleQuotedVariables10= verify checkSingleQuotedVariables "echo '`pwd`'" -prop_checkSingleQuotedVariables11= verifyNot checkSingleQuotedVariables "sed '${/lol/d}'" -prop_checkSingleQuotedVariables12= verifyNot checkSingleQuotedVariables "eval 'echo $1'" -prop_checkSingleQuotedVariables13= verifyNot checkSingleQuotedVariables "busybox awk '{print $1}'" -prop_checkSingleQuotedVariables14= verifyNot checkSingleQuotedVariables "[ -v 'bar[$foo]' ]" -prop_checkSingleQuotedVariables15= verifyNot checkSingleQuotedVariables "git filter-branch 'test $GIT_COMMIT'" -prop_checkSingleQuotedVariables16= verify checkSingleQuotedVariables "git '$a'" -prop_checkSingleQuotedVariables17= verifyNot checkSingleQuotedVariables "rename 's/(.)a/$1/g' *" -prop_checkSingleQuotedVariables18= verifyNot checkSingleQuotedVariables "echo '``'" -prop_checkSingleQuotedVariables19= verifyNot checkSingleQuotedVariables "echo '```'" -prop_checkSingleQuotedVariables20= verifyNot checkSingleQuotedVariables "mumps -run %XCMD 'W $O(^GLOBAL(5))'" -prop_checkSingleQuotedVariables21= verifyNot checkSingleQuotedVariables "mumps -run LOOP%XCMD --xec 'W $O(^GLOBAL(6))'" -prop_checkSingleQuotedVariables22= verifyNot checkSingleQuotedVariables "jq '$__loc__'" -prop_checkSingleQuotedVariables23= verifyNot checkSingleQuotedVariables "command jq '$__loc__'" -prop_checkSingleQuotedVariables24= verifyNot checkSingleQuotedVariables "exec jq '$__loc__'" -prop_checkSingleQuotedVariables25= verifyNot checkSingleQuotedVariables "exec -c -a foo jq '$__loc__'" +prop_checkSingleQuotedVariables10 = verify checkSingleQuotedVariables "echo '`pwd`'" +prop_checkSingleQuotedVariables11 = verifyNot checkSingleQuotedVariables "sed '${/lol/d}'" +prop_checkSingleQuotedVariables12 = verifyNot checkSingleQuotedVariables "eval 'echo $1'" +prop_checkSingleQuotedVariables13 = verifyNot checkSingleQuotedVariables "busybox awk '{print $1}'" +prop_checkSingleQuotedVariables14 = verifyNot checkSingleQuotedVariables "[ -v 'bar[$foo]' ]" +prop_checkSingleQuotedVariables15 = verifyNot checkSingleQuotedVariables "git filter-branch 'test $GIT_COMMIT'" +prop_checkSingleQuotedVariables16 = verify checkSingleQuotedVariables "git '$a'" +prop_checkSingleQuotedVariables17 = verifyNot checkSingleQuotedVariables "rename 's/(.)a/$1/g' *" +prop_checkSingleQuotedVariables18 = verifyNot checkSingleQuotedVariables "echo '``'" +prop_checkSingleQuotedVariables19 = verifyNot checkSingleQuotedVariables "echo '```'" +prop_checkSingleQuotedVariables20 = verifyNot checkSingleQuotedVariables "mumps -run %XCMD 'W $O(^GLOBAL(5))'" +prop_checkSingleQuotedVariables21 = verifyNot checkSingleQuotedVariables "mumps -run LOOP%XCMD --xec 'W $O(^GLOBAL(6))'" +prop_checkSingleQuotedVariables22 = verifyNot checkSingleQuotedVariables "jq '$__loc__'" +prop_checkSingleQuotedVariables23 = verifyNot checkSingleQuotedVariables "command jq '$__loc__'" +prop_checkSingleQuotedVariables24 = verifyNot checkSingleQuotedVariables "exec jq '$__loc__'" +prop_checkSingleQuotedVariables25 = verifyNot checkSingleQuotedVariables "exec -c -a foo jq '$__loc__'" checkSingleQuotedVariables params t@(T_SingleQuoted id s) = @@ -1161,6 +1177,10 @@ prop_checkNumberComparisons19 = verifyNot checkNumberComparisons "foo=1; [[ foo -eq 2 ]]" prop_checkNumberComparisons20 = verify checkNumberComparisons "[[ 2 -eq / ]]" prop_checkNumberComparisons21 = verify checkNumberComparisons "[[ foo -eq foo ]]" +prop_checkNumberComparisons22 = verify checkNumberComparisons "x=10; [[ $x > $z ]]" +prop_checkNumberComparisons23 = verify checkNumberComparisons "x=0; if [[ -n $def ]]; then x=$def; fi; while [ $x > $z ]; do lol; done" +prop_checkNumberComparisons24 = verify checkNumberComparisons "x=$RANDOM; [ $x > $z ]" +prop_checkNumberComparisons25 = verify checkNumberComparisons "[[ $((n++)) > $x ]]" checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do if isNum lhs || isNum rhs @@ -1236,9 +1256,20 @@ numChar x = isDigit x || x `elem` "+-. " isNum t = - case oversimplify t of - [v] -> all isDigit v - _ -> False + case getWordParts t of + [T_DollarArithmetic {}] -> True + [b@(T_DollarBraced id _ c)] -> + let + str = concat $ oversimplify c + var = getBracedReference str + in fromMaybe False $ do + state <- CF.getIncomingState (cfgAnalysis params) id + value <- Map.lookup var $ CF.variablesInScope state + return $ CF.numericalStatus (CF.variableValue value) >= CF.NumericalStatusMaybe + _ -> + case oversimplify t of + [v] -> all isDigit v + _ -> False isFraction t = case oversimplify t of @@ -1343,8 +1374,8 @@ prop_checkConstantIfs1 = verify checkConstantIfs "[[ foo != bar ]]" -prop_checkConstantIfs2a= verify checkConstantIfs "[ n -le 4 ]" -prop_checkConstantIfs2b= verifyNot checkConstantIfs "[[ n -le 4 ]]" +prop_checkConstantIfs2a = verify checkConstantIfs "[ n -le 4 ]" +prop_checkConstantIfs2b = verifyNot checkConstantIfs "[[ n -le 4 ]]" prop_checkConstantIfs3 = verify checkConstantIfs "[[ $n -le 4 && n != 2 ]]" prop_checkConstantIfs4 = verifyNot checkConstantIfs "[[ $n -le 3 ]]" prop_checkConstantIfs5 = verifyNot checkConstantIfs "[[ $n -le $n ]]" @@ -1449,13 +1480,14 @@ prop_checkArithmeticDeref7 = verifyNot checkArithmeticDeref "(( 10#$n ))" prop_checkArithmeticDeref8 = verifyNot checkArithmeticDeref "let i=$i+1" prop_checkArithmeticDeref9 = verifyNot checkArithmeticDeref "(( a[foo] ))" -prop_checkArithmeticDeref10= verifyNot checkArithmeticDeref "(( a[\\$foo] ))" -prop_checkArithmeticDeref11= verifyNot checkArithmeticDeref "a[$foo]=wee" -prop_checkArithmeticDeref12= verify checkArithmeticDeref "for ((i=0; $i < 3; i)); do true; done" -prop_checkArithmeticDeref13= verifyNot checkArithmeticDeref "(( $$ ))" -prop_checkArithmeticDeref14= verifyNot checkArithmeticDeref "(( $! ))" -prop_checkArithmeticDeref15= verifyNot checkArithmeticDeref "(( ${!var} ))" -prop_checkArithmeticDeref16= verifyNot checkArithmeticDeref "(( ${x+1} + ${x=42} ))" +prop_checkArithmeticDeref10 = verifyNot checkArithmeticDeref "(( a[\\$foo] ))" +prop_checkArithmeticDeref11 = verify checkArithmeticDeref "a[$foo]=wee" +prop_checkArithmeticDeref11b = verifyNot checkArithmeticDeref "declare -A a; a[$foo]=wee" +prop_checkArithmeticDeref12 = verify checkArithmeticDeref "for ((i=0; $i < 3; i)); do true; done" +prop_checkArithmeticDeref13 = verifyNot checkArithmeticDeref "(( $$ ))" +prop_checkArithmeticDeref14 = verifyNot checkArithmeticDeref "(( $! ))" +prop_checkArithmeticDeref15 = verifyNot checkArithmeticDeref "(( ${!var} ))" +prop_checkArithmeticDeref16 = verifyNot checkArithmeticDeref "(( ${x+1} + ${x=42} ))" checkArithmeticDeref params t@(TA_Expansion _ [T_DollarBraced id _ l]) = unless (isException $ concat $ oversimplify l) getWarning where @@ -1467,6 +1499,7 @@ T_Arithmetic {} -> return normalWarning T_DollarArithmetic {} -> return normalWarning T_ForArithmetic {} -> return normalWarning + T_Assignment {} -> return normalWarning T_SimpleCommand {} -> return noWarning _ -> Nothing @@ -1590,7 +1623,7 @@ prop_checkValidCondOps1 = verify checkValidCondOps "[[ a -xz b ]]" prop_checkValidCondOps2 = verify checkValidCondOps "[ -M a ]" -prop_checkValidCondOps2a= verifyNot checkValidCondOps "[ 3 \\> 2 ]" +prop_checkValidCondOps2a = verifyNot checkValidCondOps "[ 3 \\> 2 ]" prop_checkValidCondOps3 = verifyNot checkValidCondOps "[ 1 = 2 -a 3 -ge 4 ]" prop_checkValidCondOps4 = verifyNot checkValidCondOps "[[ ! -v foo ]]" checkValidCondOps _ (TC_Binary id _ s _ _) @@ -1658,11 +1691,11 @@ checkTestRedirects _ _ = return () prop_checkPS11 = verify checkPS1Assignments "PS1='\\033[1;35m\\$ '" -prop_checkPS11a= verify checkPS1Assignments "export PS1='\\033[1;35m\\$ '" +prop_checkPS11a = verify checkPS1Assignments "export PS1='\\033[1;35m\\$ '" prop_checkPSf2 = verify checkPS1Assignments "PS1='\\h \\e[0m\\$ '" prop_checkPS13 = verify checkPS1Assignments "PS1=$'\\x1b[c '" prop_checkPS14 = verify checkPS1Assignments "PS1=$'\\e[3m; '" -prop_checkPS14a= verify checkPS1Assignments "export PS1=$'\\e[3m; '" +prop_checkPS14a = verify checkPS1Assignments "export PS1=$'\\e[3m; '" prop_checkPS15 = verifyNot checkPS1Assignments "PS1='\\[\\033[1;35m\\]\\$ '" prop_checkPS16 = verifyNot checkPS1Assignments "PS1='\\[\\e1m\\e[1m\\]\\$ '" prop_checkPS17 = verifyNot checkPS1Assignments "PS1='e033x1B'" @@ -1852,6 +1885,7 @@ prop_checkSpuriousExec8 = verifyNot checkSpuriousExec "exec {origout}>&1- >tmp.log 2>&1; bar" 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 where doLists (T_Script _ _ cmds) = doList cmds False @@ -1867,7 +1901,7 @@ stripCleanup = reverse . dropWhile cleanup . reverse cleanup (T_Pipeline _ _ [cmd]) = - isCommandMatch cmd (`elem` ["echo", "exit", "printf", "return"]) + isCommandMatch cmd (`elem` [":", "echo", "exit", "printf", "return"]) || isAssignment cmd cleanup _ = False @@ -1931,7 +1965,7 @@ prop_subshellAssignmentCheck4 = verifyNotTree subshellAssignmentCheck "( A=foo; rm $A; )" prop_subshellAssignmentCheck5 = verifyTree subshellAssignmentCheck "cat foo | while read cow; do true; done; echo $cow;" prop_subshellAssignmentCheck6 = verifyTree subshellAssignmentCheck "( export lol=$(ls); ); echo $lol;" -prop_subshellAssignmentCheck6a= verifyTree subshellAssignmentCheck "( typeset -a lol=a; ); echo $lol;" +prop_subshellAssignmentCheck6a = verifyTree subshellAssignmentCheck "( typeset -a lol=a; ); echo $lol;" prop_subshellAssignmentCheck7 = verifyTree subshellAssignmentCheck "cmd | while read foo; do (( n++ )); done; echo \"$n lines\"" prop_subshellAssignmentCheck8 = verifyTree subshellAssignmentCheck "n=3 & echo $((n++))" prop_subshellAssignmentCheck9 = verifyTree subshellAssignmentCheck "read n & n=foo$n" @@ -2001,107 +2035,6 @@ writeFunc base token name values doFlow _ = return [] ----- Check whether variables could have spaces/globs -prop_checkSpacefulness1 = verifyTree checkSpacefulness "a='cow moo'; echo $a" -prop_checkSpacefulness2 = verifyNotTree checkSpacefulness "a='cow moo'; [[ $a ]]" -prop_checkSpacefulness3 = verifyNotTree checkSpacefulness "a='cow*.mp3'; echo \"$a\"" -prop_checkSpacefulness4 = verifyTree checkSpacefulness "for f in *.mp3; do echo $f; done" -prop_checkSpacefulness4a= verifyNotTree checkSpacefulness "foo=3; foo=$(echo $foo)" -prop_checkSpacefulness5 = verifyTree checkSpacefulness "a='*'; b=$a; c=lol${b//foo/bar}; echo $c" -prop_checkSpacefulness6 = verifyTree checkSpacefulness "a=foo$(lol); echo $a" -prop_checkSpacefulness7 = verifyTree checkSpacefulness "a=foo\\ bar; rm $a" -prop_checkSpacefulness8 = verifyNotTree checkSpacefulness "a=foo\\ bar; a=foo; rm $a" -prop_checkSpacefulness10= verifyTree checkSpacefulness "rm $1" -prop_checkSpacefulness11= verifyTree checkSpacefulness "rm ${10//foo/bar}" -prop_checkSpacefulness12= verifyNotTree checkSpacefulness "(( $1 + 3 ))" -prop_checkSpacefulness13= verifyNotTree checkSpacefulness "if [[ $2 -gt 14 ]]; then true; fi" -prop_checkSpacefulness14= verifyNotTree checkSpacefulness "foo=$3 env" -prop_checkSpacefulness15= verifyNotTree checkSpacefulness "local foo=$1" -prop_checkSpacefulness16= verifyNotTree checkSpacefulness "declare foo=$1" -prop_checkSpacefulness17= verifyTree checkSpacefulness "echo foo=$1" -prop_checkSpacefulness18= verifyNotTree checkSpacefulness "$1 --flags" -prop_checkSpacefulness19= verifyTree checkSpacefulness "echo $PWD" -prop_checkSpacefulness20= verifyNotTree checkSpacefulness "n+='foo bar'" -prop_checkSpacefulness21= verifyNotTree checkSpacefulness "select foo in $bar; do true; done" -prop_checkSpacefulness22= verifyNotTree checkSpacefulness "echo $\"$1\"" -prop_checkSpacefulness23= verifyNotTree checkSpacefulness "a=(1); echo ${a[@]}" -prop_checkSpacefulness24= verifyTree checkSpacefulness "a='a b'; cat <<< $a" -prop_checkSpacefulness25= verifyTree checkSpacefulness "a='s/[0-9]//g'; sed $a" -prop_checkSpacefulness26= verifyTree checkSpacefulness "a='foo bar'; echo {1,2,$a}" -prop_checkSpacefulness27= verifyNotTree checkSpacefulness "echo ${a:+'foo'}" -prop_checkSpacefulness28= verifyNotTree checkSpacefulness "exec {n}>&1; echo $n" -prop_checkSpacefulness29= verifyNotTree checkSpacefulness "n=$(stuff); exec {n}>&-;" -prop_checkSpacefulness30= verifyTree checkSpacefulness "file='foo bar'; echo foo > $file;" -prop_checkSpacefulness31= verifyNotTree checkSpacefulness "echo \"`echo \\\"$1\\\"`\"" -prop_checkSpacefulness32= verifyNotTree checkSpacefulness "var=$1; [ -v var ]" -prop_checkSpacefulness33= verifyTree checkSpacefulness "for file; do echo $file; done" -prop_checkSpacefulness34= verifyTree checkSpacefulness "declare foo$n=$1" -prop_checkSpacefulness35= verifyNotTree checkSpacefulness "echo ${1+\"$1\"}" -prop_checkSpacefulness36= verifyNotTree checkSpacefulness "arg=$#; echo $arg" -prop_checkSpacefulness37= verifyNotTree checkSpacefulness "@test 'status' {\n [ $status -eq 0 ]\n}" -prop_checkSpacefulness37v = verifyTree checkVerboseSpacefulness "@test 'status' {\n [ $status -eq 0 ]\n}" -prop_checkSpacefulness38= verifyTree checkSpacefulness "a=; echo $a" -prop_checkSpacefulness39= verifyNotTree checkSpacefulness "a=''\"\"''; b=x$a; echo $b" -prop_checkSpacefulness40= verifyNotTree checkSpacefulness "a=$((x+1)); echo $a" -prop_checkSpacefulness41= verifyNotTree checkSpacefulness "exec $1 --flags" -prop_checkSpacefulness42= verifyNotTree checkSpacefulness "run $1 --flags" -prop_checkSpacefulness43= verifyNotTree checkSpacefulness "$foo=42" -prop_checkSpacefulness44= verifyTree checkSpacefulness "#!/bin/sh\nexport var=$value" -prop_checkSpacefulness45= verifyNotTree checkSpacefulness "wait -zzx -p foo; echo $foo" -prop_checkSpacefulness46= verifyNotTree checkSpacefulness "x=0; (( x += 1 )); echo $x" - -data SpaceStatus = SpaceSome | SpaceNone | SpaceEmpty deriving (Eq) -instance Semigroup SpaceStatus where - SpaceNone <> SpaceNone = SpaceNone - SpaceSome <> _ = SpaceSome - _ <> SpaceSome = SpaceSome - SpaceEmpty <> x = x - x <> SpaceEmpty = x -instance Monoid SpaceStatus where - mempty = SpaceEmpty - mappend = (<>) - --- This is slightly awkward because we want to support structured --- optional checks based on nearly the same logic -checkSpacefulness params = checkSpacefulness' onFind params - where - emit x = tell [x] - onFind spaces token _ = - when (spaces /= SpaceNone) $ - if isDefaultAssignment (parentMap params) token - then - emit $ makeComment InfoC (getId token) 2223 - "This default assignment may cause DoS due to globbing. Quote it." - else - unless (quotesMayConflictWithSC2281 params token) $ - emit $ makeCommentWithFix InfoC (getId token) 2086 - "Double quote to prevent globbing and word splitting." - (addDoubleQuotesAround params token) - - isDefaultAssignment parents token = - let modifier = getBracedModifier $ bracedString token in - any (`isPrefixOf` modifier) ["=", ":="] - && isParamTo parents ":" token - - -- Given a T_DollarBraced, return a simplified version of the string contents. - bracedString (T_DollarBraced _ _ l) = concat $ oversimplify l - bracedString _ = error "Internal shellcheck error, please report! (bracedString on non-variable)" - -prop_checkSpacefulness4v= verifyTree checkVerboseSpacefulness "foo=3; foo=$(echo $foo)" -prop_checkSpacefulness8v= verifyTree checkVerboseSpacefulness "a=foo\\ bar; a=foo; rm $a" -prop_checkSpacefulness28v = verifyTree checkVerboseSpacefulness "exec {n}>&1; echo $n" -prop_checkSpacefulness36v = verifyTree checkVerboseSpacefulness "arg=$#; echo $arg" -prop_checkSpacefulness44v = verifyNotTree checkVerboseSpacefulness "foo=3; $foo=4" -checkVerboseSpacefulness params = checkSpacefulness' onFind params - where - onFind spaces token name = - when (spaces == SpaceNone - && name `notElem` specialVariablesWithoutSpaces - && not (quotesMayConflictWithSC2281 params token)) $ - tell [makeCommentWithFix StyleC (getId token) 2248 - "Prefer double quoting even when variables don't contain special characters." - (addDoubleQuotesAround params token)] - -- Don't suggest quotes if this will instead be autocorrected -- from $foo=bar to foo=bar. This is not pretty but ok. quotesMayConflictWithSC2281 params t = @@ -2111,76 +2044,126 @@ _ -> False addDoubleQuotesAround params token = (surroundWith (getId token) params "\"") -checkSpacefulness' - :: (SpaceStatus -> Token -> String -> Writer [TokenComment] ()) -> - Parameters -> Token -> [TokenComment] -checkSpacefulness' onFind params t = - doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params) - where - defaults = zip variablesWithoutSpaces (repeat SpaceNone) - hasSpaces name = gets (Map.findWithDefault SpaceSome name) +prop_checkSpacefulnessCfg1 = verify checkSpacefulnessCfg "a='cow moo'; echo $a" +prop_checkSpacefulnessCfg2 = verifyNot checkSpacefulnessCfg "a='cow moo'; [[ $a ]]" +prop_checkSpacefulnessCfg3 = verifyNot checkSpacefulnessCfg "a='cow*.mp3'; echo \"$a\"" +prop_checkSpacefulnessCfg4 = verify checkSpacefulnessCfg "for f in *.mp3; do echo $f; done" +prop_checkSpacefulnessCfg4a = verifyNot checkSpacefulnessCfg "foo=3; foo=$(echo $foo)" +prop_checkSpacefulnessCfg5 = verify checkSpacefulnessCfg "a='*'; b=$a; c=lol${b//foo/bar}; echo $c" +prop_checkSpacefulnessCfg6 = verify checkSpacefulnessCfg "a=foo$(lol); echo $a" +prop_checkSpacefulnessCfg7 = verify checkSpacefulnessCfg "a=foo\\ bar; rm $a" +prop_checkSpacefulnessCfg8 = verifyNot checkSpacefulnessCfg "a=foo\\ bar; a=foo; rm $a" +prop_checkSpacefulnessCfg10 = verify checkSpacefulnessCfg "rm $1" +prop_checkSpacefulnessCfg11 = verify checkSpacefulnessCfg "rm ${10//foo/bar}" +prop_checkSpacefulnessCfg12 = verifyNot checkSpacefulnessCfg "(( $1 + 3 ))" +prop_checkSpacefulnessCfg13 = verifyNot checkSpacefulnessCfg "if [[ $2 -gt 14 ]]; then true; fi" +prop_checkSpacefulnessCfg14 = verifyNot checkSpacefulnessCfg "foo=$3 env" +prop_checkSpacefulnessCfg15 = verifyNot checkSpacefulnessCfg "local foo=$1" +prop_checkSpacefulnessCfg16 = verifyNot checkSpacefulnessCfg "declare foo=$1" +prop_checkSpacefulnessCfg17 = verify checkSpacefulnessCfg "echo foo=$1" +prop_checkSpacefulnessCfg18 = verifyNot checkSpacefulnessCfg "$1 --flags" +prop_checkSpacefulnessCfg19 = verify checkSpacefulnessCfg "echo $PWD" +prop_checkSpacefulnessCfg20 = verifyNot checkSpacefulnessCfg "n+='foo bar'" +prop_checkSpacefulnessCfg21 = verifyNot checkSpacefulnessCfg "select foo in $bar; do true; done" +prop_checkSpacefulnessCfg22 = verifyNot checkSpacefulnessCfg "echo $\"$1\"" +prop_checkSpacefulnessCfg23 = verifyNot checkSpacefulnessCfg "a=(1); echo ${a[@]}" +prop_checkSpacefulnessCfg24 = verify checkSpacefulnessCfg "a='a b'; cat <<< $a" +prop_checkSpacefulnessCfg25 = verify checkSpacefulnessCfg "a='s/[0-9]//g'; sed $a" +prop_checkSpacefulnessCfg26 = verify checkSpacefulnessCfg "a='foo bar'; echo {1,2,$a}" +prop_checkSpacefulnessCfg27 = verifyNot checkSpacefulnessCfg "echo ${a:+'foo'}" +prop_checkSpacefulnessCfg28 = verifyNot checkSpacefulnessCfg "exec {n}>&1; echo $n" +prop_checkSpacefulnessCfg29 = verifyNot checkSpacefulnessCfg "n=$(stuff); exec {n}>&-;" +prop_checkSpacefulnessCfg30 = verify checkSpacefulnessCfg "file='foo bar'; echo foo > $file;" +prop_checkSpacefulnessCfg31 = verifyNot checkSpacefulnessCfg "echo \"`echo \\\"$1\\\"`\"" +prop_checkSpacefulnessCfg32 = verifyNot checkSpacefulnessCfg "var=$1; [ -v var ]" +prop_checkSpacefulnessCfg33 = verify checkSpacefulnessCfg "for file; do echo $file; done" +prop_checkSpacefulnessCfg34 = verify checkSpacefulnessCfg "declare foo$n=$1" +prop_checkSpacefulnessCfg35 = verifyNot checkSpacefulnessCfg "echo ${1+\"$1\"}" +prop_checkSpacefulnessCfg36 = verifyNot checkSpacefulnessCfg "arg=$#; echo $arg" +prop_checkSpacefulnessCfg37 = verifyNot checkSpacefulnessCfg "@test 'status' {\n [ $status -eq 0 ]\n}" +prop_checkSpacefulnessCfg37v = verify checkVerboseSpacefulnessCfg "@test 'status' {\n [ $status -eq 0 ]\n}" +prop_checkSpacefulnessCfg38 = verify checkSpacefulnessCfg "a=; echo $a" +prop_checkSpacefulnessCfg39 = verifyNot checkSpacefulnessCfg "a=''\"\"''; b=x$a; echo $b" +prop_checkSpacefulnessCfg40 = verifyNot checkSpacefulnessCfg "a=$((x+1)); echo $a" +prop_checkSpacefulnessCfg41 = verifyNot checkSpacefulnessCfg "exec $1 --flags" +prop_checkSpacefulnessCfg42 = verifyNot checkSpacefulnessCfg "run $1 --flags" +prop_checkSpacefulnessCfg43 = verifyNot checkSpacefulnessCfg "$foo=42" +prop_checkSpacefulnessCfg44 = verify checkSpacefulnessCfg "#!/bin/sh\nexport var=$value" +prop_checkSpacefulnessCfg45 = verifyNot checkSpacefulnessCfg "wait -zzx -p foo; echo $foo" +prop_checkSpacefulnessCfg46 = verifyNot checkSpacefulnessCfg "x=0; (( x += 1 )); echo $x" +prop_checkSpacefulnessCfg47 = verifyNot checkSpacefulnessCfg "x=0; (( x-- )); echo $x" +prop_checkSpacefulnessCfg48 = verifyNot checkSpacefulnessCfg "x=0; (( ++x )); echo $x" +prop_checkSpacefulnessCfg49 = verifyNot checkSpacefulnessCfg "for i in 1 2 3; do echo $i; done" +prop_checkSpacefulnessCfg50 = verify checkSpacefulnessCfg "for i in 1 2 *; do echo $i; done" +prop_checkSpacefulnessCfg51 = verify checkSpacefulnessCfg "x='foo bar'; x && x=1; echo $x" +prop_checkSpacefulnessCfg52 = verifyNot checkSpacefulnessCfg "x=1; if f; then x='foo bar'; exit; fi; echo $x" +prop_checkSpacefulnessCfg53 = verifyNot checkSpacefulnessCfg "s=1; f() { local s='a b'; }; f; echo $s" +prop_checkSpacefulnessCfg54 = verifyNot checkSpacefulnessCfg "s='a b'; f() { s=1; }; f; echo $s" +prop_checkSpacefulnessCfg55 = verify checkSpacefulnessCfg "s='a b'; x && f() { s=1; }; f; echo $s" +prop_checkSpacefulnessCfg56 = verifyNot checkSpacefulnessCfg "s=1; cat <(s='a b'); echo $s" +prop_checkSpacefulnessCfg57 = verifyNot checkSpacefulnessCfg "declare -i s=0; s=$(f); echo $s" +prop_checkSpacefulnessCfg58 = verify checkSpacefulnessCfg "f() { declare -i s; }; f; s=$(var); echo $s" +prop_checkSpacefulnessCfg59 = verifyNot checkSpacefulnessCfg "f() { declare -gi s; }; f; s=$(var); echo $s" +prop_checkSpacefulnessCfg60 = verify checkSpacefulnessCfg "declare -i s; declare +i s; s=$(foo); echo $s" +prop_checkSpacefulnessCfg61 = verify checkSpacefulnessCfg "declare -x X; y=foo$X; echo $y;" +prop_checkSpacefulnessCfg62 = verifyNot checkSpacefulnessCfg "f() { declare -x X; y=foo$X; echo $y; }" +prop_checkSpacefulnessCfg63 = verify checkSpacefulnessCfg "f && declare -i s; s='x + y'; echo $s" +prop_checkSpacefulnessCfg64 = verifyNot checkSpacefulnessCfg "declare -i s; s='x + y'; x=$s; echo $x" +prop_checkSpacefulnessCfg65 = verifyNot checkSpacefulnessCfg "f() { s=$?; echo $s; }; f" +prop_checkSpacefulnessCfg66 = verifyNot checkSpacefulnessCfg "f() { s=$?; echo $s; }" - setSpaces name status = - modify $ Map.insert name status +checkSpacefulnessCfg = checkSpacefulnessCfg' True +checkVerboseSpacefulnessCfg = checkSpacefulnessCfg' False - readF _ token name = do - spaces <- hasSpaces name - let needsQuoting = - isExpansion token - && not (isArrayExpansion token) -- There's another warning for this - && not (isCountingReference token) - && not (isQuoteFree (shellType params) parents token) - && not (isQuotedAlternativeReference token) - && not (usedAsCommandName parents token) +checkSpacefulnessCfg' :: Bool -> (Parameters -> Token -> Writer [TokenComment] ()) +checkSpacefulnessCfg' dirtyPass params token@(T_DollarBraced id _ list) = + when (needsQuoting && (dirtyPass == not isClean)) $ + unless (name `elem` specialVariablesWithoutSpaces || quotesMayConflictWithSC2281 params token) $ + if dirtyPass + then + if isDefaultAssignment (parentMap params) token + then + info (getId token) 2223 + "This default assignment may cause DoS due to globbing. Quote it." + else + infoWithFix id 2086 "Double quote to prevent globbing and word splitting." $ + addDoubleQuotesAround params token + else + styleWithFix id 2248 "Prefer double quoting even when variables don't contain special characters." $ + addDoubleQuotesAround params token - return . execWriter $ when needsQuoting $ onFind spaces token name + where + name = getBracedReference $ concat $ oversimplify list + parents = parentMap params + needsQuoting = + not (isArrayExpansion token) -- There's another warning for this + && not (isCountingReference token) + && not (isQuoteFree (shellType params) parents token) + && not (isQuotedAlternativeReference token) + && not (usedAsCommandName parents token) - where - emit x = tell [x] + isClean = fromMaybe False $ do + state <- CF.getIncomingState (cfgAnalysis params) id + value <- Map.lookup name $ CF.variablesInScope state + return $ isCleanState value - writeF _ (TA_Assignment {}) name _ = setSpaces name SpaceNone >> return [] - writeF _ _ name (DataString SourceExternal) = setSpaces name SpaceSome >> return [] - writeF _ _ name (DataString SourceInteger) = setSpaces name SpaceNone >> return [] + isCleanState state = + (all (S.member CFVPInteger) $ CF.variableProperties state) + || CF.spaceStatus (CF.variableValue state) == CF.SpaceStatusClean - writeF _ _ name (DataString (SourceFrom vals)) = do - map <- get - setSpaces name - (isSpacefulWord (\x -> Map.findWithDefault SpaceSome x map) vals) - return [] + isDefaultAssignment parents token = + let modifier = getBracedModifier $ bracedString token in + any (`isPrefixOf` modifier) ["=", ":="] + && isParamTo parents ":" token - writeF _ _ _ _ = return [] + -- Given a T_DollarBraced, return a simplified version of the string contents. + bracedString (T_DollarBraced _ _ l) = concat $ oversimplify l + bracedString _ = error $ pleaseReport "bracedString on non-variable" - parents = parentMap params +checkSpacefulnessCfg' _ _ _ = return () - isExpansion t = - case t of - (T_DollarBraced _ _ _ ) -> True - _ -> False - isSpacefulWord :: (String -> SpaceStatus) -> [Token] -> SpaceStatus - isSpacefulWord f = mconcat . map (isSpaceful f) - isSpaceful :: (String -> SpaceStatus) -> Token -> SpaceStatus - isSpaceful spacefulF x = - case x of - T_DollarExpansion _ _ -> SpaceSome - T_Backticked _ _ -> SpaceSome - T_Glob _ _ -> SpaceSome - T_Extglob {} -> SpaceSome - T_DollarArithmetic _ _ -> SpaceNone - T_Literal _ s -> fromLiteral s - T_SingleQuoted _ s -> fromLiteral s - T_DollarBraced _ _ l -> spacefulF $ getBracedReference $ concat $ oversimplify l - T_NormalWord _ w -> isSpacefulWord spacefulF w - T_DoubleQuoted _ w -> isSpacefulWord spacefulF w - _ -> SpaceEmpty - where - globspace = "*?[] \t\n" - containsAny s = any (`elem` s) - fromLiteral "" = SpaceEmpty - fromLiteral s | s `containsAny` globspace = SpaceSome - fromLiteral _ = SpaceNone - prop_CheckVariableBraces1 = verify checkVariableBraces "a='123'; echo $a" prop_CheckVariableBraces2 = verifyNot checkVariableBraces "a='123'; echo ${a}" prop_CheckVariableBraces3 = verifyNot checkVariableBraces "#shellcheck disable=SC2016\necho '$a'" @@ -2198,13 +2181,13 @@ checkVariableBraces _ _ = return () prop_checkQuotesInLiterals1 = verifyTree checkQuotesInLiterals "param='--foo=\"bar\"'; app $param" -prop_checkQuotesInLiterals1a= verifyTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; app $param" +prop_checkQuotesInLiterals1a = verifyTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; app $param" prop_checkQuotesInLiterals2 = verifyNotTree checkQuotesInLiterals "param='--foo=\"bar\"'; app \"$param\"" prop_checkQuotesInLiterals3 =verifyNotTree checkQuotesInLiterals "param=('--foo='); app \"${param[@]}\"" prop_checkQuotesInLiterals4 = verifyNotTree checkQuotesInLiterals "param=\"don't bother with this one\"; app $param" prop_checkQuotesInLiterals5 = verifyNotTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; eval app $param" prop_checkQuotesInLiterals6 = verifyTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm $param\"; $cmd" -prop_checkQuotesInLiterals6a= verifyNotTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm ${#param}\"; $cmd" +prop_checkQuotesInLiterals6a = verifyNotTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm ${#param}\"; $cmd" prop_checkQuotesInLiterals7 = verifyTree checkQuotesInLiterals "param='my\\ file'; rm $param" prop_checkQuotesInLiterals8 = verifyTree checkQuotesInLiterals "param=\"/foo/'bar baz'/etc\"; rm $param" prop_checkQuotesInLiterals9 = verifyNotTree checkQuotesInLiterals "param=\"/foo/'bar baz'/etc\"; rm ${#param}" @@ -2268,9 +2251,9 @@ verifyTree checkFunctionsUsedExternally "foo() { :; }; sudo foo" prop_checkFunctionsUsedExternally2 = verifyTree checkFunctionsUsedExternally "alias f='a'; xargs -0 f" -prop_checkFunctionsUsedExternally2b= +prop_checkFunctionsUsedExternally2b = verifyNotTree checkFunctionsUsedExternally "alias f='a'; find . -type f" -prop_checkFunctionsUsedExternally2c= +prop_checkFunctionsUsedExternally2c = verifyTree checkFunctionsUsedExternally "alias f='a'; find . -type f -exec f +" prop_checkFunctionsUsedExternally3 = verifyNotTree checkFunctionsUsedExternally "f() { :; }; echo f" @@ -2296,7 +2279,7 @@ let args = skipOver t argv let argStrings = map (\x -> (fromMaybe "" $ getLiteralString x, x)) args let candidates = getPotentialCommands name argStrings - mapM_ (checkArg name) candidates + mapM_ (checkArg name (getId t)) candidates _ -> return () checkCommand _ _ = return () @@ -2322,14 +2305,19 @@ functionsAndAliases = Map.union (functions t) (aliases t) - checkArg cmd (_, arg) = sequence_ $ do + patternContext id = + case posLine . fst <$> Map.lookup id (tokenPositions params) of + Just l -> " on line " <> show l <> "." + _ -> "." + + checkArg cmd cmdId (_, arg) = sequence_ $ do literalArg <- getUnquotedLiteral arg -- only consider unquoted literals definitionId <- Map.lookup literalArg functionsAndAliases return $ do warn (getId arg) 2033 - "Shell functions can't be passed to external commands." + "Shell functions can't be passed to external commands. Use separate script or sh -c." info definitionId 2032 $ - "Use own script or sh -c '..' to run this from " ++ cmd ++ "." + "This function can't be invoked via " ++ cmd ++ patternContext cmdId prop_checkUnused0 = verifyNotTree checkUnusedAssignments "var=foo; echo $var" prop_checkUnused1 = verifyTree checkUnusedAssignments "var=foo; echo $bar" @@ -2341,48 +2329,49 @@ prop_checkUnused7 = verifyNotTree checkUnusedAssignments "var=2; $((var))" prop_checkUnused8 = verifyTree checkUnusedAssignments "var=2; var=3;" prop_checkUnused9 = verifyNotTree checkUnusedAssignments "read ''" -prop_checkUnused10= verifyNotTree checkUnusedAssignments "read -p 'test: '" -prop_checkUnused11= verifyNotTree checkUnusedAssignments "bar=5; export foo[$bar]=3" -prop_checkUnused12= verifyNotTree checkUnusedAssignments "read foo; echo ${!foo}" -prop_checkUnused13= verifyNotTree checkUnusedAssignments "x=(1); (( x[0] ))" -prop_checkUnused14= verifyNotTree checkUnusedAssignments "x=(1); n=0; echo ${x[n]}" -prop_checkUnused15= verifyNotTree checkUnusedAssignments "x=(1); n=0; (( x[n] ))" -prop_checkUnused16= verifyNotTree checkUnusedAssignments "foo=5; declare -x foo" -prop_checkUnused16b= verifyNotTree checkUnusedAssignments "f() { local -x foo; foo=42; bar; }; f" -prop_checkUnused17= verifyNotTree checkUnusedAssignments "read -i 'foo' -e -p 'Input: ' bar; $bar;" -prop_checkUnused18= verifyNotTree checkUnusedAssignments "a=1; arr=( [$a]=42 ); echo \"${arr[@]}\"" -prop_checkUnused19= verifyNotTree checkUnusedAssignments "a=1; let b=a+1; echo $b" -prop_checkUnused20= verifyNotTree checkUnusedAssignments "a=1; PS1='$a'" -prop_checkUnused21= verifyNotTree checkUnusedAssignments "a=1; trap 'echo $a' INT" -prop_checkUnused22= verifyNotTree checkUnusedAssignments "a=1; [ -v a ]" -prop_checkUnused23= verifyNotTree checkUnusedAssignments "a=1; [ -R a ]" -prop_checkUnused24= verifyNotTree checkUnusedAssignments "mapfile -C a b; echo ${b[@]}" -prop_checkUnused25= verifyNotTree checkUnusedAssignments "readarray foo; echo ${foo[@]}" -prop_checkUnused26= verifyNotTree checkUnusedAssignments "declare -F foo" -prop_checkUnused27= verifyTree checkUnusedAssignments "var=3; [ var -eq 3 ]" -prop_checkUnused28= verifyNotTree checkUnusedAssignments "var=3; [[ var -eq 3 ]]" -prop_checkUnused29= verifyNotTree checkUnusedAssignments "var=(a b); declare -p var" -prop_checkUnused30= verifyTree checkUnusedAssignments "let a=1" -prop_checkUnused31= verifyTree checkUnusedAssignments "let 'a=1'" -prop_checkUnused32= verifyTree checkUnusedAssignments "let a=b=c; echo $a" -prop_checkUnused33= verifyNotTree checkUnusedAssignments "a=foo; [[ foo =~ ^{$a}$ ]]" -prop_checkUnused34= verifyNotTree checkUnusedAssignments "foo=1; (( t = foo )); echo $t" -prop_checkUnused35= verifyNotTree checkUnusedAssignments "a=foo; b=2; echo ${a:b}" -prop_checkUnused36= verifyNotTree checkUnusedAssignments "if [[ -v foo ]]; then true; fi" -prop_checkUnused37= verifyNotTree checkUnusedAssignments "fd=2; exec {fd}>&-" -prop_checkUnused38= verifyTree checkUnusedAssignments "(( a=42 ))" -prop_checkUnused39= verifyNotTree checkUnusedAssignments "declare -x -f foo" -prop_checkUnused40= verifyNotTree checkUnusedAssignments "arr=(1 2); num=2; echo \"${arr[@]:num}\"" -prop_checkUnused41= verifyNotTree checkUnusedAssignments "@test 'foo' {\ntrue\n}\n" -prop_checkUnused42= verifyNotTree checkUnusedAssignments "DEFINE_string foo '' ''; echo \"${FLAGS_foo}\"" -prop_checkUnused43= verifyTree checkUnusedAssignments "DEFINE_string foo '' ''" -prop_checkUnused44= verifyNotTree checkUnusedAssignments "DEFINE_string \"foo$ibar\" x y" -prop_checkUnused45= verifyTree checkUnusedAssignments "readonly foo=bar" -prop_checkUnused46= verifyTree checkUnusedAssignments "readonly foo=(bar)" -prop_checkUnused47= verifyNotTree checkUnusedAssignments "a=1; alias hello='echo $a'" -prop_checkUnused48= verifyNotTree checkUnusedAssignments "_a=1" -prop_checkUnused49= verifyNotTree checkUnusedAssignments "declare -A array; key=a; [[ -v array[$key] ]]" -prop_checkUnused50= verifyNotTree checkUnusedAssignments "foofunc() { :; }; typeset -fx foofunc" +prop_checkUnused10 = verifyNotTree checkUnusedAssignments "read -p 'test: '" +prop_checkUnused11 = verifyNotTree checkUnusedAssignments "bar=5; export foo[$bar]=3" +prop_checkUnused12 = verifyNotTree checkUnusedAssignments "read foo; echo ${!foo}" +prop_checkUnused13 = verifyNotTree checkUnusedAssignments "x=(1); (( x[0] ))" +prop_checkUnused14 = verifyNotTree checkUnusedAssignments "x=(1); n=0; echo ${x[n]}" +prop_checkUnused15 = verifyNotTree checkUnusedAssignments "x=(1); n=0; (( x[n] ))" +prop_checkUnused16 = verifyNotTree checkUnusedAssignments "foo=5; declare -x foo" +prop_checkUnused16b = verifyNotTree checkUnusedAssignments "f() { local -x foo; foo=42; bar; }; f" +prop_checkUnused17 = verifyNotTree checkUnusedAssignments "read -i 'foo' -e -p 'Input: ' bar; $bar;" +prop_checkUnused18 = verifyNotTree checkUnusedAssignments "a=1; arr=( [$a]=42 ); echo \"${arr[@]}\"" +prop_checkUnused19 = verifyNotTree checkUnusedAssignments "a=1; let b=a+1; echo $b" +prop_checkUnused20 = verifyNotTree checkUnusedAssignments "a=1; PS1='$a'" +prop_checkUnused21 = verifyNotTree checkUnusedAssignments "a=1; trap 'echo $a' INT" +prop_checkUnused22 = verifyNotTree checkUnusedAssignments "a=1; [ -v a ]" +prop_checkUnused23 = verifyNotTree checkUnusedAssignments "a=1; [ -R a ]" +prop_checkUnused24 = verifyNotTree checkUnusedAssignments "mapfile -C a b; echo ${b[@]}" +prop_checkUnused25 = verifyNotTree checkUnusedAssignments "readarray foo; echo ${foo[@]}" +prop_checkUnused26 = verifyNotTree checkUnusedAssignments "declare -F foo" +prop_checkUnused27 = verifyTree checkUnusedAssignments "var=3; [ var -eq 3 ]" +prop_checkUnused28 = verifyNotTree checkUnusedAssignments "var=3; [[ var -eq 3 ]]" +prop_checkUnused29 = verifyNotTree checkUnusedAssignments "var=(a b); declare -p var" +prop_checkUnused30 = verifyTree checkUnusedAssignments "let a=1" +prop_checkUnused31 = verifyTree checkUnusedAssignments "let 'a=1'" +prop_checkUnused32 = verifyTree checkUnusedAssignments "let a=b=c; echo $a" +prop_checkUnused33 = verifyNotTree checkUnusedAssignments "a=foo; [[ foo =~ ^{$a}$ ]]" +prop_checkUnused34 = verifyNotTree checkUnusedAssignments "foo=1; (( t = foo )); echo $t" +prop_checkUnused35 = verifyNotTree checkUnusedAssignments "a=foo; b=2; echo ${a:b}" +prop_checkUnused36 = verifyNotTree checkUnusedAssignments "if [[ -v foo ]]; then true; fi" +prop_checkUnused37 = verifyNotTree checkUnusedAssignments "fd=2; exec {fd}>&-" +prop_checkUnused38 = verifyTree checkUnusedAssignments "(( a=42 ))" +prop_checkUnused39 = verifyNotTree checkUnusedAssignments "declare -x -f foo" +prop_checkUnused40 = verifyNotTree checkUnusedAssignments "arr=(1 2); num=2; echo \"${arr[@]:num}\"" +prop_checkUnused41 = verifyNotTree checkUnusedAssignments "@test 'foo' {\ntrue\n}\n" +prop_checkUnused42 = verifyNotTree checkUnusedAssignments "DEFINE_string foo '' ''; echo \"${FLAGS_foo}\"" +prop_checkUnused43 = verifyTree checkUnusedAssignments "DEFINE_string foo '' ''" +prop_checkUnused44 = verifyNotTree checkUnusedAssignments "DEFINE_string \"foo$ibar\" x y" +prop_checkUnused45 = verifyTree checkUnusedAssignments "readonly foo=bar" +prop_checkUnused46 = verifyTree checkUnusedAssignments "readonly foo=(bar)" +prop_checkUnused47 = verifyNotTree checkUnusedAssignments "a=1; alias hello='echo $a'" +prop_checkUnused48 = verifyNotTree checkUnusedAssignments "_a=1" +prop_checkUnused49 = verifyNotTree checkUnusedAssignments "declare -A array; key=a; [[ -v array[$key] ]]" +prop_checkUnused50 = verifyNotTree checkUnusedAssignments "foofunc() { :; }; typeset -fx foofunc" +prop_checkUnused51 = verifyTree checkUnusedAssignments "x[y[z=1]]=1; echo ${x[@]}" checkUnusedAssignments params t = execWriter (mapM_ warnFor unused) where @@ -2416,40 +2405,40 @@ prop_checkUnassignedReferences7 = verifyNotTree checkUnassignedReferences "getopts ':h' foo; echo $foo" prop_checkUnassignedReferences8 = verifyNotTree checkUnassignedReferences "let 'foo = 1'; echo $foo" prop_checkUnassignedReferences9 = verifyNotTree checkUnassignedReferences "echo ${foo-bar}" -prop_checkUnassignedReferences10= verifyNotTree checkUnassignedReferences "echo ${foo:?}" -prop_checkUnassignedReferences11= verifyNotTree checkUnassignedReferences "declare -A foo; echo \"${foo[@]}\"" -prop_checkUnassignedReferences12= verifyNotTree checkUnassignedReferences "typeset -a foo; echo \"${foo[@]}\"" -prop_checkUnassignedReferences13= verifyNotTree checkUnassignedReferences "f() { local foo; echo $foo; }" -prop_checkUnassignedReferences14= verifyNotTree checkUnassignedReferences "foo=; echo $foo" -prop_checkUnassignedReferences15= verifyNotTree checkUnassignedReferences "f() { true; }; export -f f" -prop_checkUnassignedReferences16= verifyNotTree checkUnassignedReferences "declare -A foo=( [a b]=bar ); echo ${foo[a b]}" -prop_checkUnassignedReferences17= verifyNotTree checkUnassignedReferences "USERS=foo; echo $USER" -prop_checkUnassignedReferences18= verifyNotTree checkUnassignedReferences "FOOBAR=42; export FOOBAR=" -prop_checkUnassignedReferences19= verifyNotTree checkUnassignedReferences "readonly foo=bar; echo $foo" -prop_checkUnassignedReferences20= verifyNotTree checkUnassignedReferences "printf -v foo bar; echo $foo" -prop_checkUnassignedReferences21= verifyTree checkUnassignedReferences "echo ${#foo}" -prop_checkUnassignedReferences22= verifyNotTree checkUnassignedReferences "echo ${!os*}" -prop_checkUnassignedReferences23= verifyTree checkUnassignedReferences "declare -a foo; foo[bar]=42;" -prop_checkUnassignedReferences24= verifyNotTree checkUnassignedReferences "declare -A foo; foo[bar]=42;" -prop_checkUnassignedReferences25= verifyNotTree checkUnassignedReferences "declare -A foo=(); foo[bar]=42;" -prop_checkUnassignedReferences26= verifyNotTree checkUnassignedReferences "a::b() { foo; }; readonly -f a::b" -prop_checkUnassignedReferences27= verifyNotTree checkUnassignedReferences ": ${foo:=bar}" -prop_checkUnassignedReferences28= verifyNotTree checkUnassignedReferences "#!/bin/ksh\necho \"${.sh.version}\"\n" -prop_checkUnassignedReferences29= verifyNotTree checkUnassignedReferences "if [[ -v foo ]]; then echo $foo; fi" -prop_checkUnassignedReferences30= verifyNotTree checkUnassignedReferences "if [[ -v foo[3] ]]; then echo ${foo[3]}; fi" -prop_checkUnassignedReferences31= verifyNotTree checkUnassignedReferences "X=1; if [[ -v foo[$X+42] ]]; then echo ${foo[$X+42]}; fi" -prop_checkUnassignedReferences32= verifyNotTree checkUnassignedReferences "if [[ -v \"foo[1]\" ]]; then echo ${foo[@]}; fi" -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\"" -prop_checkUnassignedReferences37= verifyNotTree checkUnassignedReferences "var=howdy; printf -v 'array[0]' %s \"$var\"; printf %s \"${array[0]}\";" -prop_checkUnassignedReferences38= verifyTree (checkUnassignedReferences' True) "echo $VAR" -prop_checkUnassignedReferences39= verifyNotTree checkUnassignedReferences "builtin export var=4; echo $var" -prop_checkUnassignedReferences40= verifyNotTree checkUnassignedReferences ": ${foo=bar}" -prop_checkUnassignedReferences41= verifyNotTree checkUnassignedReferences "mapfile -t files 123; echo \"${files[@]}\"" -prop_checkUnassignedReferences42= verifyNotTree checkUnassignedReferences "mapfile files -t; echo \"${files[@]}\"" -prop_checkUnassignedReferences43= verifyNotTree checkUnassignedReferences "mapfile --future files; echo \"${files[@]}\"" +prop_checkUnassignedReferences10 = verifyNotTree checkUnassignedReferences "echo ${foo:?}" +prop_checkUnassignedReferences11 = verifyNotTree checkUnassignedReferences "declare -A foo; echo \"${foo[@]}\"" +prop_checkUnassignedReferences12 = verifyNotTree checkUnassignedReferences "typeset -a foo; echo \"${foo[@]}\"" +prop_checkUnassignedReferences13 = verifyNotTree checkUnassignedReferences "f() { local foo; echo $foo; }" +prop_checkUnassignedReferences14 = verifyNotTree checkUnassignedReferences "foo=; echo $foo" +prop_checkUnassignedReferences15 = verifyNotTree checkUnassignedReferences "f() { true; }; export -f f" +prop_checkUnassignedReferences16 = verifyNotTree checkUnassignedReferences "declare -A foo=( [a b]=bar ); echo ${foo[a b]}" +prop_checkUnassignedReferences17 = verifyNotTree checkUnassignedReferences "USERS=foo; echo $USER" +prop_checkUnassignedReferences18 = verifyNotTree checkUnassignedReferences "FOOBAR=42; export FOOBAR=" +prop_checkUnassignedReferences19 = verifyNotTree checkUnassignedReferences "readonly foo=bar; echo $foo" +prop_checkUnassignedReferences20 = verifyNotTree checkUnassignedReferences "printf -v foo bar; echo $foo" +prop_checkUnassignedReferences21 = verifyTree checkUnassignedReferences "echo ${#foo}" +prop_checkUnassignedReferences22 = verifyNotTree checkUnassignedReferences "echo ${!os*}" +prop_checkUnassignedReferences23 = verifyTree checkUnassignedReferences "declare -a foo; foo[bar]=42;" +prop_checkUnassignedReferences24 = verifyNotTree checkUnassignedReferences "declare -A foo; foo[bar]=42;" +prop_checkUnassignedReferences25 = verifyNotTree checkUnassignedReferences "declare -A foo=(); foo[bar]=42;" +prop_checkUnassignedReferences26 = verifyNotTree checkUnassignedReferences "a::b() { foo; }; readonly -f a::b" +prop_checkUnassignedReferences27 = verifyNotTree checkUnassignedReferences ": ${foo:=bar}" +prop_checkUnassignedReferences28 = verifyNotTree checkUnassignedReferences "#!/bin/ksh\necho \"${.sh.version}\"\n" +prop_checkUnassignedReferences29 = verifyNotTree checkUnassignedReferences "if [[ -v foo ]]; then echo $foo; fi" +prop_checkUnassignedReferences30 = verifyNotTree checkUnassignedReferences "if [[ -v foo[3] ]]; then echo ${foo[3]}; fi" +prop_checkUnassignedReferences31 = verifyNotTree checkUnassignedReferences "X=1; if [[ -v foo[$X+42] ]]; then echo ${foo[$X+42]}; fi" +prop_checkUnassignedReferences32 = verifyNotTree checkUnassignedReferences "if [[ -v \"foo[1]\" ]]; then echo ${foo[@]}; fi" +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\"" +prop_checkUnassignedReferences37 = verifyNotTree checkUnassignedReferences "var=howdy; printf -v 'array[0]' %s \"$var\"; printf %s \"${array[0]}\";" +prop_checkUnassignedReferences38 = verifyTree (checkUnassignedReferences' True) "echo $VAR" +prop_checkUnassignedReferences39 = verifyNotTree checkUnassignedReferences "builtin export var=4; echo $var" +prop_checkUnassignedReferences40 = verifyNotTree checkUnassignedReferences ": ${foo=bar}" +prop_checkUnassignedReferences41 = verifyNotTree checkUnassignedReferences "mapfile -t files 123; echo \"${files[@]}\"" +prop_checkUnassignedReferences42 = verifyNotTree checkUnassignedReferences "mapfile files -t; echo \"${files[@]}\"" +prop_checkUnassignedReferences43 = verifyNotTree checkUnassignedReferences "mapfile --future files; echo \"${files[@]}\"" prop_checkUnassignedReferences_minusNPlain = verifyNotTree checkUnassignedReferences "if [ -n \"$x\" ]; then echo $x; fi" prop_checkUnassignedReferences_minusZPlain = verifyNotTree checkUnassignedReferences "if [ -z \"$x\" ]; then echo \"\"; fi" prop_checkUnassignedReferences_minusNBraced = verifyNotTree checkUnassignedReferences "if [ -n \"${x}\" ]; then echo $x; fi" @@ -2827,11 +2816,11 @@ prop_checkUnpassedInFunctions7 = verifyTree checkUnpassedInFunctions "foo() { echo $1; }; foo; foo;" prop_checkUnpassedInFunctions8 = verifyNotTree checkUnpassedInFunctions "foo() { echo $((1)); }; foo;" prop_checkUnpassedInFunctions9 = verifyNotTree checkUnpassedInFunctions "foo() { echo $(($b)); }; foo;" -prop_checkUnpassedInFunctions10= verifyNotTree checkUnpassedInFunctions "foo() { echo $!; }; foo;" -prop_checkUnpassedInFunctions11= verifyNotTree checkUnpassedInFunctions "foo() { bar() { echo $1; }; bar baz; }; foo;" -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_checkUnpassedInFunctions10 = verifyNotTree checkUnpassedInFunctions "foo() { echo $!; }; foo;" +prop_checkUnpassedInFunctions11 = verifyNotTree checkUnpassedInFunctions "foo() { bar() { echo $1; }; bar baz; }; foo;" +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" checkUnpassedInFunctions params root = execWriter $ mapM_ warnForGroup referenceGroups where @@ -3006,12 +2995,12 @@ checkSuspiciousIFS _ _ = return () -prop_checkGrepQ1= verify checkShouldUseGrepQ "[[ $(foo | grep bar) ]]" -prop_checkGrepQ2= verify checkShouldUseGrepQ "[ -z $(fgrep lol) ]" -prop_checkGrepQ3= verify checkShouldUseGrepQ "[ -n \"$(foo | zgrep lol)\" ]" -prop_checkGrepQ4= verifyNot checkShouldUseGrepQ "[ -z $(grep bar | cmd) ]" -prop_checkGrepQ5= verifyNot checkShouldUseGrepQ "rm $(ls | grep file)" -prop_checkGrepQ6= verifyNot checkShouldUseGrepQ "[[ -n $(pgrep foo) ]]" +prop_checkGrepQ1 = verify checkShouldUseGrepQ "[[ $(foo | grep bar) ]]" +prop_checkGrepQ2 = verify checkShouldUseGrepQ "[ -z $(fgrep lol) ]" +prop_checkGrepQ3 = verify checkShouldUseGrepQ "[ -n \"$(foo | zgrep lol)\" ]" +prop_checkGrepQ4 = verifyNot checkShouldUseGrepQ "[ -z $(grep bar | cmd) ]" +prop_checkGrepQ5 = verifyNot checkShouldUseGrepQ "rm $(ls | grep file)" +prop_checkGrepQ6 = verifyNot checkShouldUseGrepQ "[[ -n $(pgrep foo) ]]" checkShouldUseGrepQ params t = sequence_ $ case t of TC_Nullary id _ token -> check id True token @@ -3313,6 +3302,7 @@ _:next@(TA_Unary _ "!" _):_ -> isOnlyTestInCommand next _:next@(TC_Group {}):_ -> isOnlyTestInCommand next _:next@(TA_Sequence _ [_]):_ -> isOnlyTestInCommand next + _:next@(TA_Parentesis _ _):_ -> isOnlyTestInCommand next _ -> False -- TODO: Do better $? tracking and filter on whether @@ -3595,7 +3585,6 @@ prop_checkPipeToNowhere5 = verifyNot checkPipeToNowhere "echo foo | xargs du" prop_checkPipeToNowhere6 = verifyNot checkPipeToNowhere "ls | echo $(cat)" prop_checkPipeToNowhere7 = verifyNot checkPipeToNowhere "echo foo | var=$(cat) ls" -prop_checkPipeToNowhere8 = verify checkPipeToNowhere "foo | true" prop_checkPipeToNowhere9 = verifyNot checkPipeToNowhere "mv -i f . < /dev/stdin" prop_checkPipeToNowhere10 = verify checkPipeToNowhere "ls > file | grep foo" prop_checkPipeToNowhere11 = verify checkPipeToNowhere "ls | grep foo < file" @@ -4115,13 +4104,6 @@ checkUnit :: [Token] -> Writer [TokenComment] () checkUnit unit = evalStateT (mapM_ (doAnalysis findCommands) unit) (Map.empty) - isSourced t = - let - f (T_SourceCommand {}) = True - f _ = False - in - any f $ getPath (parentMap params) t - findCommands :: Token -> StateT (Map.Map String Token) (Writer [TokenComment]) () findCommands t = case t of T_SimpleCommand _ _ (cmd:args) -> @@ -4132,7 +4114,7 @@ cmd <- gets (Map.lookup name) case cmd of Just alias -> - unless (isSourced t || shouldIgnoreCode params 2262 alias) $ do + unless (isSourced params t || shouldIgnoreCode params 2262 alias) $ do warn (getId alias) 2262 "This alias can't be defined and used in the same parsing unit. Use a function instead." info (getId t) 2263 "Since they're in the same parsing unit, this command will not refer to the previously mentioned alias." _ -> return () @@ -4143,6 +4125,14 @@ when (isVariableName name && not (null value)) $ modify (Map.insertWith (\new old -> old) name arg) +isSourced params t = + let + f (T_SourceCommand {}) = True + f _ = False + in + any f $ getPath (parentMap params) t + + -- Like groupBy, but compares pairs of adjacent elements, rather than against the first of the span prop_groupByLink1 = groupByLink (\a b -> a+1 == b) [1,2,3,2,3,7,8,9] == [[1,2,3], [2,3], [7,8,9]] prop_groupByLink2 = groupByLink (==) ([] :: [()]) == [] @@ -4706,6 +4696,7 @@ prop_checkSetESuppressed16 = verifyTree checkSetESuppressed "set -e; f(){ :; }; until set -e; f; do :; done" prop_checkSetESuppressed17 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; g(){ :; }; g f" prop_checkSetESuppressed18 = verifyNotTree checkSetESuppressed "set -e; shopt -s inherit_errexit; f(){ :; }; x=$(f)" +prop_checkSetESuppressed19 = verifyNotTree checkSetESuppressed "set -e; set -o posix; f(){ :; }; x=$(f)" checkSetESuppressed params t = if hasSetE params then runNodeAnalysis checkNode params t else [] where @@ -4783,8 +4774,12 @@ prop_checkExtraMaskedReturns33 = verifyNotTree checkExtraMaskedReturns "{ false || true; } | true" prop_checkExtraMaskedReturns34 = verifyNotTree checkExtraMaskedReturns "{ false || :; } | true" prop_checkExtraMaskedReturns35 = verifyTree checkExtraMaskedReturns "f() { local -r x=$(false); }" +prop_checkExtraMaskedReturns36 = verifyNotTree checkExtraMaskedReturns "time false" +prop_checkExtraMaskedReturns37 = verifyNotTree checkExtraMaskedReturns "time $(time false)" +prop_checkExtraMaskedReturns38 = verifyTree checkExtraMaskedReturns "x=$(time time time false) time $(time false)" -checkExtraMaskedReturns params t = runNodeAnalysis findMaskingNodes params t +checkExtraMaskedReturns params t = + runNodeAnalysis findMaskingNodes params (removeTransparentCommands t) where findMaskingNodes _ (T_Arithmetic _ list) = findMaskedNodesInList [list] findMaskingNodes _ (T_Array _ list) = findMaskedNodesInList $ allButLastSimpleCommands list @@ -4817,6 +4812,13 @@ where simpleCommands = filter containsSimpleCommand cmds + removeTransparentCommands t = + doTransform go t + where + go cmd@(T_SimpleCommand id assigns (_:args)) | isTransparentCommand cmd + = T_SimpleCommand id assigns args + go t = t + inform t = info (getId t) 2312 ("Consider invoking this command " ++ "separately to avoid masking its return value (or use '|| true' " ++ "to ignore).") @@ -4849,12 +4851,163 @@ ,"shopt" ] + isTransparentCommand t = fromMaybe False $ do + basename <- getCommandBasename t + return $ basename == "time" + parentChildPairs t = go $ parents params t where go (child:parent:rest) = (parent, child):go (parent:rest) go _ = [] hasParent pred t = any (uncurry pred) (parentChildPairs t) + + +-- hard error on negated command that is not last +prop_checkBatsTestDoesNotUseNegation1 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! true; false; }" +prop_checkBatsTestDoesNotUseNegation2 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [[ -e test ]]; false; }" +prop_checkBatsTestDoesNotUseNegation3 = verify checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [ -e test ]; false; }" +-- acceptable formats: +-- using run +prop_checkBatsTestDoesNotUseNegation4 = verifyNot checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { run ! true; }" +-- using || false +prop_checkBatsTestDoesNotUseNegation5 = verifyNot checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [[ -e test ]] || false; }" +prop_checkBatsTestDoesNotUseNegation6 = verifyNot checkBatsTestDoesNotUseNegation "#!/usr/bin/env/bats\n@test \"name\" { ! [ -e test ] || false; }" +-- only style warning when last command +prop_checkBatsTestDoesNotUseNegation7 = verifyCodes checkBatsTestDoesNotUseNegation [2314] "#!/usr/bin/env/bats\n@test \"name\" { ! true; }" +prop_checkBatsTestDoesNotUseNegation8 = verifyCodes checkBatsTestDoesNotUseNegation [2315] "#!/usr/bin/env/bats\n@test \"name\" { ! [[ -e test ]]; }" +prop_checkBatsTestDoesNotUseNegation9 = verifyCodes checkBatsTestDoesNotUseNegation [2315] "#!/usr/bin/env/bats\n@test \"name\" { ! [ -e test ]; }" + +checkBatsTestDoesNotUseNegation params t = + case t of + T_BatsTest _ _ (T_BraceGroup _ commands) -> mapM_ (check commands) commands + _ -> return () + where + check commands t = + case t of + T_Banged id (T_Pipeline _ _ [T_Redirecting _ _ (T_Condition idCondition _ _)]) -> + if t `isLastOf` commands + then style id 2315 "In Bats, ! will not fail the test if it is not the last command anymore. Fold the `!` into the conditional!" + else err id 2315 "In Bats, ! does not cause a test failure. Fold the `!` into the conditional!" + + T_Banged id cmd -> if t `isLastOf` commands + then styleWithFix id 2314 "In Bats, ! will not fail the test if it is not the last command anymore. Use `run ! ` (on Bats >= 1.5.0) instead." + (fixWith [replaceStart id params 0 "run "]) + else errWithFix id 2314 "In Bats, ! does not cause a test failure. Use 'run ! ' (on Bats >= 1.5.0) instead." + (fixWith [replaceStart id params 0 "run "]) + _ -> return () + isLastOf t commands = + case commands of + [x] -> x == t + x:rest -> isLastOf t rest + [] -> False + + +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" +checkCommandIsUnreachable params t = + case t of + T_Pipeline {} -> sequence_ $ do + state <- CF.getIncomingState (cfgAnalysis params) id + 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)." + _ -> return () + where id = getId t + + +prop_checkOverwrittenExitCode1 = verify checkOverwrittenExitCode "x; [ $? -eq 1 ] || [ $? -eq 2 ]" +prop_checkOverwrittenExitCode2 = verifyNot checkOverwrittenExitCode "x; [ $? -eq 1 ]" +prop_checkOverwrittenExitCode3 = verify checkOverwrittenExitCode "x; echo \"Exit is $?\"; [ $? -eq 0 ]" +prop_checkOverwrittenExitCode4 = verifyNot checkOverwrittenExitCode "x; [ $? -eq 0 ] && echo Success" +prop_checkOverwrittenExitCode5 = verify checkOverwrittenExitCode "x; if [ $? -eq 0 ]; then var=$?; fi" +prop_checkOverwrittenExitCode6 = verify checkOverwrittenExitCode "x; [ $? -gt 0 ] && fail=$?" +prop_checkOverwrittenExitCode7 = verifyNot checkOverwrittenExitCode "[ 1 -eq 2 ]; status=$?" +prop_checkOverwrittenExitCode8 = verifyNot checkOverwrittenExitCode "[ 1 -eq 2 ]; exit $?" +checkOverwrittenExitCode params t = + case t of + T_DollarBraced id _ val | getLiteralString val == Just "?" -> check id + _ -> return () + where + check id = sequence_ $ do + state <- CF.getIncomingState (cfgAnalysis params) id + let exitCodeIds = CF.exitCodes state + guard . not $ S.null exitCodeIds + + let idToToken = idMap params + exitCodeTokens <- sequence $ map (\k -> Map.lookup k idToToken) $ S.toList exitCodeIds + return $ do + when (all isCondition exitCodeTokens && not (usedUnconditionally t exitCodeIds)) $ + warn id 2319 "This $? refers to a condition, not a command. Assign to a variable to avoid it being overwritten." + when (all isPrinting exitCodeTokens) $ + warn id 2320 "This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten." + + isCondition t = + case t of + T_Condition {} -> True + T_SimpleCommand {} -> getCommandName t == Just "test" + _ -> False + + -- If we don't do anything based on the condition, assume we wanted the condition itself + -- This helps differentiate `x; [ $? -gt 0 ] && exit $?` vs `[ cond ]; exit $?` + usedUnconditionally t testIds = + all (\c -> CF.doesPostDominate (cfgAnalysis params) (getId t) c) testIds + + isPrinting t = + case getCommandBasename t of + Just "echo" -> True + Just "printf" -> True + _ -> False + + +prop_checkUnnecessaryArithmeticExpansionIndex1 = verify checkUnnecessaryArithmeticExpansionIndex "a[$((1+1))]=n" +prop_checkUnnecessaryArithmeticExpansionIndex2 = verifyNot checkUnnecessaryArithmeticExpansionIndex "a[1+1]=n" +prop_checkUnnecessaryArithmeticExpansionIndex3 = verifyNot checkUnnecessaryArithmeticExpansionIndex "a[$(echo $((1+1)))]=n" +prop_checkUnnecessaryArithmeticExpansionIndex4 = verifyNot checkUnnecessaryArithmeticExpansionIndex "declare -A a; a[$((1+1))]=val" +checkUnnecessaryArithmeticExpansionIndex params t = + case t of + T_Assignment _ mode var [TA_Sequence _ [ TA_Expansion _ [expansion@(T_DollarArithmetic id _)]]] val -> + styleWithFix id 2321 "Array indices are already arithmetic contexts. Prefer removing the $(( and ))." $ fix id + _ -> return () + + where + fix id = + fixWith [ + replaceStart id params 3 "", -- Remove "$((" + replaceEnd id params 2 "" -- Remove "))" + ] + + +prop_checkUnnecessaryParens1 = verify checkUnnecessaryParens "echo $(( ((1+1)) ))" +prop_checkUnnecessaryParens2 = verify checkUnnecessaryParens "x[((1+1))+1]=1" +prop_checkUnnecessaryParens3 = verify checkUnnecessaryParens "x[(1+1)]=1" +prop_checkUnnecessaryParens4 = verify checkUnnecessaryParens "$(( (x) ))" +prop_checkUnnecessaryParens5 = verify checkUnnecessaryParens "(( (x) ))" +prop_checkUnnecessaryParens6 = verifyNot checkUnnecessaryParens "x[(1+1)+1]=1" +prop_checkUnnecessaryParens7 = verifyNot checkUnnecessaryParens "(( (1*1)+1 ))" +prop_checkUnnecessaryParens8 = verifyNot checkUnnecessaryParens "(( (1)+1 ))" +checkUnnecessaryParens params t = + case t of + T_DollarArithmetic _ t -> checkLeading "$(( (x) )) is the same as $(( x ))" 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 _ ]) -> + 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 + _ -> return () + + fix id = + fixWith [ + replaceStart id params 1 "", -- Remove "(" + replaceEnd id params 1 "" -- Remove ")" + ] return [] diff --git a/src/ShellCheck/Analyzer.hs b/src/ShellCheck/Analyzer.hs --- a/src/ShellCheck/Analyzer.hs +++ b/src/ShellCheck/Analyzer.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2019 Vidar Holen + Copyright 2012-2022 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -25,6 +25,7 @@ import Data.List import Data.Monoid import qualified ShellCheck.Checks.Commands +import qualified ShellCheck.Checks.ControlFlow import qualified ShellCheck.Checks.Custom import qualified ShellCheck.Checks.ShellSupport @@ -34,19 +35,21 @@ analyzeScript spec = newAnalysisResult { arComments = filterByAnnotation spec params . nub $ - runAnalytics spec - ++ runChecker params (checkers spec params) + runChecker params (checkers spec params) } where params = makeParameters spec checkers spec params = mconcat $ map ($ params) [ + ShellCheck.Analytics.checker spec, ShellCheck.Checks.Commands.checker spec, + ShellCheck.Checks.ControlFlow.checker spec, ShellCheck.Checks.Custom.checker, ShellCheck.Checks.ShellSupport.checker ] optionalChecks = mconcat $ [ ShellCheck.Analytics.optionalChecks, - ShellCheck.Checks.Commands.optionalChecks + ShellCheck.Checks.Commands.optionalChecks, + ShellCheck.Checks.ControlFlow.optionalChecks ] diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2021 Vidar Holen + Copyright 2012-2022 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -23,13 +23,16 @@ import ShellCheck.AST import ShellCheck.ASTLib +import qualified ShellCheck.CFGAnalysis as CF import ShellCheck.Data import ShellCheck.Interface import ShellCheck.Parser +import ShellCheck.Prelude import ShellCheck.Regex import Control.Arrow (first) import Control.DeepSeq +import Control.Monad import Control.Monad.Identity import Control.Monad.RWS import Control.Monad.State @@ -87,6 +90,8 @@ hasPipefail :: Bool, -- A linear (bad) analysis of data flow variableFlow :: [StackData], + -- A map from Id to Token + idMap :: Map.Map Id Token, -- A map from Id to parent Token parentMap :: Map.Map Id Token, -- The shell type, such as Bash or Ksh @@ -96,7 +101,9 @@ -- The root node of the AST rootNode :: Token, -- map from token id to start and end position - tokenPositions :: Map.Map Id (Position, Position) + tokenPositions :: Map.Map Id (Position, Position), + -- Result from Control Flow Graph analysis (including data flow analysis) + cfgAnalysis :: CF.CFGAnalysis } deriving (Show) -- TODO: Cache results of common AST ops here @@ -189,35 +196,42 @@ } in force withFix -makeParameters spec = - let params = Parameters { +makeParameters spec = params + where + params = Parameters { rootNode = root, shellType = fromMaybe (determineShell (asFallbackShell spec) root) $ asShellType spec, hasSetE = containsSetE root, hasLastpipe = case shellType params of - Bash -> containsLastpipe root + Bash -> isOptionSet "lastpipe" root Dash -> False Sh -> False Ksh -> True, hasInheritErrexit = case shellType params of - Bash -> containsInheritErrexit root + Bash -> isOptionSet "inherit_errexit" root Dash -> True Sh -> True Ksh -> False, hasPipefail = case shellType params of - Bash -> containsPipefail root + Bash -> isOptionSet "pipefail" root Dash -> True Sh -> True - Ksh -> containsPipefail root, + Ksh -> isOptionSet "pipefail" root, shellTypeSpecified = isJust (asShellType spec) || isJust (asFallbackShell spec), + idMap = getTokenMap root, parentMap = getParentTree root, variableFlow = getVariableFlow params root, - tokenPositions = asTokenPositions spec - } in params - where root = asScript spec + tokenPositions = asTokenPositions spec, + cfgAnalysis = CF.analyzeControlFlow cfParams root + } + cfParams = CF.CFGParameters { + CF.cfLastpipe = hasLastpipe params, + CF.cfPipefail = hasPipefail params + } + root = asScript spec -- Does this script mention 'set -e' anywhere? @@ -234,13 +248,14 @@ _ -> False re = mkRegex "[[:space:]]-[^-]*e" -containsPipefail root = isNothing $ doAnalysis (guard . not . isPipefail) root + +containsSetOption opt root = isNothing $ doAnalysis (guard . not . isPipefail) root where isPipefail t = case t of T_SimpleCommand {} -> t `isUnqualifiedCommand` "set" && - ("pipefail" `elem` oversimplify t || + (opt `elem` oversimplify t || "o" `elem` map snd (getAllFlags t)) _ -> False @@ -254,12 +269,8 @@ (shopt `elem` oversimplify t) _ -> False --- Does this script mention 'shopt -s inherit_errexit' anywhere? -containsInheritErrexit = containsShopt "inherit_errexit" - --- Does this script mention 'shopt -s lastpipe' anywhere? --- Also used as a hack. -containsLastpipe = containsShopt "lastpipe" +-- Does this script mention 'shopt -s $opt' or 'set -o $opt' anywhere? +isOptionSet opt root = containsShopt opt root || containsSetOption opt root prop_determineShell0 = determineShellTest "#!/bin/sh" == Sh @@ -351,7 +362,7 @@ T_SelectIn {} -> return (not strict) _ -> Nothing - -- Check whether this assigment is self-quoting due to being a recognized + -- Check whether this assignment is self-quoting due to being a recognized -- assignment passed to a Declaration Utility. This will soon be required -- by POSIX: https://austingroupbugs.net/view.php?id=351 assignmentIsQuoting t = shellParsesParamsAsAssignments || not (isAssignmentParamToCommand t) @@ -408,12 +419,6 @@ getId word == currentId || getId (getCommandTokenOrThis t) == currentId go _ _ = False --- A list of the element and all its parents up to the root node. -getPath tree t = t : - case Map.lookup (getId t) tree of - Nothing -> [] - Just parent -> getPath tree parent - -- Version of the above taking the map from the current context -- Todo: give this the name "getPath" getPathM t = do @@ -511,13 +516,11 @@ T_SimpleCommand {} -> getModifiedVariableCommand t - TA_Unary _ "++|" v@(TA_Variable _ name _) -> - [(t, v, name, DataString $ SourceFrom [v])] - TA_Unary _ "|++" v@(TA_Variable _ name _) -> - [(t, v, name, DataString $ SourceFrom [v])] + TA_Unary _ op v@(TA_Variable _ name _) | "--" `isInfixOf` op || "++" `isInfixOf` op -> + [(t, v, name, DataString SourceInteger)] TA_Assignment _ op (TA_Variable _ name _) rhs -> do guard $ op `elem` ["=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", "&=", "^=", "|="] - return (t, t, name, DataString $ SourceFrom [rhs]) + return (t, t, name, DataString SourceInteger) T_BatsTest {} -> [ (t, t, "lines", DataArray SourceExternal), @@ -561,13 +564,7 @@ return (place, t, str, DataString SourceChecked) _ -> Nothing -isClosingFileOp op = - case op of - T_IoDuplicate _ (T_GREATAND _) "-" -> True - T_IoDuplicate _ (T_LESSAND _) "-" -> True - _ -> False - -- Consider 'export/declare -x' a reference, since it makes the var available getReferencedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal _ x:_):rest)) = case x of @@ -748,13 +745,6 @@ getModifiedVariableCommand _ = [] -getIndexReferences s = fromMaybe [] $ do - match <- matchRegex re s - index <- match !!! 0 - return $ matchAllStrings variableNameRegex index - where - re = mkRegex "(\\[.*\\])" - -- Given a NormalWord like foo or foo[$bar], get foo. -- Primarily used to get references for [[ -v foo[bar] ]] getVariableForTestDashV :: Token -> Maybe String @@ -769,18 +759,6 @@ -- in a non-constant expression (while filtering out foo$x[$y]) toStr _ = return "\0" -prop_getOffsetReferences1 = getOffsetReferences ":bar" == ["bar"] -prop_getOffsetReferences2 = getOffsetReferences ":bar:baz" == ["bar", "baz"] -prop_getOffsetReferences3 = getOffsetReferences "[foo]:bar" == ["bar"] -prop_getOffsetReferences4 = getOffsetReferences "[foo]:bar:baz" == ["bar", "baz"] -getOffsetReferences mods = fromMaybe [] $ do --- if mods start with [, then drop until ] - match <- matchRegex re mods - offsets <- match !!! 1 - return $ matchAllStrings variableNameRegex offsets - where - re = mkRegex "^(\\[.+\\])? *:([^-=?+].*)" - getReferencedVariables parents t = case t of T_DollarBraced id _ l -> let str = concat $ oversimplify l in @@ -859,17 +837,6 @@ isConfusedGlobRegex [x,'*'] | x `notElem` "\\." = True isConfusedGlobRegex _ = False -isVariableStartChar x = x == '_' || isAsciiLower x || isAsciiUpper x -isVariableChar x = isVariableStartChar x || isDigit x -isSpecialVariableChar = (`elem` "*@#?-$!") -variableNameRegex = mkRegex "[_a-zA-Z][_a-zA-Z0-9]*" - -prop_isVariableName1 = isVariableName "_fo123" -prop_isVariableName2 = not $ isVariableName "4" -prop_isVariableName3 = not $ isVariableName "test: " -isVariableName (x:r) = isVariableStartChar x && all isVariableChar r -isVariableName _ = False - getVariablesFromLiteralToken token = getVariablesFromLiteral (getLiteralStringDef " " token) @@ -882,74 +849,7 @@ where variableRegex = mkRegex "\\$\\{?([A-Za-z0-9_]+)" --- Get the variable name from an expansion like ${var:-foo} -prop_getBracedReference1 = getBracedReference "foo" == "foo" -prop_getBracedReference2 = getBracedReference "#foo" == "foo" -prop_getBracedReference3 = getBracedReference "#" == "#" -prop_getBracedReference4 = getBracedReference "##" == "#" -prop_getBracedReference5 = getBracedReference "#!" == "!" -prop_getBracedReference6 = getBracedReference "!#" == "#" -prop_getBracedReference7 = getBracedReference "!foo#?" == "foo" -prop_getBracedReference8 = getBracedReference "foo-bar" == "foo" -prop_getBracedReference9 = getBracedReference "foo:-bar" == "foo" -prop_getBracedReference10= getBracedReference "foo: -1" == "foo" -prop_getBracedReference11= getBracedReference "!os*" == "" -prop_getBracedReference11b= getBracedReference "!os@" == "" -prop_getBracedReference12= getBracedReference "!os?bar**" == "" -prop_getBracedReference13= getBracedReference "foo[bar]" == "foo" -getBracedReference s = fromMaybe s $ - nameExpansion s `mplus` takeName noPrefix `mplus` getSpecial noPrefix `mplus` getSpecial s - where - noPrefix = dropPrefix s - dropPrefix (c:rest) | c `elem` "!#" = rest - dropPrefix cs = cs - takeName s = do - let name = takeWhile isVariableChar s - guard . not $ null name - return name - getSpecial (c:_) | isSpecialVariableChar c = return [c] - getSpecial _ = fail "empty or not special" - nameExpansion ('!':next:rest) = do -- e.g. ${!foo*bar*} - guard $ isVariableChar next -- e.g. ${!@} - first <- find (not . isVariableChar) rest - guard $ first `elem` "*?@" - return "" - nameExpansion _ = Nothing - -prop_getBracedModifier1 = getBracedModifier "foo:bar:baz" == ":bar:baz" -prop_getBracedModifier2 = getBracedModifier "!var:-foo" == ":-foo" -prop_getBracedModifier3 = getBracedModifier "foo[bar]" == "[bar]" -prop_getBracedModifier4 = getBracedModifier "foo[@]@Q" == "[@]@Q" -prop_getBracedModifier5 = getBracedModifier "@@Q" == "@Q" -getBracedModifier s = headOrDefault "" $ do - let var = getBracedReference s - a <- dropModifier s - dropPrefix var a - where - dropPrefix [] t = return t - dropPrefix (a:b) (c:d) | a == c = dropPrefix b d - dropPrefix _ _ = [] - - dropModifier (c:rest) | c `elem` "#!" = [rest, c:rest] - dropModifier x = [x] - --- Useful generic functions. - --- Get element 0 or a default. Like `head` but safe. -headOrDefault _ (a:_) = a -headOrDefault def _ = def - --- Get the last element or a default. Like `last` but safe. -lastOrDefault def [] = def -lastOrDefault _ list = last list - ---- Get element n of a list, or Nothing. Like `!!` but safe. -(!!!) list i = - case drop i list of - [] -> Nothing - (r:_) -> Just r - -- Run a command if the shell is in the given list whenShell l c = do params <- ask @@ -1000,17 +900,6 @@ Ksh -> True Dash -> False Sh -> False - --- Returns whether a token is a parameter expansion without any modifiers. --- True for $var ${var} $1 $# --- False for ${#var} ${var[x]} ${var:-0} -isUnmodifiedParameterExpansion t = - case t of - T_DollarBraced _ False _ -> True - T_DollarBraced _ _ list -> - let str = concat $ oversimplify list - in getBracedReference str == str - _ -> False isTrueAssignmentSource c = case c of diff --git a/src/ShellCheck/CFG.hs b/src/ShellCheck/CFG.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/CFG.hs @@ -0,0 +1,1307 @@ +{- + Copyright 2022 Vidar Holen + + This file is part of ShellCheck. + https://www.shellcheck.net + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +{-# LANGUAGE TemplateHaskell #-} +{-# LANGUAGE DeriveAnyClass, DeriveGeneric #-} + +-- Constructs a Control Flow Graph from an AST +module ShellCheck.CFG ( + CFNode (..), + CFEdge (..), + CFEffect (..), + CFStringPart (..), + CFVariableProp (..), + CFGResult (..), + CFValue (..), + CFGraph, + CFGParameters (..), + IdTagged (..), + Scope (..), + buildGraph + , ShellCheck.CFG.runTests -- STRIP + ) + where + +import GHC.Generics (Generic) +import ShellCheck.AST +import ShellCheck.ASTLib +import ShellCheck.Data +import ShellCheck.Interface +import ShellCheck.Prelude +import ShellCheck.Regex +import Control.DeepSeq +import Control.Monad +import Control.Monad.Identity +import Data.Array.Unboxed +import Data.Array.ST +import Data.List hiding (map) +import Data.Maybe +import qualified Data.Map as M +import qualified Data.Set as S +import Control.Monad.RWS.Lazy +import Data.Graph.Inductive.Graph +import Data.Graph.Inductive.Query.DFS +import Data.Graph.Inductive.Basic +import Data.Graph.Inductive.Query.Dominators +import Data.Graph.Inductive.PatriciaTree as G +import Debug.Trace -- STRIP + +import Test.QuickCheck.All (forAllProperties) +import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) + + +-- Our basic Graph type +type CFGraph = G.Gr CFNode CFEdge + +-- Node labels in a Control Flow Graph +data CFNode = + -- A no-op node for structural purposes + CFStructuralNode + -- A no-op for graph inspection purposes + | CFEntryPoint String + -- Drop current prefix assignments + | CFDropPrefixAssignments + -- A node with a certain effect on program state + | CFApplyEffects [IdTagged CFEffect] + -- The execution of a command or function by literal string if possible + | CFExecuteCommand (Maybe String) + -- Execute a subshell. These are represented by disjoint graphs just like + -- functions, but they don't require any form of name resolution + | CFExecuteSubshell String Node Node + -- Assignment of $? + | CFSetExitCode Id + -- The virtual 'exit' at the natural end of a subshell + | CFImpliedExit + -- An exit statement resolvable at CFG build time + | CFResolvedExit + -- An exit statement only resolvable at DFA time + | CFUnresolvedExit + -- An unreachable node, serving as the unconnected end point of a range + | CFUnreachable + -- Assignment of $! + | CFSetBackgroundPid Id + deriving (Eq, Ord, Show, Generic, NFData) + +-- Edge labels in a Control Flow Graph +data CFEdge = + CFEErrExit + -- Regular control flow edge + | CFEFlow + -- An edge that a human might think exists (e.g. from a backgrounded process to its parent) + | CFEFalseFlow + -- An edge followed on exit + | CFEExit + deriving (Eq, Ord, Show, Generic, NFData) + +-- Actions we track +data CFEffect = + CFSetProps Scope String (S.Set CFVariableProp) + | CFUnsetProps Scope String (S.Set CFVariableProp) + | CFReadVariable String + | CFWriteVariable String CFValue + | CFWriteGlobal String CFValue + | CFWriteLocal String CFValue + | CFWritePrefix String CFValue + | CFDefineFunction String Id Node Node + | CFUndefine String + | CFUndefineVariable String + | CFUndefineFunction String + | CFUndefineNameref String + -- Usage implies that this is an array (e.g. it's expanded with index) + | CFHintArray String + -- Operation implies that the variable will be defined (e.g. [ -z "$var" ]) + | CFHintDefined String + deriving (Eq, Ord, Show, Generic, NFData) + +data IdTagged a = IdTagged Id a + deriving (Eq, Ord, Show, Generic, NFData) + +-- Where a variable's value comes from +data CFValue = + -- The special 'uninitialized' value + CFValueUninitialized + -- An arbitrary array value + | CFValueArray + -- An arbitrary string value + | CFValueString + -- An arbitrary integer + | CFValueInteger + -- Token 'Id' concatenates and assigns the given parts + | CFValueComputed Id [CFStringPart] + deriving (Eq, Ord, Show, Generic, NFData) + +-- Simplified computed strings +data CFStringPart = + -- A known literal string value, like 'foo' + CFStringLiteral String + -- The contents of a variable, like $foo (may not be a string) + | CFStringVariable String + -- An value that is unknown but an integer + | CFStringInteger + -- An unknown string value, for things we can't handle + | CFStringUnknown + deriving (Eq, Ord, Show, Generic, NFData) + +-- The properties of a variable +data CFVariableProp = CFVPExport | CFVPArray | CFVPAssociative | CFVPInteger + deriving (Eq, Ord, Show, Generic, NFData) + +-- Options when generating CFG +data CFGParameters = CFGParameters { + -- Whether the last element in a pipeline runs in the current shell + cfLastpipe :: Bool, + -- Whether all elements in a pipeline count towards the exit status + cfPipefail :: Bool +} + +data CFGResult = CFGResult { + -- The graph itself + cfGraph :: CFGraph, + -- Map from Id to nominal start&end node (i.e. assuming normal execution without exits) + cfIdToRange :: M.Map Id (Node, Node), + -- A set of all nodes belonging to an Id, recursively + cfIdToNodes :: M.Map Id (S.Set Node), + -- An array (from,to) saying whether 'from' postdominates 'to' + cfPostDominators :: Array Node [Node] +} + deriving (Show) + +buildGraph :: CFGParameters -> Token -> CFGResult +buildGraph params root = + let + (nextNode, base) = execRWS (buildRoot root) (newCFContext params) 0 + (nodes, edges, mapping, association) = +-- renumberTopologically $ + removeUnnecessaryStructuralNodes + base + + idToRange = M.fromList mapping + isRealEdge (from, to, edge) = case edge of CFEFlow -> True; _ -> False + onlyRealEdges = filter isRealEdge edges + (_, mainExit) = fromJust $ M.lookup (getId root) idToRange + + result = CFGResult { + cfGraph = mkGraph nodes edges, + cfIdToRange = idToRange, + cfIdToNodes = M.fromListWith S.union $ map (\(id, n) -> (id, S.singleton n)) association, + cfPostDominators = findPostDominators mainExit $ mkGraph nodes onlyRealEdges + } + in + result + +remapGraph :: M.Map Node Node -> CFW -> CFW +remapGraph remap (nodes, edges, mapping, assoc) = + ( + map (remapNode remap) nodes, + map (remapEdge remap) edges, + map (\(id, (a,b)) -> (id, (remapHelper remap a, remapHelper remap b))) mapping, + map (\(id, n) -> (id, remapHelper remap n)) assoc + ) + +prop_testRenumbering = + let + s = CFStructuralNode + before = ( + [(1,s), (3,s), (4, s), (8,s)], + [(1,3,CFEFlow), (3,4, CFEFlow), (4,8,CFEFlow)], + [(Id 0, (3,4))], + [(Id 1, 3), (Id 2, 4)] + ) + after = ( + [(0,s), (1,s), (2,s), (3,s)], + [(0,1,CFEFlow), (1,2, CFEFlow), (2,3,CFEFlow)], + [(Id 0, (1,2))], + [(Id 1, 1), (Id 2, 2)] + ) + in after == renumberGraph before + +-- Renumber the graph for prettiness, so there are no gaps in node numbers +renumberGraph :: CFW -> CFW +renumberGraph g@(nodes, edges, mapping, assoc) = + let renumbering = M.fromList (flip zip [0..] $ sort $ map fst nodes) + in remapGraph renumbering g + +prop_testRenumberTopologically = + let + s = CFStructuralNode + before = ( + [(4,s), (2,s), (3, s)], + [(4,2,CFEFlow), (2,3, CFEFlow)], + [(Id 0, (4,2))], + [] + ) + after = ( + [(0,s), (1,s), (2,s)], + [(0,1,CFEFlow), (1,2, CFEFlow)], + [(Id 0, (0,1))], + [] + ) + in after == renumberTopologically before + +-- Renumber the graph in topological order +renumberTopologically g@(nodes, edges, mapping, assoc) = + let renumbering = M.fromList (flip zip [0..] $ topsort (mkGraph nodes edges :: CFGraph)) + in remapGraph renumbering g + +prop_testRemoveStructural = + let + s = CFStructuralNode + before = ( + [(1,s), (2,s), (3, s), (4,s)], + [(1,2,CFEFlow), (2,3, CFEFlow), (3,4,CFEFlow)], + [(Id 0, (2,3))], + [(Id 0, 3)] + ) + after = ( + [(1,s), (2,s), (4,s)], + [(1,2,CFEFlow), (2,4,CFEFlow)], + [(Id 0, (2,2))], + [(Id 0, 2)] + ) + in after == removeUnnecessaryStructuralNodes before + +-- Collapse structural nodes that just form long chains like x->x->x. +-- This way we can generate them with abandon, without making DFA slower. +-- +-- Note in particular that we can't remove a structural node x in +-- foo -> x -> bar , because then the pre/post-condition for tokens +-- previously pointing to x would be wrong. +removeUnnecessaryStructuralNodes (nodes, edges, mapping, association) = + remapGraph recursiveRemapping + ( + filter (\(n, _) -> n `M.notMember` recursiveRemapping) nodes, + filter (`S.notMember` edgesToCollapse) edges, + mapping, + association + ) + where + 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 + 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 + + 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 + isRegularEdge (_, _, CFEFlow) = True + isRegularEdge _ = False + + recursiveLookup :: M.Map Node Node -> Node -> Node + recursiveLookup map node = + case M.lookup node map of + 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 + + +remapNode :: M.Map Node Node -> LNode CFNode -> LNode CFNode +remapNode m (node, label) = + (remapHelper m node, newLabel) + where + newLabel = case label of + CFApplyEffects effects -> CFApplyEffects (map (remapEffect m) effects) + CFExecuteSubshell s a b -> CFExecuteSubshell s (remapHelper m a) (remapHelper m b) + _ -> label + +remapEffect map old@(IdTagged id effect) = + case effect of + CFDefineFunction name id start end -> IdTagged id $ CFDefineFunction name id (remapHelper map start) (remapHelper map end) + _ -> old + +remapEdge :: M.Map Node Node -> LEdge CFEdge -> LEdge CFEdge +remapEdge map (from, to, label) = (remapHelper map from, remapHelper map to, label) +remapHelper map n = M.findWithDefault n n map + +data Range = Range Node Node + deriving (Eq, Show) + +data CFContext = CFContext { + cfIsCondition :: Bool, + cfIsFunction :: Bool, + cfLoopStack :: [(Node, Node)], + cfTokenStack :: [Id], + cfExitTarget :: Maybe Node, + cfReturnTarget :: Maybe Node, + cfParameters :: CFGParameters +} +newCFContext params = CFContext { + cfIsCondition = False, + cfIsFunction = False, + cfLoopStack = [], + cfTokenStack = [], + cfExitTarget = Nothing, + cfReturnTarget = Nothing, + cfParameters = params +} + +-- The monad we generate a graph in +type CFM a = RWS CFContext CFW Int a +type CFW = ([LNode CFNode], [LEdge CFEdge], [(Id, (Node, Node))], [(Id, Node)]) + +newNode :: CFNode -> CFM Node +newNode label = do + n <- get + stack <- asks cfTokenStack + put (n+1) + tell ([(n, label)], [], [], map (\c -> (c, n)) stack) + return n + +newNodeRange :: CFNode -> CFM Range +-- newNodeRange label = nodeToRange <$> newNode label +newNodeRange label = nodeToRange <$> newNode label + +-- Build a disjoint piece of the graph and return a CFExecuteSubshell. The Id is used purely for debug naming. +subshell :: Id -> String -> CFM Range -> CFM Range +subshell id reason p = do + start <- newNode $ CFEntryPoint $ "Subshell " ++ show id ++ ": " ++ reason + end <- newNode CFStructuralNode + middle <- local (\c -> c { cfExitTarget = Just end, cfReturnTarget = Just end}) p + linkRanges [nodeToRange start, middle, nodeToRange end] + newNodeRange $ CFExecuteSubshell reason start end + + +withFunctionScope p = do + end <- newNode CFStructuralNode + body <- local (\c -> c { cfReturnTarget = Just end, cfIsFunction = True }) p + linkRanges [body, nodeToRange end] + +-- Anything that happens recursively in f will be attributed to this id +under :: Id -> CFM a -> CFM a +under id f = local (\c -> c { cfTokenStack = id:(cfTokenStack c) }) f + +nodeToRange :: Node -> Range +nodeToRange n = Range n n + +link :: Node -> Node -> CFEdge -> CFM () +link from to label = do + tell ([], [(from, to, label)], [], []) + +registerNode :: Id -> Range -> CFM () +registerNode id (Range start end) = tell ([], [], [(id, (start, end))], []) + +linkRange :: Range -> Range -> CFM Range +linkRange = linkRangeAs CFEFlow + +linkRangeAs :: CFEdge -> Range -> Range -> CFM Range +linkRangeAs label (Range start mid1) (Range mid2 end) = do + link mid1 mid2 label + return (Range start end) + +-- Like linkRange but without actually linking +spanRange :: Range -> Range -> Range +spanRange (Range start mid1) (Range mid2 end) = Range start end + +linkRanges :: [Range] -> CFM Range +linkRanges [] = error "Empty range" +linkRanges (first:rest) = foldM linkRange first rest + +sequentially :: [Token] -> CFM Range +sequentially list = do + first <- newStructuralNode + rest <- mapM build list + linkRanges (first:rest) + +withContext :: (CFContext -> CFContext) -> CFM a -> CFM a +withContext = local + +withReturn :: Range -> CFM a -> CFM a +withReturn _ p = p + +asCondition :: CFM Range -> CFM Range +asCondition = withContext (\c -> c { cfIsCondition = True }) + +newStructuralNode = newNodeRange CFStructuralNode + +buildRoot :: Token -> CFM Range +buildRoot t = under (getId t) $ do + entry <- newNodeRange $ CFEntryPoint "MAIN" + impliedExit <- newNode CFImpliedExit + end <- newNode CFStructuralNode + start <- local (\c -> c { cfExitTarget = Just end, cfReturnTarget = Just impliedExit}) $ build t + range <- linkRanges [entry, start, nodeToRange impliedExit, nodeToRange end] + registerNode (getId t) range + return range + +applySingle e = CFApplyEffects [e] + +-- Build the CFG. +build :: Token -> CFM Range +build t = do + range <- under (getId t) $ build' t + registerNode (getId t) range + return range + where + build' t = case t of + T_Annotation _ _ list -> build list + T_Script _ _ list -> do + sequentially list + + TA_Assignment id op var@(TA_Variable _ name indices) rhs -> do + -- value first: (( var[x=1] = (x=2) )) runs x=1 last + value <- build rhs + subscript <- sequentially indices + read <- + if op == "=" + then none + -- This is += or something + else newNodeRange $ applySingle $ IdTagged id $ CFReadVariable name + + write <- newNodeRange $ applySingle $ IdTagged id $ CFWriteVariable name $ + if null indices + then CFValueInteger + else CFValueArray + + linkRanges [value, subscript, read, write] + + TA_Assignment id op lhs rhs -> do + -- This is likely an invalid assignment like (( 1 = 2 )), but it + -- could be e.g. x=y; (( $x = 3 )); echo $y, so expand both sides + -- without updating anything + sequentially [lhs, rhs] + + TA_Binary _ _ a b -> sequentially [a,b] + TA_Expansion _ list -> sequentially list + TA_Sequence _ list -> sequentially list + TA_Parentesis _ t -> build t + + TA_Trinary _ cond a b -> do + condition <- build cond + ifthen <- build a + elsethen <- build b + end <- newStructuralNode + linkRanges [condition, ifthen, end] + linkRanges [condition, elsethen, end] + + TA_Variable id name indices -> do + subscript <- sequentially indices + hint <- + if null indices + then none + else nodeToRange <$> newNode (applySingle $ IdTagged id $ CFHintArray name) + read <- nodeToRange <$> newNode (applySingle $ IdTagged id $ CFReadVariable name) + linkRanges [subscript, hint, read] + + TA_Unary id op (TA_Variable _ name indices) | "--" `isInfixOf` op || "++" `isInfixOf` op -> do + subscript <- sequentially indices + read <- newNodeRange $ applySingle $ IdTagged id $ CFReadVariable name + write <- newNodeRange $ applySingle $ IdTagged id $ CFWriteVariable name $ + if null indices + then CFValueInteger + else CFValueArray + linkRanges [subscript, read, write] + TA_Unary _ _ arg -> build arg + + TC_And _ SingleBracket _ lhs rhs -> do + sequentially [lhs, rhs] + + TC_And _ DoubleBracket _ lhs rhs -> do + left <- build lhs + right <- build rhs + end <- newStructuralNode + -- complete + linkRanges [left, right, end] + -- short circuit + linkRange left end + + -- TODO: Handle integer ops + TC_Binary _ mode str lhs rhs -> do + left <- build lhs + right <- build rhs + linkRange left right + + TC_Empty {} -> newStructuralNode + + TC_Group _ _ t -> build t + + -- TODO: Mark as checked + TC_Nullary _ _ arg -> build arg + + TC_Or _ SingleBracket _ lhs rhs -> sequentially [lhs, rhs] + + TC_Or _ DoubleBracket _ lhs rhs -> do + left <- build lhs + right <- build rhs + end <- newStructuralNode + -- complete + linkRanges [left, right, end] + -- short circuit + linkRange left end + + -- TODO: Handle -v, -z, -n + TC_Unary _ _ op arg -> do + build arg + + T_Arithmetic id root -> do + exe <- build root + status <- newNodeRange (CFSetExitCode id) + linkRange exe status + + T_AndIf _ lhs rhs -> do + left <- build lhs + right <- build rhs + end <- newStructuralNode + linkRange left right + linkRange right end + linkRange left end + + T_Array _ list -> sequentially list + + T_Assignment {} -> buildAssignment DefaultScope t + + T_Backgrounded id body -> do + start <- newStructuralNode + fork <- subshell id "backgrounding '&'" $ build body + pid <- newNodeRange $ CFSetBackgroundPid id + status <- newNodeRange $ CFSetExitCode id + + linkRange start fork + -- Add a join from the fork to warn about variable changes + linkRangeAs CFEFalseFlow fork pid + linkRanges [start, pid, status] + + T_Backticked id body -> + subshell id "`..` expansion" $ sequentially body + + T_Banged id cmd -> do + main <- build cmd + status <- newNodeRange (CFSetExitCode id) + linkRange main status + + T_BatsTest id _ body -> do + -- These are technically set by the 'run' command, but we'll just define them + -- up front to avoid figuring out which commands named "run" belong to Bats. + status <- newNodeRange $ applySingle $ IdTagged id $ CFWriteVariable "status" CFValueInteger + output <- newNodeRange $ applySingle $ IdTagged id $ CFWriteVariable "output" CFValueString + main <- build body + linkRanges [status, output, main] + + T_BraceExpansion _ list -> sequentially list + + T_BraceGroup id body -> + sequentially body + + T_CaseExpression id t [] -> build t + + T_CaseExpression id t list -> do + start <- newStructuralNode + token <- build t + branches <- mapM buildBranch list + end <- newStructuralNode + + let neighbors = zip branches $ tail branches + let (_, firstCond, _) = head branches + let (_, lastCond, lastBody) = last branches + + linkRange start token + linkRange token firstCond + mapM_ (uncurry $ linkBranch end) neighbors + linkRange lastBody end + + unless (any hasCatchAll list) $ + -- There's no *) branch, so assume we can fall through + void $ linkRange token end + + return $ spanRange start end + + where + -- for a | b | c, evaluate each in turn and allow short circuiting + buildCond list = do + start <- newStructuralNode + conds <- mapM build list + end <- newStructuralNode + linkRanges (start:conds) + mapM_ (`linkRange` end) conds + return $ spanRange start end + + buildBranch (typ, cond, body) = do + c <- buildCond cond + b <- sequentially body + linkRange c b + return (typ, c, b) + + linkBranch end (typ, cond, body) (_, nextCond, nextBody) = do + -- Failure case + linkRange cond nextCond + -- After body + case typ of + CaseBreak -> linkRange body end + CaseFallThrough -> linkRange body nextBody + CaseContinue -> linkRange body nextCond + + -- Find a *) if any + + hasCatchAll (_,cond,_) = any isCatchAll cond + isCatchAll c = fromMaybe False $ do + pg <- wordToExactPseudoGlob c + return $ pg `pseudoGlobIsSuperSetof` [PGMany] + + T_Condition id _ op -> do + cond <- build op + status <- newNodeRange $ CFSetExitCode id + linkRange cond status + + T_CoProc id maybeName t -> do + let name = fromMaybe "COPROC" maybeName + start <- newStructuralNode + parent <- newNodeRange $ applySingle $ IdTagged id $ CFWriteVariable name CFValueArray + child <- subshell id "coproc" $ build t + end <- newNodeRange $ CFSetExitCode id + + linkRange start parent + linkRange start child + linkRange parent end + linkRangeAs CFEFalseFlow child end + + return $ spanRange start end + T_CoProcBody _ t -> build t + + T_DollarArithmetic _ arith -> build arith + T_DollarDoubleQuoted _ list -> sequentially list + T_DollarSingleQuoted _ _ -> none + T_DollarBracket _ t -> build t + + T_DollarBraced id _ t -> do + let str = concat $ oversimplify t + let modifier = getBracedModifier str + let reference = getBracedReference str + let indices = getIndexReferences str + let offsets = getOffsetReferences str + vals <- build t + others <- mapM (\x -> nodeToRange <$> newNode (applySingle $ IdTagged id $ CFReadVariable x)) (indices ++ offsets) + deps <- linkRanges (vals:others) + read <- nodeToRange <$> newNode (applySingle $ IdTagged id $ CFReadVariable reference) + totalRead <- linkRange deps read + + if any (`isPrefixOf` modifier) ["=", ":="] + then do + optionalAssign <- newNodeRange (applySingle $ IdTagged id $ CFWriteVariable reference CFValueString) + result <- newStructuralNode + linkRange optionalAssign result + linkRange totalRead result + else return totalRead + + T_DoubleQuoted _ list -> sequentially list + + T_DollarExpansion id body -> + subshell id "$(..) expansion" $ sequentially body + + T_Extglob _ _ list -> sequentially list + + T_FdRedirect id ('{':identifier) op -> do + let name = takeWhile (/= '}') identifier + expression <- build op + rw <- newNodeRange $ + if isClosingFileOp op + then applySingle $ IdTagged id $ CFReadVariable name + else applySingle $ IdTagged id $ CFWriteVariable name CFValueInteger + + linkRange expression rw + + + T_FdRedirect _ name t -> do + build t + + T_ForArithmetic _ initT condT incT bodyT -> do + init <- build initT + cond <- build condT + body <- sequentially bodyT + inc <- build incT + end <- newStructuralNode + + -- Forward edges + linkRanges [init, cond, body, inc] + linkRange cond end + -- Backward edge + linkRange inc cond + return $ spanRange init end + + T_ForIn id name words body -> forInHelper id name words body + + -- For functions we generate an unlinked subgraph, and mention that in its definition node + T_Function id _ _ name body -> do + range <- local (\c -> c { cfExitTarget = Nothing }) $ do + entry <- newNodeRange $ CFEntryPoint $ "function " ++ name + f <- withFunctionScope $ build body + linkRange entry f + let (Range entry exit) = range + definition <- newNodeRange (applySingle $ IdTagged id $ CFDefineFunction name id entry exit) + exe <- newNodeRange (CFSetExitCode id) + linkRange definition exe + + T_Glob {} -> none + + T_HereString _ t -> build t + T_HereDoc _ _ _ _ list -> sequentially list + + T_IfExpression id ifs elses -> do + start <- newStructuralNode + branches <- doBranches start ifs elses [] + end <- newStructuralNode + mapM_ (`linkRange` end) branches + return $ spanRange start end + where + doBranches start ((conds, thens):rest) elses result = do + cond <- asCondition $ sequentially conds + action <- sequentially thens + linkRange start cond + linkRange cond action + doBranches cond rest elses (action:result) + doBranches start [] elses result = do + rest <- + if null elses + then newNodeRange (CFSetExitCode id) + else sequentially elses + linkRange start rest + return (rest:result) + + T_Include _ t -> build t + + T_IndexedElement _ indicesT valueT -> do + indices <- sequentially indicesT + value <- build valueT + linkRange indices value + + T_IoDuplicate _ op _ -> build op + + T_IoFile _ op t -> do + exp <- build t + doesntDoMuch <- build op + linkRange exp doesntDoMuch + + T_Literal {} -> none + + T_NormalWord _ list -> sequentially list + + T_OrIf _ lhs rhs -> do + left <- build lhs + right <- build rhs + end <- newStructuralNode + linkRange left right + linkRange right end + linkRange left end + + T_Pipeline _ _ [cmd] -> build cmd + T_Pipeline id _ cmds -> do + start <- newStructuralNode + hasLastpipe <- reader $ cfLastpipe . cfParameters + (leading, last) <- buildPipe hasLastpipe cmds + -- Ideally we'd let this exit code be that of the last command in the pipeline but ok + end <- newNodeRange $ CFSetExitCode id + + mapM_ (linkRange start) leading + mapM_ (\c -> linkRangeAs CFEFalseFlow c end) leading + linkRanges $ [start] ++ last ++ [end] + where + buildPipe True [x] = do + last <- build x + return ([], [last]) + buildPipe lp (first:rest) = do + this <- subshell id "pipeline" $ build first + (leading, last) <- buildPipe lp rest + return (this:leading, last) + buildPipe _ [] = return ([], []) + + T_ProcSub id op cmds -> do + start <- newStructuralNode + body <- subshell id (op ++ "() process substitution") $ sequentially cmds + end <- newStructuralNode + + linkRange start body + linkRangeAs CFEFalseFlow body end + linkRange start end + + T_Redirecting _ redirs cmd -> do + -- For simple commands, this is the other way around in bash + -- We do it in this order for comound commands like { x=name; } > "$x" + redir <- sequentially redirs + body <- build cmd + linkRange redir body + + T_SelectIn id name words body -> forInHelper id name words body + + T_SimpleCommand id vars [] -> do + -- Vars can also be empty, as in the command "> foo" + assignments <- sequentially vars + status <- newNodeRange (CFSetExitCode id) + linkRange assignments status + + T_SimpleCommand id vars list@(cmd:_) -> + handleCommand t vars list $ getUnquotedLiteral cmd + + T_SingleQuoted _ _ -> none + + T_SourceCommand _ originalCommand inlinedSource -> do + cmd <- build originalCommand + end <- newStructuralNode + inline <- withReturn end $ build inlinedSource + linkRange cmd inline + linkRange inline end + return $ spanRange cmd inline + + T_Subshell id body -> do + main <- subshell id "explicit (..) subshell" $ sequentially body + status <- newNodeRange (CFSetExitCode id) + linkRange main status + + T_UntilExpression id cond body -> whileHelper id cond body + T_WhileExpression id cond body -> whileHelper id cond body + + T_CLOBBER _ -> none + T_GREATAND _ -> none + T_LESSAND _ -> none + T_LESSGREAT _ -> none + T_DGREAT _ -> none + T_Greater _ -> none + T_Less _ -> none + T_ParamSubSpecialChar _ _ -> none + + x -> error ("Unimplemented: " ++ show x) + +-- Still in `where` clause + forInHelper id name words body = do + entry <- newStructuralNode + expansion <- sequentially words + assignmentChoice <- newStructuralNode + assignments <- + if null words || any willSplit words + then (:[]) <$> (newNodeRange $ applySingle $ IdTagged id $ CFWriteVariable name CFValueString) + else mapM (\t -> newNodeRange $ applySingle $ IdTagged id $ CFWriteVariable name $ CFValueComputed (getId t) $ tokenToParts t) words + body <- sequentially body + exit <- newStructuralNode + -- Forward edges + linkRanges [entry, expansion, assignmentChoice] + mapM_ (\t -> linkRanges [assignmentChoice, t, body]) assignments + linkRange body exit + linkRange expansion exit + -- Backward edge + linkRange body assignmentChoice + return $ spanRange entry exit + + whileHelper id cond body = do + condRange <- asCondition $ sequentially cond + bodyRange <- sequentially body + end <- newNodeRange (CFSetExitCode id) + + linkRange condRange bodyRange + linkRange bodyRange condRange + linkRange condRange end + + +handleCommand cmd vars args literalCmd = do + -- TODO: Handle assignments in declaring commands + + case literalCmd of + Just "exit" -> regularExpansion vars args $ handleExit + Just "return" -> regularExpansion vars args $ handleReturn + Just "unset" -> regularExpansionWithStatus vars args $ handleUnset args + + Just "declare" -> handleDeclare args + Just "local" -> handleDeclare args + Just "typeset" -> handleDeclare args + + Just "printf" -> regularExpansionWithStatus vars args $ handlePrintf args + Just "wait" -> regularExpansionWithStatus vars args $ handleWait args + + Just "mapfile" -> regularExpansionWithStatus vars args $ handleMapfile args + Just "readarray" -> regularExpansionWithStatus vars args $ handleMapfile args + + Just "read" -> regularExpansionWithStatus vars args $ handleRead args + + Just "DEFINE_boolean" -> regularExpansionWithStatus vars args $ handleDEFINE args + Just "DEFINE_float" -> regularExpansionWithStatus vars args $ handleDEFINE args + Just "DEFINE_integer" -> regularExpansionWithStatus vars args $ handleDEFINE args + Just "DEFINE_string" -> regularExpansionWithStatus vars args $ handleDEFINE args + + -- This will mostly behave like 'command' but ok + Just "builtin" -> + case args of + [_] -> regular + (_:newargs@(newcmd:_)) -> + handleCommand newcmd vars newargs $ getLiteralString newcmd + Just "command" -> + case args of + [_] -> regular + (_:newargs@(newcmd:_)) -> + handleOthers (getId newcmd) vars newargs $ getLiteralString newcmd + _ -> regular + + where + regular = handleOthers (getId cmd) vars args literalCmd + handleExit = do + exitNode <- reader cfExitTarget + case exitNode of + Just target -> do + exit <- newNode CFResolvedExit + link exit target CFEExit + unreachable <- newNode CFUnreachable + return $ Range exit unreachable + Nothing -> do + exit <- newNode CFUnresolvedExit + unreachable <- newNode CFUnreachable + return $ Range exit unreachable + + handleReturn = do + returnTarget <- reader cfReturnTarget + case returnTarget of + Nothing -> error $ pleaseReport "missing return target" + Just target -> do + ret <- newNode CFStructuralNode + link ret target CFEFlow + unreachable <- newNode CFUnreachable + return $ Range ret unreachable + + handleUnset (cmd:args) = do + case () of + _ | "n" `elem` flagNames -> unsetWith CFUndefineNameref + _ | "v" `elem` flagNames -> unsetWith CFUndefineVariable + _ | "f" `elem` flagNames -> unsetWith CFUndefineFunction + _ -> unsetWith CFUndefine + where + pairs :: [(String, Token)] -- [(Flag string, token)] e.g. [("-f", t), ("", myfunc)] + pairs = map (\(str, (flag, val)) -> (str, flag)) $ fromMaybe (map (\c -> ("", (c,c))) args) $ getGnuOpts "vfn" args + (names, flags) = partition (null . fst) pairs + flagNames = map fst flags + literalNames :: [(Token, String)] -- Literal names to unset, e.g. [(myfuncToken, "myfunc")] + literalNames = mapMaybe (\(_, t) -> getLiteralString t >>= (return . (,) t)) names + -- Apply a constructor like CFUndefineVariable to each literalName, and tag with its id + unsetWith c = newNodeRange $ CFApplyEffects $ map (\(token, name) -> IdTagged (getId token) $ c name) literalNames + + + variableAssignRegex = mkRegex "^([_a-zA-Z][_a-zA-Z0-9]*)=" + + handleDeclare (cmd:args) = do + isFunc <- asks cfIsFunction + -- This is a bit of a kludge: we don't have great support for things like + -- 'declare -i x=$x' so do one round with declare x=$x, followed by declare -i x + let (evaluated, assignments, added, removed) = mconcat $ map (toEffects isFunc) args + before <- sequentially $ evaluated + assignments <- newNodeRange $ CFApplyEffects assignments + addedProps <- if null added then newStructuralNode else newNodeRange $ CFApplyEffects added + removedProps <- if null removed then newStructuralNode else newNodeRange $ CFApplyEffects removed + result <- newNodeRange $ CFSetExitCode (getId cmd) + linkRanges [before, assignments, addedProps, removedProps, result] + where + opts = map fst $ getGenericOpts args + array = "a" `elem` opts || associative + associative = "A" `elem` opts + integer = "i" `elem` opts + func = "f" `elem` opts || "F" `elem` opts + global = "g" `elem` opts + export = "x" `elem` opts + writer isFunc = + case () of + _ | global -> CFWriteGlobal + _ | isFunc -> CFWriteLocal + _ -> CFWriteVariable + + scope isFunc = + case () of + _ | global -> GlobalScope + _ | isFunc -> LocalScope + _ -> DefaultScope + + addedProps = S.fromList $ concat $ [ + [ CFVPArray | array ], + [ CFVPInteger | integer ], + [ CFVPExport | export ], + [ CFVPAssociative | associative ] + ] + + removedProps = S.fromList $ concat $ [ + -- Array property can't be unset + [ CFVPInteger | 'i' `elem` unsetOptions ], + [ CFVPExport | 'e' `elem` unsetOptions ] + ] + + toEffects isFunc (T_Assignment id mode var idx t) = + let + pre = idx ++ [t] + val = [ IdTagged id $ (writer isFunc) var $ CFValueComputed (getId t) $ [ CFStringVariable var | mode == Append ] ++ tokenToParts t ] + added = [ IdTagged id $ CFSetProps (scope isFunc) var addedProps | not $ S.null addedProps ] + removed = [ IdTagged id $ CFUnsetProps (scope isFunc) var addedProps | not $ S.null removedProps ] + in + (pre, val, added, removed) + + toEffects isFunc t = + let + id = getId t + pre = [t] + literal = fromJust $ getLiteralStringExt (const $ Just "\0") t + isKnown = '\0' `notElem` literal + match = fmap head $ variableAssignRegex `matchRegex` literal + name = fromMaybe literal match + + asLiteral = + IdTagged id $ (writer isFunc) name $ + CFValueComputed (getId t) [ CFStringLiteral $ drop 1 $ dropWhile (/= '=') $ literal ] + asUnknown = + IdTagged id $ (writer isFunc) name $ + CFValueString + + added = [ IdTagged id $ CFSetProps (scope isFunc) name addedProps ] + removed = [ IdTagged id $ CFUnsetProps (scope isFunc) name removedProps ] + + in + case () of + _ | not (isVariableName name) -> (pre, [], [], []) + _ | isJust match && isKnown -> (pre, [asLiteral], added, removed) + _ | isJust match -> (pre, [asUnknown], added, removed) + -- e.g. declare -i x + _ -> (pre, [], added, removed) + + -- find "ia" from `define +i +a` + unsetOptions :: String + unsetOptions = + let + strings = mapMaybe getLiteralString args + plusses = filter ("+" `isPrefixOf`) strings + in + concatMap (drop 1) plusses + + handlePrintf (cmd:args) = + newNodeRange $ CFApplyEffects $ maybeToList findVar + where + findVar = do + flags <- getBsdOpts "v:" args + (flag, arg) <- lookup "v" flags + name <- getLiteralString arg + return $ IdTagged (getId arg) $ CFWriteVariable name CFValueString + + handleWait (cmd:args) = + newNodeRange $ CFApplyEffects $ maybeToList findVar + where + findVar = do + let flags = getGenericOpts args + (flag, arg) <- lookup "p" flags + name <- getLiteralString arg + return $ IdTagged (getId arg) $ CFWriteVariable name CFValueInteger + + handleMapfile (cmd:args) = + newNodeRange $ CFApplyEffects [findVar] + where + findVar = + let (id, name) = fromMaybe (getId cmd, "MAPFILE") $ getFromArg `mplus` getFromFallback + in IdTagged id $ CFWriteVariable name CFValueArray + + getFromArg = do + flags <- getGnuOpts flagsForMapfile args + (_, arg) <- lookup "" flags + name <- getLiteralString arg + return (getId arg, name) + + getFromFallback = + listToMaybe $ mapMaybe getIfVar $ reverse args + getIfVar c = do + name <- getLiteralString c + guard $ isVariableName name + return (getId c, name) + + handleRead (cmd:args) = newNodeRange $ CFApplyEffects main + where + main = fromMaybe fallback $ do + flags <- getGnuOpts flagsForRead args + return $ fromMaybe (withFields flags) $ withArray flags + + withArray :: [(String, (Token, Token))] -> Maybe [IdTagged CFEffect] + withArray flags = do + (_, token) <- lookup "a" flags + return $ fromMaybe [] $ do + name <- getLiteralString token + return [ IdTagged (getId token) $ CFWriteVariable name CFValueArray ] + + withFields flags = mapMaybe getAssignment flags + + getAssignment :: (String, (Token, Token)) -> Maybe (IdTagged CFEffect) + getAssignment f = do + ("", (t, _)) <- return f + name <- getLiteralString t + return $ IdTagged (getId t) $ CFWriteVariable name CFValueString + + fallback = + let + names = reverse $ map fromJust $ takeWhile isJust $ map (\c -> sequence (getId c, getLiteralString c)) $ reverse args + namesOrDefault = if null names then [(getId cmd, "REPLY")] else names + hasDashA = any (== "a") $ map fst $ getGenericOpts args + value = if hasDashA then CFValueArray else CFValueString + in + map (\(id, name) -> IdTagged id $ CFWriteVariable name value) namesOrDefault + + handleDEFINE (cmd:args) = + newNodeRange $ CFApplyEffects $ maybeToList findVar + where + findVar = do + name <- listToMaybe $ drop 1 args + str <- getLiteralString name + guard $ isVariableName str + return $ IdTagged (getId name) $ CFWriteVariable str CFValueString + + handleOthers id vars args cmd = + regularExpansion vars args $ do + exe <- newNodeRange $ CFExecuteCommand cmd + status <- newNodeRange $ CFSetExitCode id + linkRange exe status + + regularExpansion vars args p = do + args <- sequentially args + assignments <- mapM (buildAssignment PrefixScope) vars + exe <- p + dropAssignments <- + if null vars + then + return [] + else do + drop <- newNodeRange CFDropPrefixAssignments + return [drop] + + linkRanges $ [args] ++ assignments ++ [exe] ++ dropAssignments + + regularExpansionWithStatus vars args@(cmd:_) p = do + initial <- regularExpansion vars args p + status <- newNodeRange $ CFSetExitCode (getId cmd) + linkRange initial status + + +none = newStructuralNode + +data Scope = DefaultScope | GlobalScope | LocalScope | PrefixScope + deriving (Eq, Ord, Show, Generic, NFData) + +buildAssignment scope t = do + op <- case t of + T_Assignment id mode var indices value -> do + expand <- build value + index <- sequentially indices + read <- case mode of + Append -> newNodeRange (applySingle $ IdTagged id $ CFReadVariable var) + Assign -> none + let valueType = if null indices then f id value else CFValueArray + let scoper = + case scope of + PrefixScope -> CFWritePrefix + LocalScope -> CFWriteLocal + GlobalScope -> CFWriteGlobal + DefaultScope -> CFWriteVariable + write <- newNodeRange $ applySingle $ IdTagged id $ scoper var valueType + linkRanges [expand, index, read, write] + where + f :: Id -> Token -> CFValue + f id t@T_NormalWord {} = CFValueComputed id $ [CFStringVariable var | mode == Append] ++ tokenToParts t + f id t@(T_Literal _ str) = CFValueComputed id $ [CFStringVariable var | mode == Append] ++ tokenToParts t + f _ T_Array {} = CFValueArray + + registerNode (getId t) op + return op + + +tokenToParts t = + case t of + T_NormalWord _ list -> concatMap tokenToParts list + T_DoubleQuoted _ list -> concatMap tokenToParts list + T_SingleQuoted _ str -> [ CFStringLiteral str ] + T_Literal _ str -> [ CFStringLiteral str ] + T_DollarArithmetic {} -> [ CFStringInteger ] + T_DollarBracket {} -> [ CFStringInteger ] + T_DollarBraced _ _ list | isUnmodifiedParameterExpansion t -> [ CFStringVariable (getBracedReference $ concat $ oversimplify list) ] + -- Check if getLiteralString can handle it, if not it's unknown + _ -> [maybe CFStringUnknown CFStringLiteral $ getLiteralString t] + + +-- Like & but well defined when the node already exists +safeUpdate ctx@(_,node,_,_) graph = ctx & (delNode node graph) + +-- Change all subshell invocations to instead link directly to their contents. +-- This is used for producing dominator trees. +inlineSubshells :: CFGraph -> CFGraph +inlineSubshells graph = relinkedGraph + where + subshells = ufold find [] graph + find (incoming, node, label, outgoing) acc = + case label of + CFExecuteSubshell _ start end -> (node, label, start, end, incoming, outgoing):acc + _ -> acc + + relinkedGraph = foldl' relink graph subshells + relink graph (node, label, start, end, incoming, outgoing) = + let + -- Link CFExecuteSubshell to the CFEntryPoint + subshellToStart = (incoming, node, label, [(CFEFlow, start)]) + -- Link the subshell exit to the + endToNexts = (endIncoming, endNode, endLabel, outgoing) + (endIncoming, endNode, endLabel, _) = context graph end + in + subshellToStart `safeUpdate` (endToNexts `safeUpdate` graph) + +findEntryNodes :: CFGraph -> [Node] +findEntryNodes graph = ufold find [] graph + where + find (incoming, node, label, _) list = + case label of + CFEntryPoint {} | null incoming -> node:list + _ -> list + +findDominators main graph = asSetMap + where + inlined = inlineSubshells graph + entryNodes = main : findEntryNodes graph + asLists = concatMap (dom inlined) entryNodes + asSetMap = M.fromList $ map (\(node, list) -> (node, S.fromList list)) asLists + +findTerminalNodes :: CFGraph -> [Node] +findTerminalNodes graph = ufold find [] graph + where + find (_, node, label, _) list = + case label of + CFUnresolvedExit -> node:list + CFApplyEffects effects -> f effects list + _ -> list + + f [] list = list + f (IdTagged _ (CFDefineFunction _ id start end):rest) list = f rest (end:list) + f (_:rest) list = f rest list + +findPostDominators :: Node -> CFGraph -> Array Node [Node] +findPostDominators mainexit graph = asArray + where + inlined = inlineSubshells graph + terminals = findTerminalNodes inlined + (incoming, _, label, outgoing) = context graph mainexit + withExitEdges = (incoming ++ map (\c -> (CFEFlow, c)) terminals, mainexit, label, outgoing) `safeUpdate` inlined + reversed = grev withExitEdges + postDoms = dom reversed mainexit + (_, maxNode) = nodeRange graph + asArray = array (0, maxNode) postDoms + +return [] +runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/src/ShellCheck/CFGAnalysis.hs b/src/ShellCheck/CFGAnalysis.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/CFGAnalysis.hs @@ -0,0 +1,1424 @@ +{- + Copyright 2022 Vidar Holen + + This file is part of ShellCheck. + https://www.shellcheck.net + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +{-# LANGUAGE TemplateHaskell #-} +{-# LANGUAGE RankNTypes #-} +{-# LANGUAGE DeriveAnyClass, DeriveGeneric #-} +{-# LANGUAGE CPP #-} + +{- + Data Flow Analysis on a Control Flow Graph. + + This module implements a pretty standard iterative Data Flow Analysis. + For an overview of the process, see Wikipedia. + + Since shell scripts rely heavily on global variables, this DFA includes + tracking the value of globals across calls. Each function invocation is + treated as a separate DFA problem, and a caching mechanism (hopefully) + avoids any exponential explosions. + + To do efficient DFA join operations (or merges, as the code calls them), + some of the data structures have an integer version attached. On update, + the version is changed. If two states have the same version number, + a merge is skipped on the grounds that they are identical. It is easy + to unintentionally forget to update/invalidate the version number, + and bugs will ensure. + + For performance reasons, the entire code runs in plain ST, with a manual + context object Ctx being passed around. It relies heavily on mutable + STRefs. However, this turned out to be literally thousands of times faster + than my several attempts using RWST, so it can't be helped. +-} + +module ShellCheck.CFGAnalysis ( + analyzeControlFlow + ,CFGParameters (..) + ,CFGAnalysis (..) + ,ProgramState (..) + ,VariableState (..) + ,VariableValue (..) + ,VariableProperties + ,SpaceStatus (..) + ,NumericalStatus (..) + ,getIncomingState + ,getOutgoingState + ,doesPostDominate + ,ShellCheck.CFGAnalysis.runTests -- STRIP + ) where + +import Control.DeepSeq +import Control.Monad +import Control.Monad.ST +import Data.Array.Unboxed +import Data.Char +import Data.Graph.Inductive.Graph +import Data.Graph.Inductive.Query.DFS +import Data.List hiding (map) +import Data.Maybe +import Data.STRef +import Debug.Trace -- STRIP +import GHC.Generics (Generic) +import qualified Data.Map as M +import qualified Data.Set as S +import qualified ShellCheck.Data as Data +import ShellCheck.AST +import ShellCheck.CFG +import ShellCheck.Prelude + +import Test.QuickCheck + + +-- The number of iterations for DFA to stabilize +iterationCount = 1000000 +-- There have been multiple bugs where bad caching caused oscillations. +-- As a precaution, disable caching if there's this many iterations left. +fallbackThreshold = 10000 +-- The number of cache entries to keep per node +cacheEntries = 10 + +logVerbose log = do + -- traceShowM log + return () +logInfo log = do + -- traceShowM log + return () + +-- The result of the data flow analysis +data CFGAnalysis = CFGAnalysis { + graph :: CFGraph, + tokenToRange :: M.Map Id (Node, Node), + tokenToNodes :: M.Map Id (S.Set Node), + postDominators :: Array Node [Node], + nodeToData :: M.Map Node (ProgramState, ProgramState) +} deriving (Show) + +-- The program state we expose externally +data ProgramState = ProgramState { + -- internalState :: InternalState, -- For debugging + variablesInScope :: M.Map String VariableState, + exitCodes :: S.Set Id, + stateIsReachable :: Bool +} deriving (Show, Eq, Generic, NFData) + +internalToExternal :: InternalState -> ProgramState +internalToExternal s = + ProgramState { + -- Censor the literal value to avoid introducing dependencies on it. It's just for debugging. + variablesInScope = M.map censor flatVars, + -- internalState = s, -- For debugging + exitCodes = fromMaybe S.empty $ sExitCodes s, + stateIsReachable = fromMaybe True $ sIsReachable s + } + where + censor s = s { + variableValue = (variableValue s) { + literalValue = Nothing + } + } + flatVars = M.unionsWith (\_ last -> last) $ map mapStorage [sGlobalValues s, sLocalValues s, sPrefixValues s] + +-- Conveniently get the state before a token id +getIncomingState :: CFGAnalysis -> Id -> Maybe ProgramState +getIncomingState analysis id = do + (start,end) <- M.lookup id $ tokenToRange analysis + fst <$> M.lookup start (nodeToData analysis) + +-- Conveniently get the state after a token id +getOutgoingState :: CFGAnalysis -> Id -> Maybe ProgramState +getOutgoingState analysis id = do + (start,end) <- M.lookup id $ tokenToRange analysis + snd <$> M.lookup end (nodeToData analysis) + +-- Conveniently determine whether one node postdominates another, +-- i.e. whether 'target' always unconditionally runs after 'base'. +doesPostDominate :: CFGAnalysis -> Id -> Id -> Bool +doesPostDominate analysis target base = fromMaybe False $ do + (_, baseEnd) <- M.lookup base $ tokenToRange analysis + (targetStart, _) <- M.lookup target $ tokenToRange analysis + return $ targetStart `elem` (postDominators analysis ! baseEnd) + +getDataForNode analysis node = M.lookup node $ nodeToData analysis + +-- The current state of data flow at a point in the program, potentially as a diff +data InternalState = InternalState { + sVersion :: Integer, + sGlobalValues :: VersionedMap String VariableState, + sLocalValues :: VersionedMap String VariableState, + sPrefixValues :: VersionedMap String VariableState, + sFunctionTargets :: VersionedMap String FunctionValue, + sExitCodes :: Maybe (S.Set Id), + sIsReachable :: Maybe Bool +} deriving (Show, Generic, NFData) + +newInternalState = InternalState { + sVersion = 0, + sGlobalValues = vmEmpty, + sLocalValues = vmEmpty, + sPrefixValues = vmEmpty, + sFunctionTargets = vmEmpty, + sExitCodes = Nothing, + sIsReachable = Nothing +} + +unreachableState = modified newInternalState { + sIsReachable = Just False +} + +-- The default state we assume we get from the environment +createEnvironmentState :: InternalState +createEnvironmentState = do + foldl' (flip ($)) newInternalState $ concat [ + addVars Data.internalVariables unknownVariableState, + addVars Data.variablesWithoutSpaces spacelessVariableState, + addVars Data.specialIntegerVariables integerVariableState + ] + where + addVars names val = map (\name -> insertGlobal name val) names + spacelessVariableState = unknownVariableState { + variableValue = VariableValue { + literalValue = Nothing, + spaceStatus = SpaceStatusClean, + numericalStatus = NumericalStatusUnknown + } + } + integerVariableState = unknownVariableState { + variableValue = unknownIntegerValue + } + + +modified s = s { sVersion = -1 } + +insertGlobal :: String -> VariableState -> InternalState -> InternalState +insertGlobal name value state = modified state { + sGlobalValues = vmInsert name value $ sGlobalValues state +} + +insertLocal :: String -> VariableState -> InternalState -> InternalState +insertLocal name value state = modified state { + sLocalValues = vmInsert name value $ sLocalValues state +} + +insertPrefix :: String -> VariableState -> InternalState -> InternalState +insertPrefix name value state = modified state { + sPrefixValues = vmInsert name value $ sPrefixValues state +} + +insertFunction :: String -> FunctionValue -> InternalState -> InternalState +insertFunction name value state = modified state { + sFunctionTargets = vmInsert name value $ sFunctionTargets state +} + +addProperties :: S.Set CFVariableProp -> VariableState -> VariableState +addProperties props state = state { + variableProperties = S.map (S.union props) $ variableProperties state +} + +removeProperties :: S.Set CFVariableProp -> VariableState -> VariableState +removeProperties props state = state { + variableProperties = S.map (\s -> S.difference s props) $ variableProperties state +} + +setExitCode id = setExitCodes (S.singleton id) +setExitCodes set state = modified state { + sExitCodes = Just $ set +} + +-- Dependencies on values, e.g. "if there is a global variable named 'foo' without spaces" +-- This is used to see if the DFA of a function would result in the same state, so anything +-- that affects DFA must be tracked. +data StateDependency = + -- Complete variable state + DepState Scope String VariableState + -- Only variable properties (we need properties but not values for x=1) + | DepProperties Scope String VariableProperties + -- Function definition + | DepFunction String (S.Set FunctionDefinition) + -- Whether invoking the node would result in recursion (i.e., is the function on the stack?) + | DepIsRecursive Node Bool + -- The set of commands that could have provided the exit code $? + | DepExitCodes (S.Set Id) + deriving (Show, Eq, Ord, Generic, NFData) + +-- A function definition, or lack thereof +data FunctionDefinition = FunctionUnknown | FunctionDefinition String Node Node + deriving (Show, Eq, Ord, Generic, NFData) + +-- The Set of places a command name can point (it's a Set to handle conditionally defined functions) +type FunctionValue = S.Set FunctionDefinition + +-- Create an InternalState that fulfills the given dependencies +depsToState :: S.Set StateDependency -> InternalState +depsToState set = foldl insert newInternalState $ S.toList set + where + insert :: InternalState -> StateDependency -> InternalState + insert state dep = + case dep of + DepFunction name val -> insertFunction name val state + DepState scope name val -> insertIn True scope name val state + -- State includes properties and more, so don't overwrite a state with properties + DepProperties scope name props -> insertIn False scope name unknownVariableState { variableProperties = props } state + DepIsRecursive _ _ -> state + DepExitCodes s -> setExitCodes s state + + insertIn overwrite scope name val state = + let + (mapToCheck, inserter) = + case scope of + PrefixScope -> (sPrefixValues, insertPrefix) + LocalScope -> (sLocalValues, insertLocal) + GlobalScope -> (sGlobalValues, insertGlobal) + DefaultScope -> error $ pleaseReport "Unresolved scope in dependency" + + alreadyExists = isJust $ vmLookup name $ mapToCheck state + in + if overwrite || not alreadyExists + then inserter name val state + else state + +unknownFunctionValue = S.singleton FunctionUnknown + +-- The information about the value of a single variable +data VariableValue = VariableValue { + literalValue :: Maybe String, -- TODO: For debugging. Remove me. + spaceStatus :: SpaceStatus, + numericalStatus :: NumericalStatus +} + deriving (Show, Eq, Ord, Generic, NFData) + +data VariableState = VariableState { + variableValue :: VariableValue, + variableProperties :: VariableProperties +} + deriving (Show, Eq, Ord, Generic, NFData) + +-- Whether or not the value needs quoting (has spaces/globs), or we don't know +data SpaceStatus = SpaceStatusEmpty | SpaceStatusClean | SpaceStatusDirty deriving (Show, Eq, Ord, Generic, NFData) +-- +-- Whether or not the value needs quoting (has spaces/globs), or we don't know +data NumericalStatus = NumericalStatusUnknown | NumericalStatusEmpty | NumericalStatusMaybe | NumericalStatusDefinitely deriving (Show, Eq, Ord, Generic, NFData) + +-- The set of possible sets of properties for this variable +type VariableProperties = S.Set (S.Set CFVariableProp) + +defaultProperties = S.singleton S.empty + +unknownVariableState = VariableState { + variableValue = unknownVariableValue, + variableProperties = defaultProperties +} + +unknownVariableValue = VariableValue { + literalValue = Nothing, + spaceStatus = SpaceStatusDirty, + numericalStatus = NumericalStatusUnknown +} + +emptyVariableValue = unknownVariableValue { + literalValue = Just "", + spaceStatus = SpaceStatusEmpty, + numericalStatus = NumericalStatusEmpty +} + +unsetVariableState = VariableState { + variableValue = emptyVariableValue, + variableProperties = defaultProperties +} + +mergeVariableState a b = VariableState { + variableValue = mergeVariableValue (variableValue a) (variableValue b), + variableProperties = S.union (variableProperties a) (variableProperties b) +} + +mergeVariableValue a b = VariableValue { + literalValue = if literalValue a == literalValue b then literalValue a else Nothing, + spaceStatus = mergeSpaceStatus (spaceStatus a) (spaceStatus b), + numericalStatus = mergeNumericalStatus (numericalStatus a) (numericalStatus b) +} + +mergeSpaceStatus a b = + case (a,b) of + (SpaceStatusEmpty, y) -> y + (x, SpaceStatusEmpty) -> x + (SpaceStatusClean, SpaceStatusClean) -> SpaceStatusClean + _ -> SpaceStatusDirty + +mergeNumericalStatus a b = + case (a,b) of + (NumericalStatusDefinitely, NumericalStatusDefinitely) -> NumericalStatusDefinitely + (NumericalStatusDefinitely, _) -> NumericalStatusMaybe + (_, NumericalStatusDefinitely) -> NumericalStatusMaybe + (NumericalStatusMaybe, _) -> NumericalStatusMaybe + (_, NumericalStatusMaybe) -> NumericalStatusMaybe + (NumericalStatusEmpty, NumericalStatusEmpty) -> NumericalStatusEmpty + _ -> NumericalStatusUnknown + +-- A VersionedMap is a Map that keeps an additional integer version to quickly determine if it has changed. +-- * Version -1 means it's unknown (possibly and presumably changed) +-- * Version 0 means it's empty +-- * Version N means it's equal to any other map with Version N (this is required but not enforced) +data VersionedMap k v = VersionedMap { + mapVersion :: Integer, + mapStorage :: M.Map k v +} + deriving (Generic, NFData) + +-- This makes states more readable but inhibits copy-paste +instance (Show k, Show v) => Show (VersionedMap k v) where + show m = (if mapVersion m >= 0 then "V" ++ show (mapVersion m) else "U") ++ " " ++ show (mapStorage m) + +instance Eq InternalState where + (==) a b = stateIsQuickEqual a b || stateIsSlowEqual a b + +instance (Eq k, Eq v) => Eq (VersionedMap k v) where + (==) a b = vmIsQuickEqual a b || mapStorage a == mapStorage b + +instance (Ord k, Ord v) => Ord (VersionedMap k v) where + compare a b = + if vmIsQuickEqual a b + then EQ + else mapStorage a `compare` mapStorage b + + +-- A context with STRefs manually passed around to function. +-- This is done because it was dramatically much faster than any RWS type stack +data Ctx s = Ctx { + -- The current node + cNode :: STRef s Node, + -- The current input state + cInput :: STRef s InternalState, + -- The current output state + cOutput :: STRef s InternalState, + + -- The current functions/subshells stack + cStack :: [StackEntry s], + -- The input graph + cGraph :: CFGraph, + -- An incrementing counter to version maps + cCounter :: STRef s Integer, + -- A cache of input state dependencies to output effects + cCache :: STRef s (M.Map Node [(S.Set StateDependency, InternalState)]), + -- Whether the cache is enabled (see fallbackThreshold) + cEnableCache :: STRef s Bool, + -- The states resulting from data flows per invocation path + cInvocations :: STRef s (M.Map [Node] (S.Set StateDependency, M.Map Node (InternalState, InternalState))) +} + +-- Whenever a function (or subshell) is invoked, a value like this is pushed onto the stack +data StackEntry s = StackEntry { + -- The entry point of this stack entry for the purpose of detecting recursion + entryPoint :: Node, + -- Whether this is a function call (as opposed to a subshell) + isFunctionCall :: Bool, + -- The node where this entry point was invoked + callSite :: Node, + -- A mutable set of dependencies we fetched from here or higher in the stack + dependencies :: STRef s (S.Set StateDependency), + -- The original input state for this stack entry + stackState :: InternalState +} + deriving (Eq, Generic, NFData) + +#if MIN_VERSION_deepseq(1,4,2) +-- Our deepseq already has a STRef instance +#else +-- Older deepseq (for GHC < 8) lacks this instance +instance NFData (STRef s a) where + rnf = (`seq` ()) +#endif + +-- Overwrite a base state with the contents of a diff state +-- This is unrelated to join/merge. +patchState :: InternalState -> InternalState -> InternalState +patchState base diff = + case () of + _ | sVersion diff == 0 -> base + _ | sVersion base == 0 -> diff + _ | stateIsQuickEqual base diff -> diff + _ -> + InternalState { + sVersion = -1, + sGlobalValues = vmPatch (sGlobalValues base) (sGlobalValues diff), + sLocalValues = vmPatch (sLocalValues base) (sLocalValues diff), + sPrefixValues = vmPatch (sPrefixValues base) (sPrefixValues diff), + sFunctionTargets = vmPatch (sFunctionTargets base) (sFunctionTargets diff), + sExitCodes = sExitCodes diff `mplus` sExitCodes base, + sIsReachable = sIsReachable diff `mplus` sIsReachable base + } + +patchOutputM ctx diff = do + let cOut = cOutput ctx + oldState <- readSTRef cOut + let newState = patchState oldState diff + writeSTRef cOut newState + +-- Merge (aka Join) two states. This is monadic because it requires looking up +-- values from the current context. For example: +-- +-- f() { +-- foo || x=2 +-- HERE # This merge requires looking up the value of $x in the parent frame +-- } +-- x=1 +-- f +mergeState :: forall s. Ctx s -> InternalState -> InternalState -> ST s InternalState +mergeState ctx a b = do + -- Kludge: we want `readVariable` & friends not to read from an intermediate state, + -- so temporarily set a blank input. + let cin = cInput ctx + old <- readSTRef cin + writeSTRef cin newInternalState + x <- merge a b + writeSTRef cin old + return x + + where + + merge a b = + case () of + _ | sIsReachable a == Just True && sIsReachable b == Just False + || sIsReachable a == Just False && sIsReachable b == Just True -> + error $ pleaseReport "Unexpected merge of reachable and unreachable state" + _ | sIsReachable a == Just False && sIsReachable b == Just False -> + return unreachableState + _ | sVersion a >= 0 && sVersion b >= 0 && sVersion a == sVersion b -> return a + _ -> do + globals <- mergeMaps ctx mergeVariableState readGlobal (sGlobalValues a) (sGlobalValues b) + locals <- mergeMaps ctx mergeVariableState readVariable (sLocalValues a) (sLocalValues b) + prefix <- mergeMaps ctx mergeVariableState readVariable (sPrefixValues a) (sPrefixValues b) + funcs <- mergeMaps ctx S.union readFunction (sFunctionTargets a) (sFunctionTargets b) + exitCodes <- mergeMaybes ctx S.union readExitCodes (sExitCodes a) (sExitCodes b) + return $ InternalState { + sVersion = -1, + sGlobalValues = globals, + sLocalValues = locals, + sPrefixValues = prefix, + sFunctionTargets = funcs, + sExitCodes = exitCodes, + sIsReachable = liftM2 (&&) (sIsReachable a) (sIsReachable b) + } + +-- Merge a number of states, or return a default if there are no states +-- (it can't fold from newInternalState because this would be equivalent of adding a new input edge). +mergeStates :: forall s. Ctx s -> InternalState -> [InternalState] -> ST s InternalState +mergeStates ctx def list = + case list of + [] -> return def + (first:rest) -> foldM (mergeState ctx) first rest + +-- Merge two maps, key by key. If both maps have a key, the 'merger' is used. +-- If only one has the key, the 'reader' is used to fetch a second, and the two are merged as above. +mergeMaps :: (Ord k) => forall s. + Ctx s -> + (v -> v -> v) -> + (Ctx s -> k -> ST s v) -> + (VersionedMap k v) -> + (VersionedMap k v) -> + ST s (VersionedMap k v) +mergeMaps ctx merger reader a b = + if vmIsQuickEqual a b + then return a + else do + new <- M.fromDistinctAscList <$> reverse <$> f [] (M.toAscList $ mapStorage a) (M.toAscList $ mapStorage b) + vmFromMap ctx new + where + f l [] [] = return l + f l [] b = f l b [] + f l ((k,v):rest1) [] = do + other <- reader ctx k + f ((k, merger v other):l) rest1 [] + f l l1@((k1, v1):rest1) l2@((k2, v2):rest2) = + case k1 `compare` k2 of + EQ -> + f ((k1, merger v1 v2):l) rest1 rest2 + LT -> do + nv2 <- reader ctx k1 + f ((k1, merger v1 nv2):l) rest1 l2 + GT -> do + nv1 <- reader ctx k2 + f ((k2, merger nv1 v2):l) l1 rest2 + +-- Merge two Maybes, like mergeMaps for a single element +mergeMaybes ctx merger reader a b = + case (a, b) of + (Nothing, Nothing) -> return Nothing + (Just v1, Nothing) -> single v1 + (Nothing, Just v2) -> single v2 + (Just v1, Just v2) -> return $ Just $ merger v1 v2 + where + single val = do + result <- merger val <$> reader ctx + return $ Just result + +vmFromMap ctx map = return $ VersionedMap { + mapVersion = -1, + mapStorage = map +} + +-- Give a VersionedMap a version if it does not already have one. +versionMap ctx map = + if mapVersion map >= 0 + then return map + else do + v <- nextVersion ctx + return map { + mapVersion = v + } + +-- Give an InternalState a version if it does not already have one. +versionState ctx state = + if sVersion state >= 0 + then return state + else do + self <- nextVersion ctx + ssGlobalValues <- versionMap ctx $ sGlobalValues state + ssLocalValues <- versionMap ctx $ sLocalValues state + ssFunctionTargets <- versionMap ctx $ sFunctionTargets state + return state { + sVersion = self, + sGlobalValues = ssGlobalValues, + sLocalValues = ssLocalValues, + sFunctionTargets = ssFunctionTargets + } + +-- Like 'not null' but for 2+ elements +is2plus :: [a] -> Bool +is2plus l = case l of + _:_:_ -> True + _ -> False + +-- Use versions to see if two states are trivially identical +stateIsQuickEqual a b = + let + va = sVersion a + vb = sVersion b + in + va >= 0 && vb >= 0 && va == vb + +-- A manual slow path 'Eq' (it's not derived because it's part of the custom Eq instance) +stateIsSlowEqual a b = + check sGlobalValues + && check sLocalValues + && check sPrefixValues + && check sFunctionTargets + && check sIsReachable + where + check f = f a == f b + +-- Check if two VersionedMaps are trivially equal +vmIsQuickEqual :: VersionedMap k v -> VersionedMap k v -> Bool +vmIsQuickEqual a b = + let + va = mapVersion a + vb = mapVersion b + in + va >= 0 && vb >= 0 && va == vb + +-- A new, empty VersionedMap +vmEmpty = VersionedMap { + mapVersion = 0, + mapStorage = M.empty +} + +-- Map.null for VersionedMaps +vmNull :: VersionedMap k v -> Bool +vmNull m = mapVersion m == 0 || (M.null $ mapStorage m) + +-- Map.lookup for VersionedMaps +vmLookup name map = M.lookup name $ mapStorage map + +-- Map.insert for VersionedMaps +vmInsert key val map = VersionedMap { + mapVersion = -1, + mapStorage = M.insert key val $ mapStorage map +} + +-- Overwrite all keys in the first map with values from the second +vmPatch :: (Ord k) => VersionedMap k v -> VersionedMap k v -> VersionedMap k v +vmPatch base diff = + case () of + _ | mapVersion base == 0 -> diff + _ | mapVersion diff == 0 -> base + _ | vmIsQuickEqual base diff -> diff + _ -> VersionedMap { + mapVersion = -1, + mapStorage = M.unionWith (flip const) (mapStorage base) (mapStorage diff) + } + +-- Set a variable. This includes properties. Applies it to the appropriate scope. +writeVariable :: forall s. Ctx s -> String -> VariableState -> ST s () +writeVariable ctx name val = do + typ <- readVariableScope ctx name + case typ of + GlobalScope -> writeGlobal ctx name val + LocalScope -> writeLocal ctx name val + -- Prefixed variables actually become local variables in the invoked function + PrefixScope -> writeLocal ctx name val + +writeGlobal ctx name val = do + modifySTRef (cOutput ctx) $ insertGlobal name val + +writeLocal ctx name val = do + modifySTRef (cOutput ctx) $ insertLocal name val + +writePrefix ctx name val = do + modifySTRef (cOutput ctx) $ insertPrefix name val + +updateVariableValue ctx name val = do + (props, scope) <- readVariablePropertiesWithScope ctx name + let f = case scope of + GlobalScope -> writeGlobal + LocalScope -> writeLocal + PrefixScope -> writeLocal -- Updates become local + f ctx name $ VariableState { variableValue = val, variableProperties = props } + +updateGlobalValue ctx name val = do + props <- readGlobalProperties ctx name + writeGlobal ctx name VariableState { variableValue = val, variableProperties = props } + +updateLocalValue ctx name val = do + props <- readLocalProperties ctx name + writeLocal ctx name VariableState { variableValue = val, variableProperties = props } + +updatePrefixValue ctx name val = do + -- Prefix variables don't inherit properties + writePrefix ctx name VariableState { variableValue = val, variableProperties = defaultProperties } + + +-- Look up a variable value, and also return its scope +readVariableWithScope :: forall s. Ctx s -> String -> ST s (VariableState, Scope) +readVariableWithScope ctx name = lookupStack get dep def ctx name + where + def = (unknownVariableState, GlobalScope) + get = getVariableWithScope + dep k (val, scope) = DepState scope k val + +-- Look up the variable's properties. This can be done independently to avoid incurring a dependency on the value. +readVariablePropertiesWithScope :: forall s. Ctx s -> String -> ST s (VariableProperties, Scope) +readVariablePropertiesWithScope ctx name = lookupStack get dep def ctx name + where + def = (defaultProperties, GlobalScope) + get s k = do + (val, scope) <- getVariableWithScope s k + return (variableProperties val, scope) + dep k (val, scope) = DepProperties scope k val + +readVariableScope ctx name = snd <$> readVariablePropertiesWithScope ctx name + +getVariableWithScope :: InternalState -> String -> Maybe (VariableState, Scope) +getVariableWithScope s name = + case (vmLookup name $ sPrefixValues s, vmLookup name $ sLocalValues s, vmLookup name $ sGlobalValues s) of + (Just var, _, _) -> return (var, PrefixScope) + (_, Just var, _) -> return (var, LocalScope) + (_, _, Just var) -> return (var, GlobalScope) + _ -> Nothing + +undefineFunction ctx name = + writeFunction ctx name $ FunctionUnknown + +undefineVariable ctx name = + writeVariable ctx name $ unsetVariableState + +readVariable ctx name = fst <$> readVariableWithScope ctx name +readVariableProperties ctx name = fst <$> readVariablePropertiesWithScope ctx name + +readGlobal ctx name = lookupStack get dep def ctx name + where + def = unknownVariableState -- could come from the environment + get s name = vmLookup name $ sGlobalValues s + dep k v = DepState GlobalScope k v + + +readGlobalProperties ctx name = lookupStack get dep def ctx name + where + def = defaultProperties + get s name = variableProperties <$> (vmLookup name $ sGlobalValues s) + -- This dependency will fail to match if it's shadowed by a local variable, + -- such as in x=1; f() { local -i x; declare -ag x; } because we'll look at + -- x and find it to be local and not global. FIXME? + dep k v = DepProperties GlobalScope k v + +readLocal ctx name = lookupStackUntilFunction get dep def ctx name + where + def = unsetVariableState -- can't come from the environment + get s name = vmLookup name $ sLocalValues s + dep k v = DepState LocalScope k v + +-- We only want to look up the local properties of the current function, +-- though preferably even if we're in a subshell. FIXME? +readLocalProperties ctx name = fst <$> lookupStackUntilFunction get dep def ctx name + where + def = (defaultProperties, LocalScope) + with tag f = do + val <- variableProperties <$> f + return (val, tag) + + get s name = (with LocalScope $ vmLookup name $ sLocalValues s) `mplus` (with PrefixScope $ vmLookup name $ sPrefixValues s) + dep k (val, scope) = DepProperties scope k val + +readFunction ctx name = lookupStack get dep def ctx name + where + def = unknownFunctionValue + get s name = vmLookup name $ sFunctionTargets s + dep k v = DepFunction k v + +writeFunction ctx name val = do + modifySTRef (cOutput ctx) $ insertFunction name $ S.singleton val + +readExitCodes ctx = lookupStack get dep def ctx () + where + get s () = sExitCodes s + def = S.empty + dep () v = DepExitCodes v + +-- Look up each state on the stack until a value is found (or the default is used), +-- then add this value as a StateDependency. +lookupStack' :: forall s k v. + -- Whether to stop at function boundaries + Bool + -- A function that maybe finds a value from a state + -> (InternalState -> k -> Maybe v) + -- A function that creates a dependency on what was found + -> (k -> v -> StateDependency) + -- A default value, if the value can't be found anywhere + -> v + -- Context + -> Ctx s + -- The key to look up + -> k + -- Returning the result + -> ST s v +lookupStack' functionOnly get dep def ctx key = do + top <- readSTRef $ cInput ctx + case get top key of + Just v -> return v + Nothing -> f (cStack ctx) + where + f [] = return def + f (s:_) | functionOnly && isFunctionCall s = return def + f (s:rest) = do + -- Go up the stack until we find the value, and add + -- a dependency on each state (including where it was found) + res <- fromMaybe (f rest) (return <$> get (stackState s) key) + modifySTRef (dependencies s) $ S.insert $ dep key res + return res + +lookupStack = lookupStack' False +lookupStackUntilFunction = lookupStack' True + +-- Like lookupStack but without adding dependencies +peekStack get def ctx key = do + top <- readSTRef $ cInput ctx + case get top key of + Just v -> return v + Nothing -> f (cStack ctx) + where + f [] = return def + f (s:rest) = + case get (stackState s) key of + Just v -> return v + Nothing -> f rest + +-- Check if the current context fulfills a StateDependency if entering `entry` +fulfillsDependency ctx entry dep = + case dep of + DepState scope name val -> (== (val, scope)) <$> peek scope ctx name + DepProperties scope name props -> do + (state, s) <- peek scope ctx name + return $ scope == s && variableProperties state == props + DepFunction name val -> (== val) <$> peekFunc ctx name + -- Hack. Since we haven't pushed the soon-to-be invoked function on the stack, + -- it won't be found by the normal check. + DepIsRecursive node val | node == entry -> return True + DepIsRecursive node val -> return $ val == any (\f -> entryPoint f == node) (cStack ctx) + DepExitCodes val -> (== val) <$> peekStack (\s k -> sExitCodes s) S.empty ctx () + -- _ -> error $ "Unknown dep " ++ show dep + where + peek scope = peekStack getVariableWithScope $ if scope == GlobalScope then (unknownVariableState, GlobalScope) else (unsetVariableState, LocalScope) + peekFunc = peekStack (\state name -> vmLookup name $ sFunctionTargets state) unknownFunctionValue + +-- Check if the current context fulfills all StateDependencies +fulfillsDependencies ctx entry deps = + f $ S.toList deps + where + f [] = return True + f (dep:rest) = do + res <- fulfillsDependency ctx entry dep + if res + then f rest + else return False + +-- Create a brand new Ctx given a Control Flow Graph (CFG) +newCtx g = do + c <- newSTRef 1 + input <- newSTRef undefined + output <- newSTRef undefined + node <- newSTRef undefined + cache <- newSTRef M.empty + enableCache <- newSTRef True + invocations <- newSTRef M.empty + return $ Ctx { + cCounter = c, + cInput = input, + cOutput = output, + cNode = node, + cCache = cache, + cEnableCache = enableCache, + cStack = [], + cInvocations = invocations, + cGraph = g + } + +-- The next incrementing version for VersionedMaps +nextVersion ctx = do + let ctr = cCounter ctx + n <- readSTRef ctr + writeSTRef ctr $! n+1 + return n + +-- Create a new StackEntry +newStackEntry ctx point isCall = do + deps <- newSTRef S.empty + state <- readSTRef $ cOutput ctx + callsite <- readSTRef $ cNode ctx + return $ StackEntry { + entryPoint = point, + isFunctionCall = isCall, + callSite = callsite, + dependencies = deps, + stackState = state + } + +-- Call a function with a new stack entry on the stack +withNewStackFrame ctx node isCall f = do + newEntry <- newStackEntry ctx node isCall + newInput <- newSTRef newInternalState + newOutput <- newSTRef newInternalState + newNode <- newSTRef node + let newCtx = ctx { + cInput = newInput, + cOutput = newOutput, + cNode = newNode, + cStack = newEntry : cStack ctx + } + x <- f newCtx + + {- + deps <- readSTRef $ dependencies newEntry + selfcheck <- fulfillsDependencies newCtx deps + unless selfcheck $ error $ pleaseReport $ "Unmet stack dependencies on " ++ show (node, deps) + -} + + return (x, newEntry) + +-- Check if invoking this function would be a recursive loop +-- (i.e. we already have the function on the stack) +wouldBeRecursive ctx node = f (cStack ctx) + where + f [] = return False + f (s:rest) = do + res <- + if entryPoint s == node + then return True + else f rest + modifySTRef (dependencies s) $ S.insert $ DepIsRecursive node res + return res + +-- The main DFA 'transfer' function, applying the effects of a node to the output state +transfer ctx label = + --traceShow ("Transferring", label) $ + case label of + CFStructuralNode -> return () + CFEntryPoint _ -> return () + CFImpliedExit -> return () + CFResolvedExit {} -> return () + + CFExecuteCommand cmd -> transferCommand ctx cmd + CFExecuteSubshell reason entry exit -> transferSubshell ctx reason entry exit + CFApplyEffects effects -> mapM_ (\(IdTagged _ f) -> transferEffect ctx f) effects + CFSetExitCode id -> transferExitCode ctx id + + CFUnresolvedExit -> patchOutputM ctx unreachableState + CFUnreachable -> patchOutputM ctx unreachableState + + -- TODO + CFSetBackgroundPid _ -> return () + CFDropPrefixAssignments {} -> + modifySTRef (cOutput ctx) $ \c -> modified c { sPrefixValues = vmEmpty } +-- _ -> error $ "Unknown " ++ show label + + +-- Transfer the effects of a subshell invocation. This is similar to a function call +-- to allow easily discarding the effects (otherwise the InternalState would have +-- to represent subshell depth, while this way it can simply use the function stack). +transferSubshell ctx reason entry exit = do + let cout = cOutput ctx + initial <- readSTRef cout + runCached ctx entry (f entry exit) + res <- readSTRef cout + -- Clear subshell changes. TODO: track this to warn about modifications. + writeSTRef cout $ initial { + sExitCodes = sExitCodes res + } + where + f entry exit ctx = do + (states, frame) <- withNewStackFrame ctx entry False (flip dataflow $ entry) + let (_, res) = fromMaybe (error $ pleaseReport "Subshell has no exit") $ M.lookup exit states + deps <- readSTRef $ dependencies frame + registerFlowResult ctx entry states deps + return (deps, res) + +-- Transfer the effects of executing a command, i.e. the merged union of all possible function definitions. +transferCommand ctx Nothing = return () +transferCommand ctx (Just name) = do + targets <- readFunction ctx name + logVerbose ("Transferring ",name,targets) + transferMultiple ctx $ map (flip transferFunctionValue) $ S.toList targets + +-- Transfer a set of function definitions and merge the output states. +transferMultiple ctx funcs = do + logVerbose ("Transferring set of ", length funcs) + original <- readSTRef out + branches <- mapM (apply ctx original) funcs + merged <- mergeStates ctx original branches + let patched = patchState original merged + writeSTRef out patched + where + out = cOutput ctx + apply ctx original f = do + writeSTRef out original + f ctx + readSTRef out + +-- Transfer the effects of a single function definition. +transferFunctionValue ctx funcVal = + case funcVal of + FunctionUnknown -> return () + FunctionDefinition name entry exit -> do + isRecursive <- wouldBeRecursive ctx entry + if isRecursive + then return () -- TODO: Find a better strategy for recursion + else runCached ctx entry (f name entry exit) + where + f name entry exit ctx = do + (states, frame) <- withNewStackFrame ctx entry True (flip dataflow $ entry) + deps <- readSTRef $ dependencies frame + let res = + case M.lookup exit states of + Just (input, output) -> do + -- Discard local variables. TODO: track&retain variables declared local in previous scopes? + modified output { sLocalValues = vmEmpty } + Nothing -> do + -- e.g. f() { exit; } + unreachableState + registerFlowResult ctx entry states deps + return (deps, res) + +transferExitCode ctx id = do + modifySTRef (cOutput ctx) $ setExitCode id + +-- Register/save the result of a dataflow of a function. +-- At the end, all the different values from different flows are merged together. +registerFlowResult ctx entry states deps = do + -- This function is called in the context of a CFExecuteCommand and not its invoked function, + -- so manually add the current node to the stack. + current <- readSTRef $ cNode ctx + let parents = map callSite $ cStack ctx + -- A unique path to this flow context. The specific value doesn't matter, as long as it's + -- unique per invocation of the function. This is required so that 'x=1; f; x=2; f' won't + -- overwrite each other. + let path = entry : current : parents + modifySTRef (cInvocations ctx) $ M.insert path (deps, states) + + +-- Look up a node in the cache and see if the dependencies of any entries are matched. +-- In that case, reuse the previous result instead of doing a new data flow. +runCached :: forall s. Ctx s -> Node -> (Ctx s -> ST s (S.Set StateDependency, InternalState)) -> ST s () +runCached ctx node f = do + cache <- getCache ctx node + case cache of + Just v -> do + logInfo ("Running cached", node) + -- do { (deps, diff) <- f ctx; unless (v == diff) $ traceShowM ("Cache FAILED to match actual result", node, deps, diff); } + patchOutputM ctx v + + Nothing -> do + logInfo ("Cache failed", node) + (deps, diff) <- f ctx + modifySTRef (cCache ctx) (M.insertWith (\_ old -> (deps, diff):(take cacheEntries old)) node [(deps,diff)]) + logVerbose ("Recomputed cache for", node, deps) + -- do { f <- fulfillsDependencies ctx node deps; unless (f) $ traceShowM ("New dependencies FAILED to match", node, deps); } + patchOutputM ctx diff + +-- Get a cached version whose dependencies are currently fulfilled, if any. +getCache :: forall s. Ctx s -> Node -> ST s (Maybe InternalState) +getCache ctx node = do + cache <- readSTRef $ cCache ctx + enable <- readSTRef $ cEnableCache ctx + logVerbose ("Cache for", node, "length", length $ M.findWithDefault [] node cache, M.lookup node cache) + if enable + then f $ M.findWithDefault [] node cache + else return Nothing + where + f [] = return Nothing + f ((deps, value):rest) = do + match <- fulfillsDependencies ctx node deps + if match + then return $ Just value + else f rest + +-- Transfer a single CFEffect to the output state. +transferEffect ctx effect = + case effect of + CFReadVariable name -> + case name of + "?" -> void $ readExitCodes ctx + _ -> void $ readVariable ctx name + CFWriteVariable name value -> do + val <- cfValueToVariableValue ctx value + updateVariableValue ctx name val + CFWriteGlobal name value -> do + val <- cfValueToVariableValue ctx value + updateGlobalValue ctx name val + CFWriteLocal name value -> do + val <- cfValueToVariableValue ctx value + updateLocalValue ctx name val + CFWritePrefix name value -> do + val <- cfValueToVariableValue ctx value + updatePrefixValue ctx name val + + CFSetProps scope name props -> + case scope of + DefaultScope -> do + state <- readVariable ctx name + writeVariable ctx name $ addProperties props state + GlobalScope -> do + state <- readGlobal ctx name + writeGlobal ctx name $ addProperties props state + LocalScope -> do + out <- readSTRef (cOutput ctx) + state <- readLocal ctx name + writeLocal ctx name $ addProperties props state + PrefixScope -> do + -- Prefix values become local + state <- readLocal ctx name + writeLocal ctx name $ addProperties props state + + CFUnsetProps scope name props -> + case scope of + DefaultScope -> do + state <- readVariable ctx name + writeVariable ctx name $ removeProperties props state + GlobalScope -> do + state <- readGlobal ctx name + writeGlobal ctx name $ removeProperties props state + LocalScope -> do + out <- readSTRef (cOutput ctx) + state <- readLocal ctx name + writeLocal ctx name $ removeProperties props state + PrefixScope -> do + -- Prefix values become local + state <- readLocal ctx name + writeLocal ctx name $ removeProperties props state + + + CFUndefineVariable name -> undefineVariable ctx name + CFUndefineFunction name -> undefineFunction ctx name + CFUndefine name -> do + -- This should really just unset one or the other + undefineVariable ctx name + undefineFunction ctx name + CFDefineFunction name id entry exit -> + writeFunction ctx name $ FunctionDefinition name entry exit + + -- TODO + CFUndefineNameref name -> undefineVariable ctx name + CFHintArray name -> return () + CFHintDefined name -> return () +-- _ -> error $ "Unknown effect " ++ show effect + + +-- Transfer the CFG's idea of a value into our VariableState +cfValueToVariableValue ctx val = + case val of + CFValueArray -> return unknownVariableValue -- TODO: Track array status + CFValueComputed _ parts -> foldM f emptyVariableValue parts + CFValueInteger -> return unknownIntegerValue + CFValueString -> return unknownVariableValue + CFValueUninitialized -> return emptyVariableValue +-- _ -> error $ "Unknown value: " ++ show val + where + f val part = do + next <- computeValue ctx part + return $ val `appendVariableValue` next + +-- A value can be computed from 0 or more parts, such as x="literal$y$z" +computeValue ctx part = + case part of + CFStringLiteral str -> return $ literalToVariableValue str + CFStringInteger -> return unknownIntegerValue + CFStringUnknown -> return unknownVariableValue + CFStringVariable name -> variableStateToValue <$> readVariable ctx name + where + variableStateToValue state = + case () of + _ | all (CFVPInteger `S.member`) $ variableProperties state -> unknownIntegerValue + _ -> variableValue state + +-- Append two VariableValues as if with z="$x$y" +appendVariableValue :: VariableValue -> VariableValue -> VariableValue +appendVariableValue a b = + unknownVariableValue { + literalValue = liftM2 (++) (literalValue a) (literalValue b), + spaceStatus = appendSpaceStatus (spaceStatus a) (spaceStatus b), + numericalStatus = appendNumericalStatus (numericalStatus a) (numericalStatus b) + } + +appendSpaceStatus a b = + case (a,b) of + (SpaceStatusEmpty, _) -> b + (_, SpaceStatusEmpty) -> a + (SpaceStatusClean, SpaceStatusClean) -> a + _ ->SpaceStatusDirty + +appendNumericalStatus a b = + case (a,b) of + (NumericalStatusEmpty, x) -> x + (x, NumericalStatusEmpty) -> x + (NumericalStatusDefinitely, NumericalStatusDefinitely) -> NumericalStatusDefinitely + (NumericalStatusUnknown, _) -> NumericalStatusUnknown + (_, NumericalStatusUnknown) -> NumericalStatusUnknown + _ -> NumericalStatusMaybe + +unknownIntegerValue = unknownVariableValue { + literalValue = Nothing, + spaceStatus = SpaceStatusClean, + numericalStatus = NumericalStatusDefinitely +} + +literalToVariableValue str = unknownVariableValue { + literalValue = Just str, + spaceStatus = literalToSpaceStatus str, + numericalStatus = literalToNumericalStatus str +} + +withoutChanges ctx f = do + let inp = cInput ctx + let out = cOutput ctx + prevInput <- readSTRef inp + prevOutput <- readSTRef out + res <- f + writeSTRef inp prevInput + writeSTRef out prevOutput + return res + +-- Get the SpaceStatus for a literal string, i.e. if it needs quoting +literalToSpaceStatus str = + case str of + "" -> SpaceStatusEmpty + _ | all (`notElem` " \t\n*?[") str -> SpaceStatusClean + _ -> SpaceStatusDirty + +-- Get the NumericalStatus for a literal string, i.e. whether it's an integer +literalToNumericalStatus str = + case str of + "" -> NumericalStatusEmpty + '-':rest -> if isNumeric rest then NumericalStatusDefinitely else NumericalStatusUnknown + rest -> if isNumeric rest then NumericalStatusDefinitely else NumericalStatusUnknown + where + isNumeric = all isDigit + +type StateMap = M.Map Node (InternalState, InternalState) + +-- Classic, iterative Data Flow Analysis. See Wikipedia for a description of the process. +dataflow :: forall s. Ctx s -> Node -> ST s StateMap +dataflow ctx entry = do + pending <- newSTRef $ S.singleton entry + states <- newSTRef $ M.empty + -- Should probably be done via a stack frame instead + withoutChanges ctx $ + f iterationCount pending states + readSTRef states + where + graph = cGraph ctx + f 0 _ _ = error $ pleaseReport "DFA did not reach fix point" + f n pending states = do + ps <- readSTRef pending + + when (n == fallbackThreshold) $ do + -- This should never happen, but has historically been due to caching bugs. + -- Try disabling the cache and continuing. + logInfo "DFA is not stabilizing! Disabling cache." + writeSTRef (cEnableCache ctx) False + + if S.null ps + then return () + else do + let (next, rest) = S.deleteFindMin ps + nexts <- process states next + writeSTRef pending $ foldl (flip S.insert) rest nexts + f (n-1) pending states + + process states node = do + stateMap <- readSTRef states + let inputs = filter (\c -> sIsReachable c /= Just False) $ mapMaybe (\c -> fmap snd $ M.lookup c stateMap) incoming + input <- + case incoming of + [] -> return newInternalState + _ -> + case inputs of + [] -> return unreachableState + (x:rest) -> foldM (mergeState ctx) x rest + writeSTRef (cInput ctx) $ input + writeSTRef (cOutput ctx) $ input + writeSTRef (cNode ctx) $ node + transfer ctx label + newOutput <- readSTRef $ cOutput ctx + result <- + if is2plus outgoing + then + -- Version the state because we split and will probably merge later + versionState ctx newOutput + else return newOutput + writeSTRef states $ M.insert node (input, result) stateMap + case M.lookup node stateMap of + Nothing -> return outgoing + Just (oldInput, oldOutput) -> + if oldOutput == result + then return [] + else return outgoing + where + (incomingL, _, label, outgoingL) = context graph $ node + incoming = map snd $ filter isRegular $ incomingL + outgoing = map snd outgoingL + isRegular = ((== CFEFlow) . fst) + +runRoot ctx env entry exit = do + writeSTRef (cInput ctx) $ env + writeSTRef (cOutput ctx) $ env + writeSTRef (cNode ctx) $ entry + (states, frame) <- withNewStackFrame ctx entry False $ \c -> dataflow c entry + deps <- readSTRef $ dependencies frame + registerFlowResult ctx entry states deps + -- Return the final state, used to invoke functions that were declared but not invoked + return $ snd $ fromMaybe (error $ pleaseReport "Missing exit state") $ M.lookup exit states + + +analyzeControlFlow :: CFGParameters -> Token -> CFGAnalysis +analyzeControlFlow params t = + let + cfg = buildGraph params t + (entry, exit) = M.findWithDefault (error $ pleaseReport "Missing root") (getId t) (cfIdToRange cfg) + in + runST $ f cfg entry exit + where + f cfg entry exit = do + let env = createEnvironmentState + ctx <- newCtx $ cfGraph cfg + -- Do a dataflow analysis starting on the root node + exitState <- runRoot ctx env entry exit + + -- 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 + + -- Invoke all functions that were declared but not invoked + -- This is so that we still get warnings for dead code + -- (it's probably not actually dead, just used by a script that sources ours) + let declaredFunctions = getFunctionTargets exitState + let uninvoked = M.difference declaredFunctions invokedNodes + + let stragglerInput = + (env `patchState` exitState) { + -- We don't want `die() { exit $?; }; echo "Sourced"` to assume $? is always echo + sExitCodes = Nothing + } + + analyzeStragglers ctx stragglerInput uninvoked + + -- Now round up all the states from all data flows + -- (FIXME: this excludes functions that were defined in straggling functions) + invocations <- readSTRef $ cInvocations ctx + invokedStates <- flattenByNode ctx $ groupByNode $ M.map addDeps invocations + + -- 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 + + -- Convert to external states + let nodeToData = M.map (\(a,b) -> (internalToExternal a, internalToExternal b)) allStates + + return $ nodeToData `deepseq` CFGAnalysis { + graph = cfGraph cfg, + tokenToRange = cfIdToRange cfg, + tokenToNodes = cfIdToNodes cfg, + nodeToData = nodeToData, + postDominators = cfPostDominators cfg + } + + + -- Include the dependencies in the state of each function, e.g. if it depends on `x=foo` then add that. + addDeps :: (S.Set StateDependency, M.Map Node (InternalState, InternalState)) -> M.Map Node (InternalState, InternalState) + addDeps (deps, m) = let base = depsToState deps in M.map (\(a,b) -> (base `patchState` a, base `patchState` b)) m + + -- Collect all the states that each node has resulted in. + groupByNode :: forall k v. M.Map k (M.Map Node v) -> M.Map Node [v] + groupByNode pathMap = M.fromListWith (++) $ map (\(k,v) -> (k,[v])) $ concatMap M.toList $ M.elems pathMap + + -- Merge all the pre/post states for each node. This would have been a foldM if Map had one. + flattenByNode ctx m = M.fromDistinctAscList <$> (mapM (mergePair ctx) $ M.toList m) + + mergeAllStates ctx pairs = + let + (pres, posts) = unzip pairs + in do + pre <- mergeStates ctx (error $ pleaseReport "Null node states") pres + post <- mergeStates ctx (error $ pleaseReport "Null node states") posts + return (pre, post) + + mergePair ctx (node, list) = do + merged <- mergeAllStates ctx list + return (node, merged) + + -- Get the all the functions defined in an InternalState + getFunctionTargets :: InternalState -> M.Map Node FunctionDefinition + getFunctionTargets state = + let + declaredFuncs = S.unions $ M.elems $ mapStorage $ sFunctionTargets state + getFunc d = + case d of + FunctionDefinition _ entry _ -> Just (entry, d) + _ -> Nothing + funcs = mapMaybe getFunc $ S.toList declaredFuncs + in + M.fromList funcs + + +analyzeStragglers ctx state stragglers = do + mapM_ f $ M.elems stragglers + where + f def@(FunctionDefinition name entry exit) = do + writeSTRef (cInput ctx) state + writeSTRef (cOutput ctx) state + writeSTRef (cNode ctx) entry + transferFunctionValue ctx def + + + +return [] +runTests = $quickCheckAll diff --git a/src/ShellCheck/Checker.hs b/src/ShellCheck/Checker.hs --- a/src/ShellCheck/Checker.hs +++ b/src/ShellCheck/Checker.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2020 Vidar Holen + Copyright 2012-2022 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -20,9 +20,10 @@ {-# LANGUAGE TemplateHaskell #-} module ShellCheck.Checker (checkScript, ShellCheck.Checker.runTests) where +import ShellCheck.Analyzer +import ShellCheck.ASTLib import ShellCheck.Interface import ShellCheck.Parser -import ShellCheck.Analyzer import Data.Either import Data.Functor @@ -85,7 +86,7 @@ asCheckSourced = csCheckSourced spec, asExecutionMode = Executed, asTokenPositions = tokenPositions, - asOptionalChecks = csOptionalChecks spec + asOptionalChecks = getEnableDirectives root ++ csOptionalChecks spec } where as = newAnalysisSpec root let analysisMessages = maybe [] @@ -243,6 +244,9 @@ prop_canSourceDynamicWhenRedirected = null $ checkWithIncludes [("lib", "")] "#shellcheck source=lib\n. \"$1\"" +prop_canRedirectWithSpaces = + null $ checkWithIncludes [("my file", "")] "#shellcheck source=\"my file\"\n. \"$1\"" + prop_recursiveAnalysis = [2086] == checkRecursive [("lib", "echo $1")] "source lib" @@ -412,6 +416,15 @@ csCheckSourced = True } +prop_sourcePathWorksWithSpaces = result == [2086] + where + f "dir/myscript" _ ["my path"] "lib" = return "foo/lib" + result = checkWithIncludesAndSourcePath [("foo/lib", "echo $1")] f emptyCheckSpec { + csScript = "#!/bin/bash\n# shellcheck source-path='my path'\nsource lib", + csFilename = "dir/myscript", + csCheckSourced = True + } + prop_sourcePathRedirectsDirective = result == [2086] where f "dir/myscript" _ _ "lib" = return "foo/lib" @@ -483,6 +496,17 @@ csCheckSourced = True } +prop_rcCanSuppressEarlyProblems1 = null result + where + result = checkWithRc "disable=1071" emptyCheckSpec { + csScript = "#!/bin/zsh\necho $1" + } + +prop_rcCanSuppressEarlyProblems2 = null result + where + result = checkWithRc "disable=1104" emptyCheckSpec { + csScript = "!/bin/bash\necho 'hello world'" + } 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 @@ -1,5 +1,5 @@ {- - Copyright 2012-2021 Vidar Holen + Copyright 2012-2022 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -27,21 +27,28 @@ import ShellCheck.AST import ShellCheck.ASTLib import ShellCheck.AnalyzerLib +import ShellCheck.CFG +import qualified ShellCheck.CFGAnalysis as CF import ShellCheck.Data import ShellCheck.Interface import ShellCheck.Parser +import ShellCheck.Prelude import ShellCheck.Regex import Control.Monad import Control.Monad.RWS import Data.Char import Data.Functor.Identity +import qualified Data.Graph.Inductive.Graph as G import Data.List import Data.Maybe -import qualified Data.Map.Strict as Map +import qualified Data.Map.Strict as M +import qualified Data.Set as S import Test.QuickCheck.All (forAllProperties) import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) +import Debug.Trace -- STRIP + data CommandName = Exactly String | Basename String deriving (Eq, Ord) @@ -98,8 +105,10 @@ ,checkUnquotedEchoSpaces ,checkEvalArray ] - ++ map checkArgComparison declaringCommands + ++ map checkArgComparison ("alias" : declaringCommands) ++ map checkMaskedReturns declaringCommands + ++ map checkMultipleDeclaring declaringCommands + ++ map checkBackreferencingDeclaration declaringCommands optionalChecks = map fst optionalCommandChecks @@ -112,7 +121,7 @@ cdNegative = "command -v javac" }, checkWhich) ] -optionalCheckMap = Map.fromList $ map (\(desc, check) -> (cdName desc, check)) optionalCommandChecks +optionalCheckMap = M.fromList $ map (\(desc, check) -> (cdName desc, check)) optionalCommandChecks prop_verifyOptionalExamples = all check optionalCommandChecks where @@ -161,27 +170,27 @@ prop_checkGenericOptsT2 = checkGetOpts "-xy --" ["x", "y"] [] $ return . getGenericOpts -buildCommandMap :: [CommandCheck] -> Map.Map CommandName (Token -> Analysis) -buildCommandMap = foldl' addCheck Map.empty +buildCommandMap :: [CommandCheck] -> M.Map CommandName (Token -> Analysis) +buildCommandMap = foldl' addCheck M.empty where addCheck map (CommandCheck name function) = - Map.insertWith composeAnalyzers name function map + M.insertWith composeAnalyzers name function map -checkCommand :: Map.Map CommandName (Token -> Analysis) -> Token -> Analysis +checkCommand :: M.Map CommandName (Token -> Analysis) -> Token -> Analysis checkCommand map t@(T_SimpleCommand id cmdPrefix (cmd:rest)) = sequence_ $ do name <- getLiteralString cmd return $ if '/' `elem` name then - Map.findWithDefault nullCheck (Basename $ basename name) map t + M.findWithDefault nullCheck (Basename $ basename name) map t else if name == "builtin" && not (null rest) then let t' = T_SimpleCommand id cmdPrefix rest selectedBuiltin = fromMaybe "" $ getLiteralString . head $ rest - in Map.findWithDefault nullCheck (Exactly selectedBuiltin) map t' + in M.findWithDefault nullCheck (Exactly selectedBuiltin) map t' else do - Map.findWithDefault nullCheck (Exactly name) map t - Map.findWithDefault nullCheck (Basename name) map t + M.findWithDefault nullCheck (Exactly name) map t + M.findWithDefault nullCheck (Basename name) map t where basename = reverse . takeWhile (/= '/') . reverse @@ -203,22 +212,22 @@ optionals = if "all" `elem` keys then map snd optionalCommandChecks - else mapMaybe (\x -> Map.lookup x optionalCheckMap) keys + else mapMaybe (\x -> M.lookup x optionalCheckMap) keys prop_checkTr1 = verify checkTr "tr [a-f] [A-F]" prop_checkTr2 = verify checkTr "tr 'a-z' 'A-Z'" -prop_checkTr2a= verify checkTr "tr '[a-z]' '[A-Z]'" +prop_checkTr2a = verify checkTr "tr '[a-z]' '[A-Z]'" prop_checkTr3 = verifyNot checkTr "tr -d '[:lower:]'" -prop_checkTr3a= verifyNot checkTr "tr -d '[:upper:]'" -prop_checkTr3b= verifyNot checkTr "tr -d '|/_[:upper:]'" +prop_checkTr3a = verifyNot checkTr "tr -d '[:upper:]'" +prop_checkTr3b = verifyNot checkTr "tr -d '|/_[:upper:]'" prop_checkTr4 = verifyNot checkTr "ls [a-z]" prop_checkTr5 = verify checkTr "tr foo bar" prop_checkTr6 = verify checkTr "tr 'hello' 'world'" prop_checkTr8 = verifyNot checkTr "tr aeiou _____" prop_checkTr9 = verifyNot checkTr "a-z n-za-m" -prop_checkTr10= verifyNot checkTr "tr --squeeze-repeats rl lr" -prop_checkTr11= verifyNot checkTr "tr abc '[d*]'" -prop_checkTr12= verifyNot checkTr "tr '[=e=]' 'e'" +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? @@ -330,20 +339,20 @@ prop_checkGrepRe7 = verify checkGrepRe "grep *foo* file" prop_checkGrepRe8 = verify checkGrepRe "ls | grep foo*.jpg" prop_checkGrepRe9 = verifyNot checkGrepRe "grep '[0-9]*' file" -prop_checkGrepRe10= verifyNot checkGrepRe "grep '^aa*' file" -prop_checkGrepRe11= verifyNot checkGrepRe "grep --include=*.png foo" -prop_checkGrepRe12= verifyNot checkGrepRe "grep -F 'Foo*' file" -prop_checkGrepRe13= verifyNot checkGrepRe "grep -- -foo bar*" -prop_checkGrepRe14= verifyNot checkGrepRe "grep -e -foo bar*" -prop_checkGrepRe15= verifyNot checkGrepRe "grep --regex -foo bar*" -prop_checkGrepRe16= verifyNot checkGrepRe "grep --include 'Foo*' file" -prop_checkGrepRe17= verifyNot checkGrepRe "grep --exclude 'Foo*' file" -prop_checkGrepRe18= verifyNot checkGrepRe "grep --exclude-dir 'Foo*' file" -prop_checkGrepRe19= verify checkGrepRe "grep -- 'Foo*' file" -prop_checkGrepRe20= verifyNot checkGrepRe "grep --fixed-strings 'Foo*' file" -prop_checkGrepRe21= verifyNot checkGrepRe "grep -o 'x*' file" -prop_checkGrepRe22= verifyNot checkGrepRe "grep --only-matching 'x*' file" -prop_checkGrepRe23= verifyNot checkGrepRe "grep '.*' file" +prop_checkGrepRe10 = verifyNot checkGrepRe "grep '^aa*' file" +prop_checkGrepRe11 = verifyNot checkGrepRe "grep --include=*.png foo" +prop_checkGrepRe12 = verifyNot checkGrepRe "grep -F 'Foo*' file" +prop_checkGrepRe13 = verifyNot checkGrepRe "grep -- -foo bar*" +prop_checkGrepRe14 = verifyNot checkGrepRe "grep -e -foo bar*" +prop_checkGrepRe15 = verifyNot checkGrepRe "grep --regex -foo bar*" +prop_checkGrepRe16 = verifyNot checkGrepRe "grep --include 'Foo*' file" +prop_checkGrepRe17 = verifyNot checkGrepRe "grep --exclude 'Foo*' file" +prop_checkGrepRe18 = verifyNot checkGrepRe "grep --exclude-dir 'Foo*' file" +prop_checkGrepRe19 = verify checkGrepRe "grep -- 'Foo*' file" +prop_checkGrepRe20 = verifyNot checkGrepRe "grep --fixed-strings 'Foo*' file" +prop_checkGrepRe21 = verifyNot checkGrepRe "grep -o 'x*' file" +prop_checkGrepRe22 = verifyNot checkGrepRe "grep --only-matching 'x*' file" +prop_checkGrepRe23 = verifyNot checkGrepRe "grep '.*' file" checkGrepRe = CommandCheck (Basename "grep") check where check cmd = f cmd (arguments cmd) @@ -391,7 +400,7 @@ prop_checkTrapQuotes1 = verify checkTrapQuotes "trap \"echo $num\" INT" -prop_checkTrapQuotes1a= verify checkTrapQuotes "trap \"echo `ls`\" INT" +prop_checkTrapQuotes1a = verify checkTrapQuotes "trap \"echo `ls`\" INT" prop_checkTrapQuotes2 = verifyNot checkTrapQuotes "trap 'echo $num' INT" prop_checkTrapQuotes3 = verify checkTrapQuotes "trap \"echo $((1+num))\" EXIT DEBUG" checkTrapQuotes = CommandCheck (Exactly "trap") (f . arguments) where @@ -472,9 +481,16 @@ prop_checkUnusedEchoEscapes3 = verify checkUnusedEchoEscapes "echo \"n:\\t42\"" prop_checkUnusedEchoEscapes4 = verifyNot checkUnusedEchoEscapes "echo lol" prop_checkUnusedEchoEscapes5 = verifyNot checkUnusedEchoEscapes "echo -n -e '\n'" +prop_checkUnusedEchoEscapes6 = verify checkUnusedEchoEscapes "echo '\\506'" +prop_checkUnusedEchoEscapes7 = verify checkUnusedEchoEscapes "echo '\\5a'" +prop_checkUnusedEchoEscapes8 = verifyNot checkUnusedEchoEscapes "echo '\\8a'" +prop_checkUnusedEchoEscapes9 = verifyNot checkUnusedEchoEscapes "echo '\\d5a'" +prop_checkUnusedEchoEscapes10 = verify checkUnusedEchoEscapes "echo '\\x4a'" +prop_checkUnusedEchoEscapes11 = verify checkUnusedEchoEscapes "echo '\\xat'" +prop_checkUnusedEchoEscapes12 = verifyNot checkUnusedEchoEscapes "echo '\\xth'" checkUnusedEchoEscapes = CommandCheck (Basename "echo") f where - hasEscapes = mkRegex "\\\\[rnt]" + hasEscapes = mkRegex "\\\\([rntabefv\\']|[0-7]{1,3}|x([0-9]|[A-F]|[a-f]){1,2})" f cmd = whenShell [Sh, Bash, Ksh] $ unless (cmd `hasFlag` "e") $ @@ -648,19 +664,19 @@ prop_checkPrintfVar7 = verify checkPrintfVar "printf -- foo bar baz" prop_checkPrintfVar8 = verifyNot checkPrintfVar "printf '%s %s %s' \"${var[@]}\"" prop_checkPrintfVar9 = verifyNot checkPrintfVar "printf '%s %s %s\\n' *.png" -prop_checkPrintfVar10= verifyNot checkPrintfVar "printf '%s %s %s' foo bar baz" -prop_checkPrintfVar11= verifyNot checkPrintfVar "printf '%(%s%s)T' -1" -prop_checkPrintfVar12= verify checkPrintfVar "printf '%s %s\\n' 1 2 3" -prop_checkPrintfVar13= verifyNot checkPrintfVar "printf '%s %s\\n' 1 2 3 4" -prop_checkPrintfVar14= verify checkPrintfVar "printf '%*s\\n' 1" -prop_checkPrintfVar15= verifyNot checkPrintfVar "printf '%*s\\n' 1 2" -prop_checkPrintfVar16= verifyNot checkPrintfVar "printf $'string'" -prop_checkPrintfVar17= verify checkPrintfVar "printf '%-*s\\n' 1" -prop_checkPrintfVar18= verifyNot checkPrintfVar "printf '%-*s\\n' 1 2" -prop_checkPrintfVar19= verifyNot checkPrintfVar "printf '%(%s)T'" -prop_checkPrintfVar20= verifyNot checkPrintfVar "printf '%d %(%s)T' 42" -prop_checkPrintfVar21= verify checkPrintfVar "printf '%d %(%s)T'" -prop_checkPrintfVar22= verify checkPrintfVar "printf '%s\n%s' foo" +prop_checkPrintfVar10 = verifyNot checkPrintfVar "printf '%s %s %s' foo bar baz" +prop_checkPrintfVar11 = verifyNot checkPrintfVar "printf '%(%s%s)T' -1" +prop_checkPrintfVar12 = verify checkPrintfVar "printf '%s %s\\n' 1 2 3" +prop_checkPrintfVar13 = verifyNot checkPrintfVar "printf '%s %s\\n' 1 2 3 4" +prop_checkPrintfVar14 = verify checkPrintfVar "printf '%*s\\n' 1" +prop_checkPrintfVar15 = verifyNot checkPrintfVar "printf '%*s\\n' 1 2" +prop_checkPrintfVar16 = verifyNot checkPrintfVar "printf $'string'" +prop_checkPrintfVar17 = verify checkPrintfVar "printf '%-*s\\n' 1" +prop_checkPrintfVar18 = verifyNot checkPrintfVar "printf '%-*s\\n' 1 2" +prop_checkPrintfVar19 = verifyNot checkPrintfVar "printf '%(%s)T'" +prop_checkPrintfVar20 = verifyNot checkPrintfVar "printf '%d %(%s)T' 42" +prop_checkPrintfVar21 = verify checkPrintfVar "printf '%d %(%s)T'" +prop_checkPrintfVar22 = verify checkPrintfVar "printf '%s\n%s' foo" checkPrintfVar = CommandCheck (Exactly "printf") (f . arguments) where f (doubledash:rest) | getLiteralString doubledash == Just "--" = f rest @@ -674,6 +690,7 @@ let formats = getPrintfFormats string let formatCount = length formats let argCount = length more + let pluraliseIfMany word n = if n > 1 then word ++ "s" else word return $ if | argCount == 0 && formatCount == 0 -> @@ -689,7 +706,8 @@ return () -- Great: a suitable number of arguments | otherwise -> warn (getId format) 2183 $ - "This format string has " ++ show formatCount ++ " variables, but is passed " ++ show argCount ++ " arguments." + "This format string has " ++ show formatCount ++ " " ++ pluraliseIfMany "variable" formatCount ++ + ", but is passed " ++ show argCount ++ pluraliseIfMany " argument" argCount ++ "." unless ('%' `elem` concat (oversimplify format) || isLiteral format) $ info (getId format) 2059 @@ -735,7 +753,7 @@ -- \____ _____/\___ ____/ \____ ____/\_________ _________/ \______ / -- V V V V V -- flags field width precision format character rest - -- field width and precision can be specified with a '*' instead of a digit, + -- 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 @@ -941,6 +959,22 @@ unless (any isFunctionLike path) $ err (getId $ getCommandTokenOrThis t) 2168 "'local' is only valid in functions." +prop_checkMultipleDeclaring1 = verify (checkMultipleDeclaring "local") "q() { local readonly var=1; }" +prop_checkMultipleDeclaring2 = verifyNot (checkMultipleDeclaring "local") "q() { local var=1; }" +prop_checkMultipleDeclaring3 = verify (checkMultipleDeclaring "readonly") "readonly local foo=5" +prop_checkMultipleDeclaring4 = verify (checkMultipleDeclaring "export") "export readonly foo=5" +prop_checkMultipleDeclaring5 = verifyNot (checkMultipleDeclaring "local") "f() { local -r foo=5; }" +prop_checkMultipleDeclaring6 = verifyNot (checkMultipleDeclaring "declare") "declare -rx foo=5" +prop_checkMultipleDeclaring7 = verifyNot (checkMultipleDeclaring "readonly") "readonly 'local' foo=5" +checkMultipleDeclaring cmd = CommandCheck (Exactly cmd) (mapM_ check . arguments) + where + check t = sequence_ $ do + lit <- getUnquotedLiteral t + guard $ lit `elem` declaringCommands + return $ err (getId $ getCommandTokenOrThis t) 2316 $ + "This applies " ++ cmd ++ " to the variable named " ++ lit ++ + ", which is probably not what you want. Use a separate command or the appropriate `declare` options instead." + prop_checkDeprecatedTempfile1 = verify checkDeprecatedTempfile "var=$(tempfile)" prop_checkDeprecatedTempfile2 = verifyNot checkDeprecatedTempfile "tempfile=$(mktemp)" checkDeprecatedTempfile = CommandCheck (Basename "tempfile") $ @@ -987,20 +1021,20 @@ check :: Id -> [String] -> Token -> Analysis check optId opts (T_CaseExpression id _ list) = do - unless (Nothing `Map.member` handledMap) $ do - mapM_ (warnUnhandled optId id) $ catMaybes $ Map.keys notHandled + unless (Nothing `M.member` handledMap) $ do + mapM_ (warnUnhandled optId id) $ catMaybes $ M.keys notHandled - unless (any (`Map.member` handledMap) [Just "*",Just "?"]) $ + unless (any (`M.member` handledMap) [Just "*",Just "?"]) $ warn id 2220 "Invalid flags are not handled. Add a *) case." - mapM_ warnRedundant $ Map.toList notRequested + mapM_ warnRedundant $ M.toList notRequested where - handledMap = Map.fromList (concatMap getHandledStrings list) - requestedMap = Map.fromList $ map (\x -> (Just x, ())) opts + handledMap = M.fromList (concatMap getHandledStrings list) + requestedMap = M.fromList $ map (\x -> (Just x, ())) opts - notHandled = Map.difference requestedMap handledMap - notRequested = Map.difference handledMap requestedMap + notHandled = M.difference requestedMap handledMap + notRequested = M.difference handledMap requestedMap warnUnhandled optId caseId str = warn caseId 2213 $ "getopts specified -" ++ (e4m str) ++ ", but it's not handled by this 'case'." @@ -1044,10 +1078,10 @@ prop_checkCatastrophicRm5 = verifyNot checkCatastrophicRm "rm -r /home/${USER:-thing}/*" prop_checkCatastrophicRm6 = verify checkCatastrophicRm "rm --recursive /etc/*$config*" prop_checkCatastrophicRm8 = verify checkCatastrophicRm "rm -rf /home" -prop_checkCatastrophicRm10= verifyNot checkCatastrophicRm "rm -r \"${DIR}\"/{.gitignore,.gitattributes,ci}" -prop_checkCatastrophicRm11= verify checkCatastrophicRm "rm -r /{bin,sbin}/$exec" -prop_checkCatastrophicRm12= verify checkCatastrophicRm "rm -r /{{usr,},{bin,sbin}}/$exec" -prop_checkCatastrophicRm13= verifyNot checkCatastrophicRm "rm -r /{{a,b},{c,d}}/$exec" +prop_checkCatastrophicRm10 = verifyNot checkCatastrophicRm "rm -r \"${DIR}\"/{.gitignore,.gitattributes,ci}" +prop_checkCatastrophicRm11 = verify checkCatastrophicRm "rm -r /{bin,sbin}/$exec" +prop_checkCatastrophicRm12 = verify checkCatastrophicRm "rm -r /{{usr,},{bin,sbin}}/$exec" +prop_checkCatastrophicRm13 = verifyNot checkCatastrophicRm "rm -r /{{a,b},{c,d}}/$exec" prop_checkCatastrophicRmA = verify checkCatastrophicRm "rm -rf /usr /lib/nvidia-current/xorg/xorg" prop_checkCatastrophicRmB = verify checkCatastrophicRm "rm -rf \"$STEAMROOT/\"*" checkCatastrophicRm = CommandCheck (Basename "rm") $ \t -> @@ -1253,6 +1287,7 @@ prop_checkArgComparison4 = verify (checkArgComparison "export") "export a +=b" prop_checkArgComparison7 = verifyNot (checkArgComparison "declare") "declare -a +i foo" prop_checkArgComparison8 = verify (checkArgComparison "let") "let x = 0" +prop_checkArgComparison9 = verify (checkArgComparison "alias") "alias x =0" -- This mirrors checkSecondArgIsComparison but for arguments to local/readonly/declare/export checkArgComparison cmd = CommandCheck (Exactly cmd) wordsWithEqual where @@ -1353,10 +1388,10 @@ m <- asks tokenPositions redir <- getClosestCommandM t sequence_ $ do - let positions = mapMaybe (\c -> Map.lookup (getId c) m) args + let positions = mapMaybe (\c -> M.lookup (getId c) m) args let pairs = zip positions (drop 1 positions) (T_Redirecting _ redirTokens _) <- redir - let redirPositions = mapMaybe (\c -> fst <$> Map.lookup (getId c) m) redirTokens + let redirPositions = mapMaybe (\c -> fst <$> M.lookup (getId c) m) redirTokens guard $ any (hasSpacesBetween redirPositions) pairs return $ info (getId t) 2291 "Quote repeated spaces to avoid them collapsing into one." @@ -1384,6 +1419,52 @@ -- Match ${arr[@]@Q} and ${@@Q} and such T_DollarBraced _ _ l -> 'Q' `elem` getBracedModifier (concat $ oversimplify l) _ -> False + + +prop_checkBackreferencingDeclaration1 = verify (checkBackreferencingDeclaration "declare") "declare x=1 y=foo$x" +prop_checkBackreferencingDeclaration2 = verify (checkBackreferencingDeclaration "readonly") "readonly x=1 y=$((1+x))" +prop_checkBackreferencingDeclaration3 = verify (checkBackreferencingDeclaration "local") "local x=1 y=$(echo $x)" +prop_checkBackreferencingDeclaration4 = verify (checkBackreferencingDeclaration "local") "local x=1 y[$x]=z" +prop_checkBackreferencingDeclaration5 = verify (checkBackreferencingDeclaration "declare") "declare x=var $x=1" +prop_checkBackreferencingDeclaration6 = verify (checkBackreferencingDeclaration "declare") "declare x=var $x=1" +prop_checkBackreferencingDeclaration7 = verify (checkBackreferencingDeclaration "declare") "declare x=var $k=$x" +checkBackreferencingDeclaration cmd = CommandCheck (Exactly cmd) check + where + check t = foldM_ perArg M.empty $ arguments t + + perArg leftArgs t = + case t of + T_Assignment id _ name idx t -> do + warnIfBackreferencing leftArgs $ t:idx + return $ M.insert name id leftArgs + t -> do + warnIfBackreferencing leftArgs [t] + return leftArgs + + warnIfBackreferencing backrefs l = do + references <- findReferences l + let reused = M.intersection backrefs references + mapM msg $ M.toList reused + + msg (name, id) = warn id 2318 $ "This assignment is used again in this '" ++ cmd ++ "', but won't have taken effect. Use two '" ++ cmd ++ "'s." + + findReferences list = do + cfga <- asks cfgAnalysis + let graph = CF.graph cfga + let nodesMap = CF.tokenToNodes cfga + let nodes = S.unions $ map (\id -> M.findWithDefault S.empty id nodesMap) $ map getId $ list + let labels = mapMaybe (G.lab graph) $ S.toList nodes + let references = M.fromList $ concatMap refFromLabel labels + return references + + refFromLabel lab = + case lab of + CFApplyEffects effects -> mapMaybe refFromEffect effects + _ -> [] + refFromEffect e = + case e of + IdTagged id (CFReadVariable name) -> return (name, id) + _ -> Nothing return [] diff --git a/src/ShellCheck/Checks/ControlFlow.hs b/src/ShellCheck/Checks/ControlFlow.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/Checks/ControlFlow.hs @@ -0,0 +1,101 @@ +{- + Copyright 2022 Vidar Holen + + This file is part of ShellCheck. + https://www.shellcheck.net + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +{-# LANGUAGE TemplateHaskell #-} + +-- Checks that run on the Control Flow Graph (as opposed to the AST) +-- This is scaffolding for a work in progress. + +module ShellCheck.Checks.ControlFlow (checker, optionalChecks, ShellCheck.Checks.ControlFlow.runTests) where + +import ShellCheck.AST +import ShellCheck.ASTLib +import ShellCheck.CFG hiding (cfgAnalysis) +import ShellCheck.CFGAnalysis +import ShellCheck.AnalyzerLib +import ShellCheck.Data +import ShellCheck.Interface + +import Control.Monad +import Control.Monad.Reader +import Data.Graph.Inductive.Graph +import qualified Data.Map as M +import qualified Data.Set as S +import Data.List +import Data.Maybe + +import Test.QuickCheck.All (forAllProperties) +import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) + + +optionalChecks :: [CheckDescription] +optionalChecks = [] + +-- A check that runs on the entire graph +type ControlFlowCheck = Analysis +-- A check invoked once per node, with its (pre,post) data +type ControlFlowNodeCheck = LNode CFNode -> (ProgramState, ProgramState) -> Analysis +-- A check invoked once per effect, with its node's (pre,post) data +type ControlFlowEffectCheck = IdTagged CFEffect -> Node -> (ProgramState, ProgramState) -> Analysis + + +checker :: AnalysisSpec -> Parameters -> Checker +checker spec params = Checker { + perScript = const $ sequence_ controlFlowChecks, + perToken = const $ return () +} + +controlFlowChecks :: [ControlFlowCheck] +controlFlowChecks = [ + runNodeChecks controlFlowNodeChecks + ] + +controlFlowNodeChecks :: [ControlFlowNodeCheck] +controlFlowNodeChecks = [ + runEffectChecks controlFlowEffectChecks + ] + +controlFlowEffectChecks :: [ControlFlowEffectCheck] +controlFlowEffectChecks = [ + ] + +runNodeChecks :: [ControlFlowNodeCheck] -> ControlFlowCheck +runNodeChecks perNode = do + cfg <- asks cfgAnalysis + runOnAll cfg + where + getData datas n@(node, label) = do + (pre, post) <- M.lookup node datas + return (n, (pre, post)) + + runOn :: (LNode CFNode, (ProgramState, ProgramState)) -> Analysis + runOn (node, prepost) = mapM_ (\c -> c node prepost) perNode + runOnAll cfg = mapM_ runOn $ mapMaybe (getData $ nodeToData cfg) $ labNodes (graph cfg) + +runEffectChecks :: [ControlFlowEffectCheck] -> ControlFlowNodeCheck +runEffectChecks list = checkNode + where + checkNode (node, label) prepost = + case label of + CFApplyEffects effects -> mapM_ (\effect -> mapM_ (\c -> c effect node prepost) list) effects + _ -> 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 @@ -25,6 +25,7 @@ import ShellCheck.ASTLib import ShellCheck.AnalyzerLib import ShellCheck.Interface +import ShellCheck.Prelude import ShellCheck.Regex import Control.Monad @@ -91,55 +92,57 @@ prop_checkBashisms7 = verify checkBashisms "echo ${var[1]}" prop_checkBashisms8 = verify checkBashisms "echo ${!var[@]}" prop_checkBashisms9 = verify checkBashisms "echo ${!var*}" -prop_checkBashisms10= verify checkBashisms "echo ${var:4:12}" -prop_checkBashisms11= verifyNot checkBashisms "echo ${var:-4}" -prop_checkBashisms12= verify checkBashisms "echo ${var//foo/bar}" -prop_checkBashisms13= verify checkBashisms "exec -c env" -prop_checkBashisms14= verify checkBashisms "echo -n \"Foo: \"" -prop_checkBashisms15= verify checkBashisms "let n++" -prop_checkBashisms16= verify checkBashisms "echo $RANDOM" -prop_checkBashisms17= verify checkBashisms "echo $((RANDOM%6+1))" -prop_checkBashisms18= verify checkBashisms "foo &> /dev/null" -prop_checkBashisms19= verify checkBashisms "foo > file*.txt" -prop_checkBashisms20= verify checkBashisms "read -ra foo" -prop_checkBashisms21= verify checkBashisms "[ -a foo ]" -prop_checkBashisms22= verifyNot checkBashisms "[ foo -a bar ]" -prop_checkBashisms23= verify checkBashisms "trap mything ERR INT" -prop_checkBashisms24= verifyNot checkBashisms "trap mything INT TERM" -prop_checkBashisms25= verify checkBashisms "cat < /dev/tcp/host/123" -prop_checkBashisms26= verify checkBashisms "trap mything ERR SIGTERM" -prop_checkBashisms27= verify checkBashisms "echo *[^0-9]*" -prop_checkBashisms28= verify checkBashisms "exec {n}>&2" -prop_checkBashisms29= verify checkBashisms "echo ${!var}" -prop_checkBashisms30= verify checkBashisms "printf -v '%s' \"$1\"" -prop_checkBashisms31= verify checkBashisms "printf '%q' \"$1\"" -prop_checkBashisms32= verifyNot checkBashisms "#!/bin/dash\n[ foo -nt bar ]" -prop_checkBashisms33= verify checkBashisms "#!/bin/sh\necho -n foo" -prop_checkBashisms34= verifyNot checkBashisms "#!/bin/dash\necho -n foo" -prop_checkBashisms35= verifyNot checkBashisms "#!/bin/dash\nlocal foo" -prop_checkBashisms36= verifyNot checkBashisms "#!/bin/dash\nread -p foo -r bar" -prop_checkBashisms37= verifyNot checkBashisms "HOSTNAME=foo; echo $HOSTNAME" -prop_checkBashisms38= verify checkBashisms "RANDOM=9; echo $RANDOM" -prop_checkBashisms39= verify checkBashisms "foo-bar() { true; }" -prop_checkBashisms40= verify checkBashisms "echo $(/dev/null" -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" -prop_checkBashisms52= verifyNot checkBashisms "#!/bin/sh\ncmd >&2" -prop_checkBashisms53= verifyNot checkBashisms "#!/bin/sh\nprintf -- -f\n" -prop_checkBashisms54= verify checkBashisms "#!/bin/sh\nfoo+=bar" -prop_checkBashisms55= verify checkBashisms "#!/bin/sh\necho ${@%foo}" -prop_checkBashisms56= verifyNot checkBashisms "#!/bin/sh\necho ${##}" -prop_checkBashisms57= verifyNot checkBashisms "#!/bin/dash\nulimit -c 0" -prop_checkBashisms58= verify checkBashisms "#!/bin/sh\nulimit -c 0" +prop_checkBashisms10 = verify checkBashisms "echo ${var:4:12}" +prop_checkBashisms11 = verifyNot checkBashisms "echo ${var:-4}" +prop_checkBashisms12 = verify checkBashisms "echo ${var//foo/bar}" +prop_checkBashisms13 = verify checkBashisms "exec -c env" +prop_checkBashisms14 = verify checkBashisms "echo -n \"Foo: \"" +prop_checkBashisms15 = verify checkBashisms "let n++" +prop_checkBashisms16 = verify checkBashisms "echo $RANDOM" +prop_checkBashisms17 = verify checkBashisms "echo $((RANDOM%6+1))" +prop_checkBashisms18 = verify checkBashisms "foo &> /dev/null" +prop_checkBashisms19 = verify checkBashisms "foo > file*.txt" +prop_checkBashisms20 = verify checkBashisms "read -ra foo" +prop_checkBashisms21 = verify checkBashisms "[ -a foo ]" +prop_checkBashisms22 = verifyNot checkBashisms "[ foo -a bar ]" +prop_checkBashisms23 = verify checkBashisms "trap mything ERR INT" +prop_checkBashisms24 = verifyNot checkBashisms "trap mything INT TERM" +prop_checkBashisms25 = verify checkBashisms "cat < /dev/tcp/host/123" +prop_checkBashisms26 = verify checkBashisms "trap mything ERR SIGTERM" +prop_checkBashisms27 = verify checkBashisms "echo *[^0-9]*" +prop_checkBashisms28 = verify checkBashisms "exec {n}>&2" +prop_checkBashisms29 = verify checkBashisms "echo ${!var}" +prop_checkBashisms30 = verify checkBashisms "printf -v '%s' \"$1\"" +prop_checkBashisms31 = verify checkBashisms "printf '%q' \"$1\"" +prop_checkBashisms32 = verifyNot checkBashisms "#!/bin/dash\n[ foo -nt bar ]" +prop_checkBashisms33 = verify checkBashisms "#!/bin/sh\necho -n foo" +prop_checkBashisms34 = verifyNot checkBashisms "#!/bin/dash\necho -n foo" +prop_checkBashisms35 = verifyNot checkBashisms "#!/bin/dash\nlocal foo" +prop_checkBashisms36 = verifyNot checkBashisms "#!/bin/dash\nread -p foo -r bar" +prop_checkBashisms37 = verifyNot checkBashisms "HOSTNAME=foo; echo $HOSTNAME" +prop_checkBashisms38 = verify checkBashisms "RANDOM=9; echo $RANDOM" +prop_checkBashisms39 = verify checkBashisms "foo-bar() { true; }" +prop_checkBashisms40 = verify checkBashisms "echo $(/dev/null" +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" +prop_checkBashisms52 = verifyNot checkBashisms "#!/bin/sh\ncmd >&2" +prop_checkBashisms52b = verifyNot checkBashisms "#!/bin/sh\ncmd >& $var" +prop_checkBashisms52c = verify checkBashisms "#!/bin/sh\ncmd >& $dir/$var" +prop_checkBashisms53 = verifyNot checkBashisms "#!/bin/sh\nprintf -- -f\n" +prop_checkBashisms54 = verify checkBashisms "#!/bin/sh\nfoo+=bar" +prop_checkBashisms55 = verify checkBashisms "#!/bin/sh\necho ${@%foo}" +prop_checkBashisms56 = verifyNot checkBashisms "#!/bin/sh\necho ${##}" +prop_checkBashisms57 = verifyNot checkBashisms "#!/bin/dash\nulimit -c 0" +prop_checkBashisms58 = verify checkBashisms "#!/bin/sh\nulimit -c 0" prop_checkBashisms59 = verify checkBashisms "#!/bin/sh\njobs -s" prop_checkBashisms60 = verifyNot checkBashisms "#!/bin/sh\njobs -p" prop_checkBashisms61 = verifyNot checkBashisms "#!/bin/sh\njobs -lp" @@ -224,7 +227,8 @@ warnMsg id 3018 $ filter (/= '|') op ++ " is" bashism (TA_Binary id "**" _ _) = warnMsg id 3019 "exponentials are" bashism (T_FdRedirect id "&" (T_IoFile _ (T_Greater _) _)) = warnMsg id 3020 "&> is" - bashism (T_FdRedirect id "" (T_IoFile _ (T_GREATAND _) _)) = warnMsg id 3021 ">& is" + bashism (T_FdRedirect id "" (T_IoFile _ (T_GREATAND _) file)) = + unless (all isDigit $ onlyLiteralString file) $ warnMsg id 3021 ">& filename (as opposed to >& fd) is" bashism (T_FdRedirect id ('{':_) _) = warnMsg id 3022 "named file descriptors are" bashism (T_FdRedirect id num _) | all isDigit num && length num > 1 = warnMsg id 3023 "FDs outside 0-9 are" @@ -440,9 +444,9 @@ _ -> False prop_checkEchoSed1 = verify checkEchoSed "FOO=$(echo \"$cow\" | sed 's/foo/bar/g')" -prop_checkEchoSed1b= verify checkEchoSed "FOO=$(sed 's/foo/bar/g' <<< \"$cow\")" +prop_checkEchoSed1b = verify checkEchoSed "FOO=$(sed 's/foo/bar/g' <<< \"$cow\")" prop_checkEchoSed2 = verify checkEchoSed "rm $(echo $cow | sed -e 's,foo,bar,')" -prop_checkEchoSed2b= verify checkEchoSed "rm $(sed -e 's,foo,bar,' <<< $cow)" +prop_checkEchoSed2b = verify checkEchoSed "rm $(sed -e 's,foo,bar,' <<< $cow)" checkEchoSed = ForShell [Bash, Ksh] f where f (T_Redirecting id lefts r) = @@ -528,11 +532,11 @@ isMultiDim l = getBracedModifier (concat $ oversimplify l) `matches` re prop_checkPS11 = verify checkPS1Assignments "PS1='\\033[1;35m\\$ '" -prop_checkPS11a= verify checkPS1Assignments "export PS1='\\033[1;35m\\$ '" +prop_checkPS11a = verify checkPS1Assignments "export PS1='\\033[1;35m\\$ '" prop_checkPSf2 = verify checkPS1Assignments "PS1='\\h \\e[0m\\$ '" prop_checkPS13 = verify checkPS1Assignments "PS1=$'\\x1b[c '" prop_checkPS14 = verify checkPS1Assignments "PS1=$'\\e[3m; '" -prop_checkPS14a= verify checkPS1Assignments "export PS1=$'\\e[3m; '" +prop_checkPS14a = verify checkPS1Assignments "export PS1=$'\\e[3m; '" prop_checkPS15 = verifyNot checkPS1Assignments "PS1='\\[\\033[1;35m\\]\\$ '" prop_checkPS16 = verifyNot checkPS1Assignments "PS1='\\[\\e1m\\e[1m\\]\\$ '" prop_checkPS17 = verifyNot checkPS1Assignments "PS1='e033x1B'" diff --git a/src/ShellCheck/Data.hs b/src/ShellCheck/Data.hs --- a/src/ShellCheck/Data.hs +++ b/src/ShellCheck/Data.hs @@ -2,9 +2,27 @@ import ShellCheck.Interface import Data.Version (showVersion) + + +{- +If you are here because you saw an error about Paths_ShellCheck in this file, +simply comment out the import below and define the version as a constant string. + +Instead of: + + import Paths_ShellCheck (version) + shellcheckVersion = showVersion version + +Use: + + -- import Paths_ShellCheck (version) + shellcheckVersion = "kludge" + +-} + import Paths_ShellCheck (version) +shellcheckVersion = showVersion version -- VERSIONSTRING -shellcheckVersion = showVersion version -- VERSIONSTRING internalVariables = [ -- Generic @@ -12,23 +30,26 @@ -- Bash "BASH", "BASHOPTS", "BASHPID", "BASH_ALIASES", "BASH_ARGC", - "BASH_ARGV", "BASH_CMDS", "BASH_COMMAND", "BASH_EXECUTION_STRING", - "BASH_LINENO", "BASH_REMATCH", "BASH_SOURCE", "BASH_SUBSHELL", - "BASH_VERSINFO", "BASH_VERSION", "COMP_CWORD", "COMP_KEY", - "COMP_LINE", "COMP_POINT", "COMP_TYPE", "COMP_WORDBREAKS", - "COMP_WORDS", "COPROC", "DIRSTACK", "EUID", "FUNCNAME", "GROUPS", - "HISTCMD", "HOSTNAME", "HOSTTYPE", "LINENO", "MACHTYPE", "MAPFILE", - "OLDPWD", "OPTARG", "OPTIND", "OSTYPE", "PIPESTATUS", "PPID", "PWD", - "RANDOM", "READLINE_LINE", "READLINE_POINT", "REPLY", "SECONDS", - "SHELLOPTS", "SHLVL", "UID", "BASH_ENV", "BASH_XTRACEFD", "CDPATH", - "COLUMNS", "COMPREPLY", "EMACS", "ENV", "FCEDIT", "FIGNORE", + "BASH_ARGV", "BASH_ARGV0", "BASH_CMDS", "BASH_COMMAND", + "BASH_EXECUTION_STRING", "BASH_LINENO", "BASH_LOADABLES_PATH", + "BASH_REMATCH", "BASH_SOURCE", "BASH_SUBSHELL", "BASH_VERSINFO", + "BASH_VERSION", "COMP_CWORD", "COMP_KEY", "COMP_LINE", "COMP_POINT", + "COMP_TYPE", "COMP_WORDBREAKS", "COMP_WORDS", "COPROC", "DIRSTACK", + "EPOCHREALTIME", "EPOCHSECONDS", "EUID", "FUNCNAME", "GROUPS", "HISTCMD", + "HOSTNAME", "HOSTTYPE", "LINENO", "MACHTYPE", "MAPFILE", "OLDPWD", + "OPTARG", "OPTIND", "OSTYPE", "PIPESTATUS", "PPID", "PWD", "RANDOM", + "READLINE_ARGUMENT", "READLINE_LINE", "READLINE_MARK", "READLINE_POINT", + "REPLY", "SECONDS", "SHELLOPTS", "SHLVL", "SRANDOM", "UID", "BASH_COMPAT", + "BASH_ENV", "BASH_XTRACEFD", "CDPATH", "CHILD_MAX", "COLUMNS", + "COMPREPLY", "EMACS", "ENV", "EXECIGNORE", "FCEDIT", "FIGNORE", "FUNCNEST", "GLOBIGNORE", "HISTCONTROL", "HISTFILE", "HISTFILESIZE", "HISTIGNORE", "HISTSIZE", "HISTTIMEFORMAT", "HOME", "HOSTFILE", "IFS", - "IGNOREEOF", "INPUTRC", "LANG", "LC_ALL", "LC_COLLATE", "LC_CTYPE", - "LC_MESSAGES", "LC_MONETARY", "LC_NUMERIC", "LC_TIME", "LINES", "MAIL", - "MAILCHECK", "MAILPATH", "OPTERR", "PATH", "POSIXLY_CORRECT", - "PROMPT_COMMAND", "PROMPT_DIRTRIM", "PS1", "PS2", "PS3", "PS4", "SHELL", - "TIMEFORMAT", "TMOUT", "TMPDIR", "auto_resume", "histchars", "COPROC", + "IGNOREEOF", "INPUTRC", "INSIDE_EMACS", "LANG", "LC_ALL", "LC_COLLATE", + "LC_CTYPE", "LC_MESSAGES", "LC_MONETARY", "LC_NUMERIC", "LC_TIME", + "LINES", "MAIL", "MAILCHECK", "MAILPATH", "OPTERR", "PATH", + "POSIXLY_CORRECT", "PROMPT_COMMAND", "PROMPT_DIRTRIM", "PS0", "PS1", + "PS2", "PS3", "PS4", "SHELL", "TIMEFORMAT", "TMOUT", "TMPDIR", + "auto_resume", "histchars", -- Other "USER", "TZ", "TERM", "LOGNAME", "LD_LIBRARY_PATH", "LANGUAGE", "DISPLAY", @@ -43,13 +64,18 @@ "flags_error", "flags_return" ] -specialVariablesWithoutSpaces = [ - "$", "-", "?", "!", "#" +specialIntegerVariables = [ + "$", "?", "!", "#" ] + +specialVariablesWithoutSpaces = "-" : specialIntegerVariables + variablesWithoutSpaces = specialVariablesWithoutSpaces ++ [ - "BASHPID", "BASH_ARGC", "BASH_LINENO", "BASH_SUBSHELL", "EUID", "LINENO", - "OPTIND", "PPID", "RANDOM", "SECONDS", "SHELLOPTS", "SHLVL", "UID", - "COLUMNS", "HISTFILESIZE", "HISTSIZE", "LINES" + "BASHPID", "BASH_ARGC", "BASH_LINENO", "BASH_SUBSHELL", "EUID", + "EPOCHREALTIME", "EPOCHSECONDS", "LINENO", "OPTIND", "PPID", "RANDOM", + "READLINE_ARGUMENT", "READLINE_MARK", "READLINE_POINT", "SECONDS", + "SHELLOPTS", "SHLVL", "SRANDOM", "UID", "COLUMNS", "HISTFILESIZE", + "HISTSIZE", "LINES" -- shflags , "FLAGS_ERROR", "FLAGS_FALSE", "FLAGS_TRUE" @@ -95,10 +121,10 @@ nonReadingCommands = [ "alias", "basename", "bg", "cal", "cd", "chgrp", "chmod", "chown", - "cp", "du", "echo", "export", "false", "fg", "fuser", "getconf", + "cp", "du", "echo", "export", "fg", "fuser", "getconf", "getopt", "getopts", "ipcrm", "ipcs", "jobs", "kill", "ln", "ls", "locale", "mv", "printf", "ps", "pwd", "renice", "rm", "rmdir", - "set", "sleep", "touch", "trap", "true", "ulimit", "unalias", "uname" + "set", "sleep", "touch", "trap", "ulimit", "unalias", "uname" ] sampleWords = [ @@ -138,5 +164,6 @@ _ -> Nothing flagsForRead = "sreu:n:N:i:p:a:t:" +flagsForMapfile = "d:n:O:s:u:C:c:t" declaringCommands = ["local", "declare", "export", "readonly", "typeset", "let"] diff --git a/src/ShellCheck/Debug.hs b/src/ShellCheck/Debug.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/Debug.hs @@ -0,0 +1,313 @@ +{- + +This file contains useful functions for debugging and developing ShellCheck. + +To invoke them interactively, run: + + cabal repl + +At the ghci prompt, enter: + + :load ShellCheck.Debug + +You can now invoke the functions. Here are some examples: + + shellcheckString "echo $1" + stringToAst "(( x+1 ))" + stringToCfg "if foo; then bar; else baz; fi" + writeFile "/tmp/test.dot" $ stringToCfgViz "while foo; do bar; done" + +The latter file can be rendered to png with GraphViz: + + dot -Tpng /tmp/test.dot > /tmp/test.png + +To run all unit tests in a module: + + ShellCheck.Parser.runTests + ShellCheck.Analytics.runTests + +To run a specific test: + + :load ShellCheck.Analytics + prop_checkUuoc3 + +If you make code changes, reload in seconds at any time with: + + :r + +=========================================================================== + +Crash course in printf debugging in Haskell: + + import Debug.Trace + + greet 0 = return () + -- Print when a function is invoked + greet n | trace ("calling greet " ++ show n) False = undefined + greet n = do + putStrLn "Enter name" + name <- getLine + -- Print at some point in any monadic function + traceM $ "user entered " ++ name + putStrLn $ "Hello " ++ name + -- Print a value before passing it on + greet $ traceShowId (n - 1) + + +=========================================================================== + +If you want to invoke `ghci` directly, such as on `shellcheck.hs`, to +debug all of ShellCheck including I/O, you may see an error like this: + + src/ShellCheck/Data.hs:5:1: error: + Could not load module ‘Paths_ShellCheck’ + it is a hidden module in the package ‘ShellCheck-0.8.0’ + +This can easily be circumvented by running `./setgitversion` or manually +editing src/ShellCheck/Data.hs to replace the auto-deduced version number +with a constant string as indicated. + +Afterwards, you can run the ShellCheck tool, as if from the shell, with: + + $ ghci shellcheck.hs + ghci> runMain ["-x", "file.sh"] + +-} + +module ShellCheck.Debug () where + +import ShellCheck.Analyzer +import ShellCheck.AST +import ShellCheck.CFG +import ShellCheck.Checker +import ShellCheck.CFGAnalysis as CF +import ShellCheck.Interface +import ShellCheck.Parser +import ShellCheck.Prelude + +import Control.Monad +import Control.Monad.Identity +import Control.Monad.RWS +import Control.Monad.Writer +import Data.Graph.Inductive.Graph as G +import Data.List +import Data.Maybe +import qualified Data.Map as M +import qualified Data.Set as S + + +-- Run all of ShellCheck (minus output formatters) +shellcheckString :: String -> CheckResult +shellcheckString scriptString = + runIdentity $ checkScript dummySystemInterface checkSpec + where + checkSpec :: CheckSpec + checkSpec = emptyCheckSpec { + csScript = scriptString + } + +dummySystemInterface :: SystemInterface Identity +dummySystemInterface = mockedSystemInterface [ + -- A tiny, fake filesystem for sourced files + ("lib/mylib1.sh", "foo=$(cat $1 | wc -l)"), + ("lib/mylib2.sh", "bar=42") + ] + +-- Parameters used when generating Control Flow Graphs +cfgParams :: CFGParameters +cfgParams = CFGParameters { + cfLastpipe = False, + cfPipefail = False +} + +-- An example script to play with +exampleScript :: String +exampleScript = unlines [ + "#!/bin/sh", + "count=0", + "for file in *", + "do", + " (( count++ ))", + "done", + "echo $count" + ] + +-- Parse the script string into ShellCheck's ParseResult +parseScriptString :: String -> ParseResult +parseScriptString scriptString = + runIdentity $ parseScript dummySystemInterface parseSpec + where + parseSpec :: ParseSpec + parseSpec = newParseSpec { + psFilename = "myscript", + psScript = scriptString + } + + +-- Parse the script string into an Abstract Syntax Tree +stringToAst :: String -> Token +stringToAst scriptString = + case maybeRoot of + Just root -> root + Nothing -> error $ "Script failed to parse: " ++ show parserWarnings + where + parseResult :: ParseResult + parseResult = parseScriptString scriptString + + maybeRoot :: Maybe Token + maybeRoot = prRoot parseResult + + parserWarnings :: [PositionedComment] + parserWarnings = prComments parseResult + + +astToCfgResult :: Token -> CFGResult +astToCfgResult = buildGraph cfgParams + +astToDfa :: Token -> CFGAnalysis +astToDfa = analyzeControlFlow cfgParams + +astToCfg :: Token -> CFGraph +astToCfg = cfGraph . astToCfgResult + +stringToCfg :: String -> CFGraph +stringToCfg = astToCfg . stringToAst + +stringToDfa :: String -> CFGAnalysis +stringToDfa = astToDfa . stringToAst + +cfgToGraphViz :: CFGraph -> String +cfgToGraphViz = cfgToGraphVizWith show + +stringToCfgViz :: String -> String +stringToCfgViz = cfgToGraphViz . stringToCfg + +stringToDfaViz :: String -> String +stringToDfaViz = dfaToGraphViz . stringToDfa + +-- Dump a Control Flow Graph as GraphViz with extended information +stringToDetailedCfgViz :: String -> String +stringToDetailedCfgViz scriptString = cfgToGraphVizWith nodeLabel graph + where + ast :: Token + ast = stringToAst scriptString + + cfgResult :: CFGResult + cfgResult = astToCfgResult ast + + graph :: CFGraph + graph = cfGraph cfgResult + + idToToken :: M.Map Id Token + idToToken = M.fromList $ execWriter $ doAnalysis (\c -> tell [(getId c, c)]) ast + + idToNode :: M.Map Id (Node, Node) + idToNode = cfIdToRange cfgResult + + nodeToStartIds :: M.Map Node (S.Set Id) + nodeToStartIds = + M.fromListWith S.union $ + map (\(id, (start, _)) -> (start, S.singleton id)) $ + M.toList idToNode + + nodeToEndIds :: M.Map Node (S.Set Id) + nodeToEndIds = + M.fromListWith S.union $ + map (\(id, (_, end)) -> (end, S.singleton id)) $ + M.toList idToNode + + formatId :: Id -> String + formatId id = fromMaybe ("Unknown " ++ show id) $ do + (OuterToken _ token) <- M.lookup id idToToken + firstWord <- words (show token) !!! 0 + -- Strip off "Inner_" + (_ : tokenName) <- return $ dropWhile (/= '_') firstWord + return $ tokenName ++ " " ++ show id + + formatGroup :: S.Set Id -> String + formatGroup set = intercalate ", " $ map formatId $ S.toList set + + nodeLabel (node, label) = unlines [ + show node ++ ". " ++ show label, + "Begin: " ++ formatGroup (M.findWithDefault S.empty node nodeToStartIds), + "End: " ++ formatGroup (M.findWithDefault S.empty node nodeToEndIds) + ] + + +-- Dump a Control Flow Graph with Data Flow Analysis as GraphViz +dfaToGraphViz :: CF.CFGAnalysis -> String +dfaToGraphViz analysis = cfgToGraphVizWith label $ CF.graph analysis + where + label (node, label) = + let + desc = show node ++ ". " ++ show label + in + fromMaybe ("No DFA available\n\n" ++ desc) $ do + (pre, post) <- M.lookup node $ CF.nodeToData analysis + return $ unlines [ + "Precondition: " ++ show pre, + "", + desc, + "", + "Postcondition: " ++ show post + ] + + +-- Dump an Control Flow Graph to GraphViz with a given node formatter +cfgToGraphVizWith :: (LNode CFNode -> String) -> CFGraph -> String +cfgToGraphVizWith nodeLabel graph = concat [ + "digraph {\n", + concatMap dumpNode (labNodes graph), + concatMap dumpLink (labEdges graph), + tagVizEntries graph, + "}\n" + ] + where + dumpNode l@(node, label) = show node ++ " [label=" ++ quoteViz (nodeLabel l) ++ "]\n" + dumpLink (from, to, typ) = show from ++ " -> " ++ show to ++ " [style=" ++ quoteViz (edgeStyle typ) ++ "]\n" + edgeStyle CFEFlow = "solid" + edgeStyle CFEExit = "bold" + edgeStyle CFEFalseFlow = "dotted" + +quoteViz str = "\"" ++ escapeViz str ++ "\"" +escapeViz [] = [] +escapeViz (c:rest) = + case c of + '\"' -> '\\' : '\"' : escapeViz rest + '\n' -> '\\' : 'l' : escapeViz rest + '\\' -> '\\' : '\\' : escapeViz rest + _ -> c : escapeViz rest + + +-- Dump an Abstract Syntax Tree (or branch thereof) to GraphViz format +astToGraphViz :: Token -> String +astToGraphViz token = concat [ + "digraph {\n", + formatTree token, + "}\n" + ] + where + formatTree :: Token -> String + formatTree t = snd $ execRWS (doStackAnalysis push pop t) () [] + + push :: Token -> RWS () String [Int] () + push (OuterToken (Id n) inner) = do + stack <- get + put (n : stack) + case stack of + [] -> return () + (top:_) -> tell $ show top ++ " -> " ++ show n ++ "\n" + tell $ show n ++ " [label=" ++ quoteViz (show n ++ ": " ++ take 32 (show inner)) ++ "]\n" + + pop :: Token -> RWS () String [Int] () + pop _ = modify tail + + +-- For each entry point, set the rank so that they'll align in the graph +tagVizEntries :: CFGraph -> String +tagVizEntries graph = "{ rank=same " ++ rank ++ " }" + where + entries = mapMaybe find $ labNodes graph + find (node, CFEntryPoint name) = return (node, name) + find _ = Nothing + rank = unwords $ map (\(c, _) -> show c) entries diff --git a/src/ShellCheck/Fixer.hs b/src/ShellCheck/Fixer.hs --- a/src/ShellCheck/Fixer.hs +++ b/src/ShellCheck/Fixer.hs @@ -22,6 +22,8 @@ module ShellCheck.Fixer (applyFix, removeTabStops, mapPositions, Ranged(..), runTests) where import ShellCheck.Interface +import ShellCheck.Prelude +import Control.Monad import Control.Monad.State import Data.Array import Data.List @@ -35,7 +37,7 @@ end :: a -> Position overlap :: a -> a -> Bool overlap x y = - (yStart >= xStart && yStart < xEnd) || (yStart < xStart && yEnd > xStart) + xEnd > yStart && yEnd > xStart where yStart = start y yEnd = end y @@ -86,6 +88,7 @@ instance Monoid Fix where mempty = newFix mappend = (<>) + mconcat = foldl mappend mempty -- fold left to right since <> discards right on overlap instance Semigroup Fix where f1 <> f2 = @@ -228,7 +231,7 @@ let (l1, l2) = tmap posLine originalPos in when (l1 /= 1 || l2 /= 1) $ - error "ShellCheck internal error, please report: bad cross-line fix" + error $ pleaseReport "bad cross-line fix" let replacer = repString rep let shift = (length replacer) - (oldEnd - oldStart) diff --git a/src/ShellCheck/Formatter/CheckStyle.hs b/src/ShellCheck/Formatter/CheckStyle.hs --- a/src/ShellCheck/Formatter/CheckStyle.hs +++ b/src/ShellCheck/Formatter/CheckStyle.hs @@ -88,7 +88,7 @@ attr s v = concat [ s, "='", escape v, "' " ] escape = concatMap escape' escape' c = if isOk c then [c] else "&#" ++ show (ord c) ++ ";" -isOk x = any ($x) [isAsciiUpper, isAsciiLower, isDigit, (`elem` " ./")] +isOk x = any ($ x) [isAsciiUpper, isAsciiLower, isDigit, (`elem` " ./")] severity "error" = "error" severity "warning" = "warning" diff --git a/src/ShellCheck/Formatter/Diff.hs b/src/ShellCheck/Formatter/Diff.hs --- a/src/ShellCheck/Formatter/Diff.hs +++ b/src/ShellCheck/Formatter/Diff.hs @@ -203,10 +203,9 @@ buildFixMap :: [Fix] -> M.Map String Fix buildFixMap fixes = perFile where - splitFixes = concatMap splitFixByFile fixes + splitFixes = splitFixByFile $ mconcat fixes perFile = groupByMap (posFile . repStartPos . head . fixReplacements) splitFixes --- There are currently no multi-file fixes, but let's handle it anyways splitFixByFile :: Fix -> [Fix] splitFixByFile fix = map makeFix $ groupBy sameFile (fixReplacements fix) where 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 @@ -23,6 +23,7 @@ import ShellCheck.Interface import ShellCheck.Formatter.Format +import Control.DeepSeq import Data.Aeson import Data.IORef import Data.Monoid @@ -103,7 +104,7 @@ comments = crComments cr groups = groupWith sourceFile comments f :: [PositionedComment] -> IO () - f group = modifyIORef ref (\x -> comments ++ x) + f group = deepseq comments $ modifyIORef ref (\x -> comments ++ x) finish ref = do list <- readIORef ref diff --git a/src/ShellCheck/Formatter/JSON1.hs b/src/ShellCheck/Formatter/JSON1.hs --- a/src/ShellCheck/Formatter/JSON1.hs +++ b/src/ShellCheck/Formatter/JSON1.hs @@ -23,6 +23,7 @@ import ShellCheck.Interface import ShellCheck.Formatter.Format +import Control.DeepSeq import Data.Aeson import Data.IORef import Data.Monoid @@ -120,7 +121,7 @@ result <- siReadFile sys (Just True) filename let contents = either (const "") id result let comments' = makeNonVirtual comments contents - modifyIORef ref (\x -> comments' ++ x) + deepseq comments' $ modifyIORef ref (\x -> comments' ++ x) finish ref = do list <- readIORef ref 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 @@ -23,6 +23,7 @@ import ShellCheck.Interface import ShellCheck.Formatter.Format +import Control.DeepSeq import Control.Monad import Data.Array import Data.Foldable @@ -88,7 +89,7 @@ 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 + writeIORef errRef $! force . take max . nubBy equal . sort $ previous ++ current where fst3 (x,_,_) = x equal x y = fst3 x == fst3 y diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs --- a/src/ShellCheck/Parser.hs +++ b/src/ShellCheck/Parser.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2021 Vidar Holen + Copyright 2012-2022 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -27,6 +27,7 @@ import ShellCheck.ASTLib hiding (runTests) import ShellCheck.Data import ShellCheck.Interface +import ShellCheck.Prelude import Control.Applicative ((<*), (*>)) import Control.Monad @@ -37,7 +38,6 @@ import Data.List (isPrefixOf, isInfixOf, isSuffixOf, partition, sortBy, intercalate, nub, find) import Data.Maybe import Data.Monoid -import Debug.Trace -- STRIP import GHC.Exts (sortWith) import Prelude hiding (readList) import System.IO @@ -46,7 +46,7 @@ import Text.Parsec.Pos import qualified Control.Monad.Reader as Mr import qualified Control.Monad.State as Ms -import qualified Data.Map as Map +import qualified Data.Map.Strict as Map import Test.QuickCheck.All (quickCheckAll) @@ -210,7 +210,7 @@ -- 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 <$> + Map.findWithDefault (error $ pleaseReport "no parser span for id") id <$> getMap -- Create a new id with the same span as an existing one @@ -457,8 +457,8 @@ pos <- getPosition withContext (ContextName pos s) p -withAnnotations anns = - withContext (ContextAnnotation anns) +withAnnotations anns p = + if null anns then p else withContext (ContextAnnotation anns) p readConditionContents single = readCondContents `attempting` lookAhead (do @@ -556,7 +556,7 @@ notFollowedBy2 (try (spacing >> string "]")) x <- readNormalWord pos <- getPosition - when (endedWith "]" x && notArrayIndex x) $ do + when (notArrayIndex x && endedWith "]" x && not (x `containsLiteral` "[")) $ do parseProblemAt pos ErrorC 1020 $ "You need a space before the " ++ (if single then "]" else "]]") ++ "." fail "Missing space before ]" @@ -572,6 +572,7 @@ endedWith _ _ = False notArrayIndex (T_NormalWord id s@(_:T_Literal _ t:_)) = t /= "[" notArrayIndex _ = True + containsLiteral x s = s `isInfixOf` onlyLiteralString x readCondAndOp = readAndOrOp TC_And "&&" False <|> readAndOrOp TC_And "-a" True @@ -717,20 +718,20 @@ prop_a7 = isOk readArithmeticContents "3*2**10" prop_a8 = isOk readArithmeticContents "3" prop_a9 = isOk readArithmeticContents "a^!-b" -prop_a10= isOk readArithmeticContents "! $?" -prop_a11= isOk readArithmeticContents "10#08 * 16#f" -prop_a12= isOk readArithmeticContents "\"$((3+2))\" + '37'" -prop_a13= isOk readArithmeticContents "foo[9*y+x]++" -prop_a14= isOk readArithmeticContents "1+`echo 2`" -prop_a15= isOk readArithmeticContents "foo[`echo foo | sed s/foo/4/g` * 3] + 4" -prop_a16= isOk readArithmeticContents "$foo$bar" -prop_a17= isOk readArithmeticContents "i<(0+(1+1))" -prop_a18= isOk readArithmeticContents "a?b:c" -prop_a19= isOk readArithmeticContents "\\\n3 +\\\n 2" -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" +prop_a10 = isOk readArithmeticContents "! $?" +prop_a11 = isOk readArithmeticContents "10#08 * 16#f" +prop_a12 = isOk readArithmeticContents "\"$((3+2))\" + '37'" +prop_a13 = isOk readArithmeticContents "foo[9*y+x]++" +prop_a14 = isOk readArithmeticContents "1+`echo 2`" +prop_a15 = isOk readArithmeticContents "foo[`echo foo | sed s/foo/4/g` * 3] + 4" +prop_a16 = isOk readArithmeticContents "$foo$bar" +prop_a17 = isOk readArithmeticContents "i<(0+(1+1))" +prop_a18 = isOk readArithmeticContents "a?b:c" +prop_a19 = isOk readArithmeticContents "\\\n3 +\\\n 2" +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 @@ -819,11 +820,13 @@ return $ TA_Expansion id pieces readGroup = do + start <- startSpan char '(' s <- readSequence char ')' + id <- endSpan start spacing - return s + return $ TA_Parentesis id s readArithTerm = readGroup <|> readVariable <|> readExpansion @@ -923,8 +926,8 @@ 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_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}$ ]]" @@ -941,6 +944,9 @@ prop_readCondition25 = isOk readCondition "[[ lex.yy.c -ot program.l ]]" prop_readCondition26 = isOk readScript "[[ foo ]]\\\n && bar" prop_readCondition27 = not $ isOk readConditionCommand "[[ x ]] foo" +prop_readCondition28 = isOk readCondition "[[ x = [\"$1\"] ]]" +prop_readCondition29 = isOk readCondition "[[ x = [*] ]]" + readCondition = called "test expression" $ do opos <- getPosition start <- startSpan @@ -985,6 +991,10 @@ prop_readAnnotation6 = isOk readAnnotation "# shellcheck disable=SC1234 # shellcheck foo=bar\n" prop_readAnnotation7 = isOk readAnnotation "# shellcheck disable=SC1000,SC2000-SC3000,SC1001\n" prop_readAnnotation8 = isOk readAnnotation "# shellcheck disable=all\n" +prop_readAnnotation9 = isOk readAnnotation "# shellcheck source='foo bar' source-path=\"baz etc\"\n" +prop_readAnnotation10 = isOk readAnnotation "# shellcheck disable='SC1234,SC2345' enable=\"foo\" shell='bash'\n" +prop_readAnnotation11 = isOk (readAnnotationWithoutPrefix False) "external-sources='true'" + readAnnotation = called "shellcheck directive" $ do try readAnnotationPrefix many1 linewhitespace @@ -1000,12 +1010,19 @@ many linewhitespace return $ concat values where + plainOrQuoted p = quoted p <|> p + quoted p = do + c <- oneOf "'\"" + start <- getPosition + str <- many1 $ noneOf (c:"\n") + char c <|> fail "Missing terminating quote for directive." + subParse start p str readKey = do keyPos <- getPosition key <- many1 (letter <|> char '-') char '=' <|> fail "Expected '=' after directive key" annotations <- case key of - "disable" -> readElement `sepBy` char ',' + "disable" -> plainOrQuoted $ readElement `sepBy` char ',' where readElement = readRange <|> readAll readAll = do @@ -1020,21 +1037,21 @@ int <- many1 digit return $ read int - "enable" -> readName `sepBy` char ',' + "enable" -> plainOrQuoted $ readName `sepBy` char ',' where readName = EnableComment <$> many1 (letter <|> char '-') "source" -> do - filename <- many1 $ noneOf " \n" + filename <- quoted (many1 anyChar) <|> (many1 $ noneOf " \n") return [SourceOverride filename] "source-path" -> do - dirname <- many1 $ noneOf " \n" + dirname <- quoted (many1 anyChar) <|> (many1 $ noneOf " \n") return [SourcePath dirname] "shell" -> do pos <- getPosition - shell <- many1 $ noneOf " \n" + shell <- quoted (many1 anyChar) <|> (many1 $ noneOf " \n") when (isNothing $ shellForExecutable shell) $ parseNoteAt pos ErrorC 1103 "This shell type is unknown. Use e.g. sh or bash." @@ -1042,7 +1059,7 @@ "external-sources" -> do pos <- getPosition - value <- many1 letter + value <- plainOrQuoted $ many1 letter case value of "true" -> if sandboxed @@ -1696,9 +1713,9 @@ id <- endSpan start return $ T_DollarBraced id True word -prop_readDollarExpansion1= isOk readDollarExpansion "$(echo foo; ls\n)" -prop_readDollarExpansion2= isOk readDollarExpansion "$( )" -prop_readDollarExpansion3= isOk readDollarExpansion "$( command \n#comment \n)" +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 start <- startSpan try (string "$(") @@ -1790,17 +1807,17 @@ prop_readHereDoc7 = isOk readScript "cat << foo\n\\$(f ())\nfoo" prop_readHereDoc8 = isOk readScript "cat <>bar\netc\nfoo" prop_readHereDoc9 = isOk readScript "if true; then cat << foo; fi\nbar\nfoo\n" -prop_readHereDoc10= isOk readScript "if true; then cat << foo << bar; fi\nfoo\nbar\n" -prop_readHereDoc11= isOk readScript "cat << foo $(\nfoo\n)lol\nfoo\n" -prop_readHereDoc12= isOk readScript "cat << foo|cat\nbar\nfoo" -prop_readHereDoc13= isOk readScript "cat <<'#!'\nHello World\n#!\necho Done" -prop_readHereDoc14= isWarning readScript "cat << foo\nbar\nfoo \n" -prop_readHereDoc15= isWarning readScript "cat < - error "ShellCheck bug, please report (here doc trailer)." + error $ pleaseReport "unexpected heredoc trailer" -- The following cases assume no trailing text: | dashed == Undashed && (not $ null leadingSpace) -> do @@ -2095,6 +2112,7 @@ prop_readSimpleCommand12 = isWarning readSimpleCommand "elsif foo" prop_readSimpleCommand13 = isWarning readSimpleCommand "ElseIf foo" prop_readSimpleCommand14 = isWarning readSimpleCommand "elseif[$i==2]" +prop_readSimpleCommand15 = isWarning readSimpleCommand "trap 'foo\"bar' INT" readSimpleCommand = called "simple command" $ do prefix <- option [] readCmdPrefix skipAnnotationAndWarn @@ -2124,9 +2142,12 @@ id2 <- getNewIdFor id1 let result = makeSimpleCommand id1 id2 prefix [cmd] suffix - if isCommand ["source", "."] cmd - then readSource result - else return result + case () of + _ | isCommand ["source", "."] cmd -> readSource result + _ | isCommand ["trap"] cmd -> do + syntaxCheckTrap result + return result + _ -> return result where isCommand strings (T_NormalWord _ [T_Literal _ s]) = s `elem` strings isCommand _ _ = False @@ -2146,6 +2167,17 @@ parseProblemAtId (getId cmd) ErrorC 1131 "Use 'elif' to start another branch." _ -> return () + syntaxCheckTrap cmd = + case cmd of + (T_Redirecting _ _ (T_SimpleCommand _ _ (cmd:arg:_))) -> checkArg arg (getLiteralString arg) + _ -> return () + where + checkArg _ Nothing = return () + checkArg arg (Just ('-':_)) = return () + checkArg arg (Just str) = do + (start,end) <- getSpanForId (getId arg) + subParse start (tryWithErrors (readCompoundListOrEmpty >> verifyEof) <|> return ()) str + commentWarning id = parseProblemAtId id ErrorC 1127 "Was this intended as a comment? Use # in sh." @@ -2282,7 +2314,7 @@ parseProblemAt apos ErrorC 1123 "ShellCheck directives are only valid in front of complete compound commands, like 'if', not e.g. individual 'elif' branches." andOr <- withAnnotations annotations $ - chainr1 readPipeline $ do + chainl1 readPipeline $ do op <- g_AND_IF <|> g_OR_IF readLineBreak return $ case op of T_AND_IF id -> T_AndIf id @@ -2483,17 +2515,30 @@ spacing return $ T_BraceGroup id list -prop_readBatsTest = isOk readBatsTest "@test 'can parse' {\n true\n}" +prop_readBatsTest1 = isOk readBatsTest "@test 'can parse' {\n true\n}" +prop_readBatsTest2 = isOk readBatsTest "@test random text !(@*$Y&! {\n true\n}" +prop_readBatsTest3 = isOk readBatsTest "@test foo { bar { baz {\n true\n}" +prop_readBatsTest4 = isNotOk readBatsTest "@test foo \n{\n true\n}" readBatsTest = called "bats @test" $ do start <- startSpan - try $ string "@test" + try $ string "@test " spacing - name <- readNormalWord + name <- readBatsName spacing test <- readBraceGroup id <- endSpan start return $ T_BatsTest id name test + where + readBatsName = do + line <- try . lookAhead $ many1 $ noneOf "\n" + let name = reverse $ f $ reverse line + string name + -- We want everything before the last " {" in a string, so we find everything after "{ " in its reverse + f ('{':' ':rest) = dropWhile isSpace rest + f (a:rest) = f rest + f [] = "" + prop_readWhileClause = isOk readWhileClause "while [[ -e foo ]]; do sleep 1; done" readWhileClause = called "while loop" $ do start <- startSpan @@ -2521,7 +2566,7 @@ parseProblem ErrorC 1058 "Expected 'do'." return "Expected 'do'" - acceptButWarn g_Semi ErrorC 1059 "No semicolons directly after 'do'." + acceptButWarn g_Semi ErrorC 1059 "Semicolon is not allowed directly after 'do'. You can just delete it." allspacing optional (do @@ -2550,9 +2595,9 @@ prop_readForClause7 = isOk readForClause "for ((;;)) do echo $i\ndone" prop_readForClause8 = isOk readForClause "for ((;;)) ; do echo $i\ndone" prop_readForClause9 = isOk readForClause "for i do true; done" -prop_readForClause10= isOk readForClause "for ((;;)) { true; }" -prop_readForClause12= isWarning readForClause "for $a in *; do echo \"$a\"; done" -prop_readForClause13= isOk readForClause "for foo\nin\\\n bar\\\n baz\ndo true; done" +prop_readForClause10 = isOk readForClause "for ((;;)) { true; }" +prop_readForClause12 = isWarning readForClause "for $a in *; do echo \"$a\"; done" +prop_readForClause13 = isOk readForClause "for foo\nin\\\n bar\\\n baz\ndo true; done" readForClause = called "for loop" $ do pos <- getPosition (T_For id) <- g_For @@ -2684,10 +2729,10 @@ prop_readFunctionDefinition7 = isOk readFunctionDefinition "..(){ cd ..; }" prop_readFunctionDefinition8 = isOk readFunctionDefinition "foo() (ls)" prop_readFunctionDefinition9 = isOk readFunctionDefinition "function foo { true; }" -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; }" +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 @@ -2885,14 +2930,14 @@ prop_readAssignmentWord7 = isOk readAssignmentWord "a[3$n'']=42" prop_readAssignmentWord8 = isOk readAssignmentWord "a[4''$(cat foo)]=42" prop_readAssignmentWord9 = isOk readAssignmentWord "IFS= " -prop_readAssignmentWord9a= isOk readAssignmentWord "foo=" -prop_readAssignmentWord9b= isOk readAssignmentWord "foo= " -prop_readAssignmentWord9c= isOk readAssignmentWord "foo= #bar" -prop_readAssignmentWord11= isOk readAssignmentWord "foo=([a]=b [c] [d]= [e f )" -prop_readAssignmentWord12= isOk readAssignmentWord "a[b <<= 3 + c]='thing'" -prop_readAssignmentWord13= isOk readAssignmentWord "var=( (1 2) (3 4) )" -prop_readAssignmentWord14= isOk readAssignmentWord "var=( 1 [2]=(3 4) )" -prop_readAssignmentWord15= isOk readAssignmentWord "var=(1 [2]=(3 4))" +prop_readAssignmentWord9a = isOk readAssignmentWord "foo=" +prop_readAssignmentWord9b = isOk readAssignmentWord "foo= " +prop_readAssignmentWord9c = isOk readAssignmentWord "foo= #bar" +prop_readAssignmentWord11 = isOk readAssignmentWord "foo=([a]=b [c] [d]= [e f )" +prop_readAssignmentWord12 = isOk readAssignmentWord "a[b <<= 3 + c]='thing'" +prop_readAssignmentWord13 = isOk readAssignmentWord "var=( (1 2) (3 4) )" +prop_readAssignmentWord14 = isOk readAssignmentWord "var=( 1 [2]=(3 4) )" +prop_readAssignmentWord15 = isOk readAssignmentWord "var=(1 [2]=(3 4))" readAssignmentWord = readAssignmentWordExt True readWellFormedAssignment = readAssignmentWordExt False readAssignmentWordExt lenient = called "variable assignment" $ do @@ -3240,45 +3285,52 @@ prop_readScript4 = isWarning readScript "#!/usr/bin/perl\nfoo=(" prop_readScript5 = isOk readScript "#!/bin/bash\n#This is an empty script\n\n" prop_readScript6 = isOk readScript "#!/usr/bin/env -S X=FOO bash\n#This is an empty script\n\n" +prop_readScript7 = isOk readScript "#!/bin/zsh\n# shellcheck disable=SC1071\nfor f (a b); echo $f\n" readScriptFile sourced = do 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 ." - shebang <- readShebang <|> readEmptyLiteral - let (T_Literal _ shebangString) = shebang - allspacing - annotationStart <- startSpan - fileAnnotations <- readAnnotations rcAnnotations <- if sourced then return [] else do filename <- Mr.asks currentFilename readConfigFile filename - let annotations = fileAnnotations ++ rcAnnotations - 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 (executableFromShebang shebangString) - if ignoreShebang || isValidShell (executableFromShebang shebangString) /= Just False - then do - commands <- withAnnotations annotations readCompoundListOrEmpty - id <- endSpan start - verifyEof - let script = T_Annotation annotationId annotations $ - T_Script id shebang commands - reparseIndices script - else do - many anyChar - id <- endSpan start - return $ T_Script id shebang [] + -- Put the rc annotations on the stack so that one can ignore e.g. SC1084 in .shellcheckrc + withAnnotations rcAnnotations $ do + hasBom <- wasIncluded readUtf8Bom + shebang <- readShebang <|> readEmptyLiteral + let (T_Literal _ shebangString) = shebang + allspacing + annotationStart <- startSpan + fileAnnotations <- readAnnotations + -- Similarly put the filewide annotations on the stack to allow earlier suppression + withAnnotations fileAnnotations $ do + when (hasBom) $ + parseProblemAt pos ErrorC 1082 + "This file has a UTF-8 BOM. Remove it with: LC_CTYPE=C sed '1s/^...//' < yourscript ." + let annotations = fileAnnotations ++ rcAnnotations + 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 (executableFromShebang shebangString) + if ignoreShebang || isValidShell (executableFromShebang shebangString) /= Just False + then do + commands <- readCompoundListOrEmpty + id <- endSpan start + verifyEof + let script = T_Annotation annotationId annotations $ + T_Script id shebang commands + reparseIndices script + else do + many anyChar + id <- endSpan start + return $ T_Script id shebang [] + where verifyShebang pos s = do case isValidShell s of @@ -3370,16 +3422,6 @@ return $ Just . null $ parseNotes userState ++ parseProblems systemState (Left _, _) -> return Nothing --- For printf debugging: print the value of an expression --- Example: return $ dump $ T_Literal id [c] -dump :: Show a => a -> a -- STRIP -dump x = trace (show x) x -- STRIP - --- Like above, but print a specific expression: --- Example: return $ dumps ("Returning: " ++ [c]) $ T_Literal id [c] -dumps :: Show x => x -> a -> a -- STRIP -dumps t = trace (show t) -- STRIP - parseWithNotes parser = do item <- parser state <- getState @@ -3459,9 +3501,9 @@ -- Go over all T_UnparsedIndex and reparse them as either arithmetic or text -- depending on declare -A statements. -reparseIndices root = - analyze blank blank f root +reparseIndices root = process root where + process = analyze blank blank f associative = getAssociativeArrays root isAssociative s = s `elem` associative f (T_Assignment id mode name indices value) = do @@ -3486,8 +3528,9 @@ fixAssignmentIndex name word = case word of - T_UnparsedIndex id pos src -> - parsed name pos src + T_UnparsedIndex id pos src -> do + idx <- parsed name pos src + process idx -- Recursively parse for cases like x[y[z=1]]=1 _ -> return word parsed name pos src = diff --git a/src/ShellCheck/Prelude.hs b/src/ShellCheck/Prelude.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/Prelude.hs @@ -0,0 +1,51 @@ +{- + Copyright 2022 Vidar Holen + + This file is part of ShellCheck. + https://www.shellcheck.net + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} + +-- Generic basic utility functions +module ShellCheck.Prelude where + +import Data.Semigroup + + +-- Get element 0 or a default. Like `head` but safe. +headOrDefault _ (a:_) = a +headOrDefault def _ = def + +-- Get the last element or a default. Like `last` but safe. +lastOrDefault def [] = def +lastOrDefault _ list = last list + +--- Get element n of a list, or Nothing. Like `!!` but safe. +(!!!) list i = + case drop i list of + [] -> Nothing + (r:_) -> Just r + + +-- Like mconcat but for Semigroups +sconcat1 :: (Semigroup t) => [t] -> t +sconcat1 [x] = x +sconcat1 (x:xs) = x <> sconcat1 xs + +sconcatOrDefault def [] = def +sconcatOrDefault _ list = sconcat1 list + +-- For more actionable "impossible" errors +pleaseReport str = "ShellCheck internal error, please report: " ++ str diff --git a/test/shellcheck.hs b/test/shellcheck.hs --- a/test/shellcheck.hs +++ b/test/shellcheck.hs @@ -5,8 +5,11 @@ import qualified ShellCheck.Analytics import qualified ShellCheck.AnalyzerLib import qualified ShellCheck.ASTLib +import qualified ShellCheck.CFG +import qualified ShellCheck.CFGAnalysis import qualified ShellCheck.Checker import qualified ShellCheck.Checks.Commands +import qualified ShellCheck.Checks.ControlFlow import qualified ShellCheck.Checks.Custom import qualified ShellCheck.Checks.ShellSupport import qualified ShellCheck.Fixer @@ -19,8 +22,11 @@ ShellCheck.Analytics.runTests ,ShellCheck.AnalyzerLib.runTests ,ShellCheck.ASTLib.runTests + ,ShellCheck.CFG.runTests + ,ShellCheck.CFGAnalysis.runTests ,ShellCheck.Checker.runTests ,ShellCheck.Checks.Commands.runTests + ,ShellCheck.Checks.ControlFlow.runTests ,ShellCheck.Checks.Custom.runTests ,ShellCheck.Checks.ShellSupport.runTests ,ShellCheck.Fixer.runTests