Skip to content

Commit

Permalink
Deprecate IgnoredMethods option
Browse files Browse the repository at this point in the history
Follow up rubocop/rubocop#10829.

This PR deprecates `IgnoredMethods` option in integrate to `AllowedMethods` and `AllowedPatterns` option.

NOTE: It cannot be merged until rubocop/rubocop#10829 is released.
And it needs to bump the dependent version of RuboCop in gemspec.
  • Loading branch information
koic committed Jul 24, 2022
1 parent f500a79 commit 289c0ab
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog/deprecate_ignore_methods_parameter.md
@@ -0,0 +1 @@
* [#750](https://github.com/rubocop/rubocop-rails/pull/750): Deprecate `IgnoredMethods` option in integrate to `AllowedMethods` and `AllowedPatterns` option. ([@koic][])
26 changes: 26 additions & 0 deletions config/default.yml
Expand Up @@ -26,6 +26,24 @@ Lint/NumberConversion:
# Add Rails' duration methods to the ignore list for `Lint/NumberConversion`
# so that calling `to_i` on one of these does not register an offense.
# See: https://github.com/rubocop/rubocop/issues/8950
AllowedMethods:
- ago
- from_now
- second
- seconds
- minute
- minutes
- hour
- hours
- day
- days
- week
- weeks
- fortnight
- fortnights
- in_milliseconds
AllowedPatterns: []
# Deprecated.
IgnoredMethods:
- ago
- from_now
Expand Down Expand Up @@ -410,6 +428,14 @@ Rails/FindEach:
VersionChanged: '2.9'
Include:
- app/models/**/*.rb
AllowedMethods:
# Methods that don't work well with `find_each`.
- order
- limit
- select
- lock
AllowedPatterns: []
# Deprecated.
IgnoredMethods:
# Methods that don't work well with `find_each`.
- order
Expand Down
10 changes: 10 additions & 0 deletions config/obsoletion.yml
Expand Up @@ -5,3 +5,13 @@
#
extracted:
Rails/*: ~

# Cop parameters that have been changed
# Can be treated as a warning instead of a failure with `severity: warning`
changed_parameters:
- cops: Rails/FindEach
parameters: IgnoredMethods
alternatives:
- AllowedMethods
- AllowedPatterns
severity: warning
10 changes: 8 additions & 2 deletions lib/rubocop/cop/rails/find_each.rb
Expand Up @@ -13,11 +13,17 @@ module Rails
# # good
# User.all.find_each
#
# @example IgnoredMethods: ['order']
# @example AllowedMethods: ['order']
# # good
# User.order(:foo).each
#
# @example AllowedPattern: [/order/]
# # good
# User.order(:foo).each
class FindEach < Base
include ActiveRecordHelper
include AllowedMethods
include AllowedPattern
extend AutoCorrector

MSG = 'Use `find_each` instead of `each`.'
Expand Down Expand Up @@ -47,7 +53,7 @@ def ignored?(node)

method_chain = node.each_node(:send).map(&:method_name)

(cop_config['IgnoredMethods'].map(&:to_sym) & method_chain).any?
method_chain.any? { |method_name| allowed_method?(method_name) || matches_allowed_pattern?(method_name) }
end

def active_model_error_where?(node)
Expand Down
48 changes: 47 additions & 1 deletion spec/rubocop/cop/rails/find_each_spec.rb
Expand Up @@ -118,8 +118,54 @@ class C < ActiveRecord::Base
end
end

context 'allowed methods' do
let(:cop_config) { { 'AllowedMethods' => %w[order lock], 'AllowedPatterns' => [], 'IgnoredMethods' => [] } }

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 order(...) chained with other things' do
expect_no_offenses('User.order(:name).includes(:company).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 `AllowedMethods`' do
expect_offense(<<~RUBY)
User.joins(:posts).each { |u| u.something }
^^^^ Use `find_each` instead of `each`.
RUBY
end
end

context 'allowed patterns' do
let(:cop_config) { { 'AllowedMethods' => [], 'AllowedPatterns' => [/order/, /lock/], 'IgnoredMethods' => [] } }

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 order(...) chained with other things' do
expect_no_offenses('User.order(:name).includes(:company).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 `AllowedPatterns`' do
expect_offense(<<~RUBY)
User.joins(:posts).each { |u| u.something }
^^^^ Use `find_each` instead of `each`.
RUBY
end
end

context 'ignored methods' do
let(:cop_config) { { 'IgnoredMethods' => %w[order lock] } }
let(:cop_config) { { 'AllowedPatterns' => [], 'AllowedMethods' => [], '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 }')
Expand Down

0 comments on commit 289c0ab

Please sign in to comment.