Merge pull request #2733 from dasch/improve-delegate

Improve the performance of #delegate

3 files changed, 35 insertions, 10 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 8165b89cde..b6c3e91db8 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*    Make Module#delegate stop using `send` - can no longer delegate to private methods. *dasch*
+
 *    AS::Callbacks: deprecate `:rescuable` option. *Bogdan Gusiev*
 
 *    Adds Integer#ordinal to get the ordinal suffix string of an integer. *Tim Gildea*
diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb
index af92b869fd..ee8adae1cb 100644
--- a/activesupport/lib/active_support/core_ext/module/delegation.rb
+++ b/activesupport/lib/active_support/core_ext/module/delegation.rb
@@ -1,5 +1,5 @@
 class Module
-  # Provides a delegate class method to easily expose contained objects' methods
+  # Provides a delegate class method to easily expose contained objects' public methods
   # as your own. Pass one or more methods (specified as symbols or strings)
   # and the name of the target object via the <tt>:to</tt> option (also a symbol
   # or string). At least one method and the <tt>:to</tt> option are required.
@@ -124,23 +124,27 @@ class Module
     file, line = caller.first.split(':', 2)
     line = line.to_i
 
-    if allow_nil
-      methods.each do |method|
+    methods.each do |method|
+      method = method.to_s
+
+      # Attribute writer methods only accept one argument. Makes sure []=
+      # methods still accept two arguments.
+      definition = (method =~ /[^\]]=$/) ? "arg" : "*args, &block"
+
+      if allow_nil
         module_eval(<<-EOS, file, line - 2)
-          def #{method_prefix}#{method}(*args, &block)        # def customer_name(*args, &block)
+          def #{method_prefix}#{method}(#{definition})        # def customer_name(*args, &block)
             if #{to} || #{to}.respond_to?(:#{method})         #   if client || client.respond_to?(:name)
-              #{to}.__send__(:#{method}, *args, &block)       #     client.__send__(:name, *args, &block)
+              #{to}.#{method}(#{definition})                  #     client.name(*args, &block)
             end                                               #   end
           end                                                 # end
         EOS
-      end
-    else
-      methods.each do |method|
+      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}.__send__(:#{method}, *args, &block)         #   client.__send__(:name, *args, &block)
+          def #{method_prefix}#{method}(#{definition})        # def customer_name(*args, &block)
+            #{to}.#{method}(#{definition})                    #   client.name(*args, &block)
           rescue NoMethodError                                # rescue NoMethodError
             if #{to}.nil?                                     #   if client.nil?
               #{exception}                                    #     # add helpful message to the exception
diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb
index 09ca4e7296..6e1b3ca010 100644
--- a/activesupport/test/core_ext/module_test.rb
+++ b/activesupport/test/core_ext/module_test.rb
@@ -60,6 +60,14 @@ Tester = Struct.new(:client) do
   delegate :name, :to => :client, :prefix => false
 end
 
+class ParameterSet
+  delegate :[], :[]=, :to => :@params
+
+  def initialize
+    @params = {:foo => "bar"}
+  end
+end
+
 class Name
   delegate :upcase, :to => :@full_name
 
@@ -83,6 +91,17 @@ class ModuleTest < ActiveSupport::TestCase
     assert_equal "Fred", @david.place.name
   end
 
+  def test_delegation_to_index_get_method
+    @params = ParameterSet.new
+    assert_equal "bar", @params[:foo]
+  end
+
+  def test_delegation_to_index_set_method
+    @params = ParameterSet.new
+    @params[:foo] = "baz"
+    assert_equal "baz", @params[:foo]
+  end
+
   def test_delegation_down_hierarchy
     assert_equal "CHICAGO", @david.upcase
   end