Deprecate alias_method_chain in favour of Module#prepend

…as discussed #19413

7 files changed, 150 insertions, 108 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 43fb87f203..49088527ae 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Deprecate `alias_method_chain` in favour of `Module#prepend` introduced in Ruby 2.0
+
+    *Kir Shatrov*
+
 *   Added `#without` on `Enumerable` and `Array` to return a copy of an
     enumerable without the specified elements.
 
diff --git a/activesupport/lib/active_support/core_ext/module/aliasing.rb b/activesupport/lib/active_support/core_ext/module/aliasing.rb
index 0a6fadf928..25e138264e 100644
--- a/activesupport/lib/active_support/core_ext/module/aliasing.rb
+++ b/activesupport/lib/active_support/core_ext/module/aliasing.rb
@@ -21,6 +21,8 @@ class Module
   #
   # so you can safely chain foo, foo?, foo! and/or foo= with the same feature.
   def alias_method_chain(target, feature)
+    ActiveSupport::Deprecation.warn("alias_method_chain is deprecated. Please, use Module#prepend instead. From module, you can access the original method using super.")
+
     # Strip out punctuation on predicates, bang or writer methods since
     # e.g. target?_without_feature is not a valid method name.
     aliased_target, punctuation = target.to_s.sub(/([?!=])$/, ''), $1
diff --git a/activesupport/lib/active_support/core_ext/range/each.rb b/activesupport/lib/active_support/core_ext/range/each.rb
index ecef78f55f..f666480fe6 100644
--- a/activesupport/lib/active_support/core_ext/range/each.rb
+++ b/activesupport/lib/active_support/core_ext/range/each.rb
@@ -1,18 +1,22 @@
-require 'active_support/core_ext/module/aliasing'
-
 class Range #:nodoc:
 
   def each_with_time_with_zone(&block)
     ensure_iteration_allowed
     each_without_time_with_zone(&block)
   end
-  alias_method_chain :each, :time_with_zone
+  # TODO: change to Module#prepend as soon as the fix is backported to MRI 2.2:
+  # https://bugs.ruby-lang.org/issues/10847
+  alias_method :each_without_time_with_zone, :each
+  alias_method :each, :each_with_time_with_zone
 
   def step_with_time_with_zone(n = 1, &block)
     ensure_iteration_allowed
     step_without_time_with_zone(n, &block)
   end
-  alias_method_chain :step, :time_with_zone
+  # TODO: change to Module#prepend as soon as the fix is backported to MRI 2.2:
+  # https://bugs.ruby-lang.org/issues/10847
+  alias_method :step_without_time_with_zone, :step
+  alias_method :step, :step_with_time_with_zone
 
   private
   def ensure_iteration_allowed
diff --git a/activesupport/lib/active_support/core_ext/range/include_range.rb b/activesupport/lib/active_support/core_ext/range/include_range.rb
index 3a07401c8a..9d20920dd0 100644
--- a/activesupport/lib/active_support/core_ext/range/include_range.rb
+++ b/activesupport/lib/active_support/core_ext/range/include_range.rb
@@ -1,5 +1,3 @@
-require 'active_support/core_ext/module/aliasing'
-
 class Range
   # Extends the default Range#include? to support range comparisons.
   #  (1..5).include?(1..5) # => true
@@ -18,6 +16,8 @@ class Range
       include_without_range?(value)
     end
   end
-
-  alias_method_chain :include?, :range
+  # TODO: change to Module#prepend as soon as the fix is backported to MRI 2.2:
+  # https://bugs.ruby-lang.org/issues/10847
+  alias_method :include_without_range?, :include?
+  alias_method :include?, :include_with_range?
 end
diff --git a/activesupport/lib/active_support/deprecation/method_wrappers.rb b/activesupport/lib/active_support/deprecation/method_wrappers.rb
index cab8a1b14d..c74e9c40ac 100644
--- a/activesupport/lib/active_support/deprecation/method_wrappers.rb
+++ b/activesupport/lib/active_support/deprecation/method_wrappers.rb
@@ -31,12 +31,14 @@ module ActiveSupport
         method_names += options.keys
 
         method_names.each do |method_name|
-          target_module.alias_method_chain(method_name, :deprecation) do |target, punctuation|
-            target_module.send(:define_method, "#{target}_with_deprecation#{punctuation}") do |*args, &block|
+          mod = Module.new do
+            define_method(method_name) do |*args, &block|
               deprecator.deprecation_warning(method_name, options[method_name])
-              send(:"#{target}_without_deprecation#{punctuation}", *args, &block)
+              super(*args, &block)
             end
           end
+
+          target_module.prepend(mod)
         end
       end
     end
diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb
index c9c9b66a6c..bdfbadcf1d 100644
--- a/activesupport/test/core_ext/module_test.rb
+++ b/activesupport/test/core_ext/module_test.rb
@@ -358,148 +358,178 @@ class MethodAliasingTest < ActiveSupport::TestCase
     Object.instance_eval { remove_const :FooClassWithBarMethod }
   end
 
-  def test_alias_method_chain
-    assert @instance.respond_to?(:bar)
-    feature_aliases = [:bar_with_baz, :bar_without_baz]
+  def test_alias_method_chain_deprecated
+    assert_deprecated(/alias_method_chain/) do
+      Module.new do
+        def base
+        end
+
+        def base_with_deprecated
+        end
 
-    feature_aliases.each do |method|
-      assert !@instance.respond_to?(method)
+        alias_method_chain :base, :deprecated
+      end
     end
+  end
 
-    assert_equal 'bar', @instance.bar
+  def test_alias_method_chain
+    assert_deprecated(/alias_method_chain/) do
+      assert @instance.respond_to?(:bar)
+      feature_aliases = [:bar_with_baz, :bar_without_baz]
 
-    FooClassWithBarMethod.class_eval { include BarMethodAliaser }
+      feature_aliases.each do |method|
+        assert !@instance.respond_to?(method)
+      end
 
-    feature_aliases.each do |method|
-      assert_respond_to @instance, method
-    end
+      assert_equal 'bar', @instance.bar
+
+      FooClassWithBarMethod.class_eval { include BarMethodAliaser }
+
+      feature_aliases.each do |method|
+        assert_respond_to @instance, method
+      end
 
-    assert_equal 'bar_with_baz', @instance.bar
-    assert_equal 'bar', @instance.bar_without_baz
+      assert_equal 'bar_with_baz', @instance.bar
+      assert_equal 'bar', @instance.bar_without_baz
+    end
   end
 
   def test_alias_method_chain_with_punctuation_method
-    FooClassWithBarMethod.class_eval do
-      def quux!; 'quux' end
-    end
+    assert_deprecated(/alias_method_chain/) do
+      FooClassWithBarMethod.class_eval do
+        def quux!; 'quux' end
+      end
 
-    assert !@instance.respond_to?(:quux_with_baz!)
-    FooClassWithBarMethod.class_eval do
-      include BarMethodAliaser
-      alias_method_chain :quux!, :baz
-    end
-    assert_respond_to @instance, :quux_with_baz!
+      assert !@instance.respond_to?(:quux_with_baz!)
+      FooClassWithBarMethod.class_eval do
+        include BarMethodAliaser
+        alias_method_chain :quux!, :baz
+      end
+      assert_respond_to @instance, :quux_with_baz!
 
-    assert_equal 'quux_with_baz', @instance.quux!
-    assert_equal 'quux', @instance.quux_without_baz!
+      assert_equal 'quux_with_baz', @instance.quux!
+      assert_equal 'quux', @instance.quux_without_baz!
+    end
   end
 
   def test_alias_method_chain_with_same_names_between_predicates_and_bang_methods
-    FooClassWithBarMethod.class_eval do
-      def quux!; 'quux!' end
-      def quux?; true end
-      def quux=(v); 'quux=' end
-    end
+    assert_deprecated(/alias_method_chain/) do
+      FooClassWithBarMethod.class_eval do
+        def quux!; 'quux!' end
+        def quux?; true end
+        def quux=(v); 'quux=' end
+      end
 
-    assert !@instance.respond_to?(:quux_with_baz!)
-    assert !@instance.respond_to?(:quux_with_baz?)
-    assert !@instance.respond_to?(:quux_with_baz=)
+      assert !@instance.respond_to?(:quux_with_baz!)
+      assert !@instance.respond_to?(:quux_with_baz?)
+      assert !@instance.respond_to?(:quux_with_baz=)
 
-    FooClassWithBarMethod.class_eval { include BarMethodAliaser }
-    assert_respond_to @instance, :quux_with_baz!
-    assert_respond_to @instance, :quux_with_baz?
-    assert_respond_to @instance, :quux_with_baz=
+      FooClassWithBarMethod.class_eval { include BarMethodAliaser }
+      assert_respond_to @instance, :quux_with_baz!
+      assert_respond_to @instance, :quux_with_baz?
+      assert_respond_to @instance, :quux_with_baz=
 
 
-    FooClassWithBarMethod.alias_method_chain :quux!, :baz
-    assert_equal 'quux!_with_baz', @instance.quux!
-    assert_equal 'quux!', @instance.quux_without_baz!
+      FooClassWithBarMethod.alias_method_chain :quux!, :baz
+      assert_equal 'quux!_with_baz', @instance.quux!
+      assert_equal 'quux!', @instance.quux_without_baz!
 
-    FooClassWithBarMethod.alias_method_chain :quux?, :baz
-    assert_equal false, @instance.quux?
-    assert_equal true,  @instance.quux_without_baz?
+      FooClassWithBarMethod.alias_method_chain :quux?, :baz
+      assert_equal false, @instance.quux?
+      assert_equal true,  @instance.quux_without_baz?
 
-    FooClassWithBarMethod.alias_method_chain :quux=, :baz
-    assert_equal 'quux=_with_baz', @instance.send(:quux=, 1234)
-    assert_equal 'quux=', @instance.send(:quux_without_baz=, 1234)
+      FooClassWithBarMethod.alias_method_chain :quux=, :baz
+      assert_equal 'quux=_with_baz', @instance.send(:quux=, 1234)
+      assert_equal 'quux=', @instance.send(:quux_without_baz=, 1234)
+    end
   end
 
   def test_alias_method_chain_with_feature_punctuation
-    FooClassWithBarMethod.class_eval do
-      def quux; 'quux' end
-      def quux?; 'quux?' end
-      include BarMethodAliaser
-      alias_method_chain :quux, :baz!
-    end
+    assert_deprecated(/alias_method_chain/) do
+      FooClassWithBarMethod.class_eval do
+        def quux; 'quux' end
+        def quux?; 'quux?' end
+        include BarMethodAliaser
+        alias_method_chain :quux, :baz!
+      end
 
-    assert_nothing_raised do
-      assert_equal 'quux_with_baz', @instance.quux_with_baz!
-    end
+      assert_nothing_raised do
+        assert_equal 'quux_with_baz', @instance.quux_with_baz!
+      end
 
-    assert_raise(NameError) do
-      FooClassWithBarMethod.alias_method_chain :quux?, :baz!
+      assert_raise(NameError) do
+        FooClassWithBarMethod.alias_method_chain :quux?, :baz!
+      end
     end
   end
 
   def test_alias_method_chain_yields_target_and_punctuation
-    args = nil
+    assert_deprecated(/alias_method_chain/) do
+      args = nil
 
-    FooClassWithBarMethod.class_eval do
-      def quux?; end
-      include BarMethods
+      FooClassWithBarMethod.class_eval do
+        def quux?; end
+        include BarMethods
 
-      FooClassWithBarMethod.alias_method_chain :quux?, :baz do |target, punctuation|
-        args = [target, punctuation]
+        FooClassWithBarMethod.alias_method_chain :quux?, :baz do |target, punctuation|
+          args = [target, punctuation]
+        end
       end
-    end
 
-    assert_not_nil args
-    assert_equal 'quux', args[0]
-    assert_equal '?', args[1]
+      assert_not_nil args
+      assert_equal 'quux', args[0]
+      assert_equal '?', args[1]
+    end
   end
 
   def test_alias_method_chain_preserves_private_method_status
-    FooClassWithBarMethod.class_eval do
-      def duck; 'duck' end
-      include BarMethodAliaser
-      private :duck
-      alias_method_chain :duck, :orange
-    end
+    assert_deprecated(/alias_method_chain/) do
+      FooClassWithBarMethod.class_eval do
+        def duck; 'duck' end
+        include BarMethodAliaser
+        private :duck
+        alias_method_chain :duck, :orange
+      end
 
-    assert_raise NoMethodError do
-      @instance.duck
-    end
+      assert_raise NoMethodError do
+        @instance.duck
+      end
 
-    assert_equal 'duck_with_orange', @instance.instance_eval { duck }
-    assert FooClassWithBarMethod.private_method_defined?(:duck)
+      assert_equal 'duck_with_orange', @instance.instance_eval { duck }
+      assert FooClassWithBarMethod.private_method_defined?(:duck)
+    end
   end
 
   def test_alias_method_chain_preserves_protected_method_status
-    FooClassWithBarMethod.class_eval do
-      def duck; 'duck' end
-      include BarMethodAliaser
-      protected :duck
-      alias_method_chain :duck, :orange
-    end
+    assert_deprecated(/alias_method_chain/) do
+      FooClassWithBarMethod.class_eval do
+        def duck; 'duck' end
+        include BarMethodAliaser
+        protected :duck
+        alias_method_chain :duck, :orange
+      end
 
-    assert_raise NoMethodError do
-      @instance.duck
-    end
+      assert_raise NoMethodError do
+        @instance.duck
+      end
 
-    assert_equal 'duck_with_orange', @instance.instance_eval { duck }
-    assert FooClassWithBarMethod.protected_method_defined?(:duck)
+      assert_equal 'duck_with_orange', @instance.instance_eval { duck }
+      assert FooClassWithBarMethod.protected_method_defined?(:duck)
+    end
   end
 
   def test_alias_method_chain_preserves_public_method_status
-    FooClassWithBarMethod.class_eval do
-      def duck; 'duck' end
-      include BarMethodAliaser
-      public :duck
-      alias_method_chain :duck, :orange
-    end
+    assert_deprecated(/alias_method_chain/) do
+      FooClassWithBarMethod.class_eval do
+        def duck; 'duck' end
+        include BarMethodAliaser
+        public :duck
+        alias_method_chain :duck, :orange
+      end
 
-    assert_equal 'duck_with_orange', @instance.duck
-    assert FooClassWithBarMethod.public_method_defined?(:duck)
+      assert_equal 'duck_with_orange', @instance.duck
+      assert FooClassWithBarMethod.public_method_defined?(:duck)
+    end
   end
 
   def test_delegate_with_case
diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md
index 66626f41d1..f3d8e05089 100644
--- a/guides/source/active_support_core_extensions.md
+++ b/guides/source/active_support_core_extensions.md
@@ -506,6 +506,8 @@ Extensions to `Module`
 
 ### `alias_method_chain`
 
+**This method is deprecated in favour of using Module#prepend.**
+
 Using plain Ruby you can wrap methods with other methods, that's called _alias chaining_.
 
 For example, let's say you'd like params to be strings in functional tests, as they are in real requests, but still want the convenience of assigning integers and other kind of values. To accomplish that you could wrap `ActionController::TestCase#process` this way in `test/test_helper.rb`:
@@ -550,8 +552,6 @@ ActionController::TestCase.class_eval do
 end
 ```
 
-Rails uses `alias_method_chain` all over the code base. For example validations are added to `ActiveRecord::Base#save` by wrapping the method that way in a separate module specialized in validations.
-
 NOTE: Defined in `active_support/core_ext/module/aliasing.rb`.
 
 ### Attributes