From 86999ded1fc2528a7eea191fa77fe81a03f65f2a Mon Sep 17 00:00:00 2001 From: Vidar Holen Date: Sat, 30 Apr 2016 13:45:39 -0700 Subject: [PATCH] Improve 'let' parsing, trigger unused var for ((a=1)) --- ShellCheck/AST.hs | 3 +++ ShellCheck/Analytics.hs | 5 ++++- ShellCheck/AnalyzerLib.hs | 17 +++++++++++------ ShellCheck/Parser.hs | 24 +++++++++++++++++++++--- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/ShellCheck/AST.hs b/ShellCheck/AST.hs index 6aee45d..896d6d0 100644 --- a/ShellCheck/AST.hs +++ b/ShellCheck/AST.hs @@ -34,6 +34,7 @@ data CaseType = CaseBreak | CaseFallThrough | CaseContinue deriving (Show, Eq) data Token = TA_Binary Id String Token Token + | TA_Assignment Id String Token Token | TA_Expansion Id [Token] | TA_Index Id Token | TA_Sequence Id [Token] @@ -250,6 +251,7 @@ analyze f g i = delve (TC_Noary id typ token) = d1 token $ TC_Noary id typ delve (TA_Binary id op t1 t2) = d2 t1 t2 $ TA_Binary id op + delve (TA_Assignment id op t1 t2) = d2 t1 t2 $ TA_Assignment id op delve (TA_Unary id op t1) = d1 t1 $ TA_Unary id op delve (TA_Sequence id l) = dl l $ TA_Sequence id delve (TA_Trinary id t1 t2 t3) = do @@ -344,6 +346,7 @@ getId t = case t of TC_Unary id _ _ _ -> id TC_Noary id _ _ -> id TA_Binary id _ _ _ -> id + TA_Assignment id _ _ _ -> id TA_Unary id _ _ -> id TA_Sequence id _ -> id TA_Trinary id _ _ _ -> id diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs index 4fd72b5..bed7b39 100644 --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -2054,6 +2054,9 @@ prop_checkUnused26= verifyNotTree checkUnusedAssignments "declare -F foo" prop_checkUnused27= verifyTree checkUnusedAssignments "var=3; [ var -eq 3 ]" prop_checkUnused28= verifyNotTree checkUnusedAssignments "var=3; [[ var -eq 3 ]]" prop_checkUnused29= verifyNotTree checkUnusedAssignments "var=(a b); declare -p var" +prop_checkUnused30= verifyTree checkUnusedAssignments "let a=1" +prop_checkUnused31= verifyTree checkUnusedAssignments "let 'a=1'" +prop_checkUnused32= verifyTree checkUnusedAssignments "let a=b=c; echo $a" checkUnusedAssignments params t = execWriter (mapM_ warnFor unused) where flow = variableFlow params @@ -2742,7 +2745,7 @@ checkLoopVariableReassignment params token = T_ForIn _ s _ _ -> return s T_ForArithmetic _ (TA_Sequence _ - [TA_Binary _ "=" + [TA_Assignment _ "=" (TA_Expansion _ [T_Literal _ var]) _]) _ _ _ -> return var _ -> fail "not loop" diff --git a/ShellCheck/AnalyzerLib.hs b/ShellCheck/AnalyzerLib.hs index fcf46bb..6f02040 100644 --- a/ShellCheck/AnalyzerLib.hs +++ b/ShellCheck/AnalyzerLib.hs @@ -40,8 +40,6 @@ import qualified Data.Map as Map import Test.QuickCheck.All (forAllProperties) import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) -import Debug.Trace - type Analysis = ReaderT Parameters (Writer [TokenComment]) () @@ -272,7 +270,7 @@ getVariableFlow shell parents t = assignFirst _ = False setRead t = - let read = getReferencedVariables t + let read = getReferencedVariables parents t in mapM_ (\v -> modify (Reference v:)) read setWritten t = @@ -331,7 +329,7 @@ getModifiedVariables t = TA_Unary _ "|++" var -> maybeToList $ do name <- getLiteralString var return (t, t, name, DataString $ SourceFrom [t]) - TA_Binary _ op lhs rhs -> maybeToList $ do + TA_Assignment _ op lhs rhs -> maybeToList $ do guard $ op `elem` ["=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", "&=", "^=", "|="] name <- getLiteralString lhs return (t, t, name, DataString $ SourceFrom [rhs]) @@ -476,12 +474,15 @@ getIndexReferences s = fromMaybe [] $ do where re = mkRegex "(\\[.*\\])" -getReferencedVariables t = +getReferencedVariables parents t = case t of 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 + TA_Expansion id _ -> + if isArithmeticAssignment t + then [] + else getIfReference t t T_Assignment id mode str _ word -> [(t, t, str) | mode == Append] ++ specialReferences str t word @@ -518,6 +519,10 @@ getReferencedVariables t = isDereferencing = (`elem` ["-eq", "-ne", "-lt", "-le", "-gt", "-ge"]) + isArithmeticAssignment t = case getPath parents t of + this: TA_Assignment _ "=" _ _ :_ -> True + _ -> False + dataTypeFrom defaultType v = (case v of T_Array {} -> DataArray; _ -> defaultType) $ SourceFrom [v] diff --git a/ShellCheck/Parser.hs b/ShellCheck/Parser.hs index e58c16f..92c4083 100644 --- a/ShellCheck/Parser.hs +++ b/ShellCheck/Parser.hs @@ -708,7 +708,9 @@ readArithmeticContents = l <- readAssignment `sepBy` (char ',' >> spacing) return $ TA_Sequence id l - readAssignment = readTrinary `splitBy` ["=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", "&=", "^=", "|="] + readAssignment = chainr1 readTrinary readAssignmentOp + readAssignmentOp = readComboOp ["=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", "&=", "^=", "|="] TA_Assignment + readTrinary = do x <- readLogicalOr do @@ -2214,13 +2216,29 @@ readTimeSuffix = do lookAhead $ char '-' readCmdWord --- Fixme: this is a hack that doesn't handle let '++c' or let a\>b +-- Fixme: this is a hack that doesn't handle let c='4'"5" or let a\>b +readLetSuffix :: Monad m => SCParser m [Token] readLetSuffix = many1 (readIoRedirect <|> try readLetExpression <|> readCmdWord) where + readLetExpression :: Monad m => SCParser m Token readLetExpression = do startPos <- getPosition expression <- readStringForParser readCmdWord - subParse startPos readArithmeticContents expression + let (unQuoted, newPos) = kludgeAwayQuotes expression startPos + subParse newPos readArithmeticContents unQuoted + + kludgeAwayQuotes :: String -> SourcePos -> (String, SourcePos) + kludgeAwayQuotes s p = + case s of + first:rest@(_:_) -> + let (last:backwards) = reverse rest + middle = reverse backwards + in + if first `elem` "'\"" && first == last + then (middle, updatePosChar p first) + else (s, p) + x -> (s, p) + -- bash allows a=(b), ksh allows $a=(b). dash allows neither. Let's warn. readEvalSuffix = many1 (readIoRedirect <|> readCmdWord <|> evalFallback)