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 string to number coercion more strict #161

Closed JohnBrinkman closed 6 days ago

JohnBrinkman commented 3 months ago

The last number of changes to json-formula have been trending toward being more strict wrt parameter handling and coercions. We throw many more TypeErrors and ExecutionErrors than we used to. This has brought us to a place where there's one remaining coercion that now looks really inconsistent.

The string to number coercion will turn invalid numbers into a zero: "Parse string to a number. If the string is not a well-formed number, will return 0"

Recommendation

JohnBrinkman commented 2 months ago

agreed

vdua commented 2 months ago

Invalid number strings used in numeric comparison operations will cause the comparison to return false. e.g. 4 > "3a" is false (no error)

This means the check (a > b || a == b) will fail meaning the a < b is true. Either we should throw an error that they are not comparable or compare them so that this particular invariant should hold if (a >=b) => true then (a < b) => false.

JohnBrinkman commented 2 months ago

Either we should throw an error that they are not comparable or compare them so that this particular invariant should hold if (a >=b) => true then (a < b) => false.

We can't allow a comparison operator throw an error. There's no precedent for that in any language I'm aware of. A bad comparison needs to be benign.

Our language rules state: "If operands are mixed types, type coercion to number is applied before performing the comparison"

This is consistent with JavaScript:

const a = 10;
const b = "abc";

console.log("a < b", a < b);
console.log("a == b", a == b);
console.log("a > b", a > b);

=>

a < b false
a == b false
a > b false

Admittedly, Excel behaves differently. When it gets different types, it appears to convert both to string. But that doesn't seem better, as the expression: "10" < 100 will return false.

Eswcvlad commented 2 months ago

This is consistent with JavaScript

To be more precise it is consistent with IEEE 754, as non-number strings in comparisons with numbers now behave like NaN. For better or for worse. :)

We can't allow a comparison operator throw an error. There's no precedent for that in any language I'm aware of.

Well it is the case in Python, which is dynamically and strongly typed.

>>> 'abc' < 10
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '<' not supported between instances of 'str' and 'int'

But because of strong types, you don't have auto string to number conversions either, so it is not fully comparable.