From: eregontp@...
Date: 2020-03-19T01:17:43+00:00
Subject: [ruby-core:97544] [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).


matz (Yukihiro Matsumoto) wrote in #note-3:
> this makes warn-hunting (especially for Rails core contributors) more difficult.

Can you explain that?
AFAIK this will make code use `ruby2_keywords` in more places, but those places will need to change to use `*args, **kwargs`-delegation in Ruby 3+.
So it's counterproduce to not have the warnings here, because the methods missing `ruby2_keywords` would simply break in Ruby 3+.



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

* 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:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>