Merge pull request #1388 from masato-bkn/modify_rails_pluck_to_ignore_map_when_used_inside_block

Modify `Rails/Pluck` to ignore `map/collect` when used inside blocks to prevent potential N+1 queries

Author: koic

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][])

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)
 

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