diff --git a/crates/ide_assists/src/handlers/early_return.rs b/crates/ide_assists/src/handlers/early_return.rs index 174c2e29540..4886b599be0 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 97df906f829..14dea0989f8 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 4f6d68e45a5..c4bd5eaa7b7 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 6a658d4cfd8..60cf49988e1 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_call.rs b/crates/ide_assists/src/handlers/inline_call.rs new file mode 100644 index 00000000000..93f8edb1a82 --- /dev/null +++ b/crates/ide_assists/src/handlers/inline_call.rs @@ -0,0 +1,381 @@ +use ast::make; +use hir::{HasSource, PathResolution}; +use syntax::{ + ast::{self, edit::AstNodeEdit, ArgListOwner}, + ted, AstNode, +}; + +use crate::{ + assist_context::{AssistContext, Assists}, + AssistId, AssistKind, +}; + +// Assist: inline_call +// +// Inlines a function or method body. +// +// ``` +// fn add(a: u32, b: u32) -> u32 { a + b } +// fn main() { +// let x = add$0(1, 2); +// } +// ``` +// -> +// ``` +// fn add(a: u32, b: u32) -> u32 { a + b } +// fn main() { +// let x = { +// let a = 1; +// let b = 2; +// a + b +// }; +// } +// ``` +pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let (label, function, arguments, expr) = + if let Some(path_expr) = ctx.find_node_at_offset::() { + 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)) + | PathResolution::AssocItem(hir::AssocItem::Function(f)) => f, + _ => return None, + }; + ( + format!("Inline `{}`", path), + function, + call.arg_list()?.args().collect(), + ast::Expr::CallExpr(call), + ) + } else { + 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()); + (format!("Inline `{}`", name_ref), function, arguments, ast::Expr::MethodCallExpr(call)) + }; + + inline_(acc, ctx, label, function, arguments, expr) +} + +pub(crate) fn inline_( + acc: &mut Assists, + ctx: &AssistContext, + label: String, + 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()?; + + 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_call_incorrect_number_of_arguments); + return None; + } + + let new_bindings = params.into_iter().zip(arg_list); + + let body = function_source.body()?; + + acc.add( + AssistId("inline_call", AssistKind::RefactorInline), + label, + expr.syntax().text_range(), + |builder| { + // FIXME: emit type ascriptions when a coercion happens? + // FIXME: dont create locals when its not required + let statements = new_bindings + .map(|(pattern, value)| make::let_stmt(pattern, Some(value)).into()) + .chain(body.statements()); + + let original_indentation = expr.indent_level(); + let mut replacement = make::block_expr(statements, body.tail_expr()) + .reset_indent() + .indent(original_indentation); + + 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)); + }, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn no_args_or_return_value_gets_inlined_without_block() { + check_assist( + inline_call, + r#" +fn foo() { println!("Hello, World!"); } +fn main() { + fo$0o(); +} +"#, + r#" +fn foo() { println!("Hello, World!"); } +fn main() { + { + println!("Hello, World!"); + }; +} +"#, + ); + } + + #[test] + fn args_with_side_effects() { + check_assist( + inline_call, + r#" +fn foo(name: String) { println!("Hello, {}!", name); } +fn main() { + foo$0(String::from("Michael")); +} +"#, + r#" +fn foo(name: String) { println!("Hello, {}!", name); } +fn main() { + { + let name = String::from("Michael"); + println!("Hello, {}!", name); + }; +} +"#, + ); + } + + #[test] + fn not_applicable_when_incorrect_number_of_parameters_are_provided() { + cov_mark::check!(inline_call_incorrect_number_of_arguments); + check_assist_not_applicable( + inline_call, + r#" +fn add(a: u32, b: u32) -> u32 { a + b } +fn main() { let x = add$0(42); } +"#, + ); + } + + #[test] + fn function_with_multiple_statements() { + check_assist( + inline_call, + r#" +fn foo(a: u32, b: u32) -> u32 { + let x = a + b; + let y = x - b; + x * y +} + +fn main() { + let x = foo$0(1, 2); +} +"#, + r#" +fn foo(a: u32, b: u32) -> u32 { + let x = a + b; + let y = x - b; + x * y +} + +fn main() { + let x = { + let a = 1; + let b = 2; + let x = a + b; + let y = x - b; + x * y + }; +} +"#, + ); + } + + #[test] + fn function_with_self_param() { + check_assist( + inline_call, + 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_call, + 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_call, + 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_call, + 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/inline_function.rs b/crates/ide_assists/src/handlers/inline_function.rs deleted file mode 100644 index 8e56029cb3f..00000000000 --- a/crates/ide_assists/src/handlers/inline_function.rs +++ /dev/null @@ -1,201 +0,0 @@ -use ast::make; -use hir::{HasSource, PathResolution}; -use syntax::{ - ast::{self, edit::AstNodeEdit, ArgListOwner}, - AstNode, -}; - -use crate::{ - assist_context::{AssistContext, Assists}, - AssistId, AssistKind, -}; - -// Assist: inline_function -// -// Inlines a function body. -// -// ``` -// fn add(a: u32, b: u32) -> u32 { a + b } -// fn main() { -// let x = add$0(1, 2); -// } -// ``` -// -> -// ``` -// fn add(a: u32, b: u32) -> u32 { a + b } -// fn main() { -// let x = { -// let a = 1; -// let b = 2; -// a + b -// }; -// } -// ``` -pub(crate) fn inline_function(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let path_expr: ast::PathExpr = ctx.find_node_at_offset()?; - 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, - _ => return None, - }; - - let function_source = function.source(ctx.db())?; - let arguments: Vec<_> = call.arg_list()?.args().collect(); - let parameters = function_parameter_patterns(&function_source.value)?; - - if arguments.len() != parameters.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 body = function_source.value.body()?; - - acc.add( - AssistId("inline_function", AssistKind::RefactorInline), - format!("Inline `{}`", path), - call.syntax().text_range(), - |builder| { - let mut statements: Vec = Vec::new(); - - 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()) - .reset_indent() - .indent(original_indentation); - - builder.replace_ast(ast::Expr::CallExpr(call), 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}; - - use super::*; - - #[test] - fn no_args_or_return_value_gets_inlined_without_block() { - check_assist( - inline_function, - r#" -fn foo() { println!("Hello, World!"); } -fn main() { - fo$0o(); -} -"#, - r#" -fn foo() { println!("Hello, World!"); } -fn main() { - { - println!("Hello, World!"); - }; -} -"#, - ); - } - - #[test] - fn args_with_side_effects() { - check_assist( - inline_function, - r#" -fn foo(name: String) { println!("Hello, {}!", name); } -fn main() { - foo$0(String::from("Michael")); -} -"#, - r#" -fn foo(name: String) { println!("Hello, {}!", name); } -fn main() { - { - let name = String::from("Michael"); - println!("Hello, {}!", name); - }; -} -"#, - ); - } - - #[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); - check_assist_not_applicable( - inline_function, - r#" -fn add(a: u32, b: u32) -> u32 { a + b } -fn main() { let x = add$0(42); } -"#, - ); - } - - #[test] - fn function_with_multiple_statements() { - check_assist( - inline_function, - r#" -fn foo(a: u32, b: u32) -> u32 { - let x = a + b; - let y = x - b; - x * y -} - -fn main() { - let x = foo$0(1, 2); -} -"#, - r#" -fn foo(a: u32, b: u32) -> u32 { - let x = a + b; - let y = x - b; - x * y -} - -fn main() { - let x = { - let a = 1; - let b = 2; - let x = a + b; - let y = x - b; - x * y - }; -} -"#, - ); - } -} 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 0e41aeb89d3..ccc8449775a 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 86a57ce5dca..bc30020f1f9 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -85,7 +85,7 @@ mod handlers { mod generate_new; mod generate_setter; mod infer_function_return_type; - mod inline_function; + mod inline_call; mod inline_local_variable; mod introduce_named_lifetime; mod invert_if; @@ -155,7 +155,7 @@ mod handlers { generate_new::generate_new, generate_setter::generate_setter, infer_function_return_type::infer_function_return_type, - inline_function::inline_function, + inline_call::inline_call, inline_local_variable::inline_local_variable, introduce_named_lifetime::introduce_named_lifetime, invert_if::invert_if, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index c3c21317345..cae2ad57eb5 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -919,9 +919,9 @@ fn foo() -> i32 { 42i32 } } #[test] -fn doctest_inline_function() { +fn doctest_inline_call() { check_doc_test( - "inline_function", + "inline_call", r#####" fn add(a: u32, b: u32) -> u32 { a + b } fn main() { diff --git a/crates/ide_db/src/ty_filter.rs b/crates/ide_db/src/ty_filter.rs index 766d8c62888..91f1d1cb7db 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 08b00e6069e..e00ec7f1963 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,17 @@ 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 { + 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 "); } + format_to!(s, "{}", name); + s.push_str(": ())"); + ast_from_text(&s) } pub fn wildcard_pat() -> ast::WildcardPat {