Skip to content

Commit 84ffa3c

Browse files
mpvlmpvl
committed
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 <[email protected]> Reviewed-by: Marcel van Lohuizen <[email protected]>
1 parent 743365e commit 84ffa3c

File tree

5 files changed

+36
-25
lines changed

5 files changed

+36
-25
lines changed

‎encoding/protobuf/parse.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,9 +641,19 @@ func (p *protoConverter) enum(x *proto.Enum) {
641641
addComments(enum.Value, 1, nil, lastComment)
642642
}
643643

644+
// oneOf converts a Proto OneOf field to CUE. Note that Protobuf defines
645+
// a oneOf to be at most one of the fields. Rather than making each field
646+
// optional, we define oneOfs as all required fields, but add one more
647+
// disjunction allowing no fields. This makes it easier to constrain the
648+
// result to include at least one of the values.
644649
func (p *protoConverter) oneOf(x *proto.Oneof) {
645650
f := &ast.Field{
646651
Label: p.ref(x.Position),
652+
// TODO: Once we have closed structs, a oneOf is represented as a
653+
// disjunction of empty structs and closed structs with required fields.
654+
// For now we just specify the required fields. This is not correct
655+
// but more practical.
656+
// Value: &ast.StructLit{}, // Remove to make at least one required.
647657
}
648658
f.AddComment(comment(x.Comment, true))
649659

@@ -656,7 +666,8 @@ func (p *protoConverter) oneOf(x *proto.Oneof) {
656666
}
657667
switch x := v.(type) {
658668
case *proto.OneOfField:
659-
p.parseField(s, 0, x.Field)
669+
oneOf := p.parseField(s, 0, x.Field)
670+
oneOf.Optional = token.NoPos
660671

661672
default:
662673
p.messageField(s, 1, v)

‎encoding/protobuf/testdata/attributes.proto.out.cue

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,28 +73,28 @@ Attributes_AttributeValue: {
7373
// The attribute value.
7474
Attributes_AttributeValue: {
7575
// Used for values of type STRING, DNS_NAME, EMAIL_ADDRESS, and URI
76-
stringValue?: string @protobuf(2,name=string_value)
76+
stringValue: string @protobuf(2,name=string_value)
7777
} | {
7878
// Used for values of type INT64
79-
int64Value?: int64 @protobuf(3,name=int64_value)
79+
int64Value: int64 @protobuf(3,name=int64_value)
8080
} | {
8181
// Used for values of type DOUBLE
82-
doubleValue?: float64 @protobuf(4,type=double,name=double_value)
82+
doubleValue: float64 @protobuf(4,type=double,name=double_value)
8383
} | {
8484
// Used for values of type BOOL
85-
boolValue?: bool @protobuf(5,name=bool_value)
85+
boolValue: bool @protobuf(5,name=bool_value)
8686
} | {
8787
// Used for values of type BYTES
88-
bytesValue?: bytes @protobuf(6,name=bytes_value)
88+
bytesValue: bytes @protobuf(6,name=bytes_value)
8989
} | {
9090
// Used for values of type TIMESTAMP
91-
timestampValue?: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
91+
timestampValue: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
9292
} | {
9393
// Used for values of type DURATION
94-
durationValue?: time.Duration @protobuf(8,type=google.protobuf.Duration,name=duration_value)
94+
durationValue: time.Duration @protobuf(8,type=google.protobuf.Duration,name=duration_value)
9595
} | {
9696
// Used for values of type STRING_MAP
97-
stringMapValue?: Attributes_StringMap @protobuf(9,type=StringMap,name=string_map_value)
97+
stringMapValue: Attributes_StringMap @protobuf(9,type=StringMap,name=string_map_value)
9898
}
9999

100100
// Defines a string map.

‎encoding/protobuf/testdata/istio.io/api/mixer/v1/attributes_proto_gen.cue

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,28 +73,28 @@ Attributes_AttributeValue: {
7373
// The attribute value.
7474
Attributes_AttributeValue: {
7575
// Used for values of type STRING, DNS_NAME, EMAIL_ADDRESS, and URI
76-
stringValue?: string @protobuf(2,name=string_value)
76+
stringValue: string @protobuf(2,name=string_value)
7777
} | {
7878
// Used for values of type INT64
79-
int64Value?: int64 @protobuf(3,name=int64_value)
79+
int64Value: int64 @protobuf(3,name=int64_value)
8080
} | {
8181
// Used for values of type DOUBLE
82-
doubleValue?: float64 @protobuf(4,type=double,name=double_value)
82+
doubleValue: float64 @protobuf(4,type=double,name=double_value)
8383
} | {
8484
// Used for values of type BOOL
85-
boolValue?: bool @protobuf(5,name=bool_value)
85+
boolValue: bool @protobuf(5,name=bool_value)
8686
} | {
8787
// Used for values of type BYTES
88-
bytesValue?: bytes @protobuf(6,name=bytes_value)
88+
bytesValue: bytes @protobuf(6,name=bytes_value)
8989
} | {
9090
// Used for values of type TIMESTAMP
91-
timestampValue?: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
91+
timestampValue: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
9292
} | {
9393
// Used for values of type DURATION
94-
durationValue?: time.Duration @protobuf(8,type=google.protobuf.Duration,name=duration_value)
94+
durationValue: time.Duration @protobuf(8,type=google.protobuf.Duration,name=duration_value)
9595
} | {
9696
// Used for values of type STRING_MAP
97-
stringMapValue?: Attributes_StringMap @protobuf(9,type=StringMap,name=string_map_value)
97+
stringMapValue: Attributes_StringMap @protobuf(9,type=StringMap,name=string_map_value)
9898
}
9999

100100
// Defines a string map.

‎encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/api_spec_proto_gen.cue

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ HTTPAPISpecPattern: {
126126
// /dictionary/{term:1}/{term}
127127
// /search{?q*,lang}
128128
//
129-
uriTemplate?: string @protobuf(3,name=uri_template)
129+
uriTemplate: string @protobuf(3,name=uri_template)
130130
} | {
131131
// EXPERIMENTAL:
132132
//
@@ -136,7 +136,7 @@ HTTPAPISpecPattern: {
136136
//
137137
// "^/pets/(.*?)?"
138138
//
139-
regex?: string @protobuf(4)
139+
regex: string @protobuf(4)
140140
}
141141

142142
// APIKey defines the explicit configuration for generating the
@@ -155,7 +155,7 @@ APIKey: {
155155
//
156156
// GET /something?api_key=abcdef12345
157157
//
158-
query?: string @protobuf(1)
158+
query: string @protobuf(1)
159159
} | {
160160
// API key is sent in a request header. `header` represents the
161161
// header name.
@@ -166,7 +166,7 @@ APIKey: {
166166
// GET /something HTTP/1.1
167167
// X-API-Key: abcdef12345
168168
//
169-
header?: string @protobuf(2)
169+
header: string @protobuf(2)
170170
} | {
171171
// API key is sent in a
172172
// [cookie](https://swagger.io/docs/specification/authentication/cookie-authentication),
@@ -177,7 +177,7 @@ APIKey: {
177177
// GET /something HTTP/1.1
178178
// Cookie: X-API-KEY=abcdef12345
179179
//
180-
cookie?: string @protobuf(3)
180+
cookie: string @protobuf(3)
181181
}
182182

183183
// HTTPAPISpecReference defines a reference to an HTTPAPISpec. This is

‎encoding/protobuf/testdata/istio.io/api/mixer/v1/config/client/quota_proto_gen.cue

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,13 @@ StringMatch: {
7676
}
7777
StringMatch: {
7878
// exact string match
79-
exact?: string @protobuf(1)
79+
exact: string @protobuf(1)
8080
} | {
8181
// prefix-based match
82-
prefix?: string @protobuf(2)
82+
prefix: string @protobuf(2)
8383
} | {
8484
// ECMAscript style regex-based match
85-
regex?: string @protobuf(3)
85+
regex: string @protobuf(3)
8686
}
8787

8888
// Specifies a match clause to match Istio attributes

0 commit comments

Comments
 (0)