google / cel-spec

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

Syntax: has with map keys #166

Closed kyessenov closed 3 years ago

kyessenov commented 3 years ago

CEL treats map lookups and field dereferences similarly in many ways, except for has() macro. In some cases, we have to use bracket notation because the key has illegal ident characters. This prevents the use of has macro in such keys. For example:

has(request.headers['x-clientid'])

Because headers use dashes, we cannot use field notation.

The workaround is using in operator, but I wonder if permitting has(a[f]) would be better.

TristonianJones commented 3 years ago

@kyessenov you're not the only person to wonder about this. The has() macro sets a TestOnly bit on Select expressions, and the indexed expression won't work in this way. Semantically, we could convert an index expression to in under the covers has(request.headers[headerParam]) which would make it more useful.

The major risk would be that you can disable the in operator and get a check-time or runtime error. @JimLarson thoughts on expanding the scope of has() for indexed operations? From a parse, check, runtime standpoint there is no change apart from the minor drawback I mentioned for custom environments that disable in.

kyessenov commented 3 years ago

It might be OK to permit only constant string indexes and translate to Select during parsing.

JimLarson commented 3 years ago

I'd be against expanding the has() macro, for two reasons:

We allow has(e.f) for maps only because we want all forms like e.f to be treated the same. But e[k] looks different and I don't mind if it's treated differently for presence testing.

JimLarson commented 3 years ago

Hearing no objection, closing.