From 0e0de940450425891d1f5034f761bede1afe6559 Mon Sep 17 00:00:00 2001
From: Tito Sacchi <tito.sakki@gmail.com>
Date: Thu, 31 Oct 2019 17:34:10 +0100
Subject: [PATCH 1/3] Fix issue #1724
(bash: missing support for 'builtin' keyword)
Now shellcheck looks for the arguments to 'builtin' to determine
read/written variables. A change in the parser makes sure that
assignments are parsed correctly in commands that start with 'builtin'.
---
CHANGELOG.md | 1 +
src/ShellCheck/Analytics.hs | 1 +
src/ShellCheck/AnalyzerLib.hs | 4 +++-
src/ShellCheck/Parser.hs | 10 +++++++++-
4 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 48f960f..79ca5f3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
## v0.7.1 - soon
### Fixed
- `-f diff` no longer claims that it found more issues when it didn't
+- SC2154 triggers for builtins called with `builtin`
### Added
- SC2254: Suggest quoting expansions in case statements
diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
index 221caa1..4448796 100644
--- a/src/ShellCheck/Analytics.hs
+++ b/src/ShellCheck/Analytics.hs
@@ -2168,6 +2168,7 @@ prop_checkUnassignedReferences35= verifyNotTree checkUnassignedReferences "echo
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"
checkUnassignedReferences = checkUnassignedReferences' False
checkUnassignedReferences' includeGlobals params t = warnings
diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs
index 7803fdf..590889c 100644
--- a/src/ShellCheck/AnalyzerLib.hs
+++ b/src/ShellCheck/AnalyzerLib.hs
@@ -567,9 +567,11 @@ getReferencedVariableCommand _ = []
-- VariableName :: String, -- The variable name, i.e. foo
-- VariableValue :: DataType -- A description of the value being assigned, i.e. "Literal string with value foo"
-- )
-getModifiedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal _ x:_):rest)) =
+getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T_Literal _ x:_):rest)) =
filter (\(_,_,s,_) -> not ("-" `isPrefixOf` s)) $
case x of
+ "builtin" ->
+ getModifiedVariableCommand $ T_SimpleCommand id cmdPrefix rest
"read" ->
let params = map getLiteral rest
readArrayVars = getReadArrayVariables rest
diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs
index 075d486..e42762b 100644
--- a/src/ShellCheck/Parser.hs
+++ b/src/ShellCheck/Parser.hs
@@ -246,6 +246,10 @@ addParseNote n = do
parseNotes = n : parseNotes state
}
+ignoreProblemsOf p = do
+ systemState <- lift . lift $ Ms.get
+ p <* (lift . lift . Ms.put $ systemState)
+
shouldIgnoreCode code = do
context <- getCurrentContexts
checkSourced <- Mr.asks checkSourced
@@ -2041,7 +2045,11 @@ readSimpleCommand = called "simple command" $ do
Just cmd -> do
validateCommand cmd
- suffix <- option [] $ getParser readCmdSuffix cmd [
+ -- We have to ignore possible parsing problems from the lookAhead parser
+ firstArgument <- ignoreProblemsOf . optionMaybe . try . lookAhead $ readCmdWord
+ suffix <- option [] $ getParser readCmdSuffix
+ -- If `export` or other modifier commands are called with `builtin` we have to look at the first argument
+ (if isCommand ["builtin"] cmd && isJust firstArgument then fromJust firstArgument else cmd) [
(["declare", "export", "local", "readonly", "typeset"], readModifierSuffix),
(["time"], readTimeSuffix),
(["let"], readLetSuffix),
From 84ca7711c45da3083b18a5643460b620380ac143 Mon Sep 17 00:00:00 2001
From: Tito Sacchi <tito.sakki@gmail.com>
Date: Fri, 1 Nov 2019 14:28:00 +0100
Subject: [PATCH 2/3] Make command-specific checks act on `builtin ...`
Now if shellchecks encounters a command like `builtin cmd ...`
it applies the same check that would be applied to `cmd ...`.
---
src/ShellCheck/Checks/Commands.hs | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs
index 441e43f..4841ddd 100644
--- a/src/ShellCheck/Checks/Commands.hs
+++ b/src/ShellCheck/Checks/Commands.hs
@@ -105,12 +105,16 @@ buildCommandMap = foldl' addCheck Map.empty
checkCommand :: Map.Map CommandName (Token -> Analysis) -> Token -> Analysis
-checkCommand map t@(T_SimpleCommand id _ (cmd:rest)) = fromMaybe (return ()) $ do
+checkCommand map t@(T_SimpleCommand id cmdPrefix (cmd:rest)) = fromMaybe (return ()) $ do
name <- getLiteralString cmd
return $
if '/' `elem` name
then
Map.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'
else do
Map.findWithDefault nullCheck (Exactly name) map t
Map.findWithDefault nullCheck (Basename name) map t
From 5becc673b2477b98073e583fe505c4d68e4b945f Mon Sep 17 00:00:00 2001
From: Tito Sacchi <tito.sakki@gmail.com>
Date: Fri, 1 Nov 2019 14:36:15 +0100
Subject: [PATCH 3/3] Modify CHANGELOG.md
---
CHANGELOG.md | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 79ca5f3..8b9aae5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,8 @@
## v0.7.1 - soon
### Fixed
- `-f diff` no longer claims that it found more issues when it didn't
-- SC2154 triggers for builtins called with `builtin`
+- SC2154 and all command-specific checks now trigger for builtins
+ called with `builtin`
### Added
- SC2254: Suggest quoting expansions in case statements