Project

General

Profile

Actions

Bug #20180

closed

Inconsistent evaluation of `**{}` depending on position in array

Added by ozydingo (Andrew Schwartz) 10 months ago. Updated 5 months ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:116182]

Description

Reproduced on ruby:3.3 docker container

The evaluation of **{} differs if it appears alone (evaluates as empty / no content) in an array vs after another element (evaluates as an empty Hash).

args = []; kwargs = {}
[*args]
# => []
[**kwargs]
# => []
[*args, **kwargs]
# => [{}]
[*args] + [**kwargs] == [*args, **kwargs]
=> false

I claim this violates the Principle of Least Surprise. I will admit that beyond a thin example I will give below, I am struggling to come up with a more convincing pragmatic reason that this should be addressed, but due to how surprising it is and the bugs it cause our team I wanted to submit it for tracking. This may be related to https://bugs.ruby-lang.org/issues/20064?tab=notes though the issues are distinct.

Specifically, in my use case, I am writing a class responsible for adapting arguments in one form to another (json object to method args and vice versa). My tests broken when adding support for keyword args due to expectations of no args:

# RSpec
expect(foo).to have_received(:bar).with([]) # This now need `with([], {})`

Again, this is a bit thin as by itself this isn't problematic as I can add the {}. However, this does require that my test knows more about the implementation that it should. That is, my implementation might be

if kwargs.present?
  call(*args, **kwargs)
else
  call(*args)
end

This change does not change the behavior of the class, but will break the test. I therefore think this behavior of **kwargs is problematic.


Related issues 1 (0 open1 closed)

Related to Ruby master - Bug #20229: Empty keyword splat in array not removed in ARGSPUSH caseClosedActions

Updated by jeremyevans0 (Jeremy Evans) 10 months ago

I submitted a pull request to fix this: https://github.com/ruby/ruby/pull/9624

Updated by byroot (Jean Boussier) 10 months ago

  • Backport changed from 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN to 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED

Seems like this behavior changed in 2.7.

Actions #3

Updated by jeremyevans0 (Jeremy Evans) 9 months ago

  • Related to Bug #20229: Empty keyword splat in array not removed in ARGSPUSH case added

Updated by jeremyevans0 (Jeremy Evans) 9 months ago

  • Status changed from Open to Closed

Fixed by 77c1233f79a0f96a081b70da533fbbde4f3037fa. However, that adds an instruction, so the implementation is not backportable. To backport, use the change in https://github.com/ruby/ruby/pull/9624

Updated by k0kubun (Takashi Kokubun) 5 months ago

  • Status changed from Closed to Open

As per #note-4, I tried to backport https://github.com/ruby/ruby/commit/e199f5fe07f350434cc843bf52caa20f1b1ca755 (part of https://github.com/ruby/ruby/pull/9624), but it also conflicts. Could you file a backport PR to ruby_3_3?

Actions #6

Updated by k0kubun (Takashi Kokubun) 5 months ago

  • Status changed from Open to Closed

Updated by k0kubun (Takashi Kokubun) 5 months ago

  • Backport changed from 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED to 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE

Updated by nagachika (Tomoyuki Chikanaga) 5 months ago

  • Backport changed from 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE to 3.0: REQUIRED, 3.1: REQUIRED, 3.2: DONE, 3.3: DONE
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0