Fix namespaced function resolution #341
Conversation
…s are possible with some refactors
…nt resolution at evaluation time
|
@JimLarson PTAL, the PR now introduces AST rewrites at Check time to drop / consolidate parse-time AST fragments in favor of ASTs that more accurately represent the eval intent. Note, while the AST is modified, the following features have been preserved to ensure that generated ASTs are compatible with existing evaluators:
In most cases, the net effect will be simpler, more consistent evaluation of checked expressions within evaluators, even when the evaluator ignores the checker metadata produced in the reference_map and type_map. Once submitted, I'm going to apply a v0.5.0 release label to indicate the potential for a breaking change in the AST representation. |
|
PTAL whenever you have a chance @JimLarson. Thanks. |
checker/checker.go
Outdated
| } | ||
|
|
||
| func (c *checker) checkIdent(e *exprpb.Expr) { | ||
| // Rewrite the identifier to its fully qualified name. |
There was a problem hiding this comment.
Consider putting this comment at line 150, in the block where we're actually doing this rewrite.
| return | ||
| } | ||
| // Check to see whether the overload resolves. | ||
| call.Function = fn.GetName() |
There was a problem hiding this comment.
Comment "Overwrite function name with fully-qualified resolved name."
* Fix namespaced function resolution * Introduce AST rewrites at Check-time to better handle function / ident resolution at evaluation time
Type checking correctly resolves namespaced function names like
base64.encode; however, this information was being lost at evaluation time with the fragmentbase64being treated as a variable name andencodeas the function name.This PR fixes the glitch for namespaced function resolution ensuring the evaluator honors the CEL program container name and qualified function names.