search

google / cel-spec

Common Expression Language -- specification and binary representation
https://cel.dev
Apache License 2.0
2.59k stars 216 forks source link

Should `optional.none()` == `null` ? #361

Open dmateusp opened 3 weeks ago

dmateusp commented 3 weeks ago

I've been trying Optionals through cel-go.

It's confusing as a user to have equality checking optional.none() against null return false. This is the current implementation in cel-go and it seems "correct" looking at the spec.

However, for a user of CEL, not intimate with the internals of the language, would it make more sense to have optional.none() == null return true?

I'm planning on exposing CEL to non-technical users and I feel like this would be a common culprit.

TristonianJones commented 3 weeks ago

Hi @dmateusp,

For legacy reasons, the types are comparable; however, I don't think they should be and I would prefer to restrict the comparison. CEL should be null hostile since null can really only occur when the value comes from JSON or from a protobuf wrapper type such as google.protobuf.Int32Value. I may add a restriction as one of the canonical, extended validations.

That said, we've had a different conversation about optional.of(null) == optional.none(), which we feel is a change we need to make as well.

dmateusp commented 3 weeks ago

thanks for the clarifications! That makes a lot of sense, it'd be great if the compiler returned something like:

cannot compare optional and null, use .hasValue() instead for example

--

I may add a restriction as one of the canonical, extended validations.

Awesome! I was unaware of the "extended validations"