Skip to content

Replace make constructor with syntaxFactory in utils/gen trait fn body#21886

Merged
A4-Tacks merged 6 commits intorust-lang:masterfrom
Shourya742:2026-03-26-migrate-gen-trait-fn-body
Mar 30, 2026
Merged

Replace make constructor with syntaxFactory in utils/gen trait fn body#21886
A4-Tacks merged 6 commits intorust-lang:masterfrom
Shourya742:2026-03-26-migrate-gen-trait-fn-body

Conversation

@Shourya742
Copy link
Copy Markdown
Member

@Shourya742 Shourya742 commented Mar 26, 2026

This PR replaces the make constructor with SyntaxFactory in gen_trait_fn_body, which directly affects two assists that are also updated in this PR:

  1. add_missing_impl_members
  2. replace_derive_with_manual_impl

part of #15710 and #18285

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 26, 2026
@Shourya742 Shourya742 requested a review from A4-Tacks March 30, 2026 01:05
) -> Option<ast::Path> {
let mut iter = parts.into_iter();
let base = self.ident_path(iter.next()?);
let path = iter.fold(base, |base, s| {
Copy link
Copy Markdown
Member

@A4-Tacks A4-Tacks Mar 30, 2026

Choose a reason for hiding this comment

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

Why not call make::ext::path_from_idents

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I reverted this change. I had originally implemented the manual construction because all the constituent methods had already been ported to SyntaxFactory.

}

pub fn token_tree_from_node(&self, node: &SyntaxNode) -> ast::TokenTree {
make::ext::token_tree_from_node(node).clone_for_update()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't this require mapping?

Copy link
Copy Markdown
Member Author

@Shourya742 Shourya742 Mar 30, 2026

Choose a reason for hiding this comment

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

Added. Thanks

@Shourya742 Shourya742 requested a review from A4-Tacks March 30, 2026 05:41
if let Some(mut mapping) = self.mappings() {
let mut builder = SyntaxMappingBuilder::new(ast.syntax().clone());
let target = ast
.l_curly_token()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could also be l_paren and l_brack

Copy link
Copy Markdown
Member Author

@Shourya742 Shourya742 Mar 30, 2026

Choose a reason for hiding this comment

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

Mapping here is itself wrong. My bad for not thinking deeper for this. Considering there is no child of same kind in output. Even the above implementation should panic while casting to a node, considering all the children are token. Removed.

…add mapping for token tree from node constructor
@Shourya742 Shourya742 force-pushed the 2026-03-26-migrate-gen-trait-fn-body branch from 68a2a56 to b70d09d Compare March 30, 2026 08:51
Copy link
Copy Markdown
Member

@A4-Tacks A4-Tacks left a comment

Choose a reason for hiding this comment

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

@A4-Tacks A4-Tacks enabled auto-merge March 30, 2026 08:59
@A4-Tacks A4-Tacks added this pull request to the merge queue Mar 30, 2026
Merged via the queue into rust-lang:master with commit dab40c3 Mar 30, 2026
17 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 30, 2026
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