Only reindex documents if declarations are being modified

lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb

@@ -64,7 +64,6 @@ def initialize(index, dispatcher, parse_result, uri, collect_comments: false)
         :on_constant_path_or_write_node_enter,
         :on_constant_path_operator_write_node_enter,
         :on_constant_path_and_write_node_enter,
-        :on_constant_or_write_node_enter,
         :on_constant_write_node_enter,
         :on_constant_or_write_node_enter,
         :on_constant_and_write_node_enter,

lib/ruby_lsp/document.rb

@@ -40,6 +40,9 @@ class LocationNotFoundError < StandardError; end
     sig { returns(Encoding) }
     attr_reader :encoding
 
+    sig { returns(T.nilable(Edit)) }
+    attr_reader :last_edit
+
     sig { returns(T.any(Interface::SemanticTokens, Object)) }
     attr_accessor :semantic_tokens
 
@@ -54,6 +57,7 @@ def initialize(source:, version:, uri:, global_state:)
       @uri = T.let(uri, URI::Generic)
       @needs_parsing = T.let(true, T::Boolean)
       @parse_result = T.let(T.unsafe(nil), ParseResultType)
+      @last_edit = T.let(nil, T.nilable(Edit))
       parse!
     end
 
@@ -107,6 +111,19 @@ def push_edits(edits, version:)
         @version = version
         @needs_parsing = true
         @cache.clear
+
+        last_edit = edits.last
+        return unless last_edit
+
+        last_edit_range = last_edit[:range]
+
+        @last_edit = if last_edit_range[:start] == last_edit_range[:end]
+          Insert.new(last_edit_range)
+        elsif last_edit[:text].empty?
+          Delete.new(last_edit_range)
+        else
+          Replace.new(last_edit_range)
+        end
       end
     end
 
@@ -144,6 +161,25 @@ def create_scanner
       Scanner.new(@source, @encoding)
     end
 
+    class Edit
+      extend T::Sig
+      extend T::Helpers
+
+      abstract!
+
+      sig { returns(T::Hash[Symbol, T.untyped]) }
+      attr_reader :range
+
+      sig { params(range: T::Hash[Symbol, T.untyped]).void }
+      def initialize(range)
+        @range = range
+      end
+    end
+
+    class Insert < Edit; end
+    class Replace < Edit; end
+    class Delete < Edit; end
+
     class Scanner
       extend T::Sig
 

lib/ruby_lsp/ruby_document.rb

@@ -8,6 +8,22 @@ class RubyDocument < Document
 
     ParseResultType = type_member { { fixed: Prism::ParseResult } }
 
+    METHODS_THAT_CHANGE_DECLARATIONS = [
+      :private_constant,
+      :attr_reader,
+      :attr_writer,
+      :attr_accessor,
+      :alias_method,
+      :include,
+      :prepend,
+      :extend,
+      :public,
+      :protected,
+      :private,
+      :module_function,
+      :private_class_method,
+    ].freeze
+
     class SorbetLevel < T::Enum
       enums do
         None = new("none")
@@ -240,5 +256,57 @@ def locate_node(position, node_types: [])
         node_types: node_types,
       )
     end
+
+    sig { returns(T::Boolean) }
+    def last_edit_may_change_declarations?
+      # This method controls when we should index documents. If there's no recent edit and the document has just been
+      # opened, we need to index it
+      return true unless @last_edit
+
+      case @last_edit
+      when Delete
+        # Not optimized yet. It's not trivial to identify that a declaration has been removed since the source is no
+        # longer there and we don't remember the deleted text
+        true
+      when Insert, Replace
+        position_may_impact_declarations?(@last_edit.range[:start])
+      else
+        false
+      end
+    end
+
+    private
+
+    sig { params(position: T::Hash[Symbol, Integer]).returns(T::Boolean) }
+    def position_may_impact_declarations?(position)
+      node_context = locate_node(position)
+      node_at_edit = node_context.node
+
+      # Adjust to the parent when editing the constant of a class/module declaration
+      if node_at_edit.is_a?(Prism::ConstantReadNode) || node_at_edit.is_a?(Prism::ConstantPathNode)
+        node_at_edit = node_context.parent
+      end
+
+      case node_at_edit
+      when Prism::ClassNode, Prism::ModuleNode, Prism::SingletonClassNode, Prism::DefNode,
+          Prism::ConstantPathWriteNode, Prism::ConstantPathOrWriteNode, Prism::ConstantPathOperatorWriteNode,
+          Prism::ConstantPathAndWriteNode, Prism::ConstantOrWriteNode, Prism::ConstantWriteNode,
+          Prism::ConstantAndWriteNode, Prism::ConstantOperatorWriteNode, Prism::GlobalVariableAndWriteNode,
+          Prism::GlobalVariableOperatorWriteNode, Prism::GlobalVariableOrWriteNode, Prism::GlobalVariableTargetNode,
+          Prism::GlobalVariableWriteNode, Prism::InstanceVariableWriteNode, Prism::InstanceVariableAndWriteNode,
+          Prism::InstanceVariableOperatorWriteNode, Prism::InstanceVariableOrWriteNode,
+          Prism::InstanceVariableTargetNode, Prism::AliasMethodNode
+        true
+      when Prism::MultiWriteNode
+        [*node_at_edit.lefts, *node_at_edit.rest, *node_at_edit.rights].any? do |node|
+          node.is_a?(Prism::ConstantTargetNode) || node.is_a?(Prism::ConstantPathTargetNode)
+        end
+      when Prism::CallNode
+        receiver = node_at_edit.receiver
+        (!receiver || receiver.is_a?(Prism::SelfNode)) && METHODS_THAT_CHANGE_DECLARATIONS.include?(node_at_edit.name)
+      else
+        false
+      end
+    end
   end
 end

lib/ruby_lsp/server.rb

@@ -464,11 +464,19 @@ def run_combined_requests(message)
       code_lens = Requests::CodeLens.new(@global_state, uri, dispatcher)
       inlay_hint = Requests::InlayHints.new(document, T.must(@store.features_configuration.dig(:inlayHint)), dispatcher)
 
-      # 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)
+      if document.is_a?(RubyDocument) && document.last_edit_may_change_declarations?
+        # 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.synchronize do
+          send_log_message("Detected that last edit may have modified declarations. Re-indexing #{uri}")
+
+          @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
+        end
+      else
         dispatcher.dispatch(parse_result.value)
       end
 

test/requests/code_lens_expectations_test.rb

@@ -203,6 +203,9 @@ class Test < Minitest::Test; end
           params: { textDocument: { uri: uri }, position: { line: 1, character: 2 } },
         })
 
+        # Pop the re-indexing notification
+        server.pop_response
+
         result = server.pop_response
         assert_instance_of(RubyLsp::Result, result)
 

test/requests/document_symbol_expectations_test.rb

@@ -89,6 +89,10 @@ class Foo
           method: "textDocument/documentSymbol",
           params: { textDocument: { uri: uri } },
         })
+
+        # Pop the re-indexing notification
+        server.pop_response
+
         result = server.pop_response
         assert_instance_of(RubyLsp::Result, result)
 

test/ruby_document_test.rb

@@ -805,6 +805,73 @@ class Foo
     MESSAGE
   end
 
+  def test_document_tracks_latest_edit_context
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
+      class Foo
+
+      end
+    RUBY
+
+    # Insert
+    range = { start: { line: 1, character: 0 }, end: { line: 1, character: 0 } }
+    document.push_edits([{ range: range, text: "d" }], version: 2)
+
+    last_edit = T.must(document.last_edit)
+    assert_instance_of(RubyLsp::Document::Insert, last_edit)
+    assert_equal(range, last_edit.range)
+
+    # Replace
+    range = { start: { line: 1, character: 0 }, end: { line: 1, character: 1 } }
+    document.push_edits([{ range: range, text: "def" }], version: 3)
+
+    last_edit = T.must(document.last_edit)
+    assert_instance_of(RubyLsp::Document::Replace, last_edit)
+    assert_equal(range, last_edit.range)
+
+    # Delete
+    range = { start: { line: 1, character: 0 }, end: { line: 1, character: 3 } }
+    document.push_edits([{ range: range, text: "" }], version: 4)
+
+    last_edit = T.must(document.last_edit)
+    assert_instance_of(RubyLsp::Document::Delete, last_edit)
+    assert_equal(range, last_edit.range)
+
+    assert_equal(<<~RUBY, document.source)
+      class Foo
+
+      end
+    RUBY
+  end
+
+  def test_last_edit_may_change_declarations_for_inserts
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
+      class Foo
+      end
+    RUBY
+    assert_predicate(document, :last_edit_may_change_declarations?)
+
+    range = { start: { line: 0, character: 9 }, end: { line: 0, character: 9 } }
+    document.push_edits([{ range: range, text: "t" }], version: 2)
+
+    assert_instance_of(RubyLsp::Document::Insert, document.last_edit)
+    assert_predicate(document, :last_edit_may_change_declarations?)
+  end
+
+  def test_last_edit_may_change_declarations_for_replaces
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
+      class Foo
+      end
+    RUBY
+
+    assert_predicate(document, :last_edit_may_change_declarations?)
+
+    range = { start: { line: 0, character: 6 }, end: { line: 0, character: 9 } }
+    document.push_edits([{ range: range, text: "Bar" }], version: 2)
+
+    assert_instance_of(RubyLsp::Document::Replace, document.last_edit)
+    assert_predicate(document, :last_edit_may_change_declarations?)
+  end
+
   private
 
   def assert_error_edit(actual, error_range)

test/server_test.rb

@@ -925,6 +925,61 @@ class Foo
     assert_equal(["Foo::<Class:Foo>"], index.linearized_ancestors_of("Foo::<Class:Foo>"))
   end
 
+  def test_edits_outside_of_declarations_do_not_trigger_indexing
+    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\n\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: "d", 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
+    index.expects(:delete).never
+    @server.process_message({
+      id: 2,
+      method: "textDocument/documentSymbol",
+      params: { textDocument: { uri: uri } },
+    })
+
+    entries = index["Foo"]
+    assert_equal(1, entries.length)
+  end
+
   private
 
   def with_uninstalled_rubocop(&block)