ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:71798] [Ruby trunk - Bug #11762] [Open] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
@ 2015-12-02  7:38 ` colin
  2015-12-05 16:32 ` [ruby-core:71847] [Ruby trunk - Bug #11762] " colin
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: colin @ 2015-12-02  7:38 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been reported by Colin Kelley.

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: 
* ruby -v: 2.3.0-preview1
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:71847] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
  2015-12-02  7:38 ` [ruby-core:71798] [Ruby trunk - Bug #11762] [Open] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer colin
@ 2015-12-05 16:32 ` colin
  2015-12-06  5:11 ` [ruby-core:71852] " ruby-core
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: colin @ 2015-12-05 16:32 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Colin Kelley.


> I think it should return nil in this case.

Anyone else have an opinion?

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55253

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: 
* ruby -v: 2.3.0-preview1
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:71852] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
  2015-12-02  7:38 ` [ruby-core:71798] [Ruby trunk - Bug #11762] [Open] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer colin
  2015-12-05 16:32 ` [ruby-core:71847] [Ruby trunk - Bug #11762] " colin
@ 2015-12-06  5:11 ` ruby-core
  2015-12-07  7:06 ` [ruby-core:71877] [Ruby trunk - Bug #11762] [Rejected] " matz
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: ruby-core @ 2015-12-06  5:11 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Marc-Andre Lafortune.

Assignee set to Yukihiro Matsumoto

I feel that either `dig` should be safe only against `nil` somewhere in the digging path (a bit like `&.`), or it's should always safe, even when digging through unexpected objects or indices.

Currently:

    {a: 'hello'}.dig(:a, :b) # => nil
    {a: []}.dig(:a, :b) # => TypeError

I believe they should either both raise an error, or both return `nil`.

I'm unsure as to the best solution. I'd guess, like Colin, that returning `nil` is probably the best. Makes debugging harder when writing new code, but makes backward compatibility easier, since old code using `dig` wouldn't bomb if the data layout changes in the future.

Matz, what do you think about this?


diff --git a/object.c b/object.c
index ff2db0b..c863fbe 100644
--- a/object.c
+++ b/object.c
@@ -3184,7 +3184,12 @@ rb_obj_dig(int argc, VALUE *argv, VALUE obj, VALUE notfound)
                break;
              case T_ARRAY:
                if (dig_basic_p(obj, &ary)) {
-                   obj = rb_ary_at(obj, *argv);
+                   VALUE index = rb_check_to_int(*argv);
+                   if (NIL_P(index)) {
+                       obj = Qnil;
+                   } else {
+                       obj = rb_ary_at(obj, index);
+                   }
                    continue;
                }
                break;
diff --git a/test/ruby/test_array.rb b/test/ruby/test_array.rb
index 0922cb4..b117a7e 100644
--- a/test/ruby/test_array.rb
+++ b/test/ruby/test_array.rb
@@ -2655,6 +2655,7 @@ def test_dig
     h = @cls[@cls[{a: 1}], 0]
     assert_equal(1, h.dig(0, 0, :a))
     assert_nil(h.dig(1, 0))
+    assert_nil(h.dig(0, :foo))
   end
 
   private


----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55257

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview1
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?



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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [ruby-core:71877] [Ruby trunk - Bug #11762] [Rejected] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (2 preceding siblings ...)
  2015-12-06  5:11 ` [ruby-core:71852] " ruby-core
@ 2015-12-07  7:06 ` matz
  2015-12-08  1:33 ` [ruby-core:71925] [Ruby trunk - Bug #11762] [Open] " ruby-core
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: matz @ 2015-12-07  7:06 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Yukihiro Matsumoto.

Status changed from Open to Rejected

I believe `dig` should only ignore nil receiver as its description.
Hiding argument/type error is not a good idea, I think.

Matz.


----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55280

* Author: Colin Kelley
* Status: Rejected
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview1
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:71925] [Ruby trunk - Bug #11762] [Open] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (3 preceding siblings ...)
  2015-12-07  7:06 ` [ruby-core:71877] [Ruby trunk - Bug #11762] [Rejected] " matz
@ 2015-12-08  1:33 ` ruby-core
  2015-12-08  1:35 ` [ruby-core:71926] [Ruby trunk - Bug #11762] " colin
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: ruby-core @ 2015-12-08  1:33 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Marc-Andre Lafortune.

Status changed from Rejected to Open

Reopening, since \the following should raise an error:

    {a: 'hello'}.dig(:a, :b) # => nil, should raise an Error

Right?

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55330

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview1
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:71926] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (4 preceding siblings ...)
  2015-12-08  1:33 ` [ruby-core:71925] [Ruby trunk - Bug #11762] [Open] " ruby-core
@ 2015-12-08  1:35 ` colin
  2015-12-08  3:09 ` [ruby-core:71929] " ruby-core
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: colin @ 2015-12-08  1:35 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Colin Kelley.


> I'd guess, like Colin, that returning nil is probably the best. Makes debugging harder when writing new code, but makes backward compatibility easier, since old code using dig wouldn't bomb if the data layout changes in the future.

Yes, exactly. If we have to wrap the navigation with `rescue`, then we don't really need `dig` at all; we always had the option to wrap our collection navigating code with a rescue.

~~~
hash = {a: []}
result = hash.dig(:a, :b) rescue nil
~~~

would be about the same as what we can do now without `dig`:

~~~
result = hash[:a][:b] rescue nil
~~~

I think we need `dig` to navigate into a nested collection whose layout is not certain and return `nil` if the value wasn't found.  This is essentially extending the `Hash#[]` behavior to nested collections that are so common now.

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55331

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview1
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:71929] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (5 preceding siblings ...)
  2015-12-08  1:35 ` [ruby-core:71926] [Ruby trunk - Bug #11762] " colin
@ 2015-12-08  3:09 ` ruby-core
  2015-12-08  4:03 ` [ruby-core:71935] " colin
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: ruby-core @ 2015-12-08  3:09 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Marc-Andre Lafortune.


The current doc also gives examples that should return raise an error, if I understand correctly:

    a = [[1, [2, 3]]]
    a.dig(0, 0, 0)                    #=> nil 

Since `1.dig(0)` is invalid (and 1 is not `nil`), that should raise a `NoMethodError, undefined method `dig' for 1:Fixnum`, right?

Matz, could you confirm this?

Note: I'm updating the documentation a bit, so I've changed this example for now. I'll change it back if need be.

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55335

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview1
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:71935] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (6 preceding siblings ...)
  2015-12-08  3:09 ` [ruby-core:71929] " ruby-core
@ 2015-12-08  4:03 ` colin
  2015-12-12  0:06 ` [ruby-core:72068] " ruby-core
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: colin @ 2015-12-08  4:03 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Colin Kelley.

File 11762.patch added

Here is my suggested documentation on how `Hash#dig` should behave as part of a general `dig` protocol.  The patch includes equivalent changes to the documentation for `Array#dig`.

~~~
 * Extracts a nested value by navigating the given +key+
 * (and keys, recursively).  This is useful for navigating
 * parsed JSON or other nested data.  Nested data may be any
 * object that implements the +dig+ method.
 *
 * Calls the +[]+ operator to look up the key so that
 * subclasses may provide their own implementation.
 *
 * Returns the nested value, or nil if a key is not found
 * at any level, or if a nested key navigates to an object that
 * does not implement +dig+.  Does not raise exceptions.
 *
 *  == The dig Protocol
 *
 * The +dig+ method behaves like this at each level:
 *
 *    def dig(key, *keys)
 *      value = self[key] rescue nil
 *      if keys.empty? || value.nil?
 *        value
 *      elsif value.respond_to?(:dig)
 *        value.dig(*keys)
 *      else
 *        nil
 *      end
 *    end
 *
 *  == Example Usage
 *
 *    Address = Struct.new(:street, :city, :state, :country)
 *
 *    hash = {ceo: {name: "Pat", email: "pat@example.com"},
 *            managers: [
 *                {name: "Jose", email: "jose@example.com"},
 *                {name: "Sally", email: "sally@example.com"}
 *                {name: "Bob", email: "bob@example.com"}
 *            ],
 *            office: Address.new("9 Oak St", "New York", "NY", "USA")
 *           }
 *
 *    hash.dig(:ceo, :name)           #=> "Pat"
 *    hash.dig(:ceo, 0, :email)       #=> nil
 *    hash.dig(:managers, 1, :name)   #=> "Sally"
 *    hash.dig(:managers, :name)      #=> nil
 *    hash.dig(:office, :city)        #=> "New York"
~~~

This example is designed to call attention to the common case where you intended to access a hash but an array was there:

~~~
 *    hash.dig(:managers, :name)      #=> nil
~~~

I feel the `dig` method would be much less useful if a surprise as shown above were to raise an exception. (That would force a rescue wrapper. Furthermore, note that the exception would be of limited value since it would be ambiguous as to what level it occurred at.)

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55340

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview1
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:72068] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (7 preceding siblings ...)
  2015-12-08  4:03 ` [ruby-core:71935] " colin
@ 2015-12-12  0:06 ` ruby-core
  2015-12-12  1:40 ` [ruby-core:72071] " matz
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: ruby-core @ 2015-12-12  0:06 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Marc-Andre Lafortune.

ruby -v changed from 2.3.0-preview1 to 2.3.0-preview2

Matz, please confirm we should change `{a: 'hello'}.dig(:a, :b)` from returning `nil` to raising an error, since you said only `nil` should be protected against.

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55477

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:72071] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (8 preceding siblings ...)
  2015-12-12  0:06 ` [ruby-core:72068] " ruby-core
@ 2015-12-12  1:40 ` matz
  2015-12-12 21:16 ` [ruby-core:72086] " ruby-core
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: matz @ 2015-12-12  1:40 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Yukihiro Matsumoto.


Hmm,

`{a: 'hello'}.dig(:a, :b)` should raise TypeError since a string do not have method `dig`.

Matz.


----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55483

* Author: Colin Kelley
* Status: Open
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:72086] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (9 preceding siblings ...)
  2015-12-12  1:40 ` [ruby-core:72071] " matz
@ 2015-12-12 21:16 ` ruby-core
  2015-12-12 22:19 ` [ruby-core:72087] " colin
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: ruby-core @ 2015-12-12 21:16 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Marc-Andre Lafortune.


Yukihiro Matsumoto wrote:
> Hmm,
> 
> `{a: 'hello'}.dig(:a, :b)` should raise TypeError since a string do not have method `dig`.

Thanks Matz

I don't understand why it's would be a `TypeError`. I thought it would be a `NoMethodError` (or alternatively an `ArgumentError`, as reducing the number of arguments could make that error disappear).

My understanding was that a `TypeError` occurs iff an argument of an unexpected type occurs, but the error won't go away by changing the type of any argument.

Are you sure that `TypeError` is the best error to raise in this case?

Thanks

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55499

* Author: Colin Kelley
* Status: Closed
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:72087] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (10 preceding siblings ...)
  2015-12-12 21:16 ` [ruby-core:72086] " ruby-core
@ 2015-12-12 22:19 ` colin
  2015-12-13 15:51 ` [ruby-core:72099] " matz
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: colin @ 2015-12-12 22:19 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Colin Kelley.


As described by Matz, it sounds like the implementation would be equivalent to

~~~
def dig(key, *keys)
  value = self[key]        # may raise TypeError
  if keys.empty? || value.nil?
    value
  else
    value.respond_to?(:dig) or raise TypeError, "object does not respond to dig"
    value.dig(*keys)
  end
end
~~~

But I agree with Marc-Andre that a `NoMethodError` contract would be more natural as it removes the special case:

~~~
def dig(key, *keys)
  value = self[key]        # may raise TypeError
  if keys.empty? || value.nil?
    value
  else
    value.dig(*keys)
  end
end
~~~

That is,

~~~
hash.dig(a, b, c...)
~~~

would be exactly equivalent to

~~~
hash[a]&.[](b)&.[](c)...
~~~

However I must say I am disappointed that `dig` will raise exceptions.  Given that definition it won't be useful for digging in JSON responses whose format has not already been validated.  We can already write

~~~
hash[a][b][c] rescue nil
~~~

So is `dig` worth adding to Ruby at all, given the definition that raises exceptions?  It doesn't seem so to me.

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55500

* Author: Colin Kelley
* Status: Closed
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:72099] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (11 preceding siblings ...)
  2015-12-12 22:19 ` [ruby-core:72087] " colin
@ 2015-12-13 15:51 ` matz
  2015-12-13 20:13 ` [ruby-core:72101] " colin
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: matz @ 2015-12-13 15:51 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Yukihiro Matsumoto.


Suppose you have a Hash/Array tree from JSON, with some attributes optional, you can expect three types of results;

(1) a value from valid tree structure
(2) nil from optional attributes
(3) exception from invalid tree

If #dig returns nil instead of exception, as you want, we cannot distinguish case 2 and case 3.

Matz.


----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55515

* Author: Colin Kelley
* Status: Closed
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:72101] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (12 preceding siblings ...)
  2015-12-13 15:51 ` [ruby-core:72099] " matz
@ 2015-12-13 20:13 ` colin
  2015-12-22 16:49 ` [ruby-core:72438] " colin
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 17+ messages in thread
From: colin @ 2015-12-13 20:13 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Colin Kelley.


> If #dig returns nil instead of exception, as you want, we cannot distinguish case 2 and case 3.

I've looked at a lot of JSON parsing code in Ruby and haven't found examples that were looking to draw that distinction.  (Those that do would probably use a gem like [ruby-json-schema](:https://github.com/ruby-json-schema/json-schema) to validate the precise format and get a precise exception.)

What I have found is code that navigates the nested JSON response and ignores the possibility of the exception `NoMethodError: undefined method '[]' for nil:NilClass` or `TypeError: no implicit conversion of Symbol into Integer` being raised during the navigation.  I looked at several examples in my company's codebase and the first two pages of results from the Google search "ruby json parse example" and all of them suffered from the problem.

Here is the first example from [Stackoverflow](http://stackoverflow.com/questions/5410682/parsing-a-json-string-in-ruby):

~~~
parsed = JSON.parse(string) # returns a hash

p parsed["desc"]["someKey"]
p parsed["main_item"]["stats"]["a"]
~~~

and one from [Temboo](https://temboo.com/ruby/parsing-json):

~~~
title = data["items"][0]["snippet"]["title"]
description = data["items"][0]["snippet"]["description"]
~~~

and one from [richonrails](https://richonrails.com/articles/parsing-json-in-ruby-on-rails):

~~~
h["results"][0]["name"] # => "ABC Widget 2.0"
~~~

and one from [developer.yahoo](https://developer.yahoo.com/ruby/ruby-json.html):

~~~
irb(main):003:0> news['ResultSet']['Result'].each { |result|
irb(main):004:1* print "#{result['Title']} => #{result['Url']}\n"
irb(main):005:1> }
~~~

and one from [ruby-forum](https://www.ruby-forum.com/topic/4411887):

~~~
  adbreaks = parsed['TOD']['AdBreaks']['AdBreak']
  lengths_of_breaks = adbreaks.map {|ad| ad['duration'] }
  list_of_30s_breaks = adbreaks.select {|ad| ad['duration'] == 30 }

... [and later from the original poster]

I am attempting to do the following:
uri = parsed['TOD']['AdBreaks']['AdBreak'][0]['Ad'][0]['MediaFile']['Impression']['Log'][0]['uri']

but it does not work. I get the following:  NoMethodError: undefined method `[]' for nil:NilClass
~~~

I couldn't find a single example in my sample that navigates nested JSON without stepping into the `undefined method '[]' for nil:NilClass` trap.  We dread that exception because it never has enough detail to debug the problem.

That's why I'm so eager to see `dig` unify the navigation to a single contract.  It could raise an explicit exception.  But I think it would be more idiomatic Ruby to return  `nil` if the value is not found.  This makes it natural for the typical calling case where the data is considered optional:

~~~
if a = parsed["main_item"]["stats"]["a"]
  ... # use a
end
~~~

or to raise an exception itself:

~~~
a = parsed["main_item"]["stats"]["a"] or raise "unexpected format #{parsed.inspect}"
~~~


----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55517

* Author: Colin Kelley
* Status: Closed
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:72438] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (13 preceding siblings ...)
  2015-12-13 20:13 ` [ruby-core:72101] " colin
@ 2015-12-22 16:49 ` colin
  2016-01-30  2:38 ` [ruby-core:73592] " andrew
  2016-01-30  2:57 ` [ruby-core:73594] " matz
  16 siblings, 0 replies; 17+ messages in thread
From: colin @ 2015-12-22 16:49 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Colin Kelley.


Hi Matz, do you have any reactions to the above?

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-55734

* Author: Colin Kelley
* Status: Closed
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:73592] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (14 preceding siblings ...)
  2015-12-22 16:49 ` [ruby-core:72438] " colin
@ 2016-01-30  2:38 ` andrew
  2016-01-30  2:57 ` [ruby-core:73594] " matz
  16 siblings, 0 replies; 17+ messages in thread
From: andrew @ 2016-01-30  2:38 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Andrew Vit.


Yukihiro Matsumoto wrote:
> If #dig returns nil instead of exception, as you want, we cannot distinguish case 2 and case 3.

In that case, how about `dig` vs. `dig!` ?

----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-56787

* Author: Colin Kelley
* Status: Closed
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ruby-core:73594] [Ruby trunk - Bug #11762] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
       [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
                   ` (15 preceding siblings ...)
  2016-01-30  2:38 ` [ruby-core:73592] " andrew
@ 2016-01-30  2:57 ` matz
  16 siblings, 0 replies; 17+ messages in thread
From: matz @ 2016-01-30  2:57 UTC (permalink / raw
  To: ruby-core

Issue #11762 has been updated by Yukihiro Matsumoto.


Andrew,

I don't think `dig!` is a good name, because `!` usually denotes dangerous version of a method in Ruby naming convention.

Colin,

Thank you for the investigation. Your survey means most code does not consider exceptional cases. So `dig` should be, I think.
If you really want `nil` from corrupted tree, just add `rescue nil` after `dig` call. It's much better, I think, because we can't distinguish optional value and corrupted tree once we give `nil` from `dig` for both cases.

Matz.


----------------------------------------
Bug #11762: Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer
https://bugs.ruby-lang.org/issues/11762#change-56789

* Author: Colin Kelley
* Status: Closed
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v: 2.3.0-preview2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
If you try to `dig` in an Array using a symbol or string, a `TypeError` exception will be raised:

irb> ['zero', 'one', 'two'].dig(:first)
TypeError: no implicit conversion of Symbol into Integer
    from (irb):1:in `dig'
    from (irb):1

I think it should return `nil` in this case.  The most typical use case for `dig` is to dig through parsed JSON and either find the result we expected or else `nil`.  Wouldn't it defeat the purpose of `dig` if we had to wrap calls to it in a `rescue` to handle the case that an Array was present where we expected a Hash?

Can we clarify the desired behavior for this case, then update the documentation and tests to reflect that?

---Files--------------------------------
11762.patch (3.19 KB)


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-01-30  2:23 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <redmine.issue-11762.20151202073849@ruby-lang.org>
2015-12-02  7:38 ` [ruby-core:71798] [Ruby trunk - Bug #11762] [Open] Array#dig can raise TypeError: no implicit conversion of Symbol/String into Integer colin
2015-12-05 16:32 ` [ruby-core:71847] [Ruby trunk - Bug #11762] " colin
2015-12-06  5:11 ` [ruby-core:71852] " ruby-core
2015-12-07  7:06 ` [ruby-core:71877] [Ruby trunk - Bug #11762] [Rejected] " matz
2015-12-08  1:33 ` [ruby-core:71925] [Ruby trunk - Bug #11762] [Open] " ruby-core
2015-12-08  1:35 ` [ruby-core:71926] [Ruby trunk - Bug #11762] " colin
2015-12-08  3:09 ` [ruby-core:71929] " ruby-core
2015-12-08  4:03 ` [ruby-core:71935] " colin
2015-12-12  0:06 ` [ruby-core:72068] " ruby-core
2015-12-12  1:40 ` [ruby-core:72071] " matz
2015-12-12 21:16 ` [ruby-core:72086] " ruby-core
2015-12-12 22:19 ` [ruby-core:72087] " colin
2015-12-13 15:51 ` [ruby-core:72099] " matz
2015-12-13 20:13 ` [ruby-core:72101] " colin
2015-12-22 16:49 ` [ruby-core:72438] " colin
2016-01-30  2:38 ` [ruby-core:73592] " andrew
2016-01-30  2:57 ` [ruby-core:73594] " matz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).