From 8c0661b2de9cab1251c7a3c53e3375cbafce4fa0 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Sun, 10 Dec 2023 21:20:09 -0500 Subject: [PATCH 01/13] Use `GenericArgList` for `make::impl{_trait}` `make::impl_` no longer merges generic params and args in order to be in line with `make::impl_`, which doesn't do it either. --- .../src/handlers/generate_delegate_methods.rs | 6 ++--- crates/syntax/src/ast/make.rs | 24 +++++++------------ 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/crates/ide-assists/src/handlers/generate_delegate_methods.rs b/crates/ide-assists/src/handlers/generate_delegate_methods.rs index 1f92c39ad4..4f2df5633c 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_methods.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_methods.rs @@ -161,13 +161,13 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<' Some(impl_def) => edit.make_mut(impl_def), None => { let name = &strukt_name.to_string(); - let params = strukt.generic_param_list(); - let ty_params = params; + let ty_params = strukt.generic_param_list(); + let ty_args = ty_params.as_ref().map(|it| it.to_generic_args()); let where_clause = strukt.where_clause(); let impl_def = make::impl_( ty_params, - None, + ty_args, make::ty_path(make::ext::ident_path(name)), where_clause, None, diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index d5eda8f15e..e479aec30f 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -236,20 +236,14 @@ fn merge_where_clause( pub fn impl_( generic_params: Option, - generic_args: Option, + generic_args: Option, path_type: ast::Type, where_clause: Option, body: Option>>, ) -> ast::Impl { - let (gen_params, tr_gen_args) = match (generic_params, generic_args) { - (None, None) => (String::new(), String::new()), - (None, Some(args)) => (String::new(), args.to_generic_args().to_string()), - (Some(params), None) => (params.to_string(), params.to_generic_args().to_string()), - (Some(params), Some(args)) => match merge_gen_params(Some(params.clone()), Some(args)) { - Some(merged) => (params.to_string(), merged.to_generic_args().to_string()), - None => (params.to_string(), String::new()), - }, - }; + let gen_args = generic_args.map_or_else(String::new, |it| it.to_string()); + + let gen_params = generic_params.map_or_else(String::new, |it| it.to_string()); let where_clause = match where_clause { Some(pr) => pr.to_string(), @@ -261,7 +255,7 @@ pub fn impl_( None => String::new(), }; - ast_from_text(&format!("impl{gen_params} {path_type}{tr_gen_args}{where_clause}{{{}}}", body)) + ast_from_text(&format!("impl{gen_params} {path_type}{gen_args}{where_clause}{{{body}}}")) } pub fn impl_trait( @@ -282,10 +276,8 @@ pub fn impl_trait( let trait_gen_args = trait_gen_args.map(|args| args.to_string()).unwrap_or_default(); let type_gen_args = type_gen_args.map(|args| args.to_string()).unwrap_or_default(); - let gen_params = match merge_gen_params(trait_gen_params, type_gen_params) { - Some(pars) => pars.to_string(), - None => String::new(), - }; + let gen_params = merge_gen_params(trait_gen_params, type_gen_params) + .map_or_else(String::new, |it| it.to_string()); let is_negative = if is_negative { "! " } else { "" }; @@ -297,7 +289,7 @@ pub fn impl_trait( None => String::new(), }; - ast_from_text(&format!("{is_unsafe}impl{gen_params} {is_negative}{path_type}{trait_gen_args} for {ty}{type_gen_args}{where_clause}{{{}}}" , body)) + ast_from_text(&format!("{is_unsafe}impl{gen_params} {is_negative}{path_type}{trait_gen_args} for {ty}{type_gen_args}{where_clause}{{{body}}}")) } pub fn impl_trait_type(bounds: ast::TypeBoundList) -> ast::ImplTraitType { From 3924a0ef7c86540d4c84919bf1e0054e30c34711 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Sun, 10 Dec 2023 21:23:21 -0500 Subject: [PATCH 02/13] Add ast versions of `generate{_trait}_impl_text{_intransitive}` --- crates/ide-assists/src/utils.rs | 100 +++++++++++++++++++++++++++++++- crates/syntax/src/ast/make.rs | 7 ++- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 3bc585d323..0ec3f68c8f 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -470,6 +470,7 @@ pub(crate) fn find_impl_block_end(impl_def: ast::Impl, buf: &mut String) -> Opti /// Generates the surrounding `impl Type { }` including type and lifetime /// parameters. +// FIXME: migrate remaining uses to `generate_impl` pub(crate) fn generate_impl_text(adt: &ast::Adt, code: &str) -> String { generate_impl_text_inner(adt, None, true, code) } @@ -478,6 +479,7 @@ pub(crate) fn generate_impl_text(adt: &ast::Adt, code: &str) -> String { /// and lifetime parameters, with `` appended to `impl`'s generic parameters' bounds. /// /// This is useful for traits like `PartialEq`, since `impl PartialEq for U` often requires `T: PartialEq`. +// FIXME: migrate remaining uses to `generate_trait_impl` pub(crate) fn generate_trait_impl_text(adt: &ast::Adt, trait_text: &str, code: &str) -> String { generate_impl_text_inner(adt, Some(trait_text), true, code) } @@ -486,6 +488,7 @@ pub(crate) fn generate_trait_impl_text(adt: &ast::Adt, trait_text: &str, code: & /// and lifetime parameters, with `impl`'s generic parameters' bounds kept as-is. /// /// This is useful for traits like `From`, since `impl From for U` doesn't require `T: From`. +// FIXME: migrate remaining uses to `generate_trait_impl_intransitive` pub(crate) fn generate_trait_impl_text_intransitive( adt: &ast::Adt, trait_text: &str, @@ -516,7 +519,7 @@ fn generate_impl_text_inner( // Add the current trait to `bounds` if the trait is transitive, // meaning `impl Trait for U` requires `T: Trait`. if trait_is_transitive { - bounds.push(make::type_bound(trait_)); + bounds.push(make::type_bound_text(trait_)); } }; // `{ty_param}: {bounds}` @@ -574,6 +577,101 @@ fn generate_impl_text_inner( buf } +/// Generates the corresponding `impl Type {}` including type and lifetime +/// parameters. +pub(crate) fn generate_impl(adt: &ast::Adt) -> ast::Impl { + generate_impl_inner(adt, None, true) +} + +/// Generates the corresponding `impl for Type {}` including type +/// and lifetime parameters, with `` appended to `impl`'s generic parameters' bounds. +/// +/// This is useful for traits like `PartialEq`, since `impl PartialEq for U` often requires `T: PartialEq`. +pub(crate) fn generate_trait_impl(adt: &ast::Adt, trait_: ast::Type) -> ast::Impl { + generate_impl_inner(adt, Some(trait_), true) +} + +/// Generates the corresponding `impl for Type {}` including type +/// and lifetime parameters, with `impl`'s generic parameters' bounds kept as-is. +/// +/// This is useful for traits like `From`, since `impl From for U` doesn't require `T: From`. +pub(crate) fn generate_trait_impl_intransitive(adt: &ast::Adt, trait_: ast::Type) -> ast::Impl { + generate_impl_inner(adt, Some(trait_), false) +} + +fn generate_impl_inner( + adt: &ast::Adt, + trait_: Option, + trait_is_transitive: bool, +) -> ast::Impl { + // Ensure lifetime params are before type & const params + let generic_params = adt.generic_param_list().map(|generic_params| { + let lifetime_params = + generic_params.lifetime_params().map(ast::GenericParam::LifetimeParam); + let ty_or_const_params = generic_params.type_or_const_params().map(|param| { + match param { + ast::TypeOrConstParam::Type(param) => { + let param = param.clone_for_update(); + // remove defaults since they can't be specified in impls + param.remove_default(); + let mut bounds = + param.type_bound_list().map_or_else(Vec::new, |it| it.bounds().collect()); + if let Some(trait_) = &trait_ { + // Add the current trait to `bounds` if the trait is transitive, + // meaning `impl Trait for U` requires `T: Trait`. + if trait_is_transitive { + bounds.push(make::type_bound(trait_.clone())); + } + }; + // `{ty_param}: {bounds}` + let param = + make::type_param(param.name().unwrap(), make::type_bound_list(bounds)); + ast::GenericParam::TypeParam(param) + } + ast::TypeOrConstParam::Const(param) => { + let param = param.clone_for_update(); + // remove defaults since they can't be specified in impls + param.remove_default(); + ast::GenericParam::ConstParam(param) + } + } + }); + + make::generic_param_list(itertools::chain(lifetime_params, ty_or_const_params)) + }); + let generic_args = + generic_params.as_ref().map(|params| params.to_generic_args().clone_for_update()); + let ty = make::ty_path(make::ext::ident_path(&adt.name().unwrap().text())); + + let impl_ = match trait_ { + Some(trait_) => make::impl_trait( + false, + None, + None, + generic_params, + generic_args, + false, + trait_, + ty, + None, + adt.where_clause(), + None, + ), + None => make::impl_(generic_params, generic_args, ty, adt.where_clause(), None), + } + .clone_for_update(); + + // Copy any cfg attrs from the original adt + let cfg_attrs = adt + .attrs() + .filter(|attr| attr.as_simple_call().map(|(name, _arg)| name == "cfg").unwrap_or(false)); + for attr in cfg_attrs { + impl_.add_attr(attr.clone_for_update()); + } + + impl_ +} + pub(crate) fn add_method_to_adt( builder: &mut SourceChangeBuilder, adt: &ast::Adt, diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index e479aec30f..764221fc8b 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -895,7 +895,12 @@ pub fn trait_( ast_from_text(&text) } -pub fn type_bound(bound: &str) -> ast::TypeBound { +// FIXME: remove when no one depends on `generate_impl_text_inner` +pub fn type_bound_text(bound: &str) -> ast::TypeBound { + ast_from_text(&format!("fn f() {{ }}")) +} + +pub fn type_bound(bound: ast::Type) -> ast::TypeBound { ast_from_text(&format!("fn f() {{ }}")) } From 0e39257e5be05458596fb5cce9bb806081ea0cf1 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 11 Dec 2023 17:37:45 -0500 Subject: [PATCH 03/13] Migrate `extract_function` to mutable ast --- .../src/handlers/extract_function.rs | 292 +++++++++++------- crates/ide-assists/src/tests.rs | 13 +- 2 files changed, 193 insertions(+), 112 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs index 1eb28626f7..fce132f782 100644 --- a/crates/ide-assists/src/handlers/extract_function.rs +++ b/crates/ide-assists/src/handlers/extract_function.rs @@ -1,4 +1,4 @@ -use std::iter; +use std::{iter, ops::RangeInclusive}; use ast::make; use either::Either; @@ -12,27 +12,25 @@ use ide_db::{ helpers::mod_path_to_ast, imports::insert_use::{insert_use, ImportScope}, search::{FileReference, ReferenceCategory, SearchScope}, + source_change::SourceChangeBuilder, syntax_helpers::node_ext::{ for_each_tail_expr, preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr, }, FxIndexSet, RootDatabase, }; -use itertools::Itertools; -use stdx::format_to; use syntax::{ ast::{ - self, - edit::{AstNodeEdit, IndentLevel}, - AstNode, HasGenericParams, + self, edit::IndentLevel, edit_in_place::Indent, AstNode, AstToken, HasGenericParams, + HasName, }, - match_ast, ted, AstToken, SyntaxElement, + match_ast, ted, SyntaxElement, SyntaxKind::{self, COMMENT}, SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, WalkEvent, T, }; use crate::{ assist_context::{AssistContext, Assists, TreeMutator}, - utils::generate_impl_text, + utils::generate_impl, AssistId, }; @@ -134,17 +132,65 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op let new_indent = IndentLevel::from_node(&insert_after); let old_indent = fun.body.indent_level(); - builder.replace(target_range, make_call(ctx, &fun, old_indent)); + let insert_after = builder.make_syntax_mut(insert_after); + + let call_expr = make_call(ctx, &fun, old_indent); + + // Map the element range to replace into the mutable version + let elements = match &fun.body { + FunctionBody::Expr(expr) => { + // expr itself becomes the replacement target + let expr = &builder.make_mut(expr.clone()); + let node = SyntaxElement::Node(expr.syntax().clone()); + + node.clone()..=node + } + FunctionBody::Span { parent, elements, .. } => { + // Map the element range into the mutable versions + let parent = builder.make_mut(parent.clone()); + + let start = parent + .syntax() + .children_with_tokens() + .nth(elements.start().index()) + .expect("should be able to find mutable start element"); + + let end = parent + .syntax() + .children_with_tokens() + .nth(elements.end().index()) + .expect("should be able to find mutable end element"); + + start..=end + } + }; let has_impl_wrapper = insert_after.ancestors().any(|a| a.kind() == SyntaxKind::IMPL && a != insert_after); + let fn_def = format_function(ctx, module, &fun, old_indent).clone_for_update(); + + if let Some(cap) = ctx.config.snippet_cap { + if let Some(name) = fn_def.name() { + builder.add_tabstop_before(cap, name); + } + } + let fn_def = match fun.self_param_adt(ctx) { Some(adt) if anchor == Anchor::Method && !has_impl_wrapper => { - let fn_def = format_function(ctx, module, &fun, old_indent, new_indent + 1); - generate_impl_text(&adt, &fn_def).replace("{\n\n", "{") + fn_def.indent(1.into()); + + let impl_ = generate_impl(&adt); + impl_.indent(new_indent); + impl_.get_or_create_assoc_item_list().add_item(fn_def.into()); + + impl_.syntax().clone() + } + _ => { + fn_def.indent(new_indent.into()); + + fn_def.syntax().clone() } - _ => format_function(ctx, module, &fun, old_indent, new_indent), }; // There are external control flows @@ -177,12 +223,15 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op } } - let insert_offset = insert_after.text_range().end(); + // Replace the call site with the call to the new function + fixup_call_site(builder, &fun.body); + ted::replace_all(elements, vec![call_expr.into()]); - match ctx.config.snippet_cap { - Some(cap) => builder.insert_snippet(cap, insert_offset, fn_def), - None => builder.insert(insert_offset, fn_def), - }; + // Insert the newly extracted function (or impl) + ted::insert_all_raw( + ted::Position::after(insert_after), + vec![make::tokens::whitespace(&format!("\n\n{new_indent}")).into(), fn_def.into()], + ); }, ) } @@ -225,10 +274,10 @@ fn extraction_target(node: &SyntaxNode, selection_range: TextRange) -> Option None, - ast::Stmt::ExprStmt(_) | ast::Stmt::LetStmt(_) => Some(FunctionBody::from_range( + ast::Stmt::ExprStmt(_) | ast::Stmt::LetStmt(_) => FunctionBody::from_range( node.parent().and_then(ast::StmtList::cast)?, node.text_range(), - )), + ), }; } @@ -241,7 +290,7 @@ fn extraction_target(node: &SyntaxNode, selection_range: TextRange) -> Option, text_range: TextRange }, } #[derive(Debug)] @@ -569,26 +618,38 @@ impl FunctionBody { } } - fn from_range(parent: ast::StmtList, selected: TextRange) -> FunctionBody { + fn from_range(parent: ast::StmtList, selected: TextRange) -> Option { let full_body = parent.syntax().children_with_tokens(); - let mut text_range = full_body + // Get all of the elements intersecting with the selection + let mut stmts_in_selection = full_body .filter(|it| ast::Stmt::can_cast(it.kind()) || it.kind() == COMMENT) - .map(|element| element.text_range()) - .filter(|&range| selected.intersect(range).filter(|it| !it.is_empty()).is_some()) - .reduce(|acc, stmt| acc.cover(stmt)); + .filter(|it| selected.intersect(it.text_range()).filter(|it| !it.is_empty()).is_some()); - if let Some(tail_range) = parent - .tail_expr() - .map(|it| it.syntax().text_range()) - .filter(|&it| selected.intersect(it).is_some()) + let first_element = stmts_in_selection.next(); + + // If the tail expr is part of the selection too, make that the last element + // Otherwise use the last stmt + let last_element = if let Some(tail_expr) = + parent.tail_expr().filter(|it| selected.intersect(it.syntax().text_range()).is_some()) { - text_range = Some(match text_range { - Some(text_range) => text_range.cover(tail_range), - None => tail_range, - }); - } - Self::Span { parent, text_range: text_range.unwrap_or(selected) } + Some(tail_expr.syntax().clone().into()) + } else { + stmts_in_selection.last() + }; + + let elements = match (first_element, last_element) { + (None, _) => { + cov_mark::hit!(extract_function_empty_selection_is_not_applicable); + return None; + } + (Some(first), None) => first.clone()..=first, + (Some(first), Some(last)) => first..=last, + }; + + let text_range = elements.start().text_range().cover(elements.end().text_range()); + + Some(Self::Span { parent, elements, text_range }) } fn indent_level(&self) -> IndentLevel { @@ -601,7 +662,7 @@ impl FunctionBody { fn tail_expr(&self) -> Option { match &self { FunctionBody::Expr(expr) => Some(expr.clone()), - FunctionBody::Span { parent, text_range } => { + FunctionBody::Span { parent, text_range, .. } => { let tail_expr = parent.tail_expr()?; text_range.contains_range(tail_expr.syntax().text_range()).then_some(tail_expr) } @@ -611,7 +672,7 @@ impl FunctionBody { fn walk_expr(&self, cb: &mut dyn FnMut(ast::Expr)) { match self { FunctionBody::Expr(expr) => walk_expr(expr, cb), - FunctionBody::Span { parent, text_range } => { + FunctionBody::Span { parent, text_range, .. } => { parent .statements() .filter(|stmt| text_range.contains_range(stmt.syntax().text_range())) @@ -634,7 +695,7 @@ impl FunctionBody { fn preorder_expr(&self, cb: &mut dyn FnMut(WalkEvent) -> bool) { match self { FunctionBody::Expr(expr) => preorder_expr(expr, cb), - FunctionBody::Span { parent, text_range } => { + FunctionBody::Span { parent, text_range, .. } => { parent .statements() .filter(|stmt| text_range.contains_range(stmt.syntax().text_range())) @@ -657,7 +718,7 @@ impl FunctionBody { fn walk_pat(&self, cb: &mut dyn FnMut(ast::Pat)) { match self { FunctionBody::Expr(expr) => walk_patterns_in_expr(expr, cb), - FunctionBody::Span { parent, text_range } => { + FunctionBody::Span { parent, text_range, .. } => { parent .statements() .filter(|stmt| text_range.contains_range(stmt.syntax().text_range())) @@ -1151,7 +1212,7 @@ impl HasTokenAtOffset for FunctionBody { fn token_at_offset(&self, offset: TextSize) -> TokenAtOffset { match self { FunctionBody::Expr(expr) => expr.syntax().token_at_offset(offset), - FunctionBody::Span { parent, text_range } => { + FunctionBody::Span { parent, text_range, .. } => { match parent.syntax().token_at_offset(offset) { TokenAtOffset::None => TokenAtOffset::None, TokenAtOffset::Single(t) => { @@ -1316,7 +1377,19 @@ fn impl_type_name(impl_node: &ast::Impl) -> Option { Some(impl_node.self_ty()?.to_string()) } -fn make_call(ctx: &AssistContext<'_>, fun: &Function, indent: IndentLevel) -> String { +/// Fixes up the call site before the target expressions are replaced with the call expression +fn fixup_call_site(builder: &mut SourceChangeBuilder, body: &FunctionBody) { + let parent_match_arm = body.parent().and_then(ast::MatchArm::cast); + + if let Some(parent_match_arm) = parent_match_arm { + if parent_match_arm.comma_token().is_none() { + let parent_match_arm = builder.make_mut(parent_match_arm); + ted::append_child_raw(parent_match_arm.syntax(), make::token(T![,])); + } + } +} + +fn make_call(ctx: &AssistContext<'_>, fun: &Function, indent: IndentLevel) -> SyntaxNode { let ret_ty = fun.return_type(ctx); let args = make::arg_list(fun.params.iter().map(|param| param.to_arg(ctx))); @@ -1334,44 +1407,49 @@ fn make_call(ctx: &AssistContext<'_>, fun: &Function, indent: IndentLevel) -> St if fun.control_flow.is_async { call_expr = make::expr_await(call_expr); } - let expr = handler.make_call_expr(call_expr).indent(indent); - let mut_modifier = |var: &OutlivedLocal| if var.mut_usage_outside_body { "mut " } else { "" }; + let expr = handler.make_call_expr(call_expr).clone_for_update(); + expr.indent(indent); - let mut buf = String::new(); - match fun.outliving_locals.as_slice() { - [] => {} + let outliving_bindings = match fun.outliving_locals.as_slice() { + [] => None, [var] => { - let modifier = mut_modifier(var); let name = var.local.name(ctx.db()); - format_to!(buf, "let {modifier}{} = ", name.display(ctx.db())) + let name = make::name(&name.display(ctx.db()).to_string()); + Some(ast::Pat::IdentPat(make::ident_pat( + false, + var.mut_usage_outside_body, + name.into(), + ))) } vars => { - buf.push_str("let ("); - let bindings = vars.iter().format_with(", ", |local, f| { - let modifier = mut_modifier(local); - let name = local.local.name(ctx.db()); - f(&format_args!("{modifier}{}", name.display(ctx.db())))?; - Ok(()) + let binding_pats = vars.iter().map(|var| { + let name = var.local.name(ctx.db()); + let name = make::name(&name.display(ctx.db()).to_string()); + make::ident_pat(false, var.mut_usage_outside_body, name.into()).into() }); - format_to!(buf, "{bindings}"); - buf.push_str(") = "); + Some(ast::Pat::TuplePat(make::tuple_pat(binding_pats))) } - } + }; - format_to!(buf, "{expr}"); let parent_match_arm = fun.body.parent().and_then(ast::MatchArm::cast); - let insert_comma = parent_match_arm.as_ref().is_some_and(|it| it.comma_token().is_none()); - if insert_comma { - buf.push(','); - } else if parent_match_arm.is_none() + if let Some(bindings) = outliving_bindings { + // with bindings that outlive it + make::let_stmt(bindings, None, Some(expr)).syntax().clone_for_update() + } else if parent_match_arm.as_ref().is_some() { + // as a tail expr for a match arm + expr.syntax().clone() + } else if parent_match_arm.as_ref().is_none() && fun.ret_ty.is_unit() && (!fun.outliving_locals.is_empty() || !expr.is_block_like()) { - buf.push(';'); + // as an expr stmt + make::expr_stmt(expr).syntax().clone_for_update() + } else { + // as a tail expr, or a block + expr.syntax().clone() } - buf } enum FlowHandler { @@ -1500,42 +1578,25 @@ fn format_function( module: hir::Module, fun: &Function, old_indent: IndentLevel, - new_indent: IndentLevel, -) -> String { - let mut fn_def = String::new(); - - let fun_name = &fun.name; +) -> ast::Fn { + let fun_name = make::name(&fun.name.text()); let params = fun.make_param_list(ctx, module); let ret_ty = fun.make_ret_ty(ctx, module); - let body = make_body(ctx, old_indent, new_indent, fun); - let const_kw = if fun.mods.is_const { "const " } else { "" }; - let async_kw = if fun.control_flow.is_async { "async " } else { "" }; - let unsafe_kw = if fun.control_flow.is_unsafe { "unsafe " } else { "" }; + let body = make_body(ctx, old_indent, fun); let (generic_params, where_clause) = make_generic_params_and_where_clause(ctx, fun); - format_to!(fn_def, "\n\n{new_indent}{const_kw}{async_kw}{unsafe_kw}"); - match ctx.config.snippet_cap { - Some(_) => format_to!(fn_def, "fn $0{fun_name}"), - None => format_to!(fn_def, "fn {fun_name}"), - } - - if let Some(generic_params) = generic_params { - format_to!(fn_def, "{generic_params}"); - } - - format_to!(fn_def, "{params}"); - - if let Some(ret_ty) = ret_ty { - format_to!(fn_def, " {ret_ty}"); - } - - if let Some(where_clause) = where_clause { - format_to!(fn_def, " {where_clause}"); - } - - format_to!(fn_def, " {body}"); - - fn_def + make::fn_( + None, + fun_name, + generic_params, + where_clause, + params, + body, + ret_ty, + fun.control_flow.is_async, + fun.mods.is_const, + fun.control_flow.is_unsafe, + ) } fn make_generic_params_and_where_clause( @@ -1716,12 +1777,7 @@ impl FunType { } } -fn make_body( - ctx: &AssistContext<'_>, - old_indent: IndentLevel, - new_indent: IndentLevel, - fun: &Function, -) -> ast::BlockExpr { +fn make_body(ctx: &AssistContext<'_>, old_indent: IndentLevel, fun: &Function) -> ast::BlockExpr { let ret_ty = fun.return_type(ctx); let handler = FlowHandler::from_ret_ty(fun, &ret_ty); @@ -1732,7 +1788,7 @@ fn make_body( match expr { ast::Expr::BlockExpr(block) => { // If the extracted expression is itself a block, there is no need to wrap it inside another block. - let block = block.dedent(old_indent); + block.dedent(old_indent); let elements = block.stmt_list().map_or_else( || Either::Left(iter::empty()), |stmt_list| { @@ -1752,13 +1808,13 @@ fn make_body( make::hacky_block_expr(elements, block.tail_expr()) } _ => { - let expr = expr.dedent(old_indent).indent(IndentLevel(1)); + expr.reindent_to(1.into()); make::block_expr(Vec::new(), Some(expr)) } } } - FunctionBody::Span { parent, text_range } => { + FunctionBody::Span { parent, text_range, .. } => { let mut elements: Vec<_> = parent .syntax() .children_with_tokens() @@ -1801,8 +1857,8 @@ fn make_body( .map(|node_or_token| match &node_or_token { syntax::NodeOrToken::Node(node) => match ast::Stmt::cast(node.clone()) { Some(stmt) => { - let indented = stmt.dedent(old_indent).indent(body_indent); - let ast_node = indented.syntax().clone_subtree(); + stmt.reindent_to(body_indent); + let ast_node = stmt.syntax().clone_subtree(); syntax::NodeOrToken::Node(ast_node) } _ => node_or_token, @@ -1810,7 +1866,9 @@ fn make_body( _ => node_or_token, }) .collect::>(); - let tail_expr = tail_expr.map(|expr| expr.dedent(old_indent).indent(body_indent)); + if let Some(tail_expr) = &mut tail_expr { + tail_expr.reindent_to(body_indent); + } make::hacky_block_expr(elements, tail_expr) } @@ -1853,7 +1911,7 @@ fn make_body( }), }; - block.indent(new_indent) + block } fn map_tail_expr(block: ast::BlockExpr, f: impl FnOnce(ast::Expr) -> ast::Expr) -> ast::BlockExpr { @@ -2551,6 +2609,20 @@ fn $0fun_name(n: u32) -> u32 { check_assist_not_applicable(extract_function, r"fn main() { 1 + /* $0comment$0 */ 1; }"); } + #[test] + fn empty_selection_is_not_applicable() { + cov_mark::check!(extract_function_empty_selection_is_not_applicable); + check_assist_not_applicable( + extract_function, + r#" +fn main() { + $0 + + $0 +}"#, + ); + } + #[test] fn part_of_expr_stmt() { check_assist( diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index 573d69b5c6..466264d8e4 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -687,12 +687,21 @@ pub fn test_some_range(a: int) -> bool { delete: 59..60, }, Indel { - insert: "\n\nfn $0fun_name() -> i32 {\n 5\n}", + insert: "\n\nfn fun_name() -> i32 {\n 5\n}", delete: 110..110, }, ], }, - None, + Some( + SnippetEdit( + [ + ( + 0, + 124..124, + ), + ], + ), + ), ), }, file_system_edits: [], From 039b3d0abbf1ff01229dcd58be458dc86ed06485 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Thu, 14 Dec 2023 18:45:20 -0500 Subject: [PATCH 04/13] Add `make::ext::expr_self` Shortcut version of `make::expr_path(make::path_unqualified(make::path_segment_self()))` --- crates/syntax/src/ast/make.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 764221fc8b..b6e5e6a4e4 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -68,6 +68,9 @@ pub mod ext { pub fn expr_ty_new(ty: &ast::Type) -> ast::Expr { expr_from_text(&format!("{ty}::new()")) } + pub fn expr_self() -> ast::Expr { + expr_from_text("self") + } pub fn zero_number() -> ast::Expr { expr_from_text("0") From 0519414c19da1a169b564e846df87895bc248674 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Thu, 14 Dec 2023 19:10:02 -0500 Subject: [PATCH 05/13] Make `ReferenceConversion` methods return ast types --- .../src/handlers/generate_function.rs | 2 +- .../src/handlers/generate_getter_or_setter.rs | 4 ++-- crates/ide-assists/src/utils.rs | 18 +++++++++++------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/ide-assists/src/handlers/generate_function.rs b/crates/ide-assists/src/handlers/generate_function.rs index 50528e1caa..63a62abc21 100644 --- a/crates/ide-assists/src/handlers/generate_function.rs +++ b/crates/ide-assists/src/handlers/generate_function.rs @@ -1033,7 +1033,7 @@ fn fn_arg_type( if ty.is_reference() || ty.is_mutable_reference() { let famous_defs = &FamousDefs(&ctx.sema, ctx.sema.scope(fn_arg.syntax())?.krate()); convert_reference_type(ty.strip_references(), ctx.db(), famous_defs) - .map(|conversion| conversion.convert_type(ctx.db())) + .map(|conversion| conversion.convert_type(ctx.db()).to_string()) .or_else(|| ty.display_source_code(ctx.db(), target_module.into(), true).ok()) } else { ty.display_source_code(ctx.db(), target_module.into(), true).ok() diff --git a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs index 4610b7af38..5d9dec1330 100644 --- a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs +++ b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs @@ -233,8 +233,8 @@ fn generate_getter_from_info( .map(|conversion| { cov_mark::hit!(convert_reference_type); ( - conversion.convert_type(ctx.db()), - conversion.getter(record_field_info.field_name.to_string()), + conversion.convert_type(ctx.db()).to_string(), + conversion.getter(record_field_info.field_name.to_string()).to_string(), ) }) })() diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 0ec3f68c8f..10d18ee67d 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -718,8 +718,8 @@ enum ReferenceConversionType { } impl ReferenceConversion { - pub(crate) fn convert_type(&self, db: &dyn HirDatabase) -> String { - match self.conversion { + pub(crate) fn convert_type(&self, db: &dyn HirDatabase) -> ast::Type { + let ty = match self.conversion { ReferenceConversionType::Copy => self.ty.display(db).to_string(), ReferenceConversionType::AsRefStr => "&str".to_string(), ReferenceConversionType::AsRefSlice => { @@ -745,21 +745,25 @@ impl ReferenceConversion { type_arguments.next().unwrap().display(db).to_string(); format!("Result<&{first_type_argument_name}, &{second_type_argument_name}>") } - } + }; + + make::ty(&ty) } - pub(crate) fn getter(&self, field_name: String) -> String { + pub(crate) fn getter(&self, field_name: String) -> ast::Expr { + let expr = make::expr_field(make::ext::expr_self(), &field_name); + match self.conversion { - ReferenceConversionType::Copy => format!("self.{field_name}"), + ReferenceConversionType::Copy => expr, ReferenceConversionType::AsRefStr | ReferenceConversionType::AsRefSlice | ReferenceConversionType::Dereferenced | ReferenceConversionType::Option | ReferenceConversionType::Result => { if self.impls_deref { - format!("&self.{field_name}") + make::expr_ref(expr, false) } else { - format!("self.{field_name}.as_ref()") + make::expr_method_call(expr, make::name_ref("as_ref"), make::arg_list([])) } } } From f1293a8fc43e9611c11a832c8f8b53343dfc627f Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Thu, 14 Dec 2023 23:04:07 -0500 Subject: [PATCH 06/13] Add newline to body when where clause is present --- .../src/handlers/generate_delegate_trait.rs | 6 ++++-- crates/syntax/src/ast/make.rs | 18 +++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/crates/ide-assists/src/handlers/generate_delegate_trait.rs b/crates/ide-assists/src/handlers/generate_delegate_trait.rs index 37dd41f8ec..7a60287f92 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_trait.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_trait.rs @@ -956,7 +956,8 @@ where impl AnotherTrait for S where T: AnotherTrait, -{}"#, +{ +}"#, ); } @@ -1446,7 +1447,8 @@ where impl AnotherTrait for S where T: AnotherTrait, -{}"#, +{ +}"#, ); } diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index b6e5e6a4e4..9d6ed67361 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -248,8 +248,11 @@ pub fn impl_( let gen_params = generic_params.map_or_else(String::new, |it| it.to_string()); + let body_newline = + if where_clause.is_some() && body.is_none() { "\n".to_string() } else { String::new() }; + let where_clause = match where_clause { - Some(pr) => pr.to_string(), + Some(pr) => format!("\n{pr}\n"), None => " ".to_string(), }; @@ -258,7 +261,9 @@ pub fn impl_( None => String::new(), }; - ast_from_text(&format!("impl{gen_params} {path_type}{gen_args}{where_clause}{{{body}}}")) + ast_from_text(&format!( + "impl{gen_params} {path_type}{gen_args}{where_clause}{{{body_newline}{body}}}" + )) } pub fn impl_trait( @@ -284,6 +289,13 @@ pub fn impl_trait( let is_negative = if is_negative { "! " } else { "" }; + let body_newline = + if (ty_where_clause.is_some() || trait_where_clause.is_some()) && body.is_none() { + "\n".to_string() + } else { + String::new() + }; + let where_clause = merge_where_clause(ty_where_clause, trait_where_clause) .map_or_else(|| " ".to_string(), |wc| format!("\n{}\n", wc)); @@ -292,7 +304,7 @@ pub fn impl_trait( None => String::new(), }; - ast_from_text(&format!("{is_unsafe}impl{gen_params} {is_negative}{path_type}{trait_gen_args} for {ty}{type_gen_args}{where_clause}{{{body}}}")) + ast_from_text(&format!("{is_unsafe}impl{gen_params} {is_negative}{path_type}{trait_gen_args} for {ty}{type_gen_args}{where_clause}{{{body_newline}{body}}}")) } pub fn impl_trait_type(bounds: ast::TypeBoundList) -> ast::ImplTraitType { From ab2233e562dd9e7d43fb969a4cf3280995fb7d49 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Thu, 14 Dec 2023 18:23:31 -0500 Subject: [PATCH 07/13] Migrate `generate_getter_or_setter` to mutable ast --- .../src/handlers/generate_getter_or_setter.rs | 176 ++++++++---------- 1 file changed, 80 insertions(+), 96 deletions(-) diff --git a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs index 5d9dec1330..c533244535 100644 --- a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs +++ b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs @@ -1,12 +1,12 @@ use ide_db::{famous_defs::FamousDefs, source_change::SourceChangeBuilder}; use stdx::{format_to, to_lower_snake_case}; use syntax::{ - ast::{self, AstNode, HasName, HasVisibility}, - TextRange, + ast::{self, edit_in_place::Indent, make, AstNode, HasName, HasVisibility}, + ted, TextRange, }; use crate::{ - utils::{convert_reference_type, find_impl_block_end, find_struct_impl, generate_impl_text}, + utils::{convert_reference_type, find_struct_impl, generate_impl}, AssistContext, AssistId, AssistKind, Assists, GroupLabel, }; @@ -214,14 +214,14 @@ fn generate_getter_from_info( ctx: &AssistContext<'_>, info: &AssistInfo, record_field_info: &RecordFieldInfo, -) -> String { - let mut buf = String::with_capacity(512); - - let vis = info.strukt.visibility().map_or(String::new(), |v| format!("{v} ")); +) -> ast::Fn { let (ty, body) = if matches!(info.assist_type, AssistType::MutGet) { ( - format!("&mut {}", record_field_info.field_ty), - format!("&mut self.{}", record_field_info.field_name), + make::ty_ref(record_field_info.field_ty.clone(), true), + make::expr_ref( + make::expr_field(make::ext::expr_self(), &record_field_info.field_name.text()), + true, + ), ) } else { (|| { @@ -233,48 +233,61 @@ fn generate_getter_from_info( .map(|conversion| { cov_mark::hit!(convert_reference_type); ( - conversion.convert_type(ctx.db()).to_string(), - conversion.getter(record_field_info.field_name.to_string()).to_string(), + conversion.convert_type(ctx.db()), + conversion.getter(record_field_info.field_name.to_string()), ) }) })() .unwrap_or_else(|| { ( - format!("&{}", record_field_info.field_ty), - format!("&self.{}", record_field_info.field_name), + make::ty_ref(record_field_info.field_ty.clone(), false), + make::expr_ref( + make::expr_field(make::ext::expr_self(), &record_field_info.field_name.text()), + false, + ), ) }) }; - format_to!( - buf, - " {}fn {}(&{}self) -> {} {{ - {} - }}", - vis, - record_field_info.fn_name, - matches!(info.assist_type, AssistType::MutGet).then_some("mut ").unwrap_or_default(), - ty, - body, - ); + let self_param = if matches!(info.assist_type, AssistType::MutGet) { + make::mut_self_param() + } else { + make::self_param() + }; - buf + let strukt = &info.strukt; + let fn_name = make::name(&record_field_info.fn_name); + let params = make::param_list(Some(self_param), []); + let ret_type = Some(make::ret_type(ty)); + let body = make::block_expr([], Some(body)); + + make::fn_(strukt.visibility(), fn_name, None, None, params, body, ret_type, false, false, false) } -fn generate_setter_from_info(info: &AssistInfo, record_field_info: &RecordFieldInfo) -> String { - let mut buf = String::with_capacity(512); +fn generate_setter_from_info(info: &AssistInfo, record_field_info: &RecordFieldInfo) -> ast::Fn { let strukt = &info.strukt; - let fn_name = &record_field_info.fn_name; + let field_name = &record_field_info.fn_name; + let fn_name = make::name(&format!("set_{field_name}")); let field_ty = &record_field_info.field_ty; - let vis = strukt.visibility().map_or(String::new(), |v| format!("{v} ")); - format_to!( - buf, - " {vis}fn set_{fn_name}(&mut self, {fn_name}: {field_ty}) {{ - self.{fn_name} = {fn_name}; - }}" - ); - buf + // Make the param list + // `(&mut self, $field_name: $field_ty)` + let field_param = make::param( + make::ident_pat(false, false, make::name(&field_name)).into(), + field_ty.clone(), + ); + let params = make::param_list(Some(make::mut_self_param()), [field_param]); + + // Make the assignment body + // `self.$field_name = $field_name` + let self_expr = make::ext::expr_self(); + let lhs = make::expr_field(self_expr, &field_name); + let rhs = make::expr_path(make::ext::ident_path(&field_name)); + let assign_stmt = make::expr_stmt(make::expr_assignment(lhs, rhs)); + let body = make::block_expr([assign_stmt.into()], None); + + // Make the setter fn + make::fn_(strukt.visibility(), fn_name, None, None, params, body, None, false, false, false) } fn extract_and_parse( @@ -367,74 +380,45 @@ fn build_source_change( ) { let record_fields_count = info_of_record_fields.len(); - let mut buf = String::with_capacity(512); + let impl_def = if let Some(impl_def) = &assist_info.impl_def { + // We have an existing impl to add to + builder.make_mut(impl_def.clone()) + } else { + // Generate a new impl to add the methods to + let impl_def = generate_impl(&ast::Adt::Struct(assist_info.strukt.clone())); - // Check if an impl exists - if let Some(impl_def) = &assist_info.impl_def { - // Check if impl is empty - if let Some(assoc_item_list) = impl_def.assoc_item_list() { - if assoc_item_list.assoc_items().next().is_some() { - // If not empty then only insert a new line - buf.push('\n'); - } - } - } + // Insert it after the adt + let strukt = builder.make_mut(assist_info.strukt.clone()); + + ted::insert_all_raw( + ted::Position::after(strukt.syntax()), + vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()], + ); + + impl_def + }; + + let assoc_item_list = impl_def.get_or_create_assoc_item_list(); for (i, record_field_info) in info_of_record_fields.iter().enumerate() { - // this buf inserts a newline at the end of a getter - // automatically, if one wants to add one more newline - // for separating it from other assoc items, that needs - // to be handled separately - let mut getter_buf = match assist_info.assist_type { + // Make the new getter or setter fn + let new_fn = match assist_info.assist_type { AssistType::Set => generate_setter_from_info(&assist_info, record_field_info), _ => generate_getter_from_info(ctx, &assist_info, record_field_info), - }; + } + .clone_for_update(); + new_fn.indent(1.into()); - // Insert `$0` only for last getter we generate - if i == record_fields_count - 1 && ctx.config.snippet_cap.is_some() { - getter_buf = getter_buf.replacen("fn ", "fn $0", 1); + // Insert a tabstop only for last method we generate + if i == record_fields_count - 1 { + if let Some(cap) = ctx.config.snippet_cap { + if let Some(name) = new_fn.name() { + builder.add_tabstop_before(cap, name); + } + } } - // For first element we do not merge with '\n', as - // that can be inserted by impl_def check defined - // above, for other cases which are: - // - // - impl exists but it empty, here we would ideally - // not want to keep newline between impl { - // and fn () { line - // - // - next if impl itself does not exist, in this - // case we ourselves generate a new impl and that - // again ends up with the same reasoning as above - // for not keeping newline - if i == 0 { - buf = buf + &getter_buf; - } else { - buf = buf + "\n" + &getter_buf; - } - - // We don't insert a new line at the end of - // last getter as it will end up in the end - // of an impl where we would not like to keep - // getter and end of impl ( i.e. `}` ) with an - // extra line for no reason - if i < record_fields_count - 1 { - buf += "\n"; - } - } - - let start_offset = assist_info - .impl_def - .as_ref() - .and_then(|impl_def| find_impl_block_end(impl_def.to_owned(), &mut buf)) - .unwrap_or_else(|| { - buf = generate_impl_text(&ast::Adt::Struct(assist_info.strukt.clone()), &buf); - assist_info.strukt.syntax().text_range().end() - }); - - match ctx.config.snippet_cap { - Some(cap) => builder.insert_snippet(cap, start_offset, buf), - None => builder.insert(start_offset, buf), + assoc_item_list.add_item(new_fn.clone().into()); } } From e0117154cfb2ec7d05c95cbe9e1c907966dc49b5 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Fri, 15 Dec 2023 00:18:21 -0500 Subject: [PATCH 08/13] Migrate `generate_impl` to mutable ast --- .../ide-assists/src/handlers/generate_impl.rs | 126 +++++++----------- crates/ide-assists/src/tests/generated.rs | 8 +- 2 files changed, 53 insertions(+), 81 deletions(-) diff --git a/crates/ide-assists/src/handlers/generate_impl.rs b/crates/ide-assists/src/handlers/generate_impl.rs index d52d778d34..821783c283 100644 --- a/crates/ide-assists/src/handlers/generate_impl.rs +++ b/crates/ide-assists/src/handlers/generate_impl.rs @@ -1,10 +1,10 @@ -use syntax::ast::{self, AstNode, HasName}; - -use crate::{ - utils::{generate_impl_text, generate_trait_impl_text_intransitive}, - AssistContext, AssistId, AssistKind, Assists, +use syntax::{ + ast::{self, make, AstNode, HasName}, + ted, }; +use crate::{utils, AssistContext, AssistId, AssistKind, Assists}; + // Assist: generate_impl // // Adds a new inherent impl for a type. @@ -20,9 +20,7 @@ use crate::{ // data: T, // } // -// impl Ctx { -// $0 -// } +// impl Ctx {$0} // ``` pub(crate) fn generate_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let nominal = ctx.find_node_at_offset::()?; @@ -38,17 +36,22 @@ pub(crate) fn generate_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio format!("Generate impl for `{name}`"), target, |edit| { - let start_offset = nominal.syntax().text_range().end(); - match ctx.config.snippet_cap { - Some(cap) => { - let snippet = generate_impl_text(&nominal, " $0"); - edit.insert_snippet(cap, start_offset, snippet); - } - None => { - let snippet = generate_impl_text(&nominal, ""); - edit.insert(start_offset, snippet); + // Generate the impl + let impl_ = utils::generate_impl(&nominal); + + // Add a tabstop after the left curly brace + if let Some(cap) = ctx.config.snippet_cap { + if let Some(l_curly) = impl_.assoc_item_list().and_then(|it| it.l_curly_token()) { + edit.add_tabstop_after_token(cap, l_curly); } } + + // Add the impl after the adt + let nominal = edit.make_mut(nominal); + ted::insert_all_raw( + ted::Position::after(nominal.syntax()), + vec![make::tokens::blank_line().into(), impl_.syntax().clone().into()], + ); }, ) } @@ -68,9 +71,7 @@ pub(crate) fn generate_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio // data: T, // } // -// impl $0 for Ctx { -// -// } +// impl ${0:_} for Ctx {} // ``` pub(crate) fn generate_trait_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let nominal = ctx.find_node_at_offset::()?; @@ -86,17 +87,22 @@ pub(crate) fn generate_trait_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> format!("Generate trait impl for `{name}`"), target, |edit| { - let start_offset = nominal.syntax().text_range().end(); - match ctx.config.snippet_cap { - Some(cap) => { - let snippet = generate_trait_impl_text_intransitive(&nominal, "$0", ""); - edit.insert_snippet(cap, start_offset, snippet); - } - None => { - let text = generate_trait_impl_text_intransitive(&nominal, "", ""); - edit.insert(start_offset, text); + // Generate the impl + let impl_ = utils::generate_trait_impl_intransitive(&nominal, make::ty_placeholder()); + + // Make the trait type a placeholder snippet + if let Some(cap) = ctx.config.snippet_cap { + if let Some(trait_) = impl_.trait_() { + edit.add_placeholder_snippet(cap, trait_); } } + + // Add the impl after the adt + let nominal = edit.make_mut(nominal); + ted::insert_all_raw( + ted::Position::after(nominal.syntax()), + vec![make::tokens::blank_line().into(), impl_.syntax().clone().into()], + ); }, ) } @@ -117,9 +123,7 @@ mod tests { r#" struct Foo {} - impl Foo { - $0 - } + impl Foo {$0} "#, ); } @@ -134,9 +138,7 @@ mod tests { r#" struct Foo {} - impl Foo { - $0 - } + impl Foo {$0} "#, ); } @@ -151,9 +153,7 @@ mod tests { r#" struct Foo<'a, T: Foo<'a>> {} - impl<'a, T: Foo<'a>> Foo<'a, T> { - $0 - } + impl<'a, T: Foo<'a>> Foo<'a, T> {$0} "#, ); } @@ -171,9 +171,7 @@ mod tests { struct Foo<'a, T: Foo<'a>> {} #[cfg(feature = "foo")] - impl<'a, T: Foo<'a>> Foo<'a, T> { - $0 - } + impl<'a, T: Foo<'a>> Foo<'a, T> {$0} "#, ); } @@ -188,9 +186,7 @@ mod tests { r#" struct Defaulted {} - impl Defaulted { - $0 - } + impl Defaulted {$0} "#, ); } @@ -205,9 +201,7 @@ mod tests { r#" struct Defaulted<'a, 'b: 'a, T: Debug + Clone + 'a + 'b = String, const S: usize> {} - impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> Defaulted<'a, 'b, T, S> { - $0 - } + impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> Defaulted<'a, 'b, T, S> {$0} "#, ); } @@ -222,9 +216,7 @@ mod tests { r#" struct Defaulted {} - impl Defaulted { - $0 - } + impl Defaulted {$0} "#, ); } @@ -254,8 +246,7 @@ mod tests { impl Struct where T: Trait, - { - $0 + {$0 } "#, ); @@ -285,9 +276,7 @@ mod tests { r#" struct Foo {} - impl $0 for Foo { - - } + impl ${0:_} for Foo {} "#, ); } @@ -302,9 +291,7 @@ mod tests { r#" struct Foo {} - impl $0 for Foo { - - } + impl ${0:_} for Foo {} "#, ); } @@ -319,9 +306,7 @@ mod tests { r#" struct Foo<'a, T: Foo<'a>> {} - impl<'a, T: Foo<'a>> $0 for Foo<'a, T> { - - } + impl<'a, T: Foo<'a>> ${0:_} for Foo<'a, T> {} "#, ); } @@ -339,9 +324,7 @@ mod tests { struct Foo<'a, T: Foo<'a>> {} #[cfg(feature = "foo")] - impl<'a, T: Foo<'a>> $0 for Foo<'a, T> { - - } + impl<'a, T: Foo<'a>> ${0:_} for Foo<'a, T> {} "#, ); } @@ -356,9 +339,7 @@ mod tests { r#" struct Defaulted {} - impl $0 for Defaulted { - - } + impl ${0:_} for Defaulted {} "#, ); } @@ -373,9 +354,7 @@ mod tests { r#" struct Defaulted<'a, 'b: 'a, T: Debug + Clone + 'a + 'b = String, const S: usize> {} - impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> $0 for Defaulted<'a, 'b, T, S> { - - } + impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> ${0:_} for Defaulted<'a, 'b, T, S> {} "#, ); } @@ -390,9 +369,7 @@ mod tests { r#" struct Defaulted {} - impl $0 for Defaulted { - - } + impl ${0:_} for Defaulted {} "#, ); } @@ -419,11 +396,10 @@ mod tests { inner: T, } - impl $0 for Struct + impl ${0:_} for Struct where T: Trait, { - } "#, ); diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 5967cd38aa..2c82d970db 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1513,9 +1513,7 @@ struct Ctx { data: T, } -impl Ctx { - $0 -} +impl Ctx {$0} "#####, ) } @@ -1702,9 +1700,7 @@ struct Ctx { data: T, } -impl $0 for Ctx { - -} +impl ${0:_} for Ctx {} "#####, ) } From e28f5514e157a57e9d63adb9ea3f93099d033445 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Fri, 15 Dec 2023 01:22:47 -0500 Subject: [PATCH 09/13] Add `AssocItemList::add_item_at_start` Needed to recreate the behavior of `generate_new` addding the `new` method at the start of the impl --- crates/syntax/src/ast/edit_in_place.rs | 42 ++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index c9944b75b0..bc9c54d0b7 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -627,6 +627,8 @@ impl ast::Impl { } impl ast::AssocItemList { + /// Adds a new associated item after all of the existing associated items. + /// /// Attention! This function does align the first line of `item` with respect to `self`, /// but it does _not_ change indentation of other lines (if any). pub fn add_item(&self, item: ast::AssocItem) { @@ -650,6 +652,46 @@ impl ast::AssocItemList { ]; ted::insert_all(position, elements); } + + /// Adds a new associated item at the start of the associated item list. + /// + /// Attention! This function does align the first line of `item` with respect to `self`, + /// but it does _not_ change indentation of other lines (if any). + pub fn add_item_at_start(&self, item: ast::AssocItem) { + match self.assoc_items().next() { + Some(first_item) => { + let indent = IndentLevel::from_node(first_item.syntax()); + let before = Position::before(first_item.syntax()); + + ted::insert_all( + before, + vec![ + item.syntax().clone().into(), + make::tokens::whitespace(&format!("\n\n{indent}")).into(), + ], + ) + } + None => { + let (indent, position, whitespace) = match self.l_curly_token() { + Some(l_curly) => { + normalize_ws_between_braces(self.syntax()); + (IndentLevel::from_token(&l_curly) + 1, Position::after(&l_curly), "\n") + } + None => (IndentLevel::single(), Position::first_child_of(self.syntax()), ""), + }; + + let mut elements = vec![]; + + // Avoid pushing an empty whitespace token + if !indent.is_zero() || !whitespace.is_empty() { + elements.push(make::tokens::whitespace(&format!("{whitespace}{indent}")).into()) + } + elements.push(item.syntax().clone().into()); + + ted::insert_all(position, elements) + } + }; + } } impl ast::Fn { From 18ea09feca386d0a015a49c106428db160edcf7a Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Fri, 15 Dec 2023 01:24:23 -0500 Subject: [PATCH 10/13] Migrate `generate_new` to mutable ast --- .../ide-assists/src/handlers/generate_new.rs | 190 +++++++++++------- crates/ide-assists/src/tests/generated.rs | 4 +- 2 files changed, 125 insertions(+), 69 deletions(-) diff --git a/crates/ide-assists/src/handlers/generate_new.rs b/crates/ide-assists/src/handlers/generate_new.rs index 7bfd599660..22c75cd5ee 100644 --- a/crates/ide-assists/src/handlers/generate_new.rs +++ b/crates/ide-assists/src/handlers/generate_new.rs @@ -1,12 +1,13 @@ use ide_db::{ imports::import_assets::item_for_path_search, use_trivial_constructor::use_trivial_constructor, }; -use itertools::Itertools; -use stdx::format_to; -use syntax::ast::{self, AstNode, HasName, HasVisibility, StructKind}; +use syntax::{ + ast::{self, edit_in_place::Indent, make, AstNode, HasName, HasVisibility, StructKind}, + ted, +}; use crate::{ - utils::{find_impl_block_start, find_struct_impl, generate_impl_text}, + utils::{find_struct_impl, generate_impl}, AssistContext, AssistId, AssistKind, Assists, }; @@ -26,7 +27,9 @@ use crate::{ // } // // impl Ctx { -// fn $0new(data: T) -> Self { Self { data } } +// fn $0new(data: T) -> Self { +// Self { data } +// } // } // ``` pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { @@ -46,14 +49,6 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option let target = strukt.syntax().text_range(); acc.add(AssistId("generate_new", AssistKind::Generate), "Generate `new`", target, |builder| { - let mut buf = String::with_capacity(512); - - if impl_def.is_some() { - buf.push('\n'); - } - - let vis = strukt.visibility().map_or(String::new(), |v| format!("{v} ")); - let trivial_constructors = field_list .fields() .map(|f| { @@ -76,54 +71,79 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option &ty, )?; - Some(format!("{name}: {expr}")) + Some(make::record_expr_field(make::name_ref(&name.text()), Some(expr))) }) .collect::>(); - let params = field_list - .fields() - .enumerate() - .filter_map(|(i, f)| { - if trivial_constructors[i].is_none() { - let name = f.name()?; - let ty = f.ty()?; + let params = field_list.fields().enumerate().filter_map(|(i, f)| { + if trivial_constructors[i].is_none() { + let name = f.name()?; + let ty = f.ty()?; - Some(format!("{name}: {ty}")) - } else { - None - } - }) - .format(", "); + Some(make::param(make::ident_pat(false, false, name).into(), ty)) + } else { + None + } + }); + let params = make::param_list(None, params); - let fields = field_list - .fields() - .enumerate() - .filter_map(|(i, f)| { - let constructor = trivial_constructors[i].clone(); - if constructor.is_some() { - constructor - } else { - Some(f.name()?.to_string()) - } - }) - .format(", "); + let fields = field_list.fields().enumerate().filter_map(|(i, f)| { + let constructor = trivial_constructors[i].clone(); + if constructor.is_some() { + constructor + } else { + Some(make::record_expr_field(make::name_ref(&f.name()?.text()), None)) + } + }); + let fields = make::record_expr_field_list(fields); - format_to!(buf, " {vis}fn new({params}) -> Self {{ Self {{ {fields} }} }}"); + let record_expr = make::record_expr(make::ext::ident_path("Self"), fields); + let body = make::block_expr(None, Some(record_expr.into())); - let start_offset = impl_def - .and_then(|impl_def| find_impl_block_start(impl_def, &mut buf)) - .unwrap_or_else(|| { - buf = generate_impl_text(&ast::Adt::Struct(strukt.clone()), &buf); - strukt.syntax().text_range().end() - }); + let ret_type = make::ret_type(make::ty_path(make::ext::ident_path("Self"))); - match ctx.config.snippet_cap { - None => builder.insert(start_offset, buf), - Some(cap) => { - buf = buf.replace("fn new", "fn $0new"); - builder.insert_snippet(cap, start_offset, buf); + let fn_ = make::fn_( + strukt.visibility(), + make::name("new"), + None, + None, + params, + body, + Some(ret_type), + false, + false, + false, + ) + .clone_for_update(); + fn_.indent(1.into()); + + // Add a tabstop before the name + if let Some(cap) = ctx.config.snippet_cap { + if let Some(name) = fn_.name() { + builder.add_tabstop_before(cap, name); } } + + // Get the mutable version of the impl to modify + let impl_def = if let Some(impl_def) = impl_def { + builder.make_mut(impl_def) + } else { + // Generate a new impl to add the method to + let impl_def = generate_impl(&ast::Adt::Struct(strukt.clone())); + + // Insert it after the adt + let strukt = builder.make_mut(strukt.clone()); + + ted::insert_all_raw( + ted::Position::after(strukt.syntax()), + vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()], + ); + + impl_def + }; + + // Add the `new` method at the start of the impl + impl_def.get_or_create_assoc_item_list().add_item_at_start(fn_.into()); }) } @@ -148,7 +168,9 @@ struct Empty; struct Foo { empty: Empty } impl Foo { - fn $0new() -> Self { Self { empty: Empty } } + fn $0new() -> Self { + Self { empty: Empty } + } } "#, ); @@ -165,7 +187,9 @@ struct Empty; struct Foo { baz: String, empty: Empty } impl Foo { - fn $0new(baz: String) -> Self { Self { baz, empty: Empty } } + fn $0new(baz: String) -> Self { + Self { baz, empty: Empty } + } } "#, ); @@ -182,7 +206,9 @@ enum Empty { Bar } struct Foo { empty: Empty } impl Foo { - fn $0new() -> Self { Self { empty: Empty::Bar } } + fn $0new() -> Self { + Self { empty: Empty::Bar } + } } "#, ); @@ -201,7 +227,9 @@ struct Empty {} struct Foo { empty: Empty } impl Foo { - fn $0new(empty: Empty) -> Self { Self { empty } } + fn $0new(empty: Empty) -> Self { + Self { empty } + } } "#, ); @@ -218,7 +246,9 @@ enum Empty { Bar {} } struct Foo { empty: Empty } impl Foo { - fn $0new(empty: Empty) -> Self { Self { empty } } + fn $0new(empty: Empty) -> Self { + Self { empty } + } } "#, ); @@ -235,7 +265,9 @@ struct Foo {$0} struct Foo {} impl Foo { - fn $0new() -> Self { Self { } } + fn $0new() -> Self { + Self { } + } } "#, ); @@ -248,7 +280,9 @@ struct Foo {$0} struct Foo {} impl Foo { - fn $0new() -> Self { Self { } } + fn $0new() -> Self { + Self { } + } } "#, ); @@ -261,7 +295,9 @@ struct Foo<'a, T: Foo<'a>> {$0} struct Foo<'a, T: Foo<'a>> {} impl<'a, T: Foo<'a>> Foo<'a, T> { - fn $0new() -> Self { Self { } } + fn $0new() -> Self { + Self { } + } } "#, ); @@ -274,7 +310,9 @@ struct Foo { baz: String $0} struct Foo { baz: String } impl Foo { - fn $0new(baz: String) -> Self { Self { baz } } + fn $0new(baz: String) -> Self { + Self { baz } + } } "#, ); @@ -287,7 +325,9 @@ struct Foo { baz: String, qux: Vec $0} struct Foo { baz: String, qux: Vec } impl Foo { - fn $0new(baz: String, qux: Vec) -> Self { Self { baz, qux } } + fn $0new(baz: String, qux: Vec) -> Self { + Self { baz, qux } + } } "#, ); @@ -304,7 +344,9 @@ struct Foo { pub baz: String, pub qux: Vec $0} struct Foo { pub baz: String, pub qux: Vec } impl Foo { - fn $0new(baz: String, qux: Vec) -> Self { Self { baz, qux } } + fn $0new(baz: String, qux: Vec) -> Self { + Self { baz, qux } + } } "#, ); @@ -323,7 +365,9 @@ impl Foo {} struct Foo {} impl Foo { - fn $0new() -> Self { Self { } } + fn $0new() -> Self { + Self { } + } } "#, ); @@ -340,7 +384,9 @@ impl Foo { struct Foo {} impl Foo { - fn $0new() -> Self { Self { } } + fn $0new() -> Self { + Self { } + } fn qux(&self) {} } @@ -363,7 +409,9 @@ impl Foo { struct Foo {} impl Foo { - fn $0new() -> Self { Self { } } + fn $0new() -> Self { + Self { } + } fn qux(&self) {} fn baz() -> i32 { @@ -385,7 +433,9 @@ pub struct Foo {$0} pub struct Foo {} impl Foo { - pub fn $0new() -> Self { Self { } } + pub fn $0new() -> Self { + Self { } + } } "#, ); @@ -398,7 +448,9 @@ pub(crate) struct Foo {$0} pub(crate) struct Foo {} impl Foo { - pub(crate) fn $0new() -> Self { Self { } } + pub(crate) fn $0new() -> Self { + Self { } + } } "#, ); @@ -493,7 +545,9 @@ pub struct Source { } impl Source { - pub fn $0new(file_id: HirFileId, ast: T) -> Self { Self { file_id, ast } } + pub fn $0new(file_id: HirFileId, ast: T) -> Self { + Self { file_id, ast } + } pub fn map U, U>(self, f: F) -> Source { Source { file_id: self.file_id, ast: f(self.ast) } diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 2c82d970db..8ad735d0ae 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1601,7 +1601,9 @@ struct Ctx { } impl Ctx { - fn $0new(data: T) -> Self { Self { data } } + fn $0new(data: T) -> Self { + Self { data } + } } "#####, ) From 8eebf1701b94a7bd113c2c0f22503a6069752714 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Fri, 15 Dec 2023 02:26:52 -0500 Subject: [PATCH 11/13] Migrate `replace_derive_with_manual_impl` to mutable ast --- .../replace_derive_with_manual_impl.rs | 134 ++++++++++-------- 1 file changed, 75 insertions(+), 59 deletions(-) diff --git a/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs index 788cc846c2..0714b089ab 100644 --- a/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs @@ -2,15 +2,17 @@ use hir::{InFile, MacroFileIdExt, ModuleDef}; use ide_db::{helpers::mod_path_to_ast, imports::import_assets::NameToImport, items_locator}; use itertools::Itertools; use syntax::{ - ast::{self, AstNode, HasName}, + ast::{self, make, AstNode, HasName}, + ted, SyntaxKind::WHITESPACE, + T, }; use crate::{ assist_context::{AssistContext, Assists, SourceChangeBuilder}, utils::{ - add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, - generate_trait_impl_text, render_snippet, Cursor, DefaultMethods, IgnoreAssocItems, + add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, generate_trait_impl, + DefaultMethods, IgnoreAssocItems, }, AssistId, AssistKind, }; @@ -132,35 +134,59 @@ fn add_assist( label, target, |builder| { - let insert_pos = adt.syntax().text_range().end(); + let insert_after = ted::Position::after(builder.make_mut(adt.clone()).syntax()); + let impl_def_with_items = impl_def_from_trait(&ctx.sema, adt, &annotated_name, trait_, replace_trait_path); update_attribute(builder, old_derives, old_tree, old_trait_path, attr); - let trait_path = replace_trait_path.to_string(); + + let trait_path = make::ty_path(replace_trait_path.clone()); + match (ctx.config.snippet_cap, impl_def_with_items) { (None, _) => { - builder.insert(insert_pos, generate_trait_impl_text(adt, &trait_path, "")) + let impl_def = generate_trait_impl(adt, trait_path); + + ted::insert_all( + insert_after, + vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()], + ); + } + (Some(cap), None) => { + let impl_def = generate_trait_impl(adt, trait_path); + + if let Some(l_curly) = + impl_def.assoc_item_list().and_then(|it| it.l_curly_token()) + { + builder.add_tabstop_after_token(cap, l_curly); + } + + ted::insert_all( + insert_after, + vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()], + ); } - (Some(cap), None) => builder.insert_snippet( - cap, - insert_pos, - generate_trait_impl_text(adt, &trait_path, " $0"), - ), (Some(cap), Some((impl_def, first_assoc_item))) => { - let mut cursor = Cursor::Before(first_assoc_item.syntax()); - let placeholder; + let mut added_snippet = false; if let ast::AssocItem::Fn(ref func) = first_assoc_item { if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast) { if m.syntax().text() == "todo!()" { - placeholder = m; - cursor = Cursor::Replace(placeholder.syntax()); + // Make the `todo!()` a placeholder + builder.add_placeholder_snippet(cap, m); + added_snippet = true; } } } - let rendered = render_snippet(cap, impl_def.syntax(), cursor); - builder.insert_snippet(cap, insert_pos, format!("\n\n{rendered}")) + if !added_snippet { + // If we haven't already added a snippet, add a tabstop before the generated function + builder.add_tabstop_before(cap, first_assoc_item); + } + + ted::insert_all( + insert_after, + vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()], + ); } }; }, @@ -190,28 +216,7 @@ fn impl_def_from_trait( if trait_items.is_empty() { return None; } - let impl_def = { - use syntax::ast::Impl; - let text = generate_trait_impl_text(adt, trait_path.to_string().as_str(), ""); - // FIXME: `generate_trait_impl_text` currently generates two newlines - // at the front, but these leading newlines should really instead be - // inserted at the same time the impl is inserted - assert_eq!(&text[..2], "\n\n", "`generate_trait_impl_text` output changed"); - let parse = syntax::SourceFile::parse(&text[2..]); - let node = match parse.tree().syntax().descendants().find_map(Impl::cast) { - Some(it) => it, - None => { - panic!( - "Failed to make ast node `{}` from text {}", - std::any::type_name::(), - text - ) - } - }; - let node = node.clone_for_update(); - assert_eq!(node.syntax().text_range().start(), 0.into()); - node - }; + let impl_def = generate_trait_impl(adt, make::ty_path(trait_path.clone())); let first_assoc_item = add_trait_assoc_items_to_impl(sema, &trait_items, trait_, &impl_def, target_scope); @@ -238,20 +243,37 @@ fn update_attribute( let has_more_derives = !new_derives.is_empty(); if has_more_derives { - let new_derives = format!("({})", new_derives.iter().format(", ")); - builder.replace(old_tree.syntax().text_range(), new_derives); - } else { - let attr_range = attr.syntax().text_range(); - builder.delete(attr_range); + let old_tree = builder.make_mut(old_tree.clone()); - if let Some(line_break_range) = attr - .syntax() - .next_sibling_or_token() - .filter(|t| t.kind() == WHITESPACE) - .map(|t| t.text_range()) + // Make the paths into flat lists of tokens in a vec + let tt = new_derives.iter().map(|path| path.syntax().clone()).map(|node| { + node.descendants_with_tokens() + .filter_map(|element| element.into_token()) + .collect::>() + }); + // ...which are interspersed with ", " + let tt = Itertools::intersperse( + tt, + vec![make::token(T![,]).into(), make::tokens::single_space().into()], + ); + // ...wrap them into the appropriate `NodeOrToken` variant + let tt = tt.flatten().map(|token| syntax::NodeOrToken::Token(token)); + // ...and make them into a flat list of tokens + let tt = tt.collect::>(); + + let new_tree = make::token_tree(T!['('], tt).clone_for_update(); + ted::replace(old_tree.syntax(), new_tree.syntax()); + } else { + // Remove the attr and any trailing whitespace + let attr = builder.make_mut(attr.clone()); + + if let Some(line_break) = + attr.syntax().next_sibling_or_token().filter(|t| t.kind() == WHITESPACE) { - builder.delete(line_break_range); + ted::remove(line_break) } + + ted::remove(attr.syntax()) } } @@ -1168,9 +1190,7 @@ struct Foo { bar: String, } -impl Debug for Foo { - $0 -} +impl Debug for Foo {$0} "#, ) } @@ -1191,9 +1211,7 @@ pub struct Foo { bar: String, } -impl Debug for Foo { - $0 -} +impl Debug for Foo {$0} "#, ) } @@ -1211,9 +1229,7 @@ struct Foo {} #[derive(Display, Serialize)] struct Foo {} -impl Debug for Foo { - $0 -} +impl Debug for Foo {$0} "#, ) } From 116108205119d694c26bbe9450ca7ceb06a92366 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Wed, 31 Jan 2024 21:16:05 -0500 Subject: [PATCH 12/13] Remove unused code in `utils` All usages of `render_snippet` and `Cursor` have been removed as part of the migration --- crates/ide-assists/src/utils.rs | 50 +-------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-) diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 10d18ee67d..c167350be8 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -1,12 +1,10 @@ //! Assorted functions shared by several assists. -use std::ops; - pub(crate) use gen_trait_fn_body::gen_trait_fn_body; use hir::{db::HirDatabase, HasAttrs as HirHasAttrs, HirDisplay, InFile, Semantics}; use ide_db::{ famous_defs::FamousDefs, path_transform::PathTransform, - syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase, SnippetCap, + syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase, }; use stdx::format_to; use syntax::{ @@ -217,43 +215,6 @@ pub fn add_trait_assoc_items_to_impl( first_item.unwrap() } -#[derive(Clone, Copy, Debug)] -pub(crate) enum Cursor<'a> { - Replace(&'a SyntaxNode), - Before(&'a SyntaxNode), -} - -impl<'a> Cursor<'a> { - fn node(self) -> &'a SyntaxNode { - match self { - Cursor::Replace(node) | Cursor::Before(node) => node, - } - } -} - -pub(crate) fn render_snippet(_cap: SnippetCap, node: &SyntaxNode, cursor: Cursor<'_>) -> String { - assert!(cursor.node().ancestors().any(|it| it == *node)); - let range = cursor.node().text_range() - node.text_range().start(); - let range: ops::Range = range.into(); - - let mut placeholder = cursor.node().to_string(); - escape(&mut placeholder); - let tab_stop = match cursor { - Cursor::Replace(placeholder) => format!("${{0:{placeholder}}}"), - Cursor::Before(placeholder) => format!("$0{placeholder}"), - }; - - let mut buf = node.to_string(); - buf.replace_range(range, &tab_stop); - return buf; - - fn escape(buf: &mut String) { - stdx::replace(buf, '{', r"\{"); - stdx::replace(buf, '}', r"\}"); - stdx::replace(buf, '$', r"\$"); - } -} - pub(crate) fn vis_offset(node: &SyntaxNode) -> TextSize { node.children_with_tokens() .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR)) @@ -445,15 +406,6 @@ fn has_any_fn(imp: &ast::Impl, names: &[String]) -> bool { false } -/// Find the start of the `impl` block for the given `ast::Impl`. -// -// FIXME: this partially overlaps with `find_struct_impl` -pub(crate) fn find_impl_block_start(impl_def: ast::Impl, buf: &mut String) -> Option { - buf.push('\n'); - let start = impl_def.assoc_item_list().and_then(|it| it.l_curly_token())?.text_range().end(); - Some(start) -} - /// Find the end of the `impl` block for the given `ast::Impl`. // // FIXME: this partially overlaps with `find_struct_impl` From 05b8ccc3787888671bde6b8fae4175c1a1ebc37a Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Wed, 31 Jan 2024 23:08:51 -0500 Subject: [PATCH 13/13] Fix clippy lints --- .../ide-assists/src/handlers/extract_function.rs | 16 +++++----------- .../src/handlers/generate_getter_or_setter.rs | 10 ++++------ .../handlers/replace_derive_with_manual_impl.rs | 7 ++----- 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs index fce132f782..54e99e0795 100644 --- a/crates/ide-assists/src/handlers/extract_function.rs +++ b/crates/ide-assists/src/handlers/extract_function.rs @@ -187,7 +187,7 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op impl_.syntax().clone() } _ => { - fn_def.indent(new_indent.into()); + fn_def.indent(new_indent); fn_def.syntax().clone() } @@ -1416,17 +1416,13 @@ fn make_call(ctx: &AssistContext<'_>, fun: &Function, indent: IndentLevel) -> Sy [var] => { let name = var.local.name(ctx.db()); let name = make::name(&name.display(ctx.db()).to_string()); - Some(ast::Pat::IdentPat(make::ident_pat( - false, - var.mut_usage_outside_body, - name.into(), - ))) + Some(ast::Pat::IdentPat(make::ident_pat(false, var.mut_usage_outside_body, name))) } vars => { let binding_pats = vars.iter().map(|var| { let name = var.local.name(ctx.db()); let name = make::name(&name.display(ctx.db()).to_string()); - make::ident_pat(false, var.mut_usage_outside_body, name.into()).into() + make::ident_pat(false, var.mut_usage_outside_body, name).into() }); Some(ast::Pat::TuplePat(make::tuple_pat(binding_pats))) } @@ -1874,7 +1870,7 @@ fn make_body(ctx: &AssistContext<'_>, old_indent: IndentLevel, fun: &Function) - } }; - let block = match &handler { + match &handler { FlowHandler::None => block, FlowHandler::Try { kind } => { let block = with_default_tail_expr(block, make::expr_unit()); @@ -1909,9 +1905,7 @@ fn make_body(ctx: &AssistContext<'_>, old_indent: IndentLevel, fun: &Function) - let args = make::arg_list(iter::once(tail_expr)); make::expr_call(ok, args) }), - }; - - block + } } fn map_tail_expr(block: ast::BlockExpr, f: impl FnOnce(ast::Expr) -> ast::Expr) -> ast::BlockExpr { diff --git a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs index c533244535..e90a032f1c 100644 --- a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs +++ b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs @@ -272,17 +272,15 @@ fn generate_setter_from_info(info: &AssistInfo, record_field_info: &RecordFieldI // Make the param list // `(&mut self, $field_name: $field_ty)` - let field_param = make::param( - make::ident_pat(false, false, make::name(&field_name)).into(), - field_ty.clone(), - ); + let field_param = + make::param(make::ident_pat(false, false, make::name(field_name)).into(), field_ty.clone()); let params = make::param_list(Some(make::mut_self_param()), [field_param]); // Make the assignment body // `self.$field_name = $field_name` let self_expr = make::ext::expr_self(); - let lhs = make::expr_field(self_expr, &field_name); - let rhs = make::expr_path(make::ext::ident_path(&field_name)); + let lhs = make::expr_field(self_expr, field_name); + let rhs = make::expr_path(make::ext::ident_path(field_name)); let assign_stmt = make::expr_stmt(make::expr_assignment(lhs, rhs)); let body = make::block_expr([assign_stmt.into()], None); diff --git a/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs index 0714b089ab..3420d906de 100644 --- a/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs @@ -252,12 +252,9 @@ fn update_attribute( .collect::>() }); // ...which are interspersed with ", " - let tt = Itertools::intersperse( - tt, - vec![make::token(T![,]).into(), make::tokens::single_space().into()], - ); + let tt = Itertools::intersperse(tt, vec![make::token(T![,]), make::tokens::single_space()]); // ...wrap them into the appropriate `NodeOrToken` variant - let tt = tt.flatten().map(|token| syntax::NodeOrToken::Token(token)); + let tt = tt.flatten().map(syntax::NodeOrToken::Token); // ...and make them into a flat list of tokens let tt = tt.collect::>();