Merge pull request #392 from tejasbubane/fix-389

[Fix #389] Add `IgnoredMethods` configuration option for `Rails/FindEach` cop

CHANGELOG.md

@@ -7,6 +7,7 @@
 * [#362](https://github.com/rubocop-hq/rubocop-rails/pull/362): Add new `Rails/WhereEquals` cop. ([@eugeneius][])
 * [#339](https://github.com/rubocop-hq/rubocop-rails/pull/339): Add new `Rails/AttributeDefaultBlockValue` cop. ([@cilim][])
 * [#344](https://github.com/rubocop-hq/rubocop-rails/pull/344): Add new `Rails/ArelStar` cop which checks for quoted literal asterisks in `arel_table` calls. ([@flanger001][])
+* [#389](https://github.com/rubocop-hq/rubocop-rails/issues/389): Add `IgnoredMethods` config option for `Rails/FindEach` cop. ([@tejasbubane][])
 
 ### Bug fixes
 

config/default.yml

@@ -282,8 +282,15 @@ Rails/FindEach:
   StyleGuide: 'https://rails.rubystyle.guide#find-each'
   Enabled: true
   VersionAdded: '0.30'
+  VersionChanged: '2.9'
   Include:
     - app/models/**/*.rb
+  IgnoredMethods:
+    # Methods that don't work well with `find_each`.
+    - order
+    - limit
+    - select
+    - lock
 
 Rails/HasAndBelongsToMany:
   Description: 'Prefer has_many :through to has_and_belongs_to_many.'

docs/modules/ROOT/pages/cops_rails.adoc

@@ -1470,7 +1470,7 @@ User.find(id)
 | Yes
 | Yes
 | 0.30
-| -
+| 2.9
 |===
 
 This cop is used to identify usages of `all.each` and
@@ -1487,6 +1487,14 @@ User.all.each
 User.all.find_each
 ----
 
+==== IgnoredMethods: ['order']
+
+[source,ruby]
+----
+# good
+User.order(:foo).each
+----
+
 === Configurable attributes
 
 |===
@@ -1495,6 +1503,10 @@ User.all.find_each
 | Include
 | `app/models/**/*.rb`
 | Array
+
+| IgnoredMethods
+| `order`, `limit`, `select`, `lock`
+| Array
 |===
 
 === References

lib/rubocop/cop/rails/find_each.rb

@@ -12,6 +12,10 @@ module Rails
       #
       #   # good
       #   User.all.find_each
+      #
+      # @example IgnoredMethods: ['order']
+      #   # good
+      #   User.order(:foo).each
       class FindEach < Base
         extend AutoCorrector
 
@@ -22,12 +26,11 @@ class FindEach < Base
           all eager_load includes joins left_joins left_outer_joins not preload
           references unscoped where
         ].freeze
-        IGNORED_METHODS = %i[order limit select].freeze
 
         def on_send(node)
           return unless node.receiver&.send_type?
           return unless SCOPE_METHODS.include?(node.receiver.method_name)
-          return if method_chain(node).any? { |m| ignored_by_find_each?(m) }
+          return if method_chain(node).any? { |m| ignored?(m) }
 
           range = node.loc.selector
           add_offense(range) do |corrector|
@@ -41,9 +44,8 @@ def method_chain(node)
           node.each_node(:send).map(&:method_name)
         end
 
-        def ignored_by_find_each?(relation_method)
-          # Active Record's #find_each ignores various extra parameters
-          IGNORED_METHODS.include?(relation_method)
+        def ignored?(relation_method)
+          cop_config['IgnoredMethods'].include?(relation_method)
         end
       end
     end

spec/rubocop/cop/rails/find_each_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
-RSpec.describe RuboCop::Cop::Rails::FindEach do
-  subject(:cop) { described_class.new }
+RSpec.describe RuboCop::Cop::Rails::FindEach, :config do
+  subject(:cop) { described_class.new(config) }
 
   shared_examples 'register_offense' do |scope|
     it "registers an offense when using #{scope}.each" do
@@ -67,7 +67,22 @@ class C; User.all.find_each { |u| u.x }; end
     RUBY
   end
 
-  it 'does not register an offense when using order(...) earlier' do
-    expect_no_offenses('User.order(:name).all.each { |u| u.something }')
+  context 'ignored methods' do
+    let(:cop_config) { { 'IgnoredMethods' => %w[order lock] } }
+
+    it 'does not register an offense when using order(...) earlier' do
+      expect_no_offenses('User.order(:name).each { |u| u.something }')
+    end
+
+    it 'does not register an offense when using lock earlier' do
+      expect_no_offenses('User.lock.each { |u| u.something }')
+    end
+
+    it 'registers offense for methods not in `IgnoredMethods`' do
+      expect_offense(<<~RUBY)
+        User.joins(:posts).each { |u| u.something }
+                           ^^^^ Use `find_each` instead of `each`.
+      RUBY
+    end
   end
 end