encoding/protobuf: make oneOf fields required
This is not exactly correct, but closer to what is needed
in practice. Once we have closed structs, we can fix
the semantics again.
Change-Id: Ia88cc2971aca330b6edc4113680721f72cf250a2
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/2800
Reviewed-by: Jason Wang <jasonwzm@google.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/encoding/protobuf/parse.go b/encoding/protobuf/parse.go
index 8280f06..9f49916 100644
--- a/encoding/protobuf/parse.go
+++ b/encoding/protobuf/parse.go
@@ -641,9 +641,19 @@
addComments(enum.Value, 1, nil, lastComment)
}
+// oneOf converts a Proto OneOf field to CUE. Note that Protobuf defines
+// a oneOf to be at most one of the fields. Rather than making each field
+// optional, we define oneOfs as all required fields, but add one more
+// disjunction allowing no fields. This makes it easier to constrain the
+// result to include at least one of the values.
func (p *protoConverter) oneOf(x *proto.Oneof) {
f := &ast.Field{
Label: p.ref(x.Position),
+ // TODO: Once we have closed structs, a oneOf is represented as a
+ // disjunction of empty structs and closed structs with required fields.
+ // For now we just specify the required fields. This is not correct
+ // but more practical.
+ // Value: &ast.StructLit{}, // Remove to make at least one required.
}
f.AddComment(comment(x.Comment, true))
@@ -656,7 +666,8 @@
}
switch x := v.(type) {
case *proto.OneOfField:
- p.parseField(s, 0, x.Field)
+ oneOf := p.parseField(s, 0, x.Field)
+ oneOf.Optional = token.NoPos
default:
p.messageField(s, 1, v)
diff --git a/encoding/protobuf/testdata/attributes.proto.out.cue b/encoding/protobuf/testdata/attributes.proto.out.cue
index 73f2cd8..224e2e4 100644
--- a/encoding/protobuf/testdata/attributes.proto.out.cue
+++ b/encoding/protobuf/testdata/attributes.proto.out.cue
@@ -73,28 +73,28 @@
// The attribute value.
Attributes_AttributeValue: {
// Used for values of type STRING, DNS_NAME, EMAIL_ADDRESS, and URI
- stringValue?: string @protobuf(2,name=string_value)
+ stringValue: string @protobuf(2,name=string_value)
} | {
// Used for values of type INT64
- int64Value?: int64 @protobuf(3,name=int64_value)
+ int64Value: int64 @protobuf(3,name=int64_value)
} | {
// Used for values of type DOUBLE
- doubleValue?: float64 @protobuf(4,type=double,name=double_value)
+ doubleValue: float64 @protobuf(4,type=double,name=double_value)
} | {
// Used for values of type BOOL
- boolValue?: bool @protobuf(5,name=bool_value)
+ boolValue: bool @protobuf(5,name=bool_value)
} | {
// Used for values of type BYTES
- bytesValue?: bytes @protobuf(6,name=bytes_value)
+ bytesValue: bytes @protobuf(6,name=bytes_value)
} | {
// Used for values of type TIMESTAMP
- timestampValue?: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
+ timestampValue: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
} | {
// Used for values of type DURATION
- durationValue?: time.Duration @protobuf(8,type=google.protobuf.Duration,name=duration_value)
+ durationValue: time.Duration @protobuf(8,type=google.protobuf.Duration,name=duration_value)
} | {
// Used for values of type STRING_MAP
- stringMapValue?: Attributes_StringMap @protobuf(9,type=StringMap,name=string_map_value)
+ stringMapValue: Attributes_StringMap @protobuf(9,type=StringMap,name=string_map_value)
}
// Defines a string map.
diff --git a/encoding/protobuf/testdata/istio.io/api/mixer/v1/attributes_proto_gen.cue b/encoding/protobuf/testdata/istio.io/api/mixer/v1/attributes_proto_gen.cue
index 085190a..3baa07a 100644
--- a/encoding/protobuf/testdata/istio.io/api/mixer/v1/attributes_proto_gen.cue
+++ b/encoding/protobuf/testdata/istio.io/api/mixer/v1/attributes_proto_gen.cue
@@ -73,28 +73,28 @@
// The attribute value.
Attributes_AttributeValue: {
// Used for values of type STRING, DNS_NAME, EMAIL_ADDRESS, and URI
- stringValue?: string @protobuf(2,name=string_value)
+ stringValue: string @protobuf(2,name=string_value)
} | {
// Used for values of type INT64
- int64Value?: int64 @protobuf(3,name=int64_value)
+ int64Value: int64 @protobuf(3,name=int64_value)
} | {
// Used for values of type DOUBLE
- doubleValue?: float64 @protobuf(4,type=double,name=double_value)
+ doubleValue: float64 @protobuf(4,type=double,name=double_value)
} | {
// Used for values of type BOOL
- boolValue?: bool @protobuf(5,name=bool_value)
+ boolValue: bool @protobuf(5,name=bool_value)
} | {
// Used for values of type BYTES
- bytesValue?: bytes @protobuf(6,name=bytes_value)
+ bytesValue: bytes @protobuf(6,name=bytes_value)
} | {
// Used for values of type TIMESTAMP
- timestampValue?: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
+ timestampValue: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
} | {
// Used for values of type DURATION
- durationValue?: time.Duration @protobuf(8,type=google.protobuf.Duration,name=duration_value)
+ durationValue: time.Duration @protobuf(8,type=google.protobuf.Duration,name=duration_value)
} | {
// Used for values of type STRING_MAP
- stringMapValue?: Attributes_StringMap @protobuf(9,type=StringMap,name=string_map_value)
+ stringMapValue: Attributes_StringMap @protobuf(9,type=StringMap,name=string_map_value)
}
// Defines a string map.
diff --git a/encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/api_spec_proto_gen.cue b/encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/api_spec_proto_gen.cue
index b5afcbe..3650de8 100644
--- a/encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/api_spec_proto_gen.cue
+++ b/encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/api_spec_proto_gen.cue
@@ -126,7 +126,7 @@
// /dictionary/{term:1}/{term}
// /search{?q*,lang}
//
- uriTemplate?: string @protobuf(3,name=uri_template)
+ uriTemplate: string @protobuf(3,name=uri_template)
} | {
// EXPERIMENTAL:
//
@@ -136,7 +136,7 @@
//
// "^/pets/(.*?)?"
//
- regex?: string @protobuf(4)
+ regex: string @protobuf(4)
}
// APIKey defines the explicit configuration for generating the
@@ -155,7 +155,7 @@
//
// GET /something?api_key=abcdef12345
//
- query?: string @protobuf(1)
+ query: string @protobuf(1)
} | {
// API key is sent in a request header. `header` represents the
// header name.
@@ -166,7 +166,7 @@
// GET /something HTTP/1.1
// X-API-Key: abcdef12345
//
- header?: string @protobuf(2)
+ header: string @protobuf(2)
} | {
// API key is sent in a
// [cookie](https://swagger.io/docs/specification/authentication/cookie-authentication),
@@ -177,7 +177,7 @@
// GET /something HTTP/1.1
// Cookie: X-API-KEY=abcdef12345
//
- cookie?: string @protobuf(3)
+ cookie: string @protobuf(3)
}
// HTTPAPISpecReference defines a reference to an HTTPAPISpec. This is
diff --git a/encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/quota_proto_gen.cue b/encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/quota_proto_gen.cue
index 41206be..e729d01 100644
--- a/encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/quota_proto_gen.cue
+++ b/encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/quota_proto_gen.cue
@@ -76,13 +76,13 @@
}
StringMatch: {
// exact string match
- exact?: string @protobuf(1)
+ exact: string @protobuf(1)
} | {
// prefix-based match
- prefix?: string @protobuf(2)
+ prefix: string @protobuf(2)
} | {
// ECMAscript style regex-based match
- regex?: string @protobuf(3)
+ regex: string @protobuf(3)
}
// Specifies a match clause to match Istio attributes