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') 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