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.

Sign up for GitHub

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

Jump to bottom

Fix edits for convert_named_struct_to_tuple_struct #14920

Merged
merged 2 commits into from May 30, 2023
Merged

Fix edits for convert_named_struct_to_tuple_struct #14920

merged 2 commits into from May 30, 2023

Conversation

lowr
Copy link
Contributor

@lowr lowr commented May 29, 2023 •
edited

Two fixes:

  • When replacing syntax nodes, macro files weren't taken into account. Edits were simply made for node.syntax().text_range(), which would be wrong range when node is inside a macro file.
  • We do ancestor node traversal for every struct name reference to find record expressions/patterns to edit, but we didn't verify that expressions/patterns do actually refer to the struct we're operating on.

Best reviewed one commit at a time.

Fixes #13780
Fixes #14927

lowr added 2 commits May 29, 2023 19:52
@lowr
Consider macro files when replacing nodes
ab93475
@lowr
Verify name references more rigidly
033e6ac 
Previously we didn't verify that record expressions/patterns that were
found did actually point to the struct we're operating on. Moreover,
when that record expressions/patterns had missing child nodes, we would
continue traversing their ancestor nodes.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 29, 2023
@lowr lowr changed the title Overhaul convert_named_struct_to_tuple_struct Fix edits for convert_named_struct_to_tuple_struct May 29, 2023
@lowr lowr mentioned this pull request May 30, 2023
Closed
@Veykril
Copy link
Member

Veykril commented May 30, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented May 30, 2023

📌 Commit 033e6ac has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented May 30, 2023

⌛ Testing commit 033e6ac with merge 51c3ab5...

@bors
Copy link
Collaborator

bors commented May 30, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 51c3ab5 to master...

@bors bors merged commit 51c3ab5 into rust-lang:master May 30, 2023
10 checks passed
@jinohkang-theori jinohkang-theori mentioned this pull request Nov 14, 2023
Merged
bors added a commit that referenced this pull request Dec 13, 2023
@bors
Auto merge of #15887 - jinohkang-theori:fix/ide-assists/convert_tuple…
dd07f1f 
…_struct_to_named_struct/handle_refs_inside_macro_invocations, r=Veykril

Fix incorrectly replacing references in macro invocation in "Convert to named struct" assist

Fixes #15630.

Complements #13647 (same assist but missed this one), #14920 (inverse action assist).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Veykril Veykril Veykril approved these changes

Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Convert to tuple struct acting really weird sometimes struct refactor affects lint attributes... somehow?
4 participants
@lowr @Veykril @bors @rustbot