[Fix #1435] Add a new cop for PluckOnSelect.

.rubocop_todo.yml

@@ -1,18 +1,11 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2025-01-22 15:33:45 UTC using RuboCop version 1.70.0.
+# on 2025-03-12 19:37:57 UTC using RuboCop version 1.61.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 2
-# This cop supports safe autocorrection (--autocorrect).
-InternalAffairs/CopEnabled:
-  Exclude:
-    - 'lib/rubocop/cop/rails/blank.rb'
-    - 'lib/rubocop/cop/rails/present.rb'
-
 # Offense count: 7
 InternalAffairs/NodeDestructuring:
   Exclude:
@@ -22,33 +15,36 @@ InternalAffairs/NodeDestructuring:
     - 'lib/rubocop/cop/rails/relative_date_constant.rb'
     - 'lib/rubocop/cop/rails/time_zone.rb'
 
-# Offense count: 84
-InternalAffairs/OnSendWithoutOnCSend:
-  Enabled: false
+# Offense count: 2
+# This cop supports safe autocorrection (--autocorrect).
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: present?, blank?, presence, try, try!
+Lint/SafeNavigationConsistency:
+  Exclude:
+    - 'lib/rubocop/cop/rails/squished_sql_heredocs.rb'
+    - 'lib/rubocop/cop/rails/where_range.rb'
 
 # Offense count: 10
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 163
-  Exclude:
-    - 'lib/rubocop/cop/rails/file_path.rb'
+  Max: 197
 
-# Offense count: 41
+# Offense count: 42
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
   Max: 14
 
-# Offense count: 183
+# Offense count: 185
 # Configuration parameters: Prefixes, AllowedPatterns.
 # Prefixes: when, with, without
 RSpec/ContextWording:
   Enabled: false
 
-# Offense count: 1091
+# Offense count: 1096
 # Configuration parameters: CountAsOne.
 RSpec/ExampleLength:
   Max: 108
 
-# Offense count: 733
+# Offense count: 739
 RSpec/MultipleExpectations:
   Max: 5

changelog/new_pluck_on_select_cop.md

@@ -0,0 +1 @@
+* [#1435](https://github.com/rubocop/rubocop-rails/issues/1435): Add new `Rails/PluckOnSelect` cop. ([@blnoonan][])

config/default.yml

@@ -784,6 +784,15 @@ Rails/PluckInWhere:
     - conservative
     - aggressive
 
+Rails/PluckOnSelect:
+  Description: 'Do not use `pluck` on `select`.'
+  Enabled: 'pending'
+  VersionAdded: '<<next>>'
+  EnforcedStyle: conservative
+  SupportedStyles:
+    - conservative
+    - aggressive
+
 Rails/PluralizationGrammar:
   Description: 'Checks for incorrect grammar when using methods like `3.day.ago`.'
   Enabled: true

lib/rubocop/cop/rails/pluck_on_select.rb

@@ -0,0 +1,135 @@
+# typed: false
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Do not use .pluck on .select.
+      #
+      # - .select returns an ActiveRecord relation with only the selected column(s) marked for retrieval
+      # - .pluck returns an array of column values
+      #
+      # Using them together is at best redundant and at worst confusing and inefficient.
+      # When chained with .select, .pluck is unaware of any directive passed to .select
+      # (e.g. column aliases or a DISTINCT clause). This can lead to unexpected behavior.
+      #
+      # @example
+      #
+      #   # before
+      #   User.select(:id).pluck(:id)
+      #
+      #   # after
+      #   User.pluck(:id)
+      #
+      #   # The .select is redundant. Use either .select or .pluck on its own.
+      #
+      # @example
+      #
+      #   # before
+      #   User.select('id, email AS user_email').pluck('id', 'user_email')
+      #
+      #   # after
+      #   User.pluck(:id, :email)
+      #
+      #   # after
+      #   User.select(:id, 'email AS user_email')
+      #
+      #   # .pluck is unaware of the alias created by .select and will raise an "Unknown column" error.
+      #   # If you need the alias, use .select on its own. Otherwise, consider using .pluck on its own.
+      #
+      # @example
+      #
+      #   # before
+      #   User.select('DISTINCT email').pluck(:email)
+      #
+      #   # after
+      #   User.group(:email).pluck(:email)
+      #
+      #   # after
+      #   User.distinct.pluck(:email)
+      #
+      #   # after
+      #   User.distinct.select(:email)
+      #
+      #   # .pluck is unaware of .select's DISTINCT directive and will load all User emails from the
+      #   # database - including duplicates. Use either .select or .pluck on its own with .distinct,
+      #   # or use .group (which can be more efficient).
+      #
+      # @example
+      #
+      #   # before
+      #   User.select(:company_id).distinct.pluck(:company_id)
+      #
+      #   # after
+      #   User.group(:company_id).pluck(:company_id)
+      #
+      #   # after
+      #   User.distinct.pluck(:company_id)
+      #
+      #   # after
+      #   User.distinct.select(:company_id)
+      #
+      #   # The .select is redundant. Use either .select or .pluck on its own with .distinct,
+      #   # or use .group (which can be more efficient).
+      #
+      # @example EnforcedStyle: aggressive
+      #
+      #   # before
+      #   User.select(&:active?).pluck(:id)
+      #
+      #   # after
+      #   User.where(active: true).pluck(:id)
+      #
+      #   # after (caution - potentially memory-intensive)
+      #   User.select(&:active?).map(&:id)
+      #
+      #   # after (caution - potentially memory-intensive)
+      #   User.filter(&:active?).map(&:id)
+      #
+      #   # .select and .pluck make this statement look like an ActiveRecord operation, but under the hood
+      #   # .select is loading all Users into memory before filtering them using the active? method in Ruby.
+      #   # Use .where to avoid loading all Users into memory, or use .filter or .map to make it
+      #   # clear to readers this is not an ActiveRecord operation.
+      #
+      class PluckOnSelect < Base
+        include ConfigurableEnforcedStyle
+
+        RESTRICT_ON_SEND = %i[pluck].freeze
+        MSG = 'Do not use `.pluck` on `.select`.'
+
+        def on_send(node)
+          return unless node.receiver
+
+          # Check if any receiver in the chain is a select method
+          add_offense(node) if contains_select_in_chain?(node)
+        end
+
+        private
+
+        # Helper method to check if the chain contains a select method
+        def contains_select_in_chain?(node)
+          return false unless node.receiver
+
+          receiver = node.receiver
+          while receiver
+            if receiver.send_type? && receiver.method?(:select)
+              # For conservative style, ignore if select has a block argument
+              return false if style == :conservative && block_argument?(receiver)
+
+              return true
+            end
+            receiver = receiver.receiver
+          end
+          false
+        end
+
+        def block_argument?(node)
+          # Check if the first argument is a block pass (&:something)
+          node.first_argument&.block_pass_type?
+        end
+
+        alias on_csend on_send
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -89,6 +89,7 @@
 require_relative 'rails/pluck'
 require_relative 'rails/pluck_id'
 require_relative 'rails/pluck_in_where'
+require_relative 'rails/pluck_on_select'
 require_relative 'rails/pluralization_grammar'
 require_relative 'rails/presence'
 require_relative 'rails/present'