Fix handle resolve multiple methods

lib/ruby_indexer/lib/ruby_indexer/index.rb

@@ -237,20 +237,20 @@ def follow_aliased_namespace(name)
       real_parts.join("::")
     end
 
-    # Attempts to find a given method for a resolved fully qualified receiver name. Returns `nil` if the method does not
-    # exist on that receiver
-    sig { params(method_name: String, receiver_name: String).returns(T.nilable(Entry::Member)) }
+    # Attempts to find methods for a resolved fully qualified receiver name.
+    # Returns `nil` if the method does not exist on that receiver
+    sig { params(method_name: String, receiver_name: String).returns(T.nilable(T::Array[Entry::Member])) }
     def resolve_method(method_name, receiver_name)
       method_entries = self[method_name]
       owner_entries = self[receiver_name]
       return unless owner_entries && method_entries
 
       owner_name = T.must(owner_entries.first).name
       T.cast(
-        method_entries.grep(Entry::Member).find do |entry|
+        method_entries.grep(Entry::Member).select do |entry|
           T.cast(entry, Entry::Member).owner&.name == owner_name
         end,
-        T.nilable(Entry::Member),
+        T::Array[Entry::Member],
       )
     end
 

lib/ruby_indexer/test/index_test.rb

@@ -226,9 +226,9 @@ def baz; end
         end
       RUBY
 
-      entry = T.must(@index.resolve_method("baz", "Foo::Bar"))
-      assert_equal("baz", entry.name)
-      assert_equal("Foo::Bar", T.must(entry.owner).name)
+      entries = T.must(@index.resolve_method("baz", "Foo::Bar"))
+      assert_equal("baz", entries.first.name)
+      assert_equal("Foo::Bar", T.must(entries.first.owner).name)
     end
 
     def test_resolve_method_with_class_name_conflict
@@ -241,9 +241,9 @@ def Array(*args); end
         end
       RUBY
 
-      entry = T.must(@index.resolve_method("Array", "Foo"))
-      assert_equal("Array", entry.name)
-      assert_equal("Foo", T.must(entry.owner).name)
+      entries = T.must(@index.resolve_method("Array", "Foo"))
+      assert_equal("Array", entries.first.name)
+      assert_equal("Foo", T.must(entries.first.owner).name)
     end
 
     def test_resolve_method_attribute
@@ -253,9 +253,33 @@ class Foo
         end
       RUBY
 
-      entry = T.must(@index.resolve_method("bar", "Foo"))
-      assert_equal("bar", entry.name)
-      assert_equal("Foo", T.must(entry.owner).name)
+      entries = T.must(@index.resolve_method("bar", "Foo"))
+      assert_equal("bar", entries.first.name)
+      assert_equal("Foo", T.must(entries.first.owner).name)
+    end
+
+    def test_resolve_method_with_two_definitions
+      index(<<~RUBY)
+        class Foo
+          # Hello from first `bar`
+          def bar; end
+        end
+
+        class Foo
+          # Hello from second `bar`
+          def bar; end
+        end
+      RUBY
+
+      first_entry, second_entry = T.must(@index.resolve_method("bar", "Foo"))
+
+      assert_equal("bar", first_entry.name)
+      assert_equal("Foo", T.must(first_entry.owner).name)
+      assert_includes(first_entry.comments, "Hello from first `bar`")
+
+      assert_equal("bar", second_entry.name)
+      assert_equal("Foo", T.must(second_entry.owner).name)
+      assert_includes(second_entry.comments, "Hello from second `bar`")
     end
 
     def test_prefix_search_for_methods

lib/ruby_lsp/listeners/definition.rb

@@ -68,20 +68,22 @@ def handle_method_definition(node)
         message = node.message
         return unless message
 
-        target_method = @index.resolve_method(message, @nesting.join("::"))
-        return unless target_method
-
-        location = target_method.location
-        file_path = target_method.file_path
-        return if @typechecker_enabled && not_in_dependencies?(file_path)
-
-        @response_builder << Interface::Location.new(
-          uri: URI::Generic.from_path(path: file_path).to_s,
-          range: Interface::Range.new(
-            start: Interface::Position.new(line: location.start_line - 1, character: location.start_column),
-            end: Interface::Position.new(line: location.end_line - 1, character: location.end_column),
-          ),
-        )
+        methods = @index.resolve_method(message, @nesting.join("::"))
+        return unless methods
+
+        methods.each do |target_method|
+          location = target_method.location
+          file_path = target_method.file_path
+          next if @typechecker_enabled && not_in_dependencies?(file_path)
+
+          @response_builder << Interface::Location.new(
+            uri: URI::Generic.from_path(path: file_path).to_s,
+            range: Interface::Range.new(
+              start: Interface::Position.new(line: location.start_line - 1, character: location.start_column),
+              end: Interface::Position.new(line: location.end_line - 1, character: location.end_column),
+            ),
+          )
+        end
       end
 
       sig { params(node: Prism::CallNode).void }

lib/ruby_lsp/listeners/hover.rb

@@ -92,10 +92,10 @@ def on_call_node_enter(node)
         message = node.message
         return unless message
 
-        target_method = @index.resolve_method(message, @nesting.join("::"))
-        return unless target_method
+        methods = @index.resolve_method(message, @nesting.join("::"))
+        return unless methods
 
-        categorized_markdown_from_index_entries(message, target_method).each do |category, content|
+        categorized_markdown_from_index_entries(message, methods).each do |category, content|
           @response_builder.push(content, category: category)
         end
       end

lib/ruby_lsp/listeners/signature_help.rb

@@ -30,7 +30,10 @@ def on_call_node_enter(node)
         message = node.message
         return unless message
 
-        target_method = @index.resolve_method(message, @nesting.join("::"))
+        methods = @index.resolve_method(message, @nesting.join("::"))
+        return unless methods
+
+        target_method = methods.first
         return unless target_method
 
         parameters = target_method.parameters
@@ -59,7 +62,7 @@ def on_call_node_enter(node)
               parameters: parameters.map { |param| Interface::ParameterInformation.new(label: param.name) },
               documentation: Interface::MarkupContent.new(
                 kind: "markdown",
-                value: markdown_from_index_entries("", target_method),
+                value: markdown_from_index_entries("", methods),
               ),
             ),
           ],

test/requests/definition_expectations_test.rb

@@ -259,6 +259,56 @@ def foo; end
     T.must(message_queue).close
   end
 
+  def test_can_jump_to_method_with_two_definitions
+    message_queue = Thread::Queue.new
+    store = RubyLsp::Store.new
+
+    first_uri = URI("file:///folder/fake.rb")
+    first_source = <<~RUBY
+      # typed: false
+
+      class A
+        def bar
+          foo
+        end
+
+        def foo; end
+      end
+    RUBY
+
+    second_uri = URI("file:///folder/fake2.rb")
+    second_source = <<~RUBY
+      # typed: false
+
+      class A
+        def foo; end
+      end
+    RUBY
+
+    store.set(uri: first_uri, source: first_source, version: 1)
+    store.set(uri: second_uri, source: second_source, version: 1)
+
+    executor = RubyLsp::Executor.new(store, message_queue)
+
+    executor.instance_variable_get(:@index).index_single(
+      RubyIndexer::IndexablePath.new(nil, T.must(first_uri.to_standardized_path)), first_source
+    )
+    executor.instance_variable_get(:@index).index_single(
+      RubyIndexer::IndexablePath.new(nil, T.must(second_uri.to_standardized_path)), second_source
+    )
+
+    response = executor.execute({
+      method: "textDocument/definition",
+      params: { textDocument: { uri: "file:///folder/fake.rb" }, position: { character: 4, line: 4 } },
+    }).response
+
+    first_definition, second_definition = response
+    assert_equal(first_uri.to_s, first_definition.attributes[:uri])
+    assert_equal(second_uri.to_s, second_definition.attributes[:uri])
+  ensure
+    T.must(message_queue).close
+  end
+
   def test_jumping_to_method_method_calls_on_explicit_self
     message_queue = Thread::Queue.new
     store = RubyLsp::Store.new
@@ -294,7 +344,7 @@ def foo; end
     T.must(message_queue).close
   end
 
-  def test_jumping_to_method_definitions_when_declaration_does_not_exist
+  def test_does_nothing_when_declaration_does_not_exist
     message_queue = Thread::Queue.new
     store = RubyLsp::Store.new
 

test/requests/hover_expectations_test.rb

@@ -94,6 +94,45 @@ def bar
     T.must(message_queue).close
   end
 
+  def test_hovering_methods_with_two_definitions
+    message_queue = Thread::Queue.new
+    store = RubyLsp::Store.new
+
+    uri = URI("file:///fake.rb")
+    source = <<~RUBY
+      # typed: false
+
+      class A
+        # Hello from first `foo`
+        def foo; end
+
+        def bar
+          foo
+        end
+      end
+
+      class A
+        # Hello from second `foo`
+        def foo; end
+      end
+    RUBY
+    store.set(uri: uri, source: source, version: 1)
+
+    executor = RubyLsp::Executor.new(store, message_queue)
+    index = executor.instance_variable_get(:@index)
+    index.index_single(RubyIndexer::IndexablePath.new(nil, T.must(uri.to_standardized_path)), source)
+
+    response = executor.execute({
+      method: "textDocument/hover",
+      params: { textDocument: { uri: uri }, position: { character: 4, line: 7 } },
+    }).response
+
+    assert_match("Hello from first `foo`", response.contents.value)
+    assert_match("Hello from second `foo`", response.contents.value)
+  ensure
+    T.must(message_queue).close
+  end
+
   def test_hovering_methods_invoked_on_explicit_self
     message_queue = Thread::Queue.new
     store = RubyLsp::Store.new

test/requests/signature_help_test.rb

@@ -51,6 +51,49 @@ def baz
     assert_equal(0, result.active_parameter)
   end
 
+  def test_concats_documentations_from_both_definitions
+    source = <<~RUBY
+      class Foo
+        # first definition
+        def bar(a, b)
+        end
+
+        def baz
+          bar()
+        end
+      end
+
+      class Foo
+        # second definition
+        def bar(c, d)
+        end
+      end
+    RUBY
+
+    @store.set(uri: @uri, source: source, version: 1)
+    index = @executor.instance_variable_get(:@index)
+    index.index_single(RubyIndexer::IndexablePath.new(nil, T.must(@uri.to_standardized_path)), source)
+
+    result = run_request(
+      method: "textDocument/signatureHelp",
+      params: {
+        textDocument: { uri: @uri.to_s },
+        position: { line: 6, character: 7 },
+        context: {
+          triggerCharacter: "(",
+          activeSignatureHelp: nil,
+        },
+      },
+    )
+
+    signature = result.signatures.first
+
+    assert_equal("bar(a, b)", signature.label)
+    assert_equal(0, result.active_parameter)
+    assert_match("first definition", signature.documentation.value)
+    assert_match("second definition", signature.documentation.value)
+  end
+
   def test_help_after_comma
     document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri)
       class Foo