From 8bba95f293714283f6fc30cb213af54811cccff3 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 18 Jul 2011 16:06:21 +0100 Subject: Just do the method call directly in Module#delegate, if we can (we cannot for method names ending in '='). Two reasons: 1) it's faster, see https://gist.github.com/1089783 and 2) more importantly, delegate should not allow you to accidentally call private or protected methods. --- activesupport/test/core_ext/module_test.rb | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'activesupport/test/core_ext/module_test.rb') diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index a95cf1591f..074a3412d4 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -26,10 +26,20 @@ module Yz end end -Somewhere = Struct.new(:street, :city) +Somewhere = Struct.new(:street, :city) do + protected + + def protected_method + end + + private + + def private_method + end +end Someone = Struct.new(:name, :place) do - delegate :street, :city, :to_f, :to => :place + delegate :street, :city, :to_f, :protected_method, :private_method, :to => :place delegate :upcase, :to => "place.city" end @@ -69,6 +79,14 @@ class ModuleTest < Test::Unit::TestCase assert_equal "Chicago", @david.city end + def test_delegation_to_protected_method + assert_raise(NoMethodError) { @david.protected_method } + end + + def test_delegation_to_private_method + assert_raise(NoMethodError) { @david.private_method } + end + def test_delegation_down_hierarchy assert_equal "CHICAGO", @david.upcase end -- cgit v1.2.3 From 63d100ea35a7fabea25c37f654177c3828fc1dcb Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 15 Aug 2011 12:50:57 +0100 Subject: Fix the line number in the backtrace when Module#delegate raises --- activesupport/test/core_ext/module_test.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'activesupport/test/core_ext/module_test.rb') diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index 074a3412d4..c33ade8381 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -38,9 +38,12 @@ Somewhere = Struct.new(:street, :city) do end end -Someone = Struct.new(:name, :place) do +class Someone < Struct.new(:name, :place) delegate :street, :city, :to_f, :protected_method, :private_method, :to => :place delegate :upcase, :to => "place.city" + + FAILED_DELEGATE_LINE = __LINE__ + 1 + delegate :foo, :to => :place end Invoice = Struct.new(:client) do @@ -182,6 +185,15 @@ class ModuleTest < Test::Unit::TestCase end end + def test_delegation_exception_backtrace + someone = Someone.new("foo", "bar") + someone.foo + rescue NoMethodError => e + file_and_line = "#{__FILE__}:#{Someone::FAILED_DELEGATE_LINE}" + assert e.backtrace.first.include?(file_and_line), + "[#{e.backtrace.first}] did not include [#{file_and_line}]" + end + def test_parent assert_equal Yz::Zy, Yz::Zy::Cd.parent assert_equal Yz, Yz::Zy.parent -- cgit v1.2.3 From 27da0c5480ecf6b020e73f994d3240ae15b0646b Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 15 Aug 2011 13:56:04 +0100 Subject: Split up the definitions in Module#delegate depending on :allow_nil, and don't use exceptions for flow control in the :allow_nil => true case. --- activesupport/test/core_ext/module_test.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'activesupport/test/core_ext/module_test.rb') diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index c33ade8381..a24f013d4f 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -44,6 +44,9 @@ class Someone < Struct.new(:name, :place) FAILED_DELEGATE_LINE = __LINE__ + 1 delegate :foo, :to => :place + + FAILED_DELEGATE_LINE_2 = __LINE__ + 1 + delegate :bar, :to => :place, :allow_nil => true end Invoice = Struct.new(:client) do @@ -194,6 +197,15 @@ class ModuleTest < Test::Unit::TestCase "[#{e.backtrace.first}] did not include [#{file_and_line}]" end + def test_delegation_exception_backtrace_with_allow_nil + someone = Someone.new("foo", "bar") + someone.bar + rescue NoMethodError => e + file_and_line = "#{__FILE__}:#{Someone::FAILED_DELEGATE_LINE_2}" + assert e.backtrace.first.include?(file_and_line), + "[#{e.backtrace.first}] did not include [#{file_and_line}]" + end + def test_parent assert_equal Yz::Zy, Yz::Zy::Cd.parent assert_equal Yz, Yz::Zy.parent -- cgit v1.2.3 From 2e2f3f5a469cb441e52fb161647ea5fd27d98d81 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 15 Aug 2011 16:07:49 +0100 Subject: Add a test for delegating a method ending in '=' as this is a special case. --- activesupport/test/core_ext/module_test.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'activesupport/test/core_ext/module_test.rb') diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index a24f013d4f..d4ce81fdfa 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -27,6 +27,8 @@ module Yz end Somewhere = Struct.new(:street, :city) do + attr_accessor :name + protected def protected_method @@ -40,6 +42,7 @@ end class Someone < Struct.new(:name, :place) delegate :street, :city, :to_f, :protected_method, :private_method, :to => :place + delegate :name=, :to => :place, :prefix => true delegate :upcase, :to => "place.city" FAILED_DELEGATE_LINE = __LINE__ + 1 @@ -85,6 +88,11 @@ class ModuleTest < Test::Unit::TestCase assert_equal "Chicago", @david.city end + def test_delegation_to_assignment_method + @david.place_name = "Fred" + assert_equal "Fred", @david.place.name + end + def test_delegation_to_protected_method assert_raise(NoMethodError) { @david.protected_method } end -- cgit v1.2.3 From 8ba491acc31bf08cf63a83ea0a3c314c52cd020f Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Thu, 25 Aug 2011 18:51:17 +0100 Subject: Revert all the stuff to do with disallowing non-public methods for Module#delegate --- activesupport/test/core_ext/module_test.rb | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) (limited to 'activesupport/test/core_ext/module_test.rb') diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index d4ce81fdfa..449d3810e2 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -28,20 +28,10 @@ end Somewhere = Struct.new(:street, :city) do attr_accessor :name - - protected - - def protected_method - end - - private - - def private_method - end end class Someone < Struct.new(:name, :place) - delegate :street, :city, :to_f, :protected_method, :private_method, :to => :place + delegate :street, :city, :to_f, :to => :place delegate :name=, :to => :place, :prefix => true delegate :upcase, :to => "place.city" @@ -93,14 +83,6 @@ class ModuleTest < Test::Unit::TestCase assert_equal "Fred", @david.place.name end - def test_delegation_to_protected_method - assert_raise(NoMethodError) { @david.protected_method } - end - - def test_delegation_to_private_method - assert_raise(NoMethodError) { @david.private_method } - end - def test_delegation_down_hierarchy assert_equal "CHICAGO", @david.upcase end -- cgit v1.2.3 From fc5e3ff1ba0d361b7091585094d0eca68b821108 Mon Sep 17 00:00:00 2001 From: Arun Agrawal Date: Sun, 4 Sep 2011 13:18:57 +0530 Subject: We can't simply check the first line of the backtrace, because JRuby reports the call to __send__ in the backtrace. --- activesupport/test/core_ext/module_test.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'activesupport/test/core_ext/module_test.rb') diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index 449d3810e2..f11bf3dc69 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -183,8 +183,9 @@ class ModuleTest < Test::Unit::TestCase someone.foo rescue NoMethodError => e file_and_line = "#{__FILE__}:#{Someone::FAILED_DELEGATE_LINE}" - assert e.backtrace.first.include?(file_and_line), - "[#{e.backtrace.first}] did not include [#{file_and_line}]" + # We can't simply check the first line of the backtrace, because JRuby reports the call to __send__ in the backtrace. + assert e.backtrace.any?{|a| a.include?(file_and_line)}, + "[#{e.backtrace.inspect}] did not include [#{file_and_line}]" end def test_delegation_exception_backtrace_with_allow_nil @@ -192,8 +193,9 @@ class ModuleTest < Test::Unit::TestCase someone.bar rescue NoMethodError => e file_and_line = "#{__FILE__}:#{Someone::FAILED_DELEGATE_LINE_2}" - assert e.backtrace.first.include?(file_and_line), - "[#{e.backtrace.first}] did not include [#{file_and_line}]" + # We can't simply check the first line of the backtrace, because JRuby reports the call to __send__ in the backtrace. + assert e.backtrace.any?{|a| a.include?(file_and_line)}, + "[#{e.backtrace.inspect}] did not include [#{file_and_line}]" end def test_parent -- cgit v1.2.3