Merge pull request rails#52438 from Shopify/fix-delegate-to-missing

Fix `delegate_missing_to allow_nil: true` when called with implict self

Author: byroot

activesupport/lib/active_support/delegation.rb

@@ -151,37 +151,51 @@ def generate(owner, methods, location: nil, to: nil, prefix: nil, allow_nil: nil
 
       def generate_method_missing(owner, target, allow_nil: nil)
         target = target.to_s
-        target = "self.#{target}" if RESERVED_METHOD_NAMES.include?(target)
+        target = "self.#{target}" if RESERVED_METHOD_NAMES.include?(target) || target == "__target"
 
-        owner.module_eval <<-RUBY, __FILE__, __LINE__ + 1
-          def respond_to_missing?(name, include_private = false)
-            # It may look like an oversight, but we deliberately do not pass
-            # +include_private+, because they do not get delegated.
+        if allow_nil
+          owner.module_eval <<~RUBY, __FILE__, __LINE__ + 1
+            def respond_to_missing?(name, include_private = false)
+              # It may look like an oversight, but we deliberately do not pass
+              # +include_private+, because they do not get delegated.
 
-            return false if name == :marshal_dump || name == :_dump
-            #{target}.respond_to?(name) || super
-          end
+              return false if name == :marshal_dump || name == :_dump
+              #{target}.respond_to?(name) || super
+            end
 
-          def method_missing(method, ...)
-            if #{target}.respond_to?(method)
-              #{target}.public_send(method, ...)
-            else
-              begin
+            def method_missing(method, ...)
+              __target = #{target}
+              if __target.nil? && !nil.respond_to?(method)
+                nil
+              elsif __target.respond_to?(method)
+                __target.public_send(method, ...)
+              else
                 super
-              rescue NoMethodError
-                if #{target}.nil?
-                  if #{allow_nil == true}
-                    nil
-                  else
-                    raise ::ActiveSupport::DelegationError.nil_target(method, :'#{target}')
-                  end
-                else
-                  raise
-                end
               end
             end
-          end
-        RUBY
+          RUBY
+        else
+          owner.module_eval <<~RUBY, __FILE__, __LINE__ + 1
+            def respond_to_missing?(name, include_private = false)
+              # It may look like an oversight, but we deliberately do not pass
+              # +include_private+, because they do not get delegated.
+
+              return false if name == :marshal_dump || name == :_dump
+              #{target}.respond_to?(name) || super
+            end
+
+            def method_missing(method, ...)
+              __target = #{target}
+              if __target.nil? && !nil.respond_to?(method)
+                raise ::ActiveSupport::DelegationError.nil_target(method, :'#{target}')
+              elsif __target.respond_to?(method)
+                __target.public_send(method, ...)
+              else
+                super
+              end
+            end
+          RUBY
+        end
       end
     end
   end

activesupport/test/core_ext/module_test.rb

@@ -92,12 +92,20 @@ def respond_to_missing?(sym, priv = false)
 DecoratedTester = Struct.new(:client) do
   include ExtraMissing
 
+  def call_name
+    name
+  end
+
   delegate_missing_to :client
 end
 
 class DecoratedMissingAllowNil
   delegate_missing_to :case, allow_nil: true
 
+  def call_name
+    name
+  end
+
   attr_reader :case
 
   def initialize(kase)
@@ -413,6 +421,10 @@ def test_delegate_missing_to_with_method
     assert_equal "David", DecoratedTester.new(@david).name
   end
 
+  def test_delegate_missing_to_calling_on_self
+    assert_equal "David", DecoratedTester.new(@david).call_name
+  end
+
   def test_delegate_missing_to_with_reserved_methods
     assert_equal "David", DecoratedReserved.new(@david).name
   end
@@ -449,6 +461,10 @@ def test_delegate_missing_to_returns_nil_if_allow_nil_and_nil_target
     assert_nil DecoratedMissingAllowNil.new(nil).name
   end
 
+  def test_delegate_missing_with_allow_nil_when_called_on_self
+    assert_nil DecoratedMissingAllowNil.new(nil).call_name
+  end
+
   def test_delegate_missing_to_affects_respond_to
     assert_respond_to DecoratedTester.new(@david), :name
     assert_not_respond_to DecoratedTester.new(@david), :private_name