From 87886c93dc22d528d8529d8b63873f699beaf695 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 31 May 2020 12:19:32 +0900
Subject: [PATCH] Deprecate passing an Active Record object to
 `quote`/`type_cast` directly

Follow up to #27962.

#27962 only deprecated `quoted_id`, but still conservatively allowed
passing an Active Record object.

Since the quoting methods on a `connection` are low-level API and
querying API does not rely on that ability, so people should pass casted
value instead of an Active Record object if using the quoting methods
directly.
---
 activerecord/CHANGELOG.md                     |  4 ++++
 .../connection_adapters/abstract/quoting.rb   | 20 +++++++++++++------
 activerecord/test/cases/quoting_test.rb       | 10 ++++++++--
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index b095ddfb96..a669bcc494 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Deprecate passing an Active Record object to `quote`/`type_cast` directly.
+
+    *Ryuta Kamizono*
+
 *   Default engine `ENGINE=InnoDB` is no longer dumped to make schema more agnostic.
 
     Before:
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
index 45cbc4baed..d70f517342 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
@@ -9,7 +9,13 @@ module ActiveRecord
       # Quotes the column value to help prevent
       # {SQL injection attacks}[https://en.wikipedia.org/wiki/SQL_injection].
       def quote(value)
-        value = id_value_for_database(value) if value.is_a?(Base)
+        if value.is_a?(Base)
+          ActiveSupport::Deprecation.warn(<<~MSG)
+            Passing an Active Record object to `quote` directly is deprecated
+            and will be no longer quoted as id value in Rails 6.2.
+          MSG
+          value = value.id_for_database
+        end
 
         _quote(value)
       end
@@ -18,7 +24,13 @@ module ActiveRecord
       # SQLite does not understand dates, so this method will convert a Date
       # to a String.
       def type_cast(value, column = nil)
-        value = id_value_for_database(value) if value.is_a?(Base)
+        if value.is_a?(Base)
+          ActiveSupport::Deprecation.warn(<<~MSG)
+            Passing an Active Record object to `type_cast` directly is deprecated
+            and will be no longer type casted as id value in Rails 6.2.
+          MSG
+          value = value.id_for_database
+        end
 
         if column
           ActiveSupport::Deprecation.warn(<<~MSG)
@@ -202,10 +214,6 @@ module ActiveRecord
           type_map.lookup(sql_type)
         end
 
-        def id_value_for_database(value)
-          value.id_for_database
-        end
-
         def _quote(value)
           case value
           when String, Symbol, ActiveSupport::Multibyte::Chars
diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb
index 4e3f0ab848..2ff9fc1e36 100644
--- a/activerecord/test/cases/quoting_test.rb
+++ b/activerecord/test/cases/quoting_test.rb
@@ -284,12 +284,18 @@ module ActiveRecord
 
         def test_quote_ar_object
           value = DatetimePrimaryKey.new(id: @time)
-          assert_equal "'2017-02-14 12:34:56.789000'",  @connection.quote(value)
+          expected = "'2017-02-14 12:34:56.789000'"
+          assert_deprecated do
+            assert_equal expected, @connection.quote(value)
+          end
         end
 
         def test_type_cast_ar_object
           value = DatetimePrimaryKey.new(id: @time)
-          assert_equal @connection.type_cast(value.id),  @connection.type_cast(value)
+          expected = @connection.type_cast(value.id)
+          assert_deprecated do
+            assert_equal expected, @connection.type_cast(value)
+          end
         end
       end
     end