@JRubyMethod minimum argument not enforced in 9.4.3.0

[JasonLunn]

• JRuby version: jruby 9.4.3.0 (3.1.4) 2023-06-07 3086960792 OpenJDK 64-Bit Server VM 20.0.1+9 on 20.0.1+9 +jit [x86_64-darwin]
• Operating system and platform: Darwin jatl-macbookpro 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:22 PDT 2023; root:xnu-8796.121.3~7/RELEASE_X86_64 x86_64

Expected Behavior

Given a Ruby class defined in Java via @JRubyClass, expect the @JRubyMethod() annotation to enforce the minimum number of arguments passed to a method consistently with previous JRuby versions.

Actual Behavior

JRuby 9.2 and 9.3 are observed to block invocations of underlying annotated Java method if the Ruby invocation does not have sufficient arguments, whereas 9.4 is observed to pass a 0 length array to the Java function instead.

• Describe or show the actual behavior.

Foo.java:

import org.jruby.anno.JRubyClass;
import org.jruby.anno.JRubyMethod;
import org.jruby.Ruby;
import org.jruby.RubyClass;
import org.jruby.RubyModule;
import org.jruby.RubyObject;
import org.jruby.runtime.ObjectAllocator;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;

@JRubyClass(name = "Foo")
public class Foo extends RubyObject {
  public Foo(Ruby runtime, RubyClass klazz) {
    super(runtime, klazz);
  }

  @JRubyMethod(required = 1, optional = 2)
  public IRubyObject initialize(ThreadContext c, IRubyObject[] args) {
    System.out.println("Args has length " + args.length);
    return this;
  }

  public static void createBar(Ruby runtime) {
    RubyModule mFooBar = runtime.getClassFromPath("FuBar");
    RubyClass cBar =
        mFooBar.defineClassUnder(
            "Bar",
            runtime.getObject(),
            new ObjectAllocator() {
              @Override
              public IRubyObject allocate(Ruby runtime, RubyClass klazz) {
                return new Foo(runtime, klazz);
              }
            });
    cBar.defineAnnotatedMethods(Foo.class);
  }
}

BarService.java:

import java.io.IOException;
import org.jruby.Ruby;
import org.jruby.runtime.load.BasicLibraryService;
public class BarService implements BasicLibraryService {
  @Override
  public boolean basicLoad(Ruby ruby) throws IOException {
    ruby.defineModule("FuBar");
    Foo.createBar(ruby);
    return true;
  }
}

To reproduce under 9.4.3.0:

javac Foo.java BarService.java -cp /Users/jatl/.rvm//rubies/jruby-9.4.3.0/lib/jruby.jar; jar cf bar.jar Foo.class BarService.class ; rvm jruby-9.4.3.0 do ruby -v -e 'require "bar"; FuBar::Bar.new'

Output:

jruby 9.4.3.0 (3.1.4) 2023-06-07 3086960792 OpenJDK 64-Bit Server VM 20.0.1+9 on 20.0.1+9 +jit [x86_64-darwin]
Args has length 0

Contrast with behavior under 9.3.4.0:

javac Foo.java BarService.java -cp /Users/jatl/.rvm//rubies/jruby-9.4.3.0/lib/jruby.jar; jar cf bar.jar Foo.class BarService.class ; rvm jruby-9.3.4.0 do ruby -v -e 'require "bar"; FuBar::Bar.new'

Output:

jruby 9.3.4.0 (2.6.8) 2022-03-23 eff48c1ebf OpenJDK 64-Bit Server VM 20.0.1+9 on 20.0.1+9 +jit [x86_64-darwin]
ArgumentError: wrong number of arguments (given 0, expected 1..3)
     new at org/jruby/RubyClass.java:881
  <main> at -e:1

[JasonLunn]

@headius - 5565751 is the culprit commit according to git bisect.

[headius]

From Matrix chat:

@headius: There were two reasons for the original change. The first was to let the target method be at the top of the stack trace as in MRI... we have not matched that for years and it is confusing and has been reported repeatedly. The other was due to frequent bug reports about calls to Java-based Ruby methods that accept IRubyObject[] and never check arity... even in JRuby they would blindly access the array assuming the Ruby method binding had done the check. The latter is what broke this behavior; where the bindings would add arity-checking before, they do not now expecting that anything with variable arity should be doing its own checking

@headius: I agree somewhat this is maybe a big change to introduce in a minor release but it was to fix some long-standing behavior differences from MRI. It's also worth pointing out that MRI method bindings in C also require you to do your own arity checking, so this aligns with that behavior as well.

@headius: An idea: there could be an arity-checking field on the annotation that allows methods to opt out of automatic arity checking all the core JRuby methods would do it because they gained arity checks in the code, but any extensions that don't opt out would get automatic checking for varargs paths

This feature was implemented in #7860 and the default behavior has reverted to automatic arity checking by default. This feature may be removed in a future release, but as it stands right now it's fine to leave it automatic by default. My original goal was to get core JRuby methods to match MRI behavior and avoid doing extra arity checks where unnecessary.