Skip to content

Commit bddf4b5

Browse files
committed
Add import suggestion for indirect overloaded record dot
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’ ```
1 parent 91aceb8 commit bddf4b5

File tree

3 files changed

+62
-1
lines changed

3 files changed

+62
-1
lines changed

plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1844,6 +1844,30 @@ extractNotInScopeName x
18441844
= Just $ NotInScopeDataConstructor name
18451845
| Just [name] <- matchRegexUnifySpaces x "of newtype ‘([^’]*)’ is not in scope"
18461846
= Just $ NotInScopeThing name
1847+
-- Match for HasField "foo" Bar String in the context where, e.g. x.foo is
1848+
-- used, and x :: Bar.
1849+
--
1850+
-- This usually mean that the field is not in scope and the correct fix is to
1851+
-- import (Bar(foo)) or (Bar(..)).
1852+
--
1853+
-- However, it is more reliable to match for the type name instead of the field
1854+
-- name, and most of the time you'll want to import the complete type with all
1855+
-- their fields instead of the specific field.
1856+
--
1857+
-- The regex is convoluted because it accounts for:
1858+
--
1859+
-- - Qualified (or not) `HasField`
1860+
-- - The type bar is always qualified. If it is unqualified, it means that the
1861+
-- parent module is already imported, and in this context it uses an hint
1862+
-- already available in the GHC error message. However this regex accounts for
1863+
-- qualified or not, it does not cost much and should be more robust if the
1864+
-- hint changes in the future
1865+
-- - Next regex will account for polymorphic types, which appears as `HasField
1866+
-- "foo" (Bar Int)...`, e.g. see the parenthesis
1867+
| Just [_module, name] <- matchRegexUnifySpaces x "No instance for ‘.*HasField \"[^\"]+\" ([^ (.]+\\.)*([^ (.]+).*’"
1868+
= Just $ NotInScopeThing name
1869+
| Just [_module, name] <- matchRegexUnifySpaces x "No instance for ‘.*HasField \"[^\"]+\" \\(([^ .]+\\.)*([^ .]+)[^)]*\\).*’"
1870+
= Just $ NotInScopeThing name
18471871
| Just [name] <- matchRegexUnifySpaces x "ot in scope: \\(([^‘ ]+)\\)"
18481872
= Just $ NotInScopeThing name
18491873
| Just [name] <- matchRegexUnifySpaces x "ot in scope: ([^‘ ]+)"

plugins/hls-refactor-plugin/src/Development/IDE/Plugin/Plugins/Diagnostic.hs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ matchRegex message regex = case message =~~ regex of
2121
Nothing -> Nothing
2222

2323
-- | 'matchRegex' combined with 'unifySpaces'
24+
--
25+
-- >>> matchRegexUnifySpaces "hello I'm a cow" "he(ll)o"
26+
-- Just ["ll"]
2427
matchRegexUnifySpaces :: T.Text -> T.Text -> Maybe [T.Text]
2528
matchRegexUnifySpaces message = matchRegex (unifySpaces message)
2629

plugins/hls-refactor-plugin/test/Main.hs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1851,8 +1851,14 @@ suggestImportTests = testGroup "suggest import actions"
18511851
suggestAddRecordFieldImportTests :: TestTree
18521852
suggestAddRecordFieldImportTests = testGroup "suggest imports of record fields when using OverloadedRecordDot"
18531853
[ testGroup "The field is suggested when an instance resolution failure occurs"
1854-
[ ignoreForGhcVersions [GHC94, GHC96] "Extension not present <9.2, and the assist is derived from the help message in >=9.4" theTest
1854+
([ ignoreForGhcVersions [GHC94, GHC96] "Extension not present <9.2, and the assist is derived from the help message in >=9.4" theTest
18551855
]
1856+
++ [
1857+
theTestIndirect qualifiedGhcRecords polymorphicType
1858+
|
1859+
qualifiedGhcRecords <- [False, True]
1860+
, polymorphicType <- [False, True]
1861+
])
18561862
]
18571863
where
18581864
theTest = testSessionWithExtraFiles "hover" def $ \dir -> do
@@ -1873,6 +1879,34 @@ suggestAddRecordFieldImportTests = testGroup "suggest imports of record fields w
18731879
contentAfterAction <- documentContents doc
18741880
liftIO $ after @=? contentAfterAction
18751881

1882+
theTestIndirect qualifiedGhcRecords polymorphicType = testGroup
1883+
((if qualifiedGhcRecords then "qualified-" else "unqualified-")
1884+
<> ("HasField " :: String)
1885+
<>
1886+
(if polymorphicType then "polymorphic-" else "monomorphic-")
1887+
<> "type ")
1888+
. (\x -> [x]) $ testSessionWithExtraFiles "hover" def $ \dir -> do
1889+
-- Hopefully enable project indexing?
1890+
configureCheckProject True
1891+
1892+
let
1893+
before = T.unlines ["{-# LANGUAGE OverloadedRecordDot #-}", "module A where", if qualifiedGhcRecords then "" else "import GHC.Records", "import C (bar)", "spam = bar.foo"]
1894+
after = T.unlines ["{-# LANGUAGE OverloadedRecordDot #-}", "module A where", if qualifiedGhcRecords then "" else "import GHC.Records", "import C (bar)", "import B (Foo(..))", "spam = bar.foo"]
1895+
cradle = "cradle: {direct: {arguments: [-hide-all-packages, -package, base, -package, text, -package-env, -, A, B, C]}}"
1896+
liftIO $ writeFileUTF8 (dir </> "hie.yaml") cradle
1897+
liftIO $ writeFileUTF8 (dir </> "B.hs") $ unlines ["module B where", if polymorphicType then "data Foo x = Foo { foo :: x }" else "data Foo = Foo { foo :: Int }"]
1898+
liftIO $ writeFileUTF8 (dir </> "C.hs") $ unlines ["module C where", "import B", "bar = Foo 10" ]
1899+
doc <- createDoc "Test.hs" "haskell" before
1900+
waitForProgressDone
1901+
_ <- waitForDiagnostics
1902+
let defLine = 4
1903+
range = Range (Position defLine 0) (Position defLine maxBound)
1904+
actions <- getCodeActions doc range
1905+
action <- pickActionWithTitle "import B (Foo(..))" actions
1906+
executeCodeAction action
1907+
contentAfterAction <- documentContents doc
1908+
liftIO $ after @=? contentAfterAction
1909+
18761910
suggestAddCoerceMissingConstructorImportTests :: TestTree
18771911
suggestAddCoerceMissingConstructorImportTests = testGroup "suggest imports of newtype constructor when using coerce"
18781912
[ testGroup "The newtype constructor is suggested when a matching representation error"

0 commit comments

Comments
 (0)