From 773e98868daf2138b249d805e2297feeda32827a Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 19:53:40 -0400 Subject: [PATCH 1/8] Use foldr in checkFindNameGlob --- src/ShellCheck/Checks/Commands.hs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index e321102..e0a2749 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -199,12 +199,11 @@ prop_checkFindNameGlob3 = verifyNot checkFindNameGlob "find * -name '*.php'" checkFindNameGlob = CommandCheck (Basename "find") (f . arguments) where acceptsGlob s = s `elem` [ "-ilname", "-iname", "-ipath", "-iregex", "-iwholename", "-lname", "-name", "-path", "-regex", "-wholename" ] f [] = return () - f (x:xs) = g x xs - g _ [] = return () - g a (b:r) = do + f (x:xs) = foldr g (const $ return ()) xs x + g b acc a = do forM_ (getLiteralString a) $ \s -> when (acceptsGlob s && isGlob b) $ warn (getId b) 2061 $ "Quote the parameter to " ++ s ++ " so the shell won't interpret it." - g b r + acc b prop_checkNeedlessExpr = verify checkNeedlessExpr "foo=$(expr 3 + 2)" From 9027a9239fafe40463dba495bd25fdd739a4dd52 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 20:03:17 -0400 Subject: [PATCH 2/8] Use pattern matching instead of snd --- src/ShellCheck/Checks/Commands.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index e0a2749..0d887dd 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -461,8 +461,8 @@ checkMkdirDashPM = CommandCheck (Basename "mkdir") check where check t = sequence_ $ do let flags = getAllFlags t - dashP <- find ((\f -> f == "p" || f == "parents") . snd) flags - dashM <- find ((\f -> f == "m" || f == "mode") . snd) flags + dashP <- find (\(_,f) -> f == "p" || f == "parents") flags + dashM <- find (\(_,f) -> f == "m" || f == "mode") flags -- mkdir -pm 0700 dir is fine, so is ../dir, but dir/subdir is not. guard $ any couldHaveSubdirs (drop 1 $ arguments t) return $ warn (getId $ fst dashM) 2174 "When used with -p, -m only applies to the deepest directory." From e8501151dd5d4ad90687deefa434f424701b6c7c Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 20:04:54 -0400 Subject: [PATCH 3/8] Use a guard instead of unless --- src/ShellCheck/Checks/Commands.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index 0d887dd..9e19ab5 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -482,7 +482,7 @@ prop_checkNonportableSignals7 = verifyNot checkNonportableSignals "trap 'stop' i checkNonportableSignals = CommandCheck (Exactly "trap") (f . arguments) where f args = case args of - first:rest -> unless (isFlag first) $ mapM_ check rest + first:rest | not $ isFlag first -> mapM_ check rest _ -> return () check param = sequence_ $ do From fa841cb2701863b84ace7d652ff977db315e599c Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 20:08:02 -0400 Subject: [PATCH 4/8] Prefer pattern matching in undirected --- src/ShellCheck/Checks/Commands.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index 9e19ab5..19884ee 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -519,9 +519,9 @@ checkInteractiveSu = CommandCheck (Basename "su") f info (getId cmd) 2117 "To run commands as another user, use su -c or sudo." - undirected (T_Pipeline _ _ l) = length l <= 1 + undirected (T_Pipeline _ _ (_:_:_)) = False -- This should really just be modifications to stdin, but meh - undirected (T_Redirecting _ list _) = null list + undirected (T_Redirecting _ (_:_) _) = False undirected _ = True From 9747b1d5c3961fbcadf657dffe324b66dc5856a3 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 20:10:56 -0400 Subject: [PATCH 5/8] Simplify checkArg --- src/ShellCheck/Checks/Commands.hs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index 19884ee..1633dc6 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -538,9 +538,8 @@ checkSshCommandString = CommandCheck (Basename "ssh") (f . arguments) ([], hostport:r@(_:_)) -> checkArg $ last r _ -> return () checkArg (T_NormalWord _ [T_DoubleQuoted id parts]) = - case filter (not . isConstant) parts of - [] -> return () - (x:_) -> info (getId x) 2029 + forM_ (find (not . isConstant) parts) $ + \x -> info (getId x) 2029 "Note that, unescaped, this expands on the client side." checkArg _ = return () From df4928f4e3191e18abbaa5085c13c790eeb694fe Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 20:14:03 -0400 Subject: [PATCH 6/8] Use MultiWayIf instead of case-matching on () --- src/ShellCheck/Checks/Commands.hs | 32 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index 1633dc6..440b74a 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -19,6 +19,7 @@ -} {-# LANGUAGE TemplateHaskell #-} {-# LANGUAGE FlexibleContexts #-} +{-# LANGUAGE MultiWayIf #-} -- This module contains checks that examine specific commands by name. module ShellCheck.Checks.Commands (checker, optionalChecks, ShellCheck.Checks.Commands.runTests) where @@ -578,22 +579,21 @@ checkPrintfVar = CommandCheck (Exactly "printf") (f . arguments) where let formatCount = length formats let argCount = length more - return $ - case () of - () | argCount == 0 && formatCount == 0 -> - return () -- This is fine - () | formatCount == 0 && argCount > 0 -> - err (getId format) 2182 - "This printf format string has no variables. Other arguments are ignored." - () | any mayBecomeMultipleArgs more -> - return () -- We don't know so trust the user - () | argCount < formatCount && onlyTrailingTs formats argCount -> - return () -- Allow trailing %()Ts since they use the current time - () | argCount > 0 && argCount `mod` formatCount == 0 -> - return () -- Great: a suitable number of arguments - () -> - warn (getId format) 2183 $ - "This format string has " ++ show formatCount ++ " variables, but is passed " ++ show argCount ++ " arguments." + return $ if + | argCount == 0 && formatCount == 0 -> + return () -- This is fine + | formatCount == 0 && argCount > 0 -> + err (getId format) 2182 + "This printf format string has no variables. Other arguments are ignored." + | any mayBecomeMultipleArgs more -> + return () -- We don't know so trust the user + | argCount < formatCount && onlyTrailingTs formats argCount -> + return () -- Allow trailing %()Ts since they use the current time + | argCount > 0 && argCount `mod` formatCount == 0 -> + return () -- Great: a suitable number of arguments + | otherwise -> + warn (getId format) 2183 $ + "This format string has " ++ show formatCount ++ " variables, but is passed " ++ show argCount ++ " arguments." unless ('%' `elem` concat (oversimplify format) || isLiteral format) $ info (getId format) 2059 From cfa2a663afd83794edacd81ffc0ef9f972e5be26 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 20:21:14 -0400 Subject: [PATCH 7/8] Simplify checkSetAssignment --- src/ShellCheck/Checks/Commands.hs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index 440b74a..4f92fa9 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -661,17 +661,12 @@ prop_checkSetAssignment5 = verifyNot checkSetAssignment "set 'a=5'" prop_checkSetAssignment6 = verifyNot checkSetAssignment "set" checkSetAssignment = CommandCheck (Exactly "set") (f . arguments) where - f (var:value:rest) = - let str = literal var in - when (isVariableName str || isAssignment str) $ - msg (getId var) - f (var:_) = - when (isAssignment $ literal var) $ - msg (getId var) + f (var:rest) + | (not (null rest) && isVariableName str) || isAssignment str = + warn (getId var) 2121 "To assign a variable, use just 'var=value', no 'set ..'." + where str = literal var f _ = return () - msg id = warn id 2121 "To assign a variable, use just 'var=value', no 'set ..'." - isAssignment str = '=' `elem` str literal (T_NormalWord _ l) = concatMap literal l literal (T_Literal _ str) = str From ed331b816b083d8598de3e4ea77a27f2f2daa83a Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 5 Apr 2020 20:32:39 -0400 Subject: [PATCH 8/8] Simplify warnRedundant --- src/ShellCheck/Checks/Commands.hs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs index 4f92fa9..c68bdd1 100644 --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -878,10 +878,10 @@ checkWhileGetoptsCase = CommandCheck (Exactly "getopts") f warnUnhandled optId caseId str = warn caseId 2213 $ "getopts specified -" ++ str ++ ", but it's not handled by this 'case'." - warnRedundant (key, expr) = sequence_ $ do - str <- key - guard $ str `notElem` ["*", ":", "?"] - return $ warn (getId expr) 2214 "This case is not specified by getopts." + warnRedundant (Just str, expr) + | str `notElem` ["*", ":", "?"] = + warn (getId expr) 2214 "This case is not specified by getopts." + warnRedundant _ = return () getHandledStrings (_, globs, _) = map (\x -> (literal x, x)) globs