Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash/Array/etc#dig? method are too strict regarding its key type #7570

Open
bcardiff opened this issue Mar 21, 2019 · 4 comments
Open

Hash/Array/etc#dig? method are too strict regarding its key type #7570

bcardiff opened this issue Mar 21, 2019 · 4 comments
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection

Comments

@bcardiff
Copy link
Member

Extracted from #7569

The following examples works currently

h = {"a" => {"b" => [10, 20, 30]}}
h.dig? "a", "b"              # => [10, 20, 30]
h.dig? "a", "x", 0, "d", "e" # => nil
h.dig? "a", "x", 5, "d", "e" # => nil

But this does not

h.dig? "a", "b", "c", "d", "e" # => compile time error

Since the value returned by h.dig? "a", "b" responds to dig? but there is no dig? definition that can handle a String index.

The lack of that definition also prevents the following to work:

h = {"a" => {"b" => [10, 20, 30], "c" => { "d" => { "e" => 40 }}}}
h.dig? "a", "c", "d", "e" # => compile time error
@bcardiff bcardiff added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection labels Mar 21, 2019
@asterite
Copy link
Member

I think that's fine. Why passing a string where an int is expected to silently give nil?

Also, I can't see how this can be implemented without having something like responds_to?(:[], String).

@bew
Copy link
Contributor

bew commented Mar 21, 2019

Hash#dig?:

  def dig?(key : K, *subkeys)
    if (value = self[key]?) && value.responds_to?(:dig?)
      value.dig?(*subkeys)
    end
  end

The problem lies in value.responds_to?(:dig) which returns true, even if the types doesn't match, a workaround would be to loosen the types, but a better fix would be #2549 I think.

@asterite
Copy link
Member

Hm, this works:

h = {"a" => {"b" => "hello world"}}

h.dig?("a", "b", "c", "d", "e") # => nil

I guess dig? is not type-safe at all. So yeah, if we want that, the only way to implement it (I think) is with #2549

@bcardiff
Copy link
Member Author

I think we want dig? to never raise (and always compile).
We can have dig to raise and not always compile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection
Projects
None yet
Development

No branches or pull requests

3 participants