-
Notifications
You must be signed in to change notification settings - Fork 714
Implement rename and improve find-all-references in LSP #1635
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements rename functionality and enhances find-all-references in the TypeScript language server port. The changes primarily focus on adding the LSP rename command (F2) and improving reference finding for module imports/exports, contextually typed object literals, and destructuring patterns.
Key changes:
- Adds rename command support with proper handling of imports, exports, and property assignments
- Implements import/export tracking across modules for comprehensive reference finding
- Enhances object literal and destructuring pattern reference resolution
Reviewed Changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
internal/lsp/server.go |
Adds rename handler registration and LSP capability declaration |
internal/ls/findallreferences.go |
Implements rename logic and import/export reference tracking |
internal/ls/importTracker.go |
New module for tracking imports and exports across files |
internal/ls/utilities.go |
Adds utility functions for object literal elements and node tracking |
internal/checker/services.go |
Enhances property symbol resolution for contextual typing |
internal/core/core.go |
Adds deduplication utility function |
testdata/baselines/ |
Updated test baselines showing improved reference finding |
The inconsistent test failures are due to a race condition in program construction for multi-file tests in fourslash. @andrewbranch is researching. |
if searchSym != nil && search.includes(searchSym) { | ||
if rootSymbol != nil && sym.CheckFlags&ast.CheckFlagsSynthetic == 0 { | ||
return rootSymbol, kind | ||
return rootSymbol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What were we using kind
for before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simply eliminated all the code paths that either (a) ignored kind
or (b) passed it through. The Strada code base has some ugly logic that retrofits a RootSymbol
type on top of a regular Symbol
and distinguishes by checking for a kind
property. Since we can't do that in Corsa (good!), the initial port just threaded a kind
through everywhere. With these changes it is present only where needed.
I'm merging this and will work on a separate PR to port over the fourslash tests. |
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
This PR implements the following LSP functionality:
import
andexport
declarations.