ruby-changes:41309
From: nobu <ko1@a...>
Date: Wed, 30 Dec 2015 11:18:52 +0900 (JST)
Subject: [ruby-changes:41309] nobu:r53381 (trunk): Forwardable: Fix delegating to 'args' and 'block'
nobu 2015-12-30 11:18:44 +0900 (Wed, 30 Dec 2015) New Revision: 53381 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=53381 Log: Forwardable: Fix delegating to 'args' and 'block' * lib/forwardable.rb (def_instance_delegator) fix delegating to 'args' and 'block', clashing with local variables in generated methods. [ruby-core:72579] [Bug #11916] * lib/forwardable.rb (def_single_delegator): ditto. If you have a class that uses Forwardable to delegate a method to another object, and the method that returns the delegate object is called `args` or `block`, then Forwardable will fail to work. Here's a simple example: class ModelCreator extend Forwardable attr_reader :args def_delegator :args, :model_name def initialize(args) @args = args end end ModelCreator.new.model_name If you run the last line above, then you'll get: NoMethodError: undefined method `model_name' for []:Array This error occurs because `def_delegator` -- as it is written in Ruby -- uses metaprogramming to add methods to the class that will then delegate to the delegate object. So it's as if we had written: class ModelCreator extend Forwardable attr_reader :args def model_name(*args, &block) args.model_name(*args, &block) end def initialize(args) @args = args end end As you can see, `def_delegator` will not only forward the method call onto the delegate object, it will also forward any arguments provided as well. It is here that the bug arises: it splats all of the arguments into a variable which is called `args`, and because of how variable scope works in Ruby, it then attempts to call `model_name` on *this* variable and *not* our delegate object method. The fix is to call the delegate object method manually using `__send__`. (This assumes, of course, that the given receiver is, in fact, the name of a method and not the name of an instance variable, which is also a possibility.) We use `__send__` because the delegate object method could be private. So, that looks like this: def model_name(*args, &block) __send__(:args).model_name(*args, &block) end Because `def_delegators` and `delegate` use `def_delegator` internally, they also get this fix as well. Modified files: trunk/ChangeLog trunk/lib/forwardable.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 53380) +++ ChangeLog (revision 53381) @@ -1,3 +1,11 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Wed Dec 30 11:18:42 2015 Elliot Winkler <elliot.winkler@g...> + + * lib/forwardable.rb (def_instance_delegator) fix delegating to + 'args' and 'block', clashing with local variables in generated + methods. [ruby-core:72579] [Bug #11916] + + * lib/forwardable.rb (def_single_delegator): ditto. + Wed Dec 30 09:58:56 2015 Nobuyoshi Nakada <nobu@r...> * object.c (rb_class_inherited_p): search the corresponding Index: lib/forwardable.rb =================================================================== --- lib/forwardable.rb (revision 53380) +++ lib/forwardable.rb (revision 53381) @@ -178,6 +178,10 @@ module Forwardable https://github.com/ruby/ruby/blob/trunk/lib/forwardable.rb#L178 # q.push 23 #=> NoMethodError # def def_instance_delegator(accessor, method, ali = method) + if method_defined?(accessor) || private_method_defined?(accessor) + accessor = "#{accessor}()" + end + line_no = __LINE__; str = %{ def #{ali}(*args, &block) begin @@ -270,7 +274,11 @@ module SingleForwardable https://github.com/ruby/ruby/blob/trunk/lib/forwardable.rb#L274 # the method of the same name in _accessor_). If _new_name_ is # provided, it is used as the name for the delegate method. def def_single_delegator(accessor, method, ali = method) - str = %{ + if method_defined?(accessor) || private_method_defined?(accessor) + accessor = "#{accessor}()" + end + + line_no = __LINE__; str = %{ def #{ali}(*args, &block) begin #{accessor}.__send__(:#{method}, *args, &block) @@ -281,7 +289,7 @@ module SingleForwardable https://github.com/ruby/ruby/blob/trunk/lib/forwardable.rb#L289 end } - instance_eval(str, __FILE__, __LINE__) + instance_eval(str, __FILE__, line_no) end alias delegate single_delegate -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/