diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,5 +1,5 @@ Name: ShellCheck -Version: 0.3.6 +Version: 0.3.7 Synopsis: Shell script analysis tool License: AGPL-3 License-file: LICENSE @@ -43,7 +43,7 @@ json, mtl, parsec, - regex-compat, + regex-tdfa, QuickCheck >= 2.7.4 exposed-modules: ShellCheck.Analytics @@ -51,6 +51,7 @@ ShellCheck.Data ShellCheck.Options ShellCheck.Parser + ShellCheck.Regex ShellCheck.Simple other-modules: Paths_ShellCheck @@ -64,7 +65,7 @@ json, mtl, parsec, - regex-compat, + regex-tdfa, transformers, QuickCheck >= 2.7.4 main-is: shellcheck.hs @@ -79,7 +80,7 @@ json, mtl, parsec, - regex-compat, + regex-tdfa, transformers, QuickCheck >= 2.7.4 main-is: test/shellcheck.hs diff --git a/ShellCheck/AST.hs b/ShellCheck/AST.hs --- a/ShellCheck/AST.hs +++ b/ShellCheck/AST.hs @@ -19,7 +19,7 @@ import Control.Monad import Control.Monad.Identity -import qualified Text.Regex as Re +import qualified ShellCheck.Regex as Re data Id = Id Int deriving (Show, Eq, Ord) @@ -128,11 +128,13 @@ data Annotation = DisableComment Integer deriving (Show, Eq) data ConditionType = DoubleBracket | SingleBracket deriving (Show, Eq) --- I apologize for nothing! -lolHax s = Re.subRegex (Re.mkRegex "(Id [0-9]+)") (show s) "(Id 0)" -instance Eq Token where - (==) a b = lolHax a == lolHax b +-- This is an abomination. +tokenEquals :: Token -> Token -> Bool +tokenEquals a b = kludge a == kludge b + where kludge s = Re.subRegex (Re.mkRegex "\\(Id [0-9]+\\)") (show s) "(Id 0)" +instance Eq Token where + (==) = tokenEquals analyze :: Monad m => (Token -> m ()) -> (Token -> m ()) -> (Token -> Token) -> Token -> m Token analyze f g i = diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -15,7 +15,7 @@ You should have received a copy of the GNU Affero General Public License along with this program. If not, see . -} -{-# LANGUAGE TemplateHaskell #-} +{-# LANGUAGE TemplateHaskell, FlexibleContexts #-} module ShellCheck.Analytics (AnalysisOptions(..), defaultAnalysisOptions, filterByAnnotation, runAnalytics, shellForExecutable, runTests) where import Control.Arrow (first) @@ -33,7 +33,7 @@ import ShellCheck.Options import ShellCheck.Data import ShellCheck.Parser hiding (runTests) -import Text.Regex +import ShellCheck.Regex import qualified Data.Map as Map import Test.QuickCheck.All (forAllProperties) import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) @@ -240,12 +240,6 @@ potentially = fromMaybe (return ()) -matchAll re = unfoldr f - where - f str = do - (_, match, rest, _) <- matchRegexAll re str - return (match, rest) - willSplit x = case x of T_DollarBraced {} -> True @@ -280,20 +274,9 @@ suspicious = mkRegex "([A-Za-z1-9])\\*" contra = mkRegex "[^a-zA-Z1-9]\\*|[][^$+\\\\]" -matches string regex = isJust $ matchRegex regex string - headOrDefault _ (a:_) = a headOrDefault def _ = def -getAllMatches :: Regex -> String -> [[String]] -getAllMatches regex str = fromJust $ f str - where - f str = do - (_, _, rest, groups) <- matchRegexAll regex str - more <- f rest - return $ groups : more - `mappend` return [] - isConstant token = case token of T_NormalWord _ l -> all isConstant l @@ -335,16 +318,19 @@ deadSimple _ = [] -- Turn a SimpleCommand foo -avz --bar=baz into args ["a", "v", "z", "bar"] -getFlags (T_SimpleCommand _ _ (_:args)) = - let textArgs = takeWhile (/= "--") $ map (concat . deadSimple) args in +getFlagsUntil stopCondition (T_SimpleCommand _ _ (_:args)) = + let textArgs = takeWhile (not . stopCondition . snd) $ map (\x -> (x, concat $ deadSimple x)) args in concatMap flag textArgs where - flag ('-':'-':arg) = [ takeWhile (/= '=') arg ] - flag ('-':args) = map (:[]) args + flag (x, ('-':'-':arg)) = [ (x, takeWhile (/= '=') arg) ] + flag (x, ('-':args)) = map (\v -> (x, [v])) args flag _ = [] -getFlags _ = error "Internal shellcheck error, please report! (getFlags on non-command)" +getFlagsUntil _ _ = error "Internal shellcheck error, please report! (getFlags on non-command)" +getAllFlags = getFlagsUntil (== "--") +getLeadingFlags = getFlagsUntil (not . ("-" `isPrefixOf`)) + (!!!) list i = case drop i list of [] -> Nothing @@ -413,14 +399,19 @@ ["sed", "-e", v] -> checkIn v _ -> return () where - sedRe = mkRegex "^s(.)(.*)\\1(.*)\\1g?$" + -- This should have used backreferences, but TDFA doesn't support them + sedRe = mkRegex "^s(.)([^\n]*)g?$" + isSimpleSed s = fromMaybe False $ do + [first,rest] <- matchRegex sedRe s + let delimiters = filter (== (first !! 0)) rest + guard $ length delimiters == 2 + return True + acmd = deadSimple a bcmd = deadSimple b checkIn s = - case matchRegex sedRe s of - Just _ -> style id 2001 - "See if you can use ${variable//search/replace} instead." - _ -> return () + when (isSimpleSed s) $ + style id 2001 "See if you can use ${variable//search/replace} instead." checkEchoSed _ _ = return () prop_checkPipedAssignment1 = verify checkPipedAssignment "A=ls | grep foo" @@ -642,6 +633,13 @@ prop_checkBashisms17= verify checkBashisms "echo $((RANDOM%6+1))" prop_checkBashisms18= verify checkBashisms "foo &> /dev/null" prop_checkBashisms19= verify checkBashisms "foo > file*.txt" +prop_checkBashisms20= verify checkBashisms "read -ra foo" +prop_checkBashisms21= verify checkBashisms "[ -a foo ]" +prop_checkBashisms22= verifyNot checkBashisms "[ foo -a bar ]" +prop_checkBashisms23= verify checkBashisms "trap mything err int" +prop_checkBashisms24= verifyNot checkBashisms "trap mything int term" +prop_checkBashisms25= verify checkBashisms "cat < /dev/tcp/host/123" +prop_checkBashisms26= verify checkBashisms "trap mything ERR SIGTERM" checkBashisms _ = bashism where errMsg id s = err id 2040 $ "In sh, " ++ s ++ " not supported, even when sh is actually bash." @@ -660,24 +658,42 @@ bashism (TC_Binary id SingleBracket op _ _) | op `elem` [ "-nt", "-ef", "\\<", "\\>", "==" ] = warnMsg id $ op ++ " is" + bashism (TC_Unary id _ "-a" _) = + warnMsg id "unary -a in place of -e is" bashism (TA_Unary id op _) | op `elem` [ "|++", "|--", "++|", "--|"] = warnMsg id $ filter (/= '|') op ++ " is" - bashism t@(T_SimpleCommand id _ _) - | t `isCommand` "source" = - warnMsg id "'source' in place of '.' is" + bashism (TA_Binary id "**" _ _) = warnMsg id "exponentials are" bashism (T_FdRedirect id "&" (T_IoFile _ (T_Greater _) _)) = warnMsg id "&> is" - bashism t@(TA_Expansion id _) | getLiteralString t == Just "RANDOM" = - warnMsg id "RANDOM is" - bashism t@(T_DollarBraced id _) | getBracedReference (bracedString t) == "RANDOM" = - warnMsg id "$RANDOM is" - bashism (T_DollarBraced id token) = + bashism (T_IoFile id _ word) | isNetworked = + warnMsg id "/dev/{tcp,udp} is" + where + file = onlyLiteralString word + isNetworked = any (`isPrefixOf` file) ["/dev/tcp", "/dev/udp"] + + bashism t@(TA_Expansion id _) | isBashism = + warnMsg id $ fromJust str ++ " is" + where + str = getLiteralString t + isBashism = isJust str && fromJust str `elem` bashVars + bashism t@(T_DollarBraced id token) = do mapM_ check expansion + when (var `elem` bashVars) $ warnMsg id $ var ++ " is" where str = concat $ deadSimple token + var = getBracedReference (bracedString token) check (regex, feature) = when (isJust $ matchRegex regex str) $ warnMsg id feature + bashism t@(T_Pipe id "|&") = + warnMsg id "|& in place of 2>&1 | is" + bashism (T_Array id _) = + warnMsg id "arrays are" + bashism (T_IoFile id _ t) | isGlob t = + warnMsg id "redirecting to/from globs is" + bashism (T_CoProc id _ _) = + warnMsg id "coproc is" + bashism t@(T_SimpleCommand _ _ (cmd:arg:_)) | t `isCommand` "echo" && "-" `isPrefixOf` argString = unless ("--" `isPrefixOf` argString) $ -- echo "-------" @@ -688,15 +704,39 @@ warnMsg (getId arg) "exec flags are" bashism t@(T_SimpleCommand id _ _) | t `isCommand` "let" = warnMsg id "'let' is" - bashism t@(T_Pipe id "|&") = - warnMsg id "|& in place of 2>&1 | is" - bashism (T_Array id _) = - warnMsg id "arrays are" - bashism (T_IoFile id _ t) | isGlob t = - warnMsg id "redirecting to/from globs is" - bashism (T_CoProc id _ _) = - warnMsg id "coproc is" + bashism t@(T_SimpleCommand id _ (cmd:rest)) = + let name = fromMaybe "" $ getCommandName t + flags = getLeadingFlags t + in do + when (name `elem` bashCommands) $ warnMsg id $ "'" ++ name ++ "' is" + potentially $ do + allowed <- Map.lookup name allowedFlags + (word, flag) <- listToMaybe $ filter (\x -> snd x `notElem` allowed) flags + return . warnMsg (getId word) $ name ++ " -" ++ flag ++ " is" + + when (name == "source") $ warnMsg id "'source' in place of '.' is" + when (name == "trap") $ + let + check token = potentially $ do + word <- liftM (map toLower) $ getLiteralString token + guard $ word `elem` ["err", "debug", "return"] + return $ warnMsg (getId token) $ "trapping " ++ word ++ " is" + in + mapM_ check (reverse rest) + where + bashCommands = [ + "let", "caller", "builtin", "complete", "compgen", "declare", "dirs", "disown", + "enable", "mapfile", "readarray", "pushd", "popd", "shopt", "suspend", "type", + "typeset" + ] + allowedFlags = Map.fromList [ + ("read", ["r"]), + ("ulimit", ["f"]), + ("echo", []), + ("exec", []) + ] + bashism _ = return () varChars="_0-9a-zA-Z" @@ -705,9 +745,12 @@ (re $ "^![" ++ varChars ++ "]+\\[[*@]]$", "array key expansion is"), (re $ "^![" ++ varChars ++ "]+[*@]$", "name matching prefixes are"), (re $ "^[" ++ varChars ++ "]+:[^-=?+]", "string indexing is"), - (re $ "^[" ++ varChars ++ "]+(\\[.*\\])?/", "string replacement is"), - (re "^RANDOM$", "$RANDOM is") + (re $ "^[" ++ varChars ++ "]+(\\[.*\\])?/", "string replacement is") ] + bashVars = [ + "RANDOM", "LINENO", "OSTYPE", "MACHTYPE", "HOSTTYPE", "HOSTNAME", + "DIRSTACK", "EUID", "UID", "SECONDS", "SHLVL", "PIPESTATUS", "SHELLOPTS" + ] prop_checkForInQuoted = verify checkForInQuoted "for f in \"$(ls)\"; do echo foo; done" prop_checkForInQuoted2 = verifyNot checkForInQuoted "for f in \"$@\"; do echo foo; done" @@ -2017,15 +2060,15 @@ -- (Base expression, specific position, var name, assigned values) | Assignment (Token, Token, String, DataType) | Reference (Token, Token, String) - deriving (Show, Eq) + deriving (Show) data DataType = DataString DataSource | DataArray DataSource - deriving (Show, Eq) + deriving (Show) data DataSource = SourceFrom [Token] | SourceExternal | SourceDeclaration - deriving (Show, Eq) + deriving (Show) -data VariableState = Dead Token String | Alive deriving (Show, Eq) +data VariableState = Dead Token String | Alive deriving (Show) dataTypeFrom defaultType v = (case v of T_Array {} -> DataArray; _ -> defaultType) $ SourceFrom [v] @@ -2111,7 +2154,7 @@ getReference t@(T_Assignment _ _ name _ value) = [(t, t, name)] getReference t@(T_NormalWord _ [T_Literal _ name]) | not ("-" `isPrefixOf` name) = [(t, t, name)] getReference _ = [] - flags = getFlags base + flags = map snd $ getAllFlags base getReferencedVariableCommand _ = [] @@ -2144,7 +2187,7 @@ _ -> [] where - flags = getFlags base + flags = map snd $ getAllFlags base stripEquals s = let rest = dropWhile (/= '=') s in if rest == "" then "" else tail rest stripEqualsFrom (T_NormalWord id1 (T_Literal id2 s:rs)) = @@ -2220,10 +2263,11 @@ getSpecial _ = fail "empty" getIndexReferences s = fromMaybe [] $ do - (_, index, _, _) <- matchRegexAll re s - return $ matchAll variableNameRegex index + match <- matchRegex re s + index <- match !!! 0 + return $ matchAllStrings variableNameRegex index where - re = mkRegex "\\[.*\\]" + re = mkRegex "(\\[.*\\])" getReferencedVariables t = case t of @@ -2263,7 +2307,7 @@ prop_getVariablesFromLiteral1 = getVariablesFromLiteral "$foo${bar//a/b}$BAZ" == ["foo", "bar", "BAZ"] getVariablesFromLiteral string = - map (!! 0) $ getAllMatches variableRegex string + map (!! 0) $ matchAllSubgroups variableRegex string where variableRegex = mkRegex "\\$\\{?([A-Za-z0-9_]+)" @@ -3107,9 +3151,7 @@ mapM_ checkList $ getCommandSequences t where checkList list = - mapM_ checkGroup groups - where - groups = groupWith (liftM fst) $ map getTarget list + mapM_ checkGroup (groupWith (liftM fst) $ map getTarget list) checkGroup (f:_:_:_) | isJust f = style (snd $ fromJust f) 2129 "Consider using { cmd1; cmd2; } >> file instead of individual redirects." @@ -3256,7 +3298,7 @@ checkMaskedReturns _ t@(T_SimpleCommand id _ (cmd:rest)) = potentially $ do name <- getCommandName t guard $ name `elem` ["declare", "export"] - || name == "local" && "r" `notElem` getFlags t + || name == "local" && "r" `notElem` (map snd $ getAllFlags t) return $ mapM_ checkArgs rest where checkArgs (T_Assignment id _ _ _ word) | any hasReturn $ getWordParts word = diff --git a/ShellCheck/Parser.hs b/ShellCheck/Parser.hs --- a/ShellCheck/Parser.hs +++ b/ShellCheck/Parser.hs @@ -15,7 +15,7 @@ You should have received a copy of the GNU Affero General Public License along with this program. If not, see . -} -{-# LANGUAGE NoMonomorphismRestriction, TemplateHaskell #-} +{-# LANGUAGE NoMonomorphismRestriction, TemplateHaskell, FlexibleContexts #-} module ShellCheck.Parser (Note(..), Severity(..), parseShell, ParseResult(..), ParseNote(..), sortNotes, noteToParseNote, runTests, readScript) where import ShellCheck.AST @@ -2095,6 +2095,7 @@ badShells = [ "awk", "csh", + "expect", "perl", "python", "ruby", diff --git a/ShellCheck/Regex.hs b/ShellCheck/Regex.hs new file mode 100644 --- /dev/null +++ b/ShellCheck/Regex.hs @@ -0,0 +1,71 @@ +{- + This file is part of ShellCheck. + http://www.vidarholen.net/contents/shellcheck + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +-} +{-# LANGUAGE FlexibleContexts #-} + +-- Basically Text.Regex based on regex-tdfa instead of the buggy regex-posix. +module ShellCheck.Regex where + +import Data.List +import Data.Maybe +import Control.Monad +import Text.Regex.TDFA + +-- Precompile the regex +mkRegex :: String -> Regex +mkRegex str = + let make :: RegexMaker Regex CompOption ExecOption String => String -> Regex + make = makeRegex + in + make str + +-- Does the regex match? +matches :: String -> Regex -> Bool +matches = flip match + +-- Get all subgroups of the first match +matchRegex :: Regex -> String -> Maybe [String] +matchRegex re str = do + (_, _, _, groups) <- matchM re str :: Maybe (String,String,String,[String]) + return groups + +-- Get all full matches +matchAllStrings :: Regex -> String -> [String] +matchAllStrings re = unfoldr f + where + f :: String -> Maybe (String, String) + f str = do + (_, match, rest, _) <- matchM re str :: Maybe (String, String, String, [String]) + return (match, rest) + +-- Get all subgroups from all matches +matchAllSubgroups :: Regex -> String -> [[String]] +matchAllSubgroups re = unfoldr f + where + f :: String -> Maybe ([String], String) + f str = do + (_, _, rest, groups) <- matchM re str :: Maybe (String, String, String, [String]) + return (groups, rest) + +-- Replace regex in input with string +subRegex :: Regex -> String -> String -> String +subRegex re input replacement = f input + where + f str = fromMaybe str $ do + (before, match, after) <- matchM re str :: Maybe (String, String, String) + when (null match) $ error ("Internal error: substituted empty in " ++ str) + return $ before ++ replacement ++ f after