3 files changed, 88 insertions, 36 deletions

diff --git a/crates/ra_assists/src/handlers/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
index 448697d31..ab21388c8 100644
--- a/crates/ra_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
@@ -1,4 +1,4 @@
-use hir::{db::HirDatabase, HasSource, InFile};
+use hir::{HasSource, InFile};
 use ra_syntax::{
     ast::{self, edit, make, AstNode, NameOwner},
     SmolStr,
@@ -6,6 +6,7 @@ use ra_syntax::{
 
 use crate::{
     ast_transform::{self, AstTransform, QualifyPaths, SubstituteTypeParams},
+    utils::{get_missing_impl_items, resolve_target_trait},
     Assist, AssistCtx, AssistId,
 };
 
@@ -103,11 +104,9 @@ fn add_missing_impl_members_inner(
     let impl_node = ctx.find_node_at_offset::<ast::ImplBlock>()?;
     let impl_item_list = impl_node.item_list()?;
 
-    let (trait_, trait_def) = {
-        let analyzer = ctx.source_analyzer(impl_node.syntax(), None);
+    let analyzer = ctx.source_analyzer(impl_node.syntax(), None);
 
-        resolve_target_trait_def(ctx.db, &analyzer, &impl_node)?
-    };
+    let trait_ = resolve_target_trait(ctx.db, &analyzer, &impl_node)?;
 
     let def_name = |item: &ast::ImplItem| -> Option<SmolStr> {
         match item {
@@ -118,11 +117,14 @@ fn add_missing_impl_members_inner(
         .map(|it| it.text().clone())
     };
 
-    let trait_items = trait_def.item_list()?.impl_items();
-    let impl_items = impl_item_list.impl_items().collect::<Vec<_>>();
-
-    let missing_items: Vec<_> = trait_items
-        .filter(|t| def_name(t).is_some())
+    let missing_items = get_missing_impl_items(ctx.db, &analyzer, &impl_node)
+        .iter()
+        .map(|i| match i {
+            hir::AssocItem::Function(i) => ast::ImplItem::FnDef(i.source(ctx.db).value),
+            hir::AssocItem::TypeAlias(i) => ast::ImplItem::TypeAliasDef(i.source(ctx.db).value),
+            hir::AssocItem::Const(i) => ast::ImplItem::ConstDef(i.source(ctx.db).value),
+        })
+        .filter(|t| def_name(&t).is_some())
         .filter(|t| match t {
             ast::ImplItem::FnDef(def) => match mode {
                 AddMissingImplMembersMode::DefaultMethodsOnly => def.body().is_some(),
@@ -130,8 +132,8 @@ fn add_missing_impl_members_inner(
             },
             _ => mode == AddMissingImplMembersMode::NoDefaultMethods,
         })
-        .filter(|t| impl_items.iter().all(|i| def_name(i) != def_name(t)))
-        .collect();
+        .collect::<Vec<_>>();
+
     if missing_items.is_empty() {
         return None;
     }
@@ -177,27 +179,6 @@ fn add_body(fn_def: ast::FnDef) -> ast::FnDef {
     }
 }
 
-/// Given an `ast::ImplBlock`, resolves the target trait (the one being
-/// implemented) to a `ast::TraitDef`.
-fn resolve_target_trait_def(
-    db: &impl HirDatabase,
-    analyzer: &hir::SourceAnalyzer,
-    impl_block: &ast::ImplBlock,
-) -> Option<(hir::Trait, ast::TraitDef)> {
-    let ast_path = impl_block
-        .target_trait()
-        .map(|it| it.syntax().clone())
-        .and_then(ast::PathType::cast)?
-        .path()?;
-
-    match analyzer.resolve_path(db, &ast_path) {
-        Some(hir::PathResolution::Def(hir::ModuleDef::Trait(def))) => {
-            Some((def, def.source(db).value))
-        }
-        _ => None,
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index 828a8e9e8..cb124eaf0 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -9,7 +9,7 @@ mod assist_ctx;
 mod marks;
 #[cfg(test)]
 mod doc_tests;
-mod utils;
+pub mod utils;
 pub mod ast_transform;
 
 use ra_db::FileRange;
diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs
index 0d5722295..6ff44c95c 100644
--- a/crates/ra_assists/src/utils.rs
+++ b/crates/ra_assists/src/utils.rs
@@ -1,10 +1,81 @@
 //! Assorted functions shared by several assists.
 
 use ra_syntax::{
-    ast::{self, make},
-    T,
+    ast::{self, make, NameOwner},
+    AstNode, T,
 };
 
+use hir::db::HirDatabase;
+use rustc_hash::FxHashSet;
+
+pub fn get_missing_impl_items(
+    db: &impl HirDatabase,
+    analyzer: &hir::SourceAnalyzer,
+    impl_block: &ast::ImplBlock,
+) -> Vec<hir::AssocItem> {
+    // Names must be unique between constants and functions. However, type aliases
+    // may share the same name as a function or constant.
+    let mut impl_fns_consts = FxHashSet::default();
+    let mut impl_type = FxHashSet::default();
+
+    if let Some(item_list) = impl_block.item_list() {
+        for item in item_list.impl_items() {
+            match item {
+                ast::ImplItem::FnDef(f) => {
+                    if let Some(n) = f.name() {
+                        impl_fns_consts.insert(n.syntax().to_string());
+                    }
+                }
+
+                ast::ImplItem::TypeAliasDef(t) => {
+                    if let Some(n) = t.name() {
+                        impl_type.insert(n.syntax().to_string());
+                    }
+                }
+
+                ast::ImplItem::ConstDef(c) => {
+                    if let Some(n) = c.name() {
+                        impl_fns_consts.insert(n.syntax().to_string());
+                    }
+                }
+            }
+        }
+    }
+
+    resolve_target_trait(db, analyzer, impl_block).map_or(vec![], |target_trait| {
+        target_trait
+            .items(db)
+            .iter()
+            .filter(|i| match i {
+                hir::AssocItem::Function(f) => !impl_fns_consts.contains(&f.name(db).to_string()),
+                hir::AssocItem::TypeAlias(t) => !impl_type.contains(&t.name(db).to_string()),
+                hir::AssocItem::Const(c) => c
+                    .name(db)
+                    .map(|n| !impl_fns_consts.contains(&n.to_string()))
+                    .unwrap_or_default(),
+            })
+            .cloned()
+            .collect()
+    })
+}
+
+pub(crate) fn resolve_target_trait(
+    db: &impl HirDatabase,
+    analyzer: &hir::SourceAnalyzer,
+    impl_block: &ast::ImplBlock,
+) -> Option<hir::Trait> {
+    let ast_path = impl_block
+        .target_trait()
+        .map(|it| it.syntax().clone())
+        .and_then(ast::PathType::cast)?
+        .path()?;
+
+    match analyzer.resolve_path(db, &ast_path) {
+        Some(hir::PathResolution::Def(hir::ModuleDef::Trait(def))) => Some(def),
+        _ => None,
+    }
+}
+
 pub(crate) fn invert_boolean_expression(expr: ast::Expr) -> ast::Expr {
     if let Some(expr) = invert_special_case(&expr) {
         return expr;