From 6fce45445107d2f7382de3f6b28c7993940fdb34 Mon Sep 17 00:00:00 2001 From: Lei Zhu Date: Mon, 5 Apr 2021 16:46:11 +0800 Subject: [PATCH] optimize ambiguity import suggestions (#1669) --- ghcide/ghcide.cabal | 1 + .../src/Development/IDE/Plugin/CodeAction.hs | 54 +++++++++++-------- ghcide/test/data/hiding/FVec.hs | 9 ++++ ...deQualifyDuplicateRecordFields.expected.hs | 10 ++++ .../HideQualifyDuplicateRecordFields.hs | 10 ++++ .../HideQualifyDuplicateRecordFieldsSelf.hs | 5 ++ ghcide/test/exe/Main.hs | 36 +++++++++---- 7 files changed, 93 insertions(+), 32 deletions(-) create mode 100644 ghcide/test/data/hiding/FVec.hs create mode 100644 ghcide/test/data/hiding/HideQualifyDuplicateRecordFields.expected.hs create mode 100644 ghcide/test/data/hiding/HideQualifyDuplicateRecordFields.hs create mode 100644 ghcide/test/data/hiding/HideQualifyDuplicateRecordFieldsSelf.hs diff --git a/ghcide/ghcide.cabal b/ghcide/ghcide.cabal index 4cfc57a1e4..f327093eb8 100644 --- a/ghcide/ghcide.cabal +++ b/ghcide/ghcide.cabal @@ -346,6 +346,7 @@ test-suite ghcide-tests QuickCheck, quickcheck-instances, rope-utf16-splay, + regex-tdfa ^>= 1.3.1, safe, safe-exceptions, shake, diff --git a/ghcide/src/Development/IDE/Plugin/CodeAction.hs b/ghcide/src/Development/IDE/Plugin/CodeAction.hs index 994e502d3e..59c49bc740 100644 --- a/ghcide/src/Development/IDE/Plugin/CodeAction.hs +++ b/ghcide/src/Development/IDE/Plugin/CodeAction.hs @@ -848,31 +848,39 @@ suggestImportDisambiguation df (Just txt) ps@(L _ HsModule {hsmodImports}) diag@ toModuleTarget mName = ExistingImp <$> Map.lookup mName locDic parensed = "(" `T.isPrefixOf` T.strip (textInRange _range txt) + -- > removeAllDuplicates [1, 1, 2, 3, 2] = [3] + removeAllDuplicates = map head . filter ((==1) <$> length) . group . sort + hasDuplicate xs = length xs /= length (S.fromList xs) suggestions symbol mods - | Just targets <- mapM toModuleTarget mods = - sortOn fst - [ ( renderUniquify mode modNameText symbol - , disambiguateSymbol ps diag symbol mode - ) - | (modTarget, restImports) <- oneAndOthers targets - , let modName = targetModuleName modTarget - modNameText = T.pack $ moduleNameString modName - , mode <- - HideOthers restImports : - [ ToQualified parensed qual - | ExistingImp imps <- [modTarget] - , L _ qual <- nubOrd $ mapMaybe (ideclAs . unLoc) - $ NE.toList imps - ] - ++ [ToQualified parensed modName - | any (occursUnqualified symbol . unLoc) - (targetImports modTarget) - || case modTarget of - ImplicitPrelude{} -> True - _ -> False - ] + | hasDuplicate mods = case mapM toModuleTarget (removeAllDuplicates mods) of + Just targets -> suggestionsImpl symbol (map (, []) targets) + Nothing -> [] + | otherwise = case mapM toModuleTarget mods of + Just targets -> suggestionsImpl symbol (oneAndOthers targets) + Nothing -> [] + suggestionsImpl symbol targetsWithRestImports = + sortOn fst + [ ( renderUniquify mode modNameText symbol + , disambiguateSymbol ps diag symbol mode + ) + | (modTarget, restImports) <- targetsWithRestImports + , let modName = targetModuleName modTarget + modNameText = T.pack $ moduleNameString modName + , mode <- + [ ToQualified parensed qual + | ExistingImp imps <- [modTarget] + , L _ qual <- nubOrd $ mapMaybe (ideclAs . unLoc) + $ NE.toList imps ] - | otherwise = [] + ++ [ToQualified parensed modName + | any (occursUnqualified symbol . unLoc) + (targetImports modTarget) + || case modTarget of + ImplicitPrelude{} -> True + _ -> False + ] + ++ [HideOthers restImports | not (null restImports)] + ] renderUniquify HideOthers {} modName symbol = "Use " <> modName <> " for " <> symbol <> ", hiding other imports" renderUniquify (ToQualified _ qual) _ symbol = diff --git a/ghcide/test/data/hiding/FVec.hs b/ghcide/test/data/hiding/FVec.hs new file mode 100644 index 0000000000..872bb1c373 --- /dev/null +++ b/ghcide/test/data/hiding/FVec.hs @@ -0,0 +1,9 @@ +{-# LANGUAGE DuplicateRecordFields #-} + +module FVec (RecA(..), RecB(..)) where + +data Vec a + +newtype RecA a = RecA { fromList :: [a] -> Vec a } + +newtype RecB a = RecB { fromList :: [a] -> Vec a } diff --git a/ghcide/test/data/hiding/HideQualifyDuplicateRecordFields.expected.hs b/ghcide/test/data/hiding/HideQualifyDuplicateRecordFields.expected.hs new file mode 100644 index 0000000000..c41fae58c1 --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifyDuplicateRecordFields.expected.hs @@ -0,0 +1,10 @@ +module HideQualifyDuplicateRecordFields where + +import AVec +import BVec +import CVec +import DVec +import EVec +import FVec + +theFun = AVec.fromList \ No newline at end of file diff --git a/ghcide/test/data/hiding/HideQualifyDuplicateRecordFields.hs b/ghcide/test/data/hiding/HideQualifyDuplicateRecordFields.hs new file mode 100644 index 0000000000..7e3e16cd9f --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifyDuplicateRecordFields.hs @@ -0,0 +1,10 @@ +module HideQualifyDuplicateRecordFields where + +import AVec +import BVec +import CVec +import DVec +import EVec +import FVec + +theFun = fromList \ No newline at end of file diff --git a/ghcide/test/data/hiding/HideQualifyDuplicateRecordFieldsSelf.hs b/ghcide/test/data/hiding/HideQualifyDuplicateRecordFieldsSelf.hs new file mode 100644 index 0000000000..63e14db00a --- /dev/null +++ b/ghcide/test/data/hiding/HideQualifyDuplicateRecordFieldsSelf.hs @@ -0,0 +1,5 @@ +module HideQualifyDuplicateRecordFieldsSelf where + +import FVec + +x = fromList \ No newline at end of file diff --git a/ghcide/test/exe/Main.hs b/ghcide/test/exe/Main.hs index f724374bde..712237a101 100644 --- a/ghcide/test/exe/Main.hs +++ b/ghcide/test/exe/Main.hs @@ -93,14 +93,15 @@ import Test.Tasty.ExpectedFailure import Test.Tasty.HUnit import Test.Tasty.Ingredients.Rerun import Test.Tasty.QuickCheck -import Data.IORef -import Ide.PluginUtils (pluginDescToIdePlugins) -import Control.Concurrent.Async -import Ide.Types -import Data.String (IsString(fromString)) -import qualified Language.LSP.Types as LSP -import Data.IORef.Extra (atomicModifyIORef_) -import qualified Development.IDE.Plugin.HLS.GhcIde as Ghcide +import Data.IORef +import Ide.PluginUtils (pluginDescToIdePlugins) +import Control.Concurrent.Async +import Ide.Types +import Data.String (IsString(fromString)) +import qualified Language.LSP.Types as LSP +import Data.IORef.Extra (atomicModifyIORef_) +import qualified Development.IDE.Plugin.HLS.GhcIde as Ghcide +import Text.Regex.TDFA ((=~)) waitForProgressBegin :: Session () waitForProgressBegin = skipManyTill anyMessage $ satisfyMaybe $ \case @@ -1673,6 +1674,23 @@ suggestImportDisambiguationTests = testGroup "suggest import disambiguation acti compareHideFunctionTo [(8,9),(10,8)] "Replace with qualified: E.fromList" "HideFunction.expected.qualified.fromList.E.hs" + , testCase "Hide DuplicateRecordFields" $ + compareTwo + "HideQualifyDuplicateRecordFields.hs" [(9, 9)] + "Replace with qualified: AVec.fromList" + "HideQualifyDuplicateRecordFields.expected.hs" + , testCase "Duplicate record fields should not be imported" $ do + withTarget ("HideQualifyDuplicateRecordFields" <.> ".hs") [(9, 9)] $ + \_ actions -> do + liftIO $ + assertBool "Hidings should not be presented while DuplicateRecordFields exists" $ + all not [ actionTitle =~ T.pack "Use ([A-Za-z][A-Za-z0-9]*) for fromList, hiding other imports" + | InR CodeAction { _title = actionTitle } <- actions] + withTarget ("HideQualifyDuplicateRecordFieldsSelf" <.> ".hs") [(4, 4)] $ + \_ actions -> do + liftIO $ + assertBool "ambiguity from DuplicateRecordFields should not be imported" $ + null actions ] , testGroup "(++)" [ testCase "Prelude, parensed" $ @@ -1708,7 +1726,7 @@ suggestImportDisambiguationTests = testGroup "suggest import disambiguation acti contentAfterAction <- documentContents doc liftIO $ T.replace "\r\n" "\n" expected @=? contentAfterAction compareHideFunctionTo = compareTwo "HideFunction.hs" - auxFiles = ["AVec.hs", "BVec.hs", "CVec.hs", "DVec.hs", "EVec.hs"] + auxFiles = ["AVec.hs", "BVec.hs", "CVec.hs", "DVec.hs", "EVec.hs", "FVec.hs"] withTarget file locs k = withTempDir $ \dir -> runInDir dir $ do liftIO $ mapM_ (\fp -> copyFile (hidingDir fp) $ dir fp) $ file : auxFiles