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.
It requires RuboCop 1.33.0 or higher.
  • Loading branch information
koic committed Aug 4, 2022
1 parent a4fa778 commit ab3e176
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelog/change_deprecate_ignore_methods_parameter.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
2 changes: 1 addition & 1 deletion rubocop-rails.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ Gem::Specification.new do |s|
# Rack::Utils::SYMBOL_TO_STATUS_CODE, which is used by HttpStatus cop, was
# introduced in rack 1.1
s.add_runtime_dependency 'rack', '>= 1.1'
s.add_runtime_dependency 'rubocop', '>= 1.31.0', '< 2.0'
s.add_runtime_dependency 'rubocop', '>= 1.33.0', '< 2.0'
end
48 changes: 47 additions & 1 deletion spec/rubocop/cop/rails/find_each_spec.rb
Original file line number Diff line number Diff line change
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 ab3e176

Please sign in to comment.