Skip to content

Commit 8c3afef

Browse files
committed
Fix delegate_missing_to allow_nil: true when called with implict self
Fix: rails#52429 The previous implementation assumed `NoMethodError` would be raised when calling `super`, but that's not always true. If the receiver is an implicit self, the raised error will be `NameError`. It's better not to rely on exceptions for this anyways.
1 parent 077d6e8 commit 8c3afef

File tree

2 files changed

+55
-25
lines changed

2 files changed

+55
-25
lines changed

activesupport/lib/active_support/delegation.rb

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -151,37 +151,51 @@ def generate(owner, methods, location: nil, to: nil, prefix: nil, allow_nil: nil
151151

152152
def generate_method_missing(owner, target, allow_nil: nil)
153153
target = target.to_s
154-
target = "self.#{target}" if RESERVED_METHOD_NAMES.include?(target)
154+
target = "self.#{target}" if RESERVED_METHOD_NAMES.include?(target) || target == "__target"
155155

156-
owner.module_eval <<-RUBY, __FILE__, __LINE__ + 1
157-
def respond_to_missing?(name, include_private = false)
158-
# It may look like an oversight, but we deliberately do not pass
159-
# +include_private+, because they do not get delegated.
156+
if allow_nil
157+
owner.module_eval <<~RUBY, __FILE__, __LINE__ + 1
158+
def respond_to_missing?(name, include_private = false)
159+
# It may look like an oversight, but we deliberately do not pass
160+
# +include_private+, because they do not get delegated.
160161
161-
return false if name == :marshal_dump || name == :_dump
162-
#{target}.respond_to?(name) || super
163-
end
162+
return false if name == :marshal_dump || name == :_dump
163+
#{target}.respond_to?(name) || super
164+
end
164165
165-
def method_missing(method, ...)
166-
if #{target}.respond_to?(method)
167-
#{target}.public_send(method, ...)
168-
else
169-
begin
166+
def method_missing(method, ...)
167+
__target = #{target}
168+
if __target.nil? && !nil.respond_to?(method)
169+
nil
170+
elsif __target.respond_to?(method)
171+
__target.public_send(method, ...)
172+
else
170173
super
171-
rescue NoMethodError
172-
if #{target}.nil?
173-
if #{allow_nil == true}
174-
nil
175-
else
176-
raise ::ActiveSupport::DelegationError.nil_target(method, :'#{target}')
177-
end
178-
else
179-
raise
180-
end
181174
end
182175
end
183-
end
184-
RUBY
176+
RUBY
177+
else
178+
owner.module_eval <<~RUBY, __FILE__, __LINE__ + 1
179+
def respond_to_missing?(name, include_private = false)
180+
# It may look like an oversight, but we deliberately do not pass
181+
# +include_private+, because they do not get delegated.
182+
183+
return false if name == :marshal_dump || name == :_dump
184+
#{target}.respond_to?(name) || super
185+
end
186+
187+
def method_missing(method, ...)
188+
__target = #{target}
189+
if __target.nil? && !nil.respond_to?(method)
190+
raise ::ActiveSupport::DelegationError.nil_target(method, :'#{target}')
191+
elsif __target.respond_to?(method)
192+
__target.public_send(method, ...)
193+
else
194+
super
195+
end
196+
end
197+
RUBY
198+
end
185199
end
186200
end
187201
end

activesupport/test/core_ext/module_test.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,20 @@ def respond_to_missing?(sym, priv = false)
9292
DecoratedTester = Struct.new(:client) do
9393
include ExtraMissing
9494

95+
def call_name
96+
name
97+
end
98+
9599
delegate_missing_to :client
96100
end
97101

98102
class DecoratedMissingAllowNil
99103
delegate_missing_to :case, allow_nil: true
100104

105+
def call_name
106+
name
107+
end
108+
101109
attr_reader :case
102110

103111
def initialize(kase)
@@ -413,6 +421,10 @@ def test_delegate_missing_to_with_method
413421
assert_equal "David", DecoratedTester.new(@david).name
414422
end
415423

424+
def test_delegate_missing_to_calling_on_self
425+
assert_equal "David", DecoratedTester.new(@david).call_name
426+
end
427+
416428
def test_delegate_missing_to_with_reserved_methods
417429
assert_equal "David", DecoratedReserved.new(@david).name
418430
end
@@ -449,6 +461,10 @@ def test_delegate_missing_to_returns_nil_if_allow_nil_and_nil_target
449461
assert_nil DecoratedMissingAllowNil.new(nil).name
450462
end
451463

464+
def test_delegate_missing_with_allow_nil_when_called_on_self
465+
assert_nil DecoratedMissingAllowNil.new(nil).call_name
466+
end
467+
452468
def test_delegate_missing_to_affects_respond_to
453469
assert_respond_to DecoratedTester.new(@david), :name
454470
assert_not_respond_to DecoratedTester.new(@david), :private_name

0 commit comments

Comments
 (0)