Skip to content

Commit 2260f47

Browse files
authored
chore(internal/protoveneer): support oneof fields (#10271)
Proto oneofs translate into Go as an unexported type. Here is one example: type CachedContent struct { // Types that are assignable to Expiration: // *CachedContent_ExpireTime // *CachedContent_Ttl Expiration isCachedContent_Expiration `protobuf_oneof:"expiration"` ... } As the comment says, the `Expiration` field can be populated by one of two exported types, but the type of the field itself is not exported (it is an interface type that the two exported types satisfy). Oneof fields like this cause a problem for this code generator, which writes conversion code between veneers and protos that looks like return &pb.CachedContent{ Expiration: conversionFunction(...), ... } We cannot write conversion function because we cannot write its return type. This CL addresses the problem by adding two features. First, the user can configure population functions that are passed the two structs, proto and veneer. That makes it possible to set oneof fields: func popcc(p *pb.CachedContent, v *CachedContent) { if [something about v] { p.Expiration = &pb.CachedContent_ExpireTime{..} } else { p.Expiration = &pb.CachedkContent_Ttl{...} } } The second feature is the `noConvert` option, which prevents the field from being set on the struct literal. So by setting `noConvert` and specifying populate functions, A oneof field can be initialized from a corresponding veneer type.
1 parent 0dee490 commit 2260f47

5 files changed

Lines changed: 100 additions & 10 deletions

File tree

internal/protoveneer/cmd/protoveneer/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ type typeConfig struct {
5454
Fields map[string]fieldConfig
5555
// Custom conversion functions: "tofunc, fromfunc"
5656
ConvertToFrom string `yaml:"convertToFrom"`
57+
// Custom population functions, that are called after field-by-field conversion: "tofunc, fromfunc"
58+
PopulateToFrom string `yaml:"populateToFrom"`
5759
// Doc string for the type, omitting the initial type name.
5860
// This replaces the first line of the doc.
5961
Doc string
@@ -69,6 +71,9 @@ type fieldConfig struct {
6971
Type string // veneer type
7072
// Omit from output.
7173
Omit bool
74+
// Generate the type, but not conversions.
75+
// The populate functions (see [typeConfg.PopulateToFrom]) should set the field.
76+
NoConvert bool `yaml:"noConvert"`
7277
// Custom conversion functions: "tofunc, fromfunc"
7378
ConvertToFrom string `yaml:"convertToFrom"`
7479
}

internal/protoveneer/cmd/protoveneer/protoveneer.go

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func run(ctx context.Context, configFile, pbDir, outDir string) error {
114114
if !*noFormat {
115115
src, err = format.Source(src)
116116
if err != nil {
117-
return err
117+
return fmt.Errorf("formatting: %v", err)
118118
}
119119
}
120120

@@ -284,15 +284,19 @@ func buildConverterMap(typeInfos []*typeInfo, conf *config) (map[string]converte
284284
}
285285

286286
func parseCustomConverter(name, value string) (converter, error) {
287-
toFunc, fromFunc, ok := strings.Cut(value, ",")
288-
toFunc = strings.TrimSpace(toFunc)
289-
fromFunc = strings.TrimSpace(fromFunc)
290-
if !ok || toFunc == "" || fromFunc == "" {
287+
toFunc, fromFunc := parseCommaPair(value)
288+
if toFunc == "" || fromFunc == "" {
291289
return nil, fmt.Errorf(`%s: ConvertToFrom = %q, want "toFunc, fromFunc"`, name, value)
292290
}
293291
return customConverter{toFunc, fromFunc}, nil
294292
}
295293

294+
// parseCommaPair parses a string like "foo, bar" into "foo" and "bar".
295+
func parseCommaPair(s string) (string, string) {
296+
a, b, _ := strings.Cut(s, ",")
297+
return strings.TrimSpace(a), strings.TrimSpace(b)
298+
}
299+
296300
// makeConverter constructs a converter for the given type. Not every type is in the map: this
297301
// function puts together converters for types like pointers, slices and maps, as well as
298302
// named types.
@@ -340,9 +344,11 @@ type typeInfo struct {
340344
values *ast.GenDecl // the list of values for an enum
341345

342346
// These fields are added later.
343-
veneerName string // may be provided by config; else same as protoName
344-
fields []*fieldInfo // for structs
345-
valueNames []string // to generate String functions
347+
veneerName string // may be provided by config; else same as protoName
348+
fields []*fieldInfo // for structs
349+
valueNames []string // to generate String functions
350+
populateFrom string // name of function doing additional work converting from proto
351+
populateTo string // name of function doing additional work converting to proto
346352
}
347353

348354
// A fieldInfo holds information about a struct field.
@@ -351,6 +357,7 @@ type fieldInfo struct {
351357
af *ast.Field
352358
protoName, veneerName string
353359
converter converter
360+
noConvert bool
354361
}
355362

356363
// collectDecls collects declaration information from a package.
@@ -445,6 +452,15 @@ func processType(ti *typeInfo, tconf *typeConfig, typeInfos map[string]*typeInfo
445452
ti.fields = append(ti.fields, fi)
446453
}
447454
}
455+
// Other processing.
456+
if tconf != nil && tconf.PopulateToFrom != "" {
457+
toFunc, fromFunc := parseCommaPair(tconf.PopulateToFrom)
458+
if toFunc == "" || fromFunc == "" {
459+
return fmt.Errorf(`%s: PopulateToFrom = %q, want "toFunc, fromFunc"`, ti.protoName, tconf.PopulateToFrom)
460+
}
461+
ti.populateTo = toFunc
462+
ti.populateFrom = fromFunc
463+
}
448464
case *ast.Ident:
449465
// Enum type. Nothing else to do with the type itself; but see processEnumValues.
450466
default:
@@ -492,6 +508,7 @@ func processField(af *ast.Field, tc *typeConfig, typeInfos map[string]*typeInfo)
492508
}
493509
fi.converter = c
494510
}
511+
fi.noConvert = fc.NoConvert
495512
}
496513
}
497514
af.Type = veneerType(af.Type, typeInfos)
@@ -756,22 +773,44 @@ func (ti *typeInfo) generateConversionMethods(pr func(string, ...any)) {
756773
func (ti *typeInfo) generateToProto(pr func(string, ...any)) {
757774
pr("func (v *%s) toProto() *pb.%s {\n", ti.veneerName, ti.protoName)
758775
pr(" if v == nil { return nil }\n")
759-
pr(" return &pb.%s{\n", ti.protoName)
776+
if ti.populateTo == "" {
777+
pr(" return &pb.%s{\n", ti.protoName)
778+
} else {
779+
pr(" p := &pb.%s{\n", ti.protoName)
780+
}
760781
for _, f := range ti.fields {
782+
if f.noConvert {
783+
continue
784+
}
761785
pr(" %s: %s,\n", f.protoName, f.converter.genTo("v."+f.veneerName))
762786
}
763787
pr(" }\n")
788+
if ti.populateTo != "" {
789+
pr(" %s(p, v)\n", ti.populateTo)
790+
pr(" return p\n")
791+
}
764792
pr("}\n")
765793
}
766794

767795
func (ti *typeInfo) generateFromProto(pr func(string, ...any)) {
768796
pr("func (%s) fromProto(p *pb.%s) *%[1]s {\n", ti.veneerName, ti.protoName)
769797
pr(" if p == nil { return nil }\n")
770-
pr(" return &%s{\n", ti.veneerName)
798+
if ti.populateFrom == "" {
799+
pr(" return &%s{\n", ti.veneerName)
800+
} else {
801+
pr(" v := &%s{\n", ti.veneerName)
802+
}
771803
for _, f := range ti.fields {
804+
if f.noConvert {
805+
continue
806+
}
772807
pr(" %s: %s,\n", f.veneerName, f.converter.genFrom("p."+f.protoName))
773808
}
774809
pr(" }\n")
810+
if ti.populateFrom != "" {
811+
pr(" %s(v, p)\n", ti.populateFrom)
812+
pr(" return v\n")
813+
}
775814
pr("}\n")
776815
}
777816

internal/protoveneer/cmd/protoveneer/testdata/basic/basic.pb.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,13 @@ type Citation struct {
123123
Struct *structpb.Struct
124124
CreateTime *timestamppb.Timestamp
125125
}
126+
127+
type unexported interface{ u() }
128+
129+
// This demonstrates using population functions to deal with
130+
// proto oneof field, which has an unexported type.
131+
// That can be a way to deal with proto oneofs.
132+
type Pop struct {
133+
X int
134+
Y unexported
135+
}

internal/protoveneer/cmd/protoveneer/testdata/basic/config.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,9 @@ types:
4646
fields:
4747
Uri:
4848
name: URI
49+
50+
Pop:
51+
fields:
52+
Y:
53+
noConvert: true
54+
populateToFrom: popYTo, popYFrom

internal/protoveneer/cmd/protoveneer/testdata/basic/golden

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,3 +215,33 @@ func (v HarmCategory) String() string {
215215
}
216216
return fmt.Sprintf("HarmCategory(%d)", v)
217217
}
218+
219+
// Pop is this demonstrates using population functions to deal with
220+
// proto oneof field, which has an unexported type.
221+
// That can be a way to deal with proto oneofs.
222+
type Pop struct {
223+
X int
224+
Y unexported
225+
}
226+
227+
func (v *Pop) toProto() *pb.Pop {
228+
if v == nil {
229+
return nil
230+
}
231+
p := &pb.Pop{
232+
X: v.X,
233+
}
234+
popYTo(p, v)
235+
return p
236+
}
237+
238+
func (Pop) fromProto(p *pb.Pop) *Pop {
239+
if p == nil {
240+
return nil
241+
}
242+
v := &Pop{
243+
X: p.X,
244+
}
245+
popYFrom(v, p)
246+
return v
247+
}

0 commit comments

Comments
 (0)