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

Renaming in presence of type errors #3799

Closed
coot opened this issue Sep 10, 2023 · 1 comment · Fixed by #3848
Closed

Renaming in presence of type errors #3799

coot opened this issue Sep 10, 2023 · 1 comment · Fixed by #3848
Labels
component: hls-rename-plugin level: easy The issue is suited for beginners type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@coot
Copy link

coot commented Sep 10, 2023

Your environment

Which OS do you use?
Fedora
Which version of GHC do you use and how did you install it?
9.6.2 from ghcup
How is your project built (alternative: link to the project)?

Which LSP client (editor/plugin) do you use?
vim+coc.nvim
Which version of HLS do you use and how did you install it?

Have you configured HLS in any way (especially: a hie.yaml file)?
no

Steps to reproduce

Use the following file

module X where

data X a where
  X :: X Int
  Y :: X String
  Z :: X Int

a :: X Int
a = X

-- this doesn't type check on purpose:
instance Eq X where
  (==) = \_ _ -> True

Try to rename the X data type to X'.

Expected behaviour

Either nothing should be changed or everything even in presence of type errors.

Actual behaviour

module X where

data X a where
  X :: X Int
  Y :: X String
  Z :: X Int

a :: X Int
a = X

-- this doesn't type check on purpose:
instance Eq X' where
  (==) = \_ _ -> True

i.e. non of the X occurrences in the defintiion of X or a changed, only its occurrence in the Eq instance.

Debug information

@coot coot added status: needs triage type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Sep 10, 2023
@fendor
Copy link
Collaborator

fendor commented Sep 10, 2023

Hi, thank you for your bug report!

Likely, the rename handler uses useWithStale, causing it to fall back to the latest compiling file state.

I agree, renaming doesn't make sense if the module doesn't compile. I think, we should have two modi, for local renames only, we require that the module we look at compiles, for cross module renaming, the whole project needs to compile successfully.

This should be simple enough to fix in https:/haskell/haskell-language-server/blob/master/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs#L232. Instead of useWithStaleE, prefer useE.
Likely, we need something like getNamesAtPoint but without any PositionMapping.

@michaelpj michaelpj added component: hls-rename-plugin level: easy The issue is suited for beginners and removed status: needs triage labels Sep 12, 2023
sgillespie added a commit to sgillespie/haskell-language-server that referenced this issue Oct 17, 2023
sgillespie added a commit to sgillespie/haskell-language-server that referenced this issue Oct 17, 2023
sgillespie added a commit to sgillespie/haskell-language-server that referenced this issue Apr 19, 2024
sgillespie added a commit to sgillespie/haskell-language-server that referenced this issue Apr 19, 2024
sgillespie added a commit to sgillespie/haskell-language-server that referenced this issue Apr 19, 2024
michaelpj pushed a commit that referenced this issue Apr 21, 2024
* Rename only if the current module compiles (#3799)

Prefer `useE` over `useWithStaleE`

* Add a rename test that tests for compilation errors
soulomoon pushed a commit that referenced this issue Apr 22, 2024
* Rename only if the current module compiles (#3799)

Prefer `useE` over `useWithStaleE`

* Add a rename test that tests for compilation errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: hls-rename-plugin level: easy The issue is suited for beginners type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants