diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ http://www.shellcheck.net -Copyright 2012-2014, Vidar 'koala_man' Holen +Copyright 2012-2015, Vidar 'koala_man' Holen Licensed under the GNU Affero General Public License, v3 The goals of ShellCheck are: diff --git a/Setup.hs b/Setup.hs --- a/Setup.hs +++ b/Setup.hs @@ -1,2 +1,43 @@ -import Distribution.Simple -main = defaultMain +import Distribution.PackageDescription ( + HookedBuildInfo, + emptyHookedBuildInfo ) +import Distribution.Simple ( + Args, + UserHooks ( preSDist ), + defaultMainWithHooks, + simpleUserHooks ) +import Distribution.Simple.Setup ( SDistFlags ) + +-- | This requires the process package from, +-- +-- https://hackage.haskell.org/package/process +-- +import System.Process ( callCommand ) + + +-- | This will use almost the default implementation, except we switch +-- out the default pre-sdist hook with our own, 'myPreSDist'. +-- +main = defaultMainWithHooks myHooks + where + myHooks = simpleUserHooks { preSDist = myPreSDist } + + +-- | This hook will be executed before e.g. @cabal sdist@. It runs +-- pandoc to create the man page from shellcheck.1.md. If the pandoc +-- command is not found, this will fail with an error message: +-- +-- /bin/sh: pandoc: command not found +-- +-- Since the man page is listed in the Extra-Source-Files section of +-- our cabal file, a failure here should result in a failure to +-- create the distribution tarball (that's a good thing). +-- +myPreSDist :: Args -> SDistFlags -> IO HookedBuildInfo +myPreSDist _ _ = do + putStrLn "Building the man page..." + putStrLn pandoc_cmd + callCommand pandoc_cmd + return emptyHookedBuildInfo + where + pandoc_cmd = "pandoc -s -t man shellcheck.1.md -o shellcheck.1" diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,5 +1,5 @@ Name: ShellCheck -Version: 0.3.5 +Version: 0.3.6 Synopsis: Shell script analysis tool License: AGPL-3 License-file: LICENSE @@ -26,6 +26,8 @@ -- documentation README.md shellcheck.1.md + -- built with a cabal sdist hook + shellcheck.1 -- tests test/shellcheck.hs diff --git a/ShellCheck/AST.hs b/ShellCheck/AST.hs --- a/ShellCheck/AST.hs +++ b/ShellCheck/AST.hs @@ -28,7 +28,6 @@ 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 CaseType = CaseBreak | CaseFallThrough | CaseContinue deriving (Show, Eq) data Token = @@ -49,7 +48,6 @@ | T_Arithmetic Id Token | T_Array Id [Token] | T_IndexedElement Id Token Token - | T_ Id [Token] | T_Assignment Id AssignmentMode String (Maybe Token) Token | T_Backgrounded Id Token | T_Backticked Id [Token] @@ -83,7 +81,7 @@ | T_Fi Id | T_For Id | T_ForArithmetic Id Token Token Token [Token] - | T_ForIn Id ForInType [String] [Token] [Token] + | T_ForIn Id String [Token] [Token] | T_Function Id FunctionKeyword FunctionParentheses String Token | T_GREATAND Id | T_Glob Id String @@ -123,6 +121,8 @@ | T_WhileExpression Id [Token] [Token] | T_Annotation Id [Annotation] Token | T_Pipe Id String + | T_CoProc Id (Maybe String) Token + | T_CoProcBody Id Token deriving (Show) data Annotation = DisableComment Integer deriving (Show, Eq) @@ -205,7 +205,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 t v w l) = dll w l $ T_ForIn id t v + delve (T_ForIn id v w l) = dll w l $ T_ForIn id 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 @@ -248,6 +248,8 @@ delve (TA_Expansion id t) = dl t $ TA_Expansion id delve (TA_Index id t) = d1 t $ TA_Index id delve (T_Annotation id anns t) = d1 t $ T_Annotation id anns + delve (T_CoProc id var body) = d1 body $ T_CoProc id var + delve (T_CoProcBody id t) = d1 t $ T_CoProcBody id delve t = return t getId t = case t of @@ -312,7 +314,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 @@ -341,6 +343,8 @@ T_DollarBracket id _ -> id T_Annotation id _ _ -> id T_Pipe id _ -> id + T_CoProc id _ _ -> id + T_CoProcBody id _ -> id blank :: Monad m => Token -> m () blank = const $ return () diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -27,6 +27,7 @@ import Data.Function (on) import Data.List import Data.Maybe +import Data.Ord import Debug.Trace import ShellCheck.AST import ShellCheck.Options @@ -34,7 +35,8 @@ import ShellCheck.Parser hiding (runTests) import Text.Regex import qualified Data.Map as Map -import Test.QuickCheck.All (quickCheckAll) +import Test.QuickCheck.All (forAllProperties) +import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) data Parameters = Parameters { variableFlow :: [StackData], @@ -58,6 +60,7 @@ ,checkUnpassedInFunctions ,checkArrayWithoutIndex ,checkShebang + ,checkUnassignedReferences ] checksFor Sh = [ @@ -68,10 +71,6 @@ checksFor Ksh = [ checkEchoSed ] -checksFor Zsh = [ - checkTimeParameters - ,checkEchoSed - ] checksFor Bash = [ checkTimeParameters ,checkBraceExpansionVars @@ -113,7 +112,6 @@ shellForExecutable "ksh88" = return Ksh shellForExecutable "ksh93" = return Ksh -shellForExecutable "zsh" = return Zsh shellForExecutable "bash" = return Bash shellForExecutable _ = Nothing @@ -202,6 +200,9 @@ ,checkConcatenatedDollarAt ,checkFindActionPrecedence ,checkTildeInPath + ,checkFindExecWithSingleArgument + ,checkReturn + ,checkMaskedReturns ] @@ -329,6 +330,8 @@ deadSimple (T_Redirecting _ _ foo) = deadSimple foo deadSimple (T_DollarSingleQuoted _ s) = [s] deadSimple (T_Annotation _ _ s) = deadSimple s +-- Workaround for let "foo = bar" parsing +deadSimple (TA_Sequence _ [TA_Expansion _ v]) = concatMap deadSimple v deadSimple _ = [] -- Turn a SimpleCommand foo -avz --bar=baz into args ["a", "v", "z", "bar"] @@ -340,7 +343,7 @@ flag ('-':args) = map (:[]) args flag _ = [] -getFlags _ = [] +getFlags _ = error "Internal shellcheck error, please report! (getFlags on non-command)" (!!!) list i = case drop i list of @@ -360,11 +363,34 @@ verifyNotTree f s = checkTree f s == Just False checkNode f = checkTree (runNodeAnalysis f) -checkTree f s = case parseShell "-" s of +checkTree f s = case parseShell defaultAnalysisOptions "-" s of (ParseResult (Just (t, m)) _) -> Just . not . null $ runList defaultAnalysisOptions t [f] _ -> Nothing +-- Copied from https://wiki.haskell.org/Edit_distance +dist :: Eq a => [a] -> [a] -> Int +dist a b + = last (if lab == 0 then mainDiag + else if lab > 0 then lowers !! (lab - 1) + else{- < 0 -} uppers !! (-1 - lab)) + where mainDiag = oneDiag a b (head uppers) (-1 : head lowers) + uppers = eachDiag a b (mainDiag : uppers) -- upper diagonals + lowers = eachDiag b a (mainDiag : lowers) -- lower diagonals + eachDiag a [] diags = [] + eachDiag a (bch:bs) (lastDiag:diags) = oneDiag a bs nextDiag lastDiag : eachDiag a bs diags + where nextDiag = head (tail diags) + oneDiag a b diagAbove diagBelow = thisdiag + where doDiag [] b nw n w = [] + doDiag a [] nw n w = [] + doDiag (ach:as) (bch:bs) nw n w = me : (doDiag as bs me (tail n) (tail w)) + where me = if ach == bch then nw else 1 + min3 (head w) nw (head n) + firstelt = 1 + head diagBelow + thisdiag = firstelt : doDiag a b firstelt diagAbove (tail diagBelow) + lab = length a - length b + min3 x y z = if x < y then x else min y z + + prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)" checkEchoWc _ (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ @@ -594,7 +620,7 @@ prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh -l " prop_checkShebang3 = verifyTree checkShebang "ls -l" checkShebang params (T_Script id sb _) = - [Note id InfoC 2148 $ "Shebang (#!) missing. Assuming " ++ (show $ shellType params) ++ "." + [Note id ErrorC 2148 "Tips depend on target shell and yours is unknown. Add a shebang." | not (shellTypeSpecified params) && sb == "" ] prop_checkBashisms = verify checkBashisms "while read a; do :; done < <(a)" @@ -615,6 +641,7 @@ prop_checkBashisms16= verify checkBashisms "echo $RANDOM" prop_checkBashisms17= verify checkBashisms "echo $((RANDOM%6+1))" prop_checkBashisms18= verify checkBashisms "foo &> /dev/null" +prop_checkBashisms19= verify checkBashisms "foo > file*.txt" checkBashisms _ = bashism where errMsg id s = err id 2040 $ "In sh, " ++ s ++ " not supported, even when sh is actually bash." @@ -665,6 +692,10 @@ 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 _ = return () @@ -687,17 +718,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)]] _) = +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 " ++ head f ++ "='" ++ s ++ "'." + else warn id 2043 $ "This loop will only run once, with " ++ f ++ "='" ++ s ++ "'." checkForInQuoted _ _ = return () prop_checkForInCat1 = verify checkForInCat "for f in $(cat foo); do stuff; done" @@ -705,7 +736,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 = @@ -721,9 +752,9 @@ prop_checkForInLs3 = verify checkForInLs "for f in `find / -name '*.mp3'`; do mplayer \"$f\"; done" checkForInLs _ = try 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 = @@ -814,7 +845,10 @@ addNote $ note newId addNote $ note exceptId checkOccurrences _ _ = return () - getAllRedirs = concatMap (\(T_Redirecting _ ls _) -> concatMap getRedirs ls) + getAllRedirs = concatMap (\t -> + case t of + T_Redirecting _ ls _ -> concatMap getRedirs ls + _ -> []) getRedirs (T_FdRedirect _ _ (T_IoFile _ op file)) = case op of T_Greater _ -> [file] T_Less _ -> [file] @@ -910,6 +944,8 @@ prop_checkArrayWithoutIndex1 = verifyTree checkArrayWithoutIndex "foo=(a b); echo $foo" prop_checkArrayWithoutIndex2 = verifyNotTree checkArrayWithoutIndex "foo='bar baz'; foo=($foo); echo ${foo[0]}" +prop_checkArrayWithoutIndex3 = verifyTree checkArrayWithoutIndex "coproc foo while true; do echo cow; done; echo $foo" +prop_checkArrayWithoutIndex4 = verifyTree checkArrayWithoutIndex "coproc tail -f log; echo $COPROC" checkArrayWithoutIndex params _ = concat $ doVariableFlowAnalysis readF writeF Map.empty (variableFlow params) where @@ -922,7 +958,7 @@ "Expanding an array without an index only gives the first element."] readF _ _ _ = return [] - writeF _ t name (DataFrom [T_Array {}]) = do + writeF _ t name (DataArray _) = do modify (Map.insert name t) return [] writeF _ _ name _ = do @@ -1027,7 +1063,6 @@ prop_checkNumberComparisons7 = verifyNot checkNumberComparisons "[[ 3.14 == $foo ]]" prop_checkNumberComparisons8 = verify checkNumberComparisons "[[ foo <= bar ]]" prop_checkNumberComparisons9 = verify checkNumberComparisons "[ foo \\>= bar ]" -prop_checkNumberComparisons10= verify checkNumberComparisons "#!/bin/zsh -x\n[ foo >= bar ]]" 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 @@ -1056,9 +1091,7 @@ isLtGt = flip elem ["<", "\\<", ">", "\\>"] isLeGe = flip elem ["<=", "\\<=", ">=", "\\>="] - supportsDecimals = - let sh = shellType params in - sh == Ksh || sh == Zsh + supportsDecimals = (shellType params) == Ksh checkDecimals hs = when (isFraction hs && not supportsDecimals) $ err (getId hs) 2072 decimalError @@ -1359,11 +1392,12 @@ T_Redirecting {} -> return $ if strict then False else -- Not true, just a hack to prevent warning about non-expansion refs - any (isCommand t) ["local", "declare", "typeset", "export", "trap"] + any (isCommand t) ["local", "declare", "typeset", "export", "trap", "readonly"] T_DoubleQuoted _ _ -> return True T_DollarDoubleQuoted _ _ -> return True T_CaseExpression {} -> return True T_HereDoc {} -> return True + T_HereString {} -> return True T_DollarBraced {} -> return True -- When non-strict, pragmatically assume it's desirable to split here T_ForIn {} -> return (not strict) @@ -1516,6 +1550,7 @@ prop_checkUuoeVar5 = verify checkUuoeVar "foo \"$(echo \"$(date) value:\" $value)\"" prop_checkUuoeVar6 = verifyNot checkUuoeVar "foo \"$(echo files: *.png)\"" prop_checkUuoeVar7 = verifyNot checkUuoeVar "foo $(echo $(bar))" -- covered by 2005 +prop_checkUuoeVar8 = verifyNot checkUuoeVar "#!/bin/sh\nz=$(echo)" checkUuoeVar _ p = case p of T_Backticked id [cmd] -> check id cmd @@ -1533,10 +1568,13 @@ check id (T_Pipeline _ _ [T_Redirecting _ _ c]) = warnForEcho id c check _ _ = return () isCovered first rest = null rest && tokenIsJustCommandOutput first - warnForEcho id = checkUnqualifiedCommand "echo" $ \_ vars@(first:rest) -> - unless (isCovered first rest || "-" `isPrefixOf` onlyLiteralString first) $ - when (all couldBeOptimized vars) $ style id 2116 - "Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'." + warnForEcho id = checkUnqualifiedCommand "echo" $ \_ vars -> + case vars of + (first:rest) -> + unless (isCovered first rest || "-" `isPrefixOf` onlyLiteralString first) $ + when (all couldBeOptimized vars) $ style id 2116 + "Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'." + otherwise -> return () prop_checkTr1 = verify checkTr "tr [a-f] [A-F]" prop_checkTr2 = verify checkTr "tr 'a-z' 'A-Z'" @@ -1681,6 +1719,35 @@ special file = concat (deadSimple file) == "/dev/null" checkSudoRedirect _ _ = return () +prop_checkReturn1 = verifyNot checkReturn "return" +prop_checkReturn2 = verifyNot checkReturn "return 1" +prop_checkReturn3 = verifyNot checkReturn "return $var" +prop_checkReturn4 = verifyNot checkReturn "return $((a|b))" +prop_checkReturn5 = verify checkReturn "return -1" +prop_checkReturn6 = verify checkReturn "return 1000" +prop_checkReturn7 = verify checkReturn "return 'hello world'" +checkReturn _ = checkCommand "return" (const f) + where + f (first:second:_) = + err (getId second) 2151 + "Only one integer 0-255 can be returned. Use stdout for other data." + f [value] = + when (isInvalid $ literal value) $ + err (getId value) 2152 + "Can only return 0-255. Other data should be written to stdout." + f _ = return () + + isInvalid s = s == "" || any (not . isDigit) s || length s > 5 + || let value = (read s :: Integer) in value > 255 + + literal token = fromJust $ getLiteralStringExt lit token + lit (T_DollarBraced {}) = return "0" + lit (T_DollarArithmetic {}) = return "0" + lit (T_DollarExpansion {}) = return "0" + lit (T_Backticked {}) = return "0" + lit _ = return "WTF" + + prop_checkPS11 = verify checkPS1Assignments "PS1='\\033[1;35m\\$ '" prop_checkPS11a= verify checkPS1Assignments "export PS1='\\033[1;35m\\$ '" prop_checkPSf2 = verify checkPS1Assignments "PS1='\\h \\e[0m\\$ '" @@ -1707,7 +1774,7 @@ prop_checkBackticks1 = verify checkBackticks "echo `foo`" prop_checkBackticks2 = verifyNot checkBackticks "echo $(foo)" checkBackticks _ (T_Backticked id _) = - style id 2006 "Use $(..) instead of deprecated `..`" + style id 2006 "Use $(..) instead of legacy `..`." checkBackticks _ _ = return () prop_checkIndirectExpansion1 = verify checkIndirectExpansion "${foo$n}" @@ -1739,20 +1806,32 @@ prop_checkInexplicablyUnquoted3 = verifyNot checkInexplicablyUnquoted "wget --user-agent='something'" prop_checkInexplicablyUnquoted4 = verify checkInexplicablyUnquoted "echo \"VALUES (\"id\")\"" prop_checkInexplicablyUnquoted5 = verifyNot checkInexplicablyUnquoted "\"$dir\"/\"$file\"" +prop_checkInexplicablyUnquoted6 = verifyNot checkInexplicablyUnquoted "\"$dir\"some_stuff\"$file\"" checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens) where check (T_SingleQuoted _ _:T_Literal id str:_) | all isAlphaNum str = info id 2026 "This word is outside of quotes. Did you intend to 'nest '\"'single quotes'\"' instead'? " - check (T_DoubleQuoted _ _:trapped:T_DoubleQuoted _ _:_) = + check (T_DoubleQuoted _ a:trapped:T_DoubleQuoted _ b:_) = case trapped of T_DollarExpansion id _ -> warnAboutExpansion id T_DollarBraced id _ -> warnAboutExpansion id - T_Literal id s -> unless (s == "/" || s == "=") $ warnAboutLiteral id + T_Literal id s -> + unless (quotesSingleThing a && quotesSingleThing b) $ + warnAboutLiteral id _ -> return () check _ = return () + + -- If the surrounding quotes quote single things, like "$foo"_and_then_some_"$stuff", + -- the quotes were probably intentional and harmless. + quotesSingleThing x = case x of + [T_DollarExpansion _ _] -> True + [T_DollarBraced _ _] -> True + [T_Backticked _ _] -> True + _ -> False + warnAboutExpansion id = warn id 2027 "The surrounding quotes actually unquote this. Remove or escape them." warnAboutLiteral id = @@ -1766,7 +1845,7 @@ prop_checkTildeInQuotes6 = verifyNot checkTildeInQuotes "awk '$0 ~ /foo/'" checkTildeInQuotes _ = check where - verify id ('~':_) = warn id 2088 "Note that ~ does not expand in quotes." + verify id ('~':'/':_) = warn id 2088 "Note that ~ does not expand in quotes." verify _ _ = return () check (T_NormalWord _ (T_SingleQuoted id str:_)) = verify id str @@ -1795,7 +1874,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 @@ -1923,7 +2002,8 @@ 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\"" +prop_subshellAssignmentCheck15 = verifyNotTree subshellAssignmentCheck "#!/bin/ksh\ncat foo | while read bar; do a=$bar; done\necho \"$a\"" +prop_subshellAssignmentCheck16 = verifyNotTree subshellAssignmentCheck "(set -e); echo $@" subshellAssignmentCheck params t = let flow = variableFlow params check = findSubshelled flow [("oops",[])] Map.empty @@ -1935,20 +2015,27 @@ StackScope Scope | StackScopeEnd -- (Base expression, specific position, var name, assigned values) - | Assignment (Token, Token, String, DataSource) + | Assignment (Token, Token, String, DataType) | Reference (Token, Token, String) deriving (Show, Eq) -data DataSource = DataFrom [Token] | DataExternal + +data DataType = DataString DataSource | DataArray DataSource deriving (Show, Eq) +data DataSource = SourceFrom [Token] | SourceExternal | SourceDeclaration + deriving (Show, Eq) + data VariableState = Dead Token String | Alive deriving (Show, Eq) +dataTypeFrom defaultType v = (case v of T_Array {} -> DataArray; _ -> defaultType) $ SourceFrom [v] + leadType shell parents t = case t of T_DollarExpansion _ _ -> SubshellScope "$(..) expansion" T_Backticked _ _ -> SubshellScope "`..` expansion" T_Backgrounded _ _ -> SubshellScope "backgrounding &" T_Subshell _ _ -> SubshellScope "(..) group" + T_CoProcBody _ _ -> SubshellScope "coproc" T_Redirecting {} -> if fromMaybe False causesSubshell then SubshellScope "pipeline" @@ -1974,14 +2061,13 @@ Bash -> True Sh -> True Ksh -> False - Zsh -> False getModifiedVariables t = case t of T_SimpleCommand _ vars [] -> concatMap (\x -> case x of - T_Assignment id _ name _ w -> - [(x, x, name, DataFrom [w])] + T_Assignment id _ name _ w -> + [(x, x, name, dataTypeFrom DataString w)] _ -> [] ) vars c@(T_SimpleCommand {}) -> @@ -1989,27 +2075,33 @@ TA_Unary _ "++|" var -> maybeToList $ do name <- getLiteralString var - return (t, t, name, DataFrom [t]) + return (t, t, name, DataString $ SourceFrom [t]) TA_Unary _ "|++" var -> maybeToList $ do name <- getLiteralString var - return (t, t, name, DataFrom [t]) + return (t, t, name, DataString $ SourceFrom [t]) TA_Binary _ op lhs rhs -> maybeToList $ do guard $ op `elem` ["=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", "&=", "^=", "|="] name <- getLiteralString lhs - return (t, t, name, DataFrom [rhs]) + return (t, t, name, DataString $ SourceFrom [rhs]) + t@(T_CoProc _ name _) -> + [(t, t, fromMaybe "COPROC" name, DataArray SourceExternal)] + --Points to 'for' rather than variable - T_ForIn id _ strs words _ -> map (\str -> (t, t, str, DataFrom words)) strs - T_SelectIn id str words _ -> [(t, t, str, DataFrom words)] + T_ForIn id str words _ -> [(t, t, str, DataString $ SourceFrom words)] + T_SelectIn id str words _ -> [(t, t, str, DataString $ SourceFrom words)] _ -> [] -- Consider 'export/declare -x' a reference, since it makes the var available getReferencedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal _ x:_):rest)) = case x of - "export" -> concatMap getReference rest - "declare" -> if "x" `elem` getFlags base + "export" -> if "f" `elem` flags + then [] + else concatMap getReference rest + "declare" -> if "x" `elem` flags then concatMap getReference rest else [] + "readonly" -> concatMap getReference rest "trap" -> case rest of head:_ -> map (\x -> (head, head, x)) $ getVariablesFromLiteralToken head @@ -2019,6 +2111,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 getReferencedVariableCommand _ = [] @@ -2028,16 +2121,30 @@ "read" -> let params = map getLiteral rest in catMaybes . takeWhile isJust . reverse $ params + "getopts" -> + case rest of + opts:var:_ -> maybeToList $ getLiteral var + _ -> [] + "let" -> concatMap letParamToLiteral rest - "export" -> concatMap getModifierParam rest - "declare" -> concatMap getModifierParam rest - "typeset" -> concatMap getModifierParam rest - "local" -> concatMap getModifierParam rest - "set" -> [(base, base, "@", DataFrom rest)] + "export" -> + if "f" `elem` flags then [] else concatMap getModifierParamString rest + "declare" -> declaredVars + "typeset" -> declaredVars + + "local" -> concatMap getModifierParamString rest + "readonly" -> concatMap getModifierParamString rest + "set" -> maybeToList $ do + params <- getSetParams rest + return (base, base, "@", DataString $ SourceFrom params) + + "printf" -> maybeToList $ getPrintfVariable rest + _ -> [] where + flags = getFlags base stripEquals s = let rest = dropWhile (/= '=') s in if rest == "" then "" else tail rest stripEqualsFrom (T_NormalWord id1 (T_Literal id2 s:rs)) = @@ -2046,24 +2153,72 @@ T_NormalWord id1 [T_DoubleQuoted id2 [T_Literal id3 (stripEquals s)]] stripEqualsFrom t = t + declaredVars = concatMap (getModifierParam defaultType) rest + where + defaultType = if any (`elem` flags) ["a", "A"] then DataArray else DataString + getLiteral t = do s <- getLiteralString t when ("-" `isPrefixOf` s) $ fail "argument" - return (base, t, s, DataExternal) + return (base, t, s, DataString SourceExternal) - getModifierParam t@(T_Assignment _ _ name _ value) = - [(base, t, name, DataFrom [value])] - getModifierParam _ = [] + getModifierParamString = getModifierParam DataString + getModifierParam def t@(T_Assignment _ _ name _ value) = + [(base, t, name, dataTypeFrom def value)] + getModifierParam def t@(T_NormalWord {}) = maybeToList $ do + name <- getLiteralString t + guard $ isVariableName name + return (base, t, name, def SourceDeclaration) + getModifierParam _ _ = [] + letParamToLiteral token = if var == "" then [] - else [(base, token, var, DataFrom [stripEqualsFrom token])] + else [(base, token, var, DataString $ SourceFrom [stripEqualsFrom token])] where var = takeWhile isVariableChar $ dropWhile (`elem` "+-") $ concat $ deadSimple token + + getSetParams (t:_:rest) | getLiteralString t == Just "-o" = getSetParams rest + getSetParams (t:rest) = + let s = getLiteralString t in + case s of + Just "--" -> return rest + Just ('-':_) -> getSetParams rest + _ -> return (t:fromMaybe [] (getSetParams rest)) + getSetParams [] = Nothing + + getPrintfVariable list = f $ map (\x -> (x, getLiteralString x)) list + where + f ((_, Just "-v") : (t, Just var) : _) = return (base, t, var, DataString $ SourceFrom list) + f (_:rest) = f rest + f [] = fail "not found" + getModifiedVariableCommand _ = [] --- TODO: -getBracedReference s = takeWhile (`notElem` ":[#%/^,") $ dropWhile (`elem` "#!") s +prop_getBracedReference1 = getBracedReference "foo" == "foo" +prop_getBracedReference2 = getBracedReference "#foo" == "foo" +prop_getBracedReference3 = getBracedReference "#" == "#" +prop_getBracedReference4 = getBracedReference "##" == "#" +prop_getBracedReference5 = getBracedReference "#!" == "!" +prop_getBracedReference6 = getBracedReference "!#" == "#" +prop_getBracedReference7 = getBracedReference "!foo#?" == "foo" +prop_getBracedReference8 = getBracedReference "foo-bar" == "foo" +prop_getBracedReference9 = getBracedReference "foo:-bar" == "foo" +prop_getBracedReference10= getBracedReference "foo: -1" == "foo" +getBracedReference s = fromMaybe s $ + takeName noPrefix `mplus` getSpecial noPrefix `mplus` getSpecial s + where + noPrefix = dropPrefix s + dropPrefix (c:rest) = if c `elem` "!#" then rest else c:rest + dropPrefix "" = "" + takeName s = do + let name = takeWhile isVariableChar s + guard . not $ null name + return name + getSpecial (c:_) = + if c `elem` "*@#?-$!" then return [c] else fail "not special" + getSpecial _ = fail "empty" + getIndexReferences s = fromMaybe [] $ do (_, index, _, _) <- matchRegexAll re s return $ matchAll variableNameRegex index @@ -2075,13 +2230,12 @@ T_DollarBraced id l -> let str = bracedString l in (t, t, getBracedReference str) : map (\x -> (l, l, x)) (getIndexReferences str) - TA_Expansion id _ -> maybeToList $ do - str <- getLiteralStringExt literalizer t - guard . not $ null str - when (isDigit $ head str) $ fail "is a number" - return (t, t, getBracedReference str) + TA_Expansion id _ -> getIfReference t t T_Assignment id mode str _ word -> [(t, t, str) | mode == Append] ++ specialReferences str t word + + TC_Unary id _ "-v" token -> getIfReference t token + TC_Unary id _ "-R" token -> getIfReference t token x -> getReferencedVariableCommand x where -- Try to reduce false positives for unused vars only referenced from evaluated vars @@ -2098,6 +2252,12 @@ literalizer (TA_Index {}) = return "" -- x[0] becomes a reference of x literalizer _ = Nothing + getIfReference context token = maybeToList $ do + str <- getLiteralStringExt literalizer token + guard . not $ null str + when (isDigit $ head str) $ fail "is a number" + return (context, token, getBracedReference str) + -- Try to get referenced variables from a literal string like "$foo" -- Ignores tons of cases like arithmetic evaluation and array indices. prop_getVariablesFromLiteral1 = @@ -2143,12 +2303,15 @@ findSubshelled (Assignment x@(_, _, str, _):rest) ((reason,scope):lol) deadVars = findSubshelled rest ((reason, x:scope):lol) $ Map.insert str Alive deadVars findSubshelled (Reference (_, readToken, str):rest) scopes deadVars = do - case Map.findWithDefault Alive str deadVars of + unless (shouldIgnore str) $ case Map.findWithDefault Alive str deadVars of Alive -> return () Dead writeToken reason -> do info (getId writeToken) 2030 $ "Modification of " ++ str ++ " is local (to subshell caused by "++ reason ++")." info (getId readToken) 2031 $ str ++ " was modified in a subshell. That change might be lost." findSubshelled rest scopes deadVars + where + shouldIgnore str = + str `elem` ["@", "*", "IFS"] findSubshelled (StackScope (SubshellScope reason):rest) scopes deadVars = findSubshelled rest ((reason,[]):scopes) deadVars @@ -2178,19 +2341,22 @@ 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" -prop_checkSpacefulnessK = verifyNotTree checkSpacefulness "n+='foo bar'" -prop_checkSpacefulnessL = verifyNotTree checkSpacefulness "select foo in $bar; do true; done" -prop_checkSpacefulnessM = verifyNotTree checkSpacefulness "echo $\"$1\"" +prop_checkSpacefulness10= verifyTree checkSpacefulness "rm $1" +prop_checkSpacefulness11= verifyTree checkSpacefulness "rm ${10//foo/bar}" +prop_checkSpacefulness12= verifyNotTree checkSpacefulness "(( $1 + 3 ))" +prop_checkSpacefulness13= verifyNotTree checkSpacefulness "if [[ $2 -gt 14 ]]; then true; fi" +prop_checkSpacefulness14= verifyNotTree checkSpacefulness "foo=$3 env" +prop_checkSpacefulness15= verifyNotTree checkSpacefulness "local foo=$1" +prop_checkSpacefulness16= verifyNotTree checkSpacefulness "declare foo=$1" +prop_checkSpacefulness17= verifyTree checkSpacefulness "echo foo=$1" +prop_checkSpacefulness18= verifyNotTree checkSpacefulness "$1 --flags" +prop_checkSpacefulness19= verifyTree checkSpacefulness "echo $PWD" +prop_checkSpacefulness20= verifyNotTree checkSpacefulness "n+='foo bar'" +prop_checkSpacefulness21= verifyNotTree checkSpacefulness "select foo in $bar; do true; done" +prop_checkSpacefulness22= verifyNotTree checkSpacefulness "echo $\"$1\"" +prop_checkSpacefulness23= verifyNotTree checkSpacefulness "a=(1); echo ${a[@]}" +prop_checkSpacefulness24= verifyNotTree checkSpacefulness "a='a b'; cat <<< $a" +prop_checkSpacefulness25= verifyTree checkSpacefulness "a='s/[0-9]//g'; sed $a" checkSpacefulness params t = doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params) @@ -2208,23 +2374,25 @@ spaced <- hasSpaces name return [Note (getId token) InfoC 2086 warning | spaced - && not ("@" `isPrefixOf` name) -- There's another warning for this + && not (isArrayExpansion token) -- There's another warning for this && not (isCounting token) && not (isQuoteFree parents token) && not (usedAsCommandName parents token)] where warning = "Double quote to prevent globbing and word splitting." - writeF _ _ name DataExternal = do + writeF _ _ name (DataString SourceExternal) = do setSpaces name True return [] - writeF _ _ name (DataFrom vals) = do + writeF _ _ name (DataString (SourceFrom vals)) = do map <- get setSpaces name (isSpacefulWord (\x -> Map.findWithDefault True x map) vals) return [] + writeF _ _ _ _ = return [] + parents = parentMap params isCounting (T_DollarBraced id token) = @@ -2249,10 +2417,9 @@ T_DoubleQuoted _ w -> isSpacefulWord spacefulF w _ -> False where - globspace = "*? \t\n" + globspace = "*?[] \t\n" containsAny s = any (`elem` s) - 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\"" @@ -2273,7 +2440,7 @@ quoteRegex = mkRegex "\"|([/= ]|^)'|'( |$)|\\\\ " containsQuotes s = s `matches` quoteRegex - writeF _ _ name (DataFrom values) = do + writeF _ _ name (DataString (SourceFrom values)) = do quoteMap <- get let quotedVars = msum $ map (forToken quoteMap) values case quotedVars of @@ -2376,6 +2543,8 @@ prop_checkUnused19= verifyNotTree checkUnusedAssignments "a=1; let b=a+1; echo $b" prop_checkUnused20= verifyNotTree checkUnusedAssignments "a=1; PS1='$a'" prop_checkUnused21= verifyNotTree checkUnusedAssignments "a=1; trap 'echo $a' INT" +prop_checkUnused22= verifyNotTree checkUnusedAssignments "a=1; [ -v a ]" +prop_checkUnused23= verifyNotTree checkUnusedAssignments "a=1; [ -R a ]" checkUnusedAssignments params t = execWriter (mapM_ warnFor unused) where flow = variableFlow params @@ -2392,12 +2561,104 @@ unused = Map.assocs $ Map.difference assignments references warnFor (name, token) = - info (getId token) 2034 $ + warn (getId token) 2034 $ name ++ " appears unused. Verify it or export it." stripSuffix = takeWhile isVariableChar defaultMap = Map.fromList $ zip internalVariables $ repeat () +prop_checkUnassignedReferences1 = verifyTree checkUnassignedReferences "echo $foo" +prop_checkUnassignedReferences2 = verifyNotTree checkUnassignedReferences "foo=hello; echo $foo" +prop_checkUnassignedReferences3 = verifyTree checkUnassignedReferences "MY_VALUE=3; echo $MYVALUE" +prop_checkUnassignedReferences4 = verifyNotTree checkUnassignedReferences "RANDOM2=foo; echo $RANDOM" +prop_checkUnassignedReferences5 = verifyNotTree checkUnassignedReferences "declare -A foo=([bar]=baz); echo ${foo[bar]}" +prop_checkUnassignedReferences6 = verifyNotTree checkUnassignedReferences "foo=..; echo ${foo-bar}" +prop_checkUnassignedReferences7 = verifyNotTree checkUnassignedReferences "getopts ':h' foo; echo $foo" +prop_checkUnassignedReferences8 = verifyNotTree checkUnassignedReferences "let 'foo = 1'; echo $foo" +prop_checkUnassignedReferences9 = verifyNotTree checkUnassignedReferences "echo ${foo-bar}" +prop_checkUnassignedReferences10= verifyNotTree checkUnassignedReferences "echo ${foo:?}" +prop_checkUnassignedReferences11= verifyNotTree checkUnassignedReferences "declare -A foo; echo \"${foo[@]}\"" +prop_checkUnassignedReferences12= verifyNotTree checkUnassignedReferences "typeset -a foo; echo \"${foo[@]}\"" +prop_checkUnassignedReferences13= verifyNotTree checkUnassignedReferences "f() { local foo; echo $foo; }" +prop_checkUnassignedReferences14= verifyNotTree checkUnassignedReferences "foo=; echo $foo" +prop_checkUnassignedReferences15= verifyNotTree checkUnassignedReferences "f() { true; }; export -f f" +prop_checkUnassignedReferences16= verifyNotTree checkUnassignedReferences "declare -A foo=( [a b]=bar ); echo ${foo[a b]}" +prop_checkUnassignedReferences17= verifyNotTree checkUnassignedReferences "USERS=foo; echo $USER" +prop_checkUnassignedReferences18= verifyNotTree checkUnassignedReferences "FOOBAR=42; export FOOBAR=" +prop_checkUnassignedReferences19= verifyNotTree checkUnassignedReferences "readonly foo=bar; echo $foo" +prop_checkUnassignedReferences20= verifyNotTree checkUnassignedReferences "printf -v foo bar; echo $foo" +prop_checkUnassignedReferences21= verifyTree checkUnassignedReferences "echo ${#foo}" +checkUnassignedReferences params t = warnings + where + (readMap, writeMap) = execState (mapM tally $ variableFlow params) (Map.empty, Map.empty) + defaultAssigned = Map.fromList $ map (\a -> (a, ())) $ filter (not . null) internalVariables + + tally (Assignment (_, _, name, _)) = + modify (\(read, written) -> (read, Map.insert name () written)) + tally (Reference (_, place, name)) = + modify (\(read, written) -> (Map.insertWith' (const id) name place read, written)) + tally _ = return () + + unassigned = Map.toList $ Map.difference (Map.difference readMap writeMap) defaultAssigned + writtenVars = filter isVariableName $ Map.keys writeMap + + getBestMatch var = do + (match, score) <- listToMaybe best + guard $ goodMatch var match score + return match + where + matches = map (\x -> (x, match var x)) writtenVars + best = sortBy (comparing snd) matches + goodMatch var match score = + let l = length match in + l > 3 && score <= 1 + || l > 7 && score <= 2 + + isLocal = any isLower + + warningForGlobals var place = do + match <- getBestMatch var + return $ warn (getId place) 2153 $ + "Possible misspelling: " ++ var ++ " may not be assigned, but " ++ match ++ " is." + + warningForLocals var place = + return $ warn (getId place) 2154 $ + var ++ " is referenced but not assigned" ++ optionalTip ++ "." + where + optionalTip = + if var `elem` commonCommands + then " (for output from commands, use \"$(" ++ var ++ " ..." ++ ")\" )" + else fromMaybe "" $ do + match <- getBestMatch var + return $ " (did you mean '" ++ match ++ "'?)" + + warningFor var place = do + guard . not $ isInArray var place || isGuarded place + (if isLocal var then warningForLocals else warningForGlobals) var place + + warnings = execWriter . sequence $ mapMaybe (uncurry warningFor) unassigned + + -- Due to parsing, foo=( [bar]=baz ) parses 'bar' as a reference even for assoc arrays. + -- Similarly, ${foo[bar baz]} may not be referencing bar/baz. Just skip these. + isInArray var t = any isArray $ getPath (parentMap params) t + where + isArray (T_Array {}) = True + isArray (T_DollarBraced _ l) | var /= getBracedReference (bracedString l) = True + isArray _ = False + + isGuarded (T_DollarBraced _ v) = + any (`isPrefixOf` rest) ["-", ":-", "?", ":?"] + where + name = concat $ deadSimple v + rest = dropWhile isVariableChar $ dropWhile (`elem` "#!") $ name + isGuarded _ = False + + match var candidate = + if var /= candidate && (map toLower var) == (map toLower candidate) + then 1 + else dist var candidate + + prop_checkGlobsAsOptions1 = verify checkGlobsAsOptions "rm *.txt" prop_checkGlobsAsOptions2 = verify checkGlobsAsOptions "ls ??.*" prop_checkGlobsAsOptions3 = verifyNot checkGlobsAsOptions "rm -- *.txt" @@ -2512,7 +2773,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 @@ -2536,7 +2797,7 @@ warn (getId $ head cds) 2103 message message = - if shell == Bash || shell == Zsh + if shell == Bash then "Consider using ( subshell ), 'cd foo||exit', or pushd/popd instead." else "Consider using ( subshell ) or 'cd foo||exit' instead." @@ -2576,7 +2837,6 @@ (T_Function id (FunctionKeyword hasKeyword) (FunctionParentheses hasParens) _ _) = case shellType params of Bash -> return () - Zsh -> return () Ksh -> when (hasKeyword && hasParens) $ err id 2111 "ksh does not allow 'function' keyword and '()' at the same time." @@ -2588,46 +2848,50 @@ checkFunctionDeclarations _ _ = return () --- This is a lot of code for little gain. Consider whether it's worth it. prop_checkCatastrophicRm1 = verify checkCatastrophicRm "rm -r $1/$2" -prop_checkCatastrophicRm2 = verify checkCatastrophicRm "foo=$(echo bar); rm -r /home/$foo" -prop_checkCatastrophicRm3 = verify checkCatastrophicRm "foo=/home; user=$(whoami); rm -r \"$foo/$user\"" -prop_checkCatastrophicRm4 = verifyNot checkCatastrophicRm "foo=/home; user=cow; rm -r \"$foo/$user\"" -prop_checkCatastrophicRm5 = verifyNot checkCatastrophicRm "user=$(whoami); rm -r /home/${user:?Nope}" +prop_checkCatastrophicRm2 = verify checkCatastrophicRm "rm -r /home/$foo" +prop_checkCatastrophicRm3 = verifyNot checkCatastrophicRm "rm -r /home/${USER:?}/*" +prop_checkCatastrophicRm4 = verify checkCatastrophicRm "rm -fr /home/$(whoami)/*" +prop_checkCatastrophicRm5 = verifyNot checkCatastrophicRm "rm -r /home/${USER:-thing}/*" prop_checkCatastrophicRm6 = verify checkCatastrophicRm "rm --recursive /etc/*$config*" -prop_checkCatastrophicRm7 = verifyNot checkCatastrophicRm "var=$(cmd); if [ -n \"$var\" ]; then rm -r /etc/$var/*; fi" prop_checkCatastrophicRm8 = verify checkCatastrophicRm "rm -rf /home" prop_checkCatastrophicRm9 = verifyNot checkCatastrophicRm "rm -rf -- /home" +prop_checkCatastrophicRmA = verify checkCatastrophicRm "rm -rf /usr /lib/nvidia-current/xorg/xorg" +prop_checkCatastrophicRmB = verify checkCatastrophicRm "rm -rf \"$STEAMROOT/\"*" checkCatastrophicRm params t@(T_SimpleCommand id _ tokens) | t `isCommand` "rm" = when (any isRecursiveFlag simpleArgs) $ mapM_ checkWord tokens where - -- This ugly hack is based on the fact that ids generally increase - relevantMap (Id n) = liftM snd . listToMaybe . dropWhile (\(Id x, _) -> x > n) $ flowMapR - flowMapR = reverse $ (\x -> zip (scanl getScopeId (Id 0) x) (scanl addNulls defaultMap x)) $ variableFlow params simpleArgs = deadSimple t - defaultMap = Map.fromList (map (\x -> (x, Nothing)) variablesWithoutSpaces) checkWord token = case getLiteralString token of Just str -> when (notElem "--" simpleArgs && (fixPath str `elem` importantPaths)) $ - info (getId token) 2114 "Obligatory typo warning. Use 'rm --' to disable this message." + warn (getId token) 2114 "Warning: deletes a system directory. Use 'rm --' to disable this message." Nothing -> checkWord' token checkWord' token = fromMaybe (return ()) $ do - m <- relevantMap id - filename <- combine m token + filename <- getPotentialPath token let path = fixPath filename return . when (path `elem` importantPaths) $ - warn (getId token) 2115 $ "Make sure this never accidentally expands to '" ++ path ++ "'." + warn (getId token) 2115 $ "Use \"${var:?}\" to ensure this never expands to " ++ path ++ " ." fixPath filename = let normalized = skipRepeating '/' . skipRepeating '*' $ filename in if normalized == "/" then normalized else stripTrailing '/' normalized - unnullable = all isVariableChar . concat . deadSimple + getPotentialPath = getLiteralStringExt f + where + f (T_Glob _ str) = return str + f (T_DollarBraced _ word) = + let var = onlyLiteralString word in + if any (flip isInfixOf var) [":?", ":-", ":="] + then Nothing + else return "" + f _ = return "" + isRecursiveFlag "--recursive" = True isRecursiveFlag ('-':'-':_) = False isRecursiveFlag ('-':str) = 'r' `elem` str || 'R' `elem` str @@ -2638,55 +2902,12 @@ skipRepeating c (a:r) = a:skipRepeating c r skipRepeating _ [] = [] - addNulls map (Reference (_, token, name)) = - if mightBeGuarded token - then Map.insert name Nothing map - else map - addNulls map (Assignment (_, token, name, DataExternal)) = - if mightBeGuarded token - then Map.insert name Nothing map - else Map.insert name (Just "") map - addNulls m (Assignment (_, token, name, DataFrom [word])) - | mightBeGuarded token = Map.insert name Nothing m - | couldFail word = m - | otherwise = Map.insert name (combine m word) m - addNulls m (Assignment (_, token, name, DataFrom _)) = - Map.insert name Nothing m - addNulls map _ = map - - getScopeId n (Reference (_, token, _)) = getId token - getScopeId n (Assignment (_, token, _, _)) = getId token - getScopeId n _ = n - - joinMaybes :: [Maybe String] -> Maybe String - joinMaybes = foldl (liftM2 (++)) (Just "") - combine m = c - where - c (T_DollarBraced _ t) | unnullable t = - Map.findWithDefault (Just "") (concat $ deadSimple t) m - c (T_DoubleQuoted _ tokens) = joinMaybes $ map (combine m) tokens - c (T_NormalWord _ tokens) = joinMaybes $ map (combine m) tokens - c (T_Glob _ "*") = Just "*" - c t = getLiteralString t - - couldFail (T_Backticked _ _) = True - couldFail (T_DollarExpansion _ _) = True - couldFail (T_DoubleQuoted _ foo) = any couldFail foo - couldFail (T_NormalWord _ foo) = any couldFail foo - couldFail _ = False - - mightBeGuarded token = any t (getPath (parentMap params) token) - where - t (T_Condition {}) = True - t (T_OrIf {}) = True - t (T_AndIf {}) = True - t _ = False - paths = [ - "/", "/etc", "/home", "/mnt", "/usr", "/usr/share", "/usr/local", - "/var" + "", "/bin", "/etc", "/home", "/mnt", "/usr", "/usr/share", "/usr/local", + "/var", "/lib" ] - importantPaths = ["", "/*", "/*/*"] >>= (\x -> map (++x) paths) + importantPaths = filter (not . null) $ + ["", "/", "/*", "/*/*"] >>= (\x -> map (++x) paths) checkCatastrophicRm _ _ = return () @@ -2708,7 +2929,7 @@ prop_checkStderrPipe1 = verify checkStderrPipe "#!/bin/ksh\nfoo |& bar" -prop_checkStderrPipe2 = verifyNot checkStderrPipe "#!/bin/zsh\nfoo |& bar" +prop_checkStderrPipe2 = verifyNot checkStderrPipe "#!/bin/bash\nfoo |& bar" checkStderrPipe params = case shellType params of Ksh -> match @@ -2727,6 +2948,7 @@ prop_checkUnpassedInFunctions7 = verifyTree checkUnpassedInFunctions "foo() { echo $1; }; foo; foo;" prop_checkUnpassedInFunctions8 = verifyNotTree checkUnpassedInFunctions "foo() { echo $((1)); }; foo;" prop_checkUnpassedInFunctions9 = verifyNotTree checkUnpassedInFunctions "foo() { echo $(($b)); }; foo;" +prop_checkUnpassedInFunctions10= verifyNotTree checkUnpassedInFunctions "foo() { echo $!; }; foo;" checkUnpassedInFunctions params root = execWriter $ mapM_ warnForGroup referenceGroups where @@ -2844,12 +3066,10 @@ isQuoted _ = False checkTildeInPath _ _ = return () -prop_checkUnsupported1 = verifyNot checkUnsupported "#!/bin/zsh\nfunction { echo cow; }" -prop_checkUnsupported2 = verify checkUnsupported "#!/bin/sh\nfunction { echo cow; }" prop_checkUnsupported3 = verify checkUnsupported "#!/bin/sh\ncase foo in bar) baz ;& esac" prop_checkUnsupported4 = verify checkUnsupported "#!/bin/ksh\ncase foo in bar) baz ;;& esac" checkUnsupported params t = - when ((not $ null support) && (shellType params `notElem` support)) $ + when (not (null support) && (shellType params `notElem` support)) $ report name where (name, support) = shellSupport t @@ -2860,15 +3080,11 @@ -- 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]) T_CaseExpression _ _ list -> forCase (map (\(a,_,_) -> a) list) otherwise -> ("", []) where - forCase seps | any (== CaseContinue) seps = ("cases with ;;&", [Bash]) - forCase seps | any (== CaseFallThrough) seps = ("cases with ;&", [Bash, Ksh, Zsh]) + forCase seps | CaseContinue `elem` seps = ("cases with ;;&", [Bash]) + forCase seps | CaseFallThrough `elem` seps = ("cases with ;&", [Bash, Ksh]) forCase _ = ("", []) @@ -2877,7 +3093,7 @@ getCommandSequences (T_Subshell _ cmds) = [cmds] getCommandSequences (T_WhileExpression _ _ cmds) = [cmds] getCommandSequences (T_UntilExpression _ _ cmds) = [cmds] -getCommandSequences (T_ForIn _ _ _ _ cmds) = [cmds] +getCommandSequences (T_ForIn _ _ _ cmds) = [cmds] getCommandSequences (T_ForArithmetic _ _ _ _ cmds) = [cmds] getCommandSequences (T_IfExpression _ thens elses) = map snd thens ++ [elses] getCommandSequences _ = [] @@ -2957,6 +3173,7 @@ prop_checkGrepQ3= verify checkShouldUseGrepQ "[ -n \"$(foo | zgrep lol)\" ]" prop_checkGrepQ4= verifyNot checkShouldUseGrepQ "[ -z $(grep bar | cmd) ]" prop_checkGrepQ5= verifyNot checkShouldUseGrepQ "rm $(ls | grep file)" +prop_checkGrepQ6= verifyNot checkShouldUseGrepQ "[[ -n $(pgrep foo) ]]" checkShouldUseGrepQ params t = potentially $ case t of TC_Noary id _ token -> check id True token @@ -2969,8 +3186,8 @@ 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 ." + "Use " ++ flip ++ name ++ " -q instead of " ++ + "comparing output with [ " ++ op ++ " .. ]." getFinalGrep t = do cmds <- getPipeline t @@ -2985,7 +3202,7 @@ T_DollarExpansion _ [x] -> getPipeline x T_Pipeline _ _ cmds -> return cmds _ -> fail "unknown" - isGrep = isSuffixOf "grep" + isGrep = (`elem` ["grep", "egrep", "fgrep", "zgrep"]) prop_checkTestGlobs1 = verify checkTestGlobs "[ -e *.mp3 ]" prop_checkTestGlobs2 = verifyNot checkTestGlobs "[[ $a == *b* ]]" @@ -3003,7 +3220,7 @@ f list | length list < length pattern = return () f list@(_:rest) = if all id (zipWith ($) pattern list) - then warnFor (list !! ((length pattern)-1)) + then warnFor (list !! (length pattern - 1)) else f rest isMatch = isParam [ "-name", "-regex", "-iname", "-iregex", "-wholename", "-iwholename" ] isAction = isParam [ "-exec", "-execdir", "-delete", "-print", "-print0" ] @@ -3012,6 +3229,46 @@ return $ param `elem` strs warnFor t = warn (getId t) 2146 "This action ignores everything before the -o. Use \\( \\) to group." -return [] -runTests = $quickCheckAll +prop_checkFindExecWithSingleArgument1 = verify checkFindExecWithSingleArgument "find . -exec 'cat {} | wc -l' \\;" +prop_checkFindExecWithSingleArgument2 = verify checkFindExecWithSingleArgument "find . -execdir 'cat {} | wc -l' +" +prop_checkFindExecWithSingleArgument3 = verifyNot checkFindExecWithSingleArgument "find . -exec wc -l {} \\;" +checkFindExecWithSingleArgument _ = checkCommand "find" (const f) + where + f = void . sequence . mapMaybe check . tails + check (exec:arg:term:_) = do + execS <- getLiteralString exec + termS <- getLiteralString term + cmdS <- getLiteralStringExt (const $ return " ") arg + + guard $ execS `elem` ["-exec", "-execdir"] && termS `elem` [";", "+"] + guard $ cmdS `matches` commandRegex + return $ warn (getId exec) 2150 "-exec does not invoke a shell. Rewrite or use -exec sh -c .. ." + check _ = Nothing + commandRegex = mkRegex "[ |;]" + + +prop_checkMaskedReturns1 = verify checkMaskedReturns "f() { local a=$(false); }" +prop_checkMaskedReturns2 = verify checkMaskedReturns "declare a=$(false)" +prop_checkMaskedReturns3 = verify checkMaskedReturns "declare a=\"`false`\"" +prop_checkMaskedReturns4 = verifyNot checkMaskedReturns "declare a; a=$(false)" +prop_checkMaskedReturns5 = verifyNot checkMaskedReturns "f() { local -r a=$(false); }" +checkMaskedReturns _ t@(T_SimpleCommand id _ (cmd:rest)) = potentially $ do + name <- getCommandName t + guard $ name `elem` ["declare", "export"] + || name == "local" && "r" `notElem` getFlags t + return $ mapM_ checkArgs rest + where + checkArgs (T_Assignment id _ _ _ word) | any hasReturn $ getWordParts word = + warn id 2155 "Declare and assign separately to avoid masking return values." + checkArgs _ = return () + + hasReturn t = case t of + T_Backticked {} -> True + T_DollarExpansion {} -> True + _ -> False +checkMaskedReturns _ _ = return () + + +return [] +runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/ShellCheck/Data.hs b/ShellCheck/Data.hs --- a/ShellCheck/Data.hs +++ b/ShellCheck/Data.hs @@ -27,22 +27,10 @@ "LC_MESSAGES", "LC_NUMERIC", "LINES", "MAIL", "MAILCHECK", "MAILPATH", "OPTERR", "PATH", "POSIXLY_CORRECT", "PROMPT_COMMAND", "PROMPT_DIRTRIM", "PS1", "PS2", "PS3", "PS4", "SHELL", "TIMEFORMAT", - "TMOUT", "TMPDIR", "auto_resume", "histchars", + "TMOUT", "TMPDIR", "auto_resume", "histchars", "COPROC", - -- Zsh - "ARGV0", "BAUD", "cdpath", "COLUMNS", "CORRECT_IGNORE", - "DIRSTACKSIZE", "ENV", "FCEDIT", "fignore", "fpath", "histchars", - "HISTCHARS", "HISTFILE", "HISTSIZE", "HOME", "IFS", "KEYBOARD_HACK", - "KEYTIMEOUT", "LANG", "LC_ALL", "LC_COLLATE", "LC_CTYPE", - "LC_MESSAGES", "LC_NUMERIC", "LC_TIME", "LINES", "LISTMAX", - "LOGCHECK", "MAIL", "MAILCHECK", "mailpath", "manpath", "module_path", - "NULLCMD", "path", "POSTEDIT", "PROMPT", "PROMPT2", "PROMPT3", - "PROMPT4", "prompt", "PROMPT_EOL_MARK", "PS1", "PS2", "PS3", "PS4", - "psvar", "READNULLCMD", "REPORTTIME", "REPLY", "reply", "RPROMPT", - "RPS1", "RPROMPT2", "RPS2", "SAVEHIST", "SPROMPT", "STTY", "TERM", - "TERMINFO", "TIMEFMT", "TMOUT", "TMPPREFIX", "watch", "WATCHFMT", - "WORDCHARS", "ZBEEP", "ZDOTDIR", "ZLE_LINE_ABORTED", - "ZLE_REMOVE_SUFFIX_CHARS", "ZLE_SPACE_SUFFIX_CHARS" + -- Other + "USER", "TZ", "TERM" ] variablesWithoutSpaces = [ diff --git a/ShellCheck/Options.hs b/ShellCheck/Options.hs --- a/ShellCheck/Options.hs +++ b/ShellCheck/Options.hs @@ -1,6 +1,6 @@ module ShellCheck.Options where -data Shell = Ksh | Zsh | Sh | Bash +data Shell = Ksh | Sh | Bash deriving (Show, Eq) data AnalysisOptions = AnalysisOptions { diff --git a/ShellCheck/Parser.hs b/ShellCheck/Parser.hs --- a/ShellCheck/Parser.hs +++ b/ShellCheck/Parser.hs @@ -20,6 +20,7 @@ import ShellCheck.AST import ShellCheck.Data +import ShellCheck.Options import Text.Parsec import Debug.Trace import Control.Monad @@ -468,14 +469,15 @@ prop_a7 = isOk readArithmeticContents "3*2**10" prop_a8 = isOk readArithmeticContents "3" prop_a9 = isOk readArithmeticContents "a^!-b" -prop_aA = isOk readArithmeticContents "! $?" -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" -prop_a10= isOk readArithmeticContents "$foo$bar" -prop_a11= isOk readArithmeticContents "i<(0+(1+1))" +prop_a10= isOk readArithmeticContents "! $?" +prop_a11= isOk readArithmeticContents "10#08 * 16#f" +prop_a12= isOk readArithmeticContents "\"$((3+2))\" + '37'" +prop_a13= isOk readArithmeticContents "foo[9*y+x]++" +prop_a14= isOk readArithmeticContents "1+`echo 2`" +prop_a15= isOk readArithmeticContents "foo[`echo foo | sed s/foo/4/g` * 3] + 4" +prop_a16= isOk readArithmeticContents "$foo$bar" +prop_a17= isOk readArithmeticContents "i<(0+(1+1))" +prop_a18= isOk readArithmeticContents "a?b:c" readArithmeticContents = readSequence where @@ -517,7 +519,7 @@ readNormalDollar, readBraced, readBackTicked, - readNormalLiteral "+-*/=%^,]" + readNormalLiteral "+-*/=%^,]?:" ] spacing return $ TA_Expansion id pieces @@ -763,11 +765,10 @@ 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 @@ -949,7 +950,7 @@ pos <- getPosition backslash do - next <- quotable <|> oneOf "?*@!+[]{}.," + next <- quotable <|> oneOf "?*@!+[]{}.,~#" return $ if next == '\n' then "" else [next] <|> do @@ -1094,6 +1095,7 @@ string "]" return (T_DollarBracket id c) +prop_readArithmeticExpression = isOk readArithmeticExpression "((a?b:c))" readArithmeticExpression = called "((..)) command" $ do id <- getNextId try (string "((") @@ -1121,23 +1123,33 @@ return $ T_DollarExpansion id cmds prop_readDollarVariable = isOk readDollarVariable "$@" +prop_readDollarVariable2 = isOk (readDollarVariable >> anyChar) "$?!" +prop_readDollarVariable3 = isWarning (readDollarVariable >> anyChar) "$10" +prop_readDollarVariable4 = isWarning (readDollarVariable >> string "[@]") "$arr[@]" + readDollarVariable = do id <- getNextId + pos <- getPosition + let singleCharred p = do n <- p value <- wrap [n] - return (T_DollarBraced id value) `attempting` do - pos <- getPosition - num <- lookAhead $ many1 p - parseNoteAt pos ErrorC 1037 $ "$" ++ (n:num) ++ " is equivalent to ${" ++ [n] ++ "}"++ num ++"." + return (T_DollarBraced id value) - let positional = singleCharred digit + let positional = do + value <- singleCharred digit + return value `attempting` do + lookAhead digit + parseNoteAt pos ErrorC 1037 "Braces are required for positionals over 9, e.g. ${10}." + let special = singleCharred specialVariable let regular = do name <- readVariableName value <- wrap name - return $ T_DollarBraced id value + return (T_DollarBraced id value) `attempting` do + lookAhead $ void (string "[@]") <|> void (string "[*]") <|> void readArrayIndex + parseNoteAt pos ErrorC 1087 "Braces are required when expanding arrays, as in ${array[idx]}." try $ char '$' >> (positional <|> special <|> regular) @@ -1345,7 +1357,6 @@ 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 @@ -1449,7 +1460,11 @@ spacing return $ T_Pipe id ('|':qualifier) -readCommand = readCompoundCommand <|> readSimpleCommand +readCommand = choice [ + readCompoundCommand, + readCoProc, + readSimpleCommand + ] readCmdName = do f <- readNormalWord @@ -1617,7 +1632,6 @@ 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" prop_readForClause12= isWarning readForClause "for $a in *; do echo \"$a\"; done" readForClause = called "for loop" $ do pos <- getPosition @@ -1646,25 +1660,10 @@ readRegular id pos = do acceptButWarn (char '$') ErrorC 1086 "Don't use $ on the iterator name in for loops." - 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 + name <- readVariableName `thenSkip` spacing + values <- readInClause <|> (optional readSequentialSep >> return []) + 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" @@ -1785,8 +1784,32 @@ g_Rparen return () - readFunctionName = many functionChars + readFunctionName = many1 functionChars +prop_readCoProc1 = isOk readCoProc "coproc foo { echo bar; }" +prop_readCoProc2 = isOk readCoProc "coproc { echo bar; }" +prop_readCoProc3 = isOk readCoProc "coproc echo bar" +readCoProc = called "coproc" $ do + id <- getNextId + try $ do + string "coproc" + whitespace + choice [ try $ readCompoundCoProc id, readSimpleCoProc id ] + where + readCompoundCoProc id = do + var <- optionMaybe $ + readVariableName `thenSkip` whitespace + body <- readBody readCompoundCommand + return $ T_CoProc id var body + readSimpleCoProc id = do + body <- readBody readSimpleCommand + return $ T_CoProc id Nothing body + readBody parser = do + id <- getNextId + body <- parser + return $ T_CoProcBody id body + + readPattern = (readNormalWord `thenSkip` spacing) `sepBy1` (char '|' `thenSkip` spacing) prop_readCompoundCommand = isOk readCompoundCommand "{ echo foo; }>/dev/null" @@ -1842,6 +1865,7 @@ prop_readAssignmentWord7 = isOk readAssignmentWord "a[3$n'']=42" prop_readAssignmentWord8 = isOk readAssignmentWord "a[4''$(cat foo)]=42" prop_readAssignmentWord9 = isOk readAssignmentWord "IFS= " +prop_readAssignmentWord9a= isOk readAssignmentWord "foo=" prop_readAssignmentWord10= isWarning readAssignmentWord "foo$n=42" prop_readAssignmentWord11= isOk readAssignmentWord "foo=([a]=b [c] [d]= [e f )" prop_readAssignmentWord12= isOk readAssignmentWord "a[b <<= 3 + c]='thing'" @@ -1850,25 +1874,23 @@ 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 - space <- spacing + hasLeftSpace <- liftM (not . null) spacing pos <- getPosition op <- readAssignmentOp - space2 <- spacing - if space == "" && space2 /= "" + hasRightSpace <- liftM (not . null) spacing + isEndOfCommand <- liftM isJust $ optionMaybe (try . lookAhead $ (disregard (oneOf "\r\n;&|)") <|> eof)) + if not hasLeftSpace && (hasRightSpace || isEndOfCommand) then do - when (variable /= "IFS") $ + when (variable /= "IFS" && hasRightSpace) $ parseNoteAt pos WarningC 1007 "Remove space after = if trying to assign a value (for empty string, use var='' ... )." value <- readEmptyLiteral return $ T_Assignment id op variable index value else do - when (space /= "" || space2 /= "") $ + when (hasLeftSpace || hasRightSpace) $ parseNoteAt pos ErrorC 1068 "Don't put spaces around the = in assignments." value <- readArray <|> readNormalWord spacing @@ -2050,8 +2072,8 @@ verifyShell pos s = case isValidShell s of Just True -> return () - Just False -> parseProblemAt pos ErrorC 1071 "ShellCheck only supports Bourne based shell scripts, sorry!" - Nothing -> parseProblemAt pos InfoC 1008 "This shebang was unrecognized. Note that ShellCheck only handles Bourne based shells." + Just False -> parseProblemAt pos ErrorC 1071 "ShellCheck only supports sh/bash/ksh scripts. Sorry!" + Nothing -> parseProblemAt pos InfoC 1008 "This shebang was unrecognized. Note that ShellCheck only handles sh/bash/ksh." isValidShell s = let good = s == "" || any (`isPrefixOf` s) goodShells @@ -2065,9 +2087,10 @@ goodShells = [ "sh", + "ash", + "dash", "bash", - "ksh", - "zsh" + "ksh" ] badShells = [ "awk", @@ -2075,7 +2098,8 @@ "perl", "python", "ruby", - "tcsh" + "tcsh", + "zsh" ] readUtf8Bom = called "Byte Order Mark" $ string "\xFEFF" @@ -2119,13 +2143,13 @@ Message s -> if null s then Nothing else return $ s ++ "." unexpected s = "Unexpected " ++ (if null s then "eof" else s) ++ "." -parseShell filename contents = +parseShell options filename contents = case rp (parseWithNotes readScript) filename contents of (Right (script, map, notes), (parsenotes, _)) -> - ParseResult (Just (script, map)) (nub $ sortNotes $ notes ++ parsenotes) + ParseResult (Just (script, map)) (nub . sortNotes . excludeNotes $ notes ++ parsenotes) (Left err, (p, context)) -> ParseResult Nothing - (nub $ sortNotes $ p ++ notesForContext context ++ [makeErrorFor err]) + (nub . sortNotes . excludeNotes $ p ++ notesForContext context ++ [makeErrorFor err]) where isName (ContextName _ _) = True isName _ = False @@ -2134,6 +2158,7 @@ "Couldn't parse this " ++ str ++ "." second (ContextName pos str) = ParseNote pos InfoC 1009 $ "The mentioned parser error was in this " ++ str ++ "." + excludeNotes = filter (\c -> codeForParseNote c `notElem` optionExcludes options) lt x = trace (show x) x ltt t = trace (show t) diff --git a/ShellCheck/Simple.hs b/ShellCheck/Simple.hs --- a/ShellCheck/Simple.hs +++ b/ShellCheck/Simple.hs @@ -28,7 +28,7 @@ shellCheck :: AnalysisOptions -> String -> [ShellCheckComment] shellCheck options script = - let (ParseResult result notes) = parseShell "-" script in + let (ParseResult result notes) = parseShell options "-" script in let allNotes = notes ++ concat (maybeToList $ do (tree, posMap) <- result let list = runAnalytics options tree @@ -71,6 +71,9 @@ prop_optionDisablesIssue1 = null $ shellCheck (defaultAnalysisOptions { optionExcludes = [2086, 2148] }) "echo $1" + +prop_optionDisablesIssue2 = + null $ shellCheck (defaultAnalysisOptions { optionExcludes = [2148, 1037] }) "echo \"$10\"" return [] runTests = $quickCheckAll diff --git a/shellcheck.1 b/shellcheck.1 new file mode 100644 --- /dev/null +++ b/shellcheck.1 @@ -0,0 +1,188 @@ +.TH SHELLCHECK 1 "" "Shell script analysis tool" +.SH NAME +.PP +shellcheck - Shell script analysis tool +.SH SYNOPSIS +.PP +\f[B]shellcheck\f[] [\f[I]OPTIONS\f[]...] \f[I]FILES\f[]... +.SH DESCRIPTION +.PP +ShellCheck is a static analysis and linting tool for sh/bash scripts. +It\[aq]s mainly focused on handling typical beginner and intermediate +level syntax errors and pitfalls where the shell just gives a cryptic +error message or strange behavior, but it also reports on a few more +advanced issues where corner cases can cause delayed failures. +.PP +ShellCheck gives shell specific advice. +Consider the line: +.IP +.nf +\f[C] +((\ area\ =\ 3.14*r*r\ )) +\f[] +.fi +.IP \[bu] 2 +For scripts starting with \f[C]#!/bin/sh\f[] (or when using +\f[C]-s\ sh\f[]), ShellCheck will warn that \f[C]((\ ..\ ))\f[] is not +POSIX compliant (similar to checkbashisms). +.IP \[bu] 2 +For scripts starting with \f[C]#!/bin/bash\f[] (or using +\f[C]-s\ bash\f[]), ShellCheck will warn that decimals are not +supported. +.IP \[bu] 2 +For scripts starting with \f[C]#!/bin/ksh\f[] (or using +\f[C]-s\ ksh\f[]), ShellCheck will not warn at all, as \f[C]ksh\f[] +supports decimals in arithmetic contexts. +.SH OPTIONS +.TP +.B \f[B]-e\f[]\ \f[I]CODE1\f[][,\f[I]CODE2\f[]...],\ \f[B]--exclude=\f[]\f[I]CODE1\f[][,\f[I]CODE2\f[]...] +Explicitly exclude the specified codes from the report. +Subsequent \f[B]-e\f[] options are cumulative, but all the codes can be +specified at once, comma-separated as a single argument. +.RS +.RE +.TP +.B \f[B]-f\f[] \f[I]FORMAT\f[], \f[B]--format=\f[]\f[I]FORMAT\f[] +Specify the output format of shellcheck, which prints its results in the +standard output. +Subsequent \f[B]-f\f[] options are ignored, see \f[B]FORMATS\f[] below +for more information. +.RS +.RE +.TP +.B \f[B]-s\f[]\ \f[I]shell\f[],\ \f[B]--shell=\f[]\f[I]shell\f[] +Specify Bourne shell dialect. +Valid values are \f[I]sh\f[], \f[I]bash\f[] and \f[I]ksh\f[]. +The default is to use the file\[aq]s shebang, or \f[I]bash\f[] if the +target shell can\[aq]t be determined. +.RS +.RE +.TP +.B \f[B]-V\f[]\ \f[I]version\f[],\ \f[B]--version\f[] +Print version and exit. +.RS +.RE +.SH FORMATS +.TP +.B \f[B]tty\f[] +Plain text, human readable output. +This is the default. +.RS +.RE +.TP +.B \f[B]gcc\f[] +GCC compatible output. +Useful for editors that support compiling and showing syntax errors. +.RS +.PP +For example, in Vim, \f[C]:set\ makeprg=shellcheck\\\ -f\\\ gcc\\\ %\f[] +will allow using \f[C]:make\f[] to check the script, and \f[C]:cnext\f[] +to jump to the next error. +.IP +.nf +\f[C] +:::\ :\ +\f[] +.fi +.RE +.TP +.B \f[B]checkstyle\f[] +Checkstyle compatible XML output. +Supported directly or through plugins by many IDEs and build monitoring +systems. +.RS +.IP +.nf +\f[C] + + +\ \ +\ \ \ \ +\ \ \ \ ... +\ \ +\ \ ... + +\f[] +.fi +.RE +.TP +.B \f[B]json\f[] +Json is a popular serialization format that is more suitable for web +applications. +ShellCheck\[aq]s json is compact and contains only the bare minimum. +.RS +.IP +.nf +\f[C] +[ +\ \ { +\ \ \ \ "file":\ "filename", +\ \ \ \ "line":\ lineNumber, +\ \ \ \ "column":\ columnNumber, +\ \ \ \ "level":\ "severitylevel", +\ \ \ \ "code":\ errorCode, +\ \ \ \ "message":\ "warning\ message" +\ \ }, +\ \ ... +] +\f[] +.fi +.RE +.SH DIRECTIVES +.PP +ShellCheck directives can be specified as comments in the shell script +before a command or block: +.IP +.nf +\f[C] +#\ shellcheck\ key=value\ key=value +command-or-structure +\f[] +.fi +.PP +For example, to suppress SC2035 about using \f[C]\&./*.jpg\f[]: +.IP +.nf +\f[C] +#\ shellcheck\ disable=SC2035 +echo\ "Files:\ "\ *.jpg +\f[] +.fi +.PP +Here a shell brace group is used to suppress on multiple lines: +.IP +.nf +\f[C] +#\ shellcheck\ disable=SC2016 +{ +\ \ echo\ \[aq]Modifying\ $PATH\[aq] +\ \ echo\ \[aq]PATH=foo:$PATH\[aq]\ >>\ ~/.bashrc +} +\f[] +.fi +.PP +Valid keys are: +.TP +.B \f[B]disable\f[] +Disables a comma separated list of error codes for the following +command. +The command can be a simple command like \f[C]echo\ foo\f[], or a +compound command like a function definition, subshell block or loop. +.RS +.RE +.SH AUTHOR +.PP +ShellCheck is written and maintained by Vidar Holen. +.SH REPORTING BUGS +.PP +Bugs and issues can be reported on GitHub: +.PP +https://github.com/koalaman/shellcheck/issues +.SH SEE ALSO +.PP +sh(1) bash(1) diff --git a/shellcheck.1.md b/shellcheck.1.md --- a/shellcheck.1.md +++ b/shellcheck.1.md @@ -16,6 +16,20 @@ strange behavior, but it also reports on a few more advanced issues where corner cases can cause delayed failures. +ShellCheck gives shell specific advice. Consider the line: + + (( area = 3.14*r*r )) + ++ For scripts starting with `#!/bin/sh` (or when using `-s sh`), ShellCheck +will warn that `(( .. ))` is not POSIX compliant (similar to checkbashisms). + ++ For scripts starting with `#!/bin/bash` (or using `-s bash`), ShellCheck +will warn that decimals are not supported. + ++ For scripts starting with `#!/bin/ksh` (or using `-s ksh`), ShellCheck will +not warn at all, as `ksh` supports decimals in arithmetic contexts. + + # OPTIONS **-e**\ *CODE1*[,*CODE2*...],\ **--exclude=***CODE1*[,*CODE2*...] @@ -32,9 +46,9 @@ **-s**\ *shell*,\ **--shell=***shell* -: Specify Bourne shell dialect. Valid values are *sh*, *bash*, *ksh* and - *zsh*. The default is to use the file's shebang, or *bash* if the target - shell can't be determined. +: Specify Bourne shell dialect. Valid values are *sh*, *bash* and *ksh*. + The default is to use the file's shebang, or *bash* if the target shell + can't be determined. **-V**\ *version*,\ **--version** @@ -83,11 +97,12 @@ [ { - "line": line, - "column": column, - "level": level, - "code": ####, - "message": message + "file": "filename", + "line": lineNumber, + "column": columnNumber, + "level": "severitylevel", + "code": errorCode, + "message": "warning message" }, ... ] @@ -103,6 +118,14 @@ # shellcheck disable=SC2035 echo "Files: " *.jpg + +Here a shell brace group is used to suppress on multiple lines: + + # shellcheck disable=SC2016 + { + echo 'Modifying $PATH' + echo 'PATH=foo:$PATH' >> ~/.bashrc + } Valid keys are: diff --git a/shellcheck.hs b/shellcheck.hs --- a/shellcheck.hs +++ b/shellcheck.hs @@ -19,6 +19,7 @@ import Control.Monad import Control.Monad.Trans import Control.Monad.Trans.Error +import Control.Monad.Trans.List import Data.Char import Data.Maybe import Data.Monoid @@ -40,6 +41,8 @@ data Flag = Flag String String data Status = NoProblems | SomeProblems | BadInput | SupportFailure | SyntaxFailure | RuntimeException deriving (Ord, Eq) +data JsonComment = JsonComment FilePath ShellCheckComment + instance Error Status where noMsg = RuntimeException @@ -54,7 +57,7 @@ Option "f" ["format"] (ReqArg (Flag "format") "FORMAT") "output format", Option "s" ["shell"] - (ReqArg (Flag "shell") "SHELLNAME") "Specify dialect (bash,sh,ksh,zsh)", + (ReqArg (Flag "shell") "SHELLNAME") "Specify dialect (bash,sh,ksh)", Option "V" ["version"] (NoArg $ Flag "version" "true") "Print version information" ] @@ -62,8 +65,9 @@ printErr = hPutStrLn stderr -instance JSON ShellCheckComment where - showJSON c = makeObj [ +instance JSON (JsonComment) where + showJSON (JsonComment filename c) = makeObj [ + ("file", showJSON $ filename), ("line", showJSON $ scLine c), ("column", showJSON $ scColumn c), ("level", showJSON $ scSeverity c), @@ -152,10 +156,12 @@ term <- hIsTerminalDevice stdout return $ if term then colorComment else const id --- This totally ignores the filenames. Fixme? forJson :: AnalysisOptions -> [FilePath] -> IO Status forJson options files = catchExceptions $ do - comments <- liftM concat $ mapM (commentsFor options) files + comments <- runListT $ do + file <- ListT $ return files + comment <- ListT $ commentsFor options file + return $ JsonComment file comment putStrLn $ encodeStrict comments return $ checkComments comments