Skip to content

Commit ab3e176

Browse files
committed
Deprecate IgnoredMethods option
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.
1 parent a4fa778 commit ab3e176

File tree

6 files changed

+93
-4
lines changed

6 files changed

+93
-4
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#750](https://github.com/rubocop/rubocop-rails/pull/750): Deprecate `IgnoredMethods` option in integrate to `AllowedMethods` and `AllowedPatterns` option. ([@koic][])

config/default.yml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@ Lint/NumberConversion:
2626
# Add Rails' duration methods to the ignore list for `Lint/NumberConversion`
2727
# so that calling `to_i` on one of these does not register an offense.
2828
# See: https://github.com/rubocop/rubocop/issues/8950
29+
AllowedMethods:
30+
- ago
31+
- from_now
32+
- second
33+
- seconds
34+
- minute
35+
- minutes
36+
- hour
37+
- hours
38+
- day
39+
- days
40+
- week
41+
- weeks
42+
- fortnight
43+
- fortnights
44+
- in_milliseconds
45+
AllowedPatterns: []
46+
# Deprecated.
2947
IgnoredMethods:
3048
- ago
3149
- from_now
@@ -410,6 +428,14 @@ Rails/FindEach:
410428
VersionChanged: '2.9'
411429
Include:
412430
- app/models/**/*.rb
431+
AllowedMethods:
432+
# Methods that don't work well with `find_each`.
433+
- order
434+
- limit
435+
- select
436+
- lock
437+
AllowedPatterns: []
438+
# Deprecated.
413439
IgnoredMethods:
414440
# Methods that don't work well with `find_each`.
415441
- order

config/obsoletion.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,13 @@
55
#
66
extracted:
77
Rails/*: ~
8+
9+
# Cop parameters that have been changed
10+
# Can be treated as a warning instead of a failure with `severity: warning`
11+
changed_parameters:
12+
- cops: Rails/FindEach
13+
parameters: IgnoredMethods
14+
alternatives:
15+
- AllowedMethods
16+
- AllowedPatterns
17+
severity: warning

lib/rubocop/cop/rails/find_each.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,17 @@ module Rails
1313
# # good
1414
# User.all.find_each
1515
#
16-
# @example IgnoredMethods: ['order']
16+
# @example AllowedMethods: ['order']
17+
# # good
18+
# User.order(:foo).each
19+
#
20+
# @example AllowedPattern: [/order/]
1721
# # good
1822
# User.order(:foo).each
1923
class FindEach < Base
2024
include ActiveRecordHelper
25+
include AllowedMethods
26+
include AllowedPattern
2127
extend AutoCorrector
2228

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

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

50-
(cop_config['IgnoredMethods'].map(&:to_sym) & method_chain).any?
56+
method_chain.any? { |method_name| allowed_method?(method_name) || matches_allowed_pattern?(method_name) }
5157
end
5258

5359
def active_model_error_where?(node)

rubocop-rails.gemspec

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,5 @@ Gem::Specification.new do |s|
3535
# Rack::Utils::SYMBOL_TO_STATUS_CODE, which is used by HttpStatus cop, was
3636
# introduced in rack 1.1
3737
s.add_runtime_dependency 'rack', '>= 1.1'
38-
s.add_runtime_dependency 'rubocop', '>= 1.31.0', '< 2.0'
38+
s.add_runtime_dependency 'rubocop', '>= 1.33.0', '< 2.0'
3939
end

spec/rubocop/cop/rails/find_each_spec.rb

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,54 @@ class C < ActiveRecord::Base
118118
end
119119
end
120120

121+
context 'allowed methods' do
122+
let(:cop_config) { { 'AllowedMethods' => %w[order lock], 'AllowedPatterns' => [], 'IgnoredMethods' => [] } }
123+
124+
it 'does not register an offense when using order(...) earlier' do
125+
expect_no_offenses('User.order(:name).each { |u| u.something }')
126+
end
127+
128+
it 'does not register an offense when using order(...) chained with other things' do
129+
expect_no_offenses('User.order(:name).includes(:company).each { |u| u.something }')
130+
end
131+
132+
it 'does not register an offense when using lock earlier' do
133+
expect_no_offenses('User.lock.each { |u| u.something }')
134+
end
135+
136+
it 'registers offense for methods not in `AllowedMethods`' do
137+
expect_offense(<<~RUBY)
138+
User.joins(:posts).each { |u| u.something }
139+
^^^^ Use `find_each` instead of `each`.
140+
RUBY
141+
end
142+
end
143+
144+
context 'allowed patterns' do
145+
let(:cop_config) { { 'AllowedMethods' => [], 'AllowedPatterns' => [/order/, /lock/], 'IgnoredMethods' => [] } }
146+
147+
it 'does not register an offense when using order(...) earlier' do
148+
expect_no_offenses('User.order(:name).each { |u| u.something }')
149+
end
150+
151+
it 'does not register an offense when using order(...) chained with other things' do
152+
expect_no_offenses('User.order(:name).includes(:company).each { |u| u.something }')
153+
end
154+
155+
it 'does not register an offense when using lock earlier' do
156+
expect_no_offenses('User.lock.each { |u| u.something }')
157+
end
158+
159+
it 'registers offense for methods not in `AllowedPatterns`' do
160+
expect_offense(<<~RUBY)
161+
User.joins(:posts).each { |u| u.something }
162+
^^^^ Use `find_each` instead of `each`.
163+
RUBY
164+
end
165+
end
166+
121167
context 'ignored methods' do
122-
let(:cop_config) { { 'IgnoredMethods' => %w[order lock] } }
168+
let(:cop_config) { { 'AllowedPatterns' => [], 'AllowedMethods' => [], 'IgnoredMethods' => %w[order lock] } }
123169

124170
it 'does not register an offense when using order(...) earlier' do
125171
expect_no_offenses('User.order(:name).each { |u| u.something }')

0 commit comments

Comments
 (0)