haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.65k stars 355 forks source link

Automatic import of pattern synonyms does not import the correct name #627

Open expipiplus1 opened 3 years ago

expipiplus1 commented 3 years ago

With files Foo.hs and Bar.hs

module Foo where

pattern P = ()
module Bar where

bar = P

in Bar.hs observe that the code actions available include import Foo(P), not import Foo(pattern P)

expipiplus1 commented 3 years ago

associated pattern synonyms do work however they too suffer from https://github.com/haskell/haskell-language-server/issues/609#issue-746334129:

-- Foo.hs
module Foo
  ( D(P)
  ) where

data D = D
pattern P = D
peterwicksstringfield commented 3 years ago

Difficulty: that code action is based on this diagnostic from GHC:

app/Bar.hs:5:7: error:
    • Data constructor not in scope: P
    • Perhaps you want to add ‘P’ to the import list in the import of
      ‘Foo’ (app/Bar.hs:3:1-12).
  |
5 | bar = P
  |       ^

There isn't enough information in that diagnostic to deduce that P is a pattern synonym instead of a normal data constructor.

C.f. https://gitlab.haskell.org/ghc/ghc/-/issues/18466 which is about the usefulness of the underlying error message.

The code action currently uses only the diagnostic and the parsed source of the file you are importing into (module Bar in this case).

I think in order to distinguish data constructors from pattern synonyms we would need to get the parsed source for the module we are importing from (module Foo in this case), to get access to the HsDecl of the data constructor / pattern synonym (pattern P = ()) in this case). This information is not easily available in ghcide/Development/IDE/Plugin/CodeAction.

Maybe I'm missing something? But I think to fix this bug the code action would need access to IdeState?

runeksvendsen commented 2 years ago

Note that this is also an issue for the "Remove all redundant imports" action, where HLS removes the wrong imports when a pattern and type of the same name are in scope:

Before action is applied (pattern Ann is redundant):

import Nix.Expr.Types.Annotated (Ann, SrcSpan, pattern Ann)

type MyAnn f = Ann SrcSpan f

after action is applied:

import Nix.Expr.Types.Annotated (SrcSpan, pattern Ann)

type MyAnn f = Ann SrcSpan f

causing a "Not in scope"-error for Ann because it removes the Ann type from the import list rather than the Ann pattern.

michaelpj commented 3 months ago

Note that this makes me suspect that we probably also don't add the type namespace specifier when that's needed (which I think is only when the imported type is an operator?).