Merge pull request rubocop#464 from koic/fix_false_positive_for_rails…

…_find_each

[Fix rubocop#103] Fix a false positive for `Rails/FindEach`

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
 

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)
 

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

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] } }