Fix Kernel#warn override

[jeremyevans]

Description:

This makes the Kernel#warn override behave the same as the default
Kernel#warn. Previously, behavior could be different, because an
empty hash or empty keyword arguments would be swallowed by the
override, and the second to last argument could be treated as the
keyword argument by the default Kernel#warn.

For example:

warn({x:1}, {y:2}, {})

# C version outputs:
{:x=>1}
{:y=>2}

# Rubygems version raises:
unknown keyword: y (ArgumentError)

By using a keyword splat, and passing the keyword splat to the
default version, we can make behavior the same.

This issue was originally reported on the Ruby bug tracker:
https://bugs.ruby-lang.org/issues/16157#note-8

Tasks:

• Describe the problem / feature
• Write tests
• Write code to solve the problem
• Get code review from coworkers / friends

I will abide by the code of conduct.

[deivid-rodriguez]

Hi @jeremyevans! Thanks for the fix.

I added a second commit that should fix the failing test (coming from #2442 for context) and a regression test for your fix. What do you think?

[deivid-rodriguez]

mmmm.

Ruby-head build is failing 👉 https://travis-ci.org/rubygems/rubygems/jobs/584028540. That's using a ruby from a week ago (ruby 2.7.0dev (2019-09-05T13:16:21Z trunk 433c9c0) [x86_64-linux])

But it's passing for me locally against a more recent version (ruby 2.7.0dev (2019-09-09T12:27:40Z master 89c5d5a) [x86_64-linux]).

Has something change in the last week regarding this? 🤔.

[jeremyevans]

@deivid-rodriguez Thanks for working on the regression test, this looks good to me.

I think your approach when uplevel is provided is better. My approach had a bug where the new uplevel would be overridden by the originally provided one.

JRuby 9.2 treats an empty keyword hash as uplevel 0 instead of no uplevel, which I'm guessing is the reason that you aren't sending keywords on JRuby in the first call. That seems like a bug in JRuby, I'll report it upstream.

[deivid-rodriguez]

Nice @jeremyevans!

Yeah, there was one failing test on jruby, so I figured we should keep the implementation unchanged on jruby for now. Even if it's currently buggy, at least don't make it worse. I subscribed to your jruby issue so I remember to remove the conditional once they fix this.

What about the ruby-head failure? The regression test is failing precisely with the error we're supposed to be fixing here, so I'm confused 😕.

[jeremyevans]

ruby-head no longer sends an empty hash to C functions (or Ruby methods that do not accept keywords) if using an empty keyword splat when calling the method. rb_scan_args needs to be fixed so that it does not handle the last hash argument as keywords if an empty splat is provided. I started working on a patch for rb_scan_args yesterday, which I think will fix the issue.

[deivid-rodriguez]

I rebased this PR, precisely to check how it is doing against ruby-head (since it was passing for me locally against ruby 2.7.0dev (2019-09-09T12:27:40Z master 89c5d5a), and our CI no longer depends on TravisCI upgrading its ruby-head installation.

But CI doesn't seem to have been triggered.

@hbst Any ideas?

[hsbt]

Umm, We couldn't care the ruby-head version on Travis. If we hope to test the latest head version, We can use https://github.com/ruby/ruby-docker-images with GitHub Actions.

I try to switch it from RVM later.

[hsbt]

@bundlerbot r+

[ghost]

Build succeeded

• continuous-integration/travis-ci/push