Deprecate the `:distinct` option for `Relation#count`.

We moved more and more away from passing options to finder / calculation
methods. The `:distinct` option in `#count` was one of the remaining places.
Since we can now combine `Relation#distinct` with `Relation#count` the option
is no longer necessary and can be deprecated.

8 files changed, 44 insertions, 15 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 37be191e0c..6affb2aada 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,18 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   The `:distinct` option for `Relation#count` is deprecated. You
+    should use `Relation#distinct` instead.
+
+    Example:
+
+        # Before
+        Post.select(:author_name).count(distinct: true)
+
+        # After
+        Post.select(:author_name).distinct.count
+
+    *Yves Senn*
+
 *   Rename `Relation#uniq` to `Relation#distinct`. `#uniq` is still
     available as an alias but we encourage to use `#distinct` instead.
     Also `Relation#uniq_value` is aliased to `Relation#distinct_value`,
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 552f7f1117..18b7dc3668 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -174,13 +174,14 @@ module ActiveRecord
 
           reflection.klass.count_by_sql(custom_counter_sql)
         else
+          relation = scope
           if association_scope.distinct_value
             # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
             column_name ||= reflection.klass.primary_key
-            count_options[:distinct] = true
+            relation = relation.distinct
           end
 
-          value = scope.count(column_name, count_options)
+          value = relation.count(column_name)
 
           limit  = options[:limit]
           offset = options[:offset]
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 6fedfefdee..be011b22af 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -11,7 +11,7 @@ module ActiveRecord
     #   Person.count(:all)
     #   # => performs a COUNT(*) (:all is an alias for '*')
     #
-    #   Person.count(:age, distinct: true)
+    #   Person.distinct.count(:age)
     #   # => counts the number of different age values
     #
     # If +count+ is used with +group+, it returns a Hash whose keys represent the aggregated column,
@@ -199,7 +199,12 @@ module ActiveRecord
       operation = operation.to_s.downcase
 
       # If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count)
-      distinct = options[:distinct] || self.distinct_value
+      distinct = self.distinct_value
+      if options.has_key?(:distinct)
+        ActiveSupport::Deprecation.warn "The :distinct option for `Relation#count` is deprecated. " \
+          "Please use `Relation#distinct` instead. (eg. `relation.distinct.count`)"
+        distinct = options[:distinct]
+      end
 
       if operation == "count"
         column_name ||= (select_for_count || :all)
diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index 80bca7f63e..e693d34f99 100644
--- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
+++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -55,7 +55,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
     assert_nothing_raised do
       assert_equal 4, categories.count
       assert_equal 4, categories.to_a.count
-      assert_equal 3, categories.count(:distinct => true)
+      assert_equal 3, categories.distinct.count
       assert_equal 3, categories.to_a.uniq.size # Must uniq since instantiating with inner joins will get dupes
     end
   end
diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb
index 4f246f575e..918783e8f1 100644
--- a/activerecord/test/cases/associations/inner_join_association_test.rb
+++ b/activerecord/test/cases/associations/inner_join_association_test.rb
@@ -82,7 +82,7 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase
 
   def test_calculate_honors_implicit_inner_joins_and_distinct_and_conditions
     real_count = Author.all.to_a.select {|a| a.posts.any? {|p| p.title =~ /^Welcome/} }.length
-    authors_with_welcoming_post_titles = Author.all.merge!(:joins => :posts, :where => "posts.title like 'Welcome%'").calculate(:count, 'authors.id', :distinct => true)
+    authors_with_welcoming_post_titles = Author.all.merge!(joins: :posts, where: "posts.title like 'Welcome%'").distinct.calculate(:count, 'authors.id')
     assert_equal real_count, authors_with_welcoming_post_titles, "inner join and conditions should have only returned authors posting titles starting with 'Welcome'"
   end
 
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 6f8290bdc3..960c27da3e 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1105,7 +1105,7 @@ class BasicsTest < ActiveRecord::TestCase
     res6 = Post.count_by_sql "SELECT COUNT(DISTINCT p.id) FROM posts p, comments co WHERE p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id"
     res7 = nil
     assert_nothing_raised do
-      res7 = Post.where("p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id").joins("p, comments co").select("p.id").count(distinct: true)
+      res7 = Post.where("p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id").joins("p, comments co").select("p.id").distinct.count
     end
     assert_equal res6, res7
   end
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 443a09f259..c645523905 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -305,8 +305,8 @@ class CalculationsTest < ActiveRecord::TestCase
   end
 
   def test_should_count_selected_field_with_include
-    assert_equal 6, Account.includes(:firm).count(:distinct => true)
-    assert_equal 4, Account.includes(:firm).select(:credit_limit).count(:distinct => true)
+    assert_equal 6, Account.includes(:firm).distinct.count
+    assert_equal 4, Account.includes(:firm).distinct.select(:credit_limit).count
   end
 
   def test_should_not_perform_joined_include_by_default
@@ -341,6 +341,16 @@ class CalculationsTest < ActiveRecord::TestCase
     assert_equal 5, Account.count(:firm_id)
   end
 
+  def test_count_distinct_option_is_deprecated
+    assert_deprecated do
+      assert_equal 4, Account.select(:credit_limit).count(distinct: true)
+    end
+
+    assert_deprecated do
+      assert_equal 6, Account.select(:credit_limit).count(distinct: false)
+    end
+  end
+
   def test_count_with_distinct
     assert_equal 4, Account.select(:credit_limit).distinct.count
     assert_equal 4, Account.select(:credit_limit).uniq.count
@@ -352,7 +362,7 @@ class CalculationsTest < ActiveRecord::TestCase
 
   def test_should_count_field_in_joined_table
     assert_equal 5, Account.joins(:firm).count('companies.id')
-    assert_equal 4, Account.joins(:firm).count('companies.id', :distinct => true)
+    assert_equal 4, Account.joins(:firm).distinct.count('companies.id')
   end
 
   def test_should_count_field_in_joined_table_with_group_by
@@ -456,7 +466,7 @@ class CalculationsTest < ActiveRecord::TestCase
     approved_topics_count = Topic.group(:approved).count(:author_name)[true]
     assert_equal approved_topics_count, 3
     # Count the number of distinct authors for approved Topics
-    distinct_authors_for_approved_count = Topic.group(:approved).count(:author_name, :distinct => true)[true]
+    distinct_authors_for_approved_count = Topic.group(:approved).distinct.count(:author_name)[true]
     assert_equal distinct_authors_for_approved_count, 2
   end
 
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 3de2c6ab58..db81ceeb52 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -790,11 +790,11 @@ class RelationTest < ActiveRecord::TestCase
   def test_count_with_distinct
     posts = Post.all
 
-    assert_equal 3, posts.count(:comments_count, :distinct => true)
-    assert_equal 11, posts.count(:comments_count, :distinct => false)
+    assert_equal 3, posts.distinct(true).count(:comments_count)
+    assert_equal 11, posts.distinct(false).count(:comments_count)
 
-    assert_equal 3, posts.select(:comments_count).count(:distinct => true)
-    assert_equal 11, posts.select(:comments_count).count(:distinct => false)
+    assert_equal 3, posts.distinct(true).select(:comments_count).count
+    assert_equal 11, posts.distinct(false).select(:comments_count).count
   end
 
   def test_count_explicit_columns