Skip to content

Commit 3cb5705

Browse files
TristonianJonesTristonianJones
authored
Namespace resolution fix (#1256)
* Remove the treatment of standard identifiers as variables for types * Updates to type resolution rules in accordance with the spec updates
1 parent 409bcbe commit 3cb5705

13 files changed

Lines changed: 694 additions & 76 deletions

‎checker/checker.go‎

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package checker
1919
import (
2020
"fmt"
2121
"reflect"
22+
"slices"
23+
"strings"
2224

2325
"github.com/google/cel-go/common"
2426
"github.com/google/cel-go/common/ast"
@@ -104,11 +106,15 @@ func (c *checker) check(e ast.Expr) {
104106
func (c *checker) checkIdent(e ast.Expr) {
105107
identName := e.AsIdent()
106108
// Check to see if the identifier is declared.
107-
if ident := c.env.LookupIdent(identName); ident != nil {
109+
if ident := c.env.resolveSimpleIdent(identName); ident != nil {
110+
name := strings.TrimPrefix(ident.Name(), ".")
111+
if ident.requiresDisambiguation {
112+
name = "." + name
113+
}
108114
c.setType(e, ident.Type())
109-
c.setReference(e, ast.NewIdentReference(ident.Name(), ident.Value()))
115+
c.setReference(e, ast.NewIdentReference(name, ident.Value()))
110116
// Overwrite the identifier with its fully qualified name.
111-
e.SetKindCase(c.NewIdent(e.ID(), ident.Name()))
117+
e.SetKindCase(c.NewIdent(e.ID(), name))
112118
return
113119
}
114120

@@ -119,18 +125,22 @@ func (c *checker) checkIdent(e ast.Expr) {
119125
func (c *checker) checkSelect(e ast.Expr) {
120126
sel := e.AsSelect()
121127
// Before traversing down the tree, try to interpret as qualified name.
122-
qname, found := containers.ToQualifiedName(e)
128+
qualifiers, found := c.computeQualifiers(e)
123129
if found {
124-
ident := c.env.LookupIdent(qname)
130+
ident := c.env.resolveQualifiedIdent(qualifiers...)
125131
if ident != nil {
126132
// We don't check for a TestOnly expression here since the `found` result is
127133
// always going to be false for TestOnly expressions.
128134

129135
// Rewrite the node to be a variable reference to the resolved fully-qualified
130136
// variable name.
137+
name := ident.Name()
138+
if ident.requiresDisambiguation {
139+
name = "." + name
140+
}
131141
c.setType(e, ident.Type())
132-
c.setReference(e, ast.NewIdentReference(ident.Name(), ident.Value()))
133-
e.SetKindCase(c.NewIdent(e.ID(), ident.Name()))
142+
c.setReference(e, ast.NewIdentReference(name, ident.Value()))
143+
e.SetKindCase(c.NewIdent(e.ID(), name))
134144
return
135145
}
136146
}
@@ -142,6 +152,29 @@ func (c *checker) checkSelect(e ast.Expr) {
142152
c.setType(e, substitute(c.mappings, resultType, false))
143153
}
144154

155+
// computeQualifiers computes the qualified names parts of a select expression.
156+
func (c *checker) computeQualifiers(e ast.Expr) ([]string, bool) {
157+
var qualifiers []string
158+
for e.Kind() == ast.SelectKind {
159+
sel := e.AsSelect()
160+
// test only expressions are not considered for qualified name selection.
161+
if sel.IsTestOnly() {
162+
return qualifiers, false
163+
}
164+
// otherwise append the select field name to the qualifier list (reverse order)
165+
qualifiers = append(qualifiers, sel.FieldName())
166+
e = sel.Operand()
167+
// If the next operand is an identifier, then append it, reverse the name sequence
168+
// and return it to the caller.s
169+
if e.Kind() == ast.IdentKind {
170+
qualifiers = append(qualifiers, e.AsIdent())
171+
slices.Reverse(qualifiers)
172+
return qualifiers, true
173+
}
174+
}
175+
return qualifiers, false
176+
}
177+
145178
func (c *checker) checkOptSelect(e ast.Expr) {
146179
// Collect metadata related to the opt select call packaged by the parser.
147180
call := e.AsCall()
@@ -234,7 +267,7 @@ func (c *checker) checkCall(e ast.Expr) {
234267
// Regular static call with simple name.
235268
if !call.IsMemberFunction() {
236269
// Check for the existence of the function.
237-
fn := c.env.LookupFunction(fnName)
270+
fn := c.env.lookupFunction(fnName)
238271
if fn == nil {
239272
c.errors.undeclaredReference(e.ID(), c.location(e), c.env.container.Name(), fnName)
240273
c.setType(e, types.ErrorType)
@@ -256,7 +289,7 @@ func (c *checker) checkCall(e ast.Expr) {
256289
qualifiedPrefix, maybeQualified := containers.ToQualifiedName(target)
257290
if maybeQualified {
258291
maybeQualifiedName := qualifiedPrefix + "." + fnName
259-
fn := c.env.LookupFunction(maybeQualifiedName)
292+
fn := c.env.lookupFunction(maybeQualifiedName)
260293
if fn != nil {
261294
// The function name is namespaced and so preserving the target operand would
262295
// be an inaccurate representation of the desired evaluation behavior.
@@ -269,7 +302,7 @@ func (c *checker) checkCall(e ast.Expr) {
269302

270303
// Regular instance call.
271304
c.check(target)
272-
fn := c.env.LookupFunction(fnName)
305+
fn := c.env.lookupFunction(fnName)
273306
// Function found, attempt overload resolution.
274307
if fn != nil {
275308
c.resolveOverloadOrError(e, fn, target, args)
@@ -441,7 +474,7 @@ func (c *checker) checkCreateStruct(e ast.Expr) {
441474
msgVal := e.AsStruct()
442475
// Determine the type of the message.
443476
resultType := types.ErrorType
444-
ident := c.env.LookupIdent(msgVal.TypeName())
477+
ident := c.env.resolveTypeIdent(msgVal.TypeName())
445478
if ident == nil {
446479
c.errors.undeclaredReference(
447480
e.ID(), c.location(e), c.env.container.Name(), msgVal.TypeName())

‎checker/checker_test.go‎

Lines changed: 153 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2229,9 +2229,9 @@ _&&_(_==_(list~type(list(dyn))^list,
22292229
decls.NewVariable("NotAMessage", types.NewNullableType(types.IntType)),
22302230
},
22312231
},
2232-
err: `ERROR: <input>:1:12: 'wrapper(int)' is not a type
2233-
| NotAMessage{}
2234-
| ...........^`,
2232+
err: `ERROR: <input>:1:12: undeclared reference to 'NotAMessage' (in container '')
2233+
| NotAMessage{}
2234+
| ...........^`,
22352235
},
22362236
{
22372237
in: `{}.map(c,[c,type(c)])`,
@@ -2262,6 +2262,156 @@ _&&_(_==_(list~type(list(dyn))^list,
22622262
@result~list(list(dyn))^@result)~list(list(dyn))`,
22632263
outType: types.NewListType(types.NewListType(types.DynType)),
22642264
},
2265+
{
2266+
in: `[{'z': 0}].exists(y, y.z == 0)`,
2267+
env: testEnv{
2268+
idents: []*decls.VariableDecl{
2269+
decls.NewVariable("cel.example.y", types.NewMapType(types.StringType, types.IntType)),
2270+
},
2271+
},
2272+
out: `__comprehension__(
2273+
// Variable
2274+
y,
2275+
// Target
2276+
[
2277+
{
2278+
"z"~string:0~int
2279+
}~map(string, int)
2280+
]~list(map(string, int)),
2281+
// Accumulator
2282+
@result,
2283+
// Init
2284+
false~bool,
2285+
// LoopCondition
2286+
@not_strictly_false(
2287+
!_(
2288+
@result~bool^@result
2289+
)~bool^logical_not
2290+
)~bool^not_strictly_false,
2291+
// LoopStep
2292+
_||_(
2293+
@result~bool^@result,
2294+
_==_(
2295+
y~map(string, int)^y.z~int,
2296+
0~int
2297+
)~bool^equals
2298+
)~bool^logical_or,
2299+
// Result
2300+
@result~bool^@result)~bool`,
2301+
outType: types.BoolType,
2302+
},
2303+
{
2304+
in: `[{'y': 0}].exists(x, x.y == 0)`,
2305+
env: testEnv{
2306+
idents: []*decls.VariableDecl{
2307+
decls.NewVariable("x", types.NewMapType(types.StringType, types.IntType)),
2308+
},
2309+
},
2310+
out: `__comprehension__(
2311+
// Variable
2312+
x,
2313+
// Target
2314+
[
2315+
{
2316+
"y"~string:0~int
2317+
}~map(string, int)
2318+
]~list(map(string, int)),
2319+
// Accumulator
2320+
@result,
2321+
// Init
2322+
false~bool,
2323+
// LoopCondition
2324+
@not_strictly_false(
2325+
!_(
2326+
@result~bool^@result
2327+
)~bool^logical_not
2328+
)~bool^not_strictly_false,
2329+
// LoopStep
2330+
_||_(
2331+
@result~bool^@result,
2332+
_==_(
2333+
x~map(string, int)^x.y~int,
2334+
0~int
2335+
)~bool^equals
2336+
)~bool^logical_or,
2337+
// Result
2338+
@result~bool^@result)~bool`,
2339+
outType: types.BoolType,
2340+
},
2341+
{
2342+
in: `[0].exists(x, x != .x)`,
2343+
env: testEnv{
2344+
idents: []*decls.VariableDecl{
2345+
decls.NewVariable("x", types.IntType),
2346+
},
2347+
},
2348+
out: `__comprehension__(
2349+
// Variable
2350+
x,
2351+
// Target
2352+
[
2353+
0~int
2354+
]~list(int),
2355+
// Accumulator
2356+
@result,
2357+
// Init
2358+
false~bool,
2359+
// LoopCondition
2360+
@not_strictly_false(
2361+
!_(
2362+
@result~bool^@result
2363+
)~bool^logical_not
2364+
)~bool^not_strictly_false,
2365+
// LoopStep
2366+
_||_(
2367+
@result~bool^@result,
2368+
_!=_(
2369+
x~int^x,
2370+
.x~int^.x
2371+
)~bool^not_equals
2372+
)~bool^logical_or,
2373+
// Result
2374+
@result~bool^@result)~bool`,
2375+
outType: types.BoolType,
2376+
},
2377+
{
2378+
in: `[{'z': 0}].exists(y, .y.z == y.z)`,
2379+
env: testEnv{
2380+
idents: []*decls.VariableDecl{
2381+
decls.NewVariable("y.z", types.IntType),
2382+
},
2383+
},
2384+
out: `__comprehension__(
2385+
// Variable
2386+
y,
2387+
// Target
2388+
[
2389+
{
2390+
"z"~string:0~int
2391+
}~map(string, int)
2392+
]~list(map(string, int)),
2393+
// Accumulator
2394+
@result,
2395+
// Init
2396+
false~bool,
2397+
// LoopCondition
2398+
@not_strictly_false(
2399+
!_(
2400+
@result~bool^@result
2401+
)~bool^logical_not
2402+
)~bool^not_strictly_false,
2403+
// LoopStep
2404+
_||_(
2405+
@result~bool^@result,
2406+
_==_(
2407+
.y.z~int^.y.z,
2408+
y~map(string, int)^y.z~int
2409+
)~bool^equals
2410+
)~bool^logical_or,
2411+
// Result
2412+
@result~bool^@result)~bool`,
2413+
outType: types.BoolType,
2414+
},
22652415
}
22662416
}
22672417

0 commit comments

Comments
 (0)
⚡