Good find, thank you for the effort! We will review and get this merged in.

Might be a candidate for 9.2.14, @kares @enebo?

The JI spec failures can be ignored due to #6452.

The test is great, but since this is a standard Ruby behavior it would be better as a Ruby spec. Can you migrate the test into spec/ruby (under core/method or language perhaps) so other Ruby impls will benefit from it?

For sure! I've opened a PR to merge a spec that tests the same thing in ruby/spec: ruby/spec#811

What should I do with this PR while I wait for the ruby/spec one to get approved? Should I remove the test from this commit or will we merge this as is and remove the duplication later when JRuby syncs up with ruby/spec?

See ruby/spec#811 (comment), I believe Method#to_proc is always strict and does not auto-splat. It seems specific behavior of Hash#{each/each_pair/map} rather, originating from an optimization not intentionally changing semantics. Note in Ruby 3 Hash#{each/each_pair} don't exhibit this strange behavior anymore

Thanks @eregon! You're correct. I will try to come up with a better fix

@marshalium I will wait to hear back from you about the correct behavior and fix for this.

These special splatting behaviors have always been a mystery, and have gone back and forth over the years. In this case I'm getting that it's the combination of a proc-ified Method fed to Hash#each and friends resulting in unexpected behavior... where we are strictly enforcing the arity and not splatting the incoming argument array, CRuby passes two arguments explicitly rather than two arguments as an argument array.

Needless to say these exceptions are confusing and frustrating, and I'm glad this will be more normalized for 3.0.

@marshalium My five minute patch may be along the right lines:

diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java
index 40f7170f1b..59b6f4ec55 100644
--- a/core/src/main/java/org/jruby/RubyHash.java
+++ b/core/src/main/java/org/jruby/RubyHash.java
@@ -1513,7 +1513,11 @@ public class RubyHash extends RubyObject implements Map {
     private static final VisitorWithState<Block> YieldKeyValueArrayVisitor = new VisitorWithState<Block>() {
         @Override
         public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, Block block) {
-            block.yield(context, context.runtime.newArray(key, value));
+            if (block.getSignature().arityValue() == 2) {
+                block.yieldSpecific(context, key, value);
+            } else {
+                block.yield(context, context.runtime.newArray(key, value));
+            }
         }
     };
 

I have not confirmed whether this breaks anything but from discussions it sounds like this is close to the proper behavior.

Thx! I kept the removal of the unnecessary needsSplat argument and have pushed up the arity check so we can see what CI says. It seems right because Hash is a special case and so should be doing that check explicitly but it still feels like I'm not understanding something about how the JRuby code works.

Ah, here's the thing I don't understand. It's the opposite bug where lambdas are too lenient. And it's why this looked like a Method#to_proc issue and not a general arity enforcing issue.

On Ruby 2.5.8 you don't get auto-splatting when yielding two args to a lambda outside of special cases like Hash#each:

marshall@lappy7 jruby % RBENV_VERSION=2.5.8 irb
irb(main):001:0> RUBY_DESCRIPTION
=> "ruby 2.5.8p224 (2020-03-31 revision 67882) [x86_64-darwin19]"
irb(main):002:0> m = lambda { |x, y| [x, y] }
=> #<Proc:0x00007fe8d98917b8@(irb):2 (lambda)>
irb(main):003:0> [[1,2],[3,4]].each(&m)
Traceback (most recent call last):
        4: from /Users/marshall/.rbenv/versions/2.5.8/bin/irb:11:in `<main>'
        3: from (irb):3
        2: from (irb):3:in `each'
        1: from (irb):2:in `block in irb_binding'
ArgumentError (wrong number of arguments (given 1, expected 2))

In JRuby it works without error:

marshall@lappy7 jruby % RBENV_VERSION=jruby-9.2.13.0 irb
irb(main):001:0> RUBY_DESCRIPTION
=> "jruby 9.2.13.0 (2.5.7) 2020-08-03 9a89c94bcc OpenJDK 64-Bit Server VM 11.0.8+11 on 11.0.8+11 +jit [darwin-x86_64]"
irb(main):002:0>  m = lambda { |x, y| [x, y] }
=> #<Proc:0x16c3ca31@(irb):2 (lambda)>
irb(main):003:0> [[1,2],[3,4]].each(&m)
=> [[1, 2], [3, 4]]

And what's weird is that calling the lambda directly doesn't have this magic behavior:

marshall@lappy7 jruby % RBENV_VERSION=jruby-9.2.13.0 irb
irb(main):001:0> RUBY_DESCRIPTION
=> "jruby 9.2.13.0 (2.5.7) 2020-08-03 9a89c94bcc OpenJDK 64-Bit Server VM 11.0.8+11 on 11.0.8+11 +jit [darwin-x86_64]"
irb(main):002:0> m = lambda { |x, y| [x, y] }
=> #<Proc:0x53dfacba@(irb):2 (lambda)>
irb(main):003:0> m.call(1, 2)
=> [1, 2]
irb(main):004:0> m.call([1,2])
Traceback (most recent call last):
        6: from /Users/marshall/.rbenv/versions/jruby-9.2.13.0/bin/irb:13:in `<main>'
        5: from org/jruby/RubyKernel.java:1189:in `catch'
        4: from org/jruby/RubyKernel.java:1189:in `catch'
        3: from org/jruby/RubyKernel.java:1442:in `loop'
        2: from org/jruby/RubyKernel.java:1048:in `eval'
        1: from (irb):4:in `evaluate'
ArgumentError (wrong number of arguments (given 1, expected 2))

So this makes me more confident that the arity checking was just always missing and it wasn't very noticeable because this other lambda arity checking bug is here.

EDIT: I think this is the same bug: #6165

The behavior of a lambda fed to each is likely a bug in how we unwrap the lambda to use as a block. It is losing its "lambda-ness" and being treated as a normal proc. You are probably right that it is the same issue as #6165. Don't worry about that case here, since I think it is a different cause and already a known issue.

@marshalium The only failure in your latest push appears to be a JDK-downloading glitch which I have restarted. I am glad to see so many tags go away! We will review and get this merged soon.