Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #16082 - DropDemBits:structured-snippet-migrate-5, r=Veykril
internal: Migrate assists to the structured snippet API, part 5
Continuing from #15874
Migrates the following assists:
- `extract_variable`
- `generate_function`
- `replace_is_some_with_if_let_some`
- `replace_is_ok_with_if_let_ok`
| -rw-r--r-- | crates/ide-assists/src/handlers/extract_variable.rs | 223 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/generate_function.rs | 302 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs | 34 | ||||
| -rw-r--r-- | crates/ide-assists/src/tests.rs | 50 |
4 files changed, 367 insertions, 242 deletions
diff --git a/crates/ide-assists/src/handlers/extract_variable.rs b/crates/ide-assists/src/handlers/extract_variable.rs index e7c884dcb7..874b81d3b6 100644 --- a/crates/ide-assists/src/handlers/extract_variable.rs +++ b/crates/ide-assists/src/handlers/extract_variable.rs @@ -1,12 +1,8 @@ use hir::TypeInfo; -use stdx::format_to; use syntax::{ - ast::{self, AstNode}, - NodeOrToken, - SyntaxKind::{ - BLOCK_EXPR, BREAK_EXPR, CLOSURE_EXPR, COMMENT, LOOP_EXPR, MATCH_ARM, MATCH_GUARD, - PATH_EXPR, RETURN_EXPR, - }, + ast::{self, edit::IndentLevel, edit_in_place::Indent, make, AstNode, HasName}, + ted, NodeOrToken, + SyntaxKind::{BLOCK_EXPR, BREAK_EXPR, COMMENT, LOOP_EXPR, MATCH_GUARD, PATH_EXPR, RETURN_EXPR}, SyntaxNode, }; @@ -66,98 +62,140 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op .as_ref() .map_or(false, |it| matches!(it, ast::Expr::FieldExpr(_) | ast::Expr::MethodCallExpr(_))); - let reference_modifier = match ty.filter(|_| needs_adjust) { - Some(receiver_type) if receiver_type.is_mutable_reference() => "&mut ", - Some(receiver_type) if receiver_type.is_reference() => "&", - _ => "", - }; - - let var_modifier = match parent { - Some(ast::Expr::RefExpr(expr)) if expr.mut_token().is_some() => "mut ", - _ => "", - }; - let anchor = Anchor::from(&to_extract)?; - let indent = anchor.syntax().prev_sibling_or_token()?.as_token()?.clone(); let target = to_extract.syntax().text_range(); acc.add( AssistId("extract_variable", AssistKind::RefactorExtract), "Extract into variable", target, move |edit| { - let field_shorthand = - match to_extract.syntax().parent().and_then(ast::RecordExprField::cast) { - Some(field) => field.name_ref(), - None => None, - }; - - let mut buf = String::new(); - - let var_name = match &field_shorthand { - Some(it) => it.to_string(), - None => suggest_name::for_variable(&to_extract, &ctx.sema), + let field_shorthand = to_extract + .syntax() + .parent() + .and_then(ast::RecordExprField::cast) + .filter(|field| field.name_ref().is_some()); + + let (var_name, expr_replace) = match field_shorthand { + Some(field) => (field.to_string(), field.syntax().clone()), + None => ( + suggest_name::for_variable(&to_extract, &ctx.sema), + to_extract.syntax().clone(), + ), }; - let expr_range = match &field_shorthand { - Some(it) => it.syntax().text_range().cover(to_extract.syntax().text_range()), - None => to_extract.syntax().text_range(), + + let ident_pat = match parent { + Some(ast::Expr::RefExpr(expr)) if expr.mut_token().is_some() => { + make::ident_pat(false, true, make::name(&var_name)) + } + _ => make::ident_pat(false, false, make::name(&var_name)), }; - match anchor { - Anchor::Before(_) | Anchor::Replace(_) => { - format_to!(buf, "let {var_modifier}{var_name} = {reference_modifier}") + let to_extract = match ty.as_ref().filter(|_| needs_adjust) { + Some(receiver_type) if receiver_type.is_mutable_reference() => { + make::expr_ref(to_extract, true) } - Anchor::WrapInBlock(_) => { - format_to!(buf, "{{ let {var_name} = {reference_modifier}") + Some(receiver_type) if receiver_type.is_reference() => { + make::expr_ref(to_extract, false) } + _ => to_extract, }; - format_to!(buf, "{to_extract}"); - if let Anchor::Replace(stmt) = anchor { - cov_mark::hit!(test_extract_var_expr_stmt); - if stmt.semicolon_token().is_none() { - buf.push(';'); - } - match ctx.config.snippet_cap { - Some(cap) => { - let snip = buf.replace( - &format!("let {var_modifier}{var_name}"), - &format!("let {var_modifier}$0{var_name}"), - ); - edit.replace_snippet(cap, expr_range, snip) + let expr_replace = edit.make_syntax_mut(expr_replace); + let let_stmt = + make::let_stmt(ident_pat.into(), None, Some(to_extract)).clone_for_update(); + let name_expr = make::expr_path(make::ext::ident_path(&var_name)).clone_for_update(); + + match anchor { + Anchor::Before(place) => { + let prev_ws = place.prev_sibling_or_token().and_then(|it| it.into_token()); + let indent_to = IndentLevel::from_node(&place); + let insert_place = edit.make_syntax_mut(place); + + // Adjust ws to insert depending on if this is all inline or on separate lines + let trailing_ws = if prev_ws.is_some_and(|it| it.text().starts_with("\n")) { + format!("\n{indent_to}") + } else { + format!(" ") + }; + + ted::insert_all_raw( + ted::Position::before(insert_place), + vec![ + let_stmt.syntax().clone().into(), + make::tokens::whitespace(&trailing_ws).into(), + ], + ); + + ted::replace(expr_replace, name_expr.syntax()); + + if let Some(cap) = ctx.config.snippet_cap { + if let Some(ast::Pat::IdentPat(ident_pat)) = let_stmt.pat() { + if let Some(name) = ident_pat.name() { + edit.add_tabstop_before(cap, name); + } + } } - None => edit.replace(expr_range, buf), } - return; - } + Anchor::Replace(stmt) => { + cov_mark::hit!(test_extract_var_expr_stmt); - buf.push(';'); - - // We want to maintain the indent level, - // but we do not want to duplicate possible - // extra newlines in the indent block - let text = indent.text(); - if text.starts_with('\n') { - buf.push('\n'); - buf.push_str(text.trim_start_matches('\n')); - } else { - buf.push_str(text); - } + let stmt_replace = edit.make_mut(stmt); + ted::replace(stmt_replace.syntax(), let_stmt.syntax()); - edit.replace(expr_range, var_name.clone()); - let offset = anchor.syntax().text_range().start(); - match ctx.config.snippet_cap { - Some(cap) => { - let snip = buf.replace( - &format!("let {var_modifier}{var_name}"), - &format!("let {var_modifier}$0{var_name}"), - ); - edit.insert_snippet(cap, offset, snip) + if let Some(cap) = ctx.config.snippet_cap { + if let Some(ast::Pat::IdentPat(ident_pat)) = let_stmt.pat() { + if let Some(name) = ident_pat.name() { + edit.add_tabstop_before(cap, name); + } + } + } } - None => edit.insert(offset, buf), - } + Anchor::WrapInBlock(to_wrap) => { + let indent_to = to_wrap.indent_level(); + + let block = if to_wrap.syntax() == &expr_replace { + // Since `expr_replace` is the same that needs to be wrapped in a block, + // we can just directly replace it with a block + let block = + make::block_expr([let_stmt.into()], Some(name_expr)).clone_for_update(); + ted::replace(expr_replace, block.syntax()); + + block + } else { + // `expr_replace` is a descendant of `to_wrap`, so both steps need to be + // handled seperately, otherwise we wrap the wrong expression + let to_wrap = edit.make_mut(to_wrap); + + // Replace the target expr first so that we don't need to find where + // `expr_replace` is in the wrapped `to_wrap` + ted::replace(expr_replace, name_expr.syntax()); + + // Wrap `to_wrap` in a block + let block = make::block_expr([let_stmt.into()], Some(to_wrap.clone())) + .clone_for_update(); + ted::replace(to_wrap.syntax(), block.syntax()); + + block + }; + + if let Some(cap) = ctx.config.snippet_cap { + // Adding a tabstop to `name` requires finding the let stmt again, since + // the existing `let_stmt` is not actually added to the tree + let pat = block.statements().find_map(|stmt| { + let ast::Stmt::LetStmt(let_stmt) = stmt else { return None }; + let_stmt.pat() + }); + + if let Some(ast::Pat::IdentPat(ident_pat)) = pat { + if let Some(name) = ident_pat.name() { + edit.add_tabstop_before(cap, name); + } + } + } - if let Anchor::WrapInBlock(_) = anchor { - edit.insert(anchor.syntax().text_range().end(), " }"); + // fixup indentation of block + block.indent(indent_to); + } } }, ) @@ -181,7 +219,7 @@ fn valid_target_expr(node: SyntaxNode) -> Option<ast::Expr> { enum Anchor { Before(SyntaxNode), Replace(ast::ExprStmt), - WrapInBlock(SyntaxNode), + WrapInBlock(ast::Expr), } impl Anchor { @@ -204,16 +242,16 @@ impl Anchor { } if let Some(parent) = node.parent() { - if parent.kind() == CLOSURE_EXPR { + if let Some(parent) = ast::ClosureExpr::cast(parent.clone()) { cov_mark::hit!(test_extract_var_in_closure_no_block); - return Some(Anchor::WrapInBlock(node)); + return parent.body().map(Anchor::WrapInBlock); } - if parent.kind() == MATCH_ARM { + if let Some(parent) = ast::MatchArm::cast(parent) { if node.kind() == MATCH_GUARD { cov_mark::hit!(test_extract_var_in_match_guard); } else { cov_mark::hit!(test_extract_var_in_match_arm_no_block); - return Some(Anchor::WrapInBlock(node)); + return parent.expr().map(Anchor::WrapInBlock); } } } @@ -229,13 +267,6 @@ impl Anchor { None }) } - - fn syntax(&self) -> &SyntaxNode { - match self { - Anchor::Before(it) | Anchor::WrapInBlock(it) => it, - Anchor::Replace(stmt) => stmt.syntax(), - } - } } #[cfg(test)] @@ -502,7 +533,10 @@ fn main() { fn main() { let x = true; let tuple = match x { - true => { let $0var_name = 2 + 2; (var_name, true) } + true => { + let $0var_name = 2 + 2; + (var_name, true) + } _ => (0, false) }; } @@ -579,7 +613,10 @@ fn main() { "#, r#" fn main() { - let lambda = |x: u32| { let $0var_name = x * 2; var_name }; + let lambda = |x: u32| { + let $0var_name = x * 2; + var_name + }; } "#, ); diff --git a/crates/ide-assists/src/handlers/generate_function.rs b/crates/ide-assists/src/handlers/generate_function.rs index a113c817f7..5bb200e84a 100644 --- a/crates/ide-assists/src/handlers/generate_function.rs +++ b/crates/ide-assists/src/handlers/generate_function.rs @@ -8,20 +8,21 @@ use ide_db::{ famous_defs::FamousDefs, helpers::is_editable_crate, path_transform::PathTransform, + source_change::SourceChangeBuilder, FxHashMap, FxHashSet, RootDatabase, SnippetCap, }; +use itertools::Itertools; use stdx::to_lower_snake_case; use syntax::{ ast::{ - self, - edit::{AstNodeEdit, IndentLevel}, - make, AstNode, CallExpr, HasArgList, HasGenericParams, HasModuleItem, HasTypeBounds, + self, edit::IndentLevel, edit_in_place::Indent, make, AstNode, CallExpr, HasArgList, + HasGenericParams, HasModuleItem, HasTypeBounds, }, - SyntaxKind, SyntaxNode, TextRange, TextSize, + ted, SyntaxKind, SyntaxNode, TextRange, T, }; use crate::{ - utils::{convert_reference_type, find_struct_impl, render_snippet, Cursor}, + utils::{convert_reference_type, find_struct_impl}, AssistContext, AssistId, AssistKind, Assists, }; @@ -65,7 +66,7 @@ fn gen_fn(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { } let fn_name = &*name_ref.text(); - let TargetInfo { target_module, adt_name, target, file, insert_offset } = + let TargetInfo { target_module, adt_name, target, file } = fn_target_info(ctx, path, &call, fn_name)?; if let Some(m) = target_module { @@ -77,16 +78,7 @@ fn gen_fn(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let function_builder = FunctionBuilder::from_call(ctx, &call, fn_name, target_module, target)?; let text_range = call.syntax().text_range(); let label = format!("Generate {} function", function_builder.fn_name); - add_func_to_accumulator( - acc, - ctx, - text_range, - function_builder, - insert_offset, - file, - adt_name, - label, - ) + add_func_to_accumulator(acc, ctx, text_range, function_builder, file, adt_name, label) } struct TargetInfo { @@ -94,7 +86,6 @@ struct TargetInfo { adt_name: Option<hir::Name>, target: GeneratedFunctionTarget, file: FileId, - insert_offset: TextSize, } impl TargetInfo { @@ -103,9 +94,8 @@ impl TargetInfo { adt_name: Option<hir::Name>, target: GeneratedFunctionTarget, file: FileId, - insert_offset: TextSize, ) -> Self { - Self { target_module, adt_name, target, file, insert_offset } + Self { target_module, adt_name, target, file } } } @@ -156,7 +146,7 @@ fn gen_method(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { } let (impl_, file) = get_adt_source(ctx, &adt, fn_name.text().as_str())?; - let (target, insert_offset) = get_method_target(ctx, &impl_, &adt)?; + let target = get_method_target(ctx, &impl_, &adt)?; let function_builder = FunctionBuilder::from_method_call( ctx, @@ -169,16 +159,7 @@ fn gen_method(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let text_range = call.syntax().text_range(); let adt_name = if impl_.is_none() { Some(adt.name(ctx.sema.db)) } else { None }; let label = format!("Generate {} method", function_builder.fn_name); - add_func_to_accumulator( - acc, - ctx, - text_range, - function_builder, - insert_offset, - file, - adt_name, - label, - ) + add_func_to_accumulator(acc, ctx, text_range, function_builder, file, adt_name, label) } fn add_func_to_accumulator( @@ -186,23 +167,28 @@ fn add_func_to_accumulator( ctx: &AssistContext<'_>, text_range: TextRange, function_builder: FunctionBuilder, - insert_offset: TextSize, file: FileId, adt_name: Option<hir::Name>, label: String, ) -> Option<()> { - acc.add(AssistId("generate_function", AssistKind::Generate), label, text_range, |builder| { - let indent = IndentLevel::from_node(function_builder.target.syntax()); - let function_template = function_builder.render(adt_name.is_some()); - let mut func = function_template.to_string(ctx.config.snippet_cap); + acc.add(AssistId("generate_function", AssistKind::Generate), label, text_range, |edit| { + edit.edit_file(file); + + let target = function_builder.target.clone(); + let function_template = function_builder.render(); + let func = function_template.to_ast(ctx.config.snippet_cap, edit); + if let Some(name) = adt_name { + let name = make::ty_path(make::ext::ident_path(&format!("{}", name.display(ctx.db())))); + // FIXME: adt may have generic params. - func = format!("\n{indent}impl {} {{\n{func}\n{indent}}}", name.display(ctx.db())); - } - builder.edit_file(file); - match ctx.config.snippet_cap { - Some(cap) => builder.insert_snippet(cap, insert_offset, func), - None => builder.insert(insert_offset, func), + let impl_ = make::impl_(None, None, name, None, None).clone_for_update(); + + func.indent(IndentLevel(1)); + impl_.get_or_create_assoc_item_list().add_item(func.into()); + target.insert_impl_at(edit, impl_); + } else { + target.insert_fn_at(edit, func); } }) } @@ -220,36 +206,33 @@ fn get_adt_source( } struct FunctionTemplate { - leading_ws: String, fn_def: ast::Fn, ret_type: Option<ast::RetType>, should_focus_return_type: bool, - trailing_ws: String, tail_expr: ast::Expr, } impl FunctionTemplate { - fn to_string(&self, cap: Option<SnippetCap>) -> String { - let Self { leading_ws, fn_def, ret_type, should_focus_return_type, trailing_ws, tail_expr } = - self; - - let f = match cap { - Some(cap) => { - let cursor = if *should_focus_return_type { - // Focus the return type if there is one - match ret_type { - Some(ret_type) => ret_type.syntax(), - None => tail_expr.syntax(), + fn to_ast(&self, cap: Option<SnippetCap>, edit: &mut SourceChangeBuilder) -> ast::Fn { + let Self { fn_def, ret_type, should_focus_return_type, tail_expr } = self; + + if let Some(cap) = cap { + if *should_focus_return_type { + // Focus the return type if there is one + match ret_type { + Some(ret_type) => { + edit.add_placeholder_snippet(cap, ret_type.clone()); } - } else { - tail_expr.syntax() - }; - render_snippet(cap, fn_def.syntax(), Cursor::Replace(cursor)) + None => { + edit.add_placeholder_snippet(cap, tail_expr.clone()); + } + } + } else { + edit.add_placeholder_snippet(cap, tail_expr.clone()); } - None => fn_def.to_string(), - }; + } - format!("{leading_ws}{f}{trailing_ws}") + fn_def.clone() } } @@ -356,7 +339,7 @@ impl FunctionBuilder { }) } - fn render(self, is_method: bool) -> FunctionTemplate { + fn render(self) -> FunctionTemplate { let placeholder_expr = make::ext::expr_todo(); let fn_body = make::block_expr(vec![], Some(placeholder_expr)); let visibility = match self.visibility { @@ -364,7 +347,7 @@ impl FunctionBuilder { Visibility::Crate => Some(make::visibility_pub_crate()), Visibility::Pub => Some(make::visibility_pub()), }; - let mut fn_def = make::fn_( + let fn_def = make::fn_( visibility, self.fn_name, self.generic_param_list, @@ -375,34 +358,10 @@ impl FunctionBuilder { self.is_async, false, // FIXME : const and unsafe are not handled yet. false, - ); - let leading_ws; - let trailing_ws; - - match self.target { - GeneratedFunctionTarget::BehindItem(it) => { - let mut indent = IndentLevel::from_node(&it); - if is_method { - indent = indent + 1; - leading_ws = format!("{indent}"); - } else { - leading_ws = format!("\n\n{indent}"); - } - - fn_def = fn_def.indent(indent); - trailing_ws = String::new(); - } - GeneratedFunctionTarget::InEmptyItemList(it) => { - let indent = IndentLevel::from_node(&it); - let leading_indent = indent + 1; - leading_ws = format!("\n{leading_indent}"); - fn_def = fn_def.indent(leading_indent); - trailing_ws = format!("\n{indent}"); - } - }; + ) + .clone_for_update(); FunctionTemplate { - leading_ws, ret_type: fn_def.ret_type(), // PANIC: we guarantee we always create a function body with a tail expr tail_expr: fn_def @@ -412,7 +371,6 @@ impl FunctionBuilder { .expect("function body should have a tail expression"), should_focus_return_type: self.should_focus_return_type, fn_def, - trailing_ws, } } } @@ -456,40 +414,37 @@ fn get_fn_target_info( target_module: Option<Module>, call: CallExpr, ) -> Option<TargetInfo> { - let (target, file, insert_offset) = get_fn_target(ctx, target_module, call)?; - Some(TargetInfo::new(target_module, None, target, file, insert_offset)) + let (target, file) = get_fn_target(ctx, target_module, call)?; + Some(TargetInfo::new(target_module, None, target, file)) } fn get_fn_target( ctx: &AssistContext<'_>, target_module: Option<Module>, call: CallExpr, -) -> Option<(GeneratedFunctionTarget, FileId, TextSize)> { +) -> Option<(GeneratedFunctionTarget, FileId)> { let mut file = ctx.file_id(); let target = match target_module { Some(target_module) => { - let module_source = target_module.definition_source(ctx.db()); - let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, &module_source)?; + let (in_file, target) = next_space_for_fn_in_module(ctx.db(), target_module); file = in_file; target } None => next_space_for_fn_after_call_site(ast::CallableExpr::Call(call))?, }; - Some((target.clone(), file, get_insert_offset(&target))) + Some((target.clone(), file)) } fn get_method_target( ctx: &AssistContext<'_>, impl_: &Option<ast::Impl>, adt: &Adt, -) -> Option<(GeneratedFunctionTarget, TextSize)> { +) -> Option<GeneratedFunctionTarget> { let target = match impl_ { - Some(impl_) => next_space_for_fn_in_impl(impl_)?, - None => { - GeneratedFunctionTarget::BehindItem(adt.source(ctx.sema.db)?.syntax().value.clone()) - } + Some(impl_) => GeneratedFunctionTarget::InImpl(impl_.clone()), + None => GeneratedFunctionTarget::AfterItem(adt.source(ctx.sema.db)?.syntax().value.clone()), }; - Some((target.clone(), get_insert_offset(&target))) + Some(target) } fn assoc_fn_target_info( @@ -505,36 +460,120 @@ fn assoc_fn_target_info( return None; } let (impl_, file) = get_adt_source(ctx, &adt, fn_name)?; - let (target, insert_offset) = get_method_target(ctx, &impl_, &adt)?; + let target = get_method_target(ctx, &impl_, &adt)?; let adt_name = if impl_.is_none() { Some(adt.name(ctx.sema.db)) } else { None }; - Some(TargetInfo::new(target_module, adt_name, target, file, insert_offset)) -} - -fn get_insert_offset(target: &GeneratedFunctionTarget) -> TextSize { - match target { - GeneratedFunctionTarget::BehindItem(it) => it.text_range().end(), - GeneratedFunctionTarget::InEmptyItemList(it) => it.text_range().start() + TextSize::of('{'), - } + Some(TargetInfo::new(target_module, adt_name, target, file)) } #[derive(Clone)] enum GeneratedFunctionTarget { - BehindItem(SyntaxNode), + AfterItem(SyntaxNode), InEmptyItemList(SyntaxNode), + InImpl(ast::Impl), } impl GeneratedFunctionTarget { fn syntax(&self) -> &SyntaxNode { match self { - GeneratedFunctionTarget::BehindItem(it) => it, + GeneratedFunctionTarget::AfterItem(it) => it, GeneratedFunctionTarget::InEmptyItemList(it) => it, + GeneratedFunctionTarget::InImpl(it) => it.syntax(), } } fn parent(&self) -> SyntaxNode { match self { - GeneratedFunctionTarget::BehindItem(it) => it.parent().expect("item without parent"), + GeneratedFunctionTarget::AfterItem(it) => it.parent().expect("item without parent"), GeneratedFunctionTarget::InEmptyItemList(it) => it.clone(), + GeneratedFunctionTarget::InImpl(it) => it.syntax().clone(), + } + } + + fn insert_impl_at(&self, edit: &mut SourceChangeBuilder, impl_: ast::Impl) { + match self { + GeneratedFunctionTarget::AfterItem(item) => { + let item = edit.make_syntax_mut(item.clone()); + let position = if item.parent().is_some() { + ted::Position::after(&item) + } else { + ted::Position::first_child_of(&item) + }; + + let indent = IndentLevel::from_node(&item); + let leading_ws = make::tokens::whitespace(&format!("\n{indent}")); + impl_.indent(indent); + + ted::insert_all(position, vec![leading_ws.into(), impl_.syntax().clone().into()]); + } + GeneratedFunctionTarget::InEmptyItemList(item_list) => { + let item_list = edit.make_syntax_mut(item_list.clone()); + let insert_after = + item_list.children_with_tokens().find_or_first(|child| child.kind() == T!['{']); + let position = match insert_after { + Some(child) => ted::Position::after(child), + None => ted::Position::first_child_of(&item_list), + }; + + let indent = IndentLevel::from_node(&item_list); + let leading_indent = indent + 1; + let leading_ws = make::tokens::whitespace(&format!("\n{leading_indent}")); + impl_.indent(indent); + + ted::insert_all(position, vec![leading_ws.into(), impl_.syntax().clone().into()]); + } + GeneratedFunctionTarget::InImpl(_) => { + unreachable!("can't insert an impl inside an impl") + } + } + } + + fn insert_fn_at(&self, edit: &mut SourceChangeBuilder, func: ast::Fn) { + match self { + GeneratedFunctionTarget::AfterItem(item) => { + let item = edit.make_syntax_mut(item.clone()); + let position = if item.parent().is_some() { + ted::Position::after(&item) + } else { + ted::Position::first_child_of(&item) + }; + + let indent = IndentLevel::from_node(&item); + let leading_ws = make::tokens::whitespace(&format!("\n\n{indent}")); + func.indent(indent); + + ted::insert_all_raw( + position, + vec![leading_ws.into(), func.syntax().clone().into()], + ); + } + GeneratedFunctionTarget::InEmptyItemList(item_list) => { + let item_list = edit.make_syntax_mut(item_list.clone()); + let insert_after = + item_list.children_with_tokens().find_or_first(|child| child.kind() == T!['{']); + let position = match insert_after { + Some(child) => ted::Position::after(child), + None => ted::Position::first_child_of(&item_list), + }; + + let indent = IndentLevel::from_node(&item_list); + let leading_indent = indent + 1; + let leading_ws = make::tokens::whitespace(&format!("\n{leading_indent}")); + let trailing_ws = make::tokens::whitespace(&format!("\n{indent}")); + func.indent(leading_indent); + + ted::insert_all( + position, + vec![leading_ws.into(), func.syntax().clone().into(), trailing_ws.into()], + ); + } + GeneratedFunctionTarget::InImpl(impl_) => { + let impl_ = edit.make_mut(impl_.clone()); + + let leading_indent = impl_.indent_level() + 1; + func.indent(leading_indent); + + impl_.get_or_create_assoc_item_list().add_item(func.into()); + } } } } @@ -1026,43 +1065,40 @@ fn next_space_for_fn_after_call_site(expr: ast::CallableExpr) -> Option<Generate } last_ancestor = Some(next_ancestor); } - last_ancestor.map(GeneratedFunctionTarget::BehindItem) + last_ancestor.map(GeneratedFunctionTarget::AfterItem) } fn next_space_for_fn_in_module( - db: &dyn hir::db::ExpandDatabase, - module_source: &hir::InFile<hir::ModuleSource>, -) -> Option<(FileId, GeneratedFunctionTarget)> { - let file = module_source.file_id.original_file(db); + db: &dyn hir::db::HirDatabase, + target_module: hir::Module, +) -> (FileId, GeneratedFunctionTarget) { + let module_source = target_module.definition_source(db); + let file = module_source.file_id.original_file(db.upcast()); let assist_item = match &module_source.value { hir::ModuleSource::SourceFile(it) => match it.items().last() { - Some(last_item) => GeneratedFunctionTarget::BehindItem(last_item.syntax().clone()), - None => GeneratedFunctionTarget::BehindItem(it.syntax().clone()), + Some(last_item) => GeneratedFunctionTarget::AfterItem(last_item.syntax().clone()), + None => GeneratedFunctionTarget::AfterItem(it.syntax().clone()), }, hir::ModuleSource::Module(it) => match it.item_list().and_then(|it| it.items().last()) { - Some(last_item) => GeneratedFunctionTarget::BehindItem(last_item.syntax().clone()), - None => GeneratedFunctionTarget::InEmptyItemList(it.item_list()?.syntax().clone()), + Some(last_item) => GeneratedFunctionTarget::AfterItem(last_item.syntax().clone()), + None => { + let item_list = + it.item_list().expect("module definition source should have an item list"); + GeneratedFunctionTarget::InEmptyItemList(item_list.syntax().clone()) + } }, hir::ModuleSource::BlockExpr(it) => { if let Some(last_item) = it.statements().take_while(|stmt| matches!(stmt, ast::Stmt::Item(_))).last() { - GeneratedFunctionTarget::BehindItem(last_item.syntax().clone()) + GeneratedFunctionTarget::AfterItem(last_item.syntax().clone()) } else { GeneratedFunctionTarget::InEmptyItemList(it.syntax().clone()) } } }; - Some((file, assist_item)) -} -fn next_space_for_fn_in_impl(impl_: &ast::Impl) -> Option<GeneratedFunctionTarget> { - let assoc_item_list = impl_.assoc_item_list()?; - if let Some(last_item) = assoc_item_list.assoc_items().last() { - Some(GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())) - } else { - Some(GeneratedFunctionTarget::InEmptyItemList(assoc_item_list.syntax().clone())) - } + (file, assist_item) } #[derive(Clone, Copy)] diff --git a/crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs b/crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs index b1daaea1ed..09759019ba 100644 --- a/crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs +++ b/crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs @@ -1,4 +1,7 @@ -use syntax::ast::{self, AstNode}; +use syntax::{ + ast::{self, make, AstNode}, + ted, +}; use crate::{utils::suggest_name, AssistContext, AssistId, AssistKind, Assists}; @@ -42,19 +45,34 @@ pub(crate) fn replace_is_method_with_if_let_method( suggest_name::for_variable(&receiver, &ctx.sema) }; - let target = call_expr.syntax().text_range(); - let (assist_id, message, text) = if name_ref.text() == "is_some" { ("replace_is_some_with_if_let_some", "Replace `is_some` with `if let Some`", "Some") } else { ("replace_is_ok_with_if_let_ok", "Replace `is_ok` with `if let Ok`", "Ok") }; - acc.add(AssistId(assist_id, AssistKind::RefactorRewrite), message, target, |edit| { - let var_name = format!("${{0:{}}}", var_name); - let replacement = format!("let {}({}) = {}", text, var_name, receiver); - edit.replace(target, replacement); - }) + acc.add( + AssistId(assist_id, AssistKind::RefactorRewrite), + message, + call_expr.syntax().text_range(), + |edit| { + let call_expr = edit.make_mut(call_expr); + + let var_pat = make::ident_pat(false, false, make::name(&var_name)); + let pat = make::tuple_struct_pat(make::ext::ident_path(text), [var_pat.into()]); + let let_expr = make::expr_let(pat.into(), receiver).clone_for_update(); + + if let Some(cap) = ctx.config.snippet_cap { + if let Some(ast::Pat::TupleStructPat(pat)) = let_expr.pat() { + if let Some(first_var) = pat.fields().next() { + edit.add_placeholder_snippet(cap, first_var); + } + } + } + + ted::replace(call_expr.syntax(), let_expr.syntax()); + }, + ) } _ => return None, } diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index 977c8380ac..95b9eb5294 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -505,16 +505,33 @@ pub fn test_some_range(a: int) -> bool { TextEdit { indels: [ Indel { - insert: "let $0var_name = 5;\n ", - delete: 45..45, + insert: "let", + delete: 45..47, }, Indel { insert: "var_name", - delete: 59..60, + delete: 48..60, + }, + Indel { + insert: "=", + delete: 61..81, + }, + Indel { + insert: "5;\n if let 2..6 = var_name {\n true\n } else {\n false\n }", + delete: 82..108, }, ], }, - None, + Some( + SnippetEdit( + [ + ( + 0, + 49..49, + ), + ], + ), + ), ), }, file_system_edits: [], @@ -567,16 +584,33 @@ pub fn test_some_range(a: int) -> bool { TextEdit { indels: [ Indel { - insert: "let $0var_name = 5;\n ", - delete: 45..45, + insert: "let", + delete: 45..47, }, Indel { insert: "var_name", - delete: 59..60, + delete: 48..60, + }, + Indel { + insert: "=", + delete: 61..81, + }, + Indel { + insert: "5;\n if let 2..6 = var_name {\n true\n } else {\n false\n }", + delete: 82..108, }, ], }, - None, + Some( + SnippetEdit( + [ + ( + 0, + 49..49, + ), + ], + ), + ), ), }, file_system_edits: [], |