From 24c7a772aa926df6a1c222a96208a39bc356ec3c Mon Sep 17 00:00:00 2001
From: masato-bkn <37011138+masato-bkn@users.noreply.github.com>
Date: Sat, 23 Nov 2024 12:59:46 +0900
Subject: [PATCH] Modify `Rails/Pluck` to ignore `map/collect` when used inside
 blocks to prevent potential N+1 queries

---
 ...odify_rails_pluck_to_ignore_map_collect.md |  1 +
 lib/rubocop/cop/rails/pluck.rb                | 20 +++++++++
 spec/rubocop/cop/rails/pluck_spec.rb          | 42 +++++++++++++++++++
 3 files changed, 63 insertions(+)
 create mode 100644 changelog/change_modify_rails_pluck_to_ignore_map_collect.md

diff --git a/changelog/change_modify_rails_pluck_to_ignore_map_collect.md b/changelog/change_modify_rails_pluck_to_ignore_map_collect.md
new file mode 100644
index 0000000000..ab03eaf10c
--- /dev/null
+++ b/changelog/change_modify_rails_pluck_to_ignore_map_collect.md
@@ -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][])
diff --git a/lib/rubocop/cop/rails/pluck.rb b/lib/rubocop/cop/rails/pluck.rb
index a6094ad648..3030925c48 100644
--- a/lib/rubocop/cop/rails/pluck.rb
+++ b/lib/rubocop/cop/rails/pluck.rb
@@ -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.
       #
@@ -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)
 
diff --git a/spec/rubocop/cop/rails/pluck_spec.rb b/spec/rubocop/cop/rails/pluck_spec.rb
index bf425980a1..4f115e5183 100644
--- a/spec/rubocop/cop/rails/pluck_spec.rb
+++ b/spec/rubocop/cop/rails/pluck_spec.rb
@@ -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