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

Implement renaming for local variables #4185

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

GearsDatapacks
Copy link
Member

@GearsDatapacks GearsDatapacks commented Jan 18, 2025

Closes #2956
This PR implements renaming of local variables in the Language Server including:

  • Setting up the new route and server capability
  • Setting up a test environment for testing renaming
  • Tree walking to find references to the variable
  • Rejecting renames if an invalid name is provided (e.g. renaming a variable to SomeVar)

A few things I did while implementing this:

  • I used definition location to decide whether two variables were the same. That's mainly because since variable shadowing exists in Gleam, two variables with the same name don't necessarily refer to the same variable.
  • To allow renaming to also rename references to variables in clause guards, I added a definition_location field to the ClauseGuard::Var variant. Since ClauseGuard is just one type for both typed and untyped versions, this field has to be present in the untyped AST, where it has no meaning (we don't know where the variable is defined yet). That was the most straightforward solution I could find for the time being.
  • Similarly, I added clause guards to the AST visitor. I only really implemented the bare-bones for what this PR needs; it seemed a bit wasteful to implement it all. I'll probably open a separate PR to implement the full visitor for it

@GearsDatapacks GearsDatapacks marked this pull request as ready for review January 18, 2025 14:21
@GearsDatapacks
Copy link
Member Author

Not sure why checks are failing... I haven't changed anything to do with deps or cargo deny

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LSP: rename local variable
1 participant