Bug #16643: Array#dig converts keywords to positional hash

• Status: Rejected
• Assignee: -
• Target version: -
• ruby -v: ruby 2.7.0p0 (2019-12-25 revision a65e8644fb) [x86_64-linux]
• Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN

[Dan0042 (Daniel DeLorme)]

The following behavior for dig looks very weird to me:

o=Object.new
def o.dig(**kw) p [kw] end
o.dig(x:1)     
#no warning
#[{:x=>1}]
[o].dig(0, x:1)
#warning: Using the last argument as keyword parameters is deprecated
#[{:x=>1}]

o=Object.new
def o.dig(a, **kw) p [a,kw] end
o.dig(x:1)
#warning: Passing the keyword argument as the last hash parameter is deprecated
#[{:x=>1}, {}]
[o].dig(0, x:1)
#no warning
#[{:x=>1}, {}]

So the keywords are interpreted as a positional hash, resulting in the opposite of the behavior I would expect.

This would be easy to fix by changing rb_obj_dig to use

        return rb_check_funcall_with_hook_kw(obj, id_dig, argc, argv,
                                             no_dig_method, obj,
                                             RB_PASS_CALLED_KEYWORDS);

What this just an oversight? I can't think of a rationale for the current behavior.

[jeremyevans0 (Jeremy Evans)]

• Status changed from Open to Rejected

This behavior is deliberate. dig should only be overridden if you want to be able to access objects stored inside the object. dig should accept *args, using the first element to look inside the object, and passing the remaining arguments to dig on that object (or just returning that object if there are no remaining arguments).

The issue with trying to support keyword arguments in dig is that it would break cases where you are looking up objects via a hash. For example, this code without keyword argument returns the expected output:

a = Object.new
def a.dig(arg, *args)
  obj = send(arg)
  return obj if args.empty?
  obj.dig(*args)
end
def a.b
  [{{c: 1}=>2}]
end
[[a]].dig(0, 0, :b, 0, c: 1)
# => 2

Using keyword arguments breaks things in 2.7, but you get a warning that behavior changes in 3.0:

a = Object.new
def a.dig(arg, *args, **kw)
  obj = send(arg)
  return obj if args.empty?
  obj.dig(*args)
end
def a.b
  [{{c: 1}=>2}]
end
[[a]].dig(0, 0, :b, 0, c: 1)
# keyword argument warning in 2.7
# {{:c=>1}=>2}

Note that in Ruby 3.0, you get the expected value (2), even if you have dig accept keyword arguments, because the hash is passed as a positional argument and not keywords.

[Dan0042 (Daniel DeLorme)]

Are you seriously telling me that you consider it normal and correct that obj.dig(**kw) is not equivalent to [obj].dig(0, **kw) ???

I will note that your first example produces 2 whether keywords are passed with RB_PASS_CALLED_KEYWORDS or not.

And your second example produces {{:c=>1}=>2} (in 2.6 also) whether keywords are passed with RB_PASS_CALLED_KEYWORDS or not; the only difference is the warning. The reason it produces the "incorrect" result is because the method is buggy, not because the method uses keywords. If you capture the keywords and then throw them away of course the result is incorrect. It should have been something like this:

a = Object.new
def a.dig(arg, *args, **kw)
  obj = send(arg)
  return obj if args.empty? and kw.empty?
  obj.dig(*args, **kw)
end
def a.b
  [{{c: 1}=>2}]
end
[[a]].dig(0, 0, :b, 0, c: 1)
# keyword argument warning in 2.7.0
# 2

And ideally if you want {c: 1} to be used as one of the keys in the (positional) dig chain, it should have been written as {c: 1}, not as a keyword.

I sort of understand your point, that dig is not supposed to accept keyword arguments. And in that case auto-converting to a positional hash makes no difference. But in that case passing it as keywords would also have the same behavior and it would preserve the positional/keyword separation. And you never know, maybe someone will want to use keywords in their custom dig method. Just a thought experiment:

a = Object.new
def a.dig(m, *args, base: nil, **kw)
  respond_to?(m) or return nil
  obj = send(m, base) or return nil
  return obj if args.empty?
  obj.dig(*args, **kw)
end
def a.foo(base=nil)
  [42.to_s(base || 10)]
end
[[a]].dig(0, 0, :foo, 0, base: 13)
# "33"

[jeremyevans0 (Jeremy Evans)]

Dan0042 (Daniel DeLorme) wrote in #note-2:

Are you seriously telling me that you consider it normal and correct that obj.dig(**kw) is not equivalent to [obj].dig(0, **kw) ???

It should be equivalent, if dig is defined correctly. As I mentioned, dig is not supposed to accept keyword arguments. Having dig pass keyword arguments could break cases where dig is defined to accept keyword arguments for other reasons (maybe invoked through method_missing), and I don't think that is a good idea, which is why I had dig treat all arguments as positional.

Note that I rejected this issue because it is not a bug, it is the expected behavior. If you wish to change the behavior, please submit a feature request.

[mame (Yusuke Endoh)]

jeremyevans0 (Jeremy Evans) wrote in #note-3:

Dan0042 (Daniel DeLorme) wrote in #note-2:

Are you seriously telling me that you consider it normal and correct that obj.dig(**kw) is not equivalent to [obj].dig(0, **kw) ???

It should be equivalent, if dig is defined correctly.

I personally think that it should not be equivalent. obj.dig(a, b, c) was introduced as a safe-navigation like obj[a][b][c], so I think that #dig should accept only positional arguments. In fact, we cannot write obj.dig(0, key: 1, 2). So, it looks good to me to reject all keyword arguments for .dig.