Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code action not showing on multi-line imports with unused record field #4310

Open
battermann opened this issue Jun 10, 2024 · 3 comments
Open
Labels
type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@battermann
Copy link
Contributor

battermann commented Jun 10, 2024

Steps to reproduce

Given ModuleB:

module ModuleB where
newtype B = B {b1 :: String}
x = 1

and ModuleA which has an unused record field import within a multi-line import statement:

module ModuleA where
import ModuleB
  ( B (b1),
    x,
  )
x' = x

Expected behaviour

I expect the code action for removing the unused record field: Remove B, B(b1) from import to be shown on selecting the single line with the unused import.

Actual behaviour

Instead, the code action does not show:

image

However, it shows when collecting the complete range of the import statement.

image

@battermann battermann added status: needs triage type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Jun 10, 2024
@jhrcek
Copy link
Collaborator

jhrcek commented Jun 12, 2024

Thanks for the report. I managed to reproduce it using the following single file reproducer:

import Data.Monoid
   ( Sum(getSum)
   , getAp
   )
x = getAp

The great first step would be to add a failing test case to the test suite which would make it easy to isolate the root cause by rerunning the reproducer and adding Debug.Trace statements in various places.

I'll probably get to fixing this in the next few days but feel free to take a stab at it.

@battermann
Copy link
Contributor Author

It looks like the range for the warning comes from GHC, so it is probably more a GHC issue than HLS:

src/ModuleA.hs:3:1: warning: [-Wunused-imports]
    The import of ‘Sum, Sum(getSum)’
    from module ‘Data.Monoid’ is redundant
  |
3 | import Data.Monoid
  | ^^^^^^^^^^^^^^^^^^...

@jhrcek
Copy link
Collaborator

jhrcek commented Jun 12, 2024

I'm not sure if that's the issue.
My theory: It might be some logic in HLS which does something like "try to calculate the range of this code action" - which because it fails to identify the correct range due to diagnostic message containing Sum(getSum) vs us looking for range of just getSum which is not found on its own in the AST, so it falls back to code range of the whole import declaration. Then since you have both the

  • "diagnostic showing the warning" and also
  • "code action to remove the import"
    shown at the same range, you won't see the code action or something like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

No branches or pull requests

2 participants