From 2e52c2b56a2bccb2f06c90a081e27f0fe35cf814 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:11 -0500 Subject: [PATCH 01/17] Use notElem instead of not on the result of elem --- src/ShellCheck/Analytics.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 582b8cd..64a71a3 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -3169,7 +3169,7 @@ checkSplittingInArrays params t = T_DollarBraced id _ str | not (isCountingReference part) && not (isQuotedAlternativeReference part) - && not (getBracedReference (bracedString part) `elem` variablesWithoutSpaces) + && getBracedReference (bracedString part) `notElem` variablesWithoutSpaces -> warn id 2206 $ if shellType params == Ksh then "Quote to prevent word splitting/globbing, or split robustly with read -A or while read." From 3449e6be215dc3d2ec05032dcfc282897a49e836 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:13 -0500 Subject: [PATCH 02/17] Get rid of our getOpt, as it already exists as lookup --- src/ShellCheck/Analytics.hs | 2 +- src/ShellCheck/AnalyzerLib.hs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 64a71a3..e8bee4f 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -2840,7 +2840,7 @@ checkReadWithoutR _ t@T_SimpleCommand {} | t `isUnqualifiedCommand` "read" = flags = getAllFlags t has_t0 = fromMaybe False $ do parsed <- getOpts flagsForRead flags - t <- getOpt "t" parsed + t <- lookup "t" parsed str <- getLiteralString t return $ str == "0" diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 590889c..e93758f 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -960,8 +960,6 @@ getOpts string flags = process flags more <- process rest2 return $ (flag1, token1) : more -getOpt str flags = snd <$> (listToMaybe $ filter (\(f, _) -> f == str) $ flags) - supportsArrays shell = shell == Bash || shell == Ksh -- Returns true if the shell is Bash or Ksh (sorry for the name, Ksh) From 93be86f988e17b6228a23924966da3d6e827bc1b Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:14 -0500 Subject: [PATCH 03/17] Use "drop 1" instead of clumsily rewriting 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 e93758f..563f8ea 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -612,8 +612,7 @@ getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T _ -> [] where flags = map snd $ getAllFlags base - stripEquals s = let rest = dropWhile (/= '=') s in - if rest == "" then "" else tail rest + stripEquals s = drop 1 $ dropWhile (/= '=') s stripEqualsFrom (T_NormalWord id1 (T_Literal id2 s:rs)) = T_NormalWord id1 (T_Literal id2 (stripEquals s):rs) stripEqualsFrom (T_NormalWord id1 [T_DoubleQuoted id2 [T_Literal id3 s]]) = From 0f48bb78a50d4b76b2d1838ee22e3f00d5e8bf1f Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:14 -0500 Subject: [PATCH 04/17] Remove incorrect otherwise You're supposed to use otherwise where you need a Boolean, not a pattern match. This is misleadingly shadowing the real otherwise. Use _ instead. --- src/ShellCheck/Checks/ShellSupport.hs | 2 +- src/ShellCheck/Data.hs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs index 07dfdda..e340d41 100644 --- a/src/ShellCheck/Checks/ShellSupport.hs +++ b/src/ShellCheck/Checks/ShellSupport.hs @@ -487,7 +487,7 @@ checkBraceExpansionVars = ForShell [Bash] f T_DollarBraced {} -> return "$" T_DollarExpansion {} -> return "$" T_DollarArithmetic {} -> return "$" - otherwise -> return "-" + _ -> return "-" toString t = fromJust $ getLiteralStringExt literalExt t isEvaled t = do cmd <- getClosestCommandM t diff --git a/src/ShellCheck/Data.hs b/src/ShellCheck/Data.hs index 732619d..fb4a1e4 100644 --- a/src/ShellCheck/Data.hs +++ b/src/ShellCheck/Data.hs @@ -135,6 +135,6 @@ shellForExecutable name = "ksh" -> return Ksh "ksh88" -> return Ksh "ksh93" -> return Ksh - otherwise -> Nothing + _ -> Nothing flagsForRead = "sreu:n:N:i:p:a:t:" From f5c6771016e396b8a0c11f8f2c6e58cb967c8356 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:16 -0500 Subject: [PATCH 05/17] Use find instead of listToMaybe and filter --- src/ShellCheck/Analytics.hs | 6 +++--- src/ShellCheck/Checks/ShellSupport.hs | 4 ++-- src/ShellCheck/Parser.hs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index e8bee4f..e33d873 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -1234,10 +1234,10 @@ checkLiteralBreakingTest _ t = potentially $ return () comparisonWarning list = do - token <- listToMaybe $ filter hasEquals list + token <- find hasEquals list return $ err (getId token) 2077 "You need spaces around the comparison operator." tautologyWarning t s = do - token <- listToMaybe $ filter isNonEmpty $ getWordParts t + token <- find isNonEmpty $ getWordParts t return $ err (getId token) 2157 s prop_checkConstantNullary = verify checkConstantNullary "[[ '$(foo)' ]]" @@ -2910,7 +2910,7 @@ checkLoopVariableReassignment params token = where check = do str <- loopVariable token - next <- listToMaybe $ filter (\x -> loopVariable x == Just str) path + next <- find (\x -> loopVariable x == Just str) path return $ do warn (getId token) 2165 "This nested loop overrides the index variable of its parent." warn (getId next) 2167 "This parent loop has its index variable overridden." diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs index e340d41..742cfa5 100644 --- a/src/ShellCheck/Checks/ShellSupport.hs +++ b/src/ShellCheck/Checks/ShellSupport.hs @@ -340,8 +340,8 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do potentially $ do allowed' <- Map.lookup name allowedFlags allowed <- allowed' - (word, flag) <- listToMaybe $ - filter (\x -> (not . null . snd $ x) && snd x `notElem` allowed) flags + (word, flag) <- find + (\x -> (not . null . snd $ x) && snd x `notElem` allowed) flags return . warnMsg (getId word) $ name ++ " -" ++ flag ++ " is" when (name == "source") $ warnMsg id "'source' in place of '.' is" diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs index 339b50b..f295560 100644 --- a/src/ShellCheck/Parser.hs +++ b/src/ShellCheck/Parser.hs @@ -34,7 +34,7 @@ import Control.Monad.Identity import Control.Monad.Trans import Data.Char import Data.Functor -import Data.List (isPrefixOf, isInfixOf, isSuffixOf, partition, sortBy, intercalate, nub) +import Data.List (isPrefixOf, isInfixOf, isSuffixOf, partition, sortBy, intercalate, nub, find) import Data.Maybe import Data.Monoid import Debug.Trace @@ -589,7 +589,7 @@ readConditionContents single = checkTrailingOp x = fromMaybe (return ()) $ do (T_Literal id str) <- getTrailingUnquotedLiteral x - trailingOp <- listToMaybe (filter (`isSuffixOf` str) binaryTestOps) + trailingOp <- find (`isSuffixOf` str) binaryTestOps return $ parseProblemAtId id ErrorC 1108 $ "You need a space before and after the " ++ trailingOp ++ " ." From 28978a8b65b303143b2251bc286527b3095c3622 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:17 -0500 Subject: [PATCH 06/17] Use maybe instead of fromMaybe and fmap --- src/ShellCheck/Analytics.hs | 10 +++++----- src/ShellCheck/AnalyzerLib.hs | 4 ++-- src/ShellCheck/Checker.hs | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index e33d873..8d10a02 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -409,7 +409,7 @@ prop_checkArithmeticOpCommand1 = verify checkArithmeticOpCommand "i=i + 1" prop_checkArithmeticOpCommand2 = verify checkArithmeticOpCommand "foo=bar * 2" prop_checkArithmeticOpCommand3 = verifyNot checkArithmeticOpCommand "foo + opts" checkArithmeticOpCommand _ (T_SimpleCommand id [T_Assignment {}] (firstWord:_)) = - fromMaybe (return ()) $ check <$> getGlobOrLiteralString firstWord + maybe (return ()) check $ getGlobOrLiteralString firstWord where check op = when (op `elem` ["+", "-", "*", "/"]) $ @@ -493,8 +493,8 @@ checkPipePitfalls _ (T_Pipeline id _ commands) = do for ["grep", "wc"] $ \(grep:wc:_) -> - let flagsGrep = fromMaybe [] $ map snd . getAllFlags <$> getCommand grep - flagsWc = fromMaybe [] $ map snd . getAllFlags <$> getCommand wc + let flagsGrep = maybe [] (map snd . getAllFlags) $ getCommand grep + flagsWc = maybe [] (map snd . getAllFlags) $ getCommand wc in unless (any (`elem` ["o", "only-matching", "r", "R", "recursive"]) flagsGrep || any (`elem` ["m", "chars", "w", "words", "c", "bytes", "L", "max-line-length"]) flagsWc || null flagsWc) $ style (getId grep) 2126 "Consider using grep -c instead of grep|wc -l." @@ -3140,9 +3140,9 @@ checkSubshellAsTest _ t = checkParams id first second = do - when (fromMaybe False $ (`elem` unaryTestOps) <$> getLiteralString first) $ + when (maybe False (`elem` unaryTestOps) $ getLiteralString first) $ err id 2204 "(..) is a subshell. Did you mean [ .. ], a test expression?" - when (fromMaybe False $ (`elem` binaryTestOps) <$> getLiteralString second) $ + when (maybe False (`elem` binaryTestOps) $ getLiteralString second) $ warn id 2205 "(..) is a subshell. Did you mean [ .. ], a test expression?" diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 563f8ea..4dd3f9d 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -784,8 +784,8 @@ isCommand token str = isCommandMatch token (\cmd -> cmd == str || ('/' : str) ` -- Compare a command to a literal. Like above, but checks full path. isUnqualifiedCommand token str = isCommandMatch token (== str) -isCommandMatch token matcher = fromMaybe False $ - fmap matcher (getCommandName token) +isCommandMatch token matcher = maybe False + matcher (getCommandName token) -- Does this regex look like it was intended as a glob? -- True: *foo* diff --git a/src/ShellCheck/Checker.hs b/src/ShellCheck/Checker.hs index 2ea950d..2837d9a 100644 --- a/src/ShellCheck/Checker.hs +++ b/src/ShellCheck/Checker.hs @@ -88,9 +88,9 @@ checkScript sys spec = do asOptionalChecks = csOptionalChecks spec } where as = newAnalysisSpec root let analysisMessages = - fromMaybe [] $ + maybe [] (arComments . analyzeScript . analysisSpec) - <$> prRoot result + $ prRoot result let translator = tokenToPosition tokenPositions return . nub . sortMessages . filter shouldInclude $ (parseMessages ++ map translator analysisMessages) From 5487b3f229fa3ce56f105733caff6a707c39e7bb Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:18 -0500 Subject: [PATCH 07/17] Use sortOn instead of sortBy and comparing --- src/ShellCheck/Checker.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/Checker.hs b/src/ShellCheck/Checker.hs index 2837d9a..b423f2d 100644 --- a/src/ShellCheck/Checker.hs +++ b/src/ShellCheck/Checker.hs @@ -104,7 +104,7 @@ checkScript sys spec = do code = cCode (pcComment pc) severity = cSeverity (pcComment pc) - sortMessages = sortBy (comparing order) + sortMessages = sortOn order order pc = let pos = pcStartPos pc comment = pcComment pc in From d7278b95f2c840a5a764838d23a32008aaeb6168 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:19 -0500 Subject: [PATCH 08/17] Remove unnecessary "map snd" --- src/ShellCheck/Checks/Commands.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index eb0c434..7841089 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -997,8 +997,7 @@ missingDestination handler token = do args = getAllFlags token params = map fst $ filter (\(_,x) -> x == "") args hasTarget = - any (\x -> x /= "" && x `isPrefixOf` "target-directory") $ - map snd args + any (\(_,x) -> x /= "" && x `isPrefixOf` "target-directory") args prop_checkMvArguments1 = verify checkMvArguments "mv 'foo bar'" prop_checkMvArguments2 = verifyNot checkMvArguments "mv foo bar" From f25b8bd03af4997df3572973a09da025f417422c Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:20 -0500 Subject: [PATCH 09/17] Use gets instead of fmapping the result of get --- src/ShellCheck/Analytics.hs | 2 +- src/ShellCheck/Parser.hs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 8d10a02..ae8e81d 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -1961,7 +1961,7 @@ prop_checkQuotesInLiterals9 = verifyNotTree checkQuotesInLiterals "param=\"/foo/ checkQuotesInLiterals params t = doVariableFlowAnalysis readF writeF Map.empty (variableFlow params) where - getQuotes name = fmap (Map.lookup name) get + getQuotes name = gets (Map.lookup name) setQuotes name ref = modify $ Map.insert name ref deleteQuotes = modify . Map.delete parents = parentMap params diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs index f295560..fa9084e 100644 --- a/src/ShellCheck/Parser.hs +++ b/src/ShellCheck/Parser.hs @@ -325,7 +325,7 @@ parseProblem level code msg = do parseProblemAt pos level code msg setCurrentContexts c = Ms.modify (\state -> state { contextStack = c }) -getCurrentContexts = contextStack <$> Ms.get +getCurrentContexts = Ms.gets contextStack popContext = do v <- getCurrentContexts From e6e89d68fdda2fb1f8f7ec2e4485ab7a5584db15 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:20 -0500 Subject: [PATCH 10/17] Use list comprehensions instead of clunky combinations of map and filter --- src/ShellCheck/Checks/Commands.hs | 6 +++--- src/ShellCheck/Fixer.hs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index 7841089..21c6cb7 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -706,7 +706,7 @@ checkReadExpansions = CommandCheck (Exactly "read") check options = getGnuOpts flagsForRead getVars cmd = fromMaybe [] $ do opts <- options cmd - return . map snd $ filter (\(x,_) -> x == "" || x == "a") opts + return [y | (x,y) <- opts, x == "" || x == "a"] check cmd = mapM_ warning $ getVars cmd warning t = potentially $ do @@ -995,7 +995,7 @@ missingDestination handler token = do _ -> return () where args = getAllFlags token - params = map fst $ filter (\(_,x) -> x == "") args + params = [x | (x,"") <- args] hasTarget = any (\(_,x) -> x /= "" && x `isPrefixOf` "target-directory") args @@ -1083,7 +1083,7 @@ checkSudoArgs = CommandCheck (Basename "sudo") f where f t = potentially $ do opts <- parseOpts t - let nonFlags = map snd $ filter (\(flag, _) -> flag == "") opts + let nonFlags = [x | ("",x) <- opts] commandArg <- nonFlags !!! 0 command <- getLiteralString commandArg guard $ command `elem` builtins diff --git a/src/ShellCheck/Fixer.hs b/src/ShellCheck/Fixer.hs index 12de3c2..a69616e 100644 --- a/src/ShellCheck/Fixer.hs +++ b/src/ShellCheck/Fixer.hs @@ -295,7 +295,7 @@ prop_pstreeSumsCorrectly kvs targets = -- Trivial O(n * m) implementation dumbPrefixSums :: [(Int, Int)] -> [Int] -> [Int] dumbPrefixSums kvs targets = - let prefixSum target = sum . map snd . filter (\(k,v) -> k <= target) $ kvs + let prefixSum target = sum [v | (k,v) <- kvs, k <= target] in map prefixSum targets -- PSTree O(n * log m) implementation smartPrefixSums :: [(Int, Int)] -> [Int] -> [Int] From c29b6afa5600d68ff3dced9259ff0b5f59822864 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 22:50:21 -0500 Subject: [PATCH 11/17] Use null instead of comparing with empty lists --- src/ShellCheck/Analytics.hs | 4 ++-- src/ShellCheck/AnalyzerLib.hs | 4 ++-- src/ShellCheck/Checker.hs | 12 ++++++------ src/ShellCheck/Checks/Commands.hs | 6 +++--- src/ShellCheck/Parser.hs | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index ae8e81d..2888047 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -563,7 +563,7 @@ checkShebang params (T_Annotation _ list t) = isOverride _ = False checkShebang params (T_Script _ (T_Literal id sb) _) = execWriter $ do unless (shellTypeSpecified params) $ do - when (sb == "") $ + when (null sb) $ err id 2148 "Tips depend on target shell and yours is unknown. Add a shebang." when (executableFromShebang sb == "ash") $ warn id 2187 "Ash scripts will be checked as Dash. Add '# shellcheck shell=dash' to silence." @@ -2332,7 +2332,7 @@ checkWhileReadPitfalls _ (T_WhileExpression id [command] contents) checkMuncher _ = return () stdinRedirect (T_FdRedirect _ fd _) - | fd == "" || fd == "0" = True + | null fd || fd == "0" = True stdinRedirect _ = False checkWhileReadPitfalls _ _ = return () diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 4dd3f9d..e4640e7 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -643,7 +643,7 @@ getModifiedVariableCommand base@(T_SimpleCommand id cmdPrefix (T_NormalWord _ (T getModifierParam _ _ = [] letParamToLiteral token = - if var == "" + if null var then [] else [(base, token, var, DataString $ SourceFrom [stripEqualsFrom token])] where var = takeWhile isVariableChar $ dropWhile (`elem` "+-") $ concat $ oversimplify token @@ -952,7 +952,7 @@ getOpts string flags = process flags takesArg <- Map.lookup flag1 flagMap if takesArg then do - guard $ flag2 == "" + guard $ null flag2 more <- process rest return $ (flag1, token2) : more else do diff --git a/src/ShellCheck/Checker.hs b/src/ShellCheck/Checker.hs index b423f2d..6370f75 100644 --- a/src/ShellCheck/Checker.hs +++ b/src/ShellCheck/Checker.hs @@ -198,11 +198,11 @@ prop_optionDisablesBadShebang = } prop_annotationDisablesBadShebang = - [] == check "#!/usr/bin/python\n# shellcheck shell=sh\ntrue\n" + null $ check "#!/usr/bin/python\n# shellcheck shell=sh\ntrue\n" prop_canParseDevNull = - [] == check "source /dev/null" + null $ check "source /dev/null" prop_failsWhenNotSourcing = [1091, 2154] == check "source lol; echo \"$bar\"" @@ -218,7 +218,7 @@ prop_worksWhenDotting = -- FIXME: This should really be giving [1093], "recursively sourced" prop_noInfiniteSourcing = - [] == checkWithIncludes [("lib", "source lib")] "source lib" + null $ checkWithIncludes [("lib", "source lib")] "source lib" prop_canSourceBadSyntax = [1094, 2086] == checkWithIncludes [("lib", "for f; do")] "source lib; echo $1" @@ -239,10 +239,10 @@ prop_recursiveParsing = [1037] == checkRecursive [("lib", "echo \"$10\"")] "source lib" prop_nonRecursiveAnalysis = - [] == checkWithIncludes [("lib", "echo $1")] "source lib" + null $ checkWithIncludes [("lib", "echo $1")] "source lib" prop_nonRecursiveParsing = - [] == checkWithIncludes [("lib", "echo \"$10\"")] "source lib" + null $ checkWithIncludes [("lib", "echo \"$10\"")] "source lib" prop_sourceDirectiveDoesntFollowFile = null $ checkWithIncludes @@ -328,7 +328,7 @@ prop_optionIncludes4 = [2154] == checkOptionIncludes (Just [2154]) "#!/bin/sh\n var='a b'\n echo $var\n echo $bar" -prop_readsRcFile = result == [] +prop_readsRcFile = null result where result = checkWithRc "disable=2086" emptyCheckSpec { csScript = "#!/bin/sh\necho $1", diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index 21c6cb7..299a335 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -345,7 +345,7 @@ returnOrExit multi invalid = (f . arguments) invalid (getId value) f _ = return () - isInvalid s = s == "" || any (not . isDigit) s || length s > 5 + isInvalid s = null s || any (not . isDigit) s || length s > 5 || let value = (read s :: Integer) in value > 255 literal token = fromJust $ getLiteralStringExt lit token @@ -706,7 +706,7 @@ checkReadExpansions = CommandCheck (Exactly "read") check options = getGnuOpts flagsForRead getVars cmd = fromMaybe [] $ do opts <- options cmd - return [y | (x,y) <- opts, x == "" || x == "a"] + return [y | (x,y) <- opts, null x || x == "a"] check cmd = mapM_ warning $ getVars cmd warning t = potentially $ do @@ -1057,7 +1057,7 @@ checkSudoRedirect = CommandCheck (Basename "sudo") f Just (T_Redirecting _ redirs _) -> mapM_ warnAbout redirs warnAbout (T_FdRedirect _ s (T_IoFile id op file)) - | (s == "" || s == "&") && not (special file) = + | (null s || s == "&") && not (special file) = case op of T_Less _ -> info (getId op) 2024 diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs index fa9084e..727572d 100644 --- a/src/ShellCheck/Parser.hs +++ b/src/ShellCheck/Parser.hs @@ -3169,7 +3169,7 @@ readScriptFile sourced = do Nothing -> parseProblemAt pos ErrorC 1008 "This shebang was unrecognized. ShellCheck only supports sh/bash/dash/ksh. Add a 'shell' directive to specify." isValidShell s = - let good = s == "" || any (`isPrefixOf` s) goodShells + let good = null s || any (`isPrefixOf` s) goodShells bad = any (`isPrefixOf` s) badShells in if good From 8a005526cc5b1379b0c651a0505f7110c6bf0d8e Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 23:02:10 -0500 Subject: [PATCH 12/17] Use drop instead of splitAt since we only use the second half --- src/ShellCheck/Fixer.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/Fixer.hs b/src/ShellCheck/Fixer.hs index a69616e..16bdd98 100644 --- a/src/ShellCheck/Fixer.hs +++ b/src/ShellCheck/Fixer.hs @@ -200,7 +200,7 @@ doReplace start end o r = let si = fromIntegral (start-1) ei = fromIntegral (end-1) (x, xs) = splitAt si o - (y, z) = splitAt (ei - si) xs + z = drop (ei - si) xs in x ++ r ++ z From 76b798394f18307677b44e231d6922db251955be Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sat, 1 Feb 2020 23:07:16 -0500 Subject: [PATCH 13/17] Use case matching instead of null Using null followed by a head, tail, or a partial pattern match is an anti-pattern. Use case matching instead. --- src/ShellCheck/Parser.hs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs index 727572d..025fa98 100644 --- a/src/ShellCheck/Parser.hs +++ b/src/ShellCheck/Parser.hs @@ -329,12 +329,11 @@ getCurrentContexts = Ms.gets contextStack popContext = do v <- getCurrentContexts - if not $ null v - then do - let (a:r) = v + case v of + (a:r) -> do setCurrentContexts r return $ Just a - else + [] -> return Nothing pushContext c = do From 115ef290798d8d4b046d2a57470c461ce3b26d6f Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 2 Feb 2020 00:13:16 -0500 Subject: [PATCH 14/17] Use pattern matching instead of head --- src/ShellCheck/Analytics.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 2888047..cf8015b 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -1298,7 +1298,7 @@ checkArithmeticDeref params t@(TA_Expansion _ [b@(T_DollarBraced id _ _)]) = unless (isException $ bracedString b) getWarning where isException [] = True - isException s = any (`elem` "/.:#%?*@$-!+=^,") s || isDigit (head s) + isException s@(h:_) = any (`elem` "/.:#%?*@$-!+=^,") s || isDigit h getWarning = fromMaybe noWarning . msum . map warningFor $ parents params t warningFor t = case t of From 6595e14d25deed617b4c8a9c7d2162ee29d77198 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 2 Feb 2020 00:22:52 -0500 Subject: [PATCH 15/17] Adjust a pattern to avoid tail --- src/ShellCheck/Analytics.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index cf8015b..ddc3b43 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -1644,9 +1644,9 @@ checkSpuriousExec _ = doLists doList = doList' . stripCleanup -- The second parameter is True if we are in a loop -- In that case we should emit the warning also if `exec' is the last statement - doList' t@(current:following:_) False = do + doList' (current:t@(following:_)) False = do commentIfExec current - doList (tail t) False + doList t False doList' (current:tail) True = do commentIfExec current doList tail True From 392b57b8e848a3008a0851d3212a4657870230a1 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 2 Feb 2020 00:27:05 -0500 Subject: [PATCH 16/17] Use maybe instead of isJust and fromJust --- src/ShellCheck/Checks/ShellSupport.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs index 742cfa5..b643525 100644 --- a/src/ShellCheck/Checks/ShellSupport.hs +++ b/src/ShellCheck/Checks/ShellSupport.hs @@ -491,7 +491,7 @@ checkBraceExpansionVars = ForShell [Bash] f toString t = fromJust $ getLiteralStringExt literalExt t isEvaled t = do cmd <- getClosestCommandM t - return $ isJust cmd && fromJust cmd `isUnqualifiedCommand` "eval" + return $ maybe False (`isUnqualifiedCommand` "eval") cmd prop_checkMultiDimensionalArrays1 = verify checkMultiDimensionalArrays "foo[a][b]=3" From e820a5642b4029e3bfe793f59b4cd23dd22b7c20 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 2 Feb 2020 00:34:54 -0500 Subject: [PATCH 17/17] Adjust a pattern to get rid of a fromJust --- src/ShellCheck/Analytics.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index ddc3b43..501847a 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -2635,8 +2635,8 @@ checkMultipleAppends params t = where checkList list = mapM_ checkGroup (groupWith (fmap fst) $ map getTarget list) - checkGroup (f:_:_:_) | isJust f = - style (snd $ fromJust f) 2129 + checkGroup (Just (_,id):_:_:_) = + style id 2129 "Consider using { cmd1; cmd2; } >> file instead of individual redirects." checkGroup _ = return () getTarget (T_Annotation _ _ t) = getTarget t