Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Chromium OS fork's Portage Ebuild changes #2704

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion ShellCheck.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ library
directory >= 1.2.3 && < 1.4,

-- When cabal supports it, move this to setup-depends:
process
process,

-- support for scanning Gentoo eclasses
attoparsec,
text
exposed-modules:
ShellCheck.AST
ShellCheck.ASTLib
Expand All @@ -93,6 +97,7 @@ library
ShellCheck.Formatter.Quiet
ShellCheck.Interface
ShellCheck.Parser
ShellCheck.PortageVariables
ShellCheck.Prelude
ShellCheck.Regex
other-modules:
Expand Down
5 changes: 3 additions & 2 deletions shellcheck.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ not warn at all, as `ksh` supports decimals in arithmetic contexts.

: Specify Bourne shell dialect. Valid values are *sh*, *bash*, *dash* and *ksh*.
The default is to deduce the shell from the file's `shell` directive,
shebang, or `.bash/.bats/.dash/.ksh` extension, in that order. *sh* refers to
POSIX `sh` (not the system's), and will warn of portability issues.
shebang, or `.bash/.bats/.dash/.ksh/.ebuild/.eclass` extension, in that
order. *sh* refers to POSIX `sh` (not the system's), and will warn of
portability issues.

**-S**\ *SEVERITY*,\ **--severity=***severity*

Expand Down
15 changes: 14 additions & 1 deletion shellcheck.hs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import qualified ShellCheck.Analyzer
import ShellCheck.Checker
import ShellCheck.Data
import ShellCheck.Interface
import ShellCheck.PortageVariables
import ShellCheck.Regex

import qualified ShellCheck.Formatter.CheckStyle
Expand Down Expand Up @@ -240,10 +241,22 @@ runFormatter sys format options files = do
either (reportFailure filename) check input
where
check contents = do

-- If this is a Gentoo ebuild file, scan for eclasses on the system
gentooData <- case getPortageFileType filename of
NonPortageRelated -> pure Map.empty
_ -> catch (portageVariables <$> scanRepos) $ \e -> do
let warnMsg = "Error when scanning for Gentoo repos: "
let err = show (e :: IOException)
hPutStr stderr ("Warning: " ++ warnMsg ++ err)
pure Map.empty

let checkspec = (checkSpec options) {
csFilename = filename,
csScript = contents
csScript = contents,
csGentooData = gentooData
}

result <- checkScript sys checkspec
onResult format result sys
return $
Expand Down
20 changes: 18 additions & 2 deletions src/ShellCheck/ASTLib.hs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ import Numeric (showHex)

import Test.QuickCheck

arguments (T_SimpleCommand _ _ (cmd:args)) = args

-- Is this a type of loop?
isLoop t = case t of
T_WhileExpression {} -> True
Expand Down Expand Up @@ -559,11 +557,29 @@ getCommandNameFromExpansion t =
extract (T_Pipeline _ _ [cmd]) = getCommandName cmd
extract _ = Nothing

-- If a command substitution is a single command, get its argument Tokens.
-- Return an empty list if there are no arguments or the token is not a command substitution.
-- $(date +%s) = ["+%s"]
getArgumentsFromExpansion :: Token -> [Token]
getArgumentsFromExpansion t =
case t of
T_DollarExpansion _ [c] -> extract c
T_Backticked _ [c] -> extract c
T_DollarBraceCommandExpansion _ [c] -> extract c
_ -> []
where
extract (T_Pipeline _ _ [cmd]) = arguments cmd
extract _ = []

-- Get the basename of a token representing a command
getCommandBasename = fmap basename . getCommandName

basename = reverse . takeWhile (/= '/') . reverse

-- Get the arguments to a command
arguments (T_SimpleCommand _ _ (cmd:args)) = args
arguments t = maybe [] arguments (getCommand t)

isAssignment t =
case t of
T_Redirecting _ _ w -> isAssignment w
Expand Down
165 changes: 160 additions & 5 deletions src/ShellCheck/Analytics.hs

Large diffs are not rendered by default.

43 changes: 43 additions & 0 deletions src/ShellCheck/AnalyzerLib.hs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import qualified ShellCheck.CFGAnalysis as CF
import ShellCheck.Data
import ShellCheck.Interface
import ShellCheck.Parser
import ShellCheck.PortageVariables
import ShellCheck.Prelude
import ShellCheck.Regex

Expand Down Expand Up @@ -102,10 +103,20 @@ data Parameters = Parameters {
rootNode :: Token,
-- map from token id to start and end position
tokenPositions :: Map.Map Id (Position, Position),
-- detailed type of any Portage related file
portageFileType :: PortageFileType,
-- Gentoo-specific data
gentooData :: EclassMap,
-- Result from Control Flow Graph analysis (including data flow analysis)
cfgAnalysis :: CF.CFGAnalysis
} deriving (Show)

isPortageBuild :: Parameters -> Bool
isPortageBuild params = portageFileType params /= NonPortageRelated

isPortage9999Ebuild :: Parameters -> Bool
isPortage9999Ebuild params = portageFileType params == Ebuild { is9999Ebuild = True }

-- TODO: Cache results of common AST ops here
data Cache = Cache {}

Expand Down Expand Up @@ -146,6 +157,15 @@ pScript s =
}
in runIdentity $ parseScript (mockedSystemInterface []) pSpec

-- For testing. Tries to construct Parameters from a test script allowing for
-- alterations to the AnalysisSpec.
makeTestParams :: String -> (AnalysisSpec -> AnalysisSpec) -> Maybe Parameters
makeTestParams s specModifier = do
let pr = pScript s
prRoot pr
let spec = specModifier $ defaultSpec pr
return $ makeParameters spec

-- For testing. If parsed, returns whether there are any comments
producesComments :: Checker -> String -> Maybe Bool
producesComments c s = do
Expand Down Expand Up @@ -225,6 +245,8 @@ makeParameters spec = params
parentMap = getParentTree root,
variableFlow = getVariableFlow params root,
tokenPositions = asTokenPositions spec,
portageFileType = asPortageFileType spec,
gentooData = asGentooData spec,
cfgAnalysis = CF.analyzeControlFlow cfParams root
}
cfParams = CF.CFGParameters {
Expand Down Expand Up @@ -582,6 +604,15 @@ getReferencedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Litera
head:_ -> map (\x -> (base, head, x)) $ getVariablesFromLiteralToken head
_ -> []
"alias" -> [(base, token, name) | token <- rest, name <- getVariablesFromLiteralToken token]

-- tc-export makes a list of toolchain variables available, similar to export.
-- Usage tc-export CC CXX
"tc-export" -> concatMap getReference rest

-- tc-export_build_env exports the listed variables plus a bunch of BUILD_XX variables.
-- Usage tc-export_build_env BUILD_CC
"tc-export_build_env" -> concatMap getReference rest ++ concatMap buildVarReferences portageBuildFlagVariables

_ -> []
where
forDeclare =
Expand All @@ -595,6 +626,7 @@ getReferencedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Litera
getReference t@(T_NormalWord _ [T_Literal _ name]) | not ("-" `isPrefixOf` name) = [(t, t, name)]
getReference _ = []
flags = map snd $ getAllFlags base
buildVarReferences var = [(base, base, "BUILD_" ++ var), (base, base, var ++ "_FOR_BUILD")]

getReferencedVariableCommand _ = []

Expand Down Expand Up @@ -653,6 +685,13 @@ getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T
"DEFINE_integer" -> maybeToList $ getFlagVariable rest
"DEFINE_string" -> maybeToList $ getFlagVariable rest

-- tc-export creates all the variables passed to it
"tc-export" -> concatMap getModifierParamString rest

-- tc-export_build_env creates all the variables passed to it
-- plus several BUILD_ and _FOR_BUILD variables.
"tc-export_build_env" -> concatMap getModifierParamString rest ++ getBuildEnvTokens

_ -> []
where
flags = map snd $ getAllFlags base
Expand Down Expand Up @@ -743,6 +782,10 @@ getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T
return (base, n, "FLAGS_" ++ name, DataString $ SourceExternal)
getFlagVariable _ = Nothing

getBuildEnvTokens = concatMap buildVarTokens portageBuildFlagVariables
buildVarTokens var = [(base, base, "BUILD_" ++ var, DataString $ SourceExternal),
(base, base, var ++ "_FOR_BUILD", DataString $ SourceExternal)]

getModifiedVariableCommand _ = []

-- Given a NormalWord like foo or foo[$bar], get foo.
Expand Down
2 changes: 1 addition & 1 deletion src/ShellCheck/CFGAnalysis.hs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ unreachableState = modified newInternalState {
createEnvironmentState :: InternalState
createEnvironmentState = do
foldl' (flip ($)) newInternalState $ concat [
addVars Data.internalVariables unknownVariableState,
addVars Data.genericInternalVariables unknownVariableState,
addVars Data.variablesWithoutSpaces spacelessVariableState,
addVars Data.specialIntegerVariables integerVariableState
]
Expand Down
8 changes: 6 additions & 2 deletions src/ShellCheck/Checker.hs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ shellFromFilename filename = listToMaybe candidates
shellExtensions = [(".ksh", Ksh)
,(".bash", Bash)
,(".bats", Bash)
,(".dash", Dash)]
,(".dash", Dash)
,(".ebuild", Bash)
,(".eclass", Bash)]
-- The `.sh` is too generic to determine the shell:
-- We fallback to Bash in this case and emit SC2148 if there is no shebang
candidates =
Expand Down Expand Up @@ -86,7 +88,9 @@ checkScript sys spec = do
asCheckSourced = csCheckSourced spec,
asExecutionMode = Executed,
asTokenPositions = tokenPositions,
asOptionalChecks = getEnableDirectives root ++ csOptionalChecks spec
asOptionalChecks = getEnableDirectives root ++ csOptionalChecks spec,
asPortageFileType = getPortageFileType $ csFilename spec,
asGentooData = csGentooData spec
} where as = newAnalysisSpec root
let analysisMessages =
maybe []
Expand Down
72 changes: 51 additions & 21 deletions src/ShellCheck/Checks/Commands.hs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,32 @@ verify :: CommandCheck -> String -> Bool
verify f s = producesComments (getChecker [f]) s == Just True
verifyNot f s = producesComments (getChecker [f]) s == Just False

commandChecks :: [CommandCheck]
commandChecks = [
verifyDisabledCheckerInPortage :: String -> Bool
verifyDisabledCheckerInPortage = verifyDisabledCheckerInPortage2 $
Ebuild { is9999Ebuild = True }

verifyDisabledCheckerInPortage2 :: PortageFileType -> String -> Bool
verifyDisabledCheckerInPortage2 portageFileType s = fromMaybe False $ do
params <- makeTestParams s portageTypeSpec
testSpec <- makeTestSpec
return $ null $ runChecker params (checker testSpec params)
where
portageTypeSpec spec = spec {
asPortageFileType = portageFileType
}
makeTestSpec = do
let pr = pScript s
prRoot pr
return $ portageTypeSpec $ defaultSpec pr


commandCheckWhen :: Bool -> CommandCheck -> CommandCheck
commandCheckWhen predicate commandCheck = if predicate
then commandCheck
else CommandCheck (Exactly "skipped") nullCheck

commandChecks :: Parameters -> [CommandCheck]
commandChecks params = [
checkTr
,checkFindNameGlob
,checkExpr
Expand All @@ -84,15 +108,15 @@ commandChecks = [
,checkAliasesUsesArgs
,checkAliasesExpandEarly
,checkUnsetGlobs
,checkFindWithoutPath
,commandCheckWhen (not $ isPortageBuild params) checkFindWithoutPath
,checkTimeParameters
,checkTimedCommand
,checkLocalScope
,checkDeprecatedTempfile
,checkDeprecatedEgrep
,checkDeprecatedFgrep
,checkWhileGetoptsCase
,checkCatastrophicRm
,checkCatastrophicRm (isPortageBuild params)
,checkLetUsage
,checkMvArguments, checkCpArguments, checkLnArguments
,checkFindRedirections
Expand Down Expand Up @@ -206,7 +230,7 @@ getChecker list = Checker {


checker :: AnalysisSpec -> Parameters -> Checker
checker spec params = getChecker $ commandChecks ++ optionals
checker spec params = getChecker $ (commandChecks params) ++ optionals
where
keys = asOptionalChecks spec
optionals =
Expand Down Expand Up @@ -893,6 +917,7 @@ prop_checkFindWithoutPath5 = verifyNot checkFindWithoutPath "find -O3 ."
prop_checkFindWithoutPath6 = verifyNot checkFindWithoutPath "find -D exec ."
prop_checkFindWithoutPath7 = verifyNot checkFindWithoutPath "find --help"
prop_checkFindWithoutPath8 = verifyNot checkFindWithoutPath "find -Hx . -print"
prop_checkFindWithoutPathPortage = verifyDisabledCheckerInPortage "find -type f"
checkFindWithoutPath = CommandCheck (Basename "find") f
where
f t@(T_SimpleCommand _ _ (cmd:args)) =
Expand Down Expand Up @@ -1071,20 +1096,23 @@ checkWhileGetoptsCase = CommandCheck (Exactly "getopts") f
T_Redirecting _ _ x@(T_CaseExpression {}) -> return x
_ -> Nothing

prop_checkCatastrophicRm1 = verify checkCatastrophicRm "rm -r $1/$2"
prop_checkCatastrophicRm2 = verify checkCatastrophicRm "rm -r /home/$foo"
prop_checkCatastrophicRm3 = verifyNot checkCatastrophicRm "rm -r /home/${USER:?}/*"
prop_checkCatastrophicRm4 = verify checkCatastrophicRm "rm -fr /home/$(whoami)/*"
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_checkCatastrophicRmA = verify checkCatastrophicRm "rm -rf /usr /lib/nvidia-current/xorg/xorg"
prop_checkCatastrophicRmB = verify checkCatastrophicRm "rm -rf \"$STEAMROOT/\"*"
checkCatastrophicRm = CommandCheck (Basename "rm") $ \t ->
prop_checkCatastrophicRm1 = verify (checkCatastrophicRm False) "rm -r $1/$2"
prop_checkCatastrophicRm2 = verify (checkCatastrophicRm False) "rm -r /home/$foo"
prop_checkCatastrophicRm3 = verifyNot (checkCatastrophicRm False) "rm -r /home/${USER:?}/*"
prop_checkCatastrophicRm4 = verify (checkCatastrophicRm False) "rm -fr /home/$(whoami)/*"
prop_checkCatastrophicRm5 = verifyNot (checkCatastrophicRm False) "rm -r /home/${USER:-thing}/*"
prop_checkCatastrophicRm6 = verify (checkCatastrophicRm False) "rm --recursive /etc/*$config*"
prop_checkCatastrophicRm8 = verify (checkCatastrophicRm False) "rm -rf /home"
prop_checkCatastrophicRm10 = verifyNot (checkCatastrophicRm False) "rm -r \"${DIR}\"/{.gitignore,.gitattributes,ci}"
prop_checkCatastrophicRm11 = verify (checkCatastrophicRm False) "rm -r /{bin,sbin}/$exec"
prop_checkCatastrophicRm12 = verify (checkCatastrophicRm False) "rm -r /{{usr,},{bin,sbin}}/$exec"
prop_checkCatastrophicRm13 = verifyNot (checkCatastrophicRm False) "rm -r /{{a,b},{c,d}}/$exec"
prop_checkCatastrophicRmA = verify (checkCatastrophicRm False) "rm -rf /usr /lib/nvidia-current/xorg/xorg"
prop_checkCatastrophicRmB = verify (checkCatastrophicRm False) "rm -rf \"$STEAMROOT/\"*"
prop_checkCatastrophicRmED1 = verify (checkCatastrophicRm False) "rm -rf \"$ED/var/\"*"
prop_checkCatastrophicRmED2 = verifyNot (checkCatastrophicRm True) "rm -rf \"$ED/var/\"*"
checkCatastrophicRm isPortageBuild = CommandCheck (Basename "rm") $ \t ->

when (isRecursive t) $
mapM_ (mapM_ checkWord . braceExpand) $ arguments t
where
Expand Down Expand Up @@ -1114,7 +1142,7 @@ checkCatastrophicRm = CommandCheck (Basename "rm") $ \t ->
f (T_DollarBraced _ _ word) =
let var = onlyLiteralString word in
-- This shouldn't handle non-colon cases.
if any (`isInfixOf` var) [":?", ":-", ":="]
if any (`isInfixOf` var) [":?", ":-", ":="] || (isPortageBuild && var `elem` ["D", "ED"])
then Nothing
else return ""
f _ = return ""
Expand Down Expand Up @@ -1339,6 +1367,7 @@ checkMaskedReturns str = CommandCheck (Exactly str) checkCmd
checkCmd t = do
path <- getPathM t
shell <- asks shellType
portageFileType <- asks portageFileType
sequence_ $ do
name <- getCommandName t

Expand All @@ -1349,10 +1378,11 @@ checkMaskedReturns str = CommandCheck (Exactly str) checkCmd

let isLocal = not hasDashG && isLocalInFunction name && isInScopedFunction
let isReadOnly = name == "readonly" || hasDashR
let isPortageBuild = portageFileType /= NonPortageRelated

-- Don't warn about local variables that are declared readonly,
-- because the workaround `local x; x=$(false); local -r x;` is annoying
guard . not $ isLocal && isReadOnly
guard . not $ isLocal && isReadOnly || isPortageBuild

return $ mapM_ checkArgs $ arguments t

Expand Down