From 2a8170ba05c35b2a31950b2c380e0ad6247121c9 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 15:01:57 -0400 Subject: [PATCH 01/16] Use force instead of reimplementing it --- src/ShellCheck/AnalyzerLib.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 453d53d..086bd7c 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -178,7 +178,7 @@ makeCommentWithFix severity id code str fix = withFix = comment { tcFix = Just fix } - in withFix `deepseq` withFix + in force withFix makeParameters spec = let params = Parameters { From b0dbc79f6973917aa51f1b9629e7a51a03162751 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 15:07:36 -0400 Subject: [PATCH 02/16] Remove unnecessary Maybe from isQuoteFreeElement --- src/ShellCheck/AnalyzerLib.hs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 086bd7c..0a4145a 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -293,15 +293,15 @@ isQuoteFree = isQuoteFreeNode False isQuoteFreeNode strict tree t = - (isQuoteFreeElement t == Just True) || + isQuoteFreeElement t || headOrDefault False (mapMaybe isQuoteFreeContext (drop 1 $ getPath tree t)) where -- Is this node self-quoting in itself? isQuoteFreeElement t = case t of - T_Assignment {} -> return True - T_FdRedirect {} -> return True - _ -> Nothing + T_Assignment {} -> True + T_FdRedirect {} -> True + _ -> False -- Are any subnodes inherently self-quoting? isQuoteFreeContext t = From b3c04ce3d0e0e95a0b3d9be9c58c18426d899d22 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 15:21:44 -0400 Subject: [PATCH 03/16] Implement findFirst in terms of foldr --- src/ShellCheck/AnalyzerLib.hs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 0a4145a..4348438 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -388,14 +388,13 @@ parents params = getPath (parentMap params) -- Find the first match in a list where the predicate is Just True. -- Stops if it's Just False and ignores Nothing. findFirst :: (a -> Maybe Bool) -> [a] -> Maybe a -findFirst p l = - case l of - [] -> Nothing - (x:xs) -> - case p x of - Just True -> return x - Just False -> Nothing - Nothing -> findFirst p xs +findFirst p = foldr go Nothing + where + go x acc = + case p x of + Just True -> return x + Just False -> Nothing + Nothing -> acc -- Check whether a word is entirely output from a single command tokenIsJustCommandOutput t = case t of From 14ee462ccd572c14ca292b47330e87c9307e0a7c Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 15:27:11 -0400 Subject: [PATCH 04/16] Use execState instead of reimplementing it --- src/ShellCheck/AnalyzerLib.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 4348438..8f168dc 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -409,8 +409,7 @@ tokenIsJustCommandOutput t = case t of -- TODO: Replace this with a proper Control Flow Graph getVariableFlow params t = - let (_, stack) = runState (doStackAnalysis startScope endScope t) [] - in reverse stack + reverse $ execState (doStackAnalysis startScope endScope t) [] where startScope t = let scopeType = leadType params t From f55d8c45e567398cfd9effcf08ad6d5706936268 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 15:30:28 -0400 Subject: [PATCH 05/16] Simplify causesSubshell --- src/ShellCheck/AnalyzerLib.hs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 8f168dc..fd0f46d 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -460,11 +460,9 @@ leadType params t = causesSubshell = do (T_Pipeline _ _ list) <- parentPipeline - if length list <= 1 - then return False - else if not $ hasLastpipe params - then return True - else return . not $ (getId . head $ reverse list) == getId t + return $ case list of + _:_:_ -> not (hasLastpipe params) || getId (last list) /= getId t + _ -> False getModifiedVariables t = case t of From f833ee3d5ada7cf4d648fb16ef143e09550b9fac Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 15:35:09 -0400 Subject: [PATCH 06/16] Use a list comprehension instead of a concatMap with extra lists --- src/ShellCheck/AnalyzerLib.hs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index fd0f46d..31ee431 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -467,11 +467,7 @@ leadType params t = getModifiedVariables t = case t of T_SimpleCommand _ vars [] -> - concatMap (\x -> case x of - T_Assignment id _ name _ w -> - [(x, x, name, dataTypeFrom DataString w)] - _ -> [] - ) vars + [(x, x, name, dataTypeFrom DataString w) | x@(T_Assignment id _ name _ w) <- vars] c@T_SimpleCommand {} -> getModifiedVariableCommand c From 67e091674ed35cb20b9053e99e4d38e3d68286e8 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 15:36:10 -0400 Subject: [PATCH 07/16] Remove unnecessary maybeToList The functions we use here are polymorphic enough to work in the [] monad, so there's no point to use them in the Maybe monad and then convert. --- src/ShellCheck/AnalyzerLib.hs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 31ee431..188054c 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -475,7 +475,7 @@ getModifiedVariables t = [(t, v, name, DataString $ SourceFrom [v])] TA_Unary _ "|++" v@(TA_Variable _ name _) -> [(t, v, name, DataString $ SourceFrom [v])] - TA_Assignment _ op (TA_Variable _ name _) rhs -> maybeToList $ do + TA_Assignment _ op (TA_Variable _ name _) rhs -> do guard $ op `elem` ["=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", "&=", "^=", "|="] return (t, t, name, DataString $ SourceFrom [rhs]) @@ -487,17 +487,17 @@ getModifiedVariables t = -- Count [[ -v foo ]] as an "assignment". -- This is to prevent [ -v foo ] being unassigned or unused. - TC_Unary id _ "-v" token -> maybeToList $ do + TC_Unary id _ "-v" token -> do str <- fmap (takeWhile (/= '[')) $ -- Quoted index flip getLiteralStringExt token $ \x -> case x of T_Glob _ s -> return s -- Unquoted index - _ -> Nothing + _ -> [] guard . not . null $ str return (t, token, str, DataString SourceChecked) - T_DollarBraced _ _ l -> maybeToList $ do + T_DollarBraced _ _ l -> do let string = bracedString t let modifier = getBracedModifier string guard $ any (`isPrefixOf` modifier) ["=", ":="] @@ -747,9 +747,9 @@ getReferencedVariables parents t = literalizer t = case t of T_Glob _ s -> return s -- Also when parsed as globs - _ -> Nothing + _ -> [] - getIfReference context token = maybeToList $ do + getIfReference context token = do str@(h:_) <- getLiteralStringExt literalizer token when (isDigit h) $ fail "is a number" return (context, token, getBracedReference str) From f109f9ab9270dd78bd8f8dc2158c9ce4904f599e Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 15:38:16 -0400 Subject: [PATCH 08/16] Remove unnecessary as-patterns --- src/ShellCheck/AnalyzerLib.hs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 188054c..543f6e6 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -468,8 +468,8 @@ getModifiedVariables t = case t of T_SimpleCommand _ vars [] -> [(x, x, name, dataTypeFrom DataString w) | x@(T_Assignment id _ name _ w) <- vars] - c@T_SimpleCommand {} -> - getModifiedVariableCommand c + T_SimpleCommand {} -> + getModifiedVariableCommand t TA_Unary _ "++|" v@(TA_Variable _ name _) -> [(t, v, name, DataString $ SourceFrom [v])] @@ -503,10 +503,10 @@ getModifiedVariables t = guard $ any (`isPrefixOf` modifier) ["=", ":="] return (t, t, getBracedReference string, DataString $ SourceFrom [l]) - t@(T_FdRedirect _ ('{':var) op) -> -- {foo}>&2 modifies foo + T_FdRedirect _ ('{':var) op -> -- {foo}>&2 modifies foo [(t, t, takeWhile (/= '}') var, DataString SourceInteger) | not $ isClosingFileOp op] - t@(T_CoProc _ name _) -> + T_CoProc _ name _ -> [(t, t, fromMaybe "COPROC" name, DataArray SourceInteger)] --Points to 'for' rather than variable @@ -730,7 +730,7 @@ getReferencedVariables parents t = (t, t, "output") ] - t@(T_FdRedirect _ ('{':var) op) -> -- {foo}>&- references and closes foo + T_FdRedirect _ ('{':var) op -> -- {foo}>&- references and closes foo [(t, t, takeWhile (/= '}') var) | isClosingFileOp op] x -> getReferencedVariableCommand x where From e4eb2d157f8211e14b66d0718fa4d90e3fefbd8a Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 16:03:49 -0400 Subject: [PATCH 09/16] Remove an unnecessary operator section --- src/ShellCheck/AnalyzerLib.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 543f6e6..4cd4b5e 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -564,7 +564,7 @@ getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T let params = map getLiteral rest readArrayVars = getReadArrayVariables rest in - catMaybes . (++ readArrayVars) . takeWhile isJust . reverse $ params + catMaybes $ takeWhile isJust (reverse params) ++ readArrayVars "getopts" -> case rest of opts:var:_ -> maybeToList $ getLiteral var From 2ebf522a52efe4ef092009f0db6da4cdd25edf5b Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 16:07:19 -0400 Subject: [PATCH 10/16] Simplify isArrayFlag --- src/ShellCheck/AnalyzerLib.hs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 4cd4b5e..8208a5b 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -669,12 +669,10 @@ getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T map (getLiteralArray . snd) (filter (isArrayFlag . fst) (zip args (tail args))) - isArrayFlag x = fromMaybe False $ do - str <- getLiteralString x - return $ case str of - '-':'-':_ -> False - '-':str -> 'a' `elem` str - _ -> False + isArrayFlag x = case getLiteralString x of + Just ('-':'-':_) -> False + Just ('-':str) -> 'a' `elem` str + _ -> False -- get the FLAGS_ variable created by a shflags DEFINE_ call getFlagVariable (n:v:_) = do From 4604066c3729766b49da0251786286b6c1ae8caa Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 16:16:12 -0400 Subject: [PATCH 11/16] Use head instead of (!! 0) --- src/ShellCheck/AnalyzerLib.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 8208a5b..c5eaea6 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -798,7 +798,7 @@ getVariablesFromLiteralToken token = prop_getVariablesFromLiteral1 = getVariablesFromLiteral "$foo${bar//a/b}$BAZ" == ["foo", "bar", "BAZ"] getVariablesFromLiteral string = - map (!! 0) $ matchAllSubgroups variableRegex string + map head $ matchAllSubgroups variableRegex string where variableRegex = mkRegex "\\$\\{?([A-Za-z0-9_]+)" From 1cf0aa25e9a76585f34798628cf7cc7d35642970 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 16:19:18 -0400 Subject: [PATCH 12/16] Simplify dropPrefix --- src/ShellCheck/AnalyzerLib.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index c5eaea6..26e65e6 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -820,8 +820,8 @@ getBracedReference s = fromMaybe s $ nameExpansion s `mplus` takeName noPrefix `mplus` getSpecial noPrefix `mplus` getSpecial s where noPrefix = dropPrefix s - dropPrefix (c:rest) = if c `elem` "!#" then rest else c:rest - dropPrefix "" = "" + dropPrefix (c:rest) | c `elem` "!#" = rest + dropPrefix cs = cs takeName s = do let name = takeWhile isVariableChar s guard . not $ null name From ca41440a678879d586320250a4b91913d1256a6e Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 16:21:07 -0400 Subject: [PATCH 13/16] Simplify getSpecial --- src/ShellCheck/AnalyzerLib.hs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 26e65e6..95b236a 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -826,9 +826,8 @@ getBracedReference s = fromMaybe s $ let name = takeWhile isVariableChar s guard . not $ null name return name - getSpecial (c:_) = - if c `elem` "*@#?-$!" then return [c] else fail "not special" - getSpecial _ = fail "empty" + getSpecial (c:_) | c `elem` "*@#?-$!" = return [c] + getSpecial _ = fail "empty or not special" nameExpansion ('!':rest) = do -- e.g. ${!foo*bar*} let suffix = dropWhile isVariableChar rest From 0cc5ed4563ccd87b200dd41c2801f8d4d79b8f15 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 16:25:43 -0400 Subject: [PATCH 14/16] Don't bother with asks if you're just immediately binding the result anyway --- src/ShellCheck/AnalyzerLib.hs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 95b236a..32de46a 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -354,8 +354,8 @@ getClosestCommand tree t = -- Like above, if koala_man knew Haskell when starting this project. getClosestCommandM t = do - tree <- asks parentMap - return $ getClosestCommand tree t + params <- ask + return $ getClosestCommand (parentMap params) t -- Is the token used as a command name (the first word in a T_SimpleCommand)? usedAsCommandName tree token = go (getId token) (tail $ getPath tree token) @@ -377,8 +377,8 @@ getPath tree t = t : -- Version of the above taking the map from the current context -- Todo: give this the name "getPath" getPathM t = do - map <- asks parentMap - return $ getPath map t + params <- ask + return $ getPath (parentMap params) t isParentOf tree parent child = elem (getId parent) . map getId $ getPath tree child @@ -866,8 +866,8 @@ headOrDefault def _ = def -- Run a command if the shell is in the given list whenShell l c = do - shell <- asks shellType - when (shell `elem` l ) c + params <- ask + when (shellType params `elem` l ) c filterByAnnotation asSpec params = From fb55072302c14d4785875357d4400de736f4eeb9 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 16:30:59 -0400 Subject: [PATCH 15/16] Implement supportsArrays with pattern-matching --- src/ShellCheck/AnalyzerLib.hs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 32de46a..a3fa29d 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -934,7 +934,9 @@ getOpts string flags = process flags more <- process rest2 return $ (flag1, token1) : more -supportsArrays shell = shell == Bash || shell == Ksh +supportsArrays Bash = True +supportsArrays Ksh = True +supportsArrays _ = False -- Returns true if the shell is Bash or Ksh (sorry for the name, Ksh) isBashLike :: Parameters -> Bool From d22e0aa4a79ddd8182c81898b790c9b9bb850937 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 16:38:52 -0400 Subject: [PATCH 16/16] Simplify process Note to self: This is a lot like foldr or traverse, and would be trivial to implement as such if it didn't need to peek ahead when takesArg is true. I wonder if there's a clean way to implement it in terms of one of them anyway. --- src/ShellCheck/AnalyzerLib.hs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index a3fa29d..7c2440b 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -919,20 +919,15 @@ getOpts string flags = process flags flagMap = Map.fromList $ ("", False) : flagList string process [] = return [] - process [(token, flag)] = do + process ((token1, flag):rest1) = do takesArg <- Map.lookup flag flagMap - guard $ not takesArg - return [(flag, token)] - process ((token1, flag1):rest2@((token2, flag2):rest)) = do - takesArg <- Map.lookup flag1 flagMap - if takesArg - then do - guard $ null flag2 - more <- process rest - return $ (flag1, token2) : more - else do - more <- process rest2 - return $ (flag1, token1) : more + (token, rest) <- if takesArg + then case rest1 of + (token2, ""):rest2 -> return (token2, rest2) + _ -> fail "takesArg without valid arg" + else return (token1, rest1) + more <- process rest + return $ (flag, token) : more supportsArrays Bash = True supportsArrays Ksh = True