[ruby-core:118098] [Ruby master Bug#20218] aset/masgn/op_asgn with keyword arguments

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


Jeremy's argument was not super clear to me so I took a deeper look.

Using a slight variant from the script in https://bugs.ruby-lang.org/issues/20218#note-10:
```ruby
a = Class.new do
  def [](*a, **kw)
    p([a, kw])
    0
  end
  alias []= []
end.new

# Regular index
a[1, 2, bar: 3]

# Regular assignment
a[1, 2, bar: 3] = 4

# Operator assignment
a[1, 2, bar: 3] += 4

# Mass assignment
eval 'a[1, 2, bar: 3], _ = 4, 5' unless RUBY_VERSION.start_with?('3.3')
```

```
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]
[[1, 2], {:bar=>3}] # []
[[1, 2, {:bar=>3}, 4], {}] # []=
[[1, 2, {:bar=>3}], {}] # [] of []+=
[[1, 2, {:bar=>3}, 4], {}] # []= of []+=
[[1, 2, {:bar=>3}, 4], {}] # multiple-assignment []=

(same on `truffleruby 24.0.1` FWIW)

ruby 3.3.1 (2024-04-23 revision c56cd86388) [x86_64-linux]
[[1, 2], {:bar=>3}] # []
[[1, 2, {:bar=>3}, 4], {}] # []=
[[1, 2], {:bar=>3}] # [] of []+=
[[1, 2, 4], {:bar=>3}] # []= of []+=
SEGV # multiple-assignment []=
```

So in 3.2.2 the behavior was mostly consistent, kwargs in the various index methods were all treated as positional arguments.
Except for `[]` which does treat kwargs as kwargs.

In 3.3.1 `[]+=` treats kwargs as kwargs for both the `[]` and `[]=` calls (but not for a lone `[]=` call), so that's already a "silent change" yet it makes a lot of sense, these things are kwargs in syntax, it feels natural they should be kwargs in semantics.

On ruby-master, the code in https://bugs.ruby-lang.org/issues/20218#note-6 does give a SyntaxError.
The problem is for `IndexTest['foo', namespace: 'bar'] = 2`
(`IndexTest['foo', namespace: 'bar']` works fine).
A workaround is to use `IndexTest.[]=('foo', { namespace: 'bar' }, 2)` instead.

BTW let's note that code uses:
```ruby
def self.[]=(name, opts = {}, val)
```
So this seems difficult to evolve, because if `[]=` (and `[]+=`) would pass kwargs as kwargs for the `[]=` call, then that definition of `[]=` would receive `name='foo', opts=2, val={namespace: 'bar'}`.
Which would also break because of e.g. `opts[:namespace]` with `opts=2`.

To fix this, the definition would need to become

```ruby
def self.[]=(name, val, **opts)
```
but that would not work on existing Ruby releases then.

OTOH, that would already be the correct and necessary signature for the `+=` case on 3.3.1:
```ruby
module IndexTest
  @store = {}

  def self.store
    @store
  end

  def self.key(name, namespace: nil)
    name = "#{namespace}:#{name}" if namespace
    name
  end

  def self.[](name, namespace: nil)
    p [name, namespace]
    @store[key(name, namespace: namespace)] || 0
  end

  # def self.[]=(name, opts = {}, val) # no implicit conversion of Symbol into Integer (TypeError) for `opts[:namespace])` below
  def self.[]=(name, val, **opts) # works
    p [name, opts, val]
    @store[key(name, namespace: opts[:namespace])] = val
  end


end

IndexTest['foo', namespace: 'bar'] += 2
```

Given all this and the fact there is no simple way to define `[]=` in a way that accepts caller kwargs as kwargs and yet doesn't break existing code, I can see the idea why to make this a SyntaxError, as it seems the only way to make []/[]= consistent and not break existing definitions of `[]=`.
But it seems rather incompatible and not sure the consistency is worth it.
I don't really consider consistency with the block argument for []/[]=, because I think nobody ever uses that and something that probably should never have been supported given the lack of need for it.

An alternative seems to go back to 3.2-like (3.2 + *1) behavior, i.e. `[]` always receives kwargs, `[]=` never receives kwargs (to not break existing definitions of `[]=`).
The 3.3 behavior for the `[]=` call as part of `[]+=` seems surprising and hard to use (requires defining a `[]=` accepting kwargs but that won't work for a direct `[]=` call such as `a[1, kw: 2] = 3`.
(*1) OTOH the 3.3 change for the `[]` call part of `[]+=` seems good, `[]` should receive kwargs.

----------------------------------------
Bug #20218: aset/masgn/op_asgn with keyword arguments
https://bugs.ruby-lang.org/issues/20218#change-108528

* Author: jeremyevans0 (Jeremy Evans)
* Status: Closed
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
I found that use of keyword arguments in multiple assignment is broken in 3.3 and master:

```ruby
h = {a: 1}
o = []
def o.[]=(*args, **kw)
  replace([args, kw])
end

# This segfaults as RHS argument is not a hash
o[1, a: 1], _ = [1, 2]


# This passes the RHS argument as keywords to the method, treating keyword splat as positional argument
o[1, **h], _ = [{b: 3}, 2]
o
# => [[1, {:a=>1}], {:b=>3}]
```

Before 3.3, keyword arguments were treated as positional arguments.

This is similar to #19918, but for keyword arguments instead of block arguments.

@matz indicated he wanted to prohibit block arguments in aset/masgn and presumably also op_asgn (making them SyntaxErrors).  Can we also prohibit keyword arguments in aset/masgn/op_asgn?

Note that aset treats keyword arguments as regular arguments:

```ruby
o[1, a: 1] = 2
o
# => [[1, {:a=>1}, 2], {}]

o[1, **h] = {b: 3}
o
# => [[1, {:a=>2}, {:b=>3}], {}]
```

While op_asgn treats keyword arguments as keywords:

```ruby
h = {a: 2}
o = []
def o.[](*args, **kw)
  concat([:[], args, kw])
  x = Object.new
  def x.+(v)
    [:x, v]
  end
  x
end
def o.[]=(*args, **kw)
  concat([:[]=, args, kw])
end
o[1, a: 1] += 2
o
# => [:[], [1], {:a=>1}, :[]=, [1, [:x, 2]], {:a=>1}]

o.clear
o[1, **h] += {b: 3}
o
# => [:[], [1], {:a=>2}, :[]=, [1, [:x, {:b=>3}]], {:a=>2}]
```



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- [e-mail]
 To unsubscribe send an email to [e-mail]
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/