Skip to content

Commit

Permalink
Only reindex documents if declarations are being modified
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Nov 29, 2024
1 parent 826bd1a commit c103d7c
Show file tree
Hide file tree
Showing 6 changed files with 238 additions and 11 deletions.
1 change: 0 additions & 1 deletion lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
36 changes: 36 additions & 0 deletions lib/ruby_lsp/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -53,6 +56,7 @@ def initialize(source:, version:, uri:, encoding: Encoding::UTF_8)
@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

Expand Down Expand Up @@ -106,6 +110,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

# Returns `true` if the document was parsed and `false` if nothing needed parsing
Expand All @@ -125,6 +142,25 @@ def past_expensive_limit?
@source.length > MAXIMUM_CHARACTERS_FOR_EXPENSIVE_FEATURES
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

Expand Down
68 changes: 68 additions & 0 deletions lib/ruby_lsp/ruby_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -239,5 +255,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
22 changes: 12 additions & 10 deletions lib/ruby_lsp/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -494,16 +494,18 @@ 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.delete(uri, skip_require_tree: true)
RubyIndexer::DeclarationListener.new(
@global_state.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.index.delete(uri, skip_require_tree: true)
RubyIndexer::DeclarationListener.new(
@global_state.index,
dispatcher,
parse_result,
uri,
collect_comments: true,
)
end

dispatcher.dispatch(parse_result.value)

Expand Down
67 changes: 67 additions & 0 deletions test/ruby_document_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,73 @@ class Foo
MESSAGE
end

def test_document_tracks_latest_edit_context
document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: URI("file:///foo/bar.rb"))
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: 3)

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("file:///foo/bar.rb"))
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("file:///foo/bar.rb"))
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)
Expand Down
55 changes: 55 additions & 0 deletions test/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,61 @@ def test_unsaved_changes_are_indexed_when_computing_automatic_features
assert_equal(1, entries.length)
end

def test_edits_outside_of_declarations_do_not_trigger_reindexing
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)
Expand Down

0 comments on commit c103d7c

Please sign in to comment.