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