diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e6546caf7..7cbd38e389 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * [#435](https://github.com/rubocop/rubocop-rails/issues/435): Fix a false negative for `Rails/BelongsTo` when using `belongs_to` lambda block with `required` option. ([@koic][]) * [#451](https://github.com/rubocop/rubocop-rails/issues/451): Fix a false negative for `Rails/RelativeDateConstant` when a method is chained after a relative date method. ([@koic][]) * [#450](https://github.com/rubocop/rubocop-rails/issues/450): Fix a crash for `Rails/ContentTag` with nested content tags. ([@tejasbubane][]) +* [#103](https://github.com/rubocop/rubocop-rails/issues/103): Fix a false positive for `Rails/FindEach` when not inheriting `ActiveRecord::Base` and using `all.each`. ([@koic][]) ### Changes diff --git a/lib/rubocop/cop/mixin/active_record_helper.rb b/lib/rubocop/cop/mixin/active_record_helper.rb index 0d263842f3..aaa9467856 100644 --- a/lib/rubocop/cop/mixin/active_record_helper.rb +++ b/lib/rubocop/cop/mixin/active_record_helper.rb @@ -8,6 +8,13 @@ module ActiveRecordHelper WHERE_METHODS = %i[where rewhere].freeze + def_node_matcher :active_record?, <<~PATTERN + { + (const nil? :ApplicationRecord) + (const (const nil? :ActiveRecord) :Base) + } + PATTERN + def_node_search :find_set_table_name, <<~PATTERN (send self :table_name= {str sym}) PATTERN @@ -16,6 +23,10 @@ module ActiveRecordHelper (send nil? :belongs_to {str sym} ...) PATTERN + def inherit_active_record_base?(node) + node.each_ancestor(:class).any? { |class_node| active_record?(class_node.parent_class) } + end + def external_dependency_checksum return @external_dependency_checksum if defined?(@external_dependency_checksum) diff --git a/lib/rubocop/cop/rails/find_each.rb b/lib/rubocop/cop/rails/find_each.rb index c5ef88a785..328d98c0d2 100644 --- a/lib/rubocop/cop/rails/find_each.rb +++ b/lib/rubocop/cop/rails/find_each.rb @@ -17,6 +17,7 @@ module Rails # # good # User.order(:foo).each class FindEach < Base + include ActiveRecordHelper extend AutoCorrector MSG = 'Use `find_each` instead of `each`.' @@ -30,6 +31,7 @@ class FindEach < Base def on_send(node) return unless node.receiver&.send_type? return unless SCOPE_METHODS.include?(node.receiver.method_name) + return if node.receiver.receiver.nil? && !inherit_active_record_base?(node) return if ignored?(node) range = node.loc.selector diff --git a/spec/rubocop/cop/rails/find_each_spec.rb b/spec/rubocop/cop/rails/find_each_spec.rb index 07a2a6899a..41b51ab230 100644 --- a/spec/rubocop/cop/rails/find_each_spec.rb +++ b/spec/rubocop/cop/rails/find_each_spec.rb @@ -65,6 +65,42 @@ class C; User.all.find_each { |u| u.x }; end RUBY end + context 'with no receiver' do + it 'does not register an offense when not inheriting any class' do + expect_no_offenses(<<~RUBY) + class C + all.each { |u| u.x } + end + RUBY + end + + it 'does not register an offense when not inheriting `ApplicationRecord`' do + expect_no_offenses(<<~RUBY) + class C < Foo + all.each { |u| u.x } + end + RUBY + end + + it 'registers an offense when inheriting `ApplicationRecord`' do + expect_offense(<<~RUBY) + class C < ApplicationRecord + all.each { |u| u.x } + ^^^^ Use `find_each` instead of `each`. + end + RUBY + end + + it 'registers an offense when inheriting `ActiveRecord::Base`' do + expect_offense(<<~RUBY) + class C < ActiveRecord::Base + all.each { |u| u.x } + ^^^^ Use `find_each` instead of `each`. + end + RUBY + end + end + context 'ignored methods' do let(:cop_config) { { 'IgnoredMethods' => %w[order lock] } }