Patch `Delegator` to work with `#try`

`Delegator` inherits from `BasicObject`, which means that it will not
have `Object#try` defined. It will then delegate the call to the
underlying object, which will not (necessarily) respond to the method
defined in the enclosing `Delegator`.

This patches `Delegator` with the `#try` method to work around the
surprising behaviour.

Fixes #5790

3 files changed, 127 insertions, 17 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 98954c7f73..619b7e6e01 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Patch `Delegator` to work with `#try`
+
+    Fixes #5790
+
+    *Nate Smith*
+
 *   Add `Integer#positive?` and `Integer#negative?` query methods
     in the vein of `Fixnum#zero?`.
 
diff --git a/activesupport/lib/active_support/core_ext/object/try.rb b/activesupport/lib/active_support/core_ext/object/try.rb
index e0f70b9caa..6605771d90 100644
--- a/activesupport/lib/active_support/core_ext/object/try.rb
+++ b/activesupport/lib/active_support/core_ext/object/try.rb
@@ -1,4 +1,36 @@
+require 'delegate'
+
+module ActiveSupport
+  module Try #:nodoc:
+    def try(*a, &b)
+      try!(*a, &b) if a.empty? || respond_to?(a.first)
+    end
+
+    def try!(*a, &b)
+      if a.empty? && block_given?
+        if b.arity.zero?
+          instance_eval(&b)
+        else
+          yield self
+        end
+      else
+        public_send(*a, &b)
+      end
+    end
+  end
+end
+
+[Object, Delegator].each do |klass|
+  klass.include(ActiveSupport::Try)
+end
+
 class Object
+  ##
+  # :method: try
+  #
+  # :call-seq:
+  #   try(*a, &b)
+  #
   # Invokes the public method whose name goes as first argument just like
   # +public_send+ does, except that if the receiver does not respond to it the
   # call returns +nil+ rather than raising an exception.
@@ -56,30 +88,38 @@ class Object
   #
   # Please also note that +try+ is defined on +Object+. Therefore, it won't work
   # with instances of classes that do not have +Object+ among their ancestors,
-  # like direct subclasses of +BasicObject+. For example, using +try+ with
-  # +SimpleDelegator+ will delegate +try+ to the target instead of calling it on
-  # the delegator itself.
-  def try(*a, &b)
-    try!(*a, &b) if a.empty? || respond_to?(a.first)
-  end
+  # like direct subclasses of +BasicObject+.
 
+  ##
+  # :method: try!
+  #
+  # :call-seq:
+  #   try!(*a, &b)
+  #
   # Same as #try, but raises a NoMethodError exception if the receiver is
   # not +nil+ and does not implement the tried method.
   #
   #   "a".try!(:upcase) # => "A"
   #   nil.try!(:upcase) # => nil
   #   123.try!(:upcase) # => NoMethodError: undefined method `upcase' for 123:Fixnum
-  def try!(*a, &b)
-    if a.empty? && block_given?
-      if b.arity.zero?
-        instance_eval(&b)
-      else
-        yield self
-      end
-    else
-      public_send(*a, &b)
-    end
-  end
+end
+
+class Delegator
+  ##
+  # :method: try
+  #
+  # :call-seq:
+  #   try(a*, &b)
+  #
+  # See Object#try
+
+  ##
+  # :method: try!
+  #
+  # :call-seq:
+  #   try!(a*, &b)
+  #
+  # See Object#try!
 end
 
 class NilClass
diff --git a/activesupport/test/core_ext/object/try_test.rb b/activesupport/test/core_ext/object/try_test.rb
index 89438675c1..cdb76af1ac 100644
--- a/activesupport/test/core_ext/object/try_test.rb
+++ b/activesupport/test/core_ext/object/try_test.rb
@@ -96,4 +96,68 @@ class ObjectTryTest < ActiveSupport::TestCase
 
     assert_nil klass.new.try(:private_method)
   end
+
+  class Decorator < SimpleDelegator
+    def delegator_method
+      'delegator method'
+    end
+
+    def reverse
+      'overridden reverse'
+    end
+
+    private
+
+    def private_delegator_method
+      'private delegator method'
+    end
+  end
+
+  def test_try_with_method_on_delegator
+    assert_equal 'delegator method', Decorator.new(@string).try(:delegator_method)
+  end
+
+  def test_try_with_method_on_delegator_target
+    assert_equal 5, Decorator.new(@string).size
+  end
+
+  def test_try_with_overriden_method_on_delegator
+    assert_equal 'overridden reverse', Decorator.new(@string).reverse
+  end
+
+  def test_try_with_private_method_on_delegator
+    assert_nil Decorator.new(@string).try(:private_delegator_method)
+  end
+
+  def test_try_with_private_method_on_delegator_bang
+    assert_raise(NoMethodError) do
+      Decorator.new(@string).try!(:private_delegator_method)
+    end
+  end
+
+  def test_try_with_private_method_on_delegator_target
+    klass = Class.new do
+      private
+
+      def private_method
+        'private method'
+      end
+    end
+
+    assert_nil Decorator.new(klass.new).try(:private_method)
+  end
+
+  def test_try_with_private_method_on_delegator_target_bang
+    klass = Class.new do
+      private
+
+      def private_method
+        'private method'
+      end
+    end
+
+    assert_raise(NoMethodError) do
+      Decorator.new(klass.new).try!(:private_method)
+    end
+  end
 end