Skip to content

Commit 0d2113a

Browse files
committed
Add new Rails/ActiveRecordCalculation cop
1 parent 122cde0 commit 0d2113a

File tree

5 files changed

+156
-0
lines changed

5 files changed

+156
-0
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1205](https://github.com/rubocop/rubocop-rails/issues/1205): Add new `Rails/ActiveRecordCalculation` cop. ([@fatkodima][])

config/default.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ Rails/ActiveRecordAliases:
130130
VersionAdded: '0.53'
131131
SafeAutoCorrect: false
132132

133+
Rails/ActiveRecordCalculation:
134+
Description: 'Use ActiveRecord calculation methods instead of `pluck` followed by Enumerable methods.'
135+
Enabled: pending
136+
VersionAdded: '<<next>>'
137+
133138
Rails/ActiveRecordCallbacksOrder:
134139
Description: 'Order callback declarations in the order in which they will be executed.'
135140
StyleGuide: 'https://rails.rubystyle.guide/#callbacks-order'
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# Enforces the use of ActiveRecord calculation methods instead of `pluck`
7+
# followed by Enumerable methods.
8+
#
9+
# It avoids loading potentially many values into memory by doing the
10+
# calculations inside the database.
11+
#
12+
# @example
13+
# # bad
14+
# User.pluck(:id).max
15+
# User.pluck(:id).min
16+
# User.pluck(:age).sum
17+
#
18+
# # good
19+
# User.maximum(:id)
20+
# User.minimum(:id)
21+
# User.sum(:age)
22+
#
23+
# # good
24+
# User.pluck(:email).max { |email| email.length }
25+
# User.pluck(:email).max(2)
26+
# User.pluck(:id, :company_id).max
27+
# User.pluck(:age).count
28+
#
29+
class ActiveRecordCalculation < Base
30+
include RangeHelp
31+
extend AutoCorrector
32+
33+
MSG = 'Use `%<good_method>s` instead of `pluck.%<bad_method>s`.'
34+
35+
RESTRICT_ON_SEND = %i[pluck].freeze
36+
OPERATIONS_MAP = { min: :minimum, max: :maximum, sum: :sum }.freeze
37+
38+
def_node_matcher :pluck_calculation?, <<~PATTERN
39+
(send
40+
(send _ :pluck $_)
41+
${:#{OPERATIONS_MAP.keys.join(' :')}})
42+
PATTERN
43+
44+
def on_send(node)
45+
return unless (parent = node.parent)
46+
return if send_with_block?(parent)
47+
48+
pluck_calculation?(parent) do |arg_node, calculation|
49+
good_method = OPERATIONS_MAP.fetch(calculation)
50+
message = format(MSG, good_method: good_method, bad_method: calculation)
51+
offense_range = range_between(node.loc.selector.begin_pos, parent.source_range.end_pos)
52+
53+
add_offense(offense_range, message: message) do |corrector|
54+
corrector.replace(offense_range, "#{good_method}(#{arg_node.source})")
55+
end
56+
end
57+
end
58+
59+
private
60+
61+
def send_with_block?(node)
62+
node.parent&.block_type? && node.parent.send_node == node
63+
end
64+
end
65+
end
66+
end
67+
end

lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
require_relative 'rails/action_filter'
1515
require_relative 'rails/action_order'
1616
require_relative 'rails/active_record_aliases'
17+
require_relative 'rails/active_record_calculation'
1718
require_relative 'rails/active_record_callbacks_order'
1819
require_relative 'rails/active_record_override'
1920
require_relative 'rails/active_support_aliases'
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::ActiveRecordCalculation, :config do
4+
it 'registers an offense and corrects when using `pluck.max`' do
5+
expect_offense(<<~RUBY)
6+
Model.pluck(:column).max
7+
^^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`.
8+
RUBY
9+
10+
expect_correction(<<~RUBY)
11+
Model.maximum(:column)
12+
RUBY
13+
end
14+
15+
it 'registers an offense and corrects when using `pluck.min`' do
16+
expect_offense(<<~RUBY)
17+
Model.pluck(:column).min
18+
^^^^^^^^^^^^^^^^^^ Use `minimum` instead of `pluck.min`.
19+
RUBY
20+
21+
expect_correction(<<~RUBY)
22+
Model.minimum(:column)
23+
RUBY
24+
end
25+
26+
it 'registers an offense and corrects when using `pluck.sum`' do
27+
expect_offense(<<~RUBY)
28+
Model.pluck(:column).sum
29+
^^^^^^^^^^^^^^^^^^ Use `sum` instead of `pluck.sum`.
30+
RUBY
31+
32+
expect_correction(<<~RUBY)
33+
Model.sum(:column)
34+
RUBY
35+
end
36+
37+
it 'registers an offense and corrects when using `pluck.max` without receiver' do
38+
expect_offense(<<~RUBY)
39+
pluck(:column).max
40+
^^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`.
41+
RUBY
42+
43+
expect_correction(<<~RUBY)
44+
maximum(:column)
45+
RUBY
46+
end
47+
48+
it 'registers an offense and corrects when using `pluck.max` with non-literal column' do
49+
expect_offense(<<~RUBY)
50+
Model.pluck(column).max
51+
^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`.
52+
RUBY
53+
54+
expect_correction(<<~RUBY)
55+
Model.maximum(column)
56+
RUBY
57+
end
58+
59+
it 'does not register an offense when using `pluck.max` with multiple arguments' do
60+
expect_no_offenses(<<~RUBY)
61+
Model.pluck(:column1, :column2).max
62+
RUBY
63+
end
64+
65+
it 'does not register an offense when using `pluck.max` with block' do
66+
expect_no_offenses(<<~RUBY)
67+
Model.pluck(:column).max { |e| e }
68+
RUBY
69+
end
70+
71+
it 'does not register an offense when using `pluck.max` and `max` has argument' do
72+
expect_no_offenses(<<~RUBY)
73+
Model.pluck(:column).max(1)
74+
RUBY
75+
end
76+
77+
it 'does not register an offense when using `maximum`' do
78+
expect_no_offenses(<<~RUBY)
79+
Model.maximum(:column)
80+
RUBY
81+
end
82+
end

0 commit comments

Comments
 (0)