From: "Martin J. Dürst" <duerst@...>
Date: 2012-03-14T17:47:42+09:00
Subject: [ruby-core:43272] Re: [ruby-trunk - Bug #6085] Treatment of Wrong Number of Arguments

While we are at it, can we also change the extremely cryptic "for". 
Whenever I see an error message of the form "wrong number of arguments 
(X for Y)". Is it X arguments given for Y arguments expected, or X 
arguments expected for Y arguments given?

If I look at the Rubinius example (e.g. "ArgumentError: method 'upcase': 
given 1, expected 0", I don't have to worry about the directionality, 
but then I could easily think that I used an argument *value* of 1 where 
it expected an argument *value* of 0.

So the best would be an error message along the following lines:

wrong number of arguments (given: X, expected: Y)

Regards,    Martin.


On 2012/03/13 20:38, Yusuke Endoh wrote:
> Hello,
>
>> On Mon, Mar 12, 2012 at 10:31 AM, Yui NARUSE<naruse@airemix.jp>  wrote:
>>> Use one of follwing:
>>> * https://github.com/marcandre/ruby/compare/rb_arity_check
>>> * https://github.com/marcandre/ruby/compare/rb_arity_check.diff
>>> * https://github.com/marcandre/ruby/compare/rb_arity_check.patch
>>
>> Nice, thanks! I'll provide this kind of link in the future, quite helpful.
>
> Cool, thanks.
>
>
> 2012/3/13, Marc-Andre Lafortune<ruby-core@marc-andre.ca>:
>> On Mon, Mar 12, 2012 at 8:07 AM, Yusuke Endoh<mame@tsg.ne.jp>  wrote:
>>> One concern: I'm afraid if this change affects people who parses
>>> the message string of WNA.  What do you think?  There is not such
>>> people, is there?  I don't want to be pedantic, but I can't feel
>>> sure because I can no longer use Google codesearch...  Google!!
>>
>> The error type is part of the language specs, but I feel like error messages
>> are not meant to be parsed and are subject to change. In this particular
>> case, I just checked and Rubinius gives different error messages
>> (ArgumentError: method 'upcase': given 1, expected 0).
>
> Sounds good.  At least, Rubinius community does not know any actual
> case where WNA message is parsed.
>
>
>> The changes I propose
>> are also minimal in their approach and make parsing even easier!
>
> You know, making parsing easy is not the purpose or the right way.
> My concern is just about compatiblity.
>
>
>>> Anyway, I agree that the current is awkward.  If no one complains,
>>> I'm positive to import it tentatively.
>>
>> Thanks. Just let me know after you've looked at it and I'll gladly commit
>> these.
>
> Looks good to me.
> It brings not only behavior consistency but also good refactoring
> effect.
>
> I noticed some minor issues below.
>
> vm_insnhelper.c:
>
>      +static inline VALUE
>      +rb_arg_error_new(int argc, int min, int max) {
>      +    const char *template = "wrong number of arguments (%d for %d..%d)";
>      +    if (min == max) {
>      +	template = "wrong number of arguments (%d for %d)";
>      +    }
>      +    else if (max == UNLIMITED_ARGUMENTS) {
>      +	template = "wrong number of arguments (%d for %d+)";
>      +    }
>      +    return rb_exc_new3(rb_eArgError, rb_sprintf(template, argc, min, max));
>      +}
>
> It would be good to match the number of %d and actual arguments.
>
>
> eval.c:
>
>      -    if (i<  argc) goto wrong_args;
>      +    if (i<  argc) rb_raise(rb_eArgError, "wrong arguments");
>
> I guess this line can be removed, though this is not your fault.
>
>
> test/ruby/test_arity.rb
>
>      assert_equal "0 for 1",     err_mess{ "".sub!{} }
>
> This assertion fails.  Did you mean "0 for 1..2" ?
>
>
>>> Off topic.  Are you interested in improving a keyword argument?
>>> There is some issues on its implementation, but I have no time to
>>> work on it :-(
>>
>> I'm not sure I have the technical skills needed, but I can definitely try to
>> help. In any case I wanted to work on checking for named arguments and
>> giving a better error message in those cases too. What else could I help on?
>
> So far, the remaining issues I know are better error message, and #5989.
>