Repository navigation
fix dunder slots rename #4344 - #5631
asukaminato0721 wants to merge 21 commits into
Conversation
|
✅ This PR meets all requirements from our pull request checklist. Optional recommendations:
Note: this comment updates automatically after changes to the PR. Updates may take up to half an hour. |
|
This pull request has been imported. If you are a Meta employee, you can view this in D131488315. (Because this pull request was imported automatically, there will not be any future comments.) |
This comment has been minimized.
This comment has been minimized.
kinto0
left a comment
kinto0
left a comment
There was a problem hiding this comment.
Thanks, the rename tests are thorough (all assignment forms, class identity, __str__ rejection, source/stub/consumer). Requesting changes because this slice builds on the dunder_slots_entry AST match and on #5629's reference layer, both of which the reviews on #5627 and #5629 ask to replace. Once slots entries come from one record (name, content range, owning class), this change should become small: read the entry's stored content range. These comments cover only the increment over #5629.
| pub fn prepare_rename(&self, handle: &Handle, position: TextSize) -> Option<TextRange> { | ||
| let identifier_context = self.identifier_at(handle, position); | ||
| let dunder_all_entry = self.get_exports_data(handle).dunder_all_name_at(position); | ||
| let symbol_literal_range = |
There was a problem hiding this comment.
This locates the AST nodes again and calls dunder_slots_entry, and then string_literal_content_range locates the same literal a second time. With a stored slots-entry record (as for __all__ in #5588), this can read the content range directly, for both __all__ and __slots__.
| .or_else(|| { | ||
| let range = symbol_literal_range?; | ||
| let ast = self.get_ast(handle)?; | ||
| string_literal_content_range(&ast, range) |
There was a problem hiding this comment.
Same point: with stored content ranges for __all__ and __slots__ entries, this fallback and string_literal_content_range are no longer needed.
Use the existing definition lookup to read the exported symbol type. Keep string literal types for strings outside __all__.
Read the current checked module’s final export answer so buffers, stubs, and unopened dependencies supply the correct type.
Resolve __all__ entries only after identifier lookup fails. Cover go-to-type-definition and remove unrelated hover changes.
Resolve each entry to its definition before adding its string content range. Respect import aliases so references to independent names remain separate.
Preserve local alias identity and retain export occurrences in the normal reference index so unopened modules need no navigation queries.
Keep string content ranges from export extraction and resolve navigation and references through the checked binding. Preserve interface documentation without changing symbol identity.
Use string content ranges to preserve quotes and prefixes. Keep local source definitions during reference operations so rename updates both source files and their stubs. Validate the target before offering rename.
Read the owning class and literal slot name from the existing AST nodes. Use attribute definition lookup to navigate to the field declaration.
Resolve slot names against a generic instance of their owning class. This displays the field type while preserving its type parameters.
Resolve each slot string to its attribute before adding its content range. Load class bindings only for candidate modules with __slots__ fields so indexed files also return these references.
22a2d69 to
d7c6d02
Compare
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes part of #4344
stack on #5629
Test Plan
add test