number and bigint comparision

[frunzikk]

overloads.js

line: 79

for (const t of ['type', 'null', 'bool', 'string', 'int', 'double']) {
    binaryOverload(t, '==', t, (a, b) => a === b)
}

I think it's better like this, to keep comparision of number and bigint

for (const t of ['type', 'null', 'bool', 'string', 'int', 'double']) {
    binaryOverload(t, '==', t, (a, b) => isEqual(a, b))
}

[lazharichir]

agreed and perhaps have a === for type and value checks

[marcbachmann]

Can you provide me a test that's failing or causing issues?

cel-js behaves the same as cel-go here:
https://playceljs.sksop.in/?state=eyJleHByZXNzaW9uIjoiMSA9PSBkeW4oMS4wKSIsInZhcmlhYmxlcyI6IiMgSGVyZSBpcyB0aGUgaW5wdXQgZGF0YSBpbiBZQU1MIG9yIEpTT04gZm9ybWF0LlxuXG5hY2NvdW50OlxuICBiYWxhbmNlOiA1MDBcbiAgb3ZlcmRyYWZ0UHJvdGVjdGlvbjogdHJ1ZVxuICBvdmVyZHJhZnRMaW1pdDogMTAwMFxudHJhbnNhY3Rpb246XG4gIHdpdGhkcmF3YWw6IDcwMFxuIiwidmVyc2lvbiI6IjYuMy4yIn0%3D

https://playcel.undistro.io/?content=H4sIAAAAAAAAA1WPPQvCMBCG%2F8oRFwWRdBAh0MFNpX6Ak9DlTK400F4kvVpE%2FO8mODndx%2FMcvPdWljplVAFlCe7F82KlF2qpHAru%2BTFKYjPYUSTwA0ibSt5C5qmF2%2FZYQYhwuJ5P0ITYo6xqrhmtDSOLqRngjh2yJQNrrfMcnhRdxEYuMQhZ8YENSBzpD1a%2B92Kg0PlIIvKAPzVbk5c2WRN2BjZZSJH74Cilzf98vpJ8QmLXAAAA

[frunzikk]

We are checking int type to be in types array, but internally it parsed as bigint, so we are checking 1 === 1n.
isEqual is more flexible in comparison between number and bigint

[marcbachmann]

Do you have example expressions of what you're trying?
All those work as expected according to the common expression language spec 🤔

    test('should work with dyn double or int in list', () => {
      const ctx = {i: 2n, d: 2.0, doubles: [1.0, 2.0, 3.0], ints: [1n, 2n, 3n]}
      expectEval('d in doubles', true, ctx)
      expectEval('d in ints', true, ctx)
      expectEval('i in doubles', true, ctx)

      expectEval('2 in doubles', true, ctx)
      expectEval('2 in ints', true, ctx)
      expectEval('2.0 in doubles', true, ctx)
      expectEval('2.0 in ints', true, ctx)

      expectEval('d in [1, 2, 3]', true, ctx)
      expectEval('dyn(2.0) in [1, 2, 3]', true, ctx)
      expectEval('2.0 in dyn([1, 2, 3])', true, ctx)
      expectEval('2.0 in [dyn(1), dyn(2), dyn(3)]', true, ctx)
      expectEval('dyn(4.0) in [1, 2, 3]', false, ctx)
    })

Also, just changing the comparison to isEqual won't change the behavior. (as long as no other overload is registered). And common expression language usually does not support int == double comparisons unless one of them are dyn.

[frunzikk]

Expression: “day == 1”
Context: {day: 1} // not 1n
Expression eval if false, because of types number and bigint (as parsed internally), but isEqual handles this moment

[marcbachmann]

I still think this library has the intentional behavior according to the common expression language spec. We could consider supporting number type declarations on variables (which would then either be dyn<int> or dyn<double> and therefore the checks would work as you want, but I'll investigate first what cel-go supports there.

agreed and perhaps have a === for type and value checks

I really don't want diverge from the common expression language spec. So a === operator won't land for now.

[marcbachmann]

While cel-spec disallows heterogenous equality (different types), you can register an overload by yourself.
But then it's not conforming to the spec anymore.

    const env = new TestEnvironment()
    env.registerOperator('int + double: double', (a, b) => Number(a) + b)
    env.registerOperator('int == double: bool', (a, b) => Number(a) === b)
    env.registerFunction('equals(int, double): bool', (a, b) => Number(a) === b)
    env.registerFunction('equals(double, int): bool', (a, b) => a === Number(b))
    env.eval('1 + 2.5') // 3.5
    env.eval('2 == 2.0') // true
    env.eval('equals(2, 2.0)')  // true
    env.eval('equals(2.0, 2)')  // true

[marcbachmann]

As described above, there's are two workarounds to get the expected behavior.

• Either teach users to write double == 1.0
• Or register an overload if it's ok for you to not conform to the spec

I'll close this issue for now till somebody has some ideas or suggestions for improvements.

Here's also an explanation about the behavior: google/cel-go#203