diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,6 +1,6 @@ Name: ShellCheck -- Must also be updated in ShellCheck/Data.hs : -Version: 0.3.1 +Version: 0.3.2 Synopsis: Shell script analysis tool License: OtherLicense License-file: LICENSE @@ -9,7 +9,7 @@ Maintainer: vidar@vidarholen.net Homepage: http://www.shellcheck.net/ Build-Type: Simple -Cabal-Version: >= 1.6 +Cabal-Version: >= 1.8 Bug-reports: https://github.com/koalaman/shellcheck/issues Description: The goals of ShellCheck are: @@ -28,8 +28,29 @@ location: git://github.com/koalaman/shellcheck.git library - build-depends: base >= 4, base < 5, parsec, containers, regex-compat, mtl, directory, json - exposed-modules: ShellCheck.AST, ShellCheck.Data, ShellCheck.Parser, ShellCheck.Analytics, ShellCheck.Simple + build-depends: + base >= 4 && < 5, + containers, + directory, + json, + mtl, + parsec, + regex-compat + exposed-modules: + ShellCheck.Analytics + ShellCheck.AST + ShellCheck.Data + ShellCheck.Parser + ShellCheck.Simple executable shellcheck + build-depends: + ShellCheck, + base >= 4 && < 5, + containers, + directory, + json, + mtl, + parsec, + regex-compat main-is: shellcheck.hs diff --git a/ShellCheck/AST.hs b/ShellCheck/AST.hs --- a/ShellCheck/AST.hs +++ b/ShellCheck/AST.hs @@ -102,7 +102,7 @@ | T_NormalWord Id [Token] | T_OR_IF Id | T_OrIf Id (Token) (Token) - | T_Pipeline Id [Token] + | T_Pipeline Id [Token] [Token] -- [Pipe separators] [Commands] | T_ProcSub Id String [Token] | T_Rbrace Id | T_Redirecting Id [Token] Token @@ -120,6 +120,7 @@ | T_While Id | T_WhileExpression Id [Token] [Token] | T_Annotation Id [Annotation] Token + | T_Pipe Id String deriving (Show) data Annotation = DisableComment Integer deriving (Show, Eq) @@ -128,12 +129,12 @@ -- I apologize for nothing! lolHax s = Re.subRegex (Re.mkRegex "(Id [0-9]+)") (show s) "(Id 0)" instance Eq Token where - (==) a b = (lolHax a) == (lolHax b) + (==) a b = lolHax a == lolHax b analyze :: Monad m => (Token -> m ()) -> (Token -> m ()) -> (Token -> Token) -> Token -> m Token -analyze f g i t = - round t +analyze f g i = + round where round t = do f t @@ -182,7 +183,7 @@ b <- round cmd return $ T_Redirecting id a b delve (T_SimpleCommand id vars cmds) = dll vars cmds $ T_SimpleCommand id - delve (T_Pipeline id l) = dl l $ T_Pipeline id + delve (T_Pipeline id l1 l2) = dll l1 l2 $ T_Pipeline id delve (T_Banged id l) = d1 l $ T_Banged id delve (T_AndIf id t u) = d2 t u $ T_AndIf id delve (T_OrIf id t u) = d2 t u $ T_OrIf id @@ -297,7 +298,7 @@ T_Array id _ -> id T_Redirecting id _ _ -> id T_SimpleCommand id _ _ -> id - T_Pipeline id _ -> id + T_Pipeline id _ _ -> id T_Banged id _ -> id T_AndIf id _ _ -> id T_OrIf id _ _ -> id @@ -337,17 +338,18 @@ T_DollarDoubleQuoted id _ -> id T_DollarBracket id _ -> id T_Annotation id _ _ -> id + T_Pipe id _ -> id blank :: Monad m => Token -> m () blank = const $ return () -doAnalysis f t = analyze f blank id t -doStackAnalysis startToken endToken t = analyze startToken endToken id t -doTransform i t = runIdentity $ analyze blank blank i t +doAnalysis f = analyze f blank id +doStackAnalysis startToken endToken = analyze startToken endToken id +doTransform i = runIdentity . analyze blank blank i isLoop t = case t of - T_WhileExpression _ _ _ -> True - T_UntilExpression _ _ _ -> True - T_ForIn _ _ _ _ -> True - T_ForArithmetic _ _ _ _ _ -> True - T_SelectIn _ _ _ _ -> True + T_WhileExpression {} -> True + T_UntilExpression {} -> True + T_ForIn {} -> True + T_ForArithmetic {} -> True + T_SelectIn {} -> True _ -> False diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -17,20 +17,19 @@ -} module ShellCheck.Analytics (AnalysisOption(..), filterByAnnotation, runAnalytics, shellForExecutable) where -import ShellCheck.AST -import ShellCheck.Data -import ShellCheck.Parser import Control.Monad import Control.Monad.State import Control.Monad.Writer -import qualified Data.Map as Map import Data.Char import Data.Functor import Data.List import Data.Maybe import Debug.Trace +import ShellCheck.AST +import ShellCheck.Data +import ShellCheck.Parser import Text.Regex -import Data.Maybe +import qualified Data.Map as Map data Shell = Ksh | Zsh | Sh | Bash deriving (Show, Eq) @@ -48,18 +47,20 @@ treeChecks = [ runNodeAnalysis (\p t -> mapM_ (\f -> f t) $ - map (\f -> f p) (nodeChecks ++ (checksFor (shellType p)))) + map (\f -> f p) (nodeChecks ++ checksFor (shellType p))) ,subshellAssignmentCheck ,checkSpacefulness ,checkQuotesInLiterals ,checkShebang ,checkFunctionsUsedExternally ,checkUnusedAssignments + ,checkUnpassedInFunctions ] checksFor Sh = [ checkBashisms ,checkTimeParameters + ,checkForDecimals ] checksFor Ksh = [ checkEchoSed @@ -72,6 +73,7 @@ checkTimeParameters ,checkBraceExpansionVars ,checkEchoSed + ,checkForDecimals ] runAnalytics :: [AnalysisOption] -> Token -> [Note] @@ -88,18 +90,20 @@ getShellOption = fromMaybe (determineShell root) . msum $ - map ((\option -> + map (\option -> case option of ForceShell x -> return x - )) options + ) options checkList l t = concatMap (\f -> f t) l prop_determineShell0 = determineShell (T_Script (Id 0) "#!/bin/sh" []) == Sh prop_determineShell1 = determineShell (T_Script (Id 0) "#!/usr/bin/env ksh" []) == Ksh prop_determineShell2 = determineShell (T_Script (Id 0) "" []) == Bash +prop_determineShell3 = determineShell (T_Script (Id 0) "#!/bin/sh -e" []) == Sh determineShell (T_Script _ shebang _) = fromMaybe Bash . shellForExecutable $ shellFor shebang - where shellFor s | "/env " `isInfixOf` s = head ((drop 1 $ words s)++[""]) + where shellFor s | "/env " `isInfixOf` s = head (drop 1 (words s)++[""]) + shellFor s | ' ' `elem` s = shellFor $ takeWhile (/= ' ') s shellFor s = reverse . takeWhile (/= '/') . reverse $ s shellForExecutable "sh" = return Sh @@ -131,7 +135,6 @@ ,checkDoubleBracketOperators ,checkNoaryWasBinary ,checkConstantNoary - ,checkForDecimals ,checkDivBeforeMult ,checkArithmeticDeref ,checkArithmeticBadOctal @@ -144,7 +147,8 @@ ,checkTr ,checkPipedAssignment ,checkAssignAteCommand - ,checkUuoe + ,checkUuoeCmd + ,checkUuoeVar ,checkFindNameGlob ,checkGrepRe ,checkNeedlessCommands @@ -181,11 +185,15 @@ ,checkWrongArithmeticAssignment ,checkConditionalAndOrs ,checkFunctionDeclarations + ,checkCatastrophicRm + ,checkInteractiveSu + ,checkStderrPipe + ,checkSetAssignment ] -filterByAnnotation token notes = - filter (not . shouldIgnore) notes +filterByAnnotation token = + filter (not . shouldIgnore) where numFor (Note _ _ code _) = code idFor (Note id _ _ _) = id @@ -206,8 +214,9 @@ info = makeNote InfoC style = makeNote StyleC -isVariableStartChar x = x == '_' || x >= 'a' && x <= 'z' || x >= 'A' && x <= 'Z' -isVariableChar x = isVariableStartChar x || x >= '0' && x <= '9' +isVariableStartChar x = x == '_' || isAsciiLower x || isAsciiUpper x +isVariableChar x = isVariableStartChar x || isDigit x +variableNameRegex = mkRegex "[_a-zA-Z][_a-zA-Z0-9]*" prop_isVariableName1 = isVariableName "_fo123" prop_isVariableName2 = not $ isVariableName "4" @@ -215,19 +224,25 @@ isVariableName (x:r) = isVariableStartChar x && all isVariableChar r isVariableName _ = False +matchAll re = unfoldr f + where + f str = do + (_, match, rest, _) <- matchRegexAll re str + return $ (match, rest) + willSplit x = case x of - T_DollarBraced _ _ -> True - T_DollarExpansion _ _ -> True - T_Backticked _ _ -> True - T_BraceExpansion _ s -> True - T_Glob _ _ -> True - T_Extglob _ _ _ -> True + T_DollarBraced {} -> True + T_DollarExpansion {} -> True + T_Backticked {} -> True + T_BraceExpansion {} -> True + T_Glob {} -> True + T_Extglob {} -> True T_NormalWord _ l -> any willSplit l _ -> False -isGlob (T_Extglob _ _ _) = True -isGlob (T_Glob _ _) = True +isGlob (T_Extglob {}) = True +isGlob (T_Glob {}) = True isGlob (T_NormalWord _ l) = any isGlob l isGlob _ = False @@ -268,22 +283,33 @@ makeSimple t = t simplify = doTransform makeSimple -deadSimple (T_NormalWord _ l) = [concat (concatMap (deadSimple) l)] -deadSimple (T_DoubleQuoted _ l) = [(concat (concatMap (deadSimple) l))] +deadSimple (T_NormalWord _ l) = [concat (concatMap deadSimple l)] +deadSimple (T_DoubleQuoted _ l) = [(concat (concatMap deadSimple l))] deadSimple (T_SingleQuoted _ s) = [s] deadSimple (T_DollarBraced _ _) = ["${VAR}"] deadSimple (T_DollarArithmetic _ _) = ["${VAR}"] deadSimple (T_DollarExpansion _ _) = ["${VAR}"] deadSimple (T_Backticked _ _) = ["${VAR}"] deadSimple (T_Glob _ s) = [s] -deadSimple (T_Pipeline _ [x]) = deadSimple x +deadSimple (T_Pipeline _ _ [x]) = deadSimple x deadSimple (T_Literal _ x) = [x] -deadSimple (T_SimpleCommand _ vars words) = concatMap (deadSimple) words +deadSimple (T_SimpleCommand _ vars words) = concatMap deadSimple words deadSimple (T_Redirecting _ _ foo) = deadSimple foo deadSimple (T_DollarSingleQuoted _ s) = [s] deadSimple (T_Annotation _ _ s) = deadSimple s deadSimple _ = [] +-- Turn a SimpleCommand foo -avz --bar=baz into args ["a", "v", "z", "bar"] +getFlags (T_SimpleCommand _ _ (_:args)) = + let textArgs = takeWhile (/= "--") $ map (concat . deadSimple) args in + concatMap flag textArgs + where + flag ('-':'-':arg) = [ takeWhile (/= '=') arg ] + flag ('-':args) = map (:[]) args + flag _ = [] + +getFlags _ = [] + (!!!) list i = case drop i list of [] -> Nothing @@ -294,14 +320,14 @@ verifyTree f s = checkTree f s == Just True verifyNotTree f s = checkTree f s == Just False -checkNode f s = checkTree (runNodeAnalysis f) s +checkNode f = checkTree (runNodeAnalysis f) checkTree f s = case parseShell "-" s of (ParseResult (Just (t, m)) _) -> Just . not . null $ runList [] t [f] _ -> Nothing prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)" -checkEchoWc _ (T_Pipeline id [a, b]) = +checkEchoWc _ (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ case bcmd of ["wc", "-c"] -> countMsg @@ -310,12 +336,12 @@ where acmd = deadSimple a bcmd = deadSimple b - countMsg = style id 2000 $ "See if you can use ${#variable} instead." + countMsg = style id 2000 "See if you can use ${#variable} instead." checkEchoWc _ _ = return () prop_checkEchoSed1 = verify checkEchoSed "FOO=$(echo \"$cow\" | sed 's/foo/bar/g')" prop_checkEchoSed2 = verify checkEchoSed "rm $(echo $cow | sed -e 's,foo,bar,')" -checkEchoSed _ (T_Pipeline id [a, b]) = +checkEchoSed _ (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ case bcmd of ["sed", v] -> checkIn v @@ -327,14 +353,15 @@ bcmd = deadSimple b checkIn s = case matchRegex sedRe s of - Just _ -> style id 2001 $ "See if you can use ${variable//search/replace} instead." + Just _ -> style id 2001 + "See if you can use ${variable//search/replace} instead." _ -> return () checkEchoSed _ _ = return () prop_checkPipedAssignment1 = verify checkPipedAssignment "A=ls | grep foo" prop_checkPipedAssignment2 = verifyNot checkPipedAssignment "A=foo cmd | grep foo" prop_checkPipedAssignment3 = verifyNot checkPipedAssignment "A=foo" -checkPipedAssignment _ (T_Pipeline _ (T_Redirecting _ _ (T_SimpleCommand id (_:_) []):_:_)) = +checkPipedAssignment _ (T_Pipeline _ _ (T_Redirecting _ _ (T_SimpleCommand id (_:_) []):_:_)) = warn id 2036 "If you wanted to assign the output of the pipeline, use a=$(b | c) ." checkPipedAssignment _ _ = return () @@ -343,10 +370,10 @@ prop_checkAssignAteCommand3 = verify checkAssignAteCommand "A=cat foo | grep bar" prop_checkAssignAteCommand4 = verifyNot checkAssignAteCommand "A=foo ls -l" prop_checkAssignAteCommand5 = verifyNot checkAssignAteCommand "PAGER=cat grep bar" -checkAssignAteCommand _ (T_SimpleCommand id ((T_Assignment _ _ _ _ assignmentTerm):[]) (firstWord:_)) = - when ("-" `isPrefixOf` (concat $ deadSimple firstWord) || - (isCommonCommand (getLiteralString assignmentTerm) - && not (isCommonCommand (getLiteralString firstWord)))) $ +checkAssignAteCommand _ (T_SimpleCommand id (T_Assignment _ _ _ _ assignmentTerm:[]) (firstWord:_)) = + when ("-" `isPrefixOf` concat (deadSimple firstWord) || + isCommonCommand (getLiteralString assignmentTerm) + && not (isCommonCommand (getLiteralString firstWord))) $ warn id 2037 "To assign the output of a command, use var=$(cmd) ." where isCommonCommand (Just s) = s `elem` commonCommands @@ -356,7 +383,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:_)) = +checkArithmeticOpCommand _ (T_SimpleCommand id [T_Assignment {}] (firstWord:_)) = fromMaybe (return ()) $ check <$> getGlobOrLiteralString firstWord where check op = @@ -374,9 +401,8 @@ var <- match !!! 0 op <- match !!! 1 Map.lookup var references - return $ do - warn (getId val) 2100 $ - "Use $((..)) for arithmetics, e.g. i=$((i " ++ op ++ " 2))" + return . warn (getId val) 2100 $ + "Use $((..)) for arithmetics, e.g. i=$((i " ++ op ++ " 2))" where regex = mkRegex "^([_a-zA-Z][_a-zA-Z0-9]*)([+*-]).+$" references = foldl (flip ($)) Map.empty (map insertRef $ variableFlow params) @@ -399,7 +425,8 @@ prop_checkUuoc2 = verifyNot checkUuoc "cat * | grep bar" prop_checkUuoc3 = verify checkUuoc "cat $var | grep bar" prop_checkUuoc4 = verifyNot checkUuoc "cat $var" -checkUuoc _ (T_Pipeline _ ((T_Redirecting _ _ cmd):_:_)) = checkCommand "cat" f cmd +checkUuoc _ (T_Pipeline _ _ ((T_Redirecting _ _ cmd):_:_)) = + checkCommand "cat" (const f) cmd where f [word] = when (isSimple word) $ style (getId word) 2002 "Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead." @@ -421,10 +448,10 @@ prop_checkPipePitfalls4 = verifyNot checkPipePitfalls "find . -print0 | xargs -0 foo" prop_checkPipePitfalls5 = verifyNot checkPipePitfalls "ls -N | foo" prop_checkPipePitfalls6 = verify checkPipePitfalls "find . | xargs foo" -checkPipePitfalls _ (T_Pipeline id commands) = do +checkPipePitfalls _ (T_Pipeline id _ commands) = do for ["find", "xargs"] $ \(find:xargs:_) -> let args = deadSimple xargs in - when (not $ hasShortParameter args '0') $ + unless (hasShortParameter args '0') $ warn (getId find) 2038 "Use either 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow for non-alphanumeric filenames." for ["?", "echo"] $ @@ -439,10 +466,10 @@ for' ["ls", "xargs"] $ \x -> warn x 2011 "Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames." ] - when (not didLs) $ do + unless didLs $ do for ["ls", "?"] $ - \(ls:_) -> (when (not $ hasShortParameter (deadSimple ls) 'N') $ - info (getId ls) 2012 "Use find instead of ls to better handle non-alphanumeric filenames.") + \(ls:_) -> unless (hasShortParameter (deadSimple ls) 'N') $ + info (getId ls) 2012 "Use find instead of ls to better handle non-alphanumeric filenames." return () where for l f = @@ -471,7 +498,9 @@ bracedString l = concat $ deadSimple l -isMagicInQuotes (T_DollarBraced _ l) | '@' `elem` (bracedString l) = True +isMagicInQuotes (T_DollarBraced _ l) = + let string = bracedString l in + '@' `elem` string || "!" `isPrefixOf` string isMagicInQuotes _ = False prop_checkShebang1 = verifyTree checkShebang "#!/usr/bin/env bash -x\necho cow" @@ -501,29 +530,29 @@ prop_checkBashisms18= verify checkBashisms "foo &> /dev/null" checkBashisms _ = bashism where - errMsg id s = err id 2040 $ "#!/bin/sh was specified, so " ++ s ++ " is not supported, even when sh is actually bash." - warnMsg id s = warn id 2039 $ "#!/bin/sh was specified, but " ++ s ++ " is not standard." - bashism (T_ProcSub id _ _) = errMsg id "process substitution" - bashism (T_Extglob id _ _) = warnMsg id "extglob" - bashism (T_DollarSingleQuoted id _) = warnMsg id "$'..'" - bashism (T_DollarDoubleQuoted id _) = warnMsg id "$\"..\"" - bashism (T_ForArithmetic id _ _ _ _) = warnMsg id "arithmetic for loop" - bashism (T_Arithmetic id _) = warnMsg id "standalone ((..))" - bashism (T_DollarBracket id _) = warnMsg id "$[..] in place of $((..))" - bashism (T_SelectIn id _ _ _) = warnMsg id "select loop" - bashism (T_BraceExpansion id _) = warnMsg id "brace expansion" - bashism (T_Condition id DoubleBracket _) = warnMsg id "[[ ]]" - bashism (T_HereString id _) = warnMsg id "here-string" + errMsg id s = err id 2040 $ "#!/bin/sh was specified, so " ++ s ++ " not supported, even when sh is actually bash." + warnMsg id s = warn id 2039 $ "#!/bin/sh was specified, but " ++ s ++ " not standard." + bashism (T_ProcSub id _ _) = errMsg id "process substitution is" + bashism (T_Extglob id _ _) = warnMsg id "extglob is" + bashism (T_DollarSingleQuoted id _) = warnMsg id "$'..' is" + bashism (T_DollarDoubleQuoted id _) = warnMsg id "$\"..\" is" + bashism (T_ForArithmetic id _ _ _ _) = warnMsg id "arithmetic for loops are" + bashism (T_Arithmetic id _) = warnMsg id "standalone ((..)) is" + bashism (T_DollarBracket id _) = warnMsg id "$[..] in place of $((..)) is" + bashism (T_SelectIn id _ _ _) = warnMsg id "select loops are" + bashism (T_BraceExpansion id _) = warnMsg id "brace expansion is" + bashism (T_Condition id DoubleBracket _) = warnMsg id "[[ ]] is" + bashism (T_HereString id _) = warnMsg id "here-strings are" bashism (TC_Binary id SingleBracket op _ _) | op `elem` [ "-nt", "-ef", "\\<", "\\>", "==" ] = - warnMsg id op + warnMsg id $ op ++ " is" bashism (TA_Unary id op _) | op `elem` [ "|++", "|--", "++|", "--|"] = - warnMsg id (filter (/= '|') op) + warnMsg id $ (filter (/= '|') op) ++ " is" bashism t@(T_SimpleCommand id _ _) | t `isCommand` "source" = - warnMsg id "'source' in place of '.'" - bashism (T_FdRedirect id "&" (T_IoFile _ (T_Greater _) _)) = warnMsg id "&>" + warnMsg id "'source' in place of '.' is" + bashism (T_FdRedirect id "&" (T_IoFile _ (T_Greater _) _)) = warnMsg id "&> is" bashism (T_DollarBraced id token) = mapM_ check expansion where @@ -533,27 +562,31 @@ bashism t@(T_SimpleCommand _ _ (cmd:arg:_)) | t `isCommand` "echo" && "-" `isPrefixOf` argString = - when (not $ "--" `isPrefixOf` argString) $ -- echo "-------" - warnMsg (getId arg) "echo flag" + unless ("--" `isPrefixOf` argString) $ -- echo "-------" + warnMsg (getId arg) "echo flags are" where argString = (concat $ deadSimple arg) bashism t@(T_SimpleCommand _ _ (cmd:arg:_)) | t `isCommand` "exec" && "-" `isPrefixOf` (concat $ deadSimple arg) = - warnMsg (getId arg) "exec flag" + warnMsg (getId arg) "exec flags are" bashism t@(T_SimpleCommand id _ _) - | t `isCommand` "let" = warnMsg id "'let'" + | t `isCommand` "let" = warnMsg id "'let' is" bashism t@(TA_Variable id "RANDOM") = - warnMsg id "RANDOM" + warnMsg id "RANDOM is" + bashism t@(T_Pipe id "|&") = + warnMsg id "|& in place of 2>&1 | is" + bashism (T_Array id _) = + warnMsg id "arrays are" bashism _ = return() varChars="_0-9a-zA-Z" expansion = let re = mkRegex in [ - (re $ "^[" ++ varChars ++ "]+\\[.*\\]$", "array references"), - (re $ "^![" ++ varChars ++ "]+\\[[*@]]$", "array key expansion"), - (re $ "^![" ++ varChars ++ "]+[*@]$", "name matching prefix"), - (re $ "^[" ++ varChars ++ "]+:[^-=?+]", "string indexing"), - (re $ "^[" ++ varChars ++ "]+(\\[.*\\])?/", "string replacement"), - (re $ "^RANDOM$", "$RANDOM") + (re $ "^[" ++ varChars ++ "]+\\[.*\\]$", "array references are"), + (re $ "^![" ++ varChars ++ "]+\\[[*@]]$", "array key expansion is"), + (re $ "^![" ++ varChars ++ "]+[*@]$", "name matching prefixes are"), + (re $ "^[" ++ varChars ++ "]+:[^-=?+]", "string indexing is"), + (re $ "^[" ++ varChars ++ "]+(\\[.*\\])?/", "string replacement is"), + (re $ "^RANDOM$", "$RANDOM is") ] prop_checkForInQuoted = verify checkForInQuoted "for f in \"$(ls)\"; do echo foo; done" @@ -564,15 +597,16 @@ prop_checkForInQuoted4 = verify checkForInQuoted "for f in 1,2,3; do true; done" prop_checkForInQuoted4a = verifyNot checkForInQuoted "for f in foo{1,2,3}; do true; done" prop_checkForInQuoted5 = verify checkForInQuoted "for f in ls; do true; done" +prop_checkForInQuoted6 = verifyNot checkForInQuoted "for f in \"${!arr}\"; do true; done" checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) = when (any (\x -> willSplit x && not (isMagicInQuotes x)) list - || (getLiteralString word >>= (return . wouldHaveBeenGlob)) == Just True) $ + || (liftM wouldHaveBeenGlob (getLiteralString word) == Just True)) $ err id 2066 $ "Since you double quoted this, it will not word split, and the loop will only run once." checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_SingleQuoted id s]] _) = warn id 2041 $ "This is a literal string. To run as a command, use $(" ++ s ++ ")." checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_Literal id s]] _) = if ',' `elem` s - then when (not $ '{' `elem` s) $ + then unless ('{' `elem` s) $ warn id 2042 $ "Use spaces, not commas, to separate loop elements." else warn id 2043 $ "This loop will only run once, with " ++ f ++ "='" ++ s ++ "'." checkForInQuoted _ _ = return () @@ -584,7 +618,7 @@ prop_checkForInCat3 = verifyNot checkForInCat "for f in $(cat foo | grep bar | wc -l); do stuff; done" checkForInCat _ (T_ForIn _ f [T_NormalWord _ w] _) = mapM_ checkF w where - checkF (T_DollarExpansion id [T_Pipeline _ r]) + checkF (T_DollarExpansion id [T_Pipeline _ _ r]) | all isLineBased r = info id 2013 "To read lines rather than words, pipe/redirect to a 'while read' loop." checkF (T_Backticked id cmds) = checkF (T_DollarExpansion id cmds) @@ -644,9 +678,8 @@ _ -> False warnFor x = - if shouldWarn x - then info (getId x) 2014 "This will expand once before find runs, not per file found." - else return () + when(shouldWarn x) $ + info (getId x) 2014 "This will expand once before find runs, not per file found." fromWord (T_NormalWord _ l) = l fromWord _ = [] @@ -670,7 +703,7 @@ check _ = return () tree = parentMap params examine t = - unless (inUnquotableContext tree t || usedAsCommandName tree t) $ + unless (isQuoteFree tree t || usedAsCommandName tree t) $ warn (getId t) 2046 "Quote this to prevent word splitting." @@ -679,7 +712,7 @@ prop_checkRedirectToSame3 = verifyNot checkRedirectToSame "cat lol | sed -e 's/a/b/g' > foo.bar && mv foo.bar lol" prop_checkRedirectToSame4 = verifyNot checkRedirectToSame "foo /dev/null > /dev/null" prop_checkRedirectToSame5 = verifyNot checkRedirectToSame "foo > bar 2> bar" -checkRedirectToSame params s@(T_Pipeline _ list) = +checkRedirectToSame params s@(T_Pipeline _ _ list) = mapM_ (\l -> (mapM_ (\x -> doAnalysis (checkOccurences x) l) (getAllRedirs list))) list where note x = Note x InfoC 2094 $ @@ -715,7 +748,7 @@ prop_checkShorthandIf2 = verifyNot checkShorthandIf "[[ ! -z file ]] && { scp file host || echo 'Eek'; }" prop_checkShorthandIf3 = verifyNot checkShorthandIf "foo && bar || echo baz" prop_checkShorthandIf4 = verifyNot checkShorthandIf "foo && a=b || a=c" -checkShorthandIf _ (T_AndIf id _ (T_OrIf _ _ (T_Pipeline _ t))) +checkShorthandIf _ (T_AndIf id _ (T_OrIf _ _ (T_Pipeline _ _ t))) | not $ isOk t = info id 2015 "Note that A && B || C is not if-then-else. C may run when A is true." where @@ -814,28 +847,64 @@ prop_checkNumberComparisons3 = verifyNot checkNumberComparisons "[[ $foo ]] > 3" prop_checkNumberComparisons4 = verify checkNumberComparisons "[[ $foo > 2.72 ]]" prop_checkNumberComparisons5 = verify checkNumberComparisons "[[ $foo -le 2.72 ]]" -prop_checkNumberComparisons6 = verify checkNumberComparisons "[[ 3.14 = $foo ]]" -checkNumberComparisons _ (TC_Binary id typ op lhs rhs) = do - when (op `elem` ["<", ">", "<=", ">=", "\\<", "\\>", "\\<=", "\\>="]) $ do - when (isNum lhs || isNum rhs) $ err id 2071 $ "\"" ++ op ++ "\" is for string comparisons. Use " ++ (eqv op) ++" ." - mapM_ checkDecimals [lhs, rhs] +prop_checkNumberComparisons6 = verify checkNumberComparisons "[[ 3.14 -eq $foo ]]" +prop_checkNumberComparisons7 = verifyNot checkNumberComparisons "[[ 3.14 == $foo ]]" +prop_checkNumberComparisons8 = verify checkNumberComparisons "[[ foo <= bar ]]" +prop_checkNumberComparisons9 = verify checkNumberComparisons "[ foo \\>= bar ]" +prop_checkNumberComparisons10= verify checkNumberComparisons "#!/bin/zsh -x\n[ foo >= bar ]]" +checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do + if (isNum lhs || isNum rhs) + then do + when (isLtGt op) $ + err id 2071 $ + op ++ " is for string comparisons. Use " ++ (eqv op) ++ " instead." + when (isLeGe op) $ + err id 2071 $ op ++ " is not a valid operator. " ++ + "Use " ++ (eqv op) ++ " ." + else do + when (isLeGe op || isLtGt op) $ + mapM_ checkDecimals [lhs, rhs] - when (op `elem` ["-lt", "-gt", "-le", "-ge", "-eq", "=", "=="]) $ do + when (isLeGe op) $ + err id 2122 $ op ++ " is not a valid operator. " ++ + "Use '! a " ++ (invert op) ++ " b' instead." + + when (op `elem` ["-lt", "-gt", "-le", "-ge", "-eq"]) $ do mapM_ checkDecimals [lhs, rhs] where - checkDecimals hs = when (isFraction hs) $ err (getId hs) 2072 $ decimalError - decimalError = "Decimals are not supported. Either use integers only, or use bc or awk to compare." - isNum t = case deadSimple t of [v] -> all isDigit v - _ -> False - isFraction t = case deadSimple t of [v] -> isJust $ matchRegex floatRegex v - _ -> False + isLtGt = flip elem ["<", "\\<", ">", "\\>"] + isLeGe = flip elem ["<=", "\\<=", ">=", "\\>="] + + supportsDecimals = + let sh = shellType params in + sh == Ksh || sh == Zsh + checkDecimals hs = + when (isFraction hs && not supportsDecimals) $ + err (getId hs) 2072 decimalError + decimalError = "Decimals are not supported. " ++ + "Either use integers only, or use bc or awk to compare." + + isNum t = + case deadSimple t of + [v] -> all isDigit v + _ -> False + isFraction t = + case deadSimple t of + [v] -> isJust $ matchRegex floatRegex v + _ -> False + eqv ('\\':s) = eqv s eqv "<" = "-lt" eqv ">" = "-gt" eqv "<=" = "-le" eqv ">=" = "-ge" eqv _ = "the numerical equivalent" + + invert ('\\':s) = invert s + invert "<=" = ">" + invert ">=" = "<" + floatRegex = mkRegex "^[0-9]+\\.[0-9]+$" checkNumberComparisons _ _ = return () @@ -947,9 +1016,14 @@ prop_checkArithmeticDeref4 = verifyNot checkArithmeticDeref "(( ! $? ))" prop_checkArithmeticDeref5 = verifyNot checkArithmeticDeref "(($1))" prop_checkArithmeticDeref6 = verifyNot checkArithmeticDeref "(( ${a[$i]} ))" -checkArithmeticDeref _ (TA_Expansion _ (T_DollarBraced id l)) | not . excepting $ bracedString l = - style id 2004 $ "Don't use $ on variables in (( ))." +prop_checkArithmeticDeref7 = verifyNot checkArithmeticDeref "(( 10#$n ))" +checkArithmeticDeref params t@(TA_Expansion _ (T_DollarBraced id l)) = + when (not $ (excepting $ bracedString l) || inBaseExpression) $ + style id 2004 $ "$ on variables in (( )) is unnecessary." where + inBaseExpression = any isBase $ parents params t + isBase (TA_Base {}) = True + isBase _ = False excepting [] = True excepting s = (any (`elem` "/.:#%?*@[]") s) || (isDigit $ head s) checkArithmeticDeref _ _ = return () @@ -975,10 +1049,16 @@ prop_checkCommarrays1 = verify checkCommarrays "a=(1, 2)" prop_checkCommarrays2 = verify checkCommarrays "a+=(1,2,3)" prop_checkCommarrays3 = verifyNot checkCommarrays "cow=(1 \"foo,bar\" 3)" +prop_checkCommarrays4 = verifyNot checkCommarrays "cow=('one,' 'two')" checkCommarrays _ (T_Array id l) = - if any ("," `isSuffixOf`) (concatMap deadSimple l) || (length $ filter (==',') (concat $ concatMap deadSimple l)) > 1 - then warn id 2054 "Use spaces, not commas, to separate array elements." - else return () + when (any (isCommaSeparated . literal) l) $ + warn id 2054 "Use spaces, not commas, to separate array elements." + where + literal (T_NormalWord _ l) = concatMap literal l + literal (T_Literal _ str) = str + literal _ = "str" + + isCommaSeparated str = "," `isSuffixOf` str || (length $ filter (== ',') str) > 1 checkCommarrays _ _ = return () prop_checkOrNeq1 = verify checkOrNeq "if [[ $lol -ne cow || $lol -ne foo ]]; then echo foo; fi" @@ -1025,26 +1105,37 @@ f t = modify (Map.insert (getId t) t) -inUnquotableContext tree t = - case t of - TC_Noary _ DoubleBracket _ -> True - TC_Unary _ DoubleBracket _ _ -> True - TC_Binary _ DoubleBracket _ _ _ -> True - TA_Unary _ _ _ -> True - TA_Binary _ _ _ _ -> True - TA_Trinary _ _ _ _ -> True - TA_Expansion _ _ -> True - T_Assignment _ _ _ _ _ -> True - T_Redirecting _ _ _ -> - any (isCommand t) ["local", "declare", "typeset", "export"] - T_DoubleQuoted _ _ -> True - T_CaseExpression _ _ _ -> True - T_HereDoc _ _ _ _ _ -> True - T_ForIn _ _ _ _ -> True -- Pragmatically assume it's desirable here - x -> case Map.lookup (getId x) tree of - Nothing -> False - Just parent -> inUnquotableContext tree parent +-- Is this node self quoting? +isQuoteFree tree t = + (isQuoteFreeElement t == Just True) || + (head $ (mapMaybe isQuoteFreeContext $ drop 1 $ getPath tree t) ++ [False]) + where + -- Is this node self-quoting in itself? + isQuoteFreeElement t = + case t of + T_Assignment {} -> return True + _ -> Nothing + -- Are any subnodes inherently self-quoting? + isQuoteFreeContext t = + case t of + TC_Noary _ DoubleBracket _ -> return True + TC_Unary _ DoubleBracket _ _ -> return True + TC_Binary _ DoubleBracket _ _ _ -> return True + TA_Unary _ _ _ -> return True + TA_Binary _ _ _ _ -> return True + TA_Trinary _ _ _ _ -> return True + TA_Expansion _ _ -> return True + T_Assignment {} -> return True + T_Redirecting _ _ _ -> return $ + any (isCommand t) ["local", "declare", "typeset", "export"] + T_DoubleQuoted _ _ -> return True + T_CaseExpression _ _ _ -> return True + T_HereDoc _ _ _ _ _ -> return True + T_DollarBraced {} -> return True + T_ForIn _ _ _ _ -> return True -- Pragmatically assume it's desirable here + _ -> Nothing + isParamTo tree cmd t = go t where @@ -1082,14 +1173,16 @@ Nothing -> [] Just parent -> getPath tree parent +parents params t = getPath (parentMap params) t + --- Command specific checks checkCommand str f t@(T_SimpleCommand id _ (cmd:rest)) = - if t `isCommand` str then f rest else return () + if t `isCommand` str then f cmd rest else return () checkCommand _ _ _ = return () checkUnqualifiedCommand str f t@(T_SimpleCommand id _ (cmd:rest)) = - if t `isUnqualifiedCommand` str then f rest else return () + if t `isUnqualifiedCommand` str then f cmd rest else return () checkUnqualifiedCommand _ _ _ = return () getLiteralString = getLiteralStringExt (const Nothing) @@ -1137,7 +1230,7 @@ prop_checkPrintfVar2 = verifyNot checkPrintfVar "printf 'Lol: $s'" prop_checkPrintfVar3 = verify checkPrintfVar "printf -v cow $(cmd)" prop_checkPrintfVar4 = verifyNot checkPrintfVar "printf \"%${count}s\" var" -checkPrintfVar _ = checkUnqualifiedCommand "printf" f where +checkPrintfVar _ = checkUnqualifiedCommand "printf" (const f) where f (dashv:var:rest) | getLiteralString dashv == (Just "-v") = f rest f (format:params) = check format f _ = return () @@ -1147,12 +1240,12 @@ else warn (getId format) 2059 $ "Don't use variables in the printf format string. Use printf \"..%s..\" \"$foo\"." -prop_checkUuoe1 = verify checkUuoe "echo $(date)" -prop_checkUuoe1a= verify checkUuoe "echo `date`" -prop_checkUuoe2 = verify checkUuoe "echo \"$(date)\"" -prop_checkUuoe2a= verify checkUuoe "echo \"`date`\"" -prop_checkUuoe3 = verifyNot checkUuoe "echo \"The time is $(date)\"" -checkUuoe _ = checkUnqualifiedCommand "echo" f where +prop_checkUuoeCmd1 = verify checkUuoeCmd "echo $(date)" +prop_checkUuoeCmd2 = verify checkUuoeCmd "echo `date`" +prop_checkUuoeCmd3 = verify checkUuoeCmd "echo \"$(date)\"" +prop_checkUuoeCmd4 = verify checkUuoeCmd "echo \"`date`\"" +prop_checkUuoeCmd5 = verifyNot checkUuoeCmd "echo \"The time is $(date)\"" +checkUuoeCmd _ = checkUnqualifiedCommand "echo" (const f) where msg id = style id 2005 "Useless echo? Instead of 'echo $(cmd)', just use 'cmd'." f [T_NormalWord id [(T_DollarExpansion _ _)]] = msg id f [T_NormalWord id [T_DoubleQuoted _ [(T_DollarExpansion _ _)]]] = msg id @@ -1160,6 +1253,33 @@ f [T_NormalWord id [T_DoubleQuoted _ [(T_Backticked _ _)]]] = msg id f _ = return () +prop_checkUuoeVar1 = verify checkUuoeVar "for f in $(echo $tmp); do echo lol; done" +prop_checkUuoeVar2 = verify checkUuoeVar "date +`echo \"$format\"`" +prop_checkUuoeVar3 = verifyNot checkUuoeVar "foo \"$(echo -e '\r')\"" +prop_checkUuoeVar4 = verifyNot checkUuoeVar "echo $tmp" +prop_checkUuoeVar5 = verify checkUuoeVar "foo \"$(echo \"$(date) value:\" $value)\"" +prop_checkUuoeVar6 = verifyNot checkUuoeVar "foo \"$(echo files: *.png)\"" +checkUuoeVar _ p = + case p of + T_Backticked id [cmd] -> check id cmd + T_DollarExpansion id [cmd] -> check id cmd + _ -> return () + where + couldBeOptimized f = case f of + T_Glob {} -> False + T_Extglob {} -> False + T_BraceExpansion {} -> False + T_NormalWord _ l -> all couldBeOptimized l + T_DoubleQuoted _ l -> all couldBeOptimized l + _ -> True + + check id (T_Pipeline _ _ [T_Redirecting _ _ c]) = warnForEcho id c + check _ _ = return () + warnForEcho id = checkUnqualifiedCommand "echo" $ \_ vars -> + unless ("-" `isPrefixOf` (concat $ concatMap deadSimple vars)) $ + when (all couldBeOptimized vars) $ style id 2116 + "Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'." + prop_checkTr1 = verify checkTr "tr [a-f] [A-F]" prop_checkTr2 = verify checkTr "tr 'a-z' 'A-Z'" prop_checkTr2a= verify checkTr "tr '[a-z]' '[A-Z]'" @@ -1173,7 +1293,7 @@ prop_checkTr9 = verifyNot checkTr "a-z n-za-m" prop_checkTr10= verifyNot checkTr "tr --squeeze-repeats rl lr" prop_checkTr11= verifyNot checkTr "tr abc '[d*]'" -checkTr _ = checkCommand "tr" (mapM_ f) +checkTr _ = checkCommand "tr" (const $ mapM_ f) where f w | isGlob w = do -- The user will go [ab] -> '[ab]' -> 'ab'. Fixme? warn (getId w) 2060 $ "Quote parameters to tr to prevent glob expansion." @@ -1197,7 +1317,7 @@ prop_checkFindNameGlob1 = verify checkFindNameGlob "find / -name *.php" prop_checkFindNameGlob2 = verify checkFindNameGlob "find / -type f -ipath *(foo)" prop_checkFindNameGlob3 = verifyNot checkFindNameGlob "find * -name '*.php'" -checkFindNameGlob _ = checkCommand "find" f where +checkFindNameGlob _ = checkCommand "find" (const f) where acceptsGlob (Just s) = s `elem` [ "-ilname", "-iname", "-ipath", "-iregex", "-iwholename", "-lname", "-name", "-path", "-regex", "-wholename" ] acceptsGlob _ = False f [] = return () @@ -1219,7 +1339,7 @@ prop_checkGrepRe8 = verify checkGrepRe "ls | grep foo*.jpg" prop_checkGrepRe9 = verifyNot checkGrepRe "grep '[0-9]*' file" -checkGrepRe _ = checkCommand "grep" f where +checkGrepRe _ = checkCommand "grep" (const f) where -- --regex=*(extglob) doesn't work. Fixme? skippable (Just s) = not ("--regex=" `isPrefixOf` s) && "-" `isPrefixOf` s skippable _ = False @@ -1241,7 +1361,7 @@ prop_checkTrapQuotes1a= verify checkTrapQuotes "trap \"echo `ls`\" INT" prop_checkTrapQuotes2 = verifyNot checkTrapQuotes "trap 'echo $num' INT" prop_checkTrapQuotes3 = verify checkTrapQuotes "trap \"echo $((1+num))\" EXIT DEBUG" -checkTrapQuotes _ = checkCommand "trap" f where +checkTrapQuotes _ = checkCommand "trap" (const f) where f (x:_) = checkTrap x f _ = return () checkTrap (T_NormalWord _ [T_DoubleQuoted _ rs]) = mapM_ checkExpansions rs @@ -1257,11 +1377,11 @@ prop_checkTimeParameters2 = verifyNot checkTimeParameters "time sleep 10" prop_checkTimeParameters3 = verifyNot checkTimeParameters "time -p foo" checkTimeParameters _ = checkUnqualifiedCommand "time" f where - f (x:_) = let s = concat $ deadSimple x in + f cmd (x:_) = let s = concat $ deadSimple x in if "-" `isPrefixOf` s && s /= "-p" then - info (getId x) 2023 "The shell may override 'time' as seen in man time(1). Use 'command time ..' for that one." + info (getId cmd) 2023 "The shell may override 'time' as seen in man time(1). Use 'command time ..' for that one." else return () - f _ = return () + f _ _ = return () prop_checkTestRedirects1 = verify checkTestRedirects "test 3 > 1" prop_checkTestRedirects2 = verifyNot checkTestRedirects "test 3 \\> 1" @@ -1414,7 +1534,7 @@ doLists _ = return () stripCleanup = reverse . dropWhile cleanup . reverse - cleanup (T_Pipeline _ [cmd]) = + cleanup (T_Pipeline _ _ [cmd]) = isCommandMatch cmd (`elem` ["echo", "exit"]) cleanup _ = False @@ -1424,7 +1544,7 @@ doList (tail t) doList' _ = return () - commentIfExec (T_Pipeline id list) = + commentIfExec (T_Pipeline id _ list) = mapM_ commentIfExec $ take 1 list commentIfExec (T_Redirecting _ _ f@( T_SimpleCommand id _ (cmd:arg:_))) = @@ -1456,7 +1576,7 @@ prop_checkUnusedEchoEscapes2 = verifyNot checkUnusedEchoEscapes "echo -e 'foi\\nbar'" prop_checkUnusedEchoEscapes3 = verify checkUnusedEchoEscapes "echo \"n:\\t42\"" prop_checkUnusedEchoEscapes4 = verifyNot checkUnusedEchoEscapes "echo lol" -checkUnusedEchoEscapes _ = checkCommand "echo" f +checkUnusedEchoEscapes _ = checkCommand "echo" (const f) where isDashE = mkRegex "^-.*e" hasEscapes = mkRegex "\\\\[rnt]" @@ -1499,7 +1619,7 @@ prop_checkSshCmdStr1 = verify checkSshCommandString "ssh host \"echo $PS1\"" prop_checkSshCmdStr2 = verifyNot checkSshCommandString "ssh host \"ls foo\"" prop_checkSshCmdStr3 = verifyNot checkSshCommandString "ssh \"$host\"" -checkSshCommandString _ = checkCommand "ssh" f +checkSshCommandString _ = checkCommand "ssh" (const f) where nonOptions args = filter (\x -> not $ "-" `isPrefixOf` (concat $ deadSimple x)) args @@ -1566,11 +1686,11 @@ parentPipeline = do parent <- Map.lookup (getId t) parents case parent of - T_Pipeline _ _ -> return parent + T_Pipeline _ _ _ -> return parent _ -> Nothing causesSubshell = do - (T_Pipeline _ list) <- parentPipeline + (T_Pipeline _ _ list) <- parentPipeline if length list <= 1 then return False else if lastCreatesSubshell @@ -1607,10 +1727,13 @@ T_SelectIn id str words _ -> [(t, t, str, DataFrom words)] _ -> [] --- Consider 'export' a reference, since it makes the var available +-- Consider 'export/declare -x' a reference, since it makes the var available getReferencedVariableCommand base@(T_SimpleCommand _ _ ((T_NormalWord _ ((T_Literal _ x):_)):rest)) = case x of "export" -> concatMap getReference rest + "declare" -> if "x" `elem` getFlags base + then concatMap getReference rest + else [] _ -> [] where getReference t@(T_Assignment _ _ name _ value) = [(t, t, name)] @@ -1661,11 +1784,19 @@ -- TODO: getBracedReference s = takeWhile (\x -> not $ x `elem` ":[#%/^,") $ dropWhile (`elem` "#!") s +getIndexReferences s = fromMaybe [] $ do + (_, index, _, _) <- matchRegexAll re s + return $ matchAll variableNameRegex index + where + re = mkRegex "\\[.*\\]" getReferencedVariables t = case t of - T_DollarBraced id l -> map (\x -> (t, t, x)) $ [getBracedReference $ bracedString l] - TA_Variable id str -> [(t, t, str)] + T_DollarBraced id l -> let str = bracedString l in + (t, t, getBracedReference str) : + (map (\x -> (l, l, x)) $ getIndexReferences str) + TA_Variable id str -> + map (\x -> (t, t, x)) $ (getBracedReference str):(getIndexReferences str) T_Assignment id Append str _ _ -> [(t, t, str)] x -> getReferencedVariableCommand x @@ -1728,7 +1859,6 @@ doFlow _ = return [] ---- Check whether variables could have spaces/globs -prop_checkSpacefulness0 = verifyTree checkSpacefulness "for f in *.mp3; do echo $f; done" prop_checkSpacefulness1 = verifyTree checkSpacefulness "a='cow moo'; echo $a" prop_checkSpacefulness2 = verifyNotTree checkSpacefulness "a='cow moo'; [[ $a ]]" prop_checkSpacefulness3 = verifyNotTree checkSpacefulness "a='cow*.mp3'; echo \"$a\"" @@ -1748,6 +1878,7 @@ prop_checkSpacefulnessH = verifyTree checkSpacefulness "echo foo=$1" prop_checkSpacefulnessI = verifyNotTree checkSpacefulness "$1 --flags" prop_checkSpacefulnessJ = verifyTree checkSpacefulness "echo $PWD" +prop_checkSpacefulnessK = verifyNotTree checkSpacefulness "n+='foo bar'" checkSpacefulness params t = doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params) @@ -1766,7 +1897,7 @@ if spaced && (not $ "@" `isPrefixOf` name) -- There's another warning for this && (not $ isCounting token) - && (not $ inUnquotableContext parents token) + && (not $ isQuoteFree parents token) && (not $ usedAsCommandName parents token) then return [(Note (getId token) InfoC 2086 warning)] else return [] @@ -1823,7 +1954,7 @@ checkQuotesInLiterals params t = doVariableFlowAnalysis readF writeF Map.empty (variableFlow params) where - getQuotes name = get >>= (return . Map.lookup name) + getQuotes name = liftM (Map.lookup name) get setQuotes name ref = modify $ Map.insert name ref deleteQuotes = modify . Map.delete parents = parentMap params @@ -1855,7 +1986,7 @@ assignment <- getQuotes name if isJust assignment && not (isParamTo parents "eval" expr) - && not (inUnquotableContext parents expr) + && not (isQuoteFree parents expr) then return [ Note (fromJust assignment)WarningC 2089 $ "Quotes/backslashes will be treated literally. Use an array.", @@ -1921,22 +2052,27 @@ prop_checkUnused10= verifyNotTree checkUnusedAssignments "read -p 'test: '" prop_checkUnused11= verifyNotTree checkUnusedAssignments "bar=5; export foo[$bar]=3" prop_checkUnused12= verifyNotTree checkUnusedAssignments "read foo; echo ${!foo}" +prop_checkUnused13= verifyNotTree checkUnusedAssignments "x=(1); (( x[0] ))" +prop_checkUnused14= verifyNotTree checkUnusedAssignments "x=(1); n=0; echo ${x[n]}" +prop_checkUnused15= verifyNotTree checkUnusedAssignments "x=(1); n=0; (( x[n] ))" +prop_checkUnused16= verifyNotTree checkUnusedAssignments "foo=5; declare -x foo" checkUnusedAssignments params t = snd $ runWriter (mapM_ checkAssignment flow) where flow = variableFlow params references = foldl (flip ($)) defaultMap (map insertRef flow) insertRef (Reference (base, token, name)) = - Map.insert name () + Map.insert (stripSuffix name) () insertRef _ = id checkAssignment (Assignment (_, token, name, _)) | isVariableName name = case Map.lookup name references of Just _ -> return () - Nothing -> do + Nothing -> info (getId token) 2034 $ name ++ " appears unused. Verify it or export it." checkAssignment _ = return () + stripSuffix str = takeWhile isVariableChar str defaultMap = Map.fromList $ zip internalVariables $ repeat () prop_checkGlobsAsOptions1 = verify checkGlobsAsOptions "rm *.txt" @@ -1974,14 +2110,14 @@ where munchers = [ "ssh", "ffmpeg", "mplayer" ] - isStdinReadCommand (T_Pipeline _ [T_Redirecting id redirs cmd]) = + isStdinReadCommand (T_Pipeline _ _ [T_Redirecting id redirs cmd]) = let plaintext = deadSimple cmd in head (plaintext ++ [""]) == "read" - && (not $ "-u" `elem` plaintext) + && ("-u" `notElem` plaintext) && all (not . stdinRedirect) redirs isStdinReadCommand _ = False - checkMuncher (T_Pipeline _ ((T_Redirecting _ redirs cmd):_)) = do + checkMuncher (T_Pipeline _ _ ((T_Redirecting _ redirs cmd):_)) = do let name = fromMaybe "" $ getCommandBasename cmd when ((not . any stdinRedirect $ redirs) && (name `elem` munchers)) $ do info id 2095 $ @@ -2011,7 +2147,7 @@ T_SimpleCommand _ vars (_:_) -> mapM_ checkVar vars otherwise -> check rest checkVar (T_Assignment aId mode aName Nothing value) | - aName == name && (not $ aId `elem` idPath) = do + aName == name && (aId `notElem` idPath) = do warn aId 2097 "This assignment is only seen by the forked process." warn id 2098 "This expansion will not see the mentioned assignment." checkVar _ = return () @@ -2028,7 +2164,7 @@ && contents /= ":" then warn id 2101 "Named class needs outer [], e.g. [[:digit:]]." else - if (not $ '[' `elem` contents) && hasDupes + if ('[' `notElem` contents) && hasDupes then info id 2102 "Ranges can only match single chars (mentioned due to duplicates)." else return () where @@ -2060,7 +2196,7 @@ _ -> False getCmd (T_Annotation id _ x) = getCmd x - getCmd (T_Pipeline id [x]) = getCommandName x + getCmd (T_Pipeline id _ [x]) = getCommandName x getCmd _ = Nothing doList list = @@ -2119,3 +2255,213 @@ when (hasKeyword && not hasParens) $ warn id 2113 "'function' keyword is non-standard. Use 'foo()' instead of 'function foo'." checkFunctionDeclarations _ _ = return () + + +-- This is a lot of code for little gain. Consider whether it's worth it. +prop_checkCatastrophicRm1 = verify checkCatastrophicRm "rm -r $1/$2" +prop_checkCatastrophicRm2 = verify checkCatastrophicRm "foo=$(echo bar); rm -r /home/$foo" +prop_checkCatastrophicRm3 = verify checkCatastrophicRm "foo=/home; user=$(whoami); rm -r \"$foo/$user\"" +prop_checkCatastrophicRm4 = verifyNot checkCatastrophicRm "foo=/home; user=cow; rm -r \"$foo/$user\"" +prop_checkCatastrophicRm5 = verifyNot checkCatastrophicRm "user=$(whoami); rm -r /home/${user:?Nope}" +prop_checkCatastrophicRm6 = verify checkCatastrophicRm "rm --recursive /etc/*$config*" +prop_checkCatastrophicRm7 = verifyNot checkCatastrophicRm "var=$(cmd); if [ -n \"$var\" ]; then rm -r /etc/$var/*; fi" +prop_checkCatastrophicRm8 = verify checkCatastrophicRm "rm -rf /home" +prop_checkCatastrophicRm9 = verifyNot checkCatastrophicRm "rm -rf -- /home" +checkCatastrophicRm params t@(T_SimpleCommand id _ tokens) | t `isCommand` "rm" = + when (any isRecursiveFlag $ simpleArgs) $ + mapM_ checkWord tokens + where + -- This ugly hack is based on the fact that ids generally increase + relevantMap (Id n) = liftM snd . listToMaybe . dropWhile (\(Id x, _) -> x > n) $ flowMapR + flowMapR = reverse $ (\x -> zip (scanl getScopeId (Id 0) x) (scanl addNulls defaultMap x)) $ variableFlow params + simpleArgs = deadSimple t + defaultMap = Map.fromList (map (\x -> (x, Nothing)) variablesWithoutSpaces) + + checkWord token = + case getLiteralString token of + Just str -> + when (all (/= "--") simpleArgs && (fixPath str `elem` importantPaths)) $ + info (getId token) 2114 $ "Obligatory typo warning. Use 'rm --' to disable this message." + Nothing -> + checkWord' token + + checkWord' token = fromMaybe (return ()) $ do + m <- relevantMap id + filename <- combine m token + let path = fixPath filename + return . when (path `elem` importantPaths) $ do + warn (getId token) 2115 $ "Make sure this never accidentally expands to '" ++ path ++ "'." + + fixPath filename = + let normalized = skipRepeating '/' . skipRepeating '*' $ filename in + if normalized == "/" then normalized else stripTrailing '/' $ normalized + + unnullable = all isVariableChar . concat . deadSimple + isRecursiveFlag "--recursive" = True + isRecursiveFlag ('-':'-':_) = False + isRecursiveFlag ('-':str) = 'r' `elem` str || 'R' `elem` str + isRecursiveFlag _ = False + + stripTrailing c = reverse . dropWhile (== c) . reverse + skipRepeating c (a:b:rest) | a == b && b == c = skipRepeating c (b:rest) + skipRepeating c (a:r) = a:(skipRepeating c r) + skipRepeating _ [] = [] + + addNulls map (Reference (_, token, name)) = + if mightBeGuarded token + then Map.insert name Nothing map + else map + addNulls map (Assignment (_, token, name, DataExternal)) = + if mightBeGuarded token + then Map.insert name Nothing map + else Map.insert name (Just "") map + addNulls m (Assignment (_, token, name, DataFrom [word])) = + if mightBeGuarded token + then Map.insert name Nothing m + else + if couldFail word + then m + else Map.insert name ((combine m) word) m + addNulls m (Assignment (_, token, name, DataFrom _)) = + Map.insert name Nothing m + addNulls map _ = map + + getScopeId n (Reference (_, token, _)) = getId token + getScopeId n (Assignment (_, token, _, _)) = getId token + getScopeId n _ = n + + joinMaybes :: [Maybe String] -> Maybe String + joinMaybes = foldl (liftM2 (++)) (Just "") + combine m token = c token + where + c (T_DollarBraced _ t) | unnullable t = + Map.findWithDefault (Just "") (concat $ deadSimple t) m + c (T_DoubleQuoted _ tokens) = joinMaybes $ map (combine m) tokens + c (T_NormalWord _ tokens) = joinMaybes $ map (combine m) tokens + c (T_Glob _ "*") = Just "*" + c t = getLiteralString t + + couldFail (T_Backticked _ _) = True + couldFail (T_DollarExpansion _ _) = True + couldFail (T_DoubleQuoted _ foo) = any couldFail foo + couldFail (T_NormalWord _ foo) = any couldFail foo + couldFail _ = False + + mightBeGuarded token = any t (getPath (parentMap params) token) + where + t (T_Condition _ _ _) = True + t (T_OrIf _ _ _) = True + t (T_AndIf _ _ _) = True + t _ = False + + paths = [ + "/", "/etc", "/home", "/mnt", "/usr", "/usr/share", "/usr/local", + "/var" + ] + importantPaths = ["", "/*", "/*/*"] >>= (\x -> map (++x) paths) +checkCatastrophicRm _ _ = return () + + +prop_checkInteractiveSu1 = verify checkInteractiveSu "su; rm file; su $USER" +prop_checkInteractiveSu2 = verify checkInteractiveSu "su foo; something; exit" +prop_checkInteractiveSu3 = verifyNot checkInteractiveSu "echo rm | su foo" +prop_checkInteractiveSu4 = verifyNot checkInteractiveSu "su root < script" +checkInteractiveSu params = checkCommand "su" f + where + f cmd l = when (length l <= 1) $ + when (all undirected $ getPath (parentMap params) cmd) $ + info (getId cmd) 2117 + "To run commands as another user, use su -c or sudo." + + undirected (T_Pipeline _ _ l) = length l <= 1 + -- This should really just be modifications to stdin, but meh + undirected (T_Redirecting _ list _) = null list + undirected _ = True + + +prop_checkStderrPipe1 = verify checkStderrPipe "#!/bin/ksh\nfoo |& bar" +prop_checkStderrPipe2 = verifyNot checkStderrPipe "#!/bin/zsh\nfoo |& bar" +checkStderrPipe params = + case shellType params of + Ksh -> match + _ -> const $ return () + where + match (T_Pipe id "|&") = + err id 2118 "Ksh does not support |&. Use 2>&1 |." + match _ = return () + +prop_checkUnpassedInFunctions1 = verifyTree checkUnpassedInFunctions "foo() { echo $1; }; foo" +prop_checkUnpassedInFunctions2 = verifyNotTree checkUnpassedInFunctions "foo() { echo $1; };" +prop_checkUnpassedInFunctions3 = verifyNotTree checkUnpassedInFunctions "foo() { echo $lol; }; foo" +prop_checkUnpassedInFunctions4 = verifyNotTree checkUnpassedInFunctions "foo() { echo $0; }; foo" +prop_checkUnpassedInFunctions5 = verifyNotTree checkUnpassedInFunctions "foo() { echo $1; }; foo 'lol'; foo" +checkUnpassedInFunctions params root = + execWriter $ mapM_ warnForGroup referenceGroups + where + functionMap :: Map.Map String Token + functionMap = Map.fromList $ + map (\t@(T_Function _ _ _ name _) -> (name,t)) functions + functions = execWriter $ doAnalysis (tell . maybeToList . findFunction) root + findFunction t@(T_DollarBraced id token) = do + str <- getLiteralString token + unless (isPositional str) $ fail "Not positional" + let path = getPath (parentMap params) t + find isFunction path + findFunction _ = Nothing + + isFunction (T_Function {}) = True + isFunction _ = False + + referenceList :: [(String, Bool, Token)] + referenceList = execWriter $ + doAnalysis (fromMaybe (return ()) . checkCommand) root + checkCommand :: Token -> Maybe (Writer [(String, Bool, Token)] ()) + checkCommand t@(T_SimpleCommand _ _ (cmd:args)) = do + str <- getLiteralString cmd + unless (Map.member str functionMap) $ fail "irrelevant" + return $ tell [(str, null args, t)] + checkCommand _ = Nothing + + isPositional str = str == "*" || str == "@" + || (all isDigit str && str /= "0") + + isArgumentless (_, b, _) = b + referenceGroups = Map.elems $ foldr updateWith Map.empty referenceList + updateWith x@(name, _, _) = Map.insertWith (++) name [x] + + warnForGroup group = + when (all isArgumentless group) $ do + mapM_ suggestParams group + warnForDeclaration group + + suggestParams (name, _, thing) = + info (getId thing) 2119 $ + "Use " ++ name ++ " \"$@\" if function's $1 should mean script's $1." + warnForDeclaration ((name, _, _):_) = + warn (getId . fromJust $ Map.lookup name functionMap) 2120 $ + name ++ " references arguments, but none are ever passed." + + +prop_checkSetAssignment1 = verify checkSetAssignment "set foo 42" +prop_checkSetAssignment2 = verify checkSetAssignment "set foo = 42" +prop_checkSetAssignment3 = verify checkSetAssignment "set foo=42" +prop_checkSetAssignment4 = verifyNot checkSetAssignment "set -- if=/dev/null" +prop_checkSetAssignment5 = verifyNot checkSetAssignment "set 'a=5'" +prop_checkSetAssignment6 = verifyNot checkSetAssignment "set" +checkSetAssignment params = checkUnqualifiedCommand "set" f + where + f cmd (var:value:rest) = + let str = literal var in + when (isVariableName str || isAssignment str) $ + msg (getId var) + f cmd (var:_) = + when (isAssignment $ literal var) $ + msg (getId 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 + literal _ = "*" diff --git a/ShellCheck/Data.hs b/ShellCheck/Data.hs --- a/ShellCheck/Data.hs +++ b/ShellCheck/Data.hs @@ -1,6 +1,6 @@ module ShellCheck.Data where -shellcheckVersion = "0.3.1" -- Must also be updated in ShellCheck.cabal +shellcheckVersion = "0.3.2" -- Must also be updated in ShellCheck.cabal internalVariables = [ -- Generic diff --git a/ShellCheck/Parser.hs b/ShellCheck/Parser.hs --- a/ShellCheck/Parser.hs +++ b/ShellCheck/Parser.hs @@ -46,14 +46,18 @@ quotableChars = "|&;<>()\\ '\t\n\r\xA0" ++ doubleQuotableChars quotable = nbsp <|> unicodeDoubleQuote <|> oneOf quotableChars bracedQuotable = oneOf "}\"$`'" -doubleQuotableChars = "\"$`\x201C\x201D" +doubleQuotableChars = "\"$`" ++ unicodeDoubleQuoteChars doubleQuotable = unicodeDoubleQuote <|> oneOf doubleQuotableChars whitespace = oneOf " \t\n" <|> carriageReturn <|> nbsp linewhitespace = oneOf " \t" <|> nbsp +suspectCharAfterQuotes = variableChars <|> char '%' + extglobStartChars = "?*@!+" extglobStart = oneOf extglobStartChars +unicodeDoubleQuoteChars = "\x201C\x201D\x2033\x2036" + prop_spacing = isOk spacing " \\\n # Comment" spacing = do x <- many (many1 linewhitespace <|> (try $ string "\\\n")) @@ -78,7 +82,7 @@ unicodeDoubleQuote = do pos <- getPosition - char '\x201C' <|> char '\x201D' + oneOf unicodeDoubleQuoteChars parseProblemAt pos WarningC 1015 "This is a unicode double quote. Delete and retype it." return '"' @@ -313,9 +317,11 @@ when (endedWith "]" x) $ do parseProblemAt pos ErrorC 1020 $ "You need a space before the " ++ (if single then "]" else "]]") ++ "." + fail "Missing space before ]" when (single && endedWith ")" x) $ do parseProblemAt pos ErrorC 1021 $ "You need a space before the \\)" + fail "Missing space before )" disregard spacing return x where endedWith str (T_NormalWord id s@(_:_)) = @@ -327,6 +333,7 @@ id <- getNextId x <- try (string "&&" <|> string "-a") softCondSpacing + skipLineFeeds return $ TC_And id typ x readCondOrOp = do @@ -334,6 +341,7 @@ id <- getNextId x <- try (string "||" <|> string "-o") softCondSpacing + skipLineFeeds return $ TC_Or id typ x readCondNoaryOrBinary = do @@ -412,7 +420,17 @@ str <- string "|" return $ T_Literal id str - readCondTerm = readCondNot <|> readCondExpr + skipLineFeeds = do + pos <- getPosition + spacing <- allspacing + when (single && '\n' `elem` spacing) $ + parseProblemAt pos ErrorC 1080 "In [ ] you need \\ before line feeds." + + readCondTerm = do + term <- readCondNot <|> readCondExpr + skipLineFeeds + return term + readCondNot = do id <- getNextId char '!' @@ -611,6 +629,9 @@ prop_readCondition7 = isOk readCondition "[[ ${line} =~ ^[[:space:]]*# ]]" prop_readCondition8 = isOk readCondition "[[ $l =~ ogg|flac ]]" prop_readCondition9 = isOk readCondition "[ foo -a -f bar ]" +prop_readCondition10= isOk readCondition "[[ a == b \n || c == d ]]" +prop_readCondition11= isOk readCondition "[[ a == b || \n c == d ]]" +prop_readCondition12= isWarning readCondition "[ a == b \n -o c == d ]" readCondition = called "test expression" $ do opos <- getPosition id <- getNextId @@ -739,18 +760,30 @@ prop_readSingleQuoted = isOk readSingleQuoted "'foo bar'" prop_readSingleQuoted2 = isWarning readSingleQuoted "'foo bar\\'" prop_readsingleQuoted3 = isWarning readSingleQuoted "\x2018hello\x2019" +prop_readSingleQuoted4 = isWarning readNormalWord "'it's" +prop_readSingleQuoted5 = isWarning readSimpleCommand "foo='bar\ncow 'arg" +prop_readSingleQuoted6 = isOk readSimpleCommand "foo='bar cow 'arg" readSingleQuoted = called "single quoted string" $ do id <- getNextId + startPos <- getPosition singleQuote s <- readSingleQuotedPart `reluctantlyTill` singleQuote - pos <- getPosition + let string = concat s + endPos <- getPosition singleQuote "end of single quoted string" - let string = concat s - return (T_SingleQuoted id string) `attempting` do - x <- lookAhead anyChar - when (isAlpha x && not (null string) && isAlpha (last string)) $ parseProblemAt pos WarningC 1011 "This apostrophe terminated the single quoted string!" + optional $ do + c <- try . lookAhead $ suspectCharAfterQuotes <|> oneOf "'" + if (not (null string) && isAlpha c && isAlpha (last string)) + then + parseProblemAt endPos WarningC 1011 $ + "This apostrophe terminated the single quoted string!" + else + when ('\n' `elem` string && not ("\n" `isPrefixOf` string)) $ + suggestForgotClosingQuote startPos endPos "single quoted string" + return (T_SingleQuoted id string) + readSingleQuotedLiteral = do singleQuote strs <- many1 readSingleQuotedPart @@ -763,13 +796,24 @@ prop_readBackTicked = isOk readBackTicked "`ls *.mp3`" prop_readBackTicked2 = isOk readBackTicked "`grep \"\\\"\"`" +prop_readBackTicked3 = isWarning readBackTicked "´grep \"\\\"\"´" +prop_readBackTicked4 = isOk readBackTicked "`echo foo\necho bar`" +prop_readBackTicked5 = isOk readSimpleCommand "echo `foo`bar" +prop_readBackTicked6 = isWarning readSimpleCommand "echo `foo\necho `bar" readBackTicked = called "backtick expansion" $ do id <- getNextId - pos <- getPosition - char '`' + startPos <- getPosition + backtick subStart <- getPosition - subString <- readGenericLiteral "`" - char '`' + subString <- readGenericLiteral "`´" + endPos <- getPosition + backtick + + optional $ do + c <- try . lookAhead $ suspectCharAfterQuotes + when ('\n' `elem` subString && not ("\n" `isPrefixOf` subString)) $ do + suggestForgotClosingQuote startPos endPos "backtick expansion" + -- Result positions may be off due to escapes result <- subParse subStart readCompoundList (unEscape subString) return $ T_Backticked id result @@ -778,6 +822,12 @@ unEscape ('\\':x:rest) | x `elem` "$`\\" = x : unEscape rest unEscape ('\\':'\n':rest) = unEscape rest unEscape (c:rest) = c : unEscape rest + backtick = + disregard (char '`') <|> do + pos <- getPosition + char '´' + parseProblemAt pos ErrorC 1077 $ + "For command expansion, the tick should slant left (` vs ´)." subParse pos parser input = do lastPosition <- getPosition @@ -792,13 +842,32 @@ prop_readDoubleQuoted = isOk readDoubleQuoted "\"Hello $FOO\"" prop_readDoubleQuoted2 = isOk readDoubleQuoted "\"$'\"" prop_readDoubleQuoted3 = isWarning readDoubleQuoted "\x201Chello\x201D" +prop_readDoubleQuoted4 = isWarning readSimpleCommand "\"foo\nbar\"foo" +prop_readDoubleQuoted5 = isOk readSimpleCommand "lol \"foo\nbar\" etc" readDoubleQuoted = called "double quoted string" $ do id <- getNextId + startPos <- getPosition doubleQuote x <- many doubleQuotedPart + endPos <- getPosition doubleQuote "end of double quoted string" + optional $ do + try . lookAhead $ suspectCharAfterQuotes <|> oneOf "$\"" + when (any hasLineFeed x && not (startsWithLineFeed x)) $ + suggestForgotClosingQuote startPos endPos "double quoted string" return $ T_DoubleQuoted id x + where + startsWithLineFeed ((T_Literal _ ('\n':_)):_) = True + startsWithLineFeed _ = False + hasLineFeed (T_Literal _ str) | '\n' `elem` str = True + hasLineFeed _ = False +suggestForgotClosingQuote startPos endPos name = do + parseProblemAt startPos WarningC 1078 $ + "Did you forget to close this " ++ name ++ "?" + parseProblemAt endPos InfoC 1079 $ + "This is actually an end quote, but due to next char it looks suspect." + doubleQuotedPart = readDoubleLiteral <|> readDoubleQuotedDollar <|> readBackTicked readDoubleQuotedLiteral = do @@ -1325,13 +1394,25 @@ readPipeSequence = do id <- getNextId - list <- readCommand `sepBy1` (readPipe `thenSkip` (spacing >> readLineBreak)) + (cmds, pipes) <- sepBy1WithSeparators readCommand + (readPipe `thenSkip` (spacing >> readLineBreak)) spacing - return $ T_Pipeline id list + return $ T_Pipeline id pipes cmds + where + sepBy1WithSeparators p s = do + let elems = p >>= \x -> return ([x], []) + let seps = do + separator <- s + return $ \(a,b) (c,d) -> (a++c, b ++ d ++ [separator]) + elems `chainl1` seps readPipe = do notFollowedBy2 g_OR_IF - char '|' `thenSkip` spacing + id <- getNextId + char '|' + qualifier <- string "&" <|> return "" + spacing + return $ T_Pipe id ('|':qualifier) readCommand = (readCompoundCommand <|> readSimpleCommand) @@ -1682,7 +1763,8 @@ if space == "" && space2 /= "" then do when (variable /= "IFS") $ - parseNoteAt pos InfoC 1007 $ "Note that 'var= value' (with space after equals sign) is similar to 'var=\"\"; value'." + parseNoteAt pos WarningC 1007 + "Remove space after = if trying to assign a value (for empty string, use var='' ... )." value <- readEmptyLiteral return $ T_Assignment id op variable index value else do @@ -1729,15 +1811,23 @@ notFollowedBy2 $ char '(' return $ t id -tryWordToken s t = tryParseWordToken (string s) t `thenSkip` spacing -tryParseWordToken parser t = try $ do +tryWordToken s t = tryParseWordToken s t `thenSkip` spacing +tryParseWordToken keyword t = try $ do id <- getNextId - parser + str <- anycaseString keyword optional (do try . lookAhead $ char '[' parseProblem ErrorC 1069 "You need a space before the [.") try $ lookAhead (keywordSeparator) + when (str /= keyword) $ + parseProblem ErrorC 1081 $ + "Scripts are case sensitive. Use '" ++ keyword ++ "', not '" ++ str ++ "'." return $ t id + +anycaseString str = + mapM anycaseChar str + where + anycaseChar c = char (toLower c) <|> char (toUpper c) g_AND_IF = tryToken "&&" T_AND_IF g_OR_IF = tryToken "||" T_OR_IF diff --git a/shellcheck.hs b/shellcheck.hs --- a/shellcheck.hs +++ b/shellcheck.hs @@ -69,7 +69,7 @@ return $ Just (opts, files) (_, _, errors) -> do - printErr $ (concat errors) ++ "\n" ++ usageInfo header options + printErr $ concat errors ++ "\n" ++ usageInfo header options exitWith syntaxFailure formats = Map.fromList [ @@ -84,7 +84,7 @@ return $ and output where clear = ansi 0 - ansi n = "\x1B[" ++ (show n) ++ "m" + ansi n = "\x1B[" ++ show n ++ "m" colorForLevel "error" = 31 -- red colorForLevel "warning" = 33 -- yellow @@ -94,7 +94,8 @@ colorForLevel "source" = 0 -- none colorForLevel _ = 0 -- none - colorComment level comment = (ansi $ colorForLevel level) ++ comment ++ clear + colorComment level comment = + ansi (colorForLevel level) ++ comment ++ clear doFile path = do contents <- readContents path @@ -112,15 +113,17 @@ then "" else fileLines !! (lineNum - 1) putStrLn "" - putStrLn $ colorFunc "message" ("In " ++ filename ++" line " ++ (show $ lineNum) ++ ":") + putStrLn $ colorFunc "message" + ("In " ++ filename ++" line " ++ show lineNum ++ ":") putStrLn (colorFunc "source" line) - mapM (\c -> putStrLn (colorFunc (scSeverity c) $ cuteIndent c)) x + mapM_ (\c -> putStrLn (colorFunc (scSeverity c) $ cuteIndent c)) x putStrLn "" ) groups return $ null comments cuteIndent comment = - (replicate ((scColumn comment) - 1) ' ') ++ "^-- " ++ (code $ scCode comment) ++ ": " ++ (scMessage comment) + replicate (scColumn comment - 1) ' ' ++ + "^-- " ++ code (scCode comment) ++ ": " ++ scMessage comment code code = "SC" ++ (show code) @@ -131,7 +134,7 @@ -- This totally ignores the filenames. Fixme? forJson options files = do comments <- liftM concat $ mapM (commentsFor options) files - putStrLn $ encodeStrict $ comments + putStrLn $ encodeStrict comments return . null $ comments -- Mimic GCC "file:line:col: (error|warning|note): message" format @@ -178,8 +181,8 @@ severity "warning" = "warning" severity _ = "info" attr s v = concat [ s, "='", escape v, "' " ] - escape msg = concatMap escape' msg - escape' c = if isOk c then [c] else "&#" ++ (show $ ord c) ++ ";" + escape = concatMap escape' + escape' c = if isOk c then [c] else "&#" ++ show (ord c) ++ ";" isOk x = any ($x) [isAsciiUpper, isAsciiLower, isDigit, (`elem` " ./")] formatFile name comments = concat [ @@ -226,7 +229,7 @@ real (_:rest) r v target = real rest (r+1) (v+1) target getOption [] _ = Nothing -getOption ((Flag var val):_) name | name == var = return val +getOption (Flag var val:_) name | name == var = return val getOption (_:rest) flag = getOption rest flag getOptions options name = @@ -247,8 +250,8 @@ in map (Prelude.read . clean) elements :: [Int] -excludeCodes codes comments = - filter (not . hasCode) comments +excludeCodes codes = + filter (not . hasCode) where hasCode c = scCode c `elem` codes @@ -265,7 +268,7 @@ exitWith code process Nothing = return False -process (Just (options, files)) = do +process (Just (options, files)) = let format = fromMaybe "tty" $ getOption options "format" in case Map.lookup format formats of Nothing -> do @@ -281,11 +284,9 @@ when (isJust $ getOption opts "version") printVersionAndExit let shell = getOption opts "shell" in - if isNothing shell - then return () - else when (isNothing $ shell >>= shellForExecutable) $ do - printErr $ "Unknown shell: " ++ (fromJust shell) - exitWith supportFailure + when (isJust shell && isNothing (shell >>= shellForExecutable)) $ do + printErr $ "Unknown shell: " ++ (fromJust shell) + exitWith supportFailure when (null files) $ do printErr "No files specified.\n"