Deprecate passing arguments and block at the same time to `count` and `sum` in `ActiveRecord::Calculations`

`select`, `count`, and `sum` in `Relation` are also `Enumerable` method
that can be passed block. `select` with block already doesn't take
arguments since 4fc3366. This is follow up of that.

3 files changed, 36 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index d17bbf80ca..1f8163db12 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Deprecate passing arguments and block at the same time to `count` and `sum` in `ActiveRecord::Calculations`.
+
+    *Ryuta Kamizono*
+
 *   Loading model schema from database is now thread-safe.
 
     Fixes #28589.
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 861b2125d4..c562f214c9 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -37,7 +37,16 @@ module ActiveRecord
     # Note: not all valid {Relation#select}[rdoc-ref:QueryMethods#select] expressions are valid #count expressions. The specifics differ
     # between databases. In invalid cases, an error from the database is thrown.
     def count(column_name = nil)
-      return super() if block_given?
+      if block_given?
+        unless column_name.nil?
+          ActiveSupport::Deprecation.warn \
+            "When `count' is called with a block, it ignores other arguments. " \
+            "This behavior is now deprecated and will result in an ArgumentError in Rails 5.3."
+        end
+
+        return super()
+      end
+
       calculate(:count, column_name)
     end
 
@@ -73,7 +82,16 @@ module ActiveRecord
     #
     #   Person.sum(:age) # => 4562
     def sum(column_name = nil)
-      return super() if block_given?
+      if block_given?
+        unless column_name.nil?
+          ActiveSupport::Deprecation.warn \
+            "When `sum' is called with a block, it ignores other arguments. " \
+            "This behavior is now deprecated and will result in an ArgumentError in Rails 5.3."
+        end
+
+        return super()
+      end
+
       calculate(:sum, column_name)
     end
 
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 3214d778d4..93f8ab18c2 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -809,4 +809,16 @@ class CalculationsTest < ActiveRecord::TestCase
   def test_group_by_attribute_with_custom_type
     assert_equal({ "proposed" => 2, "published" => 2 }, Book.group(:status).count)
   end
+
+  def test_deprecate_count_with_block_and_column_name
+    assert_deprecated do
+      assert_equal 6, Account.count(:firm_id) { true }
+    end
+  end
+
+  def test_deprecate_sum_with_block_and_column_name
+    assert_deprecated do
+      assert_equal 6, Account.sum(:firm_id) { 1 }
+    end
+  end
 end