haskell-language-server icon indicating copy to clipboard operation
haskell-language-server copied to clipboard

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

Open battermann opened this issue 1 year ago • 3 comments

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 avatar Jun 10 '24 13:06 battermann

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.

jhrcek avatar Jun 12 '24 04:06 jhrcek

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
  | ^^^^^^^^^^^^^^^^^^...

battermann avatar Jun 12 '24 08:06 battermann

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.

jhrcek avatar Jun 12 '24 09:06 jhrcek