diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,5 +1,6 @@ Name: ShellCheck -Version: 0.3.0 +-- Must also be updated in ShellCheck/Data.hs : +Version: 0.3.1 Synopsis: Shell script analysis tool License: OtherLicense License-file: LICENSE diff --git a/ShellCheck/AST.hs b/ShellCheck/AST.hs --- a/ShellCheck/AST.hs +++ b/ShellCheck/AST.hs @@ -26,6 +26,8 @@ data Quoted = Quoted | Unquoted deriving (Show, Eq) data Dashed = Dashed | Undashed deriving (Show, Eq) data AssignmentMode = Assign | Append deriving (Show, Eq) +data FunctionKeyword = FunctionKeyword Bool deriving (Show, Eq) +data FunctionParentheses = FunctionParentheses Bool deriving (Show, Eq) data Token = TA_Base Id String Token @@ -80,7 +82,7 @@ | T_For Id | T_ForArithmetic Id Token Token Token [Token] | T_ForIn Id String [Token] [Token] - | T_Function Id String Token + | T_Function Id FunctionKeyword FunctionParentheses String Token | T_GREATAND Id | T_Glob Id String | T_Greater Id @@ -218,7 +220,7 @@ return $ T_ForArithmetic id x y z list delve (T_Script id s l) = dl l $ T_Script id s - delve (T_Function id name body) = d1 body $ T_Function id name + delve (T_Function id a b name body) = d1 body $ T_Function id a b name delve (T_Condition id typ token) = d1 token $ T_Condition id typ delve (T_Extglob id str l) = dl l $ T_Extglob id str delve (T_DollarBraced id op) = d1 op $ T_DollarBraced id @@ -308,7 +310,7 @@ T_ForIn id _ _ _ -> id T_SelectIn id _ _ _ -> id T_CaseExpression id _ _ -> id - T_Function id _ _ -> id + T_Function id _ _ _ _ -> id T_Arithmetic id _ -> id T_Script id _ _ -> id T_Condition id _ _ -> id @@ -342,3 +344,10 @@ doStackAnalysis startToken endToken t = analyze startToken endToken id t doTransform i t = runIdentity $ analyze blank blank i t +isLoop t = case t of + 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 @@ -15,13 +15,14 @@ You should have received a copy of the GNU Affero General Public License along with this program. If not, see . -} -module ShellCheck.Analytics where +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 @@ -34,60 +35,88 @@ data Shell = Ksh | Zsh | Sh | Bash deriving (Show, Eq) -genericChecks = [ - runBasicAnalysis (\x -> mapM_ (flip ($) x) basicChecks) - ,runBasicTreeAnalysis treeChecks +data Parameters = Parameters { + variableFlow :: [StackData], + parentMap :: Map.Map Id Token, + shellType :: Shell + } + +data AnalysisOption = ForceShell Shell + +-- Checks that are run on the AST root +treeChecks :: [Parameters -> Token -> [Note]] +treeChecks = [ + runNodeAnalysis + (\p t -> mapM_ (\f -> f t) $ + map (\f -> f p) (nodeChecks ++ (checksFor (shellType p)))) ,subshellAssignmentCheck ,checkSpacefulness ,checkQuotesInLiterals ,checkShebang ,checkFunctionsUsedExternally ,checkUnusedAssignments - ,checkWrongArithmeticAssignment ] -checksFor Sh = map runBasicAnalysis [ +checksFor Sh = [ checkBashisms ,checkTimeParameters - ,checkCdAndBack Sh ] -checksFor Ksh = map runBasicAnalysis [ +checksFor Ksh = [ checkEchoSed - ,checkCdAndBack Ksh ] -checksFor Zsh = map runBasicAnalysis [ +checksFor Zsh = [ checkTimeParameters ,checkEchoSed - ,checkCdAndBack Zsh ] -checksFor Bash = map runBasicAnalysis [ +checksFor Bash = [ checkTimeParameters ,checkBraceExpansionVars ,checkEchoSed - ,checkCdAndBack Bash ] -runAllAnalytics root m = addToMap notes m - where shell = determineShell root - notes = checkList ((checksFor shell) ++ genericChecks) root +runAnalytics :: [AnalysisOption] -> Token -> [Note] +runAnalytics options root = runList options root treeChecks +runList options root list = notes + where + params = Parameters { + shellType = getShellOption, + parentMap = getParentTree root, + variableFlow = getVariableFlow (shellType params) (parentMap params) root + } + notes = concatMap (\f -> f params root) list + + getShellOption = + fromMaybe (determineShell root) . msum $ + map ((\option -> + case option of + ForceShell x -> return x + )) options + checkList l t = concatMap (\f -> f t) l -addToMap list map = foldr (\(id,note) m -> Map.adjust (\(Metadata pos notes) -> Metadata pos (note:notes)) id m) map list 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 -determineShell (T_Script _ shebang _) = normalize $ shellFor shebang +determineShell (T_Script _ shebang _) = fromMaybe Bash . shellForExecutable $ shellFor shebang where shellFor s | "/env " `isInfixOf` s = head ((drop 1 $ words s)++[""]) shellFor s = reverse . takeWhile (/= '/') . reverse $ s - normalize "sh" = Sh - normalize "ksh" = Ksh - normalize "zsh" = Zsh - normalize "bash" = Bash - normalize _ = Bash -runBasicAnalysis f t = snd $ runState (doAnalysis f t) [] -basicChecks = [ +shellForExecutable "sh" = return Sh +shellForExecutable "ash" = return Sh +shellForExecutable "dash" = return Sh + +shellForExecutable "ksh" = return Ksh +shellForExecutable "ksh88" = return Ksh +shellForExecutable "ksh93" = return Ksh + +shellForExecutable "zsh" = return Zsh +shellForExecutable "bash" = return Bash +shellForExecutable _ = Nothing + +-- Checks that are run on each node in the AST +runNodeAnalysis f p t = execWriter (doAnalysis (f p) t) +nodeChecks = [ checkUuoc ,checkPipePitfalls ,checkForInQuoted @@ -143,29 +172,26 @@ ,checkWhileReadPitfalls ,checkArithmeticOpCommand ,checkCharRangeGlob - ] -treeChecks = [ - checkUnquotedExpansions + ,checkUnquotedExpansions ,checkSingleQuotedVariables ,checkRedirectToSame ,checkPrefixAssignmentReference + ,checkLoopKeywordScope + ,checkCdAndBack + ,checkWrongArithmeticAssignment + ,checkConditionalAndOrs + ,checkFunctionDeclarations ] -runBasicTreeAnalysis checks token = - checkList (map runTree checks) token - where - parentTree = getParentTree token - runTree f t = runBasicAnalysis (flip f $ parentTree) t - -filterByAnnotation token metadataMap = - Map.mapWithKey removeVals metadataMap +filterByAnnotation token notes = + filter (not . shouldIgnore) notes where - removeVals id (Metadata pos notes) = - Metadata pos $ filter (not . shouldIgnore id . numFor) notes - numFor (Note _ code _) = code - shouldIgnore id num = - any (shouldIgnoreFor num) $ getPath parents (T_Bang id) + numFor (Note _ _ code _) = code + idFor (Note id _ _ _) = id + shouldIgnore note = + any (shouldIgnoreFor (numFor note)) $ + getPath parents (T_Bang $ idFor note) shouldIgnoreFor num (T_Annotation _ anns _) = any hasNum anns where @@ -173,11 +199,12 @@ shouldIgnoreFor _ _ = False parents = getParentTree token -addNoteFor id note = modify ((id, note):) -warn id code note = addNoteFor id $ Note WarningC code $ note -err id code note = addNoteFor id $ Note ErrorC code $ note -info id code note = addNoteFor id $ Note InfoC code $ note -style id code note = addNoteFor id $ Note StyleC code $ note +addNote note = tell [note] +makeNote severity id code note = addNote $ Note id severity code note +warn = makeNote WarningC +err = makeNote ErrorC +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' @@ -262,22 +289,19 @@ [] -> Nothing (r:_) -> Just r -verify f s = checkBasic f s == Just True -verifyNot f s = checkBasic f s == Just False -verifyFull f s = checkFull f s == Just True -verifyNotFull f s = checkFull f s == Just False +verify f s = checkNode f s == Just True +verifyNot f s = checkNode f s == Just False verifyTree f s = checkTree f s == Just True verifyNotTree f s = checkTree f s == Just False -checkBasic f s = checkFull (runBasicAnalysis f) s -checkTree f s = checkFull (runBasicTreeAnalysis [f]) s -checkFull f s = case parseShell "-" s of - (ParseResult (Just (t, m)) _) -> Just . not . null $ f t +checkNode f s = checkTree (runNodeAnalysis f) s +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 @@ -287,11 +311,11 @@ acmd = deadSimple a bcmd = deadSimple b countMsg = style id 2000 $ "See if you can use ${#variable} instead." -checkEchoWc _ = return () +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 @@ -305,51 +329,60 @@ case matchRegex sedRe s of Just _ -> style id 2001 $ "See if you can use ${variable//search/replace} instead." _ -> return () -checkEchoSed _ = 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 () +checkPipedAssignment _ _ = return () prop_checkAssignAteCommand1 = verify checkAssignAteCommand "A=ls -l" prop_checkAssignAteCommand2 = verify checkAssignAteCommand "A=ls --sort=$foo" 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:_)) = +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) ." + (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 isCommonCommand _ = False -checkAssignAteCommand _ = return () +checkAssignAteCommand _ _ = return () 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 = when (op `elem` ["+", "-", "*", "/"]) $ warn (getId firstWord) 2099 $ "Use $((..)) for arithmetics, e.g. i=$((i " ++ op ++ " 2))" -checkArithmeticOpCommand _ = return () +checkArithmeticOpCommand _ _ = return () -prop_checkWrongArit = verifyFull checkWrongArithmeticAssignment "i=i+1" -prop_checkWrongArit2 = verifyFull checkWrongArithmeticAssignment "n=2; i=n*2" -checkWrongArithmeticAssignment t = runBasicAnalysis f t +prop_checkWrongArit = verify checkWrongArithmeticAssignment "i=i+1" +prop_checkWrongArit2 = verify checkWrongArithmeticAssignment "n=2; i=n*2" +checkWrongArithmeticAssignment params (T_SimpleCommand id ((T_Assignment _ _ _ _ val):[]) []) = + fromMaybe (return ()) $ do + str <- getNormalString val + match <- matchRegex regex str + 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))" where regex = mkRegex "^([_a-zA-Z][_a-zA-Z0-9]*)([+*-]).+$" - flow = getVariableFlow t - references = foldl (flip ($)) Map.empty (map insertRef flow) + references = foldl (flip ($)) Map.empty (map insertRef $ variableFlow params) insertRef (Assignment (_, _, name, _)) = Map.insert name () - insertRef _ = id + insertRef _ = Prelude.id getNormalString (T_NormalWord _ words) = do parts <- foldl (liftM2 (\x y -> x ++ [y])) (Just []) $ map getLiterals words @@ -359,24 +392,14 @@ getLiterals (T_Literal _ s) = return s getLiterals (T_Glob _ s) = return s getLiterals _ = Nothing +checkWrongArithmeticAssignment _ _ = return () - f (T_SimpleCommand id ((T_Assignment _ _ _ _ val):[]) []) = - fromMaybe (return ()) $ do - str <- getNormalString val - match <- matchRegex regex str - 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))" - f _ = return () prop_checkUuoc1 = verify checkUuoc "cat foo | grep bar" 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" f cmd where f [word] = when (isSimple word) $ style (getId word) 2002 "Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead." @@ -384,21 +407,21 @@ isSimple (T_NormalWord _ parts) = all isSimple parts isSimple (T_DollarBraced _ _) = True isSimple x = not $ willSplit x -checkUuoc _ = return () +checkUuoc _ _ = return () prop_checkNeedlessCommands = verify checkNeedlessCommands "foo=$(expr 3 + 2)" prop_checkNeedlessCommands2 = verify checkNeedlessCommands "foo=`echo \\`expr 3 + 2\\``" prop_checkNeedlessCommands3 = verifyNot checkNeedlessCommands "foo=$(expr foo : regex)" -checkNeedlessCommands cmd@(T_SimpleCommand id _ _) | +checkNeedlessCommands _ cmd@(T_SimpleCommand id _ _) | cmd `isCommand` "expr" && (not $ ":" `elem` deadSimple cmd) = style id 2003 "expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]]." -checkNeedlessCommands _ = return () +checkNeedlessCommands _ _ = return () prop_checkPipePitfalls3 = verify checkPipePitfalls "ls | grep -v mp3" 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') $ @@ -431,7 +454,7 @@ first func (x:_) = func (getId x) first _ _ = return () hasShortParameter list char = any (\x -> "-" `isPrefixOf` x && char `elem` x) list -checkPipePitfalls _ = return () +checkPipePitfalls _ _ = return () indexOfSublists sub all = f 0 all where @@ -451,12 +474,11 @@ isMagicInQuotes (T_DollarBraced _ l) | '@' `elem` (bracedString l) = True isMagicInQuotes _ = False -prop_checkShebang1 = verifyFull checkShebang "#!/usr/bin/env bash -x\necho cow" -prop_checkShebang2 = verifyNotFull checkShebang "#! /bin/sh -l " -checkShebang (T_Script id sb _) = +prop_checkShebang1 = verifyTree checkShebang "#!/usr/bin/env bash -x\necho cow" +prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh -l " +checkShebang _ (T_Script id sb _) = if (length $ words sb) > 2 then - let note = Note ErrorC 2096 $ "On most OS, shebangs can only specify a single parameter." - in [(id, note)] + [Note id ErrorC 2096 $ "On most OS, shebangs can only specify a single parameter."] else [] prop_checkBashisms = verify checkBashisms "while read a; do :; done < <(a)" @@ -477,7 +499,7 @@ prop_checkBashisms16= verify checkBashisms "echo $RANDOM" prop_checkBashisms17= verify checkBashisms "echo $((RANDOM%6+1))" prop_checkBashisms18= verify checkBashisms "foo &> /dev/null" -checkBashisms = bashism +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." @@ -540,25 +562,27 @@ prop_checkForInQuoted2b = verify checkForInQuoted "for f in \"*.mp3\"; do echo foo; done" prop_checkForInQuoted3 = verify checkForInQuoted "for f in 'find /'; do true; done" 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" -checkForInQuoted (T_ForIn _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) = +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) $ 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]] _) = +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]] _) = +checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_Literal id s]] _) = if ',' `elem` s - then warn id 2042 $ "Use spaces, not commas, to separate loop elements." + then when (not $ '{' `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 () +checkForInQuoted _ _ = return () prop_checkForInCat1 = verify checkForInCat "for f in $(cat foo); do stuff; done" prop_checkForInCat1a= verify checkForInCat "for f in `cat foo`; do stuff; done" prop_checkForInCat2 = verify checkForInCat "for f in $(cat foo | grep lol); do stuff; done" prop_checkForInCat2a= verify checkForInCat "for f in `cat foo | grep lol`; do stuff; done" 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 +checkForInCat _ (T_ForIn _ f [T_NormalWord _ w] _) = mapM_ checkF w where checkF (T_DollarExpansion id [T_Pipeline _ r]) | all isLineBased r = @@ -567,12 +591,12 @@ checkF _ = return () isLineBased cmd = any (cmd `isCommand`) ["grep", "fgrep", "egrep", "sed", "cat", "awk", "cut", "sort"] -checkForInCat _ = return () +checkForInCat _ _ = return () prop_checkForInLs = verify checkForInLs "for f in $(ls *.mp3); do mplayer \"$f\"; done" prop_checkForInLs2 = verify checkForInLs "for f in `ls *.mp3`; do mplayer \"$f\"; done" prop_checkForInLs3 = verify checkForInLs "for f in `find / -name '*.mp3'`; do mplayer \"$f\"; done" -checkForInLs t = try t +checkForInLs _ t = try t where try (T_ForIn _ f [T_NormalWord _ [T_DollarExpansion id [x]]] _) = check id f x @@ -594,7 +618,7 @@ prop_checkFindExec4 = verifyNot checkFindExec "find / -name '*.php' -exec foo {} +" prop_checkFindExec5 = verifyNot checkFindExec "find / -execdir bash -c 'a && b' \\;" prop_checkFindExec6 = verify checkFindExec "find / -type d -execdir rm *.jpg \\;" -checkFindExec cmd@(T_SimpleCommand _ _ t@(h:r)) | cmd `isCommand` "find" = do +checkFindExec _ cmd@(T_SimpleCommand _ _ t@(h:r)) | cmd `isCommand` "find" = do c <- broken r False when c $ do let wordId = getId $ last t in @@ -626,61 +650,64 @@ fromWord (T_NormalWord _ l) = l fromWord _ = [] -checkFindExec _ = return () +checkFindExec _ _ = return () -prop_checkUnquotedExpansions1 = verifyTree checkUnquotedExpansions "rm $(ls)" -prop_checkUnquotedExpansions1a= verifyTree checkUnquotedExpansions "rm `ls`" -prop_checkUnquotedExpansions2 = verifyTree checkUnquotedExpansions "rm foo$(date)" -prop_checkUnquotedExpansions3 = verifyTree checkUnquotedExpansions "[ $(foo) == cow ]" -prop_checkUnquotedExpansions3a= verifyTree checkUnquotedExpansions "[ ! $(foo) ]" -prop_checkUnquotedExpansions4 = verifyNotTree checkUnquotedExpansions "[[ $(foo) == cow ]]" -prop_checkUnquotedExpansions5 = verifyNotTree checkUnquotedExpansions "for f in $(cmd); do echo $f; done" -prop_checkUnquotedExpansions6 = verifyNotTree checkUnquotedExpansions "$(cmd)" -prop_checkUnquotedExpansions7 = verifyNotTree checkUnquotedExpansions "cat << foo\n$(ls)\nfoo" -checkUnquotedExpansions t tree = +prop_checkUnquotedExpansions1 = verify checkUnquotedExpansions "rm $(ls)" +prop_checkUnquotedExpansions1a= verify checkUnquotedExpansions "rm `ls`" +prop_checkUnquotedExpansions2 = verify checkUnquotedExpansions "rm foo$(date)" +prop_checkUnquotedExpansions3 = verify checkUnquotedExpansions "[ $(foo) == cow ]" +prop_checkUnquotedExpansions3a= verify checkUnquotedExpansions "[ ! $(foo) ]" +prop_checkUnquotedExpansions4 = verifyNot checkUnquotedExpansions "[[ $(foo) == cow ]]" +prop_checkUnquotedExpansions5 = verifyNot checkUnquotedExpansions "for f in $(cmd); do echo $f; done" +prop_checkUnquotedExpansions6 = verifyNot checkUnquotedExpansions "$(cmd)" +prop_checkUnquotedExpansions7 = verifyNot checkUnquotedExpansions "cat << foo\n$(ls)\nfoo" +checkUnquotedExpansions params t = check t where check t@(T_DollarExpansion _ _) = examine t check t@(T_Backticked _ _) = examine t check _ = return () + tree = parentMap params examine t = unless (inUnquotableContext tree t || usedAsCommandName tree t) $ warn (getId t) 2046 "Quote this to prevent word splitting." -prop_checkRedirectToSame = verifyTree checkRedirectToSame "cat foo > foo" -prop_checkRedirectToSame2 = verifyTree checkRedirectToSame "cat lol | sed -e 's/a/b/g' > lol" -prop_checkRedirectToSame3 = verifyNotTree checkRedirectToSame "cat lol | sed -e 's/a/b/g' > foo.bar && mv foo.bar lol" -prop_checkRedirectToSame4 = verifyNotTree checkRedirectToSame "foo /dev/null > /dev/null" -prop_checkRedirectToSame5 = verifyNotTree checkRedirectToSame "foo > bar 2> bar" -checkRedirectToSame s@(T_Pipeline _ list) parents = +prop_checkRedirectToSame = verify checkRedirectToSame "cat foo > foo" +prop_checkRedirectToSame2 = verify checkRedirectToSame "cat lol | sed -e 's/a/b/g' > lol" +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) = mapM_ (\l -> (mapM_ (\x -> doAnalysis (checkOccurences x) l) (getAllRedirs list))) list - where checkOccurences t@(T_NormalWord exceptId x) u@(T_NormalWord newId y) = - when (exceptId /= newId - && x == y - && not (isOutput t && isOutput u) - && not (special t)) $ do - let note = Note InfoC 2094 $ "Make sure not to read and write the same file in the same pipeline." - addNoteFor newId $ note - addNoteFor exceptId $ note - checkOccurences _ _ = return () - getAllRedirs l = concatMap (\(T_Redirecting _ ls _) -> concatMap getRedirs ls) l - getRedirs (T_FdRedirect _ _ (T_IoFile _ op file)) = - case op of T_Greater _ -> [file] - T_Less _ -> [file] - T_DGREAT _ -> [file] - _ -> [] - getRedirs _ = [] - special x = "/dev/" `isPrefixOf` (concat $ deadSimple x) - isOutput t = - case drop 1 $ getPath parents t of - (T_IoFile _ op _):_ -> - case op of - T_Greater _ -> True - T_DGREAT _ -> True - _ -> False - _ -> False + where + note x = Note x InfoC 2094 $ + "Make sure not to read and write the same file in the same pipeline." + checkOccurences t@(T_NormalWord exceptId x) u@(T_NormalWord newId y) = + when (exceptId /= newId + && x == y + && not (isOutput t && isOutput u) + && not (special t)) $ do + addNote $ note newId + addNote $ note exceptId + checkOccurences _ _ = return () + getAllRedirs l = concatMap (\(T_Redirecting _ ls _) -> concatMap getRedirs ls) l + getRedirs (T_FdRedirect _ _ (T_IoFile _ op file)) = + case op of T_Greater _ -> [file] + T_Less _ -> [file] + T_DGREAT _ -> [file] + _ -> [] + getRedirs _ = [] + special x = "/dev/" `isPrefixOf` (concat $ deadSimple x) + isOutput t = + case drop 1 $ getPath (parentMap params) t of + (T_IoFile _ op _):_ -> + case op of + T_Greater _ -> True + T_DGREAT _ -> True + _ -> False + _ -> False checkRedirectToSame _ _ = return () @@ -688,7 +715,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 @@ -696,13 +723,13 @@ name <- getCommandBasename t return $ name `elem` ["echo", "exit"]) isOk _ = False -checkShorthandIf _ = return () +checkShorthandIf _ _ = return () prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done" -checkDollarStar (T_NormalWord _ [(T_DollarBraced id l)]) | (bracedString l) == "*" = +checkDollarStar _ (T_NormalWord _ [(T_DollarBraced id l)]) | (bracedString l) == "*" = warn id 2048 $ "Use \"$@\" (with quotes) to prevent whitespace problems." -checkDollarStar _ = return () +checkDollarStar _ _ = return () prop_checkUnquotedDollarAt = verify checkUnquotedDollarAt "ls $@" @@ -711,17 +738,17 @@ prop_checkUnquotedDollarAt3 = verifyNot checkUnquotedDollarAt "ls ${#foo[@]}" prop_checkUnquotedDollarAt4 = verifyNot checkUnquotedDollarAt "ls \"$@\"" prop_checkUnquotedDollarAt5 = verifyNot checkUnquotedDollarAt "ls ${foo/@/ at }" -checkUnquotedDollarAt (T_NormalWord _ [T_DollarBraced id l]) = +checkUnquotedDollarAt _ (T_NormalWord _ [T_DollarBraced id l]) = let string = bracedString l failing = err id 2068 $ "Add double quotes around ${" ++ string ++ "}, otherwise it's just like $* and breaks on spaces." in do when ("@" `isPrefixOf` string) failing when (not ("#" `isPrefixOf` string) && "[@]" `isInfixOf` string) failing -checkUnquotedDollarAt _ = return () +checkUnquotedDollarAt _ _ = return () prop_checkStderrRedirect = verify checkStderrRedirect "test 2>&1 > cow" prop_checkStderrRedirect2 = verifyNot checkStderrRedirect "test > cow 2>&1" -checkStderrRedirect (T_Redirecting _ [ +checkStderrRedirect _ (T_Redirecting _ [ T_FdRedirect id "2" (T_IoFile _ (T_GREATAND _) (T_NormalWord _ [T_Literal _ "1"])), T_FdRedirect _ _ (T_IoFile _ op _) ] _) = case op of @@ -729,29 +756,37 @@ T_DGREAT _ -> error _ -> return () where error = err id 2069 $ "The order of the 2>&1 and the redirect matters. The 2>&1 has to be last." -checkStderrRedirect _ = return () +checkStderrRedirect _ _ = return () lt x = trace ("FAILURE " ++ (show x)) x ltt t x = trace ("FAILURE " ++ (show t)) x -prop_checkSingleQuotedVariables = verifyTree checkSingleQuotedVariables "echo '$foo'" -prop_checkSingleQuotedVariables2 = verifyTree checkSingleQuotedVariables "echo 'lol$1.jpg'" -prop_checkSingleQuotedVariables3 = verifyNotTree checkSingleQuotedVariables "sed 's/foo$/bar/'" -prop_checkSingleQuotedVariables3a= verifyTree checkSingleQuotedVariables "sed 's/${foo}/bar/'" -prop_checkSingleQuotedVariables3b= verifyTree checkSingleQuotedVariables "sed 's/$(echo cow)/bar/'" -prop_checkSingleQuotedVariables3c= verifyTree checkSingleQuotedVariables "sed 's/$((1+foo))/bar/'" -prop_checkSingleQuotedVariables4 = verifyNotTree checkSingleQuotedVariables "awk '{print $1}'" -prop_checkSingleQuotedVariables5 = verifyNotTree checkSingleQuotedVariables "trap 'echo $SECONDS' EXIT" -checkSingleQuotedVariables t@(T_SingleQuoted id s) parents = - case matchRegex re s of - Just [] -> unless (probablyOk t) $ info id 2016 $ "Expressions don't expand in single quotes, use double quotes for that." - _ -> return () +prop_checkSingleQuotedVariables = verify checkSingleQuotedVariables "echo '$foo'" +prop_checkSingleQuotedVariables2 = verify checkSingleQuotedVariables "echo 'lol$1.jpg'" +prop_checkSingleQuotedVariables3 = verifyNot checkSingleQuotedVariables "sed 's/foo$/bar/'" +prop_checkSingleQuotedVariables3a= verify checkSingleQuotedVariables "sed 's/${foo}/bar/'" +prop_checkSingleQuotedVariables3b= verify checkSingleQuotedVariables "sed 's/$(echo cow)/bar/'" +prop_checkSingleQuotedVariables3c= verify checkSingleQuotedVariables "sed 's/$((1+foo))/bar/'" +prop_checkSingleQuotedVariables4 = verifyNot checkSingleQuotedVariables "awk '{print $1}'" +prop_checkSingleQuotedVariables5 = verifyNot checkSingleQuotedVariables "trap 'echo $SECONDS' EXIT" +prop_checkSingleQuotedVariables6 = verifyNot checkSingleQuotedVariables "sed -n '$p'" +prop_checkSingleQuotedVariables6a= verify checkSingleQuotedVariables "sed -n '$pattern'" +checkSingleQuotedVariables params t@(T_SingleQuoted id s) = + when (s `matches` re) $ + if "sed" == commandName + then unless (s `matches` sedContra) showMessage + else unless isProbablyOk showMessage where - probablyOk t = fromMaybe False $ do + parents = parentMap params + showMessage = info id 2016 $ + "Expressions don't expand in single quotes, use double quotes for that." + commandName = fromMaybe "" $ do cmd <- getClosestCommand parents t name <- getCommandBasename cmd - return $ name `elem` [ + return name + + isProbablyOk = commandName `elem` [ "trap" ,"sh" ,"bash" @@ -759,19 +794,20 @@ ,"zsh" ,"ssh" ] - || "awk" `isSuffixOf` name - || "perl" `isPrefixOf` name + || "awk" `isSuffixOf` commandName + || "perl" `isPrefixOf` commandName re = mkRegex "\\$[{(0-9a-zA-Z_]" + sedContra = mkRegex "\\$[dp]($|[^a-zA-Z])" checkSingleQuotedVariables _ _ = return () prop_checkUnquotedN = verify checkUnquotedN "if [ -n $foo ]; then echo cow; fi" prop_checkUnquotedN2 = verify checkUnquotedN "[ -n $cow ]" prop_checkUnquotedN3 = verifyNot checkUnquotedN "[[ -n $foo ]] && echo cow" -checkUnquotedN (T_Condition _ SingleBracket (TC_Unary _ SingleBracket "-n" (T_NormalWord id [t]))) | willSplit t = +checkUnquotedN _ (T_Condition _ SingleBracket (TC_Unary _ SingleBracket "-n" (T_NormalWord id [t]))) | willSplit t = err id 2070 "Always true because you failed to quote. Use [[ ]] instead." -checkUnquotedN _ = return () +checkUnquotedN _ _ = return () prop_checkNumberComparisons1 = verify checkNumberComparisons "[[ $foo < 3 ]]" prop_checkNumberComparisons2 = verify checkNumberComparisons "[[ 0 >= $(cmd) ]]" @@ -779,7 +815,7 @@ 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 +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] @@ -801,50 +837,63 @@ eqv ">=" = "-ge" eqv _ = "the numerical equivalent" floatRegex = mkRegex "^[0-9]+\\.[0-9]+$" -checkNumberComparisons _ = return () +checkNumberComparisons _ _ = return () prop_checkSingleBracketOperators1 = verify checkSingleBracketOperators "[ test =~ foo ]" prop_checkSingleBracketOperators2 = verify checkSingleBracketOperators "[ $foo > $bar ]" prop_checkSingleBracketOperators3 = verifyNot checkSingleBracketOperators "[[ foo < bar ]]" prop_checkSingleBracketOperators5 = verify checkSingleBracketOperators "until [ $n <= $z ]; do echo foo; done" -checkSingleBracketOperators (TC_Binary id typ op lhs rhs) +checkSingleBracketOperators _ (TC_Binary id typ op lhs rhs) | typ == SingleBracket && op `elem` ["<", ">", "<=", ">="] = err id 2073 $ "Can't use " ++ op ++" in [ ]. Escape it or use [[..]]." -checkSingleBracketOperators (TC_Binary id typ op lhs rhs) +checkSingleBracketOperators _ (TC_Binary id typ op lhs rhs) | typ == SingleBracket && op == "=~" = err id 2074 $ "Can't use " ++ op ++" in [ ]. Use [[..]] instead." -checkSingleBracketOperators _ = return () +checkSingleBracketOperators _ _ = return () prop_checkDoubleBracketOperators1 = verify checkDoubleBracketOperators "[[ 3 \\< 4 ]]" prop_checkDoubleBracketOperators3 = verifyNot checkDoubleBracketOperators "[[ foo < bar ]]" -checkDoubleBracketOperators x@(TC_Binary id typ op lhs rhs) +checkDoubleBracketOperators _ x@(TC_Binary id typ op lhs rhs) | typ == DoubleBracket && op `elem` ["\\<", "\\>", "\\<=", "\\>="] = err id 2075 $ "Escaping " ++ op ++" is required in [..], but invalid in [[..]]" -checkDoubleBracketOperators _ = return () +checkDoubleBracketOperators _ _ = return () +prop_checkConditionalAndOrs1 = verify checkConditionalAndOrs "[ foo && bar ]" +prop_checkConditionalAndOrs2 = verify checkConditionalAndOrs "[[ foo -o bar ]]" +prop_checkConditionalAndOrs3 = verifyNot checkConditionalAndOrs "[[ foo || bar ]]" +checkConditionalAndOrs _ (TC_And id SingleBracket "&&" _ _) = + err id 2107 "You can't use && inside [..]. Use -a instead." +checkConditionalAndOrs _ (TC_And id DoubleBracket "-a" _ _) = + err id 2108 "In [[..]], use && instead of -a." +checkConditionalAndOrs _ (TC_Or id SingleBracket "||" _ _) = + err id 2109 "You can't use || inside [..]. Use -o instead." +checkConditionalAndOrs _ (TC_Or id DoubleBracket "-o" _ _) = + err id 2110 "In [[..]], use || instead of -o." +checkConditionalAndOrs _ _ = return () + prop_checkQuotedCondRegex1 = verify checkQuotedCondRegex "[[ $foo =~ \"bar\" ]]" prop_checkQuotedCondRegex2 = verify checkQuotedCondRegex "[[ $foo =~ 'cow' ]]" prop_checkQuotedCondRegex3 = verifyNot checkQuotedCondRegex "[[ $foo =~ $foo ]]" -checkQuotedCondRegex (TC_Binary _ _ "=~" _ rhs) = +checkQuotedCondRegex _ (TC_Binary _ _ "=~" _ rhs) = case rhs of T_NormalWord id [T_DoubleQuoted _ _] -> error id T_NormalWord id [T_SingleQuoted _ _] -> error id _ -> return () where error id = err id 2076 $ "Don't quote rhs of =~, it'll match literally rather than as a regex." -checkQuotedCondRegex _ = return () +checkQuotedCondRegex _ _ = return () prop_checkGlobbedRegex1 = verify checkGlobbedRegex "[[ $foo =~ *foo* ]]" prop_checkGlobbedRegex2 = verify checkGlobbedRegex "[[ $foo =~ f* ]]" prop_checkGlobbedRegex2a = verify checkGlobbedRegex "[[ $foo =~ \\#* ]]" prop_checkGlobbedRegex3 = verifyNot checkGlobbedRegex "[[ $foo =~ $foo ]]" prop_checkGlobbedRegex4 = verifyNot checkGlobbedRegex "[[ $foo =~ ^c.* ]]" -checkGlobbedRegex (TC_Binary _ DoubleBracket "=~" _ rhs) = +checkGlobbedRegex _ (TC_Binary _ DoubleBracket "=~" _ rhs) = let s = concat $ deadSimple rhs in if isConfusedGlobRegex s then warn (getId rhs) 2049 $ "=~ is for regex. Use == for globs." else return () -checkGlobbedRegex _ = return () +checkGlobbedRegex _ _ = return () prop_checkConstantIfs1 = verify checkConstantIfs "[[ foo != bar ]]" @@ -852,45 +901,45 @@ prop_checkConstantIfs3 = verify checkConstantIfs "[[ $n -le 4 && n -ge 2 ]]" prop_checkConstantIfs4 = verifyNot checkConstantIfs "[[ $n -le 3 ]]" prop_checkConstantIfs5 = verifyNot checkConstantIfs "[[ $n -le $n ]]" -checkConstantIfs (TC_Binary id typ op lhs rhs) +checkConstantIfs _ (TC_Binary id typ op lhs rhs) | op `elem` [ "==", "!=", "<=", ">=", "-eq", "-ne", "-lt", "-le", "-gt", "-ge", "=~", ">", "<", "="] = do when (isJust lLit && isJust rLit) $ warn id 2050 $ "This expression is constant. Did you forget the $ on a variable?" where lLit = getLiteralString lhs rLit = getLiteralString rhs -checkConstantIfs _ = return () +checkConstantIfs _ _ = return () prop_checkNoaryWasBinary = verify checkNoaryWasBinary "[[ a==$foo ]]" prop_checkNoaryWasBinary2 = verify checkNoaryWasBinary "[ $foo=3 ]" prop_checkNoaryWasBinary3 = verify checkNoaryWasBinary "[ $foo!=3 ]" -checkNoaryWasBinary (TC_Noary _ _ t@(T_NormalWord id l)) | not $ isConstant t = do +checkNoaryWasBinary _ (TC_Noary _ _ t@(T_NormalWord id l)) | not $ isConstant t = do let str = concat $ deadSimple t when ('=' `elem` str) $ err id 2077 $ "You need spaces around the comparison operator." -checkNoaryWasBinary _ = return () +checkNoaryWasBinary _ _ = return () prop_checkConstantNoary = verify checkConstantNoary "[[ '$(foo)' ]]" prop_checkConstantNoary2 = verify checkConstantNoary "[ \"-f lol\" ]" prop_checkConstantNoary3 = verify checkConstantNoary "[[ cmd ]]" prop_checkConstantNoary4 = verify checkConstantNoary "[[ ! cmd ]]" -checkConstantNoary (TC_Noary _ _ t@(T_NormalWord id _)) | isConstant t = do +checkConstantNoary _ (TC_Noary _ _ t@(T_NormalWord id _)) | isConstant t = do err id 2078 $ "This expression is constant. Did you forget a $ somewhere?" -checkConstantNoary _ = return () +checkConstantNoary _ _ = return () prop_checkBraceExpansionVars = verify checkBraceExpansionVars "echo {1..$n}" -checkBraceExpansionVars (T_BraceExpansion id s) | '$' `elem` s = +checkBraceExpansionVars _ (T_BraceExpansion id s) | '$' `elem` s = warn id 2051 $ "Bash doesn't support variables in brace expansions." -checkBraceExpansionVars _ = return () +checkBraceExpansionVars _ _ = return () prop_checkForDecimals = verify checkForDecimals "((3.14*c))" -checkForDecimals (TA_Literal id s) | any (== '.') s = do +checkForDecimals _ (TA_Literal id s) | any (== '.') s = do err id 2079 $ "(( )) doesn't support decimals. Use bc or awk." -checkForDecimals _ = return () +checkForDecimals _ _ = return () prop_checkDivBeforeMult = verify checkDivBeforeMult "echo $((c/n*100))" prop_checkDivBeforeMult2 = verifyNot checkDivBeforeMult "echo $((c*100/n))" -checkDivBeforeMult (TA_Binary _ "*" (TA_Binary id "/" _ _) _) = do +checkDivBeforeMult _ (TA_Binary _ "*" (TA_Binary id "/" _ _) _) = do info id 2017 $ "Increase precision by replacing a/b*c with a*c/b." -checkDivBeforeMult _ = return () +checkDivBeforeMult _ _ = return () prop_checkArithmeticDeref = verify checkArithmeticDeref "echo $((3+$foo))" prop_checkArithmeticDeref2 = verify checkArithmeticDeref "cow=14; (( s+= $cow ))" @@ -898,68 +947,66 @@ 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 = +checkArithmeticDeref _ (TA_Expansion _ (T_DollarBraced id l)) | not . excepting $ bracedString l = style id 2004 $ "Don't use $ on variables in (( ))." where excepting [] = True excepting s = (any (`elem` "/.:#%?*@[]") s) || (isDigit $ head s) -checkArithmeticDeref _ = return () +checkArithmeticDeref _ _ = return () prop_checkArithmeticBadOctal1 = verify checkArithmeticBadOctal "(( 0192 ))" prop_checkArithmeticBadOctal2 = verifyNot checkArithmeticBadOctal "(( 0x192 ))" prop_checkArithmeticBadOctal3 = verifyNot checkArithmeticBadOctal "(( 1 ^ 0777 ))" -checkArithmeticBadOctal (TA_Base id "0" (TA_Literal _ str)) | '9' `elem` str || '8' `elem` str = +checkArithmeticBadOctal _ (TA_Base id "0" (TA_Literal _ str)) | '9' `elem` str || '8' `elem` str = err id 2080 $ "Numbers with leading 0 are considered octal." -checkArithmeticBadOctal _ = return () +checkArithmeticBadOctal _ _ = return () prop_checkComparisonAgainstGlob = verify checkComparisonAgainstGlob "[[ $cow == $bar ]]" prop_checkComparisonAgainstGlob2 = verifyNot checkComparisonAgainstGlob "[[ $cow == \"$bar\" ]]" prop_checkComparisonAgainstGlob3 = verify checkComparisonAgainstGlob "[ $cow = *foo* ]" prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow = foo ]" -checkComparisonAgainstGlob (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _])) | op == "=" || op == "==" = +checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _])) | op == "=" || op == "==" = warn id 2053 $ "Quote the rhs of = in [[ ]] to prevent glob interpretation." -checkComparisonAgainstGlob (TC_Binary _ SingleBracket op _ word) +checkComparisonAgainstGlob _ (TC_Binary _ SingleBracket op _ word) | (op == "=" || op == "==") && isGlob word = err (getId word) 2081 $ "[ .. ] can't match globs. Use [[ .. ]] or grep." -checkComparisonAgainstGlob _ = return () +checkComparisonAgainstGlob _ _ = return () prop_checkCommarrays1 = verify checkCommarrays "a=(1, 2)" prop_checkCommarrays2 = verify checkCommarrays "a+=(1,2,3)" prop_checkCommarrays3 = verifyNot checkCommarrays "cow=(1 \"foo,bar\" 3)" -checkCommarrays (T_Array id l) = +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 () -checkCommarrays _ = return () +checkCommarrays _ _ = return () prop_checkOrNeq1 = verify checkOrNeq "if [[ $lol -ne cow || $lol -ne foo ]]; then echo foo; fi" prop_checkOrNeq2 = verify checkOrNeq "(( a!=lol || a!=foo ))" prop_checkOrNeq3 = verify checkOrNeq "[ \"$a\" != lol || \"$a\" != foo ]" prop_checkOrNeq4 = verifyNot checkOrNeq "[ a != $cow || b != $foo ]" -- This only catches the most idiomatic cases. Fixme? -checkOrNeq (TC_Or id typ op (TC_Binary _ _ op1 word1 _) (TC_Binary _ _ op2 word2 _)) +checkOrNeq _ (TC_Or id typ op (TC_Binary _ _ op1 word1 _) (TC_Binary _ _ op2 word2 _)) | word1 == word2 && (op1 == op2 && (op1 == "-ne" || op1 == "!=")) = warn id 2055 $ "You probably wanted " ++ (if typ == SingleBracket then "-a" else "&&") ++ " here." -checkOrNeq (TA_Binary id "||" (TA_Binary _ "!=" word1 _) (TA_Binary _ "!=" word2 _)) +checkOrNeq _ (TA_Binary id "||" (TA_Binary _ "!=" word1 _) (TA_Binary _ "!=" word2 _)) | word1 == word2 = warn id 2056 "You probably wanted && here." -checkOrNeq _ = return () +checkOrNeq _ _ = return () -allModifiedVariables t = snd $ runState (doAnalysis (\x -> modify $ (++) (getModifiedVariables x)) t) [] - prop_checkValidCondOps1 = verify checkValidCondOps "[[ a -xz b ]]" prop_checkValidCondOps2 = verify checkValidCondOps "[ -M a ]" prop_checkValidCondOps2a= verifyNot checkValidCondOps "[ 3 \\> 2 ]" prop_checkValidCondOps3 = verifyNot checkValidCondOps "[ 1 = 2 -a 3 -ge 4 ]" prop_checkValidCondOps4 = verifyNot checkValidCondOps "[[ ! -v foo ]]" -checkValidCondOps (TC_Binary id _ s _ _) +checkValidCondOps _ (TC_Binary id _ s _ _) | not (s `elem` ["-nt", "-ot", "-ef", "==", "!=", "<=", ">=", "-eq", "-ne", "-lt", "-le", "-gt", "-ge", "=~", ">", "<", "=", "\\<", "\\>", "\\<=", "\\>="]) = warn id 2057 "Unknown binary operator." -checkValidCondOps (TC_Unary id _ s _) +checkValidCondOps _ (TC_Unary id _ s _) | not (s `elem` [ "!", "-a", "-b", "-c", "-d", "-e", "-f", "-g", "-h", "-L", "-k", "-p", "-r", "-s", "-S", "-t", "-u", "-w", "-x", "-O", "-G", "-N", "-z", "-n", "-o", "-v", "-R"]) = warn id 2058 "Unknown unary operator." -checkValidCondOps _ = return () +checkValidCondOps _ _ = return () --- Context seeking @@ -1090,7 +1137,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" f where f (dashv:var:rest) | getLiteralString dashv == (Just "-v") = f rest f (format:params) = check format f _ = return () @@ -1105,7 +1152,7 @@ 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 +checkUuoe _ = checkUnqualifiedCommand "echo" 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 @@ -1126,22 +1173,21 @@ 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" (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." - f word = case getLiteralString word of - Just "a-z" -> info (getId word) 2018 "Use '[:lower:]' to support accents and foreign alphabets." - Just "A-Z" -> info (getId word) 2019 "Use '[:upper:]' to support accents and foreign alphabets." - - Just s -> do -- Eliminate false positives by only looking for dupes in SET2? - when ((not $ "-" `isPrefixOf` s || "[:" `isInfixOf` s) && duplicated s) $ - info (getId word) 2020 "tr replaces sets of chars, not words (mentioned due to duplicates)." - - unless ("[:" `isPrefixOf` s) $ - when ("[" `isPrefixOf` s && "]" `isSuffixOf` s && (length s > 2) && (not $ '*' `elem` s)) $ - info (getId word) 2021 "Don't use [] around ranges in tr, it replaces literal square brackets." - Nothing -> return () + f word = + case getLiteralString word of + Just "a-z" -> info (getId word) 2018 "Use '[:lower:]' to support accents and foreign alphabets." + Just "A-Z" -> info (getId word) 2019 "Use '[:upper:]' to support accents and foreign alphabets." + Just s -> do -- Eliminate false positives by only looking for dupes in SET2? + when ((not $ "-" `isPrefixOf` s || "[:" `isInfixOf` s) && duplicated s) $ + info (getId word) 2020 "tr replaces sets of chars, not words (mentioned due to duplicates)." + unless ("[:" `isPrefixOf` s) $ + when ("[" `isPrefixOf` s && "]" `isSuffixOf` s && (length s > 2) && (not $ '*' `elem` s)) $ + info (getId word) 2021 "Don't use [] around ranges in tr, it replaces literal square brackets." + Nothing -> return () duplicated s = let relevant = filter isAlpha s @@ -1151,7 +1197,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" f where acceptsGlob (Just s) = s `elem` [ "-ilname", "-iname", "-ipath", "-iregex", "-iwholename", "-lname", "-name", "-path", "-regex", "-wholename" ] acceptsGlob _ = False f [] = return () @@ -1173,7 +1219,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" f where -- --regex=*(extglob) doesn't work. Fixme? skippable (Just s) = not ("--regex=" `isPrefixOf` s) && "-" `isPrefixOf` s skippable _ = False @@ -1195,7 +1241,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" f where f (x:_) = checkTrap x f _ = return () checkTrap (T_NormalWord _ [T_DoubleQuoted _ rs]) = mapM_ checkExpansions rs @@ -1210,7 +1256,7 @@ prop_checkTimeParameters1 = verify checkTimeParameters "time -f lol sleep 10" prop_checkTimeParameters2 = verifyNot checkTimeParameters "time sleep 10" prop_checkTimeParameters3 = verifyNot checkTimeParameters "time -p foo" -checkTimeParameters = checkUnqualifiedCommand "time" f where +checkTimeParameters _ = checkUnqualifiedCommand "time" f where f (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." @@ -1220,9 +1266,9 @@ prop_checkTestRedirects1 = verify checkTestRedirects "test 3 > 1" prop_checkTestRedirects2 = verifyNot checkTestRedirects "test 3 \\> 1" prop_checkTestRedirects3 = verify checkTestRedirects "/usr/bin/test $var > $foo" -checkTestRedirects (T_Redirecting id redirs@(redir:_) cmd) | cmd `isCommand` "test" = +checkTestRedirects _ (T_Redirecting id redirs@(redir:_) cmd) | cmd `isCommand` "test" = warn (getId redir) 2065 $ "This is interpretted as a shell file redirection, not a comparison." -checkTestRedirects _ = return () +checkTestRedirects _ _ = return () prop_checkSudoRedirect1 = verify checkSudoRedirect "sudo echo 3 > /proc/file" prop_checkSudoRedirect2 = verify checkSudoRedirect "sudo cmd < input" @@ -1231,7 +1277,7 @@ prop_checkSudoRedirect5 = verifyNot checkSudoRedirect "sudo cmd 2>&1" prop_checkSudoRedirect6 = verifyNot checkSudoRedirect "sudo cmd 2> log" prop_checkSudoRedirect7 = verifyNot checkSudoRedirect "sudo cmd > /dev/null 2>&1" -checkSudoRedirect (T_Redirecting _ redirs cmd) | cmd `isCommand` "sudo" = +checkSudoRedirect _ (T_Redirecting _ redirs cmd) | cmd `isCommand` "sudo" = mapM_ warnAbout redirs where warnAbout (T_FdRedirect _ s (T_IoFile id op file)) @@ -1249,7 +1295,7 @@ _ -> return () warnAbout _ = return () special file = (concat $ deadSimple file) == "/dev/null" -checkSudoRedirect _ = return () +checkSudoRedirect _ _ = return () prop_checkPS11 = verify checkPS1Assignments "PS1='\\033[1;35m\\$ '" prop_checkPS11a= verify checkPS1Assignments "export PS1='\\033[1;35m\\$ '" @@ -1261,7 +1307,7 @@ prop_checkPS16 = verifyNot checkPS1Assignments "PS1='\\[\\e1m\\e[1m\\]\\$ '" prop_checkPS17 = verifyNot checkPS1Assignments "PS1='e033x1B'" prop_checkPS18 = verifyNot checkPS1Assignments "PS1='\\[\\e\\]'" -checkPS1Assignments (T_Assignment _ _ "PS1" _ word) = warnFor word +checkPS1Assignments _ (T_Assignment _ _ "PS1" _ word) = warnFor word where warnFor word = let contents = concat $ deadSimple word in @@ -1272,20 +1318,20 @@ isJust $ matchRegex escapeRegex unenclosed enclosedRegex = mkRegex "\\\\\\[.*\\\\\\]" -- FIXME: shouldn't be eager escapeRegex = mkRegex "\\\\x1[Bb]|\\\\e|\x1B|\\\\033" -checkPS1Assignments _ = return () +checkPS1Assignments _ _ = return () prop_checkBackticks1 = verify checkBackticks "echo `foo`" prop_checkBackticks2 = verifyNot checkBackticks "echo $(foo)" -checkBackticks (T_Backticked id _) = +checkBackticks _ (T_Backticked id _) = style id 2006 "Use $(..) instead of deprecated `..`" -checkBackticks _ = return () +checkBackticks _ _ = return () prop_checkIndirectExpansion1 = verify checkIndirectExpansion "${foo$n}" prop_checkIndirectExpansion2 = verifyNot checkIndirectExpansion "${foo//$n/lol}" prop_checkIndirectExpansion3 = verify checkIndirectExpansion "${$#}" prop_checkIndirectExpansion4 = verify checkIndirectExpansion "${var${n}_$((i%2))}" prop_checkIndirectExpansion5 = verifyNot checkIndirectExpansion "${bar}" -checkIndirectExpansion (T_DollarBraced i (T_NormalWord _ contents)) = +checkIndirectExpansion _ (T_DollarBraced i (T_NormalWord _ contents)) = when (isIndirection contents) $ err i 2082 "To expand via indirection, use name=\"foo$n\"; echo \"${!name}\"." where @@ -1302,12 +1348,12 @@ else Just False _ -> Just False -checkIndirectExpansion _ = return () +checkIndirectExpansion _ _ = return () prop_checkInexplicablyUnquoted1 = verify checkInexplicablyUnquoted "echo 'var='value';'" prop_checkInexplicablyUnquoted2 = verifyNot checkInexplicablyUnquoted "'foo'*" prop_checkInexplicablyUnquoted3 = verifyNot checkInexplicablyUnquoted "wget --user-agent='something'" -checkInexplicablyUnquoted (T_NormalWord id tokens) = mapM_ check (tails tokens) +checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens) where check ((T_SingleQuoted _ _):(T_Literal id str):_) | all isAlphaNum str = @@ -1322,14 +1368,14 @@ check _ = return () warnAbout id = info id 2027 $ "Surrounding quotes actually unquotes this (\"inside\"$outside\"inside\"). Did you forget your quote level?" -checkInexplicablyUnquoted _ = return () +checkInexplicablyUnquoted _ _ = return () prop_checkTildeInQuotes1 = verify checkTildeInQuotes "var=\"~/out.txt\"" prop_checkTildeInQuotes2 = verify checkTildeInQuotes "foo > '~/dir'" prop_checkTildeInQuotes4 = verifyNot checkTildeInQuotes "~/file" prop_checkTildeInQuotes5 = verifyNot checkTildeInQuotes "echo '/~foo/cow'" prop_checkTildeInQuotes6 = verifyNot checkTildeInQuotes "awk '$0 ~ /foo/'" -checkTildeInQuotes = check +checkTildeInQuotes _ = check where verify id ('~':_) = warn id 2088 "Note that ~ does not expand in quotes." verify _ _ = return () @@ -1341,10 +1387,10 @@ prop_checkLonelyDotDash1 = verify checkLonelyDotDash "./ file" prop_checkLonelyDotDash2 = verifyNot checkLonelyDotDash "./file" -checkLonelyDotDash t@(T_Redirecting id _ _) +checkLonelyDotDash _ t@(T_Redirecting id _ _) | isUnqualifiedCommand t "./" = err id 2083 "Don't add spaces after the slash in './file'." -checkLonelyDotDash _ = return () +checkLonelyDotDash _ _ = return () prop_checkSpuriousExec1 = verify checkSpuriousExec "exec foo; true" @@ -1354,13 +1400,14 @@ prop_checkSpuriousExec5 = verifyNot checkSpuriousExec "exec > file; cmd" prop_checkSpuriousExec6 = verify checkSpuriousExec "exec foo > file; cmd" prop_checkSpuriousExec7 = verifyNot checkSpuriousExec "exec file; echo failed; exit 3" -checkSpuriousExec = doLists +checkSpuriousExec _ = doLists where doLists (T_Script _ _ cmds) = doList cmds doLists (T_BraceGroup _ cmds) = doList cmds doLists (T_WhileExpression _ _ cmds) = doList cmds doLists (T_UntilExpression _ _ cmds) = doList cmds doLists (T_ForIn _ _ _ cmds) = doList cmds + doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds doLists (T_IfExpression _ thens elses) = do mapM_ (\(_, l) -> doList l) thens doList elses @@ -1391,7 +1438,7 @@ prop_checkSpuriousExpansion2 = verify checkSpuriousExpansion "while \"$(cmd)\"; do :; done" prop_checkSpuriousExpansion3 = verifyNot checkSpuriousExpansion "$(cmd) --flag1 --flag2" prop_checkSpuriousExpansion4 = verify checkSpuriousExpansion "$((i++))" -checkSpuriousExpansion (T_SimpleCommand _ _ [T_NormalWord _ [word]]) = check word +checkSpuriousExpansion _ (T_SimpleCommand _ _ [T_NormalWord _ [word]]) = check word where check word = case word of T_DollarExpansion id _ -> @@ -1402,14 +1449,14 @@ err id 2084 "Remove '$' or use '_=$((expr))' to avoid executing output." T_DoubleQuoted id [subword] -> check subword _ -> return () -checkSpuriousExpansion _ = return () +checkSpuriousExpansion _ _ = return () prop_checkUnusedEchoEscapes1 = verify checkUnusedEchoEscapes "echo 'foo\\nbar\\n'" 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" f where isDashE = mkRegex "^-.*e" hasEscapes = mkRegex "\\\\[rnt]" @@ -1431,13 +1478,13 @@ prop_checkDollarBrackets1 = verify checkDollarBrackets "echo $[1+2]" prop_checkDollarBrackets2 = verifyNot checkDollarBrackets "echo $((1+2))" -checkDollarBrackets (T_DollarBracket id _) = +checkDollarBrackets _ (T_DollarBracket id _) = style id 2007 "Use $((..)) instead of deprecated $[..]" -checkDollarBrackets _ = return () +checkDollarBrackets _ _ = return () prop_checkSshHereDoc1 = verify checkSshHereDoc "ssh host << foo\necho $PATH\nfoo" prop_checkSshHereDoc2 = verifyNot checkSshHereDoc "ssh host << 'foo'\necho $PATH\nfoo" -checkSshHereDoc (T_Redirecting _ redirs cmd) +checkSshHereDoc _ (T_Redirecting _ redirs cmd) | cmd `isCommand` "ssh" = mapM_ checkHereDoc redirs where @@ -1446,13 +1493,13 @@ | not (all isConstant tokens) = warn id 2087 $ "Quote '" ++ token ++ "' to make here document expansions happen on the server side rather than on the client." checkHereDoc _ = return () -checkSshHereDoc _ = return () +checkSshHereDoc _ _ = return () -- This is hard to get right without properly parsing ssh args 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" f where nonOptions args = filter (\x -> not $ "-" `isPrefixOf` (concat $ deadSimple x)) args @@ -1469,23 +1516,26 @@ --- Subshell detection -prop_subshellAssignmentCheck = verifyFull subshellAssignmentCheck "cat foo | while read bar; do a=$bar; done; echo \"$a\"" -prop_subshellAssignmentCheck2 = verifyNotFull subshellAssignmentCheck "while read bar; do a=$bar; done < file; echo \"$a\"" -prop_subshellAssignmentCheck3 = verifyFull subshellAssignmentCheck "( A=foo; ); rm $A" -prop_subshellAssignmentCheck4 = verifyNotFull subshellAssignmentCheck "( A=foo; rm $A; )" -prop_subshellAssignmentCheck5 = verifyFull subshellAssignmentCheck "cat foo | while read cow; do true; done; echo $cow;" -prop_subshellAssignmentCheck6 = verifyFull subshellAssignmentCheck "( export lol=$(ls); ); echo $lol;" -prop_subshellAssignmentCheck6a= verifyFull subshellAssignmentCheck "( typeset -a lol=a; ); echo $lol;" -prop_subshellAssignmentCheck7 = verifyFull subshellAssignmentCheck "cmd | while read foo; do (( n++ )); done; echo \"$n lines\"" -prop_subshellAssignmentCheck8 = verifyFull subshellAssignmentCheck "n=3 & echo $((n++))" -prop_subshellAssignmentCheck9 = verifyFull subshellAssignmentCheck "read n & n=foo$n" -prop_subshellAssignmentCheck10 = verifyFull subshellAssignmentCheck "(( n <<= 3 )) & (( n |= 4 )) &" -prop_subshellAssignmentCheck11 = verifyFull subshellAssignmentCheck "cat /etc/passwd | while read line; do let n=n+1; done\necho $n" -prop_subshellAssignmentCheck12 = verifyFull subshellAssignmentCheck "cat /etc/passwd | while read line; do let ++n; done\necho $n" -subshellAssignmentCheck t = - let flow = getVariableFlow t +prop_subshellAssignmentCheck = verifyTree subshellAssignmentCheck "cat foo | while read bar; do a=$bar; done; echo \"$a\"" +prop_subshellAssignmentCheck2 = verifyNotTree subshellAssignmentCheck "while read bar; do a=$bar; done < file; echo \"$a\"" +prop_subshellAssignmentCheck3 = verifyTree subshellAssignmentCheck "( A=foo; ); rm $A" +prop_subshellAssignmentCheck4 = verifyNotTree subshellAssignmentCheck "( A=foo; rm $A; )" +prop_subshellAssignmentCheck5 = verifyTree subshellAssignmentCheck "cat foo | while read cow; do true; done; echo $cow;" +prop_subshellAssignmentCheck6 = verifyTree subshellAssignmentCheck "( export lol=$(ls); ); echo $lol;" +prop_subshellAssignmentCheck6a= verifyTree subshellAssignmentCheck "( typeset -a lol=a; ); echo $lol;" +prop_subshellAssignmentCheck7 = verifyTree subshellAssignmentCheck "cmd | while read foo; do (( n++ )); done; echo \"$n lines\"" +prop_subshellAssignmentCheck8 = verifyTree subshellAssignmentCheck "n=3 & echo $((n++))" +prop_subshellAssignmentCheck9 = verifyTree subshellAssignmentCheck "read n & n=foo$n" +prop_subshellAssignmentCheck10 = verifyTree subshellAssignmentCheck "(( n <<= 3 )) & (( n |= 4 )) &" +prop_subshellAssignmentCheck11 = verifyTree subshellAssignmentCheck "cat /etc/passwd | while read line; do let n=n+1; done\necho $n" +prop_subshellAssignmentCheck12 = verifyTree subshellAssignmentCheck "cat /etc/passwd | while read line; do let ++n; done\necho $n" +prop_subshellAssignmentCheck13 = verifyTree subshellAssignmentCheck "#!/bin/bash\necho foo | read bar; echo $bar" +prop_subshellAssignmentCheck14 = verifyNotTree subshellAssignmentCheck "#!/bin/ksh93\necho foo | read bar; echo $bar" +prop_subshellAssignmentCheck15 = verifyNotTree subshellAssignmentCheck "#!/bin/zsh\ncat foo | while read bar; do a=$bar; done\necho \"$a\"" +subshellAssignmentCheck params t = + let flow = variableFlow params check = findSubshelled flow [("oops",[])] Map.empty - in snd $ runState check [] + in snd $ runWriter check data Scope = SubshellScope String | NoneScope deriving (Show, Eq) @@ -1501,17 +1551,39 @@ data VariableState = Dead Token String | Alive deriving (Show, Eq) -leadType t = +leadType shell parents t = case t of T_DollarExpansion _ _ -> SubshellScope "$(..) expansion" T_Backticked _ _ -> SubshellScope "`..` expansion" T_Backgrounded _ _ -> SubshellScope "backgrounding &" T_Subshell _ _ -> SubshellScope "(..) group" - -- This considers the whole pipeline one subshell. Consider fixing. - T_Pipeline _ (_:_:[]) -> SubshellScope "pipeline" + T_Redirecting _ _ _ -> + if fromMaybe False causesSubshell + then SubshellScope "pipeline" + else NoneScope _ -> NoneScope + where + parentPipeline = do + parent <- Map.lookup (getId t) parents + case parent of + T_Pipeline _ _ -> return parent + _ -> Nothing + causesSubshell = do + (T_Pipeline _ list) <- parentPipeline + if length list <= 1 + then return False + else if lastCreatesSubshell + then return True + else return . not $ (getId . head $ reverse list) == (getId t) + lastCreatesSubshell = + case shell of + Bash -> True + Sh -> True + Ksh -> False + Zsh -> False + getModifiedVariables t = case t of T_SimpleCommand _ vars [] -> @@ -1597,18 +1669,18 @@ T_Assignment id Append str _ _ -> [(t, t, str)] x -> getReferencedVariableCommand x -getVariableFlow t = +getVariableFlow shell parents t = let (_, stack) = runState (doStackAnalysis startScope endScope t) [] in reverse stack where startScope t = - let scopeType = leadType t + let scopeType = leadType shell parents t in do when (scopeType /= NoneScope) $ modify ((StackScope scopeType):) if assignFirst t then setWritten t else return () endScope t = - let scopeType = leadType t + let scopeType = leadType shell parents t in do setRead t if assignFirst t then return () else setWritten t @@ -1645,11 +1717,10 @@ foldl (\m (_, token, var, _) -> Map.insert var (Dead token reason) m) deadVars scope -doVariableFlowAnalysis readFunc writeFunc empty t = fst $ runState ( +doVariableFlowAnalysis readFunc writeFunc empty flow = fst $ runState ( foldM (\list x -> do { l <- doFlow x; return $ l ++ list; }) [] flow ) empty where - flow = getVariableFlow t doFlow (Reference (base, token, name)) = readFunc base token name doFlow (Assignment (base, token, name, values)) = @@ -1657,29 +1728,29 @@ doFlow _ = return [] ---- Check whether variables could have spaces/globs -prop_checkSpacefulness0 = verifyFull checkSpacefulness "for f in *.mp3; do echo $f; done" -prop_checkSpacefulness1 = verifyFull checkSpacefulness "a='cow moo'; echo $a" -prop_checkSpacefulness2 = verifyNotFull checkSpacefulness "a='cow moo'; [[ $a ]]" -prop_checkSpacefulness3 = verifyNotFull checkSpacefulness "a='cow*.mp3'; echo \"$a\"" -prop_checkSpacefulness4 = verifyFull checkSpacefulness "for f in *.mp3; do echo $f; done" -prop_checkSpacefulness4a= verifyNotFull checkSpacefulness "foo=3; foo=$(echo $foo)" -prop_checkSpacefulness5 = verifyFull checkSpacefulness "a='*'; b=$a; c=lol${b//foo/bar}; echo $c" -prop_checkSpacefulness6 = verifyFull checkSpacefulness "a=foo$(lol); echo $a" -prop_checkSpacefulness7 = verifyFull checkSpacefulness "a=foo\\ bar; rm $a" -prop_checkSpacefulness8 = verifyNotFull checkSpacefulness "a=foo\\ bar; a=foo; rm $a" -prop_checkSpacefulnessA = verifyFull checkSpacefulness "rm $1" -prop_checkSpacefulnessB = verifyFull checkSpacefulness "rm ${10//foo/bar}" -prop_checkSpacefulnessC = verifyNotFull checkSpacefulness "(( $1 + 3 ))" -prop_checkSpacefulnessD = verifyNotFull checkSpacefulness "if [[ $2 -gt 14 ]]; then true; fi" -prop_checkSpacefulnessE = verifyNotFull checkSpacefulness "foo=$3 env" -prop_checkSpacefulnessF = verifyNotFull checkSpacefulness "local foo=$1" -prop_checkSpacefulnessG = verifyNotFull checkSpacefulness "declare foo=$1" -prop_checkSpacefulnessH = verifyFull checkSpacefulness "echo foo=$1" -prop_checkSpacefulnessI = verifyNotFull checkSpacefulness "$1 --flags" -prop_checkSpacefulnessJ = verifyFull checkSpacefulness "echo $PWD" +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\"" +prop_checkSpacefulness4 = verifyTree checkSpacefulness "for f in *.mp3; do echo $f; done" +prop_checkSpacefulness4a= verifyNotTree checkSpacefulness "foo=3; foo=$(echo $foo)" +prop_checkSpacefulness5 = verifyTree checkSpacefulness "a='*'; b=$a; c=lol${b//foo/bar}; echo $c" +prop_checkSpacefulness6 = verifyTree checkSpacefulness "a=foo$(lol); echo $a" +prop_checkSpacefulness7 = verifyTree checkSpacefulness "a=foo\\ bar; rm $a" +prop_checkSpacefulness8 = verifyNotTree checkSpacefulness "a=foo\\ bar; a=foo; rm $a" +prop_checkSpacefulnessA = verifyTree checkSpacefulness "rm $1" +prop_checkSpacefulnessB = verifyTree checkSpacefulness "rm ${10//foo/bar}" +prop_checkSpacefulnessC = verifyNotTree checkSpacefulness "(( $1 + 3 ))" +prop_checkSpacefulnessD = verifyNotTree checkSpacefulness "if [[ $2 -gt 14 ]]; then true; fi" +prop_checkSpacefulnessE = verifyNotTree checkSpacefulness "foo=$3 env" +prop_checkSpacefulnessF = verifyNotTree checkSpacefulness "local foo=$1" +prop_checkSpacefulnessG = verifyNotTree checkSpacefulness "declare foo=$1" +prop_checkSpacefulnessH = verifyTree checkSpacefulness "echo foo=$1" +prop_checkSpacefulnessI = verifyNotTree checkSpacefulness "$1 --flags" +prop_checkSpacefulnessJ = verifyTree checkSpacefulness "echo $PWD" -checkSpacefulness t = - doVariableFlowAnalysis readF writeF (Map.fromList defaults) t +checkSpacefulness params t = + doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params) where defaults = zip variablesWithoutSpaces (repeat False) @@ -1697,7 +1768,7 @@ && (not $ isCounting token) && (not $ inUnquotableContext parents token) && (not $ usedAsCommandName parents token) - then return [(getId token, Note InfoC 2086 warning)] + then return [(Note (getId token) InfoC 2086 warning)] else return [] where warning = "Double quote to prevent globbing and word splitting." @@ -1712,7 +1783,7 @@ (isSpacefulWord (\x -> Map.findWithDefault True x map) vals) return [] - parents = getParentTree t + parents = parentMap params isCounting (T_DollarBraced id token) = case concat $ deadSimple token of @@ -1740,53 +1811,68 @@ containsAny s chars = any (\c -> c `elem` s) chars -prop_checkQuotesInLiterals1 = verifyFull checkQuotesInLiterals "param='--foo=\"bar\"'; app $param" -prop_checkQuotesInLiterals1a= verifyFull checkQuotesInLiterals "param=\"--foo='lolbar'\"; app $param" -prop_checkQuotesInLiterals2 = verifyNotFull checkQuotesInLiterals "param='--foo=\"bar\"'; app \"$param\"" -prop_checkQuotesInLiterals3 =verifyNotFull checkQuotesInLiterals "param=('--foo='); app \"${param[@]}\"" -prop_checkQuotesInLiterals4 = verifyNotFull checkQuotesInLiterals "param=\"don't bother with this one\"; app $param" -prop_checkQuotesInLiterals5 = verifyNotFull checkQuotesInLiterals "param=\"--foo='lolbar'\"; eval app $param" -checkQuotesInLiterals t = - doVariableFlowAnalysis readF writeF Map.empty t +prop_checkQuotesInLiterals1 = verifyTree checkQuotesInLiterals "param='--foo=\"bar\"'; app $param" +prop_checkQuotesInLiterals1a= verifyTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; app $param" +prop_checkQuotesInLiterals2 = verifyNotTree checkQuotesInLiterals "param='--foo=\"bar\"'; app \"$param\"" +prop_checkQuotesInLiterals3 =verifyNotTree checkQuotesInLiterals "param=('--foo='); app \"${param[@]}\"" +prop_checkQuotesInLiterals4 = verifyNotTree checkQuotesInLiterals "param=\"don't bother with this one\"; app $param" +prop_checkQuotesInLiterals5 = verifyNotTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; eval app $param" +prop_checkQuotesInLiterals6 = verifyTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm $param\"; $cmd" +prop_checkQuotesInLiterals6a= verifyNotTree checkQuotesInLiterals "param='my\\ file'; cmd=\"rm ${#param}\"; $cmd" +prop_checkQuotesInLiterals7 = verifyTree checkQuotesInLiterals "param='my\\ file'; rm $param" +checkQuotesInLiterals params t = + doVariableFlowAnalysis readF writeF Map.empty (variableFlow params) where getQuotes name = get >>= (return . Map.lookup name) setQuotes name ref = modify $ Map.insert name ref deleteQuotes = modify . Map.delete - parents = getParentTree t - quoteRegex = mkRegex "\"|([= ]|^)'|'( |$)" - containsQuotes s = isJust $ matchRegex quoteRegex s + parents = parentMap params + quoteRegex = mkRegex "\"|([= ]|^)'|'( |$)|\\\\ " + containsQuotes s = s `matches` quoteRegex - -- Just catch the most blatant cases of foo='--cow="lol bert"'; cmd $foo, since that's 99% writeF _ _ name (DataFrom values) = do - let quotedVars = filter (\v -> containsQuotes (concat $ deadSimple v)) values + quoteMap <- get + let quotedVars = msum $ map (forToken quoteMap) values case quotedVars of - [] -> deleteQuotes name - x:_ -> setQuotes name (getId x) + Nothing -> deleteQuotes name + Just x -> setQuotes name x return [] writeF _ _ _ _ = return [] + forToken map (T_DollarBraced id t) = + -- skip getBracedReference here to avoid false positives on PE + Map.lookup (concat . deadSimple $ t) map + forToken quoteMap (T_DoubleQuoted id tokens) = + msum $ map (forToken quoteMap) tokens + forToken quoteMap (T_NormalWord id tokens) = + msum $ map (forToken quoteMap) tokens + forToken _ t = + if containsQuotes (concat $ deadSimple t) + then return $ getId t + else Nothing + readF _ expr name = do assignment <- getQuotes name if isJust assignment && not (isParamTo parents "eval" expr) && not (inUnquotableContext parents expr) then return [ - (fromJust assignment, - Note WarningC 2089 "Word splitting will treat quotes as literals. Use an array."), - (getId expr, - Note WarningC 2090 "Embedded quotes in this variable will not be respected.") - ] + Note (fromJust assignment)WarningC 2089 $ + "Quotes/backslashes will be treated literally. Use an array.", + Note (getId expr) WarningC 2090 $ + "Quotes/backslashes in this variable will not be respected." + ] else return [] prop_checkFunctionsUsedExternally1 = - verifyFull checkFunctionsUsedExternally "foo() { :; }; sudo foo" + verifyTree checkFunctionsUsedExternally "foo() { :; }; sudo foo" prop_checkFunctionsUsedExternally2 = - verifyFull checkFunctionsUsedExternally "alias f='a'; xargs -n 1 f" + verifyTree checkFunctionsUsedExternally "alias f='a'; xargs -n 1 f" prop_checkFunctionsUsedExternally3 = - verifyNotFull checkFunctionsUsedExternally "f() { :; }; echo f" -checkFunctionsUsedExternally t = - runBasicAnalysis checkCommand t + verifyNotTree checkFunctionsUsedExternally "f() { :; }; echo f" +checkFunctionsUsedExternally params t = + runNodeAnalysis checkCommand params t where invokingCmds = [ "chroot", @@ -1797,14 +1883,15 @@ "sudo", "xargs" ] - checkCommand t@(T_SimpleCommand _ _ (cmd:args)) = + checkCommand _ t@(T_SimpleCommand _ _ (cmd:args)) = let name = fromMaybe "" $ getCommandBasename t in when (name `elem` invokingCmds) $ mapM_ (checkArg name) args - checkCommand _ = return () + checkCommand _ _ = return () - functions = Map.fromList $ runBasicAnalysis findFunctions t - findFunctions (T_Function id name _) = modify ((name, id):) + analyse f t = snd $ runState (doAnalysis f t) [] + functions = Map.fromList $ analyse findFunctions t + findFunctions (T_Function id _ _ name _) = modify ((name, id):) findFunctions t@(T_SimpleCommand id _ (_:args)) | t `isUnqualifiedCommand` "alias" = mapM_ getAlias args findFunctions _ = return () @@ -1821,22 +1908,22 @@ info id 2032 $ "Use own script or sh -c '..' to run this from " ++ cmd ++ "." -prop_checkUnused0 = verifyNotFull checkUnusedAssignments "var=foo; echo $var" -prop_checkUnused1 = verifyFull checkUnusedAssignments "var=foo; echo $bar" -prop_checkUnused2 = verifyNotFull checkUnusedAssignments "var=foo; export var;" -prop_checkUnused3 = verifyFull checkUnusedAssignments "for f in *; do echo '$f'; done" -prop_checkUnused4 = verifyFull checkUnusedAssignments "local i=0" -prop_checkUnused5 = verifyNotFull checkUnusedAssignments "read lol; echo $lol" -prop_checkUnused6 = verifyNotFull checkUnusedAssignments "var=4; (( var++ ))" -prop_checkUnused7 = verifyNotFull checkUnusedAssignments "var=2; $((var))" -prop_checkUnused8 = verifyFull checkUnusedAssignments "var=2; var=3;" -prop_checkUnused9 = verifyNotFull checkUnusedAssignments "read ''" -prop_checkUnused10= verifyNotFull checkUnusedAssignments "read -p 'test: '" -prop_checkUnused11= verifyNotFull checkUnusedAssignments "bar=5; export foo[$bar]=3" -prop_checkUnused12= verifyNotFull checkUnusedAssignments "read foo; echo ${!foo}" -checkUnusedAssignments t = snd $ runState (mapM_ checkAssignment flow) [] +prop_checkUnused0 = verifyNotTree checkUnusedAssignments "var=foo; echo $var" +prop_checkUnused1 = verifyTree checkUnusedAssignments "var=foo; echo $bar" +prop_checkUnused2 = verifyNotTree checkUnusedAssignments "var=foo; export var;" +prop_checkUnused3 = verifyTree checkUnusedAssignments "for f in *; do echo '$f'; done" +prop_checkUnused4 = verifyTree checkUnusedAssignments "local i=0" +prop_checkUnused5 = verifyNotTree checkUnusedAssignments "read lol; echo $lol" +prop_checkUnused6 = verifyNotTree checkUnusedAssignments "var=4; (( var++ ))" +prop_checkUnused7 = verifyNotTree checkUnusedAssignments "var=2; $((var))" +prop_checkUnused8 = verifyTree checkUnusedAssignments "var=2; var=3;" +prop_checkUnused9 = verifyNotTree checkUnusedAssignments "read ''" +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}" +checkUnusedAssignments params t = snd $ runWriter (mapM_ checkAssignment flow) where - flow = getVariableFlow t + flow = variableFlow params references = foldl (flip ($)) defaultMap (map insertRef flow) insertRef (Reference (base, token, name)) = Map.insert name () @@ -1855,7 +1942,7 @@ prop_checkGlobsAsOptions1 = verify checkGlobsAsOptions "rm *.txt" prop_checkGlobsAsOptions2 = verify checkGlobsAsOptions "ls ??.*" prop_checkGlobsAsOptions3 = verifyNot checkGlobsAsOptions "rm -- *.txt" -checkGlobsAsOptions (T_SimpleCommand _ _ args) = +checkGlobsAsOptions _ (T_SimpleCommand _ _ args) = mapM_ check $ takeWhile (not . isEndOfArgs) args where check v@(T_NormalWord _ ((T_Glob id s):_)) | s == "*" || s == "?" = @@ -1871,7 +1958,7 @@ "::::" -> True _ -> False -checkGlobsAsOptions _ = return () +checkGlobsAsOptions _ _ = return () prop_checkWhileReadPitfalls1 = verify checkWhileReadPitfalls "while read foo; do ssh $foo uptime; done < file" @@ -1881,7 +1968,7 @@ prop_checkWhileReadPitfalls5 = verifyNot checkWhileReadPitfalls "while read foo; do echo ls | ssh $foo; done" prop_checkWhileReadPitfalls6 = verifyNot checkWhileReadPitfalls "while read foo <&3; do ssh $foo; done 3< foo" -checkWhileReadPitfalls (T_WhileExpression id [command] contents) +checkWhileReadPitfalls _ (T_WhileExpression id [command] contents) | isStdinReadCommand command = do mapM_ checkMuncher contents where @@ -1906,16 +1993,16 @@ stdinRedirect (T_FdRedirect _ fd _) | fd == "" || fd == "0" = True stdinRedirect _ = False -checkWhileReadPitfalls _ = return () +checkWhileReadPitfalls _ _ = return () -prop_checkPrefixAssign1 = verifyTree checkPrefixAssignmentReference "var=foo echo $var" -prop_checkPrefixAssign2 = verifyNotTree checkPrefixAssignmentReference "var=$(echo $var) cmd" -checkPrefixAssignmentReference t@(T_DollarBraced id value) tree = +prop_checkPrefixAssign1 = verify checkPrefixAssignmentReference "var=foo echo $var" +prop_checkPrefixAssign2 = verifyNot checkPrefixAssignmentReference "var=$(echo $var) cmd" +checkPrefixAssignmentReference params t@(T_DollarBraced id value) = check path where name = getBracedReference $ bracedString value - path = getPath tree t + path = getPath (parentMap params) t idPath = map getId path check [] = return () @@ -1935,7 +2022,7 @@ prop_checkCharRangeGlob2 = verifyNot checkCharRangeGlob "ls *[[:digit:]].jpg" prop_checkCharRangeGlob3 = verify checkCharRangeGlob "ls [10-15]" prop_checkCharRangeGlob4 = verifyNot checkCharRangeGlob "ls [a-zA-Z]" -checkCharRangeGlob (T_Glob id str) | isCharClass str = +checkCharRangeGlob _ (T_Glob id str) | isCharClass str = if ":" `isPrefixOf` contents && ":" `isSuffixOf` contents && contents /= ":" @@ -1948,16 +2035,18 @@ isCharClass str = "[" `isPrefixOf` str && "]" `isSuffixOf` str contents = drop 1 . take ((length str) - 1) $ str hasDupes = any (>1) . map length . group . sort . filter (/= '-') $ contents -checkCharRangeGlob _ = return () +checkCharRangeGlob _ _ = return () -prop_checkCdAndBack1 = verify (checkCdAndBack Sh) "for f in *; do cd $f; git pull; cd ..; done" -prop_checkCdAndBack2 = verifyNot (checkCdAndBack Sh) "for f in *; do cd $f || continue; git pull; cd ..; done" -prop_checkCdAndBack3 = verifyNot (checkCdAndBack Sh) "while [[ $PWD != / ]]; do cd ..; done" -checkCdAndBack shell = doLists +prop_checkCdAndBack1 = verify checkCdAndBack "for f in *; do cd $f; git pull; cd ..; done" +prop_checkCdAndBack2 = verifyNot checkCdAndBack "for f in *; do cd $f || continue; git pull; cd ..; done" +prop_checkCdAndBack3 = verifyNot checkCdAndBack "while [[ $PWD != / ]]; do cd ..; done" +checkCdAndBack params = doLists where + shell = shellType params doLists (T_ForIn _ _ _ cmds) = doList cmds + doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds doLists (T_WhileExpression _ _ cmds) = doList cmds doLists (T_UntilExpression _ _ cmds) = doList cmds doLists (T_IfExpression _ thens elses) = do @@ -1983,3 +2072,50 @@ if shell == Bash || shell == Zsh then "Consider using ( subshell ), 'cd foo||exit', or pushd/popd instead." else "Consider using ( subshell ) or 'cd foo||exit' instead." + +prop_checkLoopKeywordScope1 = verify checkLoopKeywordScope "continue 2" +prop_checkLoopKeywordScope2 = verify checkLoopKeywordScope "for f; do ( break; ); done" +prop_checkLoopKeywordScope3 = verify checkLoopKeywordScope "if true; then continue; fi" +prop_checkLoopKeywordScope4 = verifyNot checkLoopKeywordScope "while true; do break; done" +prop_checkLoopKeywordScope5 = verify checkLoopKeywordScope "if true; then break; fi" +prop_checkLoopKeywordScope6 = verify checkLoopKeywordScope "while true; do true | { break; }; done" +prop_checkLoopKeywordScope7 = verifyNot checkLoopKeywordScope "#!/bin/ksh\nwhile true; do true | { break; }; done" +checkLoopKeywordScope params t | + name `elem` map Just ["continue", "break"] = + if not $ any isLoop path + then if any isFunction $ take 1 path + -- breaking at a source/function invocation is an abomination. Let's ignore it. + then err (getId t) 2104 $ "In functions, use return instead of " ++ (fromJust name) ++ "." + else err (getId t) 2105 $ (fromJust name) ++ " is only valid in loops." + else case map subshellType $ filter (not . isFunction) path of + (Just str):_ -> warn (getId t) 2106 $ + "This only exits the subshell caused by the " ++ str ++ "." + _ -> return () + where + name = getCommandName t + path = let p = getPath (parentMap params) t in filter relevant p + subshellType t = case leadType (shellType params) (parentMap params) t of + NoneScope -> Nothing + SubshellScope str -> return str + isFunction t = case t of T_Function _ _ _ _ _ -> True; _ -> False + relevant t = isLoop t || isFunction t || isJust (subshellType t) +checkLoopKeywordScope _ _ = return () + + +prop_checkFunctionDeclarations1 = verify checkFunctionDeclarations "#!/bin/ksh\nfunction foo() { command foo --lol \"$@\"; }" +prop_checkFunctionDeclarations2 = verify checkFunctionDeclarations "#!/bin/dash\nfunction foo { lol; }" +prop_checkFunctionDeclarations3 = verifyNot checkFunctionDeclarations "foo() { echo bar; }" +checkFunctionDeclarations params + (T_Function id (FunctionKeyword hasKeyword) (FunctionParentheses hasParens) _ _) = + case (shellType params) of + Bash -> return () + Zsh -> return () + Ksh -> do + when (hasKeyword && hasParens) $ + err id 2111 "ksh does not allow 'function' keyword and '()' at the same time." + Sh -> do + when (hasKeyword && hasParens) $ + warn id 2112 "'function' keyword is non-standard. Delete it." + when (hasKeyword && not hasParens) $ + warn id 2113 "'function' keyword is non-standard. Use 'foo()' instead of 'function foo'." +checkFunctionDeclarations _ _ = return () diff --git a/ShellCheck/Data.hs b/ShellCheck/Data.hs --- a/ShellCheck/Data.hs +++ b/ShellCheck/Data.hs @@ -1,8 +1,10 @@ module ShellCheck.Data where +shellcheckVersion = "0.3.1" -- Must also be updated in ShellCheck.cabal + internalVariables = [ -- Generic - "", "_", + "", "_", "rest", "REST", -- Bash "BASH", "BASHOPTS", "BASHPID", "BASH_ALIASES", "BASH_ARGC", @@ -41,13 +43,13 @@ ] variablesWithoutSpaces = [ - "$", "-", "?", "!", + "$", "-", "?", "!", "BASHPID", "BASH_ARGC", "BASH_LINENO", "BASH_SUBSHELL", "EUID", "LINENO", "OPTIND", "PPID", "RANDOM", "SECONDS", "SHELLOPTS", "SHLVL", "UID", "COLUMNS", "HISTFILESIZE", "HISTSIZE", "LINES" ] -commonCommands = [ +commonCommands = [ "admin", "alias", "ar", "asa", "at", "awk", "basename", "batch", "bc", "bg", "break", "c99", "cal", "cat", "cd", "cflow", "chgrp", "chmod", "chown", "cksum", "cmp", "colon", "comm", "command", @@ -70,5 +72,5 @@ "unalias", "uname", "uncompress", "unexpand", "unget", "uniq", "unlink", "unset", "uucp", "uudecode", "uuencode", "uustat", "uux", "val", "vi", "wait", "wc", "what", "who", "write", "xargs", "yacc", - "zcat" + "zcat" ] diff --git a/ShellCheck/Parser.hs b/ShellCheck/Parser.hs --- a/ShellCheck/Parser.hs +++ b/ShellCheck/Parser.hs @@ -17,7 +17,7 @@ -} {-# LANGUAGE NoMonomorphismRestriction #-} -module ShellCheck.Parser (Note(..), Severity(..), parseShell, ParseResult(..), ParseNote(..), notesFromMap, Metadata(..), sortNotes) where +module ShellCheck.Parser (Note(..), Severity(..), parseShell, ParseResult(..), ParseNote(..), sortNotes, noteToParseNote) where import ShellCheck.AST import ShellCheck.Data @@ -34,8 +34,6 @@ import Text.Parsec.Error import GHC.Exts (sortWith) -lastError = 1074 - backslash = char '\\' linefeed = (optional carriageReturn) >> char '\n' singleQuote = char '\'' <|> unicodeSingleQuote @@ -100,20 +98,20 @@ return ' ' --------- Message/position annotation on top of user state -data Note = Note Severity Code String deriving (Show, Eq) +data Note = Note Id Severity Code String deriving (Show, Eq) data ParseNote = ParseNote SourcePos Severity Code String deriving (Show, Eq) -data Metadata = Metadata SourcePos [Note] deriving (Show) data Severity = ErrorC | WarningC | InfoC | StyleC deriving (Show, Eq, Ord) data Context = ContextName SourcePos String | ContextAnnotation [Annotation] type Code = Integer -codeForNote (Note _ code _) = code codeForParseNote (ParseNote _ _ code _) = code +noteToParseNote map (Note id severity code message) = + ParseNote pos severity code message + where + pos = fromJust $ Map.lookup id map initialState = (Id $ -1, Map.empty, []) -getInitialMeta pos = Metadata pos [] - getLastId = do (id, _, _) <- getState return id @@ -121,7 +119,7 @@ getNextIdAt sourcepos = do (id, map, notes) <- getState let newId = incId id - let newMap = Map.insert newId (getInitialMeta sourcepos) map + let newMap = Map.insert newId sourcepos map putState (newId, newMap, notes) return newId where incId (Id n) = (Id $ n+1) @@ -189,12 +187,7 @@ Ms.modify (\(list, current) -> ((ParseNote pos level code msg):list, current)) -- Store non-parse problems inside -addNoteFor id note = modifyMap $ Map.adjust (\(Metadata pos notes) -> Metadata pos (note:notes)) id -addNote note = do - id <- getLastId - addNoteFor id note - parseNote c l a = do pos <- getPosition parseNoteAt pos c l a @@ -269,6 +262,7 @@ where typ = if single then SingleBracket else DoubleBracket readCondBinaryOp = try $ do + optional guardArithmetic id <- getNextId op <- (choice $ (map tryOp ["==", "!=", "<=", ">=", "=~", ">", "<", "=", "\\<=", "\\>=", "\\<", "\\>"])) <|> otherOp hardCondSpacing @@ -284,6 +278,13 @@ when (s == "-a" || s == "-o") $ fail "Wrong operator" return $ TC_Binary id typ s + guardArithmetic = do + try . lookAhead $ disregard (oneOf "+*/%") <|> disregard (string "- ") + parseProblem ErrorC 1076 $ + if single + then "Trying to do math? Use e.g. [ $((i/2+7)) -ge 18 ]." + else "Trying to do math? Use e.g. [[ $((i/2+7)) -ge 18 ]]." + readCondUnaryExp = do op <- readCondUnaryOp pos <- getPosition @@ -325,16 +326,13 @@ readCondAndOp = do id <- getNextId x <- try (string "&&" <|> string "-a") - when (single && x == "&&") $ addNoteFor id $ Note ErrorC 1022 "You can't use && inside [..]. Use [[..]] instead." - when (not single && x == "-a") $ addNoteFor id $ Note ErrorC 1023 "In [[..]], use && instead of -a." softCondSpacing return $ TC_And id typ x readCondOrOp = do + optional guardArithmetic id <- getNextId x <- try (string "||" <|> string "-o") - when (single && x == "||") $ addNoteFor id $ Note ErrorC 1024 "You can't use || inside [..]. Use [[..]] instead." - when (not single && x == "-o") $ addNoteFor id $ Note ErrorC 1025 "In [[..]], use || instead of -o." softCondSpacing return $ TC_Or id typ x @@ -444,6 +442,8 @@ prop_aB = isOk readArithmeticContents "10#08 * 16#f" prop_aC = isOk readArithmeticContents "\"$((3+2))\" + '37'" prop_aD = isOk readArithmeticContents "foo[9*y+x]++" +prop_aE = isOk readArithmeticContents "1+`echo 2`" +prop_aF = isOk readArithmeticContents "foo[`echo foo | sed s/foo/4/g` * 3] + 4" readArithmeticContents = readSequence where @@ -477,7 +477,7 @@ readExpansion = do id <- getNextId - x <- readNormalDollar + x <- readNormalDollar <|> readBackTicked spacing return $ TA_Expansion id x @@ -1348,6 +1348,8 @@ prop_readIfClause = isOk readIfClause "if false; then foo; elif true; then stuff; more stuff; else cows; fi" prop_readIfClause2 = isWarning readIfClause "if false; then; echo oo; fi" prop_readIfClause3 = isWarning readIfClause "if false; then true; else; echo lol; fi" +prop_readIfClause4 = isWarning readIfClause "if false; then true; else if true; then echo lol; fi" +prop_readIfClause5 = isOk readIfClause "if false; then true; else\nif true; then echo lol; fi; fi" readIfClause = called "if expression" $ do id <- getNextId pos <- getPosition @@ -1389,7 +1391,9 @@ readElifPart = called "elif clause" $ do pos <- getPosition - g_Elif + correctElif <- elif + when (not correctElif) $ + parseProblemAt pos ErrorC 1075 "Use 'elif' instead of 'else if'." allspacing condition <- readTerm g_Then @@ -1398,8 +1402,12 @@ verifyNotEmptyIf "then" action <- readTerm return (condition, action) + where + elif = (g_Elif >> return True) <|> + (try $ g_Else >> g_If >> return False) readElsePart = called "else clause" $ do + pos <- getPosition g_Else acceptButWarn g_Semi ErrorC 1053 "No semicolons directly after 'else'." allspacing @@ -1478,15 +1486,14 @@ prop_readForClause7 = isOk readForClause "for ((;;)) do echo $i\ndone" prop_readForClause8 = isOk readForClause "for ((;;)) ; do echo $i\ndone" prop_readForClause9 = isOk readForClause "for i do true; done" +prop_readForClause10= isOk readForClause "for ((;;)) { true; }" readForClause = called "for loop" $ do pos <- getPosition (T_For id) <- g_For spacing - typ <- (readRegular <|> readArithmetic) - group <- readDoGroup pos - typ id group + readRegular id pos <|> readArithmetic id pos where - readArithmetic = called "arithmetic for condition" $ do + readArithmetic id pos = called "arithmetic for condition" $ do try $ string "((" x <- readArithmeticContents char ';' >> spacing @@ -1497,13 +1504,19 @@ string "))" spacing optional $ readSequentialSep >> spacing - return $ \id group -> (return $ T_ForArithmetic id x y z group) + group <- readBraced <|> readDoGroup pos + return $ T_ForArithmetic id x y z group - readRegular = do + readBraced = do + (T_BraceGroup _ list) <- readBraceGroup + return list + + readRegular id pos = do name <- readVariableName spacing values <- readInClause <|> (optional readSequentialSep >> return []) - return $ \id group -> (return $ T_ForIn id name values group) + group <- readDoGroup pos + return $ T_ForIn id name values group prop_readSelectClause1 = isOk readSelectClause "select foo in *; do echo $foo; done" prop_readSelectClause2 = isOk readSelectClause "select foo; do echo $foo; done" @@ -1569,61 +1582,52 @@ prop_readFunctionDefinition = isOk readFunctionDefinition "foo() { command foo --lol \"$@\"; }" prop_readFunctionDefinition1 = isOk readFunctionDefinition "foo (){ command foo --lol \"$@\"; }" -prop_readFunctionDefinition2 = isWarning readFunctionDefinition "function foo() { command foo --lol \"$@\"; }" -prop_readFunctionDefinition3 = isWarning readFunctionDefinition "function foo { lol; }" prop_readFunctionDefinition4 = isWarning readFunctionDefinition "foo(a, b) { true; }" prop_readFunctionDefinition5 = isOk readFunctionDefinition ":(){ :|:;}" prop_readFunctionDefinition6 = isOk readFunctionDefinition "?(){ foo; }" prop_readFunctionDefinition7 = isOk readFunctionDefinition "..(){ cd ..; }" prop_readFunctionDefinition8 = isOk readFunctionDefinition "foo() (ls)" readFunctionDefinition = called "function" $ do - id <- getNextId - name <- try readFunctionSignature + functionSignature <- try readFunctionSignature allspacing (disregard (lookAhead $ oneOf "{(") <|> parseProblem ErrorC 1064 "Expected a { to open the function definition.") group <- readBraceGroup <|> readSubshell - return $ T_Function id name group - - -readFunctionSignature = do - readWithFunction <|> readWithoutFunction + return $ functionSignature group where - readWithFunction = do - pos <- getPosition - try $ do - string "function" - whitespace - parseProblemAt pos InfoC 1005 "Drop the keyword 'function'. It's optional in Bash but invalid in other shells." - spacing - name <- readFunctionName - optional spacing - pos <- getPosition - readParens <|> do - parseProblemAt pos InfoC 1006 "Include '()' after the function name (in addition to dropping 'function')." - return name - - readWithoutFunction = try $ do - name <- readFunctionName - optional spacing - readParens - return name - - readParens = do - g_Lparen - optional spacing - g_Rparen <|> do - parseProblem ErrorC 1065 "Trying to declare parameters? Don't. Use () and refer to params as $1, $2.." - many $ noneOf "\n){" - g_Rparen - return () + readFunctionSignature = do + readWithFunction <|> readWithoutFunction + where + readWithFunction = do + id <- getNextId + try $ do + string "function" + whitespace + spacing + name <- readFunctionName + optional spacing + hasParens <- wasIncluded readParens + return $ T_Function id (FunctionKeyword True) (FunctionParentheses hasParens) name - readFunctionName = many1 functionChars + readWithoutFunction = try $ do + id <- getNextId + name <- readFunctionName + optional spacing + readParens + return $ T_Function id (FunctionKeyword False) (FunctionParentheses True) name + readParens = do + g_Lparen + optional spacing + g_Rparen <|> do + parseProblem ErrorC 1065 "Trying to declare parameters? Don't. Use () and refer to params as $1, $2.." + many $ noneOf "\n){" + g_Rparen + return () + readFunctionName = many1 functionChars readPattern = (readNormalWord `thenSkip` spacing) `sepBy1` (char '|' `thenSkip` spacing) - prop_readCompoundCommand = isOk readCompoundCommand "{ echo foo; }>/dev/null" readCompoundCommand = do id <- getNextId @@ -1865,7 +1869,7 @@ checkString parser string = case rp (parser >> eof >> getState) "-" string of - (Right (tree, map, notes), (problems, _)) -> (True, (notesFromMap map) ++ notes ++ problems) + (Right (tree, map, notes), (problems, _)) -> (True, notes ++ problems) (Left _, (n, _)) -> (False, n) parseWithNotes parser = do @@ -1874,16 +1878,11 @@ parseNotes <- getParseNotes return (item, map, nub . sortNotes $ parseNotes) -toParseNotes (Metadata pos list) = map (\(Note level code note) -> ParseNote pos level code note) list -notesFromMap map = Map.fold (\x -> (++) (toParseNotes x)) [] map - -getAllNotes result = (concatMap (notesFromMap . snd) (maybeToList . parseResult $ result)) ++ (parseNotes result) - compareNotes (ParseNote pos1 level1 _ s1) (ParseNote pos2 level2 _ s2) = compare (pos1, level1) (pos2, level2) sortNotes = sortBy compareNotes -data ParseResult = ParseResult { parseResult :: Maybe (Token, Map.Map Id Metadata), parseNotes :: [ParseNote] } deriving (Show) +data ParseResult = ParseResult { parseResult :: Maybe (Token, Map.Map Id SourcePos), parseNotes :: [ParseNote] } deriving (Show) makeErrorFor parsecError = ParseNote (errorPos parsecError) ErrorC 1072 $ getStringFromParsec $ errorMessages parsecError @@ -1903,9 +1902,11 @@ parseShell filename contents = do case rp (parseWithNotes readScript) filename contents of - (Right (script, map, notes), (parsenotes, _)) -> ParseResult (Just (script, map)) (nub $ sortNotes $ notes ++ parsenotes) - (Left err, (p, context)) -> ParseResult Nothing (nub $ sortNotes $ p ++ (notesForContext context) ++ ([makeErrorFor err])) - + (Right (script, map, notes), (parsenotes, _)) -> + ParseResult (Just (script, map)) (nub $ sortNotes $ notes ++ parsenotes) + (Left err, (p, context)) -> + ParseResult Nothing + (nub $ sortNotes $ p ++ (notesForContext context) ++ ([makeErrorFor err])) where isName (ContextName _ _) = True isName _ = False diff --git a/ShellCheck/Simple.hs b/ShellCheck/Simple.hs --- a/ShellCheck/Simple.hs +++ b/ShellCheck/Simple.hs @@ -25,28 +25,28 @@ prop_findsParseIssue = - let comments = shellCheck "echo \"$12\"" in + let comments = shellCheck "echo \"$12\"" [] in (length comments) == 1 && (scCode $ head comments) == 1037 prop_commentDisablesParseIssue1 = - null $ shellCheck "#shellcheck disable=SC1037\necho \"$12\"" + null $ shellCheck "#shellcheck disable=SC1037\necho \"$12\"" [] prop_commentDisablesParseIssue2 = - null $ shellCheck "#shellcheck disable=SC1037\n#lol\necho \"$12\"" + null $ shellCheck "#shellcheck disable=SC1037\n#lol\necho \"$12\"" [] prop_findsAnalysisIssue = - let comments = shellCheck "echo $1" in + let comments = shellCheck "echo $1" [] in (length comments) == 1 && (scCode $ head comments) == 2086 prop_commentDisablesAnalysisIssue1 = - null $ shellCheck "#shellcheck disable=SC2086\necho $1" + null $ shellCheck "#shellcheck disable=SC2086\necho $1" [] prop_commentDisablesAnalysisIssue2 = - null $ shellCheck "#shellcheck disable=SC2086\n#lol\necho $1" + null $ shellCheck "#shellcheck disable=SC2086\n#lol\necho $1" [] -shellCheck :: String -> [ShellCheckComment] -shellCheck script = +shellCheck :: String -> [AnalysisOption] -> [ShellCheckComment] +shellCheck script options = let (ParseResult result notes) = parseShell "-" script in let allNotes = notes ++ (concat $ maybeToList $ do - (tree, map) <- result - let newMap = runAllAnalytics tree map - return $ notesFromMap $ filterByAnnotation tree newMap + (tree, posMap) <- result + let list = runAnalytics options tree + return $ map (noteToParseNote posMap) $ filterByAnnotation tree list ) in map formatNote $ nub $ sortNotes allNotes diff --git a/shellcheck.hs b/shellcheck.hs --- a/shellcheck.hs +++ b/shellcheck.hs @@ -18,10 +18,13 @@ import Control.Exception import Control.Monad import Data.Char +import Data.Maybe import GHC.Exts import GHC.IO.Device import Prelude hiding (catch) +import ShellCheck.Data import ShellCheck.Simple +import ShellCheck.Analytics import System.Console.GetOpt import System.Directory import System.Environment @@ -37,7 +40,11 @@ Option ['f'] ["format"] (ReqArg (Flag "format") "FORMAT") "output format", Option ['e'] ["exclude"] - (ReqArg (Flag "exclude") "CODE1,CODE2..") "exclude types of warnings" + (ReqArg (Flag "exclude") "CODE1,CODE2..") "exclude types of warnings", + Option ['s'] ["shell"] + (ReqArg (Flag "shell") "SHELLNAME") "Specify dialect (bash,sh,ksh,zsh)", + Option ['V'] ["version"] + (NoArg $ Flag "version" "true") "Print version information" ] printErr = hPutStrLn stderr @@ -57,14 +64,9 @@ parseArguments argv = case getOpt Permute options argv of - (opts, files, []) -> - if not $ null files - then - return $ Just (opts, files) - else do - printErr "No files specified.\n" - printErr $ usageInfo header options - exitWith syntaxFailure + (opts, files, []) -> do + verifyOptions opts files + return $ Just (opts, files) (_, _, errors) -> do printErr $ (concat errors) ++ "\n" ++ usageInfo header options @@ -200,8 +202,15 @@ liftM (getComments options) $ readContents file getComments options contents = - excludeCodes (getExclusions options) $ shellCheck contents + excludeCodes (getExclusions options) $ shellCheck contents analysisOptions + where + analysisOptions = catMaybes [ shellOption ] + shellOption = do + option <- getOption options "shell" + sh <- shellForExecutable option + return $ ForceShell sh + readContents file = if file == "-" then getContents else readFile file -- Realign comments from a tabstop of 8 to 1 @@ -216,9 +225,9 @@ real rest (r+1) (v + 8 - (v `mod` 8)) target real (_:rest) r v target = real rest (r+1) (v+1) target -getOption [] _ def = def -getOption ((Flag var val):_) name _ | name == var = val -getOption (_:rest) flag def = getOption rest flag def +getOption [] _ = Nothing +getOption ((Flag var val):_) name | name == var = return val +getOption (_:rest) flag = getOption rest flag getOptions options name = map (\(Flag _ val) -> val) . filter (\(Flag var _) -> var == name) $ options @@ -256,8 +265,8 @@ exitWith code process Nothing = return False -process (Just (options, files)) = - let format = getOption options "format" "tty" in +process (Just (options, files)) = do + let format = fromMaybe "tty" $ getOption options "format" in case Map.lookup format formats of Nothing -> do printErr $ "Unknown format " ++ format @@ -268,3 +277,24 @@ Just f -> do f options files +verifyOptions opts files = do + 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 (null files) $ do + printErr "No files specified.\n" + printErr $ usageInfo header options + exitWith syntaxFailure + +printVersionAndExit = do + putStrLn $ "ShellCheck - shell script analysis tool" + putStrLn $ "version: " ++ shellcheckVersion + putStrLn $ "license: GNU Affero General Public License, version 3" + putStrLn $ "website: http://www.shellcheck.net" + exitWith ExitSuccess