Issue #16511 has been updated by Dan0042 (Daniel DeLorme).


> I'd guess it's fairly uncommon to call a method not accepting keyword arguments with `**`.

Maybe, who knows. I'd rather not make assumptions on what is "uncommon".

> Could you show a tentative timeline for your proposal?
> I.e., when each deprecation is introduced and when behavior is changed.

Ok, fair enough. The methods and table below should cover all possible warnings and errors.

```ruby
def foo(lead, opt=nil, k:nil)
  {lead: lead, opt: opt, k: k}
end
def bar(lead, opt=nil, **kw)
  {lead: lead, opt: opt, kw: kw}
end
def deleg(m, *rest)
  send(m, *rest)
end
```

| method call            | 2.6 / 2.7 result                            | 2.7      | 3.0 to 3.2                                 | 3.3 to 3.4 (2.7 is eol) | 3.5                                | warning message                                                                  |
|------------------------|---------------------------------------------|----------|--------------------------------------------|-------------------------|------------------------------------|----------------------------------------------------------------------------------|
| foo(k:1)               | {:lead=>{:k=>1}, :opt=>nil, :k=>nil}        | warning | warning                                   | warning                 | error: wrong number of arguments   | Passing the keyword argument as the last hash parameter is deprecated            |
| bar(k:1)               | {:lead=>{:k=>1}, :opt=>nil, :kw=>{}}        | warning | warning                                   | warning                 | error: wrong number of arguments   | Passing the keyword argument as the last hash parameter is deprecated            |
| foo(1, "a"=>2)         | {:lead=>1, :opt=>{"a"=>2}, :k=>nil}         | warning | warning                                   | warning                 | error: wrong number of arguments   | Passing the keyword argument as the last hash parameter is deprecated            |
| bar(1, "a"=>2)         | {:lead=>1, :opt=>nil, :kw=>{"a"=>2}}        |          |                                            |                         |                                    |                                                                                  |
| foo(1, "a"=>2, k:3)    | {:lead=>1, :opt=>{"a"=>2}, :k=>3}           | warning  | error: wrong number of arguments           |                         |                                    | Splitting the last argument into positional and keyword parameters is deprecated |
| bar(1, "a"=>2, k:3)    | {:lead=>1, :opt=>nil, :kw=>{"a"=>2, :k=>3}} |          |                                            |                         |                                    |                                                                                  |
| foo(1, {"a"=>2, k:3})  | {:lead=>1, :opt=>{"a"=>2}, :k=>3}           | warning  | {:lead=>1, :opt=>{"a"=>2, :k=>3}, :k=>nil} |                         |                                    | Splitting the last argument into positional and keyword parameters is deprecated |
| bar(1, {"a"=>2, k:3})  | {:lead=>1, :opt=>{"a"=>2}, :kw=>{:k=>3}}    | warning  | {:lead=>1, :opt=>{"a"=>2, :k=>3}, :k=>nil} |                         |                                    | Splitting the last argument into positional and keyword parameters is deprecated |
| foo(1, {k:2})          | {:lead=>1, :opt=>nil, :k=>2}                | warning  | {:lead=>1, :opt=>{:k=>2}, :k=>nil}         |                         |                                    | Using the last argument as keyword parameters is deprecated                      |
| foo(1, 2, {k:3})       | {:lead=>1, :opt=>2, :k=>3}                  | warning | warning                                   | warning                 | error: wrong number of arguments   | Using the last argument as keyword parameters is deprecated                      |
| deleg(:foo, 1, k:2)    | {:lead=>1, :opt=>nil, :k=>2}                |          | opt-in warning                            | warning                 | {:lead=>1, :opt=>{:k=>2}, :k=>nil} | Using the last argument as keyword parameters is deprecated                      |
| deleg(:foo, 1, 2, k:3) | {:lead=>1, :opt=>2, :k=>3}                  |          | opt-in warning                            | warning                 | error: wrong number of arguments   | Using the last argument as keyword parameters is deprecated                      |

Blank cells mean there's no change from the previous version.
warning means there's a warning except if it comes from a gem (from a file in Gem::RUBYGEMS_DIR)
opt-in warning is for people who want to stop supporting ruby 2.x before 2.7 eol.

> Treating as keywords even for `foo(h)` if `h` is flagged seems like it would significantly increase the chance to not behave as intended, as Jeremy showed.
> Considering minimum arity like in https://bugs.ruby-lang.org/issues/16511#note-8 would IMHO breaks the separation and add a new exceptional case.
> It's likely to also be very brittle if there are non-required arguments.

I'm not sure what you consider as "intended" behavior, but for me it means that
1) positional hash should not be converted to keywords; 
2) keywords should be not be converted to positional hash (except for positional-only methods);
3) during the transition period, delegation should work for keywords even if not using `**kwrest`;
4) everything else should continue working as it worked in the past.

Since the behavior of a flagged hash in 2.7 is _exactly_ like any hash in 2.6, the compatibility for that case is 100%. At least for now, but it would still warn in 3.3. And considering minimum arity is something that was _already_ being done in 2.6, so it's not some new exceptional case. And it would still show a warning, so it's not breaking separation. There's nothing brittle in there; would you care to elaborate?

> Would such behavior be eventually removed in your proposal? Performance-wise it likely has some cost as it causes extra checks.

Yes, it would be removed. It's somewhat equivalent to ruby2_keywords by default but more comprehensive. Performance-wise it's very unlikely to have a measurable cost, based on the code I've seen in vm_args.c

> The `ruby2_keywords` rule to only consider `*args` is a very precise way to detect delegation in Ruby.
> For that reason I think it's valuable to only have special behavior for that case, and only until we can safely migrate to `(*args, **kwargs)`-delegation.

It's actually not that precise. I think this is Jeremy's main worry about ruby2_keywords by default, that `*args` is not always used only for delegation.

> If I understand correctly from the first post, your proposal has more steps than mine.

No, it has the same number of steps, it's just more lenient about things that we can afford to be lenient about. The "Steps" in the proposal are only conceptual steps, in order to explain the logic. It's still one round of deprecations in 2.7 (Steps 1-3) and another in 3.warn (Step 4). And for end-users it boils down to the same thing: if you see a warning, fix it. There's just fewer warnings to fix at once.


> > Destructuring iteration is compatible
> 
> That would only work in very few cases.
> That's an abuse of the automatic conversion, let's kill it, there is no way to preserve it without breaking the separation.

I disagree this is abusive or hacky code. Although I never used it before, my first impression was of a very clean and powerful idiom. #16494 only showed a `words` array so I don't know how it was built. Perhaps neither of your `JSON.load` or `[**h1, **h2]` examples is relevant. What's important though is that it _can_ be made to work quite easily, without having to restructure everything. And since it behaves differently based on having a positional hash or a keyword (flagged) hash, that means separation is preserved, at least to some extent.


----------------------------------------
Feature #16511: Staged warnings and better compatibility for keyword arguments in 2.7.1
https://bugs.ruby-lang.org/issues/16511#change-84253

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
----------------------------------------
As an alternative to #16463 and #16494 I'd like to propose this approach, which I believe allows a **much** more flexible path for migration of keyword arguments.

The idea is to indicate for every Hash object if it's intended to represent a _keyword_ hash or a _data_ hash. This extra information is then used to generate more granular warnings depending on a user's compatibility needs.

The "keywordness" of a hash would be indicated by a **flag** on the Hash object; this is already implemented in 2.7 and is the approach favored by Matz. Let's call this flagged hash a "KwHash", and a non-flagged hash is just a "Hash". Note: this could also be implemented via a **subclass** of Hash (I personally favor this object-oriented approach) which was the original idea in this proposal.

I'll try to describe the idea in detail by breaking it down into figurative steps. (Skip to "Putting it all together" for the TL;DR version.) Imagine starting with ruby 2.6 and then:

### Step 1

When a double-splat or a brace-less hash is used, instead of a Hash it creates a KwHash.

```ruby
def foo(x) x end
foo(k:1).class      #=> KwHash
foo(**hash).class   #=> KwHash
[k:1].last.class    #=> KwHash
[**hash].last.class #=> KwHash
{**hash}.class      #=> Hash
```

At this point we haven't introduced any real change. Everything that worked before is still working the same way.
(With a minor exception if using the subclass approach: unusual code like `kw.class == Hash` would now return false.)

### Step 2

When there is ambiguity due to optional vs keyword argument, we rely on the last argument being Hash or KwHash to disambiguate.

```ruby
def foo(x=nil, **kw)
  [x,kw]
end
foo({k:1}) #=> [{k:1},{}]
foo(k:1)   #=> [nil,{k:1}]
```

This is the _minimum_ amount of incompatibility required to solve ALL bugs previously reported with keyword arguments. (#8040, #8316, #9898, #10856, #11236, #11967, #12104, #12717, #12821, #13336, #13647, #14130, etc.) 

The warnings for this would be about an impending _change of behavior_ in the _next ruby version_, where `foo({k:1})` is no longer interpreted as keyword argument.

### Step 3

Introduce additional incompatibility to improve clarity of design. Here we deprecate the automatic conversion of Hash to keyword argument; only KwHash is accepted. With a deprecation/warning phase, of course. The "automatic" promotion of a KwHash to a keyword argument follows the same rules as a Hash in 2.6; since the KwHash is conceptually intended to represent keyword arguments, this conversion makes sense in a way that a normal data Hash doesn't. We've taken the "last positional hash" concept and split it into "conceptually a hash" and "conceptually keyword arguments". _Most importantly_, all the changes required to silence these warnings are _compatible with 2.6_.

```ruby
def foo(x, **kw); end
foo(k:1)      # ArgumentError because x not specified
foo(1, {k:1}) # ArgumentError because too many arguments; Hash cannot be converted to KwHashs
opts = [k:1].first
foo(opts)     # opts is a KwHash therefore used as keyword argument; ArgumentError because x not specified
foo(1, opts)  # opts is a KwHash therefore used as keyword argument
```

The warnings for this would be about upcoming _errors_ for positional arguments: `foo(x:1)` will be "given 0, expected 1" and `foo(1,{x:2})` will be "given 2, expected 1". Such errors are useful when developing, but there is no new functionality per se, just a stricter syntax. So it's less important to escalate to an error and we can keep the warnings for longer than Step 2.

At this point we have achieved _almost-full_ **dynamic** keyword separation, as opposed to the current _almost-full_ **static** approach. I want to make the point here that, yes, keyword arguments **are** separated, it's just a different paradigm. With static separation, a keyword argument is defined lexically by a double-splat. With dynamic separation, a keyword argument is when the last argument is a KwHash. {{Note: I'm saying "almost-full" because KwHash is not promoted to keywords in `def foo(a,**kw);end;foo(x:1)` and because static keywords are auto-demoted to positional in `def foo(a);end;foo(x:1)`}}

Any form of delegation works with no change required. This preserves the behavior of 2.6 but only for KwHash objects. This is similar to having 2.7 with `ruby2_keywords` enabled by default. But also different in some ways; most notably it allows the case shown in #16494 to work by default:

```ruby
array = [x:1]
array.push(x:2)
array.map{ |x:| x } #=> [1,2]
[{x:3}].map{ |x:| x } #=> but this warns, as it should
```

The current approach does not allow this to work at all. The solution proposed in #16494 has all the same flaws as Hash-based keyword arguments; what happens to `each{ |x=nil,**kw| }` ? This solution allows a KwHash to be converted to... keywords. Very unsurprising.

Given that ruby is a dynamically-typed language I feel that dynamic typing of keywords if a more natural fit than static typing. But I realize that many disagree with that, which is why we continue to...

### Step 4

Introduce additional incompatibility to reach static/lexical separation of keyword arguments. Here we require that even a KwHash should be passed with a double-splat in order to qualify as a keyword argument.

```ruby
def bar(**kw)
end
def foo(**kw)
  bar(kw)   #=> error; KwHash passed without **
  bar(**kw) #=> ok
end
```

At this point we've reached the same behavior as 2.7. Delegation needs to be fixed, but as we know the changes required to silence these warnings are **not** compatible with 2.6 or 2.7. The warnings for this are _fundamentally not fixable_ as long as Step 2 has not been fixed. This is the core reason why `ruby2_keywords` is currently necessary in 2.7. So in the version after 2.7 we can enable these warnings by default since it's now possible to fix delegation to use static keywords. Except that gem authors who need to stay compatible with 2.7 cannot yet make these changes, so we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.7. And we keep them as warnings instead of errors until ruby 2.7 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` just to temporarily silence warnings, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.7 is EOL these become controlled by `Warning[:deprecated]` which tells people they **have** to fix their code. Which is just like the eventual deprecation of `ruby2_keywords`, just without the busy work of adding `ruby2_keywords` statements in the first place. But again, this introduces no new functionality, just a stricter syntax. So we can play nice and leave the warnings for a few years before changing to errors.

The question remains of how to handle #16494 here. Either disallow it entirely, but I think that would be a shame. Or just like #16494 suggests, allow hash unpacking in non-lambda Proc. Except that now it can be a KwHash instead of a Hash, which at least preserves dynamic keyword separation.

## Putting it all together (TL;DR)

The idea is _not_ to reimplement keyword argument separation; all that is needed is to implement the things above that are not in 2.7:
* Create a KwHash object for brace-less and double-splatted hashes.
* Differentiate the various types of warnings and allow to toggle on/off separately
  * Step 2 warnings _must_ be fixed now; cannot toggle off
  * Step 3 warnings _should_ be fixed now but you don't absolutely need to upgrade your gems just for that
  * Step 4 warnings _should_ be fixed in next version unless you need to support 2.7

I think that's all, really...

### Pros
* Cleaner way to solve #16494
* Better compatibility (at least until 2.6 is EOL)
   * delegation
   * storing an argument list that ends with a KwHash
   * destructuring iteration (#16494)
* We can avoid the "unfortunate corner case" as described in the [release notes](https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/)
   * in 2.7 only do not output "Step 4" warnings, leave delegation like it was
   * in 2.8 the "Step 3" warnings have been fixed and a Hash will not be converted to keyword arguments
   * delegation can now safely be fixed to use the `**` syntax
* ruby2_keywords is not required, which is desirable because
   * it's a hidden flag _hack_
   * it requires to change the code now, and change it _again_ when ruby2_keywords is deprecated; twice the work; twice the gem upgrades
   * it was supposed to be used only for people who need to support 2.6 or below, but it's being misunderstood as an acceptable way to fix delegation in general
   * there's the non-zero risk that ruby2_keywords will never be removed, leaving us with a permanent "hack mode"
      * dynamic keywords are by far preferable to supporting ruby2_keywords forever
* Likely _better performance_, as the KwHash class can be optimized specifically for the characteristics of keyword arguments.
* More flexible migration
   * Allow more time to upgrade the hard stuff in Step 4
   * Can reach the _same_ goal as the current static approach
   * Larger "support zone" https://xkcd.com/2224/
   * Instead of wide-ranging incompatibilities all at once, there's the _possibility_ of making it finer-grained and more gradual
      * rubyists can _choose_ to migrate all at once or in smaller chunks
   * It hedges the risks by keeping more possibilities open for now.
   * It allows to cop-out at Step 3 if Step 4 turns out too hard because it breaks too much stuff

### Cons
* It allows to cop-out at Step 3 if Step 4 turns out too hard because it breaks too much stuff




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