diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,6 +1,6 @@ Name: ShellCheck -- Must also be updated in ShellCheck/Data.hs : -Version: 0.3.2 +Version: 0.3.3 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 @@ -28,6 +28,7 @@ data AssignmentMode = Assign | Append deriving (Show, Eq) data FunctionKeyword = FunctionKeyword Bool deriving (Show, Eq) data FunctionParentheses = FunctionParentheses Bool deriving (Show, Eq) +data ForInType = NormalForIn | ShortForIn deriving (Show, Eq) data Token = TA_Base Id String Token @@ -81,7 +82,7 @@ | T_Fi Id | T_For Id | T_ForArithmetic Id Token Token Token [Token] - | T_ForIn Id String [Token] [Token] + | T_ForIn Id ForInType [String] [Token] [Token] | T_Function Id FunctionKeyword FunctionParentheses String Token | T_GREATAND Id | T_Glob Id String @@ -202,7 +203,7 @@ delve (T_BraceGroup id l) = dl l $ T_BraceGroup id delve (T_WhileExpression id c l) = dll c l $ T_WhileExpression id delve (T_UntilExpression id c l) = dll c l $ T_UntilExpression id - delve (T_ForIn id v w l) = dll w l $ T_ForIn id v + delve (T_ForIn id t v w l) = dll w l $ T_ForIn id t v delve (T_SelectIn id v w l) = dll w l $ T_SelectIn id v delve (T_CaseExpression id word cases) = do newWord <- round word @@ -308,7 +309,7 @@ T_BraceGroup id _ -> id T_WhileExpression id _ _ -> id T_UntilExpression id _ _ -> id - T_ForIn id _ _ _ -> id + T_ForIn id _ _ _ _ -> id T_SelectIn id _ _ _ -> id T_CaseExpression id _ _ -> id T_Function id _ _ _ _ -> id diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -55,6 +55,7 @@ ,checkFunctionsUsedExternally ,checkUnusedAssignments ,checkUnpassedInFunctions + ,checkArrayWithoutIndex ] checksFor Sh = [ @@ -120,6 +121,8 @@ -- Checks that are run on each node in the AST runNodeAnalysis f p t = execWriter (doAnalysis (f p) t) + +nodeChecks :: [Parameters -> Token -> Writer [Note] ()] nodeChecks = [ checkUuoc ,checkPipePitfalls @@ -189,6 +192,15 @@ ,checkInteractiveSu ,checkStderrPipe ,checkSetAssignment + ,checkOverridingPath + ,checkArrayAsString + ,checkUnsupported + ,checkMultipleAppends + ,checkAliasesExpandEarly + ,checkSuspiciousIFS + ,checkAliasesUsesArgs + ,checkShouldUseGrepQ + ,checkTestGlobs ] @@ -224,6 +236,8 @@ isVariableName (x:r) = isVariableStartChar x && all isVariableChar r isVariableName _ = False +potentially = fromMaybe (return ()) + matchAll re = unfoldr f where f str = do @@ -252,9 +266,17 @@ isConfusedGlobRegex [x,'*'] | x /= '\\' = True isConfusedGlobRegex _ = False -isPotentiallyConfusedGlobRegex = - let re = mkRegex "[a-z1-9]\\*" in - isJust . matchRegex re +getSuspiciousRegexWildcard str = + if (not $ str `matches` contra) + then do + match <- matchRegex suspicious str + str <- match !!! 0 + str !!! 0 + else + fail "looks good" + where + suspicious = mkRegex "([A-Za-z1-9])\\*" + contra = mkRegex "[^a-zA-Z1-9]\\*|[][^$+\\\\]" matches string regex = isJust $ matchRegex regex string @@ -315,9 +337,16 @@ [] -> Nothing (r:_) -> Just r +verify :: (Parameters -> Token -> Writer [a] ()) -> String -> Bool verify f s = checkNode f s == Just True + +verifyNot :: (Parameters -> Token -> Writer [a] ()) -> String -> Bool verifyNot f s = checkNode f s == Just False + +verifyTree :: (Parameters -> Token -> [a]) -> String -> Bool verifyTree f s = checkTree f s == Just True + +verifyNotTree :: (Parameters -> Token -> [a]) -> String -> Bool verifyNotTree f s = checkTree f s == Just False checkNode f = checkTree (runNodeAnalysis f) @@ -439,9 +468,14 @@ 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 _ _) | - cmd `isCommand` "expr" && (not $ ":" `elem` deadSimple cmd) = +prop_checkNeedlessCommands4 = verifyNot checkNeedlessCommands "foo=$(expr foo \\< regex)" +checkNeedlessCommands _ cmd@(T_SimpleCommand id _ args) | + cmd `isCommand` "expr" && (not $ any (`elem` words) exceptions) = style id 2003 "expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]]." + where + -- These operators are hard to replicate in POSIX + exceptions = [ ":", "<", ">", "<=", ">=" ] + words = mapMaybe getLiteralString args checkNeedlessCommands _ _ = return () prop_checkPipePitfalls3 = verify checkPipePitfalls "ls | grep -v mp3" @@ -460,6 +494,9 @@ for' ["ps", "grep"] $ \x -> info x 2009 "Consider using pgrep instead of grepping ps output." + for' ["grep", "wc"] $ + \x -> style x 2126 "Consider using grep -c instead of grep|wc." + didLs <- liftM or . sequence $ [ for' ["ls", "grep"] $ \x -> warn x 2010 "Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.", @@ -498,11 +535,38 @@ bracedString l = concat $ deadSimple l -isMagicInQuotes (T_DollarBraced _ l) = + +isArrayExpansion (T_DollarBraced _ l) = let string = bracedString l in - '@' `elem` string || "!" `isPrefixOf` string -isMagicInQuotes _ = False + "@" `isPrefixOf` string || + not ("#" `isPrefixOf` string) && "[@]" `isInfixOf` string +isArrayExpansion _ = False +-- Is it certain that this arg will becomes multiple args? +willBecomeMultipleArgs t = willConcatInAssignment t || f t + where + f (T_Extglob {}) = True + f (T_Glob {}) = True + f (T_BraceExpansion {}) = True + f (T_DoubleQuoted _ parts) = any f parts + f (T_NormalWord _ parts) = any f parts + f _ = False + +willConcatInAssignment t@(T_DollarBraced {}) = isArrayExpansion t +willConcatInAssignment (T_DoubleQuoted _ parts) = any willConcatInAssignment parts +willConcatInAssignment (T_NormalWord _ parts) = any willConcatInAssignment parts +willConcatInAssignment _ = False + +-- Is it possible that this arg becomes multiple args? +mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f t + where + f (T_DollarBraced _ l) = + let string = bracedString l in + "!" `isPrefixOf` string + f (T_DoubleQuoted _ parts) = any f parts + f (T_NormalWord _ parts) = any f parts + f _ = False + prop_checkShebang1 = verifyTree checkShebang "#!/usr/bin/env bash -x\necho cow" prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh -l " checkShebang _ (T_Script id sb _) = @@ -598,17 +662,17 @@ prop_checkForInQuoted4a = verifyNot checkForInQuoted "for f in foo{1,2,3}; do true; done" prop_checkForInQuoted5 = verify checkForInQuoted "for f in ls; do true; done" prop_checkForInQuoted6 = verifyNot checkForInQuoted "for f in \"${!arr}\"; do true; done" -checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) = - when (any (\x -> willSplit x && not (isMagicInQuotes x)) list +checkForInQuoted _ (T_ForIn _ _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) = + when (any (\x -> willSplit x && not (mayBecomeMultipleArgs x)) list || (liftM wouldHaveBeenGlob (getLiteralString word) == Just True)) $ err id 2066 $ "Since you double quoted this, it will not word split, and the loop will only run once." -checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_SingleQuoted id s]] _) = +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 unless ('{' `elem` s) $ warn id 2042 $ "Use spaces, not commas, to separate loop elements." - else warn id 2043 $ "This loop will only run once, with " ++ f ++ "='" ++ s ++ "'." + else warn id 2043 $ "This loop will only run once, with " ++ (head f) ++ "='" ++ s ++ "'." checkForInQuoted _ _ = return () prop_checkForInCat1 = verify checkForInCat "for f in $(cat foo); do stuff; done" @@ -616,7 +680,7 @@ 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 = @@ -632,17 +696,17 @@ prop_checkForInLs3 = verify checkForInLs "for f in `find / -name '*.mp3'`; do mplayer \"$f\"; done" checkForInLs _ t = try t where - try (T_ForIn _ f [T_NormalWord _ [T_DollarExpansion id [x]]] _) = + try (T_ForIn _ _ f [T_NormalWord _ [T_DollarExpansion id [x]]] _) = check id f x - try (T_ForIn _ f [T_NormalWord _ [T_Backticked id [x]]] _) = + try (T_ForIn _ _ f [T_NormalWord _ [T_Backticked id [x]]] _) = check id f x try _ = return () check id f x = case deadSimple x of ("ls":n) -> let warntype = if any ("-" `isPrefixOf`) n then warn else err in - warntype id 2045 $ "Iterate over globs whenever possible (e.g. 'for f in */*.wav'), as for loops over ls will fail for filenames like 'my file*.txt'." - ("find":_) -> warn id 2044 $ "Use find -exec or a while read loop instead, as for loops over find will fail for filenames like 'my file*.txt'." + warntype id 2045 "Iterating over ls output is fragile. Use globs." + ("find":_) -> warn id 2044 "For loops over find output are fragile. Use find -exec or a while read loop." _ -> return () @@ -754,14 +818,20 @@ where isOk [t] = isAssignment t || (fromMaybe False $ do name <- getCommandBasename t - return $ name `elem` ["echo", "exit"]) + return $ name `elem` ["echo", "exit", "return"]) isOk _ = False checkShorthandIf _ _ = return () prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done" -checkDollarStar _ (T_NormalWord _ [(T_DollarBraced id l)]) | (bracedString l) == "*" = - warn id 2048 $ "Use \"$@\" (with quotes) to prevent whitespace problems." +prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*" +checkDollarStar p t@(T_NormalWord _ [(T_DollarBraced id l)]) + | (bracedString l) == "*" = + unless isAssigned $ + warn id 2048 $ "Use \"$@\" (with quotes) to prevent whitespace problems." + where + path = getPath (parentMap p) t + isAssigned = any isAssignment . take 2 $ path checkDollarStar _ _ = return () @@ -771,14 +841,56 @@ 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]) = - 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 +prop_checkUnquotedDollarAt6 = verifyNot checkUnquotedDollarAt "a=$@" +checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not isAssigned = + flip mapM_ (take 1 $ filter isArrayExpansion parts) $ \x -> do + err (getId x) 2068 $ + "Double quote array expansions, otherwise they're like $* and break on spaces." + where + path = getPath (parentMap p) word + isAssigned = any isAssignment . take 2 $ path checkUnquotedDollarAt _ _ = return () + +prop_checkArrayAsString1 = verify checkArrayAsString "a=$@" +prop_checkArrayAsString2 = verify checkArrayAsString "a=\"${arr[@]}\"" +prop_checkArrayAsString3 = verify checkArrayAsString "a=*.png" +prop_checkArrayAsString4 = verify checkArrayAsString "a={1..10}" +prop_checkArrayAsString5 = verifyNot checkArrayAsString "a='*.gif'" +prop_checkArrayAsString6 = verifyNot checkArrayAsString "a=$*" +prop_checkArrayAsString7 = verifyNot checkArrayAsString "a=( $@ )" +checkArrayAsString _ (T_Assignment id _ _ _ word) = + if willConcatInAssignment word + then + warn (getId word) 2124 + "Assigning an array to a string! Assign as array, or use * instead of @ to concatenate." + else + when (willBecomeMultipleArgs word) $ + warn (getId word) 2125 + "Brace expansions and globs are literal in assignments. Quote it or use an array." +checkArrayAsString _ _ = return () + +prop_checkArrayWithoutIndex1 = verifyTree checkArrayWithoutIndex "foo=(a b); echo $foo" +prop_checkArrayWithoutIndex2 = verifyNotTree checkArrayWithoutIndex "foo='bar baz'; foo=($foo); echo ${foo[0]}" +checkArrayWithoutIndex params _ = + concat $ doVariableFlowAnalysis readF writeF Map.empty (variableFlow params) + where + readF _ (T_DollarBraced id token) _ = do + map <- get + return . maybeToList $ do + name <- getLiteralString token + assignment <- Map.lookup name map + return [(Note id WarningC 2128 + "Expanding an array without an index only gives the first element.")] + readF _ _ _ = return [] + + writeF _ t name (DataFrom [T_Array {}]) = do + modify (Map.insert name t) + return [] + writeF _ _ name _ = do + modify (Map.delete name) + return [] + prop_checkStderrRedirect = verify checkStderrRedirect "test 2>&1 > cow" prop_checkStderrRedirect2 = verifyNot checkStderrRedirect "test > cow 2>&1" checkStderrRedirect _ (T_Redirecting _ [ @@ -805,6 +917,7 @@ prop_checkSingleQuotedVariables5 = verifyNot checkSingleQuotedVariables "trap 'echo $SECONDS' EXIT" prop_checkSingleQuotedVariables6 = verifyNot checkSingleQuotedVariables "sed -n '$p'" prop_checkSingleQuotedVariables6a= verify checkSingleQuotedVariables "sed -n '$pattern'" +prop_checkSingleQuotedVariables7 = verifyNot checkSingleQuotedVariables "PS1='$PWD \\$ '" checkSingleQuotedVariables params t@(T_SingleQuoted id s) = when (s `matches` re) $ if "sed" == commandName @@ -819,19 +932,29 @@ name <- getCommandBasename cmd return name - isProbablyOk = commandName `elem` [ + isProbablyOk = + (any isOkAssignment $ take 3 $ getPath parents t) + || commandName `elem` [ "trap" ,"sh" ,"bash" ,"ksh" ,"zsh" ,"ssh" + ,"xprop" + ,"alias" ] || "awk" `isSuffixOf` commandName || "perl" `isPrefixOf` commandName + commonlyQuoted = ["PS1", "PS2", "PS3", "PS4", "PROMPT_COMMAND"] + isOkAssignment t = + case t of + T_Assignment _ _ name _ _ -> name `elem` commonlyQuoted + otherwise -> False + re = mkRegex "\\$[{(0-9a-zA-Z_]" - sedContra = mkRegex "\\$[dp]($|[^a-zA-Z])" + sedContra = mkRegex "\\$[dpsaic]($|[^a-zA-Z])" checkSingleQuotedVariables _ _ = return () @@ -852,8 +975,11 @@ prop_checkNumberComparisons8 = verify checkNumberComparisons "[[ foo <= bar ]]" prop_checkNumberComparisons9 = verify checkNumberComparisons "[ foo \\>= bar ]" prop_checkNumberComparisons10= verify checkNumberComparisons "#!/bin/zsh -x\n[ foo >= bar ]]" +prop_checkNumberComparisons11= verify checkNumberComparisons "[[ $foo -eq 'N' ]]" +prop_checkNumberComparisons12= verify checkNumberComparisons "[ x$foo -gt x${N} ]" checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do - if (isNum lhs || isNum rhs) + if (isNum lhs && (not $ isNonNum rhs) + || isNum rhs && (not $ isNonNum lhs)) then do when (isLtGt op) $ err id 2071 $ @@ -871,6 +997,7 @@ when (op `elem` ["-lt", "-gt", "-le", "-ge", "-eq"]) $ do mapM_ checkDecimals [lhs, rhs] + checkStrings [lhs, rhs] where isLtGt = flip elem ["<", "\\<", ">", "\\>"] @@ -885,6 +1012,17 @@ decimalError = "Decimals are not supported. " ++ "Either use integers only, or use bc or awk to compare." + checkStrings hs = + mapM_ stringError . take 1 . filter isNonNum $ hs + + isNonNum t = fromMaybe False $ do + s <- getLiteralStringExt (const $ return "") t + return . not . all numChar $ s + numChar x = isDigit x || x `elem` "+-. " + + stringError t = err (getId t) 2130 $ + op ++ " is for integer comparisons. Use " ++ (seqv op) ++ " instead." + isNum t = case deadSimple t of [v] -> all isDigit v @@ -901,6 +1039,15 @@ eqv ">=" = "-ge" eqv _ = "the numerical equivalent" + esc = if typ == SingleBracket then "\\" else "" + seqv "-ge" = "! a " ++ esc ++ "< b" + seqv "-gt" = esc ++ ">" + seqv "-le" = "! a " ++ esc ++ "> b" + seqv "-lt" = esc ++ "<" + seqv "-eq" = "=" + seqv "-ne" = "!=" + seqv _ = "the string equivalent" + invert ('\\':s) = invert s invert "<=" = ">" invert ">=" = "<" @@ -994,9 +1141,10 @@ err id 2078 $ "This expression is constant. Did you forget a $ somewhere?" checkConstantNoary _ _ = return () -prop_checkBraceExpansionVars = verify checkBraceExpansionVars "echo {1..$n}" -checkBraceExpansionVars _ (T_BraceExpansion id s) | '$' `elem` s = - warn id 2051 $ "Bash doesn't support variables in brace expansions." +prop_checkBraceExpansionVars1 = verify checkBraceExpansionVars "echo {1..$n}" +prop_checkBraceExpansionVars2 = verifyNot checkBraceExpansionVars "echo {1,3,$n}" +checkBraceExpansionVars _ (T_BraceExpansion id s) | "..$" `isInfixOf` s = + warn id 2051 $ "Bash doesn't support variables in brace range expansions." checkBraceExpansionVars _ _ = return () prop_checkForDecimals = verify checkForDecimals "((3.14*c))" @@ -1133,7 +1281,9 @@ T_CaseExpression _ _ _ -> return True T_HereDoc _ _ _ _ _ -> return True T_DollarBraced {} -> return True - T_ForIn _ _ _ _ -> return True -- Pragmatically assume it's desirable here + -- Pragmatically assume it's desirable to split here + T_ForIn {} -> return True + T_SelectIn {} -> return True _ -> Nothing isParamTo tree cmd t = @@ -1204,6 +1354,13 @@ isLiteral t = isJust $ getLiteralString t +-- turn a NormalWord like foo="bar $baz" into a series of constituent elements like [foo=,bar ,$baz] +getWordParts t = g t + where + g (T_NormalWord _ l) = concatMap g l + g (T_DoubleQuoted _ l) = l + g other = [other] + isCommand token str = isCommandMatch token (\cmd -> cmd == str || ("/" ++ str) `isSuffixOf` cmd) isUnqualifiedCommand token str = isCommandMatch token (\cmd -> cmd == str) @@ -1224,6 +1381,7 @@ isAssignment (T_Annotation _ _ w) = isAssignment w isAssignment (T_Redirecting _ _ w) = isAssignment w isAssignment (T_SimpleCommand _ (w:_) []) = True +isAssignment (T_Assignment {}) = True isAssignment _ = False prop_checkPrintfVar1 = verify checkPrintfVar "printf \"Lol: $s\"" @@ -1338,24 +1496,31 @@ prop_checkGrepRe7 = verify checkGrepRe "grep *foo* file" prop_checkGrepRe8 = verify checkGrepRe "ls | grep foo*.jpg" prop_checkGrepRe9 = verifyNot checkGrepRe "grep '[0-9]*' file" +prop_checkGrepRe10= verifyNot checkGrepRe "grep '^aa*' file" +prop_checkGrepRe11= verifyNot checkGrepRe "grep --include=*.png foo" checkGrepRe _ = checkCommand "grep" (const f) where -- --regex=*(extglob) doesn't work. Fixme? skippable (Just s) = not ("--regex=" `isPrefixOf` s) && "-" `isPrefixOf` s skippable _ = False f [] = return () - f (x:r) | skippable (getLiteralString x) = f r + f (x:r) | skippable (getLiteralStringExt (const $ return "_") x) = f r f (re:_) = do when (isGlob re) $ do warn (getId re) 2062 $ "Quote the grep pattern so the shell won't interpret it." let string = concat $ deadSimple re if isConfusedGlobRegex string then warn (getId re) 2063 $ "Grep uses regex, but this looks like a glob." - else - if (isPotentiallyConfusedGlobRegex string) - then info (getId re) 2022 "Note that c* does not mean \"c followed by anything\" in regex." - else return () + else potentially $ do + char <- getSuspiciousRegexWildcard string + return $ info (getId re) 2022 $ + "Note that unlike globs, " ++ [char] ++ "* here matches '" ++ [char, char, char] ++ "' but not '" ++ (wordStartingWith char) ++ "'." +wordStartingWith c = + head . filter ([c] `isPrefixOf`) $ candidates + where + candidates = + sampleWords ++ (map (\(x:r) -> (toUpper x) : r) sampleWords) ++ [c:"test"] prop_checkTrapQuotes1 = verify checkTrapQuotes "trap \"echo $num\" INT" prop_checkTrapQuotes1a= verify checkTrapQuotes "trap \"echo `ls`\" INT" @@ -1473,6 +1638,8 @@ prop_checkInexplicablyUnquoted1 = verify checkInexplicablyUnquoted "echo 'var='value';'" prop_checkInexplicablyUnquoted2 = verifyNot checkInexplicablyUnquoted "'foo'*" prop_checkInexplicablyUnquoted3 = verifyNot checkInexplicablyUnquoted "wget --user-agent='something'" +prop_checkInexplicablyUnquoted4 = verify checkInexplicablyUnquoted "echo \"VALUES (\"id\")\"" +prop_checkInexplicablyUnquoted5 = verifyNot checkInexplicablyUnquoted "\"$dir\"/\"$file\"" checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens) where check ((T_SingleQuoted _ _):(T_Literal id str):_) @@ -1481,13 +1648,16 @@ check ((T_DoubleQuoted _ _):trapped:(T_DoubleQuoted _ _):_) = case trapped of - T_DollarExpansion id _ -> warnAbout id - T_DollarBraced id _ -> warnAbout id + T_DollarExpansion id _ -> warnAboutExpansion id + T_DollarBraced id _ -> warnAboutExpansion id + T_Literal id s -> unless (s == "/") $ warnAboutLiteral id _ -> return () check _ = return () - warnAbout id = - info id 2027 $ "Surrounding quotes actually unquotes this (\"inside\"$outside\"inside\"). Did you forget your quote level?" + warnAboutExpansion id = + warn id 2027 $ "The surrounding quotes actually unquote this. Remove or escape them." + warnAboutLiteral id = + warn id 2140 $ "The double quotes around this do nothing. Remove or escape them." checkInexplicablyUnquoted _ _ = return () prop_checkTildeInQuotes1 = verify checkTildeInQuotes "var=\"~/out.txt\"" @@ -1526,7 +1696,7 @@ 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_ForIn _ _ _ _ cmds) = doList cmds doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds doLists (T_IfExpression _ thens elses) = do mapM_ (\(_, l) -> doList l) thens @@ -1576,11 +1746,14 @@ prop_checkUnusedEchoEscapes2 = verifyNot checkUnusedEchoEscapes "echo -e 'foi\\nbar'" prop_checkUnusedEchoEscapes3 = verify checkUnusedEchoEscapes "echo \"n:\\t42\"" prop_checkUnusedEchoEscapes4 = verifyNot checkUnusedEchoEscapes "echo lol" +prop_checkUnusedEchoEscapes5 = verifyNot checkUnusedEchoEscapes "echo -n -e '\n'" checkUnusedEchoEscapes _ = checkCommand "echo" (const f) where isDashE = mkRegex "^-.*e" hasEscapes = mkRegex "\\\\[rnt]" - f (arg:_) | (concat $ deadSimple arg) `matches` isDashE = return () + f args | (concat $ concatMap deadSimple allButLast) `matches` isDashE = + return () + where allButLast = reverse . drop 1 . reverse $ args f args = mapM_ checkEscapes args checkEscapes (T_NormalWord _ args) = @@ -1723,7 +1896,7 @@ else [] --Points to 'for' rather than variable - T_ForIn id str words _ -> [(t, t, str, DataFrom words)] + T_ForIn id _ strs words _ -> map (\str -> (t, t, str, DataFrom words)) strs T_SelectIn id str words _ -> [(t, t, str, DataFrom words)] _ -> [] @@ -1745,7 +1918,9 @@ getModifiedVariableCommand base@(T_SimpleCommand _ _ ((T_NormalWord _ ((T_Literal _ x):_)):rest)) = filter (\(_,_,s,_) -> not ("-" `isPrefixOf` s)) $ case x of - "read" -> concatMap getLiteral rest + "read" -> + let params = map getLiteral rest in + catMaybes . takeWhile isJust . reverse $ params "let" -> concatMap letParamToLiteral rest "export" -> concatMap getModifierParam rest @@ -1763,13 +1938,10 @@ (T_NormalWord id1 [T_DoubleQuoted id2 [T_Literal id3 (stripEquals s)]]) stripEqualsFrom t = t - getLiteral t@(T_NormalWord _ [T_Literal _ s]) = - [(base, t, s, DataExternal)] - getLiteral t@(T_NormalWord _ [T_SingleQuoted _ s]) = - [(base, t, s, DataExternal)] - getLiteral t@(T_NormalWord _ [T_DoubleQuoted _ [T_Literal id s]]) = - [(base, t, s, DataExternal)] - getLiteral x = [] + getLiteral t = do + s <- getLiteralString t + when ("-" `isPrefixOf` s) $ fail "argument" + return (base, t, s, DataExternal) getModifierParam t@(T_Assignment _ _ name _ value) = [(base, t, name, DataFrom [value])] @@ -1817,8 +1989,8 @@ if assignFirst t then return () else setWritten t when (scopeType /= NoneScope) $ modify ((StackScopeEnd):) - assignFirst (T_ForIn _ _ _ _) = True - assignFirst (T_SelectIn _ _ _ _) = True + assignFirst (T_ForIn {}) = True + assignFirst (T_SelectIn {}) = True assignFirst _ = False setRead t = @@ -1879,6 +2051,7 @@ prop_checkSpacefulnessI = verifyNotTree checkSpacefulness "$1 --flags" prop_checkSpacefulnessJ = verifyTree checkSpacefulness "echo $PWD" prop_checkSpacefulnessK = verifyNotTree checkSpacefulness "n+='foo bar'" +prop_checkSpacefulnessL = verifyNotTree checkSpacefulness "select foo in $bar; do true; done" checkSpacefulness params t = doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params) @@ -2056,6 +2229,7 @@ prop_checkUnused14= verifyNotTree checkUnusedAssignments "x=(1); n=0; echo ${x[n]}" prop_checkUnused15= verifyNotTree checkUnusedAssignments "x=(1); n=0; (( x[n] ))" prop_checkUnused16= verifyNotTree checkUnusedAssignments "foo=5; declare -x foo" +prop_checkUnused17= verifyNotTree checkUnusedAssignments "read -i 'foo' -e -p 'Input: ' bar; $bar;" checkUnusedAssignments params t = snd $ runWriter (mapM_ checkAssignment flow) where flow = variableFlow params @@ -2103,6 +2277,7 @@ prop_checkWhileReadPitfalls4 = verifyNot checkWhileReadPitfalls "while read foo; do ssh $foo hostname < /dev/null; done" 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" +prop_checkWhileReadPitfalls7 = verify checkWhileReadPitfalls "while read foo; do if true; then ssh $foo uptime; fi; done < file" checkWhileReadPitfalls _ (T_WhileExpression id [command] contents) | isStdinReadCommand command = do @@ -2117,13 +2292,19 @@ && all (not . stdinRedirect) redirs isStdinReadCommand _ = False - checkMuncher (T_Pipeline _ _ ((T_Redirecting _ redirs cmd):_)) = do - let name = fromMaybe "" $ getCommandBasename cmd - when ((not . any stdinRedirect $ redirs) && (name `elem` munchers)) $ do - info id 2095 $ - name ++ " may swallow stdin, preventing this loop from working properly." - warn (getId cmd) 2095 $ - "Add < /dev/null to prevent " ++ name ++ " from swallowing stdin." + checkMuncher (T_Pipeline _ _ ((T_Redirecting _ redirs cmd):_)) | not $ any stdinRedirect redirs = do + case cmd of + (T_IfExpression _ thens elses) -> + mapM_ checkMuncher . concat $ (map fst thens) ++ (map snd thens) ++ [elses] + + _ -> potentially $ do + name <- getCommandBasename cmd + guard $ name `elem` munchers + return $ do + info id 2095 $ + name ++ " may swallow stdin, preventing this loop from working properly." + warn (getId cmd) 2095 $ + "Add < /dev/null to prevent " ++ name ++ " from swallowing stdin." checkMuncher _ = return () stdinRedirect (T_FdRedirect _ fd _) @@ -2158,7 +2339,9 @@ 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 = +prop_checkCharRangeGlob5 = verifyNot checkCharRangeGlob "tr -d [a-zA-Z]" -- tr has 2060 +checkCharRangeGlob p t@(T_Glob id str) | + isCharClass str && not (isParamTo (parentMap p) "tr" t) = if ":" `isPrefixOf` contents && ":" `isSuffixOf` contents && contents /= ":" @@ -2181,7 +2364,7 @@ checkCdAndBack params = doLists where shell = shellType params - doLists (T_ForIn _ _ _ cmds) = doList cmds + doLists (T_ForIn _ _ _ _ cmds) = doList cmds doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds doLists (T_WhileExpression _ _ cmds) = doList cmds doLists (T_UntilExpression _ _ cmds) = doList cmds @@ -2465,3 +2648,170 @@ literal (T_NormalWord _ l) = concatMap literal l literal (T_Literal _ str) = str literal _ = "*" + + +prop_checkOverridingPath1 = verify checkOverridingPath "PATH=\"$var/$foo\"" +prop_checkOverridingPath2 = verify checkOverridingPath "PATH=\"mydir\"" +prop_checkOverridingPath3 = verify checkOverridingPath "PATH=/cow/foo" +prop_checkOverridingPath4 = verifyNot checkOverridingPath "PATH=/cow/foo/bin" +prop_checkOverridingPath5 = verifyNot checkOverridingPath "PATH='/bin:/sbin'" +prop_checkOverridingPath6 = verifyNot checkOverridingPath "PATH=\"$var/$foo\" cmd" +prop_checkOverridingPath7 = verifyNot checkOverridingPath "PATH=$OLDPATH" +prop_checkOverridingPath8 = verifyNot checkOverridingPath "PATH=$PATH:/stuff" +checkOverridingPath _ (T_SimpleCommand _ vars []) = + mapM_ checkVar vars + where + checkVar (T_Assignment id Assign "PATH" Nothing word) = + let string = concat $ deadSimple word + in unless (any (`isInfixOf` string) ["/bin", "/sbin" ]) $ do + when ('/' `elem` string && ':' `notElem` string) $ notify id + when (isLiteral word && ':' `notElem` string && '/' `notElem` string) $ notify id + checkVar _ = return () + notify id = warn id 2123 "PATH is the shell search path. Use another name." +checkOverridingPath _ _ = return () + + +prop_checkUnsupported1 = verifyNot checkUnsupported "#!/bin/zsh\nfunction { echo cow; }" +prop_checkUnsupported2 = verify checkUnsupported "#!/bin/sh\nfunction { echo cow; }" +checkUnsupported params t = + when (shellType params `notElem` support) $ + report name + where + (name, support) = shellSupport t + report s = err (getId t) 2127 $ + "To use " ++ s ++ ", specify #!/usr/bin/env " ++ + (map toLower . intercalate " or " . map show $ support) + +-- TODO: Move more of these checks here +shellSupport t = + case t of + T_Function _ _ _ "" _ -> ("anonymous functions", [Zsh]) + T_ForIn _ _ (_:_:_) _ _ -> ("multi-index for loops", [Zsh]) + T_ForIn _ ShortForIn _ _ _ -> ("short form for loops", [Zsh]) + T_ProcSub _ "=" _ -> ("=(..) process substitution", [Zsh]) + otherwise -> ("", [Bash, Ksh, Sh, Zsh]) + +getCommandSequences t = + f t + where + f (T_Script _ _ cmds) = [cmds] + f (T_BraceGroup _ cmds) = [cmds] + f (T_Subshell _ cmds) = [cmds] + f (T_WhileExpression _ _ cmds) = [cmds] + f (T_UntilExpression _ _ cmds) = [cmds] + f (T_ForIn _ _ _ _ cmds) = [cmds] + f (T_ForArithmetic _ _ _ _ cmds) = [cmds] + f (T_IfExpression _ thens elses) = (map snd thens) ++ [elses] + f _ = [] + +groupWith f l = groupBy (\x y -> f x == f y) l + +prop_checkMultipleAppends1 = verify checkMultipleAppends "foo >> file; bar >> file; baz >> file;" +prop_checkMultipleAppends2 = verify checkMultipleAppends "foo >> file; bar | grep f >> file; baz >> file;" +prop_checkMultipleAppends3 = verifyNot checkMultipleAppends "foo < file; bar < file; baz < file;" +checkMultipleAppends params t = + mapM_ checkList $ getCommandSequences t + where + checkList list = + mapM_ checkGroup groups + where + groups = groupWith (liftM fst) $ map getTarget list + checkGroup (f:_:_:_) | isJust f = + style (snd $ fromJust f) 2129 + "Consider using { cmd1; cmd2; } >> file instead of individual redirects." + checkGroup _ = return () + getTarget (T_Pipeline _ _ args@(_:_)) = getTarget (last args) + getTarget (T_Redirecting id list _) = do + file <- (mapMaybe getAppend list) !!! 0 + return (file, id) + getTarget _ = Nothing + getAppend (T_FdRedirect _ _ (T_IoFile _ (T_DGREAT {}) f)) = return f + getAppend _ = Nothing + + +prop_checkAliasesExpandEarly1 = verify checkAliasesExpandEarly "alias foo=\"echo $PWD\"" +prop_checkAliasesExpandEarly2 = verifyNot checkAliasesExpandEarly "alias -p" +prop_checkAliasesExpandEarly3 = verifyNot checkAliasesExpandEarly "alias foo='echo {1..10}'" +checkAliasesExpandEarly params = + checkUnqualifiedCommand "alias" (const f) + where + f = mapM_ checkArg + checkArg arg | '=' `elem` (concat $ deadSimple arg) = + flip mapM_ (take 1 $ filter (not . isLiteral) $ getWordParts arg) $ + \x -> warn (getId x) 2139 "This expands when defined, not when used. Consider escaping." + checkArg _ = return () + +prop_checkSuspiciousIFS1 = verify checkSuspiciousIFS "IFS=\"\\n\"" +prop_checkSuspiciousIFS2 = verifyNot checkSuspiciousIFS "IFS=$'\\t'" +checkSuspiciousIFS params (T_Assignment id Assign "IFS" Nothing value) = + potentially $ do + str <- getLiteralString value + return $ check str + where + n = if (shellType params == Sh) then "''" else "$'\\n'" + t = if (shellType params == Sh) then "\"$(printf '\\t')\"" else "$'\\t'" + check value = + case value of + "\\n" -> suggest n + "/n" -> suggest n + "\\t" -> suggest t + "/t" -> suggest t + _ -> return () + suggest r = warn id 2141 $ "Did you mean IFS=" ++ r ++ " ?" +checkSuspiciousIFS _ _ = return () + +prop_checkAliasesUsesArgs1 = verify checkAliasesUsesArgs "alias a='cp $1 /a'" +prop_checkAliasesUsesArgs2 = verifyNot checkAliasesUsesArgs "alias $1='foo'" +prop_checkAliasesUsesArgs3 = verify checkAliasesUsesArgs "alias a=\"echo \\${@}\"" +checkAliasesUsesArgs params = + checkUnqualifiedCommand "alias" (const f) + where + re = mkRegex "\\$\\{?[0-9*@]" + f = mapM_ checkArg + checkArg arg = + let string = fromJust $ getLiteralStringExt (const $ return "_") arg in + when ('=' `elem` string && string `matches` re) $ + err (getId arg) 2142 + "Aliases can't use positional parameters. Use a function." + +prop_checkGrepQ1= verify checkShouldUseGrepQ "[[ $(foo | grep bar) ]]" +prop_checkGrepQ2= verify checkShouldUseGrepQ "[ -z $(fgrep lol) ]" +prop_checkGrepQ3= verify checkShouldUseGrepQ "[ -n \"$(foo | zgrep lol)\" ]" +prop_checkGrepQ4= verifyNot checkShouldUseGrepQ "[ -z $(grep bar | cmd) ]" +prop_checkGrepQ5= verifyNot checkShouldUseGrepQ "rm $(ls | grep file)" +checkShouldUseGrepQ params t = + potentially $ case t of + TC_Noary id _ token -> check id True token + TC_Unary id _ "-n" token -> check id True token + TC_Unary id _ "-z" token -> check id False token + _ -> fail "not check" + where + check id bool token = do + name <- getFinalGrep token + let op = if bool then "-n" else "-z" + let flip = if bool then "" else "! " + return . style id 2143 $ + "Instead of [ " ++ op ++ " $(foo | " ++ name ++ " bar) ], " ++ + "use " ++ flip ++ "foo | " ++ name ++ " -q bar ." + + getFinalGrep t = do + cmds <- getPipeline t + guard . not . null $ cmds + name <- getCommandBasename $ last cmds + guard . isGrep $ name + return name + getPipeline t = + case t of + T_NormalWord _ [x] -> getPipeline x + T_DoubleQuoted _ [x] -> getPipeline x + T_DollarExpansion _ [x] -> getPipeline x + T_Pipeline _ _ cmds -> return cmds + _ -> fail "unknown" + isGrep = isSuffixOf "grep" + +prop_checkTestGlobs1 = verify checkTestGlobs "[ -e *.mp3 ]" +prop_checkTestGlobs2 = verifyNot checkTestGlobs "[[ $a == *b* ]]" +checkTestGlobs params (TC_Unary _ _ op token) | isGlob token = + err (getId token) 2144 $ + op ++ " doesn't work with globs. Use a for loop." +checkTestGlobs _ _ = return () diff --git a/ShellCheck/Data.hs b/ShellCheck/Data.hs --- a/ShellCheck/Data.hs +++ b/ShellCheck/Data.hs @@ -1,6 +1,6 @@ module ShellCheck.Data where -shellcheckVersion = "0.3.2" -- Must also be updated in ShellCheck.cabal +shellcheckVersion = "0.3.3" -- Must also be updated in ShellCheck.cabal internalVariables = [ -- Generic @@ -73,4 +73,12 @@ "unlink", "unset", "uucp", "uudecode", "uuencode", "uustat", "uux", "val", "vi", "wait", "wc", "what", "who", "write", "xargs", "yacc", "zcat" + ] + +sampleWords = [ + "alpha", "bravo", "charlie", "delta", "echo", "foxtrot", + "golf", "hotel", "india", "juliett", "kilo", "lima", "mike", + "november", "oscar", "papa", "quebec", "romeo", "sierra", + "tango", "uniform", "victor", "whiskey", "xray", "yankee", + "zulu" ] diff --git a/ShellCheck/Parser.hs b/ShellCheck/Parser.hs --- a/ShellCheck/Parser.hs +++ b/ShellCheck/Parser.hs @@ -64,6 +64,11 @@ optional readComment return $ concat x +spacing1 = do + spacing <- spacing + when (null spacing) $ fail "no spacing" + return spacing + prop_allspacing = isOk allspacing "#foo" prop_allspacing2 = isOk allspacing " #foo\n # bar\n#baz\n" prop_allspacing3 = isOk allspacing "#foo\n#bar\n#baz\n" @@ -403,7 +408,7 @@ where readGlobLiteral = do id <- getNextId - s <- many1 (extglobStart <|> oneOf "[]$") + s <- many1 (extglobStart <|> oneOf "{}[]$") return $ T_Literal id s readGroup = called "regex grouping" $ do id <- getNextId @@ -632,6 +637,7 @@ prop_readCondition10= isOk readCondition "[[ a == b \n || c == d ]]" prop_readCondition11= isOk readCondition "[[ a == b || \n c == d ]]" prop_readCondition12= isWarning readCondition "[ a == b \n -o c == d ]" +prop_readCondition13= isOk readCondition "[[ foo =~ ^fo{1,3}$ ]]" readCondition = called "test expression" $ do opos <- getPosition id <- getNextId @@ -700,6 +706,8 @@ prop_readNormalWord2 = isOk readNormalWord "foo**(foo)!!!(@@(bar))" prop_readNormalWord3 = isOk readNormalWord "foo#" prop_readNormalWord4 = isOk readNormalWord "$\"foo\"$'foo\nbar'" +prop_readNormalWord5 = isWarning readNormalWord "${foo}}" +prop_readNormalWord6 = isOk readNormalWord "foo/{}" readNormalWord = readNormalishWord "" readNormalishWord end = do @@ -710,14 +718,24 @@ return $ T_NormalWord id x checkPossibleTermination pos [T_Literal _ x] = - if x `elem` ["do", "done", "then", "fi", "esac", "}"] + if x `elem` ["do", "done", "then", "fi", "esac"] then parseProblemAt pos WarningC 1010 $ "Use semicolon or linefeed before '" ++ x ++ "' (or quote to make it literal)." else return () checkPossibleTermination _ _ = return () readNormalWordPart end = do checkForParenthesis - readSingleQuoted <|> readDoubleQuoted <|> readGlob <|> readNormalDollar <|> readBraced <|> readBackTicked <|> readProcSub <|> (readNormalLiteral end) + choice [ + readSingleQuoted, + readDoubleQuoted, + readGlob, + readNormalDollar, + readBraced, + readBackTicked, + readProcSub, + readNormalLiteral end, + readLiteralCurlyBraces + ] where checkForParenthesis = do return () `attempting` do @@ -725,7 +743,20 @@ lookAhead $ char '(' parseProblemAt pos ErrorC 1036 "'(' is invalid here. Did you forget to escape it?" + readLiteralCurlyBraces = do + id <- getNextId + str <- findParam <|> literalBraces + return $ T_Literal id str + findParam = try $ string "{}" + literalBraces = do + pos <- getPosition + c <- oneOf "{}" + parseProblemAt pos WarningC 1083 $ + "This " ++ [c] ++ " is literal. Check expression (missing ;/\\n?) or quote it." + return [c] + + readSpacePart = do id <- getNextId x <- many1 whitespace @@ -745,10 +776,11 @@ prop_readProcSub1 = isOk readProcSub "<(echo test | wc -l)" prop_readProcSub2 = isOk readProcSub "<( if true; then true; fi )" +prop_readProcSub3 = isOk readProcSub "=(ls)" readProcSub = called "process substitution" $ do id <- getNextId dir <- try $ do - x <- oneOf "<>" + x <- oneOf "<>=" char '(' return [x] allspacing @@ -922,7 +954,7 @@ return $ T_Literal id [c] readNormalLiteralPart end = do - readNormalEscaped <|> (many1 $ noneOf (end ++ quotableChars ++ extglobStartChars ++ "[")) + readNormalEscaped <|> (many1 $ noneOf (end ++ quotableChars ++ extglobStartChars ++ "[{}")) readNormalEscaped = called "escaped char" $ do pos <- getPosition @@ -934,10 +966,12 @@ do next <- anyChar case escapedChar next of - Just name -> parseNoteAt pos WarningC 1012 $ "\\" ++ [next] ++ " is just literal '" ++ [next] ++ "' here. For " ++ name ++ ", use \"$(printf \"\\" ++ [next] ++ "\")\"." + Just name -> parseNoteAt pos WarningC 1012 $ "\\" ++ [next] ++ " is just literal '" ++ [next] ++ "' here. For " ++ name ++ ", use " ++ (alternative next) ++ " instead." Nothing -> parseNoteAt pos InfoC 1001 $ "This \\" ++ [next] ++ " will be a regular '" ++ [next] ++ "' in this context." return [next] where + alternative 'n' = "a quoted, literal line feed" + alternative t = "\"$(printf \"\\" ++ [t] ++ "\")\"" escapedChar 'n' = Just "line feed" escapedChar 't' = Just "tab" escapedChar 'r' = Just "carriage return" @@ -1027,7 +1061,10 @@ char '{' str <- many1 ((readDoubleQuotedLiteral >>= (strip)) <|> readGenericLiteral1 (oneOf "}\"" <|> whitespace)) char '}' - return $ T_BraceExpansion id $ concat str + let result = concat str + unless (',' `elem` result || ".." `isInfixOf` result) $ + fail "Not a brace expression" + return $ T_BraceExpansion id $ result readNormalDollar = readDollarExpression <|> readDollarDoubleQuote <|> readDollarSingleQuote <|> readDollarLonely readDoubleQuotedDollar = readDollarExpression <|> readDollarLonely @@ -1077,7 +1114,7 @@ prop_readDollarBraced1 = isOk readDollarBraced "${foo//bar/baz}" prop_readDollarBraced2 = isOk readDollarBraced "${foo/'{cow}'}" -prop_readDollarBraced3 = isOk readDollarBraced "${foo%%$(echo cow})}" +prop_readDollarBraced3 = isOk readDollarBraced "${foo%%$(echo cow\\})}" prop_readDollarBraced4 = isOk readDollarBraced "${foo#\\}}" readDollarBraced = called "parameter expansion" $ do id <- getNextId @@ -1131,7 +1168,6 @@ pos <- getPosition char '$' n <- lookAhead (anyChar <|> (eof >> return '_')) - when (n /= '\'') $ parseNoteAt pos StyleC 1000 "$ is not used specially and should therefore be escaped." return $ T_Literal id "$" prop_readHereDoc = isOk readHereDoc "<< foo\nlol\ncow\nfoo" @@ -1266,8 +1302,8 @@ readNewlineList = many1 ((newline <|> carriageReturn) `thenSkip` spacing) readLineBreak = optional readNewlineList -prop_roflol = isWarning readScript "a &; b" -prop_roflol2 = isOk readScript "a & b" +prop_readSeparator1 = isWarning readScript "a &; b" +prop_readSeparator2 = isOk readScript "a & b" readSeparatorOp = do notFollowedBy2 (g_AND_IF <|> g_DSEMI) notFollowedBy2 (string "&>") @@ -1276,6 +1312,8 @@ spacing pos <- getPosition char ';' + -- In case statements we might have foo & ;; + notFollowedBy2 $ char ';' parseProblemAt pos ErrorC 1045 "It's not 'foo &; bar', just 'foo & bar'." return '&' ) <|> char ';' <|> char '&' @@ -1317,6 +1355,7 @@ prop_readSimpleCommand4 = isOk readSimpleCommand "typeset -a foo=(lol)" prop_readSimpleCommand5 = isOk readSimpleCommand "time if true; then echo foo; fi" prop_readSimpleCommand6 = isOk readSimpleCommand "time -p ( ls -l; )" +prop_readSimpleCommand7 = isOk readSimpleCommand "cat =(ls)" readSimpleCommand = called "simple command" $ do id1 <- getNextId id2 <- getNextId @@ -1568,6 +1607,7 @@ prop_readForClause8 = isOk readForClause "for ((;;)) ; do echo $i\ndone" prop_readForClause9 = isOk readForClause "for i do true; done" prop_readForClause10= isOk readForClause "for ((;;)) { true; }" +prop_readForClause11= isOk readForClause "for a b in *; do echo $a $b; done" readForClause = called "for loop" $ do pos <- getPosition (T_For id) <- g_For @@ -1593,12 +1633,26 @@ return list readRegular id pos = do - name <- readVariableName - spacing - values <- readInClause <|> (optional readSequentialSep >> return []) - group <- readDoGroup pos - return $ T_ForIn id name values group + names <- readNames + readShort names <|> readLong names + where + readLong names = do + values <- readInClause <|> (optional readSequentialSep >> return []) + group <- readDoGroup pos + return $ T_ForIn id NormalForIn names values group + readShort names = do + char '(' + allspacing + words <- many (readNormalWord `thenSkip` allspacing) + char ')' + allspacing + command <- readAndOr + return $ T_ForIn id ShortForIn names words [command] + readNames = + reluctantlyTill1 (readVariableName `thenSkip` spacing) $ + disregard g_Do <|> disregard readInClause <|> disregard readSequentialSep + prop_readSelectClause1 = isOk readSelectClause "select foo in *; do echo $foo; done" prop_readSelectClause2 = isOk readSelectClause "select foo; do echo $foo; done" readSelectClause = called "select loop" $ do @@ -1632,6 +1686,7 @@ prop_readCaseClause = isOk readCaseClause "case foo in a ) lol; cow;; b|d) fooo; esac" prop_readCaseClause2 = isOk readCaseClause "case foo\n in * ) echo bar;; esac" +prop_readCaseClause3 = isOk readCaseClause "case foo\n in * ) echo bar & ;; esac" readCaseClause = called "case expression" $ do id <- getNextId g_Case @@ -1705,7 +1760,7 @@ g_Rparen return () - readFunctionName = many1 functionChars + readFunctionName = many functionChars readPattern = (readNormalWord `thenSkip` spacing) `sepBy1` (char '|' `thenSkip` spacing) @@ -1753,6 +1808,9 @@ pos <- getPosition optional (char '$' >> parseNote ErrorC 1066 "Don't use $ on the left side of assignments.") variable <- readVariableName + notFollowedBy2 $ do -- Special case for zsh =(..) syntax + spacing1 + string "=(" optional (readNormalDollar >> parseNoteAt pos ErrorC 1067 "For indirection, use (associative) arrays or 'read \"var$n\" <<< \"value\"'") index <- optionMaybe readArrayIndex @@ -1893,6 +1951,10 @@ readScript = do id <- getNextId pos <- getPosition + optional $ do + readUtf8Bom + parseProblem ErrorC 1082 $ + "This file has a UTF-8 BOM. Remove it with: LC_CTYPE=C sed '1s/^...//' < yourscript ." sb <- option "" readShebang verifyShell pos (getShell sb) if (isValidShell $ getShell sb) /= Just False @@ -1951,6 +2013,8 @@ "ruby", "tcsh" ] + + readUtf8Bom = called "Byte Order Mark" $ string "\xFEFF" rp p filename contents = Ms.runState (runParserT p initialState filename contents) ([], [])