suggestImportDisambiguation does not interact well with DuplicateRecordFields

[berberman]

Your environment

Output of haskell-language-server --probe-tools or haskell-language-server-wrapper --probe-tools:

haskell-language-server version: 1.0.0.0 (GHC: 8.10.3) (PATH: /home/berberman/.cabal/store/ghc-8.10.3/haskell-language-server-1.0.0.0-c7859e8f45ef6bd2fac7adc65982f4e497ee3c40299bcc89c5915aaa0534ebc3/bin/haskell-language-server)
Tool versions found on the $PATH
cabal:          3.2.0.0
stack:          Not found
ghc:            8.10.3

Which lsp-client do you use:
vscode

Expected behaviour

There should be no suggestion, because the ambiguity comes from DuplicateRecordFields, rather than imports.

Actual behaviour

These code actions are meaningless in this case.

[adamgundry]

If you want to be really clever, in this situation you could suggest adding import qualified A as RecA (RecA(..) and replace with RecA.foo. But perhaps that's a bit complex to figure out.

[July541]

For the original issue, I just wondering if we have another file B.hs simultaneously:

module B where

newtype RecC = RecC { foo :: Int }

What suggestions should ghcide gives? Maybe Replace with qualified: B.foo (cause hiding can not work for duplicated records) is not a bad choice.

@adamgundry 's idea seems an appropriate solution, which can eliminate the worry above, but it may make the original function suggestImportDisambiguation too huge to maintain, can anyone give some advice?

---

Meanwhile, we must notice that if we choose to keep the foo from B and hide others in the current version, it will print double foo in A.

If we choose foo from A and hide others, it will add hiding (foo) to the two imports simultaneously, it will use foo in RecB due to RecB is at the bottom of the RecA.

Our fix should consider these two situations. Please point out if I missed any other conditions.

[July541]

@berberman I am trying to fix this issue by just remove all ambiguities that are from the same module, does it make sense?

[berberman]

@July541 Nice, what will it look like? Could you give an example?

[July541]

@berberman Here are two samples:

1. With the same records name in exactly one module, we will ignore all the ambiguities due to they are not caused by ambiguity imports.
   

2. For the mixture of real ambiguity records and DuplicateRecordFields caused ambiguity, we only keep the ambiguity one from files without DuplicateRecordFields extension as suggestions and drop others.
   

[berberman]

Looks great, thanks!

[July541]

@berberman Can you point me out where I can include some tests? I am not sure about how to integrate with a test.

[berberman]

@July541 You could find related tests here:

haskell-language-server/ghcide/test/exe/Main.hs

Line 1592 in 3e3d782

suggestImportDisambiguationTests :: TestTree