Merge pull request #135 from santib/fix-uniq-before-pluck

Fix UniqBeforePluck cop

CHANGELOG.md

@@ -15,6 +15,7 @@
 * [#238](https://github.com/rubocop-hq/rubocop-rails/issues/238): Fix auto correction for `Rails/IndexBy` when the `.to_h` invocation is separated in multiple lines. ([@diogoosorio][])
 * [#248](https://github.com/rubocop-hq/rubocop-rails/pull/248): Fix a false positive for `Rails/SaveBang` when `update` is called on `ENV`. ([@eugeneius][])
 * [#251](https://github.com/rubocop-hq/rubocop-rails/pull/251): Fix a false positive for `Rails/FilePath` when the result of `Rails.root.join` is interpolated at the end of a string. ([@eugeneius][])
+* [#91](https://github.com/rubocop-hq/rubocop-rails/issues/91): Fix `Rails/UniqBeforePluck` to not recommend using `uniq` in `ActiveRecord::Relation`s anymore since it was deprecated in Rails 5.0. ([@santib][], [@ghiculescu][])
 
 ### Changes
 
@@ -193,3 +194,4 @@
 [@tejasbubane]: https://github.com/tejasbubane
 [@diogoosorio]: https://github.com/diogoosorio
 [@tabuchi0919]: https://github.com/tabuchi0919
+[@ghiculescu]: https://github.com/ghiculescu

config/default.yml

@@ -534,7 +534,7 @@ Rails/UniqBeforePluck:
   Description: 'Prefer the use of uniq or distinct before pluck.'
   Enabled: true
   VersionAdded: '0.40'
-  VersionChanged: '0.47'
+  VersionChanged: '2.6'
   EnforcedStyle: conservative
   SupportedStyles:
     - conservative

docs/modules/ROOT/pages/cops_rails.adoc

@@ -3243,22 +3243,23 @@ Time.at(timestamp).in_time_zone
 | Yes
 | Yes
 | 0.40
-| 0.47
+| 2.6
 |===
 
-Prefer the use of uniq (or distinct), before pluck instead of after.
+Prefer the use of distinct, before pluck instead of after.
 
-The use of uniq before pluck is preferred because it executes within
+The use of distinct before pluck is preferred because it executes within
 the database.
 
 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) before uniq are added as offenses.
+(i.e. a model class) before distinct are added as offenses.
 
 When the EnforcedStyle is aggressive then all calls to pluck before
-uniq are added as offenses. This may lead to false positives as the cop
-cannot distinguish between calls to pluck on an ActiveRecord::Relation
-vs a call to pluck on an ActiveRecord::Associations::CollectionProxy.
+distinct are added as offenses. This may lead to false positives
+as the cop cannot distinguish between calls to pluck on an
+ActiveRecord::Relation vs a call to pluck on an
+ActiveRecord::Associations::CollectionProxy.
 
 Autocorrect is disabled by default for this cop since it may generate
 false positives.
@@ -3270,10 +3271,10 @@ false positives.
 [source,ruby]
 ----
 # bad
-Model.pluck(:id).uniq
+Model.pluck(:id).distinct
 
 # good
-Model.uniq.pluck(:id)
+Model.distinct.pluck(:id)
 ----
 
 ==== EnforcedStyle: aggressive
@@ -3282,17 +3283,17 @@ Model.uniq.pluck(:id)
 ----
 # bad
 # this will return a Relation that pluck is called on
-Model.where(cond: true).pluck(:id).uniq
+Model.where(cond: true).pluck(:id).distinct
 
 # bad
 # an association on an instance will return a CollectionProxy
-instance.assoc.pluck(:id).uniq
+instance.assoc.pluck(:id).distinct
 
 # bad
-Model.pluck(:id).uniq
+Model.pluck(:id).distinct
 
 # good
-Model.uniq.pluck(:id)
+Model.distinct.pluck(:id)
 ----
 
 === Configurable attributes

lib/rubocop/cop/rails/uniq_before_pluck.rb

@@ -3,50 +3,51 @@
 module RuboCop
   module Cop
     module Rails
-      # Prefer the use of uniq (or distinct), before pluck instead of after.
+      # Prefer the use of distinct, before pluck instead of after.
       #
-      # The use of uniq before pluck is preferred because it executes within
+      # The use of distinct before pluck is preferred because it executes within
       # the database.
       #
       # 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) before uniq are added as offenses.
+      # (i.e. a model class) before distinct are added as offenses.
       #
       # When the EnforcedStyle is aggressive then all calls to pluck before
-      # uniq are added as offenses. This may lead to false positives as the cop
-      # cannot distinguish between calls to pluck on an ActiveRecord::Relation
-      # vs a call to pluck on an ActiveRecord::Associations::CollectionProxy.
+      # distinct are added as offenses. This may lead to false positives
+      # as the cop cannot distinguish between calls to pluck on an
+      # ActiveRecord::Relation vs a call to pluck on an
+      # ActiveRecord::Associations::CollectionProxy.
       #
       # Autocorrect is disabled by default for this cop since it may generate
       # false positives.
       #
       # @example EnforcedStyle: conservative (default)
       #   # bad
-      #   Model.pluck(:id).uniq
+      #   Model.pluck(:id).distinct
       #
       #   # good
-      #   Model.uniq.pluck(:id)
+      #   Model.distinct.pluck(:id)
       #
       # @example EnforcedStyle: aggressive
       #   # bad
       #   # this will return a Relation that pluck is called on
-      #   Model.where(cond: true).pluck(:id).uniq
+      #   Model.where(cond: true).pluck(:id).distinct
       #
       #   # bad
       #   # an association on an instance will return a CollectionProxy
-      #   instance.assoc.pluck(:id).uniq
+      #   instance.assoc.pluck(:id).distinct
       #
       #   # bad
-      #   Model.pluck(:id).uniq
+      #   Model.pluck(:id).distinct
       #
       #   # good
-      #   Model.uniq.pluck(:id)
+      #   Model.distinct.pluck(:id)
       #
       class UniqBeforePluck < RuboCop::Cop::Cop
         include ConfigurableEnforcedStyle
         include RangeHelp
 
-        MSG = 'Use `%<method>s` before `pluck`.'
+        MSG = 'Use `distinct` before `pluck`.'
         NEWLINE = "\n"
         PATTERN = '[!^block (send (send %<type>s :pluck ...) ' \
                   '${:uniq :distinct} ...)]'
@@ -75,7 +76,7 @@ def autocorrect(node)
             method = node.method_name
 
             corrector.remove(dot_method_with_whitespace(method, node))
-            corrector.insert_before(node.receiver.loc.dot.begin, ".#{method}")
+            corrector.insert_before(node.receiver.loc.dot.begin, '.distinct')
           end
         end
 

spec/rubocop/cop/rails/uniq_before_pluck_spec.rb

@@ -8,9 +8,9 @@
       if action == :correct
         it "finds the use of #{method} after pluck in #{source}" do
           inspect_source(source)
-          expect(cop.messages).to eq(["Use `#{method}` before `pluck`."])
+          expect(cop.messages).to eq(['Use `distinct` before `pluck`.'])
           expect(cop.highlights).to eq([method])
-          corrected_source = corrected || "Model.#{method}.pluck(:id)"
+          corrected_source = corrected || 'Model.distinct.pluck(:id)'
           expect(autocorrect_source(source)).to eq(corrected_source)
         end
       else
@@ -57,11 +57,11 @@
   shared_examples_for 'mode dependent offenses' do |method, action|
     it_behaves_like 'UniqBeforePluck cop', method,
                     "Model.scope.pluck(:id).#{method}", action,
-                    "Model.scope.#{method}.pluck(:id)"
+                    'Model.scope.distinct.pluck(:id)'
 
     it_behaves_like 'UniqBeforePluck cop', method,
                     "instance.assoc.pluck(:id).#{method}", action,
-                    "instance.assoc.#{method}.pluck(:id)"
+                    'instance.assoc.distinct.pluck(:id)'
   end
 
   %w[uniq distinct].each do |method|