diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -101,4 +101,10 @@ cabal build cabal test +## Reporting bugs + +Please use the Github issue tracker for any bugs or feature suggestions: + +https://github.com/koalaman/shellcheck/issues + Happy ShellChecking! diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,5 +1,5 @@ Name: ShellCheck -Version: 0.3.8 +Version: 0.4.0 Synopsis: Shell script analysis tool License: GPL-3 License-file: LICENSE @@ -41,18 +41,21 @@ containers, directory, json, - mtl, + mtl >= 2.2.1, parsec, regex-tdfa, QuickCheck >= 2.7.4 exposed-modules: - ShellCheck.Analytics ShellCheck.AST + ShellCheck.ASTLib + ShellCheck.Analytics + ShellCheck.Analyzer + ShellCheck.Checker ShellCheck.Data - ShellCheck.Options + ShellCheck.Formatter.Format + ShellCheck.Interface ShellCheck.Parser ShellCheck.Regex - ShellCheck.Simple other-modules: Paths_ShellCheck @@ -63,10 +66,9 @@ containers, directory, json, - mtl, + mtl >= 2.2.1, parsec, regex-tdfa, - transformers, QuickCheck >= 2.7.4 main-is: shellcheck.hs @@ -78,10 +80,9 @@ containers, directory, json, - mtl, + mtl >= 2.2.1, parsec, 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 @@ -1,4 +1,6 @@ {- + Copyright 2012-2015 Vidar Holen + This file is part of ShellCheck. http://www.vidarholen.net/contents/shellcheck @@ -70,6 +72,7 @@ | T_DollarDoubleQuoted Id [Token] | T_DollarExpansion Id [Token] | T_DollarSingleQuoted Id String + | T_DollarBraceCommandExpansion Id [Token] | T_Done Id | T_DoubleQuoted Id [Token] | T_EOF Id @@ -123,9 +126,10 @@ | T_Pipe Id String | T_CoProc Id (Maybe String) Token | T_CoProcBody Id Token + | T_Include Id Token Token -- . & source: SimpleCommand T_Script deriving (Show) -data Annotation = DisableComment Integer deriving (Show, Eq) +data Annotation = DisableComment Integer | SourceOverride String deriving (Show, Eq) data ConditionType = DoubleBracket | SingleBracket deriving (Show, Eq) -- This is an abomination. @@ -171,6 +175,7 @@ delve (T_DoubleQuoted id list) = dl list $ T_DoubleQuoted id delve (T_DollarDoubleQuoted id list) = dl list $ T_DollarDoubleQuoted id delve (T_DollarExpansion id list) = dl list $ T_DollarExpansion id + delve (T_DollarBraceCommandExpansion id list) = dl list $ T_DollarBraceCommandExpansion id delve (T_BraceExpansion id list) = dl list $ T_BraceExpansion id delve (T_Backticked id list) = dl list $ T_Backticked id delve (T_DollarArithmetic id c) = d1 c $ T_DollarArithmetic id @@ -253,6 +258,7 @@ 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_Include id includer script) = d2 includer script $ T_Include id delve t = return t getId t = case t of @@ -298,6 +304,7 @@ T_DollarBraced id _ -> id T_DollarArithmetic id _ -> id T_BraceExpansion id _ -> id + T_DollarBraceCommandExpansion id _ -> id T_IoFile id _ _ -> id T_HereDoc id _ _ _ _ -> id T_HereString id _ -> id @@ -348,6 +355,7 @@ T_Pipe id _ -> id T_CoProc id _ _ -> id T_CoProcBody id _ -> id + T_Include id _ _ -> id blank :: Monad m => Token -> m () blank = const $ return () @@ -355,10 +363,3 @@ doStackAnalysis startToken endToken = analyze startToken endToken id doTransform i = runIdentity . analyze blank blank i -isLoop t = case t of - T_WhileExpression {} -> True - T_UntilExpression {} -> True - T_ForIn {} -> True - T_ForArithmetic {} -> True - T_SelectIn {} -> True - _ -> False diff --git a/ShellCheck/ASTLib.hs b/ShellCheck/ASTLib.hs new file mode 100644 --- /dev/null +++ b/ShellCheck/ASTLib.hs @@ -0,0 +1,240 @@ +{- + Copyright 2012-2015 Vidar Holen + + 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 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +module ShellCheck.ASTLib where + +import ShellCheck.AST + +import Control.Monad +import Data.List +import Data.Maybe + +-- Is this a type of loop? +isLoop t = case t of + T_WhileExpression {} -> True + T_UntilExpression {} -> True + T_ForIn {} -> True + T_ForArithmetic {} -> True + T_SelectIn {} -> True + _ -> False + +-- Will this split into multiple words when used as an argument? +willSplit x = + case x of + T_DollarBraced {} -> True + T_DollarExpansion {} -> True + T_Backticked {} -> True + T_BraceExpansion {} -> True + T_Glob {} -> True + T_Extglob {} -> True + T_NormalWord _ l -> any willSplit l + _ -> False + +isGlob (T_Extglob {}) = True +isGlob (T_Glob {}) = True +isGlob (T_NormalWord _ l) = any isGlob l +isGlob _ = False + +-- Is this shell word a constant? +isConstant token = + case token of + T_NormalWord _ l -> all isConstant l + T_DoubleQuoted _ l -> all isConstant l + T_SingleQuoted _ _ -> True + T_Literal _ _ -> True + _ -> False + +-- Is this an empty literal? +isEmpty token = + case token of + T_NormalWord _ l -> all isEmpty l + T_DoubleQuoted _ l -> all isEmpty l + T_SingleQuoted _ "" -> True + T_Literal _ "" -> True + _ -> False + +-- Quick&lazy oversimplification of commands, throwing away details +-- and returning a list like ["find", ".", "-name", "${VAR}*" ]. +oversimplify token = + case token of + (T_NormalWord _ l) -> [concat (concatMap oversimplify l)] + (T_DoubleQuoted _ l) -> [concat (concatMap oversimplify l)] + (T_SingleQuoted _ s) -> [s] + (T_DollarBraced _ _) -> ["${VAR}"] + (T_DollarArithmetic _ _) -> ["${VAR}"] + (T_DollarExpansion _ _) -> ["${VAR}"] + (T_Backticked _ _) -> ["${VAR}"] + (T_Glob _ s) -> [s] + (T_Pipeline _ _ [x]) -> oversimplify x + (T_Literal _ x) -> [x] + (T_SimpleCommand _ vars words) -> concatMap oversimplify words + (T_Redirecting _ _ foo) -> oversimplify foo + (T_DollarSingleQuoted _ s) -> [s] + (T_Annotation _ _ s) -> oversimplify s + -- Workaround for let "foo = bar" parsing + (TA_Sequence _ [TA_Expansion _ v]) -> concatMap oversimplify v + otherwise -> [] + + +-- Turn a SimpleCommand foo -avz --bar=baz into args "a", "v", "z", "bar", +-- each in a tuple of (token, stringFlag). +getFlagsUntil stopCondition (T_SimpleCommand _ _ (_:args)) = + let textArgs = takeWhile (not . stopCondition . snd) $ map (\x -> (x, concat $ oversimplify x)) args in + concatMap flag textArgs + where + flag (x, '-':'-':arg) = [ (x, takeWhile (/= '=') arg) ] + flag (x, '-':args) = map (\v -> (x, [v])) args + flag _ = [] +getFlagsUntil _ _ = error "Internal shellcheck error, please report! (getFlags on non-command)" + +-- Get all flags in a GNU way, up until -- +getAllFlags = getFlagsUntil (== "--") +-- Get all flags in a BSD way, up until first non-flag argument +getLeadingFlags = getFlagsUntil (not . ("-" `isPrefixOf`)) + + +-- Given a T_DollarBraced, return a simplified version of the string contents. +bracedString (T_DollarBraced _ l) = concat $ oversimplify l +bracedString _ = error "Internal shellcheck error, please report! (bracedString on non-variable)" + +-- Is this an expansion of multiple items of an array? +isArrayExpansion t@(T_DollarBraced _ _) = + let string = bracedString t in + "@" `isPrefixOf` string || + not ("#" `isPrefixOf` string) && "[@]" `isInfixOf` string +isArrayExpansion _ = False + +-- Is it possible that this arg becomes multiple args? +mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f t + where + f t@(T_DollarBraced _ _) = + let string = bracedString t in + "!" `isPrefixOf` string + f (T_DoubleQuoted _ parts) = any f parts + f (T_NormalWord _ parts) = any f parts + f _ = False + +-- Is it certain that this word will becomes multiple words? +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 + +-- This does token cause implicit concatenation in assignments? +willConcatInAssignment token = + case token of + t@(T_DollarBraced {}) -> isArrayExpansion t + (T_DoubleQuoted _ parts) -> any willConcatInAssignment parts + (T_NormalWord _ parts) -> any willConcatInAssignment parts + _ -> False + +-- Maybe get the literal string corresponding to this token +getLiteralString :: Token -> Maybe String +getLiteralString = getLiteralStringExt (const Nothing) + +-- Definitely get a literal string, skipping over all non-literals +onlyLiteralString :: Token -> String +onlyLiteralString = fromJust . getLiteralStringExt (const $ return "") + +-- Maybe get a literal string, but only if it's an unquoted argument. +getUnquotedLiteral (T_NormalWord _ list) = + liftM concat $ mapM str list + where + str (T_Literal _ s) = return s + str _ = Nothing +getUnquotedLiteral _ = Nothing + +-- Maybe get the literal string of this token and any globs in it. +getGlobOrLiteralString = getLiteralStringExt f + where + f (T_Glob _ str) = return str + f _ = Nothing + +-- Maybe get the literal value of a token, using a custom function +-- to map unrecognized Tokens into strings. +getLiteralStringExt :: (Token -> Maybe String) -> Token -> Maybe String +getLiteralStringExt more = g + where + allInList = liftM concat . mapM g + g (T_DoubleQuoted _ l) = allInList l + g (T_DollarDoubleQuoted _ l) = allInList l + g (T_NormalWord _ l) = allInList l + g (TA_Expansion _ l) = allInList l + g (T_SingleQuoted _ s) = return s + g (T_Literal _ s) = return s + g x = more x + +-- Is this token a string literal? +isLiteral t = isJust $ getLiteralString t + + +-- Turn a NormalWord like foo="bar $baz" into a series of constituent elements like [foo=,bar ,$baz] +getWordParts (T_NormalWord _ l) = concatMap getWordParts l +getWordParts (T_DoubleQuoted _ l) = l +getWordParts other = [other] + +-- Return a list of NormalWords that would result from brace expansion +braceExpand (T_NormalWord id list) = take 1000 $ do + items <- mapM part list + return $ T_NormalWord id items + where + part (T_BraceExpansion id items) = do + item <- items + braceExpand item + part x = return x + +-- Maybe get the command name of a token representing a command +getCommandName t = + case t of + T_Redirecting _ _ w -> getCommandName w + T_SimpleCommand _ _ (w:_) -> getLiteralString w + T_Annotation _ _ t -> getCommandName t + otherwise -> Nothing + +-- Get the basename of a token representing a command +getCommandBasename = liftM basename . getCommandName + where + basename = reverse . takeWhile (/= '/') . reverse + +isAssignment t = + case t of + T_Redirecting _ _ w -> isAssignment w + T_SimpleCommand _ (w:_) [] -> True + T_Assignment {} -> True + T_Annotation _ _ w -> isAssignment w + otherwise -> False + +-- Get the list of commands from tokens that contain them, such as +-- the body of while loops and if statements. +getCommandSequences t = + case t of + T_Script _ _ cmds -> [cmds] + T_BraceGroup _ cmds -> [cmds] + T_Subshell _ cmds -> [cmds] + T_WhileExpression _ _ cmds -> [cmds] + T_UntilExpression _ _ cmds -> [cmds] + T_ForIn _ _ _ cmds -> [cmds] + T_ForArithmetic _ _ _ _ cmds -> [cmds] + T_IfExpression _ thens elses -> map snd thens ++ [elses] + otherwise -> [] + diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -1,4 +1,6 @@ {- + Copyright 2012-2015 Vidar Holen + This file is part of ShellCheck. http://www.vidarholen.net/contents/shellcheck @@ -16,10 +18,18 @@ along with this program. If not, see . -} {-# LANGUAGE TemplateHaskell, FlexibleContexts #-} -module ShellCheck.Analytics (AnalysisOptions(..), defaultAnalysisOptions, filterByAnnotation, runAnalytics, shellForExecutable, runTests) where +module ShellCheck.Analytics (runAnalytics, ShellCheck.Analytics.runTests) where +import ShellCheck.AST +import ShellCheck.ASTLib +import ShellCheck.Data +import ShellCheck.Parser +import ShellCheck.Interface +import ShellCheck.Regex + import Control.Arrow (first) import Control.Monad +import Control.Monad.Identity import Control.Monad.State import Control.Monad.Writer import Data.Char @@ -29,11 +39,6 @@ import Data.Maybe import Data.Ord import Debug.Trace -import ShellCheck.AST -import ShellCheck.Options -import ShellCheck.Data -import ShellCheck.Parser hiding (runTests) -import ShellCheck.Regex import qualified Data.Map as Map import Test.QuickCheck.All (forAllProperties) import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) @@ -46,7 +51,7 @@ } -- Checks that are run on the AST root -treeChecks :: [Parameters -> Token -> [Note]] +treeChecks :: [Parameters -> Token -> [TokenComment]] treeChecks = [ runNodeAnalysis (\p t -> (mapM_ ((\ f -> f t) . (\ f -> f p)) @@ -61,6 +66,7 @@ ,checkArrayWithoutIndex ,checkShebang ,checkUnassignedReferences + ,checkUncheckedCd ] checksFor Sh = [ @@ -78,23 +84,31 @@ ,checkForDecimals ] -runAnalytics :: AnalysisOptions -> Token -> [Note] -runAnalytics options root = runList options root treeChecks +runAnalytics :: AnalysisSpec -> AnalysisResult +runAnalytics options = AnalysisResult { + arComments = + nub . filterByAnnotation (asScript options) $ + runList options treeChecks + } -runList options root list = notes +runList :: AnalysisSpec -> [Parameters -> Token -> [TokenComment]] + -> [TokenComment] +runList spec list = notes where + root = asScript spec params = Parameters { - shellType = fromMaybe (determineShell root) $ optionShellType options, - shellTypeSpecified = isJust $ optionShellType options, + shellType = fromMaybe (determineShell root) $ asShellType spec, + shellTypeSpecified = isJust $ asShellType spec, parentMap = getParentTree root, - variableFlow = getVariableFlow (shellType params) (parentMap params) root + variableFlow = + getVariableFlow (shellType params) (parentMap params) root } - notes = filter (\c -> getCode c `notElem` optionExcludes options) $ concatMap (\f -> f params root) list - getCode (Note _ _ c _) = c - + notes = concatMap (\f -> f params root) list checkList l t = concatMap (\f -> f t) l +getCode (TokenComment _ (Comment _ c _)) = c + prop_determineShell0 = determineShell (T_Script (Id 0) "#!/bin/sh" []) == Sh prop_determineShell1 = determineShell (T_Script (Id 0) "#!/usr/bin/env ksh" []) == Ksh prop_determineShell2 = determineShell (T_Script (Id 0) "" []) == Bash @@ -104,21 +118,10 @@ shellFor s | ' ' `elem` s = shellFor $ takeWhile (/= ' ') s shellFor s = reverse . takeWhile (/= '/') . reverse $ s -shellForExecutable "sh" = return Sh -shellForExecutable "ash" = return Sh -shellForExecutable "dash" = return Sh - -shellForExecutable "ksh" = return Ksh -shellForExecutable "ksh88" = return Ksh -shellForExecutable "ksh93" = return Ksh - -shellForExecutable "bash" = return Bash -shellForExecutable _ = Nothing - -- Checks that are run on each node in the AST runNodeAnalysis f p t = execWriter (doAnalysis (f p) t) -nodeChecks :: [Parameters -> Token -> Writer [Note] ()] +nodeChecks :: [Parameters -> Token -> Writer [TokenComment] ()] nodeChecks = [ checkUuoc ,checkPipePitfalls @@ -132,7 +135,7 @@ ,checkNumberComparisons ,checkSingleBracketOperators ,checkDoubleBracketOperators - ,checkNoaryWasBinary + ,checkLiteralBreakingTest ,checkConstantNoary ,checkDivBeforeMult ,checkArithmeticDeref @@ -204,31 +207,39 @@ ,checkReturn ,checkMaskedReturns ,checkInjectableFindSh + ,checkReadWithoutR + ,checkExportedExpansions + ,checkLoopVariableReassignment ] filterByAnnotation token = filter (not . shouldIgnore) where - numFor (Note _ _ code _) = code - idFor (Note id _ _ _) = id + idFor (TokenComment id _) = id shouldIgnore note = - any (shouldIgnoreFor (numFor note)) $ + any (shouldIgnoreFor (getCode note)) $ getPath parents (T_Bang $ idFor note) shouldIgnoreFor num (T_Annotation _ anns _) = any hasNum anns where hasNum (DisableComment ts) = num == ts + shouldIgnoreFor _ (T_Include {}) = True -- Ignore included files shouldIgnoreFor _ _ = False parents = getParentTree token -addNote note = tell [note] -makeNote severity id code note = addNote $ Note id severity code note -warn = makeNote WarningC -err = makeNote ErrorC -info = makeNote InfoC -style = makeNote StyleC +makeComment :: Severity -> Id -> Code -> String -> TokenComment +makeComment severity id code note = + TokenComment id $ Comment severity code note +addComment note = tell [note] + +warn :: MonadWriter [TokenComment] m => Id -> Code -> String -> m () +warn id code str = addComment $ makeComment WarningC id code str +err id code str = addComment $ makeComment ErrorC id code str +info id code str = addComment $ makeComment InfoC id code str +style id code str = addComment $ makeComment StyleC id code str + isVariableStartChar x = x == '_' || isAsciiLower x || isAsciiUpper x isVariableChar x = isVariableStartChar x || isDigit x variableNameRegex = mkRegex "[_a-zA-Z][_a-zA-Z0-9]*" @@ -241,22 +252,6 @@ potentially = fromMaybe (return ()) -willSplit x = - case x of - T_DollarBraced {} -> True - T_DollarExpansion {} -> True - T_Backticked {} -> True - T_BraceExpansion {} -> True - T_Glob {} -> True - T_Extglob {} -> True - T_NormalWord _ l -> any willSplit l - _ -> False - -isGlob (T_Extglob {}) = True -isGlob (T_Glob {}) = True -isGlob (T_NormalWord _ l) = any isGlob l -isGlob _ = False - wouldHaveBeenGlob s = '*' `elem` s isConfusedGlobRegex ('*':_) = True @@ -278,83 +273,43 @@ headOrDefault _ (a:_) = a headOrDefault def _ = def -isConstant token = - case token of - T_NormalWord _ l -> all isConstant l - T_DoubleQuoted _ l -> all isConstant l - T_SingleQuoted _ _ -> True - T_Literal _ _ -> True - _ -> False -isEmpty token = - case token of - T_NormalWord _ l -> all isEmpty l - T_DoubleQuoted _ l -> all isEmpty l - T_SingleQuoted _ "" -> True - T_Literal _ "" -> True - _ -> False - -makeSimple (T_NormalWord _ [f]) = f -makeSimple (T_Redirecting _ _ f) = f -makeSimple (T_Annotation _ _ f) = f -makeSimple t = t -simplify = doTransform makeSimple - -deadSimple (T_NormalWord _ l) = [concat (concatMap deadSimple l)] -deadSimple (T_DoubleQuoted _ l) = [concat (concatMap deadSimple l)] -deadSimple (T_SingleQuoted _ s) = [s] -deadSimple (T_DollarBraced _ _) = ["${VAR}"] -deadSimple (T_DollarArithmetic _ _) = ["${VAR}"] -deadSimple (T_DollarExpansion _ _) = ["${VAR}"] -deadSimple (T_Backticked _ _) = ["${VAR}"] -deadSimple (T_Glob _ s) = [s] -deadSimple (T_Pipeline _ _ [x]) = deadSimple x -deadSimple (T_Literal _ x) = [x] -deadSimple (T_SimpleCommand _ vars words) = concatMap deadSimple words -deadSimple (T_Redirecting _ _ foo) = deadSimple foo -deadSimple (T_DollarSingleQuoted _ s) = [s] -deadSimple (T_Annotation _ _ s) = deadSimple s --- 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"] -getFlagsUntil stopCondition (T_SimpleCommand _ _ (_:args)) = - let textArgs = takeWhile (not . stopCondition . snd) $ map (\x -> (x, concat $ deadSimple x)) args in - concatMap flag textArgs - where - flag (x, ('-':'-':arg)) = [ (x, takeWhile (/= '=') arg) ] - flag (x, ('-':args)) = map (\v -> (x, [v])) args - flag _ = [] - -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 (r:_) -> Just r -verify :: (Parameters -> Token -> Writer [Note] ()) -> String -> Bool +verify :: (Parameters -> Token -> Writer [TokenComment] ()) -> String -> Bool verify f s = checkNode f s == Just True -verifyNot :: (Parameters -> Token -> Writer [Note] ()) -> String -> Bool +verifyNot :: (Parameters -> Token -> Writer [TokenComment] ()) -> String -> Bool verifyNot f s = checkNode f s == Just False -verifyTree :: (Parameters -> Token -> [Note]) -> String -> Bool -verifyTree f s = checkTree f s == Just True +verifyTree :: (Parameters -> Token -> [TokenComment]) -> String -> Bool +verifyTree f s = producesComments f s == Just True -verifyNotTree :: (Parameters -> Token -> [Note]) -> String -> Bool -verifyNotTree f s = checkTree f s == Just False +verifyNotTree :: (Parameters -> Token -> [TokenComment]) -> String -> Bool +verifyNotTree f s = producesComments f s == Just False -checkNode f = checkTree (runNodeAnalysis f) -checkTree f s = case parseShell defaultAnalysisOptions "-" s of - (ParseResult (Just (t, m)) _) -> Just . not . null $ runList defaultAnalysisOptions t [f] - _ -> Nothing +defaultSpec root = AnalysisSpec { + asScript = root, + asShellType = Nothing, + asExecutionMode = Executed +} +checkNode f = producesComments (runNodeAnalysis f) +producesComments :: (Parameters -> Token -> [TokenComment]) -> String -> Maybe Bool +producesComments f s = do + root <- prRoot pResult + return . not . null $ runList (defaultSpec root) [f] + where + pSpec = ParseSpec { + psFilename = "script", + psScript = s + } + pResult = runIdentity $ parseScript (mockedSystemInterface []) pSpec + -- Copied from https://wiki.haskell.org/Edit_distance dist :: Eq a => [a] -> [a] -> Int dist a b @@ -370,7 +325,7 @@ 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)) + 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) @@ -379,6 +334,21 @@ hasFloatingPoint params = shellType params == Ksh +-- Checks whether the current parent path is part of a condition +isCondition [] = False +isCondition [_] = False +isCondition (child:parent:rest) = + getId child `elem` map getId (getConditionChildren parent) || isCondition (parent:rest) + where + getConditionChildren t = + case t of + T_AndIf _ left right -> [left] + T_OrIf id left right -> [left] + T_IfExpression id conditions elses -> concatMap (take 1 . reverse . fst) conditions + T_WhileExpression id c l -> take 1 . reverse $ c + T_UntilExpression id c l -> take 1 . reverse $ c + _ -> [] + prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)" checkEchoWc _ (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ @@ -387,8 +357,8 @@ ["wc", "-m"] -> countMsg _ -> return () where - acmd = deadSimple a - bcmd = deadSimple b + acmd = oversimplify a + bcmd = oversimplify b countMsg = style id 2000 "See if you can use ${#variable} instead." checkEchoWc _ _ = return () @@ -405,12 +375,12 @@ sedRe = mkRegex "^s(.)([^\n]*)g?$" isSimpleSed s = fromMaybe False $ do [first,rest] <- matchRegex sedRe s - let delimiters = filter (== (first !! 0)) rest + let delimiters = filter (== (head first)) rest guard $ length delimiters == 2 return True - acmd = deadSimple a - bcmd = deadSimple b + acmd = oversimplify a + bcmd = oversimplify b checkIn s = when (isSimpleSed s) $ style id 2001 "See if you can use ${variable//search/replace} instead." @@ -429,7 +399,7 @@ prop_checkAssignAteCommand4 = verifyNot checkAssignAteCommand "A=foo ls -l" prop_checkAssignAteCommand5 = verifyNot checkAssignAteCommand "PAGER=cat grep bar" checkAssignAteCommand _ (T_SimpleCommand id (T_Assignment _ _ _ _ assignmentTerm:[]) (firstWord:_)) = - when ("-" `isPrefixOf` concat (deadSimple firstWord) || + when ("-" `isPrefixOf` concat (oversimplify firstWord) || isCommonCommand (getLiteralString assignmentTerm) && not (isCommonCommand (getLiteralString firstWord))) $ warn id 2037 "To assign the output of a command, use var=$(cmd) ." @@ -484,12 +454,14 @@ prop_checkUuoc3 = verify checkUuoc "cat $var | grep bar" prop_checkUuoc4 = verifyNot checkUuoc "cat $var" prop_checkUuoc5 = verifyNot checkUuoc "cat \"$@\"" +prop_checkUuoc6 = verifyNot checkUuoc "cat -n | grep bar" checkUuoc _ (T_Pipeline _ _ (T_Redirecting _ _ cmd:_:_)) = checkCommand "cat" (const f) cmd where - f [word] = unless (mayBecomeMultipleArgs word) $ + f [word] = unless (mayBecomeMultipleArgs word || isOption word) $ style (getId word) 2002 "Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead." f _ = return () + isOption word = "-" `isPrefixOf` onlyLiteralString word checkUuoc _ _ = return () prop_checkNeedlessCommands = verify checkNeedlessCommands "foo=$(expr 3 + 2)" @@ -513,7 +485,7 @@ checkPipePitfalls _ (T_Pipeline id _ commands) = do for ["find", "xargs"] $ \(find:xargs:_) -> - let args = deadSimple xargs ++ deadSimple find + let args = oversimplify xargs ++ oversimplify find in unless (any ($ args) [ hasShortParameter '0', @@ -540,12 +512,12 @@ ] unless didLs $ do for ["ls", "?"] $ - \(ls:_) -> unless (hasShortParameter 'N' (deadSimple ls)) $ + \(ls:_) -> unless (hasShortParameter 'N' (oversimplify ls)) $ info (getId ls) 2012 "Use find instead of ls to better handle non-alphanumeric filenames." return () where for l f = - let indices = indexOfSublists l (map (headOrDefault "" . deadSimple) commands) + let indices = indexOfSublists l (map (headOrDefault "" . oversimplify) commands) in do mapM_ (f . (\ n -> take (length l) $ drop n commands)) indices return . not . null $ indices @@ -571,49 +543,16 @@ match _ _ = False -bracedString l = concat $ deadSimple l - -isArrayExpansion (T_DollarBraced _ l) = - let string = bracedString l in - "@" `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_checkShebangParameters1 = verifyTree checkShebangParameters "#!/usr/bin/env bash -x\necho cow" prop_checkShebangParameters2 = verifyNotTree checkShebangParameters "#! /bin/sh -l " checkShebangParameters _ (T_Script id sb _) = - [Note id ErrorC 2096 "On most OS, shebangs can only specify a single parameter." | length (words sb) > 2] + [makeComment ErrorC id 2096 "On most OS, shebangs can only specify a single parameter." | length (words sb) > 2] prop_checkShebang1 = verifyNotTree checkShebang "#!/usr/bin/env bash -x\necho cow" prop_checkShebang2 = verifyNotTree checkShebang "#! /bin/sh -l " prop_checkShebang3 = verifyTree checkShebang "ls -l" checkShebang params (T_Script id sb _) = - [Note id ErrorC 2148 "Tips depend on target shell and yours is unknown. Add a shebang." + [makeComment ErrorC id 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)" @@ -642,6 +581,8 @@ 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" +prop_checkBashisms27= verify checkBashisms "echo *[^0-9]*" +prop_checkBashisms28= verify checkBashisms "exec {n}>&2" checkBashisms _ = bashism where errMsg id s = err id 2040 $ "In sh, " ++ s ++ " not supported, even when sh is actually bash." @@ -667,11 +608,14 @@ warnMsg id $ filter (/= '|') op ++ " is" bashism (TA_Binary id "**" _ _) = warnMsg id "exponentials are" bashism (T_FdRedirect id "&" (T_IoFile _ (T_Greater _) _)) = warnMsg id "&> is" + bashism (T_FdRedirect id ('{':_) _) = warnMsg id "named file descriptors are" 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_Glob id str) | "[^" `isInfixOf` str = + warnMsg id "^ in place of ! in glob bracket expressions is" bashism t@(TA_Expansion id _) | isBashism = warnMsg id $ fromJust str ++ " is" @@ -682,8 +626,8 @@ mapM_ check expansion when (var `elem` bashVars) $ warnMsg id $ var ++ " is" where - str = concat $ deadSimple token - var = getBracedReference (bracedString token) + str = bracedString t + var = getBracedReference str check (regex, feature) = when (isJust $ matchRegex regex str) $ warnMsg id feature @@ -700,9 +644,9 @@ | t `isCommand` "echo" && "-" `isPrefixOf` argString = unless ("--" `isPrefixOf` argString) $ -- echo "-------" warnMsg (getId arg) "echo flags are" - where argString = concat $ deadSimple arg + where argString = concat $ oversimplify arg bashism t@(T_SimpleCommand _ _ (cmd:arg:_)) - | t `isCommand` "exec" && "-" `isPrefixOf` concat (deadSimple arg) = + | t `isCommand` "exec" && "-" `isPrefixOf` concat (oversimplify arg) = warnMsg (getId arg) "exec flags are" bashism t@(T_SimpleCommand id _ _) | t `isCommand` "let" = warnMsg id "'let' is" @@ -803,7 +747,7 @@ check id f x try _ = return () check id f x = - case deadSimple x of + case oversimplify x of ("ls":n) -> let warntype = if any ("-" `isPrefixOf`) n then warn else err in warntype id 2045 "Iterating over ls output is fragile. Use globs." @@ -865,6 +809,7 @@ where check t@(T_DollarExpansion _ _) = examine t check t@(T_Backticked _ _) = examine t + check t@(T_DollarBraceCommandExpansion _ _) = examine t check _ = return () tree = parentMap params examine t = @@ -880,15 +825,15 @@ checkRedirectToSame params s@(T_Pipeline _ _ list) = mapM_ (\l -> (mapM_ (\x -> doAnalysis (checkOccurrences x) l) (getAllRedirs list))) list where - note x = Note x InfoC 2094 + note x = makeComment InfoC x 2094 "Make sure not to read and write the same file in the same pipeline." checkOccurrences t@(T_NormalWord exceptId x) u@(T_NormalWord newId y) = when (exceptId /= newId && x == y && not (isOutput t && isOutput u) && not (special t)) $ do - addNote $ note newId - addNote $ note exceptId + addComment $ note newId + addComment $ note exceptId checkOccurrences _ _ = return () getAllRedirs = concatMap (\t -> case t of @@ -900,7 +845,7 @@ T_DGREAT _ -> [file] _ -> [] getRedirs _ = [] - special x = "/dev/" `isPrefixOf` concat (deadSimple x) + special x = "/dev/" `isPrefixOf` concat (oversimplify x) isOutput t = case drop 1 $ getPath (parentMap params) t of T_IoFile _ op _:_ -> @@ -930,8 +875,8 @@ prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done" prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*" -checkDollarStar p t@(T_NormalWord _ [T_DollarBraced id l]) - | bracedString l == "*" = +checkDollarStar p t@(T_NormalWord _ [b@(T_DollarBraced id _)]) + | bracedString b == "*" = unless isAssigned $ warn id 2048 "Use \"$@\" (with quotes) to prevent whitespace problems." where @@ -949,10 +894,16 @@ prop_checkUnquotedDollarAt6 = verifyNot checkUnquotedDollarAt "a=$@" prop_checkUnquotedDollarAt7 = verify checkUnquotedDollarAt "for f in ${var[@]}; do true; done" prop_checkUnquotedDollarAt8 = verifyNot checkUnquotedDollarAt "echo \"${args[@]:+${args[@]}}\"" +prop_checkUnquotedDollarAt9 = verifyNot checkUnquotedDollarAt "echo ${args[@]:+\"${args[@]}\"}" checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not $ isStrictlyQuoteFree (parentMap p) word = forM_ (take 1 $ filter isArrayExpansion parts) $ \x -> - err (getId x) 2068 - "Double quote array expansions to avoid re-splitting elements." + unless (isAlternative x) $ + err (getId x) 2068 + "Double quote array expansions to avoid re-splitting elements." + where + -- Fixme: should detect whether the alterantive is quoted + isAlternative b@(T_DollarBraced _ t) = ":+" `isInfixOf` bracedString b + isAlternative _ = False checkUnquotedDollarAt _ _ = return () prop_checkConcatenatedDollarAt1 = verify checkConcatenatedDollarAt "echo \"foo$@\"" @@ -1001,7 +952,7 @@ return . maybeToList $ do name <- getLiteralString token assignment <- Map.lookup name map - return [Note id WarningC 2128 + return [makeComment WarningC id 2128 "Expanding an array without an index only gives the first element."] readF _ _ _ = return [] @@ -1180,11 +1131,11 @@ op ++ " is for integer comparisons. Use " ++ seqv op ++ " instead." isNum t = - case deadSimple t of + case oversimplify t of [v] -> all isDigit v _ -> False isFraction t = - case deadSimple t of + case oversimplify t of [v] -> isJust $ matchRegex floatRegex v _ -> False @@ -1233,16 +1184,26 @@ prop_checkConditionalAndOrs1 = verify checkConditionalAndOrs "[ foo && bar ]" prop_checkConditionalAndOrs2 = verify checkConditionalAndOrs "[[ foo -o bar ]]" prop_checkConditionalAndOrs3 = verifyNot checkConditionalAndOrs "[[ foo || bar ]]" -checkConditionalAndOrs _ (TC_And id SingleBracket "&&" _ _) = - err id 2107 "You can't use && inside [..]. Use -a instead." -checkConditionalAndOrs _ (TC_And id DoubleBracket "-a" _ _) = - err id 2108 "In [[..]], use && instead of -a." -checkConditionalAndOrs _ (TC_Or id SingleBracket "||" _ _) = - err id 2109 "You can't use || inside [..]. Use -o instead." -checkConditionalAndOrs _ (TC_Or id DoubleBracket "-o" _ _) = - err id 2110 "In [[..]], use || instead of -o." -checkConditionalAndOrs _ _ = return () +prop_checkConditionalAndOrs4 = verify checkConditionalAndOrs "[ foo -a bar ]" +prop_checkConditionalAndOrs5 = verify checkConditionalAndOrs "[ -z 3 -o a = b ]" +checkConditionalAndOrs _ t = + case t of + (TC_And id SingleBracket "&&" _ _) -> + err id 2107 "Instead of [ a && b ], use [ a ] && [ b ]." + (TC_And id DoubleBracket "-a" _ _) -> + err id 2108 "In [[..]], use && instead of -a." + (TC_Or id SingleBracket "||" _ _) -> + err id 2109 "Instead of [ a || b ], use [ a ] || [ b ]." + (TC_Or id DoubleBracket "-o" _ _) -> + err id 2110 "In [[..]], use || instead of -o." + (TC_And id SingleBracket "-a" _ _) -> + warn id 2166 "Prefer [ p ] && [ q ] as [ p -a q ] is not well defined." + (TC_Or id SingleBracket "-o" _ _) -> + warn id 2166 "Prefer [ p ] || [ q ] as [ p -o q ] is not well defined." + + otherwise -> return () + prop_checkQuotedCondRegex1 = verify checkQuotedCondRegex "[[ $foo =~ \"bar\" ]]" prop_checkQuotedCondRegex2 = verify checkQuotedCondRegex "[[ $foo =~ 'cow' ]]" prop_checkQuotedCondRegex3 = verifyNot checkQuotedCondRegex "[[ $foo =~ $foo ]]" @@ -1261,7 +1222,7 @@ prop_checkGlobbedRegex3 = verifyNot checkGlobbedRegex "[[ $foo =~ $foo ]]" prop_checkGlobbedRegex4 = verifyNot checkGlobbedRegex "[[ $foo =~ ^c.* ]]" checkGlobbedRegex _ (TC_Binary _ DoubleBracket "=~" _ rhs) = - let s = concat $ deadSimple rhs in + let s = concat $ oversimplify rhs in when (isConfusedGlobRegex s) $ warn (getId rhs) 2049 "=~ is for regex. Use == for globs." checkGlobbedRegex _ _ = return () @@ -1280,20 +1241,58 @@ rLit = getLiteralString rhs checkConstantIfs _ _ = return () -prop_checkNoaryWasBinary = verify checkNoaryWasBinary "[[ a==$foo ]]" -prop_checkNoaryWasBinary2 = verify checkNoaryWasBinary "[ $foo=3 ]" -prop_checkNoaryWasBinary3 = verify checkNoaryWasBinary "[ $foo!=3 ]" -checkNoaryWasBinary _ (TC_Noary _ _ t@(T_NormalWord id l)) | not $ isConstant t = do - let str = concat $ deadSimple t - when ('=' `elem` str) $ err id 2077 "You need spaces around the comparison operator." -checkNoaryWasBinary _ _ = return () +prop_checkLiteralBreakingTest = verify checkLiteralBreakingTest "[[ a==$foo ]]" +prop_checkLiteralBreakingTest2 = verify checkLiteralBreakingTest "[ $foo=3 ]" +prop_checkLiteralBreakingTest3 = verify checkLiteralBreakingTest "[ $foo!=3 ]" +prop_checkLiteralBreakingTest4 = verify checkLiteralBreakingTest "[ \"$(ls) \" ]" +prop_checkLiteralBreakingTest5 = verify checkLiteralBreakingTest "[ -n \"$(true) \" ]" +prop_checkLiteralBreakingTest6 = verify checkLiteralBreakingTest "[ -z $(true)z ]" +prop_checkLiteralBreakingTest7 = verifyNot checkLiteralBreakingTest "[ -z $(true) ]" +prop_checkLiteralBreakingTest8 = verifyNot checkLiteralBreakingTest "[ $(true)$(true) ]" +checkLiteralBreakingTest _ t = potentially $ + case t of + (TC_Noary _ _ w@(T_NormalWord _ l)) -> do + guard . not $ isConstant w + comparisonWarning l `mplus` tautologyWarning w "Argument to implicit -n is always true due to literal strings." + (TC_Unary _ _ op w@(T_NormalWord _ l)) -> do + guard $ not $ isConstant w + case op of + "-n" -> tautologyWarning w "Argument to -n is always true due to literal strings." + "-z" -> tautologyWarning w "Argument to -z is always false due to literal strings." + _ -> fail "not relevant" + _ -> fail "not my problem" + where + hasEquals = matchToken ('=' `elem`) + isNonEmpty = matchToken (not . null) + matchToken m t = isJust $ do + str <- getLiteralString t + guard $ m str + return () + comparisonWarning list = do + token <- listToMaybe $ filter hasEquals list + return $ err (getId token) 2077 "You need spaces around the comparison operator." + tautologyWarning t s = do + token <- listToMaybe $ filter isNonEmpty $ getWordParts t + return $ err (getId token) 2157 s + prop_checkConstantNoary = verify checkConstantNoary "[[ '$(foo)' ]]" prop_checkConstantNoary2 = verify checkConstantNoary "[ \"-f lol\" ]" prop_checkConstantNoary3 = verify checkConstantNoary "[[ cmd ]]" prop_checkConstantNoary4 = verify checkConstantNoary "[[ ! cmd ]]" -checkConstantNoary _ (TC_Noary _ _ t@(T_NormalWord id _)) | isConstant t = - err id 2078 "This expression is constant. Did you forget a $ somewhere?" +prop_checkConstantNoary5 = verify checkConstantNoary "[[ true ]]" +prop_checkConstantNoary6 = verify checkConstantNoary "[ 1 ]" +prop_checkConstantNoary7 = verify checkConstantNoary "[ false ]" +checkConstantNoary _ (TC_Noary _ _ t) | isConstant t = + case fromMaybe "" $ getLiteralString t of + "false" -> err (getId t) 2158 "[ false ] is true. Remove the brackets." + "0" -> err (getId t) 2159 "[ 0 ] is true. Use 'false' instead." + "true" -> style (getId t) 2160 "Instead of '[ true ]', just use 'true'." + "1" -> style (getId t) 2161 "Instead of '[ 1 ]', use 'true'." + _ -> err (getId t) 2078 "This expression is constant. Did you forget a $ somewhere?" + where + string = fromMaybe "" $ getLiteralString t + checkConstantNoary _ _ = return () prop_checkBraceExpansionVars1 = verify checkBraceExpansionVars "echo {1..$n}" @@ -1341,8 +1340,8 @@ prop_checkArithmeticDeref11= verifyNot checkArithmeticDeref "a[$foo]=wee" prop_checkArithmeticDeref12= verify checkArithmeticDeref "for ((i=0; $i < 3; i)); do true; done" prop_checkArithmeticDeref13= verifyNot checkArithmeticDeref "(( $$ ))" -checkArithmeticDeref params t@(TA_Expansion _ [T_DollarBraced id l]) = - unless (isException $ bracedString l) getWarning +checkArithmeticDeref params t@(TA_Expansion _ [b@(T_DollarBraced id _)]) = + unless (isException $ bracedString b) getWarning where isException [] = True isException s = any (`elem` "/.:#%?*@$") s || isDigit (head s) @@ -1462,6 +1461,7 @@ isQuoteFreeElement t = case t of T_Assignment {} -> return True + T_FdRedirect {} -> return True _ -> Nothing -- Are any subnodes inherently self-quoting? @@ -1539,52 +1539,6 @@ when (t `isUnqualifiedCommand` str) $ f cmd rest checkUnqualifiedCommand _ _ _ = return () -getLiteralString = getLiteralStringExt (const Nothing) - -getGlobOrLiteralString = getLiteralStringExt f - where - f (T_Glob _ str) = return str - f _ = Nothing - -getLiteralStringExt more = g - where - allInList = liftM concat . mapM g - g (T_DoubleQuoted _ l) = allInList l - g (T_DollarDoubleQuoted _ l) = allInList l - g (T_NormalWord _ l) = allInList l - g (TA_Expansion _ l) = allInList l - g (T_SingleQuoted _ s) = return s - g (T_Literal _ s) = return s - g x = more x - -isLiteral t = isJust $ getLiteralString t - --- Get a literal string ignoring all non-literals -onlyLiteralString :: Token -> String -onlyLiteralString = fromJust . getLiteralStringExt (const $ return "") - --- Turn a NormalWord like foo="bar $baz" into a series of constituent elements like [foo=,bar ,$baz] -getWordParts (T_NormalWord _ l) = concatMap getWordParts l -getWordParts (T_DoubleQuoted _ l) = l -getWordParts other = [other] - -getUnquotedLiteral (T_NormalWord _ list) = - liftM concat $ mapM str list - where - str (T_Literal _ s) = return s - str _ = Nothing -getUnquotedLiteral _ = Nothing - --- Return a list of NormalWords resulting from brace expansion -braceExpand (T_NormalWord id list) = take 1000 $ do - items <- mapM part list - return $ T_NormalWord id items - where - part (T_BraceExpansion id items) = do - item <- items - braceExpand item - part x = return x - isCommand token str = isCommandMatch token (\cmd -> cmd == str || ('/' : str) `isSuffixOf` cmd) isUnqualifiedCommand token str = isCommandMatch token (== str) @@ -1592,22 +1546,6 @@ cmd <- getCommandName token return $ matcher cmd -getCommandName (T_Redirecting _ _ w) = - getCommandName w -getCommandName (T_SimpleCommand _ _ (w:_)) = - getLiteralString w -getCommandName (T_Annotation _ _ t) = getCommandName t -getCommandName _ = Nothing - -getCommandBasename = liftM basename . getCommandName -basename = reverse . takeWhile (/= '/') . reverse - -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\"" prop_checkPrintfVar2 = verifyNot checkPrintfVar "printf 'Lol: $s'" prop_checkPrintfVar3 = verify checkPrintfVar "printf -v cow $(cmd)" @@ -1617,10 +1555,19 @@ f (format:params) = check format f _ = return () check format = - unless ('%' `elem` concat (deadSimple format) || isLiteral format) $ + unless ('%' `elem` concat (oversimplify format) || isLiteral format) $ warn (getId format) 2059 "Don't use variables in the printf format string. Use printf \"..%s..\" \"$foo\"." + +-- Check whether a word is entirely output from a single command +tokenIsJustCommandOutput t = case t of + T_NormalWord id [T_DollarExpansion _ _] -> True + T_NormalWord id [T_DoubleQuoted _ [T_DollarExpansion _ _]] -> True + T_NormalWord id [T_Backticked _ _] -> True + T_NormalWord id [T_DoubleQuoted _ [T_Backticked _ _]] -> True + _ -> False + prop_checkUuoeCmd1 = verify checkUuoeCmd "echo $(date)" prop_checkUuoeCmd2 = verify checkUuoeCmd "echo `date`" prop_checkUuoeCmd3 = verify checkUuoeCmd "echo \"$(date)\"" @@ -1631,14 +1578,6 @@ f [token] = when (tokenIsJustCommandOutput token) $ msg (getId token) f _ = return () --- Check whether a word is entirely output from a single command -tokenIsJustCommandOutput t = case t of - T_NormalWord id [T_DollarExpansion _ _] -> True - T_NormalWord id [T_DoubleQuoted _ [T_DollarExpansion _ _]] -> True - T_NormalWord id [T_Backticked _ _] -> True - T_NormalWord id [T_DoubleQuoted _ [T_Backticked _ _]] -> True - _ -> False - prop_checkUuoeVar1 = verify checkUuoeVar "for f in $(echo $tmp); do echo lol; done" prop_checkUuoeVar2 = verify checkUuoeVar "date +`echo \"$format\"`" prop_checkUuoeVar3 = verifyNot checkUuoeVar "foo \"$(echo -e '\r')\"" @@ -1742,7 +1681,7 @@ f (re:_) = do when (isGlob re) $ warn (getId re) 2062 "Quote the grep pattern so the shell won't interpret it." - let string = concat $ deadSimple re + let string = concat $ oversimplify re if isConfusedGlobRegex string then warn (getId re) 2063 "Grep uses regex, but this looks like a glob." else potentially $ do @@ -1776,7 +1715,7 @@ prop_checkTimeParameters2 = verifyNot checkTimeParameters "time sleep 10" prop_checkTimeParameters3 = verifyNot checkTimeParameters "time -p foo" checkTimeParameters _ = checkUnqualifiedCommand "time" f where - f cmd (x:_) = let s = concat $ deadSimple x in + f cmd (x:_) = let s = concat $ oversimplify x in when ("-" `isPrefixOf` s && s /= "-p") $ info (getId cmd) 2023 "The shell may override 'time' as seen in man time(1). Use 'command time ..' for that one." f _ _ = return () @@ -1812,7 +1751,7 @@ "sudo doesn't affect redirects. Use .. | sudo tee -a file" _ -> return () warnAbout _ = return () - special file = concat (deadSimple file) == "/dev/null" + special file = concat (oversimplify file) == "/dev/null" checkSudoRedirect _ _ = return () prop_checkReturn1 = verifyNot checkReturn "return" @@ -1857,7 +1796,7 @@ checkPS1Assignments _ (T_Assignment _ _ "PS1" _ word) = warnFor word where warnFor word = - let contents = concat $ deadSimple word in + let contents = concat $ oversimplify word in when (containsUnescaped contents) $ info (getId word) 2025 "Make sure all escape sequences are enclosed in \\[..\\] to prevent line wrapping issues" containsUnescaped s = @@ -1931,7 +1870,7 @@ 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." + warn id 2140 "Word is on the form \"A\"B\"C\" (B indicated). Did you mean \"ABC\" or \"A\\\"B\\\"C\"?" checkInexplicablyUnquoted _ _ = return () prop_checkTildeInQuotes1 = verify checkTildeInQuotes "var=\"~/out.txt\"" @@ -2025,7 +1964,7 @@ where isDashE = mkRegex "^-.*e" hasEscapes = mkRegex "\\\\[rnt]" - f args | concat (concatMap deadSimple allButLast) `matches` isDashE = + f args | concat (concatMap oversimplify allButLast) `matches` isDashE = return () where allButLast = reverse . drop 1 . reverse $ args f args = mapM_ checkEscapes args @@ -2069,7 +2008,7 @@ checkSshCommandString _ = checkCommand "ssh" (const f) where nonOptions = - filter (\x -> not $ "-" `isPrefixOf` concat (deadSimple x)) + filter (\x -> not $ "-" `isPrefixOf` concat (oversimplify x)) f args = case nonOptions args of (hostport:r@(_:_)) -> checkArg $ last r @@ -2100,6 +2039,8 @@ prop_subshellAssignmentCheck14 = verifyNotTree subshellAssignmentCheck "#!/bin/ksh93\necho foo | read bar; echo $bar" prop_subshellAssignmentCheck15 = verifyNotTree subshellAssignmentCheck "#!/bin/ksh\ncat foo | while read bar; do a=$bar; done\necho \"$a\"" prop_subshellAssignmentCheck16 = verifyNotTree subshellAssignmentCheck "(set -e); echo $@" +prop_subshellAssignmentCheck17 = verifyNotTree subshellAssignmentCheck "foo=${ { bar=$(baz); } 2>&1; }; echo $foo $bar" +prop_subshellAssignmentCheck18 = verifyTree subshellAssignmentCheck "( exec {n}>&2; ); echo $n" subshellAssignmentCheck params t = let flow = variableFlow params check = findSubshelled flow [("oops",[])] Map.empty @@ -2118,7 +2059,7 @@ data DataType = DataString DataSource | DataArray DataSource deriving (Show) -data DataSource = SourceFrom [Token] | SourceExternal | SourceDeclaration +data DataSource = SourceFrom [Token] | SourceExternal | SourceDeclaration | SourceInteger deriving (Show) data VariableState = Dead Token String | Alive deriving (Show) @@ -2158,6 +2099,12 @@ Sh -> True Ksh -> False +isClosingFileOp op = + case op of + T_IoFile _ (T_GREATAND _) (T_NormalWord _ [T_Literal _ "-"]) -> True + T_IoFile _ (T_LESSAND _) (T_NormalWord _ [T_Literal _ "-"]) -> True + _ -> False + getModifiedVariables t = case t of T_SimpleCommand _ vars [] -> @@ -2180,8 +2127,11 @@ name <- getLiteralString lhs return (t, t, name, DataString $ SourceFrom [rhs]) + t@(T_FdRedirect _ ('{':var) op) -> -- {foo}>&2 modifies foo + [(t, t, takeWhile (/= '}') var, DataString SourceInteger) | not $ isClosingFileOp op] + t@(T_CoProc _ name _) -> - [(t, t, fromMaybe "COPROC" name, DataArray SourceExternal)] + [(t, t, fromMaybe "COPROC" name, DataArray SourceInteger)] --Points to 'for' rather than variable T_ForIn id str words _ -> [(t, t, str, DataString $ SourceFrom words)] @@ -2275,7 +2225,7 @@ if var == "" then [] else [(base, token, var, DataString $ SourceFrom [stripEqualsFrom token])] - where var = takeWhile isVariableChar $ dropWhile (`elem` "+-") $ concat $ deadSimple token + where var = takeWhile isVariableChar $ dropWhile (`elem` "+-") $ concat $ oversimplify token getSetParams (t:_:rest) | getLiteralString t == Just "-o" = getSetParams rest getSetParams (t:rest) = @@ -2299,7 +2249,7 @@ lastArg <- listToMaybe (reverse arguments) name <- getLiteralString lastArg guard $ isVariableName name - return (base, lastArg, name, DataArray $ SourceExternal) + return (base, lastArg, name, DataArray SourceExternal) getModifiedVariableCommand _ = [] @@ -2313,8 +2263,10 @@ prop_getBracedReference8 = getBracedReference "foo-bar" == "foo" prop_getBracedReference9 = getBracedReference "foo:-bar" == "foo" prop_getBracedReference10= getBracedReference "foo: -1" == "foo" +prop_getBracedReference11= getBracedReference "!os*" == "" +prop_getBracedReference12= getBracedReference "!os?bar**" == "" getBracedReference s = fromMaybe s $ - takeName noPrefix `mplus` getSpecial noPrefix `mplus` getSpecial s + nameExpansion s `mplus` takeName noPrefix `mplus` getSpecial noPrefix `mplus` getSpecial s where noPrefix = dropPrefix s dropPrefix (c:rest) = if c `elem` "!#" then rest else c:rest @@ -2327,6 +2279,14 @@ if c `elem` "*@#?-$!" then return [c] else fail "not special" getSpecial _ = fail "empty" + nameExpansion ('!':rest) = do -- e.g. ${!foo*bar*} + let suffix = dropWhile isVariableChar rest + guard $ suffix /= rest -- e.g. ${!@} + first <- suffix !!! 0 + guard $ first `elem` "*?" + return "" + nameExpansion _ = Nothing + getIndexReferences s = fromMaybe [] $ do match <- matchRegex re s index <- match !!! 0 @@ -2336,7 +2296,7 @@ getReferencedVariables t = case t of - T_DollarBraced id l -> let str = bracedString l in + T_DollarBraced id l -> let str = bracedString t in (t, t, getBracedReference str) : map (\x -> (l, l, x)) (getIndexReferences str) TA_Expansion id _ -> getIfReference t t @@ -2345,6 +2305,9 @@ TC_Unary id _ "-v" token -> getIfReference t token TC_Unary id _ "-R" token -> getIfReference t token + + t@(T_FdRedirect _ ('{':var) op) -> -- {foo}>&- references and closes foo + [(t, t, takeWhile (/= '}') var) | isClosingFileOp op] x -> getReferencedVariableCommand x where -- Try to reduce false positives for unused vars only referenced from evaluated vars @@ -2430,6 +2393,17 @@ foldl (\m (_, token, var, _) -> Map.insert var (Dead token reason) m) deadVars scope + +-- FIXME: This is a very strange way of doing it. +-- For each variable read/write, run a stateful function that emits +-- comments. The comments are collected and returned. +doVariableFlowAnalysis :: + (Token -> Token -> String -> State t [v]) + -> (Token -> Token -> String -> DataType -> State t [v]) + -> t + -> [StackData] + -> [v] + doVariableFlowAnalysis readFunc writeFunc empty flow = evalState ( foldM (\list x -> do { l <- doFlow x; return $ l ++ list; }) [] flow ) empty @@ -2467,6 +2441,10 @@ prop_checkSpacefulness24= verifyTree checkSpacefulness "a='a b'; cat <<< $a" prop_checkSpacefulness25= verifyTree checkSpacefulness "a='s/[0-9]//g'; sed $a" prop_checkSpacefulness26= verifyTree checkSpacefulness "a='foo bar'; echo {1,2,$a}" +prop_checkSpacefulness27= verifyNotTree checkSpacefulness "echo ${a:+'foo'}" +prop_checkSpacefulness28= verifyNotTree checkSpacefulness "exec {n}>&1; echo $n" +prop_checkSpacefulness29= verifyNotTree checkSpacefulness "n=$(stuff); exec {n}>&-;" +prop_checkSpacefulness30= verifyTree checkSpacefulness "file='foo bar'; echo foo > $file;" checkSpacefulness params t = doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params) @@ -2482,18 +2460,18 @@ readF _ token name = do spaced <- hasSpaces name - return [Note (getId token) InfoC 2086 warning | + return [makeComment InfoC (getId token) 2086 warning | spaced && not (isArrayExpansion token) -- There's another warning for this && not (isCounting token) && not (isQuoteFree parents token) + && not (isQuotedAlternative token) && not (usedAsCommandName parents token)] where warning = "Double quote to prevent globbing and word splitting." - writeF _ _ name (DataString SourceExternal) = do - setSpaces name True - return [] + writeF _ _ name (DataString SourceExternal) = setSpaces name True >> return [] + writeF _ _ name (DataString SourceInteger) = setSpaces name False >> return [] writeF _ _ name (DataString (SourceFrom vals)) = do map <- get @@ -2506,11 +2484,18 @@ parents = parentMap params isCounting (T_DollarBraced id token) = - case concat $ deadSimple token of + case concat $ oversimplify token of '#':_ -> True _ -> False isCounting _ = False + -- FIXME: doesn't handle ${a:+$var} vs ${a:+"$var"} + isQuotedAlternative t = + case t of + T_DollarBraced _ _ -> + ":+" `isInfixOf` bracedString t + _ -> False + isSpacefulWord :: (String -> Bool) -> [Token] -> Bool isSpacefulWord f = any (isSpaceful f) isSpaceful :: (String -> Bool) -> Token -> Bool @@ -2522,7 +2507,7 @@ T_Extglob {} -> True T_Literal _ s -> s `containsAny` globspace T_SingleQuoted _ s -> s `containsAny` globspace - T_DollarBraced _ l -> spacefulF $ getBracedReference $ bracedString l + T_DollarBraced _ _ -> spacefulF $ getBracedReference $ bracedString x T_NormalWord _ w -> isSpacefulWord spacefulF w T_DoubleQuoted _ w -> isSpacefulWord spacefulF w _ -> False @@ -2561,13 +2546,13 @@ forToken map (T_DollarBraced id t) = -- skip getBracedReference here to avoid false positives on PE - Map.lookup (concat . deadSimple $ t) map + Map.lookup (concat . oversimplify $ t) map forToken quoteMap (T_DoubleQuoted id tokens) = msum $ map (forToken quoteMap) tokens forToken quoteMap (T_NormalWord id tokens) = msum $ map (forToken quoteMap) tokens forToken _ t = - if containsQuotes (concat $ deadSimple t) + if containsQuotes (concat $ oversimplify t) then return $ getId t else Nothing @@ -2578,9 +2563,9 @@ && not (isParamTo parents "eval" expr) && not (isQuoteFree parents expr) then [ - Note (fromJust assignment)WarningC 2089 + makeComment WarningC (fromJust assignment) 2089 "Quotes/backslashes will be treated literally. Use an array.", - Note (getId expr) WarningC 2090 + makeComment WarningC (getId expr) 2090 "Quotes/backslashes in this variable will not be respected." ] else []) @@ -2619,7 +2604,7 @@ | t `isUnqualifiedCommand` "alias" = mapM_ getAlias args findFunctions _ = return () getAlias arg = - let string = concat $ deadSimple arg + let string = concat $ oversimplify arg in when ('=' `elem` string) $ modify ((takeWhile (/= '=') string, getId arg):) checkArg cmd arg = potentially $ do @@ -2701,6 +2686,7 @@ 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}" +prop_checkUnassignedReferences22= verifyNotTree checkUnassignedReferences "echo ${!os*}" checkUnassignedReferences params t = warnings where (readMap, writeMap) = execState (mapM tally $ variableFlow params) (Map.empty, Map.empty) @@ -2756,14 +2742,14 @@ isInArray var t = any isArray $ getPath (parentMap params) t where isArray (T_Array {}) = True - isArray (T_DollarBraced _ l) | var /= getBracedReference (bracedString l) = True + isArray b@(T_DollarBraced _ _) | var /= getBracedReference (bracedString b) = True isArray _ = False isGuarded (T_DollarBraced _ v) = any (`isPrefixOf` rest) ["-", ":-", "?", ":?"] where - name = concat $ deadSimple v - rest = dropWhile isVariableChar $ dropWhile (`elem` "#!") $ name + name = concat $ oversimplify v + rest = dropWhile isVariableChar $ dropWhile (`elem` "#!") name isGuarded _ = False match var candidate = @@ -2780,12 +2766,12 @@ where check v@(T_NormalWord _ (T_Glob id s:_)) | s == "*" || s == "?" = info id 2035 $ - "Use ./" ++ concat (deadSimple v) + "Use ./" ++ concat (oversimplify v) ++ " so names with dashes won't become options." check _ = return () isEndOfArgs t = - case concat $ deadSimple t of + case concat $ oversimplify t of "--" -> True ":::" -> True "::::" -> True @@ -2809,7 +2795,7 @@ munchers = [ "ssh", "ffmpeg", "mplayer" ] isStdinReadCommand (T_Pipeline _ _ [T_Redirecting id redirs cmd]) = - let plaintext = deadSimple cmd + let plaintext = oversimplify cmd in head (plaintext ++ [""]) == "read" && ("-u" `notElem` plaintext) && all (not . stdinRedirect) redirs @@ -2841,7 +2827,7 @@ checkPrefixAssignmentReference params t@(T_DollarBraced id value) = check path where - name = getBracedReference $ bracedString value + name = getBracedReference $ bracedString t path = getPath (parentMap params) t idPath = map getId path @@ -2883,6 +2869,7 @@ prop_checkCdAndBack1 = verify checkCdAndBack "for f in *; do cd $f; git pull; cd ..; done" prop_checkCdAndBack2 = verifyNot checkCdAndBack "for f in *; do cd $f || continue; git pull; cd ..; done" prop_checkCdAndBack3 = verifyNot checkCdAndBack "while [[ $PWD != / ]]; do cd ..; done" +prop_checkCdAndBack4 = verify checkCdAndBack "cd $tmp; foo; cd -" checkCdAndBack params = doLists where shell = shellType params @@ -2890,13 +2877,14 @@ doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds doLists (T_WhileExpression _ _ cmds) = doList cmds doLists (T_UntilExpression _ _ cmds) = doList cmds + doLists (T_Script _ _ cmds) = doList cmds doLists (T_IfExpression _ thens elses) = do mapM_ (\(_, l) -> doList l) thens doList elses doLists _ = return () isCdRevert t = - case deadSimple t of + case oversimplify t of ["cd", p] -> p `elem` ["..", "-"] _ -> False @@ -2907,12 +2895,9 @@ doList list = let cds = filter ((== Just "cd") . getCmd) list in when (length cds >= 2 && isCdRevert (last cds)) $ - warn (getId $ head cds) 2103 message + info (getId $ last cds) 2103 message - message = - if shell == Bash - then "Consider using ( subshell ), 'cd foo||exit', or pushd/popd instead." - else "Consider using ( subshell ) or 'cd foo||exit' instead." + message = "Use a ( subshell ) to avoid having to cd back." prop_checkLoopKeywordScope1 = verify checkLoopKeywordScope "continue 2" prop_checkLoopKeywordScope2 = verify checkLoopKeywordScope "for f; do ( break; ); done" @@ -2979,7 +2964,7 @@ when (any isRecursiveFlag simpleArgs) $ mapM_ (mapM_ checkWord . braceExpand) tokens where - simpleArgs = deadSimple t + simpleArgs = oversimplify t checkWord token = case getLiteralString token of @@ -3163,7 +3148,7 @@ mapM_ checkVar vars where checkVar (T_Assignment id Assign "PATH" Nothing word) = - let string = concat $ deadSimple word + let string = concat $ oversimplify 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 @@ -3190,6 +3175,7 @@ prop_checkUnsupported3 = verify checkUnsupported "#!/bin/sh\ncase foo in bar) baz ;& esac" prop_checkUnsupported4 = verify checkUnsupported "#!/bin/ksh\ncase foo in bar) baz ;;& esac" +prop_checkUnsupported5 = verify checkUnsupported "#!/bin/bash\necho \"${ ls; }\"" checkUnsupported params t = when (not (null support) && (shellType params `notElem` support)) $ report name @@ -3203,6 +3189,7 @@ shellSupport t = case t of T_CaseExpression _ _ list -> forCase (map (\(a,_,_) -> a) list) + T_DollarBraceCommandExpansion {} -> ("${ ..; } command expansion", [Ksh]) otherwise -> ("", []) where forCase seps | CaseContinue `elem` seps = ("cases with ;;&", [Bash]) @@ -3210,16 +3197,6 @@ forCase _ = ("", []) -getCommandSequences (T_Script _ _ cmds) = [cmds] -getCommandSequences (T_BraceGroup _ cmds) = [cmds] -getCommandSequences (T_Subshell _ cmds) = [cmds] -getCommandSequences (T_WhileExpression _ _ cmds) = [cmds] -getCommandSequences (T_UntilExpression _ _ cmds) = [cmds] -getCommandSequences (T_ForIn _ _ _ cmds) = [cmds] -getCommandSequences (T_ForArithmetic _ _ _ _ cmds) = [cmds] -getCommandSequences (T_IfExpression _ thens elses) = map snd thens ++ [elses] -getCommandSequences _ = [] - groupWith f = groupBy ((==) `on` f) prop_checkMultipleAppends1 = verify checkMultipleAppends "foo >> file; bar >> file; baz >> file;" @@ -3234,6 +3211,7 @@ style (snd $ fromJust f) 2129 "Consider using { cmd1; cmd2; } >> file instead of individual redirects." checkGroup _ = return () + getTarget (T_Annotation _ _ t) = getTarget t getTarget (T_Pipeline _ _ args@(_:_)) = getTarget (last args) getTarget (T_Redirecting id list _) = do file <- mapMaybe getAppend list !!! 0 @@ -3250,7 +3228,7 @@ checkUnqualifiedCommand "alias" (const f) where f = mapM_ checkArg - checkArg arg | '=' `elem` concat (deadSimple arg) = + checkArg arg | '=' `elem` concat (oversimplify arg) = forM_ (take 1 $ filter (not . isLiteral) $ getWordParts arg) $ \x -> warn (getId x) 2139 "This expands when defined, not when used. Consider escaping." checkArg _ = return () @@ -3339,7 +3317,7 @@ pattern = [isMatch, const True, isParam ["-o", "-or"], isMatch, const True, isAction] f list | length list < length pattern = return () f list@(_:rest) = - if all id (zipWith ($) pattern list) + if and (zipWith ($) pattern list) then warnFor (list !! (length pattern - 1)) else f rest isMatch = isParam [ "-name", "-regex", "-iname", "-iregex", "-wholename", "-iwholename" ] @@ -3376,7 +3354,7 @@ checkMaskedReturns _ t@(T_SimpleCommand id _ (cmd:rest)) = potentially $ do name <- getCommandName t guard $ name `elem` ["declare", "export"] - || name == "local" && "r" `notElem` (map snd $ getAllFlags t) + || name == "local" && "r" `notElem` map snd (getAllFlags t) return $ mapM_ checkArgs rest where checkArgs (T_Assignment id _ _ _ word) | any hasReturn $ getWordParts word = @@ -3413,6 +3391,75 @@ when ("{}" `isInfixOf` arg) $ warn id 2156 "Injecting filenames is fragile and insecure. Use parameters." +prop_checkReadWithoutR1 = verify checkReadWithoutR "read -a foo" +prop_checkReadWithoutR2 = verifyNot checkReadWithoutR "read -ar foo" +checkReadWithoutR _ t@(T_SimpleCommand {}) | t `isUnqualifiedCommand` "read" = + unless ("r" `elem` map snd (getAllFlags t)) $ + info (getId t) 2162 "read without -r will mangle backslashes." +checkReadWithoutR _ _ = return () + +prop_checkExportedExpansions1 = verify checkExportedExpansions "export $foo" +prop_checkExportedExpansions2 = verify checkExportedExpansions "export \"$foo\"" +prop_checkExportedExpansions3 = verifyNot checkExportedExpansions "export foo" +checkExportedExpansions _ = checkUnqualifiedCommand "export" (const check) + where + check = mapM_ checkForVariables + checkForVariables f = + case getWordParts f of + [t@(T_DollarBraced {})] -> + warn (getId t) 2163 "Exporting an expansion rather than a variable." + _ -> return () + + +prop_checkUncheckedCd1 = verifyTree checkUncheckedCd "cd ~/src; rm -r foo" +prop_checkUncheckedCd2 = verifyNotTree checkUncheckedCd "cd ~/src || exit; rm -r foo" +prop_checkUncheckedCd3 = verifyNotTree checkUncheckedCd "set -e; cd ~/src; rm -r foo" +prop_checkUncheckedCd4 = verifyNotTree checkUncheckedCd "if cd foo; then rm foo; fi" +prop_checkUncheckedCd5 = verifyTree checkUncheckedCd "if true; then cd foo; fi" +prop_checkUncheckedCd6 = verifyNotTree checkUncheckedCd "cd .." +checkUncheckedCd params root = + if hasSetE then [] else execWriter $ doAnalysis checkElement root + where + checkElement t@(T_SimpleCommand {}) = + when(t `isUnqualifiedCommand` "cd" + && not (isCdDotDot t) + && not (isCondition $ getPath (parentMap params) t)) $ + warn (getId t) 2164 "Use cd ... || exit in case cd fails." + checkElement _ = return () + isCdDotDot t = oversimplify t == ["cd", ".."] + hasSetE = isNothing $ doAnalysis (guard . not . isSetE) root + isSetE t = + case t of + T_SimpleCommand {} -> + t `isUnqualifiedCommand` "set" && "e" `elem` map snd (getAllFlags t) + _ -> False + +prop_checkLoopVariableReassignment1 = verify checkLoopVariableReassignment "for i in *; do for i in *.bar; do true; done; done" +prop_checkLoopVariableReassignment2 = verify checkLoopVariableReassignment "for i in *; do for((i=0; i<3; i++)); do true; done; done" +prop_checkLoopVariableReassignment3 = verifyNot checkLoopVariableReassignment "for i in *; do for j in *.bar; do true; done; done" +checkLoopVariableReassignment params token = + potentially $ case token of + T_ForIn {} -> check + T_ForArithmetic {} -> check + _ -> Nothing + where + check = do + str <- loopVariable token + next <- listToMaybe $ filter (\x -> loopVariable x == Just str) path + return $ do + warn (getId token) 2165 "This nested loop overrides the index variable of its parent." + warn (getId next) 2165 "This parent loop has its index variable overridden." + path = drop 1 $ getPath (parentMap params) token + loopVariable :: Token -> Maybe String + loopVariable t = + case t of + T_ForIn _ s _ _ -> return s + T_ForArithmetic _ + (TA_Sequence _ + [TA_Binary _ "=" + (TA_Expansion _ [T_Literal _ var]) _]) + _ _ _ -> return var + _ -> fail "not loop" return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/ShellCheck/Analyzer.hs b/ShellCheck/Analyzer.hs new file mode 100644 --- /dev/null +++ b/ShellCheck/Analyzer.hs @@ -0,0 +1,27 @@ +{- + Copyright 2012-2015 Vidar Holen + + 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 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +module ShellCheck.Analyzer (analyzeScript) where + +import ShellCheck.Interface +import ShellCheck.Analytics + +-- TODO: Clean up the cruft this is layered on +analyzeScript :: AnalysisSpec -> AnalysisResult +analyzeScript = runAnalytics diff --git a/ShellCheck/Checker.hs b/ShellCheck/Checker.hs new file mode 100644 --- /dev/null +++ b/ShellCheck/Checker.hs @@ -0,0 +1,159 @@ +{- + Copyright 2012-2015 Vidar Holen + + 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 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +{-# LANGUAGE TemplateHaskell #-} +module ShellCheck.Checker (checkScript, ShellCheck.Checker.runTests) where + +import ShellCheck.Interface +import ShellCheck.Parser +import ShellCheck.Analyzer + +import Data.Either +import Data.Functor +import Data.List +import Data.Maybe +import Data.Ord +import Control.Monad.Identity +import qualified Data.Map as Map +import qualified System.IO +import Prelude hiding (readFile) +import Control.Monad + +import Test.QuickCheck.All + +tokenToPosition map (TokenComment id c) = fromMaybe fail $ do + position <- Map.lookup id map + return $ PositionedComment position c + where + fail = error "Internal shellcheck error: id doesn't exist. Please report!" + +checkScript :: Monad m => SystemInterface m -> CheckSpec -> m CheckResult +checkScript sys spec = do + results <- checkScript (csScript spec) + return CheckResult { + crFilename = csFilename spec, + crComments = results + } + where + checkScript contents = do + result <- parseScript sys ParseSpec { + psFilename = csFilename spec, + psScript = contents + } + let parseMessages = prComments result + let analysisMessages = + fromMaybe [] $ + (arComments . analyzeScript . analysisSpec) + <$> prRoot result + let translator = tokenToPosition (prTokenPositions result) + return . nub . sortMessages . filter shouldInclude $ + (parseMessages ++ map translator analysisMessages) + + shouldInclude (PositionedComment _ (Comment _ code _)) = + code `notElem` csExcludedWarnings spec + + sortMessages = sortBy (comparing order) + order (PositionedComment pos (Comment severity code message)) = + (posFile pos, posLine pos, posColumn pos, severity, code, message) + getPosition (PositionedComment pos _) = pos + + analysisSpec root = + AnalysisSpec { + asScript = root, + asShellType = csShellTypeOverride spec, + asExecutionMode = Executed + } + +getErrors sys spec = + sort . map getCode . crComments $ + runIdentity (checkScript sys spec) + where + getCode (PositionedComment _ (Comment _ code _)) = code + +check = checkWithIncludes [] + +checkWithIncludes includes src = + getErrors + (mockedSystemInterface includes) + emptyCheckSpec { + csScript = src, + csExcludedWarnings = [2148] + } + +prop_findsParseIssue = check "echo \"$12\"" == [1037] + +prop_commentDisablesParseIssue1 = + null $ check "#shellcheck disable=SC1037\necho \"$12\"" +prop_commentDisablesParseIssue2 = + null $ check "#shellcheck disable=SC1037\n#lol\necho \"$12\"" + +prop_findsAnalysisIssue = + check "echo $1" == [2086] +prop_commentDisablesAnalysisIssue1 = + null $ check "#shellcheck disable=SC2086\necho $1" +prop_commentDisablesAnalysisIssue2 = + null $ check "#shellcheck disable=SC2086\n#lol\necho $1" + +prop_optionDisablesIssue1 = + null $ getErrors + (mockedSystemInterface []) + emptyCheckSpec { + csScript = "echo $1", + csExcludedWarnings = [2148, 2086] + } + +prop_optionDisablesIssue2 = + null $ getErrors + (mockedSystemInterface []) + emptyCheckSpec { + csScript = "echo \"$10\"", + csExcludedWarnings = [2148, 1037] + } + +prop_canParseDevNull = + [] == check "source /dev/null" + +prop_failsWhenNotSourcing = + [1091, 2154] == check "source lol; echo \"$bar\"" + +prop_worksWhenSourcing = + null $ checkWithIncludes [("lib", "bar=1")] "source lib; echo \"$bar\"" + +prop_worksWhenDotting = + null $ checkWithIncludes [("lib", "bar=1")] ". lib; echo \"$bar\"" + +prop_noInfiniteSourcing = + [] == checkWithIncludes [("lib", "source lib")] "source lib" + +prop_canSourceBadSyntax = + [1094, 2086] == checkWithIncludes [("lib", "for f; do")] "source lib; echo $1" + +prop_cantSourceDynamic = + [1090] == checkWithIncludes [("lib", "")] ". \"$1\"" + +prop_canSourceDynamicWhenRedirected = + null $ checkWithIncludes [("lib", "")] "#shellcheck source=lib\n. \"$1\"" + +prop_sourceDirectiveDoesntFollowFile = + null $ checkWithIncludes + [("foo", "source bar"), ("bar", "baz=3")] + "#shellcheck source=foo\n. \"$1\"; echo \"$baz\"" + +return [] +runTests = $quickCheckAll diff --git a/ShellCheck/Data.hs b/ShellCheck/Data.hs --- a/ShellCheck/Data.hs +++ b/ShellCheck/Data.hs @@ -1,5 +1,6 @@ module ShellCheck.Data where +import ShellCheck.Interface import Data.Version (showVersion) import Paths_ShellCheck (version) @@ -73,3 +74,15 @@ "tango", "uniform", "victor", "whiskey", "xray", "yankee", "zulu" ] + +shellForExecutable :: String -> Maybe Shell +shellForExecutable "sh" = return Sh +shellForExecutable "ash" = return Sh +shellForExecutable "dash" = return Sh + +shellForExecutable "ksh" = return Ksh +shellForExecutable "ksh88" = return Ksh +shellForExecutable "ksh93" = return Ksh + +shellForExecutable "bash" = return Bash +shellForExecutable _ = Nothing diff --git a/ShellCheck/Formatter/Format.hs b/ShellCheck/Formatter/Format.hs new file mode 100644 --- /dev/null +++ b/ShellCheck/Formatter/Format.hs @@ -0,0 +1,61 @@ +{- + Copyright 2012-2015 Vidar Holen + + 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 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +module ShellCheck.Formatter.Format where + +import ShellCheck.Data +import ShellCheck.Interface + +-- A formatter that carries along an arbitrary piece of data +data Formatter = Formatter { + header :: IO (), + onResult :: CheckResult -> String -> IO (), + onFailure :: FilePath -> ErrorMessage -> IO (), + footer :: IO () +} + +lineNo (PositionedComment pos _) = posLine pos +colNo (PositionedComment pos _) = posColumn pos +codeNo (PositionedComment _ (Comment _ code _)) = code +messageText (PositionedComment _ (Comment _ _ t)) = t + +severityText :: PositionedComment -> String +severityText (PositionedComment _ (Comment c _ _)) = + case c of + ErrorC -> "error" + WarningC -> "warning" + InfoC -> "info" + StyleC -> "style" + +-- Realign comments from a tabstop of 8 to 1 +makeNonVirtual comments contents = + map fix comments + where + ls = lines contents + fix c@(PositionedComment pos comment) = PositionedComment pos { + posColumn = + if lineNo c > 0 && lineNo c <= fromIntegral (length ls) + then real (ls !! fromIntegral (lineNo c - 1)) 0 0 (colNo c) + else colNo c + } comment + real _ r v target | target <= v = r + real [] r v _ = r -- should never happen + real ('\t':rest) r v target = + real rest (r+1) (v + 8 - (v `mod` 8)) target + real (_:rest) r v target = real rest (r+1) (v+1) target diff --git a/ShellCheck/Interface.hs b/ShellCheck/Interface.hs new file mode 100644 --- /dev/null +++ b/ShellCheck/Interface.hs @@ -0,0 +1,103 @@ +{- + Copyright 2012-2015 Vidar Holen + + 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 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +module ShellCheck.Interface where + +import ShellCheck.AST +import Control.Monad.Identity +import qualified Data.Map as Map + + +data SystemInterface m = SystemInterface { + -- Read a file by filename, or return an error + siReadFile :: String -> m (Either ErrorMessage String) +} + +-- ShellCheck input and output +data CheckSpec = CheckSpec { + csFilename :: String, + csScript :: String, + csExcludedWarnings :: [Integer], + csShellTypeOverride :: Maybe Shell +} deriving (Show, Eq) + +data CheckResult = CheckResult { + crFilename :: String, + crComments :: [PositionedComment] +} deriving (Show, Eq) + +emptyCheckSpec = CheckSpec { + csFilename = "", + csScript = "", + csExcludedWarnings = [], + csShellTypeOverride = Nothing +} + +-- Parser input and output +data ParseSpec = ParseSpec { + psFilename :: String, + psScript :: String +} deriving (Show, Eq) + +data ParseResult = ParseResult { + prComments :: [PositionedComment], + prTokenPositions :: Map.Map Id Position, + prRoot :: Maybe Token +} deriving (Show, Eq) + +-- Analyzer input and output +data AnalysisSpec = AnalysisSpec { + asScript :: Token, + asShellType :: Maybe Shell, + asExecutionMode :: ExecutionMode +} + +data AnalysisResult = AnalysisResult { + arComments :: [TokenComment] +} + +-- Supporting data types +data Shell = Ksh | Sh | Bash deriving (Show, Eq) +data ExecutionMode = Executed | Sourced deriving (Show, Eq) + +type ErrorMessage = String +type Code = Integer + +data Severity = ErrorC | WarningC | InfoC | StyleC deriving (Show, Eq, Ord) +data Position = Position { + posFile :: String, -- Filename + posLine :: Integer, -- 1 based source line + posColumn :: Integer -- 1 based source column, where tabs are 8 +} deriving (Show, Eq) + +data Comment = Comment Severity Code String deriving (Show, Eq) +data PositionedComment = PositionedComment Position Comment deriving (Show, Eq) +data TokenComment = TokenComment Id Comment deriving (Show, Eq) + +-- For testing +mockedSystemInterface :: [(String, String)] -> SystemInterface Identity +mockedSystemInterface files = SystemInterface { + siReadFile = rf +} + where + rf file = + case filter ((== file) . fst) files of + [] -> return $ Left "File not included in mock." + [(_, contents)] -> return $ Right contents + diff --git a/ShellCheck/Options.hs b/ShellCheck/Options.hs deleted file mode 100644 --- a/ShellCheck/Options.hs +++ /dev/null @@ -1,14 +0,0 @@ -module ShellCheck.Options where - -data Shell = Ksh | Sh | Bash - deriving (Show, Eq) - -data AnalysisOptions = AnalysisOptions { - optionShellType :: Maybe Shell, - optionExcludes :: [Integer] -} - -defaultAnalysisOptions = AnalysisOptions { - optionShellType = Nothing, - optionExcludes = [] -} diff --git a/ShellCheck/Parser.hs b/ShellCheck/Parser.hs --- a/ShellCheck/Parser.hs +++ b/ShellCheck/Parser.hs @@ -1,4 +1,6 @@ {- + Copyright 2012-2015 Vidar Holen + This file is part of ShellCheck. http://www.vidarholen.net/contents/shellcheck @@ -16,26 +18,39 @@ along with this program. If not, see . -} {-# LANGUAGE NoMonomorphismRestriction, TemplateHaskell, FlexibleContexts #-} -module ShellCheck.Parser (Note(..), Severity(..), parseShell, ParseResult(..), ParseNote(..), sortNotes, noteToParseNote, runTests, readScript) where +module ShellCheck.Parser (parseScript, runTests) where import ShellCheck.AST +import ShellCheck.ASTLib import ShellCheck.Data -import ShellCheck.Options -import Text.Parsec -import Debug.Trace +import ShellCheck.Interface + +import Control.Applicative ((<*)) import Control.Monad -import Control.Arrow (first) +import Control.Monad.Identity +import Control.Monad.Trans import Data.Char +import Data.Functor import Data.List (isPrefixOf, isInfixOf, isSuffixOf, partition, sortBy, intercalate, nub) -import qualified Data.Map as Map -import qualified Control.Monad.State as Ms import Data.Maybe +import Data.Monoid +import Debug.Trace +import GHC.Exts (sortWith) import Prelude hiding (readList) import System.IO +import Text.Parsec hiding (runParser, ()) import Text.Parsec.Error -import GHC.Exts (sortWith) +import Text.Parsec.Pos +import qualified Control.Monad.Reader as Mr +import qualified Control.Monad.State as Ms +import qualified Data.Map as Map + import Test.QuickCheck.All (quickCheckAll) +type SCBase m = Mr.ReaderT (SystemInterface m) (Ms.StateT SystemState m) +type SCParser m v = ParsecT String UserState (SCBase m) v + +backslash :: Monad m => SCParser m Char backslash = char '\\' linefeed = optional carriageReturn >> char '\n' singleQuote = char '\'' <|> unicodeSingleQuote @@ -44,14 +59,14 @@ variableChars = upper <|> lower <|> digit <|> oneOf "_" functionChars = variableChars <|> oneOf ":+-.?" specialVariable = oneOf "@*#?-$!" -tokenDelimiter = oneOf "&|;<> \t\n\r" <|> nbsp +tokenDelimiter = oneOf "&|;<> \t\n\r" <|> almostSpace quotableChars = "|&;<>()\\ '\t\n\r\xA0" ++ doubleQuotableChars -quotable = nbsp <|> unicodeDoubleQuote <|> oneOf quotableChars +quotable = almostSpace <|> unicodeDoubleQuote <|> oneOf quotableChars bracedQuotable = oneOf "}\"$`'" doubleQuotableChars = "\"$`" ++ unicodeDoubleQuoteChars doubleQuotable = unicodeDoubleQuote <|> oneOf doubleQuotableChars -whitespace = oneOf " \t\n" <|> carriageReturn <|> nbsp -linewhitespace = oneOf " \t" <|> nbsp +whitespace = oneOf " \t\n" <|> carriageReturn <|> almostSpace +linewhitespace = oneOf " \t" <|> almostSpace suspectCharAfterQuotes = variableChars <|> char '%' @@ -103,35 +118,54 @@ parseNote ErrorC 1017 "Literal carriage return. Run script through tr -d '\\r' ." char '\r' -nbsp = do - parseNote ErrorC 1018 "This is a  . Delete it and retype as space." - char '\xA0' - return ' ' +almostSpace = + choice [ + check '\xA0' "unicode non-breaking space", + check '\x200B' "unicode zerowidth space" + ] + where + check c name = do + parseNote ErrorC 1018 $ "This is a " ++ name ++ ". Delete and retype it." + char c + return ' ' --------- Message/position annotation on top of user state data Note = Note Id Severity Code String deriving (Show, Eq) data ParseNote = ParseNote SourcePos Severity Code String deriving (Show, Eq) -data Severity = ErrorC | WarningC | InfoC | StyleC deriving (Show, Eq, Ord) -data Context = ContextName SourcePos String | ContextAnnotation [Annotation] deriving (Show) -type Code = Integer +data Context = + ContextName SourcePos String + | ContextAnnotation [Annotation] + | ContextSource String + deriving (Show) +data UserState = UserState { + lastId :: Id, + positionMap :: Map.Map Id SourcePos, + parseNotes :: [ParseNote] +} +initialUserState = UserState { + lastId = Id $ -1, + positionMap = Map.empty, + parseNotes = [] +} + codeForParseNote (ParseNote _ _ code _) = code noteToParseNote map (Note id severity code message) = ParseNote pos severity code message where pos = fromJust $ Map.lookup id map -initialState = (Id $ -1, Map.empty, []) -getLastId = do - (id, _, _) <- getState - return id +getLastId = lastId <$> getState getNextIdAt sourcepos = do - (id, map, notes) <- getState - let newId = incId id - let newMap = Map.insert newId sourcepos map - putState (newId, newMap, notes) + state <- getState + let newId = incId (lastId state) + let newMap = Map.insert newId sourcepos (positionMap state) + putState $ state { + lastId = newId, + positionMap = newMap + } return newId where incId (Id n) = Id $ n+1 @@ -139,23 +173,16 @@ pos <- getPosition getNextIdAt pos -modifyMap f = do - (id, map, parsenotes) <- getState - putState (id, f map, parsenotes) - -getMap = do - (_, map, _) <- getState - return map - -getParseNotes = do - (_, _, notes) <- getState - return notes +getMap = positionMap <$> getState +getParseNotes = parseNotes <$> getState addParseNote n = do irrelevant <- shouldIgnoreCode (codeForParseNote n) unless irrelevant $ do - (a, b, notes) <- getState - putState (a, b, n:notes) + state <- getState + putState $ state { + parseNotes = n : parseNotes state + } shouldIgnoreCode code = do context <- getCurrentContexts @@ -163,20 +190,57 @@ where disabling (ContextAnnotation list) = any disabling' list + disabling (ContextSource _) = True -- Don't add messages for sourced files disabling _ = False disabling' (DisableComment n) = code == n + disabling' _ = False +shouldFollow file = do + context <- getCurrentContexts + if any isThisFile context + then return False + else + if length (filter isSource context) >= 100 + then do + parseProblem ErrorC 1092 "Stopping at 100 'source' frames :O" + return False + else + return True + where + isSource (ContextSource _) = True + isSource _ = False + isThisFile (ContextSource name) | name == file = True + isThisFile _= False + +getSourceOverride = do + context <- getCurrentContexts + return . msum . map findFile $ takeWhile isSameFile context + where + isSameFile (ContextSource _) = False + isSameFile _ = True + + findFile (ContextAnnotation list) = msum $ map getFile list + findFile _ = Nothing + getFile (SourceOverride str) = Just str + getFile _ = Nothing + -- Store potential parse problems outside of parsec + +data SystemState = SystemState { + contextStack :: [Context], + parseProblems :: [ParseNote] +} +initialSystemState = SystemState { + contextStack = [], + parseProblems = [] +} + parseProblem level code msg = do pos <- getPosition parseProblemAt pos level code msg -setCurrentContexts c = - Ms.modify (\(list, _) -> (list, c)) - -getCurrentContexts = do - (_, context) <- Ms.get - return context +setCurrentContexts c = Ms.modify (\state -> state { contextStack = c }) +getCurrentContexts = contextStack <$> Ms.get popContext = do v <- getCurrentContexts @@ -195,7 +259,11 @@ parseProblemAt pos level code msg = do irrelevant <- shouldIgnoreCode code unless irrelevant $ - Ms.modify (first ((:) (ParseNote pos level code msg))) + Ms.modify (\state -> state { + parseProblems = note:parseProblems state + }) + where + note = ParseNote pos level code msg -- Store non-parse problems inside @@ -212,9 +280,9 @@ return r unexpecting s p = try $ - (try p >> unexpected s) <|> return () + (try p >> fail ("Unexpected " ++ s)) <|> return () -notFollowedBy2 = unexpecting "keyword/token" +notFollowedBy2 = unexpecting "" disregard = void @@ -315,7 +383,7 @@ readCondUnaryExp = do op <- readCondUnaryOp pos <- getPosition - (readCondWord >>= return . op) `orFail` do + liftM op readCondWord `orFail` do parseProblemAt pos ErrorC 1019 "Expected this to be an argument to the unary condition." return "Expected an argument for the unary operator" @@ -484,10 +552,16 @@ prop_a16= isOk readArithmeticContents "$foo$bar" prop_a17= isOk readArithmeticContents "i<(0+(1+1))" prop_a18= isOk readArithmeticContents "a?b:c" +prop_a19= isOk readArithmeticContents "\\\n3 +\\\n 2" +prop_a20= isOk readArithmeticContents "a ? b ? c : d : e" +prop_a21= isOk readArithmeticContents "a ? b : c ? d : e" +prop_a22= isOk readArithmeticContents "!!a" readArithmeticContents = readSequence where - spacing = many whitespace + spacing = + let lf = try (string "\\\n") >> return '\n' + in many (whitespace <|> lf) splitBy x ops = chainl1 x (readBinary ops) readBinary ops = readComboOp ops TA_Binary @@ -547,16 +621,15 @@ readAssignment = readTrinary `splitBy` ["=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", "&=", "^=", "|="] readTrinary = do - let part = readLogicalOr - x <- part + x <- readLogicalOr do id <- getNextId string "?" spacing - y <- part + y <- readTrinary string ":" spacing - z <- part + z <- readTrinary return $ TA_Trinary id x y z <|> return x @@ -578,7 +651,7 @@ id <- getNextId op <- oneOf "!~" spacing - x <- readAnySigned + x <- readAnyNegated return $ TA_Unary id [op] x readAnySigned = readSigned <|> readAnycremented @@ -652,7 +725,7 @@ condition <- readConditionContents single cpos <- getPosition - close <- try (string "]]") <|> string "]" + close <- try (string "]]") <|> string "]" <|> fail "Expected test to end here" when (open == "[[" && close /= "]]") $ parseProblemAt cpos ErrorC 1033 "Did you mean ]] ?" when (open == "[" && close /= "]" ) $ parseProblemAt opos ErrorC 1034 "Did you mean [[ ?" spacing @@ -666,10 +739,11 @@ prop_readAnnotation1 = isOk readAnnotation "# shellcheck disable=1234,5678\n" prop_readAnnotation2 = isOk readAnnotation "# shellcheck disable=SC1234 disable=SC5678\n" +prop_readAnnotation3 = isOk readAnnotation "# shellcheck disable=SC1234 source=/dev/null disable=SC5678\n" readAnnotation = called "shellcheck annotation" $ do try readAnnotationPrefix many1 linewhitespace - values <- many1 readDisable + values <- many1 (readDisable <|> readSourceOverride) linefeed many linewhitespace return $ concat values @@ -681,6 +755,11 @@ optional $ string "SC" int <- many1 digit return $ DisableComment (read int) + + readSourceOverride = forKey "source" $ do + filename <- many1 $ noneOf " \n" + return [SourceOverride filename] + forKey s p = do try $ string s char '=' @@ -796,7 +875,7 @@ s <- readSingleQuotedPart `reluctantlyTill` singleQuote let string = concat s endPos <- getPosition - singleQuote "end of single quoted string" + singleQuote <|> fail "Expected end of single quoted string" optional $ do c <- try . lookAhead $ suspectCharAfterQuotes <|> oneOf "'" @@ -871,18 +950,32 @@ setPosition lastPosition return result +inSeparateContext parser = do + context <- Ms.get + success context <|> failure context + where + success c = do + res <- try parser + Ms.put c + return res + failure c = do + Ms.put c + fail "" + prop_readDoubleQuoted = isOk readDoubleQuoted "\"Hello $FOO\"" prop_readDoubleQuoted2 = isOk readDoubleQuoted "\"$'\"" prop_readDoubleQuoted3 = isWarning readDoubleQuoted "\x201Chello\x201D" prop_readDoubleQuoted4 = isWarning readSimpleCommand "\"foo\nbar\"foo" prop_readDoubleQuoted5 = isOk readSimpleCommand "lol \"foo\nbar\" etc" +prop_readDoubleQuoted6 = isOk readSimpleCommand "echo \"${ ls; }\"" +prop_readDoubleQuoted7 = isOk readSimpleCommand "echo \"${ ls;}bar\"" readDoubleQuoted = called "double quoted string" $ do id <- getNextId startPos <- getPosition doubleQuote x <- many doubleQuotedPart endPos <- getPosition - doubleQuote "end of double quoted string" + doubleQuote <|> fail "Expected end of double quoted string" optional $ do try . lookAhead $ suspectCharAfterQuotes <|> oneOf "$\"" when (any hasLineFeed x && not (startsWithLineFeed x)) $ @@ -1081,7 +1174,7 @@ readNormalDollar = readDollarExpression <|> readDollarDoubleQuote <|> readDollarSingleQuote <|> readDollarLonely readDoubleQuotedDollar = readDollarExpression <|> readDollarLonely -readDollarExpression = readDollarArithmetic <|> readDollarBracket <|> readDollarBraced <|> readDollarExpansion <|> readDollarVariable +readDollarExpression = readDollarArithmetic <|> readDollarBracket <|> readDollarBraceCommandExpansion <|> readDollarBraced <|> readDollarExpansion <|> readDollarVariable prop_readDollarSingleQuote = isOk readDollarSingleQuote "$'foo\\\'lol'" readDollarSingleQuote = called "$'..' expression" $ do @@ -1098,7 +1191,7 @@ char '$' doubleQuote x <- many doubleQuotedPart - doubleQuote "end of translated double quoted string" + doubleQuote <|> fail "Expected end of translated double quoted string" return $ T_DollarDoubleQuoted id x prop_readDollarArithmetic = isOk readDollarArithmetic "$(( 3 * 4 +5))" @@ -1125,6 +1218,18 @@ string "))" return (T_Arithmetic id c) +prop_readDollarBraceCommandExpansion1 = isOk readDollarBraceCommandExpansion "${ ls; }" +prop_readDollarBraceCommandExpansion2 = isOk readDollarBraceCommandExpansion "${\nls\n}" +readDollarBraceCommandExpansion = called "ksh ${ ..; } command expansion" $ do + id <- getNextId + try $ do + string "${" + whitespace + allspacing + term <- readTerm + char '}' <|> fail "Expected } to end the ksh ${ ..; } command expansion" + return $ T_DollarBraceCommandExpansion id term + prop_readDollarBraced1 = isOk readDollarBraced "${foo//bar/baz}" prop_readDollarBraced2 = isOk readDollarBraced "${foo/'{cow}'}" prop_readDollarBraced3 = isOk readDollarBraced "${foo%%$(echo cow\\})}" @@ -1143,7 +1248,7 @@ id <- getNextId try (string "$(") cmds <- readCompoundListOrEmpty - char ')' "end of $(..) expression" + char ')' <|> fail "Expected end of $(..) expression" return $ T_DollarExpansion id cmds prop_readDollarVariable = isOk readDollarVariable "$@" @@ -1202,6 +1307,7 @@ prop_readHereDoc5 = isOk readHereDoc "<<- !foo\nbar\n!foo" prop_readHereDoc6 = isOk readHereDoc "<< foo\\ bar\ncow\nfoo bar" prop_readHereDoc7 = isOk readHereDoc "<< foo\n\\$(f ())\nfoo" +prop_readHereDoc8 = isOk readHereDoc "<>bar\netc\nfoo" readHereDoc = called "here document" $ do fid <- getNextId pos <- getPosition @@ -1244,7 +1350,7 @@ liftM concat $ many1 (escaped <|> quoted <|> normal) where quoted = liftM stripLiteral readDoubleQuotedLiteral <|> readSingleQuotedLiteral - normal = anyChar `reluctantlyTill1` (whitespace <|> oneOf ";&)'\"\\") + normal = anyChar `reluctantlyTill1` (whitespace <|> oneOf "<>;&)'\"\\") escaped = do -- surely the user must be doing something wrong at this point char '\\' c <- anyChar @@ -1295,6 +1401,13 @@ file <- readFilename return $ T_FdRedirect id "" $ T_IoFile id op file +readIoVariable = try $ do + char '{' + x <- readVariableName + char '}' + lookAhead readIoFileOp + return $ "{" ++ x ++ "}" + readIoNumber = try $ do x <- many1 digit <|> string "&" lookAhead readIoFileOp @@ -1304,9 +1417,11 @@ prop_readIoNumberRedirect2 = isOk readIoNumberRedirect "2> lol" prop_readIoNumberRedirect3 = isOk readIoNumberRedirect "4>&-" prop_readIoNumberRedirect4 = isOk readIoNumberRedirect "&> lol" +prop_readIoNumberRedirect5 = isOk readIoNumberRedirect "{foo}>&2" +prop_readIoNumberRedirect6 = isOk readIoNumberRedirect "{foo}<&-" readIoNumberRedirect = do id <- getNextId - n <- readIoNumber + n <- readIoVariable <|> readIoNumber op <- readHereString <|> readHereDoc <|> readIoFile let actualOp = case op of T_FdRedirect _ "" x -> x spacing @@ -1374,7 +1489,6 @@ redirection (T_FdRedirect {}) = True redirection _ = False - prop_readSimpleCommand = isOk readSimpleCommand "echo test > file" prop_readSimpleCommand2 = isOk readSimpleCommand "cmd &> file" prop_readSimpleCommand3 = isOk readSimpleCommand "export foo=(bar baz)" @@ -1382,20 +1496,26 @@ prop_readSimpleCommand5 = isOk readSimpleCommand "time if true; then echo foo; fi" prop_readSimpleCommand6 = isOk readSimpleCommand "time -p ( ls -l; )" readSimpleCommand = called "simple command" $ do + pos <- getPosition id1 <- getNextId id2 <- getNextId prefix <- option [] readCmdPrefix cmd <- option Nothing $ do { f <- readCmdName; return $ Just f; } - when (null prefix && isNothing cmd) $ fail "No command" + when (null prefix && isNothing cmd) $ fail "Expected a command" case cmd of Nothing -> return $ makeSimpleCommand id1 id2 prefix [] [] Just cmd -> do suffix <- option [] $ getParser readCmdSuffix cmd [ (["declare", "export", "local", "readonly", "typeset"], readModifierSuffix), (["time"], readTimeSuffix), - (["let"], readLetSuffix) + (["let"], readLetSuffix), + (["eval"], readEvalSuffix) ] - return $ makeSimpleCommand id1 id2 prefix [cmd] suffix + + let result = makeSimpleCommand id1 id2 prefix [cmd] suffix + if isCommand ["source", "."] cmd + then readSource pos result + else return result where isCommand strings (T_NormalWord _ [T_Literal _ s]) = s `elem` strings isCommand _ _ = False @@ -1405,6 +1525,55 @@ then action else getParser def cmd rest + +readSource :: Monad m => SourcePos -> Token -> SCParser m Token +readSource pos t@(T_Redirecting _ _ (T_SimpleCommand _ _ (cmd:file:_))) = do + override <- getSourceOverride + let literalFile = override `mplus` getLiteralString file + case literalFile of + Nothing -> do + parseNoteAt pos WarningC 1090 + "Can't follow non-constant source. Use a directive to specify location." + return t + Just filename -> do + proceed <- shouldFollow filename + if not proceed + then do + parseNoteAt pos InfoC 1093 + "This file appears to be recursively sourced. Ignoring." + return t + else do + sys <- Mr.ask + input <- + if filename == "/dev/null" -- always allow /dev/null + then return (Right "") + else system $ siReadFile sys filename + case input of + Left err -> do + parseNoteAt pos InfoC 1091 $ + "Not following: " ++ err + return t + Right script -> do + id <- getNextIdAt pos + + let included = do + src <- subRead filename script + return $ T_Include id t src + + let failed = do + parseNoteAt pos WarningC 1094 + "Parsing of sourced file failed. Ignoring it." + return t + + included <|> failed + where + subRead name script = + withContext (ContextSource name) $ + inSeparateContext $ + subParse (initialPos name) readScript script +readSource _ t = return t + + prop_readPipeline = isOk readPipeline "! cat /etc/issue | grep -i ubuntu" prop_readPipeline2 = isWarning readPipeline "!cat /etc/issue | grep -i ubuntu" prop_readPipeline3 = isOk readPipeline "for f; do :; done|cat" @@ -1490,15 +1659,8 @@ readSimpleCommand ] -readCmdName = do - f <- readNormalWord - spacing - return f - -readCmdWord = do - f <- readNormalWord - spacing - return f +readCmdName = readCmdWord +readCmdWord = readNormalWord <* spacing prop_readIfClause = isOk readIfClause "if false; then foo; elif true; then stuff; more stuff; else cows; fi" prop_readIfClause2 = isWarning readIfClause "if false; then; echo oo; fi" @@ -1515,7 +1677,7 @@ g_Fi `orFail` do parseProblemAt pos ErrorC 1046 "Couldn't find 'fi' for this 'if'." parseProblem ErrorC 1047 "Expected 'fi' matching previously mentioned 'if'." - return "Expected 'fi'." + return "Expected 'fi'" return $ T_IfExpression id ((condition, action):elifs) elses @@ -1531,13 +1693,13 @@ allspacing condition <- readTerm - ifNextToken (g_Fi <|> g_Elif) $ + ifNextToken (g_Fi <|> g_Elif <|> g_Else) $ parseProblemAt pos ErrorC 1049 "Did you forget the 'then' for this 'if'?" called "then clause" $ do g_Then `orFail` do parseProblem ErrorC 1050 "Expected 'then'." - return "Expected 'then'." + return "Expected 'then'" acceptButWarn g_Semi ErrorC 1051 "No semicolons directly after 'then'." allspacing @@ -1554,7 +1716,7 @@ allspacing condition <- readTerm - ifNextToken (g_Fi <|> g_Elif) $ + ifNextToken (g_Fi <|> g_Elif <|> g_Else) $ parseProblemAt pos ErrorC 1049 "Did you forget the 'then' for this 'elif'?" g_Then @@ -1587,7 +1749,7 @@ allspacing list <- readCompoundList allspacing - char ')' + char ')' <|> fail ") closing the subshell" return $ T_Subshell id list prop_readBraceGroup = isOk readBraceGroup "{ a; b | c | d; e; }" @@ -1630,7 +1792,7 @@ g_Do `orFail` do parseProblem ErrorC 1058 "Expected 'do'." - return "Expected 'do'." + return "Expected 'do'" acceptButWarn g_Semi ErrorC 1059 "No semicolons directly after 'do'." allspacing @@ -1643,7 +1805,7 @@ g_Done `orFail` do parseProblemAt pos ErrorC 1061 "Couldn't find 'done' for this 'do'." parseProblem ErrorC 1062 "Expected 'done' matching previously mentioned 'do'." - return "Expected 'done'." + return "Expected 'done'" return commands @@ -1730,10 +1892,10 @@ g_Case word <- readNormalWord allspacing - g_In + g_In <|> fail "Expected 'in'" readLineBreak list <- readCaseList - g_Esac + g_Esac <|> fail "Expected 'esac' to close the case statement" return $ T_CaseExpression id word list readCaseList = many readCaseItem @@ -1788,20 +1950,20 @@ whitespace spacing name <- readFunctionName - optional spacing + spacing hasParens <- wasIncluded readParens return $ T_Function id (FunctionKeyword True) (FunctionParentheses hasParens) name readWithoutFunction = try $ do id <- getNextId name <- readFunctionName - optional spacing + spacing readParens return $ T_Function id (FunctionKeyword False) (FunctionParentheses True) name readParens = do g_Lparen - optional spacing + spacing g_Rparen <|> do parseProblem ErrorC 1065 "Trying to declare parameters? Don't. Use () and refer to params as $1, $2.." many $ noneOf "\n){" @@ -1840,7 +2002,7 @@ readCompoundCommand = do id <- getNextId cmd <- choice [ readBraceGroup, readArithmeticExpression, readSubshell, readCondition, readWhileClause, readUntilClause, readIfClause, readForClause, readSelectClause, readCaseClause, readFunctionDefinition] - optional spacing + spacing redirs <- many readIoRedirect unless (null redirs) $ optional $ do lookAhead $ try (spacing >> needsSeparator) @@ -1876,6 +2038,15 @@ expression <- readStringForParser readCmdWord subParse startPos readArithmeticContents expression +-- bash allows a=(b), ksh allows $a=(b). dash allows neither. Let's warn. +readEvalSuffix = many1 (readIoRedirect <|> readCmdWord <|> evalFallback) + where + evalFallback = do + pos <- getPosition + lookAhead $ char '(' + parseProblemAt pos WarningC 1098 "Quote/escape special characters when using eval, e.g. eval \"a=(b)\"." + fail "Unexpected parentheses. Make sure to quote when eval'ing as shell parsers differ." + -- Get whatever a parser would parse as a string readStringForParser parser = do pos <- lookAhead (parser >> getPosition) @@ -1923,8 +2094,19 @@ spacing return $ T_Assignment id op variable index value where - readAssignmentOp = - (string "+=" >> return Append) <|> (string "=" >> return Assign) + readAssignmentOp = do + pos <- getPosition + unexpecting "" $ string "===" + choice [ + string "+=" >> return Append, + do + try (string "==") + parseProblemAt pos ErrorC 1097 + "Unexpected ==. For assignment, use =. For comparison, use [/[[." + return Assign, + + string "=" >> return Assign + ] readEmptyLiteral = do id <- getNextId return $ T_Literal id "" @@ -1941,7 +2123,7 @@ char '(' allspacing words <- readElement `reluctantlyTill` char ')' - char ')' + char ')' <|> fail "Expected ) to close array assignment" return $ T_Array id words where readElement = (readIndexed <|> readRegular) `thenSkip` allspacing @@ -1975,17 +2157,22 @@ tryParseWordToken keyword t = try $ do id <- getNextId str <- anycaseString keyword - optional (do + + optional $ do try . lookAhead $ char '[' - parseProblem ErrorC 1069 "You need a space before the [.") + parseProblem ErrorC 1069 "You need a space before the [." + optional $ do + try . lookAhead $ char '#' + parseProblem ErrorC 1099 "You need a space before the #." + try $ lookAhead keywordSeparator when (str /= keyword) $ parseProblem ErrorC 1081 $ "Scripts are case sensitive. Use '" ++ keyword ++ "', not '" ++ str ++ "'." return $ t id -anycaseString str = - mapM anycaseChar str str +anycaseString = + mapM anycaseChar where anycaseChar c = char (toLower c) <|> char (toUpper c) @@ -2016,7 +2203,10 @@ g_Select = tryWordToken "select" T_Select g_In = tryWordToken "in" T_In g_Lbrace = tryWordToken "{" T_Lbrace -g_Rbrace = tryWordToken "}" T_Rbrace +g_Rbrace = do -- handled specially due to ksh echo "${ foo; }bar" + id <- getNextId + char '}' + return $ T_Rbrace id g_Lparen = tryToken "(" T_Lparen g_Rparen = tryToken ")" T_Rparen @@ -2141,16 +2331,22 @@ readUtf8Bom = called "Byte Order Mark" $ string "\xFEFF" -rp p filename contents = Ms.runState (runParserT p initialState filename contents) ([], []) -isWarning p s = fst cs && (not . null . snd $ cs) where cs = checkString p s -isOk p s = fst cs && (null . snd $ cs) where cs = checkString p s +isWarning p s = parsesCleanly p s == Just False +isOk p s = parsesCleanly p s == Just True -checkString parser string = - case rp (parser >> eof >> getState) "-" string of - (Right (tree, map, notes), (problems, _)) -> (True, notes ++ problems) - (Left _, (n, _)) -> (False, n) +testParse string = runIdentity $ do + (res, _) <- runParser (mockedSystemInterface []) readScript "-" string + return res +parsesCleanly parser string = runIdentity $ do + (res, sys) <- runParser (mockedSystemInterface []) + (parser >> eof >> getState) "-" string + case (res, sys) of + (Right userState, systemState) -> + return $ Just . null $ parseNotes userState ++ parseProblems systemState + (Left _, _) -> return Nothing + parseWithNotes parser = do item <- parser map <- getMap @@ -2161,8 +2357,6 @@ sortNotes = sortBy compareNotes -data ParseResult = ParseResult { parseResult :: Maybe (Token, Map.Map Id SourcePos), parseNotes :: [ParseNote] } deriving (Show) - makeErrorFor parsecError = ParseNote (errorPos parsecError) ErrorC 1072 $ getStringFromParsec $ errorMessages parsecError @@ -2174,19 +2368,45 @@ where f err = case err of - UnExpect s -> return $ unexpected s - SysUnExpect s -> return $ unexpected s - Expect s -> return $ "Expected " ++ s ++ "." + UnExpect s -> Nothing -- Due to not knowing Parsec, none of these + SysUnExpect s -> Nothing -- are actually helpful. has been hidden + Expect s -> Nothing -- and we only show explicit fail statements. Message s -> if null s then Nothing else return $ s ++ "." - unexpected s = "Unexpected " ++ (if null s then "eof" else s) ++ "." -parseShell options filename contents = - case rp (parseWithNotes readScript) filename contents of - (Right (script, map, notes), (parsenotes, _)) -> - ParseResult (Just (script, map)) (nub . sortNotes . excludeNotes $ notes ++ parsenotes) - (Left err, (p, context)) -> - ParseResult Nothing - (nub . sortNotes . excludeNotes $ p ++ notesForContext context ++ [makeErrorFor err]) +runParser :: Monad m => + SystemInterface m -> + SCParser m v -> + String -> + String -> + m (Either ParseError v, SystemState) + +runParser sys p filename contents = + Ms.runStateT + (Mr.runReaderT + (runParserT p initialUserState filename contents) + sys) + initialSystemState +system = lift . lift . lift + +parseShell sys name contents = do + (result, state) <- runParser sys (parseWithNotes readScript) name contents + case result of + Right (script, tokenMap, notes) -> + return ParseResult { + prComments = map toPositionedComment $ nub $ notes ++ parseProblems state, + prTokenPositions = Map.map posToPos tokenMap, + prRoot = Just script + } + Left err -> + return ParseResult { + prComments = + map toPositionedComment $ + notesForContext (contextStack state) + ++ [makeErrorFor err] + ++ parseProblems state, + prTokenPositions = Map.empty, + prRoot = Nothing + } where isName (ContextName _ _) = True isName _ = False @@ -2195,7 +2415,25 @@ "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) + + +toPositionedComment :: ParseNote -> PositionedComment +toPositionedComment (ParseNote pos severity code message) = + PositionedComment (posToPos pos) $ Comment severity code message + +posToPos :: SourcePos -> Position +posToPos sp = Position { + posFile = sourceName sp, + posLine = fromIntegral $ sourceLine sp, + posColumn = fromIntegral $ sourceColumn sp +} + +-- TODO: Clean up crusty old code that this is layered on top of +parseScript :: Monad m => + SystemInterface m -> ParseSpec -> m ParseResult +parseScript sys spec = + parseShell sys (psFilename spec) (psScript spec) + lt x = trace (show x) x ltt t = trace (show t) diff --git a/ShellCheck/Regex.hs b/ShellCheck/Regex.hs --- a/ShellCheck/Regex.hs +++ b/ShellCheck/Regex.hs @@ -1,4 +1,6 @@ {- + Copyright 2012-2015 Vidar Holen + This file is part of ShellCheck. http://www.vidarholen.net/contents/shellcheck @@ -69,3 +71,10 @@ (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 + +-- Split a string based on a regex. +splitOn :: String -> Regex -> [String] +splitOn input re = + case matchM re input :: Maybe (String, String, String) of + Just (before, match, after) -> before : after `splitOn` re + Nothing -> [input] diff --git a/ShellCheck/Simple.hs b/ShellCheck/Simple.hs deleted file mode 100644 --- a/ShellCheck/Simple.hs +++ /dev/null @@ -1,80 +0,0 @@ -{- - 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 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 General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program. If not, see . --} -{-# LANGUAGE TemplateHaskell #-} -module ShellCheck.Simple (shellCheck, ShellCheckComment, scLine, scColumn, scSeverity, scCode, scMessage, runTests) where - -import Data.List -import Data.Maybe -import ShellCheck.Analytics hiding (runTests) -import ShellCheck.Options -import ShellCheck.Parser hiding (runTests) -import Test.QuickCheck.All (quickCheckAll) -import Text.Parsec.Pos - -shellCheck :: AnalysisOptions -> String -> [ShellCheckComment] -shellCheck options script = - let (ParseResult result notes) = parseShell options "-" script in - let allNotes = notes ++ concat (maybeToList $ do - (tree, posMap) <- result - let list = runAnalytics options tree - return $ map (noteToParseNote posMap) $ filterByAnnotation tree list - ) - in - map formatNote $ nub $ sortNotes allNotes - -data ShellCheckComment = ShellCheckComment { scLine :: Int, scColumn :: Int, scSeverity :: String, scCode :: Int, scMessage :: String } - -instance Show ShellCheckComment where - show c = concat ["(", show $ scLine c, ",", show $ scColumn c, ") ", scSeverity c, ": ", show (scCode c), " ", scMessage c] - -severityToString s = - case s of - ErrorC -> "error" - WarningC -> "warning" - InfoC -> "info" - StyleC -> "style" - -formatNote (ParseNote pos severity code text) = - ShellCheckComment (sourceLine pos) (sourceColumn pos) (severityToString severity) (fromIntegral code) text - -testCheck = shellCheck defaultAnalysisOptions { optionExcludes = [2148] } -- Ignore #! warnings -prop_findsParseIssue = - let comments = testCheck "echo \"$12\"" in - length comments == 1 && scCode (head comments) == 1037 -prop_commentDisablesParseIssue1 = - null $ testCheck "#shellcheck disable=SC1037\necho \"$12\"" -prop_commentDisablesParseIssue2 = - null $ testCheck "#shellcheck disable=SC1037\n#lol\necho \"$12\"" - -prop_findsAnalysisIssue = - let comments = testCheck "echo $1" in - length comments == 1 && scCode (head comments) == 2086 -prop_commentDisablesAnalysisIssue1 = - null $ testCheck "#shellcheck disable=SC2086\necho $1" -prop_commentDisablesAnalysisIssue2 = - null $ testCheck "#shellcheck disable=SC2086\n#lol\necho $1" - -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 --- a/shellcheck.1 +++ b/shellcheck.1 @@ -58,10 +58,19 @@ .RS .RE .TP -.B \f[B]-V\f[]\ \f[I]version\f[],\ \f[B]--version\f[] -Print version and exit. +.B \f[B]-V\f[],\ \f[B]--version\f[] +Print version information and exit. .RS .RE +.TP +.B \f[B]-x\f[],\ \f[B]-external-sources\f[] +Follow \[aq]source\[aq] statements even when the file is not specified +as input. +By default, \f[C]shellcheck\f[] will only follow files specified on the +command line (plus \f[C]/dev/null\f[]). +This option allows following any file the script may \f[C]source\f[]. +.RS +.RE .SH FORMATS .TP .B \f[B]tty\f[] @@ -154,10 +163,21 @@ \f[] .fi .PP -Here a shell brace group is used to suppress on multiple lines: +To tell ShellCheck where to look for an otherwise dynamically determined +file: .IP .nf \f[C] +#\ shellcheck\ source=./lib.sh +source\ "$(find_install_dir)/lib.sh" +\f[] +.fi +.PP +Here a shell brace group is used to suppress a warning on multiple +lines: +.IP +.nf +\f[C] #\ shellcheck\ disable=SC2016 { \ \ echo\ \[aq]Modifying\ $PATH\[aq] @@ -175,6 +195,28 @@ compound command like a function definition, subshell block or loop. .RS .RE +.TP +.B \f[B]source\f[] +Overrides the filename included by a \f[C]source\f[]/\f[C]\&.\f[] +statement. +This can be used to tell shellcheck where to look for a file whose name +is determined at runtime, or to skip a source by telling it to use +\f[C]/dev/null\f[]. +.RS +.RE +.SH ENVIRONMENT VARIABLES +.PP +The environment variable \f[C]SHELLCHECK_OPTS\f[] can be set with +default flags: +.IP +.nf +\f[C] +export\ SHELLCHECK_OPTS=\[aq]--shell=bash\ --exclude=SC2016\[aq] +\f[] +.fi +.PP +Its value will be split on spaces and prepended to the command line on +each invocation. .SH AUTHOR .PP ShellCheck is written and maintained by Vidar Holen. @@ -183,6 +225,11 @@ Bugs and issues can be reported on GitHub: .PP https://github.com/koalaman/shellcheck/issues +.SH COPYRIGHT +.PP +Copyright 2012-2015, Vidar Holen. +Licensed under the GNU General Public License version 3 or later, see +http://gnu.org/licenses/gpl.html .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 @@ -50,10 +50,17 @@ The default is to use the file's shebang, or *bash* if the target shell can't be determined. -**-V**\ *version*,\ **--version** +**-V**,\ **--version** -: Print version and exit. +: Print version information and exit. +**-x**,\ **-external-sources** + +: Follow 'source' statements even when the file is not specified as input. + By default, `shellcheck` will only follow files specified on the command + line (plus `/dev/null`). This option allows following any file the script + may `source`. + # FORMATS **tty** @@ -119,8 +126,13 @@ # shellcheck disable=SC2035 echo "Files: " *.jpg -Here a shell brace group is used to suppress on multiple lines: +To tell ShellCheck where to look for an otherwise dynamically determined file: + # shellcheck source=./lib.sh + source "$(find_install_dir)/lib.sh" + +Here a shell brace group is used to suppress a warning on multiple lines: + # shellcheck disable=SC2016 { echo 'Modifying $PATH' @@ -134,7 +146,19 @@ The command can be a simple command like `echo foo`, or a compound command like a function definition, subshell block or loop. +**source** +: Overrides the filename included by a `source`/`.` statement. This can be + used to tell shellcheck where to look for a file whose name is determined + at runtime, or to skip a source by telling it to use `/dev/null`. +# ENVIRONMENT VARIABLES +The environment variable `SHELLCHECK_OPTS` can be set with default flags: + + export SHELLCHECK_OPTS='--shell=bash --exclude=SC2016' + +Its value will be split on spaces and prepended to the command line on each +invocation. + # AUTHOR ShellCheck is written and maintained by Vidar Holen. @@ -142,6 +166,12 @@ Bugs and issues can be reported on GitHub: https://github.com/koalaman/shellcheck/issues + +# COPYRIGHT +Copyright 2012-2015, Vidar Holen. +Licensed under the GNU General Public License version 3 or later, +see http://gnu.org/licenses/gpl.html + # SEE ALSO diff --git a/shellcheck.hs b/shellcheck.hs --- a/shellcheck.hs +++ b/shellcheck.hs @@ -1,4 +1,6 @@ {- + Copyright 2012-2015 Vidar Holen + This file is part of ShellCheck. http://www.vidarholen.net/contents/shellcheck @@ -15,44 +17,58 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . -} +import ShellCheck.Data +import ShellCheck.Checker +import ShellCheck.Interface +import ShellCheck.Regex + +import ShellCheck.Formatter.Format +import qualified ShellCheck.Formatter.CheckStyle +import qualified ShellCheck.Formatter.GCC +import qualified ShellCheck.Formatter.JSON +import qualified ShellCheck.Formatter.TTY + import Control.Exception import Control.Monad -import Control.Monad.Trans -import Control.Monad.Trans.Error -import Control.Monad.Trans.List +import Control.Monad.Except import Data.Char -import Data.List +import Data.Functor +import Data.Either +import qualified Data.Map as Map import Data.Maybe import Data.Monoid -import GHC.Exts -import GHC.IO.Device import Prelude hiding (catch) -import ShellCheck.Data -import ShellCheck.Options -import ShellCheck.Simple -import ShellCheck.Analytics import System.Console.GetOpt import System.Directory import System.Environment import System.Exit -import System.Info import System.IO -import Text.JSON -import qualified Data.Map as Map 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 +data Status = + NoProblems + | SomeProblems + | BadInput + | SupportFailure + | SyntaxFailure + | RuntimeException + deriving (Ord, Eq, Show) instance Monoid Status where mempty = NoProblems mappend = max -header = "Usage: shellcheck [OPTIONS...] FILES..." +data Options = Options { + checkSpec :: CheckSpec, + externalSources :: Bool +} + +defaultOptions = Options { + checkSpec = emptyCheckSpec, + externalSources = False +} + +usageHeader = "Usage: shellcheck [OPTIONS...] FILES..." options = [ Option "e" ["exclude"] (ReqArg (Flag "exclude") "CODE1,CODE2..") "exclude types of warnings", @@ -60,204 +76,30 @@ (ReqArg (Flag "format") "FORMAT") "output format", Option "s" ["shell"] (ReqArg (Flag "shell") "SHELLNAME") "Specify dialect (bash,sh,ksh)", + Option "x" ["external-sources"] + (NoArg $ Flag "externals" "true") "Allow 'source' outside of FILES.", Option "V" ["version"] (NoArg $ Flag "version" "true") "Print version information" ] -printErr = hPutStrLn stderr - - -instance JSON (JsonComment) where - showJSON (JsonComment filename c) = makeObj [ - ("file", showJSON $ filename), - ("line", showJSON $ scLine c), - ("column", showJSON $ scColumn c), - ("level", showJSON $ scSeverity c), - ("code", showJSON $ scCode c), - ("message", showJSON $ scMessage c) - ] - readJSON = undefined +printErr = lift . hPutStrLn stderr -parseArguments :: [String] -> ErrorT Status IO ([Flag], [FilePath]) +parseArguments :: [String] -> ExceptT Status IO ([Flag], [FilePath]) parseArguments argv = case getOpt Permute options argv of (opts, files, []) -> return (opts, files) (_, _, errors) -> do - liftIO . printErr $ concat errors ++ "\n" ++ usageInfo header options + printErr $ concat errors ++ "\n" ++ usageInfo usageHeader options throwError SyntaxFailure -formats :: Map.Map String (AnalysisOptions -> [FilePath] -> IO Status) +formats :: Map.Map String (IO Formatter) formats = Map.fromList [ - ("json", forJson), - ("gcc", forGcc), - ("checkstyle", forCheckstyle), - ("tty", forTty) + ("checkstyle", ShellCheck.Formatter.CheckStyle.format), + ("gcc", ShellCheck.Formatter.GCC.format), + ("json", ShellCheck.Formatter.JSON.format), + ("tty", ShellCheck.Formatter.TTY.format) ] -toStatus = liftM (either id (const NoProblems)) . runErrorT - -catchExceptions :: IO Status -> IO Status -catchExceptions action = action -- action `catch` handler - where - handler err = do - printErr $ show (err :: SomeException) - return RuntimeException - -checkComments comments = if null comments then NoProblems else SomeProblems - -forTty :: AnalysisOptions -> [FilePath] -> IO Status -forTty options files = do - output <- mapM doFile files - return $ mconcat output - where - clear = ansi 0 - ansi n = "\x1B[" ++ show n ++ "m" - - colorForLevel "error" = 31 -- red - colorForLevel "warning" = 33 -- yellow - colorForLevel "info" = 32 -- green - colorForLevel "style" = 32 -- green - colorForLevel "message" = 1 -- bold - colorForLevel "source" = 0 -- none - colorForLevel _ = 0 -- none - - colorComment level comment = - ansi (colorForLevel level) ++ comment ++ clear - - doFile path = catchExceptions $ do - contents <- readContents path - doInput path contents - - doInput filename contents = do - let fileLines = lines contents - let lineCount = length fileLines - let comments = getComments options contents - let groups = groupWith scLine comments - colorFunc <- getColorFunc - mapM_ (\x -> do - let lineNum = scLine (head x) - let line = if lineNum < 1 || lineNum > lineCount - then "" - else fileLines !! (lineNum - 1) - putStrLn "" - putStrLn $ colorFunc "message" - ("In " ++ filename ++" line " ++ show lineNum ++ ":") - putStrLn (colorFunc "source" line) - mapM_ (\c -> putStrLn (colorFunc (scSeverity c) $ cuteIndent c)) x - putStrLn "" - ) groups - return . checkComments $ comments - - cuteIndent comment = - replicate (scColumn comment - 1) ' ' ++ - "^-- " ++ code (scCode comment) ++ ": " ++ scMessage comment - - code code = "SC" ++ show code - - getColorFunc = do - term <- hIsTerminalDevice stdout - let windows = "mingw" `isPrefixOf` os - return $ if term && not windows then colorComment else const id - -forJson :: AnalysisOptions -> [FilePath] -> IO Status -forJson options files = catchExceptions $ do - comments <- runListT $ do - file <- ListT $ return files - comment <- ListT $ commentsFor options file - return $ JsonComment file comment - putStrLn $ encodeStrict comments - return $ checkComments comments - --- Mimic GCC "file:line:col: (error|warning|note): message" format -forGcc :: AnalysisOptions -> [FilePath] -> IO Status -forGcc options files = do - files <- mapM process files - return $ mconcat files - where - process file = catchExceptions $ do - contents <- readContents file - let comments = makeNonVirtual (getComments options contents) contents - mapM_ (putStrLn . format file) comments - return $ checkComments comments - - format filename c = concat [ - filename, ":", - show $ scLine c, ":", - show $ scColumn c, ": ", - case scSeverity c of - "error" -> "error" - "warning" -> "warning" - _ -> "note", - ": ", - concat . lines $ scMessage c, - " [SC", show $ scCode c, "]" - ] - --- Checkstyle compatible output. A bit of a hack to avoid XML dependencies -forCheckstyle :: AnalysisOptions -> [FilePath] -> IO Status -forCheckstyle options files = do - putStrLn "" - putStrLn "" - statuses <- mapM process files - putStrLn "" - return $ mconcat statuses - where - process file = catchExceptions $ do - comments <- commentsFor options file - putStrLn (formatFile file comments) - return $ checkComments comments - - severity "error" = "error" - severity "warning" = "warning" - severity _ = "info" - attr s v = concat [ s, "='", escape v, "' " ] - escape = concatMap escape' - escape' c = if isOk c then [c] else "&#" ++ show (ord c) ++ ";" - isOk x = any ($x) [isAsciiUpper, isAsciiLower, isDigit, (`elem` " ./")] - - formatFile name comments = concat [ - "\n", - concatMap format comments, - "" - ] - - format c = concat [ - "\n" - ] - -commentsFor options file = liftM (getComments options) $ readContents file - -getComments = shellCheck - -readContents :: FilePath -> IO String -readContents file = - if file == "-" - then getContents - else readFile file - --- Realign comments from a tabstop of 8 to 1 -makeNonVirtual comments contents = - map fix comments - where - ls = lines contents - fix c = c { - scColumn = - if scLine c > 0 && scLine c <= length ls - then real (ls !! (scLine c - 1)) 0 0 (scColumn c) - else scColumn c - } - real _ r v target | target <= v = r - real [] r v _ = r -- should never happen - real ('\t':rest) r v target = - real rest (r+1) (v + 8 - (v `mod` 8)) target - real (_:rest) r v target = real rest (r+1) (v+1) target - getOption [] _ = Nothing getOption (Flag var val:_) name | name == var = return val getOption (_:rest) flag = getOption rest flag @@ -280,13 +122,19 @@ in map (Prelude.read . clean) elements :: [Int] -excludeCodes codes = - filter (not . hasCode) +toStatus = liftM (either id id) . runExceptT + +getEnvArgs = do + opts <- getEnv "SHELLCHECK_OPTS" `catch` cantWaitForLookupEnv + return . filter (not . null) $ opts `splitOn` mkRegex " +" where - hasCode c = scCode c `elem` codes + cantWaitForLookupEnv :: IOException -> IO String + cantWaitForLookupEnv = const $ return "" main = do - args <- getArgs + params <- getArgs + envOpts <- getEnvArgs + let args = envOpts ++ params status <- toStatus $ do (flags, files) <- parseArguments args process flags files @@ -301,53 +149,137 @@ SupportFailure -> ExitFailure 4 RuntimeException -> ExitFailure 2 -process :: [Flag] -> [FilePath] -> ErrorT Status IO () +process :: [Flag] -> [FilePath] -> ExceptT Status IO Status process flags files = do - options <- foldM (flip parseOption) defaultAnalysisOptions flags + options <- foldM (flip parseOption) defaultOptions flags verifyFiles files let format = fromMaybe "tty" $ getOption flags "format" - case Map.lookup format formats of - Nothing -> do - liftIO $ do + formatter <- + case Map.lookup format formats of + Nothing -> do printErr $ "Unknown format " ++ format printErr "Supported formats:" mapM_ (printErr . write) $ Map.keys formats - throwError SupportFailure - where write s = " " ++ s - Just f -> ErrorT $ liftM Left $ f options files + throwError SupportFailure + where write s = " " ++ s + Just f -> ExceptT $ fmap Right f + sys <- lift $ ioInterface options files + lift $ runFormatter sys formatter options files +runFormatter :: SystemInterface IO -> Formatter -> Options -> [FilePath] + -> IO Status +runFormatter sys format options files = do + header format + result <- foldM f NoProblems files + footer format + return result + where + f :: Status -> FilePath -> IO Status + f status file = do + newStatus <- process file `catch` handler file + return $ status `mappend` newStatus + handler :: FilePath -> IOException -> IO Status + handler file e = do + onFailure format file (show e) + return RuntimeException + + process :: FilePath -> IO Status + process filename = do + contents <- inputFile filename + let checkspec = (checkSpec options) { + csFilename = filename, + csScript = contents + } + result <- checkScript sys checkspec + onResult format result contents + return $ + if null (crComments result) + then NoProblems + else SomeProblems + parseOption flag options = case flag of Flag "shell" str -> - fromMaybe (die $ "Unknown shell: " ++ str) $ do - shell <- shellForExecutable str - return $ return options { optionShellType = Just shell } + fromMaybe (die $ "Unknown shell: " ++ str) $ do + shell <- shellForExecutable str + return $ return options { + checkSpec = (checkSpec options) { + csShellTypeOverride = Just shell + } + } Flag "exclude" str -> do new <- mapM parseNum $ split ',' str - let old = optionExcludes options - return options { optionExcludes = new ++ old } + let old = csExcludedWarnings . checkSpec $ options + return options { + checkSpec = (checkSpec options) { + csExcludedWarnings = new ++ old + } + } Flag "version" _ -> do liftIO printVersion throwError NoProblems + Flag "externals" _ -> do + return options { + externalSources = True + } + _ -> return options where die s = do - liftIO $ printErr s + printErr s throwError SupportFailure parseNum ('S':'C':str) = parseNum str parseNum num = do unless (all isDigit num) $ do - liftIO . printErr $ "Bad exclusion: " ++ num + printErr $ "Bad exclusion: " ++ num throwError SyntaxFailure return (Prelude.read num :: Integer) +ioInterface options files = do + inputs <- mapM normalize files + return SystemInterface { + siReadFile = get inputs + } + where + get inputs file = do + ok <- allowable inputs file + if ok + then (Right <$> inputFile file) `catch` handler + else return $ Left (file ++ " was not specified as input (see shellcheck -x).") + + where + handler :: IOException -> IO (Either ErrorMessage String) + handler ex = return . Left $ show ex + + allowable inputs x = + if externalSources options + then return True + else do + path <- normalize x + return $ path `elem` inputs + + normalize x = + canonicalizePath x `catch` fallback x + where + fallback :: FilePath -> IOException -> IO FilePath + fallback path _ = return path + +inputFile file = do + contents <- + if file == "-" + then getContents + else readFile file + + seq (length contents) $ + return contents + verifyFiles files = when (null files) $ do - liftIO $ printErr "No files specified.\n" - liftIO $ printErr $ usageInfo header options + printErr "No files specified.\n" + printErr $ usageInfo usageHeader options throwError SyntaxFailure printVersion = do diff --git a/test/shellcheck.hs b/test/shellcheck.hs --- a/test/shellcheck.hs +++ b/test/shellcheck.hs @@ -2,15 +2,17 @@ import Control.Monad import System.Exit -import qualified ShellCheck.Simple +import qualified ShellCheck.Checker import qualified ShellCheck.Analytics import qualified ShellCheck.Parser main = do putStrLn "Running ShellCheck tests..." - results <- sequence [ShellCheck.Simple.runTests, - ShellCheck.Analytics.runTests, - ShellCheck.Parser.runTests] - if and results then exitSuccess - else exitFailure - + results <- sequence [ + ShellCheck.Checker.runTests, + ShellCheck.Analytics.runTests, + ShellCheck.Parser.runTests + ] + if and results + then exitSuccess + else exitFailure