From 85929745f1ad6afc0ada7b920502e73cc3b15dc4 Mon Sep 17 00:00:00 2001 From: Guillaume Bouchard Date: Sun, 9 Mar 2025 15:42:44 +0400 Subject: [PATCH 1/7] feat: introduce import suggestion for coerce MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a context where we want to `coerce` to a type for which constructor is not in scope, GHC fails with (e.g., for `Sum Int`): ``` • Couldn't match representation of type ‘Int’ with that of ‘Sum Int’ arising from a use of ‘coerce’ The data constructor ‘base-4.18.2.1:Data.Semigroup.Internal.Sum’ of newtype ‘Sum’ is not in scope ``` This code action detects the missing `newtype` and suggests to add the required import. This is convenient because otherwise the user need to interpret the error message and most of the time manually find which module and type to import. Note that a better implementation could try to decet that the type is already imported (if that's the case) and just suggest to add the constructor (e.g. `(..)`) in the import list, but this is too much complexity to implement. It could lead to duplicated import lines which will be "cleaned" by formatter or other extensions. --- .../src/Development/IDE/Plugin/CodeAction.hs | 2 ++ plugins/hls-refactor-plugin/test/Main.hs | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+) 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 3252d6b33a..e47d059192 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -1840,6 +1840,8 @@ extractNotInScopeName x = Just $ NotInScopeDataConstructor name | Just [name] <- matchRegexUnifySpaces x "ot in scope: type constructor or class [^‘]*‘([^’]*)’" = Just $ NotInScopeTypeConstructorOrClass name + | Just [name] <- matchRegexUnifySpaces x "of newtype ‘([^’]*)’ is not in scope" + = Just $ NotInScopeThing name | Just [name] <- matchRegexUnifySpaces x "ot in scope: \\(([^‘ ]+)\\)" = Just $ NotInScopeThing name | Just [name] <- matchRegexUnifySpaces x "ot in scope: ([^‘ ]+)" diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs index 7cb37f2785..0245d80a48 100644 --- a/plugins/hls-refactor-plugin/test/Main.hs +++ b/plugins/hls-refactor-plugin/test/Main.hs @@ -300,6 +300,7 @@ codeActionTests = testGroup "code actions" , suggestImportClassMethodTests , suggestImportTests , suggestAddRecordFieldImportTests + , suggestAddCoerceMissingConstructorImportTests , suggestHideShadowTests , fixConstructorImportTests , fixModuleImportTypoTests @@ -1871,6 +1872,30 @@ suggestAddRecordFieldImportTests = testGroup "suggest imports of record fields w contentAfterAction <- documentContents doc liftIO $ after @=? contentAfterAction +suggestAddCoerceMissingConstructorImportTests :: TestTree +suggestAddCoerceMissingConstructorImportTests = testGroup "suggest imports of newtype constructor when using coerce" + [ testGroup "The newtype constructor is suggested when a matching representation error" + [ theTest + ] + ] + where + theTest = testSessionWithExtraFiles "hover" def $ \dir -> do + configureCheckProject False + let before = T.unlines ["module A where", "import Data.Coerce (coerce)", "import Data.Semigroup (Sum)", "bar = coerce (10 :: Int) :: Sum Int"] + after = T.unlines ["module A where", "import Data.Coerce (coerce)", "import Data.Semigroup (Sum)", "import Data.Semigroup (Sum(..))", "bar = coerce (10 :: Int) :: Sum Int"] + cradle = "cradle: {direct: {arguments: [-hide-all-packages, -package, base, -package, text, -package-env, -, A]}}" + liftIO $ writeFileUTF8 (dir "hie.yaml") cradle + doc <- createDoc "Test.hs" "haskell" before + waitForProgressDone + _ <- waitForDiagnostics + let defLine = 3 + range = Range (Position defLine 0) (Position defLine maxBound) + actions <- getCodeActions doc range + action <- pickActionWithTitle "import Data.Semigroup (Sum(..))" actions + executeCodeAction action + contentAfterAction <- documentContents doc + liftIO $ after @=? contentAfterAction + suggestImportDisambiguationTests :: TestTree suggestImportDisambiguationTests = testGroup "suggest import disambiguation actions" From 41b28c519a91aa908355c0ed61952b58dba54be1 Mon Sep 17 00:00:00 2001 From: Guillaume Bouchard Date: Sun, 9 Mar 2025 19:07:50 +0400 Subject: [PATCH 2/7] feat: Add import suggestion for missing in scope constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For example, `deriving instance Generic (Sum Int)`, but it should work for other deriving which indirectly requires a complete access to the type constructor. ``` Can't make a derived instance of ‘Generic (Sum Int)’: The data constructors of ‘Sum’ are not all in scope so you cannot derive an instance for it ``` --- .../src/Development/IDE/Plugin/CodeAction.hs | 2 ++ plugins/hls-refactor-plugin/test/Main.hs | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+) 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 e47d059192..cb09b75545 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -1840,6 +1840,8 @@ extractNotInScopeName x = Just $ NotInScopeDataConstructor name | Just [name] <- matchRegexUnifySpaces x "ot in scope: type constructor or class [^‘]*‘([^’]*)’" = Just $ NotInScopeTypeConstructorOrClass name + | Just [name] <- matchRegexUnifySpaces x "The data constructors of ‘([^ ]+)’ are not all in scope" + = Just $ NotInScopeDataConstructor name | Just [name] <- matchRegexUnifySpaces x "of newtype ‘([^’]*)’ is not in scope" = Just $ NotInScopeThing name | Just [name] <- matchRegexUnifySpaces x "ot in scope: \\(([^‘ ]+)\\)" diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs index 0245d80a48..fed4dbe7db 100644 --- a/plugins/hls-refactor-plugin/test/Main.hs +++ b/plugins/hls-refactor-plugin/test/Main.hs @@ -301,6 +301,7 @@ codeActionTests = testGroup "code actions" , suggestImportTests , suggestAddRecordFieldImportTests , suggestAddCoerceMissingConstructorImportTests + , suggestAddGenericMissingConstructorImportTests , suggestHideShadowTests , fixConstructorImportTests , fixModuleImportTypoTests @@ -1896,6 +1897,31 @@ suggestAddCoerceMissingConstructorImportTests = testGroup "suggest imports of ne contentAfterAction <- documentContents doc liftIO $ after @=? contentAfterAction +suggestAddGenericMissingConstructorImportTests :: TestTree +suggestAddGenericMissingConstructorImportTests = testGroup "suggest imports of type constructors when using generic deriving" + [ testGroup "The type constructors are suggested when not in scope" + [ theTest + ] + ] + where + theTest = testSessionWithExtraFiles "hover" def $ \dir -> do + configureCheckProject False + let + before = T.unlines ["module A where", "import GHC.Generics", "import Data.Semigroup (Sum)", "deriving instance Generic (Sum Int)"] + after = T.unlines ["module A where", "import GHC.Generics", "import Data.Semigroup (Sum)", "import Data.Semigroup (Sum(..))", "deriving instance Generic (Sum Int)"] + cradle = "cradle: {direct: {arguments: [-hide-all-packages, -package, base, -package, text, -package-env, -, A]}}" + liftIO $ writeFileUTF8 (dir "hie.yaml") cradle + doc <- createDoc "Test.hs" "haskell" before + waitForProgressDone + _ <- waitForDiagnostics + let defLine = 3 + range = Range (Position defLine 0) (Position defLine maxBound) + actions <- getCodeActions doc range + action <- pickActionWithTitle "import Data.Semigroup (Sum(..))" actions + executeCodeAction action + contentAfterAction <- documentContents doc + liftIO $ after @=? contentAfterAction + suggestImportDisambiguationTests :: TestTree suggestImportDisambiguationTests = testGroup "suggest import disambiguation actions" From 32d85c59481cf88fbb33738a76cc0c3c58328bcf Mon Sep 17 00:00:00 2001 From: Guillaume Bouchard Date: Tue, 11 Mar 2025 19:04:06 +0400 Subject: [PATCH 3/7] Add import suggestion for indirect overloaded record dot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For example, the following code `foo.titi` when the type of `foo` (e.g. `Bar` here is not in scope and not from an already imported module (e.g. the type exists indirectly because here `foo :: Bar` comes from another module). If the module which contains `Bar` is already imported, GHC already gives an hint to add `titi` to the `import Bar` line and this is already correctly handled by HLS. ``` No instance for ‘HasField "titi" Bar.Bar String’ arising from selecting the field ‘titi’ ``` --- .../src/Development/IDE/Plugin/CodeAction.hs | 24 +++++++++++++ .../IDE/Plugin/Plugins/Diagnostic.hs | 3 ++ plugins/hls-refactor-plugin/test/Main.hs | 36 ++++++++++++++++++- 3 files changed, 62 insertions(+), 1 deletion(-) 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 cb09b75545..86b02dffb1 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -1844,6 +1844,30 @@ extractNotInScopeName x = Just $ NotInScopeDataConstructor name | Just [name] <- matchRegexUnifySpaces x "of newtype ‘([^’]*)’ is not in scope" = Just $ NotInScopeThing name + -- Match for HasField "foo" Bar String in the context where, e.g. x.foo is + -- used, and x :: Bar. + -- + -- This usually mean that the field is not in scope and the correct fix is to + -- import (Bar(foo)) or (Bar(..)). + -- + -- However, it is more reliable to match for the type name instead of the field + -- name, and most of the time you'll want to import the complete type with all + -- their fields instead of the specific field. + -- + -- The regex is convoluted because it accounts for: + -- + -- - Qualified (or not) `HasField` + -- - The type bar is always qualified. If it is unqualified, it means that the + -- parent module is already imported, and in this context it uses an hint + -- already available in the GHC error message. However this regex accounts for + -- qualified or not, it does not cost much and should be more robust if the + -- hint changes in the future + -- - Next regex will account for polymorphic types, which appears as `HasField + -- "foo" (Bar Int)...`, e.g. see the parenthesis + | Just [_module, name] <- matchRegexUnifySpaces x "No instance for ‘.*HasField \"[^\"]+\" ([^ (.]+\\.)*([^ (.]+).*’" + = Just $ NotInScopeThing name + | Just [_module, name] <- matchRegexUnifySpaces x "No instance for ‘.*HasField \"[^\"]+\" \\(([^ .]+\\.)*([^ .]+)[^)]*\\).*’" + = Just $ NotInScopeThing name | Just [name] <- matchRegexUnifySpaces x "ot in scope: \\(([^‘ ]+)\\)" = Just $ NotInScopeThing name | Just [name] <- matchRegexUnifySpaces x "ot in scope: ([^‘ ]+)" diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/Plugins/Diagnostic.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/Plugins/Diagnostic.hs index d64edbd0e2..7facc8f54c 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/Plugins/Diagnostic.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/Plugins/Diagnostic.hs @@ -21,6 +21,9 @@ matchRegex message regex = case message =~~ regex of Nothing -> Nothing -- | 'matchRegex' combined with 'unifySpaces' +-- +-- >>> matchRegexUnifySpaces "hello I'm a cow" "he(ll)o" +-- Just ["ll"] matchRegexUnifySpaces :: T.Text -> T.Text -> Maybe [T.Text] matchRegexUnifySpaces message = matchRegex (unifySpaces message) diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs index fed4dbe7db..8b7747750e 100644 --- a/plugins/hls-refactor-plugin/test/Main.hs +++ b/plugins/hls-refactor-plugin/test/Main.hs @@ -1851,8 +1851,14 @@ suggestImportTests = testGroup "suggest import actions" suggestAddRecordFieldImportTests :: TestTree suggestAddRecordFieldImportTests = testGroup "suggest imports of record fields when using OverloadedRecordDot" [ testGroup "The field is suggested when an instance resolution failure occurs" - [ ignoreForGhcVersions [GHC94, GHC96] "Extension not present <9.2, and the assist is derived from the help message in >=9.4" theTest + ([ ignoreForGhcVersions [GHC94, GHC96] "Extension not present <9.2, and the assist is derived from the help message in >=9.4" theTest ] + ++ [ + theTestIndirect qualifiedGhcRecords polymorphicType + | + qualifiedGhcRecords <- [False, True] + , polymorphicType <- [False, True] + ]) ] where theTest = testSessionWithExtraFiles "hover" def $ \dir -> do @@ -1873,6 +1879,34 @@ suggestAddRecordFieldImportTests = testGroup "suggest imports of record fields w contentAfterAction <- documentContents doc liftIO $ after @=? contentAfterAction + theTestIndirect qualifiedGhcRecords polymorphicType = testGroup + ((if qualifiedGhcRecords then "qualified-" else "unqualified-") + <> ("HasField " :: String) + <> + (if polymorphicType then "polymorphic-" else "monomorphic-") + <> "type ") + . (\x -> [x]) $ testSessionWithExtraFiles "hover" def $ \dir -> do + -- Hopefully enable project indexing? + configureCheckProject True + + let + before = T.unlines ["{-# LANGUAGE OverloadedRecordDot #-}", "module A where", if qualifiedGhcRecords then "" else "import GHC.Records", "import C (bar)", "spam = bar.foo"] + after = T.unlines ["{-# LANGUAGE OverloadedRecordDot #-}", "module A where", if qualifiedGhcRecords then "" else "import GHC.Records", "import C (bar)", "import B (Foo(..))", "spam = bar.foo"] + cradle = "cradle: {direct: {arguments: [-hide-all-packages, -package, base, -package, text, -package-env, -, A, B, C]}}" + liftIO $ writeFileUTF8 (dir "hie.yaml") cradle + liftIO $ writeFileUTF8 (dir "B.hs") $ unlines ["module B where", if polymorphicType then "data Foo x = Foo { foo :: x }" else "data Foo = Foo { foo :: Int }"] + liftIO $ writeFileUTF8 (dir "C.hs") $ unlines ["module C where", "import B", "bar = Foo 10" ] + doc <- createDoc "Test.hs" "haskell" before + waitForProgressDone + _ <- waitForDiagnostics + let defLine = 4 + range = Range (Position defLine 0) (Position defLine maxBound) + actions <- getCodeActions doc range + action <- pickActionWithTitle "import B (Foo(..))" actions + executeCodeAction action + contentAfterAction <- documentContents doc + liftIO $ after @=? contentAfterAction + suggestAddCoerceMissingConstructorImportTests :: TestTree suggestAddCoerceMissingConstructorImportTests = testGroup "suggest imports of newtype constructor when using coerce" [ testGroup "The newtype constructor is suggested when a matching representation error" From c26abf42a398bbf02fdba950fd4dd5a8cc918f4a Mon Sep 17 00:00:00 2001 From: Guillaume Bouchard Date: Wed, 12 Mar 2025 12:11:57 +0400 Subject: [PATCH 4/7] fix: add support for GHC 9.4 for import indirect type field This correct previous commit by handling ghc 9.4 parethensis instead of "tick". --- .../src/Development/IDE/Plugin/CodeAction.hs | 4 ++-- 1 file changed, 2 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 86b02dffb1..7c8704b59b 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -1864,9 +1864,9 @@ extractNotInScopeName x -- hint changes in the future -- - Next regex will account for polymorphic types, which appears as `HasField -- "foo" (Bar Int)...`, e.g. see the parenthesis - | Just [_module, name] <- matchRegexUnifySpaces x "No instance for ‘.*HasField \"[^\"]+\" ([^ (.]+\\.)*([^ (.]+).*’" + | Just [_module, name] <- matchRegexUnifySpaces x "No instance for [‘(].*HasField \"[^\"]+\" ([^ (.]+\\.)*([^ (.]+).*[’)]" = Just $ NotInScopeThing name - | Just [_module, name] <- matchRegexUnifySpaces x "No instance for ‘.*HasField \"[^\"]+\" \\(([^ .]+\\.)*([^ .]+)[^)]*\\).*’" + | Just [_module, name] <- matchRegexUnifySpaces x "No instance for [‘(].*HasField \"[^\"]+\" \\(([^ .]+\\.)*([^ .]+)[^)]*\\).*[’)]" = Just $ NotInScopeThing name | Just [name] <- matchRegexUnifySpaces x "ot in scope: \\(([^‘ ]+)\\)" = Just $ NotInScopeThing name From 63a00f02e3987c262d76e5d8ea1a37fd055177a4 Mon Sep 17 00:00:00 2001 From: Guillaume Bouchard Date: Tue, 1 Apr 2025 09:57:00 +0400 Subject: [PATCH 5/7] tests: add regex unit test for extractNotInScopeName --- .../src/Development/IDE/Plugin/CodeAction.hs | 6 ++-- plugins/hls-refactor-plugin/test/Main.hs | 32 +++++++++++++++++++ 2 files changed, 36 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 7c8704b59b..f978358c0e 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -12,7 +12,9 @@ module Development.IDE.Plugin.CodeAction fillHolePluginDescriptor, extendImportPluginDescriptor, -- * For testing - matchRegExMultipleImports + matchRegExMultipleImports, + extractNotInScopeName, + NotInScope(..) ) where import Control.Applicative ((<|>)) @@ -1825,7 +1827,7 @@ data NotInScope = NotInScopeDataConstructor T.Text | NotInScopeTypeConstructorOrClass T.Text | NotInScopeThing T.Text - deriving Show + deriving (Show, Eq) notInScope :: NotInScope -> T.Text notInScope (NotInScopeDataConstructor t) = t diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs index 8b7747750e..a256ee327f 100644 --- a/plugins/hls-refactor-plugin/test/Main.hs +++ b/plugins/hls-refactor-plugin/test/Main.hs @@ -46,6 +46,7 @@ import Development.IDE.Plugin.CodeAction (matchRegExMultipleImp import Test.Hls import qualified Development.IDE.GHC.ExactPrint +import Development.IDE.Plugin.CodeAction (NotInScope (..)) import qualified Development.IDE.Plugin.CodeAction as Refactor import qualified Test.AddArgument @@ -68,6 +69,7 @@ tests = , codeActionTests , codeActionHelperFunctionTests , completionTests + , extractNotInScopeNameTests ] initializeTests :: TestTree @@ -1907,6 +1909,36 @@ suggestAddRecordFieldImportTests = testGroup "suggest imports of record fields w contentAfterAction <- documentContents doc liftIO $ after @=? contentAfterAction +extractNotInScopeNameTests :: TestTree +extractNotInScopeNameTests = + testGroup "extractNotInScopeName" [ + testGroup "HasField" [ + testGroup "unqualified" [ + testGroup "nice ticks" [ + testCase "Simple type" $ Refactor.extractNotInScopeName "No instance for ‘HasField \"baz\" Cheval Bool’" @=? Just (NotInScopeThing "Cheval"), + testCase "Parametric type" $ Refactor.extractNotInScopeName "No instance for ‘HasField \"bar\" (Hibou Int) a0’" @=? Just (NotInScopeThing "Hibou"), + testCase "Parametric type" $ Refactor.extractNotInScopeName "No instance for ‘HasField \"foo\" (Tortue Int) Int’" @=? Just (NotInScopeThing "Tortue") + ], + testGroup "parenthesis" [ + testCase "Simple type" $ Refactor.extractNotInScopeName "No instance for ‘HasField \"blup\" Calamar Bool’" @=? Just (NotInScopeThing "Calamar"), + testCase "Parametric type" $ Refactor.extractNotInScopeName "No instance for ‘HasField \"biz\" (Ornithorink Int) a0’" @=? Just (NotInScopeThing "Ornithorink"), + testCase "Parametric type" $ Refactor.extractNotInScopeName "No instance for ‘HasField \"blork\" (Salamandre Int) Int’" @=? Just (NotInScopeThing "Salamandre") + ] + ], + testGroup "qualified" [ + testGroup "nice ticks" [ + testCase "Simple type" $ Refactor.extractNotInScopeName "No instance for ‘GHC.HasField \"baz\" Cheval Bool’" @=? Just (NotInScopeThing "Cheval"), + testCase "Parametric type" $ Refactor.extractNotInScopeName "No instance for ‘Record.HasField \"bar\" (Hibou Int) a0’" @=? Just (NotInScopeThing "Hibou"), + testCase "Parametric type" $ Refactor.extractNotInScopeName "No instance for ‘Youpi.HasField \"foo\" (Tortue Int) Int’" @=? Just (NotInScopeThing "Tortue") + ], + testGroup "parenthesis" [ + testCase "Simple type" $ Refactor.extractNotInScopeName "No instance for ‘GHC.Tortue.HasField \"blup\" Calamar Bool’" @=? Just (NotInScopeThing "Calamar"), + testCase "Parametric type" $ Refactor.extractNotInScopeName "No instance for ‘Youpi.Salamandre.HasField \"biz\" (Ornithorink Int) a0’" @=? Just (NotInScopeThing "Ornithorink"), + testCase "Parametric type" $ Refactor.extractNotInScopeName "No instance for ‘Foo.Bar.HasField \"blork\" (Salamandre Int) Int’" @=? Just (NotInScopeThing "Salamandre") + ] + ] + ] + ] suggestAddCoerceMissingConstructorImportTests :: TestTree suggestAddCoerceMissingConstructorImportTests = testGroup "suggest imports of newtype constructor when using coerce" [ testGroup "The newtype constructor is suggested when a matching representation error" From c6248774e9eac828968e3ef0b6653b24ee2301d4 Mon Sep 17 00:00:00 2001 From: Guillaume Bouchard Date: Tue, 1 Apr 2025 14:43:19 +0400 Subject: [PATCH 6/7] feat: add test and import suggestion for not in scope record field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In ghc 9.6 we had: ``` Not in scope ‘modelStatusTag’ ``` In 9.10 we have: ``` Not in scope: record field ‘modelStatusTag’ ``` Introducing the new regex in order to match it AND a test to ensure no future regression. The regression is due to the fact that there is a matcher which catch `Nat in scope: .*`, hence it was matching "record" instead of "modelStatusTag". --- .../src/Development/IDE/Plugin/CodeAction.hs | 4 +++ plugins/hls-refactor-plugin/test/Main.hs | 32 +++++++++++++++++++ 2 files changed, 36 insertions(+) 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 f978358c0e..d87175405a 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -1870,6 +1870,10 @@ extractNotInScopeName x = Just $ NotInScopeThing name | Just [_module, name] <- matchRegexUnifySpaces x "No instance for [‘(].*HasField \"[^\"]+\" \\(([^ .]+\\.)*([^ .]+)[^)]*\\).*[’)]" = Just $ NotInScopeThing name + -- The order of the "Not in scope" is important, for example, some of the + -- matcher may catch the "record" value instead of the value later. + | Just [name] <- matchRegexUnifySpaces x "Not in scope: record field ‘([^’]*)’" + = Just $ NotInScopeThing name | Just [name] <- matchRegexUnifySpaces x "ot in scope: \\(([^‘ ]+)\\)" = Just $ NotInScopeThing name | Just [name] <- matchRegexUnifySpaces x "ot in scope: ([^‘ ]+)" diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs index a256ee327f..3f3196fc21 100644 --- a/plugins/hls-refactor-plugin/test/Main.hs +++ b/plugins/hls-refactor-plugin/test/Main.hs @@ -320,6 +320,7 @@ codeActionTests = testGroup "code actions" , addImplicitParamsConstraintTests , removeExportTests , Test.AddArgument.tests + , suggestAddRecordFieldUpdateImportTests ] insertImportTests :: TestTree @@ -1909,9 +1910,40 @@ suggestAddRecordFieldImportTests = testGroup "suggest imports of record fields w contentAfterAction <- documentContents doc liftIO $ after @=? contentAfterAction +suggestAddRecordFieldUpdateImportTests :: TestTree +suggestAddRecordFieldUpdateImportTests = testGroup "suggest imports of record fields in update" + [ testGroup "implicit import of type" [theTest ] ] + where + theTest = testSessionWithExtraFiles "hover" def $ \dir -> do + configureCheckProject True + + let + before = T.unlines ["module C where", "import B", "biz = bar { foo = 100 }"] + after = T.unlines ["module C where", "import B", "import A (Foo(..))", "biz = bar { foo = 100 }"] + cradle = "cradle: {direct: {arguments: [-hide-all-packages, -package, base, -package, text, -package-env, -, A, B, C]}}" + liftIO $ writeFileUTF8 (dir "hie.yaml") cradle + liftIO $ writeFileUTF8 (dir "A.hs") $ unlines ["module A where", "data Foo = Foo { foo :: Int }"] + liftIO $ writeFileUTF8 (dir "B.hs") $ unlines ["module B where", "import A", "bar = Foo 10" ] + doc <- createDoc "Test.hs" "haskell" before + waitForProgressDone + diags <- waitForDiagnostics + liftIO $ print diags + let defLine = 2 + range = Range (Position defLine 0) (Position defLine maxBound) + actions <- getCodeActions doc range + liftIO $ print actions + action <- pickActionWithTitle "import A (Foo(..))" actions + executeCodeAction action + contentAfterAction <- documentContents doc + liftIO $ after @=? contentAfterAction + extractNotInScopeNameTests :: TestTree extractNotInScopeNameTests = testGroup "extractNotInScopeName" [ + testGroup "record field" [ + testCase ">=ghc 910" $ Refactor.extractNotInScopeName "Not in scope: ‘foo’" @=? Just (NotInScopeThing "foo"), + testCase " Date: Thu, 3 Apr 2025 15:12:42 +0400 Subject: [PATCH 7/7] fix: include the field selector when looking for missing symbol In GHC >= 9.8, the namespace for record selector changed and is now part of a new namespace. This allows for duplicated record field names in the same module. This hence generated a few issues in HLS when looking for a symbol using `lookupOccEnv`: the current implementation was only doing lookup in type, data and var namespaces. This commit uses `lookupOccEnv_AllNameSpaces`, so it may be more efficient (one lookup instead of two), but it also incluse the symbols from record field selectors and this will actually fix most import suggestion logic when a record field selector is not found. Note that the function is not available in `ghc` <= 9.6, hence the `CPP` and fallsback implementation, which uses the previous implementation. See https://gitlab.haskell.org/ghc/ghc/-/wikis/migration/9.8#new-namespace-for-record-fields --- .../src/Development/IDE/Plugin/CodeAction.hs | 25 +++++++++++++++++-- 1 file changed, 23 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 d87175405a..93c7b912e0 100644 --- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs +++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs @@ -1574,13 +1574,34 @@ extractQualifiedModuleNameFromMissingName (T.strip -> missing) modNameP = fmap snd $ RE.withMatched $ conIDP `sepBy1` RE.sym '.' +-- | A Backward compatible implementation of `lookupOccEnv_AllNameSpaces` for +-- GHC <=9.6 +-- +-- It looks for a symbol name in all known namespaces, including types, +-- variables, and fieldnames. +-- +-- Note that on GHC >= 9.8, the record selectors are not in the `mkVarOrDataOcc` +-- anymore, but are in a custom namespace, see +-- https://gitlab.haskell.org/ghc/ghc/-/wikis/migration/9.8#new-namespace-for-record-fields, +-- hence we need to use this "AllNamespaces" implementation, otherwise we'll +-- miss them. +lookupOccEnvAllNamespaces :: ExportsMap -> T.Text -> [IdentInfo] +#if MIN_VERSION_ghc(9,7,0) +lookupOccEnvAllNamespaces exportsMap name = Set.toList $ mconcat (lookupOccEnv_AllNameSpaces (getExportsMap exportsMap) (mkTypeOcc name)) +#else +lookupOccEnvAllNamespaces exportsMap name = maybe [] Set.toList $ + lookupOccEnv (getExportsMap exportsMap) (mkVarOrDataOcc name) + <> lookupOccEnv (getExportsMap exportsMap) (mkTypeOcc name) -- look up the modified unknown name in the export map +#endif + + constructNewImportSuggestions :: ExportsMap -> (Maybe T.Text, NotInScope) -> Maybe [T.Text] -> QualifiedImportStyle -> [ImportSuggestion] constructNewImportSuggestions exportsMap (qual, thingMissing) notTheseModules qis = nubOrdBy simpleCompareImportSuggestion [ suggestion | Just name <- [T.stripPrefix (maybe "" (<> ".") qual) $ notInScope thingMissing] -- strip away qualified module names from the unknown name - , identInfo <- maybe [] Set.toList $ lookupOccEnv (getExportsMap exportsMap) (mkVarOrDataOcc name) - <> lookupOccEnv (getExportsMap exportsMap) (mkTypeOcc name) -- look up the modified unknown name in the export map + + , identInfo <- lookupOccEnvAllNamespaces exportsMap name -- look up the modified unknown name in the export map , canUseIdent thingMissing identInfo -- check if the identifier information retrieved can be used , moduleNameText identInfo `notElem` fromMaybe [] notTheseModules -- check if the module of the identifier is allowed , suggestion <- renderNewImport identInfo -- creates a list of import suggestions for the retrieved identifier information