Index files as they are modified

lib/ruby_indexer/lib/ruby_indexer/index.rb

@@ -469,16 +469,23 @@ def instance_variable_completion_candidates(name, owner_name)
 
     # 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)
-      uri = indexable.to_uri.to_s
-      original_entries = @uris_to_entries[uri]
+    sig { params(uri: URI::Generic, indexable: IndexablePath).void }
+    def handle_change(uri, indexable)
+      handling_ancestor_change(uri) do
+        delete(indexable)
+        index_single(indexable)
+      end
+    end
 
-      delete(indexable)
-      index_single(indexable)
+    # Handle ancestor changes that may occur when invoking the given block. The goal of this method is to automatically
+    # synchronize ancestors when handling changes in the index
+    sig { params(indexable_or_uri: T.any(IndexablePath, URI::Generic), block: T.proc.void).void }
+    def handling_ancestor_change(indexable_or_uri, &block)
+      uri = (indexable_or_uri.is_a?(IndexablePath) ? indexable_or_uri.to_uri : indexable_or_uri).to_s
 
+      original_entries = @uris_to_entries[uri]
+      block.call
       updated_entries = @uris_to_entries[uri]
-
       return unless original_entries && updated_entries
 
       # A change in one ancestor may impact several different others, which could be including that ancestor through

lib/ruby_indexer/test/index_test.rb

@@ -796,7 +796,7 @@ class Bar
             end
           RUBY
 
-          @index.handle_change(indexable_path)
+          @index.handle_change(indexable_path.to_uri, indexable_path)
           assert_empty(@index.instance_variable_get(:@ancestors))
           assert_equal(["Bar", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar"))
         end
@@ -833,7 +833,7 @@ def baz; end
             end
           RUBY
 
-          @index.handle_change(indexable_path)
+          @index.handle_change(indexable_path.to_uri, indexable_path)
           refute_empty(@index.instance_variable_get(:@ancestors))
           assert_equal(["Bar", "Foo", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar"))
         end
@@ -866,7 +866,7 @@ class Bar
             end
           RUBY
 
-          @index.handle_change(indexable_path)
+          @index.handle_change(indexable_path.to_uri, indexable_path)
           assert_empty(@index.instance_variable_get(:@ancestors))
           assert_equal(["Bar", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Bar"))
         end

lib/ruby_lsp/server.rb

@@ -297,8 +297,9 @@ def text_document_did_open(message)
 
     sig { params(message: T::Hash[Symbol, T.untyped]).void }
     def text_document_did_close(message)
+      uri = message.dig(:params, :textDocument, :uri)
+
       @mutex.synchronize do
-        uri = message.dig(:params, :textDocument, :uri)
         @store.delete(uri)
 
         # Clear diagnostics for the closed file, so that they no longer appear in the problems tab
@@ -309,6 +310,12 @@ def text_document_did_close(message)
           ),
         )
       end
+
+      # When we receive a didClose notification for an unsaved (which happens when the user saves the file or when it's
+      # simply closed), we need to remove declarations related to it from the index because immediately after we will
+      # receive a didChangeWatchedFiles notification related to the saved file creation. Otherwise, we end up with
+      # duplicate entries
+      @global_state.index.delete(uri) if uri.scheme == "untitled"
     end
 
     sig { params(message: T::Hash[Symbol, T.untyped]).void }
@@ -362,9 +369,21 @@ def run_combined_requests(message)
       document_symbol = Requests::DocumentSymbol.new(uri, dispatcher)
       document_link = Requests::DocumentLink.new(uri, parse_result.comments, dispatcher)
       code_lens = Requests::CodeLens.new(@global_state, uri, dispatcher)
-
       semantic_highlighting = Requests::SemanticHighlighting.new(@global_state, dispatcher)
-      dispatcher.dispatch(parse_result.value)
+
+      # If we're running the combined requests for a Ruby document and that document's URI is either `file` or
+      # `untitled` (a save vs an unsaved Ruby file), then we run indexing at the same time to capture new declarations.
+      # The reason we have to check for the scheme is because, if we were to accidentally index a URI for a `git` scheme
+      # for example, we would end up with duplicate entries in the index
+      if document.is_a?(RubyDocument) && (uri.scheme == "file" || uri.scheme == "untitled")
+        @global_state.index.handling_ancestor_change(uri) do
+          @global_state.index.delete(uri)
+          RubyIndexer::DeclarationListener.new(@global_state.index, dispatcher, parse_result, uri)
+          dispatcher.dispatch(parse_result.value)
+        end
+      else
+        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
@@ -668,7 +687,9 @@ def workspace_did_change_watched_files(message)
         when Constant::FileChangeType::CREATED
           index.index_single(indexable)
         when Constant::FileChangeType::CHANGED
-          index.handle_change(indexable)
+          # We only want to handle changes if the files being modified are not currently opened in the editor. For files
+          # that are open, the indexing happens when they are modified as part of running the combined requests
+          index.handle_change(uri, indexable) unless @store.has?(uri)
         when Constant::FileChangeType::DELETED
           index.delete(indexable)
         end

lib/ruby_lsp/store.rb

@@ -95,6 +95,13 @@ def delete(uri)
       @state.delete(uri.to_s)
     end
 
+    # Returns `true` if the store contains the document with the given URI, which means that document is being managed
+    # by the client
+    sig { params(uri: URI::Generic).returns(T::Boolean) }
+    def has?(uri)
+      @state.key?(uri.to_s)
+    end
+
     sig do
       type_parameters(:T)
         .params(

test/server_test.rb

@@ -428,6 +428,23 @@ def test_changed_file_only_indexes_ruby
     })
   end
 
+  def test_handles_file_modifications_for_documents_not_managed_by_client
+    @server.global_state.index.expects(:index_single).once.with do |indexable|
+      indexable.full_path == "/foo.rb"
+    end
+    @server.process_message({
+      method: "workspace/didChangeWatchedFiles",
+      params: {
+        changes: [
+          {
+            uri: URI("file:///foo.rb"),
+            type: RubyLsp::Constant::FileChangeType::CHANGED,
+          },
+        ],
+      },
+    })
+  end
+
   def test_workspace_addons
     create_test_addons
     @server.load_addons
@@ -544,6 +561,74 @@ def test_closing_document_before_computing_features_does_not_error
     end
   end
 
+  def test_indexing_on_file_changes
+    uri = URI("untitled:Untitled-1")
+    index = @server.global_state.index
+
+    capture_io do
+      @server.process_message({
+        method: "textDocument/didOpen",
+        params: {
+          textDocument: {
+            uri: uri,
+            text: "class Foo\nend",
+            version: 1,
+            languageId: "ruby",
+          },
+        },
+      })
+
+      # Initial indexing for an unsaved file
+      @server.process_message({
+        id: 1,
+        method: "textDocument/documentSymbol",
+        params: {
+          textDocument: {
+            uri: uri,
+          },
+        },
+      })
+
+      assert_equal(1, index["Foo"].length)
+
+      # Changing the file should re-index, but not create duplicates
+      @server.process_message({
+        id: 1,
+        method: "textDocument/didChange",
+        params: {
+          textDocument: {
+            uri: uri,
+            version: 2,
+          },
+          contentChanges: [{ range: { start: { line: 0, character: 9 }, end: { line: 0, character: 9 } }, text: "\n" }],
+        },
+      })
+
+      @server.process_message({
+        id: 1,
+        method: "textDocument/documentSymbol",
+        params: {
+          textDocument: {
+            uri: uri,
+          },
+        },
+      })
+
+      assert_equal(1, index["Foo"].length)
+
+      # Closing the file should clear the related entries
+      @server.process_message({
+        method: "textDocument/didClose",
+        params: {
+          textDocument: {
+            uri: uri,
+          },
+        },
+      })
+      assert_nil(index["Foo"])
+    end
+  end
+
   private
 
   def with_uninstalled_rubocop(&block)