From e7274a8ec43b5b20e42842e730dbabae58d2e6a2 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Thu, 5 Sep 2019 12:25:14 -0700 Subject: Convert empty keyword hash to required positional argument and warn In general, we want to ignore empty keyword hashes. The only case where we want to allow them for backwards compatibility is when they are necessary to satify the final required positional argument. In that case, we want to not ignore them, but we do want to warn, as that will be going away in Ruby 3. This commit implements this support for regular methods and attr_writer methods. In order to allow send to forward arguments correctly, send no longer removes empty keyword hashes. It is the responsibility of the final method to remove the empty keyword hashes now. This change was necessary as otherwise send could remove the empty keyword hashes before the regular or attr_writer methods could move them to required positional arguments. For completeness, add tests for keyword handling regular methods calls. This makes rb_warn_keyword_to_last_hash non-static in vm_args.c so it can be reused in vm_insnhelper.c, and also moves declarations before statements in the rb_warn_* functions in vm_args.c. --- test/ruby/test_keyword.rb | 118 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 112 insertions(+), 6 deletions(-) (limited to 'test/ruby') diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index b337343420..81ac432979 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -177,6 +177,100 @@ class TestKeywordArguments < Test::Unit::TestCase assert_equal(["bar", 111111], f[str: "bar", num: 111111]) end + def test_regular_kwsplat + kw = {} + h = {:a=>1} + h2 = {'a'=>1} + h3 = {'a'=>1, :a=>1} + + c = Object.new + def c.m(*args) + args + end + assert_equal([], c.m(**{})) + assert_equal([], c.m(**kw)) + assert_equal([h], c.m(**h)) + assert_equal([h], c.m(a: 1)) + assert_equal([h2], c.m(**h2)) + assert_equal([h3], c.m(**h3)) + assert_equal([h3], c.m(a: 1, **h2)) + + c.singleton_class.remove_method(:m) + def c.m; end + assert_nil(c.m(**{})) + assert_nil(c.m(**kw)) + assert_raise(ArgumentError) { c.m(**h) } + assert_raise(ArgumentError) { c.m(a: 1) } + assert_raise(ArgumentError) { c.m(**h2) } + assert_raise(ArgumentError) { c.m(**h3) } + assert_raise(ArgumentError) { c.m(a: 1, **h2) } + + c.singleton_class.remove_method(:m) + def c.m(args) + args + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal(kw, c.m(**{})) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal(kw, c.m(**kw)) + end + assert_equal(h, c.m(**h)) + assert_equal(h, c.m(a: 1)) + assert_equal(h2, c.m(**h2)) + assert_equal(h3, c.m(**h3)) + assert_equal(h3, c.m(a: 1, **h2)) + + c.singleton_class.remove_method(:m) + def c.m(**args) + args + end + assert_equal(kw, c.m(**{})) + assert_equal(kw, c.m(**kw)) + assert_equal(h, c.m(**h)) + assert_equal(h, c.m(a: 1)) + assert_equal(h2, c.m(**h2)) + assert_equal(h3, c.m(a: 1, **h2)) + + c.singleton_class.remove_method(:m) + def c.m(arg, **args) + [arg, args] + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + c.m(**{}) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + c.m(**kw) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h, kw], c.m(**h)) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h, kw], c.m(a: 1)) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h2, kw], c.m(**h2)) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h3, kw], c.m(**h3)) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h3, kw], c.m(a: 1, **h2)) + end + + c.singleton_class.remove_method(:m) + def c.m(arg=1, **args) + [arg=1, args] + end + assert_equal([1, kw], c.m(**{})) + assert_equal([1, kw], c.m(**kw)) + assert_equal([1, h], c.m(**h)) + assert_equal([1, h], c.m(a: 1)) + assert_equal([1, h2], c.m(**h2)) + assert_equal([1, h3], c.m(**h3)) + assert_equal([1, h3], c.m(a: 1, **h2)) + end + def test_lambda_kwsplat_call kw = {} h = {:a=>1} @@ -459,8 +553,12 @@ class TestKeywordArguments < Test::Unit::TestCase def c.m(args) args end - assert_raise(ArgumentError) { c.send(:m, **{}) } - assert_raise(ArgumentError) { c.send(:m, **kw) } + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal(kw, c.send(:m, **{})) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal(kw, c.send(:m, **kw)) + end assert_equal(h, c.send(:m, **h)) assert_equal(h, c.send(:m, a: 1)) assert_equal(h2, c.send(:m, **h2)) @@ -482,8 +580,12 @@ class TestKeywordArguments < Test::Unit::TestCase def c.m(arg, **args) [arg, args] end - assert_raise(ArgumentError) { c.send(:m, **{}) } - assert_raise(ArgumentError) { c.send(:m, **kw) } + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + c.send(:m, **{}) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + c.send(:m, **kw) + end assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do assert_equal([h, kw], c.send(:m, **h)) end @@ -824,8 +926,12 @@ class TestKeywordArguments < Test::Unit::TestCase class << c attr_writer :m end - assert_raise(ArgumentError) { c.send(:m=, **{}) } - assert_raise(ArgumentError) { c.send(:m=, **kw) } + assert_warn(/The keyword argument for `m=' is passed as the last hash parameter/) do + c.send(:m=, **{}) + end + assert_warn(/The keyword argument for `m=' is passed as the last hash parameter/) do + c.send(:m=, **kw) + end assert_equal(h, c.send(:m=, **h)) assert_equal(h, c.send(:m=, a: 1)) assert_equal(h2, c.send(:m=, **h2)) -- cgit v1.2.3