aboutsummaryrefslogtreecommitdiff
path: root/crates/assists
diff options
context:
space:
mode:
Diffstat (limited to 'crates/assists')
-rw-r--r--crates/assists/src/handlers/early_return.rs2
-rw-r--r--crates/assists/src/handlers/extract_function.rs1147
-rw-r--r--crates/assists/src/handlers/generate_function.rs7
-rw-r--r--crates/assists/src/tests.rs5
4 files changed, 1036 insertions, 125 deletions
diff --git a/crates/assists/src/handlers/early_return.rs b/crates/assists/src/handlers/early_return.rs
index 8bbbb7ed5..6b87c3c05 100644
--- a/crates/assists/src/handlers/early_return.rs
+++ b/crates/assists/src/handlers/early_return.rs
@@ -88,7 +88,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext)
88 88
89 let early_expression: ast::Expr = match parent_container.kind() { 89 let early_expression: ast::Expr = match parent_container.kind() {
90 WHILE_EXPR | LOOP_EXPR => make::expr_continue(), 90 WHILE_EXPR | LOOP_EXPR => make::expr_continue(),
91 FN => make::expr_return(), 91 FN => make::expr_return(None),
92 _ => return None, 92 _ => return None,
93 }; 93 };
94 94
diff --git a/crates/assists/src/handlers/extract_function.rs b/crates/assists/src/handlers/extract_function.rs
index 4bddd4eec..4372479b9 100644
--- a/crates/assists/src/handlers/extract_function.rs
+++ b/crates/assists/src/handlers/extract_function.rs
@@ -1,4 +1,4 @@
1use std::{fmt, iter}; 1use std::iter;
2 2
3use ast::make; 3use ast::make;
4use either::Either; 4use either::Either;
@@ -16,9 +16,9 @@ use syntax::{
16 edit::{AstNodeEdit, IndentLevel}, 16 edit::{AstNodeEdit, IndentLevel},
17 AstNode, 17 AstNode,
18 }, 18 },
19 AstToken, Direction, SyntaxElement, 19 SyntaxElement,
20 SyntaxKind::{self, BLOCK_EXPR, BREAK_EXPR, COMMENT, PATH_EXPR, RETURN_EXPR}, 20 SyntaxKind::{self, BLOCK_EXPR, BREAK_EXPR, COMMENT, PATH_EXPR, RETURN_EXPR},
21 SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, T, 21 SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, WalkEvent, T,
22}; 22};
23use test_utils::mark; 23use test_utils::mark;
24 24
@@ -84,6 +84,7 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
84 // We should not have variables that outlive body if we have expression block 84 // We should not have variables that outlive body if we have expression block
85 return None; 85 return None;
86 } 86 }
87 let control_flow = external_control_flow(&body)?;
87 88
88 let target_range = body.text_range(); 89 let target_range = body.text_range();
89 90
@@ -98,16 +99,17 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
98 name: "fun_name".to_string(), 99 name: "fun_name".to_string(),
99 self_param: self_param.map(|(_, pat)| pat), 100 self_param: self_param.map(|(_, pat)| pat),
100 params, 101 params,
102 control_flow,
101 ret_ty, 103 ret_ty,
102 body, 104 body,
103 vars_defined_in_body_and_outlive, 105 vars_defined_in_body_and_outlive,
104 }; 106 };
105 107
106 builder.replace(target_range, format_replacement(ctx, &fun));
107
108 let new_indent = IndentLevel::from_node(&insert_after); 108 let new_indent = IndentLevel::from_node(&insert_after);
109 let old_indent = fun.body.indent_level(); 109 let old_indent = fun.body.indent_level();
110 110
111 builder.replace(target_range, format_replacement(ctx, &fun, old_indent));
112
111 let fn_def = format_function(ctx, module, &fun, old_indent, new_indent); 113 let fn_def = format_function(ctx, module, &fun, old_indent, new_indent);
112 let insert_offset = insert_after.text_range().end(); 114 let insert_offset = insert_after.text_range().end();
113 builder.insert(insert_offset, fn_def); 115 builder.insert(insert_offset, fn_def);
@@ -115,11 +117,104 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
115 ) 117 )
116} 118}
117 119
120fn external_control_flow(body: &FunctionBody) -> Option<ControlFlow> {
121 let mut ret_expr = None;
122 let mut try_expr = None;
123 let mut break_expr = None;
124 let mut continue_expr = None;
125 let (syntax, text_range) = match body {
126 FunctionBody::Expr(expr) => (expr.syntax(), expr.syntax().text_range()),
127 FunctionBody::Span { parent, text_range } => (parent.syntax(), *text_range),
128 };
129
130 let mut nested_loop = None;
131 let mut nested_scope = None;
132
133 for e in syntax.preorder() {
134 let e = match e {
135 WalkEvent::Enter(e) => e,
136 WalkEvent::Leave(e) => {
137 if nested_loop.as_ref() == Some(&e) {
138 nested_loop = None;
139 }
140 if nested_scope.as_ref() == Some(&e) {
141 nested_scope = None;
142 }
143 continue;
144 }
145 };
146 if nested_scope.is_some() {
147 continue;
148 }
149 if !text_range.contains_range(e.text_range()) {
150 continue;
151 }
152 match e.kind() {
153 SyntaxKind::LOOP_EXPR | SyntaxKind::WHILE_EXPR | SyntaxKind::FOR_EXPR => {
154 if nested_loop.is_none() {
155 nested_loop = Some(e);
156 }
157 }
158 SyntaxKind::FN
159 | SyntaxKind::CONST
160 | SyntaxKind::STATIC
161 | SyntaxKind::IMPL
162 | SyntaxKind::MODULE => {
163 if nested_scope.is_none() {
164 nested_scope = Some(e);
165 }
166 }
167 SyntaxKind::RETURN_EXPR => {
168 ret_expr = Some(ast::ReturnExpr::cast(e).unwrap());
169 }
170 SyntaxKind::TRY_EXPR => {
171 try_expr = Some(ast::TryExpr::cast(e).unwrap());
172 }
173 SyntaxKind::BREAK_EXPR if nested_loop.is_none() => {
174 break_expr = Some(ast::BreakExpr::cast(e).unwrap());
175 }
176 SyntaxKind::CONTINUE_EXPR if nested_loop.is_none() => {
177 continue_expr = Some(ast::ContinueExpr::cast(e).unwrap());
178 }
179 _ => {}
180 }
181 }
182
183 if try_expr.is_some() {
184 // FIXME: support try
185 return None;
186 }
187
188 let kind = match (ret_expr, break_expr, continue_expr) {
189 (Some(r), None, None) => match r.expr() {
190 Some(expr) => Some(FlowKind::ReturnValue(expr)),
191 None => Some(FlowKind::Return),
192 },
193 (Some(_), _, _) => {
194 mark::hit!(external_control_flow_return_and_bc);
195 return None;
196 }
197 (None, Some(_), Some(_)) => {
198 mark::hit!(external_control_flow_break_and_continue);
199 return None;
200 }
201 (None, Some(b), None) => match b.expr() {
202 Some(expr) => Some(FlowKind::BreakValue(expr)),
203 None => Some(FlowKind::Break),
204 },
205 (None, None, Some(_)) => Some(FlowKind::Continue),
206 (None, None, None) => None,
207 };
208
209 Some(ControlFlow { kind })
210}
211
118#[derive(Debug)] 212#[derive(Debug)]
119struct Function { 213struct Function {
120 name: String, 214 name: String,
121 self_param: Option<ast::SelfParam>, 215 self_param: Option<ast::SelfParam>,
122 params: Vec<Param>, 216 params: Vec<Param>,
217 control_flow: ControlFlow,
123 ret_ty: RetType, 218 ret_ty: RetType,
124 body: FunctionBody, 219 body: FunctionBody,
125 vars_defined_in_body_and_outlive: Vec<Local>, 220 vars_defined_in_body_and_outlive: Vec<Local>,
@@ -134,6 +229,11 @@ struct Param {
134 is_copy: bool, 229 is_copy: bool,
135} 230}
136 231
232#[derive(Debug)]
233struct ControlFlow {
234 kind: Option<FlowKind>,
235}
236
137#[derive(Debug, Clone, Copy, PartialEq, Eq)] 237#[derive(Debug, Clone, Copy, PartialEq, Eq)]
138enum ParamKind { 238enum ParamKind {
139 Value, 239 Value,
@@ -142,6 +242,30 @@ enum ParamKind {
142 MutRef, 242 MutRef,
143} 243}
144 244
245#[derive(Debug, Eq, PartialEq)]
246enum FunType {
247 Unit,
248 Single(hir::Type),
249 Tuple(Vec<hir::Type>),
250}
251
252impl Function {
253 fn return_type(&self, ctx: &AssistContext) -> FunType {
254 match &self.ret_ty {
255 RetType::Expr(ty) if ty.is_unit() => FunType::Unit,
256 RetType::Expr(ty) => FunType::Single(ty.clone()),
257 RetType::Stmt => match self.vars_defined_in_body_and_outlive.as_slice() {
258 [] => FunType::Unit,
259 [var] => FunType::Single(var.ty(ctx.db())),
260 vars => {
261 let types = vars.iter().map(|v| v.ty(ctx.db())).collect();
262 FunType::Tuple(types)
263 }
264 },
265 }
266 }
267}
268
145impl ParamKind { 269impl ParamKind {
146 fn is_ref(&self) -> bool { 270 fn is_ref(&self) -> bool {
147 matches!(self, ParamKind::SharedRef | ParamKind::MutRef) 271 matches!(self, ParamKind::SharedRef | ParamKind::MutRef)
@@ -158,26 +282,78 @@ impl Param {
158 } 282 }
159 } 283 }
160 284
161 fn value_prefix(&self) -> &'static str { 285 fn to_arg(&self, ctx: &AssistContext) -> ast::Expr {
286 let var = path_expr_from_local(ctx, self.var);
162 match self.kind() { 287 match self.kind() {
163 ParamKind::Value | ParamKind::MutValue => "", 288 ParamKind::Value | ParamKind::MutValue => var,
164 ParamKind::SharedRef => "&", 289 ParamKind::SharedRef => make::expr_ref(var, false),
165 ParamKind::MutRef => "&mut ", 290 ParamKind::MutRef => make::expr_ref(var, true),
166 } 291 }
167 } 292 }
168 293
169 fn type_prefix(&self) -> &'static str { 294 fn to_param(&self, ctx: &AssistContext, module: hir::Module) -> ast::Param {
170 match self.kind() { 295 let var = self.var.name(ctx.db()).unwrap().to_string();
171 ParamKind::Value | ParamKind::MutValue => "", 296 let var_name = make::name(&var);
172 ParamKind::SharedRef => "&", 297 let pat = match self.kind() {
173 ParamKind::MutRef => "&mut ", 298 ParamKind::MutValue => make::ident_mut_pat(var_name),
299 ParamKind::Value | ParamKind::SharedRef | ParamKind::MutRef => {
300 make::ident_pat(var_name)
301 }
302 };
303
304 let ty = make_ty(&self.ty, ctx, module);
305 let ty = match self.kind() {
306 ParamKind::Value | ParamKind::MutValue => ty,
307 ParamKind::SharedRef => make::ty_ref(ty, false),
308 ParamKind::MutRef => make::ty_ref(ty, true),
309 };
310
311 make::param(pat.into(), ty)
312 }
313}
314
315/// Control flow that is exported from extracted function
316///
317/// E.g.:
318/// ```rust,no_run
319/// loop {
320/// $0
321/// if 42 == 42 {
322/// break;
323/// }
324/// $0
325/// }
326/// ```
327#[derive(Debug, Clone)]
328enum FlowKind {
329 /// Return without value (`return;`)
330 Return,
331 /// Return with value (`return $expr;`)
332 ReturnValue(ast::Expr),
333 /// Break without value (`return;`)
334 Break,
335 /// Break with value (`break $expr;`)
336 BreakValue(ast::Expr),
337 /// Continue
338 Continue,
339}
340
341impl FlowKind {
342 fn make_expr(&self, expr: Option<ast::Expr>) -> ast::Expr {
343 match self {
344 FlowKind::Return | FlowKind::ReturnValue(_) => make::expr_return(expr),
345 FlowKind::Break | FlowKind::BreakValue(_) => make::expr_break(expr),
346 FlowKind::Continue => {
347 stdx::always!(expr.is_none(), "continue with value is not possible");
348 make::expr_continue()
349 }
174 } 350 }
175 } 351 }
176 352
177 fn mut_pattern(&self) -> &'static str { 353 fn expr_ty(&self, ctx: &AssistContext) -> Option<hir::Type> {
178 match self.kind() { 354 match self {
179 ParamKind::MutValue => "mut ", 355 FlowKind::ReturnValue(expr) | FlowKind::BreakValue(expr) => ctx.sema.type_of_expr(expr),
180 _ => "", 356 FlowKind::Return | FlowKind::Break | FlowKind::Continue => None,
181 } 357 }
182 } 358 }
183} 359}
@@ -195,14 +371,6 @@ impl RetType {
195 RetType::Stmt => true, 371 RetType::Stmt => true,
196 } 372 }
197 } 373 }
198
199 fn as_fn_ret(&self) -> Option<&hir::Type> {
200 match self {
201 RetType::Stmt => None,
202 RetType::Expr(ty) if ty.is_unit() => None,
203 RetType::Expr(ty) => Some(ty),
204 }
205 }
206} 374}
207 375
208/// Semantically same as `ast::Expr`, but preserves identity when using only part of the Block 376/// Semantically same as `ast::Expr`, but preserves identity when using only part of the Block
@@ -234,7 +402,7 @@ impl FunctionBody {
234 fn indent_level(&self) -> IndentLevel { 402 fn indent_level(&self) -> IndentLevel {
235 match &self { 403 match &self {
236 FunctionBody::Expr(expr) => IndentLevel::from_node(expr.syntax()), 404 FunctionBody::Expr(expr) => IndentLevel::from_node(expr.syntax()),
237 FunctionBody::Span { parent, .. } => IndentLevel::from_node(parent.syntax()), 405 FunctionBody::Span { parent, .. } => IndentLevel::from_node(parent.syntax()) + 1,
238 } 406 }
239 } 407 }
240 408
@@ -668,9 +836,24 @@ fn scope_for_fn_insertion_node(node: &SyntaxNode, anchor: Anchor) -> Option<Synt
668 last_ancestor 836 last_ancestor
669} 837}
670 838
671fn format_replacement(ctx: &AssistContext, fun: &Function) -> String { 839fn format_replacement(ctx: &AssistContext, fun: &Function, indent: IndentLevel) -> String {
672 let mut buf = String::new(); 840 let ret_ty = fun.return_type(ctx);
673 841
842 let args = fun.params.iter().map(|param| param.to_arg(ctx));
843 let args = make::arg_list(args);
844 let call_expr = if fun.self_param.is_some() {
845 let self_arg = make::expr_path(make_path_from_text("self"));
846 make::expr_method_call(self_arg, &fun.name, args)
847 } else {
848 let func = make::expr_path(make_path_from_text(&fun.name));
849 make::expr_call(func, args)
850 };
851
852 let handler = FlowHandler::from_ret_ty(fun, &ret_ty);
853
854 let expr = handler.make_expr(call_expr).indent(indent);
855
856 let mut buf = String::new();
674 match fun.vars_defined_in_body_and_outlive.as_slice() { 857 match fun.vars_defined_in_body_and_outlive.as_slice() {
675 [] => {} 858 [] => {}
676 [var] => format_to!(buf, "let {} = ", var.name(ctx.db()).unwrap()), 859 [var] => format_to!(buf, "let {} = ", var.name(ctx.db()).unwrap()),
@@ -683,34 +866,123 @@ fn format_replacement(ctx: &AssistContext, fun: &Function) -> String {
683 buf.push_str(") = "); 866 buf.push_str(") = ");
684 } 867 }
685 } 868 }
686 869 format_to!(buf, "{}", expr);
687 if fun.self_param.is_some() { 870 if fun.ret_ty.is_unit()
688 format_to!(buf, "self."); 871 && (!fun.vars_defined_in_body_and_outlive.is_empty() || !expr.is_block_like())
689 } 872 {
690 format_to!(buf, "{}(", fun.name); 873 buf.push(';');
691 format_arg_list_to(&mut buf, fun, ctx);
692 format_to!(buf, ")");
693
694 if fun.ret_ty.is_unit() {
695 format_to!(buf, ";");
696 } 874 }
697
698 buf 875 buf
699} 876}
700 877
701fn format_arg_list_to(buf: &mut String, fun: &Function, ctx: &AssistContext) { 878enum FlowHandler {
702 let mut it = fun.params.iter(); 879 None,
703 if let Some(param) = it.next() { 880 If { action: FlowKind },
704 format_arg_to(buf, ctx, param); 881 IfOption { action: FlowKind },
882 MatchOption { none: FlowKind },
883 MatchResult { err: FlowKind },
884}
885
886impl FlowHandler {
887 fn from_ret_ty(fun: &Function, ret_ty: &FunType) -> FlowHandler {
888 match &fun.control_flow.kind {
889 None => FlowHandler::None,
890 Some(flow_kind) => {
891 let action = flow_kind.clone();
892 if *ret_ty == FunType::Unit {
893 match flow_kind {
894 FlowKind::Return | FlowKind::Break | FlowKind::Continue => {
895 FlowHandler::If { action }
896 }
897 FlowKind::ReturnValue(_) | FlowKind::BreakValue(_) => {
898 FlowHandler::IfOption { action }
899 }
900 }
901 } else {
902 match flow_kind {
903 FlowKind::Return | FlowKind::Break | FlowKind::Continue => {
904 FlowHandler::MatchOption { none: action }
905 }
906 FlowKind::ReturnValue(_) | FlowKind::BreakValue(_) => {
907 FlowHandler::MatchResult { err: action }
908 }
909 }
910 }
911 }
912 }
705 } 913 }
706 for param in it { 914
707 buf.push_str(", "); 915 fn make_expr(&self, call_expr: ast::Expr) -> ast::Expr {
708 format_arg_to(buf, ctx, param); 916 match self {
917 FlowHandler::None => call_expr,
918 FlowHandler::If { action } => {
919 let action = action.make_expr(None);
920 let stmt = make::expr_stmt(action);
921 let block = make::block_expr(iter::once(stmt.into()), None);
922 let condition = make::condition(call_expr, None);
923 make::expr_if(condition, block, None)
924 }
925 FlowHandler::IfOption { action } => {
926 let path = make_path_from_text("Some");
927 let value_pat = make::ident_pat(make::name("value"));
928 let pattern = make::tuple_struct_pat(path, iter::once(value_pat.into()));
929 let cond = make::condition(call_expr, Some(pattern.into()));
930 let value = make::expr_path(make_path_from_text("value"));
931 let action_expr = action.make_expr(Some(value));
932 let action_stmt = make::expr_stmt(action_expr);
933 let then = make::block_expr(iter::once(action_stmt.into()), None);
934 make::expr_if(cond, then, None)
935 }
936 FlowHandler::MatchOption { none } => {
937 let some_name = "value";
938
939 let some_arm = {
940 let path = make_path_from_text("Some");
941 let value_pat = make::ident_pat(make::name(some_name));
942 let pat = make::tuple_struct_pat(path, iter::once(value_pat.into()));
943 let value = make::expr_path(make_path_from_text(some_name));
944 make::match_arm(iter::once(pat.into()), value)
945 };
946 let none_arm = {
947 let path = make_path_from_text("None");
948 let pat = make::path_pat(path);
949 make::match_arm(iter::once(pat), none.make_expr(None))
950 };
951 let arms = make::match_arm_list(vec![some_arm, none_arm]);
952 make::expr_match(call_expr, arms)
953 }
954 FlowHandler::MatchResult { err } => {
955 let ok_name = "value";
956 let err_name = "value";
957
958 let ok_arm = {
959 let path = make_path_from_text("Ok");
960 let value_pat = make::ident_pat(make::name(ok_name));
961 let pat = make::tuple_struct_pat(path, iter::once(value_pat.into()));
962 let value = make::expr_path(make_path_from_text(ok_name));
963 make::match_arm(iter::once(pat.into()), value)
964 };
965 let err_arm = {
966 let path = make_path_from_text("Err");
967 let value_pat = make::ident_pat(make::name(err_name));
968 let pat = make::tuple_struct_pat(path, iter::once(value_pat.into()));
969 let value = make::expr_path(make_path_from_text(err_name));
970 make::match_arm(iter::once(pat.into()), err.make_expr(Some(value)))
971 };
972 let arms = make::match_arm_list(vec![ok_arm, err_arm]);
973 make::expr_match(call_expr, arms)
974 }
975 }
709 } 976 }
710} 977}
711 978
712fn format_arg_to(buf: &mut String, ctx: &AssistContext, param: &Param) { 979fn make_path_from_text(text: &str) -> ast::Path {
713 format_to!(buf, "{}{}", param.value_prefix(), param.var.name(ctx.db()).unwrap()); 980 make::path_unqualified(make::path_segment(make::name_ref(text)))
981}
982
983fn path_expr_from_local(ctx: &AssistContext, var: Local) -> ast::Expr {
984 let name = var.name(ctx.db()).unwrap().to_string();
985 make::expr_path(make_path_from_text(&name))
714} 986}
715 987
716fn format_function( 988fn format_function(
@@ -721,91 +993,99 @@ fn format_function(
721 new_indent: IndentLevel, 993 new_indent: IndentLevel,
722) -> String { 994) -> String {
723 let mut fn_def = String::new(); 995 let mut fn_def = String::new();
724 format_to!(fn_def, "\n\n{}fn $0{}(", new_indent, fun.name); 996 let params = make_param_list(ctx, module, fun);
725 format_function_param_list_to(&mut fn_def, ctx, module, fun); 997 let ret_ty = make_ret_ty(ctx, module, fun);
726 fn_def.push(')'); 998 let body = make_body(ctx, old_indent, new_indent, fun);
727 format_function_ret_to(&mut fn_def, ctx, module, fun); 999 format_to!(fn_def, "\n\n{}fn $0{}{}", new_indent, fun.name, params);
728 fn_def.push(' '); 1000 if let Some(ret_ty) = ret_ty {
729 format_function_body_to(&mut fn_def, ctx, old_indent, new_indent, fun); 1001 format_to!(fn_def, " {}", ret_ty);
1002 }
1003 format_to!(fn_def, " {}", body);
730 1004
731 fn_def 1005 fn_def
732} 1006}
733 1007
734fn format_function_param_list_to( 1008fn make_param_list(ctx: &AssistContext, module: hir::Module, fun: &Function) -> ast::ParamList {
735 fn_def: &mut String, 1009 let self_param = fun.self_param.clone();
736 ctx: &AssistContext, 1010 let params = fun.params.iter().map(|param| param.to_param(ctx, module));
737 module: hir::Module, 1011 make::param_list(self_param, params)
738 fun: &Function,
739) {
740 let mut it = fun.params.iter();
741 if let Some(self_param) = &fun.self_param {
742 format_to!(fn_def, "{}", self_param);
743 } else if let Some(param) = it.next() {
744 format_param_to(fn_def, ctx, module, param);
745 }
746 for param in it {
747 fn_def.push_str(", ");
748 format_param_to(fn_def, ctx, module, param);
749 }
750}
751
752fn format_param_to(fn_def: &mut String, ctx: &AssistContext, module: hir::Module, param: &Param) {
753 format_to!(
754 fn_def,
755 "{}{}: {}{}",
756 param.mut_pattern(),
757 param.var.name(ctx.db()).unwrap(),
758 param.type_prefix(),
759 format_type(&param.ty, ctx, module)
760 );
761} 1012}
762 1013
763fn format_function_ret_to( 1014impl FunType {
764 fn_def: &mut String, 1015 fn make_ty(&self, ctx: &AssistContext, module: hir::Module) -> ast::Type {
765 ctx: &AssistContext, 1016 match self {
766 module: hir::Module, 1017 FunType::Unit => make::ty_unit(),
767 fun: &Function, 1018 FunType::Single(ty) => make_ty(ty, ctx, module),
768) { 1019 FunType::Tuple(types) => match types.as_slice() {
769 if let Some(ty) = fun.ret_ty.as_fn_ret() { 1020 [] => {
770 format_to!(fn_def, " -> {}", format_type(ty, ctx, module)); 1021 stdx::never!("tuple type with 0 elements");
771 } else { 1022 make::ty_unit()
772 match fun.vars_defined_in_body_and_outlive.as_slice() {
773 [] => {}
774 [var] => {
775 format_to!(fn_def, " -> {}", format_type(&var.ty(ctx.db()), ctx, module));
776 }
777 [v0, vs @ ..] => {
778 format_to!(fn_def, " -> ({}", format_type(&v0.ty(ctx.db()), ctx, module));
779 for var in vs {
780 format_to!(fn_def, ", {}", format_type(&var.ty(ctx.db()), ctx, module));
781 } 1023 }
782 fn_def.push(')'); 1024 [ty] => {
783 } 1025 stdx::never!("tuple type with 1 element");
1026 make_ty(ty, ctx, module)
1027 }
1028 types => {
1029 let types = types.iter().map(|ty| make_ty(ty, ctx, module));
1030 make::ty_tuple(types)
1031 }
1032 },
784 } 1033 }
785 } 1034 }
786} 1035}
787 1036
788fn format_function_body_to( 1037fn make_ret_ty(ctx: &AssistContext, module: hir::Module, fun: &Function) -> Option<ast::RetType> {
789 fn_def: &mut String, 1038 let ty = fun.return_type(ctx);
1039 let handler = FlowHandler::from_ret_ty(fun, &ty);
1040 let ret_ty = match &handler {
1041 FlowHandler::None => {
1042 if matches!(ty, FunType::Unit) {
1043 return None;
1044 }
1045 ty.make_ty(ctx, module)
1046 }
1047 FlowHandler::If { .. } => make::ty("bool"),
1048 FlowHandler::IfOption { action } => {
1049 let handler_ty = action
1050 .expr_ty(ctx)
1051 .map(|ty| make_ty(&ty, ctx, module))
1052 .unwrap_or_else(make::ty_unit);
1053 make::ty_generic(make::name_ref("Option"), iter::once(handler_ty))
1054 }
1055 FlowHandler::MatchOption { .. } => {
1056 make::ty_generic(make::name_ref("Option"), iter::once(ty.make_ty(ctx, module)))
1057 }
1058 FlowHandler::MatchResult { err } => {
1059 let handler_ty =
1060 err.expr_ty(ctx).map(|ty| make_ty(&ty, ctx, module)).unwrap_or_else(make::ty_unit);
1061 make::ty_generic(make::name_ref("Result"), vec![ty.make_ty(ctx, module), handler_ty])
1062 }
1063 };
1064 Some(make::ret_type(ret_ty))
1065}
1066
1067fn make_body(
790 ctx: &AssistContext, 1068 ctx: &AssistContext,
791 old_indent: IndentLevel, 1069 old_indent: IndentLevel,
792 new_indent: IndentLevel, 1070 new_indent: IndentLevel,
793 fun: &Function, 1071 fun: &Function,
794) { 1072) -> ast::BlockExpr {
1073 let ret_ty = fun.return_type(ctx);
1074 let handler = FlowHandler::from_ret_ty(fun, &ret_ty);
795 let block = match &fun.body { 1075 let block = match &fun.body {
796 FunctionBody::Expr(expr) => { 1076 FunctionBody::Expr(expr) => {
797 let expr = rewrite_body_segment(ctx, &fun.params, expr.syntax()); 1077 let expr = rewrite_body_segment(ctx, &fun.params, &handler, expr.syntax());
798 let expr = ast::Expr::cast(expr).unwrap(); 1078 let expr = ast::Expr::cast(expr).unwrap();
799 let expr = expr.dedent(old_indent).indent(IndentLevel(1)); 1079 let expr = expr.dedent(old_indent).indent(IndentLevel(1));
800 let block = make::block_expr(Vec::new(), Some(expr)); 1080
801 block.indent(new_indent) 1081 make::block_expr(Vec::new(), Some(expr))
802 } 1082 }
803 FunctionBody::Span { parent, text_range } => { 1083 FunctionBody::Span { parent, text_range } => {
804 let mut elements: Vec<_> = parent 1084 let mut elements: Vec<_> = parent
805 .syntax() 1085 .syntax()
806 .children() 1086 .children()
807 .filter(|it| text_range.contains_range(it.text_range())) 1087 .filter(|it| text_range.contains_range(it.text_range()))
808 .map(|it| rewrite_body_segment(ctx, &fun.params, &it)) 1088 .map(|it| rewrite_body_segment(ctx, &fun.params, &handler, &it))
809 .collect(); 1089 .collect();
810 1090
811 let mut tail_expr = match elements.pop() { 1091 let mut tail_expr = match elements.pop() {
@@ -821,10 +1101,9 @@ fn format_function_body_to(
821 [] => {} 1101 [] => {}
822 [var] => { 1102 [var] => {
823 tail_expr = Some(path_expr_from_local(ctx, *var)); 1103 tail_expr = Some(path_expr_from_local(ctx, *var));
824 }, 1104 }
825 vars => { 1105 vars => {
826 let exprs = vars.iter() 1106 let exprs = vars.iter().map(|var| path_expr_from_local(ctx, *var));
827 .map(|var| path_expr_from_local(ctx, *var));
828 let expr = make::expr_tuple(exprs); 1107 let expr = make::expr_tuple(exprs);
829 tail_expr = Some(expr); 1108 tail_expr = Some(expr);
830 } 1109 }
@@ -839,33 +1118,70 @@ fn format_function_body_to(
839 } 1118 }
840 }); 1119 });
841 1120
1121 let body_indent = IndentLevel(1);
1122 let elements = elements.map(|stmt| stmt.dedent(old_indent).indent(body_indent));
1123 let tail_expr = tail_expr.map(|expr| expr.dedent(old_indent).indent(body_indent));
842 1124
843 let block = make::block_expr(elements, tail_expr); 1125 make::block_expr(elements, tail_expr)
844 block.dedent(old_indent).indent(new_indent)
845 } 1126 }
846 }; 1127 };
847 1128
1129 let block = match &handler {
1130 FlowHandler::None => block,
1131 FlowHandler::If { .. } => {
1132 let lit_false = ast::Literal::cast(make::tokens::literal("false").parent()).unwrap();
1133 with_tail_expr(block, lit_false.into())
1134 }
1135 FlowHandler::IfOption { .. } => {
1136 let none = make::expr_path(make_path_from_text("None"));
1137 with_tail_expr(block, none)
848 } 1138 }
849 1139 FlowHandler::MatchOption { .. } => map_tail_expr(block, |tail_expr| {
850 format_to!(fn_def, "{}", block); 1140 let some = make::expr_path(make_path_from_text("Some"));
1141 let args = make::arg_list(iter::once(tail_expr));
1142 make::expr_call(some, args)
1143 }),
1144 FlowHandler::MatchResult { .. } => map_tail_expr(block, |tail_expr| {
1145 let some = make::expr_path(make_path_from_text("Ok"));
1146 let args = make::arg_list(iter::once(tail_expr));
1147 make::expr_call(some, args)
1148 }),
1149 };
1150
1151 block.indent(new_indent)
851} 1152}
852 1153
853fn path_expr_from_local(ctx: &AssistContext, var: Local) -> ast::Expr { 1154fn map_tail_expr(block: ast::BlockExpr, f: impl FnOnce(ast::Expr) -> ast::Expr) -> ast::BlockExpr {
854 let name = var.name(ctx.db()).unwrap().to_string(); 1155 let tail_expr = match block.tail_expr() {
855 let path = make::path_unqualified(make::path_segment(make::name_ref(&name))); 1156 Some(tail_expr) => tail_expr,
856 make::expr_path(path) 1157 None => return block,
1158 };
1159 make::block_expr(block.statements(), Some(f(tail_expr)))
1160}
1161
1162fn with_tail_expr(block: ast::BlockExpr, tail_expr: ast::Expr) -> ast::BlockExpr {
1163 let stmt_tail = block.tail_expr().map(|expr| make::expr_stmt(expr).into());
1164 let stmts = block.statements().chain(stmt_tail);
1165 make::block_expr(stmts, Some(tail_expr))
857} 1166}
858 1167
859fn format_type(ty: &hir::Type, ctx: &AssistContext, module: hir::Module) -> String { 1168fn format_type(ty: &hir::Type, ctx: &AssistContext, module: hir::Module) -> String {
860 ty.display_source_code(ctx.db(), module.into()).ok().unwrap_or_else(|| "()".to_string()) 1169 ty.display_source_code(ctx.db(), module.into()).ok().unwrap_or_else(|| "()".to_string())
861} 1170}
862 1171
1172fn make_ty(ty: &hir::Type, ctx: &AssistContext, module: hir::Module) -> ast::Type {
1173 let ty_str = format_type(ty, ctx, module);
1174 make::ty(&ty_str)
1175}
1176
863fn rewrite_body_segment( 1177fn rewrite_body_segment(
864 ctx: &AssistContext, 1178 ctx: &AssistContext,
865 params: &[Param], 1179 params: &[Param],
1180 handler: &FlowHandler,
866 syntax: &SyntaxNode, 1181 syntax: &SyntaxNode,
867) -> SyntaxNode { 1182) -> SyntaxNode {
868 fix_param_usages(ctx, params, syntax) 1183 let syntax = fix_param_usages(ctx, params, syntax);
1184 update_external_control_flow(handler, &syntax)
869} 1185}
870 1186
871/// change all usages to account for added `&`/`&mut` for some params 1187/// change all usages to account for added `&`/`&mut` for some params
@@ -906,6 +1222,98 @@ fn fix_param_usages(ctx: &AssistContext, params: &[Param], syntax: &SyntaxNode)
906 rewriter.rewrite(syntax) 1222 rewriter.rewrite(syntax)
907} 1223}
908 1224
1225fn update_external_control_flow(handler: &FlowHandler, syntax: &SyntaxNode) -> SyntaxNode {
1226 let mut rewriter = SyntaxRewriter::default();
1227
1228 let mut nested_loop = None;
1229 let mut nested_scope = None;
1230 for event in syntax.preorder() {
1231 let node = match event {
1232 WalkEvent::Enter(e) => {
1233 match e.kind() {
1234 SyntaxKind::LOOP_EXPR | SyntaxKind::WHILE_EXPR | SyntaxKind::FOR_EXPR => {
1235 if nested_loop.is_none() {
1236 nested_loop = Some(e.clone());
1237 }
1238 }
1239 SyntaxKind::FN
1240 | SyntaxKind::CONST
1241 | SyntaxKind::STATIC
1242 | SyntaxKind::IMPL
1243 | SyntaxKind::MODULE => {
1244 if nested_scope.is_none() {
1245 nested_scope = Some(e.clone());
1246 }
1247 }
1248 _ => {}
1249 }
1250 e
1251 }
1252 WalkEvent::Leave(e) => {
1253 if nested_loop.as_ref() == Some(&e) {
1254 nested_loop = None;
1255 }
1256 if nested_scope.as_ref() == Some(&e) {
1257 nested_scope = None;
1258 }
1259 continue;
1260 }
1261 };
1262 if nested_scope.is_some() {
1263 continue;
1264 }
1265 let expr = match ast::Expr::cast(node) {
1266 Some(e) => e,
1267 None => continue,
1268 };
1269 match expr {
1270 ast::Expr::ReturnExpr(return_expr) if nested_scope.is_none() => {
1271 let expr = return_expr.expr();
1272 if let Some(replacement) = make_rewritten_flow(handler, expr) {
1273 rewriter.replace_ast(&return_expr.into(), &replacement);
1274 }
1275 }
1276 ast::Expr::BreakExpr(break_expr) if nested_loop.is_none() => {
1277 let expr = break_expr.expr();
1278 if let Some(replacement) = make_rewritten_flow(handler, expr) {
1279 rewriter.replace_ast(&break_expr.into(), &replacement);
1280 }
1281 }
1282 ast::Expr::ContinueExpr(continue_expr) if nested_loop.is_none() => {
1283 if let Some(replacement) = make_rewritten_flow(handler, None) {
1284 rewriter.replace_ast(&continue_expr.into(), &replacement);
1285 }
1286 }
1287 _ => {
1288 // do nothing
1289 }
1290 }
1291 }
1292
1293 rewriter.rewrite(syntax)
1294}
1295
1296fn make_rewritten_flow(handler: &FlowHandler, arg_expr: Option<ast::Expr>) -> Option<ast::Expr> {
1297 let value = match handler {
1298 FlowHandler::None => return None,
1299 FlowHandler::If { .. } => {
1300 ast::Literal::cast(make::tokens::literal("true").parent()).unwrap().into()
1301 }
1302 FlowHandler::IfOption { .. } => {
1303 let expr = arg_expr.unwrap_or_else(|| make::expr_tuple(Vec::new()));
1304 let args = make::arg_list(iter::once(expr));
1305 make::expr_call(make::expr_path(make_path_from_text("Some")), args)
1306 }
1307 FlowHandler::MatchOption { .. } => make::expr_path(make_path_from_text("None")),
1308 FlowHandler::MatchResult { .. } => {
1309 let expr = arg_expr.unwrap_or_else(|| make::expr_tuple(Vec::new()));
1310 let args = make::arg_list(iter::once(expr));
1311 make::expr_call(make::expr_path(make_path_from_text("Err")), args)
1312 }
1313 };
1314 Some(make::expr_return(Some(value)))
1315}
1316
909#[cfg(test)] 1317#[cfg(test)]
910mod tests { 1318mod tests {
911 use crate::tests::{check_assist, check_assist_not_applicable}; 1319 use crate::tests::{check_assist, check_assist_not_applicable};
@@ -2074,6 +2482,66 @@ fn $0fun_name(c: &Counter) {
2074 } 2482 }
2075 2483
2076 #[test] 2484 #[test]
2485 fn copy_used_after() {
2486 check_assist(
2487 extract_function,
2488 r##"
2489#[lang = "copy"]
2490pub trait Copy {}
2491impl Copy for i32 {}
2492fn foo() {
2493 let n = 0;
2494 $0let m = n;$0
2495 let k = n;
2496}"##,
2497 r##"
2498#[lang = "copy"]
2499pub trait Copy {}
2500impl Copy for i32 {}
2501fn foo() {
2502 let n = 0;
2503 fun_name(n);
2504 let k = n;
2505}
2506
2507fn $0fun_name(n: i32) {
2508 let m = n;
2509}"##,
2510 )
2511 }
2512
2513 #[test]
2514 fn copy_custom_used_after() {
2515 check_assist(
2516 extract_function,
2517 r##"
2518#[lang = "copy"]
2519pub trait Copy {}
2520struct Counter(i32);
2521impl Copy for Counter {}
2522fn foo() {
2523 let c = Counter(0);
2524 $0let n = c.0;$0
2525 let m = c.0;
2526}"##,
2527 r##"
2528#[lang = "copy"]
2529pub trait Copy {}
2530struct Counter(i32);
2531impl Copy for Counter {}
2532fn foo() {
2533 let c = Counter(0);
2534 fun_name(c);
2535 let m = c.0;
2536}
2537
2538fn $0fun_name(c: Counter) {
2539 let n = c.0;
2540}"##,
2541 );
2542 }
2543
2544 #[test]
2077 fn indented_stmts() { 2545 fn indented_stmts() {
2078 check_assist( 2546 check_assist(
2079 extract_function, 2547 extract_function,
@@ -2134,4 +2602,441 @@ mod bar {
2134}", 2602}",
2135 ); 2603 );
2136 } 2604 }
2605
2606 #[test]
2607 fn break_loop() {
2608 check_assist(
2609 extract_function,
2610 r##"
2611enum Option<T> {
2612 #[lang = "None"] None,
2613 #[lang = "Some"] Some(T),
2614}
2615use Option::*;
2616fn foo() {
2617 loop {
2618 let n = 1;
2619 $0let m = n + 1;
2620 break;
2621 let k = 2;$0
2622 let h = 1 + k;
2623 }
2624}"##,
2625 r##"
2626enum Option<T> {
2627 #[lang = "None"] None,
2628 #[lang = "Some"] Some(T),
2629}
2630use Option::*;
2631fn foo() {
2632 loop {
2633 let n = 1;
2634 let k = match fun_name(n) {
2635 Some(value) => value,
2636 None => break,
2637 };
2638 let h = 1 + k;
2639 }
2640}
2641
2642fn $0fun_name(n: i32) -> Option<i32> {
2643 let m = n + 1;
2644 return None;
2645 let k = 2;
2646 Some(k)
2647}"##,
2648 );
2649 }
2650
2651 #[test]
2652 fn return_to_parent() {
2653 check_assist(
2654 extract_function,
2655 r##"
2656#[lang = "copy"]
2657pub trait Copy {}
2658impl Copy for i32 {}
2659enum Result<T, E> {
2660 #[lang = "Ok"] Ok(T),
2661 #[lang = "Err"] Err(E),
2662}
2663use Result::*;
2664fn foo() -> i64 {
2665 let n = 1;
2666 $0let m = n + 1;
2667 return 1;
2668 let k = 2;$0
2669 (n + k) as i64
2670}"##,
2671 r##"
2672#[lang = "copy"]
2673pub trait Copy {}
2674impl Copy for i32 {}
2675enum Result<T, E> {
2676 #[lang = "Ok"] Ok(T),
2677 #[lang = "Err"] Err(E),
2678}
2679use Result::*;
2680fn foo() -> i64 {
2681 let n = 1;
2682 let k = match fun_name(n) {
2683 Ok(value) => value,
2684 Err(value) => return value,
2685 };
2686 (n + k) as i64
2687}
2688
2689fn $0fun_name(n: i32) -> Result<i32, i64> {
2690 let m = n + 1;
2691 return Err(1);
2692 let k = 2;
2693 Ok(k)
2694}"##,
2695 );
2696 }
2697
2698 #[test]
2699 fn break_and_continue() {
2700 mark::check!(external_control_flow_break_and_continue);
2701 check_assist_not_applicable(
2702 extract_function,
2703 r##"
2704fn foo() {
2705 loop {
2706 let n = 1;
2707 $0let m = n + 1;
2708 break;
2709 let k = 2;
2710 continue;
2711 let k = k + 1;$0
2712 let r = n + k;
2713 }
2714}"##,
2715 );
2716 }
2717
2718 #[test]
2719 fn return_and_break() {
2720 mark::check!(external_control_flow_return_and_bc);
2721 check_assist_not_applicable(
2722 extract_function,
2723 r##"
2724fn foo() {
2725 loop {
2726 let n = 1;
2727 $0let m = n + 1;
2728 break;
2729 let k = 2;
2730 return;
2731 let k = k + 1;$0
2732 let r = n + k;
2733 }
2734}"##,
2735 );
2736 }
2737
2738 #[test]
2739 fn break_loop_with_if() {
2740 check_assist(
2741 extract_function,
2742 r##"
2743fn foo() {
2744 loop {
2745 let mut n = 1;
2746 $0let m = n + 1;
2747 break;
2748 n += m;$0
2749 let h = 1 + n;
2750 }
2751}"##,
2752 r##"
2753fn foo() {
2754 loop {
2755 let mut n = 1;
2756 if fun_name(&mut n) {
2757 break;
2758 }
2759 let h = 1 + n;
2760 }
2761}
2762
2763fn $0fun_name(n: &mut i32) -> bool {
2764 let m = *n + 1;
2765 return true;
2766 *n += m;
2767 false
2768}"##,
2769 );
2770 }
2771
2772 #[test]
2773 fn break_loop_nested() {
2774 check_assist(
2775 extract_function,
2776 r##"
2777fn foo() {
2778 loop {
2779 let mut n = 1;
2780 $0let m = n + 1;
2781 if m == 42 {
2782 break;
2783 }$0
2784 let h = 1;
2785 }
2786}"##,
2787 r##"
2788fn foo() {
2789 loop {
2790 let mut n = 1;
2791 if fun_name(n) {
2792 break;
2793 }
2794 let h = 1;
2795 }
2796}
2797
2798fn $0fun_name(n: i32) -> bool {
2799 let m = n + 1;
2800 if m == 42 {
2801 return true;
2802 }
2803 false
2804}"##,
2805 );
2806 }
2807
2808 #[test]
2809 fn return_from_nested_loop() {
2810 check_assist(
2811 extract_function,
2812 r##"
2813fn foo() {
2814 loop {
2815 let n = 1;
2816 $0
2817 let k = 1;
2818 loop {
2819 return;
2820 }
2821 let m = k + 1;$0
2822 let h = 1 + m;
2823 }
2824}"##,
2825 r##"
2826fn foo() {
2827 loop {
2828 let n = 1;
2829 let m = match fun_name() {
2830 Some(value) => value,
2831 None => return,
2832 };
2833 let h = 1 + m;
2834 }
2835}
2836
2837fn $0fun_name() -> Option<i32> {
2838 let k = 1;
2839 loop {
2840 return None;
2841 }
2842 let m = k + 1;
2843 Some(m)
2844}"##,
2845 );
2846 }
2847
2848 #[test]
2849 fn break_from_nested_loop() {
2850 check_assist(
2851 extract_function,
2852 r##"
2853fn foo() {
2854 loop {
2855 let n = 1;
2856 $0let k = 1;
2857 loop {
2858 break;
2859 }
2860 let m = k + 1;$0
2861 let h = 1 + m;
2862 }
2863}"##,
2864 r##"
2865fn foo() {
2866 loop {
2867 let n = 1;
2868 let m = fun_name();
2869 let h = 1 + m;
2870 }
2871}
2872
2873fn $0fun_name() -> i32 {
2874 let k = 1;
2875 loop {
2876 break;
2877 }
2878 let m = k + 1;
2879 m
2880}"##,
2881 );
2882 }
2883
2884 #[test]
2885 fn break_from_nested_and_outer_loops() {
2886 check_assist(
2887 extract_function,
2888 r##"
2889fn foo() {
2890 loop {
2891 let n = 1;
2892 $0let k = 1;
2893 loop {
2894 break;
2895 }
2896 if k == 42 {
2897 break;
2898 }
2899 let m = k + 1;$0
2900 let h = 1 + m;
2901 }
2902}"##,
2903 r##"
2904fn foo() {
2905 loop {
2906 let n = 1;
2907 let m = match fun_name() {
2908 Some(value) => value,
2909 None => break,
2910 };
2911 let h = 1 + m;
2912 }
2913}
2914
2915fn $0fun_name() -> Option<i32> {
2916 let k = 1;
2917 loop {
2918 break;
2919 }
2920 if k == 42 {
2921 return None;
2922 }
2923 let m = k + 1;
2924 Some(m)
2925}"##,
2926 );
2927 }
2928
2929 #[test]
2930 fn return_from_nested_fn() {
2931 check_assist(
2932 extract_function,
2933 r##"
2934fn foo() {
2935 loop {
2936 let n = 1;
2937 $0let k = 1;
2938 fn test() {
2939 return;
2940 }
2941 let m = k + 1;$0
2942 let h = 1 + m;
2943 }
2944}"##,
2945 r##"
2946fn foo() {
2947 loop {
2948 let n = 1;
2949 let m = fun_name();
2950 let h = 1 + m;
2951 }
2952}
2953
2954fn $0fun_name() -> i32 {
2955 let k = 1;
2956 fn test() {
2957 return;
2958 }
2959 let m = k + 1;
2960 m
2961}"##,
2962 );
2963 }
2964
2965 #[test]
2966 fn break_with_value() {
2967 check_assist(
2968 extract_function,
2969 r##"
2970fn foo() -> i32 {
2971 loop {
2972 let n = 1;
2973 $0let k = 1;
2974 if k == 42 {
2975 break 3;
2976 }
2977 let m = k + 1;$0
2978 let h = 1;
2979 }
2980}"##,
2981 r##"
2982fn foo() -> i32 {
2983 loop {
2984 let n = 1;
2985 if let Some(value) = fun_name() {
2986 break value;
2987 }
2988 let h = 1;
2989 }
2990}
2991
2992fn $0fun_name() -> Option<i32> {
2993 let k = 1;
2994 if k == 42 {
2995 return Some(3);
2996 }
2997 let m = k + 1;
2998 None
2999}"##,
3000 );
3001 }
3002
3003 #[test]
3004 fn break_with_value_and_return() {
3005 check_assist(
3006 extract_function,
3007 r##"
3008fn foo() -> i64 {
3009 loop {
3010 let n = 1;
3011 $0
3012 let k = 1;
3013 if k == 42 {
3014 break 3;
3015 }
3016 let m = k + 1;$0
3017 let h = 1 + m;
3018 }
3019}"##,
3020 r##"
3021fn foo() -> i64 {
3022 loop {
3023 let n = 1;
3024 let m = match fun_name() {
3025 Ok(value) => value,
3026 Err(value) => break value,
3027 };
3028 let h = 1 + m;
3029 }
3030}
3031
3032fn $0fun_name() -> Result<i32, i64> {
3033 let k = 1;
3034 if k == 42 {
3035 return Err(3);
3036 }
3037 let m = k + 1;
3038 Ok(m)
3039}"##,
3040 );
3041 }
2137} 3042}
diff --git a/crates/assists/src/handlers/generate_function.rs b/crates/assists/src/handlers/generate_function.rs
index 1805c1dfd..959824981 100644
--- a/crates/assists/src/handlers/generate_function.rs
+++ b/crates/assists/src/handlers/generate_function.rs
@@ -215,8 +215,11 @@ fn fn_args(
215 }); 215 });
216 } 216 }
217 deduplicate_arg_names(&mut arg_names); 217 deduplicate_arg_names(&mut arg_names);
218 let params = arg_names.into_iter().zip(arg_types).map(|(name, ty)| make::param(name, ty)); 218 let params = arg_names
219 Some((None, make::param_list(params))) 219 .into_iter()
220 .zip(arg_types)
221 .map(|(name, ty)| make::param(make::ident_pat(make::name(&name)).into(), make::ty(&ty)));
222 Some((None, make::param_list(None, params)))
220} 223}
221 224
222/// Makes duplicate argument names unique by appending incrementing numbers. 225/// Makes duplicate argument names unique by appending incrementing numbers.
diff --git a/crates/assists/src/tests.rs b/crates/assists/src/tests.rs
index 5b9992f15..720f561a1 100644
--- a/crates/assists/src/tests.rs
+++ b/crates/assists/src/tests.rs
@@ -195,6 +195,7 @@ fn assist_order_if_expr() {
195 let assists = Assist::get(&db, &TEST_CONFIG, false, frange); 195 let assists = Assist::get(&db, &TEST_CONFIG, false, frange);
196 let mut assists = assists.iter(); 196 let mut assists = assists.iter();
197 197
198 assert_eq!(assists.next().expect("expected assist").label, "Extract into function");
198 assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); 199 assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
199 assert_eq!(assists.next().expect("expected assist").label, "Replace with match"); 200 assert_eq!(assists.next().expect("expected assist").label, "Replace with match");
200} 201}
@@ -220,6 +221,7 @@ fn assist_filter_works() {
220 let assists = Assist::get(&db, &cfg, false, frange); 221 let assists = Assist::get(&db, &cfg, false, frange);
221 let mut assists = assists.iter(); 222 let mut assists = assists.iter();
222 223
224 assert_eq!(assists.next().expect("expected assist").label, "Extract into function");
223 assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); 225 assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
224 assert_eq!(assists.next().expect("expected assist").label, "Replace with match"); 226 assert_eq!(assists.next().expect("expected assist").label, "Replace with match");
225 } 227 }
@@ -228,9 +230,10 @@ fn assist_filter_works() {
228 let mut cfg = TEST_CONFIG; 230 let mut cfg = TEST_CONFIG;
229 cfg.allowed = Some(vec![AssistKind::RefactorExtract]); 231 cfg.allowed = Some(vec![AssistKind::RefactorExtract]);
230 let assists = Assist::get(&db, &cfg, false, frange); 232 let assists = Assist::get(&db, &cfg, false, frange);
231 assert_eq!(assists.len(), 1); 233 assert_eq!(assists.len(), 2);
232 234
233 let mut assists = assists.iter(); 235 let mut assists = assists.iter();
236 assert_eq!(assists.next().expect("expected assist").label, "Extract into function");
234 assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); 237 assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
235 } 238 }
236 239