Skip to content

Commit

Permalink
Add extended-analysis directive to toggle DFA
Browse files Browse the repository at this point in the history
  • Loading branch information
koalaman committed Feb 4, 2024
1 parent 1565091 commit d80fdfa
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 22 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## Git
### Added
- Added support for busybox sh
- Added flag --rcfile to specify an rc file by name.
- Added `extended-analysis=true` directive to enable/disable dataflow analysis
(with a corresponding --extended-analysis flag).
- SC2324: Warn when x+=1 appends instead of increments
- SC2325: Warn about multiple `!`s in dash/sh.
- SC2326: Warn about `foo | ! bar` in bash/dash/sh.
Expand All @@ -9,7 +13,6 @@
- SC3015: Warn bashism `test _ =~ _` like in [ ]
- SC3016: Warn bashism `test -v _` like in [ ]
- SC3017: Warn bashism `test -a _` like in [ ]
- Added support for busybox sh

### Fixed
- source statements with here docs now work correctly
Expand Down
13 changes: 13 additions & 0 deletions shellcheck.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ not warn at all, as `ksh` supports decimals in arithmetic contexts.
options are cumulative, but all the codes can be specified at once,
comma-separated as a single argument.

**--extended-analysis=true/false**

: Enable/disable Dataflow Analysis to identify more issues (default true). If
ShellCheck uses too much CPU/RAM when checking scripts with several
thousand lines of code, extended analysis can be disabled with this flag
or a directive. This flag overrides directives and rc files.

**-f** *FORMAT*, **--format=***FORMAT*

: Specify the output format of shellcheck, which prints its results in the
Expand Down Expand Up @@ -249,6 +256,12 @@ Valid keys are:
: Enable an optional check by name, as listed with **--list-optional**.
Only file-wide `enable` directives are considered.

**extended-analysis**
: Set to true/false to enable/disable dataflow analysis. Specifying
`# shellcheck extended-analysis=false` in particularly large (2000+ line)
auto-generated scripts will reduce ShellCheck's resource usage at the
expense of certain checks. Extended analysis is enabled by default.

**external-sources**
: Set to `true` in `.shellcheckrc` to always allow ShellCheck to open
arbitrary files from 'source' statements (the way most tools do).
Expand Down
18 changes: 18 additions & 0 deletions shellcheck.hs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ options = [
(ReqArg (Flag "include") "CODE1,CODE2..") "Consider only given types of warnings",
Option "e" ["exclude"]
(ReqArg (Flag "exclude") "CODE1,CODE2..") "Exclude types of warnings",
Option "" ["extended-analysis"]
(ReqArg (Flag "extended-analysis") "bool") "Perform dataflow analysis (default true)",
Option "f" ["format"]
(ReqArg (Flag "format") "FORMAT") $
"Output format (" ++ formatList ++ ")",
Expand Down Expand Up @@ -384,6 +386,14 @@ parseOption flag options =
}
}

Flag "extended-analysis" str -> do
value <- parseBool str
return options {
checkSpec = (checkSpec options) {
csExtendedAnalysis = Just value
}
}

-- This flag is handled specially in 'process'
Flag "format" _ -> return options

Expand All @@ -401,6 +411,14 @@ parseOption flag options =
throwError SyntaxFailure
return (Prelude.read num :: Integer)

parseBool str = do
case str of
"true" -> return True
"false" -> return False
_ -> do
printErr $ "Invalid boolean, expected true/false: " ++ str
throwError SyntaxFailure

ioInterface :: Options -> [FilePath] -> IO (SystemInterface IO)
ioInterface options files = do
inputs <- mapM normalize files
Expand Down
1 change: 1 addition & 0 deletions src/ShellCheck/AST.hs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ data Annotation =
| ShellOverride String
| SourcePath String
| ExternalSources Bool
| ExtendedAnalysis Bool
deriving (Show, Eq)
data ConditionType = DoubleBracket | SingleBracket deriving (Show, Eq)

Expand Down
6 changes: 6 additions & 0 deletions src/ShellCheck/ASTLib.hs
Original file line number Diff line number Diff line change
Expand Up @@ -910,5 +910,11 @@ getEnableDirectives root =
T_Annotation _ list _ -> [s | EnableComment s <- list]
_ -> []

getExtendedAnalysisDirective :: Token -> Maybe Bool
getExtendedAnalysisDirective root =
case root of
T_Annotation _ list _ -> listToMaybe $ [s | ExtendedAnalysis s <- list]
_ -> Nothing

return []
runTests = $quickCheckAll
21 changes: 13 additions & 8 deletions src/ShellCheck/Analytics.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,8 @@ checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do
str = concat $ oversimplify c
var = getBracedReference str
in fromMaybe False $ do
state <- CF.getIncomingState (cfgAnalysis params) id
cfga <- cfgAnalysis params
state <- CF.getIncomingState cfga id
value <- Map.lookup var $ CF.variablesInScope state
return $ CF.numericalStatus (CF.variableValue value) >= CF.NumericalStatusMaybe
_ ->
Expand Down Expand Up @@ -2143,7 +2144,8 @@ checkSpacefulnessCfg' dirtyPass params token@(T_DollarBraced id _ list) =
&& not (usedAsCommandName parents token)

isClean = fromMaybe False $ do
state <- CF.getIncomingState (cfgAnalysis params) id
cfga <- cfgAnalysis params
state <- CF.getIncomingState cfga id
value <- Map.lookup name $ CF.variablesInScope state
return $ isCleanState value

Expand Down Expand Up @@ -4896,7 +4898,8 @@ prop_checkCommandIsUnreachable3 = verifyNot checkCommandIsUnreachable "foo; bar
checkCommandIsUnreachable params t =
case t of
T_Pipeline {} -> sequence_ $ do
state <- CF.getIncomingState (cfgAnalysis params) id
cfga <- cfgAnalysis params
state <- CF.getIncomingState cfga 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)."
Expand All @@ -4918,14 +4921,15 @@ checkOverwrittenExitCode params t =
_ -> return ()
where
check id = sequence_ $ do
state <- CF.getIncomingState (cfgAnalysis params) id
cfga <- cfgAnalysis params
state <- CF.getIncomingState cfga id
let exitCodeIds = CF.exitCodes state
guard . not $ S.null exitCodeIds

let idToToken = idMap params
exitCodeTokens <- traverse (\k -> Map.lookup k idToToken) $ S.toList exitCodeIds
return $ do
when (all isCondition exitCodeTokens && not (usedUnconditionally t exitCodeIds)) $
when (all isCondition exitCodeTokens && not (usedUnconditionally cfga 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."
Expand All @@ -4938,8 +4942,8 @@ checkOverwrittenExitCode params t =

-- 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
usedUnconditionally cfga t testIds =
all (\c -> CF.doesPostDominate cfga (getId t) c) testIds

isPrinting t =
case getCommandBasename t of
Expand Down Expand Up @@ -5009,7 +5013,8 @@ prop_checkPlusEqualsNumber9 = verifyNot checkPlusEqualsNumber "declare -ia var;
checkPlusEqualsNumber params t =
case t of
T_Assignment id Append var _ word -> sequence_ $ do
state <- CF.getIncomingState (cfgAnalysis params) id
cfga <- cfgAnalysis params
state <- CF.getIncomingState cfga id
guard $ isNumber state word
guard . not $ fromMaybe False $ CF.variableMayBeDeclaredInteger state var
return $ warn id 2324 "var+=1 will append, not increment. Use (( var += 1 )), declare -i var, or quote number to silence."
Expand Down
8 changes: 6 additions & 2 deletions src/ShellCheck/AnalyzerLib.hs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ data Parameters = Parameters {
-- map from token id to start and end position
tokenPositions :: Map.Map Id (Position, Position),
-- Result from Control Flow Graph analysis (including data flow analysis)
cfgAnalysis :: CF.CFGAnalysis
cfgAnalysis :: Maybe CF.CFGAnalysis
} deriving (Show)

-- TODO: Cache results of common AST ops here
Expand Down Expand Up @@ -197,8 +197,10 @@ makeCommentWithFix severity id code str fix =
}
in force withFix

-- makeParameters :: CheckSpec -> Parameters
makeParameters spec = params
where
extendedAnalysis = fromMaybe True $ msum [asExtendedAnalysis spec, getExtendedAnalysisDirective root]
params = Parameters {
rootNode = root,
shellType = fromMaybe (determineShell (asFallbackShell spec) root) $ asShellType spec,
Expand Down Expand Up @@ -229,7 +231,9 @@ makeParameters spec = params
parentMap = getParentTree root,
variableFlow = getVariableFlow params root,
tokenPositions = asTokenPositions spec,
cfgAnalysis = CF.analyzeControlFlow cfParams root
cfgAnalysis = do
guard extendedAnalysis
return $ CF.analyzeControlFlow cfParams root
}
cfParams = CF.CFGParameters {
CF.cfLastpipe = hasLastpipe params,
Expand Down
40 changes: 40 additions & 0 deletions src/ShellCheck/Checker.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import ShellCheck.ASTLib
import ShellCheck.Interface
import ShellCheck.Parser

import Debug.Trace -- DO NOT SUBMIT
import Data.Either
import Data.Functor
import Data.List
Expand Down Expand Up @@ -86,6 +87,7 @@ checkScript sys spec = do
asCheckSourced = csCheckSourced spec,
asExecutionMode = Executed,
asTokenPositions = tokenPositions,
asExtendedAnalysis = csExtendedAnalysis spec,
asOptionalChecks = getEnableDirectives root ++ csOptionalChecks spec
} where as = newAnalysisSpec root
let analysisMessages =
Expand Down Expand Up @@ -520,5 +522,43 @@ prop_hereDocsWillHaveParsedIndices = null result
where
result = check "#!/bin/bash\nmy_array=(a b)\ncat <<EOF >> ./test\n $(( 1 + my_array[1] ))\nEOF"

prop_rcCanSuppressDfa = null result
where
result = checkWithRc "extended-analysis=false" emptyCheckSpec {
csScript = "#!/bin/sh\nexit; foo;"
}

prop_fileCanSuppressDfa = null $ traceShowId result
where
result = checkWithRc "" emptyCheckSpec {
csScript = "#!/bin/sh\n# shellcheck extended-analysis=false\nexit; foo;"
}

prop_fileWinsWhenSuppressingDfa1 = null result
where
result = checkWithRc "extended-analysis=true" emptyCheckSpec {
csScript = "#!/bin/sh\n# shellcheck extended-analysis=false\nexit; foo;"
}

prop_fileWinsWhenSuppressingDfa2 = result == [2317]
where
result = checkWithRc "extended-analysis=false" emptyCheckSpec {
csScript = "#!/bin/sh\n# shellcheck extended-analysis=true\nexit; foo;"
}

prop_flagWinsWhenSuppressingDfa1 = result == [2317]
where
result = checkWithRc "extended-analysis=false" emptyCheckSpec {
csScript = "#!/bin/sh\n# shellcheck extended-analysis=false\nexit; foo;",
csExtendedAnalysis = Just True
}

prop_flagWinsWhenSuppressingDfa2 = null result
where
result = checkWithRc "extended-analysis=true" emptyCheckSpec {
csScript = "#!/bin/sh\n# shellcheck extended-analysis=true\nexit; foo;",
csExtendedAnalysis = Just False
}

return []
runTests = $quickCheckAll
18 changes: 10 additions & 8 deletions src/ShellCheck/Checks/Commands.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1430,26 +1430,28 @@ prop_checkBackreferencingDeclaration6 = verify (checkBackreferencingDeclaration
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
check t = do
cfga <- asks cfgAnalysis
when (isJust cfga) $
foldM_ (perArg $ fromJust cfga) M.empty $ arguments t

perArg leftArgs t =
perArg cfga leftArgs t =
case t of
T_Assignment id _ name idx t -> do
warnIfBackreferencing leftArgs $ t:idx
warnIfBackreferencing cfga leftArgs $ t:idx
return $ M.insert name id leftArgs
t -> do
warnIfBackreferencing leftArgs [t]
warnIfBackreferencing cfga leftArgs [t]
return leftArgs

warnIfBackreferencing backrefs l = do
references <- findReferences l
warnIfBackreferencing cfga backrefs l = do
references <- findReferences cfga 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
findReferences cfga list = do
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
Expand Down
2 changes: 1 addition & 1 deletion src/ShellCheck/Checks/ControlFlow.hs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ controlFlowEffectChecks = [
runNodeChecks :: [ControlFlowNodeCheck] -> ControlFlowCheck
runNodeChecks perNode = do
cfg <- asks cfgAnalysis
runOnAll cfg
sequence_ $ runOnAll <$> cfg
where
getData datas n@(node, label) = do
(pre, post) <- M.lookup node datas
Expand Down
8 changes: 6 additions & 2 deletions src/ShellCheck/Interface.hs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
module ShellCheck.Interface
(
SystemInterface(..)
, CheckSpec(csFilename, csScript, csCheckSourced, csIncludedWarnings, csExcludedWarnings, csShellTypeOverride, csMinSeverity, csIgnoreRC, csOptionalChecks)
, CheckSpec(csFilename, csScript, csCheckSourced, csIncludedWarnings, csExcludedWarnings, csShellTypeOverride, csMinSeverity, csIgnoreRC, csExtendedAnalysis, csOptionalChecks)
, CheckResult(crFilename, crComments)
, ParseSpec(psFilename, psScript, psCheckSourced, psIgnoreRC, psShellTypeOverride)
, ParseResult(prComments, prTokenPositions, prRoot)
, AnalysisSpec(asScript, asShellType, asFallbackShell, asExecutionMode, asCheckSourced, asTokenPositions, asOptionalChecks)
, AnalysisSpec(asScript, asShellType, asFallbackShell, asExecutionMode, asCheckSourced, asTokenPositions, asExtendedAnalysis, asOptionalChecks)
, AnalysisResult(arComments)
, FormatterOptions(foColorOption, foWikiLinkCount)
, Shell(Ksh, Sh, Bash, Dash, BusyboxSh)
Expand Down Expand Up @@ -100,6 +100,7 @@ data CheckSpec = CheckSpec {
csIncludedWarnings :: Maybe [Integer],
csShellTypeOverride :: Maybe Shell,
csMinSeverity :: Severity,
csExtendedAnalysis :: Maybe Bool,
csOptionalChecks :: [String]
} deriving (Show, Eq)

Expand All @@ -124,6 +125,7 @@ emptyCheckSpec = CheckSpec {
csIncludedWarnings = Nothing,
csShellTypeOverride = Nothing,
csMinSeverity = StyleC,
csExtendedAnalysis = Nothing,
csOptionalChecks = []
}

Expand Down Expand Up @@ -174,6 +176,7 @@ data AnalysisSpec = AnalysisSpec {
asExecutionMode :: ExecutionMode,
asCheckSourced :: Bool,
asOptionalChecks :: [String],
asExtendedAnalysis :: Maybe Bool,
asTokenPositions :: Map.Map Id (Position, Position)
}

Expand All @@ -184,6 +187,7 @@ newAnalysisSpec token = AnalysisSpec {
asExecutionMode = Executed,
asCheckSourced = False,
asOptionalChecks = [],
asExtendedAnalysis = Nothing,
asTokenPositions = Map.empty
}

Expand Down
10 changes: 10 additions & 0 deletions src/ShellCheck/Parser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,16 @@ readAnnotationWithoutPrefix sandboxed = do
"This shell type is unknown. Use e.g. sh or bash."
return [ShellOverride shell]

"extended-analysis" -> do
pos <- getPosition
value <- plainOrQuoted $ many1 letter
case value of
"true" -> return [ExtendedAnalysis True]
"false" -> return [ExtendedAnalysis False]
_ -> do
parseNoteAt pos ErrorC 1146 "Unknown extended-analysis value. Expected true/false."
return []

"external-sources" -> do
pos <- getPosition
value <- plainOrQuoted $ many1 letter
Expand Down

0 comments on commit d80fdfa

Please sign in to comment.