[Fix rubocop#310] Add EnforcedStyle to Rails/PluckInWhere

Fixes rubocop#310.

This PR adds `EnforcedStyle` to `Rails/PluckInWhere`. By default,
it does not register an offense if `pluck` method's receiver is a variable.

CHANGELOG.md

@@ -17,6 +17,7 @@
 
 * [#312](https://github.com/rubocop-hq/rubocop-rails/pull/312): Mark `Rails/MailerName` as unsafe for auto-correct. ([@eugeneius][])
 * [#294](https://github.com/rubocop-hq/rubocop-rails/pull/294): Update `Rails/ReversibleMigration` to register offenses for `remove_columns` and `remove_index`. ([@philcoggins][])
+* [#310](https://github.com/rubocop-hq/rubocop-rails/issues/310): Add `EnforcedStyle` to `Rails/PluckInWhere`. By default, it does not register an offense if `pluck` method's receiver is a variable. ([@koic][])
 
 ## 2.7.1 (2020-07-26)
 

config/default.yml

@@ -435,6 +435,11 @@ Rails/PluckInWhere:
   Enabled: 'pending'
   Safe: false
   VersionAdded: '2.7'
+  VersionChanged: '2.8'
+  EnforcedStyle: conservative
+  SupportedStyles:
+    - conservative
+    - aggressive
 
 Rails/PluralizationGrammar:
   Description: 'Checks for incorrect grammar when using methods like `3.day.ago`.'

docs/modules/ROOT/pages/cops_rails.adoc

@@ -2495,7 +2495,7 @@ end
 | No
 | Yes (Unsafe)
 | 2.7
-| -
+| 2.8
 |===
 
 This cop identifies places where `pluck` is used in `where` query methods
@@ -2504,6 +2504,18 @@ and can be replaced with `select`.
 Since `pluck` is an eager method and hits the database immediately,
 using `select` helps to avoid additional database queries.
 
+This cop has two different enforcement modes. When the EnforcedStyle
+is conservative (the default) then only calls to `pluck` on a constant
+(i.e. a model class) in the `where` is used as offenses.
+
+When the EnforcedStyle is aggressive then all calls to `pluck` in the
+`where` is used as offenses. This may lead to false positives
+as the cop cannot replace to `select` between calls to `pluck` on an
+`ActiveRecord::Relation` instance vs a call to `pluck` on an `Array` instance.
+
+Autocorrect is disabled by default for this cop since it may generate
+false positives.
+
 === Examples
 
 [source,ruby]
@@ -2513,7 +2525,34 @@ Post.where(user_id: User.active.pluck(:id))
 
 # good
 Post.where(user_id: User.active.select(:id))
+Post.where(user_id: active_users.select(:id))
+----
+
+==== EnforcedStyle: conservative (default)
+
+[source,ruby]
+----
+# good
+Post.where(user_id: active_users.pluck(:id))
+----
+
+==== EnforcedStyle: aggressive
+
+[source,ruby]
 ----
+# bad
+Post.where(user_id: active_users.pluck(:id))
+----
+
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| EnforcedStyle
+| `conservative`
+| `conservative`, `aggressive`
+|===
 
 == Rails/PluralizationGrammar
 

lib/rubocop/cop/rails/pluck_in_where.rb

@@ -9,27 +9,64 @@ module Rails
       # Since `pluck` is an eager method and hits the database immediately,
       # using `select` helps to avoid additional database queries.
       #
+      # This cop has two different enforcement modes. When the EnforcedStyle
+      # is conservative (the default) then only calls to `pluck` on a constant
+      # (i.e. a model class) in the `where` is used as offenses.
+      #
+      # When the EnforcedStyle is aggressive then all calls to `pluck` in the
+      # `where` is used as offenses. This may lead to false positives
+      # as the cop cannot replace to `select` between calls to `pluck` on an
+      # `ActiveRecord::Relation` instance vs a call to `pluck` on an `Array` instance.
+      #
+      # Autocorrect is disabled by default for this cop since it may generate
+      # false positives.
+      #
       # @example
       #   # bad
       #   Post.where(user_id: User.active.pluck(:id))
       #
       #   # good
       #   Post.where(user_id: User.active.select(:id))
+      #   Post.where(user_id: active_users.select(:id))
+      #
+      # @example EnforcedStyle: conservative (default)
+      #   # good
+      #   Post.where(user_id: active_users.pluck(:id))
+      #
+      # @example EnforcedStyle: aggressive
+      #   # bad
+      #   Post.where(user_id: active_users.pluck(:id))
       #
       class PluckInWhere < Cop
         include ActiveRecordHelper
+        include ConfigurableEnforcedStyle
 
         MSG = 'Use `select` instead of `pluck` within `where` query method.'
 
         def on_send(node)
-          add_offense(node, location: :selector) if node.method?(:pluck) && in_where?(node)
+          return unless node.method?(:pluck) && in_where?(node)
+          return if style == :conservative && !root_receiver(node)&.const_type?
+
+          add_offense(node, location: :selector)
         end
 
         def autocorrect(node)
           lambda do |corrector|
             corrector.replace(node.loc.selector, 'select')
           end
         end
+
+        private
+
+        def root_receiver(node)
+          receiver = node.receiver
+
+          if receiver&.send_type?
+            root_receiver(receiver)
+          else
+            receiver
+          end
+        end
       end
     end
   end

spec/rubocop/cop/rails/pluck_in_where_spec.rb

@@ -1,45 +1,84 @@
 # frozen_string_literal: true
 
-RSpec.describe RuboCop::Cop::Rails::PluckInWhere do
-  subject(:cop) { described_class.new }
-
-  it 'registers an offense and corrects when using `pluck` in `where`' do
-    expect_offense(<<~RUBY)
-      Post.where(user_id: User.pluck(:id))
-                               ^^^^^ Use `select` instead of `pluck` within `where` query method.
-    RUBY
-
-    expect_correction(<<~RUBY)
-      Post.where(user_id: User.select(:id))
-    RUBY
+RSpec.describe RuboCop::Cop::Rails::PluckInWhere, :config do
+  subject(:cop) { described_class.new(config) }
+
+  let(:cop_config) do
+    { 'EnforcedStyle' => enforced_style }
   end
 
-  it 'registers an offense and corrects when using `pluck` in `rewhere`' do
-    expect_offense(<<~RUBY)
-      Post.rewhere('user_id IN (?)', User.pluck(:id))
-                                          ^^^^^ Use `select` instead of `pluck` within `where` query method.
-    RUBY
+  shared_examples 'receiver is a constant for `pluck`' do
+    it 'registers an offense and corrects when using `pluck` in `where` for constant' do
+      expect_offense(<<~RUBY)
+        Post.where(user_id: User.active.pluck(:id))
+                                        ^^^^^ Use `select` instead of `pluck` within `where` query method.
+      RUBY
 
-    expect_correction(<<~RUBY)
-      Post.rewhere('user_id IN (?)', User.select(:id))
-    RUBY
-  end
+      expect_correction(<<~RUBY)
+        Post.where(user_id: User.active.select(:id))
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `pluck` in `rewhere` for constant' do
+      expect_offense(<<~RUBY)
+        Post.rewhere('user_id IN (?)', User.active.pluck(:id))
+                                                   ^^^^^ Use `select` instead of `pluck` within `where` query method.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        Post.rewhere('user_id IN (?)', User.active.select(:id))
+      RUBY
+    end
+
+    it 'does not register an offense when using `select` in `where`' do
+      expect_no_offenses(<<~RUBY)
+        Post.where(user_id: User.active.select(:id))
+      RUBY
+    end
+
+    it 'does not register an offense when using `pluck` chained with other method calls in `where`' do
+      expect_no_offenses(<<~RUBY)
+        Post.where(user_id: User.pluck(:id).map(&:to_i))
+      RUBY
+    end
 
-  it 'does not register an offense when using `select` in `where`' do
-    expect_no_offenses(<<~RUBY)
-      Post.where(user_id: User.select(:id))
-    RUBY
+    it 'does not register an offense when using `select` in query methods other than `where`' do
+      expect_no_offenses(<<~RUBY)
+        Post.order(columns.pluck(:name))
+      RUBY
+    end
   end
 
-  it 'does not register an offense when using `pluck` chained with other method calls in `where`' do
-    expect_no_offenses(<<~RUBY)
-      Post.where(user_id: User.pluck(:id).map(&:to_i))
-    RUBY
+  context 'EnforcedStyle: conservative' do
+    let(:enforced_style) { 'conservative' }
+
+    it_behaves_like 'receiver is a constant for `pluck`'
+
+    context 'receiver is a variable for `pluck`' do
+      it 'does not register an offense when using `pluck` in `where`' do
+        expect_no_offenses(<<~RUBY)
+          Post.where(user_id: users.active.pluck(:id))
+        RUBY
+      end
+    end
   end
 
-  it 'does not register an offense when using `select` in query methods other than `where`' do
-    expect_no_offenses(<<~RUBY)
-      Post.order(columns.pluck(:name))
-    RUBY
+  context 'EnforcedStyle: aggressive' do
+    let(:enforced_style) { 'aggressive' }
+
+    it_behaves_like 'receiver is a constant for `pluck`'
+
+    context 'receiver is a variable for `pluck`' do
+      it 'registers and corrects an offense when using `pluck` in `where`' do
+        expect_offense(<<~RUBY)
+          Post.where(user_id: users.active.pluck(:id))
+                                           ^^^^^ Use `select` instead of `pluck` within `where` query method.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Post.where(user_id: users.active.select(:id))
+        RUBY
+      end
+    end
   end
 end