search

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 354 forks source link

Rename a constructor renames all record fields #2915

Open deemp opened 2 years ago

deemp commented 2 years ago

Your environment

Which OS do you use: Ubuntu 20.04 Which LSP client (editor/plugin) do you use: VS Code, Haskell extension v2.2.0, ghc 9.2.0, HLS 1.7.0.0 Describe your project (alternative: link to the project): A .yml parser.

Steps to reproduce

See commit 1 Next, try to rename Column using the Rename action- commit 2

Expected behaviour

Only the constructor name is updated

Actual behaviour

All fields of a record are renamed the same as the new consructor name

Include debug information

OliverMadine commented 2 years ago

Hi,

Thanks for the report!

Please could you update the link to the commit for the steps to reproduce? It looks like the repository might be private.

@br4ch1st0chr0n3

deemp commented 2 years ago

Hi, sorry for that. Should be public now, @OliverMadine.

OliverMadine commented 2 years ago

It seems like the issue is hie files including record field names as references to the constructor. This is only happening with ghc >= 9.

@wz1000 maybe you have some more insight?

pepeiborra commented 2 years ago

It seems like the issue is hie files including record field names as references to the constructor. This is only happening with ghc >= 9.

Is this a bug or a feature?

If it is a bug, we will need to report it in the GHC bug tracker and ideally contribute a fix. And back port the fixes to hie-compat: https://github.com/haskell/haskell-language-server/tree/master/hie-compat

michaelpj commented 8 months ago

Is this still a problem?

konn commented 8 months ago

Yes. I've gotten bitten by this yesterday at least with HLS 2.5.0.0.

konn commented 8 months ago

I've just tested and confirmed that it still occurs with HLS 2.6.0.0.

soulomoon commented 4 months ago

Perhaps the type RefMap a = Map Identifier [(Span, IdentifierDetails a)] should contains the NodeOrigin to indicate if it is evidence variable or not

jhrcek commented 4 months ago

Note that I took a stab at fixing this in https://github.com/haskell/haskell-language-server/pull/4089

That fix works, but requires changes on hiedb side (skipping indexing of all Identifiers whose NodeOrigin is GeneratedInfo). We agreed with @michaelpj that instead of that approach, we should rather add something like a boolean column for indexed identifiers saying whether that identifier is generated or not. This would then be used by the rename plugin which would filter out all generated occurrences of given identifier, thus avoiding this bug via different route.

soulomoon commented 4 months ago

Yes that fix looks good

I also make an issue for the type change as well https://gitlab.haskell.org/ghc/ghc/-/issues/24751