From 63188282e935f9c1628fa62134bc46840ba7bad2 Mon Sep 17 00:00:00 2001 From: Vidar Holen Date: Sat, 28 Feb 2015 18:43:22 -0800 Subject: [PATCH] Add warning for vars that are referenced but not assigned. --- ShellCheck/Analytics.hs | 123 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 113 insertions(+), 10 deletions(-) diff --git a/ShellCheck/Analytics.hs b/ShellCheck/Analytics.hs index 516711e..c2447a3 100644 --- a/ShellCheck/Analytics.hs +++ b/ShellCheck/Analytics.hs @@ -27,6 +27,7 @@ import Data.Functor import Data.Function (on) import Data.List import Data.Maybe +import Data.Ord import Debug.Trace import ShellCheck.AST import ShellCheck.Options @@ -58,6 +59,7 @@ treeChecks = [ ,checkUnpassedInFunctions ,checkArrayWithoutIndex ,checkShebang + ,checkUnassignedReferences ] checksFor Sh = [ @@ -331,6 +333,8 @@ 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"] @@ -367,6 +371,29 @@ checkTree f s = case parseShell "-" s of _ -> Nothing +-- Copied from https://wiki.haskell.org/Edit_distance +dist :: Eq a => [a] -> [a] -> Int +dist a b + = last (if lab == 0 then mainDiag + else if lab > 0 then lowers !! (lab - 1) + else{- < 0 -} uppers !! (-1 - lab)) + where mainDiag = oneDiag a b (head uppers) (-1 : head lowers) + uppers = eachDiag a b (mainDiag : uppers) -- upper diagonals + lowers = eachDiag b a (mainDiag : lowers) -- lower diagonals + eachDiag a [] diags = [] + eachDiag a (bch:bs) (lastDiag:diags) = oneDiag a bs nextDiag lastDiag : eachDiag a bs diags + where nextDiag = head (tail diags) + oneDiag a b diagAbove diagBelow = thisdiag + where doDiag [] b nw n w = [] + doDiag a [] nw n w = [] + doDiag (ach:as) (bch:bs) nw n w = me : (doDiag as bs me (tail n) (tail w)) + where me = if ach == bch then nw else 1 + min3 (head w) nw (head n) + firstelt = 1 + head diagBelow + thisdiag = firstelt : doDiag a b firstelt diagAbove (tail diagBelow) + lab = length a - length b + min3 x y z = if x < y then x else min y z + + prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)" checkEchoWc _ (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ @@ -2093,6 +2120,11 @@ getModifiedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal "read" -> let params = map getLiteral rest in catMaybes . takeWhile isJust . reverse $ params + "getopts" -> + case rest of + opts:var:_ -> maybeToList $ getLiteral var + _ -> [] + "let" -> concatMap letParamToLiteral rest "export" -> concatMap getModifierParam rest @@ -2139,17 +2171,16 @@ getModifiedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal getModifiedVariableCommand _ = [] --- TODO: getBracedReference s = - case filter (not . null) [ - dropSuffix $ dropPrefix s, - dropSuffix s, - s] of - (a:_) -> a - [] -> error "Internal ShellCheck error (empty braced reference). Please file a bug!" + let name = takeName $ dropPrefix s in + if null name then s else name where - dropSuffix = takeWhile (`notElem` ":[#%/^,") dropPrefix = dropWhile (`elem` "#!") + takeName s = + let special = getSpecial s in + if null special then takeWhile isVariableChar s else special + getSpecial (c:_) = + if c `elem` "*@#?-$!" then [c] else "" getIndexReferences s = fromMaybe [] $ do (_, index, _, _) <- matchRegexAll re s @@ -2491,12 +2522,84 @@ checkUnusedAssignments params t = execWriter (mapM_ warnFor unused) unused = Map.assocs $ Map.difference assignments references warnFor (name, token) = - info (getId token) 2034 $ + warn (getId token) 2034 $ name ++ " appears unused. Verify it or export it." stripSuffix = takeWhile isVariableChar defaultMap = Map.fromList $ zip internalVariables $ repeat () +prop_checkUnassignedReferences1 = verifyTree checkUnassignedReferences "echo $foo" +prop_checkUnassignedReferences2 = verifyNotTree checkUnassignedReferences "foo=hello; echo $foo" +prop_checkUnassignedReferences3 = verifyTree checkUnassignedReferences "MY_VALUE=3; echo $MYVALUE" +prop_checkUnassignedReferences4 = verifyNotTree checkUnassignedReferences "RANDOM2=foo; echo $RANDOM" +prop_checkUnassignedReferences5 = verifyNotTree checkUnassignedReferences "declare -A foo=([bar]=baz); echo ${foo[bar]}" +prop_checkUnassignedReferences6 = verifyNotTree checkUnassignedReferences "foo=..; echo ${foo-bar}" +prop_checkUnassignedReferences7 = verifyNotTree checkUnassignedReferences "getopts ':h' foo; echo $foo" +prop_checkUnassignedReferences8 = verifyNotTree checkUnassignedReferences "let 'foo = 1'; echo $foo" +prop_checkUnassignedReferences9 = verifyNotTree checkUnassignedReferences "echo ${foo-bar}" +prop_checkUnassignedReferences10= verifyNotTree checkUnassignedReferences "echo ${foo:?}" +checkUnassignedReferences params t = warnings + where + (readMap, writeMap) = execState (mapM tally $ variableFlow params) (Map.empty, Map.empty) + defaultAssigned = Map.fromList $ map (\a -> (a, ())) $ filter (not . null) internalVariables + + tally (Assignment (_, _, name, _)) = + modify (\(read, written) -> (read, Map.insert name () written)) + tally (Reference (_, place, name)) = + modify (\(read, written) -> (Map.insertWith' (const id) name place read, written)) + tally _ = return () + + unassigned = Map.toList $ Map.difference (Map.difference readMap writeMap) defaultAssigned + writtenVars = filter isVariableName $ Map.keys writeMap + + getBestMatch var = do + (match, score) <- listToMaybe best + guard $ goodMatch var match score + return match + where + matches = map (\x -> (x, dist var x)) writtenVars + best = sortBy (comparing snd) matches + goodMatch var match score = + let l = length match in + l > 3 && score <= 1 + || l > 7 && score <= 2 + + isLocal = any isLower + + warningForGlobals var place = do + match <- getBestMatch var + return $ warn (getId place) 2153 $ + "Possible misspelling: " ++ var ++ " may not be assigned, but " ++ match ++ " is." + + warningForLocals var place = + return $ warn (getId place) 2154 $ + var ++ " is referenced but apparently never assigned" ++ optionalTip ++ "." + where + optionalTip = fromMaybe "" $ do + match <- getBestMatch var + return $ " (did you mean '" ++ match ++ "'?)" + + warningFor var place = do + guard . not $ isInArray place || isGuarded place + (if isLocal var then warningForLocals else warningForGlobals) var place + + warnings = execWriter . sequence $ mapMaybe (uncurry warningFor) unassigned + + -- Due to parsing, foo=( [bar]=baz ) parses 'bar' as a reference even for assoc arrays. + -- This works around it by ignoring references in array assignemnts. + isInArray t = any isArray $ getPath (parentMap params) t + where + isArray (T_Array {}) = True + isArray _ = False + + isGuarded (T_DollarBraced _ v) = + any (`isPrefixOf` rest) ["-", ":-", "?", ":?"] + where + name = concat $ deadSimple v + rest = dropWhile isVariableChar $ dropWhile (`elem` "#!") $ name + isGuarded _ = False + + prop_checkGlobsAsOptions1 = verify checkGlobsAsOptions "rm *.txt" prop_checkGlobsAsOptions2 = verify checkGlobsAsOptions "ls ??.*" prop_checkGlobsAsOptions3 = verifyNot checkGlobsAsOptions "rm -- *.txt" @@ -3091,5 +3194,5 @@ checkFindExecWithSingleArgument _ = checkCommand "find" (const f) check _ = Nothing commandRegex = mkRegex "[ |;]" -return [] + runTests = $quickCheckAll