Add Module#ruby2_keywords for passing keywords through regular argument splats

This approach uses a flag bit on the final hash object in the regular splat,
as opposed to a previous approach that used a VM frame flag.  The hash flag
approach is less invasive, and handles some cases that the VM frame flag
approach does not, such as saving the argument splat array and splatting it
later:

  ruby2_keywords def foo(*args)
    @args = args
    bar
  end
  def bar
    baz(*@args)
  end
  def baz(*args, **kw)
    [args, kw]
  end
  foo(a:1)    #=> [[], {a: 1}]
  foo({a: 1}, **{}) #=> [[{a: 1}], {}]

  foo({a: 1}) #=> 2.7: [[], {a: 1}] # and warning
  foo({a: 1}) #=> 3.0: [[{a: 1}], {}]

It doesn't handle some cases that the VM frame flag handles, such as when
the final hash object is replaced using Hash#merge, but those cases are
probably less common and are unlikely to properly support keyword
argument separation.

Use ruby2_keywords to handle argument delegation in the delegate library.

2 files changed, 291 insertions, 0 deletions

diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb
index 1dbde80cd5..1cfa982f0c 100644
--- a/test/ruby/test_keyword.rb
+++ b/test/ruby/test_keyword.rb
@@ -2306,6 +2306,278 @@ class TestKeywordArguments < Test::Unit::TestCase
     assert_raise(ArgumentError) { m.call(42, a: 1, **h2) }
   end
 
+  def test_ruby2_keywords
+    c = Class.new do
+      ruby2_keywords def foo(meth, *args)
+        send(meth, *args)
+      end
+
+      ruby2_keywords def foo_bar(*args)
+        bar(*args)
+      end
+
+      ruby2_keywords def foo_baz(*args)
+        baz(*args)
+      end
+
+      ruby2_keywords def foo_mod(meth, *args)
+        args << 1
+        send(meth, *args)
+      end
+
+      ruby2_keywords def foo_bar_mod(*args)
+        args << 1
+        bar(*args)
+      end
+
+      ruby2_keywords def foo_baz_mod(*args)
+        args << 1
+        baz(*args)
+      end
+
+      def bar(*args, **kw)
+        [args, kw]
+      end
+
+      def baz(*args)
+        args
+      end
+
+      ruby2_keywords def foo_dbar(*args)
+        dbar(*args)
+      end
+
+      ruby2_keywords def foo_dbaz(*args)
+        dbaz(*args)
+      end
+
+      define_method(:dbar) do |*args, **kw|
+        [args, kw]
+      end
+
+      define_method(:dbaz) do |*args|
+        args
+      end
+
+      ruby2_keywords def block(*args)
+        ->(*args, **kw){[args, kw]}.(*args)
+      end
+
+      ruby2_keywords def cfunc(*args)
+        self.class.new(*args).init_args
+      end
+
+      ruby2_keywords def store_foo(meth, *args)
+        @stored_args = args
+        use(meth)
+      end
+      def use(meth)
+        send(meth, *@stored_args)
+      end
+
+      attr_reader :init_args
+      def initialize(*args, **kw)
+        @init_args = [args, kw]
+      end
+    end
+
+    mmkw = Class.new do
+      def method_missing(*args, **kw)
+        [args, kw]
+      end
+    end
+
+    mmnokw = Class.new do
+      def method_missing(*args)
+        args
+      end
+    end
+
+    implicit_super = Class.new(c) do
+      ruby2_keywords def bar(*args)
+        super
+      end
+
+      ruby2_keywords def baz(*args)
+        super
+      end
+    end
+
+    explicit_super = Class.new(c) do
+      ruby2_keywords def bar(*args)
+        super(*args)
+      end
+
+      ruby2_keywords def baz(*args)
+        super(*args)
+      end
+    end
+
+    h1 = {a: 1}
+    o = c.new
+
+    assert_equal([[1], h1], o.foo(:bar, 1, :a=>1))
+    assert_equal([1, h1], o.foo(:baz, 1, :a=>1))
+    assert_equal([[1], h1], o.store_foo(:bar, 1, :a=>1))
+    assert_equal([1, h1], o.store_foo(:baz, 1, :a=>1))
+    assert_equal([[1], h1], o.foo_bar(1, :a=>1))
+    assert_equal([1, h1], o.foo_baz(1, :a=>1))
+
+    assert_equal([[1], h1], o.foo(:bar, 1, **h1))
+    assert_equal([1, h1], o.foo(:baz, 1, **h1))
+    assert_equal([[1], h1], o.store_foo(:bar, 1, **h1))
+    assert_equal([1, h1], o.store_foo(:baz, 1, **h1))
+    assert_equal([[1], h1], o.foo_bar(1, **h1))
+    assert_equal([1, h1], o.foo_baz(1, **h1))
+
+    assert_equal([[h1], {}], o.foo(:bar, h1, **{}))
+    assert_equal([h1], o.foo(:baz, h1, **{}))
+    assert_equal([[h1], {}], o.store_foo(:bar, h1, **{}))
+    assert_equal([h1], o.store_foo(:baz, h1, **{}))
+    assert_equal([[h1], {}], o.foo_bar(h1, **{}))
+    assert_equal([h1], o.foo_baz(h1, **{}))
+
+    assert_warn(/The last argument is used as the keyword parameter.* for `bar'/m) do
+      assert_equal([[1], h1], o.foo(:bar, 1, h1))
+    end
+    assert_equal([1, h1], o.foo(:baz, 1, h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for `bar'/m) do
+      assert_equal([[1], h1], o.store_foo(:bar, 1, h1))
+    end
+    assert_equal([1, h1], o.store_foo(:baz, 1, h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for `bar'/m) do
+      assert_equal([[1], h1], o.foo_bar(1, h1))
+    end
+    assert_equal([1, h1], o.foo_baz(1, h1))
+
+    assert_equal([[1, h1, 1], {}], o.foo_mod(:bar, 1, :a=>1))
+    assert_equal([1, h1, 1], o.foo_mod(:baz, 1, :a=>1))
+    assert_equal([[1, h1, 1], {}], o.foo_bar_mod(1, :a=>1))
+    assert_equal([1, h1, 1], o.foo_baz_mod(1, :a=>1))
+
+    assert_equal([[1, h1, 1], {}], o.foo_mod(:bar, 1, **h1))
+    assert_equal([1, h1, 1], o.foo_mod(:baz, 1, **h1))
+    assert_equal([[1, h1, 1], {}], o.foo_bar_mod(1, **h1))
+    assert_equal([1, h1, 1], o.foo_baz_mod(1, **h1))
+
+    assert_equal([[h1, {}, 1], {}], o.foo_mod(:bar, h1, **{}))
+    assert_equal([h1, {}, 1], o.foo_mod(:baz, h1, **{}))
+    assert_equal([[h1, {}, 1], {}], o.foo_bar_mod(h1, **{}))
+    assert_equal([h1, {}, 1], o.foo_baz_mod(h1, **{}))
+
+    assert_equal([[1, h1, 1], {}], o.foo_mod(:bar, 1, h1))
+    assert_equal([1, h1, 1], o.foo_mod(:baz, 1, h1))
+    assert_equal([[1, h1, 1], {}], o.foo_bar_mod(1, h1))
+    assert_equal([1, h1, 1], o.foo_baz_mod(1, h1))
+
+    assert_equal([[1], h1], o.foo(:dbar, 1, :a=>1))
+    assert_equal([1, h1], o.foo(:dbaz, 1, :a=>1))
+    assert_equal([[1], h1], o.store_foo(:dbar, 1, :a=>1))
+    assert_equal([1, h1], o.store_foo(:dbaz, 1, :a=>1))
+    assert_equal([[1], h1], o.foo_dbar(1, :a=>1))
+    assert_equal([1, h1], o.foo_dbaz(1, :a=>1))
+
+    assert_equal([[1], h1], o.foo(:dbar, 1, **h1))
+    assert_equal([1, h1], o.foo(:dbaz, 1, **h1))
+    assert_equal([[1], h1], o.store_foo(:dbar, 1, **h1))
+    assert_equal([1, h1], o.store_foo(:dbaz, 1, **h1))
+    assert_equal([[1], h1], o.foo_dbar(1, **h1))
+    assert_equal([1, h1], o.foo_dbaz(1, **h1))
+
+    assert_equal([[h1], {}], o.foo(:dbar, h1, **{}))
+    assert_equal([h1], o.foo(:dbaz, h1, **{}))
+    assert_equal([[h1], {}], o.store_foo(:dbar, h1, **{}))
+    assert_equal([h1], o.store_foo(:dbaz, h1, **{}))
+    assert_equal([[h1], {}], o.foo_dbar(h1, **{}))
+    assert_equal([h1], o.foo_dbaz(h1, **{}))
+
+    assert_warn(/The last argument is used as the keyword parameter.* for method/m) do
+      assert_equal([[1], h1], o.foo(:dbar, 1, h1))
+    end
+    assert_equal([1, h1], o.foo(:dbaz, 1, h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for method/m) do
+      assert_equal([[1], h1], o.store_foo(:dbar, 1, h1))
+    end
+    assert_equal([1, h1], o.store_foo(:dbaz, 1, h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for method/m) do
+      assert_equal([[1], h1], o.foo_dbar(1, h1))
+    end
+    assert_equal([1, h1], o.foo_dbaz(1, h1))
+
+    assert_equal([[1], h1], o.block(1, :a=>1))
+    assert_equal([[1], h1], o.block(1, **h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for `call'/m) do
+      assert_equal([[1], h1], o.block(1, h1))
+    end
+    assert_equal([[h1], {}], o.block(h1, **{}))
+
+    assert_equal([[1], h1], o.cfunc(1, :a=>1))
+    assert_equal([[1], h1], o.cfunc(1, **h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for `initialize'/m) do
+      assert_equal([[1], h1], o.cfunc(1, h1))
+    end
+    assert_equal([[h1], {}], o.cfunc(h1, **{}))
+
+    o = mmkw.new
+    assert_equal([[:b, 1], h1], o.b(1, :a=>1))
+    assert_equal([[:b, 1], h1], o.b(1, **h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for `method_missing'/m) do
+      assert_equal([[:b, 1], h1], o.b(1, h1))
+    end
+    assert_equal([[:b, h1], {}], o.b(h1, **{}))
+
+    o = mmnokw.new
+    assert_equal([:b, 1, h1], o.b(1, :a=>1))
+    assert_equal([:b, 1, h1], o.b(1, **h1))
+    assert_equal([:b, 1, h1], o.b(1, h1))
+    assert_equal([:b, h1], o.b(h1, **{}))
+
+    o = implicit_super.new
+    assert_equal([[1], h1], o.bar(1, :a=>1))
+    assert_equal([[1], h1], o.bar(1, **h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for `bar'/m) do
+      assert_equal([[1], h1], o.bar(1, h1))
+    end
+    assert_equal([[h1], {}], o.bar(h1, **{}))
+
+    assert_equal([1, h1], o.baz(1, :a=>1))
+    assert_equal([1, h1], o.baz(1, **h1))
+    assert_equal([1, h1], o.baz(1, h1))
+    assert_equal([h1], o.baz(h1, **{}))
+
+    o = explicit_super.new
+    assert_equal([[1], h1], o.bar(1, :a=>1))
+    assert_equal([[1], h1], o.bar(1, **h1))
+    assert_warn(/The last argument is used as the keyword parameter.* for `bar'/m) do
+      assert_equal([[1], h1], o.bar(1, h1))
+    end
+    assert_equal([[h1], {}], o.bar(h1, **{}))
+
+    assert_equal([1, h1], o.baz(1, :a=>1))
+    assert_equal([1, h1], o.baz(1, **h1))
+    assert_equal([1, h1], o.baz(1, h1))
+    assert_equal([h1], o.baz(h1, **{}))
+
+    assert_warn(/Skipping set of ruby2_keywords flag for bar \(method not defined in Ruby, method accepts keywords, or method does not accept argument splat\)/) do
+      assert_nil(c.send(:ruby2_keywords, :bar))
+    end
+
+    sc = Class.new(c)
+    assert_warn(/Skipping set of ruby2_keywords flag for bar \(can only set in method defining module\)/) do
+      sc.send(:ruby2_keywords, :bar)
+    end
+    m = Module.new
+    assert_warn(/Skipping set of ruby2_keywords flag for system \(can only set in method defining module\)/) do
+      m.send(:ruby2_keywords, :system)
+    end
+
+    assert_raise(NameError) { c.send(:ruby2_keywords, "a5e36ccec4f5080a1d5e63f8") }
+    assert_raise(NameError) { c.send(:ruby2_keywords, :quux) }
+
+    c.freeze
+    assert_raise(FrozenError) { c.send(:ruby2_keywords, :baz) }
+  end
+
   def test_dig_kwsplat
     kw = {}
     h = {:a=>1}
diff --git a/test/test_delegate.rb b/test/test_delegate.rb
index 38e38ad781..9634681797 100644
--- a/test/test_delegate.rb
+++ b/test/test_delegate.rb
@@ -177,6 +177,25 @@ class TestDelegateClass < Test::Unit::TestCase
     assert_not_operator(s0, :eql?, "bar")
   end
 
+  def test_keyword_and_hash
+    foo = Object.new
+    def foo.bar(*args)
+      args
+    end
+    def foo.foo(*args, **kw)
+      [args, kw]
+    end
+    d = SimpleDelegator.new(foo)
+    assert_equal([[], {}], d.foo)
+    assert_equal([], d.bar)
+    assert_equal([[], {:a=>1}], d.foo(:a=>1))
+    assert_equal([{:a=>1}], d.bar(:a=>1))
+    assert_warn(/The last argument is used as the keyword parameter.* for `foo'/m) do
+      assert_equal([[], {:a=>1}], d.foo({:a=>1}))
+    end
+    assert_equal([{:a=>1}], d.bar({:a=>1}))
+  end
+
   class Foo
     private
     def delegate_test_private