Replace IndexablePath with URI (#2916)

### Motivation

Second step for #1908

This PR gets rid of `IndexablePath` and standardizes on using URI. This is necessary because `IndexablePath` is coupled with file paths on disk, which is incompatible with the URI scheme for unsaved files (`untitled:Untitled-1`) - a scenario where the files don't exist on disk.

### Implementation

The important changes are just in the URI extension file. Everything else is essentially just renaming things from `IndexablePath` to URI.

**Note**: this PR takes a slightly different approach than my previous attempt. I previously tried to create a separate URI class for the indexer, fully disconnected from the original `URI::Generic` class. That turned out to be a terrible idea that lead to an enormous amount of duplication, so the approach here is just to extend `URI::Generic` a bit more to handle require paths.

### Automated Tests

Added a couple of new tests for the URI extension.

exe/ruby-lsp

@@ -139,9 +139,9 @@ if options[:doctor]
 
   puts "Globbing for indexable files"
 
-  index.configuration.indexables.each do |indexable|
-    puts "indexing: #{indexable.full_path}"
-    index.index_single(indexable)
+  index.configuration.indexables.each do |uri|
+    puts "indexing: #{uri.full_path}"
+    index.index_single(uri)
   end
   return
 end

exe/ruby-lsp-check

@@ -46,10 +46,10 @@ puts "Verifying that indexing executes successfully. This may take a while..."
 index = RubyIndexer::Index.new
 indexables = index.configuration.indexables
 
-indexables.each_with_index do |indexable, i|
-  index.index_single(indexable)
+indexables.each_with_index do |uri, i|
+  index.index_single(uri)
 rescue => e
-  errors[indexable.full_path] = e
+  errors[uri.full_path] = e
 ensure
   print("\033[M\033[0KIndexed #{i + 1}/#{indexables.length}") unless ENV["CI"]
 end

lib/ruby_indexer/lib/ruby_indexer/configuration.rb

@@ -81,7 +81,7 @@ def merged_excluded_file_pattern
         .then { |dirs| File.join(@workspace_path, "{#{dirs.join(",")}}/**/*") }
     end
 
-    sig { returns(T::Array[IndexablePath]) }
+    sig { returns(T::Array[URI::Generic]) }
     def indexables
       excluded_gems = @excluded_gems - @included_gems
       locked_gems = Bundler.locked_gems&.specs
@@ -107,12 +107,12 @@ def indexables
           [included_path, relative_path]
         end
 
-      indexables = T.let([], T::Array[IndexablePath])
+      indexables = T.let([], T::Array[URI::Generic])
 
       # Handle top level files separately. The path below is an optimization to prevent descending down directories that
       # are going to be excluded anyway, so we need to handle top level scripts separately
       Dir.glob(File.join(@workspace_path, "*.rb"), flags).each do |path|
-        indexables << IndexablePath.new(nil, path)
+        indexables << URI::Generic.from_path(path: path)
       end
 
       # Add user specified patterns
@@ -134,7 +134,7 @@ def indexables
               load_path_entry = $LOAD_PATH.find { |load_path| path.start_with?(load_path) }
             end
 
-            indexables << IndexablePath.new(load_path_entry, path)
+            indexables << URI::Generic.from_path(path: path, load_path_entry: load_path_entry)
           end
         end
       end
@@ -152,7 +152,7 @@ def indexables
       # Remove user specified patterns
       indexables.reject! do |indexable|
         excluded_patterns.any? do |pattern|
-          File.fnmatch?(pattern, indexable.full_path, File::FNM_PATHNAME | File::FNM_EXTGLOB)
+          File.fnmatch?(pattern, T.must(indexable.full_path), File::FNM_PATHNAME | File::FNM_EXTGLOB)
         end
       end
 
@@ -184,12 +184,12 @@ def indexables
           # If the default_path is a directory, we index all the Ruby files in it
           indexables.concat(
             Dir.glob(File.join(default_path, "**", "*.rb"), File::FNM_PATHNAME | File::FNM_EXTGLOB).map! do |path|
-              IndexablePath.new(RbConfig::CONFIG["rubylibdir"], path)
+              URI::Generic.from_path(path: path, load_path_entry: RbConfig::CONFIG["rubylibdir"])
             end,
           )
         elsif pathname.extname == ".rb"
           # If the default_path is a Ruby file, we index it
-          indexables << IndexablePath.new(RbConfig::CONFIG["rubylibdir"], default_path)
+          indexables << URI::Generic.from_path(path: default_path, load_path_entry: RbConfig::CONFIG["rubylibdir"])
         end
       end
 
@@ -207,7 +207,9 @@ def indexables
         indexables.concat(
           spec.require_paths.flat_map do |require_path|
             load_path_entry = File.join(spec.full_gem_path, require_path)
-            Dir.glob(File.join(load_path_entry, "**", "*.rb")).map! { |path| IndexablePath.new(load_path_entry, path) }
+            Dir.glob(File.join(load_path_entry, "**", "*.rb")).map! do |path|
+              URI::Generic.from_path(path: path, load_path_entry: load_path_entry)
+            end
           end,
         )
       rescue Gem::MissingSpecError
@@ -216,7 +218,7 @@ def indexables
         # just ignore if they're missing
       end
 
-      indexables.uniq!(&:full_path)
+      indexables.uniq!(&:to_s)
       indexables
     end
 

lib/ruby_indexer/lib/ruby_indexer/index.rb

@@ -35,7 +35,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[IndexablePath].new, PrefixTree[IndexablePath])
+      @require_paths_tree = T.let(PrefixTree[URI::Generic].new, PrefixTree[URI::Generic])
 
       # Holds the linearized ancestors list for every namespace
       @ancestors = T.let({}, T::Hash[String, T::Array[String]])
@@ -55,11 +55,14 @@ def register_included_hook(module_name, &hook)
       (@included_hooks[module_name] ||= []) << hook
     end
 
-    sig { params(indexable: IndexablePath).void }
-    def delete(indexable)
+    sig { params(uri: URI::Generic).void }
+    def delete(uri)
+      full_path = uri.full_path
+      return unless full_path
+
       # For each constant discovered in `path`, delete the associated entry from the index. If there are no entries
       # left, delete the constant from the index.
-      @files_to_entries[indexable.full_path]&.each do |entry|
+      @files_to_entries[full_path]&.each do |entry|
         name = entry.name
         entries = @entries[name]
         next unless entries
@@ -77,9 +80,9 @@ def delete(indexable)
         end
       end
 
-      @files_to_entries.delete(indexable.full_path)
+      @files_to_entries.delete(full_path)
 
-      require_path = indexable.require_path
+      require_path = uri.require_path
       @require_paths_tree.delete(require_path) if require_path
     end
 
@@ -97,7 +100,7 @@ def [](fully_qualified_name)
       @entries[fully_qualified_name.delete_prefix("::")]
     end
 
-    sig { params(query: String).returns(T::Array[IndexablePath]) }
+    sig { params(query: String).returns(T::Array[URI::Generic]) }
     def search_require_paths(query)
       @require_paths_tree.search(query)
     end
@@ -342,16 +345,16 @@ def resolve(name, nesting, seen_names = [])
       nil
     end
 
-    # Index all files for the given indexable paths, which defaults to what is configured. A block can be used to track
-    # and control indexing progress. That block is invoked with the current progress percentage and should return `true`
-    # to continue indexing or `false` to stop indexing.
+    # Index all files for the given URIs, which defaults to what is configured. A block can be used to track and control
+    # indexing progress. That block is invoked with the current progress percentage and should return `true` to continue
+    # indexing or `false` to stop indexing.
     sig do
       params(
-        indexable_paths: T::Array[IndexablePath],
+        uris: T::Array[URI::Generic],
         block: T.nilable(T.proc.params(progress: Integer).returns(T::Boolean)),
       ).void
     end
-    def index_all(indexable_paths: @configuration.indexables, &block)
+    def index_all(uris: @configuration.indexables, &block)
       # When troubleshooting an indexing issue, e.g. through irb, it's not obvious that `index_all` will augment the
       # existing index values, meaning it may contain 'stale' entries. This check ensures that the user is aware of this
       # behavior and can take appropriate action.
@@ -363,37 +366,40 @@ def index_all(indexable_paths: @configuration.indexables, &block)
 
       RBSIndexer.new(self).index_ruby_core
       # Calculate how many paths are worth 1% of progress
-      progress_step = (indexable_paths.length / 100.0).ceil
+      progress_step = (uris.length / 100.0).ceil
 
-      indexable_paths.each_with_index do |path, index|
+      uris.each_with_index do |uri, index|
         if block && index % progress_step == 0
           progress = (index / progress_step) + 1
           break unless block.call(progress)
         end
 
-        index_single(path, collect_comments: false)
+        index_single(uri, collect_comments: false)
       end
     end
 
-    sig { params(indexable_path: IndexablePath, source: T.nilable(String), collect_comments: T::Boolean).void }
-    def index_single(indexable_path, source = nil, collect_comments: true)
-      content = source || File.read(indexable_path.full_path)
+    sig { params(uri: URI::Generic, source: T.nilable(String), collect_comments: T::Boolean).void }
+    def index_single(uri, source = nil, collect_comments: true)
+      full_path = uri.full_path
+      return unless full_path
+
+      content = source || File.read(full_path)
       dispatcher = Prism::Dispatcher.new
 
       result = Prism.parse(content)
       listener = DeclarationListener.new(
         self,
         dispatcher,
         result,
-        indexable_path.full_path,
+        full_path,
         collect_comments: collect_comments,
       )
       dispatcher.dispatch(result.value)
 
       indexing_errors = listener.indexing_errors.uniq
 
-      require_path = indexable_path.require_path
-      @require_paths_tree.insert(require_path, indexable_path) if require_path
+      require_path = uri.require_path
+      @require_paths_tree.insert(require_path, uri) if require_path
 
       if indexing_errors.any?
         indexing_errors.each do |error|
@@ -405,7 +411,7 @@ def index_single(indexable_path, source = nil, collect_comments: true)
       # it
     rescue SystemStackError => e
       if e.backtrace&.first&.include?("prism")
-        $stderr.puts "Prism error indexing #{indexable_path.full_path}: #{e.message}"
+        $stderr.puts "Prism error indexing #{T.must(full_path)}: #{e.message}"
       else
         raise
       end
@@ -600,16 +606,19 @@ def instance_variable_completion_candidates(name, owner_name)
       variables
     end
 
-    # Synchronizes a change made to the given indexable path. This method will ensure that new declarations are indexed,
-    # removed declarations removed and that the ancestor linearization cache is cleared if necessary
-    sig { params(indexable: IndexablePath).void }
-    def handle_change(indexable)
-      original_entries = @files_to_entries[indexable.full_path]
+    # Synchronizes a change made to the given URI. This method will ensure that new declarations are indexed, removed
+    # declarations removed and that the ancestor linearization cache is cleared if necessary
+    sig { params(uri: URI::Generic).void }
+    def handle_change(uri)
+      full_path = uri.full_path
+      return unless full_path
+
+      original_entries = @files_to_entries[full_path]
 
-      delete(indexable)
-      index_single(indexable)
+      delete(uri)
+      index_single(uri)
 
-      updated_entries = @files_to_entries[indexable.full_path]
+      updated_entries = @files_to_entries[full_path]
 
       return unless original_entries && updated_entries
 

lib/ruby_indexer/lib/ruby_indexer/uri.rb

@@ -13,8 +13,15 @@ class Generic
     class << self
       extend T::Sig
 
-      sig { params(path: String, fragment: T.nilable(String), scheme: String).returns(URI::Generic) }
-      def from_path(path:, fragment: nil, scheme: "file")
+      sig do
+        params(
+          path: String,
+          fragment: T.nilable(String),
+          scheme: String,
+          load_path_entry: T.nilable(String),
+        ).returns(URI::Generic)
+      end
+      def from_path(path:, fragment: nil, scheme: "file", load_path_entry: nil)
         # On Windows, if the path begins with the disk name, we need to add a leading slash to make it a valid URI
         escaped_path = if /^[A-Z]:/i.match?(path)
           PARSER.escape("/#{path}")
@@ -25,10 +32,27 @@ def from_path(path:, fragment: nil, scheme: "file")
           PARSER.escape(path)
         end
 
-        build(scheme: scheme, path: escaped_path, fragment: fragment)
+        uri = build(scheme: scheme, path: escaped_path, fragment: fragment)
+
+        if load_path_entry
+          uri.require_path = path.delete_prefix("#{load_path_entry}/").delete_suffix(".rb")
+        end
+
+        uri
       end
     end
 
+    sig { returns(T.nilable(String)) }
+    attr_accessor :require_path
+
+    sig { params(load_path_entry: String).void }
+    def add_require_path_from_load_entry(load_path_entry)
+      path = to_standardized_path
+      return unless path
+
+      self.require_path = path.delete_prefix("#{load_path_entry}/").delete_suffix(".rb")
+    end
+
     sig { returns(T.nilable(String)) }
     def to_standardized_path
       parsed_path = path
@@ -44,5 +68,7 @@ def to_standardized_path
         unescaped_path
       end
     end
+
+    alias_method :full_path, :to_standardized_path
   end
 end

lib/ruby_indexer/ruby_indexer.rb

@@ -5,7 +5,6 @@
 require "did_you_mean"
 
 require "ruby_indexer/lib/ruby_indexer/uri"
-require "ruby_indexer/lib/ruby_indexer/indexable_path"
 require "ruby_indexer/lib/ruby_indexer/declaration_listener"
 require "ruby_indexer/lib/ruby_indexer/reference_finder"
 require "ruby_indexer/lib/ruby_indexer/enhancement"

lib/ruby_indexer/test/classes_and_modules_test.rb

@@ -200,7 +200,7 @@ class Foo
 
       assert_entry("Foo", Entry::Class, "/fake/path/foo.rb:0-0:1-3")
 
-      @index.delete(IndexablePath.new(nil, "/fake/path/foo.rb"))
+      @index.delete(URI::Generic.from_path(path: "/fake/path/foo.rb"))
       refute_entry("Foo")
 
       assert_no_indexed_entries
@@ -618,10 +618,12 @@ class ::Qux
     end
 
     def test_lazy_comment_fetching_uses_correct_line_breaks_for_rendering
-      path = "lib/ruby_lsp/node_context.rb"
-      indexable = IndexablePath.new("#{Dir.pwd}/lib", path)
+      uri = URI::Generic.from_path(
+        load_path_entry: "#{Dir.pwd}/lib",
+        path: "#{Dir.pwd}/lib/ruby_lsp/node_context.rb",
+      )
 
-      @index.index_single(indexable, collect_comments: false)
+      @index.index_single(uri, collect_comments: false)
 
       entry = @index["RubyLsp::NodeContext"].first
 
@@ -632,9 +634,12 @@ def test_lazy_comment_fetching_uses_correct_line_breaks_for_rendering
     end
 
     def test_lazy_comment_fetching_does_not_fail_if_file_gets_deleted
-      indexable = IndexablePath.new("#{Dir.pwd}/lib", "lib/ruby_lsp/does_not_exist.rb")
+      uri = URI::Generic.from_path(
+        load_path_entry: "#{Dir.pwd}/lib",
+        path: "#{Dir.pwd}/lib/ruby_lsp/does_not_exist.rb",
+      )
 
-      @index.index_single(indexable, <<~RUBY, collect_comments: false)
+      @index.index_single(uri, <<~RUBY, collect_comments: false)
         class Foo
         end
       RUBY