From 4a87d2a3de0fa9b919dee35241a35580ab91f24a Mon Sep 17 00:00:00 2001 From: Ng Zhi An Date: Sat, 20 Oct 2018 22:09:42 -0700 Subject: [PATCH] Expose token positions in params, use that to construct fixes --- src/ShellCheck/Analytics.hs | 35 ++++++++++++++++++++++++++++----- src/ShellCheck/AnalyzerLib.hs | 6 ++++-- src/ShellCheck/Checker.hs | 18 +++++++++-------- src/ShellCheck/Formatter/TTY.hs | 22 ++++++++------------- src/ShellCheck/Interface.hs | 27 +++++++------------------ 5 files changed, 59 insertions(+), 49 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index b2e227c..d211add 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -241,6 +241,31 @@ isCondition (child:parent:rest) = T_UntilExpression id c l -> take 1 . reverse $ c _ -> [] +-- helpers to build replacements +replace_start id params n r = + let tp = tokenPositions params + (start, _) = tp Map.! id + new_end = start { + posColumn = posColumn start + n + } + in + [R start new_end r] +replace_end id params n r = + -- because of the way we count columns 1-based + -- we have to offset end columns by 1 + let tp = tokenPositions params + (_, end) = tp Map.! id + new_start = end { + posColumn = posColumn end - n + 1 + } + new_end = end { + posColumn = posColumn end + 1 + } + in + [R new_start new_end r] +surround_with id params s = + (replace_start id params 0 s) ++ (replace_end id params 0 s) + prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)" checkEchoWc _ (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ @@ -1335,10 +1360,10 @@ checkPS1Assignments _ _ = return () prop_checkBackticks1 = verify checkBackticks "echo `foo`" prop_checkBackticks2 = verifyNot checkBackticks "echo $(foo)" prop_checkBackticks3 = verifyNot checkBackticks "echo `#inlined comment` foo" -checkBackticks _ (T_Backticked id list) | not (null list) = +checkBackticks params (T_Backticked id list) | not (null list) = addComment $ makeCommentWithFix StyleC id 2006 "Use $(...) notation instead of legacy backticked `...`." - ((replaceStart 1 "$(") ++ (replaceEnd 1 ")")) + ((replace_start id params 1 "$(") ++ (replace_end id params 1 ")")) -- style id 2006 "Use $(...) notation instead of legacy backticked `...`." checkBackticks _ _ = return () @@ -1644,7 +1669,7 @@ checkSpacefulness params t = "This default assignment may cause DoS due to globbing. Quote it." else makeCommentWithFix InfoC (getId token) 2086 - "Double quote to prevent globbing and word splitting." (surroundWith "\"") + "Double quote to prevent globbing and word splitting." (surround_with (getId token) params "\"") -- makeComment InfoC (getId token) 2086 -- "Double quote to prevent globbing and word splitting." @@ -2545,7 +2570,7 @@ checkUncheckedCdPushdPopd params root = && not (isCondition $ getPath (parentMap params) t)) $ -- warn (getId t) 2164 "Use 'cd ... || exit' or 'cd ... || return' in case cd fails." warnWithFix (getId t) 2164 "Use 'cd ... || exit' or 'cd ... || return' in case cd fails." - (replaceEnd 0 " || exit") + (replace_end (getId t) params 0 " || exit") checkElement _ = return () name t = fromMaybe "" $ getCommandName t isSafeDir t = case oversimplify t of @@ -2702,7 +2727,7 @@ checkArrayAssignmentIndices params root = T_Literal id str -> [(id,str)] _ -> [] guard $ '=' `elem` str - return $ warnWithFix id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it." (surroundWith "\"") + return $ warnWithFix id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it." (surround_with id params "\"") in if null literalEquals && isAssociative then warn (getId t) 2190 "Elements in associative arrays need index, e.g. array=( [index]=value ) ." diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs index 4bf68e7..1639ff6 100644 --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -81,7 +81,8 @@ data Parameters = Parameters { parentMap :: Map.Map Id Token, -- A map from Id to parent Token shellType :: Shell, -- The shell type, such as Bash or Ksh shellTypeSpecified :: Bool, -- True if shell type was forced via flags - rootNode :: Token -- The root node of the AST + rootNode :: Token, -- The root node of the AST + tokenPositions :: Map.Map Id (Position, Position) -- map from token id to start and end position } -- TODO: Cache results of common AST ops here @@ -177,7 +178,8 @@ makeParameters spec = shellTypeSpecified = isJust $ asShellType spec, parentMap = getParentTree root, - variableFlow = getVariableFlow params root + variableFlow = getVariableFlow params root, + tokenPositions = asTokenPositions spec } in params where root = asScript spec diff --git a/src/ShellCheck/Checker.hs b/src/ShellCheck/Checker.hs index b9e7927..7ac9c91 100644 --- a/src/ShellCheck/Checker.hs +++ b/src/ShellCheck/Checker.hs @@ -64,11 +64,20 @@ checkScript sys spec = do psShellTypeOverride = csShellTypeOverride spec } let parseMessages = prComments result + let tokenPositions = prTokenPositions result + let analysisSpec root = + as { + asScript = root, + asShellType = csShellTypeOverride spec, + asCheckSourced = csCheckSourced spec, + asExecutionMode = Executed, + asTokenPositions = tokenPositions + } where as = newAnalysisSpec root let analysisMessages = fromMaybe [] $ (arComments . analyzeScript . analysisSpec) <$> prRoot result - let translator = tokenToPosition (prTokenPositions result) + let translator = tokenToPosition tokenPositions return . nub . sortMessages . filter shouldInclude $ (parseMessages ++ map translator analysisMessages) @@ -91,13 +100,6 @@ checkScript sys spec = do cMessage comment) getPosition = pcStartPos - analysisSpec root = - as { - asScript = root, - asShellType = csShellTypeOverride spec, - asCheckSourced = csCheckSourced spec, - asExecutionMode = Executed - } where as = newAnalysisSpec root getErrors sys spec = sort . map getCode . crComments $ diff --git a/src/ShellCheck/Formatter/TTY.hs b/src/ShellCheck/Formatter/TTY.hs index dc3f32e..a1c98b1 100644 --- a/src/ShellCheck/Formatter/TTY.hs +++ b/src/ShellCheck/Formatter/TTY.hs @@ -118,8 +118,8 @@ outputForFile color sys comments = do let fileLines = lines contents let lineCount = fromIntegral $ length fileLines let groups = groupWith lineNo comments - mapM_ (\x -> do - let lineNum = lineNo (head x) + mapM_ (\commentsForLine -> do + let lineNum = lineNo (head commentsForLine) let line = if lineNum < 1 || lineNum > lineCount then "" else fileLines !! fromIntegral (lineNum - 1) @@ -127,9 +127,10 @@ outputForFile color sys comments = do putStrLn $ color "message" $ "In " ++ fileName ++" line " ++ show lineNum ++ ":" putStrLn (color "source" line) - mapM_ (\c -> putStrLn (color (severityText c) $ cuteIndent c)) x + mapM_ (\c -> putStrLn (color (severityText c) $ cuteIndent c)) commentsForLine putStrLn "" - mapM_ (\c -> putStrLn "Did you mean:" >> putStrLn (fixedString c line)) x + -- in the spirit of error prone + mapM_ (\c -> putStrLn "Did you mean:" >> putStrLn (fixedString c line)) commentsForLine ) groups -- need to do something smart about sorting by end index @@ -141,16 +142,9 @@ fixedString comment line = apply_replacement rs line 0 where apply_replacement [] s _ = s - apply_replacement ((Start n r):xs) s offset = - let start = (posColumn . pcStartPos) comment - end = start + n - z = do_replace start end s r - len_r = (fromIntegral . length) r in - apply_replacement xs z (offset + (end - start) + len_r) - apply_replacement ((End n r):xs) s offset = - -- tricky math because column is 1 based - let end = (posColumn . pcEndPos) comment + 1 - start = end - n + apply_replacement ((R startp endp r):xs) s offset = + let start = posColumn startp + end = posColumn endp z = do_replace start end s r len_r = (fromIntegral . length) r in apply_replacement xs z (offset + (end - start) + len_r) diff --git a/src/ShellCheck/Interface.hs b/src/ShellCheck/Interface.hs index 69d452d..ef9dd44 100644 --- a/src/ShellCheck/Interface.hs +++ b/src/ShellCheck/Interface.hs @@ -24,7 +24,7 @@ module ShellCheck.Interface , CheckResult(crFilename, crComments) , ParseSpec(psFilename, psScript, psCheckSourced, psShellTypeOverride) , ParseResult(prComments, prTokenPositions, prRoot) - , AnalysisSpec(asScript, asShellType, asExecutionMode, asCheckSourced) + , AnalysisSpec(asScript, asShellType, asExecutionMode, asCheckSourced, asTokenPositions) , AnalysisResult(arComments) , FormatterOptions(foColorOption, foWikiLinkCount) , Shell(Ksh, Sh, Bash, Dash) @@ -50,10 +50,7 @@ module ShellCheck.Interface , newPositionedComment , newComment , Fix - , Replacement(Start, End) - , surroundWith - , replaceStart - , replaceEnd + , Replacement(R) ) where import ShellCheck.AST @@ -132,14 +129,16 @@ data AnalysisSpec = AnalysisSpec { asScript :: Token, asShellType :: Maybe Shell, asExecutionMode :: ExecutionMode, - asCheckSourced :: Bool + asCheckSourced :: Bool, + asTokenPositions :: Map.Map Id (Position, Position) } newAnalysisSpec token = AnalysisSpec { asScript = token, asShellType = Nothing, asExecutionMode = Executed, - asCheckSourced = False + asCheckSourced = False, + asTokenPositions = Map.empty } newtype AnalysisResult = AnalysisResult { @@ -198,23 +197,11 @@ newComment = Comment { -- only support single line for now data Replacement = - Start Integer String - | End Integer String + R Position Position String deriving (Show, Eq) type Fix = [Replacement] -surroundWith s = - (replaceStart 0 s) ++ (replaceEnd 0 s) - --- replace first n chars -replaceStart n r = - [ Start n r ] - --- replace last n chars -replaceEnd n r = - [ End n r ] - data PositionedComment = PositionedComment { pcStartPos :: Position, pcEndPos :: Position,