Use indexed paths to provide jump to require definition (#975)

Now that we have all require paths indexed, we do not need to search the
$LOAD_PATH to find the location of a require. We can just access the
index and jump directly

Author: vinistock

lib/ruby_indexer/lib/ruby_indexer/index.rb

@@ -28,7 +28,7 @@ def initialize
       @files_to_entries = T.let({}, T::Hash[String, T::Array[Entry]])
 
       # Holds all require paths for every indexed item so that we can provide autocomplete for requires
-      @require_paths_tree = T.let(PrefixTree[String].new, PrefixTree[String])
+      @require_paths_tree = T.let(PrefixTree[IndexablePath].new, PrefixTree[IndexablePath])
     end
 
     sig { params(indexable: IndexablePath).void }
@@ -73,7 +73,7 @@ def [](fully_qualified_name)
       @entries[fully_qualified_name.delete_prefix("::")]
     end
 
-    sig { params(query: String).returns(T::Array[String]) }
+    sig { params(query: String).returns(T::Array[IndexablePath]) }
     def search_require_paths(query)
       @require_paths_tree.search(query)
     end
@@ -147,7 +147,7 @@ def index_single(indexable_path, source = nil)
       visitor.run
 
       require_path = indexable_path.require_path
-      @require_paths_tree.insert(require_path, require_path) if require_path
+      @require_paths_tree.insert(require_path, indexable_path) if require_path
     rescue Errno::EISDIR
       # If `path` is a directory, just ignore it and continue indexing
     end

lib/ruby_indexer/test/index_test.rb

@@ -136,7 +136,7 @@ class Foo
         end
       RUBY
 
-      assert_equal(["path/foo", "path/other_foo"], @index.search_require_paths("path"))
+      assert_equal(["path/foo", "path/other_foo"], @index.search_require_paths("path").map(&:require_path))
     end
 
     def test_searching_for_entries_based_on_prefix

lib/ruby_lsp/requests/definition.rb

@@ -67,13 +67,15 @@ def on_command(node)
         string = argument.parts.first
         return unless string.is_a?(SyntaxTree::TStringContent)
 
-        required_file = "#{string.value}.rb"
-
         case message
         when "require"
-          candidate = find_file_in_load_path(required_file)
+          entry = @index.search_require_paths(string.value).find do |indexable_path|
+            indexable_path.require_path == string.value
+          end
+
+          if entry
+            candidate = entry.full_path
 
-          if candidate
             @response = Interface::Location.new(
               uri: URI::Generic.from_path(path: candidate).to_s,
               range: Interface::Range.new(
@@ -83,19 +85,18 @@ def on_command(node)
             )
           end
         when "require_relative"
+          required_file = "#{string.value}.rb"
           path = @uri.to_standardized_path
           current_folder = path ? Pathname.new(CGI.unescape(path)).dirname : Dir.pwd
           candidate = File.expand_path(File.join(current_folder, required_file))
 
-          if candidate
-            @response = Interface::Location.new(
-              uri: URI::Generic.from_path(path: candidate).to_s,
-              range: Interface::Range.new(
-                start: Interface::Position.new(line: 0, character: 0),
-                end: Interface::Position.new(line: 0, character: 0),
-              ),
-            )
-          end
+          @response = Interface::Location.new(
+            uri: URI::Generic.from_path(path: candidate).to_s,
+            range: Interface::Range.new(
+              start: Interface::Position.new(line: 0, character: 0),
+              end: Interface::Position.new(line: 0, character: 0),
+            ),
+          )
         end
       end
 
@@ -133,18 +134,6 @@ def find_in_index(value)
           )
         end
       end
-
-      sig { params(file: String).returns(T.nilable(String)) }
-      def find_file_in_load_path(file)
-        return unless file.include?("/")
-
-        $LOAD_PATH.each do |p|
-          found = Dir.glob("**/#{file}", base: p).first
-          return "#{p}/#{found}" if found
-        end
-
-        nil
-      end
     end
   end
 end

lib/ruby_lsp/requests/path_completion.rb

@@ -33,8 +33,8 @@ def initialize(index, emitter, message_queue)
 
       sig { params(node: SyntaxTree::TStringContent).void }
       def on_tstring_content(node)
-        @index.search_require_paths(node.value).sort!.each do |path|
-          @response << build_completion(path, node)
+        @index.search_require_paths(node.value).map!(&:require_path).sort!.each do |path|
+          @response << build_completion(T.must(path), node)
         end
       end
 

test/expectations/definition/requires.exp.json

@@ -1,6 +1,6 @@
 {
     "result": {
-        "uri": "file:///ruby_lsp/utils.rb",
+        "uri": "file:///ruby_lsp/event_emitter.rb",
         "range": {
             "start": {
                 "line": 0,

test/fixtures/requires.rb

@@ -1 +1 @@
-require "ruby_lsp/utils"
+require "ruby_lsp/event_emitter"

test/integration_test.rb

@@ -105,7 +105,13 @@ def test_definition
     initialize_lsp(["definition"])
     open_file_with("require 'ruby_lsp/utils'")
 
-    assert_telemetry("textDocument/didOpen")
+    read_response("textDocument/didOpen")
+
+    # Populate the index
+    send_request("initialized")
+    read_response("initialized")
+    # Read the response for the progress end notification
+    read_response("$/progress")
 
     response = make_request(
       "textDocument/definition",

test/requests/definition_expectations_test.rb

@@ -18,7 +18,7 @@ def run_expectations(source)
     index = executor.instance_variable_get(:@index)
     index.index_single(
       RubyIndexer::IndexablePath.new(
-        nil,
+        "#{Dir.pwd}/lib",
         File.expand_path(
           "../../test/fixtures/class_reference_target.rb",
           __dir__,
@@ -34,6 +34,15 @@ def run_expectations(source)
         ),
       ),
     )
+    index.index_single(
+      RubyIndexer::IndexablePath.new(
+        "#{Dir.pwd}/lib",
+        File.expand_path(
+          "../../lib/ruby_lsp/event_emitter.rb",
+          __dir__,
+        ),
+      ),
+    )
 
     begin
       # We need to pretend that Sorbet is not a dependency or else we can't properly test
@@ -93,4 +102,35 @@ def test_jumping_to_default_gems
   ensure
     T.must(message_queue).close
   end
+
+  def test_jumping_to_default_require_of_a_gem
+    message_queue = Thread::Queue.new
+
+    store = RubyLsp::Store.new
+    store.set(uri: URI("file:///folder/fake.rb"), source: <<~RUBY, version: 1)
+      require "bundler"
+    RUBY
+
+    executor = RubyLsp::Executor.new(store, message_queue)
+
+    uri = URI::Generic.from_path(path: "#{RbConfig::CONFIG["rubylibdir"]}/bundler.rb")
+    executor.instance_variable_get(:@index).index_single(
+      RubyIndexer::IndexablePath.new(RbConfig::CONFIG["rubylibdir"], T.must(uri.to_standardized_path)),
+    )
+
+    Dir.glob("#{RbConfig::CONFIG["rubylibdir"]}/bundler/*.rb").each do |path|
+      executor.instance_variable_get(:@index).index_single(
+        RubyIndexer::IndexablePath.new(RbConfig::CONFIG["rubylibdir"], path),
+      )
+    end
+
+    response = executor.execute({
+      method: "textDocument/definition",
+      params: { textDocument: { uri: "file:///folder/fake.rb" }, position: { character: 10, line: 0 } },
+    }).response
+
+    assert_equal(uri.to_s, response.attributes[:uri])
+  ensure
+    T.must(message_queue).close
+  end
 end