From b830b6bcb19e15134ff93bae528af07cb92aea12 Mon Sep 17 00:00:00 2001 From: Nathan Maxson Date: Thu, 14 Sep 2023 18:42:39 +0300 Subject: [PATCH 1/5] change refine imports for qualified imports --- .../src/Ide/Plugin/ExplicitImports.hs | 12 ++++++++---- plugins/hls-explicit-imports-plugin/test/Main.hs | 1 + .../test/testdata/RefineQualified.expected.hs | 11 +++++++++++ .../test/testdata/RefineQualified.hs | 10 ++++++++++ .../test/testdata/hie.yaml | 2 +- 5 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 plugins/hls-explicit-imports-plugin/test/testdata/RefineQualified.expected.hs create mode 100644 plugins/hls-explicit-imports-plugin/test/testdata/RefineQualified.hs diff --git a/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs b/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs index 13864da29a..00e6e63e8d 100644 --- a/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs +++ b/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs @@ -479,16 +479,20 @@ filterByImport _ _ = Nothing constructImport :: LImportDecl GhcRn -> (ModuleName, [AvailInfo]) -> LImportDecl GhcRn #if MIN_VERSION_ghc(9,5,0) -constructImport (L lim imd@ImportDecl {ideclName = L _ _, ideclImportList = Just (hiding, L _ names)}) +constructImport (L lim imd@ImportDecl {ideclName = L _ _, ideclQualified = qualified, ideclImportList = Just (hiding, L _ names)}) #else -constructImport (L lim imd@ImportDecl{ideclName = L _ _, ideclHiding = Just (hiding, L _ names)}) +constructImport (L lim imd@ImportDecl{ideclName = L _ _, ideclQualified = qualified, ideclHiding = Just (hiding, L _ names)}) #endif (newModuleName, avails) = L lim imd { ideclName = noLocA newModuleName #if MIN_VERSION_ghc(9,5,0) - , ideclImportList = Just (hiding, noLocA newNames) + , ideclImportList = if hiding == Exactly && qualified /= NotQualified + then Nothing + else Just (hiding, noLocA newNames) #else - , ideclHiding = Just (hiding, noLocA newNames) + , ideclHiding = if hiding == Exactly && qualified /= NotQualified + then Nothing + else Just (hiding, noLocA newNames) #endif } where newNames = filter (\n -> any (n `containsAvail`) avails) names diff --git a/plugins/hls-explicit-imports-plugin/test/Main.hs b/plugins/hls-explicit-imports-plugin/test/Main.hs index 9cc1fb482b..71aef3f1c2 100644 --- a/plugins/hls-explicit-imports-plugin/test/Main.hs +++ b/plugins/hls-explicit-imports-plugin/test/Main.hs @@ -31,6 +31,7 @@ main = defaultTestRunner $ testGroup "import-actions" "Refine Imports" [ codeActionGoldenTest "RefineWithOverride" 3 1 , codeLensGoldenTest isRefineImports "RefineUsualCase" 1 + , codeLensGoldenTest isRefineImports "RefineQualified" 1 ], testGroup "Make imports explicit" diff --git a/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualified.expected.hs b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualified.expected.hs new file mode 100644 index 0000000000..e94827bccd --- /dev/null +++ b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualified.expected.hs @@ -0,0 +1,11 @@ +module Main where + +import qualified RefineB as RA +import qualified RefineC as RA +import RefineD +import Data.List (intercalate) + +main :: IO () +main = putStrLn + $ "hello " + <> intercalate ", " [RA.b1, RA.c1, e2] diff --git a/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualified.hs b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualified.hs new file mode 100644 index 0000000000..78e7f4566b --- /dev/null +++ b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualified.hs @@ -0,0 +1,10 @@ +module Main where + +import qualified RefineA as RA +import RefineD +import Data.List (intercalate) + +main :: IO () +main = putStrLn + $ "hello " + <> intercalate ", " [RA.b1, RA.c1, e2] diff --git a/plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml b/plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml index 42efdf316e..04f19891ef 100644 --- a/plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml +++ b/plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml @@ -1,4 +1,3 @@ - cradle: direct: arguments: @@ -9,6 +8,7 @@ cradle: - ExplicitA.hs - ExplicitB.hs - RefineUsualCase.hs + - RefineQualified.hs - RefineWithOverride.hs - RefineA.hs - RefineB.hs From 53722805b5dd387c69b1dcf7c9b8c2e961cc5e15 Mon Sep 17 00:00:00 2001 From: Nathan Maxson Date: Fri, 15 Sep 2023 21:11:34 +0300 Subject: [PATCH 2/5] Fix ghc compat, attempt edge case fix --- .../src/Ide/Plugin/ExplicitImports.hs | 65 ++++++++++--------- .../hls-explicit-imports-plugin/test/Main.hs | 3 +- .../RefineQualifiedExplicit.expected.hs | 11 ++++ .../test/testdata/RefineQualifiedExplicit.hs | 10 +++ 4 files changed, 56 insertions(+), 33 deletions(-) create mode 100644 plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.expected.hs create mode 100644 plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.hs diff --git a/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs b/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs index a81ac20166..b07354db0d 100644 --- a/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs +++ b/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs @@ -316,42 +316,32 @@ minimalImportsRule recorder modFilter = defineNoDiagnostics (cmapWithPrio LogSha return $ mi_exports $ hirModIface imp_hir -- Use the GHC api to extract the "minimal" imports - (imports, mbMinImports) <- MaybeT $ liftIO $ extractMinimalImports hsc tmr + locationImportWithMinimal <- MaybeT $ liftIO $ extractMinimalImports hsc tmr - let importsMap = - Map.fromList - [ (realSrcSpanStart l, printOutputable i) - | L (locA -> RealSrcSpan l _) i <- mbMinImports - ] - minimalImportsResult = - [ (range, (minImport, ExplicitImport)) - | imp@(L _ impDecl) <- imports + let minimalImportsResult = + [ (range, (printOutputable minImport, ExplicitImport)) + | (location, impDecl, minImport) <- locationImportWithMinimal , not (isQualifiedImport impDecl) , not (isExplicitImport impDecl) , let L _ moduleName = ideclName impDecl , modFilter moduleName - , RealSrcSpan location _ <- [getLoc imp] - , let range = realSrcSpanToRange location - , Just minImport <- [Map.lookup (realSrcSpanStart location) importsMap] - ] + , let range = realSrcSpanToRange location] + refineImportsResult = [ (range, (T.intercalate "\n" - . map (printOutputable . constructImport i) + . map (printOutputable . constructImport origImport minImport) . Map.toList $ filteredInnerImports, RefineImport)) -- for every minimal imports - | minImports <- [mbMinImports] - , i@(L _ ImportDecl{ideclName = L _ mn}) <- minImports + | (location, origImport, minImport@(ImportDecl{ideclName = L _ mn})) <- locationImportWithMinimal -- (almost) no one wants to see an refine import list for Prelude , mn /= moduleName pRELUDE -- we check for the inner imports , Just innerImports <- [Map.lookup mn import2Map] -- and only get those symbols used - , Just filteredInnerImports <- [filterByImport i innerImports] + , Just filteredInnerImports <- [filterByImport minImport innerImports] -- if no symbols from this modules then don't need to generate new import , not $ null filteredInnerImports - -- get the location - , RealSrcSpan location _ <- [getLoc i] -- and then convert that to a Range , let range = realSrcSpanToRange location ] @@ -370,7 +360,7 @@ minimalImportsRule recorder modFilter = defineNoDiagnostics (cmapWithPrio LogSha extractMinimalImports :: HscEnvEq -> TcModuleResult -> - IO (Maybe ([LImportDecl GhcRn], [LImportDecl GhcRn])) + IO (Maybe [(RealSrcSpan, ImportDecl GhcRn, ImportDecl GhcRn)]) extractMinimalImports hsc TcModuleResult {..} = runMaybeT $ do -- extract the original imports and the typechecking environment let tcEnv = tmrTypechecked @@ -391,8 +381,17 @@ extractMinimalImports hsc TcModuleResult {..} = runMaybeT $ do (_, Just minimalImports) <- liftIO $ initTcWithGbl (hscEnv hsc) tcEnv srcSpan $ getMinimalImports usage + let minimalImportsMap = + Map.fromList + [ (realSrcSpanStart l, impDecl) + | L (locA -> RealSrcSpan l _) impDecl <- minimalImports + ] + results = + [ (location, imp, minImport) + | L (locA -> RealSrcSpan location _) imp <- imports + , Just minImport <- [Map.lookup (realSrcSpanStart location) minimalImportsMap]] -- return both the original imports and the computed minimal ones - return (imports, minimalImports) + return results where notExported :: [String] -> LImportDecl GhcRn -> Bool notExported [] _ = True @@ -451,11 +450,11 @@ abbreviateImportTitle input = -------------------------------------------------------------------------------- -filterByImport :: LImportDecl GhcRn -> Map.Map ModuleName [AvailInfo] -> Maybe (Map.Map ModuleName [AvailInfo]) +filterByImport :: ImportDecl GhcRn -> Map.Map ModuleName [AvailInfo] -> Maybe (Map.Map ModuleName [AvailInfo]) #if MIN_VERSION_ghc(9,5,0) -filterByImport (L _ ImportDecl{ideclImportList = Just (_, L _ names)}) +filterByImport (ImportDecl{ideclImportList = Just (_, L _ names)}) #else -filterByImport (L _ ImportDecl{ideclHiding = Just (_, L _ names)}) +filterByImport (ImportDecl{ideclHiding = Just (_, L _ names)}) #endif avails = -- if there is a function defined in the current module and is used @@ -474,20 +473,20 @@ filterByImport (L _ ImportDecl{ideclHiding = Just (_, L _ names)}) $ Map.elems res filterByImport _ _ = Nothing -constructImport :: LImportDecl GhcRn -> (ModuleName, [AvailInfo]) -> LImportDecl GhcRn +constructImport :: ImportDecl GhcRn -> ImportDecl GhcRn -> (ModuleName, [AvailInfo]) -> ImportDecl GhcRn #if MIN_VERSION_ghc(9,5,0) -constructImport (L lim imd@ImportDecl {ideclName = L _ _, ideclQualified = qualified, ideclImportList = Just (hiding, L _ names)}) +constructImport ImportDecl{ideclQualified = qualified, ideclHiding = origHiding} imd@ImportDecl{ideclImportList = Just (hiding, L _ names)} #else -constructImport (L lim imd@ImportDecl{ideclName = L _ _, ideclQualified = qualified, ideclHiding = Just (hiding, L _ names)}) +constructImport ImportDecl{ideclQualified = qualified, ideclHiding = origHiding} imd@ImportDecl{ideclHiding = Just (hiding, L _ names)} #endif - (newModuleName, avails) = L lim imd + (newModuleName, avails) = imd { ideclName = noLocA newModuleName #if MIN_VERSION_ghc(9,5,0) - , ideclImportList = if hiding == Exactly && qualified /= NotQualified + , ideclImportList = if noExplicitList origHiding && qualified /= NotQualified then Nothing else Just (hiding, noLocA newNames) #else - , ideclHiding = if hiding == Exactly && qualified /= NotQualified + , ideclHiding = if noExplicitList origHiding && qualified /= NotQualified then Nothing else Just (hiding, noLocA newNames) #endif @@ -498,5 +497,7 @@ constructImport (L lim imd@ImportDecl{ideclName = L _ _, ideclQualified = qualif containsAvail name avail = any (\an -> printOutputable an == (printOutputable . ieName . unLoc $ name)) $ availNamesWithSelectors avail - -constructImport lim _ = lim + noExplicitList mList = case mList of + Nothing -> True + Just (_, xs) -> not $ null xs +constructImport _ lim _ = lim diff --git a/plugins/hls-explicit-imports-plugin/test/Main.hs b/plugins/hls-explicit-imports-plugin/test/Main.hs index 71aef3f1c2..1ff799bbfb 100644 --- a/plugins/hls-explicit-imports-plugin/test/Main.hs +++ b/plugins/hls-explicit-imports-plugin/test/Main.hs @@ -31,7 +31,8 @@ main = defaultTestRunner $ testGroup "import-actions" "Refine Imports" [ codeActionGoldenTest "RefineWithOverride" 3 1 , codeLensGoldenTest isRefineImports "RefineUsualCase" 1 - , codeLensGoldenTest isRefineImports "RefineQualified" 1 + , codeLensGoldenTest isRefineImports "RefineQualified" 0 + , codeLensGoldenTest isRefineImports "RefineQualifiedExplicit" 0 ], testGroup "Make imports explicit" diff --git a/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.expected.hs b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.expected.hs new file mode 100644 index 0000000000..add221e907 --- /dev/null +++ b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.expected.hs @@ -0,0 +1,11 @@ +module Main where + +import qualified RefineB as RA (b1) +import qualified RefineC as RA (c1) +import RefineD +import Data.List (intercalate) + +main :: IO () +main = putStrLn + $ "hello " + <> intercalate ", " [RA.b1, RA.c1, e2] diff --git a/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.hs b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.hs new file mode 100644 index 0000000000..03e65d5d88 --- /dev/null +++ b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.hs @@ -0,0 +1,10 @@ +module Main where + +import qualified RefineA as RA (b1, c1) +import RefineD +import Data.List (intercalate) + +main :: IO () +main = putStrLn + $ "hello " + <> intercalate ", " [RA.b1, RA.c1, e2] From 3410ad0ee36161eda25ebb86371f8b686fa3831e Mon Sep 17 00:00:00 2001 From: Nathan Maxson Date: Fri, 15 Sep 2023 21:12:05 +0300 Subject: [PATCH 3/5] hie.yaml fix --- plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml b/plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml index 04f19891ef..369b6e1dd3 100644 --- a/plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml +++ b/plugins/hls-explicit-imports-plugin/test/testdata/hie.yaml @@ -9,6 +9,7 @@ cradle: - ExplicitB.hs - RefineUsualCase.hs - RefineQualified.hs + - RefineQualifiedExplicit.hs - RefineWithOverride.hs - RefineA.hs - RefineB.hs From c92f0d106c1383fba1884dab15f64b078fec8c6f Mon Sep 17 00:00:00 2001 From: Nathan Maxson Date: Tue, 19 Sep 2023 16:50:15 +0300 Subject: [PATCH 4/5] Implement michealpj's suggestions --- .../src/Ide/Plugin/ExplicitImports.hs | 10 ++++------ .../test/testdata/RefineQualifiedExplicit.expected.hs | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs b/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs index b07354db0d..238492bb28 100644 --- a/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs +++ b/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs @@ -29,7 +29,7 @@ import qualified Data.IntMap as IM (IntMap, elems, fromList, (!?)) import Data.IORef (readIORef) import qualified Data.Map.Strict as Map -import Data.Maybe (mapMaybe) +import Data.Maybe (isNothing, mapMaybe) import qualified Data.Set as S import Data.String (fromString) import qualified Data.Text as T @@ -482,11 +482,11 @@ constructImport ImportDecl{ideclQualified = qualified, ideclHiding = origHiding} (newModuleName, avails) = imd { ideclName = noLocA newModuleName #if MIN_VERSION_ghc(9,5,0) - , ideclImportList = if noExplicitList origHiding && qualified /= NotQualified + , ideclImportList = if isNothing origHiding && qualified /= NotQualified then Nothing else Just (hiding, noLocA newNames) #else - , ideclHiding = if noExplicitList origHiding && qualified /= NotQualified + , ideclHiding = if isNothing origHiding && qualified /= NotQualified then Nothing else Just (hiding, noLocA newNames) #endif @@ -497,7 +497,5 @@ constructImport ImportDecl{ideclQualified = qualified, ideclHiding = origHiding} containsAvail name avail = any (\an -> printOutputable an == (printOutputable . ieName . unLoc $ name)) $ availNamesWithSelectors avail - noExplicitList mList = case mList of - Nothing -> True - Just (_, xs) -> not $ null xs + constructImport _ lim _ = lim diff --git a/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.expected.hs b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.expected.hs index add221e907..11e38aabfc 100644 --- a/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.expected.hs +++ b/plugins/hls-explicit-imports-plugin/test/testdata/RefineQualifiedExplicit.expected.hs @@ -1,7 +1,7 @@ module Main where -import qualified RefineB as RA (b1) -import qualified RefineC as RA (c1) +import qualified RefineB as RA ( b1 ) +import qualified RefineC as RA ( c1 ) import RefineD import Data.List (intercalate) From c8756321216e12734b7761f2a80f11dfe83a3742 Mon Sep 17 00:00:00 2001 From: Nathan Maxson Date: Tue, 19 Sep 2023 16:56:09 +0300 Subject: [PATCH 5/5] Fix 9.6 compatibility --- .../src/Ide/Plugin/ExplicitImports.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs b/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs index 238492bb28..98bae2028b 100644 --- a/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs +++ b/plugins/hls-explicit-imports-plugin/src/Ide/Plugin/ExplicitImports.hs @@ -475,7 +475,7 @@ filterByImport _ _ = Nothing constructImport :: ImportDecl GhcRn -> ImportDecl GhcRn -> (ModuleName, [AvailInfo]) -> ImportDecl GhcRn #if MIN_VERSION_ghc(9,5,0) -constructImport ImportDecl{ideclQualified = qualified, ideclHiding = origHiding} imd@ImportDecl{ideclImportList = Just (hiding, L _ names)} +constructImport ImportDecl{ideclQualified = qualified, ideclImportList = origHiding} imd@ImportDecl{ideclImportList = Just (hiding, L _ names)} #else constructImport ImportDecl{ideclQualified = qualified, ideclHiding = origHiding} imd@ImportDecl{ideclHiding = Just (hiding, L _ names)} #endif