diff options
author | Andrew White <andyw@pixeltrix.co.uk> | 2013-07-11 08:56:47 +0100 |
---|---|---|
committer | Andrew White <andyw@pixeltrix.co.uk> | 2013-07-11 09:05:32 +0100 |
commit | 07a4c76a07641840892463eb934400abdf694927 (patch) | |
tree | 5b8ac35d3fb08f76e87274aedb3a3d637ce3b347 /activesupport | |
parent | e7e81b4580f46c3d5229fdef3f0749802a9206da (diff) | |
download | rails-07a4c76a07641840892463eb934400abdf694927.tar.gz rails-07a4c76a07641840892463eb934400abdf694927.tar.bz2 rails-07a4c76a07641840892463eb934400abdf694927.zip |
Only raise DelegationError if it's is the source of the exception
This fixes situations where nested NoMethodError exceptions are masked
by delegations. This would cause confusion especially where there was a
problem in the Rails booting process because of a delegation in the
routes reloading code.
Fixes #10559
Diffstat (limited to 'activesupport')
-rw-r--r-- | activesupport/CHANGELOG.md | 6 | ||||
-rw-r--r-- | activesupport/lib/active_support/core_ext/module/delegation.rb | 21 | ||||
-rw-r--r-- | activesupport/test/core_ext/module_test.rb | 27 |
3 files changed, 44 insertions, 10 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 8b0b7c8d70..c00cbaaa08 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,9 @@ +* Only raise `Module::DelegationError` if it's the source of the exception. + + Fixes #10559 + + *Andrew White* + * Make `Time.at_with_coercion` retain the second fraction and return local time. Fixes #11350 diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index 1aa72da743..0318f9a568 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -183,16 +183,17 @@ class Module exception = %(raise DelegationError, "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}") module_eval(<<-EOS, file, line - 2) - def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block) - _ = #{to} # _ = client - _.#{method}(#{definition}) # _.name(*args, &block) - rescue NoMethodError # rescue NoMethodError - if _.nil? # if _.nil? - #{exception} # # add helpful message to the exception - else # else - raise # raise - end # end - end # end + def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block) + _ = #{to} # _ = client + _.#{method}(#{definition}) # _.name(*args, &block) + rescue NoMethodError => e # rescue NoMethodError => e + location = "%s:%d:in `%s'" % [__FILE__, __LINE__ - 2, '#{method_prefix}#{method}'] # location = "%s:%d:in `%s'" % [__FILE__, __LINE__ - 2, 'customer_name'] + if _.nil? && e.backtrace.first == location # if _.nil? && e.backtrace.first == location + #{exception} # # add helpful message to the exception + else # else + raise # raise + end # end + end # end EOS end end diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index ccf537e075..283b13ff8b 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -66,6 +66,23 @@ Tester = Struct.new(:client) do delegate :name, :to => :client, :prefix => false end +Product = Struct.new(:name) do + delegate :name, :to => :manufacturer, :prefix => true + delegate :name, :to => :type, :prefix => true + + def manufacturer + @manufacturer ||= begin + nil.unknown_method + end + end + + def type + @type ||= begin + nil.type_name + end + end +end + class ParameterSet delegate :[], :[]=, :to => :@params @@ -264,6 +281,16 @@ class ModuleTest < ActiveSupport::TestCase assert_equal [3], se.ints end + def test_delegation_doesnt_mask_nested_no_method_error_on_nil_receiver + product = Product.new('Widget') + + # Nested NoMethodError is a different name from the delegation + assert_raise(NoMethodError) { product.manufacturer_name } + + # Nested NoMethodError is the same name as the delegation + assert_raise(NoMethodError) { product.type_name } + end + def test_parent assert_equal Yz::Zy, Yz::Zy::Cd.parent assert_equal Yz, Yz::Zy.parent |