Use a special case delegation for writer methods

dasch · Oct 11, 2011 · 7cae32f · 7cae32f
1 parent cdf4634
commit 7cae32f
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 33 deletions.
diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb
@@ -127,40 +127,40 @@ def delegate(*methods)
         end
 
       if method.to_s =~ /[^]]=/
-        deprecation = <<-DEPRECATION
-          if args.length > 1
-            ActiveSupport::Deprecation.warn(
-              'Writer methods should only accept one argument. Support ' +
-              'for multiple arguments will be removed in the future.', caller)
-          end
-        DEPRECATION
-
-        definition = "args"
+        # The method is an attribute writer, so we can't do `method=(*args)`.
+        module_eval(<<-EOS, file, line - 2)
+          def #{prefix}#{method}(*args, &block)                                    # def customer_name(*args, &block)
+            if args.length > 1 || block_given?                                     #   if args.length > 1 || block_given?
+              ActiveSupport::Deprecation.warn(                                     #     ActiveSupport::Deprecation.warn(
+                'Writer methods should only accept one argument. Support ' +       #       'Writer methods should only accept one argument. Support ' +
+                'for multiple arguments will be removed in the future.', caller)   #       'for multiple arguments will be removed in the future.', caller)
+              #{to}.__send__(#{method.inspect}, *args, &block)                     #     client.__send__(:invoices=, *args, &block)
+            else                                                                   #   else
+              #{to}.#{method}(args.first)                                          #     client.invoices=(args.first)
+            end                                                                    #   end
+          end                                                                      # end
+        EOS
       else
-        deprecation = ""
-        definition = "*args, &block"
+        module_eval(<<-EOS, file, line - 1)
+          def #{prefix}#{method}(*args, &block)                                    # def customer_name(*args, &block)
+            #{to}.#{method}(*args, &block)                                         #   client.name(*args, &block)
+          rescue NoMethodError => e                                                # rescue NoMethodError => e
+            begin                                                                  #   begin
+              result = #{to}.__send__(#{method.inspect}, *args, &block)            #     result = client.__send__(:name, *args, &block)
+            rescue NoMethodError                                                   #   rescue NoMethodError
+              if #{to}.nil?                                                        #     if client.nil?
+                #{on_nil}                                                          #       return # depends on :allow_nil
+              else                                                                 #     else
+                raise(e)                                                           #       raise(e)
+              end                                                                  #     end
+            else                                                                   #   else
+              ActiveSupport::Deprecation.warn(                                     #     ActiveSupport::Deprecation.warn(
+                'Delegating to non-public methods is deprecated.', caller)         #       'Delegating to non-public methods is deprecated.', caller)
+              result                                                               #     result
+            end                                                                    #   end
+          end                                                                      # end
+        EOS
       end
-
-      module_eval(<<-EOS, file, line - 2)
-        def #{prefix}#{method}(#{definition})                                    # def customer_name(*args, &block)
-          #{deprecation}                                                         #
-          #{to}.#{method}(#{definition})                                         #   client.name(*args, &block)
-        rescue NoMethodError => e                                                # rescue NoMethodError => e
-          begin                                                                  #   begin
-            result = #{to}.__send__(#{method.inspect}, #{definition})            #     result = client.__send__(:name, *args, &block)
-          rescue NoMethodError                                                   #   rescue NoMethodError
-            if #{to}.nil?                                                        #     if client.nil?
-              #{on_nil}                                                          #       return # depends on :allow_nil
-            else                                                                 #     else
-              raise(e)                                                           #       raise(e)
-            end                                                                  #     end
-          else                                                                   #   else
-            ActiveSupport::Deprecation.warn(                                     #     ActiveSupport::Deprecation.warn(
-              'Delegating to non-public methods is deprecated.', caller)         #       'Delegating to non-public methods is deprecated.', caller)
-            result                                                               #     result
-          end                                                                    #   end
-        end                                                                      # end
-      EOS
     end
   end
 end
diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb
@@ -35,6 +35,9 @@ class Someone < Struct.new(:name, :place)
   FAILED_DELEGATE_LINE = __LINE__ + 1
   delegate :foo, :to => :place
 
+  def occupation=(a, b)
+  end
+
   private
   def something_private
     "PRIVATE"
@@ -62,7 +65,7 @@ def something_protected
 
 Tester = Struct.new(:client) do
   delegate :name, :to => :client, :prefix => false
-  delegate :something_private, :something_protected, :to => :client
+  delegate :something_private, :something_protected, :occupation=, :to => :client
 end
 
 class Name
@@ -113,6 +116,12 @@ def test_deprecates_delegation_to_protected_methods
     end
   end
 
+  def test_deprecates_delegation_to_attr_writer_with_multiple_args
+    assert_deprecated do
+      Tester.new(@david).send(:occupation=, 1, 2)
+    end
+  end
+
   def test_delegation_prefix
     invoice = Invoice.new(@david)
     assert_equal invoice.client_name, "David"