'Remove redundant imports' code action not showing in neovim 0.9 #3857
Comments
asivitz
added
status: needs triage
type: bug
|
Is it possible to get the request as correctly-formatted JSON? I don't know what the thing being printed by neovim is, but it isn't correct JSON. It's not at all obvious to me what's going wrong. Can you run HLS with debug logging on? I think someone will just need to debug a bit. |
|
I'm affected by this too. From what @asivitz posted it seems that neovim is sending a different range information? Here's the diff between 0.7 and 0.9 requests: 0.9: I'll try to reproduce this on my machine later. |
|
Like I said, it's really not obvious to me what's happening here. It would help to get some proper JSON for the problematic request, and then I think someone just needs to take a look at the code and see if it's doing something silly. Debug logging may help. |
|
For example, what on earth does |
michaelpj
added
status: needs info
|
Just popping in to say I'm still seeing this behavior on neovim Is this reproducible on (e.g.) VS Code? Might be the quickest way to narrow down whether it's a HLS or neovim issue... |
Removing redundant import works for me in vscode. |
Yup, me too. (Now that I've installed it again, heh.) Which definitely suggests it's a problem with neovim! I filed neovim/neovim#27318, cribbing the logs from @asivitz's original report. Hope that's cool! |
Discovered while investigating issue haskell#3857. Technically, there are only two required fields in an incoming `Diagnostic` object: `range` and `message`. However, the HLS was comparing all fields to determine equality. This caused mismatches when neovim stopped sending the (optional) `tags` field.
|
Well, I opened a PR that fixes the issue: #4051 ...but I'm not sure if it's the "right" thing to do! Meanwhile though, if you're affected by this, building HLS from the above branch should get you going again. |
|
Good sleuthing! I commented on the PR. I do think neovim is behaving unhelpfully here, but the spec doesn't tell people what to do and anyway I think we probably shouldn't rely on what they're doing anyway. |
|
They did in fact fix this on the neovim side. Could someone verify if that has in fact fixed this issue? |
I just tried with the nightly build of neovim from Feb 7 (which appears to contain the fix) and HLS
And here are the code actions for the same line with my patched HLS (and same version of neovim):
Haven't done any debugging or anything yet... EDIT: Looks like the tags still aren't coming through... I commented on the nvim issue. |
Rather than doing a full compare with incoming `Diagnostic` objects from the client. This brings the "remove redundant imports/exports" code actions more in line with behavior described in haskell#4056, and has the pleasant side-effect of fixing broken code actions in neovim (haskell#3857).
|
While we wait on the neovim fix, I figured I'd take a crack at fixing it the right way on the HLS side. PR here! |
…ions are in scope (#4063) * Use *only* incoming range to determine which code actions are in scope Rather than doing a full compare with incoming `Diagnostic` objects from the client. This brings the "remove redundant imports/exports" code actions more in line with behavior described in #4056, and has the pleasant side-effect of fixing broken code actions in neovim (#3857). * Remove redundant imports ;) * Rename param for clarity --------- Co-authored-by: fendor <fendor@users.noreply.github.com>
…ions are in scope (haskell#4063) * Use *only* incoming range to determine which code actions are in scope Rather than doing a full compare with incoming `Diagnostic` objects from the client. This brings the "remove redundant imports/exports" code actions more in line with behavior described in haskell#4056, and has the pleasant side-effect of fixing broken code actions in neovim (haskell#3857). * Remove redundant imports ;) * Rename param for clarity --------- Co-authored-by: fendor <fendor@users.noreply.github.com>
|
I forgot to mention it here, but now that PR #4063 is merged, I think this bug can be closed out 🎉
|
Your environment
Which OS do you use?
MacOS
Which version of GHC do you use and how did you install it?
9.4.7 from ghcup
How is your project built (alternative: link to the project)?
stack
Which LSP client (editor/plugin) do you use?
nvim + lspconfig (the neovim native lsp)
Which version of HLS do you use and how did you install it?
2.4.0.0 from ghcup
Have you configured HLS in any way (especially: a
hie.yamlfile)?Yes, a hie.yaml
Steps to reproduce
Open a file using neovim 0.9. Add a redundant import. Ask for code actions on this line.
Expected behaviour
There is a code action to 'Remove import' and 'Remove all redundant imports', as well as 'Disable 'unused-imports' warnings'
Actual behaviour
There is only a code action to disable the warnings
Debug information
The code actions do appear under neovim 0.7. Here are RPC logs for the two versions. Looks like the request data changed a bit between 0.7 and 0.9.
NEOVIM 0.7 REQUEST
NEOVIM 0.9 REQUEST
NEOVIM 0.7 RESPONSE LOGS (INCLUDING DEBUG INFO)
NEOVIM 0.9 RESPONSE LOGS (INCLUDING DEBUG INFO)
Thanks!
The text was updated successfully, but these errors were encountered: