From e68bf9793bdbd8c5584fe5eed92511ee65255c27 Mon Sep 17 00:00:00 2001 From: Tommy Crumrine Date: Thu, 14 Nov 2024 12:09:28 -0600 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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/5] 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