[ty] Implemented support for "rename" language server feature#19551
[ty] Implemented support for "rename" language server feature#19551dhruvmanila merged 3 commits intoastral-sh:mainfrom
Conversation
|
MichaReiser
left a comment
There was a problem hiding this comment.
This is great.
Do you know how editors handle renames if multiple LSPs provide the feature?
I'm asking because our go to definition implementation still has some "holes". Meaning, ty will miss renames where another LSP (that users might run side to side with ty) would do a proper rename. Or do editors merge the rename result from all LSPs (union vs taking the result from one LSP)?
If editors only pick the result from one LSP, it migth then be best to wait landing this feature and add an explicit client setting for users to opt in to our experimental renaming.
We're fine landing this if editors union the result.
Editors don't merge results from multiple language servers for the "rename" feature. There's really no good way to do it. Instead, they pick one language server and use its results. We can certainly wait to land this feature if you're concerned about this. It's up to you. It sounds like we'll be able to plug the "nonlocal" and "global" hole soon, so maybe it's best if we wait for that fix to land first. |
|
Yeah, it might be good to keep this disable for the moment. We could also land this but not register the provider and @dhruvmanila could enable it once he added support for client settings. |
|
I can take this up now that we have proper client settings infrastructure in place. |
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
* origin/main: [ty] Implemented support for "rename" language server feature (#19551) [ty] Reduce size of member table (#19572) [ty] Move server capabilities creation (#19798) [ty] Repurpose `FunctionType.into_bound_method_type` to return `BoundMethodType` (#19793) [ty] Validate writes to `TypedDict` keys (#19782) [ty] Add support for using the test command emitted when a mdtest fails (#19794)
## Summary This PR is a follow-up from #19551 and adds a new `ty.experimental.rename` setting to conditionally register for the rename capability. The complementary PR in ty VS Code extension is astral-sh/ty-vscode#111. This is done using dynamic registration after the settings have been resolved. The experimental group is part of the global settings because they're applied for all workspaces that are managed by the client. ## Test Plan Add E2E tests. In VS Code, with the following setting: ```json { "ty.experimental.rename": "true", "python.languageServer": "None" } ``` I get the relevant log entry: ``` 2025-08-07 16:05:40.598709000 DEBUG client_response{id=3 method="client/registerCapability"}: Registered rename capability ``` And, I'm able to rename a symbol. Once I set it to `false`, then I can see this log entry: ``` 2025-08-07 16:08:39.027876000 DEBUG Rename capability is disabled in the client settings ``` And, I don't see the "Rename Symbol" open in the VS Code dropdown. https://github.com/user-attachments/assets/501659df-ba96-4252-bf51-6f22acb4920b
* dcreager/bound-typevar: (24 commits) more comment fix this isn't true anymore fix inner function in overload implementation ecosystem error Update Rust toolchain to 1.89 (#19807) [ty] Add `ty.inlayHints.variableTypes` server option (#19780) synthetic typevars for type materializations are bound [ty] Add failing tests for tuple subclasses (#19803) [ty] Add `ty.experimental.rename` server setting (#19800) clippy make BoundTypeVarInstance interned [ty] Implemented support for "rename" language server feature (#19551) [ty] Reduce size of member table (#19572) [ty] Move server capabilities creation (#19798) separate types! tmp allow KnownInstance::TypeVar in type expressions bind typevar in Generic/Protocol base class reference [ty] Repurpose `FunctionType.into_bound_method_type` to return `BoundMethodType` (#19793) remove unneeded GenericContext::with_binding_context create KnownInstanceType::TypeVar for PEP 695 too ...
This PR adds support for the "rename" language server feature. It builds upon existing functionality used for "go to references".
The "rename" feature involves two language server requests. The first is a "prepare rename" request that determines whether renaming should be possible for the identifier at the current offset. The second is a "rename" request that returns a list of file ranges where the rename should be applied.
Care must be taken when attempting to rename symbols that span files, especially if the symbols are defined in files that are not part of the project. We don't want to modify code in the user's Python environment or in the vendored stub files.
I found a few bugs in the "go to references" feature when implementing "rename", and those bug fixes are included in this PR.