From 8afdfdcfc92720162920aa6b59954ff4a1bfe831 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Tue, 16 Apr 2019 16:43:08 +0900
Subject: Refactor `ActiveSupport::Deprecation.deprecate_methods` not to expose
 internal methods

In #33325, `deprecate_methods` is replaced from `prepend` to completely
emurated `alias_method_chain`, it exposed two internal methods
`xxx_with_deprecation` and `xxx_without_deprecation`.

After that, #34648 restored the `prepend` implementation, which doesn't
expose any internal methods, so we no longer be able to ensure to always
expose that internal methods.

As I said at https://github.com/rails/rails/pull/33325#issuecomment-409016725,
I think that internal methods exposed is not a specification but a
limitation when using `alias_method_chain`, there is no longer a reason
to follow that limitation.
---
 .../active_support/deprecation/method_wrappers.rb  | 25 ++++++----------------
 .../test/deprecation/method_wrappers_test.rb       |  8 -------
 2 files changed, 7 insertions(+), 26 deletions(-)

(limited to 'activesupport')

diff --git a/activesupport/lib/active_support/deprecation/method_wrappers.rb b/activesupport/lib/active_support/deprecation/method_wrappers.rb
index d99571790f..7c0a54a1d0 100644
--- a/activesupport/lib/active_support/deprecation/method_wrappers.rb
+++ b/activesupport/lib/active_support/deprecation/method_wrappers.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "active_support/core_ext/array/extract_options"
+require "active_support/core_ext/module/redefine_method"
 
 module ActiveSupport
   class Deprecation
@@ -52,29 +53,17 @@ module ActiveSupport
         options = method_names.extract_options!
         deprecator = options.delete(:deprecator) || self
         method_names += options.keys
-        mod = Module.new
+        mod = nil
 
         method_names.each do |method_name|
           if target_module.method_defined?(method_name) || target_module.private_method_defined?(method_name)
-            aliased_method, punctuation = method_name.to_s.sub(/([?!=])$/, ""), $1
-            with_method = "#{aliased_method}_with_deprecation#{punctuation}"
-            without_method = "#{aliased_method}_without_deprecation#{punctuation}"
-
-            target_module.define_method(with_method) do |*args, &block|
+            method = target_module.instance_method(method_name)
+            target_module.redefine_method(method_name) do |*args, &block|
               deprecator.deprecation_warning(method_name, options[method_name])
-              send(without_method, *args, &block)
-            end
-
-            target_module.alias_method(without_method, method_name)
-            target_module.alias_method(method_name, with_method)
-
-            case
-            when target_module.protected_method_defined?(without_method)
-              target_module.send(:protected, method_name)
-            when target_module.private_method_defined?(without_method)
-              target_module.send(:private, method_name)
+              method.bind(self).call(*args, &block)
             end
           else
+            mod ||= Module.new
             mod.define_method(method_name) do |*args, &block|
               deprecator.deprecation_warning(method_name, options[method_name])
               super(*args, &block)
@@ -82,7 +71,7 @@ module ActiveSupport
           end
         end
 
-        target_module.prepend(mod) unless mod.instance_methods(false).empty?
+        target_module.prepend(mod) if mod
       end
     end
   end
diff --git a/activesupport/test/deprecation/method_wrappers_test.rb b/activesupport/test/deprecation/method_wrappers_test.rb
index 18729941bc..0aa3233aab 100644
--- a/activesupport/test/deprecation/method_wrappers_test.rb
+++ b/activesupport/test/deprecation/method_wrappers_test.rb
@@ -89,12 +89,4 @@ class MethodWrappersTest < ActiveSupport::TestCase
     warning = /old_method is deprecated and will be removed from Rails \d.\d \(use new_method instead\)/
     assert_deprecated(warning) { assert_equal "abc", @klass.old_method }
   end
-
-  def test_method_with_without_deprecation_is_exposed
-    ActiveSupport::Deprecation.deprecate_methods(@klass, old_method: :new_method)
-
-    warning = /old_method is deprecated and will be removed from Rails \d.\d \(use new_method instead\)/
-    assert_deprecated(warning) { assert_equal "abc", @klass.new.old_method_with_deprecation }
-    assert_equal "abc", @klass.new.old_method_without_deprecation
-  end
 end
-- 
cgit v1.2.3