Unnamed repository; edit this file 'description' to name the repository.
| -rw-r--r-- | crates/ide-assists/src/handlers/inline_call.rs | 264 |
1 files changed, 172 insertions, 92 deletions
diff --git a/crates/ide-assists/src/handlers/inline_call.rs b/crates/ide-assists/src/handlers/inline_call.rs index 21f2249a19..0f5b42bf22 100644 --- a/crates/ide-assists/src/handlers/inline_call.rs +++ b/crates/ide-assists/src/handlers/inline_call.rs @@ -1,6 +1,5 @@ use std::collections::BTreeSet; -use ast::make; use either::Either; use hir::{ FileRange, PathResolution, Semantics, TypeInfo, @@ -11,7 +10,6 @@ use ide_db::{ EditionedFileId, RootDatabase, base_db::Crate, defs::Definition, - imports::insert_use::remove_path_if_in_use_stmt, path_transform::PathTransform, search::{FileReference, FileReferenceNode, SearchScope}, source_change::SourceChangeBuilder, @@ -21,9 +19,11 @@ use itertools::{Itertools, izip}; use syntax::{ AstNode, NodeOrToken, SyntaxKind, ast::{ - self, HasArgList, HasGenericArgs, Pat, PathExpr, edit::IndentLevel, edit_in_place::Indent, + self, HasArgList, HasGenericArgs, Pat, PathExpr, + edit::{AstNodeEdit, IndentLevel}, + make, }, - ted, + syntax_editor::SyntaxEditor, }; use crate::{ @@ -108,35 +108,62 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) -> let mut remove_def = true; let mut inline_refs_for_file = |file_id: EditionedFileId, refs: Vec<FileReference>| { + use syntax::syntax_editor::Removable; + let file_id = file_id.file_id(ctx.db()); builder.edit_file(file_id); let call_krate = ctx.sema.file_to_module_def(file_id).map(|it| it.krate(ctx.db())); let count = refs.len(); - // The collects are required as we are otherwise iterating while mutating 🙅♀️🙅♂️ - let (name_refs, name_refs_use) = split_refs_and_uses(builder, refs, Some); + // Split refs into call-site name refs and use-tree paths + let (name_refs, use_trees): (Vec<ast::NameRef>, Vec<ast::UseTree>) = refs + .into_iter() + .filter_map(|file_ref| match file_ref.name { + FileReferenceNode::NameRef(name_ref) => Some(name_ref), + _ => None, + }) + .partition_map(|name_ref| { + match name_ref.syntax().ancestors().find_map(ast::UseTree::cast) { + Some(use_tree) => Either::Right(use_tree), + None => Either::Left(name_ref), + } + }); let call_infos: Vec<_> = name_refs .into_iter() .filter_map(|it| CallInfo::from_name_ref(it, call_krate?.into())) // FIXME: do not handle callsites in macros' parameters, because // directly inlining into macros may cause errors. .filter(|call_info| !ctx.sema.hir_file_for(call_info.node.syntax()).is_macro()) - .map(|call_info| { - let mut_node = builder.make_syntax_mut(call_info.node.syntax().clone()); - (call_info, mut_node) - }) .collect(); - let replaced = call_infos - .into_iter() - .map(|(call_info, mut_node)| { - let replacement = - inline(&ctx.sema, def_file, function, &func_body, ¶ms, &call_info); - ted::replace(mut_node, replacement.syntax()); - }) - .count(); - if replaced + name_refs_use.len() == count { - // we replaced all usages in this file, so we can remove the imports - name_refs_use.iter().for_each(remove_path_if_in_use_stmt); - } else { + if let Some(first) = call_infos.first() { + let mut editor = builder.make_editor(first.node.syntax()); + let replaced = call_infos + .into_iter() + .map(|call_info| { + let replacement = inline( + &ctx.sema, def_file, function, &func_body, ¶ms, &call_info, + ); + editor.replace(call_info.node.syntax(), replacement.syntax()); + }) + .count(); + if replaced + use_trees.len() == count { + // we replaced all usages in this file, so we can remove the imports + for use_tree in &use_trees { + if use_tree.use_tree_list().is_some() || use_tree.star_token().is_some() + { + continue; + } + if let Some(use_) = use_tree.syntax().parent().and_then(ast::Use::cast) + { + use_.remove(&mut editor); + } else { + use_tree.remove(&mut editor); + } + } + } else { + remove_def = false; + } + builder.add_file_edits(file_id, editor); + } else if use_trees.len() != count { remove_def = false; } }; @@ -148,7 +175,9 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) -> None => builder.edit_file(vfs_def_file), } if remove_def { - builder.delete(ast_func.syntax().text_range()); + let mut editor = builder.make_editor(ast_func.syntax()); + editor.delete(ast_func.syntax()); + builder.add_file_edits(vfs_def_file, editor); } }, ) @@ -320,19 +349,28 @@ fn inline( CallInfo { node, arguments, generic_arg_list, krate }: &CallInfo, ) -> ast::Expr { let file_id = sema.hir_file_for(fn_body.syntax()); - let mut body = if let Some(macro_file) = file_id.macro_file() { + let body_to_clone = if let Some(macro_file) = file_id.macro_file() { cov_mark::hit!(inline_call_defined_in_macro); let span_map = sema.db.expansion_span_map(macro_file); let body_prettified = prettify_macro_expansion(sema.db, fn_body.syntax().clone(), &span_map, *krate); - if let Some(body) = ast::BlockExpr::cast(body_prettified) { - body - } else { - fn_body.clone_for_update() - } + if let Some(body) = ast::BlockExpr::cast(body_prettified) { body } else { fn_body.clone() } } else { - fn_body.clone_for_update() + fn_body.clone() }; + + // Capture indent level before SyntaxEditor re-roots via clone_subtree. + // For non-macro bodies, body_to_clone still has its parent, so from_node + // finds the whitespace before `{` and returns the correct source indent level. + // For macro/prettified bodies (already re-rooted at 0), from_node returns 0, + // which is also correct since prettify_macro_expansion rebuilds indent from 0. + let original_body_indent = IndentLevel::from_node(body_to_clone.syntax()); + + // The original body's start offset — needed to adjust FileReference ranges + // since SyntaxEditor::with_ast_node re-roots the tree to offset 0 + let body_offset = body_to_clone.syntax().text_range().start(); + let (mut editor, body) = SyntaxEditor::with_ast_node(&body_to_clone); + let usages_for_locals = |local| { Definition::Local(local) .usages(sema) @@ -355,7 +393,7 @@ fn inline( .map(|FileReference { name, range, .. }| match name { FileReferenceNode::NameRef(_) => body .syntax() - .covering_element(range) + .covering_element(range - body_offset) .ancestors() .nth(3) .and_then(ast::PathExpr::cast), @@ -368,48 +406,59 @@ fn inline( }) .collect(); - if function.self_param(sema.db).is_some() { - let this = || { - make::name_ref("this") - .syntax() - .clone_for_update() - .first_token() - .expect("NameRef should have had a token.") - }; - if let Some(self_local) = params[0].2.as_local(sema.db) { - usages_for_locals(self_local) - .filter_map(|FileReference { name, range, .. }| match name { - FileReferenceNode::NameRef(_) => Some(body.syntax().covering_element(range)), - _ => None, - }) - .for_each(|usage| { - ted::replace(usage, this()); - }); - } - } + let has_self_param = function.self_param(sema.db).is_some(); + + // Collect all self token usages upfront (needed when a let binding is emitted). + // When self can be directly inlined, the parameter loop handles those PathExprs; + // when a let stmt is needed, we rename all self tokens to `this` here. + let self_token_usages: Vec<_> = if has_self_param { + params[0] + .2 + .as_local(sema.db) + .map(|self_local| { + usages_for_locals(self_local) + .filter_map(|FileReference { name, range, .. }| match name { + FileReferenceNode::NameRef(_) => { + Some(body.syntax().covering_element(range - body_offset)) + } + _ => None, + }) + .collect() + }) + .unwrap_or_default() + } else { + Vec::new() + }; - // We should place the following code after last usage of `usages_for_locals` - // because `ted::replace` will change the offset in syntax tree, which makes - // `FileReference` incorrect + // Replace `Self` type keywords with the actual impl type if let Some(imp) = sema.ancestors_with_macros(fn_body.syntax().clone()).find_map(ast::Impl::cast) && !node.syntax().ancestors().any(|anc| &anc == imp.syntax()) && let Some(t) = imp.self_ty() { - while let Some(self_tok) = body + let self_tokens: Vec<_> = body .syntax() .descendants_with_tokens() .filter_map(NodeOrToken::into_token) - .find(|tok| tok.kind() == SyntaxKind::SELF_TYPE_KW) - { - let replace_with = t.clone_subtree().syntax().clone_for_update(); - if !is_in_type_path(&self_tok) - && let Some(ty) = ast::Type::cast(replace_with.clone()) - && let Some(generic_arg_list) = ty.generic_arg_list() - { - ted::remove(generic_arg_list.syntax()); + .filter(|tok| tok.kind() == SyntaxKind::SELF_TYPE_KW) + .collect(); + for self_tok in self_tokens { + let mut replace_with = t.clone_subtree().syntax().clone_subtree(); + if !is_in_type_path(&self_tok) { + if let Some(ty) = ast::Type::cast(replace_with.clone()) { + if let Some(generic_arg_list) = ty.generic_arg_list() { + // Build replacement without generics using a sub-editor + let (mut sub_editor, sub_root) = SyntaxEditor::new(replace_with.clone()); + let generic_node = sub_root + .descendants() + .find(|n| n.text_range() == generic_arg_list.syntax().text_range()) + .unwrap(); + sub_editor.delete(generic_node); + replace_with = sub_editor.finish().new_root().clone(); + } + } } - ted::replace(self_tok, replace_with); + editor.replace(self_tok, replace_with); } } @@ -428,7 +477,7 @@ fn inline( } } - let mut let_stmts = Vec::new(); + let mut let_stmts: Vec<ast::Stmt> = Vec::new(); // Inline parameter expressions or generate `let` statements depending on whether inlining works or not. for ((pat, param_ty, param), usages, expr) in izip!(params, param_use_nodes, arguments) { @@ -486,30 +535,46 @@ fn inline( } } }; - let_stmts - .push(make::let_stmt(this_pat.into(), ty, Some(expr)).clone_for_update().into()) + let_stmts.push(make::let_stmt(this_pat.into(), ty, Some(expr)).into()) } else { - let_stmts.push( - make::let_stmt(pat.clone(), ty, Some(expr.clone())).clone_for_update().into(), - ); + let_stmts.push(make::let_stmt(pat.clone(), ty, Some(expr.clone())).into()); } }; + let is_self_param = + has_self_param && param.name(sema.db).is_some_and(|name| name == sym::self_); + // check if there is a local var in the function that conflicts with parameter // if it does then emit a let statement and continue if func_let_vars.contains(&expr.syntax().text().to_string()) { + if is_self_param { + for usage in &self_token_usages { + let this_token = make::name_ref("this") + .syntax() + .clone_subtree() + .first_token() + .expect("NameRef should have had a token."); + editor.replace(usage.clone(), this_token); + } + } insert_let_stmt(); continue; } - let inline_direct = |usage, replacement: &ast::Expr| { - if let Some(field) = path_expr_as_record_field(usage) { - cov_mark::hit!(inline_call_inline_direct_field); - field.replace_expr(replacement.clone_for_update()); - } else { - ted::replace(usage.syntax(), replacement.syntax().clone_for_update()); - } - }; + let inline_direct = + |editor: &mut SyntaxEditor, usage: &PathExpr, replacement: &ast::Expr| { + if let Some(field) = path_expr_as_record_field(usage) { + cov_mark::hit!(inline_call_inline_direct_field); + let field_name = field.field_name().unwrap(); + let new_field = make::record_expr_field( + make::name_ref(&field_name.text()), + Some(replacement.clone()), + ); + editor.replace(field.syntax(), new_field.syntax()); + } else { + editor.replace(usage.syntax(), replacement.syntax().clone_subtree()); + } + }; match usages { // inline single use closure arguments @@ -519,29 +584,45 @@ fn inline( { cov_mark::hit!(inline_call_inline_closure); let expr = make::expr_paren(expr.clone()).into(); - inline_direct(usage, &expr); + inline_direct(&mut editor, usage, &expr); } // inline single use literals [usage] if matches!(expr, ast::Expr::Literal(_)) => { cov_mark::hit!(inline_call_inline_literal); - inline_direct(usage, expr); + inline_direct(&mut editor, usage, expr); } // inline direct local arguments [_, ..] if expr_as_name_ref(expr).is_some() => { cov_mark::hit!(inline_call_inline_locals); - usages.iter().for_each(|usage| inline_direct(usage, expr)); + usages.iter().for_each(|usage| inline_direct(&mut editor, usage, expr)); } // can't inline, emit a let statement _ => { + if is_self_param { + // Rename all `self` tokens to `this` so the let binding matches. + for usage in &self_token_usages { + let this_token = make::name_ref("this") + .syntax() + .clone_subtree() + .first_token() + .expect("NameRef should have had a token."); + editor.replace(usage.clone(), this_token); + } + } insert_let_stmt(); } } } + // Apply all edits to get the transformed body + let edit = editor.finish(); + let mut body = ast::BlockExpr::cast(edit.new_root().clone()) + .expect("editor root should still be a BlockExpr"); + + // Apply generic substitution (needs immutable tree) if let Some(generic_arg_list) = generic_arg_list.clone() && let Some((target, source)) = &sema.scope(node.syntax()).zip(sema.scope(fn_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()), @@ -553,31 +634,30 @@ fn inline( let is_async_fn = function.is_async(sema.db); if is_async_fn { cov_mark::hit!(inline_call_async_fn); - body = make::async_move_block_expr(body.statements(), body.tail_expr()).clone_for_update(); + body = make::async_move_block_expr(body.statements(), body.tail_expr()); // Arguments should be evaluated outside the async block, and then moved into it. if !let_stmts.is_empty() { cov_mark::hit!(inline_call_async_fn_with_let_stmts); - body.indent(IndentLevel(1)); - body = make::block_expr(let_stmts, Some(body.into())).clone_for_update(); + body = body.indent(IndentLevel(1)); + body = make::block_expr(let_stmts, Some(body.into())); } - } else if let Some(stmt_list) = body.stmt_list() { - let position = stmt_list.l_curly_token().expect("L_CURLY for StatementList is missing."); - let_stmts.into_iter().rev().for_each(|let_stmt| { - ted::insert(ted::Position::after(position.clone()), let_stmt.syntax().clone()); - }); + } else if !let_stmts.is_empty() { + // Prepend let statements to the body's existing statements + let stmts: Vec<ast::Stmt> = let_stmts.into_iter().chain(body.statements()).collect(); + body = make::block_expr(stmts, body.tail_expr()); } let original_indentation = match node { ast::CallableExpr::Call(it) => it.indent_level(), ast::CallableExpr::MethodCall(it) => it.indent_level(), }; - body.reindent_to(original_indentation); + body = body.dedent(original_body_indent).indent(original_indentation); let no_stmts = body.statements().next().is_none(); match body.tail_expr() { Some(expr) if matches!(expr, ast::Expr::ClosureExpr(_)) && no_stmts => { - make::expr_paren(expr).clone_for_update().into() + make::expr_paren(expr).into() } Some(expr) if !is_async_fn && no_stmts => expr, _ => match node @@ -587,7 +667,7 @@ fn inline( .and_then(|bin_expr| bin_expr.lhs()) { Some(lhs) if lhs.syntax() == node.syntax() => { - make::expr_paren(ast::Expr::BlockExpr(body)).clone_for_update().into() + make::expr_paren(ast::Expr::BlockExpr(body)).into() } _ => ast::Expr::BlockExpr(body), }, |