From 46e86c6aa2d070d2c3ae8b22ee18961bd305e11f Mon Sep 17 00:00:00 2001 From: Hayashi Mikihiro <34ttrweoewiwe28@gmail.com> Date: Sat, 26 Jul 2025 13:26:39 +0900 Subject: [PATCH] Migrate PathTransform to SyntaxEditor Signed-off-by: Hayashi Mikihiro <34ttrweoewiwe28@gmail.com> --- .../extract_struct_from_enum_variant.rs | 29 +++-- .../src/handlers/generate_delegate_methods.rs | 8 +- .../src/handlers/generate_delegate_trait.rs | 50 ++++---- .../src/handlers/generate_function.rs | 29 +++-- .../ide-assists/src/handlers/inline_call.rs | 9 +- crates/ide-assists/src/utils.rs | 9 +- .../src/completions/item_list/trait_impl.rs | 4 +- crates/ide-db/src/path_transform.rs | 116 ++++++++++++------ 8 files changed, 168 insertions(+), 86 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs index b9c42285d2..9095b1825f 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs @@ -16,8 +16,9 @@ use syntax::{ SyntaxKind::*, SyntaxNode, T, ast::{ - self, AstNode, HasAttrs, HasGenericParams, HasName, HasVisibility, edit::IndentLevel, - edit_in_place::Indent, make, + self, AstNode, HasAttrs, HasGenericParams, HasName, HasVisibility, + edit::{AstNodeEdit, IndentLevel}, + make, }, match_ast, ted, }; @@ -110,20 +111,30 @@ pub(crate) fn extract_struct_from_enum_variant( let generics = generic_params.as_ref().map(|generics| generics.clone_for_update()); // resolve GenericArg in field_list to actual type - let field_list = field_list.clone_for_update(); - if let Some((target_scope, source_scope)) = + let field_list = if let Some((target_scope, source_scope)) = ctx.sema.scope(enum_ast.syntax()).zip(ctx.sema.scope(field_list.syntax())) { - PathTransform::generic_transformation(&target_scope, &source_scope) - .apply(field_list.syntax()); - } + let field_list = field_list.reset_indent(); + let field_list = + PathTransform::generic_transformation(&target_scope, &source_scope) + .apply(field_list.syntax()); + match_ast! { + match field_list { + ast::RecordFieldList(field_list) => Either::Left(field_list), + ast::TupleFieldList(field_list) => Either::Right(field_list), + _ => unreachable!(), + } + } + } else { + field_list.clone_for_update() + }; let def = create_struct_def(variant_name.clone(), &variant, &field_list, generics, &enum_ast); let enum_ast = variant.parent_enum(); let indent = enum_ast.indent_level(); - def.reindent_to(indent); + let def = def.indent(indent); ted::insert_all( ted::Position::before(enum_ast.syntax()), @@ -279,7 +290,7 @@ fn create_struct_def( field_list.clone().into() } }; - field_list.reindent_to(IndentLevel::single()); + let field_list = field_list.indent(IndentLevel::single()); let strukt = make::struct_(enum_vis, name, generics, field_list).clone_for_update(); diff --git a/crates/ide-assists/src/handlers/generate_delegate_methods.rs b/crates/ide-assists/src/handlers/generate_delegate_methods.rs index ca66cb69dc..6063898076 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_methods.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_methods.rs @@ -114,9 +114,13 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<' let source_scope = ctx.sema.scope(v.syntax()); let target_scope = ctx.sema.scope(strukt.syntax()); if let (Some(s), Some(t)) = (source_scope, target_scope) { - PathTransform::generic_transformation(&t, &s).apply(v.syntax()); + ast::Fn::cast( + PathTransform::generic_transformation(&t, &s).apply(v.syntax()), + ) + .unwrap_or(v) + } else { + v } - v } None => return, }; diff --git a/crates/ide-assists/src/handlers/generate_delegate_trait.rs b/crates/ide-assists/src/handlers/generate_delegate_trait.rs index 2bdc85e785..e96250f3c5 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_trait.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_trait.rs @@ -255,7 +255,6 @@ fn generate_impl( delegee: &Delegee, edition: Edition, ) -> Option { - let delegate: ast::Impl; let db = ctx.db(); let ast_strukt = &strukt.strukt; let strukt_ty = make::ty_path(make::ext::ident_path(&strukt.name.to_string())); @@ -266,7 +265,7 @@ fn generate_impl( let bound_def = ctx.sema.source(delegee.to_owned())?.value; let bound_params = bound_def.generic_param_list(); - delegate = make::impl_trait( + let delegate = make::impl_trait( delegee.is_unsafe(db), bound_params.clone(), bound_params.map(|params| params.to_generic_args()), @@ -304,7 +303,7 @@ fn generate_impl( let target_scope = ctx.sema.scope(strukt.strukt.syntax())?; let source_scope = ctx.sema.scope(bound_def.syntax())?; let transform = PathTransform::generic_transformation(&target_scope, &source_scope); - transform.apply(delegate.syntax()); + ast::Impl::cast(transform.apply(delegate.syntax())) } Delegee::Impls(trait_, old_impl) => { let old_impl = ctx.sema.source(old_impl.to_owned())?.value; @@ -358,20 +357,28 @@ fn generate_impl( // 2.3) Instantiate generics with `transform_impl`, this step also // remove unused params. - let mut trait_gen_args = old_impl.trait_()?.generic_arg_list(); - if let Some(trait_args) = &mut trait_gen_args { - *trait_args = trait_args.clone_for_update(); - transform_impl(ctx, ast_strukt, &old_impl, &transform_args, trait_args.syntax())?; - } + let trait_gen_args = old_impl.trait_()?.generic_arg_list().and_then(|trait_args| { + let trait_args = &mut trait_args.clone_for_update(); + if let Some(new_args) = transform_impl( + ctx, + ast_strukt, + &old_impl, + &transform_args, + trait_args.clone_subtree(), + ) { + *trait_args = new_args.clone_subtree(); + Some(new_args) + } else { + None + } + }); let type_gen_args = strukt_params.clone().map(|params| params.to_generic_args()); - let path_type = make::ty(&trait_.name(db).display_no_db(edition).to_smolstr()).clone_for_update(); - transform_impl(ctx, ast_strukt, &old_impl, &transform_args, path_type.syntax())?; - + let path_type = transform_impl(ctx, ast_strukt, &old_impl, &transform_args, path_type)?; // 3) Generate delegate trait impl - delegate = make::impl_trait( + let delegate = make::impl_trait( trait_.is_unsafe(db), trait_gen_params, trait_gen_args, @@ -385,7 +392,6 @@ fn generate_impl( None, ) .clone_for_update(); - // Goto link : https://doc.rust-lang.org/reference/paths.html#qualified-paths let qualified_path_type = make::path_from_text(&format!("<{} as {}>", field_ty, delegate.trait_()?)); @@ -398,7 +404,7 @@ fn generate_impl( .filter(|item| matches!(item, AssocItem::MacroCall(_)).not()) { let item = item.clone_for_update(); - transform_impl(ctx, ast_strukt, &old_impl, &transform_args, item.syntax())?; + let item = transform_impl(ctx, ast_strukt, &old_impl, &transform_args, item)?; let assoc = process_assoc_item(item, qualified_path_type.clone(), field_name)?; delegate_assoc_items.add_item(assoc); @@ -408,19 +414,18 @@ fn generate_impl( if let Some(wc) = delegate.where_clause() { remove_useless_where_clauses(&delegate.trait_()?, &delegate.self_ty()?, wc); } + Some(delegate) } } - - Some(delegate) } -fn transform_impl( +fn transform_impl( ctx: &AssistContext<'_>, strukt: &ast::Struct, old_impl: &ast::Impl, args: &Option, - syntax: &syntax::SyntaxNode, -) -> Option<()> { + syntax: N, +) -> Option { let source_scope = ctx.sema.scope(old_impl.self_ty()?.syntax())?; let target_scope = ctx.sema.scope(strukt.syntax())?; let hir_old_impl = ctx.sema.to_impl_def(old_impl)?; @@ -437,8 +442,7 @@ fn transform_impl( }, ); - transform.apply(syntax); - Some(()) + N::cast(transform.apply(syntax.syntax())) } fn remove_instantiated_params( @@ -570,9 +574,7 @@ where let scope = ctx.sema.scope(item.syntax())?; let transform = PathTransform::adt_transformation(&scope, &scope, hir_adt, args.clone()); - transform.apply(item.syntax()); - - Some(item) + N::cast(transform.apply(item.syntax())) } fn has_self_type(trait_: hir::Trait, ctx: &AssistContext<'_>) -> Option<()> { diff --git a/crates/ide-assists/src/handlers/generate_function.rs b/crates/ide-assists/src/handlers/generate_function.rs index 78ae815dc8..3290a70e1c 100644 --- a/crates/ide-assists/src/handlers/generate_function.rs +++ b/crates/ide-assists/src/handlers/generate_function.rs @@ -743,17 +743,30 @@ fn fn_generic_params( let where_preds: Vec = where_preds.into_iter().map(|it| it.node.clone_for_update()).collect(); - // 4. Rewrite paths - if let Some(param) = generic_params.first() { - let source_scope = ctx.sema.scope(param.syntax())?; - let target_scope = ctx.sema.scope(&target.parent())?; - if source_scope.module() != target_scope.module() { + let (generic_params, where_preds): (Vec, Vec) = + if let Some(param) = generic_params.first() + && let source_scope = ctx.sema.scope(param.syntax())? + && let target_scope = ctx.sema.scope(&target.parent())? + && source_scope.module() != target_scope.module() + { + // 4. Rewrite paths let transform = PathTransform::generic_transformation(&target_scope, &source_scope); let generic_params = generic_params.iter().map(|it| it.syntax()); let where_preds = where_preds.iter().map(|it| it.syntax()); - transform.apply_all(generic_params.chain(where_preds)); - } - } + transform + .apply_all(generic_params.chain(where_preds)) + .into_iter() + .filter_map(|it| { + if let Some(it) = ast::GenericParam::cast(it.clone()) { + Some(either::Either::Left(it)) + } else { + ast::WherePred::cast(it).map(either::Either::Right) + } + }) + .partition_map(|it| it) + } else { + (generic_params, where_preds) + }; let generic_param_list = make::generic_param_list(generic_params); let where_clause = diff --git a/crates/ide-assists/src/handlers/inline_call.rs b/crates/ide-assists/src/handlers/inline_call.rs index b7b8bc604a..1549b414dc 100644 --- a/crates/ide-assists/src/handlers/inline_call.rs +++ b/crates/ide-assists/src/handlers/inline_call.rs @@ -537,8 +537,13 @@ fn inline( if let Some(generic_arg_list) = generic_arg_list.clone() { if let Some((target, source)) = &sema.scope(node.syntax()).zip(sema.scope(fn_body.syntax())) { - PathTransform::function_call(target, source, function, generic_arg_list) - .apply(body.syntax()); + body.reindent_to(IndentLevel(0)); + if let Some(new_body) = ast::BlockExpr::cast( + PathTransform::function_call(target, source, function, generic_arg_list) + .apply(body.syntax()), + ) { + body = new_body; + } } } diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 3457367645..12fff32be0 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -191,7 +191,7 @@ pub fn add_trait_assoc_items_to_impl( original_items .iter() .map(|InFile { file_id, value: original_item }| { - let cloned_item = { + let mut cloned_item = { if let Some(macro_file) = file_id.macro_file() { let span_map = sema.db.expansion_span_map(macro_file); let item_prettified = prettify_macro_expansion( @@ -207,17 +207,18 @@ pub fn add_trait_assoc_items_to_impl( } } original_item.clone_for_update() - }; + } + .reset_indent(); if let Some(source_scope) = sema.scope(original_item.syntax()) { // FIXME: Paths in nested macros are not handled well. See // `add_missing_impl_members::paths_in_nested_macro_should_get_transformed` test. let transform = PathTransform::trait_impl(target_scope, &source_scope, trait_, impl_.clone()); - transform.apply(cloned_item.syntax()); + cloned_item = ast::AssocItem::cast(transform.apply(cloned_item.syntax())).unwrap(); } cloned_item.remove_attrs_and_docs(); - cloned_item.reset_indent() + cloned_item }) .map(|item| { match &item { diff --git a/crates/ide-completion/src/completions/item_list/trait_impl.rs b/crates/ide-completion/src/completions/item_list/trait_impl.rs index 975c2f0225..bcf8c0ec52 100644 --- a/crates/ide-completion/src/completions/item_list/trait_impl.rs +++ b/crates/ide-completion/src/completions/item_list/trait_impl.rs @@ -276,7 +276,7 @@ fn get_transformed_assoc_item( let assoc_item = assoc_item.clone_for_update(); // FIXME: Paths in nested macros are not handled well. See // `macro_generated_assoc_item2` test. - transform.apply(assoc_item.syntax()); + let assoc_item = ast::AssocItem::cast(transform.apply(assoc_item.syntax()))?; assoc_item.remove_attrs_and_docs(); Some(assoc_item) } @@ -301,7 +301,7 @@ fn get_transformed_fn( let fn_ = fn_.clone_for_update(); // FIXME: Paths in nested macros are not handled well. See // `macro_generated_assoc_item2` test. - transform.apply(fn_.syntax()); + let fn_ = ast::Fn::cast(transform.apply(fn_.syntax()))?; fn_.remove_attrs_and_docs(); match async_ { AsyncSugaring::Desugar => { diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs index 36e28dfa4c..b7432d89c7 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -12,7 +12,7 @@ use span::Edition; use syntax::{ NodeOrToken, SyntaxNode, ast::{self, AstNode, HasGenericArgs, make}, - ted, + syntax_editor::{self, SyntaxEditor}, }; #[derive(Default, Debug)] @@ -129,15 +129,18 @@ impl<'a> PathTransform<'a> { } } - pub fn apply(&self, syntax: &SyntaxNode) { + #[must_use] + pub fn apply(&self, syntax: &SyntaxNode) -> SyntaxNode { self.build_ctx().apply(syntax) } - pub fn apply_all<'b>(&self, nodes: impl IntoIterator) { + #[must_use] + pub fn apply_all<'b>( + &self, + nodes: impl IntoIterator, + ) -> Vec { let ctx = self.build_ctx(); - for node in nodes { - ctx.apply(node); - } + nodes.into_iter().map(|node| ctx.apply(&node.clone())).collect() } fn prettify_target_node(&self, node: SyntaxNode) -> SyntaxNode { @@ -237,7 +240,7 @@ impl<'a> PathTransform<'a> { Some((k.name(db).display(db, target_edition).to_string(), v.lifetime()?)) }) .collect(); - let ctx = Ctx { + let mut ctx = Ctx { type_substs, const_substs, lifetime_substs, @@ -273,42 +276,75 @@ fn preorder_rev(item: &SyntaxNode) -> impl Iterator { } impl Ctx<'_> { - fn apply(&self, item: &SyntaxNode) { + fn apply(&self, item: &SyntaxNode) -> SyntaxNode { // `transform_path` may update a node's parent and that would break the // tree traversal. Thus all paths in the tree are collected into a vec // so that such operation is safe. - let paths = preorder_rev(item).filter_map(ast::Path::cast).collect::>(); - for path in paths { - self.transform_path(path); - } - - preorder_rev(item).filter_map(ast::Lifetime::cast).for_each(|lifetime| { + let item = self.transform_path(item).clone_subtree(); + let mut editor = SyntaxEditor::new(item.clone()); + preorder_rev(&item).filter_map(ast::Lifetime::cast).for_each(|lifetime| { if let Some(subst) = self.lifetime_substs.get(&lifetime.syntax().text().to_string()) { - ted::replace(lifetime.syntax(), subst.clone_subtree().clone_for_update().syntax()); + editor + .replace(lifetime.syntax(), subst.clone_subtree().clone_for_update().syntax()); } }); + + editor.finish().new_root().clone() } - fn transform_default_values(&self, defaulted_params: Vec) { + fn transform_default_values(&mut self, defaulted_params: Vec) { // By now the default values are simply copied from where they are declared // and should be transformed. As any value is allowed to refer to previous // generic (both type and const) parameters, they should be all iterated left-to-right. for param in defaulted_params { - let value = match param { - Either::Left(k) => self.type_substs.get(&k).unwrap().syntax(), - Either::Right(k) => self.const_substs.get(&k).unwrap(), + let value = match ¶m { + Either::Left(k) => self.type_substs.get(k).unwrap().syntax(), + Either::Right(k) => self.const_substs.get(k).unwrap(), }; // `transform_path` may update a node's parent and that would break the // tree traversal. Thus all paths in the tree are collected into a vec // so that such operation is safe. - let paths = preorder_rev(value).filter_map(ast::Path::cast).collect::>(); - for path in paths { - self.transform_path(path); + let new_value = self.transform_path(value); + match param { + Either::Left(k) => { + self.type_substs.insert(k, ast::Type::cast(new_value.clone()).unwrap()); + } + Either::Right(k) => { + self.const_substs.insert(k, new_value.clone()); + } } } } - fn transform_path(&self, path: ast::Path) -> Option<()> { + fn transform_path(&self, path: &SyntaxNode) -> SyntaxNode { + fn find_child_paths(root_path: &SyntaxNode) -> Vec { + let mut result = Vec::new(); + for child in root_path.children() { + if let Some(child_path) = ast::Path::cast(child.clone()) { + result.push(child_path); + } else { + result.extend(find_child_paths(&child)); + } + } + result + } + let root_path = path.clone_subtree(); + let result = find_child_paths(&root_path); + let mut editor = SyntaxEditor::new(root_path.clone()); + for sub_path in result { + let new = self.transform_path(sub_path.syntax()); + editor.replace(sub_path.syntax(), new); + } + let update_sub_item = editor.finish().new_root().clone().clone_subtree(); + let item = find_child_paths(&update_sub_item); + let mut editor = SyntaxEditor::new(update_sub_item); + for sub_path in item { + self.transform_path_(&mut editor, &sub_path); + } + editor.finish().new_root().clone() + } + + fn transform_path_(&self, editor: &mut SyntaxEditor, path: &ast::Path) -> Option<()> { if path.qualifier().is_some() { return None; } @@ -320,8 +356,7 @@ impl Ctx<'_> { // don't try to qualify sole `self` either, they are usually locals, but are returned as modules due to namespace clashing return None; } - - let resolution = self.source_scope.speculative_resolve(&path)?; + let resolution = self.source_scope.speculative_resolve(path)?; match resolution { hir::PathResolution::TypeParam(tp) => { @@ -361,12 +396,12 @@ impl Ctx<'_> { let segment = make::path_segment_ty(subst.clone(), trait_ref); let qualified = make::path_from_segments(std::iter::once(segment), false); - ted::replace(path.syntax(), qualified.clone_for_update().syntax()); + editor.replace(path.syntax(), qualified.clone_for_update().syntax()); } else if let Some(path_ty) = ast::PathType::cast(parent) { let old = path_ty.syntax(); if old.parent().is_some() { - ted::replace(old, subst.clone_subtree().clone_for_update().syntax()); + editor.replace(old, subst.clone_subtree().clone_for_update().syntax()); } else { // Some `path_ty` has no parent, especially ones made for default value // of type parameters. @@ -378,13 +413,13 @@ impl Ctx<'_> { } let start = path_ty.syntax().first_child().map(NodeOrToken::Node)?; let end = path_ty.syntax().last_child().map(NodeOrToken::Node)?; - ted::replace_all( + editor.replace_all( start..=end, new.syntax().children().map(NodeOrToken::Node).collect::>(), ); } } else { - ted::replace( + editor.replace( path.syntax(), subst.clone_subtree().clone_for_update().syntax(), ); @@ -410,17 +445,28 @@ impl Ctx<'_> { }; let found_path = self.target_module.find_path(self.source_scope.db, def, cfg)?; let res = mod_path_to_ast(&found_path, self.target_edition).clone_for_update(); + let mut res_editor = SyntaxEditor::new(res.syntax().clone_subtree()); if let Some(args) = path.segment().and_then(|it| it.generic_arg_list()) { if let Some(segment) = res.segment() { - let old = segment.get_or_create_generic_arg_list(); - ted::replace(old.syntax(), args.clone_subtree().syntax().clone_for_update()) + if let Some(old) = segment.generic_arg_list() { + res_editor.replace( + old.syntax(), + args.clone_subtree().syntax().clone_for_update(), + ) + } else { + res_editor.insert( + syntax_editor::Position::last_child_of(segment.syntax()), + args.clone_subtree().syntax().clone_for_update(), + ); + } } } - ted::replace(path.syntax(), res.syntax()) + let res = res_editor.finish().new_root().clone(); + editor.replace(path.syntax().clone(), res); } hir::PathResolution::ConstParam(cp) => { if let Some(subst) = self.const_substs.get(&cp) { - ted::replace(path.syntax(), subst.clone_subtree().clone_for_update()); + editor.replace(path.syntax(), subst.clone_subtree().clone_for_update()); } } hir::PathResolution::SelfType(imp) => { @@ -457,13 +503,13 @@ impl Ctx<'_> { mod_path_to_ast(&found_path, self.target_edition).qualifier() { let res = make::path_concat(qual, path_ty.path()?).clone_for_update(); - ted::replace(path.syntax(), res.syntax()); + editor.replace(path.syntax(), res.syntax()); return Some(()); } } } - ted::replace(path.syntax(), ast_ty.syntax()); + editor.replace(path.syntax(), ast_ty.syntax()); } hir::PathResolution::Local(_) | hir::PathResolution::Def(_)