From 03f69b80161d4d5d2225aed8d5305d44c5bca2b9 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Thu, 28 Nov 2024 10:30:32 -0500 Subject: [PATCH] Start accepting any URI scheme in the index --- exe/ruby-lsp | 4 +- exe/ruby-lsp-check | 2 +- lib/ruby_indexer/lib/ruby_indexer/entry.rb | 13 ++- lib/ruby_indexer/lib/ruby_indexer/index.rb | 80 ++++++++----------- .../test/classes_and_modules_test.rb | 2 +- lib/ruby_indexer/test/index_test.rb | 49 +++++++++--- lib/ruby_indexer/test/test_case.rb | 4 +- lib/ruby_lsp/server.rb | 6 +- lib/ruby_lsp/test_helper.rb | 21 ++--- rakelib/index.rake | 3 +- test/requests/completion_test.rb | 2 +- test/requests/definition_expectations_test.rb | 14 ++-- test/requests/workspace_symbol_test.rb | 2 +- test/server_test.rb | 1 + 14 files changed, 112 insertions(+), 91 deletions(-) diff --git a/exe/ruby-lsp b/exe/ruby-lsp index e831ab561..a82c176fa 100755 --- a/exe/ruby-lsp +++ b/exe/ruby-lsp @@ -140,8 +140,8 @@ if options[:doctor] puts "Globbing for indexable files" index.configuration.indexable_uris.each do |uri| - puts "indexing: #{uri.full_path}" - index.index_single(uri) + puts "indexing: #{uri}" + index.index_file(uri) end return end diff --git a/exe/ruby-lsp-check b/exe/ruby-lsp-check index 991eca597..70e3aee60 100755 --- a/exe/ruby-lsp-check +++ b/exe/ruby-lsp-check @@ -47,7 +47,7 @@ index = RubyIndexer::Index.new uris = index.configuration.indexable_uris uris.each_with_index do |uri, i| - index.index_single(uri) + index.index_file(uri) rescue => e errors[uri.full_path] = e ensure diff --git a/lib/ruby_indexer/lib/ruby_indexer/entry.rb b/lib/ruby_indexer/lib/ruby_indexer/entry.rb index 9d8a70f1d..327f211f7 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/entry.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/entry.rb @@ -60,19 +60,24 @@ def private? sig { returns(String) } def file_name - File.basename(file_path) + if @uri.scheme == "untitled" + T.must(@uri.opaque) + else + File.basename(T.must(file_path)) + end end - sig { returns(String) } + sig { returns(T.nilable(String)) } def file_path - T.must(@uri.full_path) + @uri.full_path end sig { returns(String) } def comments @comments ||= begin # Parse only the comments based on the file path, which is much faster than parsing the entire file - parsed_comments = Prism.parse_file_comments(file_path) + path = file_path + parsed_comments = path ? Prism.parse_file_comments(path) : [] # Group comments based on whether they belong to a single block of comments grouped = parsed_comments.slice_when do |left, right| diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index 7128eb2b6..3c60163e3 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -29,10 +29,11 @@ def initialize # Holds references to where entries where discovered so that we can easily delete them # { - # "/my/project/foo.rb" => [#, #], - # "/my/project/bar.rb" => [#], + # "file:///my/project/foo.rb" => [#, #], + # "file:///my/project/bar.rb" => [#], + # "untitled:Untitled-1" => [#], # } - @files_to_entries = T.let({}, T::Hash[String, T::Array[Entry]]) + @uris_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[URI::Generic].new, PrefixTree[URI::Generic]) @@ -57,12 +58,10 @@ def register_included_hook(module_name, &hook) sig { params(uri: URI::Generic).void } def delete(uri) - full_path = uri.full_path - return unless full_path - + key = uri.to_s # 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[full_path]&.each do |entry| + @uris_to_entries[key]&.each do |entry| name = entry.name entries = @entries[name] next unless entries @@ -80,7 +79,7 @@ def delete(uri) end end - @files_to_entries.delete(full_path) + @uris_to_entries.delete(key) require_path = uri.require_path @require_paths_tree.delete(require_path) if require_path @@ -91,7 +90,7 @@ def add(entry, skip_prefix_tree: false) name = entry.name (@entries[name] ||= []) << entry - (@files_to_entries[entry.file_path] ||= []) << entry + (@uris_to_entries[entry.uri.to_s] ||= []) << entry @entries_tree.insert(name, T.must(@entries[name])) unless skip_prefix_tree end @@ -374,49 +373,40 @@ def index_all(uris: @configuration.indexable_uris, &block) break unless block.call(progress) end - index_single(uri, collect_comments: false) + index_file(uri, collect_comments: false) end end - 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) + sig { params(uri: URI::Generic, source: String, collect_comments: T::Boolean).void } + def index_single(uri, source, collect_comments: true) dispatcher = Prism::Dispatcher.new - result = Prism.parse(content) - listener = DeclarationListener.new( - self, - dispatcher, - result, - uri, - collect_comments: collect_comments, - ) + result = Prism.parse(source) + listener = DeclarationListener.new(self, dispatcher, result, uri, collect_comments: collect_comments) dispatcher.dispatch(result.value) - indexing_errors = listener.indexing_errors.uniq - require_path = uri.require_path @require_paths_tree.insert(require_path, uri) if require_path - if indexing_errors.any? - indexing_errors.each do |error| - $stderr.puts error - end - end - rescue Errno::EISDIR, Errno::ENOENT - # If `path` is a directory, just ignore it and continue indexing. If the file doesn't exist, then we also ignore - # it + indexing_errors = listener.indexing_errors.uniq + indexing_errors.each { |error| $stderr.puts(error) } if indexing_errors.any? rescue SystemStackError => e if e.backtrace&.first&.include?("prism") - $stderr.puts "Prism error indexing #{T.must(full_path)}: #{e.message}" + $stderr.puts "Prism error indexing #{uri}: #{e.message}" else raise end end + # Indexes a File URI by reading the contents from disk + sig { params(uri: URI::Generic, collect_comments: T::Boolean).void } + def index_file(uri, collect_comments: true) + index_single(uri, File.read(T.must(uri.full_path)), collect_comments: collect_comments) + rescue Errno::EISDIR, Errno::ENOENT + # If `path` is a directory, just ignore it and continue indexing. If the file doesn't exist, then we also ignore + # it + end + # Follows aliases in a namespace. The algorithm keeps checking if the name is an alias and then recursively follows # it. The idea is that we test the name in parts starting from the complete name to the first namespace. For # `Foo::Bar::Baz`, we would test: @@ -608,17 +598,15 @@ def instance_variable_completion_candidates(name, owner_name) # 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] + sig { params(uri: URI::Generic, source: String).void } + def handle_change(uri, source) + key = uri.to_s + original_entries = @uris_to_entries[key] delete(uri) - index_single(uri) + index_single(uri, source) - updated_entries = @files_to_entries[full_path] + updated_entries = @uris_to_entries[key] return unless original_entries && updated_entries @@ -684,12 +672,12 @@ def existing_or_new_singleton_class(name) sig do type_parameters(:T).params( - path: String, + uri: String, type: T.nilable(T::Class[T.all(T.type_parameter(:T), Entry)]), ).returns(T.nilable(T.any(T::Array[Entry], T::Array[T.type_parameter(:T)]))) end - def entries_for(path, type = nil) - entries = @files_to_entries[path] + def entries_for(uri, type = nil) + entries = @uris_to_entries[uri.to_s] return entries unless type entries&.grep(type) diff --git a/lib/ruby_indexer/test/classes_and_modules_test.rb b/lib/ruby_indexer/test/classes_and_modules_test.rb index b49112ac7..91227a966 100644 --- a/lib/ruby_indexer/test/classes_and_modules_test.rb +++ b/lib/ruby_indexer/test/classes_and_modules_test.rb @@ -623,7 +623,7 @@ def test_lazy_comment_fetching_uses_correct_line_breaks_for_rendering path: "#{Dir.pwd}/lib/ruby_lsp/node_context.rb", ) - @index.index_single(uri, collect_comments: false) + @index.index_file(uri, collect_comments: false) entry = @index["RubyLsp::NodeContext"].first diff --git a/lib/ruby_indexer/test/index_test.rb b/lib/ruby_indexer/test/index_test.rb index 96bf768b2..80a5a8dc4 100644 --- a/lib/ruby_indexer/test/index_test.rb +++ b/lib/ruby_indexer/test/index_test.rb @@ -124,7 +124,7 @@ def test_index_single_ignores_directories FileUtils.mkdir(path) begin - @index.index_single(URI::Generic.from_path(path: path)) + @index.index_file(URI::Generic.from_path(path: path)) ensure FileUtils.rm_r(path) end @@ -351,14 +351,14 @@ def test_indexing_prism_fixtures_succeeds fixtures.each do |fixture| uri = URI::Generic.from_path(path: fixture) - @index.index_single(uri) + @index.index_file(uri) end refute_empty(@index) end def test_index_single_does_not_fail_for_non_existing_file - @index.index_single(URI::Generic.from_path(path: "/fake/path/foo.rb")) + @index.index_file(URI::Generic.from_path(path: "/fake/path/foo.rb")) entries_after_indexing = @index.names assert_equal(@default_indexed_entries.keys, entries_after_indexing) end @@ -787,7 +787,7 @@ class Bar RUBY uri = URI::Generic.from_path(path: File.join(dir, "foo.rb")) - @index.index_single(uri) + @index.index_file(uri) assert_equal(["Bar", "Foo", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar")) @@ -800,7 +800,7 @@ class Bar end RUBY - @index.handle_change(uri) + @index.handle_change(uri, File.read(T.must(uri.full_path))) assert_empty(@index.instance_variable_get(:@ancestors)) assert_equal(["Bar", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar")) end @@ -821,7 +821,7 @@ class Bar RUBY uri = URI::Generic.from_path(path: File.join(dir, "foo.rb")) - @index.index_single(uri) + @index.index_file(uri) assert_equal(["Bar", "Foo", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar")) @@ -837,7 +837,7 @@ def baz; end end RUBY - @index.handle_change(uri) + @index.handle_change(uri, File.read(T.must(uri.full_path))) refute_empty(@index.instance_variable_get(:@ancestors)) assert_equal(["Bar", "Foo", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar")) end @@ -857,7 +857,7 @@ class Bar < Foo RUBY uri = URI::Generic.from_path(path: File.join(dir, "foo.rb")) - @index.index_single(uri) + @index.index_file(uri) assert_equal(["Bar", "Foo", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar")) @@ -870,7 +870,7 @@ class Bar end RUBY - @index.handle_change(uri) + @index.handle_change(uri, File.read(T.must(uri.full_path))) assert_empty(@index.instance_variable_get(:@ancestors)) assert_equal(["Bar", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar")) end @@ -1889,13 +1889,13 @@ def self.my_singleton_def; end end RUBY - entries = @index.entries_for("/fake/path/foo.rb", Entry) + entries = @index.entries_for("file:///fake/path/foo.rb", Entry) assert_equal(["Foo", "Bar", "my_def", "Bar::", "my_singleton_def"], entries.map(&:name)) - entries = @index.entries_for("/fake/path/foo.rb", RubyIndexer::Entry::Namespace) + entries = @index.entries_for("file:///fake/path/foo.rb", RubyIndexer::Entry::Namespace) assert_equal(["Foo", "Bar", "Bar::"], entries.map(&:name)) - entries = @index.entries_for("/fake/path/foo.rb") + entries = @index.entries_for("file:///fake/path/foo.rb") assert_equal(["Foo", "Bar", "my_def", "Bar::", "my_singleton_def"], entries.map(&:name)) end @@ -2066,5 +2066,30 @@ def test_prevents_multiple_calls_to_index_all @index.index_all end end + + def test_index_can_handle_entries_from_untitled_scheme + uri = URI("untitled:Untitled-1") + + index(<<~RUBY, uri: uri) + class Foo + end + RUBY + + entry = @index["Foo"]&.first + refute_nil(entry, "Expected indexer to be able to handle unsaved URIs") + assert_equal("untitled:Untitled-1", entry.uri.to_s) + assert_equal("Untitled-1", entry.file_name) + assert_nil(entry.file_path) + + @index.handle_change(uri, <<~RUBY) + # I added this comment! + class Foo + end + RUBY + + entry = @index["Foo"]&.first + refute_nil(entry, "Expected indexer to be able to handle unsaved URIs") + assert_equal("I added this comment!", entry.comments) + end end end diff --git a/lib/ruby_indexer/test/test_case.rb b/lib/ruby_indexer/test/test_case.rb index e949d34e8..7bc36c56a 100644 --- a/lib/ruby_indexer/test/test_case.rb +++ b/lib/ruby_indexer/test/test_case.rb @@ -13,8 +13,8 @@ def setup private - def index(source) - @index.index_single(URI::Generic.from_path(path: "/fake/path/foo.rb"), source) + def index(source, uri: URI::Generic.from_path(path: "/fake/path/foo.rb")) + @index.index_single(uri, source) end def assert_entry(expected_name, type, expected_location, visibility: nil) diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb index 60db56e8f..eb488cf4d 100644 --- a/lib/ruby_lsp/server.rb +++ b/lib/ruby_lsp/server.rb @@ -1004,11 +1004,13 @@ def workspace_did_change_watched_files(message) load_path_entry = $LOAD_PATH.find { |load_path| file_path.start_with?(load_path) } uri.add_require_path_from_load_entry(load_path_entry) if load_path_entry + content = File.read(file_path) + case change[:type] when Constant::FileChangeType::CREATED - index.index_single(uri) + index.index_single(uri, content) when Constant::FileChangeType::CHANGED - index.handle_change(uri) + index.handle_change(uri, content) when Constant::FileChangeType::DELETED index.delete(uri) end diff --git a/lib/ruby_lsp/test_helper.rb b/lib/ruby_lsp/test_helper.rb index ea91a4b34..98ecf6bf1 100644 --- a/lib/ruby_lsp/test_helper.rb +++ b/lib/ruby_lsp/test_helper.rb @@ -36,20 +36,21 @@ def with_server(source = nil, uri = Kernel.URI("file:///fake.rb"), stub_no_typec }, }, }) + + server.global_state.index.index_single(uri, source) end - server.global_state.index.index_single( - URI::Generic.from_path(path: T.must(uri.to_standardized_path)), - source, - ) server.load_addons(include_project_addons: false) if load_addons - block.call(server, uri) - ensure - if load_addons - RubyLsp::Addon.addons.each(&:deactivate) - RubyLsp::Addon.addons.clear + + begin + block.call(server, uri) + ensure + if load_addons + RubyLsp::Addon.addons.each(&:deactivate) + RubyLsp::Addon.addons.clear + end + server.run_shutdown end - T.must(server).run_shutdown end end end diff --git a/rakelib/index.rake b/rakelib/index.rake index 6b715d904..aee2d530d 100644 --- a/rakelib/index.rake +++ b/rakelib/index.rake @@ -84,8 +84,7 @@ task "index:topgems": ["download:topgems"] do errors = Dir[File.join(directory, "**", "*.rb")].filter_map do |filepath| print(".") - code = File.read(filepath) - index.index_single(URI::Generic.from_path(path: filepath), code) + index.index_file(URI::Generic.from_path(path: filepath)) nil rescue => e errors << { message: e.message, file: filepath } diff --git a/test/requests/completion_test.rb b/test/requests/completion_test.rb index ac441e569..8dff7f507 100644 --- a/test/requests/completion_test.rb +++ b/test/requests/completion_test.rb @@ -1600,7 +1600,7 @@ def with_file_structure(server, &block) URI::Generic.from_path(load_path_entry: tmpdir, path: path) end - uris.each { |uri| index.index_single(uri) } + uris.each { |uri| index.index_file(uri) } block.call(tmpdir) ensure $LOAD_PATH.delete(tmpdir) diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb index 340153760..a66867fc6 100644 --- a/test/requests/definition_expectations_test.rb +++ b/test/requests/definition_expectations_test.rb @@ -14,7 +14,7 @@ def run_expectations(source) index = server.global_state.index - index.index_single( + index.index_file( URI::Generic.from_path( load_path_entry: "#{Dir.pwd}/lib", path: File.expand_path( @@ -23,7 +23,7 @@ def run_expectations(source) ), ), ) - index.index_single( + index.index_file( URI::Generic.from_path( path: File.expand_path( "../../test/fixtures/constant_reference_target.rb", @@ -31,7 +31,7 @@ def run_expectations(source) ), ), ) - index.index_single( + index.index_file( URI::Generic.from_path( load_path_entry: "#{Dir.pwd}/lib", path: File.expand_path( @@ -75,7 +75,7 @@ def run_expectations(source) def test_jumping_to_default_gems with_server("Pathname") do |server, uri| index = server.global_state.index - index.index_single(URI::Generic.from_path(path: "#{RbConfig::CONFIG["rubylibdir"]}/pathname.rb")) + index.index_file(URI::Generic.from_path(path: "#{RbConfig::CONFIG["rubylibdir"]}/pathname.rb")) server.process_message( id: 1, method: "textDocument/definition", @@ -163,10 +163,10 @@ def test_jumping_to_default_require_of_a_gem path: "#{RbConfig::CONFIG["rubylibdir"]}/bundler.rb", load_path_entry: RbConfig::CONFIG["rubylibdir"], ) - index.index_single(bundler_uri) + index.index_file(bundler_uri) Dir.glob("#{RbConfig::CONFIG["rubylibdir"]}/bundler/*.rb").each do |path| - index.index_single(URI::Generic.from_path(load_path_entry: RbConfig::CONFIG["rubylibdir"], path: path)) + index.index_file(URI::Generic.from_path(load_path_entry: RbConfig::CONFIG["rubylibdir"], path: path)) end server.process_message( @@ -230,7 +230,7 @@ def test_definition_addons create_definition_addon with_server(source, stub_no_typechecker: true) do |server, uri| - server.global_state.index.index_single( + server.global_state.index.index_file( URI::Generic.from_path( load_path_entry: "#{Dir.pwd}/lib", path: File.expand_path( diff --git a/test/requests/workspace_symbol_test.rb b/test/requests/workspace_symbol_test.rb index 7e9441b99..edefc5d9d 100644 --- a/test/requests/workspace_symbol_test.rb +++ b/test/requests/workspace_symbol_test.rb @@ -66,7 +66,7 @@ class Bar; end end def test_does_not_include_symbols_from_dependencies - @index.index_single(URI::Generic.from_path(path: "#{RbConfig::CONFIG["rubylibdir"]}/pathname.rb")) + @index.index_file(URI::Generic.from_path(path: "#{RbConfig::CONFIG["rubylibdir"]}/pathname.rb")) result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "Pathname").perform assert_empty(result) diff --git a/test/server_test.rb b/test/server_test.rb index db5b95485..8cc87ce37 100644 --- a/test/server_test.rb +++ b/test/server_test.rb @@ -439,6 +439,7 @@ def test_backtrace_is_printed_to_stderr_on_exceptions end def test_changed_file_only_indexes_ruby + File.expects(:read).with("/foo.rb").returns("class Foo\nend") @server.global_state.index.expects(:index_single).once.with do |uri| uri.full_path == "/foo.rb" end