ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:96878] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
@ 2020-01-16  4:30 ` daniel
  2020-01-16  7:33 ` [ruby-core:96897] " eregontp
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-16  4:30 UTC (permalink / raw)
  To: ruby-core

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

----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

### 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. And always use the last KwHash argument if the method supports keyword arguments. With a deprecation/warning phase, of course. But 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
```

At this point we have achieved _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.

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 importantly_, 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]
```

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96897] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
  2020-01-16  4:30 ` [ruby-core:96878] [Ruby master Feature#16511] Subclass of Hash for keyword arguments daniel
@ 2020-01-16  7:33 ` eregontp
  2020-01-16  8:47 ` [ruby-core:96906] " matz
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: eregontp @ 2020-01-16  7:33 UTC (permalink / raw)
  To: ruby-core

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


Thank you for filing this and explaining your idea.

My understanding is this is basically a different way to represent a flagged Hash, i.e., flagged Hash becomes KwHash in this proposal.
Compared to #16463, what are the differences except the representation of the flagged Hash?

`Step 2` is not done in Ruby 2.7.0, I think it's too incompatible and needs warnings before changing that behavior.
It will work like that in 3.0 though.

> Most importantly, it allows the case shown in #16494 to work by default:
> `array = [x:1]; array.push(x:2); array.map{ |x:| x } #=> [1,2]`

But `array = [{x:1}]` would warn, right? And `array << {x:2}` would also warn.
In such a case I consider the hashes in `array` as data Hashes, e.g., coming from JSON.parse, and so IMHO should not be KwHash.
This idea doesn't really solve that, just makes some cases easier.

----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83911

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

### 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. And always use the last KwHash argument if the method supports keyword arguments. With a deprecation/warning phase, of course. But 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
```

At this point we have achieved _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.

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 importantly_, 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]
```

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96906] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
  2020-01-16  4:30 ` [ruby-core:96878] [Ruby master Feature#16511] Subclass of Hash for keyword arguments daniel
  2020-01-16  7:33 ` [ruby-core:96897] " eregontp
@ 2020-01-16  8:47 ` matz
  2020-01-16  8:57 ` [ruby-core:96907] " shevegen
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: matz @ 2020-01-16  8:47 UTC (permalink / raw)
  To: ruby-core

Issue #16511 has been updated by matz (Yukihiro Matsumoto).


I was going to use an internal flag instead of making them subclass. Subclassing standard class has often made trouble in the past. I am negative.

Matz.


----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83921

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

### 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. And always use the last KwHash argument if the method supports keyword arguments. With a deprecation/warning phase, of course. But 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
```

At this point we have achieved _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.

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 importantly_, 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]
```

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96907] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (2 preceding siblings ...)
  2020-01-16  8:47 ` [ruby-core:96906] " matz
@ 2020-01-16  8:57 ` shevegen
  2020-01-16 15:55 ` [ruby-core:96912] " daniel
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: shevegen @ 2020-01-16  8:57 UTC (permalink / raw)
  To: ruby-core

Issue #16511 has been updated by shevegen (Robert A. Heiler).


Matz already commented. :)

I will briefly give my opinion. I think aside from special cases, one issue is that
ruby users have to understand why different/specialized hashes are used, be this
KwHash or HashWithIndifferentAccess or any other variant. IMO, from that point of
view, I'd personally prefer to not have special names and special subclasses be
used if it could be avoided and keep (core) ruby simple(r). (A tiny issue may 
also be the name; KwHash reads somewhat strangely.)

----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83924

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

### 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. And always use the last KwHash argument if the method supports keyword arguments. With a deprecation/warning phase, of course. But 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
```

At this point we have achieved _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.

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 importantly_, 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]
```

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96912] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (3 preceding siblings ...)
  2020-01-16  8:57 ` [ruby-core:96907] " shevegen
@ 2020-01-16 15:55 ` daniel
  2020-01-16 17:36 ` [ruby-core:96913] " merch-redmine
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-16 15:55 UTC (permalink / raw)
  To: ruby-core

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


matz (Yukihiro Matsumoto) wrote:
> I was going to use an internal flag instead of making them subclass.

Eregon (Benoit Daloze) wrote:
> My understanding is this is basically a different way to represent a flagged Hash, i.e., flagged Hash becomes KwHash in this proposal.
> Compared to #16463, what are the differences except the representation of the flagged Hash?

Well, I guess everything I described above could be achieved with a flag as well. After all when you get down to it an object's class is also just a flag that defines which set of behavior the object has. It's just that naturally I consider an object-oriented design preferable to a flag-oriented design.

The difference with #16463 is that _every_ instance of a double-splat or brace-less hash would have the flag, instead of just the ones passed to a method with a `*rest` argument.

But I guess expanding the use of the flag in this way would be possible even in 2.7.x, and after that all that's needed is to add a separate set of warnings for hashes with the flag.

> `Step 2` is not done in Ruby 2.7.0, I think it's too incompatible and needs warnings before changing that behavior.
> It will work like that in 3.0 though.

Yes, of course there's a warning phase, I neglected to say it explicitly. I just wanted to illustrate that fixing the _bugs_ of keyword arguments required few changes, while fixing the _design_ requires more.

> > Most importantly, it allows the case shown in #16494 to work by default:
> > `array = [x:1]; array.push(x:2); array.map{ |x:| x } #=> [1,2]`
> 
> But `array = [{x:1}]` would warn, right? And `array << {x:2}` would also warn.
> In such a case I consider the hashes in `array` as data Hashes, e.g., coming from JSON.parse, and so IMHO should not be KwHash.

Yes, exactly. That way it's possible to opt in/out to the behavior.

> This idea doesn't really solve that, just makes some cases easier.

????
Yes, it does solve that, it's the _entire point_ of this proposal.

> Subclassing standard class has often made trouble in the past.

I'd really like to know what kind of trouble. From what I know/remember there have been issues with e.g. `Subclass.select{}` returning an instance of `BaseClass`, but in this case it's exactly what we'd want, just like `flaggedhash.select{}` would return a non-flagged hash.

----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83930

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

### 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. And always use the last KwHash argument if the method supports keyword arguments. With a deprecation/warning phase, of course. But 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
```

At this point we have achieved _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.

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 importantly_, 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]
```

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96913] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (4 preceding siblings ...)
  2020-01-16 15:55 ` [ruby-core:96912] " daniel
@ 2020-01-16 17:36 ` merch-redmine
  2020-01-16 19:27 ` [ruby-core:96915] " daniel
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: merch-redmine @ 2020-01-16 17:36 UTC (permalink / raw)
  To: ruby-core

Issue #16511 has been updated by jeremyevans0 (Jeremy Evans).


I believe this approach would break the following code:

```ruby
def debug_log(arg, output: $stderr)
  output.print(arg.inspect)
  output.print("\n")
end
def bar(*args, **opts)
  debug_log(args)
  debug_log(opts)
  # do something
end
bar(:baz, quux:1)
```

With the master branch, `{quux:1}` is considered a keyword argument by `bar`, but passed as a positional argument to `debug_log`.  This works fine as you would expect it to.  With the `KwHash` approach, `{quux:1}` ends up being passed as a keyword argument to `debug_log`, resulting in `ArgumentError`.  This results in even greater backwards compatibility issues than the ruby2_keywords by default approach.

In general, whether something is a keyword argument or a positional argument is something that should be decided on a per-call site basis, it shouldn't be a property of an object.  `ruby2_keywords` is strictly a work around to support older Ruby versions without requiring separate code paths, to ease supporting multiple Ruby versions at once.  When targeting only Ruby 3+, there would be no reason to use it (except performance when using CRuby).

The `KwHash` approach would result in the problems that keyword argument separation was designed to avoid, since you would have cases where positional arguments to methods are treated as keyword arguments depending on how they were created.  With the current master branch, that can only happen in limited cases on an opt-in basis when using `ruby2_keywords`.  Presumably, because it is opt-in, there will not be cases where positional arguments are ever mistreated as keyword arguments, without the deliberate misuse of `ruby2_keywords`.

----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83931

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

### 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. And always use the last KwHash argument if the method supports keyword arguments. With a deprecation/warning phase, of course. But 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
```

At this point we have achieved _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.

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 importantly_, 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]
```

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96915] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (5 preceding siblings ...)
  2020-01-16 17:36 ` [ruby-core:96913] " merch-redmine
@ 2020-01-16 19:27 ` daniel
  2020-01-17  4:04 ` [ruby-core:96921] " daniel
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-16 19:27 UTC (permalink / raw)
  To: ruby-core

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


jeremyevans0 (Jeremy Evans) wrote:
> I believe this approach would break the following code:
> 
> ```ruby
> def debug_log(arg, output: $stderr)
>   output.print(arg.inspect)
>   output.print("\n")
> end
> def bar(*args, **opts)
>   debug_log(args)
>   debug_log(opts)
>   # do something
> end
> bar(:baz, quux:1)
> ```

You're right! Thank you for finding an actual specific case (and a plausible one at that!) In this case, in ruby 2.7.KwHash, you'd get a warning to the effect that the KwHash will be used for keyword arguments instead of positional, just as if you wrote `debug_log(**opts)` in 2.7; and you'd need to opt-in to the desired behavior with `debug_log(opts.to_h)` or `debug_log({**opts})`. Whereas in 2.6 and 2.7 it works with no warning. With the "Step 4" warnings you'd also get a warning to the effect that `opts` should be passed with a double-splat. Just like 2.7 the basic rule is still to warn for any change in behavior first.

But still, I believe that cases like this where you'd want a keyword hash to be interpreted as a regular hash are quite rare (although I _could_ be wrong on that point). So overall I still believe in the KwHash approach.

> This results in even greater backwards compatibility issues than the ruby2_keywords by default approach.

I don't understand your notion of backward compatibility. It seems to me like you're conflating "backward compatibility" with "desired behavior in 3.0". Backward compatibility is when things that worked in 2.6 still work the same way with no change. The example you showed demonstrates a backward compatibility breakage, yes. But _many_ more other cases are not backward compatible in 2.7 and become backward compatible with the KwHash or ruby2_keywords by default approach.

> In general, whether something is a keyword argument or a positional argument is something that should be decided on a per-call site basis, it shouldn't be a property of an object.

I know this is your opinion, but I hope you can realize it is an _opinion_ about what the design _should_ be like. My opinion differs, on the basis of putting more importance on backward compatibility.

> `ruby2_keywords` is strictly a work around to support older Ruby versions without requiring separate code paths, to ease supporting multiple Ruby versions at once.  When targeting only Ruby 3+, there would be no reason to use it (except performance when using CRuby).

But "supporting multiple Ruby versions at once" is not some rare thing, it's what _all_ gems should do (until 2.6 is EOL).

> The `KwHash` approach would result in the problems that keyword argument separation was designed to avoid

No, this is incorrect. If you take into account the "Step 4" I described above, the result would be _exactly the same_ as the current master branch, with the exception that passing a KwHash as last argument must be disambiguated as either `debug_log(**opts)` or `debug_log({**opts})`. The biggest difference is how much pain and inconvenience we cause to other ruby programmers along the way. I really think that's worth minimizing.

----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83933

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

### 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. And always use the last KwHash argument if the method supports keyword arguments. With a deprecation/warning phase, of course. But 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
```

At this point we have achieved _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.

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 importantly_, 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]
```

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96921] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (6 preceding siblings ...)
  2020-01-16 19:27 ` [ruby-core:96915] " daniel
@ 2020-01-17  4:04 ` daniel
  2020-01-17 16:20 ` [ruby-core:96930] " daniel
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-17  4:04 UTC (permalink / raw)
  To: ruby-core

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

Description updated

I'm rethinking the issue pointed out by Jeremy. It makes no sense to introduce a new kind of incompatibility for the sake of a _compatibility_ layer. The behavior of KwHash must be identical to the behavior of Hash in 2.6, meaning that a positional KwHash is only promoted to keyword argument if there are enough other positional arguments to satisfy the minimum arity. So the example given by Jeremy would work. Conceptually it's not ideal, but the important thing in this case is backward compatibility.

----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83940

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

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

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

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96930] [Ruby master Feature#16511] Subclass of Hash for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (7 preceding siblings ...)
  2020-01-17  4:04 ` [ruby-core:96921] " daniel
@ 2020-01-17 16:20 ` daniel
  2020-01-20  0:51 ` [ruby-core:96947] [Ruby master Feature#16511] Staged warnings " daniel
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-17 16:20 UTC (permalink / raw)
  To: ruby-core

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


jeremyevans0 (Jeremy Evans) wrote:
> In this case the minimum arity is met, but you would still end up with ArgumentError because it would treat the `debug_log(opts)` call as passing keywords instead of a positional hash, and `debug_log` doesn't support a `quux` keyword.

But most importantly it's compatible with 2.6. In other words since it breaks in 2.6 this case "doesn't exist" in real production code,  and there's no "real" compatibility issue. It's also rational; since opts is a KwHash it conceptually makes sense to interpret it as keyword arguments. Maybe not in your static paradigm, but dynamically yes. So we just keep this backward compatibility with warnings for a while longer until 2.6 (or 2.7?) is EOL. Keyword arguments have worked _mostly_ fine since 2.0, there's no reason to be in such a rush to break things right now now now.

If you really want the new "fixed" but backward incompatible behavior, you have to _opt in_ to it with `debug_log({**opts})` or such. Unlike 2.6 this would finally be possible. It's far "better" to opt in to the incompatible behavior for a few cases like this than to force people to opt out of the incompatible behavior for a much larger number of cases. And by "better" here I mean more respectful of people's time and trouble.

----------------------------------------
Feature #16511: Subclass of Hash for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83957

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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 have a subclass of Hash (let's name it "KwHash") which provides a clean, object-oriented design with various benefits.

I'll try to describe the idea by breaking it down into figurative steps. 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 the ONLY exception being code like `kw.class == Hash` which now returns false. But no one actually writes code like that; it's always `kw.is_a?(Hash)`, which still returns true.

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

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

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

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| }` ? The subclass-based 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. So here we introduce a way to _silence **only** these "Step 4" warnings_, for people who need to remain compatible with 2.6. And we keep them as warnings instead of errors until ruby 2.6 is EOL.

So instead of having to update a bunch of places with `ruby2_keywords` right now, it's a single flag like `Warning[:ruby3_keywords]`. Once ruby 2.6 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.

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

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 when a double-splat is used.
* If a warning is due to a KwHash instead of a Hash, make it a different kind of warning that can be toggled off separately from the Hash warnings (and that will stay as warnings until 2.6 is EOL)

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:96947] [Ruby master Feature#16511] Staged warnings for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (8 preceding siblings ...)
  2020-01-17 16:20 ` [ruby-core:96930] " daniel
@ 2020-01-20  0:51 ` daniel
  2020-01-29  3:37 ` [ruby-core:97015] " daniel
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-20  0:51 UTC (permalink / raw)
  To: ruby-core

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

Description updated
Subject changed from Subclass of Hash for keyword arguments to Staged warnings for keyword arguments

I'm adjusting the proposal to focus on the goal (flexible deprecation) rather than the means (subclass or flag)

----------------------------------------
Feature #16511: Staged warnings for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-83974

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:97015] [Ruby master Feature#16511] Staged warnings for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (9 preceding siblings ...)
  2020-01-20  0:51 ` [ruby-core:96947] [Ruby master Feature#16511] Staged warnings " daniel
@ 2020-01-29  3:37 ` daniel
  2020-01-29 10:42 ` [ruby-core:97018] " eregontp
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-29  3:37 UTC (permalink / raw)
  To: ruby-core

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


Ok, it took a while to play with the vm_args code enough to really understand it, but I finally have my own **proof of concept** ready! お待たせしました。
https://github.com/dan42/ruby/tree/kwarg-warnings-16511
Please check it out. I'm sure there are still some bugs to work out, though, so don't take that to mean that the entire idea is flawed.

Note: I heavily refactored the vm_args code in order to understand it, but _please_ don't focus on that; this is intended as proof of concept to demonstrate the benefits of this approach, not a polished pull request. (Although I _am_ pretty happy about how those refactorings turned out.)

Below I refer to 2.7.e for proposal #16463; and 2.7.d / 3.x.d for this proposal.

Benefits:

### Three levels of urgency in warnings

[FIX NOW]: must be fixed now since behavior will be different in 3.0, for example Hash is no longer converted to keywords
[fix now]: will eventually raise error; should be fixed now but there's no hurry or benefit to break people's stuff in 3.0
[fix in 3.x]: will eventually raise error; fix is not compatible with 2.x; therefore message is not displayed in 2.7; fix once you're ready to drop support for ruby 2.x

This is because there are different needs:
* app developer cares about his current version of ruby only
* gem developer cares about remaining compatible with a few versions
* gem user doesn't want to upgrade to a new major version of the gem with an incompatible API

In this proof of concept the three labels above are simply appended to the warnings, but in a final version they would instead change the _visibility_ of the warnings.

### Delegation is backward compatible

```ruby
def foo(*args); p args; end
def bar(**kw); p kw; end
def deleg(m,*args); send(m,*args); end
ruby2_keywords def deleg2(m,*args); send(m,*args); end
def delegV(m,a); send(m,a); end
                               # 2.7      2.7.e    2.7.d
deleg(:bar, x:1)  #=> {:x=>1}  # warning  -        -    
deleg2(:bar, x:1) #=> {:x=>1}  # -        -        -    
delegV(:bar, x:1) #=> {:x=>1}  # warning  warning  -    
deleg(:foo, 2)    #=> [2]
deleg2(:foo, 2)   #=> [2]
delegV(:foo, 2)   #=> [2]
```

This example demonstrates that delegation _can_ break even if it doesn't use `*args`. In this case you'd have to change `delegV` to use `*args` delegation with ruby2_keywords. I acknowledge this is unlikely to happen in practice, but since it's theoretically possible... all else being equal why not choose the approach that handles even this remote possibility?

### Keywords-to-positional is compatible with 2.6

```ruby
def foo(*args); p args; end
h0 = {}
h1 = {k: 42}
foo()     #=> []  
foo(**h0) #=> [{}]       (like 2.6, unlike 2.7)
foo(**h1) #=> [{:k=>42}]
```

Since it's been decided that the conversion from keywords to positional will remain forever for the sake of compatibility, I strongly believe it makes no sense to introduce an incompatibility in this compatibility behavior.

### `**kw` delegation doesn't introduce additional hash 

```ruby
def foo(*args); p args; end
def deleg(*a,**k); foo(*a,**k); end
h0 = {}
h1 = {k: 42}
p foo()    ==deleg()      #=> [] == []                 #=> true, unlike [] == [{}] in 2.6
p foo(**h0)==deleg(**h0)  #=> [{}] == [{}]             #=> true               
p foo(**h1)==deleg(**h1)  #=> [{:k=>42}] == [{:k=>42}] #=> true
p foo({})  ==deleg({})    #=> [{}] == [{}]             #=> true, unlike [{}] == [] in 2.7 ... but with [FIX NOW] warning
```

I introduced a distinction between an empty kwsplat and a "nonexistent" kwsplat; the later is not converted to an empty positional hash. I've come to believe this is necessary in order to have both Keywords-to-positional compatibility and `**kw` delegation work at the same time.

### Destructuring iteration is compatible

```ruby
p [x:1].push(x:2).map{ |x:| x }     #=> [1, 2]  with warning in 2.7 and 2.7.e and 3.x.d
p [{x:1}].push({x:2}).map{ |x:| x } #=> [1, 2]  with warning in 2.7 and 2.7.e and 2.7.d
```

It may not be a common pattern, but at least 2 people have reported this being an issue. Since there's a solution that allows this to work, why not take it? In the end maybe matz will decide to kill this feature but I would prefer to have an actual discussion on this rather than rush the "kill" decision just because the current approach doesn't allow this flexibility.

----------------------------------------
Feature #16511: Staged warnings for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-84103

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:97018] [Ruby master Feature#16511] Staged warnings for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (10 preceding siblings ...)
  2020-01-29  3:37 ` [ruby-core:97015] " daniel
@ 2020-01-29 10:42 ` eregontp
  2020-01-29 14:16 ` [ruby-core:97019] " daniel
  2020-01-29 16:57 ` [ruby-core:97021] " daniel
  13 siblings, 0 replies; 14+ messages in thread
From: eregontp @ 2020-01-29 10:42 UTC (permalink / raw)
  To: ruby-core

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


FWIW, I also discussed to keep `**empty_hash` in the case it's passed by the user in https://bugs.ruby-lang.org/issues/16519#note-5
Jeremy seems clearly against it, and I get that `**empty_hash` always passing nothing is simpler to understand.

I'm not sure what's the design of your proposal.
What do you change to keep *args-delegation working?

It seems unlikely Ruby core would accept to backport anything more complicated than #16463 to 2.7.

----------------------------------------
Feature #16511: Staged warnings for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-84107

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:97019] [Ruby master Feature#16511] Staged warnings for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (11 preceding siblings ...)
  2020-01-29 10:42 ` [ruby-core:97018] " eregontp
@ 2020-01-29 14:16 ` daniel
  2020-01-29 16:57 ` [ruby-core:97021] " daniel
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-29 14:16 UTC (permalink / raw)
  To: ruby-core

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


> FWIW, I also discussed to keep `**empty_hash` in the case it's passed by the user in https://bugs.ruby-lang.org/issues/16519#note-5

Ah, I missed that one. I was actually inspired to keep the behavior by your PR in #16463. I haven't posted that much there but that's because I very much agree with what you're saying. I can't thank you enough for the efforts you've been putting in this.

> Jeremy seems clearly against it, and I get that `**empty_hash` always passing nothing is simpler to understand.

Now that I understand the vm_args code I've gained a better appreciation for Jeremy's viewpoint. The splatting of an empty hash interacts in a really nasty way with `*rest` and `**kwrest` delegation. But I don't think that always passing nothing is simpler...

```ruby
def foo(opts); p opts; end
opts = {}
opts[:k] = 1 if some_condition
foo(**opts)
```

In 2.7 this results in a warning depending on `some_condition`, that's really not simple to understand.  :-/

> I'm not sure what's the design of your proposal.
> What do you change to keep *args-delegation working?

Well, the design is described in extremely thorough detail above, although I realize it's a massive wall of text to read. I guess you can boil it down to having the keyword flag set on _every_ splatted hash instead of just when accepting a `*rest` argument. That way is much more logical; it's a coherent part of the design instead of a special-case hack (as explained in the proposal description).

> It seems unlikely Ruby core would accept to backport anything more complicated than #16463 to 2.7.

Despite having demonstrable benefits? That would make me sad. Enabling ruby2_keywords by default gets 90% of the benefit for 10% of the complexity, but I think it's worth going to 100%, and I've done it.

----------------------------------------
Feature #16511: Staged warnings for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-84108

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ruby-core:97021] [Ruby master Feature#16511] Staged warnings for keyword arguments
       [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
                   ` (12 preceding siblings ...)
  2020-01-29 14:16 ` [ruby-core:97019] " daniel
@ 2020-01-29 16:57 ` daniel
  13 siblings, 0 replies; 14+ messages in thread
From: daniel @ 2020-01-29 16:57 UTC (permalink / raw)
  To: ruby-core

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


> gem user doesn't want to upgrade to a new major version of the gem with an incompatible API

I think this part deserves explanation because so far I haven't seen much discussion of what happens to people who _use_ gems.
Please consider this scenario:

* author of gem "foobar" fixes the keyword warnings in ruby 2.7 and publishes a new version 3.1.0 (only v3 is supported)
* gem user finds keyword warnings coming from foobar 1.4.7
* he upgrades teeny version to 1.4.12 but the warnings are still there
* he upgrades minor version to 1.6.3 but the warnings are still there
* he must upgrade to 3.1.0 but the API has changed and requires various fixes to the app
* he can't upgrade to ruby 3.0 as long as this isn't done
* so he has to fix the app to use foobar 3.1.0
* **3 years pass...** ruby2_keywords are being deprecated
* author of gem "foobar" changes ruby2_keywords to `**kwrest` delegation and publishes a new version 4.2.0 (only v4 is supported)
* user of gem "foobar" finds keyword warnings coming from the gem
* he upgrades teeny version to 3.1.7 but the warnings are still there
* he upgrades minor version to 3.2.2 but the warnings are still there
* he must upgrade to 4.2.0 but the API has changed and requires various fixes to the app
* he can't upgrade to ruby 3.3 as long as this isn't done
* so he has to fix the app _AGAIN_, to use foobar 4.2.0

This is a worst-case scenario but it doesn't sound so farfetched to me.

What I would like to see is this:

* author of gem "foobar" fixes the keyword warnings (except [fix in 3.x]) and publishes a new version 3.1.0 (only v3 is supported)
* only [FIX NOW] warnings are displayed in the case of gems in RUBYGEMS_DIR, and they are pretty rare
* user of gem "foobar" finds no keyword warnings coming from the gem, nothing to upgrade except his own code
* **3 years pass...** [fix now] and [fix in 3.x] warnings are enabled by default even in RUBYGEMS_DIR
* author of gem "foobar" fixes the warnings to use `**` syntax and publishes a new version 4.2.0 (only v4 is supported)
* gem user finds keyword warnings coming from foobar 1.4.7
* he upgrades teeny version to 1.4.12 but the warnings are still there
* he upgrades minor version to 1.6.3 but the warnings are still there
* he must upgrade to 4.2.0 but the API has changed and requires various fixes to the app
* he can't upgrade to ruby 3.4 as long as this isn't done, but that's in two years
* so he has some time to fix the app, ONCE, to use foobar 4.2.0

Isn't that better? Note that the worst case would still involve two upgrades, but it would be significantly less likely: only if a gem has warnings with _both_ [FIX NOW] and [fix in 3.x] warnings.


----------------------------------------
Feature #16511: Staged warnings for keyword arguments
https://bugs.ruby-lang.org/issues/16511#change-84111

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-01-29 16:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <redmine.issue-16511.20200116043024@ruby-lang.org>
2020-01-16  4:30 ` [ruby-core:96878] [Ruby master Feature#16511] Subclass of Hash for keyword arguments daniel
2020-01-16  7:33 ` [ruby-core:96897] " eregontp
2020-01-16  8:47 ` [ruby-core:96906] " matz
2020-01-16  8:57 ` [ruby-core:96907] " shevegen
2020-01-16 15:55 ` [ruby-core:96912] " daniel
2020-01-16 17:36 ` [ruby-core:96913] " merch-redmine
2020-01-16 19:27 ` [ruby-core:96915] " daniel
2020-01-17  4:04 ` [ruby-core:96921] " daniel
2020-01-17 16:20 ` [ruby-core:96930] " daniel
2020-01-20  0:51 ` [ruby-core:96947] [Ruby master Feature#16511] Staged warnings " daniel
2020-01-29  3:37 ` [ruby-core:97015] " daniel
2020-01-29 10:42 ` [ruby-core:97018] " eregontp
2020-01-29 14:16 ` [ruby-core:97019] " daniel
2020-01-29 16:57 ` [ruby-core:97021] " daniel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).