Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improved error message for "wrong number of arguments" #5491

Merged
merged 2 commits into from
Dec 3, 2018
Merged

improved error message for "wrong number of arguments" #5491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83d33f8
improved error message for "wrong number of arguments"
ahorek Dec 2, 2018
f18a5ab
untag passing spec
ahorek Dec 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions core/src/main/java/org/jruby/Ruby.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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,11 +3612,31 @@ 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) {
return newRaiseException(getArgumentError(), str(this, "wrong number of arguments calling `", ids(this, name), ("` (" + got + " for " + expected + ")")));
return newArgumentError(name, got, expected, expected);
}

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

public RaiseException newErrnoEBADFError() {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/RubyFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
28 changes: 14 additions & 14 deletions core/src/main/java/org/jruby/runtime/Arity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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,43 +251,42 @@ 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
public static void raiseArgumentError(Ruby runtime, int length, int min, int max, boolean hasKwargs) {
if (length < min) throw runtime.newArgumentError(length, min);
if (max > -1 && length > max) {
if (length < min) throw runtime.newArgumentError(length, min, max);
if (max > UNLIMITED_ARGUMENTS && length > max) {
if (hasKwargs && length == max + 1) {
// 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);
}
}

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

// FIXME: JRuby 2/next should change this name since it only sometimes raises an error
public static void raiseArgumentError(Ruby runtime, String name, int length, int min, int max, boolean hasKwargs) {
if (length < min) throw runtime.newArgumentError(name, length, min);
if (max > -1 && length > max) {
if (length < min) throw runtime.newArgumentError(name, length, min, max);
if (max > UNLIMITED_ARGUMENTS && length > max) {
if (hasKwargs && length == max + 1) {
// we have an extra arg, but kwargs active; let it fall through to assignment
return;
}
throw runtime.newArgumentError(name, length, max);
throw runtime.newArgumentError(name, length, min, max);
}
}

Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/org/jruby/runtime/Helpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion test/jruby/test_higher_javasupport.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1844,7 +1844,7 @@ def test_raised_errors_on_array_proxy
fail 'expected to raise'
rescue ArgumentError => e
msg = e.message
assert msg.start_with?("wrong number of arguments calling `length` (1 for 0)"), msg
assert msg.start_with?("wrong number of arguments calling `length` (given 1, expected 0)"), msg
end

begin # array proxy class
Expand Down