Skip to content

Commit

Permalink
Optional entry support for map and message literal construction (#603)
Browse files Browse the repository at this point in the history
* Support for optional field setting on map and message literals. 
* Update references to optional type construction
  • Loading branch information
TristonianJones authored Nov 8, 2022
1 parent 051835c commit 218aabe
Show file tree
Hide file tree
Showing 36 changed files with 1,366 additions and 591 deletions.
12 changes: 6 additions & 6 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ http_archive(
],
)

# googleapis as of 9/9/2022
# googleapis as of 10/31/2022
http_archive(
name = "com_google_googleapis",
sha256 = "1f742f6cafe616fe73302db010e0b7ee6579cb1ce06010427b7d0995cbd80ce4",
strip_prefix = "googleapis-6a813acf535e4746fa4a135ce23547bb6425c26d",
sha256 = "d4f8f9ded24ba62605387b8d327297b9013b4f75214023a18a3ffea43ce61146",
strip_prefix = "googleapis-5ffa37352ced7e6a48148a334c144a1c3a50e8e0",
urls = [
"https://github.com/googleapis/googleapis/archive/6a813acf535e4746fa4a135ce23547bb6425c26d.tar.gz",
"https://github.com/googleapis/googleapis/archive/5ffa37352ced7e6a48148a334c144a1c3a50e8e0.tar.gz",
],
)

Expand Down Expand Up @@ -68,11 +68,11 @@ go_repository(
tag = "v1.28.1",
)

# Generated Google APIs protos for Golang 9/9/2022
# Generated Google APIs protos for Golang 10/27/2022
go_repository(
name = "org_golang_google_genproto",
build_file_proto_mode = "disable_global",
commit = "69f6226f97e558dbaa68715071622af0d86b3a17",
commit = "527a21cfbd71ba0fb6ef7b6a341287e0869af45a",
importpath = "google.golang.org/genproto",
)

Expand Down
90 changes: 86 additions & 4 deletions cel/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1879,11 +1879,14 @@ func TestDynamicDispatch(t *testing.T) {
func TestOptionalValues(t *testing.T) {
env, err := NewEnv(
OptionalTypes(true),
// Container and test message types.
Container("google.expr.proto2.test"),
Types(&proto2pb.TestAllTypes{}),
// Test variables.
Variable("m", MapType(StringType, MapType(StringType, StringType))),
Variable("optm", OptionalType(MapType(StringType, MapType(StringType, StringType)))),
Variable("l", ListType(StringType)),
Variable("optm", OptionalType(MapType(StringType, MapType(StringType, StringType)))),
Variable("optl", OptionalType(ListType(StringType))),

Variable("x", OptionalType(IntType)),
Variable("y", OptionalType(IntType)),
Variable("z", IntType),
Expand Down Expand Up @@ -2097,6 +2100,84 @@ func TestOptionalValues(t *testing.T) {
},
out: 7,
},
{
expr: `{?'nested_map': optional.ofNonZeroValue({?'map': m.?c})}`,
in: map[string]any{
"m": map[string]any{
"c": map[string]string{
"dashed-index": "goodbye",
},
},
},
out: map[string]any{
"nested_map": map[string]any{
"map": map[string]string{
"dashed-index": "goodbye",
},
},
},
},
{
expr: `{?'nested_map': optional.ofNonZeroValue({?'map': m.?c}), 'singleton': true}`,
in: map[string]any{
"m": map[string]any{},
},
out: map[string]any{
"singleton": true,
},
},
{
expr: `optional.ofNonZeroValue({?'nested_map': optional.ofNonZeroValue({?'map': m.?c})})`,
in: map[string]any{
"m": map[string]any{},
},
out: types.OptionalNone,
},
{
expr: `TestAllTypes{?single_double_wrapper: optional.ofNonZeroValue(0.0)}`,
out: &proto2pb.TestAllTypes{},
},
{
expr: `optional.ofNonZeroValue(TestAllTypes{?single_double_wrapper: optional.ofNonZeroValue(0.0)})`,
out: types.OptionalNone,
},
{
expr: `TestAllTypes{
?map_string_string: m[?'nested']
}`,
in: map[string]any{
"m": map[string]any{
"nested": map[string]any{},
},
},
out: &proto2pb.TestAllTypes{},
},
{
expr: `TestAllTypes{
?map_string_string: optional.ofNonZeroValue(m[?'nested'].orValue({}))
}`,
in: map[string]any{
"m": map[string]any{
"nested": map[string]any{},
},
},
out: &proto2pb.TestAllTypes{},
},
{
expr: `TestAllTypes{
?map_string_string: m[?'nested']
}`,
in: map[string]any{
"m": map[string]any{
"nested": map[string]any{
"hello": "world",
},
},
},
out: &proto2pb.TestAllTypes{
MapStringString: map[string]string{"hello": "world"},
},
},
}

for i, tst := range tests {
Expand All @@ -2114,8 +2195,9 @@ func TestOptionalValues(t *testing.T) {
if err != nil && err.Error() != tc.out {
t.Errorf("prg.Eval() got %v, wanted %v", err, tc.out)
}
if err == nil && out.Equal(types.DefaultTypeAdapter.NativeToValue(tc.out)) != types.True {
t.Errorf("prg.Eval() got %v, wanted %v", out, tc.out)
want := adapter.NativeToValue(tc.out)
if err == nil && out.Equal(want) != types.True {
t.Errorf("prg.Eval() got %v, wanted %v", out, want)
}
})
}
Expand Down
53 changes: 34 additions & 19 deletions checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (c *checker) checkSelectField(e, operand *exprpb.Expr, field string, option

// If the target type was optional coming in, then the result must be optional going out.
if isOpt || optional {
return decls.NewAbstractType("optional", resultType)
return decls.NewOptionalType(resultType)
}
return resultType
}
Expand Down Expand Up @@ -429,22 +429,31 @@ func (c *checker) checkCreateStruct(e *exprpb.Expr) {

func (c *checker) checkCreateMap(e *exprpb.Expr) {
mapVal := e.GetStructExpr()
var keyType *exprpb.Type
var valueType *exprpb.Type
var mapKeyType *exprpb.Type
var mapValueType *exprpb.Type
for _, ent := range mapVal.GetEntries() {
key := ent.GetMapKey()
c.check(key)
keyType = c.joinTypes(c.location(key), keyType, c.getType(key))

c.check(ent.Value)
valueType = c.joinTypes(c.location(ent.Value), valueType, c.getType(ent.Value))
mapKeyType = c.joinTypes(c.location(key), mapKeyType, c.getType(key))

val := ent.GetValue()
c.check(val)
valType := c.getType(val)
if ent.GetOptionalEntry() {
var isOptional bool
valType, isOptional = maybeUnwrapOptional(valType)
if !isOptional && !isDyn(valType) {
c.errors.typeMismatch(c.location(val), decls.NewOptionalType(valType), valType)
}
}
mapValueType = c.joinTypes(c.location(val), mapValueType, valType)
}
if keyType == nil {
if mapKeyType == nil {
// If the map is empty, assign free type variables to typeKey and value type.
keyType = c.newTypeVar()
valueType = c.newTypeVar()
mapKeyType = c.newTypeVar()
mapValueType = c.newTypeVar()
}
c.setType(e, decls.NewMapType(keyType, valueType))
c.setType(e, decls.NewMapType(mapKeyType, mapValueType))
}

func (c *checker) checkCreateMessage(e *exprpb.Expr) {
Expand Down Expand Up @@ -486,15 +495,21 @@ func (c *checker) checkCreateMessage(e *exprpb.Expr) {
c.check(value)

fieldType := decls.Error
if t, found := c.lookupFieldType(
c.locationByID(ent.GetId()),
messageType.GetMessageType(),
field); found {
fieldType = t.Type
ft, found := c.lookupFieldType(c.locationByID(ent.GetId()), messageType.GetMessageType(), field)
if found {
fieldType = ft.Type
}

valType := c.getType(value)
if ent.GetOptionalEntry() {
var isOptional bool
valType, isOptional = maybeUnwrapOptional(valType)
if !isOptional && !isDyn(valType) {
c.errors.typeMismatch(c.location(value), decls.NewOptionalType(valType), valType)
}
}
if !c.isAssignable(fieldType, c.getType(value)) {
c.errors.fieldTypeMismatch(
c.locationByID(ent.Id), field, fieldType, c.getType(value))
if !c.isAssignable(fieldType, valType) {
c.errors.fieldTypeMismatch(c.locationByID(ent.Id), field, fieldType, valType)
}
}
}
Expand Down
76 changes: 69 additions & 7 deletions checker/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2043,7 +2043,7 @@ _&&_(_==_(list~type(list(dyn))^list,
decls.NewVar("a", decls.NewMapType(decls.String, decls.String)),
},
},
outType: decls.NewAbstractType("optional", decls.String),
outType: decls.NewOptionalType(decls.String),
out: `_?._(
a~map(string, string)^a,
"b"
Expand All @@ -2053,27 +2053,27 @@ _&&_(_==_(list~type(list(dyn))^list,
in: `a.b`,
env: testEnv{
idents: []*exprpb.Decl{
decls.NewVar("a", decls.NewAbstractType("optional", decls.NewMapType(decls.String, decls.String))),
decls.NewVar("a", decls.NewOptionalType(decls.NewMapType(decls.String, decls.String))),
},
},
outType: decls.NewAbstractType("optional", decls.String),
outType: decls.NewOptionalType(decls.String),
out: `a~optional(map(string, string))^a.b~optional(string)`,
},
{
in: `a.dynamic`,
env: testEnv{
idents: []*exprpb.Decl{
decls.NewVar("a", decls.NewAbstractType("optional", decls.Dyn)),
decls.NewVar("a", decls.NewOptionalType(decls.Dyn)),
},
},
outType: decls.NewAbstractType("optional", decls.Dyn),
outType: decls.NewOptionalType(decls.Dyn),
out: `a~optional(dyn)^a.dynamic~optional(dyn)`,
},
{
in: `has(a.dynamic)`,
env: testEnv{
idents: []*exprpb.Decl{
decls.NewVar("a", decls.NewAbstractType("optional", decls.Dyn)),
decls.NewVar("a", decls.NewOptionalType(decls.Dyn)),
},
},
outType: decls.Bool,
Expand All @@ -2083,7 +2083,7 @@ _&&_(_==_(list~type(list(dyn))^list,
in: `has(a.?b.c)`,
env: testEnv{
idents: []*exprpb.Decl{
decls.NewVar("a", decls.NewAbstractType("optional", decls.NewMapType(decls.String, decls.Dyn))),
decls.NewVar("a", decls.NewOptionalType(decls.NewMapType(decls.String, decls.Dyn))),
},
},
outType: decls.Bool,
Expand All @@ -2092,6 +2092,68 @@ _&&_(_==_(list~type(list(dyn))^list,
"b"
)~optional(dyn).c~test-only~~bool`,
},
{
in: `{?'key': {'a': 'b'}.?value}`,
outType: decls.NewMapType(decls.String, decls.String),
out: `{
?"key"~string:_?._(
{
"a"~string:"b"~string
}~map(string, string),
"value"
)~optional(string)
}~map(string, string)`,
},
{
in: `{?'key': {'a': 'b'}.?value}.key`,
outType: decls.String,
out: `{
?"key"~string:_?._(
{
"a"~string:"b"~string
}~map(string, string),
"value"
)~optional(string)
}~map(string, string).key~string`,
},
{
in: `{?'nested': a.b}`,
env: testEnv{
idents: []*exprpb.Decl{
decls.NewVar("a", decls.NewOptionalType(decls.NewMapType(decls.String, decls.String))),
},
},
outType: decls.NewMapType(decls.String, decls.String),
out: `{
?"nested"~string:a~optional(map(string, string))^a.b~optional(string)
}~map(string, string)`,
},
{
in: `{?'key': 'hi'}`,
err: `ERROR: <input>:1:10: expected type 'optional(string)' but found 'string'
| {?'key': 'hi'}
| .........^`,
},
{
in: `TestAllTypes{?single_int32: {}.?i}`,
container: "google.expr.proto2.test",
out: `google.expr.proto2.test.TestAllTypes{
?single_int32:_?._(
{}~map(dyn, int),
"i"
)~optional(int)
}~google.expr.proto2.test.TestAllTypes^google.expr.proto2.test.TestAllTypes`,
outType: decls.NewObjectType(
"google.expr.proto2.test.TestAllTypes",
),
},
{
in: `TestAllTypes{?single_int32: 1}`,
container: "google.expr.proto2.test",
err: `ERROR: <input>:1:29: expected type 'optional(int)' but found 'int'
| TestAllTypes{?single_int32: 1}
| ............................^`,
},
}

var testEnvs = map[string]testEnv{
Expand Down
8 changes: 7 additions & 1 deletion checker/decls/decls.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
package decls

import (
exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
emptypb "google.golang.org/protobuf/types/known/emptypb"
structpb "google.golang.org/protobuf/types/known/structpb"
exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
)

var (
Expand Down Expand Up @@ -64,6 +64,12 @@ func NewAbstractType(name string, paramTypes ...*exprpb.Type) *exprpb.Type {
ParameterTypes: paramTypes}}}
}

// 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)
}

// NewFunctionType creates a function invocation contract, typically only used
// by type-checking steps after overload resolution.
func NewFunctionType(resultType *exprpb.Type,
Expand Down
6 changes: 6 additions & 0 deletions common/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ func (w *debugWriter) appendObject(obj *exprpb.Expr_CreateStruct) {
w.append(",")
w.appendLine()
}
if entry.GetOptionalEntry() {
w.append("?")
}
w.append(entry.GetFieldKey())
w.append(":")
w.Buffer(entry.GetValue())
Expand All @@ -191,6 +194,9 @@ func (w *debugWriter) appendMap(obj *exprpb.Expr_CreateStruct) {
w.append(",")
w.appendLine()
}
if entry.GetOptionalEntry() {
w.append("?")
}
w.Buffer(entry.GetMapKey())
w.append(":")
w.Buffer(entry.GetValue())
Expand Down
Loading

0 comments on commit 218aabe

Please sign in to comment.