[ruby-core:96974] [Ruby master Bug#16519] pp [Hash.ruby2_keywords_hash({})] shows `[nil]`

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


Eregon (Benoit Daloze) wrote:
> jeremyevans0 (Jeremy Evans) wrote:
> > With ruby2_keywords by default, it's used for all methods accepting args splat and not keywords, making this type of issue much more common.
> 
> Do you have any existing real code example where that branch causes problems?

Do you mean other than the example in this ticket?  No.  Do I expect other real world examples like this one exist?  Yes.

> I agree it is more likely to have a few problems similar to this one than explicit `ruby2_keywords`.
> OTOH, it would also make migrating thousands of delegation calls (i.e., all delegation cases passing kwargs) so much easier.
> It's a trade-off. I'd take one case like this needing `, **{})` over finding where to add 1000 explicit `ruby2_keywords` any day.

It is a trade-off.  I agree that in the majority of cases, `ruby2_keywords` by default does what you want.  However, I don't think that's worth the cases where it doesn't, such as this one.  A codebase that does not use `ruby2_keywords` would never be subject to these issues, but if you have `ruby2_keywords` by default, you can be subject to these issues even if you never use `ruby2_keywords` in any code.

> > This is by design.  Empty keyword splats are equivalent to passing no arguments, just as empty regular splats are equivalent to passing no arguments.  I consider the Ruby <2.7 behavior a bug in this regard.  When you consider that `**{}` never passed arguments (was removed by the parser), I think the intended behavior for empty keyword splats was not to pass arguments.
> 
> Yes, I understand this point of view. But the fact is when calling a method not accepting keyword arguments, with syntactic keywords, there is inconsistency between empty keyword arguments and non-empty as shown above.
> That's clearly not ideal and makes thinking about `**` complicated because it might pass a positional argument or not.
> It's something we could have avoided by raising if the method does not accept keyword arguments (but that's probably too incompatible).

I disagree that it is "clearly not ideal".  I think it makes the most sense and is the ideal behavior.  Conceptually, an empty keyword splat is no arguments, unlike an empty positional hash, which is a single empty argument. Likewise, a non-empty keyword splat is some arguments.  For a method that does not accept keywords, any keyword arguments are combined into a single hash, if any keyword arguments are provided.  The following two cases should be treated the same (assume `foo` does not accept keywords):

```
foo()
foo(**{})
```

In both cases, no keywords are passed to foo.

> > I think the current behavior of always removing empty keyword splats (including implicit splats of empty flagged hashes) makes much more sense.
> 
> It's also the only construct in Ruby where a Hash argument can fully disappear.

When you do `foo(**{})`, you can consider the hash disappearing. So treating `foo(*[empty_flagged_hash])` the same is consistent.

> I'd argue if the user passed keyword arguments (e.g., with `**h`), even empty they should be preserved and not ignored, especially when calling a method not accepting keyword arguments.
> I think removing it is counter-intuitive in some cases: for example it causes these weird cases for `p(**empty_hash)` which prints nothing when `p(**non_empty)` prints as expected.

`p(**empty_hash)` prints a newline, the same as `p()`, which is what I would expect when passing no keyword arguments.

----------------------------------------
Bug #16519: pp [Hash.ruby2_keywords_hash({})] shows `[nil]`
https://bugs.ruby-lang.org/issues/16519#change-84031

* Author: Eregon (Benoit Daloze)
* Status: Closed
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.8.0dev (2020-01-21T13:45:10Z master 5798d35ff6) [x86_64-linux]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: REQUIRED
----------------------------------------
This happens on `master`:

```
$ ruby -ve 'ruby2_keywords def flag(*a); a.last; end; pp [flag(**{})]'
ruby 2.8.0dev (2020-01-21T13:45:10Z master 5798d35ff6) [x86_64-linux]
[nil]
```

Of course it should be `[{}]`, as it is for `pp [{}]`.

On 2.7.0 it warns (should be fixed, it's valid to `pp` a flagged Hash):
```
$ ruby -ve 'ruby2_keywords def flag(*a); a.last; end; pp [flag(**{})]'
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-linux]
[/home/eregon/.rubies/ruby-2.7.0/lib/ruby/2.7.0/pp.rb:226: warning: Passing the keyword argument as the last hash parameter is deprecated
/home/eregon/.rubies/ruby-2.7.0/lib/ruby/2.7.0/pp.rb:334: warning: The called method is defined here
{}]
```

The warning being in the middle of the output is a fun fact here.
Lines it refers to (still the same on current master):
https://github.com/ruby/ruby/blob/v2_7_0/lib/pp.rb#L226
https://github.com/ruby/ruby/blob/v2_7_0/lib/pp.rb#L334

This is very confusing as it can happen during `test-all` and then show output such as:
```
<[{:a=>1}]> expected but was
<[{:a=>1}, nil]>.
```
when the reality is (can be verified with `p` before the `assert_equal`):
```
<[{:a=>1}]> expected but was
<[{:a=>1}, {}]>.
```



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

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