Skip to content

Commit

Permalink
Add syntax for escaped field selectors. (#1002)
Browse files Browse the repository at this point in the history
Add syntax for escaped field selectors.

Introduces syntax for escaped fields that are not normally
syntactically legal field selections with backticks.

This PR is limited to fields, but would also consider adding for namespaced
names (e.g. `in.example.Foo`) in a later update.

* Changes applied from intial draft feedback:

- Reduce set of allowed escaped identifiers
- Additional testing
  • Loading branch information
jnthntatum authored Dec 16, 2024
1 parent 8b07a00 commit a108e9e
Show file tree
Hide file tree
Showing 22 changed files with 1,828 additions and 1,571 deletions.
47 changes: 47 additions & 0 deletions cel/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2171,6 +2171,53 @@ func TestParserRecursionLimit(t *testing.T) {
}
}

func TestQuotedFields(t *testing.T) {
testCases := []struct {
expr string
errorSubstr string
out ref.Val
}{
{
expr: "{'key-1': 64}.`key-1`",
out: types.Int(64),
},
{
expr: "{'key-1': 64}.`key-2`",
errorSubstr: "no such key: key-2",
},
{
expr: "has({'key-1': 64}.`key-1`)",
out: types.True,
},
{
expr: "has({'key-1': 64}.`key-2`)",
out: types.False,
},
}
for _, tc := range testCases {
tc := tc
t.Run(tc.expr, func(t *testing.T) {
env := testEnv(t, ParserRecursionLimit(10),
EnableIdentifierEscapeSyntax())
out, err := interpret(t, env,
tc.expr, map[string]any{})

if tc.errorSubstr != "" {
if err == nil || !strings.Contains(err.Error(), tc.errorSubstr) {
t.Fatalf("prg.Eval() wanted error containing '%s' got %v", tc.errorSubstr, err)
}
}

if tc.out != nil {
if tc.out != out {
t.Errorf("prg.Eval() wanted %v got %v", tc.out, out)
}
}
})

}
}

func TestDynamicDispatch(t *testing.T) {
env := testEnv(t,
HomogeneousAggregateLiterals(),
Expand Down
3 changes: 3 additions & 0 deletions cel/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,9 @@ func (e *Env) configure(opts []EnvOption) (*Env, error) {
if e.HasFeature(featureVariadicLogicalASTs) {
prsrOpts = append(prsrOpts, parser.EnableVariadicOperatorASTs(true))
}
if e.HasFeature(featureIdentEscapeSyntax) {
prsrOpts = append(prsrOpts, parser.EnableIdentEscapeSyntax(true))
}
e.prsr, err = parser.NewParser(prsrOpts...)
if err != nil {
return nil, err
Expand Down
9 changes: 9 additions & 0 deletions cel/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ const (
// Enable error generation when a presence test or optional field selection is
// performed on a primitive type.
featureEnableErrorOnBadPresenceTest

// Enable escape syntax for field identifiers (`).
featureIdentEscapeSyntax
)

// EnvOption is a functional interface for configuring the environment.
Expand Down Expand Up @@ -618,6 +621,12 @@ func EnableMacroCallTracking() EnvOption {
return features(featureEnableMacroCallTracking, true)
}

// EnableIdentifierEscapeSyntax enables identifier escaping (`) syntax for
// fields.
func EnableIdentifierEscapeSyntax() EnvOption {
return features(featureIdentEscapeSyntax, true)
}

// CrossTypeNumericComparisons makes it possible to compare across numeric types, e.g. double < int
func CrossTypeNumericComparisons(enabled bool) EnvOption {
return features(featureCrossTypeNumericComparisons, enabled)
Expand Down
19 changes: 19 additions & 0 deletions ext/protos_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,21 @@ func TestProtos(t *testing.T) {
{expr: `proto.getExt(ExampleType{}, google.expr.proto2.test.nested_example) == ExampleType{}`},
{expr: `proto.getExt(ExampleType{}, google.expr.proto2.test.ExtendedExampleType.extended_examples) == []`},
{expr: `proto.getExt(ExampleType{}, google.expr.proto2.test.ExtendedExampleType.enum_ext) == GlobalEnum.GOO`},
{expr: "ExampleType{`in`: 64}.`in` == 64"},
// TODO(issue/1095): legal parse, but can't assign extension fields without updates to type checker and runtime
// ExampleType{`google.expr.proto2.test.int32_ext`: 42}.`google.expr.proto2.test.int32_ext` == 42
{expr: `proto.getExt(msg, google.expr.proto2.test.int32_ext) == 42`},
{expr: "msg.`google.expr.proto2.test.int32_ext` == 42"},
{expr: `proto.getExt(msg, google.expr.proto2.test.int32_wrapper_ext) == 21`},
{expr: "msg.`google.expr.proto2.test.int32_wrapper_ext` == 21"},
{expr: `proto.hasExt(msg, google.expr.proto2.test.nested_example)`},
{expr: "has(msg.`google.expr.proto2.test.nested_example`)"},
{expr: `proto.getExt(msg, google.expr.proto2.test.nested_example) == ExampleType{name: 'nested'}`},
{expr: "msg.`google.expr.proto2.test.nested_example` == ExampleType{name: 'nested'}"},
{expr: `proto.getExt(msg, google.expr.proto2.test.ExtendedExampleType.extended_examples) == ['example1', 'example2']`},
{expr: "msg.`google.expr.proto2.test.ExtendedExampleType.extended_examples` == ['example1', 'example2']"},
{expr: `proto.getExt(msg, google.expr.proto2.test.ExtendedExampleType.enum_ext) == GlobalEnum.GAZ`},
{expr: "msg.`google.expr.proto2.test.ExtendedExampleType.enum_ext` == GlobalEnum.GAZ"},
}

env := testProtosEnv(t)
Expand Down Expand Up @@ -174,6 +183,15 @@ func TestProtosParseErrors(t *testing.T) {
| proto.getExt(ExtendedExampleType{}, has(google.expr.proto2.test.int32_ext))
| .......................................^`,
},
{
expr: `ExampleType{}.in`,
err: `ERROR: <input>:1:15: Syntax error: no viable alternative at input '.in'
| ExampleType{}.in
| ..............^
ERROR: <input>:1:17: Syntax error: mismatched input '<EOF>' expecting {'[', '{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, NUM_UINT, STRING, BYTES, IDENTIFIER}
| ExampleType{}.in
| ................^`,
},
}
env := testProtosEnv(t)
for i, tst := range protosTests {
Expand All @@ -198,6 +216,7 @@ func testProtosEnv(t *testing.T, opts ...cel.EnvOption) *cel.Env {
cel.Container("google.expr.proto2.test"),
cel.Types(&proto2pb.ExampleType{}, &proto2pb.ExternalMessageType{}),
cel.Variable("msg", cel.ObjectType("google.expr.proto2.test.ExampleType")),
cel.EnableIdentifierEscapeSyntax(),
Protos(),
}
env, err := cel.NewEnv(append(baseOpts, opts...)...)
Expand Down
13 changes: 10 additions & 3 deletions parser/gen/CEL.g4
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,14 @@ unary

member
: primary # PrimaryExpr
| member op='.' (opt='?')? id=IDENTIFIER # Select
| member op='.' (opt='?')? id=escapeIdent # Select
| member op='.' id=IDENTIFIER open='(' args=exprList? ')' # MemberCall
| member op='[' (opt='?')? index=expr ']' # Index
;

primary
: leadingDot='.'? id=IDENTIFIER (op='(' args=exprList? ')')? # IdentOrGlobalCall
: leadingDot='.'? id=IDENTIFIER # Ident
| leadingDot='.'? id=IDENTIFIER (op='(' args=exprList? ')') # GlobalCall
| '(' e=expr ')' # Nested
| op='[' elems=listInit? ','? ']' # CreateList
| op='{' entries=mapInitializerList? ','? '}' # CreateStruct
Expand All @@ -80,13 +81,18 @@ fieldInitializerList
;

optField
: (opt='?')? IDENTIFIER
: (opt='?')? escapeIdent
;

mapInitializerList
: keys+=optExpr cols+=':' values+=expr (',' keys+=optExpr cols+=':' values+=expr)*
;

escapeIdent
: id=IDENTIFIER # SimpleIdentifier
| id=ESC_IDENTIFIER # EscapedIdentifier
;

optExpr
: (opt='?')? e=expr
;
Expand Down Expand Up @@ -198,3 +204,4 @@ STRING
BYTES : ('b' | 'B') STRING;

IDENTIFIER : (LETTER | '_') ( LETTER | DIGIT | '_')*;
ESC_IDENTIFIER : '`' (LETTER | DIGIT | '_' | '.' | '-' | '/' | ' ')+ '`';
5 changes: 4 additions & 1 deletion parser/gen/CEL.interp

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions parser/gen/CEL.tokens
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ NUM_UINT=33
STRING=34
BYTES=35
IDENTIFIER=36
ESC_IDENTIFIER=37
'=='=1
'!='=2
'in'=3
Expand Down
Loading

0 comments on commit a108e9e

Please sign in to comment.