Skip to content

Commit

Permalink
Modify Rails/Pluck to ignore map/collect when used inside blocks …
Browse files Browse the repository at this point in the history
…to prevent potential N+1 queries
  • Loading branch information
masato-bkn committed Nov 23, 2024
1 parent d5b012f commit 24c7a77
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +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][])
20 changes: 20 additions & 0 deletions lib/rubocop/cop/rails/pluck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ module Rails
# element in an enumerable. When called on an Active Record relation, it
# results in a more efficient query that only selects the necessary key.
#
# NOTE: If the receiver's relation is not loaded and `pluck` is used inside an iteration,
# it may result in N+1 queries because `pluck` queries the database on each iteration.
# This cop ignores offenses for `map/collect` when they are suspected to be part of an iteration
# to prevent such potential issues.
#
# [source,ruby]
# ----
# users = User.all
# 5.times do
# users.map { |user| user[:foo] } # Only one query is executed
# end
#
# users = User.all
# 5.times do
# users.pluck(:id) # A query is executed on every iteration
# end
# ----
#
# @safety
# This cop is unsafe because model can use column aliases.
#
Expand Down Expand Up @@ -42,6 +60,8 @@ class Pluck < Base
PATTERN

def on_block(node)
return if node.each_ancestor(:block, :numblock).any?

pluck_candidate?(node) do |argument, key|
next if key.regexp_type? || !use_one_block_argument?(argument)

Expand Down
42 changes: 42 additions & 0 deletions spec/rubocop/cop/rails/pluck_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,48 @@
end
end
end

context "when `#{method}` is used in block" do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
n.each do |x|
x.#{method} { |a| a[:foo] }
end
RUBY
end
end

context "when `#{method}` is used in block with other operations" do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
n.each do |x|
do_something
x.#{method} { |a| a[:foo] }
end
RUBY
end
end

context "when `#{method}` is used in numblock" do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
n.each do
_1.#{method} { |a| a[:foo] }
end
RUBY
end
end

context "when `#{method}` is used in numblock with other operations" do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
n.each do
do_something
_1.#{method} { |a| a[:foo] }
end
RUBY
end
end
end

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

0 comments on commit 24c7a77

Please sign in to comment.