adobe / json-formula

Query language for JSON documents
http://opensource.adobe.com/json-formula/
Apache License 2.0
19 stars 8 forks source link

Dynamic typing of `name` in deepScan #151

Closed Eswcvlad closed 4 months ago

Eswcvlad commented 5 months ago

I don't think this is specifically type coercion from spec, but more of JavaScript indexing implementation leaking a bit... So I wanted to mention this separately.

As far as I can tell, something like deepScan(x, 1) and deepScan(x, "1") are totally equivalent, regardless of what x is. I.e. name is treated as both string key "1" and 1 index for arrays.

At the same time deepScan(x, 1.1) is different and only works as string key "1.1" and is not trimmed to integer for arrays (I would guess after typing changes it should only be 1 as array index).

Examples:

deepScan({'1': [{'1': 22}, {'1': 55}]}, 1) -> [[{"1": 22}, {"1": 55}], 22, {"1": 55}, 55]
deepScan({'1': [{'1': 22}, {'1': 55}]}, "1") -> [[{"1": 22}, {"1": 55}], 22, {"1": 55}, 55]
deepScan({'1.1': [{'1.1': 22}, {'1.1': 55}]}, 1.1) -> [[{"1.1": 22}, {"1.1": 55}], 22, 55]
JohnBrinkman commented 5 months ago

Hmm. I suppose we can lean into our new rule that we do not coerce when multiple parameter types are allowed. In that case, a second parameter that's 1.1 would fail with a TypeError, as it's neither a string or integer.

Eswcvlad commented 5 months ago

In that case, a second parameter that's 1.1 would fail with a TypeError, as it's neither a string or integer.

Not sure if it is a good idea... I would, personally, leave number -> integer intact, as it is hard to be strict like that with floats...

JohnBrinkman commented 5 months ago

I would... leave number -> integer intact

But that leaves us with an ambiguous coercion. 1.1 can be coerced to either string or integer.

Eswcvlad commented 5 months ago

Well, personally, I wasn't thinking of integer as a specific type, but just number with its fractional part not being used by the function. As more of a marker that the fractional part is not relevant.

JohnBrinkman commented 4 months ago

I will change deepScan() so that it name is a number, we match values only on nested arrays. If it's a string we'll match on nested objects