Merge pull request #36037 from kamipo/deprecate_methods

Refactor `ActiveSupport::Deprecation.deprecate_methods` not to expose internal methods
rails · Apr 19, 2019 · 75aeb36 · 75aeb36
2 parents f656bb3 + 8afdfdc
commit 75aeb36
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 26 deletions.
diff --git 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,37 +53,25 @@ def deprecate_methods(target_module, *method_names)
         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)
             end
           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
@@ -89,12 +89,4 @@ def old_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.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