Bug #16339
closedForwardable#def_delegator warns and is incorrect on trunk when passed keyword arguments
Description
Because it uses *args, &block
-style delegation.
require 'forwardable'
class C
extend Forwardable
def_delegator :self, :target, :delegate
def target(*args, **kwargs)
[args, kwargs]
end
end
p C.new.target(1, b: 2) # => [[1], {:b=>2}]
p C.new.delegate(1, b: 2)
# ruby-trunk/lib/ruby/2.7.0/forwardable.rb:231: warning: The last argument is used as the keyword parameter
# del.rb:6: warning: for `target' defined here
# => [[1], {:b=>2}]
p C.new.target({}, **{}) # => [[{}], {}]
p C.new.delegate({}, **{})
# ruby-trunk/lib/ruby/2.7.0/forwardable.rb:231: warning: The last argument is used as the keyword parameter
# del.rb:6: warning: for `target' defined here
# => [[], {}]
Which also illustrates we're missing important tests/specs for Forwardable.
Updated by jeremyevans0 (Jeremy Evans) over 4 years ago
forwardable
should be fixed to use ruby2_keywords
. I'm guessing the reason this wasn't done initially is because I worked on forwardable keyword argument support before I developed ruby2_keywords
.
Updated by Eregon (Benoit Daloze) over 4 years ago
2.6 behaves like:
p C.new.target({}, **{}) # => [[], {}]
p C.new.delegate({}, **{}) # => [[], {}]
So I'm not sure what's "correct" for that last case.
jeremyevans0 (Jeremy Evans) wrote:
forwardable
should be fixed to useruby2_keywords
.
Right, or ...
maybe.
I'm guessing the reason this wasn't done initially is because I worked on forwardable keyword argument support before I developed
ruby2_keywords
.
Didn't forwardable always support keyword arguments since it just did (*args, &block)
delegation?
Updated by jeremyevans0 (Jeremy Evans) over 4 years ago
Looks like the forwardable keyword argument support was never merged. My previous attempt, developed before ruby2_keywords
, added **kw
to method definition and method calls on 2.7+. I've updated the pull request to use ruby2_keywords
: https://github.com/ruby/forwardable/pull/5 . One failing CI test, but it looks unrelated. This should probably be merged soon, before preview3.
Updated by jeremyevans0 (Jeremy Evans) over 4 years ago
- Status changed from Open to Closed
Fixed by 9fa0166a580e72adf02562b7d60672c6c362d4b7.