adobe / json-formula

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

Make signatures for value() and hasProperty() consistent #176

Closed JohnBrinkman closed 3 months ago

JohnBrinkman commented 3 months ago

The implementation signature for value() allows the first parameter to be one of: array, object or null -- although the documentation doesn't mention null -- and the null case is not working. hasProperty() allows only array, object for the first parameter.

Allowing null for the first parameter to both functions is useful, as source data can reasonably have an element that is (e.g.) either an object or null.

Additionally: part of the motivation for adding hasProperty() in the first place was for applications that implement hidden properties. e.g. we can check: hasProperty(field, "$name"). In the case where field where field holds a null value, this check should still succeed.

Recommendation

Eswcvlad commented 3 months ago

for both functions, if the first parameter is null, do not validate the type of the second parameter

Wouldn't it be better to still check, that it is either string or number? Null is kind of a corner case for both, so, ultimately, it would fall into one of the normal cases in a valid script. And without implicit conversions validating types is quick and easy anyway.

It would also be a bit odd, when you, for example, pass a lambda to a string | integer parameter and it just works without an error.

JohnBrinkman commented 3 months ago

Wouldn't it be better to still check, that it is either string or number?

Yes, that makes sense.

JohnBrinkman commented 3 months ago

Fixed with: https://github.com/adobe/json-formula/commit/eb9b4c0aa55c47c300175688653331c0b9d9b040