[ruby-core:97543] [Ruby master Bug#16466] `*args -> *args` delegation should be warned when the last hash has a `ruby2_keywords` flag

Issue #16466 has been updated by Eregon (Benoit Daloze).


This means adding `ruby2_keywords` is not only cumbersome and difficult, but it's also not sufficient as a reminder of which methods to update in Ruby 3+.

This is an implementation detail leak, and I think it makes no sense for other implementations to follow it.

Not fixing it in MRI forces other implementations to have the same implementation bug, or produce more warnings which is very unfortunate.
Please reconsider.

----------------------------------------
Bug #16466: `*args -> *args` delegation should be warned when the last hash has a `ruby2_keywords` flag
https://bugs.ruby-lang.org/issues/16466#change-84702

* Author: mame (Yusuke Endoh)
* Status: Closed
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
(This ticket is derived from https://github.com/rails/rails/pull/38105#discussion_r361798251)

Currently, the following code displays no warnings.

```ruby
def baz(**kw)
end

def bar(*args)
  baz(*args)
end

ruby2_keywords def foo(*args)
  bar(*args)
end

foo(k: 1)
```

However, I think this code should be warned.  `bar` should be marked by `ruby2_keywords`.  This is because `ruby2_keywords` will be used as a mark to indicate "we need to rewrite this method from `def foo(*args)` to `def foo(*args, **kwargs)`" after `ruby2_keywords` is deprecated in far future.  If no warning is emitted for the code above, a user will rewrite `foo` as:

```
def foo(*args, *kwargs)
  bar(*args, *kwargs)
end
```

and will not modify the definitions of `bar` and `baz`.  Actually, the resulting code is broken; `bar` must be also modified like `foo`.


@jeremyevans0 Is this intentional?  If not, what do you think the following patch?

```diff
diff --git a/vm_args.c b/vm_args.c
index 7bf61cefe7..7077c7b3c1 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -801,6 +801,8 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
             if (RB_TYPE_P(rest_last, T_HASH) &&
                 (((struct RHash *)rest_last)->basic.flags & RHASH_PASS_AS_KEYWORDS)) {
                 rest_last = rb_hash_dup(rest_last);
+                arg_rest_dup(args);
+                RARRAY_ASET(args->rest, len - 1, rest_last);
                 kw_flag |= VM_CALL_KW_SPLAT;
                 if (iseq->body->param.flags.ruby2_keywords) {
                     remove_empty_keyword_hash = 0;
```

This patch prints the following warnings for the code above.

```
test.rb:5: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
test.rb:1: warning: The called method `baz' is defined here
```



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

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>