Record quick fixes based on fields should be deduped

[ndmitchell]

Not sure what version of ghcide I'm on...

[jneira]

Reproduced with lastest hls and ghcide (although i got only 2 duplicates)
It suggests one for each record field (my record has only two), in the problems pane:

Illegal use of punning for field ‘a’
Use NamedFieldPuns to permit this

Illegal use of punning for field ‘b’
Use NamedFieldPuns to permit this

[gdevanla]

I can take a look at this issue. Any good place to start?

[jneira]

@gdevanla I think the code that trigger extension completion will be:

https://github.com/haskell/ghcide/blob/863392b9b94777a069a2a31e9b909d1ce45e93d4/src/Development/IDE/Plugin/CodeAction.hs#L526

Not known much about it though, it comes from the time where this was part of daml, according to blame.

[ishmum123]

@jneira this might be a noob question. What's the consequence of changing the line to this
= [("Add " <> x <> " extension", [TextEdit (Range (Position 0 0) (Position 0 0)) $ "{-# LANGUAGE " <> x <> " #-}\n"]) | x <- (Set.toList $ Set.fromList exts) ]
?

[jneira]

it could work, a test case would be the definitive evidence 🙂

[ishmum123]

@jneira can you help me a bit? I am a bit lost on where to provide the pull request for ghcide

[jneira]

@ishmum123 sorry for the delay, the pull request should be done in this repo, now that ghcide lives here.

But the code responsive of suggest language extensions has been removed from ghcide, cause hls had already a plugin to do the same.
However it suffer the same bug, i think the relevant code could be:

haskell-language-server/plugins/default/src/Ide/Plugin/Pragmas.hs

Line 71 in 3145830

pragmas = concatMap (\d -> genPragma dflags (d ^. J.message)) diags

[Ailrun]

I'm not super confident, but I think it wont't fix this issue as this issue is caused by multiple diagnostics, not by a single diagnostic.

[jneira]

Mmm, @Ailrun you are right and the fix becomes more complex, i am afraid. ghc is giving us a waning for each field so the alternatives i can think of is:

• fix ghc to only emit one warning per record constructor (that would be the right one, but we would be blocked upstream)
• filter diagnostics in hls (not sure if possible or feasible)
• only emit one code action although we get several diagnostics. It would be feasible i we can access to the set of actual diagnostics (and i think there is a field stored somewhere) or code actions and only emit the new code action if it is not redundant

[berberman]

You can use getDiagnostics to get global diagnostics, like this:

haskell-language-server/ghcide/src/Development/IDE/Plugin/CodeAction.hs

Line 82 in 79c5cc7

diag <- fmap (\(_, _, d) -> d) . filter (\(p, _, _) -> mbFile == Just p) <$> getDiagnostics state

[ishmum123]

@berberman you probably did a much better job than I did. I was working on it for a while, so thought I should push the fix if it's of any use.

[berberman]

@ishmum123 Oops, really sorry, I should ask first if someone has already started working on this :(