From 0a4e09c45d1ec18050836cc832d2c20a0cc40154 Mon Sep 17 00:00:00 2001 From: Jose Camacho Date: Mon, 27 Jan 2025 17:29:04 -0600 Subject: [PATCH 1/6] avoid collecting duplicates when collecting references --- .../lib/ruby_indexer/reference_finder.rb | 5 ++-- .../test/reference_finder_test.rb | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index 3c24f0450..fb7a4d749 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -341,15 +341,16 @@ def collect_constant_references(name, location) entries = @index.resolve(name, @stack) return unless entries - previous_reference = @references.last + previous_locations = [@references.last&.location].compact entries.each do |entry| next unless entry.name == @target.fully_qualified_name # When processing a class/module declaration, we eagerly handle the constant reference. To avoid duplicates, # when we find the constant node defining the namespace, then we have to check if it wasn't already added - next if previous_reference&.location == location + next if previous_locations.find{ |previous_location| location == previous_location } + previous_locations << location @references << Reference.new(name, location, declaration: false) end end diff --git a/lib/ruby_indexer/test/reference_finder_test.rb b/lib/ruby_indexer/test/reference_finder_test.rb index efc047514..45e3fe7f9 100644 --- a/lib/ruby_indexer/test/reference_finder_test.rb +++ b/lib/ruby_indexer/test/reference_finder_test.rb @@ -274,6 +274,30 @@ def name assert_equal(8, refs[1].location.start_line) end + def test_accounts_for_reopened_classes + refs = find_const_references("Foo", <<~RUBY) + class Foo + end + class Foo + class Bar + end + end + + Foo.new + RUBY + + assert_equal(3, refs.size) + + assert_equal("Foo", refs[0].name) + assert_equal(1, refs[0].location.start_line) + + assert_equal("Foo", refs[1].name) + assert_equal(3, refs[1].location.start_line) + + assert_equal("Foo", refs[2].name) + assert_equal(8, refs[2].location.start_line) + end + private def find_const_references(const_name, source) From 2abd76b70fc8267a98aa104eba075120ec1e857b Mon Sep 17 00:00:00 2001 From: Jose Camacho Date: Tue, 28 Jan 2025 11:39:06 -0600 Subject: [PATCH 2/6] stop collectin constant on node enter, change collect constant logic --- .../lib/ruby_indexer/reference_finder.rb | 43 +++++++------------ .../test/reference_finder_test.rb | 2 +- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index fb7a4d749..3a2afaeeb 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -126,15 +126,7 @@ def references sig { params(node: Prism::ClassNode).void } def on_class_node_enter(node) - constant_path = node.constant_path - name = constant_path.slice - nesting = actual_nesting(name) - - if @target.is_a?(ConstTarget) && nesting.join("::") == @target.fully_qualified_name - @references << Reference.new(name, constant_path.location, declaration: true) - end - - @stack << name + @stack << node.constant_path.slice end sig { params(node: Prism::ClassNode).void } @@ -144,15 +136,7 @@ def on_class_node_leave(node) sig { params(node: Prism::ModuleNode).void } def on_module_node_enter(node) - constant_path = node.constant_path - name = constant_path.slice - nesting = actual_nesting(name) - - if @target.is_a?(ConstTarget) && nesting.join("::") == @target.fully_qualified_name - @references << Reference.new(name, constant_path.location, declaration: true) - end - - @stack << name + @stack << node.constant_path.slice end sig { params(node: Prism::ModuleNode).void } @@ -341,18 +325,23 @@ def collect_constant_references(name, location) entries = @index.resolve(name, @stack) return unless entries - previous_locations = [@references.last&.location].compact + previous_reference = @references.last + entry = entries.first - entries.each do |entry| - next unless entry.name == @target.fully_qualified_name + return unless entry.name == @target.fully_qualified_name - # When processing a class/module declaration, we eagerly handle the constant reference. To avoid duplicates, - # when we find the constant node defining the namespace, then we have to check if it wasn't already added - next if previous_locations.find{ |previous_location| location == previous_location } + # When processing a class/module declaration, we eagerly handle the constant reference. To avoid duplicates, + # when we find the constant node defining the namespace, then we have to check if it wasn't already added + return if previous_reference&.location == location - previous_locations << location - @references << Reference.new(name, location, declaration: false) - end + # Get the fully qualified name of the constant we just resolved + full_constant_name = T.must(entry).name + + # Check if this constant is a namespace declaration + # (class or module) + declaration = @index[full_constant_name].any? { |e| e.is_a?(Entry::Namespace) } + + @references << Reference.new(name, location, declaration: declaration) end sig { params(name: String, location: Prism::Location, declaration: T::Boolean).void } diff --git a/lib/ruby_indexer/test/reference_finder_test.rb b/lib/ruby_indexer/test/reference_finder_test.rb index 45e3fe7f9..6cf17e303 100644 --- a/lib/ruby_indexer/test/reference_finder_test.rb +++ b/lib/ruby_indexer/test/reference_finder_test.rb @@ -285,7 +285,7 @@ class Bar Foo.new RUBY - + assert_equal(3, refs.size) assert_equal("Foo", refs[0].name) From 323a1e7bf112c2c41901774bc53fc0b0b288d70d Mon Sep 17 00:00:00 2001 From: Jose Camacho Date: Wed, 29 Jan 2025 14:06:53 -0600 Subject: [PATCH 3/6] change matching logic --- lib/ruby_indexer/lib/ruby_indexer/location.rb | 12 +++++++ .../lib/ruby_indexer/reference_finder.rb | 34 +++++++++++-------- .../test/reference_finder_test.rb | 5 +-- lib/ruby_lsp/requests/references.rb | 1 + lib/ruby_lsp/requests/rename.rb | 2 +- 5 files changed, 37 insertions(+), 17 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/location.rb b/lib/ruby_indexer/lib/ruby_indexer/location.rb index 08c9553b9..711d9af36 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/location.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/location.rb @@ -44,5 +44,17 @@ def initialize(start_line, end_line, start_column, end_column) @start_column = start_column @end_column = end_column end + + sig do + params( + another_location: T.any(Location, Prism::Location) + ).returns(T::Boolean) + end + def ==(another_location) + start_line == another_location.start_line && + end_line == another_location.end_line && + start_column == another_location.start_column && + end_column == another_location.end_column + end end end diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index 3a2afaeeb..4656fc4a3 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -75,12 +75,14 @@ def initialize(name, location, declaration:) target: Target, index: RubyIndexer::Index, dispatcher: Prism::Dispatcher, + uri: URI::Generic, include_declarations: T::Boolean, ).void end - def initialize(target, index, dispatcher, include_declarations: true) + def initialize(target, index, dispatcher, uri, include_declarations: true) @target = target @index = index + @uri = uri @include_declarations = include_declarations @stack = T.let([], T::Array[String]) @references = T.let([], T::Array[Reference]) @@ -325,21 +327,25 @@ def collect_constant_references(name, location) entries = @index.resolve(name, @stack) return unless entries - previous_reference = @references.last - entry = entries.first - - return unless entry.name == @target.fully_qualified_name - - # When processing a class/module declaration, we eagerly handle the constant reference. To avoid duplicates, - # when we find the constant node defining the namespace, then we have to check if it wasn't already added - return if previous_reference&.location == location + # Filter down to all constant declarations that match the expected name and type + matching_entries = entries.select do |e| + [ + Entry::Namespace, + Entry::Constant, + Entry::ConstantAlias, + Entry::UnresolvedConstantAlias, + Entry::Class + ].include?(e.class) && + e.name == @target.fully_qualified_name + end - # Get the fully qualified name of the constant we just resolved - full_constant_name = T.must(entry).name + return if matching_entries.empty? - # Check if this constant is a namespace declaration - # (class or module) - declaration = @index[full_constant_name].any? { |e| e.is_a?(Entry::Namespace) } + # If any of the matching entries have the same location as the constant and were + # defined in the same file, then it is that constant's declaration + declaration = matching_entries.any? do |e| + e.name_location == location && e.uri == @uri + end @references << Reference.new(name, location, declaration: declaration) end diff --git a/lib/ruby_indexer/test/reference_finder_test.rb b/lib/ruby_indexer/test/reference_finder_test.rb index 6cf17e303..4f11f3be4 100644 --- a/lib/ruby_indexer/test/reference_finder_test.rb +++ b/lib/ruby_indexer/test/reference_finder_test.rb @@ -317,11 +317,12 @@ def find_instance_variable_references(instance_variable_name, source) def find_references(target, source) file_path = "/fake.rb" + uri = URI::Generic.from_path(path: file_path) index = Index.new - index.index_single(URI::Generic.from_path(path: file_path), source) + index.index_single(uri, source) parse_result = Prism.parse(source) dispatcher = Prism::Dispatcher.new - finder = ReferenceFinder.new(target, index, dispatcher) + finder = ReferenceFinder.new(target, index, dispatcher, uri) dispatcher.visit(parse_result.value) finder.references end diff --git a/lib/ruby_lsp/requests/references.rb b/lib/ruby_lsp/requests/references.rb index 919190b2d..8fb99597d 100644 --- a/lib/ruby_lsp/requests/references.rb +++ b/lib/ruby_lsp/requests/references.rb @@ -158,6 +158,7 @@ def collect_references(target, parse_result, uri) target, @global_state.index, dispatcher, + uri, include_declarations: @params.dig(:context, :includeDeclaration) || true, ) dispatcher.visit(parse_result.value) diff --git a/lib/ruby_lsp/requests/rename.rb b/lib/ruby_lsp/requests/rename.rb index 4e588586a..d3947c020 100644 --- a/lib/ruby_lsp/requests/rename.rb +++ b/lib/ruby_lsp/requests/rename.rb @@ -179,7 +179,7 @@ def collect_text_edits(target, name) end def collect_changes(target, parse_result, name, uri) dispatcher = Prism::Dispatcher.new - finder = RubyIndexer::ReferenceFinder.new(target, @global_state.index, dispatcher) + finder = RubyIndexer::ReferenceFinder.new(target, @global_state.index, dispatcher, uri) dispatcher.visit(parse_result.value) finder.references.map do |reference| From de154abbc62e5c3f3357cfb7bd0fa4fafdb14dba Mon Sep 17 00:00:00 2001 From: Victor Jose Camacho Date: Thu, 30 Jan 2025 10:05:44 -0600 Subject: [PATCH 4/6] Update lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb Co-authored-by: Vinicius Stock --- lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index 4656fc4a3..f886b3f74 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -334,8 +334,7 @@ def collect_constant_references(name, location) Entry::Constant, Entry::ConstantAlias, Entry::UnresolvedConstantAlias, - Entry::Class - ].include?(e.class) && + ].any? { |klass| e.is_a?(klass) } && e.name == @target.fully_qualified_name end From be19bbf43a9503305f0ad0e677328aae4d4d43bf Mon Sep 17 00:00:00 2001 From: Jose Camacho Date: Thu, 30 Jan 2025 11:59:06 -0600 Subject: [PATCH 5/6] sorbet tc --- lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index f886b3f74..8b040a4f9 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -343,7 +343,7 @@ def collect_constant_references(name, location) # If any of the matching entries have the same location as the constant and were # defined in the same file, then it is that constant's declaration declaration = matching_entries.any? do |e| - e.name_location == location && e.uri == @uri + e.uri == @uri && e.name_location == location end @references << Reference.new(name, location, declaration: declaration) From 491c694ecbcf2e8fa1178fca716a7b5220f57229 Mon Sep 17 00:00:00 2001 From: Jose Camacho Date: Fri, 31 Jan 2025 14:48:39 -0600 Subject: [PATCH 6/6] rubocop --- lib/ruby_indexer/lib/ruby_indexer/location.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/location.rb b/lib/ruby_indexer/lib/ruby_indexer/location.rb index 711d9af36..684298287 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/location.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/location.rb @@ -47,14 +47,14 @@ def initialize(start_line, end_line, start_column, end_column) sig do params( - another_location: T.any(Location, Prism::Location) + other: T.any(Location, Prism::Location), ).returns(T::Boolean) end - def ==(another_location) - start_line == another_location.start_line && - end_line == another_location.end_line && - start_column == another_location.start_column && - end_column == another_location.end_column + def ==(other) + start_line == other.start_line && + end_line == other.end_line && + start_column == other.start_column && + end_column == other.end_column end end end