Skip to content

Migrate replace derive with manual impl and add_missing_impl_members to use SyntaxEditor #20293

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
fn add_trait_assoc_items_to_impl use ted yet.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 23, 2025
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 25, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 25, 2025
@ShoyuVanilla
Copy link
Member

Oh, the test added from #19938 are failing

@@ -185,71 +185,65 @@ pub fn add_trait_assoc_items_to_impl(
trait_: hir::Trait,
impl_: &ast::Impl,
target_scope: &hir::SemanticsScope<'_>,
) -> ast::AssocItem {
) -> Vec<ast::AssocItem> {
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 have #[must_use] attributes here, as this is not mutating existing syntax tree anymore but returns a new one. (And this would help preventing tests in #19938 failing, though I've merged it after this PR was created 😅 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for finding the fix spot!

@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 25, 2025
@Hmikihiro
Copy link
Contributor Author

Sorry, I made a mistake merge.

@Hmikihiro Hmikihiro force-pushed the migrate_replace_derive_with_manual_impl branch from 50f8ef6 to 82dfdac Compare July 25, 2025 15:29
@rustbot rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. has-merge-commits labels Jul 25, 2025
@ShoyuVanilla ShoyuVanilla enabled auto-merge July 25, 2025 15:31
@ShoyuVanilla ShoyuVanilla added this pull request to the merge queue Jul 25, 2025
Merged via the queue into rust-lang:master with commit 51e77c9 Jul 25, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 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