From 4a4d9f7a90dc0605992c4f774a8d9a1323ad6d1e Mon Sep 17 00:00:00 2001 From: krk Date: Thu, 31 Oct 2019 21:10:58 +0100 Subject: Handle IfLet in convert_to_guarded_return. --- crates/ra_assists/src/assists/early_return.rs | 183 ++++++++++++++++++++++---- 1 file changed, 155 insertions(+), 28 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/assists/early_return.rs b/crates/ra_assists/src/assists/early_return.rs index ad6c5695a..827170f8f 100644 --- a/crates/ra_assists/src/assists/early_return.rs +++ b/crates/ra_assists/src/assists/early_return.rs @@ -37,7 +37,9 @@ use crate::{ // ``` pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { let if_expr: ast::IfExpr = ctx.find_node_at_offset()?; - let expr = if_expr.condition()?.expr()?; + let cond = if_expr.condition()?; + let pat = &cond.pat(); + let expr = cond.expr()?; let then_block = if_expr.then_branch()?.block()?; if if_expr.else_branch().is_some() { return None; @@ -63,8 +65,8 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt let parent_container = parent_block.syntax().parent()?.parent()?; let early_expression = match parent_container.kind() { - WHILE_EXPR | LOOP_EXPR => Some("continue;"), - FN_DEF => Some("return;"), + WHILE_EXPR | LOOP_EXPR => Some("continue"), + FN_DEF => Some("return"), _ => None, }?; @@ -77,31 +79,67 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt ctx.add_assist(AssistId("convert_to_guarded_return"), "convert to guarded return", |edit| { let if_indent_level = IndentLevel::from_node(&if_expr.syntax()); - let new_if_expr = - if_indent_level.increase_indent(make::if_expression(&expr, early_expression)); - let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); - let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); - let end_of_then = - if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { - end_of_then.prev_sibling_or_token().unwrap() - } else { - end_of_then - }; - let mut new_if_and_then_statements = new_if_expr.syntax().children_with_tokens().chain( - then_block_items - .syntax() - .children_with_tokens() - .skip(1) - .take_while(|i| *i != end_of_then), - ); - let new_block = replace_children( - &parent_block.syntax(), - RangeInclusive::new( - if_expr.clone().syntax().clone().into(), - if_expr.syntax().clone().into(), - ), - &mut new_if_and_then_statements, - ); + let new_block = match pat { + None => { + // If. + let early_expression = &(early_expression.to_owned() + ";"); + let new_if_expr = + if_indent_level.increase_indent(make::if_expression(&expr, early_expression)); + let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); + let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); + let end_of_then = + if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { + end_of_then.prev_sibling_or_token().unwrap() + } else { + end_of_then + }; + let mut new_if_and_then_statements = + new_if_expr.syntax().children_with_tokens().chain( + then_block_items + .syntax() + .children_with_tokens() + .skip(1) + .take_while(|i| *i != end_of_then), + ); + replace_children( + &parent_block.syntax(), + RangeInclusive::new( + if_expr.clone().syntax().clone().into(), + if_expr.syntax().clone().into(), + ), + &mut new_if_and_then_statements, + ) + } + _ => { + // If-let. + let new_match_expr = + if_indent_level.increase_indent(make::let_match_early(expr, early_expression)); + let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); + let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); + let end_of_then = + if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { + end_of_then.prev_sibling_or_token().unwrap() + } else { + end_of_then + }; + let mut then_statements = new_match_expr.syntax().children_with_tokens().chain( + then_block_items + .syntax() + .children_with_tokens() + .skip(1) + .take_while(|i| *i != end_of_then), + ); + let new_block = replace_children( + &parent_block.syntax(), + RangeInclusive::new( + if_expr.clone().syntax().clone().into(), + if_expr.syntax().clone().into(), + ), + &mut then_statements, + ); + new_block + } + }; edit.target(if_expr.syntax().text_range()); edit.replace_ast(parent_block, ast::Block::cast(new_block).unwrap()); edit.set_cursor(cursor_position); @@ -143,6 +181,37 @@ mod tests { ); } + #[test] + fn convert_let_inside_fn() { + check_assist( + convert_to_guarded_return, + r#" + fn main(n: Option) { + bar(); + if<|> let Some(n) = n { + foo(n); + + //comment + bar(); + } + } + "#, + r#" + fn main(n: Option) { + bar(); + le<|>t n = match n { + Some(it) => it, + None => return, + }; + foo(n); + + //comment + bar(); + } + "#, + ); + } + #[test] fn convert_inside_while() { check_assist( @@ -171,6 +240,35 @@ mod tests { ); } + #[test] + fn convert_let_inside_while() { + check_assist( + convert_to_guarded_return, + r#" + fn main() { + while true { + if<|> let Some(n) = n { + foo(n); + bar(); + } + } + } + "#, + r#" + fn main() { + while true { + le<|>t n = match n { + Some(it) => it, + None => continue, + }; + foo(n); + bar(); + } + } + "#, + ); + } + #[test] fn convert_inside_loop() { check_assist( @@ -199,6 +297,35 @@ mod tests { ); } + #[test] + fn convert_let_inside_loop() { + check_assist( + convert_to_guarded_return, + r#" + fn main() { + loop { + if<|> let Some(n) = n { + foo(n); + bar(); + } + } + } + "#, + r#" + fn main() { + loop { + le<|>t n = match n { + Some(it) => it, + None => continue, + }; + foo(n); + bar(); + } + } + "#, + ); + } + #[test] fn ignore_already_converted_if() { check_assist_not_applicable( -- cgit v1.2.3 From 1841a39f865b38041e84cd80840587dbe57d62c1 Mon Sep 17 00:00:00 2001 From: krk Date: Fri, 1 Nov 2019 16:58:09 +0100 Subject: Remove variable pat. --- crates/ra_assists/src/assists/early_return.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/assists/early_return.rs b/crates/ra_assists/src/assists/early_return.rs index 827170f8f..4322d3737 100644 --- a/crates/ra_assists/src/assists/early_return.rs +++ b/crates/ra_assists/src/assists/early_return.rs @@ -38,7 +38,6 @@ use crate::{ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { let if_expr: ast::IfExpr = ctx.find_node_at_offset()?; let cond = if_expr.condition()?; - let pat = &cond.pat(); let expr = cond.expr()?; let then_block = if_expr.then_branch()?.block()?; if if_expr.else_branch().is_some() { @@ -79,7 +78,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt ctx.add_assist(AssistId("convert_to_guarded_return"), "convert to guarded return", |edit| { let if_indent_level = IndentLevel::from_node(&if_expr.syntax()); - let new_block = match pat { + let new_block = match cond.pat() { None => { // If. let early_expression = &(early_expression.to_owned() + ";"); -- cgit v1.2.3 From 91ab3f876020872f1f692577792569b7b6c8239d Mon Sep 17 00:00:00 2001 From: krk Date: Fri, 1 Nov 2019 18:18:58 +0100 Subject: Support paths other than "Some". --- crates/ra_assists/src/assists/early_return.rs | 64 +++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 8 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/assists/early_return.rs b/crates/ra_assists/src/assists/early_return.rs index 4322d3737..8536b015b 100644 --- a/crates/ra_assists/src/assists/early_return.rs +++ b/crates/ra_assists/src/assists/early_return.rs @@ -3,7 +3,7 @@ use std::ops::RangeInclusive; use hir::db::HirDatabase; use ra_syntax::{ algo::replace_children, - ast::{self, edit::IndentLevel, make}, + ast::{self, edit::IndentLevel, make, Pat::TupleStructPat}, AstNode, SyntaxKind::{FN_DEF, LOOP_EXPR, L_CURLY, R_CURLY, WHILE_EXPR, WHITESPACE}, }; @@ -38,6 +38,21 @@ use crate::{ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { let if_expr: ast::IfExpr = ctx.find_node_at_offset()?; let cond = if_expr.condition()?; + let mut if_let_ident: Option = None; + + // Check if there is an IfLet that we can handle. + match cond.pat() { + None => {} // No IfLet, supported. + Some(TupleStructPat(ref pat)) if pat.args().count() == 1usize => match &pat.path() { + Some(p) => match p.qualifier() { + None => if_let_ident = Some(p.syntax().text().to_string()), + _ => return None, + }, + _ => return None, + }, + _ => return None, // Unsupported IfLet. + }; + let expr = cond.expr()?; let then_block = if_expr.then_branch()?.block()?; if if_expr.else_branch().is_some() { @@ -78,7 +93,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt ctx.add_assist(AssistId("convert_to_guarded_return"), "convert to guarded return", |edit| { let if_indent_level = IndentLevel::from_node(&if_expr.syntax()); - let new_block = match cond.pat() { + let new_block = match if_let_ident { None => { // If. let early_expression = &(early_expression.to_owned() + ";"); @@ -109,10 +124,13 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt &mut new_if_and_then_statements, ) } - _ => { + Some(if_let_ident) => { // If-let. - let new_match_expr = - if_indent_level.increase_indent(make::let_match_early(expr, early_expression)); + let new_match_expr = if_indent_level.increase_indent(make::let_match_early( + expr, + &if_let_ident, + early_expression, + )); let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); let end_of_then = @@ -128,15 +146,14 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt .skip(1) .take_while(|i| *i != end_of_then), ); - let new_block = replace_children( + replace_children( &parent_block.syntax(), RangeInclusive::new( if_expr.clone().syntax().clone().into(), if_expr.syntax().clone().into(), ), &mut then_statements, - ); - new_block + ) } }; edit.target(if_expr.syntax().text_range()); @@ -211,6 +228,37 @@ mod tests { ); } + #[test] + fn convert_let_ok_inside_fn() { + check_assist( + convert_to_guarded_return, + r#" + fn main(n: Option) { + bar(); + if<|> let Ok(n) = n { + foo(n); + + //comment + bar(); + } + } + "#, + r#" + fn main(n: Option) { + bar(); + le<|>t n = match n { + Ok(it) => it, + None => return, + }; + foo(n); + + //comment + bar(); + } + "#, + ); + } + #[test] fn convert_inside_while() { check_assist( -- cgit v1.2.3 From bc14f500a0b0e1349d0f795b85dde5946da113bd Mon Sep 17 00:00:00 2001 From: krk Date: Fri, 1 Nov 2019 18:34:42 +0100 Subject: Extract common parts of match arms in convert_to_guarded_return assist. --- crates/ra_assists/src/assists/early_return.rs | 87 ++++++++++++--------------- 1 file changed, 37 insertions(+), 50 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/assists/early_return.rs b/crates/ra_assists/src/assists/early_return.rs index 8536b015b..570a07a20 100644 --- a/crates/ra_assists/src/assists/early_return.rs +++ b/crates/ra_assists/src/assists/early_return.rs @@ -3,9 +3,10 @@ use std::ops::RangeInclusive; use hir::db::HirDatabase; use ra_syntax::{ algo::replace_children, - ast::{self, edit::IndentLevel, make, Pat::TupleStructPat}, + ast::{self, edit::IndentLevel, make, Block, Pat::TupleStructPat}, AstNode, SyntaxKind::{FN_DEF, LOOP_EXPR, L_CURLY, R_CURLY, WHILE_EXPR, WHITESPACE}, + SyntaxNode, }; use crate::{ @@ -97,68 +98,54 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt None => { // If. let early_expression = &(early_expression.to_owned() + ";"); - let new_if_expr = + let new_expr = if_indent_level.increase_indent(make::if_expression(&expr, early_expression)); - let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); - let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); - let end_of_then = - if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { - end_of_then.prev_sibling_or_token().unwrap() - } else { - end_of_then - }; - let mut new_if_and_then_statements = - new_if_expr.syntax().children_with_tokens().chain( - then_block_items - .syntax() - .children_with_tokens() - .skip(1) - .take_while(|i| *i != end_of_then), - ); - replace_children( - &parent_block.syntax(), - RangeInclusive::new( - if_expr.clone().syntax().clone().into(), - if_expr.syntax().clone().into(), - ), - &mut new_if_and_then_statements, - ) + replace(new_expr, &then_block, &parent_block, &if_expr) } Some(if_let_ident) => { // If-let. - let new_match_expr = if_indent_level.increase_indent(make::let_match_early( + let new_expr = if_indent_level.increase_indent(make::let_match_early( expr, &if_let_ident, early_expression, )); - let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); - let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); - let end_of_then = - if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { - end_of_then.prev_sibling_or_token().unwrap() - } else { - end_of_then - }; - let mut then_statements = new_match_expr.syntax().children_with_tokens().chain( - then_block_items - .syntax() - .children_with_tokens() - .skip(1) - .take_while(|i| *i != end_of_then), - ); - replace_children( - &parent_block.syntax(), - RangeInclusive::new( - if_expr.clone().syntax().clone().into(), - if_expr.syntax().clone().into(), - ), - &mut then_statements, - ) + replace(new_expr, &then_block, &parent_block, &if_expr) } }; edit.target(if_expr.syntax().text_range()); edit.replace_ast(parent_block, ast::Block::cast(new_block).unwrap()); edit.set_cursor(cursor_position); + + fn replace( + new_expr: impl AstNode, + then_block: &Block, + parent_block: &Block, + if_expr: &ast::IfExpr, + ) -> SyntaxNode { + let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); + let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); + let end_of_then = + if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { + end_of_then.prev_sibling_or_token().unwrap() + } else { + end_of_then + }; + let mut then_statements = new_expr.syntax().children_with_tokens().chain( + then_block_items + .syntax() + .children_with_tokens() + .skip(1) + .take_while(|i| *i != end_of_then), + ); + replace_children( + &parent_block.syntax(), + RangeInclusive::new( + if_expr.clone().syntax().clone().into(), + if_expr.syntax().clone().into(), + ), + &mut then_statements, + ) + } }) } -- cgit v1.2.3