Skip to content

Commit

Permalink
improved error message for "wrong number of arguments"
Browse files Browse the repository at this point in the history
  • Loading branch information
@ahorek
ahorek committed Dec 3, 2018
1 parent b004861 commit 602878a
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 33 deletions.
13 changes: 12 additions & 1 deletion core/src/main/java/org/jruby/Ruby.java
View file
Expand Up @@ -212,6 +212,7 @@
import static org.jruby.util.RubyStringBuilder.str;
import static org.jruby.util.RubyStringBuilder.ids;
import static org.jruby.util.RubyStringBuilder.types;
import static org.jruby.runtime.Arity.UNLIMITED_ARGUMENTS;

/**
* The Ruby object represents the top-level of a JRuby "instance" in a given VM.
Expand Down Expand Up @@ -3611,7 +3612,17 @@ public RaiseException newArgumentError(String message) {
}

public RaiseException newArgumentError(int got, int expected) {
return newRaiseException(getArgumentError(), "wrong number of arguments (" + got + " for " + expected + ")");
return newArgumentError(got, expected, expected);
}

public RaiseException newArgumentError(int got, int min, int max) {
if (min == max) {
return newRaiseException(getArgumentError(), "wrong number of arguments (given " + got + ", expected " + min + ")");
} else if (max == UNLIMITED_ARGUMENTS) {
return newRaiseException(getArgumentError(), "wrong number of arguments (given " + got + ", expected " + min + "+)");
} else {
return newRaiseException(getArgumentError(), "wrong number of arguments (given " + got + ", expected " + min + ".." + max + ")");
}
}

public RaiseException newArgumentError(String name, int got, int expected) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/RubyFile.java
View file
Expand Up @@ -1427,7 +1427,7 @@ protected IRubyObject openFile(ThreadContext context, IRubyObject args[]) {
if (!args[3].isNil()) {
options = TypeConverter.convertToTypeWithCheck(context, args[3], context.runtime.getHash(), sites(context).to_hash_checked);
if (options.isNil()) {
throw runtime.newArgumentError("wrong number of arguments (4 for 1..3)");
throw runtime.newArgumentError(4, 1, 3);
}
}
vperm(pm, args[2]);
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/org/jruby/RubyStruct.java
View file
Expand Up @@ -371,7 +371,7 @@ public IRubyObject initialize(ThreadContext context, IRubyObject[] args) {
IRubyObject keywordInit = RubyStruct.getInternalVariable(classOf(), KEYWORD_INIT_VAR);

if (keywordInit.isTrue()) {
if (args.length != 1) throw context.runtime.newArgumentError("wrong number of arguments (given " + args.length + ", expected 0)");
if (args.length != 1) throw context.runtime.newArgumentError(args.length, 0);

return initialize(context, args[0]);
} else {
Expand Down Expand Up @@ -412,7 +412,7 @@ public IRubyObject initialize(ThreadContext context, IRubyObject arg0) {
if (keywordInit.isTrue()) {
IRubyObject maybeKwargs = ArgsUtil.getOptionsArg(runtime, arg0);

if (maybeKwargs.isNil()) throw context.runtime.newArgumentError("wrong number of arguments (given 1, expected 0)");
if (maybeKwargs.isNil()) throw context.runtime.newArgumentError(1, 0);

setupStructValuesFromHash(context, (RubyHash) maybeKwargs);

Expand All @@ -427,7 +427,7 @@ public IRubyObject initialize(ThreadContext context, IRubyObject arg0) {
public IRubyObject initialize(ThreadContext context, IRubyObject arg0, IRubyObject arg1) {
IRubyObject keywordInit = RubyStruct.getInternalVariable(classOf(), KEYWORD_INIT_VAR);
if (keywordInit.isTrue()) {
throw context.runtime.newArgumentError("wrong number of arguments (given 2, expected 0)");
throw context.runtime.newArgumentError(2, 0);
}

return initializeInternal(context, 2, arg0, arg1, context.nil);
Expand All @@ -437,7 +437,7 @@ public IRubyObject initialize(ThreadContext context, IRubyObject arg0, IRubyObje
public IRubyObject initialize(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2) {
IRubyObject keywordInit = RubyStruct.getInternalVariable(classOf(), KEYWORD_INIT_VAR);
if (keywordInit.isTrue()) {
throw context.runtime.newArgumentError("wrong number of arguments (given 3, expected 0)");
throw context.runtime.newArgumentError(3, 0);
}

return initializeInternal(context, 3, arg0, arg1, arg2);
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
View file
Expand Up @@ -86,6 +86,7 @@
import static org.jruby.runtime.Block.Type.LAMBDA;
import static org.jruby.util.RubyStringBuilder.str;
import static org.jruby.util.RubyStringBuilder.ids;
import static org.jruby.runtime.Arity.UNLIMITED_ARGUMENTS;

public class IRRuntimeHelpers {
private static final Logger LOG = LoggerFactory.getLogger(IRRuntimeHelpers.class);
Expand Down Expand Up @@ -535,7 +536,7 @@ public static void checkArity(ThreadContext context, StaticScope scope, Object[]
if (keywordArgs != null) argsLength -= 1;

if ((block == null || block.type.checkArity) && (argsLength < required || (!rest && argsLength > (required + opt)))) {
Arity.raiseArgumentError(context.runtime, argsLength, required, required + opt);
Arity.raiseArgumentError(context.runtime, argsLength, required, rest ? UNLIMITED_ARGUMENTS : (required + opt));
}
}

Expand Down
13 changes: 6 additions & 7 deletions core/src/main/java/org/jruby/runtime/Arity.java
View file
Expand Up @@ -55,6 +55,7 @@ public final class Arity implements Serializable {
public final static Arity ONE_REQUIRED = newArity(-2);
public final static Arity TWO_REQUIRED = newArity(-3);
public final static Arity THREE_REQUIRED = newArity(-4);
public final static int UNLIMITED_ARGUMENTS = -1;

private Arity(int value) {
this.value = value;
Expand Down Expand Up @@ -163,11 +164,11 @@ public void checkArity(Ruby runtime, IRubyObject[] args) {
public void checkArity(Ruby runtime, int length) {
if (isFixed()) {
if (length != required()) {
throw runtime.newArgumentError("wrong number of arguments (" + length + " for " + required() + ")");
throw runtime.newArgumentError(length, required());
}
} else {
if (length < required()) {
throw runtime.newArgumentError("wrong number of arguments (" + length + " for " + required() + ")");
throw runtime.newArgumentError(length, required());
}
}
}
Expand Down Expand Up @@ -250,14 +251,12 @@ public static void raiseArgumentError(Ruby runtime, IRubyObject[] args, int min,

// FIXME: JRuby 2/next should change this name since it only sometimes raises an error
public static void raiseArgumentError(Ruby runtime, int length, int min, int max) {
if (length < min) throw runtime.newArgumentError(length, min);
if (max > -1 && length > max) throw runtime.newArgumentError(length, max);
if (length < min || (max > UNLIMITED_ARGUMENTS && length > max)) throw runtime.newArgumentError(length, min, max);
}

// FIXME: JRuby 2/next should change this name since it only sometimes raises an error
public static void raiseArgumentError(ThreadContext context, int length, int min, int max) {
if (length < min) throw context.runtime.newArgumentError(length, min);
if (max > -1 && length > max) throw context.runtime.newArgumentError(length, max);
raiseArgumentError(context.runtime, length, min, max);
}

// FIXME: JRuby 2/next should change this name since it only sometimes raises an error
Expand All @@ -268,7 +267,7 @@ public static void raiseArgumentError(Ruby runtime, int length, int min, int max
// we have an extra arg, but kwargs active; let it fall through to assignment
return;
}
throw runtime.newArgumentError(length, max);
throw runtime.newArgumentError(length, min, max);
}
}

Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/org/jruby/runtime/Helpers.java
View file
Expand Up @@ -612,26 +612,26 @@ public static void handleArgumentSizes(ThreadContext context, Ruby runtime, int
if (rest < 0) {
// no opt, no rest, exact match
if (given != required) {
throw runtime.newArgumentError("wrong number of arguments (" + given + " for " + required + ")");
throw runtime.newArgumentError(given, required);
}
} else {
// only rest, must be at least required
if (given < required) {
throw runtime.newArgumentError("wrong number of arguments (" + given + " for " + required + ")");
throw runtime.newArgumentError(given, required);
}
}
} else {
if (rest < 0) {
// opt but no rest, must be at least required and no more than required + opt
if (given < required) {
throw runtime.newArgumentError("wrong number of arguments (" + given + " for " + required + ")");
throw runtime.newArgumentError(given, required);
} else if (given > (required + opt)) {
throw runtime.newArgumentError("wrong number of arguments (" + given + " for " + (required + opt) + ")");
throw runtime.newArgumentError(given, required + opt);
}
} else {
// opt and rest, must be at least required
if (given < required) {
throw runtime.newArgumentError("wrong number of arguments (" + given + " for " + required + ")");
throw runtime.newArgumentError(given, required);
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions core/src/main/java/org/jruby/runtime/Signature.java
View file
Expand Up @@ -14,6 +14,8 @@
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.util.TypeConverter;

import static org.jruby.runtime.Arity.UNLIMITED_ARGUMENTS;

/**
* A representation of a Ruby method signature (argument layout, min/max, keyword layout, rest args).
*/
Expand Down Expand Up @@ -269,18 +271,18 @@ public String toString() {

public void checkArity(Ruby runtime, IRubyObject[] args) {
if (args.length < required()) {
throw runtime.newArgumentError("wrong number of arguments (" + args.length + " for " + required() + ")");
throw runtime.newArgumentError(args.length, required(), hasRest() ? UNLIMITED_ARGUMENTS : (required() + opt));
}
if (rest == Rest.NONE || rest == Rest.ANON) {
// no rest, so we have a maximum
if (args.length > required() + opt()) {
if (hasKwargs() && !TypeConverter.checkHashType(runtime, args[args.length - 1]).isNil()) {
// we have kwargs and a potential kwargs hash, check with length - 1
if (args.length - 1 > required() + opt()) {
throw runtime.newArgumentError("wrong number of arguments (" + args.length + " for " + (required() + opt) + ")");
throw runtime.newArgumentError(args.length, required(), hasRest() ? UNLIMITED_ARGUMENTS : (required() + opt));
}
} else {
throw runtime.newArgumentError("wrong number of arguments (" + args.length + " for " + (required() + opt) + ")");
throw runtime.newArgumentError(args.length, required(), hasRest() ? UNLIMITED_ARGUMENTS : (required() + opt));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/ruby/jruby/kernel/enumerable.rb
View file
@@ -1,6 +1,6 @@
module Enumerable
def slice_before(filter = (no_filter = true; nil), &block)
raise ArgumentError.new("wrong number of arguments (0 for 1)") if (no_filter && !block) || (!no_filter && block)
raise ArgumentError.new("wrong number of arguments (given 0, expected 1)") if (no_filter && !block) || (!no_filter && block)

state = nil

Expand Down Expand Up @@ -33,7 +33,7 @@ def slice_before(filter = (no_filter = true; nil), &block)
end

def slice_after(filter = (no_filter = true; nil), &block)
raise ArgumentError.new("wrong number of arguments (0 for 1)") if no_filter && !block
raise ArgumentError.new("wrong number of arguments (given 0, expected 1)") if no_filter && !block
raise ArgumentError.new("cannot pass both filter argument and block") if !no_filter && block

state = nil
Expand Down
4 changes: 2 additions & 2 deletions core/src/test/java/org/jruby/javasupport/TestJava.java
View file
Expand Up @@ -112,10 +112,10 @@ public void testJavaConstructorExceptionHandling() throws Exception {
assert false;
}
catch (RaiseException ex) {
assertEquals("(ArgumentError) wrong number of arguments (0 for 1)", ex.getMessage());
assertEquals("(ArgumentError) wrong number of arguments (given 0, expected 1)", ex.getMessage());
assertNull(ex.getCause());
assertNotNull(ex.getException());
assertEquals("wrong number of arguments (0 for 1)", ex.getException().getMessage().toString());
assertEquals("wrong number of arguments (given 0, expected 1)", ex.getException().getMessage().toString());
}

try {
Expand Down
7 changes: 0 additions & 7 deletions spec/tags/ruby/language/def_tags.txt
View file
@@ -1,9 +1,2 @@
fails:A nested method definition creates a method in the surrounding context when evaluated in a def expr.method
fails:A method definition always resets the visibility to public for nested definitions at the toplevel
fails:An instance method raises an error with too few arguments
fails:An instance method raises an error with too many arguments
fails:An instance method definition with a splat requires the presence of any arguments that precede the *
fails:An instance method with a default argument evaluates the default when required arguments precede it
fails:An instance method with a default argument prefers to assign to a default argument before a splat argument
fails:a method definition that sets more than one default parameter all to the same value only allows overriding the default value of the first such parameter in each set
fails:a method definition that sets more than one default parameter all to the same value treats the argument after the multi-parameter normally

0 comments on commit 602878a

Please sign in to comment.