Deduplicate methods in completion by function ID and not by name

Because duplicates can be found with traits. Worse, the inherent methods could be private, and we'll discover that only later. But even if they're not they're different methods, and its seems worthy to present them all to the user.

2 files changed, 47 insertions, 8 deletions

diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/completions/dot.rs b/src/tools/rust-analyzer/crates/ide-completion/src/completions/dot.rs
index 129964f8387..b9ef68cc2d2 100644
--- a/src/tools/rust-analyzer/crates/ide-completion/src/completions/dot.rs
+++ b/src/tools/rust-analyzer/crates/ide-completion/src/completions/dot.rs
@@ -2,7 +2,7 @@
 
 use std::ops::ControlFlow;
 
-use hir::{Complete, HasContainer, ItemContainer, MethodCandidateCallback, Name};
+use hir::{Complete, Function, HasContainer, ItemContainer, MethodCandidateCallback};
 use ide_db::FxHashSet;
 use syntax::SmolStr;
 
@@ -237,7 +237,10 @@ fn complete_methods(
     struct Callback<'a, F> {
         ctx: &'a CompletionContext<'a>,
         f: F,
-        seen_methods: FxHashSet<Name>,
+        // We deliberately deduplicate by function ID and not name, because while inherent methods cannot be
+        // duplicated, trait methods can. And it is still useful to show all of them (even when there
+        // is also an inherent method, especially considering that it may be private, and filtered later).
+        seen_methods: FxHashSet<Function>,
     }
 
     impl<F> MethodCandidateCallback for Callback<'_, F>
@@ -247,9 +250,7 @@ fn complete_methods(
         // We don't want to exclude inherent trait methods - that is, methods of traits available from
         // `where` clauses or `dyn Trait`.
         fn on_inherent_method(&mut self, func: hir::Function) -> ControlFlow<()> {
-            if func.self_param(self.ctx.db).is_some()
-                && self.seen_methods.insert(func.name(self.ctx.db))
-            {
+            if func.self_param(self.ctx.db).is_some() && self.seen_methods.insert(func) {
                 (self.f)(func);
             }
             ControlFlow::Continue(())
@@ -265,9 +266,7 @@ fn complete_methods(
                 return ControlFlow::Continue(());
             }
 
-            if func.self_param(self.ctx.db).is_some()
-                && self.seen_methods.insert(func.name(self.ctx.db))
-            {
+            if func.self_param(self.ctx.db).is_some() && self.seen_methods.insert(func) {
                 (self.f)(func);
             }
 
diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/tests/expression.rs b/src/tools/rust-analyzer/crates/ide-completion/src/tests/expression.rs
index 7a0d0044412..af6ef34761e 100644
--- a/src/tools/rust-analyzer/crates/ide-completion/src/tests/expression.rs
+++ b/src/tools/rust-analyzer/crates/ide-completion/src/tests/expression.rs
@@ -2632,3 +2632,43 @@ fn let_in_condition() {
 fn let_in_let_chain() {
     check_edit("let", r#"fn f() { if true && $0 {} }"#, r#"fn f() { if true && let $1 = $0 {} }"#);
 }
+
+#[test]
+fn private_inherent_and_public_trait() {
+    check(
+        r#"
+struct Foo;
+
+mod private {
+    impl super::Foo {
+        fn method(&self) {}
+    }
+}
+
+trait Trait {
+    fn method(&self) {}
+}
+impl Trait for Foo {}
+
+fn main() {
+    Foo.$0
+}
+    "#,
+        expect![[r#"
+            me method() (as Trait) fn(&self)
+            sn box            Box::new(expr)
+            sn call           function(expr)
+            sn const                const {}
+            sn dbg                dbg!(expr)
+            sn dbgr              dbg!(&expr)
+            sn deref                   *expr
+            sn let                       let
+            sn letm                  let mut
+            sn match           match expr {}
+            sn ref                     &expr
+            sn refm                &mut expr
+            sn return            return expr
+            sn unsafe              unsafe {}
+        "#]],
+    );
+}