Skip to content

Migrate convert_tuple_struct_to_named_struct assist to use SyntaxEditor #20311

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

Hmikihiro
Copy link
Contributor

part of #15710 and #18285

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 27, 2025
@Hmikihiro Hmikihiro force-pushed the migrate_convert_tuple_struct_to_named_struct branch from 3025fab to a74d6ec Compare July 27, 2025 04:55
@rustbot

This comment has been minimized.

@rustbot rustbot added has-merge-commits S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 27, 2025
@Hmikihiro Hmikihiro force-pushed the migrate_convert_tuple_struct_to_named_struct branch from cd07345 to 475ebb8 Compare July 27, 2025 13:57
@rustbot rustbot removed has-merge-commits S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 27, 2025
let tuple_fields_text_range = tuple_fields.syntax().text_range();

edit.edit_file(ctx.vfs_file_id());
let record_fields = ast::make::record_field_list(record_fields).clone_for_update();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to use

especially when we need .clone_for_update() here. Though it still calls clone_for_update() in its body, at least we can gather them up in one place and bulk-fix later.

@@ -245,7 +275,7 @@ fn generate_names(fields: impl Iterator<Item = ast::TupleField>) -> Vec<ast::Nam
.enumerate()
.map(|(i, _)| {
let idx = i + 1;
ast::make::name(&format!("field{idx}"))
ast::make::name(&format!("field{idx}")).clone_for_update()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the above, with

pub fn name(&self, name: &str) -> ast::Name {

Everything else looks good to me!

Copy link
Member

@ShoyuVanilla ShoyuVanilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ShoyuVanilla ShoyuVanilla added this pull request to the merge queue Jul 30, 2025
Merged via the queue into rust-lang:master with commit b0d2487 Jul 30, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants