Avoid showing duplicate completions (#2215)

* Avoid showing duplicate completions for overidden methods

* PR Feedback

lib/ruby_indexer/lib/ruby_indexer/entry.rb

@@ -483,6 +483,9 @@ class MethodAlias < Entry
       sig { returns(T.any(Member, MethodAlias)) }
       attr_reader :target
 
+      sig { returns(T.nilable(Entry::Namespace)) }
+      attr_reader :owner
+
       sig { params(target: T.any(Member, MethodAlias), unresolved_alias: UnresolvedMethodAlias).void }
       def initialize(target, unresolved_alias)
         full_comments = ["Alias for #{target.name}\n"]
@@ -498,12 +501,13 @@ def initialize(target, unresolved_alias)
         )
 
         @target = target
+        @owner = T.let(unresolved_alias.owner, T.nilable(Entry::Namespace))
       end
 
-      sig { returns(T.nilable(Entry::Namespace)) }
-      def owner
-        @target.owner
-      end
+      # sig { returns(T.nilable(Entry::Namespace)) }
+      # def owner
+      #   @target.owner
+      # end
 
       sig { returns(T::Array[Parameter]) }
       def parameters

lib/ruby_indexer/lib/ruby_indexer/index.rb

@@ -150,17 +150,42 @@ def method_completion_candidates(name, receiver_name)
       ancestors = linearized_ancestors_of(receiver_name)
 
       candidates = name ? prefix_search(name).flatten : @entries.values.flatten
-      candidates.filter_map do |entry|
-        case entry
-        when Entry::Member, Entry::MethodAlias
-          entry if ancestors.any?(entry.owner&.name)
-        when Entry::UnresolvedMethodAlias
-          if ancestors.any?(entry.owner&.name)
-            resolved_alias = resolve_method_alias(entry, receiver_name)
-            resolved_alias if resolved_alias.is_a?(Entry::MethodAlias)
-          end
+      completion_items = candidates.each_with_object({}) do |entry, hash|
+        unless entry.is_a?(Entry::Member) || entry.is_a?(Entry::MethodAlias) ||
+            entry.is_a?(Entry::UnresolvedMethodAlias)
+          next
+        end
+
+        entry_name = entry.name
+        ancestor_index = ancestors.index(entry.owner&.name)
+        existing_entry, existing_entry_index = hash[entry_name]
+
+        # Conditions for matching a method completion candidate:
+        # 1. If an ancestor_index was found, it means that this method is owned by the receiver. The exact index is
+        # where in the ancestor chain the method was found. For example, if the ancestors are ["A", "B", "C"] and we
+        # found the method declared in `B`, then the ancestors index is 1
+        #
+        # 2. We already established that this method is owned by the receiver. Now, check if we already added a
+        # completion candidate for this method name. If not, then we just go and add it (the left hand side of the or)
+        #
+        # 3. If we had already found a method entry for the same name, then we need to check if the current entry that
+        # we are comparing appears first in the hierarchy or not. For example, imagine we have the method `open` defined
+        # in both `File` and its parent `IO`. If we first find the method `open` in `IO`, it will be inserted into the
+        # hash. Then, when we find the entry for `open` owned by `File`, we need to replace `IO.open` by `File.open`,
+        # since `File.open` appears first in the hierarchy chain and is therefore the correct method being invoked. The
+        # last part of the conditional checks if the current entry was found earlier in the hierarchy chain, in which
+        # case we must update the existing entry to avoid showing the wrong method declaration for overridden methods
+        next unless ancestor_index && (!existing_entry || ancestor_index < existing_entry_index)
+
+        if entry.is_a?(Entry::UnresolvedMethodAlias)
+          resolved_alias = resolve_method_alias(entry, receiver_name)
+          hash[entry_name] = [resolved_alias, ancestor_index] if resolved_alias.is_a?(Entry::MethodAlias)
+        else
+          hash[entry_name] = [entry, ancestor_index]
         end
       end
+
+      completion_items.values.map!(&:first)
     end
 
     # Resolve a constant to its declaration based on its name and the nesting where the reference was found. Parameter

lib/ruby_indexer/test/index_test.rb

@@ -1509,5 +1509,37 @@ class Foo
 
       assert_empty(@index.method_completion_candidates("bar", "Foo"))
     end
+
+    def test_completion_does_not_duplicate_overridden_methods
+      index(<<~RUBY)
+        class Foo
+          def bar; end
+        end
+
+        class Baz < Foo
+          def bar; end
+        end
+      RUBY
+
+      entries = @index.method_completion_candidates("bar", "Baz")
+      assert_equal(["bar"], entries.map(&:name))
+      assert_equal("Baz", T.must(entries.first.owner).name)
+    end
+
+    def test_completion_does_not_duplicate_methods_overridden_by_aliases
+      index(<<~RUBY)
+        class Foo
+          def bar; end
+        end
+
+        class Baz < Foo
+          alias bar to_s
+        end
+      RUBY
+
+      entries = @index.method_completion_candidates("bar", "Baz")
+      assert_equal(["bar"], entries.map(&:name))
+      assert_equal("Baz", T.must(entries.first.owner).name)
+    end
   end
 end