Import disambiguation: Corrects handling of fully-applied and one-sided sectioned operators in qualifying strategy (#1294)

* disambiguation: Corrects infix and one-sided section operator qualification

* Renames `expected` files to have `.hs` extension

* Just use parensed only

Author: konn

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,18 @@ 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 "<dummy>" _range) $ \df -> do
-                liftParseAST @(HsExpr GhcPs) df $
+         in [ if parensed
+                then Rewrite (rangeToSrcSpan "<dummy>" _range) $ \df ->
+                    liftParseAST @(HsExpr GhcPs) df $
                     prettyPrint $
                         HsVar @GhcPs noExtField $
                             L (UnhelpfulSpan "") rdr
+                else Rewrite (rangeToSrcSpan "<dummy>" _range) $ \df ->
+                    liftParseAST @RdrName df $
+                    prettyPrint $ L (UnhelpfulSpan "") rdr
             ]
 
 findImportDeclByRange :: [LImportDecl GhcPs] -> Range -> Maybe (LImportDecl GhcPs)

ghcide/test/data/hiding/HideFunction.hs.expected.append.E renamed to ghcide/test/data/hiding/HideFunction.expected.append.E.hs

@@ -0,0 +1,5 @@
+module HideQualifyInfix where
+
+import AVec
+
+infixed xs ys = xs Prelude.++ ys

ghcide/test/data/hiding/HideFunction.hs.expected.append.Prelude renamed to ghcide/test/data/hiding/HideFunction.expected.append.Prelude.hs

@@ -0,0 +1,5 @@
+module HideQualifyInfix where
+
+import AVec
+
+infixed xs ys = xs ++ ys

ghcide/test/data/hiding/HideFunction.hs.expected.fromList.A renamed to ghcide/test/data/hiding/HideFunction.expected.fromList.A.hs

@@ -0,0 +1,5 @@
+module HideQualifySectionLeft where
+
+import AVec
+
+sectLeft xs = (Prelude.++ xs)

ghcide/test/data/hiding/HideFunction.hs.expected.fromList.B renamed to ghcide/test/data/hiding/HideFunction.expected.fromList.B.hs

@@ -0,0 +1,5 @@
+module HideQualifySectionLeft where
+
+import AVec
+
+sectLeft xs = (++ xs)

ghcide/test/data/hiding/HideFunction.hs.expected.qualified.append.Prelude renamed to ghcide/test/data/hiding/HideFunction.expected.qualified.append.Prelude.hs

@@ -0,0 +1,5 @@
+module HideQualifySectionRight where
+
+import AVec
+
+sectLeft xs = (xs Prelude.++)

ghcide/test/data/hiding/HideFunction.hs.expected.qualified.fromList.E renamed to ghcide/test/data/hiding/HideFunction.expected.qualified.fromList.E.hs

@@ -0,0 +1,5 @@
+module HideQualifySectionRight where
+
+import AVec
+
+sectLeft xs = (xs ++)

ghcide/test/data/hiding/HidePreludeIndented.hs.expected renamed to ghcide/test/data/hiding/HidePreludeIndented.expected.hs

@@ -1483,38 +1483,38 @@ 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)"
         [ testCase "AVec" $
             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,13 +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" $
+        [ 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.expected.hs"
+        , testCase "Prelude, left section" $
+            compareTwo
+                "HideQualifySectionLeft.hs" [(4,15)]
+                "Replace with qualified: Prelude.++"
+                "HideQualifySectionLeft.expected.hs"
+        , testCase "Prelude, right section" $
+            compareTwo
+                "HideQualifySectionRight.hs" [(4,18)]
+                "Replace with qualified: Prelude.++"
+                "HideQualifySectionRight.expected.hs"
         ]
     ]
   ]