From c27ea7fda3bcd8c7825ca8c44f360a9a88c295d0 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Mon, 8 Feb 2021 17:02:35 +0800 Subject: [PATCH 01/10] Add code action for hiding shadowed identifiers from imports --- .../src/Development/IDE/Plugin/CodeAction.hs | 88 ++++++++++++++++++- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction.hs b/ghcide/src/Development/IDE/Plugin/CodeAction.hs index c5f0ae6067..72d2595c8d 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction.hs @@ -58,6 +58,12 @@ import OccName import qualified GHC.LanguageExtensions as Lang import Control.Lens (alaf) import Data.Monoid (Ap(..)) +import TcRnTypes (TcGblEnv(..), ImportAvails(..)) +import HscTypes (ImportedModsVal(..), importedByUser) +import RdrName (GlobalRdrElt(..), lookupGlobalRdrEnv) +import SrcLoc (realSrcSpanStart) +import Module (moduleEnvElts) +import qualified Data.Map as M descriptor :: PluginId -> PluginDescriptor IdeState descriptor plId = @@ -80,11 +86,13 @@ codeAction lsp state _ (TextDocumentIdentifier uri) _range CodeActionContext{_di let text = Rope.toText . (_text :: VirtualFile -> Rope.Rope) <$> contents mbFile = toNormalizedFilePath' <$> uriToFilePath uri diag <- fmap (\(_, _, d) -> d) . filter (\(p, _, _) -> mbFile == Just p) <$> getDiagnostics state - (ideOptions, join -> parsedModule, join -> env, join -> annotatedPS) <- runAction "CodeAction" state $ - (,,,) <$> getIdeOptions + (ideOptions, join -> parsedModule, join -> env, join -> annotatedPS, join -> tcM, join -> har) <- runAction "CodeAction" state $ + (,,,,,) <$> getIdeOptions <*> getParsedModule `traverse` mbFile <*> use GhcSession `traverse` mbFile <*> use GetAnnotatedParsedSource `traverse` mbFile + <*> use TypeCheck `traverse` mbFile + <*> use GetHieAst `traverse` mbFile -- This is quite expensive 0.6-0.7s on GHC let pkgExports = envPackageExports <$> env localExports <- readVar (exportsMap $ shakeExtras state) @@ -93,7 +101,7 @@ codeAction lsp state _ (TextDocumentIdentifier uri) _range CodeActionContext{_di df = ms_hspp_opts . pm_mod_summary <$> parsedModule actions = [ mkCA title [x] edit - | x <- xs, (title, tedit) <- suggestAction exportsMap ideOptions parsedModule text df annotatedPS x + | x <- xs, (title, tedit) <- suggestAction exportsMap ideOptions parsedModule text df annotatedPS tcM har x , let edit = WorkspaceEdit (Just $ Map.singleton uri $ List tedit) Nothing ] actions' = caRemoveRedundantImports parsedModule text diag xs uri @@ -123,9 +131,11 @@ suggestAction -> Maybe T.Text -> Maybe DynFlags -> Maybe (Annotated ParsedSource) + -> Maybe TcModuleResult + -> Maybe HieAstResult -> Diagnostic -> [(T.Text, [TextEdit])] -suggestAction packageExports ideOptions parsedModule text df annSource diag = +suggestAction packageExports ideOptions parsedModule text df annSource tcM har diag = concat -- Order these suggestions by priority [ suggestSignature True diag @@ -140,6 +150,7 @@ suggestAction packageExports ideOptions parsedModule text df annSource diag = , suggestAddTypeAnnotationToSatisfyContraints text diag , rewrite df annSource $ \df ps -> suggestConstraint df ps diag , rewrite df annSource $ \_ ps -> suggestImplicitParameter ps diag + , rewrite df annSource $ \_ ps -> suggestHideShadow ps tcM har diag ] ++ concat [ suggestNewDefinition ideOptions pm text diag ++ suggestNewImport packageExports pm diag @@ -169,6 +180,75 @@ findInstanceHead df instanceHead decls = findDeclContainingLoc :: Position -> [Located a] -> Maybe (Located a) findDeclContainingLoc loc = find (\(L l _) -> loc `isInsideSrcSpan` l) +-- This binding for ‘mod’ shadows the existing binding +-- imported from ‘Prelude’ at haskell-language-server/ghcide/src/Development/IDE/Plugin/CodeAction.hs:10:8-40 +-- (and originally defined in ‘GHC.Real’)typecheck(-Wname-shadowing) +suggestHideShadow :: ParsedSource -> Maybe TcModuleResult -> Maybe HieAstResult -> Diagnostic -> [(T.Text, [Rewrite])] +suggestHideShadow pm@(L _ HsModule {hsmodImports}) mTcM mHar Diagnostic {_message, _range} + | Just tcM <- mTcM, + Just har <- mHar, + Just [identifier, modName, s] <- + matchRegexUnifySpaces + _message + "This binding for ‘([^`]+)’ shadows the existing binding imported from ‘([^`]+)’ at ([^ ]*)", + [s'] <- [x | (x, "") <- readSrcSpan $ T.unpack s], + isUnusedImportedId tcM har (T.unpack identifier) (T.unpack modName) (RealSrcSpan s'), + title <- "Hide " <> identifier <> " from " <> modName, + implicitPrelude <- null $ findImportDeclByModuleName hsmodImports "Prelude" = + if modName == "Prelude" && implicitPrelude + then [(title, maybeToList $ hideImplicitPreludeSymbol (T.unpack identifier) pm)] + else [(title, hideOrRemoveId hsmodImports (T.unpack identifier) (T.unpack modName))] + | otherwise = [] + +hideOrRemoveId :: [LImportDecl GhcPs] -> String -> String -> [Rewrite] +hideOrRemoveId lImportDecls identifier modName + | Just decl <- findImportDeclByModuleName lImportDecls modName = + [hideSymbol identifier decl] + | otherwise = + [] + +findImportDeclByModuleName :: [LImportDecl GhcPs] -> String -> Maybe (LImportDecl GhcPs) +findImportDeclByModuleName decls modName = flip find decls $ \case + (L _ ImportDecl {..}) -> modName == moduleNameString (unLoc ideclName) + _ -> error "impossible" + +isTheSameLine :: SrcSpan -> SrcSpan -> Bool +isTheSameLine s1 s2 + | Just sl1 <- getStartLine s1, + Just sl2 <- getStartLine s2 = + sl1 == sl2 + | otherwise = False + +getStartLine :: SrcSpan -> Maybe Int +getStartLine s + | RealSrcSpan s' <- s, + startLoc <- realSrcSpanStart s', + startLine <- srcLocLine startLoc = + Just startLine + | otherwise = Nothing + +isUnusedImportedId :: TcModuleResult -> HieAstResult -> String -> String -> SrcSpan -> Bool +isUnusedImportedId + TcModuleResult {tmrTypechecked = TcGblEnv {tcg_imports = ImportAvails {imp_mods}}} + HAR {refMap = rf} + identifier + modName + importSpan + | occ <- mkVarOcc identifier, + impModsVals <- importedByUser . concat $ moduleEnvElts imp_mods, + Just rdrEnv <- + listToMaybe + [ imv_all_exports + | ImportedModsVal {..} <- impModsVals, + imv_name == mkModuleName modName, + isTheSameLine imv_span importSpan + ], + [GRE {..}] <- lookupGlobalRdrEnv rdrEnv occ, + importedIdentifier <- Right gre_name, + refs <- M.lookup importedIdentifier rf = + maybe True null refs + | otherwise = False + suggestDisableWarning :: ParsedModule -> Maybe T.Text -> Diagnostic -> [(T.Text, [TextEdit])] suggestDisableWarning pm contents Diagnostic{..} | Just (StringValue (T.stripPrefix "-W" -> Just w)) <- _code = From a8ea66415cc6503c993b2fde61a1369ac9a832d2 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Tue, 9 Feb 2021 13:28:45 +0800 Subject: [PATCH 02/10] Insert to the line above module decls if there are no existing import decls --- .../IDE/Plugin/CodeAction/ExactPrint.hs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs b/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs index 68198752c9..e7489ff75c 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs @@ -31,7 +31,7 @@ import Development.IDE.GHC.Compat hiding (parseExpr) import Development.IDE.GHC.ExactPrint ( Annotate, ASTElement(parseAST) ) import FieldLabel (flLabel) -import GhcPlugins (sigPrec) +import GhcPlugins (sigPrec, mkRealSrcLoc) import Language.Haskell.GHC.ExactPrint import Language.Haskell.GHC.ExactPrint.Types (DeltaPos (DP), KeywordId (G), mkAnnKey) import Language.Haskell.LSP.Types @@ -40,7 +40,6 @@ import Outputable (ppr, showSDocUnsafe, showSDoc) import Retrie.GHC (rdrNameOcc, unpackFS, mkRealSrcSpan, realSrcSpanEnd) import Development.IDE.Spans.Common import Development.IDE.GHC.Error -import Safe (lastMay) import Data.Generics (listify) import GHC.Exts (IsList (fromList)) @@ -205,6 +204,7 @@ extendImport mparent identifier lDecl@(L l _) = -- extendImportTopLevel "foo" AST: -- -- import A --> Error +-- import A (foo) --> Error -- import A (bar) --> import A (bar, foo) extendImportTopLevel :: DynFlags -> String -> LImportDecl GhcPs -> TransformT (Either String) (LImportDecl GhcPs) extendImportTopLevel df idnetifier (L l it@ImportDecl {..}) @@ -408,13 +408,15 @@ deleteFromImport (T.pack -> symbol) (L l idecl) llies@(L lieLoc lies) _ =do (filter ((/= symbol) . T.pack . unpackFS . flLabel . unLoc) flds) killLie v = Just v +-- | Insert a import declaration hiding a symbole from Prelude hideImplicitPreludeSymbol :: String -> ParsedSource -> Maybe Rewrite hideImplicitPreludeSymbol symbol (L _ HsModule{..}) = do - existingImp <- lastMay hsmodImports - exisImpSpan <- realSpan $ getLoc existingImp - let indentation = srcSpanStartCol exisImpSpan - beg = realSrcSpanEnd exisImpSpan + let predLine old = mkRealSrcLoc (srcLocFile old) (srcLocLine old - 1) (srcLocCol old) + existingImpSpan = (fmap (id,) . realSpan . getLoc) =<< lastMaybe hsmodImports + existingDeclSpan = (fmap (predLine, ) . realSpan . getLoc) =<< headMaybe hsmodDecls + (f, s) <- existingImpSpan <|> existingDeclSpan + let beg = f $ realSrcSpanEnd s ran = RealSrcSpan $ mkRealSrcSpan beg beg pure $ Rewrite ran $ \df -> do let symOcc = mkVarOcc symbol @@ -424,6 +426,6 @@ hideImplicitPreludeSymbol symbol (L _ HsModule{..}) = do -- Re-labeling is needed to reflect annotations correctly L _ idecl0 <- liftParseAST @(ImportDecl GhcPs) df $ T.unpack impStmt let idecl = L ran idecl0 - addSimpleAnnT idecl (DP (1,indentation - 1)) - [(G AnnImport, DP (1, indentation - 1))] + addSimpleAnnT idecl (DP (1, 0)) + [(G AnnImport, DP (1, 0))] pure idecl From 368165b157f7be3a17e4beb528b23dac754fa686 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Tue, 9 Feb 2021 14:42:44 +0800 Subject: [PATCH 03/10] Support handling multi imports --- .../src/Development/IDE/Plugin/CodeAction.hs | 51 ++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction.hs b/ghcide/src/Development/IDE/Plugin/CodeAction.hs index 72d2595c8d..7d263b3326 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction.hs @@ -64,6 +64,7 @@ import RdrName (GlobalRdrElt(..), lookupGlobalRdrEnv) import SrcLoc (realSrcSpanStart) import Module (moduleEnvElts) import qualified Data.Map as M +import qualified Data.Set as S descriptor :: PluginId -> PluginDescriptor IdeState descriptor plId = @@ -180,25 +181,44 @@ findInstanceHead df instanceHead decls = findDeclContainingLoc :: Position -> [Located a] -> Maybe (Located a) findDeclContainingLoc loc = find (\(L l _) -> loc `isInsideSrcSpan` l) +-- Single: -- This binding for ‘mod’ shadows the existing binding -- imported from ‘Prelude’ at haskell-language-server/ghcide/src/Development/IDE/Plugin/CodeAction.hs:10:8-40 -- (and originally defined in ‘GHC.Real’)typecheck(-Wname-shadowing) +-- Multi: +--This binding for ‘pack’ shadows the existing bindings +-- imported from ‘Data.ByteString’ at B.hs:6:1-22 +-- imported from ‘Data.ByteString.Lazy’ at B.hs:8:1-27 +-- imported from ‘Data.Text’ at B.hs:7:1-16 + suggestHideShadow :: ParsedSource -> Maybe TcModuleResult -> Maybe HieAstResult -> Diagnostic -> [(T.Text, [Rewrite])] suggestHideShadow pm@(L _ HsModule {hsmodImports}) mTcM mHar Diagnostic {_message, _range} - | Just tcM <- mTcM, - Just har <- mHar, - Just [identifier, modName, s] <- + | Just [identifier, modName, s] <- + matchRegexUnifySpaces + _message + "This binding for ‘([^`]+)’ shadows the existing binding imported from ‘([^`]+)’ at ([^ ]*)" + = suggests identifier modName s + | Just [identifier] <- matchRegexUnifySpaces _message - "This binding for ‘([^`]+)’ shadows the existing binding imported from ‘([^`]+)’ at ([^ ]*)", - [s'] <- [x | (x, "") <- readSrcSpan $ T.unpack s], - isUnusedImportedId tcM har (T.unpack identifier) (T.unpack modName) (RealSrcSpan s'), - title <- "Hide " <> identifier <> " from " <> modName, - implicitPrelude <- null $ findImportDeclByModuleName hsmodImports "Prelude" = - if modName == "Prelude" && implicitPrelude - then [(title, maybeToList $ hideImplicitPreludeSymbol (T.unpack identifier) pm)] - else [(title, hideOrRemoveId hsmodImports (T.unpack identifier) (T.unpack modName))] + "This binding for ‘([^`]+)’ shadows the existing bindings", + Just matched <- allMatchRegexUnifySpaces _message "imported from ‘([^’]+)’ at ([^ ]*)", + mods <- [(modName, s) | [_, modName, s] <- matched], + result <- nubOrdBy (compare `on` fst) $ mods >>= uncurry (suggests identifier), + hideAll <- ("Hide " <> identifier <> " from all occurence imports", concat $ snd <$> result) + = result <> [hideAll] | otherwise = [] + where + suggests identifier modName s + | Just tcM <- mTcM, + Just har <- mHar, + [s'] <- [x | (x, "") <- readSrcSpan $ T.unpack s], + isUnusedImportedId tcM har (T.unpack identifier) (T.unpack modName) (RealSrcSpan s'), + title <- "Hide " <> identifier <> " from " <> modName + = if modName == "Prelude" && (null $ findImportDeclByModuleName hsmodImports "Prelude") + then [(title, maybeToList $ hideImplicitPreludeSymbol (T.unpack identifier) pm)] + else [(title, hideOrRemoveId hsmodImports (T.unpack identifier) (T.unpack modName))] + | otherwise = [] hideOrRemoveId :: [LImportDecl GhcPs] -> String -> String -> [Rewrite] hideOrRemoveId lImportDecls identifier modName @@ -220,12 +240,7 @@ isTheSameLine s1 s2 | otherwise = False getStartLine :: SrcSpan -> Maybe Int -getStartLine s - | RealSrcSpan s' <- s, - startLoc <- realSrcSpanStart s', - startLine <- srcLocLine startLoc = - Just startLine - | otherwise = Nothing +getStartLine x = srcLocLine . realSrcSpanStart <$> realSpan x isUnusedImportedId :: TcModuleResult -> HieAstResult -> String -> String -> SrcSpan -> Bool isUnusedImportedId @@ -246,7 +261,7 @@ isUnusedImportedId [GRE {..}] <- lookupGlobalRdrEnv rdrEnv occ, importedIdentifier <- Right gre_name, refs <- M.lookup importedIdentifier rf = - maybe True null refs + maybe True (null . filter (\(_, IdentifierDetails{..}) -> identInfo == S.singleton Use)) refs | otherwise = False suggestDisableWarning :: ParsedModule -> Maybe T.Text -> Diagnostic -> [(T.Text, [TextEdit])] From e6f26244156e0db5c337aa9c35621461153c36d9 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Wed, 10 Feb 2021 18:03:43 +0800 Subject: [PATCH 04/10] Remove trailing comma in processed import lists --- ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs | 7 +++++-- ghcide/test/data/hiding/HideFunction.expected.append.E.hs | 2 +- .../data/hiding/HideFunction.expected.append.Prelude.hs | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs b/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs index e7489ff75c..774710ce1f 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs @@ -382,6 +382,8 @@ deleteFromImport (T.pack -> symbol) (L l idecl) llies@(L lieLoc lies) _ =do lidecl' = L l $ idecl { ideclHiding = Just (False, edited) } + -- avoid import A (foo,) + maybe (pure ()) removeTrailingCommaT $ lastMaybe deletedLies when (not (null lies) && null deletedLies) $ do transferAnn llies edited id addSimpleAnnT edited dp00 @@ -417,6 +419,7 @@ hideImplicitPreludeSymbol symbol (L _ HsModule{..}) = do existingDeclSpan = (fmap (predLine, ) . realSpan . getLoc) =<< headMaybe hsmodDecls (f, s) <- existingImpSpan <|> existingDeclSpan let beg = f $ realSrcSpanEnd s + indentation = srcSpanStartCol s ran = RealSrcSpan $ mkRealSrcSpan beg beg pure $ Rewrite ran $ \df -> do let symOcc = mkVarOcc symbol @@ -426,6 +429,6 @@ hideImplicitPreludeSymbol symbol (L _ HsModule{..}) = do -- Re-labeling is needed to reflect annotations correctly L _ idecl0 <- liftParseAST @(ImportDecl GhcPs) df $ T.unpack impStmt let idecl = L ran idecl0 - addSimpleAnnT idecl (DP (1, 0)) - [(G AnnImport, DP (1, 0))] + addSimpleAnnT idecl (DP (1, indentation - 1)) + [(G AnnImport, DP (1, indentation - 1))] pure idecl diff --git a/ghcide/test/data/hiding/HideFunction.expected.append.E.hs b/ghcide/test/data/hiding/HideFunction.expected.append.E.hs index 94d333b24a..3448baa4f4 100644 --- a/ghcide/test/data/hiding/HideFunction.expected.append.E.hs +++ b/ghcide/test/data/hiding/HideFunction.expected.append.E.hs @@ -1,7 +1,7 @@ module HideFunction where import AVec (fromList) -import BVec (fromList,) +import BVec (fromList) import CVec hiding ((++), cons) import DVec hiding ((++), cons, snoc) import EVec as E diff --git a/ghcide/test/data/hiding/HideFunction.expected.append.Prelude.hs b/ghcide/test/data/hiding/HideFunction.expected.append.Prelude.hs index 0b202451f0..78d1cd879b 100644 --- a/ghcide/test/data/hiding/HideFunction.expected.append.Prelude.hs +++ b/ghcide/test/data/hiding/HideFunction.expected.append.Prelude.hs @@ -1,7 +1,7 @@ module HideFunction where import AVec (fromList) -import BVec (fromList,) +import BVec (fromList) import CVec hiding ((++), cons) import DVec hiding ((++), cons, snoc) import EVec as E hiding ((++)) From bea3eb8f97329493a701861824042a5e1ffdc6e4 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Wed, 10 Feb 2021 18:42:23 +0800 Subject: [PATCH 05/10] Add tests --- ghcide/test/exe/Main.hs | 177 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) diff --git a/ghcide/test/exe/Main.hs b/ghcide/test/exe/Main.hs index 877ddd1f9c..8402e61af7 100644 --- a/ghcide/test/exe/Main.hs +++ b/ghcide/test/exe/Main.hs @@ -693,6 +693,7 @@ codeActionTests = testGroup "code actions" , removeImportTests , extendImportTests , suggestImportTests + , suggestHideShadowTests , suggestImportDisambiguationTests , disableWarningTests , fixConstructorImportTests @@ -1586,6 +1587,182 @@ suggestImportDisambiguationTests = testGroup "suggest import disambiguation acti k doc actions withHideFunction = withTarget ("HideFunction" <.> "hs") +suggestHideShadowTests :: TestTree +suggestHideShadowTests = + testGroup + "suggest hide shadow" + [ testGroup + "single" + [ testOneCodeAction + "hide unsued" + "Hide on from Data.Function" + (1, 2) + (1, 4) + [ "import Data.Function" + , "f on = on" + , "g on = on" + ] + [ "import Data.Function hiding (on)" + , "f on = on" + , "g on = on" + ] + , testOneCodeAction + "delete unsued" + "Hide on from Data.Function" + (1, 2) + (1, 4) + [ "import Data.Function ((&), on)" + , "f on = on" + ] + [ "import Data.Function ((&))" + , "f on = on" + ] + , testOneCodeAction + "hide operator" + "Hide & from Data.Function" + (1, 2) + (1, 5) + [ "import Data.Function" + , "f (&) = (&)" + ] + [ "import Data.Function hiding ((&))" + , "f (&) = (&)" + ] + , testOneCodeAction + "remove operator" + "Hide & from Data.Function" + (1, 2) + (1, 5) + [ "import Data.Function ((&), on)" + , "f (&) = (&)" + ] + [ "import Data.Function ( on)" + , "f (&) = (&)" + ] + , noCodeAction + "don't remove already used" + (2, 2) + (2, 4) + [ "import Data.Function" + , "g = on" + , "f on = on" + ] + ] + , testGroup + "multi" + [ testOneCodeAction + "hide from B" + "Hide ++ from B" + (2, 2) + (2, 6) + [ "import B" + , "import C" + , "f (++) = (++)" + ] + [ "import B hiding ((++))" + , "import C" + , "f (++) = (++)" + ] + , testOneCodeAction + "hide from C" + "Hide ++ from C" + (2, 2) + (2, 6) + [ "import B" + , "import C" + , "f (++) = (++)" + ] + [ "import B" + , "import C hiding ((++))" + , "f (++) = (++)" + ] + ] + , testOneCodeAction + "hide from Prelude" + "Hide ++ from Prelude" + (2, 2) + (2, 6) + [ "import B" + , "import C" + , "f (++) = (++)" + ] + [ "import B" + , "import C" + , "import Prelude hiding ((++))" + , "f (++) = (++)" + ] + , testMultiCodeActions + "manual hide all" + [ "Hide ++ from Prelude" + , "Hide ++ from C" + , "Hide ++ from B" + ] + (2, 2) + (2, 6) + [ "import B" + , "import C" + , "f (++) = (++)" + ] + [ "import B hiding ((++))" + , "import C hiding ((++))" + , "import Prelude hiding ((++))" + , "f (++) = (++)" + ] + , testOneCodeAction + "auto hide all" + "" + (2, 2) + (2, 6) + [ "import B" + , "import C" + , "f (++) = (++)" + ] + [ "import B hiding ((++))" + , "import C ((++))" + , "import Prelude hiding ((++))" + , "f (++) = (++)" + ] + ] + where + testOneCodeAction testName actionName start end origin expected = + helper testName start end origin expected $ \cas -> do + action <- liftIO $ pickActionWithTitle actionName cas + executeCodeAction action + noCodeAction testName start end origin = + helper testName start end origin origin $ \cas -> do + liftIO $ cas @?= [] + testMultiCodeActions testName actionNames start end origin expected = + helper testName start end origin expected $ \cas -> do + let r = [ca | (CACodeAction ca) <- cas, ca ^. L.title `elem` actionNames] + liftIO $ + (length r == length actionNames) + @? "Expected " <> show actionNames <> ", but got " <> show cas <> " which is not its superset" + forM_ r executeCodeAction + helper testName (line1, col1) (line2, col2) origin expected k = testSession testName $ do + void $ createDoc "B.hs" "haskell" $ T.unlines docB + void $ createDoc "C.hs" "haskell" $ T.unlines docC + doc <- createDoc "A.hs" "haskell" $ T.unlines (header <> origin) + void waitForDiagnostics + void (skipManyTill anyMessage message :: Session WorkDoneProgressEndNotification) + cas <- getCodeActions doc (Range (Position (line1 + length header) col1) (Position (line2 + length header) col2)) + void $ k [x | x@(CACodeAction ca) <- cas, "Hide" `T.isPrefixOf` (ca ^. L.title)] + contentAfter <- documentContents doc + liftIO $ contentAfter @?= T.unlines (header <> expected) + header = + [ "{-# OPTIONS_GHC -Wname-shadowing #-}" + , "module A where" + , "" + ] + -- for multi group + docB = + [ "module B where" + , "(++) = id" + ] + docC = + [ "module C where" + , "(++) = id" + ] + disableWarningTests :: TestTree disableWarningTests = testGroup "disable warnings" $ From 78dcf888489c18637f51aee52709d0bb9dc24e54 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Wed, 10 Feb 2021 18:49:35 +0800 Subject: [PATCH 06/10] Make hlint happy --- ghcide/src/Development/IDE/Plugin/CodeAction.hs | 6 +++--- ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction.hs b/ghcide/src/Development/IDE/Plugin/CodeAction.hs index 7d263b3326..071a8c9dd1 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction.hs @@ -3,7 +3,7 @@ {-# LANGUAGE DuplicateRecordFields #-} {-# LANGUAGE CPP #-} -#include "ghc-api-version.h" + -- | Go to the definition of a variable. module Development.IDE.Plugin.CodeAction @@ -215,7 +215,7 @@ suggestHideShadow pm@(L _ HsModule {hsmodImports}) mTcM mHar Diagnostic {_messag [s'] <- [x | (x, "") <- readSrcSpan $ T.unpack s], isUnusedImportedId tcM har (T.unpack identifier) (T.unpack modName) (RealSrcSpan s'), title <- "Hide " <> identifier <> " from " <> modName - = if modName == "Prelude" && (null $ findImportDeclByModuleName hsmodImports "Prelude") + = if modName == "Prelude" && null (findImportDeclByModuleName hsmodImports "Prelude") then [(title, maybeToList $ hideImplicitPreludeSymbol (T.unpack identifier) pm)] else [(title, hideOrRemoveId hsmodImports (T.unpack identifier) (T.unpack modName))] | otherwise = [] @@ -261,7 +261,7 @@ isUnusedImportedId [GRE {..}] <- lookupGlobalRdrEnv rdrEnv occ, importedIdentifier <- Right gre_name, refs <- M.lookup importedIdentifier rf = - maybe True (null . filter (\(_, IdentifierDetails{..}) -> identInfo == S.singleton Use)) refs + maybe True (not . any (\(_, IdentifierDetails{..}) -> identInfo == S.singleton Use)) refs | otherwise = False suggestDisableWarning :: ParsedModule -> Maybe T.Text -> Diagnostic -> [(T.Text, [TextEdit])] diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs b/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs index 774710ce1f..86c1defd4d 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs @@ -42,6 +42,7 @@ import Development.IDE.Spans.Common import Development.IDE.GHC.Error import Data.Generics (listify) import GHC.Exts (IsList (fromList)) +import Control.Monad.Extra (whenJust) ------------------------------------------------------------------------------ @@ -383,7 +384,7 @@ deleteFromImport (T.pack -> symbol) (L l idecl) llies@(L lieLoc lies) _ =do { ideclHiding = Just (False, edited) } -- avoid import A (foo,) - maybe (pure ()) removeTrailingCommaT $ lastMaybe deletedLies + whenJust (lastMaybe deletedLies) removeTrailingCommaT when (not (null lies) && null deletedLies) $ do transferAnn llies edited id addSimpleAnnT edited dp00 From 12295e4d3ad659d0f37a64de717bb509e95f6377 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Wed, 10 Feb 2021 18:51:57 +0800 Subject: [PATCH 07/10] Fix macro --- ghcide/src/Development/IDE/Plugin/CodeAction.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction.hs b/ghcide/src/Development/IDE/Plugin/CodeAction.hs index 071a8c9dd1..3fc58bfaf2 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction.hs @@ -3,7 +3,7 @@ {-# LANGUAGE DuplicateRecordFields #-} {-# LANGUAGE CPP #-} - +#include "ghc-api-version.h" -- | Go to the definition of a variable. module Development.IDE.Plugin.CodeAction From fe62d196d88dba8298497e4ff4d30aa1e635dc67 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Wed, 10 Feb 2021 20:50:43 +0800 Subject: [PATCH 08/10] Fix a test suite --- ghcide/test/exe/Main.hs | 90 ++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/ghcide/test/exe/Main.hs b/ghcide/test/exe/Main.hs index 8402e61af7..b21492f854 100644 --- a/ghcide/test/exe/Main.hs +++ b/ghcide/test/exe/Main.hs @@ -1676,51 +1676,51 @@ suggestHideShadowTests = , "import C hiding ((++))" , "f (++) = (++)" ] - ] - , testOneCodeAction - "hide from Prelude" - "Hide ++ from Prelude" - (2, 2) - (2, 6) - [ "import B" - , "import C" - , "f (++) = (++)" - ] - [ "import B" - , "import C" - , "import Prelude hiding ((++))" - , "f (++) = (++)" - ] - , testMultiCodeActions - "manual hide all" - [ "Hide ++ from Prelude" - , "Hide ++ from C" - , "Hide ++ from B" - ] - (2, 2) - (2, 6) - [ "import B" - , "import C" - , "f (++) = (++)" - ] - [ "import B hiding ((++))" - , "import C hiding ((++))" - , "import Prelude hiding ((++))" - , "f (++) = (++)" - ] - , testOneCodeAction - "auto hide all" - "" - (2, 2) - (2, 6) - [ "import B" - , "import C" - , "f (++) = (++)" - ] - [ "import B hiding ((++))" - , "import C ((++))" - , "import Prelude hiding ((++))" - , "f (++) = (++)" + , testOneCodeAction + "hide from Prelude" + "Hide ++ from Prelude" + (2, 2) + (2, 6) + [ "import B" + , "import C" + , "f (++) = (++)" + ] + [ "import B" + , "import C" + , "import Prelude hiding ((++))" + , "f (++) = (++)" + ] + , testMultiCodeActions + "manual hide all" + [ "Hide ++ from Prelude" + , "Hide ++ from C" + , "Hide ++ from B" + ] + (2, 2) + (2, 6) + [ "import B" + , "import C" + , "f (++) = (++)" + ] + [ "import B hiding ((++))" + , "import C hiding ((++))" + , "import Prelude hiding ((++))" + , "f (++) = (++)" + ] + , testOneCodeAction + "auto hide all" + "Hide ++ from all occurence imports" + (2, 2) + (2, 6) + [ "import B" + , "import C" + , "f (++) = (++)" + ] + [ "import B hiding ((++))" + , "import C ((++))" + , "import Prelude hiding ((++))" + , "f (++) = (++)" + ] ] ] where From bfc2b31ba957df18af8d95c236d2f4aa9c560c66 Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Thu, 11 Feb 2021 09:30:22 +0800 Subject: [PATCH 09/10] Update test --- ghcide/test/exe/Main.hs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/ghcide/test/exe/Main.hs b/ghcide/test/exe/Main.hs index b21492f854..33c7322de9 100644 --- a/ghcide/test/exe/Main.hs +++ b/ghcide/test/exe/Main.hs @@ -1606,6 +1606,17 @@ suggestHideShadowTests = , "f on = on" , "g on = on" ] + , testOneCodeAction + "extend hiding unsued" + "Hide on from Data.Function" + (1, 2) + (1, 4) + [ "import Data.Function hiding ((&))" + , "f on = on" + ] + [ "import Data.Function hiding (on, (&))" + , "f on = on" + ] , testOneCodeAction "delete unsued" "Hide on from Data.Function" @@ -1717,7 +1728,7 @@ suggestHideShadowTests = , "f (++) = (++)" ] [ "import B hiding ((++))" - , "import C ((++))" + , "import C hiding ((++))" , "import Prelude hiding ((++))" , "f (++) = (++)" ] From 439a09d841cfa0458cc540922b4ca656a88798cb Mon Sep 17 00:00:00 2001 From: Potato Hatsue <1793913507@qq.com> Date: Thu, 11 Feb 2021 14:29:32 +0800 Subject: [PATCH 10/10] Minor refactor --- .../src/Development/IDE/Plugin/CodeAction.hs | 50 ++++++++----------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction.hs b/ghcide/src/Development/IDE/Plugin/CodeAction.hs index 3fc58bfaf2..67a1b5cf0f 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction.hs @@ -190,14 +190,13 @@ findDeclContainingLoc loc = find (\(L l _) -> loc `isInsideSrcSpan` l) -- imported from ‘Data.ByteString’ at B.hs:6:1-22 -- imported from ‘Data.ByteString.Lazy’ at B.hs:8:1-27 -- imported from ‘Data.Text’ at B.hs:7:1-16 - suggestHideShadow :: ParsedSource -> Maybe TcModuleResult -> Maybe HieAstResult -> Diagnostic -> [(T.Text, [Rewrite])] suggestHideShadow pm@(L _ HsModule {hsmodImports}) mTcM mHar Diagnostic {_message, _range} | Just [identifier, modName, s] <- matchRegexUnifySpaces _message - "This binding for ‘([^`]+)’ shadows the existing binding imported from ‘([^`]+)’ at ([^ ]*)" - = suggests identifier modName s + "This binding for ‘([^`]+)’ shadows the existing binding imported from ‘([^`]+)’ at ([^ ]*)" = + suggests identifier modName s | Just [identifier] <- matchRegexUnifySpaces _message @@ -205,27 +204,21 @@ suggestHideShadow pm@(L _ HsModule {hsmodImports}) mTcM mHar Diagnostic {_messag Just matched <- allMatchRegexUnifySpaces _message "imported from ‘([^’]+)’ at ([^ ]*)", mods <- [(modName, s) | [_, modName, s] <- matched], result <- nubOrdBy (compare `on` fst) $ mods >>= uncurry (suggests identifier), - hideAll <- ("Hide " <> identifier <> " from all occurence imports", concat $ snd <$> result) - = result <> [hideAll] + hideAll <- ("Hide " <> identifier <> " from all occurence imports", concat $ snd <$> result) = + result <> [hideAll] | otherwise = [] - where - suggests identifier modName s - | Just tcM <- mTcM, - Just har <- mHar, - [s'] <- [x | (x, "") <- readSrcSpan $ T.unpack s], - isUnusedImportedId tcM har (T.unpack identifier) (T.unpack modName) (RealSrcSpan s'), - title <- "Hide " <> identifier <> " from " <> modName - = if modName == "Prelude" && null (findImportDeclByModuleName hsmodImports "Prelude") - then [(title, maybeToList $ hideImplicitPreludeSymbol (T.unpack identifier) pm)] - else [(title, hideOrRemoveId hsmodImports (T.unpack identifier) (T.unpack modName))] - | otherwise = [] - -hideOrRemoveId :: [LImportDecl GhcPs] -> String -> String -> [Rewrite] -hideOrRemoveId lImportDecls identifier modName - | Just decl <- findImportDeclByModuleName lImportDecls modName = - [hideSymbol identifier decl] - | otherwise = - [] + where + suggests identifier modName s + | Just tcM <- mTcM, + Just har <- mHar, + [s'] <- [x | (x, "") <- readSrcSpan $ T.unpack s], + isUnusedImportedId tcM har (T.unpack identifier) (T.unpack modName) (RealSrcSpan s'), + mDecl <- findImportDeclByModuleName hsmodImports $ T.unpack modName, + title <- "Hide " <> identifier <> " from " <> modName = + if modName == "Prelude" && null mDecl + then [(title, maybeToList $ hideImplicitPreludeSymbol (T.unpack identifier) pm)] + else maybeToList $ (title,) . pure . hideSymbol (T.unpack identifier) <$> mDecl + | otherwise = [] findImportDeclByModuleName :: [LImportDecl GhcPs] -> String -> Maybe (LImportDecl GhcPs) findImportDeclByModuleName decls modName = flip find decls $ \case @@ -238,14 +231,13 @@ isTheSameLine s1 s2 Just sl2 <- getStartLine s2 = sl1 == sl2 | otherwise = False - -getStartLine :: SrcSpan -> Maybe Int -getStartLine x = srcLocLine . realSrcSpanStart <$> realSpan x + where + getStartLine x = srcLocLine . realSrcSpanStart <$> realSpan x isUnusedImportedId :: TcModuleResult -> HieAstResult -> String -> String -> SrcSpan -> Bool isUnusedImportedId TcModuleResult {tmrTypechecked = TcGblEnv {tcg_imports = ImportAvails {imp_mods}}} - HAR {refMap = rf} + HAR {refMap} identifier modName importSpan @@ -260,8 +252,8 @@ isUnusedImportedId ], [GRE {..}] <- lookupGlobalRdrEnv rdrEnv occ, importedIdentifier <- Right gre_name, - refs <- M.lookup importedIdentifier rf = - maybe True (not . any (\(_, IdentifierDetails{..}) -> identInfo == S.singleton Use)) refs + refs <- M.lookup importedIdentifier refMap = + maybe True (not . any (\(_, IdentifierDetails {..}) -> identInfo == S.singleton Use)) refs | otherwise = False suggestDisableWarning :: ParsedModule -> Maybe T.Text -> Diagnostic -> [(T.Text, [TextEdit])]