Skip to content

Commit

Permalink
Fix the optional type name and type identifier resolution (#870)
Browse files Browse the repository at this point in the history
  • Loading branch information
TristonianJones authored Jan 7, 2024
1 parent 7f70747 commit d45f67f
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 20 deletions.
4 changes: 4 additions & 0 deletions cel/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2365,6 +2365,10 @@ func TestOptionalValuesEval(t *testing.T) {
},
out: true,
},
{
expr: `type(optional.none()) == optional_type`,
out: true,
},
{
// return the value of m.c['dashed-index'], no magic in the optional.of() call.
expr: `optional.ofNonZeroValue('').or(optional.of(m.c['dashed-index'])).orValue('default value')`,
Expand Down
55 changes: 40 additions & 15 deletions checker/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2044,7 +2044,26 @@ _&&_(_==_(list~type(list(dyn))^list,
out: `_?._(
a~map(string, string)^a,
"b"
)~optional(string)^select_optional_field`,
)~optional_type(string)^select_optional_field`,
},
{
in: `type(a.?b) == optional_type`,
env: testEnv{
optionalSyntax: true,
idents: []*decls.VariableDecl{
decls.NewVariable("a", types.NewMapType(types.StringType, types.StringType)),
},
},
outType: types.BoolType,
out: `_==_(
type(
_?._(
a~map(string, string)^a,
"b"
)~optional_type(string)^select_optional_field
)~type(optional_type(string))^type,
optional_type~type(optional_type)^optional_type
)~bool^equals`,
},
{
in: `a.b`,
Expand All @@ -2054,7 +2073,7 @@ _&&_(_==_(list~type(list(dyn))^list,
},
},
outType: types.NewOptionalType(types.StringType),
out: `a~optional(map(string, string))^a.b~optional(string)`,
out: `a~optional_type(map(string, string))^a.b~optional_type(string)`,
},
{
in: `a.dynamic`,
Expand All @@ -2064,7 +2083,7 @@ _&&_(_==_(list~type(list(dyn))^list,
},
},
outType: types.NewOptionalType(types.DynType),
out: `a~optional(dyn)^a.dynamic~optional(dyn)`,
out: `a~optional_type(dyn)^a.dynamic~optional_type(dyn)`,
},
{
in: `has(a.dynamic)`,
Expand All @@ -2074,7 +2093,7 @@ _&&_(_==_(list~type(list(dyn))^list,
},
},
outType: types.BoolType,
out: `a~optional(dyn)^a.dynamic~test-only~~bool`,
out: `a~optional_type(dyn)^a.dynamic~test-only~~bool`,
},
{
in: `has(a.?b.c)`,
Expand All @@ -2086,9 +2105,9 @@ _&&_(_==_(list~type(list(dyn))^list,
},
outType: types.BoolType,
out: `_?._(
a~optional(map(string, dyn))^a,
a~optional_type(map(string, dyn))^a,
"b"
)~optional(dyn)^select_optional_field.c~test-only~~bool`,
)~optional_type(dyn)^select_optional_field.c~test-only~~bool`,
},
{
in: `{?'key': {'a': 'b'}.?value}`,
Expand All @@ -2100,7 +2119,7 @@ _&&_(_==_(list~type(list(dyn))^list,
"a"~string:"b"~string
}~map(string, string),
"value"
)~optional(string)^select_optional_field
)~optional_type(string)^select_optional_field
}~map(string, string)`,
},
{
Expand All @@ -2113,7 +2132,7 @@ _&&_(_==_(list~type(list(dyn))^list,
"a"~string:"b"~string
}~map(string, string),
"value"
)~optional(string)^select_optional_field
)~optional_type(string)^select_optional_field
}~map(string, string).key~string`,
},
{
Expand All @@ -2126,13 +2145,13 @@ _&&_(_==_(list~type(list(dyn))^list,
},
outType: types.NewMapType(types.StringType, types.StringType),
out: `{
?"nested"~string:a~optional(map(string, string))^a.b~optional(string)
?"nested"~string:a~optional_type(map(string, string))^a.b~optional_type(string)
}~map(string, string)`,
},
{
in: `{?'key': 'hi'}`,
env: testEnv{optionalSyntax: true},
err: `ERROR: <input>:1:10: expected type 'optional(string)' but found 'string'
err: `ERROR: <input>:1:10: expected type 'optional_type(string)' but found 'string'
| {?'key': 'hi'}
| .........^`,
},
Expand All @@ -2147,15 +2166,15 @@ _&&_(_==_(list~type(list(dyn))^list,
},
outType: types.NewListType(types.StringType),
out: `[
a~optional(string)^a,
b~optional(string)^b,
a~optional_type(string)^a,
b~optional_type(string)^b,
"world"~string
]~list(string)`,
},
{
in: `[?'value']`,
env: testEnv{optionalSyntax: true},
err: `ERROR: <input>:1:3: expected type 'optional(string)' but found 'string'
err: `ERROR: <input>:1:3: expected type 'optional_type(string)' but found 'string'
| [?'value']
| ..^`,
},
Expand All @@ -2167,7 +2186,7 @@ _&&_(_==_(list~type(list(dyn))^list,
?single_int32:_?._(
{}~map(dyn, int),
"i"
)~optional(int)^select_optional_field
)~optional_type(int)^select_optional_field
}~google.expr.proto2.test.TestAllTypes^google.expr.proto2.test.TestAllTypes`,
outType: types.NewObjectType(
"google.expr.proto2.test.TestAllTypes",
Expand All @@ -2177,7 +2196,7 @@ _&&_(_==_(list~type(list(dyn))^list,
in: `TestAllTypes{?single_int32: 1}`,
container: "google.expr.proto2.test",
env: testEnv{optionalSyntax: true},
err: `ERROR: <input>:1:29: expected type 'optional(int)' but found 'int'
err: `ERROR: <input>:1:29: expected type 'optional_type(int)' but found 'int'
| TestAllTypes{?single_int32: 1}
| ............................^`,
},
Expand Down Expand Up @@ -2307,6 +2326,12 @@ func TestCheck(t *testing.T) {
}

reg, err := types.NewRegistry(&proto2pb.TestAllTypes{}, &proto3pb.TestAllTypes{})
if tc.env.optionalSyntax {
err = reg.RegisterType(types.OptionalType)
if err != nil {
t.Fatalf("reg.RegisterType(optional_type) failed: %v", err)
}
}
if err != nil {
t.Fatalf("types.NewRegistry() failed: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion checker/decls/decls.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func NewAbstractType(name string, paramTypes ...*exprpb.Type) *exprpb.Type {
// NewOptionalType constructs an abstract type indicating that the parameterized type
// may be contained within the object.
func NewOptionalType(paramType *exprpb.Type) *exprpb.Type {
return NewAbstractType("optional", paramType)
return NewAbstractType("optional_type", paramType)
}

// NewFunctionType creates a function invocation contract, typically only used
Expand Down
8 changes: 8 additions & 0 deletions checker/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,14 @@ func (e *Env) LookupIdent(name string) *decls.VariableDecl {
return decl
}

if i, found := e.provider.FindIdent(candidate); found {
if t, ok := i.(*types.Type); ok {
decl := decls.NewVariable(candidate, types.NewTypeTypeWithParam(t))
e.declarations.AddIdent(decl)
return decl
}
}

// Next try to import this as an enum value by splitting the name in a type prefix and
// the enum inside.
if enumValue := e.provider.EnumValue(candidate); enumValue.Type() != types.ErrType {
Expand Down
2 changes: 1 addition & 1 deletion checker/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func isError(t *types.Type) bool {

func isOptional(t *types.Type) bool {
if t.Kind() == types.OpaqueKind {
return t.TypeName() == "optional"
return t.TypeName() == "optional_type"
}
return false
}
Expand Down
2 changes: 1 addition & 1 deletion common/types/optional.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (

var (
// OptionalType indicates the runtime type of an optional value.
OptionalType = NewOpaqueType("optional")
OptionalType = NewOpaqueType("optional_type")

// OptionalNone is a sentinel value which is used to indicate an empty optional value.
OptionalNone = &Optional{}
Expand Down
2 changes: 1 addition & 1 deletion common/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ func NewNullableType(wrapped *Type) *Type {

// NewOptionalType creates an abstract parameterized type instance corresponding to CEL's notion of optional.
func NewOptionalType(param *Type) *Type {
return NewOpaqueType("optional", param)
return NewOpaqueType("optional_type", param)
}

// NewOpaqueType creates an abstract parameterized type with a given name.
Expand Down
2 changes: 1 addition & 1 deletion common/types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestTypeString(t *testing.T) {
},
{
in: NewOptionalType(NewListType(StringType)),
out: "optional(list(string))",
out: "optional_type(list(string))",
},
{
in: NewObjectType("my.type.Message"),
Expand Down

0 comments on commit d45f67f

Please sign in to comment.