From e68bf9793bdbd8c5584fe5eed92511ee65255c27 Mon Sep 17 00:00:00 2001 From: Tommy Crumrine Date: Thu, 14 Nov 2024 12:09:28 -0600 Subject: [PATCH 1/7] Adds support for attr_reader, attr_writer, attr_accessor with some tests. Prob can use some cleanup --- .../lib/ruby_indexer/reference_finder.rb | 17 ++- .../test/reference_finder_test.rb | 130 ++++++++++++++++++ 2 files changed, 146 insertions(+), 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 956b166b24..459577e307 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -264,12 +264,27 @@ def on_def_node_leave(node) sig { params(node: Prism::CallNode).void } def on_call_node_enter(node) - if @target.is_a?(MethodTarget) && (name = node.name.to_s) == @target.method_name + return unless @target.is_a?(MethodTarget) + + if (name = node.name.to_s) == @target.method_name @references << Reference.new(name, T.must(node.message_loc), declaration: false) + elsif node.name == :attr_writer && attr_writer_matches_method(node.arguments.arguments, @target.method_name) + @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) + elsif node.name == :attr_reader && attr_reader_matches_method(node.arguments.arguments, @target.method_name) + @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) + elsif node.name == :attr_accessor && (attr_reader_matches_method(node.arguments.arguments, @target.method_name) || attr_writer_matches_method(node.arguments.arguments, @target.method_name)) + @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) end end private + def attr_reader_matches_method(arguments, method_name) + arguments.map(&:unescaped).include?(method_name) + end + + def attr_writer_matches_method(arguments, method_name) + arguments.map(&:unescaped).any? { |unescaped_arg| "#{unescaped_arg}=" == method_name } + end sig { params(name: String).returns(T::Array[String]) } def actual_nesting(name) diff --git a/lib/ruby_indexer/test/reference_finder_test.rb b/lib/ruby_indexer/test/reference_finder_test.rb index 0d4627a8f7..2c84a3fbfd 100644 --- a/lib/ruby_indexer/test/reference_finder_test.rb +++ b/lib/ruby_indexer/test/reference_finder_test.rb @@ -143,6 +143,136 @@ def baz assert_equal(9, refs[1].location.start_line) end + def test_matches_attr_writer + refs = find_method_references("foo=", <<~RUBY) + class Bar + def foo + end + + attr_writer :foo + + def baz + self.foo = 1 + self.foo + end + end + RUBY + + # We want to match `foo=` but not `foo` + assert_equal(2, refs.size) + + assert_equal("foo=", refs[0].name) + assert_equal(5, refs[0].location.start_line) + + assert_equal("foo=", refs[1].name) + assert_equal(8, refs[1].location.start_line) + end + + def test_matches_attr_reader + refs = find_method_references("foo", <<~RUBY) + class Bar + def foo=(value) + end + + attr_reader :foo + + def baz + self.foo = 1 + self.foo + end + end + RUBY + + # We want to match `foo=` but not `foo` + assert_equal(2, refs.size) + + assert_equal("foo", refs[0].name) + assert_equal(5, refs[0].location.start_line) + + assert_equal("foo", refs[1].name) + assert_equal(9, refs[1].location.start_line) + end + + def test_matches_attr_accessor + refs = find_method_references("foo=", <<~RUBY) + class Bar + attr_accessor :foo + + def baz + self.foo = 1 + self.foo + end + end + RUBY + + # We want to match `foo=` but not `foo` + assert_equal(2, refs.size) + + assert_equal("foo=", refs[0].name) + assert_equal(2, refs[0].location.start_line) + + assert_equal("foo=", refs[1].name) + assert_equal(5, refs[1].location.start_line) + + refs = find_method_references("foo", <<~RUBY) + class Bar + attr_accessor :foo + + def baz + self.foo = 1 + self.foo + end + end + RUBY + + + assert_equal("foo", refs[0].name) + assert_equal(2, refs[0].location.start_line) + + assert_equal("foo", refs[1].name) + assert_equal(6, refs[1].location.start_line) + end + + def test_matches_attr_accessor_multi + refs = find_method_references("foo=", <<~RUBY) + class Bar + attr_accessor :bar, :foo + + def baz + self.foo = 1 + self.foo + end + end + RUBY + + # We want to match `foo=` but not `foo` + assert_equal(2, refs.size) + + assert_equal("foo=", refs[0].name) + assert_equal(2, refs[0].location.start_line) + + assert_equal("foo=", refs[1].name) + assert_equal(5, refs[1].location.start_line) + + refs = find_method_references("foo", <<~RUBY) + class Bar + attr_accessor :foo + + def baz + self.foo = 1 + self.foo + end + end + RUBY + + + assert_equal("foo", refs[0].name) + assert_equal(2, refs[0].location.start_line) + + assert_equal("foo", refs[1].name) + assert_equal(6, refs[1].location.start_line) + end + def test_find_inherited_methods refs = find_method_references("foo", <<~RUBY) class Bar From 9a466c998992bd8be5ef346ee5059d1f508dcbca Mon Sep 17 00:00:00 2001 From: Tommy Crumrine Date: Thu, 14 Nov 2024 12:23:39 -0600 Subject: [PATCH 2/7] using multi attrs in both cases for the multi test, thanks graphite lol --- lib/ruby_indexer/test/reference_finder_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ruby_indexer/test/reference_finder_test.rb b/lib/ruby_indexer/test/reference_finder_test.rb index 2c84a3fbfd..bec52cb708 100644 --- a/lib/ruby_indexer/test/reference_finder_test.rb +++ b/lib/ruby_indexer/test/reference_finder_test.rb @@ -256,7 +256,7 @@ def baz refs = find_method_references("foo", <<~RUBY) class Bar - attr_accessor :foo + attr_accessor :bar, :foo def baz self.foo = 1 From 47d28f58b209b7bc9d05bf065f48fb33de417eee Mon Sep 17 00:00:00 2001 From: Tommy Crumrine Date: Thu, 14 Nov 2024 17:24:02 -0600 Subject: [PATCH 3/7] Handles when one of the arguments to attr_* is a method call --- .../lib/ruby_indexer/reference_finder.rb | 14 +++++++++---- .../test/reference_finder_test.rb | 20 +++++++++++++++++-- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index 459577e307..5adf8f5533 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -264,7 +264,7 @@ def on_def_node_leave(node) sig { params(node: Prism::CallNode).void } def on_call_node_enter(node) - return unless @target.is_a?(MethodTarget) + return unless @target.is_a?(MethodTarget) if (name = node.name.to_s) == @target.method_name @references << Reference.new(name, T.must(node.message_loc), declaration: false) @@ -272,18 +272,24 @@ def on_call_node_enter(node) @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) elsif node.name == :attr_reader && attr_reader_matches_method(node.arguments.arguments, @target.method_name) @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) - elsif node.name == :attr_accessor && (attr_reader_matches_method(node.arguments.arguments, @target.method_name) || attr_writer_matches_method(node.arguments.arguments, @target.method_name)) + elsif node.name == :attr_accessor && (attr_reader_matches_method( + node.arguments.arguments, + @target.method_name, + ) || attr_writer_matches_method(node.arguments.arguments, @target.method_name)) @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) end end private + def attr_reader_matches_method(arguments, method_name) - arguments.map(&:unescaped).include?(method_name) + arguments.select { |ar| ar.respond_to?(:unescaped) }.map(&:unescaped).include?(method_name) end def attr_writer_matches_method(arguments, method_name) - arguments.map(&:unescaped).any? { |unescaped_arg| "#{unescaped_arg}=" == method_name } + arguments.select do |ar| + ar.respond_to?(:unescaped) + end.map(&:unescaped).any? { |unescaped_arg| "#{unescaped_arg}=" == method_name } end sig { params(name: String).returns(T::Array[String]) } diff --git a/lib/ruby_indexer/test/reference_finder_test.rb b/lib/ruby_indexer/test/reference_finder_test.rb index bec52cb708..8c3d385b47 100644 --- a/lib/ruby_indexer/test/reference_finder_test.rb +++ b/lib/ruby_indexer/test/reference_finder_test.rb @@ -143,6 +143,24 @@ def baz assert_equal(9, refs[1].location.start_line) end + def test_matches_attr_writer_with_call_node_argument + refs = find_method_references("foo=", <<~RUBY) + class Bar + attr_reader :foo, bar + + def baz + self.foo = 1 + self.foo + end + end + RUBY + + assert_equal(1, refs.size) + + assert_equal("foo=", refs[0].name) + assert_equal(5, refs[0].location.start_line) + end + def test_matches_attr_writer refs = find_method_references("foo=", <<~RUBY) class Bar @@ -225,7 +243,6 @@ def baz end RUBY - assert_equal("foo", refs[0].name) assert_equal(2, refs[0].location.start_line) @@ -265,7 +282,6 @@ def baz end RUBY - assert_equal("foo", refs[0].name) assert_equal(2, refs[0].location.start_line) From 0cd8075711464491a8b8294baca2e8d8776465ad Mon Sep 17 00:00:00 2001 From: Tommy Crumrine Date: Fri, 15 Nov 2024 13:36:58 -0600 Subject: [PATCH 4/7] cleanup and fix types --- .../lib/ruby_indexer/reference_finder.rb | 44 +++++++++++++------ 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index 5adf8f5533..3fd95181dc 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -268,28 +268,44 @@ def on_call_node_enter(node) if (name = node.name.to_s) == @target.method_name @references << Reference.new(name, T.must(node.message_loc), declaration: false) - elsif node.name == :attr_writer && attr_writer_matches_method(node.arguments.arguments, @target.method_name) - @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) - elsif node.name == :attr_reader && attr_reader_matches_method(node.arguments.arguments, @target.method_name) - @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) - elsif node.name == :attr_accessor && (attr_reader_matches_method( - node.arguments.arguments, - @target.method_name, - ) || attr_writer_matches_method(node.arguments.arguments, @target.method_name)) + elsif attr_method_references?(node) @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) end end private + sig { params(node: Prism::CallNode).returns(T::Boolean) } + def attr_method_references?(node) + case node.name + when :attr_reader + attr_reader_references?(unescaped_argument_names(node)) + when :attr_writer + attr_writer_references?(unescaped_argument_names(node)) + when :attr_accessor + attr_accessor_references?(unescaped_argument_names(node)) + else + false + end + end + + sig { params(node: Prism::CallNode).returns(T::Array[String]) } + def unescaped_argument_names(node) + node.arguments.arguments.select { |arg| arg.respond_to?(:unescaped) }.map(&:unescaped) + end + + sig { params(argument_names: T::Array[String]).returns(T::Boolean) } + def attr_reader_references?(argument_names) + argument_names.include?(@target.method_name) + end - def attr_reader_matches_method(arguments, method_name) - arguments.select { |ar| ar.respond_to?(:unescaped) }.map(&:unescaped).include?(method_name) + sig { params(argument_names: T::Array[String]).returns(T::Boolean) } + def attr_writer_references?(argument_names) + argument_names.any? { |arg| "#{arg}=" == @target.method_name } end - def attr_writer_matches_method(arguments, method_name) - arguments.select do |ar| - ar.respond_to?(:unescaped) - end.map(&:unescaped).any? { |unescaped_arg| "#{unescaped_arg}=" == method_name } + sig { params(argument_names: T::Array[String]).returns(T::Boolean) } + def attr_accessor_references?(argument_names) + argument_names.any? { |arg| "#{arg}=" == @target.method_name || arg == @target.method_name } end sig { params(name: String).returns(T::Array[String]) } From c77f6a5bec6c29a2d39c17142942bbdb546ba6ba Mon Sep 17 00:00:00 2001 From: Tommy Crumrine Date: Fri, 15 Nov 2024 15:16:56 -0600 Subject: [PATCH 5/7] lint --- lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index 3fd95181dc..64a90f3018 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -274,6 +274,7 @@ def on_call_node_enter(node) end private + sig { params(node: Prism::CallNode).returns(T::Boolean) } def attr_method_references?(node) case node.name From a2025e43b2dae79e185f2f331938afa910270e15 Mon Sep 17 00:00:00 2001 From: Tommy Crumrine Date: Wed, 6 Aug 2025 19:31:42 -0500 Subject: [PATCH 6/7] sigs to RBS --- .../lib/ruby_indexer/reference_finder.rb | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index fc5050182d..fc438447d8 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -293,7 +293,7 @@ def on_call_node_enter(node) private - sig { params(node: Prism::CallNode).returns(T::Boolean) } + #: (Prism::CallNode node) -> bool def attr_method_references?(node) case node.name when :attr_reader @@ -307,41 +307,26 @@ def attr_method_references?(node) end end - sig { params(node: Prism::CallNode).returns(T::Array[String]) } + #: (Prism::CallNode node) -> Array[String] def unescaped_argument_names(node) node.arguments.arguments.select { |arg| arg.respond_to?(:unescaped) }.map(&:unescaped) end - sig { params(argument_names: T::Array[String]).returns(T::Boolean) } + #: (Array[String] argument_names) -> bool def attr_reader_references?(argument_names) argument_names.include?(@target.method_name) end - sig { params(argument_names: T::Array[String]).returns(T::Boolean) } + #: (Array[String] argument_names) -> bool def attr_writer_references?(argument_names) argument_names.any? { |arg| "#{arg}=" == @target.method_name } end - sig { params(argument_names: T::Array[String]).returns(T::Boolean) } + #: (Array[String] argument_names) -> bool def attr_accessor_references?(argument_names) argument_names.any? { |arg| "#{arg}=" == @target.method_name || arg == @target.method_name } end - sig { params(name: String).returns(T::Array[String]) } - def actual_nesting(name) - nesting = @stack + [name] - corrected_nesting = [] - - nesting.reverse_each do |name| - corrected_nesting.prepend(name.delete_prefix("::")) - - break if name.start_with?("::") - end - - corrected_nesting - end - - sig { params(name: String, location: Prism::Location).void } #: (String name, Prism::Location location) -> void def collect_constant_references(name, location) return unless @target.is_a?(ConstTarget) From 98b4ca7c9baae816b2d22e00d406f3fc1c7aef76 Mon Sep 17 00:00:00 2001 From: Tommy Crumrine Date: Wed, 6 Aug 2025 20:35:25 -0500 Subject: [PATCH 7/7] Handle blank / incomplete arguments and strings --- .../lib/ruby_indexer/reference_finder.rb | 8 ++- .../test/reference_finder_test.rb | 72 ++++++++++++++++++- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb index fc438447d8..2469454b82 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb @@ -285,9 +285,9 @@ def on_call_node_enter(node) return unless @target.is_a?(MethodTarget) if (name = node.name.to_s) == @target.method_name - @references << Reference.new(name, T.must(node.message_loc), declaration: false) + @references << Reference.new(name, node.message_loc, declaration: false) elsif attr_method_references?(node) - @references << Reference.new(@target.method_name, T.must(node.message_loc), declaration: true) + @references << Reference.new(@target.method_name, node.message_loc, declaration: true) end end @@ -309,6 +309,8 @@ def attr_method_references?(node) #: (Prism::CallNode node) -> Array[String] def unescaped_argument_names(node) + return [] if node.arguments.nil? + node.arguments.arguments.select { |arg| arg.respond_to?(:unescaped) }.map(&:unescaped) end @@ -324,7 +326,7 @@ def attr_writer_references?(argument_names) #: (Array[String] argument_names) -> bool def attr_accessor_references?(argument_names) - argument_names.any? { |arg| "#{arg}=" == @target.method_name || arg == @target.method_name } + argument_names.any? { |arg| ["#{arg}=", arg].include?(@target.method_name) } end #: (String name, Prism::Location location) -> void diff --git a/lib/ruby_indexer/test/reference_finder_test.rb b/lib/ruby_indexer/test/reference_finder_test.rb index 557860ad64..35637cbc04 100644 --- a/lib/ruby_indexer/test/reference_finder_test.rb +++ b/lib/ruby_indexer/test/reference_finder_test.rb @@ -146,7 +146,7 @@ def baz def test_matches_attr_writer_with_call_node_argument refs = find_method_references("foo=", <<~RUBY) class Bar - attr_reader :foo, bar + attr_writer :foo, bar def baz self.foo = 1 @@ -155,10 +155,15 @@ def baz end RUBY - assert_equal(1, refs.size) + assert_equal(2, refs.size) assert_equal("foo=", refs[0].name) - assert_equal(5, refs[0].location.start_line) + assert_equal(2, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) + + assert_equal("foo=", refs[1].name) + assert_equal(5, refs[1].location.start_line) + assert_equal(false, refs[1].declaration) end def test_matches_attr_writer @@ -181,9 +186,11 @@ def baz assert_equal("foo=", refs[0].name) assert_equal(5, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) assert_equal("foo=", refs[1].name) assert_equal(8, refs[1].location.start_line) + assert_equal(false, refs[1].declaration) end def test_matches_attr_reader @@ -206,9 +213,11 @@ def baz assert_equal("foo", refs[0].name) assert_equal(5, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) assert_equal("foo", refs[1].name) assert_equal(9, refs[1].location.start_line) + assert_equal(false, refs[1].declaration) end def test_matches_attr_accessor @@ -228,9 +237,11 @@ def baz assert_equal("foo=", refs[0].name) assert_equal(2, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) assert_equal("foo=", refs[1].name) assert_equal(5, refs[1].location.start_line) + assert_equal(false, refs[1].declaration) refs = find_method_references("foo", <<~RUBY) class Bar @@ -245,9 +256,11 @@ def baz assert_equal("foo", refs[0].name) assert_equal(2, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) assert_equal("foo", refs[1].name) assert_equal(6, refs[1].location.start_line) + assert_equal(false, refs[1].declaration) end def test_matches_attr_accessor_multi @@ -267,9 +280,11 @@ def baz assert_equal("foo=", refs[0].name) assert_equal(2, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) assert_equal("foo=", refs[1].name) assert_equal(5, refs[1].location.start_line) + assert_equal(false, refs[1].declaration) refs = find_method_references("foo", <<~RUBY) class Bar @@ -284,11 +299,62 @@ def baz assert_equal("foo", refs[0].name) assert_equal(2, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) assert_equal("foo", refs[1].name) assert_equal(6, refs[1].location.start_line) + assert_equal(false, refs[1].declaration) end + def test_matches_attr_emtpy + ruby_code = <<~RUBY + class Bar + def foo=(value) + end + + attr_reader + + def baz + foo + end + end + RUBY + refs = find_method_references("foo", ruby_code) + assert_equal(1, refs.size) + assert_equal(8, refs[0].location.start_line) + assert_equal(false, refs[0].declaration) + refs = find_method_references("foo=", ruby_code) + assert_equal(1, refs.size) + assert_equal(2, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) + refs = find_method_references("baz", ruby_code) + assert_equal(1, refs.size) + assert_equal(7, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) + end + + def test_matches_attr_string + ruby_code = <<~RUBY + class Bar + def foo=(value) + end + + attr_reader 'foo' + + def baz + foo + end + end + RUBY + + refs = find_method_references("foo", ruby_code) + assert_equal(2, refs.size) + assert_equal(5, refs[0].location.start_line) + assert_equal(true, refs[0].declaration) + assert_equal(8, refs[1].location.start_line) + assert_equal(false, refs[1].declaration) + end + def test_find_inherited_methods refs = find_method_references("foo", <<~RUBY) class Bar