From 73bef854ab854ab1a289944966444453e6f4aadf Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 29 Apr 2020 10:38:18 +0200 Subject: Move shared assist code to utils --- .../src/handlers/replace_let_with_if_let.rs | 16 ++----- .../src/handlers/replace_unwrap_with_match.rs | 52 ++++++++-------------- crates/ra_assists/src/utils.rs | 15 ++++++- 3 files changed, 35 insertions(+), 48 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs index bdbaae389..fb134f677 100644 --- a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs +++ b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs @@ -1,6 +1,5 @@ use std::iter::once; -use hir::Adt; use ra_syntax::{ ast::{ self, @@ -12,6 +11,7 @@ use ra_syntax::{ use crate::{ assist_ctx::{Assist, AssistCtx}, + utils::happy_try_variant, AssistId, }; @@ -45,20 +45,10 @@ pub(crate) fn replace_let_with_if_let(ctx: AssistCtx) -> Option { let init = let_stmt.initializer()?; let original_pat = let_stmt.pat()?; let ty = ctx.sema.type_of_expr(&init)?; - let enum_ = match ty.as_adt() { - Some(Adt::Enum(it)) => it, - _ => return None, - }; - let happy_case = - [("Result", "Ok"), ("Option", "Some")].iter().find_map(|(known_type, happy_case)| { - if &enum_.name(ctx.db).to_string() == known_type { - return Some(happy_case); - } - None - }); + let happy_variant = happy_try_variant(ctx.sema, &ty); ctx.add_assist(AssistId("replace_let_with_if_let"), "Replace with if-let", |edit| { - let with_placeholder: ast::Pat = match happy_case { + let with_placeholder: ast::Pat = match happy_variant { None => make::placeholder_pat().into(), Some(var_name) => make::tuple_struct_pat( make::path_unqualified(make::path_segment(make::name_ref(var_name))), diff --git a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs index 62cb7a763..89211b44a 100644 --- a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs @@ -1,12 +1,11 @@ use std::iter; use ra_syntax::{ - ast::{self, make}, + ast::{self, edit::IndentLevel, make}, AstNode, }; -use crate::{Assist, AssistCtx, AssistId}; -use ast::edit::IndentLevel; +use crate::{utils::happy_try_variant, Assist, AssistCtx, AssistId}; // Assist: replace_unwrap_with_match // @@ -38,42 +37,27 @@ pub(crate) fn replace_unwrap_with_match(ctx: AssistCtx) -> Option { } let caller = method_call.expr()?; let ty = ctx.sema.type_of_expr(&caller)?; + let happy_variant = happy_try_variant(ctx.sema, &ty)?; - let type_name = ty.as_adt()?.name(ctx.sema.db).to_string(); + ctx.add_assist(AssistId("replace_unwrap_with_match"), "Replace unwrap with match", |edit| { + let ok_path = make::path_unqualified(make::path_segment(make::name_ref(happy_variant))); + let it = make::bind_pat(make::name("a")).into(); + let ok_tuple = make::tuple_struct_pat(ok_path, iter::once(it)).into(); - for (unwrap_type, variant_name) in [("Result", "Ok"), ("Option", "Some")].iter() { - if &type_name == unwrap_type { - return ctx.add_assist( - AssistId("replace_unwrap_with_match"), - "Replace unwrap with match", - |edit| { - let ok_path = - make::path_unqualified(make::path_segment(make::name_ref(variant_name))); - let it = make::bind_pat(make::name("a")).into(); - let ok_tuple = make::tuple_struct_pat(ok_path, iter::once(it)).into(); + let bind_path = make::path_unqualified(make::path_segment(make::name_ref("a"))); + let ok_arm = make::match_arm(iter::once(ok_tuple), make::expr_path(bind_path)); - let bind_path = make::path_unqualified(make::path_segment(make::name_ref("a"))); - let ok_arm = make::match_arm(iter::once(ok_tuple), make::expr_path(bind_path)); + let unreachable_call = make::unreachable_macro_call().into(); + let err_arm = make::match_arm(iter::once(make::placeholder_pat().into()), unreachable_call); - let unreachable_call = make::unreachable_macro_call().into(); - let err_arm = make::match_arm( - iter::once(make::placeholder_pat().into()), - unreachable_call, - ); + let match_arm_list = make::match_arm_list(vec![ok_arm, err_arm]); + let match_expr = make::expr_match(caller.clone(), match_arm_list); + let match_expr = IndentLevel::from_node(method_call.syntax()).increase_indent(match_expr); - let match_arm_list = make::match_arm_list(vec![ok_arm, err_arm]); - let match_expr = make::expr_match(caller.clone(), match_arm_list); - let match_expr = - IndentLevel::from_node(method_call.syntax()).increase_indent(match_expr); - - edit.target(method_call.syntax().text_range()); - edit.set_cursor(caller.syntax().text_range().start()); - edit.replace_ast::(method_call.into(), match_expr); - }, - ); - } - } - None + edit.target(method_call.syntax().text_range()); + edit.set_cursor(caller.syntax().text_range().start()); + edit.replace_ast::(method_call.into(), match_expr); + }) } #[cfg(test)] diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs index 3d6c59bda..316c8b20f 100644 --- a/crates/ra_assists/src/utils.rs +++ b/crates/ra_assists/src/utils.rs @@ -1,7 +1,7 @@ //! Assorted functions shared by several assists. pub(crate) mod insert_use; -use hir::Semantics; +use hir::{Adt, Semantics, Type}; use ra_ide_db::RootDatabase; use ra_syntax::{ ast::{self, make, NameOwner}, @@ -99,3 +99,16 @@ fn invert_special_case(expr: &ast::Expr) -> Option { _ => None, } } + +pub(crate) fn happy_try_variant(sema: &Semantics, ty: &Type) -> Option<&'static str> { + let enum_ = match ty.as_adt() { + Some(Adt::Enum(it)) => it, + _ => return None, + }; + [("Result", "Ok"), ("Option", "Some")].iter().find_map(|(known_type, happy_case)| { + if &enum_.name(sema.db).to_string() == known_type { + return Some(*happy_case); + } + None + }) +} -- cgit v1.2.3 From 7c3c289dab46d1dbe9196549c81307f291e631f7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 29 Apr 2020 11:57:06 +0200 Subject: Use specific pattern when translating if-let-else to match We *probably* should actually use the same machinery here, as we do for fill match arms, but just special-casing options and results seems to be a good first step. --- .../src/handlers/replace_if_let_with_match.rs | 78 ++++++++++++++++++++-- .../src/handlers/replace_let_with_if_let.rs | 4 +- .../src/handlers/replace_unwrap_with_match.rs | 4 +- crates/ra_assists/src/utils.rs | 57 +++++++++++++--- 4 files changed, 123 insertions(+), 20 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/replace_if_let_with_match.rs b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs index 0a0a88f3d..9841f6980 100644 --- a/crates/ra_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs @@ -1,11 +1,10 @@ use ra_fmt::unwrap_trivial_block; use ra_syntax::{ - ast::{self, make}, + ast::{self, edit::IndentLevel, make}, AstNode, }; -use crate::{Assist, AssistCtx, AssistId}; -use ast::edit::IndentLevel; +use crate::{utils::TryEnum, Assist, AssistCtx, AssistId}; // Assist: replace_if_let_with_match // @@ -44,15 +43,21 @@ pub(crate) fn replace_if_let_with_match(ctx: AssistCtx) -> Option { ast::ElseBranch::IfExpr(_) => return None, }; - ctx.add_assist(AssistId("replace_if_let_with_match"), "Replace with match", |edit| { + let sema = ctx.sema; + ctx.add_assist(AssistId("replace_if_let_with_match"), "Replace with match", move |edit| { let match_expr = { let then_arm = { let then_expr = unwrap_trivial_block(then_block); - make::match_arm(vec![pat], then_expr) + make::match_arm(vec![pat.clone()], then_expr) }; let else_arm = { + let pattern = sema + .type_of_pat(&pat) + .and_then(|ty| TryEnum::from_ty(sema, &ty)) + .map(|it| it.sad_pattern()) + .unwrap_or_else(|| make::placeholder_pat().into()); let else_expr = unwrap_trivial_block(else_block); - make::match_arm(vec![make::placeholder_pat().into()], else_expr) + make::match_arm(vec![pattern], else_expr) }; make::expr_match(expr, make::match_arm_list(vec![then_arm, else_arm])) }; @@ -68,6 +73,7 @@ pub(crate) fn replace_if_let_with_match(ctx: AssistCtx) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::helpers::{check_assist, check_assist_target}; #[test] @@ -145,4 +151,64 @@ impl VariantData { }", ); } + + #[test] + fn special_case_option() { + check_assist( + replace_if_let_with_match, + r#" +enum Option { Some(T), None } +use Option::*; + +fn foo(x: Option) { + <|>if let Some(x) = x { + println!("{}", x) + } else { + println!("none") + } +} + "#, + r#" +enum Option { Some(T), None } +use Option::*; + +fn foo(x: Option) { + <|>match x { + Some(x) => println!("{}", x), + None => println!("none"), + } +} + "#, + ); + } + + #[test] + fn special_case_result() { + check_assist( + replace_if_let_with_match, + r#" +enum Result { Ok(T), Err(E) } +use Result::*; + +fn foo(x: Result) { + <|>if let Ok(x) = x { + println!("{}", x) + } else { + println!("none") + } +} + "#, + r#" +enum Result { Ok(T), Err(E) } +use Result::*; + +fn foo(x: Result) { + <|>match x { + Ok(x) => println!("{}", x), + Err(_) => println!("none"), + } +} + "#, + ); + } } diff --git a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs index fb134f677..0cf23b754 100644 --- a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs +++ b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs @@ -11,7 +11,7 @@ use ra_syntax::{ use crate::{ assist_ctx::{Assist, AssistCtx}, - utils::happy_try_variant, + utils::TryEnum, AssistId, }; @@ -45,7 +45,7 @@ pub(crate) fn replace_let_with_if_let(ctx: AssistCtx) -> Option { let init = let_stmt.initializer()?; let original_pat = let_stmt.pat()?; let ty = ctx.sema.type_of_expr(&init)?; - let happy_variant = happy_try_variant(ctx.sema, &ty); + let happy_variant = TryEnum::from_ty(ctx.sema, &ty).map(|it| it.happy_case()); ctx.add_assist(AssistId("replace_let_with_if_let"), "Replace with if-let", |edit| { let with_placeholder: ast::Pat = match happy_variant { diff --git a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs index 89211b44a..62d4ea522 100644 --- a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs @@ -5,7 +5,7 @@ use ra_syntax::{ AstNode, }; -use crate::{utils::happy_try_variant, Assist, AssistCtx, AssistId}; +use crate::{utils::TryEnum, Assist, AssistCtx, AssistId}; // Assist: replace_unwrap_with_match // @@ -37,7 +37,7 @@ pub(crate) fn replace_unwrap_with_match(ctx: AssistCtx) -> Option { } let caller = method_call.expr()?; let ty = ctx.sema.type_of_expr(&caller)?; - let happy_variant = happy_try_variant(ctx.sema, &ty)?; + let happy_variant = TryEnum::from_ty(ctx.sema, &ty)?.happy_case(); ctx.add_assist(AssistId("replace_unwrap_with_match"), "Replace unwrap with match", |edit| { let ok_path = make::path_unqualified(make::path_segment(make::name_ref(happy_variant))); diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs index 316c8b20f..61f8bd1dc 100644 --- a/crates/ra_assists/src/utils.rs +++ b/crates/ra_assists/src/utils.rs @@ -1,6 +1,8 @@ //! Assorted functions shared by several assists. pub(crate) mod insert_use; +use std::iter; + use hir::{Adt, Semantics, Type}; use ra_ide_db::RootDatabase; use ra_syntax::{ @@ -100,15 +102,50 @@ fn invert_special_case(expr: &ast::Expr) -> Option { } } -pub(crate) fn happy_try_variant(sema: &Semantics, ty: &Type) -> Option<&'static str> { - let enum_ = match ty.as_adt() { - Some(Adt::Enum(it)) => it, - _ => return None, - }; - [("Result", "Ok"), ("Option", "Some")].iter().find_map(|(known_type, happy_case)| { - if &enum_.name(sema.db).to_string() == known_type { - return Some(*happy_case); +#[derive(Clone, Copy)] +pub(crate) enum TryEnum { + Result, + Option, +} + +impl TryEnum { + const ALL: [TryEnum; 2] = [TryEnum::Option, TryEnum::Result]; + + pub(crate) fn from_ty(sema: &Semantics, ty: &Type) -> Option { + let enum_ = match ty.as_adt() { + Some(Adt::Enum(it)) => it, + _ => return None, + }; + TryEnum::ALL.iter().find_map(|&var| { + if &enum_.name(sema.db).to_string() == var.type_name() { + return Some(var); + } + None + }) + } + + pub(crate) fn happy_case(self) -> &'static str { + match self { + TryEnum::Result => "Ok", + TryEnum::Option => "Some", } - None - }) + } + + pub(crate) fn sad_pattern(self) -> ast::Pat { + match self { + TryEnum::Result => make::tuple_struct_pat( + make::path_unqualified(make::path_segment(make::name_ref("Err"))), + iter::once(make::placeholder_pat().into()), + ) + .into(), + TryEnum::Option => make::bind_pat(make::name("None")).into(), + } + } + + fn type_name(self) -> &'static str { + match self { + TryEnum::Result => "Result", + TryEnum::Option => "Option", + } + } } -- cgit v1.2.3