From 0e1b44fadc0d063f9e1337c03550ec17a99e3001 Mon Sep 17 00:00:00 2001 From: Berk Ozkutuk Date: Fri, 2 Dec 2022 14:18:54 +0100 Subject: [PATCH 1/2] Make redundant import removal work on patsyn imports --- .../src/Development/IDE/Plugin/CodeAction.hs | 11 ++++++++- test/functional/FunctionalCodeAction.hs | 23 +++++++++++++++---- .../src/CodeActionRedundant.hs | 5 ++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs index 3a45c0f154..8cdd53a5d3 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -1841,7 +1841,16 @@ smallerRangesForBindingExport lies b = ranges' _ = [] rangesForBinding' :: String -> LIE GhcPs -> [SrcSpan] -rangesForBinding' b (L (locA -> l) x@IEVar{}) | T.unpack (printOutputable x) == b = [l] +rangesForBinding' b (L (locA -> l) (IEVar _ nm)) +#if !MIN_VERSION_ghc(9,2,0) + | L _ (IEPattern (L _ b')) <- nm +#else + | L _ (IEPattern _ (L _ b')) <- nm +#endif + , T.unpack (printOutputable b') == b + = [l] +rangesForBinding' b (L (locA -> l) x@IEVar{}) + | T.unpack (printOutputable x) == b = [l] rangesForBinding' b (L (locA -> l) x@IEThingAbs{}) | T.unpack (printOutputable x) == b = [l] rangesForBinding' b (L (locA -> l) (IEThingAll _ x)) | T.unpack (printOutputable x) == b = [l] #if !MIN_VERSION_ghc(9,2,0) diff --git a/test/functional/FunctionalCodeAction.hs b/test/functional/FunctionalCodeAction.hs index b28038cc5b..04941c5aa1 100644 --- a/test/functional/FunctionalCodeAction.hs +++ b/test/functional/FunctionalCodeAction.hs @@ -177,16 +177,21 @@ redundantImportTests = testGroup "redundant import code actions" [ doc <- openDoc "src/CodeActionRedundant.hs" "haskell" diags <- waitForDiagnosticsFromSource doc "typecheck" - liftIO $ expectDiagnostic diags ["The import of", "Data.List", "is redundant"] + liftIO $ expectDiagnostic diags [ "The import of", "Data.List", "is redundant" ] + liftIO $ expectDiagnostic diags [ "Empty", "from module", "Data.Sequence" ] mActions <- getAllCodeActions doc let allActions = map fromAction mActions actionTitles = map (view L.title) allActions - liftIO $ actionTitles `shouldContain` ["Remove import", "Remove all redundant imports"] + liftIO $ actionTitles `shouldContain` + [ "Remove import" + , "Remove Empty from import" + , "Remove all redundant imports" + ] - let mbRemoveAction = find (\x -> x ^. L.title == "Remove import") allActions + let mbRemoveAction = find (\x -> x ^. L.title == "Remove all redundant imports") allActions case mbRemoveAction of Just removeAction -> do @@ -203,7 +208,17 @@ redundantImportTests = testGroup "redundant import code actions" [ -- provides workspace edit property which skips round trip to -- the server contents <- documentContents doc - liftIO $ contents @?= "{-# OPTIONS_GHC -Wunused-imports #-}\nmodule CodeActionRedundant where\nmain :: IO ()\nmain = putStrLn \"hello\"\n" + liftIO $ contents @?= T.unlines + [ "{-# OPTIONS_GHC -Wunused-imports #-}" + , "{-# LANGUAGE PatternSynonyms #-}" + , "module CodeActionRedundant where" + , "-- We need a non-reduntant import in the import list" + , "-- to properly test the removal of the singular redundant item" + , "import Data.Sequence (singleton)" + , "main :: IO ()" + , "main = putStrLn \"hello\"" + , " where unused = Data.Sequence.singleton 42" + ] , testCase "doesn't touch other imports" $ runSession hlsCommand noLiteralCaps "test/testdata/redundantImportTest/" $ do doc <- openDoc "src/MultipleImports.hs" "haskell" diff --git a/test/testdata/redundantImportTest/src/CodeActionRedundant.hs b/test/testdata/redundantImportTest/src/CodeActionRedundant.hs index f56d232f27..168868e3b9 100644 --- a/test/testdata/redundantImportTest/src/CodeActionRedundant.hs +++ b/test/testdata/redundantImportTest/src/CodeActionRedundant.hs @@ -1,5 +1,10 @@ {-# OPTIONS_GHC -Wunused-imports #-} +{-# LANGUAGE PatternSynonyms #-} module CodeActionRedundant where import Data.List +-- We need a non-reduntant import in the import list +-- to properly test the removal of the singular redundant item +import Data.Sequence (pattern Empty, singleton) main :: IO () main = putStrLn "hello" + where unused = Data.Sequence.singleton 42 From a048c2bcd55806daef76c194a800778a891190fb Mon Sep 17 00:00:00 2001 From: Berk Ozkutuk Date: Fri, 2 Dec 2022 14:27:51 +0100 Subject: [PATCH 2/2] Make stylish-haskell happy --- .../src/Development/IDE/Plugin/CodeAction.hs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs index 8cdd53a5d3..2b98b95a77 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -1841,14 +1841,17 @@ smallerRangesForBindingExport lies b = ranges' _ = [] rangesForBinding' :: String -> LIE GhcPs -> [SrcSpan] -rangesForBinding' b (L (locA -> l) (IEVar _ nm)) #if !MIN_VERSION_ghc(9,2,0) +rangesForBinding' b (L (locA -> l) (IEVar _ nm)) | L _ (IEPattern (L _ b')) <- nm + , T.unpack (printOutputable b') == b + = [l] #else +rangesForBinding' b (L (locA -> l) (IEVar _ nm)) | L _ (IEPattern _ (L _ b')) <- nm -#endif , T.unpack (printOutputable b') == b = [l] +#endif rangesForBinding' b (L (locA -> l) x@IEVar{}) | T.unpack (printOutputable x) == b = [l] rangesForBinding' b (L (locA -> l) x@IEThingAbs{}) | T.unpack (printOutputable x) == b = [l]