Skip to content

Commit 24c7a77

Browse files
committed
Modify Rails/Pluck to ignore map/collect when used inside blocks to prevent potential N+1 queries
1 parent d5b012f commit 24c7a77

File tree

3 files changed

+63
-0
lines changed

3 files changed

+63
-0
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1388](https://github.com/rubocop/rubocop-rails/pull/1388): Modify `Rails/Pluck` to ignore `map/collect` when used inside blocks to prevent potential N+1 queries. ([@masato-bkn][])

lib/rubocop/cop/rails/pluck.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,24 @@ module Rails
99
# element in an enumerable. When called on an Active Record relation, it
1010
# results in a more efficient query that only selects the necessary key.
1111
#
12+
# NOTE: If the receiver's relation is not loaded and `pluck` is used inside an iteration,
13+
# it may result in N+1 queries because `pluck` queries the database on each iteration.
14+
# This cop ignores offenses for `map/collect` when they are suspected to be part of an iteration
15+
# to prevent such potential issues.
16+
#
17+
# [source,ruby]
18+
# ----
19+
# users = User.all
20+
# 5.times do
21+
# users.map { |user| user[:foo] } # Only one query is executed
22+
# end
23+
#
24+
# users = User.all
25+
# 5.times do
26+
# users.pluck(:id) # A query is executed on every iteration
27+
# end
28+
# ----
29+
#
1230
# @safety
1331
# This cop is unsafe because model can use column aliases.
1432
#
@@ -42,6 +60,8 @@ class Pluck < Base
4260
PATTERN
4361

4462
def on_block(node)
63+
return if node.each_ancestor(:block, :numblock).any?
64+
4565
pluck_candidate?(node) do |argument, key|
4666
next if key.regexp_type? || !use_one_block_argument?(argument)
4767

spec/rubocop/cop/rails/pluck_spec.rb

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,48 @@
128128
end
129129
end
130130
end
131+
132+
context "when `#{method}` is used in block" do
133+
it 'does not register an offense' do
134+
expect_no_offenses(<<~RUBY)
135+
n.each do |x|
136+
x.#{method} { |a| a[:foo] }
137+
end
138+
RUBY
139+
end
140+
end
141+
142+
context "when `#{method}` is used in block with other operations" do
143+
it 'does not register an offense' do
144+
expect_no_offenses(<<~RUBY)
145+
n.each do |x|
146+
do_something
147+
x.#{method} { |a| a[:foo] }
148+
end
149+
RUBY
150+
end
151+
end
152+
153+
context "when `#{method}` is used in numblock" do
154+
it 'does not register an offense' do
155+
expect_no_offenses(<<~RUBY)
156+
n.each do
157+
_1.#{method} { |a| a[:foo] }
158+
end
159+
RUBY
160+
end
161+
end
162+
163+
context "when `#{method}` is used in numblock with other operations" do
164+
it 'does not register an offense' do
165+
expect_no_offenses(<<~RUBY)
166+
n.each do
167+
do_something
168+
_1.#{method} { |a| a[:foo] }
169+
end
170+
RUBY
171+
end
172+
end
131173
end
132174

133175
context 'when using Rails 4.2 or older', :rails42 do

0 commit comments

Comments
 (0)