4 files changed, 127 insertions, 89 deletions

diff --git a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
index 03806724a..49deb6701 100644
--- a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
+++ b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
@@ -1,11 +1,12 @@
+use ra_ide_db::RootDatabase;
 use ra_syntax::{
     ast::{self, AstNode, NameOwner},
     TextSize,
 };
 use stdx::format_to;
 
-use crate::{Assist, AssistCtx, AssistId};
-use ra_ide_db::RootDatabase;
+use crate::{utils::FamousDefs, Assist, AssistCtx, AssistId};
+use test_utils::tested_by;
 
 // Assist add_from_impl_for_enum
 //
@@ -41,7 +42,8 @@ pub(crate) fn add_from_impl_for_enum(ctx: AssistCtx) -> Option<Assist> {
         _ => return None,
     };
 
-    if already_has_from_impl(ctx.sema, &variant) {
+    if existing_from_impl(ctx.sema, &variant).is_some() {
+        tested_by!(test_add_from_impl_already_exists);
         return None;
     }
 
@@ -70,41 +72,33 @@ impl From<{0}> for {1} {{
     )
 }
 
-fn already_has_from_impl(
+fn existing_from_impl(
     sema: &'_ hir::Semantics<'_, RootDatabase>,
     variant: &ast::EnumVariant,
-) -> bool {
-    let scope = sema.scope(&variant.syntax());
+) -> Option<()> {
+    let variant = sema.to_def(variant)?;
+    let enum_ = variant.parent_enum(sema.db);
+    let krate = enum_.module(sema.db).krate();
 
-    let from_path = ast::make::path_from_text("From");
-    let from_hir_path = match hir::Path::from_ast(from_path) {
-        Some(p) => p,
-        None => return false,
-    };
-    let from_trait = match scope.resolve_hir_path(&from_hir_path) {
-        Some(hir::PathResolution::Def(hir::ModuleDef::Trait(t))) => t,
-        _ => return false,
-    };
+    let from_trait = FamousDefs(sema, krate).core_convert_From()?;
 
-    let e: hir::Enum = match sema.to_def(&variant.parent_enum()) {
-        Some(e) => e,
-        None => return false,
-    };
-    let e_ty = e.ty(sema.db);
+    let enum_type = enum_.ty(sema.db);
 
-    let hir_enum_var: hir::EnumVariant = match sema.to_def(variant) {
-        Some(ev) => ev,
-        None => return false,
-    };
-    let var_ty = hir_enum_var.fields(sema.db)[0].signature_ty(sema.db);
+    let wrapped_type = variant.fields(sema.db).get(0)?.signature_ty(sema.db);
 
-    e_ty.impls_trait(sema.db, from_trait, &[var_ty])
+    if enum_type.impls_trait(sema.db, from_trait, &[wrapped_type]) {
+        Some(())
+    } else {
+        None
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+
     use crate::helpers::{check_assist, check_assist_not_applicable};
+    use test_utils::covers;
 
     #[test]
     fn test_add_from_impl_for_enum() {
@@ -136,36 +130,40 @@ mod tests {
         );
     }
 
+    fn check_not_applicable(ra_fixture: &str) {
+        let fixture =
+            format!("//- main.rs crate:main deps:core\n{}\n{}", ra_fixture, FamousDefs::FIXTURE);
+        check_assist_not_applicable(add_from_impl_for_enum, &fixture)
+    }
+
     #[test]
     fn test_add_from_impl_no_element() {
-        check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One }");
+        check_not_applicable("enum A { <|>One }");
     }
 
     #[test]
     fn test_add_from_impl_more_than_one_element_in_tuple() {
-        check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One(u32, String) }");
+        check_not_applicable("enum A { <|>One(u32, String) }");
     }
 
     #[test]
     fn test_add_from_impl_struct_variant() {
-        check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One { x: u32 } }");
+        check_not_applicable("enum A { <|>One { x: u32 } }");
     }
 
     #[test]
     fn test_add_from_impl_already_exists() {
-        check_assist_not_applicable(
-            add_from_impl_for_enum,
-            r#"enum A { <|>One(u32), }
+        covers!(test_add_from_impl_already_exists);
+        check_not_applicable(
+            r#"
+enum A { <|>One(u32), }
 
 impl From<u32> for A {
     fn from(v: u32) -> Self {
         A::One(v)
     }
 }
-
-pub trait From<T> {
-    fn from(T) -> Self;
-}"#,
+"#,
         );
     }
 
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<Assist> {
         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<Assist> {
 #[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<T> { Some(T), None }
+use Option::*;
+
+fn foo(x: Option<i32>) {
+    <|>if let Some(x) = x {
+        println!("{}", x)
+    } else {
+        println!("none")
+    }
+}
+           "#,
+            r#"
+enum Option<T> { Some(T), None }
+use Option::*;
+
+fn foo(x: Option<i32>) {
+    <|>match x {
+        Some(x) => println!("{}", x),
+        None => println!("none"),
+    }
+}
+           "#,
+        );
+    }
+
+    #[test]
+    fn special_case_result() {
+        check_assist(
+            replace_if_let_with_match,
+            r#"
+enum Result<T, E> { Ok(T), Err(E) }
+use Result::*;
+
+fn foo(x: Result<i32, ()>) {
+    <|>if let Ok(x) = x {
+        println!("{}", x)
+    } else {
+        println!("none")
+    }
+}
+           "#,
+            r#"
+enum Result<T, E> { Ok(T), Err(E) }
+use Result::*;
+
+fn foo(x: Result<i32, ()>) {
+    <|>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 bdbaae389..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
@@ -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::TryEnum,
     AssistId,
 };
 
@@ -45,20 +45,10 @@ pub(crate) fn replace_let_with_if_let(ctx: AssistCtx) -> Option<Assist> {
     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 = 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_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..62d4ea522 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::TryEnum, Assist, AssistCtx, AssistId};
 
 // Assist: replace_unwrap_with_match
 //
@@ -38,42 +37,27 @@ pub(crate) fn replace_unwrap_with_match(ctx: AssistCtx) -> Option<Assist> {
     }
     let caller = method_call.expr()?;
     let ty = ctx.sema.type_of_expr(&caller)?;
+    let happy_variant = TryEnum::from_ty(ctx.sema, &ty)?.happy_case();
 
-    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::<ast::Expr>(method_call.into(), match_expr);
-                },
-            );
-        }
-    }
-    None
+        edit.target(method_call.syntax().text_range());
+        edit.set_cursor(caller.syntax().text_range().start());
+        edit.replace_ast::<ast::Expr>(method_call.into(), match_expr);
+    })
 }
 
 #[cfg(test)]