diff --git a/CHANGELOG.md b/CHANGELOG.md index 37433de0c6..e87da2f8bd 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/config/default.yml b/config/default.yml index 3e9ab31894..7d2fe4fd5f 100644 --- a/config/default.yml +++ b/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.' diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index cf656da300..49814c9dc6 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -1470,7 +1470,7 @@ User.find(id) | Yes | Yes | 0.30 -| - +| 2.8.2 |=== 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 diff --git a/lib/rubocop/cop/rails/find_each.rb b/lib/rubocop/cop/rails/find_each.rb index 03e58acf37..b34d3556b7 100644 --- a/lib/rubocop/cop/rails/find_each.rb +++ b/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 diff --git a/spec/rubocop/cop/rails/find_each_spec.rb b/spec/rubocop/cop/rails/find_each_spec.rb index 7d05133415..51ac8e57f4 100644 --- a/spec/rubocop/cop/rails/find_each_spec.rb +++ b/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