From 018255efe3e456aa8d712f68a714d5c6e010d03f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 13 Nov 2019 10:27:21 +0300 Subject: Minor cleanup --- crates/ra_assists/src/assists/early_return.rs | 39 ++++++++++++++------------- 1 file changed, 20 insertions(+), 19 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/early_return.rs b/crates/ra_assists/src/assists/early_return.rs index 570a07a20..f44610001 100644 --- a/crates/ra_assists/src/assists/early_return.rs +++ b/crates/ra_assists/src/assists/early_return.rs @@ -38,27 +38,27 @@ use crate::{ // ``` pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { let if_expr: ast::IfExpr = ctx.find_node_at_offset()?; + if if_expr.else_branch().is_some() { + return None; + } + 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 if_let_ident = match cond.pat() { + None => None, // No IfLet, supported. + Some(TupleStructPat(pat)) if pat.args().count() == 1 => { + let path = pat.path()?; + match path.qualifier() { + None => Some(path.syntax().to_string()), + Some(_) => return None, + } + } + Some(_) => return None, // Unsupported IfLet. }; let expr = cond.expr()?; let then_block = if_expr.then_branch()?.block()?; - if if_expr.else_branch().is_some() { - return None; - } let parent_block = if_expr.syntax().parent()?.ancestors().find_map(ast::Block::cast)?; @@ -100,7 +100,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt let early_expression = &(early_expression.to_owned() + ";"); let new_expr = if_indent_level.increase_indent(make::if_expression(&expr, early_expression)); - replace(new_expr, &then_block, &parent_block, &if_expr) + replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) } Some(if_let_ident) => { // If-let. @@ -109,7 +109,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt &if_let_ident, early_expression, )); - replace(new_expr, &then_block, &parent_block, &if_expr) + replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) } }; edit.target(if_expr.syntax().text_range()); @@ -117,7 +117,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt edit.set_cursor(cursor_position); fn replace( - new_expr: impl AstNode, + new_expr: &SyntaxNode, then_block: &Block, parent_block: &Block, if_expr: &ast::IfExpr, @@ -130,7 +130,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt } else { end_of_then }; - let mut then_statements = new_expr.syntax().children_with_tokens().chain( + let mut then_statements = new_expr.children_with_tokens().chain( then_block_items .syntax() .children_with_tokens() @@ -151,9 +151,10 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt #[cfg(test)] mod tests { - use super::*; use crate::helpers::{check_assist, check_assist_not_applicable}; + use super::*; + #[test] fn convert_inside_fn() { check_assist( -- cgit v1.2.3 From 2a69d584d6afc842d6dc8503f3fd3a0a691ea385 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 13 Nov 2019 10:54:50 +0300 Subject: Add a bit of types --- crates/ra_assists/src/assists/early_return.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/early_return.rs b/crates/ra_assists/src/assists/early_return.rs index f44610001..8507a60fb 100644 --- a/crates/ra_assists/src/assists/early_return.rs +++ b/crates/ra_assists/src/assists/early_return.rs @@ -45,12 +45,12 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt let cond = if_expr.condition()?; // Check if there is an IfLet that we can handle. - let if_let_ident = match cond.pat() { + let bound_ident = match cond.pat() { None => None, // No IfLet, supported. Some(TupleStructPat(pat)) if pat.args().count() == 1 => { let path = pat.path()?; match path.qualifier() { - None => Some(path.syntax().to_string()), + None => Some(path.segment()?.name_ref()?), Some(_) => return None, } } @@ -94,7 +94,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 if_let_ident { + let new_block = match bound_ident { None => { // If. let early_expression = &(early_expression.to_owned() + ";"); @@ -102,11 +102,11 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt if_indent_level.increase_indent(make::if_expression(&expr, early_expression)); replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) } - Some(if_let_ident) => { + Some(bound_ident) => { // If-let. let new_expr = if_indent_level.increase_indent(make::let_match_early( expr, - &if_let_ident, + &bound_ident.syntax().to_string(), early_expression, )); replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) -- cgit v1.2.3 From e177c65e36f432821087b215b83c2dad1c97f478 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 13 Nov 2019 11:40:51 +0300 Subject: Use strongly-typed ast building for early-return assist --- crates/ra_assists/src/assists/early_return.rs | 95 ++++++++++++++++++++------- 1 file changed, 71 insertions(+), 24 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assists/early_return.rs b/crates/ra_assists/src/assists/early_return.rs index 8507a60fb..264412526 100644 --- a/crates/ra_assists/src/assists/early_return.rs +++ b/crates/ra_assists/src/assists/early_return.rs @@ -1,4 +1,4 @@ -use std::ops::RangeInclusive; +use std::{iter::once, ops::RangeInclusive}; use hir::db::HirDatabase; use ra_syntax::{ @@ -45,19 +45,22 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Opt let cond = if_expr.condition()?; // Check if there is an IfLet that we can handle. - let bound_ident = match cond.pat() { + let if_let_pat = match cond.pat() { None => None, // No IfLet, supported. Some(TupleStructPat(pat)) if pat.args().count() == 1 => { let path = pat.path()?; match path.qualifier() { - None => Some(path.segment()?.name_ref()?), + None => { + let bound_ident = pat.args().next().unwrap(); + Some((path, bound_ident)) + } Some(_) => return None, } } Some(_) => return None, // Unsupported IfLet. }; - let expr = cond.expr()?; + let cond_expr = cond.expr()?; let then_block = if_expr.then_branch()?.block()?; let parent_block = if_expr.syntax().parent()?.ancestors().find_map(ast::Block::cast)?; @@ -79,11 +82,11 @@ 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"), - _ => None, - }?; + let early_expression: ast::Expr = match parent_container.kind() { + WHILE_EXPR | LOOP_EXPR => make::expr_continue().into(), + FN_DEF => make::expr_return().into(), + _ => return None, + }; if then_block.syntax().first_child_or_token().map(|t| t.kind() == L_CURLY).is_none() { return None; @@ -94,22 +97,43 @@ 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 bound_ident { + let new_block = match if_let_pat { None => { // If. - let early_expression = &(early_expression.to_owned() + ";"); - let new_expr = - if_indent_level.increase_indent(make::if_expression(&expr, early_expression)); + let early_expression = &(early_expression.syntax().to_string() + ";"); + let new_expr = if_indent_level + .increase_indent(make::if_expression(&cond_expr, early_expression)); replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) } - Some(bound_ident) => { + Some((path, bound_ident)) => { // If-let. - let new_expr = if_indent_level.increase_indent(make::let_match_early( - expr, - &bound_ident.syntax().to_string(), - early_expression, - )); - replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) + let match_expr = { + let happy_arm = make::match_arm( + once( + make::tuple_struct_pat( + path, + once(make::bind_pat(make::name("it")).into()), + ) + .into(), + ), + make::expr_path(make::path_from_name_ref(make::name_ref("it"))).into(), + ); + + let sad_arm = make::match_arm( + // FIXME: would be cool to use `None` or `Err(_)` if appropriate + once(make::placeholder_pat().into()), + early_expression.into(), + ); + + make::expr_match(cond_expr, make::match_arm_list(vec![happy_arm, sad_arm])) + }; + + let let_stmt = make::let_stmt( + make::bind_pat(make::name(&bound_ident.syntax().to_string())).into(), + Some(match_expr.into()), + ); + let let_stmt = if_indent_level.increase_indent(let_stmt); + replace(let_stmt.syntax(), &then_block, &parent_block, &if_expr) } }; edit.target(if_expr.syntax().text_range()); @@ -205,7 +229,7 @@ mod tests { bar(); le<|>t n = match n { Some(it) => it, - None => return, + _ => return, }; foo(n); @@ -216,6 +240,29 @@ mod tests { ); } + #[test] + fn convert_if_let_result() { + check_assist( + convert_to_guarded_return, + r#" + fn main() { + if<|> let Ok(x) = Err(92) { + foo(x); + } + } + "#, + r#" + fn main() { + le<|>t x = match Err(92) { + Ok(it) => it, + _ => return, + }; + foo(x); + } + "#, + ); + } + #[test] fn convert_let_ok_inside_fn() { check_assist( @@ -236,7 +283,7 @@ mod tests { bar(); le<|>t n = match n { Ok(it) => it, - None => return, + _ => return, }; foo(n); @@ -294,7 +341,7 @@ mod tests { while true { le<|>t n = match n { Some(it) => it, - None => continue, + _ => continue, }; foo(n); bar(); @@ -351,7 +398,7 @@ mod tests { loop { le<|>t n = match n { Some(it) => it, - None => continue, + _ => continue, }; foo(n); bar(); -- cgit v1.2.3