6 files changed, 237 insertions, 90 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)]
diff --git a/crates/ra_assists/src/marks.rs b/crates/ra_assists/src/marks.rs
index 6c2a2b8b6..8d910205f 100644
--- a/crates/ra_assists/src/marks.rs
+++ b/crates/ra_assists/src/marks.rs
@@ -8,4 +8,5 @@ test_utils::marks![
     test_not_inline_mut_variable
     test_not_applicable_if_variable_unused
     change_visibility_field_false_positive
+    test_add_from_impl_already_exists
 ];
diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs
index 3d6c59bda..efd988697 100644
--- a/crates/ra_assists/src/utils.rs
+++ b/crates/ra_assists/src/utils.rs
@@ -1,7 +1,9 @@
 //! Assorted functions shared by several assists.
 pub(crate) mod insert_use;
 
-use hir::Semantics;
+use std::iter;
+
+use hir::{Adt, Crate, Semantics, Trait, Type};
 use ra_ide_db::RootDatabase;
 use ra_syntax::{
     ast::{self, make, NameOwner},
@@ -99,3 +101,109 @@ fn invert_special_case(expr: &ast::Expr) -> Option<ast::Expr> {
         _ => None,
     }
 }
+
+#[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<RootDatabase>, ty: &Type) -> Option<TryEnum> {
+        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",
+        }
+    }
+
+    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",
+        }
+    }
+}
+
+/// Helps with finding well-know things inside the standard library. This is
+/// somewhat similar to the known paths infra inside hir, but it different; We
+/// want to make sure that IDE specific paths don't become interesting inside
+/// the compiler itself as well.
+pub(crate) struct FamousDefs<'a, 'b>(pub(crate) &'a Semantics<'b, RootDatabase>, pub(crate) Crate);
+
+#[allow(non_snake_case)]
+impl FamousDefs<'_, '_> {
+    #[cfg(test)]
+    pub(crate) const FIXTURE: &'static str = r#"
+//- /libcore.rs crate:core
+pub mod convert{
+    pub trait From<T> {
+        fn from(T) -> Self;
+    }
+}
+
+pub mod prelude { pub use crate::convert::From }
+#[prelude_import]
+pub use prelude::*;
+"#;
+
+    pub(crate) fn core_convert_From(&self) -> Option<Trait> {
+        self.find_trait("core:convert:From")
+    }
+
+    fn find_trait(&self, path: &str) -> Option<Trait> {
+        let db = self.0.db;
+        let mut path = path.split(':');
+        let trait_ = path.next_back()?;
+        let std_crate = path.next()?;
+        let std_crate = self
+            .1
+            .dependencies(db)
+            .into_iter()
+            .find(|dep| &dep.name.to_string() == std_crate)?
+            .krate;
+
+        let mut module = std_crate.root_module(db)?;
+        for segment in path {
+            module = module.children(db).find_map(|child| {
+                let name = child.name(db)?;
+                if &name.to_string() == segment {
+                    Some(child)
+                } else {
+                    None
+                }
+            })?;
+        }
+        let def =
+            module.scope(db, None).into_iter().find(|(name, _def)| &name.to_string() == trait_)?.1;
+        match def {
+            hir::ScopeDef::ModuleDef(hir::ModuleDef::Trait(it)) => Some(it),
+            _ => None,
+        }
+    }
+}