lib/rubocop/cop/rails/uniq_before_pluck.rb in rubocop-rails-2.12.4 vs lib/rubocop/cop/rails/uniq_before_pluck.rb in rubocop-rails-2.13.0

@@ -1,90 +1,84 @@
 # frozen_string_literal: true
 
 module RuboCop
   module Cop
     module Rails
-      # Prefer the use of distinct, before pluck instead of after.
+      # Prefer using `distinct` before `pluck` instead of `uniq` after `pluck`.
       #
-      # The use of distinct before pluck is preferred because it executes within
+      # The use of distinct before pluck is preferred because it executes by
       # 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 distinct are added as offenses.
+      # is `conservative` (the default), then only calls to `pluck` on a constant
+      # (i.e. a model class) before `uniq` are added as offenses.
       #
-      # When the EnforcedStyle is aggressive then all calls to pluck before
+      # When the EnforcedStyle is `aggressive` then all calls to `pluck` before
       # distinct are added as offenses. This may lead to false positives
-      # as the cop cannot distinguish between calls to pluck on an
+      # as the cop cannot distinguish between calls to `pluck` on an
       # ActiveRecord::Relation vs a call to pluck on an
       # ActiveRecord::Associations::CollectionProxy.
       #
-      # This cop is unsafe because the behavior may change depending on the
-      # database collation.
-      # Autocorrect is disabled by default for this cop since it may generate
-      # false positives.
+      # @safety
+      #   This cop is unsafe for autocorrection because the behavior may change
+      #   depending on the database collation.
       #
       # @example EnforcedStyle: conservative (default)
-      #   # bad
-      #   Model.pluck(:id).uniq
+      #   # bad - redundantly fetches duplicate values
+      #   Album.pluck(:band_name).uniq
       #
       #   # good
-      #   Model.distinct.pluck(:id)
+      #   Album.distinct.pluck(:band_name)
       #
       # @example EnforcedStyle: aggressive
-      #   # bad
-      #   # this will return a Relation that pluck is called on
-      #   Model.where(cond: true).pluck(:id).uniq
+      #   # bad - redundantly fetches duplicate values
+      #   Album.pluck(:band_name).uniq
       #
-      #   # bad
-      #   # an association on an instance will return a CollectionProxy
-      #   instance.assoc.pluck(:id).uniq
+      #   # bad - redundantly fetches duplicate values
+      #   Album.where(year: 1985).pluck(:band_name).uniq
       #
-      #   # bad
-      #   Model.pluck(:id).uniq
+      #   # bad - redundantly fetches duplicate values
+      #   customer.favourites.pluck(:color).uniq
       #
       #   # good
-      #   Model.distinct.pluck(:id)
+      #   Album.distinct.pluck(:band_name)
+      #   Album.distinct.where(year: 1985).pluck(:band_name)
+      #   customer.favourites.distinct.pluck(:color)
       #
       class UniqBeforePluck < Base
         include ConfigurableEnforcedStyle
         include RangeHelp
         extend AutoCorrector
 
         MSG = 'Use `distinct` before `pluck`.'
-        RESTRICT_ON_SEND = %i[uniq distinct pluck].freeze
+        RESTRICT_ON_SEND = %i[uniq].freeze
         NEWLINE = "\n"
-        PATTERN = '[!^block (send (send %<type>s :pluck ...) ' \
-                  '${:uniq :distinct} ...)]'
+        PATTERN = '[!^block (send (send %<type>s :pluck ...) :uniq ...)]'
 
         def_node_matcher :conservative_node_match,
                          format(PATTERN, type: 'const')
 
         def_node_matcher :aggressive_node_match,
                          format(PATTERN, type: '_')
 
         def on_send(node)
-          method = if style == :conservative
-                     conservative_node_match(node)
-                   else
-                     aggressive_node_match(node)
-                   end
+          uniq = if style == :conservative
+                   conservative_node_match(node)
+                 else
+                   aggressive_node_match(node)
+                 end
 
-          return unless method
+          return unless uniq
 
           add_offense(node.loc.selector) do |corrector|
             method = node.method_name
 
             corrector.remove(dot_method_with_whitespace(method, node))
             corrector.insert_before(node.receiver.loc.dot.begin, '.distinct')
           end
         end
 
         private
-
-        def style_parameter_name
-          'EnforcedStyle'
-        end
 
         def dot_method_with_whitespace(method, node)
           range_between(dot_method_begin_pos(method, node),
                         node.loc.selector.end_pos)
         end