diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index 3c60163e3..73aca7a59 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -48,6 +48,8 @@ def initialize ) @configuration = T.let(RubyIndexer::Configuration.new, Configuration) + + @initial_indexing_completed = T.let(false, T::Boolean) end # Register an included `hook` that will be executed when `module_name` is included into any namespace @@ -56,8 +58,8 @@ def register_included_hook(module_name, &hook) (@included_hooks[module_name] ||= []) << hook end - sig { params(uri: URI::Generic).void } - def delete(uri) + sig { params(uri: URI::Generic, skip_require_paths_tree: T::Boolean).void } + def delete(uri, skip_require_paths_tree: false) 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. @@ -80,6 +82,7 @@ def delete(uri) end @uris_to_entries.delete(key) + return if skip_require_paths_tree require_path = uri.require_path @require_paths_tree.delete(require_path) if require_path @@ -357,12 +360,13 @@ def index_all(uris: @configuration.indexable_uris, &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. - # binding.break - if @entries.any? + if @initial_indexing_completed raise IndexNotEmptyError, "The index is not empty. To prevent invalid entries, `index_all` can only be called once." end + @initial_indexing_completed = true + RBSIndexer.new(self).index_ruby_core # Calculate how many paths are worth 1% of progress progress_step = (uris.length / 100.0).ceil @@ -597,17 +601,24 @@ def instance_variable_completion_candidates(name, owner_name) end # 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, source: String).void } - def handle_change(uri, source) + # declarations removed and that the ancestor linearization cache is cleared if necessary. If a block is passed, the + # consumer of this API has to handle deleting and inserting/updating entries in the index instead of passing the + # document's source (used to handle unsaved changes to files) + sig do + params(uri: URI::Generic, source: T.nilable(String), block: T.nilable(T.proc.params(index: Index).void)).void + end + def handle_change(uri, source = nil, &block) key = uri.to_s original_entries = @uris_to_entries[key] - delete(uri) - index_single(uri, source) + if block + block.call(self) + else + delete(uri) + index_single(uri, T.must(source)) + end updated_entries = @uris_to_entries[key] - return unless original_entries && updated_entries # A change in one ancestor may impact several different others, which could be including that ancestor through diff --git a/lib/ruby_indexer/test/index_test.rb b/lib/ruby_indexer/test/index_test.rb index 80a5a8dc4..c79c6d2ec 100644 --- a/lib/ruby_indexer/test/index_test.rb +++ b/lib/ruby_indexer/test/index_test.rb @@ -2061,7 +2061,8 @@ def test_build_non_redundant_name end def test_prevents_multiple_calls_to_index_all - # For this test class, `index_all` is already called once in `setup`. + @index.index_all + assert_raises(Index::IndexNotEmptyError) do @index.index_all end diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb index f975e065e..a3f1976a4 100644 --- a/lib/ruby_lsp/server.rb +++ b/lib/ruby_lsp/server.rb @@ -493,7 +493,14 @@ def run_combined_requests(message) document_link = Requests::DocumentLink.new(uri, parse_result.comments, dispatcher) code_lens = Requests::CodeLens.new(@global_state, uri, dispatcher) inlay_hint = Requests::InlayHints.new(document, T.must(@store.features_configuration.dig(:inlayHint)), dispatcher) - dispatcher.dispatch(parse_result.value) + + # Re-index the file as it is modified. This mode of indexing updates entries only. Require path trees are only + # updated on save + @global_state.index.handle_change(uri) do |index| + index.delete(uri, skip_require_paths_tree: true) + RubyIndexer::DeclarationListener.new(index, dispatcher, parse_result, uri, collect_comments: true) + dispatcher.dispatch(parse_result.value) + end # Store all responses retrieve in this round of visits in the cache and then return the response for the request # we actually received diff --git a/test/server_test.rb b/test/server_test.rb index fae9bef5a..735ace549 100644 --- a/test/server_test.rb +++ b/test/server_test.rb @@ -796,6 +796,136 @@ def test_cancelling_requests_returns_expected_error_code assert_equal("Request 1 was cancelled", error.message) end + def test_unsaved_changes_are_indexed_when_computing_automatic_features + uri = URI("file:///foo.rb") + index = @server.global_state.index + + # Simulate opening a file. First, send the notification to open the file with a class inside + @server.process_message({ + method: "textDocument/didOpen", + params: { + textDocument: { + uri: uri, + text: +"class Foo\nend", + version: 1, + languageId: "ruby", + }, + }, + }) + # Fire the automatic features requests to trigger indexing + @server.process_message({ + id: 1, + method: "textDocument/documentSymbol", + params: { textDocument: { uri: uri } }, + }) + + entries = index["Foo"] + assert_equal(1, entries.length) + + # Modify the file without saving + @server.process_message({ + method: "textDocument/didChange", + params: { + textDocument: { uri: uri, version: 2 }, + contentChanges: [ + { text: " def bar\n end\n", range: { start: { line: 1, character: 0 }, end: { line: 1, character: 0 } } }, + ], + }, + }) + + # Parse the document after it was modified. This occurs automatically when we receive a text document request, to + # avoid parsing the document multiple times, but that depends on request coming in through the STDIN pipe, which + # isn't reproduced here. Parsing manually matches what happens normally + store = @server.instance_variable_get(:@store) + store.get(uri).parse! + + # Trigger the automatic features again + @server.process_message({ + id: 2, + method: "textDocument/documentSymbol", + params: { textDocument: { uri: uri } }, + }) + + # There should still only be one entry for each declaration, but we should have picked up the new ones + entries = index["Foo"] + assert_equal(1, entries.length) + + entries = index["bar"] + assert_equal(1, entries.length) + end + + def test_ancestors_are_recomputed_even_on_unsaved_changes + uri = URI("file:///foo.rb") + index = @server.global_state.index + source = +<<~RUBY + module Bar; end + + class Foo + extend Bar + end + RUBY + + # Simulate opening a file. First, send the notification to open the file with a class inside + @server.process_message({ + method: "textDocument/didOpen", + params: { + textDocument: { + uri: uri, + text: source, + version: 1, + languageId: "ruby", + }, + }, + }) + # Fire the automatic features requests to trigger indexing + @server.process_message({ + id: 1, + method: "textDocument/documentSymbol", + params: { textDocument: { uri: uri } }, + }) + + assert_equal(["Foo::", "Bar"], index.linearized_ancestors_of("Foo::")) + + # Delete the extend + @server.process_message({ + method: "textDocument/didChange", + params: { + textDocument: { uri: uri, version: 2 }, + contentChanges: [ + { text: "", range: { start: { line: 3, character: 0 }, end: { line: 3, character: 12 } } }, + ], + }, + }) + + # Parse the document after it was modified. This occurs automatically when we receive a text document request, to + # avoid parsing the document multiple times, but that depends on request coming in through the STDIN pipe, which + # isn't reproduced here. Parsing manually matches what happens normally + store = @server.instance_variable_get(:@store) + document = store.get(uri) + + assert_equal(<<~RUBY, document.source) + module Bar; end + + class Foo + + end + RUBY + + document.parse! + + # Trigger the automatic features again + @server.process_message({ + id: 2, + method: "textDocument/documentSymbol", + params: { textDocument: { uri: uri } }, + }) + + result = find_message(RubyLsp::Result, id: 2) + refute_nil(result) + + assert_equal(["Foo::"], index.linearized_ancestors_of("Foo::")) + end + private def with_uninstalled_rubocop(&block)