Issue #8040 has been updated by mame (Yusuke Endoh).

Status changed from Open to Assigned
Assignee set to matz (Yukihiro Matsumoto)

This is a duplicate of #7529 which was once rejected by matz.  I translate his reason:

> Unfortunately, it is the spec; we cannot distinguish whether the hash is for a keyword or for just an argument.
> Please add {} at the last.

But, it may be good that we consider the hash for a keyword *only when* the number of arguments is more than the expected mandatory parameters.


diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 0c447aa..2432288 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1065,12 +1065,12 @@ vm_caller_setup_args(const rb_thread_t *th, rb_control_frame_t *cfp, rb_call_inf
 }
 
 static inline int
-vm_callee_setup_keyword_arg(const rb_iseq_t *iseq, int argc, VALUE *orig_argv, VALUE *kwd)
+vm_callee_setup_keyword_arg(const rb_iseq_t *iseq, int argc, int m, VALUE *orig_argv, VALUE *kwd)
 {
     VALUE keyword_hash;
     int i, j;
 
-    if (argc > 0 &&
+    if (argc > m &&
 	!NIL_P(keyword_hash = rb_check_hash_type(orig_argv[argc-1]))) {
 	argc--;
 	keyword_hash = rb_hash_dup(keyword_hash);
@@ -1109,7 +1109,7 @@ vm_callee_setup_arg_complex(rb_thread_t *th, rb_call_info_t *ci, const rb_iseq_t
 
     /* keyword argument */
     if (iseq->arg_keyword != -1) {
-	argc = vm_callee_setup_keyword_arg(iseq, argc, orig_argv, &keyword_hash);
+	argc = vm_callee_setup_keyword_arg(iseq, argc, m, orig_argv, &keyword_hash);
     }
 
     /* mandatory */
@@ -2127,7 +2127,7 @@ vm_yield_setup_block_args(rb_thread_t *th, const rb_iseq_t * iseq,
 
     /* keyword argument */
     if (iseq->arg_keyword != -1) {
-	argc = vm_callee_setup_keyword_arg(iseq, argc, argv, &keyword_hash);
+	argc = vm_callee_setup_keyword_arg(iseq, argc, m, argv, &keyword_hash);
     }
 
     /*

-- 
Yusuke Endoh <mame / tsg.ne.jp>
----------------------------------------
Bug #8040: Unexpect behavior when using keyword arguments
https://bugs.ruby-lang.org/issues/8040#change-37451

Author: pabloh (Pablo Herrero)
Status: Assigned
Priority: Normal
Assignee: matz (Yukihiro Matsumoto)
Category: 
Target version: 
ruby -v: 2.0.0-p0


=begin
There is an odd behavior when calling methods with the new keyword arguments syntax, when you have a method defined with mandatory arguments that also takes options, like this:

  def foo value, **keywords
    puts [value,keywords].inspect
  end
  
  foo("somthing") #This works
  foo("somthing", key: 'value') #This also works
  
  foo(Hash.new(something: 'else')) #This raises 'ArgumentError: wrong number of arguments (0 for 1)'

This feels weird regardless the fact that keyword arguments are a Hash at the bottom, since you ARE PASSING an argument.

Other side effect from this, is that when you call the method ((|foo|)) with a single argument, you can't anticipate how many argument you will be actually passing at runtime unless you know beforehand the argument's class.

What's maybe even more concerning is the fact than this happens even when you pass an argument which class derives from Hash:

  class MyDirectory < Hash; end
   
  foo(MyDirectory.new(something: 'else')) #This also raises 'ArgumentError: wrong number of arguments (0 for 1)'


Besides finding this behavior surprising, I think this could also possibly lead to old code been unexpectedly broken when updating old methods that takes options to the new syntax.

For example if you have this code:

  def foo_with_options argument, options = {}
    #Do some stuff with options
  end
  
  
  #And at someplace else...
  
  my_hash_thingy = Hash.new
  foo_with_options(my_hash_thingy) #Only passing mandatory argument, with no options, works fine.


When updating to the new syntax:

  def foo_with_options argument, an_option: 'value1', another_option: 'value2'
    #Do some stuff with options
  end
  
  
  
  #And at someplace else...
  
  my_hash_thingy = Hash.new
  foo_with_options(my_hash_thingy) #Only passing mandatory argument, with no options, doesn't work anymore.
=end


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