From fbc7647c91e6d0af5a209e69abfe2ec495d7bb13 Mon Sep 17 00:00:00 2001 From: Tejas Bubane Date: Tue, 24 Nov 2020 23:07:24 +0530 Subject: [PATCH] [Fix #389] Add `IgnoredMethods` configuration option for `Rails/FindEach` cop Closes #389 --- CHANGELOG.md | 1 + config/default.yml | 7 +++++++ docs/modules/ROOT/pages/cops_rails.adoc | 12 ++++++++++++ lib/rubocop/cop/rails/find_each.rb | 12 +++++++----- spec/rubocop/cop/rails/find_each_spec.rb | 16 ++++++++++++---- 5 files changed, 39 insertions(+), 9 deletions(-) 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..ac2499773c 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.8.2' 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..c3d4bc18b3 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -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..ac4df10873 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,15 @@ 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 end end