aboutsummaryrefslogtreecommitdiff
path: root/crates/assists/src/handlers
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-02-16 14:01:09 +0000
committerGitHub <[email protected]>2021-02-16 14:01:09 +0000
commit88e8b0a5fa17075475c75941932056f7289c5bcf (patch)
tree84889038ca64a9ee4775ecf5605ed0213b90a0b7 /crates/assists/src/handlers
parent00d5a9563af9c84cdea9d029a9583be0513cc459 (diff)
parent37a8cec6386d57e76b3067af48c78867aa9922ed (diff)
Merge #7620
7620: Support control flow in `extract_function` assist r=matklad a=cpud36 Support `return`ing from outer function, `break`ing and `continue`ing outer loops when extracting function. # Example Transforms ```rust fn foo() -> i32 { let items = [1,2,3]; let mut sum = 0; for &item in items { <|>if item == 42 { break; }<|> sum += item; } sum } ``` Into ```rust fn foo() -> i32 { let items = [1,2,3]; let mut sum = 0; for &item in items { if fun_name(item) { break; } sum += item; } sum } fn fun_name(item: i32) -> bool { if item == 42 { return true; } false } ``` ![add_explicit_type_infer_type](https://user-images.githubusercontent.com/4218373/107544222-0fadf280-6bdb-11eb-9625-ed6194ba92c0.gif) # Features Supported variants - break and function does not return => uses `bool` and plain if - break and function does return => uses `Option<T>` and matches on it - break with value and function does not return => uses `Option<T>` and if let - break with value and function does return => uses `Result<T, U>` and matches on t - same for `return` and `continue`(but we can't continue with value) Assist does handle nested loops and nested items(like functions, modules, impls) Try `expr?` operator is allowed together with `return Err(_)` and `return None`. `return expr` is not allowed. # Not supported ## Mixing `return` with `break` or `continue` If we have e.g. a `return` and a `break` in the selected code, it is unclear what the produced code should look like. We can try `Result<T, Option<U>>` or something like that, but it isn't idiomatic, nor it is established. Otherwise, implementation is relatively simple. ## `break` with label Not sure how to handle different labels for multiple `break`s. [edit] implemented try `expr?` Co-authored-by: Vladyslav Katasonov <[email protected]>
Diffstat (limited to 'crates/assists/src/handlers')
-rw-r--r--crates/assists/src/handlers/early_return.rs2
-rw-r--r--crates/assists/src/handlers/extract_function.rs1687
-rw-r--r--crates/assists/src/handlers/generate_function.rs7
3 files changed, 1459 insertions, 237 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 d876eabca..9f34cc725 100644
--- a/crates/assists/src/handlers/extract_function.rs
+++ b/crates/assists/src/handlers/extract_function.rs
@@ -1,3 +1,6 @@
1use std::iter;
2
3use ast::make;
1use either::Either; 4use either::Either;
2use hir::{HirDisplay, Local}; 5use hir::{HirDisplay, Local};
3use ide_db::{ 6use ide_db::{
@@ -13,9 +16,9 @@ use syntax::{
13 edit::{AstNodeEdit, IndentLevel}, 16 edit::{AstNodeEdit, IndentLevel},
14 AstNode, 17 AstNode,
15 }, 18 },
16 AstToken, Direction, SyntaxElement, 19 SyntaxElement,
17 SyntaxKind::{self, BLOCK_EXPR, BREAK_EXPR, COMMENT, PATH_EXPR, RETURN_EXPR}, 20 SyntaxKind::{self, BLOCK_EXPR, BREAK_EXPR, COMMENT, PATH_EXPR, RETURN_EXPR},
18 SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, T, 21 SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, WalkEvent, T,
19}; 22};
20use test_utils::mark; 23use test_utils::mark;
21 24
@@ -81,11 +84,9 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
81 // 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
82 return None; 85 return None;
83 } 86 }
87 let control_flow = external_control_flow(ctx, &body)?;
84 88
85 let target_range = match &body { 89 let target_range = body.text_range();
86 FunctionBody::Expr(expr) => expr.syntax().text_range(),
87 FunctionBody::Span { .. } => ctx.frange.range,
88 };
89 90
90 acc.add( 91 acc.add(
91 AssistId("extract_function", crate::AssistKind::RefactorExtract), 92 AssistId("extract_function", crate::AssistKind::RefactorExtract),
@@ -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,140 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
115 ) 117 )
116} 118}
117 119
120fn external_control_flow(ctx: &AssistContext, 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 let kind = match (try_expr, ret_expr, break_expr, continue_expr) {
184 (Some(e), None, None, None) => {
185 let func = e.syntax().ancestors().find_map(ast::Fn::cast)?;
186 let def = ctx.sema.to_def(&func)?;
187 let ret_ty = def.ret_type(ctx.db());
188 let kind = try_kind_of_ty(ret_ty, ctx)?;
189
190 Some(FlowKind::Try { kind })
191 }
192 (Some(_), Some(r), None, None) => match r.expr() {
193 Some(expr) => {
194 if let Some(kind) = expr_err_kind(&expr, ctx) {
195 Some(FlowKind::TryReturn { expr, kind })
196 } else {
197 mark::hit!(external_control_flow_try_and_return_non_err);
198 return None;
199 }
200 }
201 None => return None,
202 },
203 (Some(_), _, _, _) => {
204 mark::hit!(external_control_flow_try_and_bc);
205 return None;
206 }
207 (None, Some(r), None, None) => match r.expr() {
208 Some(expr) => Some(FlowKind::ReturnValue(expr)),
209 None => Some(FlowKind::Return),
210 },
211 (None, Some(_), _, _) => {
212 mark::hit!(external_control_flow_return_and_bc);
213 return None;
214 }
215 (None, None, Some(_), Some(_)) => {
216 mark::hit!(external_control_flow_break_and_continue);
217 return None;
218 }
219 (None, None, Some(b), None) => match b.expr() {
220 Some(expr) => Some(FlowKind::BreakValue(expr)),
221 None => Some(FlowKind::Break),
222 },
223 (None, None, None, Some(_)) => Some(FlowKind::Continue),
224 (None, None, None, None) => None,
225 };
226
227 Some(ControlFlow { kind })
228}
229
230/// Checks is expr is `Err(_)` or `None`
231fn expr_err_kind(expr: &ast::Expr, ctx: &AssistContext) -> Option<TryKind> {
232 let func_name = match expr {
233 ast::Expr::CallExpr(call_expr) => call_expr.expr()?,
234 ast::Expr::PathExpr(_) => expr.clone(),
235 _ => return None,
236 };
237 let text = func_name.syntax().text();
238
239 if text == "Err" {
240 Some(TryKind::Result { ty: ctx.sema.type_of_expr(expr)? })
241 } else if text == "None" {
242 Some(TryKind::Option)
243 } else {
244 None
245 }
246}
247
118#[derive(Debug)] 248#[derive(Debug)]
119struct Function { 249struct Function {
120 name: String, 250 name: String,
121 self_param: Option<ast::SelfParam>, 251 self_param: Option<ast::SelfParam>,
122 params: Vec<Param>, 252 params: Vec<Param>,
253 control_flow: ControlFlow,
123 ret_ty: RetType, 254 ret_ty: RetType,
124 body: FunctionBody, 255 body: FunctionBody,
125 vars_defined_in_body_and_outlive: Vec<Local>, 256 vars_defined_in_body_and_outlive: Vec<Local>,
@@ -134,6 +265,11 @@ struct Param {
134 is_copy: bool, 265 is_copy: bool,
135} 266}
136 267
268#[derive(Debug)]
269struct ControlFlow {
270 kind: Option<FlowKind>,
271}
272
137#[derive(Debug, Clone, Copy, PartialEq, Eq)] 273#[derive(Debug, Clone, Copy, PartialEq, Eq)]
138enum ParamKind { 274enum ParamKind {
139 Value, 275 Value,
@@ -142,6 +278,30 @@ enum ParamKind {
142 MutRef, 278 MutRef,
143} 279}
144 280
281#[derive(Debug, Eq, PartialEq)]
282enum FunType {
283 Unit,
284 Single(hir::Type),
285 Tuple(Vec<hir::Type>),
286}
287
288impl Function {
289 fn return_type(&self, ctx: &AssistContext) -> FunType {
290 match &self.ret_ty {
291 RetType::Expr(ty) if ty.is_unit() => FunType::Unit,
292 RetType::Expr(ty) => FunType::Single(ty.clone()),
293 RetType::Stmt => match self.vars_defined_in_body_and_outlive.as_slice() {
294 [] => FunType::Unit,
295 [var] => FunType::Single(var.ty(ctx.db())),
296 vars => {
297 let types = vars.iter().map(|v| v.ty(ctx.db())).collect();
298 FunType::Tuple(types)
299 }
300 },
301 }
302 }
303}
304
145impl ParamKind { 305impl ParamKind {
146 fn is_ref(&self) -> bool { 306 fn is_ref(&self) -> bool {
147 matches!(self, ParamKind::SharedRef | ParamKind::MutRef) 307 matches!(self, ParamKind::SharedRef | ParamKind::MutRef)
@@ -158,30 +318,121 @@ impl Param {
158 } 318 }
159 } 319 }
160 320
161 fn value_prefix(&self) -> &'static str { 321 fn to_arg(&self, ctx: &AssistContext) -> ast::Expr {
322 let var = path_expr_from_local(ctx, self.var);
162 match self.kind() { 323 match self.kind() {
163 ParamKind::Value | ParamKind::MutValue => "", 324 ParamKind::Value | ParamKind::MutValue => var,
164 ParamKind::SharedRef => "&", 325 ParamKind::SharedRef => make::expr_ref(var, false),
165 ParamKind::MutRef => "&mut ", 326 ParamKind::MutRef => make::expr_ref(var, true),
166 } 327 }
167 } 328 }
168 329
169 fn type_prefix(&self) -> &'static str { 330 fn to_param(&self, ctx: &AssistContext, module: hir::Module) -> ast::Param {
170 match self.kind() { 331 let var = self.var.name(ctx.db()).unwrap().to_string();
171 ParamKind::Value | ParamKind::MutValue => "", 332 let var_name = make::name(&var);
172 ParamKind::SharedRef => "&", 333 let pat = match self.kind() {
173 ParamKind::MutRef => "&mut ", 334 ParamKind::MutValue => make::ident_mut_pat(var_name),
335 ParamKind::Value | ParamKind::SharedRef | ParamKind::MutRef => {
336 make::ident_pat(var_name)
337 }
338 };
339
340 let ty = make_ty(&self.ty, ctx, module);
341 let ty = match self.kind() {
342 ParamKind::Value | ParamKind::MutValue => ty,
343 ParamKind::SharedRef => make::ty_ref(ty, false),
344 ParamKind::MutRef => make::ty_ref(ty, true),
345 };
346
347 make::param(pat.into(), ty)
348 }
349}
350
351/// Control flow that is exported from extracted function
352///
353/// E.g.:
354/// ```rust,no_run
355/// loop {
356/// $0
357/// if 42 == 42 {
358/// break;
359/// }
360/// $0
361/// }
362/// ```
363#[derive(Debug, Clone)]
364enum FlowKind {
365 /// Return without value (`return;`)
366 Return,
367 /// Return with value (`return $expr;`)
368 ReturnValue(ast::Expr),
369 Try {
370 kind: TryKind,
371 },
372 TryReturn {
373 expr: ast::Expr,
374 kind: TryKind,
375 },
376 /// Break without value (`return;`)
377 Break,
378 /// Break with value (`break $expr;`)
379 BreakValue(ast::Expr),
380 /// Continue
381 Continue,
382}
383
384#[derive(Debug, Clone)]
385enum TryKind {
386 Option,
387 Result { ty: hir::Type },
388}
389
390impl FlowKind {
391 fn make_result_handler(&self, expr: Option<ast::Expr>) -> ast::Expr {
392 match self {
393 FlowKind::Return | FlowKind::ReturnValue(_) => make::expr_return(expr),
394 FlowKind::Break | FlowKind::BreakValue(_) => make::expr_break(expr),
395 FlowKind::Try { .. } | FlowKind::TryReturn { .. } => {
396 stdx::never!("cannot have result handler with try");
397 expr.unwrap_or_else(|| make::expr_return(None))
398 }
399 FlowKind::Continue => {
400 stdx::always!(expr.is_none(), "continue with value is not possible");
401 make::expr_continue()
402 }
174 } 403 }
175 } 404 }
176 405
177 fn mut_pattern(&self) -> &'static str { 406 fn expr_ty(&self, ctx: &AssistContext) -> Option<hir::Type> {
178 match self.kind() { 407 match self {
179 ParamKind::MutValue => "mut ", 408 FlowKind::ReturnValue(expr)
180 _ => "", 409 | FlowKind::BreakValue(expr)
410 | FlowKind::TryReturn { expr, .. } => ctx.sema.type_of_expr(expr),
411 FlowKind::Try { .. } => {
412 stdx::never!("try does not have defined expr_ty");
413 None
414 }
415 FlowKind::Return | FlowKind::Break | FlowKind::Continue => None,
181 } 416 }
182 } 417 }
183} 418}
184 419
420fn try_kind_of_ty(ty: hir::Type, ctx: &AssistContext) -> Option<TryKind> {
421 if ty.is_unknown() {
422 // We favour Result for `expr?`
423 return Some(TryKind::Result { ty });
424 }
425 let adt = ty.as_adt()?;
426 let name = adt.name(ctx.db());
427 // FIXME: use lang items to determine if it is std type or user defined
428 // E.g. if user happens to define type named `Option`, we would have false positive
429 match name.to_string().as_str() {
430 "Option" => Some(TryKind::Option),
431 "Result" => Some(TryKind::Result { ty }),
432 _ => None,
433 }
434}
435
185#[derive(Debug)] 436#[derive(Debug)]
186enum RetType { 437enum RetType {
187 Expr(hir::Type), 438 Expr(hir::Type),
@@ -195,21 +446,13 @@ impl RetType {
195 RetType::Stmt => true, 446 RetType::Stmt => true,
196 } 447 }
197 } 448 }
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} 449}
207 450
208/// Semantically same as `ast::Expr`, but preserves identity when using only part of the Block 451/// Semantically same as `ast::Expr`, but preserves identity when using only part of the Block
209#[derive(Debug)] 452#[derive(Debug)]
210enum FunctionBody { 453enum FunctionBody {
211 Expr(ast::Expr), 454 Expr(ast::Expr),
212 Span { elements: Vec<SyntaxElement>, leading_indent: String }, 455 Span { parent: ast::BlockExpr, text_range: TextRange },
213} 456}
214 457
215impl FunctionBody { 458impl FunctionBody {
@@ -226,58 +469,28 @@ impl FunctionBody {
226 } 469 }
227 } 470 }
228 471
229 fn from_range(node: &SyntaxNode, range: TextRange) -> Option<FunctionBody> { 472 fn from_range(node: SyntaxNode, text_range: TextRange) -> Option<FunctionBody> {
230 let mut first = node.token_at_offset(range.start()).left_biased()?; 473 let block = ast::BlockExpr::cast(node)?;
231 let last = node.token_at_offset(range.end()).right_biased()?; 474 Some(Self::Span { parent: block, text_range })
232
233 let mut leading_indent = String::new();
234
235 let leading_trivia = first
236 .siblings_with_tokens(Direction::Prev)
237 .skip(1)
238 .take_while(|e| e.kind() == SyntaxKind::WHITESPACE && e.as_token().is_some());
239
240 for e in leading_trivia {
241 let token = e.as_token().unwrap();
242 let text = token.text();
243 match text.rfind('\n') {
244 Some(pos) => {
245 leading_indent = text[pos..].to_owned();
246 break;
247 }
248 None => first = token.clone(),
249 }
250 }
251
252 let mut elements: Vec<_> = first
253 .siblings_with_tokens(Direction::Next)
254 .take_while(|e| e.as_token() != Some(&last))
255 .collect();
256
257 if !(last.kind() == SyntaxKind::WHITESPACE && last.text().lines().count() <= 2) {
258 elements.push(last.into());
259 }
260
261 Some(FunctionBody::Span { elements, leading_indent })
262 } 475 }
263 476
264 fn indent_level(&self) -> IndentLevel { 477 fn indent_level(&self) -> IndentLevel {
265 match &self { 478 match &self {
266 FunctionBody::Expr(expr) => IndentLevel::from_node(expr.syntax()), 479 FunctionBody::Expr(expr) => IndentLevel::from_node(expr.syntax()),
267 FunctionBody::Span { elements, .. } => elements 480 FunctionBody::Span { parent, .. } => IndentLevel::from_node(parent.syntax()) + 1,
268 .iter()
269 .filter_map(SyntaxElement::as_node)
270 .map(IndentLevel::from_node)
271 .min_by_key(|level| level.0)
272 .expect("body must contain at least one node"),
273 } 481 }
274 } 482 }
275 483
276 fn tail_expr(&self) -> Option<ast::Expr> { 484 fn tail_expr(&self) -> Option<ast::Expr> {
277 match &self { 485 match &self {
278 FunctionBody::Expr(expr) => Some(expr.clone()), 486 FunctionBody::Expr(expr) => Some(expr.clone()),
279 FunctionBody::Span { elements, .. } => { 487 FunctionBody::Span { parent, text_range } => {
280 elements.iter().rev().find_map(|e| e.as_node()).cloned().and_then(ast::Expr::cast) 488 let tail_expr = parent.tail_expr()?;
489 if text_range.contains_range(tail_expr.syntax().text_range()) {
490 Some(tail_expr)
491 } else {
492 None
493 }
281 } 494 }
282 } 495 }
283 } 496 }
@@ -285,11 +498,11 @@ impl FunctionBody {
285 fn descendants(&self) -> impl Iterator<Item = SyntaxNode> + '_ { 498 fn descendants(&self) -> impl Iterator<Item = SyntaxNode> + '_ {
286 match self { 499 match self {
287 FunctionBody::Expr(expr) => Either::Right(expr.syntax().descendants()), 500 FunctionBody::Expr(expr) => Either::Right(expr.syntax().descendants()),
288 FunctionBody::Span { elements, .. } => Either::Left( 501 FunctionBody::Span { parent, text_range } => Either::Left(
289 elements 502 parent
290 .iter() 503 .syntax()
291 .filter_map(SyntaxElement::as_node) 504 .descendants()
292 .flat_map(SyntaxNode::descendants), 505 .filter(move |it| text_range.contains_range(it.text_range())),
293 ), 506 ),
294 } 507 }
295 } 508 }
@@ -297,10 +510,7 @@ impl FunctionBody {
297 fn text_range(&self) -> TextRange { 510 fn text_range(&self) -> TextRange {
298 match self { 511 match self {
299 FunctionBody::Expr(expr) => expr.syntax().text_range(), 512 FunctionBody::Expr(expr) => expr.syntax().text_range(),
300 FunctionBody::Span { elements, .. } => TextRange::new( 513 FunctionBody::Span { parent: _, text_range } => *text_range,
301 elements.first().unwrap().text_range().start(),
302 elements.last().unwrap().text_range().end(),
303 ),
304 } 514 }
305 } 515 }
306 516
@@ -321,30 +531,27 @@ impl HasTokenAtOffset for FunctionBody {
321 fn token_at_offset(&self, offset: TextSize) -> TokenAtOffset<SyntaxToken> { 531 fn token_at_offset(&self, offset: TextSize) -> TokenAtOffset<SyntaxToken> {
322 match self { 532 match self {
323 FunctionBody::Expr(expr) => expr.syntax().token_at_offset(offset), 533 FunctionBody::Expr(expr) => expr.syntax().token_at_offset(offset),
324 FunctionBody::Span { elements, .. } => { 534 FunctionBody::Span { parent, text_range } => {
325 stdx::always!(self.text_range().contains(offset)); 535 match parent.syntax().token_at_offset(offset) {
326 let mut iter = elements 536 TokenAtOffset::None => TokenAtOffset::None,
327 .iter() 537 TokenAtOffset::Single(t) => {
328 .filter(|element| element.text_range().contains_inclusive(offset)); 538 if text_range.contains_range(t.text_range()) {
329 let element1 = iter.next().expect("offset does not fall into body"); 539 TokenAtOffset::Single(t)
330 let element2 = iter.next(); 540 } else {
331 stdx::always!(iter.next().is_none(), "> 2 tokens at offset"); 541 TokenAtOffset::None
332 let t1 = match element1 { 542 }
333 syntax::NodeOrToken::Node(node) => node.token_at_offset(offset), 543 }
334 syntax::NodeOrToken::Token(token) => TokenAtOffset::Single(token.clone()), 544 TokenAtOffset::Between(a, b) => {
335 }; 545 match (
336 let t2 = element2.map(|e| match e { 546 text_range.contains_range(a.text_range()),
337 syntax::NodeOrToken::Node(node) => node.token_at_offset(offset), 547 text_range.contains_range(b.text_range()),
338 syntax::NodeOrToken::Token(token) => TokenAtOffset::Single(token.clone()), 548 ) {
339 }); 549 (true, true) => TokenAtOffset::Between(a, b),
340 550 (true, false) => TokenAtOffset::Single(a),
341 match t2 { 551 (false, true) => TokenAtOffset::Single(b),
342 Some(t2) => match (t1.clone().right_biased(), t2.clone().left_biased()) { 552 (false, false) => TokenAtOffset::None,
343 (Some(e1), Some(e2)) => TokenAtOffset::Between(e1, e2), 553 }
344 (Some(_), None) => t1, 554 }
345 (None, _) => t2,
346 },
347 None => t1,
348 } 555 }
349 } 556 }
350 } 557 }
@@ -389,7 +596,7 @@ fn extraction_target(node: &SyntaxNode, selection_range: TextRange) -> Option<Fu
389 // we have selected a few statements in a block 596 // we have selected a few statements in a block
390 // so covering_element returns the whole block 597 // so covering_element returns the whole block
391 if node.kind() == BLOCK_EXPR { 598 if node.kind() == BLOCK_EXPR {
392 let body = FunctionBody::from_range(&node, selection_range); 599 let body = FunctionBody::from_range(node.clone(), selection_range);
393 if body.is_some() { 600 if body.is_some() {
394 return body; 601 return body;
395 } 602 }
@@ -400,7 +607,7 @@ fn extraction_target(node: &SyntaxNode, selection_range: TextRange) -> Option<Fu
400 // so we try to expand covering_element to parent and repeat the previous 607 // so we try to expand covering_element to parent and repeat the previous
401 if let Some(parent) = node.parent() { 608 if let Some(parent) = node.parent() {
402 if parent.kind() == BLOCK_EXPR { 609 if parent.kind() == BLOCK_EXPR {
403 let body = FunctionBody::from_range(&parent, selection_range); 610 let body = FunctionBody::from_range(parent, selection_range);
404 if body.is_some() { 611 if body.is_some() {
405 return body; 612 return body;
406 } 613 }
@@ -642,13 +849,9 @@ fn either_syntax(value: &Either<ast::IdentPat, ast::SelfParam>) -> &SyntaxNode {
642 849
643/// checks if local variable is used after(outside of) body 850/// checks if local variable is used after(outside of) body
644fn var_outlives_body(ctx: &AssistContext, body: &FunctionBody, var: &Local) -> bool { 851fn var_outlives_body(ctx: &AssistContext, body: &FunctionBody, var: &Local) -> bool {
645 let usages = Definition::Local(*var) 852 let usages = LocalUsages::find(ctx, *var);
646 .usages(&ctx.sema) 853 let has_usages = usages.iter().any(|reference| body.preceedes_range(reference.range));
647 .in_scope(SearchScope::single_file(ctx.frange.file_id)) 854 has_usages
648 .all();
649 let mut usages = usages.iter().flat_map(|(_, rs)| rs.iter());
650
651 usages.any(|reference| body.preceedes_range(reference.range))
652} 855}
653 856
654fn body_return_ty(ctx: &AssistContext, body: &FunctionBody) -> Option<RetType> { 857fn body_return_ty(ctx: &AssistContext, body: &FunctionBody) -> Option<RetType> {
@@ -675,10 +878,7 @@ enum Anchor {
675fn scope_for_fn_insertion(body: &FunctionBody, anchor: Anchor) -> Option<SyntaxNode> { 878fn scope_for_fn_insertion(body: &FunctionBody, anchor: Anchor) -> Option<SyntaxNode> {
676 match body { 879 match body {
677 FunctionBody::Expr(e) => scope_for_fn_insertion_node(e.syntax(), anchor), 880 FunctionBody::Expr(e) => scope_for_fn_insertion_node(e.syntax(), anchor),
678 FunctionBody::Span { elements, .. } => { 881 FunctionBody::Span { parent, .. } => scope_for_fn_insertion_node(parent.syntax(), anchor),
679 let node = elements.iter().find_map(|e| e.as_node())?;
680 scope_for_fn_insertion_node(&node, anchor)
681 }
682 } 882 }
683} 883}
684 884
@@ -711,9 +911,24 @@ fn scope_for_fn_insertion_node(node: &SyntaxNode, anchor: Anchor) -> Option<Synt
711 last_ancestor 911 last_ancestor
712} 912}
713 913
714fn format_replacement(ctx: &AssistContext, fun: &Function) -> String { 914fn format_replacement(ctx: &AssistContext, fun: &Function, indent: IndentLevel) -> String {
715 let mut buf = String::new(); 915 let ret_ty = fun.return_type(ctx);
916
917 let args = fun.params.iter().map(|param| param.to_arg(ctx));
918 let args = make::arg_list(args);
919 let call_expr = if fun.self_param.is_some() {
920 let self_arg = make::expr_path(make_path_from_text("self"));
921 make::expr_method_call(self_arg, &fun.name, args)
922 } else {
923 let func = make::expr_path(make_path_from_text(&fun.name));
924 make::expr_call(func, args)
925 };
926
927 let handler = FlowHandler::from_ret_ty(fun, &ret_ty);
716 928
929 let expr = handler.make_call_expr(call_expr).indent(indent);
930
931 let mut buf = String::new();
717 match fun.vars_defined_in_body_and_outlive.as_slice() { 932 match fun.vars_defined_in_body_and_outlive.as_slice() {
718 [] => {} 933 [] => {}
719 [var] => format_to!(buf, "let {} = ", var.name(ctx.db()).unwrap()), 934 [var] => format_to!(buf, "let {} = ", var.name(ctx.db()).unwrap()),
@@ -726,34 +941,131 @@ fn format_replacement(ctx: &AssistContext, fun: &Function) -> String {
726 buf.push_str(") = "); 941 buf.push_str(") = ");
727 } 942 }
728 } 943 }
729 944 format_to!(buf, "{}", expr);
730 if fun.self_param.is_some() { 945 if fun.ret_ty.is_unit()
731 format_to!(buf, "self."); 946 && (!fun.vars_defined_in_body_and_outlive.is_empty() || !expr.is_block_like())
732 } 947 {
733 format_to!(buf, "{}(", fun.name); 948 buf.push(';');
734 format_arg_list_to(&mut buf, fun, ctx);
735 format_to!(buf, ")");
736
737 if fun.ret_ty.is_unit() {
738 format_to!(buf, ";");
739 } 949 }
740
741 buf 950 buf
742} 951}
743 952
744fn format_arg_list_to(buf: &mut String, fun: &Function, ctx: &AssistContext) { 953enum FlowHandler {
745 let mut it = fun.params.iter(); 954 None,
746 if let Some(param) = it.next() { 955 Try { kind: TryKind },
747 format_arg_to(buf, ctx, param); 956 If { action: FlowKind },
957 IfOption { action: FlowKind },
958 MatchOption { none: FlowKind },
959 MatchResult { err: FlowKind },
960}
961
962impl FlowHandler {
963 fn from_ret_ty(fun: &Function, ret_ty: &FunType) -> FlowHandler {
964 match &fun.control_flow.kind {
965 None => FlowHandler::None,
966 Some(flow_kind) => {
967 let action = flow_kind.clone();
968 if *ret_ty == FunType::Unit {
969 match flow_kind {
970 FlowKind::Return | FlowKind::Break | FlowKind::Continue => {
971 FlowHandler::If { action }
972 }
973 FlowKind::ReturnValue(_) | FlowKind::BreakValue(_) => {
974 FlowHandler::IfOption { action }
975 }
976 FlowKind::Try { kind } | FlowKind::TryReturn { kind, .. } => {
977 FlowHandler::Try { kind: kind.clone() }
978 }
979 }
980 } else {
981 match flow_kind {
982 FlowKind::Return | FlowKind::Break | FlowKind::Continue => {
983 FlowHandler::MatchOption { none: action }
984 }
985 FlowKind::ReturnValue(_) | FlowKind::BreakValue(_) => {
986 FlowHandler::MatchResult { err: action }
987 }
988 FlowKind::Try { kind } | FlowKind::TryReturn { kind, .. } => {
989 FlowHandler::Try { kind: kind.clone() }
990 }
991 }
992 }
993 }
994 }
748 } 995 }
749 for param in it { 996
750 buf.push_str(", "); 997 fn make_call_expr(&self, call_expr: ast::Expr) -> ast::Expr {
751 format_arg_to(buf, ctx, param); 998 match self {
999 FlowHandler::None => call_expr,
1000 FlowHandler::Try { kind: _ } => make::expr_try(call_expr),
1001 FlowHandler::If { action } => {
1002 let action = action.make_result_handler(None);
1003 let stmt = make::expr_stmt(action);
1004 let block = make::block_expr(iter::once(stmt.into()), None);
1005 let condition = make::condition(call_expr, None);
1006 make::expr_if(condition, block, None)
1007 }
1008 FlowHandler::IfOption { action } => {
1009 let path = make_path_from_text("Some");
1010 let value_pat = make::ident_pat(make::name("value"));
1011 let pattern = make::tuple_struct_pat(path, iter::once(value_pat.into()));
1012 let cond = make::condition(call_expr, Some(pattern.into()));
1013 let value = make::expr_path(make_path_from_text("value"));
1014 let action_expr = action.make_result_handler(Some(value));
1015 let action_stmt = make::expr_stmt(action_expr);
1016 let then = make::block_expr(iter::once(action_stmt.into()), None);
1017 make::expr_if(cond, then, None)
1018 }
1019 FlowHandler::MatchOption { none } => {
1020 let some_name = "value";
1021
1022 let some_arm = {
1023 let path = make_path_from_text("Some");
1024 let value_pat = make::ident_pat(make::name(some_name));
1025 let pat = make::tuple_struct_pat(path, iter::once(value_pat.into()));
1026 let value = make::expr_path(make_path_from_text(some_name));
1027 make::match_arm(iter::once(pat.into()), value)
1028 };
1029 let none_arm = {
1030 let path = make_path_from_text("None");
1031 let pat = make::path_pat(path);
1032 make::match_arm(iter::once(pat), none.make_result_handler(None))
1033 };
1034 let arms = make::match_arm_list(vec![some_arm, none_arm]);
1035 make::expr_match(call_expr, arms)
1036 }
1037 FlowHandler::MatchResult { err } => {
1038 let ok_name = "value";
1039 let err_name = "value";
1040
1041 let ok_arm = {
1042 let path = make_path_from_text("Ok");
1043 let value_pat = make::ident_pat(make::name(ok_name));
1044 let pat = make::tuple_struct_pat(path, iter::once(value_pat.into()));
1045 let value = make::expr_path(make_path_from_text(ok_name));
1046 make::match_arm(iter::once(pat.into()), value)
1047 };
1048 let err_arm = {
1049 let path = make_path_from_text("Err");
1050 let value_pat = make::ident_pat(make::name(err_name));
1051 let pat = make::tuple_struct_pat(path, iter::once(value_pat.into()));
1052 let value = make::expr_path(make_path_from_text(err_name));
1053 make::match_arm(iter::once(pat.into()), err.make_result_handler(Some(value)))
1054 };
1055 let arms = make::match_arm_list(vec![ok_arm, err_arm]);
1056 make::expr_match(call_expr, arms)
1057 }
1058 }
752 } 1059 }
753} 1060}
754 1061
755fn format_arg_to(buf: &mut String, ctx: &AssistContext, param: &Param) { 1062fn make_path_from_text(text: &str) -> ast::Path {
756 format_to!(buf, "{}{}", param.value_prefix(), param.var.name(ctx.db()).unwrap()); 1063 make::path_unqualified(make::path_segment(make::name_ref(text)))
1064}
1065
1066fn path_expr_from_local(ctx: &AssistContext, var: Local) -> ast::Expr {
1067 let name = var.name(ctx.db()).unwrap().to_string();
1068 make::expr_path(make_path_from_text(&name))
757} 1069}
758 1070
759fn format_function( 1071fn format_function(
@@ -764,132 +1076,233 @@ fn format_function(
764 new_indent: IndentLevel, 1076 new_indent: IndentLevel,
765) -> String { 1077) -> String {
766 let mut fn_def = String::new(); 1078 let mut fn_def = String::new();
767 format_to!(fn_def, "\n\n{}fn $0{}(", new_indent, fun.name); 1079 let params = make_param_list(ctx, module, fun);
768 format_function_param_list_to(&mut fn_def, ctx, module, fun); 1080 let ret_ty = make_ret_ty(ctx, module, fun);
769 fn_def.push(')'); 1081 let body = make_body(ctx, old_indent, new_indent, fun);
770 format_function_ret_to(&mut fn_def, ctx, module, fun); 1082 format_to!(fn_def, "\n\n{}fn $0{}{}", new_indent, fun.name, params);
771 fn_def.push_str(" {"); 1083 if let Some(ret_ty) = ret_ty {
772 format_function_body_to(&mut fn_def, ctx, old_indent, new_indent, fun); 1084 format_to!(fn_def, " {}", ret_ty);
773 format_to!(fn_def, "{}}}", new_indent); 1085 }
1086 format_to!(fn_def, " {}", body);
774 1087
775 fn_def 1088 fn_def
776} 1089}
777 1090
778fn format_function_param_list_to( 1091fn make_param_list(ctx: &AssistContext, module: hir::Module, fun: &Function) -> ast::ParamList {
779 fn_def: &mut String, 1092 let self_param = fun.self_param.clone();
780 ctx: &AssistContext, 1093 let params = fun.params.iter().map(|param| param.to_param(ctx, module));
781 module: hir::Module, 1094 make::param_list(self_param, params)
782 fun: &Function,
783) {
784 let mut it = fun.params.iter();
785 if let Some(self_param) = &fun.self_param {
786 format_to!(fn_def, "{}", self_param);
787 } else if let Some(param) = it.next() {
788 format_param_to(fn_def, ctx, module, param);
789 }
790 for param in it {
791 fn_def.push_str(", ");
792 format_param_to(fn_def, ctx, module, param);
793 }
794}
795
796fn format_param_to(fn_def: &mut String, ctx: &AssistContext, module: hir::Module, param: &Param) {
797 format_to!(
798 fn_def,
799 "{}{}: {}{}",
800 param.mut_pattern(),
801 param.var.name(ctx.db()).unwrap(),
802 param.type_prefix(),
803 format_type(&param.ty, ctx, module)
804 );
805} 1095}
806 1096
807fn format_function_ret_to( 1097impl FunType {
808 fn_def: &mut String, 1098 fn make_ty(&self, ctx: &AssistContext, module: hir::Module) -> ast::Type {
809 ctx: &AssistContext, 1099 match self {
810 module: hir::Module, 1100 FunType::Unit => make::ty_unit(),
811 fun: &Function, 1101 FunType::Single(ty) => make_ty(ty, ctx, module),
812) { 1102 FunType::Tuple(types) => match types.as_slice() {
813 if let Some(ty) = fun.ret_ty.as_fn_ret() { 1103 [] => {
814 format_to!(fn_def, " -> {}", format_type(ty, ctx, module)); 1104 stdx::never!("tuple type with 0 elements");
815 } else { 1105 make::ty_unit()
816 match fun.vars_defined_in_body_and_outlive.as_slice() {
817 [] => {}
818 [var] => {
819 format_to!(fn_def, " -> {}", format_type(&var.ty(ctx.db()), ctx, module));
820 }
821 [v0, vs @ ..] => {
822 format_to!(fn_def, " -> ({}", format_type(&v0.ty(ctx.db()), ctx, module));
823 for var in vs {
824 format_to!(fn_def, ", {}", format_type(&var.ty(ctx.db()), ctx, module));
825 } 1106 }
826 fn_def.push(')'); 1107 [ty] => {
827 } 1108 stdx::never!("tuple type with 1 element");
1109 make_ty(ty, ctx, module)
1110 }
1111 types => {
1112 let types = types.iter().map(|ty| make_ty(ty, ctx, module));
1113 make::ty_tuple(types)
1114 }
1115 },
828 } 1116 }
829 } 1117 }
830} 1118}
831 1119
832fn format_function_body_to( 1120fn make_ret_ty(ctx: &AssistContext, module: hir::Module, fun: &Function) -> Option<ast::RetType> {
833 fn_def: &mut String, 1121 let fun_ty = fun.return_type(ctx);
1122 let handler = FlowHandler::from_ret_ty(fun, &fun_ty);
1123 let ret_ty = match &handler {
1124 FlowHandler::None => {
1125 if matches!(fun_ty, FunType::Unit) {
1126 return None;
1127 }
1128 fun_ty.make_ty(ctx, module)
1129 }
1130 FlowHandler::Try { kind: TryKind::Option } => {
1131 make::ty_generic(make::name_ref("Option"), iter::once(fun_ty.make_ty(ctx, module)))
1132 }
1133 FlowHandler::Try { kind: TryKind::Result { ty: parent_ret_ty } } => {
1134 let handler_ty = parent_ret_ty
1135 .type_parameters()
1136 .nth(1)
1137 .map(|ty| make_ty(&ty, ctx, module))
1138 .unwrap_or_else(make::ty_unit);
1139 make::ty_generic(
1140 make::name_ref("Result"),
1141 vec![fun_ty.make_ty(ctx, module), handler_ty],
1142 )
1143 }
1144 FlowHandler::If { .. } => make::ty("bool"),
1145 FlowHandler::IfOption { action } => {
1146 let handler_ty = action
1147 .expr_ty(ctx)
1148 .map(|ty| make_ty(&ty, ctx, module))
1149 .unwrap_or_else(make::ty_unit);
1150 make::ty_generic(make::name_ref("Option"), iter::once(handler_ty))
1151 }
1152 FlowHandler::MatchOption { .. } => {
1153 make::ty_generic(make::name_ref("Option"), iter::once(fun_ty.make_ty(ctx, module)))
1154 }
1155 FlowHandler::MatchResult { err } => {
1156 let handler_ty =
1157 err.expr_ty(ctx).map(|ty| make_ty(&ty, ctx, module)).unwrap_or_else(make::ty_unit);
1158 make::ty_generic(
1159 make::name_ref("Result"),
1160 vec![fun_ty.make_ty(ctx, module), handler_ty],
1161 )
1162 }
1163 };
1164 Some(make::ret_type(ret_ty))
1165}
1166
1167fn make_body(
834 ctx: &AssistContext, 1168 ctx: &AssistContext,
835 old_indent: IndentLevel, 1169 old_indent: IndentLevel,
836 new_indent: IndentLevel, 1170 new_indent: IndentLevel,
837 fun: &Function, 1171 fun: &Function,
838) { 1172) -> ast::BlockExpr {
839 match &fun.body { 1173 let ret_ty = fun.return_type(ctx);
1174 let handler = FlowHandler::from_ret_ty(fun, &ret_ty);
1175 let block = match &fun.body {
840 FunctionBody::Expr(expr) => { 1176 FunctionBody::Expr(expr) => {
841 fn_def.push('\n'); 1177 let expr = rewrite_body_segment(ctx, &fun.params, &handler, expr.syntax());
842 let expr = expr.dedent(old_indent).indent(new_indent + 1); 1178 let expr = ast::Expr::cast(expr).unwrap();
843 let expr = fix_param_usages(ctx, &fun.params, expr.syntax()); 1179 let expr = expr.dedent(old_indent).indent(IndentLevel(1));
844 format_to!(fn_def, "{}{}", new_indent + 1, expr); 1180
845 fn_def.push('\n'); 1181 make::block_expr(Vec::new(), Some(expr))
846 } 1182 }
847 FunctionBody::Span { elements, leading_indent } => { 1183 FunctionBody::Span { parent, text_range } => {
848 format_to!(fn_def, "{}", leading_indent); 1184 let mut elements: Vec<_> = parent
849 let new_indent_str = format!("\n{}", new_indent + 1); 1185 .syntax()
850 for mut element in elements { 1186 .children()
851 let new_ws; 1187 .filter(|it| text_range.contains_range(it.text_range()))
852 if let Some(ws) = element.as_token().cloned().and_then(ast::Whitespace::cast) { 1188 .map(|it| rewrite_body_segment(ctx, &fun.params, &handler, &it))
853 let text = ws.syntax().text(); 1189 .collect();
854 if text.contains('\n') { 1190
855 let new_text = text.replace(&format!("\n{}", old_indent), &new_indent_str); 1191 let mut tail_expr = match elements.pop() {
856 new_ws = ast::make::tokens::whitespace(&new_text).into(); 1192 Some(node) => ast::Expr::cast(node.clone()).or_else(|| {
857 element = &new_ws; 1193 elements.push(node);
858 } 1194 None
859 } 1195 }),
1196 None => None,
1197 };
860 1198
861 match element { 1199 if tail_expr.is_none() {
862 syntax::NodeOrToken::Node(node) => { 1200 match fun.vars_defined_in_body_and_outlive.as_slice() {
863 format_to!(fn_def, "{}", fix_param_usages(ctx, &fun.params, node)); 1201 [] => {}
1202 [var] => {
1203 tail_expr = Some(path_expr_from_local(ctx, *var));
864 } 1204 }
865 syntax::NodeOrToken::Token(token) => { 1205 vars => {
866 format_to!(fn_def, "{}", token); 1206 let exprs = vars.iter().map(|var| path_expr_from_local(ctx, *var));
1207 let expr = make::expr_tuple(exprs);
1208 tail_expr = Some(expr);
867 } 1209 }
868 } 1210 }
869 } 1211 }
870 if !fn_def.ends_with('\n') { 1212
871 fn_def.push('\n'); 1213 let elements = elements.into_iter().filter_map(|node| match ast::Stmt::cast(node) {
872 } 1214 Some(stmt) => Some(stmt),
1215 None => {
1216 stdx::never!("block contains non-statement");
1217 None
1218 }
1219 });
1220
1221 let body_indent = IndentLevel(1);
1222 let elements = elements.map(|stmt| stmt.dedent(old_indent).indent(body_indent));
1223 let tail_expr = tail_expr.map(|expr| expr.dedent(old_indent).indent(body_indent));
1224
1225 make::block_expr(elements, tail_expr)
873 } 1226 }
874 } 1227 };
875 1228
876 match fun.vars_defined_in_body_and_outlive.as_slice() { 1229 let block = match &handler {
877 [] => {} 1230 FlowHandler::None => block,
878 [var] => format_to!(fn_def, "{}{}\n", new_indent + 1, var.name(ctx.db()).unwrap()), 1231 FlowHandler::Try { kind } => {
879 [v0, vs @ ..] => { 1232 let block = with_default_tail_expr(block, make::expr_unit());
880 format_to!(fn_def, "{}({}", new_indent + 1, v0.name(ctx.db()).unwrap()); 1233 map_tail_expr(block, |tail_expr| {
881 for var in vs { 1234 let constructor = match kind {
882 format_to!(fn_def, ", {}", var.name(ctx.db()).unwrap()); 1235 TryKind::Option => "Some",
883 } 1236 TryKind::Result { .. } => "Ok",
884 fn_def.push_str(")\n"); 1237 };
1238 let func = make::expr_path(make_path_from_text(constructor));
1239 let args = make::arg_list(iter::once(tail_expr));
1240 make::expr_call(func, args)
1241 })
1242 }
1243 FlowHandler::If { .. } => {
1244 let lit_false = ast::Literal::cast(make::tokens::literal("false").parent()).unwrap();
1245 with_tail_expr(block, lit_false.into())
885 } 1246 }
1247 FlowHandler::IfOption { .. } => {
1248 let none = make::expr_path(make_path_from_text("None"));
1249 with_tail_expr(block, none)
1250 }
1251 FlowHandler::MatchOption { .. } => map_tail_expr(block, |tail_expr| {
1252 let some = make::expr_path(make_path_from_text("Some"));
1253 let args = make::arg_list(iter::once(tail_expr));
1254 make::expr_call(some, args)
1255 }),
1256 FlowHandler::MatchResult { .. } => map_tail_expr(block, |tail_expr| {
1257 let ok = make::expr_path(make_path_from_text("Ok"));
1258 let args = make::arg_list(iter::once(tail_expr));
1259 make::expr_call(ok, args)
1260 }),
1261 };
1262
1263 block.indent(new_indent)
1264}
1265
1266fn map_tail_expr(block: ast::BlockExpr, f: impl FnOnce(ast::Expr) -> ast::Expr) -> ast::BlockExpr {
1267 let tail_expr = match block.tail_expr() {
1268 Some(tail_expr) => tail_expr,
1269 None => return block,
1270 };
1271 make::block_expr(block.statements(), Some(f(tail_expr)))
1272}
1273
1274fn with_default_tail_expr(block: ast::BlockExpr, tail_expr: ast::Expr) -> ast::BlockExpr {
1275 match block.tail_expr() {
1276 Some(_) => block,
1277 None => make::block_expr(block.statements(), Some(tail_expr)),
886 } 1278 }
887} 1279}
888 1280
1281fn with_tail_expr(block: ast::BlockExpr, tail_expr: ast::Expr) -> ast::BlockExpr {
1282 let stmt_tail = block.tail_expr().map(|expr| make::expr_stmt(expr).into());
1283 let stmts = block.statements().chain(stmt_tail);
1284 make::block_expr(stmts, Some(tail_expr))
1285}
1286
889fn format_type(ty: &hir::Type, ctx: &AssistContext, module: hir::Module) -> String { 1287fn format_type(ty: &hir::Type, ctx: &AssistContext, module: hir::Module) -> String {
890 ty.display_source_code(ctx.db(), module.into()).ok().unwrap_or_else(|| "()".to_string()) 1288 ty.display_source_code(ctx.db(), module.into()).ok().unwrap_or_else(|| "()".to_string())
891} 1289}
892 1290
1291fn make_ty(ty: &hir::Type, ctx: &AssistContext, module: hir::Module) -> ast::Type {
1292 let ty_str = format_type(ty, ctx, module);
1293 make::ty(&ty_str)
1294}
1295
1296fn rewrite_body_segment(
1297 ctx: &AssistContext,
1298 params: &[Param],
1299 handler: &FlowHandler,
1300 syntax: &SyntaxNode,
1301) -> SyntaxNode {
1302 let syntax = fix_param_usages(ctx, params, syntax);
1303 update_external_control_flow(handler, &syntax)
1304}
1305
893/// change all usages to account for added `&`/`&mut` for some params 1306/// change all usages to account for added `&`/`&mut` for some params
894fn fix_param_usages(ctx: &AssistContext, params: &[Param], syntax: &SyntaxNode) -> SyntaxNode { 1307fn fix_param_usages(ctx: &AssistContext, params: &[Param], syntax: &SyntaxNode) -> SyntaxNode {
895 let mut rewriter = SyntaxRewriter::default(); 1308 let mut rewriter = SyntaxRewriter::default();
@@ -919,7 +1332,7 @@ fn fix_param_usages(ctx: &AssistContext, params: &[Param], syntax: &SyntaxNode)
919 rewriter.replace_ast(&node.clone().into(), &node.expr().unwrap()); 1332 rewriter.replace_ast(&node.clone().into(), &node.expr().unwrap());
920 } 1333 }
921 Some(_) | None => { 1334 Some(_) | None => {
922 rewriter.replace_ast(&path, &ast::make::expr_prefix(T![*], path.clone())); 1335 rewriter.replace_ast(&path, &make::expr_prefix(T![*], path.clone()));
923 } 1336 }
924 }; 1337 };
925 } 1338 }
@@ -928,6 +1341,98 @@ fn fix_param_usages(ctx: &AssistContext, params: &[Param], syntax: &SyntaxNode)
928 rewriter.rewrite(syntax) 1341 rewriter.rewrite(syntax)
929} 1342}
930 1343
1344fn update_external_control_flow(handler: &FlowHandler, syntax: &SyntaxNode) -> SyntaxNode {
1345 let mut rewriter = SyntaxRewriter::default();
1346
1347 let mut nested_loop = None;
1348 let mut nested_scope = None;
1349 for event in syntax.preorder() {
1350 let node = match event {
1351 WalkEvent::Enter(e) => {
1352 match e.kind() {
1353 SyntaxKind::LOOP_EXPR | SyntaxKind::WHILE_EXPR | SyntaxKind::FOR_EXPR => {
1354 if nested_loop.is_none() {
1355 nested_loop = Some(e.clone());
1356 }
1357 }
1358 SyntaxKind::FN
1359 | SyntaxKind::CONST
1360 | SyntaxKind::STATIC
1361 | SyntaxKind::IMPL
1362 | SyntaxKind::MODULE => {
1363 if nested_scope.is_none() {
1364 nested_scope = Some(e.clone());
1365 }
1366 }
1367 _ => {}
1368 }
1369 e
1370 }
1371 WalkEvent::Leave(e) => {
1372 if nested_loop.as_ref() == Some(&e) {
1373 nested_loop = None;
1374 }
1375 if nested_scope.as_ref() == Some(&e) {
1376 nested_scope = None;
1377 }
1378 continue;
1379 }
1380 };
1381 if nested_scope.is_some() {
1382 continue;
1383 }
1384 let expr = match ast::Expr::cast(node) {
1385 Some(e) => e,
1386 None => continue,
1387 };
1388 match expr {
1389 ast::Expr::ReturnExpr(return_expr) if nested_scope.is_none() => {
1390 let expr = return_expr.expr();
1391 if let Some(replacement) = make_rewritten_flow(handler, expr) {
1392 rewriter.replace_ast(&return_expr.into(), &replacement);
1393 }
1394 }
1395 ast::Expr::BreakExpr(break_expr) if nested_loop.is_none() => {
1396 let expr = break_expr.expr();
1397 if let Some(replacement) = make_rewritten_flow(handler, expr) {
1398 rewriter.replace_ast(&break_expr.into(), &replacement);
1399 }
1400 }
1401 ast::Expr::ContinueExpr(continue_expr) if nested_loop.is_none() => {
1402 if let Some(replacement) = make_rewritten_flow(handler, None) {
1403 rewriter.replace_ast(&continue_expr.into(), &replacement);
1404 }
1405 }
1406 _ => {
1407 // do nothing
1408 }
1409 }
1410 }
1411
1412 rewriter.rewrite(syntax)
1413}
1414
1415fn make_rewritten_flow(handler: &FlowHandler, arg_expr: Option<ast::Expr>) -> Option<ast::Expr> {
1416 let value = match handler {
1417 FlowHandler::None | FlowHandler::Try { .. } => return None,
1418 FlowHandler::If { .. } => {
1419 ast::Literal::cast(make::tokens::literal("true").parent()).unwrap().into()
1420 }
1421 FlowHandler::IfOption { .. } => {
1422 let expr = arg_expr.unwrap_or_else(|| make::expr_tuple(Vec::new()));
1423 let args = make::arg_list(iter::once(expr));
1424 make::expr_call(make::expr_path(make_path_from_text("Some")), args)
1425 }
1426 FlowHandler::MatchOption { .. } => make::expr_path(make_path_from_text("None")),
1427 FlowHandler::MatchResult { .. } => {
1428 let expr = arg_expr.unwrap_or_else(|| make::expr_tuple(Vec::new()));
1429 let args = make::arg_list(iter::once(expr));
1430 make::expr_call(make::expr_path(make_path_from_text("Err")), args)
1431 }
1432 };
1433 Some(make::expr_return(Some(value)))
1434}
1435
931#[cfg(test)] 1436#[cfg(test)]
932mod tests { 1437mod tests {
933 use crate::tests::{check_assist, check_assist_not_applicable}; 1438 use crate::tests::{check_assist, check_assist_not_applicable};
@@ -2096,6 +2601,66 @@ fn $0fun_name(c: &Counter) {
2096 } 2601 }
2097 2602
2098 #[test] 2603 #[test]
2604 fn copy_used_after() {
2605 check_assist(
2606 extract_function,
2607 r##"
2608#[lang = "copy"]
2609pub trait Copy {}
2610impl Copy for i32 {}
2611fn foo() {
2612 let n = 0;
2613 $0let m = n;$0
2614 let k = n;
2615}"##,
2616 r##"
2617#[lang = "copy"]
2618pub trait Copy {}
2619impl Copy for i32 {}
2620fn foo() {
2621 let n = 0;
2622 fun_name(n);
2623 let k = n;
2624}
2625
2626fn $0fun_name(n: i32) {
2627 let m = n;
2628}"##,
2629 )
2630 }
2631
2632 #[test]
2633 fn copy_custom_used_after() {
2634 check_assist(
2635 extract_function,
2636 r##"
2637#[lang = "copy"]
2638pub trait Copy {}
2639struct Counter(i32);
2640impl Copy for Counter {}
2641fn foo() {
2642 let c = Counter(0);
2643 $0let n = c.0;$0
2644 let m = c.0;
2645}"##,
2646 r##"
2647#[lang = "copy"]
2648pub trait Copy {}
2649struct Counter(i32);
2650impl Copy for Counter {}
2651fn foo() {
2652 let c = Counter(0);
2653 fun_name(c);
2654 let m = c.0;
2655}
2656
2657fn $0fun_name(c: Counter) {
2658 let n = c.0;
2659}"##,
2660 );
2661 }
2662
2663 #[test]
2099 fn indented_stmts() { 2664 fn indented_stmts() {
2100 check_assist( 2665 check_assist(
2101 extract_function, 2666 extract_function,
@@ -2156,4 +2721,658 @@ mod bar {
2156}", 2721}",
2157 ); 2722 );
2158 } 2723 }
2724
2725 #[test]
2726 fn break_loop() {
2727 check_assist(
2728 extract_function,
2729 r##"
2730enum Option<T> {
2731 #[lang = "None"] None,
2732 #[lang = "Some"] Some(T),
2733}
2734use Option::*;
2735fn foo() {
2736 loop {
2737 let n = 1;
2738 $0let m = n + 1;
2739 break;
2740 let k = 2;$0
2741 let h = 1 + k;
2742 }
2743}"##,
2744 r##"
2745enum Option<T> {
2746 #[lang = "None"] None,
2747 #[lang = "Some"] Some(T),
2748}
2749use Option::*;
2750fn foo() {
2751 loop {
2752 let n = 1;
2753 let k = match fun_name(n) {
2754 Some(value) => value,
2755 None => break,
2756 };
2757 let h = 1 + k;
2758 }
2759}
2760
2761fn $0fun_name(n: i32) -> Option<i32> {
2762 let m = n + 1;
2763 return None;
2764 let k = 2;
2765 Some(k)
2766}"##,
2767 );
2768 }
2769
2770 #[test]
2771 fn return_to_parent() {
2772 check_assist(
2773 extract_function,
2774 r##"
2775#[lang = "copy"]
2776pub trait Copy {}
2777impl Copy for i32 {}
2778enum Result<T, E> {
2779 #[lang = "Ok"] Ok(T),
2780 #[lang = "Err"] Err(E),
2781}
2782use Result::*;
2783fn foo() -> i64 {
2784 let n = 1;
2785 $0let m = n + 1;
2786 return 1;
2787 let k = 2;$0
2788 (n + k) as i64
2789}"##,
2790 r##"
2791#[lang = "copy"]
2792pub trait Copy {}
2793impl Copy for i32 {}
2794enum Result<T, E> {
2795 #[lang = "Ok"] Ok(T),
2796 #[lang = "Err"] Err(E),
2797}
2798use Result::*;
2799fn foo() -> i64 {
2800 let n = 1;
2801 let k = match fun_name(n) {
2802 Ok(value) => value,
2803 Err(value) => return value,
2804 };
2805 (n + k) as i64
2806}
2807
2808fn $0fun_name(n: i32) -> Result<i32, i64> {
2809 let m = n + 1;
2810 return Err(1);
2811 let k = 2;
2812 Ok(k)
2813}"##,
2814 );
2815 }
2816
2817 #[test]
2818 fn break_and_continue() {
2819 mark::check!(external_control_flow_break_and_continue);
2820 check_assist_not_applicable(
2821 extract_function,
2822 r##"
2823fn foo() {
2824 loop {
2825 let n = 1;
2826 $0let m = n + 1;
2827 break;
2828 let k = 2;
2829 continue;
2830 let k = k + 1;$0
2831 let r = n + k;
2832 }
2833}"##,
2834 );
2835 }
2836
2837 #[test]
2838 fn return_and_break() {
2839 mark::check!(external_control_flow_return_and_bc);
2840 check_assist_not_applicable(
2841 extract_function,
2842 r##"
2843fn foo() {
2844 loop {
2845 let n = 1;
2846 $0let m = n + 1;
2847 break;
2848 let k = 2;
2849 return;
2850 let k = k + 1;$0
2851 let r = n + k;
2852 }
2853}"##,
2854 );
2855 }
2856
2857 #[test]
2858 fn break_loop_with_if() {
2859 check_assist(
2860 extract_function,
2861 r##"
2862fn foo() {
2863 loop {
2864 let mut n = 1;
2865 $0let m = n + 1;
2866 break;
2867 n += m;$0
2868 let h = 1 + n;
2869 }
2870}"##,
2871 r##"
2872fn foo() {
2873 loop {
2874 let mut n = 1;
2875 if fun_name(&mut n) {
2876 break;
2877 }
2878 let h = 1 + n;
2879 }
2880}
2881
2882fn $0fun_name(n: &mut i32) -> bool {
2883 let m = *n + 1;
2884 return true;
2885 *n += m;
2886 false
2887}"##,
2888 );
2889 }
2890
2891 #[test]
2892 fn break_loop_nested() {
2893 check_assist(
2894 extract_function,
2895 r##"
2896fn foo() {
2897 loop {
2898 let mut n = 1;
2899 $0let m = n + 1;
2900 if m == 42 {
2901 break;
2902 }$0
2903 let h = 1;
2904 }
2905}"##,
2906 r##"
2907fn foo() {
2908 loop {
2909 let mut n = 1;
2910 if fun_name(n) {
2911 break;
2912 }
2913 let h = 1;
2914 }
2915}
2916
2917fn $0fun_name(n: i32) -> bool {
2918 let m = n + 1;
2919 if m == 42 {
2920 return true;
2921 }
2922 false
2923}"##,
2924 );
2925 }
2926
2927 #[test]
2928 fn return_from_nested_loop() {
2929 check_assist(
2930 extract_function,
2931 r##"
2932fn foo() {
2933 loop {
2934 let n = 1;
2935 $0
2936 let k = 1;
2937 loop {
2938 return;
2939 }
2940 let m = k + 1;$0
2941 let h = 1 + m;
2942 }
2943}"##,
2944 r##"
2945fn foo() {
2946 loop {
2947 let n = 1;
2948 let m = match fun_name() {
2949 Some(value) => value,
2950 None => return,
2951 };
2952 let h = 1 + m;
2953 }
2954}
2955
2956fn $0fun_name() -> Option<i32> {
2957 let k = 1;
2958 loop {
2959 return None;
2960 }
2961 let m = k + 1;
2962 Some(m)
2963}"##,
2964 );
2965 }
2966
2967 #[test]
2968 fn break_from_nested_loop() {
2969 check_assist(
2970 extract_function,
2971 r##"
2972fn foo() {
2973 loop {
2974 let n = 1;
2975 $0let k = 1;
2976 loop {
2977 break;
2978 }
2979 let m = k + 1;$0
2980 let h = 1 + m;
2981 }
2982}"##,
2983 r##"
2984fn foo() {
2985 loop {
2986 let n = 1;
2987 let m = fun_name();
2988 let h = 1 + m;
2989 }
2990}
2991
2992fn $0fun_name() -> i32 {
2993 let k = 1;
2994 loop {
2995 break;
2996 }
2997 let m = k + 1;
2998 m
2999}"##,
3000 );
3001 }
3002
3003 #[test]
3004 fn break_from_nested_and_outer_loops() {
3005 check_assist(
3006 extract_function,
3007 r##"
3008fn foo() {
3009 loop {
3010 let n = 1;
3011 $0let k = 1;
3012 loop {
3013 break;
3014 }
3015 if k == 42 {
3016 break;
3017 }
3018 let m = k + 1;$0
3019 let h = 1 + m;
3020 }
3021}"##,
3022 r##"
3023fn foo() {
3024 loop {
3025 let n = 1;
3026 let m = match fun_name() {
3027 Some(value) => value,
3028 None => break,
3029 };
3030 let h = 1 + m;
3031 }
3032}
3033
3034fn $0fun_name() -> Option<i32> {
3035 let k = 1;
3036 loop {
3037 break;
3038 }
3039 if k == 42 {
3040 return None;
3041 }
3042 let m = k + 1;
3043 Some(m)
3044}"##,
3045 );
3046 }
3047
3048 #[test]
3049 fn return_from_nested_fn() {
3050 check_assist(
3051 extract_function,
3052 r##"
3053fn foo() {
3054 loop {
3055 let n = 1;
3056 $0let k = 1;
3057 fn test() {
3058 return;
3059 }
3060 let m = k + 1;$0
3061 let h = 1 + m;
3062 }
3063}"##,
3064 r##"
3065fn foo() {
3066 loop {
3067 let n = 1;
3068 let m = fun_name();
3069 let h = 1 + m;
3070 }
3071}
3072
3073fn $0fun_name() -> i32 {
3074 let k = 1;
3075 fn test() {
3076 return;
3077 }
3078 let m = k + 1;
3079 m
3080}"##,
3081 );
3082 }
3083
3084 #[test]
3085 fn break_with_value() {
3086 check_assist(
3087 extract_function,
3088 r##"
3089fn foo() -> i32 {
3090 loop {
3091 let n = 1;
3092 $0let k = 1;
3093 if k == 42 {
3094 break 3;
3095 }
3096 let m = k + 1;$0
3097 let h = 1;
3098 }
3099}"##,
3100 r##"
3101fn foo() -> i32 {
3102 loop {
3103 let n = 1;
3104 if let Some(value) = fun_name() {
3105 break value;
3106 }
3107 let h = 1;
3108 }
3109}
3110
3111fn $0fun_name() -> Option<i32> {
3112 let k = 1;
3113 if k == 42 {
3114 return Some(3);
3115 }
3116 let m = k + 1;
3117 None
3118}"##,
3119 );
3120 }
3121
3122 #[test]
3123 fn break_with_value_and_return() {
3124 check_assist(
3125 extract_function,
3126 r##"
3127fn foo() -> i64 {
3128 loop {
3129 let n = 1;
3130 $0
3131 let k = 1;
3132 if k == 42 {
3133 break 3;
3134 }
3135 let m = k + 1;$0
3136 let h = 1 + m;
3137 }
3138}"##,
3139 r##"
3140fn foo() -> i64 {
3141 loop {
3142 let n = 1;
3143 let m = match fun_name() {
3144 Ok(value) => value,
3145 Err(value) => break value,
3146 };
3147 let h = 1 + m;
3148 }
3149}
3150
3151fn $0fun_name() -> Result<i32, i64> {
3152 let k = 1;
3153 if k == 42 {
3154 return Err(3);
3155 }
3156 let m = k + 1;
3157 Ok(m)
3158}"##,
3159 );
3160 }
3161
3162 #[test]
3163 fn try_option() {
3164 check_assist(
3165 extract_function,
3166 r##"
3167enum Option<T> { None, Some(T), }
3168use Option::*;
3169fn bar() -> Option<i32> { None }
3170fn foo() -> Option<()> {
3171 let n = bar()?;
3172 $0let k = foo()?;
3173 let m = k + 1;$0
3174 let h = 1 + m;
3175 Some(())
3176}"##,
3177 r##"
3178enum Option<T> { None, Some(T), }
3179use Option::*;
3180fn bar() -> Option<i32> { None }
3181fn foo() -> Option<()> {
3182 let n = bar()?;
3183 let m = fun_name()?;
3184 let h = 1 + m;
3185 Some(())
3186}
3187
3188fn $0fun_name() -> Option<i32> {
3189 let k = foo()?;
3190 let m = k + 1;
3191 Some(m)
3192}"##,
3193 );
3194 }
3195
3196 #[test]
3197 fn try_option_unit() {
3198 check_assist(
3199 extract_function,
3200 r##"
3201enum Option<T> { None, Some(T), }
3202use Option::*;
3203fn foo() -> Option<()> {
3204 let n = 1;
3205 $0let k = foo()?;
3206 let m = k + 1;$0
3207 let h = 1 + n;
3208 Some(())
3209}"##,
3210 r##"
3211enum Option<T> { None, Some(T), }
3212use Option::*;
3213fn foo() -> Option<()> {
3214 let n = 1;
3215 fun_name()?;
3216 let h = 1 + n;
3217 Some(())
3218}
3219
3220fn $0fun_name() -> Option<()> {
3221 let k = foo()?;
3222 let m = k + 1;
3223 Some(())
3224}"##,
3225 );
3226 }
3227
3228 #[test]
3229 fn try_result() {
3230 check_assist(
3231 extract_function,
3232 r##"
3233enum Result<T, E> { Ok(T), Err(E), }
3234use Result::*;
3235fn foo() -> Result<(), i64> {
3236 let n = 1;
3237 $0let k = foo()?;
3238 let m = k + 1;$0
3239 let h = 1 + m;
3240 Ok(())
3241}"##,
3242 r##"
3243enum Result<T, E> { Ok(T), Err(E), }
3244use Result::*;
3245fn foo() -> Result<(), i64> {
3246 let n = 1;
3247 let m = fun_name()?;
3248 let h = 1 + m;
3249 Ok(())
3250}
3251
3252fn $0fun_name() -> Result<i32, i64> {
3253 let k = foo()?;
3254 let m = k + 1;
3255 Ok(m)
3256}"##,
3257 );
3258 }
3259
3260 #[test]
3261 fn try_option_with_return() {
3262 check_assist(
3263 extract_function,
3264 r##"
3265enum Option<T> { None, Some(T) }
3266use Option::*;
3267fn foo() -> Option<()> {
3268 let n = 1;
3269 $0let k = foo()?;
3270 if k == 42 {
3271 return None;
3272 }
3273 let m = k + 1;$0
3274 let h = 1 + m;
3275 Some(())
3276}"##,
3277 r##"
3278enum Option<T> { None, Some(T) }
3279use Option::*;
3280fn foo() -> Option<()> {
3281 let n = 1;
3282 let m = fun_name()?;
3283 let h = 1 + m;
3284 Some(())
3285}
3286
3287fn $0fun_name() -> Option<i32> {
3288 let k = foo()?;
3289 if k == 42 {
3290 return None;
3291 }
3292 let m = k + 1;
3293 Some(m)
3294}"##,
3295 );
3296 }
3297
3298 #[test]
3299 fn try_result_with_return() {
3300 check_assist(
3301 extract_function,
3302 r##"
3303enum Result<T, E> { Ok(T), Err(E), }
3304use Result::*;
3305fn foo() -> Result<(), i64> {
3306 let n = 1;
3307 $0let k = foo()?;
3308 if k == 42 {
3309 return Err(1);
3310 }
3311 let m = k + 1;$0
3312 let h = 1 + m;
3313 Ok(())
3314}"##,
3315 r##"
3316enum Result<T, E> { Ok(T), Err(E), }
3317use Result::*;
3318fn foo() -> Result<(), i64> {
3319 let n = 1;
3320 let m = fun_name()?;
3321 let h = 1 + m;
3322 Ok(())
3323}
3324
3325fn $0fun_name() -> Result<i32, i64> {
3326 let k = foo()?;
3327 if k == 42 {
3328 return Err(1);
3329 }
3330 let m = k + 1;
3331 Ok(m)
3332}"##,
3333 );
3334 }
3335
3336 #[test]
3337 fn try_and_break() {
3338 mark::check!(external_control_flow_try_and_bc);
3339 check_assist_not_applicable(
3340 extract_function,
3341 r##"
3342enum Option<T> { None, Some(T) }
3343use Option::*;
3344fn foo() -> Option<()> {
3345 loop {
3346 let n = Some(1);
3347 $0let m = n? + 1;
3348 break;
3349 let k = 2;
3350 let k = k + 1;$0
3351 let r = n + k;
3352 }
3353 Some(())
3354}"##,
3355 );
3356 }
3357
3358 #[test]
3359 fn try_and_return_ok() {
3360 mark::check!(external_control_flow_try_and_return_non_err);
3361 check_assist_not_applicable(
3362 extract_function,
3363 r##"
3364enum Result<T, E> { Ok(T), Err(E), }
3365use Result::*;
3366fn foo() -> Result<(), i64> {
3367 let n = 1;
3368 $0let k = foo()?;
3369 if k == 42 {
3370 return Ok(1);
3371 }
3372 let m = k + 1;$0
3373 let h = 1 + m;
3374 Ok(())
3375}"##,
3376 );
3377 }
2159} 3378}
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.