From da7b28213ef65ef4a0d702831d1d358f37e4f114 Mon Sep 17 00:00:00 2001 From: Vidar Holen Date: Tue, 17 Aug 2021 21:53:27 -0700 Subject: [PATCH] Recognize wait -p as assigning a variable (fixes #2179) --- src/ShellCheck/ASTLib.hs | 33 +++++++++++++++++++++++++++++++ src/ShellCheck/Analytics.hs | 2 ++ src/ShellCheck/AnalyzerLib.hs | 19 +++++++++--------- src/ShellCheck/Checks/Commands.hs | 12 +++++++++++ 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/src/ShellCheck/ASTLib.hs b/src/ShellCheck/ASTLib.hs index 019fc3c..7b5efdd 100644 --- a/src/ShellCheck/ASTLib.hs +++ b/src/ShellCheck/ASTLib.hs @@ -228,6 +228,39 @@ getOpts (gnu, arbitraryLongOpts) string longopts args = process args listToArgs = map (\x -> ("", (x, x))) + +-- Generic getOpts that doesn't rely on a format string, but may also be inaccurate. +-- This provides a best guess interpretation instead of failing when new options are added. +-- +-- "--" is treated as end of arguments +-- "--anything[=foo]" is treated as a long option without argument +-- "-any" is treated as -a -n -y, with the next arg as an option to -y unless it starts with - +-- anything else is an argument +getGenericOpts :: [Token] -> [(String, (Token, Token))] +getGenericOpts = process + where + process (token:rest) = + case getLiteralStringDef "\0" token of + "--" -> map (\c -> ("", (c,c))) rest + '-':'-':word -> (takeWhile (`notElem` "\0=") word, (token, token)) : process rest + '-':optString -> + let opts = takeWhile (/= '\0') optString + in + case rest of + next:_ | "-" `isPrefixOf` getLiteralStringDef "\0" next -> + map (\c -> ([c], (token, token))) opts ++ process rest + next:remainder -> + case reverse opts of + last:initial -> + map (\c -> ([c], (token, token))) (reverse initial) + ++ [([last], (token, next))] + ++ process remainder + [] -> process remainder + [] -> map (\c -> ([c], (token, token))) opts + _ -> ("", (token, token)) : process rest + process [] = [] + + -- Is this an expansion of multiple items of an array? isArrayExpansion (T_DollarBraced _ _ l) = let string = concat $ oversimplify l in diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 9aff3ab..e1e55fd 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -1968,6 +1968,7 @@ 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" data SpaceStatus = SpaceSome | SpaceNone | SpaceEmpty deriving (Eq) instance Semigroup SpaceStatus where @@ -2381,6 +2382,7 @@ prop_checkUnassignedReferences_minusNDefault = verifyNotTree checkUnassignedRefe prop_checkUnassignedReferences_minusZDefault = verifyNotTree checkUnassignedReferences "if [ -z \"${x:-}\" ]; then echo \"\"; fi" prop_checkUnassignedReferences50 = verifyNotTree checkUnassignedReferences "echo ${foo:+bar}" prop_checkUnassignedReferences51 = verifyNotTree checkUnassignedReferences "echo ${foo:+$foo}" +prop_checkUnassignedReferences52 = verifyNotTree checkUnassignedReferences "wait -p pid; echo $pid" checkUnassignedReferences = checkUnassignedReferences' False checkUnassignedReferences' includeGlobals params t = warnings diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 9ae0160..633543a 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -617,6 +617,7 @@ getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T return (base, base, "@", DataString $ SourceFrom params) "printf" -> maybeToList $ getPrintfVariable rest + "wait" -> maybeToList $ getWaitVariable rest "mapfile" -> maybeToList $ getMapfileArray base rest "readarray" -> maybeToList $ getMapfileArray base rest @@ -674,15 +675,15 @@ getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T _ -> return (t:fromMaybe [] (getSetParams rest)) getSetParams [] = Nothing - getPrintfVariable list = f $ map (\x -> (x, getLiteralString x)) list - where - f ((_, Just "-v") : (t, Just var) : _) = return (base, t, varName, varType $ SourceFrom list) - where - (varName, varType) = case elemIndex '[' var of - Just i -> (take i var, DataArray) - Nothing -> (var, DataString) - f (_:rest) = f rest - f [] = fail "not found" + getPrintfVariable list = getFlagAssignedVariable "v" (SourceFrom list) $ getBsdOpts "v:" list + getWaitVariable list = getFlagAssignedVariable "p" SourceInteger $ return $ getGenericOpts list + + getFlagAssignedVariable str dataSource maybeFlags = do + flags <- maybeFlags + (_, (flag, value)) <- find ((== str) . fst) flags + variableName <- getLiteralStringExt (const $ return "!") value + let (baseName, index) = span (/= '[') variableName + return (base, value, baseName, (if null index then DataString else DataArray) dataSource) -- mapfile has some curious syntax allowing flags plus 0..n variable names -- where only the first non-option one is used if any. diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index c0cd9a3..6c7bdc5 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -138,18 +138,30 @@ prop_checkGetOptsS3 = checkGetOpts "-f -x" ["f", "x"] [] $ getOpts (True, True) prop_checkGetOptsS4 = checkGetOpts "-f -x" ["f"] [] $ getOpts (True, True) "f:" [] prop_checkGetOptsS5 = checkGetOpts "-fx" [] [] $ getOpts (True, True) "fx:" [] +prop_checkGenericOptsS1 = checkGetOpts "-f x" ["f"] [] $ return . getGenericOpts +prop_checkGenericOptsS2 = checkGetOpts "-abc x" ["a", "b", "c"] [] $ return . getGenericOpts +prop_checkGenericOptsS3 = checkGetOpts "-abc -x" ["a", "b", "c", "x"] [] $ return . getGenericOpts +prop_checkGenericOptsS4 = checkGetOpts "-x" ["x"] [] $ return . getGenericOpts + -- Long options prop_checkGetOptsL1 = checkGetOpts "--foo=bar baz" ["foo"] ["baz"] $ getOpts (True, False) "" [("foo", True)] prop_checkGetOptsL2 = checkGetOpts "--foo bar baz" ["foo"] ["baz"] $ getOpts (True, False) "" [("foo", True)] prop_checkGetOptsL3 = checkGetOpts "--foo baz" ["foo"] ["baz"] $ getOpts (True, True) "" [] prop_checkGetOptsL4 = checkGetOpts "--foo baz" [] [] $ getOpts (True, False) "" [] +prop_checkGenericOptsL1 = checkGetOpts "--foo=bar" ["foo"] [] $ return . getGenericOpts +prop_checkGenericOptsL2 = checkGetOpts "--foo bar" ["foo"] ["bar"] $ return . getGenericOpts +prop_checkGenericOptsL3 = checkGetOpts "-x --foo" ["x", "foo"] [] $ return . getGenericOpts + -- Know when to terminate prop_checkGetOptsT1 = checkGetOpts "-a x -b" ["a", "b"] ["x"] $ getOpts (True, True) "ab" [] prop_checkGetOptsT2 = checkGetOpts "-a x -b" ["a"] ["x","-b"] $ getOpts (False, True) "ab" [] prop_checkGetOptsT3 = checkGetOpts "-a -- -b" ["a"] ["-b"] $ getOpts (True, True) "ab" [] prop_checkGetOptsT4 = checkGetOpts "-a -- -b" ["a", "b"] [] $ getOpts (True, True) "a:b" [] +prop_checkGenericOptsT1 = checkGetOpts "-x -- -y" ["x"] ["-y"] $ return . getGenericOpts +prop_checkGenericOptsT2 = checkGetOpts "-xy --" ["x", "y"] [] $ return . getGenericOpts + buildCommandMap :: [CommandCheck] -> Map.Map CommandName (Token -> Analysis) buildCommandMap = foldl' addCheck Map.empty