Skip to content

Commit

Permalink
Fix a false positive for Rails/RedundantActiveRecordAllMethod
Browse files Browse the repository at this point in the history
Resolves #1114 (comment)

This PR fixes a false positive for `Rails/RedundantActiveRecordAllMethod`
when using some `Enumerable`'s methods with block argument.

e.g. `Enumerable#find` and `ActiveRecord::Base#find` have different arguments,
So false positives can be prevented.
  • Loading branch information
koic committed Sep 28, 2023
1 parent 0a57811 commit e586f7f
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1126](https://github.com/rubocop/rubocop-rails/pull/1126): Fix a false positive for `Rails/RedundantActiveRecordAllMethod` when using some `Enumerable`'s methods with block argument. ([@koic][])
11 changes: 10 additions & 1 deletion lib/rubocop/cop/rails/redundant_active_record_all_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,14 @@ class RedundantActiveRecordAllMethod < Base
without
].to_set.freeze

POSSIBLE_ENUMERABLE_BLOCK_METHODS = %i[any? count find none? one? select sum].freeze

def_node_matcher :followed_by_query_method?, <<~PATTERN
(send (send _ :all) QUERYING_METHODS ...)
PATTERN

def on_send(node)
return unless followed_by_query_method?(node.parent)
return if !followed_by_query_method?(node.parent) || possible_enumerable_block_method?(node)
return if node.receiver ? allowed_receiver?(node.receiver) : !inherit_active_record_base?(node)

range_of_all_method = offense_range(node)
Expand All @@ -150,6 +152,13 @@ def on_send(node)

private

def possible_enumerable_block_method?(node)
parent = node.parent
return false unless POSSIBLE_ENUMERABLE_BLOCK_METHODS.include?(parent.method_name)

parent.parent&.block_type? || parent.parent&.numblock_type? || parent.first_argument&.block_pass_type?
end

def offense_range(node)
range_between(node.loc.selector.begin_pos, node.source_range.end_pos)
end
Expand Down
42 changes: 42 additions & 0 deletions spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,48 @@ class User < ::ActiveRecord::Base
end
RUBY
end

context 'using `any?`' do
it 'does not register an offense when using `any?` with block' do
expect_no_offenses(<<~RUBY)
User.all.any? { |item| item.do_something }
RUBY
end

it 'does not register an offense when using `any?` with numbered block' do
expect_no_offenses(<<~RUBY)
User.all.any? { _1.do_something }
RUBY
end

it 'does not register an offense when using `any?` with symbol block' do
expect_no_offenses(<<~RUBY)
User.all.any?(&:do_something)
RUBY
end
end

described_class::POSSIBLE_ENUMERABLE_BLOCK_METHODS.each do |method|
context "using `#{method}`" do
it "does not register an offense when using `#{method}` with block" do
expect_no_offenses(<<~RUBY)
User.all.#{method} { |item| item.do_something }
RUBY
end

it "does not register an offense when using `#{method}` with numbered block" do
expect_no_offenses(<<~RUBY)
User.all.#{method} { _1.do_something }
RUBY
end

it "does not register an offense when using `#{method}` with symbol block" do
expect_no_offenses(<<~RUBY)
User.all.#{method}(&:do_something)
RUBY
end
end
end
end

context 'with `AllowedReceivers` config' do
Expand Down

0 comments on commit e586f7f

Please sign in to comment.