[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,15 @@ 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.
+
 === Examples
 
 [source,ruby]
@@ -2513,8 +2522,35 @@ 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,61 @@ 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.
+ #
  # @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