From d645b81b289cf5667c717371364925f582af8ab4 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Sat, 6 Mar 2021 14:03:55 -0800 Subject: [PATCH 1/2] generate_function assist infer return type --- .../src/handlers/generate_function.rs | 46 +++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs index 3870b7e75a..fd4f2fbed8 100644 --- a/crates/ide_assists/src/handlers/generate_function.rs +++ b/crates/ide_assists/src/handlers/generate_function.rs @@ -104,6 +104,7 @@ struct FunctionBuilder { fn_name: ast::Name, type_params: Option, params: ast::ParamList, + ret_type: Option, file: FileId, needs_pub: bool, } @@ -131,8 +132,9 @@ impl FunctionBuilder { let target_module = target_module.or_else(|| ctx.sema.scope(target.syntax()).module())?; let fn_name = fn_name(&path)?; let (type_params, params) = fn_args(ctx, target_module, &call)?; + let ret_type = fn_ret_type(ctx, target_module, &call); - Some(Self { target, fn_name, type_params, params, file, needs_pub }) + Some(Self { target, fn_name, type_params, params, ret_type, file, needs_pub }) } fn render(self) -> FunctionTemplate { @@ -145,7 +147,7 @@ impl FunctionBuilder { self.type_params, self.params, fn_body, - Some(make::ret_type(make::ty_unit())), + Some(self.ret_type.unwrap_or_else(|| make::ret_type(make::ty_unit()))), ); let leading_ws; let trailing_ws; @@ -223,6 +225,23 @@ fn fn_args( Some((None, make::param_list(None, params))) } +fn fn_ret_type( + ctx: &AssistContext, + target_module: hir::Module, + call: &ast::CallExpr, +) -> Option { + let ty = ctx.sema.type_of_expr(&ast::Expr::CallExpr(call.clone()))?; + if ty.is_unknown() { + return None; + } + + if let Ok(rendered) = ty.display_source_code(ctx.db(), target_module.into()) { + Some(make::ret_type(make::ty(&rendered))) + } else { + None + } +} + /// Makes duplicate argument names unique by appending incrementing numbers. /// /// ``` @@ -546,7 +565,7 @@ impl Baz { } } -fn bar(baz: Baz) ${0:-> ()} { +fn bar(baz: Baz) ${0:-> Baz} { todo!() } ", @@ -1059,6 +1078,27 @@ pub(crate) fn bar() ${0:-> ()} { ) } + #[test] + fn add_function_with_return_type() { + check_assist( + generate_function, + r" +fn main() { + let x: u32 = foo$0(); +} +", + r" +fn main() { + let x: u32 = foo(); +} + +fn foo() ${0:-> u32} { + todo!() +} +", + ) + } + #[test] fn add_function_not_applicable_if_function_already_exists() { check_assist_not_applicable( From b275e609051f217f330509da26cf74bf941cf972 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Sat, 6 Mar 2021 14:29:45 -0800 Subject: [PATCH 2/2] generate_function assist don't render snippet if ret type inferred --- .../src/handlers/generate_function.rs | 73 ++++++++++++------- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs index fd4f2fbed8..6f95b1a073 100644 --- a/crates/ide_assists/src/handlers/generate_function.rs +++ b/crates/ide_assists/src/handlers/generate_function.rs @@ -83,17 +83,18 @@ struct FunctionTemplate { leading_ws: String, fn_def: ast::Fn, ret_type: ast::RetType, + should_render_snippet: bool, trailing_ws: String, file: FileId, } impl FunctionTemplate { fn to_string(&self, cap: Option) -> String { - let f = match cap { - Some(cap) => { + let f = match (cap, self.should_render_snippet) { + (Some(cap), true) => { render_snippet(cap, self.fn_def.syntax(), Cursor::Replace(self.ret_type.syntax())) } - None => self.fn_def.to_string(), + _ => self.fn_def.to_string(), }; format!("{}{}{}", self.leading_ws, f, self.trailing_ws) } @@ -104,7 +105,8 @@ struct FunctionBuilder { fn_name: ast::Name, type_params: Option, params: ast::ParamList, - ret_type: Option, + ret_type: ast::RetType, + should_render_snippet: bool, file: FileId, needs_pub: bool, } @@ -132,9 +134,44 @@ impl FunctionBuilder { let target_module = target_module.or_else(|| ctx.sema.scope(target.syntax()).module())?; let fn_name = fn_name(&path)?; let (type_params, params) = fn_args(ctx, target_module, &call)?; - let ret_type = fn_ret_type(ctx, target_module, &call); - Some(Self { target, fn_name, type_params, params, ret_type, file, needs_pub }) + // should_render_snippet intends to express a rough level of confidence about + // the correctness of the return type. + // + // If we are able to infer some return type, and that return type is not unit, we + // don't want to render the snippet. The assumption here is in this situation the + // return type is just as likely to be correct as any other part of the generated + // function. + // + // In the case where the return type is inferred as unit it is likely that the + // user does in fact intend for this generated function to return some non unit + // type, but that the current state of their code doesn't allow that return type + // to be accurately inferred. + let (ret_ty, should_render_snippet) = { + match ctx.sema.type_of_expr(&ast::Expr::CallExpr(call.clone())) { + Some(ty) if ty.is_unknown() || ty.is_unit() => (make::ty_unit(), true), + Some(ty) => { + let rendered = ty.display_source_code(ctx.db(), target_module.into()); + match rendered { + Ok(rendered) => (make::ty(&rendered), false), + Err(_) => (make::ty_unit(), true), + } + } + None => (make::ty_unit(), true), + } + }; + let ret_type = make::ret_type(ret_ty); + + Some(Self { + target, + fn_name, + type_params, + params, + ret_type, + should_render_snippet, + file, + needs_pub, + }) } fn render(self) -> FunctionTemplate { @@ -147,7 +184,7 @@ impl FunctionBuilder { self.type_params, self.params, fn_body, - Some(self.ret_type.unwrap_or_else(|| make::ret_type(make::ty_unit()))), + Some(self.ret_type), ); let leading_ws; let trailing_ws; @@ -173,6 +210,7 @@ impl FunctionBuilder { insert_offset, leading_ws, ret_type: fn_def.ret_type().unwrap(), + should_render_snippet: self.should_render_snippet, fn_def, trailing_ws, file: self.file, @@ -225,23 +263,6 @@ fn fn_args( Some((None, make::param_list(None, params))) } -fn fn_ret_type( - ctx: &AssistContext, - target_module: hir::Module, - call: &ast::CallExpr, -) -> Option { - let ty = ctx.sema.type_of_expr(&ast::Expr::CallExpr(call.clone()))?; - if ty.is_unknown() { - return None; - } - - if let Ok(rendered) = ty.display_source_code(ctx.db(), target_module.into()) { - Some(make::ret_type(make::ty(&rendered))) - } else { - None - } -} - /// Makes duplicate argument names unique by appending incrementing numbers. /// /// ``` @@ -565,7 +586,7 @@ impl Baz { } } -fn bar(baz: Baz) ${0:-> Baz} { +fn bar(baz: Baz) -> Baz { todo!() } ", @@ -1092,7 +1113,7 @@ fn main() { let x: u32 = foo(); } -fn foo() ${0:-> u32} { +fn foo() -> u32 { todo!() } ",