Skip to content

Code action to remove redundant imports is sometimes not created #4220

Closed
@jhrcek

Description

@jhrcek
Collaborator

Using hls 2.7.0.0 in vscode with ghc 9.4.8, I sometimes don't get "remove redundant imports" code action.

Here's a one-module reproducer (assuming you have a cabal project with postgresql-simple dependency):

module Main where

import Database.PostgreSQL.Simple.Types (In (..), PGArray (..), Values (..), fromIdentifier, fromQuery)

main :: IO ()
main = do
    let array = PGArray []
        in_ = In []
        values = Values [] []
    putStrLn "Hello, Haskell!"

Although warning is generated and displayed on hover:

The import of ‘Identifier(fromIdentifier), Query(fromQuery)’
from module ‘Database.PostgreSQL.Simple.Types’ is redundanttypecheck(-Wunused-imports)

The usual code action to remove unused import is not created.
Not a big deal, but it's breaking my flow, as now I have to focus on what the warning is exactly saying and fix it manually 😄

Activity

added
type: bugSomething isn't right: doesn't work as intended, documentation is missing/outdated, etc..
on May 10, 2024
changed the title [-]Code action to remove redundant imports is sometimes missing[/-] [+]Code action to remove redundant imports is sometimes not created[/+] on May 10, 2024
added
HackathonThis issue is suitable for hackathon sessions
and removed on May 10, 2024
jhrcek

jhrcek commented on May 12, 2024

@jhrcek
CollaboratorAuthor
Hint for anyone looking into fixing this
This seems to happen when the unused field is a record field selector (for both data and newtypes). When these are unused, GHC warning actually mentions `RecordConstructor(recordFieldName)` in the diagnostic.

The place where this breaks down is in this function:

rangesForBinding' :: String -> LIE GhcPs -> [SrcSpan]
rangesForBinding' b (L (locA -> l) (IEVar _ nm))
| L _ (IEPattern _ (L _ b')) <- nm
, T.unpack (printOutputable b') == b
= [l]
rangesForBinding' b (L (locA -> l) x@IEVar{})
| T.unpack (printOutputable x) == b = [l]
rangesForBinding' b (L (locA -> l) x@IEThingAbs{}) | T.unpack (printOutputable x) == b = [l]
rangesForBinding' b (L (locA -> l) (IEThingAll _ x)) | T.unpack (printOutputable x) == b = [l]
rangesForBinding' b (L (locA -> l) (IEThingWith _ thing _ inners))
| T.unpack (printOutputable thing) == b = [l]
| otherwise =
[ locA l' | L l' x <- inners, T.unpack (printOutputable x) == b]
rangesForBinding' _ _ = []

which gets passed b::String like RecordConstructor(recordFieldName), tries to compare it with printOutputable b' which at this point is just recordFieldName, which are not equal and so no code action is generated for these.

battermann

battermann commented on Jun 9, 2024

@battermann
Contributor

Looking into this

added a commit that references this issue on Jun 11, 2024
597da9d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    HackathonThis issue is suitable for hackathon sessionslevel: easyThe issue is suited for beginnerstype: bugSomething isn't right: doesn't work as intended, documentation is missing/outdated, etc..

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @michaelpj@jhrcek@battermann@fendor

        Issue actions

          Code action to remove redundant imports is sometimes not created · Issue #4220 · haskell/haskell-language-server