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.

1 files changed, 112 insertions, 6 deletions

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))