fix(rename): scope member renames to resolved declarations#160
Open
calebdw wants to merge 1 commit into
Open
Conversation
5e595fc to
428a9a3
Compare
349e049 to
6ec2b45
Compare
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Renaming a method was too broad.
If you renamed a method like
recalculate()on one class, the server could also rename otherrecalculate()methods elsewhere in the codebase, including unrelated classes. This was especially bad for private methods, where the safe scope should be very narrow.The root issue was that rename reused the same conservative matching used by
Find References. That behavior is acceptable for search results, where unresolved member calls can be included as a fallback, but it is not safe for workspace edits.There was also a second scoping problem around inheritance: when renaming a concrete implementation method, hierarchy expansion could walk up to an interface or parent class and then back down into sibling implementations that should not have been touched.
Fix
This PR makes member rename stricter than
Find References:This means:
Tests
Added regression coverage for: