From: Marc-Andre Lafortune <ruby-core@...>
Date: 2012-03-15T06:10:57+09:00
Subject: [ruby-core:43283] [ruby-trunk - Bug #6085] Treatment of Wrong Number of Arguments


Issue #6085 has been updated by Marc-Andre Lafortune.


Hi,

Yusuke Endoh wrote:
>  You know, making parsing easy is not the purpose or the right way.
>  My concern is just about compatiblity.

Agreed.

>  Looks good to me.
>  It brings not only behavior consistency but also good refactoring
>  effect.

Thank you for reviewing my patch!

>  I noticed some minor issues below.
>  
>  vm_insnhelper.c:
>  It would be good to match the number of %d and actual arguments.

Ok, fixed.

>  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.

Oh, forgot to mention this, as I wasn't sure if Module#mix is finalized yet. That line is in case someone calls mix with two hashes, e.g. `mix(Enumerable, {1 => 2}, {1 => 2})`. I thought "wrong arguments" was a bit better than "WNA (3 for 1..3)". Once we have better error messages in case of named arguments, this might change.

>  test/ruby/test_arity.rb
>  
>      assert_equal "0 for 1",     err_mess{ "".sub!{} }
>  
>  This assertion fails.  Did you mean "0 for 1..2" ?

Yes, indeed. Fixed.

I've committed the modified patch to trunk as r35023 and r35024.

>  So far, the remaining issues I know are better error message, and #5989.

Good thing Nobu took care of #5989. I'll open an issue for the error message in case of keyword arguments when I get a chance.

Thanks
--
Marc-Andr��
----------------------------------------
Bug #6085: Treatment of Wrong Number of Arguments
https://bugs.ruby-lang.org/issues/6085#change-24584

Author: Marc-Andre Lafortune
Status: Open
Priority: Normal
Assignee: Yusuke Endoh
Category: core
Target version: 2.0.0
ruby -v: r34800


For brevity, let me abbreviate:

    WNA = "wrong number of arguments"

Ruby could provide more accurate information when raising an ArgumentError for WNA.

Example:

  def foo(a, b=42); end
  foo         # => WNA (0 for 1)
  for(1,2,3)  # => WNA (3 for 2)

It would be strictly superior if the message said instead "WNA (0 for 1..2)" and "WNA (3 for 1..2)":
  * more useful as it gives more information at a glance
  * consistent with calling builtin methods:

    "".index        # => WNA (0 for 1..2)
    "".index(1,2,3) # => WNA (3 for 1..2)

Ruby is also not always consistent in its wording when there is a *rest argument:

  Enumerator.new # => WNA (0 for 1+)
  [].insert      # => WNA (at least 1)
  
  File.chown     # => WNA (0 for 2+)
  Process.kill   # => WNA (0 for at least 2)

While reviewing and factorizing all WNA errors, I also found a problematic case:

  "".sub(//)    # => WNA (1 for 1..2)

It would probably less confusing if it said (1 for 2), as the form without a block requires 2 parameters. Same applies to `sub!`

Also, `Module#define_method` could say "WNA (3 for 1)" when it actually accepts only up to 2 arguments.

I've implemented two patches that address all these issues.

The first one improves the error message when calling user methods and lambdas.

The second harmonizes the builtin methods and fixes the few that need to be fixed.

The two commits can be found here:

https://github.com/marcandre/ruby/commits/rb_arity_check

Complete list of changes:
  * Improvements:

    "".sub(//):          WNA (1 for 1..2) => WNA (1 for 2)
      (same with sub)
    Module#define_method: WNA (3 for 1)    => WNA (3 for 1..2)
    exec:                 WNA              => WNA (0 for 1+)
    Hash.new(1){}:        WNA              => WNA (1 for 0)
    instance_eval("", "", 1, 2)
                          WNA instance_eval(...) or instance_eval{...} 
                                           => WNA (4 for 1..3)
      (same with module_eval and class_eval)
    Module#mix:           WNA (4 for 1)    => WNA (4 for 1..3)
    Module#mix, with incorrect arguments: WNA (2 for 1) => wrong arguments

Wording change:
  * Change of language:   WNA (at least 1) => WNA (0 for 1+)
    [].insert
    extend
    "".delete!
    "".count

  * Process.kill:         WNA (0 for at least 2) => WNA (0 for 2+)

Also, builtin functions calling `rb_scan_args` with both optional arguments and a rest argument would generate an error of the form "WNA (0 for 2..3+)". After this patch, this would now read "WNA (0 for 2+)", again for consistency. The only two such cases I found are in `ext/win32ole.c`

In addition to giving a more consistent error handling, these commits pave the way to:
- improved error reporting for parameters with named parameters (forthcoming issue)
- improved checking for Proc#curry (see bug #5747)



-- 
http://bugs.ruby-lang.org/