From 1da9b5ee6ee570eec6a651d1deca0045c5e4be00 Mon Sep 17 00:00:00 2001 From: Hiromi ISHII Date: Tue, 2 Feb 2021 23:16:59 +0900 Subject: [PATCH 1/3] disambiguation: Corrects infix and one-sided section operator qualification --- .../src/Development/IDE/Plugin/CodeAction.hs | 34 +++++++++++++------ ghcide/test/data/hiding/HideQualifyInfix.hs | 5 +++ .../data/hiding/HideQualifyInfix.hs.expected | 5 +++ .../data/hiding/HideQualifySectionLeft.hs | 5 +++ .../hiding/HideQualifySectionLeft.hs.expected | 5 +++ .../data/hiding/HideQualifySectionRight.hs | 5 +++ .../HideQualifySectionRight.hs.expected | 5 +++ ghcide/test/exe/Main.hs | 17 +++++++++- 8 files changed, 69 insertions(+), 12 deletions(-) create mode 100644 ghcide/test/data/hiding/HideQualifyInfix.hs create mode 100644 ghcide/test/data/hiding/HideQualifyInfix.hs.expected create mode 100644 ghcide/test/data/hiding/HideQualifySectionLeft.hs create mode 100644 ghcide/test/data/hiding/HideQualifySectionLeft.hs.expected create mode 100644 ghcide/test/data/hiding/HideQualifySectionRight.hs create mode 100644 ghcide/test/data/hiding/HideQualifySectionRight.hs.expected diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction.hs b/ghcide/src/Development/IDE/Plugin/CodeAction.hs index e966522373..58cad4f31b 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction.hs @@ -132,7 +132,7 @@ suggestAction packageExports ideOptions parsedModule text df annSource diag = [ suggestSignature True diag , rewrite df annSource $ \_ ps -> suggestExtendImport packageExports ps diag , rewrite df annSource $ \df ps -> - suggestImportDisambiguation df ps diag + suggestImportDisambiguation df text ps diag , suggestFillTypeWildcard diag , suggestFixConstructorImport text diag , suggestModuleTypo diag @@ -705,8 +705,12 @@ suggestExtendImport exportsMap (L _ HsModule {hsmodImports}) Diagnostic{_range=_ , parent = Nothing , isDatacon = False} -data HidingMode = HideOthers [ModuleTarget] - | ToQualified ModuleName +data HidingMode + = HideOthers [ModuleTarget] + | ToQualified + Bool + -- ^ Parenthesised? + ModuleName deriving (Show) data ModuleTarget @@ -730,10 +734,11 @@ isPreludeImplicit = xopt Lang.ImplicitPrelude -- | Suggests disambiguation for ambiguous symbols. suggestImportDisambiguation :: DynFlags -> + Maybe T.Text -> ParsedSource -> Diagnostic -> [(T.Text, [Rewrite])] -suggestImportDisambiguation df ps@(L _ HsModule {hsmodImports}) diag@Diagnostic {..} +suggestImportDisambiguation df (Just txt) ps@(L _ HsModule {hsmodImports}) diag@Diagnostic {..} | Just [ambiguous] <- matchRegexUnifySpaces _message @@ -759,7 +764,8 @@ suggestImportDisambiguation df ps@(L _ HsModule {hsmodImports}) diag@Diagnostic = Just $ ImplicitPrelude $ maybe [] NE.toList (Map.lookup "Prelude" locDic) toModuleTarget mName = ExistingImp <$> Map.lookup mName locDic - + parensed = + "(" `T.isPrefixOf` T.strip (textInRange _range txt) suggestions symbol mods | Just targets <- mapM toModuleTarget mods = sortOn fst @@ -771,12 +777,12 @@ suggestImportDisambiguation df ps@(L _ HsModule {hsmodImports}) diag@Diagnostic modNameText = T.pack $ moduleNameString modName , mode <- HideOthers restImports : - [ ToQualified qual + [ ToQualified parensed qual | ExistingImp imps <- [modTarget] , L _ qual <- nubOrd $ mapMaybe (ideclAs . unLoc) $ NE.toList imps ] - ++ [ToQualified modName + ++ [ToQualified parensed modName | any (occursUnqualified symbol . unLoc) (targetImports modTarget) || case modTarget of @@ -787,11 +793,12 @@ suggestImportDisambiguation df ps@(L _ HsModule {hsmodImports}) diag@Diagnostic | otherwise = [] renderUniquify HideOthers {} modName symbol = "Use " <> modName <> " for " <> symbol <> ", hiding other imports" - renderUniquify (ToQualified qual) _ symbol = + renderUniquify (ToQualified _ qual) _ symbol = "Replace with qualified: " <> T.pack (moduleNameString qual) <> "." <> symbol +suggestImportDisambiguation _ _ _ _ = [] occursUnqualified :: T.Text -> ImportDecl GhcPs -> Bool occursUnqualified symbol ImportDecl{..} @@ -832,14 +839,19 @@ disambiguateSymbol pm Diagnostic {..} (T.unpack -> symbol) = \case else hideSymbol symbol <$> imps | ImplicitPrelude imps <- hiddens0 ] - (ToQualified qualMod) -> + (ToQualified parensed qualMod) -> let occSym = mkVarOcc symbol rdr = Qual qualMod occSym - in [ Rewrite (rangeToSrcSpan "" _range) $ \df -> do - liftParseAST @(HsExpr GhcPs) df $ + asExpr = parensed && isSymOcc occSym + in [ if asExpr + then Rewrite (rangeToSrcSpan "" _range) $ \df -> + liftParseAST @(HsExpr GhcPs) df $ prettyPrint $ HsVar @GhcPs noExtField $ L (UnhelpfulSpan "") rdr + else Rewrite (rangeToSrcSpan "" _range) $ \df -> + liftParseAST @RdrName df $ + prettyPrint $ L (UnhelpfulSpan "") rdr ] findImportDeclByRange :: [LImportDecl GhcPs] -> Range -> Maybe (LImportDecl GhcPs) diff --git a/ghcide/test/data/hiding/HideQualifyInfix.hs b/ghcide/test/data/hiding/HideQualifyInfix.hs new file mode 100644 index 0000000000..3fffc4e804 --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifyInfix.hs @@ -0,0 +1,5 @@ +module HideQualifyInfix where + +import AVec + +infixed xs ys = xs ++ ys diff --git a/ghcide/test/data/hiding/HideQualifyInfix.hs.expected b/ghcide/test/data/hiding/HideQualifyInfix.hs.expected new file mode 100644 index 0000000000..151171b2f4 --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifyInfix.hs.expected @@ -0,0 +1,5 @@ +module HideQualifyInfix where + +import AVec + +infixed xs ys = xs Prelude.++ ys diff --git a/ghcide/test/data/hiding/HideQualifySectionLeft.hs b/ghcide/test/data/hiding/HideQualifySectionLeft.hs new file mode 100644 index 0000000000..598c95f191 --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifySectionLeft.hs @@ -0,0 +1,5 @@ +module HideQualifySectionLeft where + +import AVec + +sectLeft xs = (++ xs) diff --git a/ghcide/test/data/hiding/HideQualifySectionLeft.hs.expected b/ghcide/test/data/hiding/HideQualifySectionLeft.hs.expected new file mode 100644 index 0000000000..d029cf54bd --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifySectionLeft.hs.expected @@ -0,0 +1,5 @@ +module HideQualifySectionLeft where + +import AVec + +sectLeft xs = (Prelude.++ xs) diff --git a/ghcide/test/data/hiding/HideQualifySectionRight.hs b/ghcide/test/data/hiding/HideQualifySectionRight.hs new file mode 100644 index 0000000000..89876605c6 --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifySectionRight.hs @@ -0,0 +1,5 @@ +module HideQualifySectionRight where + +import AVec + +sectLeft xs = (xs ++) diff --git a/ghcide/test/data/hiding/HideQualifySectionRight.hs.expected b/ghcide/test/data/hiding/HideQualifySectionRight.hs.expected new file mode 100644 index 0000000000..3f306a4254 --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifySectionRight.hs.expected @@ -0,0 +1,5 @@ +module HideQualifySectionRight where + +import AVec + +sectLeft xs = (xs Prelude.++) diff --git a/ghcide/test/exe/Main.hs b/ghcide/test/exe/Main.hs index d856ec2c5c..780a6535f0 100644 --- a/ghcide/test/exe/Main.hs +++ b/ghcide/test/exe/Main.hs @@ -1539,10 +1539,25 @@ suggestImportDisambiguationTests = testGroup "suggest import disambiguation acti "HideFunction.hs.expected.qualified.fromList.E" ] , testGroup "(++)" - [ testCase "Prelude" $ + [ testCase "Prelude, parensed" $ compareHideFunctionTo [(8,9),(10,8)] "Replace with qualified: Prelude.++" "HideFunction.hs.expected.qualified.append.Prelude" + , testCase "Prelude, infix" $ + compareTwo + "HideQualifyInfix.hs" [(4,19)] + "Replace with qualified: Prelude.++" + "HideQualifyInfix.hs.expected" + , testCase "Prelude, left section" $ + compareTwo + "HideQualifySectionLeft.hs" [(4,15)] + "Replace with qualified: Prelude.++" + "HideQualifySectionLeft.hs.expected" + , testCase "Prelude, right section" $ + compareTwo + "HideQualifySectionRight.hs" [(4,18)] + "Replace with qualified: Prelude.++" + "HideQualifySectionRight.hs.expected" ] ] ] From fec9fec7cc678d3905f07677a1a88b003971479d Mon Sep 17 00:00:00 2001 From: Hiromi ISHII Date: Wed, 3 Feb 2021 14:38:03 +0900 Subject: [PATCH 2/3] Renames `expected` files to have `.hs` extension --- ...nd.E => HideFunction.expected.append.E.hs} | 0 ...> HideFunction.expected.append.Prelude.hs} | 0 ....A => HideFunction.expected.fromList.A.hs} | 0 ....B => HideFunction.expected.fromList.B.hs} | 0 ...tion.expected.qualified.append.Prelude.hs} | 0 ...Function.expected.qualified.fromList.E.hs} | 0 ...pected => HidePreludeIndented.expected.hs} | 0 ....expected => HideQualifyInfix.expected.hs} | 0 ...ted => HideQualifySectionLeft.expected.hs} | 0 ...ed => HideQualifySectionRight.expected.hs} | 0 ...e.hs.expected.A => HideType.expected.A.hs} | 0 ...e.hs.expected.E => HideType.expected.E.hs} | 0 ghcide/test/exe/Main.hs | 24 +++++++++---------- 13 files changed, 12 insertions(+), 12 deletions(-) rename ghcide/test/data/hiding/{HideFunction.hs.expected.append.E => HideFunction.expected.append.E.hs} (100%) rename ghcide/test/data/hiding/{HideFunction.hs.expected.append.Prelude => HideFunction.expected.append.Prelude.hs} (100%) rename ghcide/test/data/hiding/{HideFunction.hs.expected.fromList.A => HideFunction.expected.fromList.A.hs} (100%) rename ghcide/test/data/hiding/{HideFunction.hs.expected.fromList.B => HideFunction.expected.fromList.B.hs} (100%) rename ghcide/test/data/hiding/{HideFunction.hs.expected.qualified.append.Prelude => HideFunction.expected.qualified.append.Prelude.hs} (100%) rename ghcide/test/data/hiding/{HideFunction.hs.expected.qualified.fromList.E => HideFunction.expected.qualified.fromList.E.hs} (100%) rename ghcide/test/data/hiding/{HidePreludeIndented.hs.expected => HidePreludeIndented.expected.hs} (100%) rename ghcide/test/data/hiding/{HideQualifyInfix.hs.expected => HideQualifyInfix.expected.hs} (100%) rename ghcide/test/data/hiding/{HideQualifySectionLeft.hs.expected => HideQualifySectionLeft.expected.hs} (100%) rename ghcide/test/data/hiding/{HideQualifySectionRight.hs.expected => HideQualifySectionRight.expected.hs} (100%) rename ghcide/test/data/hiding/{HideType.hs.expected.A => HideType.expected.A.hs} (100%) rename ghcide/test/data/hiding/{HideType.hs.expected.E => HideType.expected.E.hs} (100%) diff --git a/ghcide/test/data/hiding/HideFunction.hs.expected.append.E b/ghcide/test/data/hiding/HideFunction.expected.append.E.hs similarity index 100% rename from ghcide/test/data/hiding/HideFunction.hs.expected.append.E rename to ghcide/test/data/hiding/HideFunction.expected.append.E.hs diff --git a/ghcide/test/data/hiding/HideFunction.hs.expected.append.Prelude b/ghcide/test/data/hiding/HideFunction.expected.append.Prelude.hs similarity index 100% rename from ghcide/test/data/hiding/HideFunction.hs.expected.append.Prelude rename to ghcide/test/data/hiding/HideFunction.expected.append.Prelude.hs diff --git a/ghcide/test/data/hiding/HideFunction.hs.expected.fromList.A b/ghcide/test/data/hiding/HideFunction.expected.fromList.A.hs similarity index 100% rename from ghcide/test/data/hiding/HideFunction.hs.expected.fromList.A rename to ghcide/test/data/hiding/HideFunction.expected.fromList.A.hs diff --git a/ghcide/test/data/hiding/HideFunction.hs.expected.fromList.B b/ghcide/test/data/hiding/HideFunction.expected.fromList.B.hs similarity index 100% rename from ghcide/test/data/hiding/HideFunction.hs.expected.fromList.B rename to ghcide/test/data/hiding/HideFunction.expected.fromList.B.hs diff --git a/ghcide/test/data/hiding/HideFunction.hs.expected.qualified.append.Prelude b/ghcide/test/data/hiding/HideFunction.expected.qualified.append.Prelude.hs similarity index 100% rename from ghcide/test/data/hiding/HideFunction.hs.expected.qualified.append.Prelude rename to ghcide/test/data/hiding/HideFunction.expected.qualified.append.Prelude.hs diff --git a/ghcide/test/data/hiding/HideFunction.hs.expected.qualified.fromList.E b/ghcide/test/data/hiding/HideFunction.expected.qualified.fromList.E.hs similarity index 100% rename from ghcide/test/data/hiding/HideFunction.hs.expected.qualified.fromList.E rename to ghcide/test/data/hiding/HideFunction.expected.qualified.fromList.E.hs diff --git a/ghcide/test/data/hiding/HidePreludeIndented.hs.expected b/ghcide/test/data/hiding/HidePreludeIndented.expected.hs similarity index 100% rename from ghcide/test/data/hiding/HidePreludeIndented.hs.expected rename to ghcide/test/data/hiding/HidePreludeIndented.expected.hs diff --git a/ghcide/test/data/hiding/HideQualifyInfix.hs.expected b/ghcide/test/data/hiding/HideQualifyInfix.expected.hs similarity index 100% rename from ghcide/test/data/hiding/HideQualifyInfix.hs.expected rename to ghcide/test/data/hiding/HideQualifyInfix.expected.hs diff --git a/ghcide/test/data/hiding/HideQualifySectionLeft.hs.expected b/ghcide/test/data/hiding/HideQualifySectionLeft.expected.hs similarity index 100% rename from ghcide/test/data/hiding/HideQualifySectionLeft.hs.expected rename to ghcide/test/data/hiding/HideQualifySectionLeft.expected.hs diff --git a/ghcide/test/data/hiding/HideQualifySectionRight.hs.expected b/ghcide/test/data/hiding/HideQualifySectionRight.expected.hs similarity index 100% rename from ghcide/test/data/hiding/HideQualifySectionRight.hs.expected rename to ghcide/test/data/hiding/HideQualifySectionRight.expected.hs diff --git a/ghcide/test/data/hiding/HideType.hs.expected.A b/ghcide/test/data/hiding/HideType.expected.A.hs similarity index 100% rename from ghcide/test/data/hiding/HideType.hs.expected.A rename to ghcide/test/data/hiding/HideType.expected.A.hs diff --git a/ghcide/test/data/hiding/HideType.hs.expected.E b/ghcide/test/data/hiding/HideType.expected.E.hs similarity index 100% rename from ghcide/test/data/hiding/HideType.hs.expected.E rename to ghcide/test/data/hiding/HideType.expected.E.hs diff --git a/ghcide/test/exe/Main.hs b/ghcide/test/exe/Main.hs index 780a6535f0..cf117b05c8 100644 --- a/ghcide/test/exe/Main.hs +++ b/ghcide/test/exe/Main.hs @@ -1483,25 +1483,25 @@ suggestImportDisambiguationTests = testGroup "suggest import disambiguation acti [ testCase "AVec" $ compareHideFunctionTo [(8,9),(10,8)] "Use AVec for fromList, hiding other imports" - "HideFunction.hs.expected.fromList.A" + "HideFunction.expected.fromList.A.hs" , testCase "BVec" $ compareHideFunctionTo [(8,9),(10,8)] "Use BVec for fromList, hiding other imports" - "HideFunction.hs.expected.fromList.B" + "HideFunction.expected.fromList.B.hs" ] , testGroup "(++)" [ testCase "EVec" $ compareHideFunctionTo [(8,9),(10,8)] "Use EVec for ++, hiding other imports" - "HideFunction.hs.expected.append.E" + "HideFunction.expected.append.E.hs" , testCase "Prelude" $ compareHideFunctionTo [(8,9),(10,8)] "Use Prelude for ++, hiding other imports" - "HideFunction.hs.expected.append.Prelude" + "HideFunction.expected.append.Prelude.hs" , testCase "AVec, indented" $ compareTwo "HidePreludeIndented.hs" [(3,8)] "Use AVec for ++, hiding other imports" - "HidePreludeIndented.hs.expected" + "HidePreludeIndented.expected.hs" ] , testGroup "Vec (type)" @@ -1509,12 +1509,12 @@ suggestImportDisambiguationTests = testGroup "suggest import disambiguation acti compareTwo "HideType.hs" [(8,15)] "Use AVec for Vec, hiding other imports" - "HideType.hs.expected.A" + "HideType.expected.A.hs" , testCase "EVec" $ compareTwo "HideType.hs" [(8,15)] "Use EVec for Vec, hiding other imports" - "HideType.hs.expected.E" + "HideType.expected.E.hs" ] ] , testGroup "Qualify strategy" @@ -1536,28 +1536,28 @@ suggestImportDisambiguationTests = testGroup "suggest import disambiguation acti [ testCase "EVec" $ compareHideFunctionTo [(8,9),(10,8)] "Replace with qualified: E.fromList" - "HideFunction.hs.expected.qualified.fromList.E" + "HideFunction.expected.qualified.fromList.E.hs" ] , testGroup "(++)" [ testCase "Prelude, parensed" $ compareHideFunctionTo [(8,9),(10,8)] "Replace with qualified: Prelude.++" - "HideFunction.hs.expected.qualified.append.Prelude" + "HideFunction.expected.qualified.append.Prelude.hs" , testCase "Prelude, infix" $ compareTwo "HideQualifyInfix.hs" [(4,19)] "Replace with qualified: Prelude.++" - "HideQualifyInfix.hs.expected" + "HideQualifyInfix.expected.hs" , testCase "Prelude, left section" $ compareTwo "HideQualifySectionLeft.hs" [(4,15)] "Replace with qualified: Prelude.++" - "HideQualifySectionLeft.hs.expected" + "HideQualifySectionLeft.expected.hs" , testCase "Prelude, right section" $ compareTwo "HideQualifySectionRight.hs" [(4,18)] "Replace with qualified: Prelude.++" - "HideQualifySectionRight.hs.expected" + "HideQualifySectionRight.expected.hs" ] ] ] From 932e0c4f8cca27bc120b9425c880049ee5cd5250 Mon Sep 17 00:00:00 2001 From: Hiromi ISHII Date: Wed, 3 Feb 2021 15:14:57 +0900 Subject: [PATCH 3/3] Just use parensed only --- ghcide/src/Development/IDE/Plugin/CodeAction.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction.hs b/ghcide/src/Development/IDE/Plugin/CodeAction.hs index 58cad4f31b..08e79e703b 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction.hs @@ -842,8 +842,7 @@ disambiguateSymbol pm Diagnostic {..} (T.unpack -> symbol) = \case (ToQualified parensed qualMod) -> let occSym = mkVarOcc symbol rdr = Qual qualMod occSym - asExpr = parensed && isSymOcc occSym - in [ if asExpr + in [ if parensed then Rewrite (rangeToSrcSpan "" _range) $ \df -> liftParseAST @(HsExpr GhcPs) df $ prettyPrint $