From 688398febce6d76bb01a13f99ed607e9371a5c6a Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 3 Jul 2021 01:33:34 +0200 Subject: [PATCH] feat: Implement `inline_method` assist --- .../ide_assists/src/handlers/early_return.rs | 2 +- .../src/handlers/extract_function.rs | 12 +- .../src/handlers/fill_match_arms.rs | 5 +- .../src/handlers/generate_function.rs | 7 +- .../src/handlers/inline_function.rs | 313 +++++++++++++++--- .../src/handlers/replace_unwrap_with_match.rs | 2 +- crates/ide_assists/src/lib.rs | 1 + crates/ide_db/src/ty_filter.rs | 2 +- crates/syntax/src/ast/make.rs | 28 +- 9 files changed, 300 insertions(+), 72 deletions(-) diff --git a/crates/ide_assists/src/handlers/early_return.rs b/crates/ide_assists/src/handlers/early_return.rs index 174c2e2954..4886b599be 100644 --- a/crates/ide_assists/src/handlers/early_return.rs +++ b/crates/ide_assists/src/handlers/early_return.rs @@ -127,7 +127,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext) let happy_arm = { let pat = make::tuple_struct_pat( path, - once(make::ident_pat(make::name("it")).into()), + once(make::ext::simple_ident_pat(make::name("it")).into()), ); let expr = { let path = make::ext::ident_path("it"); diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 97df906f82..14dea0989f 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -341,9 +341,9 @@ impl Param { let var = self.var.name(ctx.db()).unwrap().to_string(); let var_name = make::name(&var); let pat = match self.kind() { - ParamKind::MutValue => make::ident_mut_pat(var_name), + ParamKind::MutValue => make::ident_pat(false, true, var_name), ParamKind::Value | ParamKind::SharedRef | ParamKind::MutRef => { - make::ident_pat(var_name) + make::ext::simple_ident_pat(var_name) } }; @@ -1072,7 +1072,7 @@ impl FlowHandler { } FlowHandler::IfOption { action } => { let path = make::ext::ident_path("Some"); - let value_pat = make::ident_pat(make::name("value")); + let value_pat = make::ext::simple_ident_pat(make::name("value")); let pattern = make::tuple_struct_pat(path, iter::once(value_pat.into())); let cond = make::condition(call_expr, Some(pattern.into())); let value = make::expr_path(make::ext::ident_path("value")); @@ -1086,7 +1086,7 @@ impl FlowHandler { let some_arm = { let path = make::ext::ident_path("Some"); - let value_pat = make::ident_pat(make::name(some_name)); + let value_pat = make::ext::simple_ident_pat(make::name(some_name)); let pat = make::tuple_struct_pat(path, iter::once(value_pat.into())); let value = make::expr_path(make::ext::ident_path(some_name)); make::match_arm(iter::once(pat.into()), None, value) @@ -1105,14 +1105,14 @@ impl FlowHandler { let ok_arm = { let path = make::ext::ident_path("Ok"); - let value_pat = make::ident_pat(make::name(ok_name)); + let value_pat = make::ext::simple_ident_pat(make::name(ok_name)); let pat = make::tuple_struct_pat(path, iter::once(value_pat.into())); let value = make::expr_path(make::ext::ident_path(ok_name)); make::match_arm(iter::once(pat.into()), None, value) }; let err_arm = { let path = make::ext::ident_path("Err"); - let value_pat = make::ident_pat(make::name(err_name)); + let value_pat = make::ext::simple_ident_pat(make::name(err_name)); let pat = make::tuple_struct_pat(path, iter::once(value_pat.into())); let value = make::expr_path(make::ext::ident_path(err_name)); make::match_arm( diff --git a/crates/ide_assists/src/handlers/fill_match_arms.rs b/crates/ide_assists/src/handlers/fill_match_arms.rs index 4f6d68e45a..c4bd5eaa7b 100644 --- a/crates/ide_assists/src/handlers/fill_match_arms.rs +++ b/crates/ide_assists/src/handlers/fill_match_arms.rs @@ -262,8 +262,9 @@ fn build_pat(db: &RootDatabase, module: hir::Module, var: ExtendedVariant) -> Op make::tuple_struct_pat(path, pats).into() } ast::StructKind::Record(field_list) => { - let pats = - field_list.fields().map(|f| make::ident_pat(f.name().unwrap()).into()); + let pats = field_list + .fields() + .map(|f| make::ext::simple_ident_pat(f.name().unwrap()).into()); make::record_pat(path, pats).into() } ast::StructKind::Unit => make::path_pat(path), diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs index 6a658d4cfd..60cf49988e 100644 --- a/crates/ide_assists/src/handlers/generate_function.rs +++ b/crates/ide_assists/src/handlers/generate_function.rs @@ -256,10 +256,9 @@ fn fn_args( }); } deduplicate_arg_names(&mut arg_names); - let params = arg_names - .into_iter() - .zip(arg_types) - .map(|(name, ty)| make::param(make::ident_pat(make::name(&name)).into(), make::ty(&ty))); + let params = arg_names.into_iter().zip(arg_types).map(|(name, ty)| { + make::param(make::ext::simple_ident_pat(make::name(&name)).into(), make::ty(&ty)) + }); Some((None, make::param_list(None, params))) } diff --git a/crates/ide_assists/src/handlers/inline_function.rs b/crates/ide_assists/src/handlers/inline_function.rs index 8e56029cb3..b00569bf01 100644 --- a/crates/ide_assists/src/handlers/inline_function.rs +++ b/crates/ide_assists/src/handlers/inline_function.rs @@ -2,7 +2,7 @@ use ast::make; use hir::{HasSource, PathResolution}; use syntax::{ ast::{self, edit::AstNodeEdit, ArgListOwner}, - AstNode, + ted, AstNode, }; use crate::{ @@ -10,6 +10,56 @@ use crate::{ AssistId, AssistKind, }; +// Assist: inline_method +// +// Inlines a method body. +// +// ``` +// struct Foo(u32); +// impl Foo { +// fn add(self, a: u32) -> Self { +// Foo(self.0 + a) +// } +// } +// fn main() { +// let x = Foo(3).add$0(2); +// } +// ``` +// -> +// ``` +// struct Foo(u32); +// impl Foo { +// fn add(self, a: u32) -> Self { +// Foo(self.0 + a) +// } +// } +// fn main() { +// let x = { +// let this = Foo(3); +// let a = 2; +// Foo(this.0 + a) +// }; +// } +// ``` +pub(crate) fn inline_method(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let name_ref: ast::NameRef = ctx.find_node_at_offset()?; + let call = name_ref.syntax().parent().and_then(ast::MethodCallExpr::cast)?; + let receiver = call.receiver()?; + let function = ctx.sema.resolve_method_call(&call)?; + let mut arguments = vec![receiver]; + arguments.extend(call.arg_list()?.args()); + + inline_( + acc, + ctx, + "inline_method", + &format!("Inline `{}`", name_ref), + function, + arguments, + ast::Expr::MethodCallExpr(call), + ) +} + // Assist: inline_function // // Inlines a function body. @@ -36,60 +86,95 @@ pub(crate) fn inline_function(acc: &mut Assists, ctx: &AssistContext) -> Option< let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; let path = path_expr.path()?; - let function = match ctx.sema.resolve_path(&path)? { - PathResolution::Def(hir::ModuleDef::Function(f)) => f, + let function = match dbg!(ctx.sema.resolve_path(&path)?) { + PathResolution::Def(hir::ModuleDef::Function(f)) + | PathResolution::AssocItem(hir::AssocItem::Function(f)) => f, _ => return None, }; + inline_( + acc, + ctx, + "inline_function", + &format!("Inline `{}`", path), + function, + call.arg_list()?.args().collect(), + ast::Expr::CallExpr(call), + ) +} - let function_source = function.source(ctx.db())?; - let arguments: Vec<_> = call.arg_list()?.args().collect(); - let parameters = function_parameter_patterns(&function_source.value)?; +pub(crate) fn inline_( + acc: &mut Assists, + ctx: &AssistContext, + assist_id: &'static str, + label: &str, + function: hir::Function, + arg_list: Vec, + expr: ast::Expr, +) -> Option<()> { + let hir::InFile { value: function_source, .. } = function.source(ctx.db())?; + let param_list = function_source.param_list()?; - if arguments.len() != parameters.len() { + let mut params = Vec::new(); + if let Some(self_param) = param_list.self_param() { + // FIXME this should depend on the receiver as well as the self_param + params.push( + make::ident_pat( + self_param.amp_token().is_some(), + self_param.mut_token().is_some(), + make::name("this"), + ) + .into(), + ); + } + for param in param_list.params() { + params.push(param.pat()?); + } + + if arg_list.len() != params.len() { // Can't inline the function because they've passed the wrong number of // arguments to this function cov_mark::hit!(inline_function_incorrect_number_of_arguments); return None; } - let new_bindings = parameters.into_iter().zip(arguments); + let new_bindings = params.into_iter().zip(arg_list); - let body = function_source.value.body()?; + let body = function_source.body()?; acc.add( - AssistId("inline_function", AssistKind::RefactorInline), - format!("Inline `{}`", path), - call.syntax().text_range(), + AssistId(assist_id, AssistKind::RefactorInline), + label, + expr.syntax().text_range(), |builder| { - let mut statements: Vec = Vec::new(); + // FIXME: emit type ascriptions when a coercion happens? + let statements = new_bindings + .map(|(pattern, value)| make::let_stmt(pattern, Some(value)).into()) + .chain(body.statements()); - for (pattern, value) in new_bindings { - statements.push(make::let_stmt(pattern, Some(value)).into()); - } - - statements.extend(body.statements()); - - let original_indentation = call.indent_level(); - let replacement = make::block_expr(statements, body.tail_expr()) + let original_indentation = expr.indent_level(); + let mut replacement = make::block_expr(statements, body.tail_expr()) .reset_indent() .indent(original_indentation); - builder.replace_ast(ast::Expr::CallExpr(call), ast::Expr::BlockExpr(replacement)); + if param_list.self_param().is_some() { + replacement = replacement.clone_for_update(); + let this = make::name_ref("this").syntax().clone_for_update(); + // FIXME dont look into descendant methods + replacement + .syntax() + .descendants() + .filter_map(ast::NameRef::cast) + .filter(|n| n.self_token().is_some()) + .collect::>() + .into_iter() + .rev() + .for_each(|self_ref| ted::replace(self_ref.syntax(), &this)); + } + builder.replace_ast(expr, ast::Expr::BlockExpr(replacement)); }, ) } -fn function_parameter_patterns(value: &ast::Fn) -> Option> { - let mut patterns = Vec::new(); - - for param in value.param_list()?.params() { - let pattern = param.pat()?; - patterns.push(pattern); - } - - Some(patterns) -} - #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; @@ -139,19 +224,6 @@ fn main() { ); } - #[test] - fn method_inlining_isnt_supported() { - check_assist_not_applicable( - inline_function, - r" -struct Foo; -impl Foo { fn bar(&self) {} } - -fn main() { Foo.bar$0(); } -", - ); - } - #[test] fn not_applicable_when_incorrect_number_of_parameters_are_provided() { cov_mark::check!(inline_function_incorrect_number_of_arguments); @@ -195,6 +267,155 @@ fn main() { x * y }; } +"#, + ); + } + + #[test] + fn function_with_self_param() { + check_assist( + inline_function, + r#" +struct Foo(u32); + +impl Foo { + fn add(self, a: u32) -> Self { + Foo(self.0 + a) + } +} + +fn main() { + let x = Foo::add$0(Foo(3), 2); +} +"#, + r#" +struct Foo(u32); + +impl Foo { + fn add(self, a: u32) -> Self { + Foo(self.0 + a) + } +} + +fn main() { + let x = { + let this = Foo(3); + let a = 2; + Foo(this.0 + a) + }; +} +"#, + ); + } + + #[test] + fn method_by_val() { + check_assist( + inline_method, + r#" +struct Foo(u32); + +impl Foo { + fn add(self, a: u32) -> Self { + Foo(self.0 + a) + } +} + +fn main() { + let x = Foo(3).add$0(2); +} +"#, + r#" +struct Foo(u32); + +impl Foo { + fn add(self, a: u32) -> Self { + Foo(self.0 + a) + } +} + +fn main() { + let x = { + let this = Foo(3); + let a = 2; + Foo(this.0 + a) + }; +} +"#, + ); + } + + #[test] + fn method_by_ref() { + check_assist( + inline_method, + r#" +struct Foo(u32); + +impl Foo { + fn add(&self, a: u32) -> Self { + Foo(self.0 + a) + } +} + +fn main() { + let x = Foo(3).add$0(2); +} +"#, + r#" +struct Foo(u32); + +impl Foo { + fn add(&self, a: u32) -> Self { + Foo(self.0 + a) + } +} + +fn main() { + let x = { + let ref this = Foo(3); + let a = 2; + Foo(this.0 + a) + }; +} +"#, + ); + } + + #[test] + fn method_by_ref_mut() { + check_assist( + inline_method, + r#" +struct Foo(u32); + +impl Foo { + fn clear(&mut self) { + self.0 = 0; + } +} + +fn main() { + let mut foo = Foo(3); + foo.clear$0(); +} +"#, + r#" +struct Foo(u32); + +impl Foo { + fn clear(&mut self) { + self.0 = 0; + } +} + +fn main() { + let mut foo = Foo(3); + { + let ref mut this = foo; + this.0 = 0; + }; +} "#, ); } diff --git a/crates/ide_assists/src/handlers/replace_unwrap_with_match.rs b/crates/ide_assists/src/handlers/replace_unwrap_with_match.rs index 0e41aeb89d..ccc8449775 100644 --- a/crates/ide_assists/src/handlers/replace_unwrap_with_match.rs +++ b/crates/ide_assists/src/handlers/replace_unwrap_with_match.rs @@ -52,7 +52,7 @@ pub(crate) fn replace_unwrap_with_match(acc: &mut Assists, ctx: &AssistContext) target, |builder| { let ok_path = make::ext::ident_path(happy_variant); - let it = make::ident_pat(make::name("it")).into(); + let it = make::ext::simple_ident_pat(make::name("it")).into(); let ok_tuple = make::tuple_struct_pat(ok_path, iter::once(it)).into(); let bind_path = make::ext::ident_path("it"); diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 86a57ce5dc..ac05491c6a 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -156,6 +156,7 @@ mod handlers { generate_setter::generate_setter, infer_function_return_type::infer_function_return_type, inline_function::inline_function, + inline_function::inline_method, inline_local_variable::inline_local_variable, introduce_named_lifetime::introduce_named_lifetime, invert_if::invert_if, diff --git a/crates/ide_db/src/ty_filter.rs b/crates/ide_db/src/ty_filter.rs index 766d8c6288..91f1d1cb7d 100644 --- a/crates/ide_db/src/ty_filter.rs +++ b/crates/ide_db/src/ty_filter.rs @@ -47,7 +47,7 @@ impl TryEnum { iter::once(make::wildcard_pat().into()), ) .into(), - TryEnum::Option => make::ident_pat(make::name("None")).into(), + TryEnum::Option => make::ext::simple_ident_pat(make::name("None")).into(), } } diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 08b00e6069..ea0f27756f 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -21,6 +21,13 @@ use crate::{ast, AstNode, SourceFile, SyntaxKind, SyntaxToken}; pub mod ext { use super::*; + pub fn simple_ident_pat(name: ast::Name) -> ast::IdentPat { + return from_text(&name.text()); + + fn from_text(text: &str) -> ast::IdentPat { + ast_from_text(&format!("fn f({}: ())", text)) + } + } pub fn ident_path(ident: &str) -> ast::Path { path_unqualified(path_segment(name_ref(ident))) } @@ -330,19 +337,18 @@ pub fn arg_list(args: impl IntoIterator) -> ast::ArgList { ast_from_text(&format!("fn main() {{ ()({}) }}", args.into_iter().format(", "))) } -pub fn ident_pat(name: ast::Name) -> ast::IdentPat { - return from_text(&name.text()); - - fn from_text(text: &str) -> ast::IdentPat { - ast_from_text(&format!("fn f({}: ())", text)) +pub fn ident_pat(ref_: bool, mut_: bool, name: ast::Name) -> ast::IdentPat { + use std::fmt::Write as _; + let mut s = String::from("fn f("); + if ref_ { + s.push_str("ref "); } -} -pub fn ident_mut_pat(name: ast::Name) -> ast::IdentPat { - return from_text(&name.text()); - - fn from_text(text: &str) -> ast::IdentPat { - ast_from_text(&format!("fn f(mut {}: ())", text)) + if mut_ { + s.push_str("mut "); } + let _ = write!(s, "{}", name); + s.push_str(": ())"); + ast_from_text(&s) } pub fn wildcard_pat() -> ast::WildcardPat {