Skip to content

Commit d19e782

Browse files
TristonianJonesTristonianJones
authored
Support zero-value literals in presence test inlining and fix shadowing bugs (#1280)
1 parent 7c461fc commit d19e782

2 files changed

Lines changed: 358 additions & 10 deletions

File tree

‎cel/inlining.go‎

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,38 @@ func (opt *inliningOptimizer) rewritePresenceExpr(ctx *OptimizerContext, prev, i
178178
))
179179
return
180180
}
181+
if zeroValExpr, ok := zeroValueExpr(ctx, inlinedType); ok {
182+
ctx.UpdateExpr(prev,
183+
ctx.NewCall(operators.NotEquals,
184+
inlined, zeroValExpr))
185+
return
186+
}
181187
ctx.ReportErrorAtID(prev.ID(), "unable to inline expression type %v into presence test", inlinedType)
182188
}
183189

190+
// zeroValueExpr creates an expression representing the empty or zero value for the given type
191+
// Note: bytes, lists, maps, and strings are supported via the `SizerType` trait.
192+
func zeroValueExpr(ctx *OptimizerContext, t *Type) (ast.Expr, bool) {
193+
// Note: bytes, strings, lists, and maps are covered by the "sizer-type" check
194+
switch t.Kind() {
195+
case types.BoolKind:
196+
return ctx.NewLiteral(types.False), true
197+
case types.DoubleKind:
198+
return ctx.NewLiteral(types.Double(0)), true
199+
case types.DurationKind:
200+
return ctx.NewCall(overloads.TypeConvertDuration, ctx.NewLiteral(types.String("0s"))), true
201+
case types.IntKind:
202+
return ctx.NewLiteral(types.IntZero), true
203+
case types.TimestampKind:
204+
return ctx.NewCall(overloads.TypeConvertTimestamp, ctx.NewLiteral(types.Int(0))), true
205+
case types.StructKind:
206+
return ctx.NewStruct(t.TypeName(), []ast.EntryExpr{}), true
207+
case types.UintKind:
208+
return ctx.NewLiteral(types.Uint(0)), true
209+
}
210+
return nil, false
211+
}
212+
184213
// isBindable indicates whether the inlined type can be used within a cel.bind() if the expression
185214
// being replaced occurs within a presence test. Value types with a size() method or field selection
186215
// support can be bound.
@@ -212,17 +241,43 @@ func isBindable(matches []ast.NavigableExpr, inlined ast.Expr, inlinedType *Type
212241
// field selection. This may be a future refinement.
213242
func (opt *inliningOptimizer) matchVariable(varName string) ast.ExprMatcher {
214243
return func(e ast.NavigableExpr) bool {
215-
if e.Kind() == ast.IdentKind && e.AsIdent() == varName {
216-
return true
244+
name, found := maybeAsVariableName(e)
245+
if !found || name != varName {
246+
return false
247+
}
248+
249+
// Determine whether the variable being referenced has been shadowed by a comprehension
250+
p, hasParent := e.Parent()
251+
for hasParent {
252+
if p.Kind() != ast.ComprehensionKind {
253+
p, hasParent = p.Parent()
254+
continue
255+
}
256+
// If the inline variable name matches any of the comprehension variables at any scope,
257+
// return false as the variable has been shadowed.
258+
compre := p.AsComprehension()
259+
if varName == compre.AccuVar() || varName == compre.IterVar() || varName == compre.IterVar2() {
260+
return false
261+
}
262+
p, hasParent = p.Parent()
217263
}
218-
if e.Kind() == ast.SelectKind {
219-
sel := e.AsSelect()
220-
// While the `ToQualifiedName` call could take the select directly, this
221-
// would skip presence tests from possible matches, which we would like
222-
// to include.
223-
qualName, found := containers.ToQualifiedName(sel.Operand())
224-
return found && qualName+"."+sel.FieldName() == varName
264+
265+
return true
266+
}
267+
}
268+
269+
func maybeAsVariableName(e ast.NavigableExpr) (string, bool) {
270+
if e.Kind() == ast.IdentKind {
271+
return e.AsIdent(), true
272+
}
273+
if e.Kind() == ast.SelectKind {
274+
sel := e.AsSelect()
275+
// While the `ToQualifiedName` call could take the select directly, this
276+
// would skip presence tests from possible matches, which we would like
277+
// to include.
278+
if qualName, found := containers.ToQualifiedName(sel.Operand()); found {
279+
return qualName + "." + sel.FieldName(), true
225280
}
226-
return false
227281
}
282+
return "", false
228283
}

‎cel/inlining_test.go‎

Lines changed: 293 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,299 @@ import (
2222
proto3pb "github.com/google/cel-go/test/proto3pb"
2323
)
2424

25+
func TestInliningOptimizerNoopShadow(t *testing.T) {
26+
type varExpr struct {
27+
name string
28+
alias string
29+
t *cel.Type
30+
expr string
31+
}
32+
tests := []struct {
33+
name string
34+
expr string
35+
vars []varExpr
36+
inlined string
37+
}{
38+
{
39+
name: "shadow at parent",
40+
expr: `[0].exists(shadowed_ident, shadowed_ident == 0)`,
41+
vars: []varExpr{
42+
{
43+
name: "shadowed_ident",
44+
t: cel.IntType,
45+
expr: "1",
46+
},
47+
},
48+
inlined: `[0].exists(shadowed_ident, shadowed_ident == 0)`,
49+
},
50+
{
51+
name: "shadow in ancestor",
52+
expr: `[[1]].all(shadowed_ident, shadowed_ident.all(shadowed, shadowed + 1 == 2))`,
53+
vars: []varExpr{
54+
{
55+
name: "shadowed_ident",
56+
t: cel.IntType,
57+
expr: "42",
58+
},
59+
},
60+
inlined: `[[1]].all(shadowed_ident, shadowed_ident.all(shadowed, shadowed + 1 == 2))`,
61+
},
62+
}
63+
for _, tst := range tests {
64+
tc := tst
65+
t.Run(tc.name, func(t *testing.T) {
66+
opts := []cel.EnvOption{
67+
cel.OptionalTypes(),
68+
cel.EnableMacroCallTracking(),
69+
}
70+
varDecls := make([]cel.EnvOption, len(tc.vars))
71+
for i, v := range tc.vars {
72+
varDecls[i] = cel.Variable(v.name, v.t)
73+
}
74+
e, err := cel.NewEnv(append(varDecls, opts...)...)
75+
if err != nil {
76+
t.Fatalf("NewEnv() failed: %v", err)
77+
}
78+
inlinedVars := []*cel.InlineVariable{}
79+
for _, v := range tc.vars {
80+
if v.expr == "" {
81+
continue
82+
}
83+
checked, iss := e.Compile(v.expr)
84+
if iss.Err() != nil {
85+
t.Fatalf("Compile(%q) failed: %v", v.expr, iss.Err())
86+
}
87+
if v.alias == "" {
88+
inlinedVars = append(inlinedVars, cel.NewInlineVariable(v.name, checked))
89+
} else {
90+
inlinedVars = append(inlinedVars, cel.NewInlineVariableWithAlias(v.name, v.alias, checked))
91+
}
92+
}
93+
checked, iss := e.Compile(tc.expr)
94+
if iss.Err() != nil {
95+
t.Fatalf("Compile() failed: %v", iss.Err())
96+
}
97+
opt, err := cel.NewStaticOptimizer(cel.NewInliningOptimizer(inlinedVars...))
98+
if err != nil {
99+
t.Fatalf("NewStaticOptimizer() failed: %v", err)
100+
}
101+
optimized, iss := opt.Optimize(e, checked)
102+
if iss.Err() != nil {
103+
t.Fatalf("Optimize() generated an invalid AST: %v", iss.Err())
104+
}
105+
inlined, err := cel.AstToString(optimized)
106+
if err != nil {
107+
t.Fatalf("cel.AstToString() failed: %v", err)
108+
}
109+
if inlined != tc.inlined {
110+
t.Errorf("inlined got %q, wanted %q", inlined, tc.inlined)
111+
}
112+
})
113+
}
114+
}
115+
116+
func TestInliningOptimizerPresenceTests(t *testing.T) {
117+
type varExpr struct {
118+
name string
119+
alias string
120+
t *cel.Type
121+
expr string
122+
}
123+
tests := []struct {
124+
name string
125+
expr string
126+
vars []varExpr
127+
inlined string
128+
}{
129+
{
130+
name: "presence with bool literal",
131+
expr: `has(msg.single_any.processing_purpose)`,
132+
vars: []varExpr{
133+
{
134+
name: "msg.single_any.processing_purpose",
135+
t: cel.BoolType,
136+
expr: "true",
137+
},
138+
},
139+
inlined: `true != false`,
140+
},
141+
{
142+
name: "presence with bytes literal",
143+
expr: `has(msg.single_any.processing_purpose)`,
144+
vars: []varExpr{
145+
{
146+
name: "msg.single_any.processing_purpose",
147+
t: cel.BytesType,
148+
expr: "b'abc'",
149+
},
150+
},
151+
inlined: `b"\141\142\143".size() != 0`,
152+
},
153+
{
154+
name: "presence with double literal",
155+
expr: `has(msg.single_any.processing_purpose)`,
156+
vars: []varExpr{
157+
{
158+
name: "msg.single_any.processing_purpose",
159+
t: cel.DoubleType,
160+
expr: "42.0",
161+
},
162+
},
163+
inlined: `42.0 != 0.0`,
164+
},
165+
{
166+
name: "presence with duration literal",
167+
expr: `has(msg.single_any.processing_purpose)`,
168+
vars: []varExpr{
169+
{
170+
name: "msg.single_any.processing_purpose",
171+
t: cel.DurationType,
172+
expr: "duration('1s')",
173+
},
174+
},
175+
inlined: `duration("1s") != duration("0s")`,
176+
},
177+
{
178+
name: "presence with int literal",
179+
expr: `has(msg.single_any.processing_purpose)`,
180+
vars: []varExpr{
181+
{
182+
name: "msg.single_any.processing_purpose",
183+
t: cel.IntType,
184+
expr: "1",
185+
},
186+
},
187+
inlined: `1 != 0`,
188+
},
189+
{
190+
name: "presence with list literal",
191+
expr: `has(msg.single_any.processing_purpose)`,
192+
vars: []varExpr{
193+
{
194+
name: "msg.single_any.processing_purpose",
195+
t: cel.ListType(cel.StringType),
196+
expr: "['foo', 'bar']",
197+
},
198+
},
199+
inlined: `["foo", "bar"].size() != 0`,
200+
},
201+
{
202+
name: "presence with map literal",
203+
expr: `has(msg.single_any.processing_purpose)`,
204+
vars: []varExpr{
205+
{
206+
name: "msg.single_any.processing_purpose",
207+
t: cel.MapType(cel.StringType, cel.StringType),
208+
expr: "{'foo': 'bar'}",
209+
},
210+
},
211+
inlined: `{"foo": "bar"}.size() != 0`,
212+
},
213+
{
214+
name: "presence with string literal",
215+
expr: `has(msg.single_any.processing_purpose)`,
216+
vars: []varExpr{
217+
{
218+
name: "msg.single_any.processing_purpose",
219+
t: cel.StringType,
220+
expr: "'foo'",
221+
},
222+
},
223+
inlined: `"foo".size() != 0`,
224+
},
225+
{
226+
name: "presence with struct literal",
227+
expr: `has(msg.single_any.processing_purpose)`,
228+
vars: []varExpr{
229+
{
230+
name: "msg.single_any.processing_purpose",
231+
t: cel.ObjectType("google.expr.proto3.test.TestAllTypes"),
232+
expr: "TestAllTypes{single_int64: 1}",
233+
},
234+
},
235+
inlined: `google.expr.proto3.test.TestAllTypes{single_int64: 1} != google.expr.proto3.test.TestAllTypes{}`,
236+
},
237+
{
238+
name: "presence with timestamp literal",
239+
expr: `has(msg.single_any.processing_purpose)`,
240+
vars: []varExpr{
241+
{
242+
name: "msg.single_any.processing_purpose",
243+
t: cel.TimestampType,
244+
expr: "timestamp(123)",
245+
},
246+
},
247+
inlined: `timestamp(123) != timestamp(0)`,
248+
},
249+
{
250+
name: "presence with uint literal",
251+
expr: `has(msg.single_any.processing_purpose)`,
252+
vars: []varExpr{
253+
{
254+
name: "msg.single_any.processing_purpose",
255+
t: cel.UintType,
256+
expr: "1u",
257+
},
258+
},
259+
inlined: `1u != 0u`,
260+
},
261+
}
262+
for _, tst := range tests {
263+
tc := tst
264+
t.Run(tc.name, func(t *testing.T) {
265+
opts := []cel.EnvOption{
266+
cel.Container("google.expr.proto3.test"),
267+
cel.Types(&proto3pb.TestAllTypes{}),
268+
cel.Variable("msg", cel.ObjectType("google.expr.proto3.test.TestAllTypes")),
269+
cel.OptionalTypes(),
270+
cel.EnableMacroCallTracking(),
271+
}
272+
varDecls := make([]cel.EnvOption, len(tc.vars))
273+
for i, v := range tc.vars {
274+
varDecls[i] = cel.Variable(v.name, v.t)
275+
}
276+
e, err := cel.NewEnv(append(varDecls, opts...)...)
277+
if err != nil {
278+
t.Fatalf("NewEnv() failed: %v", err)
279+
}
280+
inlinedVars := []*cel.InlineVariable{}
281+
for _, v := range tc.vars {
282+
if v.expr == "" {
283+
continue
284+
}
285+
checked, iss := e.Compile(v.expr)
286+
if iss.Err() != nil {
287+
t.Fatalf("Compile(%q) failed: %v", v.expr, iss.Err())
288+
}
289+
if v.alias == "" {
290+
inlinedVars = append(inlinedVars, cel.NewInlineVariable(v.name, checked))
291+
} else {
292+
inlinedVars = append(inlinedVars, cel.NewInlineVariableWithAlias(v.name, v.alias, checked))
293+
}
294+
}
295+
checked, iss := e.Compile(tc.expr)
296+
if iss.Err() != nil {
297+
t.Fatalf("Compile() failed: %v", iss.Err())
298+
}
299+
opt, err := cel.NewStaticOptimizer(cel.NewInliningOptimizer(inlinedVars...))
300+
if err != nil {
301+
t.Fatalf("NewStaticOptimizer() failed: %v", err)
302+
}
303+
optimized, iss := opt.Optimize(e, checked)
304+
if iss.Err() != nil {
305+
t.Fatalf("Optimize() generated an invalid AST: %v", iss.Err())
306+
}
307+
inlined, err := cel.AstToString(optimized)
308+
if err != nil {
309+
t.Fatalf("cel.AstToString() failed: %v", err)
310+
}
311+
if inlined != tc.inlined {
312+
t.Errorf("inlined got %q, wanted %q", inlined, tc.inlined)
313+
}
314+
})
315+
}
316+
}
317+
25318
func TestInliningOptimizer(t *testing.T) {
26319
type varExpr struct {
27320
name string

0 commit comments

Comments
 (0)
⚡