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.

2 files changed, 34 insertions, 17 deletions

diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb
index 654e3a01c6..fe17359a24 100644
--- a/activesupport/lib/active_support/core_ext/module/delegation.rb
+++ b/activesupport/lib/active_support/core_ext/module/delegation.rb
@@ -126,24 +126,29 @@ class Module
       method = method.to_s
       call   = (method[-1..-1] == '=') ? "public_send(:#{method}, " : "#{method}("
 
-      on_nil =
-        if allow_nil
-          'return'
-        else
-          %(raise "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}")
-        end
+      if allow_nil
+        module_eval(<<-EOS, file, line - 2)
+          def #{method_prefix}#{method}(*args, &block)        # def customer_name(*args, &block)
+            if #{to} || #{to}.respond_to?(:#{method})         #   if client || client.respond_to?(:name)
+              #{to}.#{call}*args, &block)                     #     client.name(*args, &block)
+            end                                               #   end
+          end                                                 # end
+        EOS
+      else
+        exception = %(raise "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}")
 
-      module_eval(<<-EOS, file, line - 1)
-        def #{method_prefix}#{method}(*args, &block)        # def customer_name(*args, &block)
-          #{to}.#{call}*args, &block)                       #   client.name(*args, &block)
-        rescue NoMethodError                                # rescue NoMethodError
-          if #{to}.nil?                                     #   if client.nil?
-            #{on_nil}                                       #     return # depends on :allow_nil
-          else                                              #   else
-            raise                                           #     raise
-          end                                               #   end
-        end                                                 # end
-      EOS
+        module_eval(<<-EOS, file, line - 1)
+          def #{method_prefix}#{method}(*args, &block)        # def customer_name(*args, &block)
+            #{to}.#{call}*args, &block)                       #   client.name(*args, &block)
+          rescue NoMethodError                                # rescue NoMethodError
+            if #{to}.nil?                                     #   if client.nil?
+              #{exception}                                    #     # add helpful message to the exception
+            else                                              #   else
+              raise                                           #     raise
+            end                                               #   end
+          end                                                 # end
+        EOS
+      end
     end
   end
 end
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