Skip to content

Commit

Permalink
Add new Rails/ActiveRecordCalculation cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Sep 22, 2024
1 parent 122cde0 commit 8223b8d
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_active_record_calculation_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1205](https://github.com/rubocop/rubocop-rails/issues/1205): Add new `Rails/ActiveRecordCalculation` cop. ([@fatkodima][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ Rails/ActiveRecordAliases:
VersionAdded: '0.53'
SafeAutoCorrect: false

Rails/ActiveRecordCalculation:
Description: 'Use ActiveRecord calculation methods instead of `pluck` followed by Enumerable methods.'
Enabled: pending
VersionAdded: '<<next>>'

Rails/ActiveRecordCallbacksOrder:
Description: 'Order callback declarations in the order in which they will be executed.'
StyleGuide: 'https://rails.rubystyle.guide/#callbacks-order'
Expand Down
64 changes: 64 additions & 0 deletions lib/rubocop/cop/rails/active_record_calculation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# Enforces the use of ActiveRecord calculation methods instead of `pluck`
# followed by Enumerable methods.
#
# @example
# # bad
# User.pluck(:id).max
# User.pluck(:id).min
# User.pluck(:age).sum
#
# # good
# User.maximum(:id)
# User.minimum(:id)
# User.sum(:age)
#
# # good
# User.pluck(:email).max { |email| email.length }
# User.pluck(:email).max(2)
# User.pluck(:id, :company_id).max
# User.pluck(:age).count
#
class ActiveRecordCalculation < Base
include RangeHelp
extend AutoCorrector

MSG = 'Use `%<good_method>s` instead of `pluck.%<bad_method>s`.'

RESTRICT_ON_SEND = %i[pluck].freeze
OPERATIONS_MAP = { min: :minimum, max: :maximum, sum: :sum }.freeze

def_node_matcher :pluck_calculation?, <<~PATTERN
(send
(send _ :pluck $_)
${:#{OPERATIONS_MAP.keys.join(' :')}})
PATTERN

def on_send(node)
return unless (parent = node.parent)
return if send_with_block?(parent)

pluck_calculation?(parent) do |arg_node, calculation|
good_method = OPERATIONS_MAP.fetch(calculation)
message = format(MSG, good_method: good_method, bad_method: calculation)
offense_range = range_between(node.loc.selector.begin_pos, parent.source_range.end_pos)

add_offense(offense_range, message: message) do |corrector|
corrector.replace(offense_range, "#{good_method}(#{arg_node.source})")
end
end
end

private

def send_with_block?(node)
node.parent&.block_type? && node.parent.send_node == node
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
require_relative 'rails/action_filter'
require_relative 'rails/action_order'
require_relative 'rails/active_record_aliases'
require_relative 'rails/active_record_calculation'
require_relative 'rails/active_record_callbacks_order'
require_relative 'rails/active_record_override'
require_relative 'rails/active_support_aliases'
Expand Down
82 changes: 82 additions & 0 deletions spec/rubocop/cop/rails/active_record_calculation_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::ActiveRecordCalculation, :config do
it 'registers an offense and corrects when using `pluck.max`' do
expect_offense(<<~RUBY)
Model.pluck(:column).max
^^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`.
RUBY

expect_correction(<<~RUBY)
Model.maximum(:column)
RUBY
end

it 'registers an offense and corrects when using `pluck.min`' do
expect_offense(<<~RUBY)
Model.pluck(:column).min
^^^^^^^^^^^^^^^^^^ Use `minimum` instead of `pluck.min`.
RUBY

expect_correction(<<~RUBY)
Model.minimum(:column)
RUBY
end

it 'registers an offense and corrects when using `pluck.sum`' do
expect_offense(<<~RUBY)
Model.pluck(:column).sum
^^^^^^^^^^^^^^^^^^ Use `sum` instead of `pluck.sum`.
RUBY

expect_correction(<<~RUBY)
Model.sum(:column)
RUBY
end

it 'registers an offense and corrects when using `pluck.max` without receiver' do
expect_offense(<<~RUBY)
pluck(:column).max
^^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`.
RUBY

expect_correction(<<~RUBY)
maximum(:column)
RUBY
end

it 'registers an offense and corrects when using `pluck.max` with non-literal column' do
expect_offense(<<~RUBY)
Model.pluck(column).max
^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`.
RUBY

expect_correction(<<~RUBY)
Model.maximum(column)
RUBY
end

it 'does not register an offense when using `pluck.max` with multiple arguments' do
expect_no_offenses(<<~RUBY)
Model.pluck(:column1, :column2).max
RUBY
end

it 'does not register an offense when using `pluck.max` with block' do
expect_no_offenses(<<~RUBY)
Model.pluck(:column).max { |e| e }
RUBY
end

it 'does not register an offense when using `pluck.max` and `max` has argument' do
expect_no_offenses(<<~RUBY)
Model.pluck(:column).max(1)
RUBY
end

it 'does not register an offense when using `maximum`' do
expect_no_offenses(<<~RUBY)
Model.maximum(:column)
RUBY
end
end

0 comments on commit 8223b8d

Please sign in to comment.