Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate IgnoredMethods option #750

Merged
merged 1 commit into from Aug 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/change_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
2 changes: 1 addition & 1 deletion rubocop-rails.gemspec
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
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