From 32ee6dec94932ed7a0e0baa36ff1c7810422658a Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Sun, 9 May 2021 07:06:35 +0100 Subject: [PATCH 01/10] protoc-gen-debug: add optional support Signed-off-by: Sarthak Gupta --- protoc-gen-debug/main.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/protoc-gen-debug/main.go b/protoc-gen-debug/main.go index 20b8755..27159a7 100644 --- a/protoc-gen-debug/main.go +++ b/protoc-gen-debug/main.go @@ -5,6 +5,7 @@ package main import ( "bytes" + "google.golang.org/protobuf/types/pluginpb" "io" "io/ioutil" "log" @@ -41,8 +42,10 @@ func main() { log.Fatal("unable to write request to disk: ", err) } - data, err = proto.Marshal(&plugin_go.CodeGeneratorResponse{}) - if err != nil { + var supportedFeatures = uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) + if data, err = proto.Marshal(&plugin_go.CodeGeneratorResponse{ + SupportedFeatures: &supportedFeatures, + }); err != nil { log.Fatal("unable to marshal response payload: ", err) } From 7424ee84698183cbe2b8e0002cb423d9542fd2cc Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Sun, 9 May 2021 07:08:28 +0100 Subject: [PATCH 02/10] field: fix missing optional support, update protoc Signed-off-by: Sarthak Gupta --- .travis.yml | 5 +-- field.go | 12 ++++++ go.mod | 1 + lang/go/testdata/names/types/proto3.proto | 22 ++++++++++ lang/go/type_name.go | 2 +- lang/go/type_name_test.go | 49 ++++++++++++++++------- 6 files changed, 73 insertions(+), 18 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7ab01b8..a083cda 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,10 @@ language: go -go: "1.14.6" +go: "1.16.0" go_import_path: github.com/lyft/protoc-gen-star env: matrix: - - PROTOC_VER="3.5.1" - - PROTOC_VER="3.6.1" + - PROTOC_VER="3.16.0" before_install: - mkdir -p $GOPATH/bin diff --git a/field.go b/field.go index d1fa9d0..1efe08b 100644 --- a/field.go +++ b/field.go @@ -30,6 +30,11 @@ type Field interface { // will only be true if the syntax is proto2. Required() bool + // Proto3Optional returns whether or not the field is a proto 3 optional field. This + // will only be true if the syntax is proto3, and otherwise false. + // Info: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md#presence-in-proto3-apis + Proto3Optional() bool + setMessage(m Message) setOneOf(o OneOf) addType(t FieldType) @@ -66,6 +71,13 @@ func (f *field) Required() bool { f.desc.GetLabel() == descriptor.FieldDescriptorProto_LABEL_REQUIRED } +func (f *field) Proto3Optional() bool { + if f.desc.Proto3Optional != nil { + return *f.desc.Proto3Optional + } + return false +} + func (f *field) addType(t FieldType) { t.setField(f) f.typ = t diff --git a/go.mod b/go.mod index 3c40def..d68d685 100644 --- a/go.mod +++ b/go.mod @@ -6,4 +6,5 @@ require ( github.com/golang/protobuf v1.5.2 github.com/spf13/afero v1.3.3 github.com/stretchr/testify v1.6.1 + google.golang.org/protobuf v1.26.0 // indirect ) diff --git a/lang/go/testdata/names/types/proto3.proto b/lang/go/testdata/names/types/proto3.proto index a797dd9..2a565f5 100644 --- a/lang/go/testdata/names/types/proto3.proto +++ b/lang/go/testdata/names/types/proto3.proto @@ -42,4 +42,26 @@ message Proto3 { enum Enum {VALUE = 0;} message Message {} + + message Optional { + optional double double = 1; + optional float float = 2; + optional int64 int64 = 3; + optional sfixed64 sfixed64 = 4; + optional sint64 sint64 = 5; + optional uint64 uint64 = 6; + optional fixed64 fixed64 = 7; + optional int32 int32 = 8; + optional sfixed32 sfixed32 = 9; + optional sint32 sint32 = 10; + optional uint32 uint32 = 11; + optional fixed32 fixed32 = 12; + optional bool bool = 13; + optional string string = 14; + optional bytes bytes = 15; + optional Enum enum = 16; + optional google.protobuf.Syntax ext_enum = 17; + optional Optional msg = 18; + optional google.protobuf.Duration ext_msg = 19; + } } diff --git a/lang/go/type_name.go b/lang/go/type_name.go index 9f24821..33210cc 100644 --- a/lang/go/type_name.go +++ b/lang/go/type_name.go @@ -25,7 +25,7 @@ func (c context) Type(f pgs.Field) TypeName { t = scalarType(ft.ProtoType()) } - if f.Syntax() == pgs.Proto2 { + if f.Syntax() == pgs.Proto2 || (f.Syntax() == pgs.Proto3 && f.Proto3Optional()) { return t.Pointer() } diff --git a/lang/go/type_name_test.go b/lang/go/type_name_test.go index 133e8b5..66ab087 100644 --- a/lang/go/type_name_test.go +++ b/lang/go/type_name_test.go @@ -37,19 +37,19 @@ func TestType(t *testing.T) { {"Proto2.string", "*string"}, {"Proto2.bytes", "[]byte"}, {"Proto2.enum", "*Proto2_Enum"}, - {"Proto2.ext_enum", "*ptype.Syntax"}, + {"Proto2.ext_enum", "*typepb.Syntax"}, {"Proto2.msg", "*Proto2_Required"}, - {"Proto2.ext_msg", "*duration.Duration"}, + {"Proto2.ext_msg", "*durationpb.Duration"}, {"Proto2.repeated_scalar", "[]float64"}, {"Proto2.repeated_enum", "[]Proto2_Enum"}, - {"Proto2.repeated_ext_enum", "[]ptype.Syntax"}, + {"Proto2.repeated_ext_enum", "[]typepb.Syntax"}, {"Proto2.repeated_msg", "[]*Proto2_Required"}, - {"Proto2.repeated_ext_msg", "[]*duration.Duration"}, + {"Proto2.repeated_ext_msg", "[]*durationpb.Duration"}, {"Proto2.map_scalar", "map[string]float32"}, {"Proto2.map_enum", "map[int32]Proto2_Enum"}, - {"Proto2.map_ext_enum", "map[uint64]ptype.Syntax"}, + {"Proto2.map_ext_enum", "map[uint64]typepb.Syntax"}, {"Proto2.map_msg", "map[uint32]*Proto2_Required"}, - {"Proto2.map_ext_msg", "map[int64]*duration.Duration"}, + {"Proto2.map_ext_msg", "map[int64]*durationpb.Duration"}, // proto2 syntax, required {"Proto2.Required.double", "*float64"}, @@ -68,9 +68,9 @@ func TestType(t *testing.T) { {"Proto2.Required.string", "*string"}, {"Proto2.Required.bytes", "[]byte"}, {"Proto2.Required.enum", "*Proto2_Enum"}, - {"Proto2.Required.ext_enum", "*ptype.Syntax"}, + {"Proto2.Required.ext_enum", "*typepb.Syntax"}, {"Proto2.Required.msg", "*Proto2_Required"}, - {"Proto2.Required.ext_msg", "*duration.Duration"}, + {"Proto2.Required.ext_msg", "*durationpb.Duration"}, {"Proto3.double", "float64"}, {"Proto3.float", "float32"}, @@ -88,19 +88,40 @@ func TestType(t *testing.T) { {"Proto3.string", "string"}, {"Proto3.bytes", "[]byte"}, {"Proto3.enum", "Proto3_Enum"}, - {"Proto3.ext_enum", "ptype.Syntax"}, + {"Proto3.ext_enum", "typepb.Syntax"}, {"Proto3.msg", "*Proto3_Message"}, - {"Proto3.ext_msg", "*duration.Duration"}, + {"Proto3.ext_msg", "*durationpb.Duration"}, {"Proto3.repeated_scalar", "[]float64"}, {"Proto3.repeated_enum", "[]Proto3_Enum"}, - {"Proto3.repeated_ext_enum", "[]ptype.Syntax"}, + {"Proto3.repeated_ext_enum", "[]typepb.Syntax"}, {"Proto3.repeated_msg", "[]*Proto3_Message"}, - {"Proto3.repeated_ext_msg", "[]*duration.Duration"}, + {"Proto3.repeated_ext_msg", "[]*durationpb.Duration"}, {"Proto3.map_scalar", "map[string]float32"}, {"Proto3.map_enum", "map[int32]Proto3_Enum"}, - {"Proto3.map_ext_enum", "map[uint64]ptype.Syntax"}, + {"Proto3.map_ext_enum", "map[uint64]typepb.Syntax"}, {"Proto3.map_msg", "map[uint32]*Proto3_Message"}, - {"Proto3.map_ext_msg", "map[int64]*duration.Duration"}, + {"Proto3.map_ext_msg", "map[int64]*durationpb.Duration"}, + + // proto3 syntax optional + {"Proto3.Optional.double", "*float64"}, + {"Proto3.Optional.float", "*float32"}, + {"Proto3.Optional.int64", "*int64"}, + {"Proto3.Optional.sfixed64", "*int64"}, + {"Proto3.Optional.sint64", "*int64"}, + {"Proto3.Optional.uint64", "*uint64"}, + {"Proto3.Optional.fixed64", "*uint64"}, + {"Proto3.Optional.int32", "*int32"}, + {"Proto3.Optional.sfixed32", "*int32"}, + {"Proto3.Optional.sint32", "*int32"}, + {"Proto3.Optional.uint32", "*uint32"}, + {"Proto3.Optional.fixed32", "*uint32"}, + {"Proto3.Optional.bool", "*bool"}, + {"Proto3.Optional.string", "*string"}, + {"Proto3.Optional.bytes", "[]byte"}, + {"Proto3.Optional.enum", "*Proto3_Enum"}, + {"Proto3.Optional.ext_enum", "*typepb.Syntax"}, + {"Proto3.Optional.msg", "*Proto3_Optional"}, + {"Proto3.Optional.ext_msg", "*durationpb.Duration"}, } for _, test := range tests { From 523296668bebe4bb29904ac3fb9f533a056b2de8 Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Sun, 9 May 2021 07:10:02 +0100 Subject: [PATCH 03/10] proto: updated notes for proto3 field presence Signed-off-by: Sarthak Gupta --- proto.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proto.go b/proto.go index 3ce2dd9..f615e7c 100644 --- a/proto.go +++ b/proto.go @@ -13,10 +13,10 @@ const ( // See: https://developers.google.com/protocol-buffers/docs/proto Proto2 Syntax = "" - // Proto3 syntax only allows for optional fields, but defaults to the zero - // value of that particular type. Most of the field types in the generated go - // structs are value types. - // See: https://developers.google.com/protocol-buffers/docs/proto3 + // Proto3 syntax permits the use of "optional" field presence. Non optional fields default to the zero + // value of that particular type if not defined. + // Most of the field types in the generated go structs are value types. + // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md#presence-in-proto3-apis Proto3 Syntax = "proto3" ) From 9d6de96ba7e8b560f92cf8f1aacb565e4eb773fa Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Sun, 9 May 2021 07:23:09 +0100 Subject: [PATCH 04/10] travis: revert golang version Signed-off-by: Sarthak Gupta --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a083cda..0991525 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,5 @@ language: go -go: "1.16.0" +go: "1.14.6" go_import_path: github.com/lyft/protoc-gen-star env: From 69f0811692de0fa02fa8677f421c3ed56cd208c5 Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Tue, 11 May 2021 03:57:20 +0100 Subject: [PATCH 05/10] generator: add proto3 optional init option Signed-off-by: Sarthak Gupta --- field.go | 12 ------------ init_option.go | 8 ++++++++ persister.go | 11 +++++------ 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/field.go b/field.go index 1efe08b..d1fa9d0 100644 --- a/field.go +++ b/field.go @@ -30,11 +30,6 @@ type Field interface { // will only be true if the syntax is proto2. Required() bool - // Proto3Optional returns whether or not the field is a proto 3 optional field. This - // will only be true if the syntax is proto3, and otherwise false. - // Info: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md#presence-in-proto3-apis - Proto3Optional() bool - setMessage(m Message) setOneOf(o OneOf) addType(t FieldType) @@ -71,13 +66,6 @@ func (f *field) Required() bool { f.desc.GetLabel() == descriptor.FieldDescriptorProto_LABEL_REQUIRED } -func (f *field) Proto3Optional() bool { - if f.desc.Proto3Optional != nil { - return *f.desc.Proto3Optional - } - return false -} - func (f *field) addType(t FieldType) { t.setField(f) f.typ = t diff --git a/init_option.go b/init_option.go index aba7d86..efbe8b7 100644 --- a/init_option.go +++ b/init_option.go @@ -1,6 +1,7 @@ package pgs import ( + "google.golang.org/protobuf/types/pluginpb" "io" "os" @@ -47,3 +48,10 @@ func FileSystem(fs afero.Fs) InitOption { return func(g *Generator) { g.persiste func BiDirectional() InitOption { return func(g *Generator) { g.workflow = &onceWorkflow{workflow: &standardWorkflow{BiDi: true}} } } + +func SupportProto3Optional() InitOption { + return func(g *Generator) { + p3Optional := uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) + g.persister.SetSupportedField(&p3Optional) + } +} diff --git a/persister.go b/persister.go index 5cbcece..ebfafb2 100644 --- a/persister.go +++ b/persister.go @@ -8,12 +8,12 @@ import ( "github.com/golang/protobuf/proto" plugin_go "github.com/golang/protobuf/protoc-gen-go/plugin" "github.com/spf13/afero" - "google.golang.org/protobuf/types/pluginpb" ) type persister interface { SetDebugger(d Debugger) SetFS(fs afero.Fs) + SetSupportedField(f *uint64) AddPostProcessor(proc ...PostProcessor) Persist(a ...Artifact) *plugin_go.CodeGeneratorResponse } @@ -21,22 +21,21 @@ type persister interface { type stdPersister struct { Debugger - fs afero.Fs - procs []PostProcessor + fs afero.Fs + procs []PostProcessor + supportedField *uint64 } func newPersister() *stdPersister { return &stdPersister{fs: afero.NewOsFs()} } func (p *stdPersister) SetDebugger(d Debugger) { p.Debugger = d } func (p *stdPersister) SetFS(fs afero.Fs) { p.fs = fs } +func (p *stdPersister) SetSupportedField(f *uint64) { p.supportedField = f } func (p *stdPersister) AddPostProcessor(proc ...PostProcessor) { p.procs = append(p.procs, proc...) } func (p *stdPersister) Persist(arts ...Artifact) *plugin_go.CodeGeneratorResponse { resp := new(plugin_go.CodeGeneratorResponse) - supportedFeatures := uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) - resp.SupportedFeatures = &supportedFeatures - for _, a := range arts { switch a := a.(type) { case GeneratorFile: From 552c18ef51200b4c11274d27ae7e96182ef65d6b Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Tue, 11 May 2021 16:14:14 +0100 Subject: [PATCH 06/10] pgs: add support for proto3 field presence Signed-off-by: Sarthak Gupta --- field.go | 33 ++++++++++++++++++++ field_test.go | 73 ++++++++++++++++++++++++++++++++++++++++++++ field_type.go | 5 ++- field_type_test.go | 23 +++++++++++++- lang/go/type_name.go | 2 +- message.go | 29 ++++++++++++++++++ message_test.go | 46 ++++++++++++++++++++++++++++ oneof.go | 10 ++++++ oneof_test.go | 14 +++++++++ persister.go | 1 + 10 files changed, 231 insertions(+), 5 deletions(-) diff --git a/field.go b/field.go index d1fa9d0..072b01d 100644 --- a/field.go +++ b/field.go @@ -17,8 +17,14 @@ type Field interface { Message() Message // InOneOf returns true if the field is in a OneOf of the parent Message. + // This will return true for synthetic oneofs (proto3 field presence) as well. InOneOf() bool + // InRealOneOf returns true if the field is in a OneOf of the parent Message. + // This will return false for synthetic oneofs, and will only include 'real' oneofs. + // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + InRealOneOf() bool + // OneOf returns the OneOf that this field is apart of. Nil is returned if // the field is not within a OneOf. OneOf() OneOf @@ -26,6 +32,13 @@ type Field interface { // Type returns the FieldType of this Field. Type() FieldType + // HasPresence returns true for all fields that have explicit presence as defined by: + // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + HasPresence() bool + + // Optional returns whether or not the field is labeled as optional. + Optional() bool + // Required returns whether or not the field is labeled as required. This // will only be true if the syntax is proto2. Required() bool @@ -56,11 +69,31 @@ func (f *field) SourceCodeInfo() SourceCodeInfo { return f.info } func (f *field) Descriptor() *descriptor.FieldDescriptorProto { return f.desc } func (f *field) Message() Message { return f.msg } func (f *field) InOneOf() bool { return f.oneof != nil } +func (f *field) InRealOneOf() bool { return f.InOneOf() && !f.desc.GetProto3Optional() } func (f *field) OneOf() OneOf { return f.oneof } func (f *field) Type() FieldType { return f.typ } func (f *field) setMessage(m Message) { f.msg = m } func (f *field) setOneOf(o OneOf) { f.oneof = o } +func (f *field) HasPresence() bool { + if f.Type().IsRepeated() || f.Type().IsMap() { + return false + } + + if f.Syntax() == Proto3 && !f.Optional() && !(f.Type().IsEmbed() || f.InRealOneOf()) { + return false + } + + return true +} + +func (f *field) Optional() bool { + if f.Syntax() == Proto3 { + return f.desc.GetProto3Optional() + } + return f.desc.GetLabel() == descriptor.FieldDescriptorProto_LABEL_OPTIONAL +} + func (f *field) Required() bool { return f.Syntax().SupportsRequiredPrefix() && f.desc.GetLabel() == descriptor.FieldDescriptorProto_LABEL_REQUIRED diff --git a/field_test.go b/field_test.go index 6915a34..dca3ec7 100644 --- a/field_test.go +++ b/field_test.go @@ -99,6 +99,59 @@ func TestField_OneOf(t *testing.T) { assert.True(t, f.InOneOf()) } +func TestField_InRealOneOf(t *testing.T) { + t.Parallel() + + f := dummyField() + assert.False(t, f.InRealOneOf()) + + f = dummyOneOfField(false) + assert.True(t, f.InRealOneOf()) + + f = dummyOneOfField(true) + assert.False(t, f.InRealOneOf()) +} + +func TestField_HasPresence(t *testing.T) { + t.Parallel() + + f := &field{} + f.addType(&repT{scalarT: &scalarT{}}) + assert.False(t, f.HasPresence()) + + f.addType(&mapT{repT: &repT{scalarT: &scalarT{}}}) + assert.False(t, f.HasPresence()) + + f.msg = dummyMsg() + f.addType(&scalarT{}) + assert.False(t, f.HasPresence()) + + opt := true + f.desc = &descriptor.FieldDescriptorProto{Proto3Optional: &opt} + assert.True(t, f.HasPresence()) +} + +func TestField_Optional(t *testing.T) { + t.Parallel() + + optLabel := descriptor.FieldDescriptorProto_LABEL_OPTIONAL + + f := &field{msg: &msg{parent: dummyFile()}} + assert.False(t, f.Optional()) + + f.desc = &descriptor.FieldDescriptorProto{Label: &optLabel} + assert.False(t, f.Optional()) + + f = dummyField() + assert.False(t, f.Optional()) + + f = dummyOneOfField(false) + assert.False(t, f.Optional()) + + f = dummyOneOfField(true) + assert.True(t, f.Optional()) +} + func TestField_Type(t *testing.T) { t.Parallel() @@ -194,3 +247,23 @@ func dummyField() *field { f.addType(t) return f } + +func dummyOneOfField(synthetic bool) *field { + m := dummyMsg() + o := dummyOneof() + str := descriptor.FieldDescriptorProto_TYPE_STRING + var oIndex int32 + oIndex = 1 + f := &field{desc: &descriptor.FieldDescriptorProto{ + Name: proto.String("field"), + Type: &str, + OneofIndex: &oIndex, + Proto3Optional: &synthetic, + }} + o.addField(f) + m.addField(f) + m.addOneOf(o) + t := &scalarT{} + f.addType(t) + return f +} diff --git a/field_type.go b/field_type.go index d752f02..b3dd779 100644 --- a/field_type.go +++ b/field_type.go @@ -22,8 +22,7 @@ type FieldType interface { // repeated fields containing embeds will still return false. IsEmbed() bool - // IsOptional returns true if the message's syntax is not Proto2 or - // the field is prefixed as optional. + // IsOptional returns true if the field is prefixed as optional. IsOptional() bool // IsRequired returns true if and only if the field is prefixed as required. @@ -84,7 +83,7 @@ func (s *scalarT) Element() FieldTypeElem { return nil } func (s *scalarT) Key() FieldTypeElem { return nil } func (s *scalarT) IsOptional() bool { - return !s.fld.Syntax().SupportsRequiredPrefix() || s.ProtoLabel() == Optional + return s.fld.Optional() } func (s *scalarT) IsRequired() bool { diff --git a/field_type_test.go b/field_type_test.go index 6388e8e..75f2553 100644 --- a/field_type_test.go +++ b/field_type_test.go @@ -89,7 +89,7 @@ func TestScalarT_IsOptional(t *testing.T) { t.Parallel() s := &scalarT{} - f := dummyField() + f := dummyOneOfField(true) f.addType(s) assert.True(t, s.IsOptional()) @@ -106,6 +106,27 @@ func TestScalarT_IsOptional(t *testing.T) { assert.False(t, s.IsOptional()) } +func TestScalarT_IsNotOptional(t *testing.T) { + t.Parallel() + + s := &scalarT{} + f := dummyField() + f.addType(s) + + assert.False(t, s.IsOptional()) + + fl := dummyFile() + fl.desc.Syntax = nil + f.Message().setParent(fl) + + assert.True(t, s.IsOptional()) + + req := descriptor.FieldDescriptorProto_LABEL_REQUIRED + f.desc.Label = &req + + assert.False(t, s.IsOptional()) +} + func TestScalarT_IsRequired(t *testing.T) { t.Parallel() diff --git a/lang/go/type_name.go b/lang/go/type_name.go index 33210cc..9f3a1e2 100644 --- a/lang/go/type_name.go +++ b/lang/go/type_name.go @@ -25,7 +25,7 @@ func (c context) Type(f pgs.Field) TypeName { t = scalarType(ft.ProtoType()) } - if f.Syntax() == pgs.Proto2 || (f.Syntax() == pgs.Proto3 && f.Proto3Optional()) { + if f.HasPresence() { return t.Pointer() } diff --git a/message.go b/message.go index fa15a2c..0f5daa3 100644 --- a/message.go +++ b/message.go @@ -29,9 +29,18 @@ type Message interface { // OneOfFields returns only the fields contained within OneOf blocks. OneOfFields() []Field + // SyntheticOneOfFields returns only the fields contained within synthetic OneOf blocks. + // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + SyntheticOneOfFields() []Field + // OneOfs returns the OneOfs contained within this Message. OneOfs() []OneOf + // RealOneOfs returns the OneOfs contained within this Message. + // This excludes synthetic OneOfs. + // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + RealOneOfs() []OneOf + // Extensions returns all of the Extensions applied to this Message. Extensions() []Extension @@ -139,6 +148,26 @@ func (m *msg) OneOfFields() (f []Field) { return f } +func (m *msg) SyntheticOneOfFields() (f []Field) { + for _, o := range m.oneofs { + if o.IsSynthetic() { + f = append(f, o.Fields()...) + } + } + + return f +} + +func (m *msg) RealOneOfs() (r []OneOf) { + for _, o := range m.oneofs { + if !o.IsSynthetic() { + r = append(r, o) + } + } + + return r +} + func (m *msg) Imports() (i []File) { // Mapping for avoiding duplicate entries mp := make(map[string]File, len(m.fields)) diff --git a/message_test.go b/message_test.go index 65ab251..b6eec25 100644 --- a/message_test.go +++ b/message_test.go @@ -211,6 +211,52 @@ func TestMsg_OneOfs(t *testing.T) { assert.Len(t, m.OneOfs(), 1) } +func TestMsg_SyntheticOneOfFields_And_RealOneOfs(t *testing.T) { + t.Parallel() + + oSyn := &oneof{} + oSyn.flds = []Field{dummyOneOfField(true)} + oSyn.flds[0].setOneOf(oSyn) + + oReal := &oneof{} + oReal.flds = []Field{dummyField(), dummyField()} + oReal.flds[0].setOneOf(oReal) + oReal.flds[1].setOneOf(oReal) + + // no one offs + m := dummyMsg() + assert.Len(t, m.OneOfFields(), 0, "oneof fields") + assert.Len(t, m.SyntheticOneOfFields(), 0, "synthetic oneof fields") + assert.Len(t, m.OneOfs(), 0, "oneofs") + assert.Len(t, m.RealOneOfs(), 0, "real oneofs") + + // one real oneof + m.addField(oReal.flds[0]) + m.addField(oReal.flds[1]) + m.addOneOf(oReal) + assert.Len(t, m.OneOfFields(), 2, "oneof fields") + assert.Len(t, m.SyntheticOneOfFields(), 0, "synthetic oneof fields") + assert.Len(t, m.OneOfs(), 1, "oneofs") + assert.Len(t, m.RealOneOfs(), 1, "real oneofs") + + // one real, one synthetic oneof + m.addField(oSyn.flds[0]) + m.addOneOf(oSyn) + assert.Len(t, m.OneOfFields(), 3, "oneof fields") + assert.Len(t, m.SyntheticOneOfFields(), 1, "synthetic oneof fields") + assert.Len(t, m.OneOfs(), 2, "oneofs") + assert.Len(t, m.RealOneOfs(), 1, "real oneofs") + + // one synthetic oneof + m = dummyMsg() + m.addField(oSyn.flds[0]) + m.addOneOf(oSyn) + assert.Len(t, m.OneOfFields(), 1, "oneof fields") + assert.Len(t, m.SyntheticOneOfFields(), 1, "synthetic oneof fields") + assert.Len(t, m.OneOfs(), 1, "oneofs") + assert.Len(t, m.RealOneOfs(), 0, "real oneofs") +} + func TestMsg_Extension(t *testing.T) { // cannot be parallel m := &msg{desc: &descriptor.DescriptorProto{}} diff --git a/oneof.go b/oneof.go index c0d7a42..212d14c 100644 --- a/oneof.go +++ b/oneof.go @@ -19,6 +19,10 @@ type OneOf interface { // Fields returns all fields contained within this OneOf. Fields() []Field + // IsSynthetic returns true if this is a proto3 synthetic oneof. + // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + IsSynthetic() bool + setMessage(m Message) addField(f Field) } @@ -52,6 +56,12 @@ func (o *oneof) Descriptor() *descriptor.OneofDescriptorProto { return o.desc } func (o *oneof) Message() Message { return o.msg } func (o *oneof) setMessage(m Message) { o.msg = m } +func (o *oneof) IsSynthetic() bool { + return o.Syntax() == Proto3 && + len(o.flds) == 1 && + !o.flds[0].InRealOneOf() +} + func (o *oneof) Imports() (i []File) { // Mapping for avoiding duplicate entries mp := make(map[string]File, len(o.flds)) diff --git a/oneof_test.go b/oneof_test.go index a03ab3e..d0eef9c 100644 --- a/oneof_test.go +++ b/oneof_test.go @@ -119,6 +119,20 @@ func TestOneof_Fields(t *testing.T) { assert.Len(t, o.Fields(), 1) } +func TestOneof_IsSynthetic(t *testing.T) { + t.Parallel() + + o := &oneof{msg: &msg{parent: dummyFile()}} + assert.False(t, o.IsSynthetic()) + + o.flds = []Field{dummyField()} + o.flds[0].setOneOf(o) + assert.False(t, o.IsSynthetic()) + + o.flds = []Field{dummyOneOfField(true)} + assert.True(t, o.IsSynthetic()) +} + func TestOneof_Accept(t *testing.T) { t.Parallel() diff --git a/persister.go b/persister.go index ebfafb2..98a565c 100644 --- a/persister.go +++ b/persister.go @@ -35,6 +35,7 @@ func (p *stdPersister) AddPostProcessor(proc ...PostProcessor) { p.procs = appen func (p *stdPersister) Persist(arts ...Artifact) *plugin_go.CodeGeneratorResponse { resp := new(plugin_go.CodeGeneratorResponse) + resp.SupportedFeatures = p.supportedField for _, a := range arts { switch a := a.(type) { From e0bab4844e07fcdd98acd18205f49387961e1499 Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Tue, 11 May 2021 16:20:05 +0100 Subject: [PATCH 07/10] pgs: add support for proto3 field presence Signed-off-by: Sarthak Gupta --- init_option.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/init_option.go b/init_option.go index efbe8b7..4f22d9d 100644 --- a/init_option.go +++ b/init_option.go @@ -49,6 +49,8 @@ func BiDirectional() InitOption { return func(g *Generator) { g.workflow = &onceWorkflow{workflow: &standardWorkflow{BiDi: true}} } } +// SupportProto3Optional adds support for proto3 field presence. +// See: https://github.com/protocolbuffers/protobuf/blob/master/docs/implementing_proto3_presence.md#signaling-that-your-code-generator-supports-proto3-optional func SupportProto3Optional() InitOption { return func(g *Generator) { p3Optional := uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) From 2d3f0a89cd3441396f5f4bc81474c81897a128d8 Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Sun, 13 Jun 2021 19:50:14 +0100 Subject: [PATCH 08/10] pgs (field presence): nit/style fixes Signed-off-by: Sarthak Gupta --- field.go | 34 +++++++++++++++++++++------------- field_test.go | 12 ++++++------ field_type.go | 2 +- init_option.go | 10 ++++------ message.go | 4 ++-- oneof.go | 2 +- persister.go | 12 ++++++------ proto.go | 2 +- 8 files changed, 42 insertions(+), 36 deletions(-) diff --git a/field.go b/field.go index 072b01d..445d7f7 100644 --- a/field.go +++ b/field.go @@ -22,10 +22,10 @@ type Field interface { // InRealOneOf returns true if the field is in a OneOf of the parent Message. // This will return false for synthetic oneofs, and will only include 'real' oneofs. - // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + // See: https://github.com/protocolbuffers/protobuf/blob/v3.17.0/docs/field_presence.md InRealOneOf() bool - // OneOf returns the OneOf that this field is apart of. Nil is returned if + // OneOf returns the OneOf that this field is a part of. Nil is returned if // the field is not within a OneOf. OneOf() OneOf @@ -33,13 +33,13 @@ type Field interface { Type() FieldType // HasPresence returns true for all fields that have explicit presence as defined by: - // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + // See: https://github.com/protocolbuffers/protobuf/blob/v3.17.0/docs/field_presence.md HasPresence() bool - // Optional returns whether or not the field is labeled as optional. - Optional() bool + // HasOptionalKeyword returns whether the field is labeled as optional. + HasOptionalKeyword() bool - // Required returns whether or not the field is labeled as required. This + // Required returns whether the field is labeled as required. This // will only be true if the syntax is proto2. Required() bool @@ -69,25 +69,33 @@ func (f *field) SourceCodeInfo() SourceCodeInfo { return f.info } func (f *field) Descriptor() *descriptor.FieldDescriptorProto { return f.desc } func (f *field) Message() Message { return f.msg } func (f *field) InOneOf() bool { return f.oneof != nil } -func (f *field) InRealOneOf() bool { return f.InOneOf() && !f.desc.GetProto3Optional() } func (f *field) OneOf() OneOf { return f.oneof } func (f *field) Type() FieldType { return f.typ } func (f *field) setMessage(m Message) { f.msg = m } func (f *field) setOneOf(o OneOf) { f.oneof = o } +func (f *field) InRealOneOf() bool { + return f.InOneOf() && !f.desc.GetProto3Optional() +} + func (f *field) HasPresence() bool { - if f.Type().IsRepeated() || f.Type().IsMap() { - return false + if f.InOneOf() { + return true + } + + if f.Type().IsEmbed() { + return true } - if f.Syntax() == Proto3 && !f.Optional() && !(f.Type().IsEmbed() || f.InRealOneOf()) { - return false + if f.HasOptionalKeyword() || + (f.Syntax() == Proto2 && !f.Type().IsRepeated() && !f.Type().IsMap()) { + return true } - return true + return false } -func (f *field) Optional() bool { +func (f *field) HasOptionalKeyword() bool { if f.Syntax() == Proto3 { return f.desc.GetProto3Optional() } diff --git a/field_test.go b/field_test.go index dca3ec7..98c792c 100644 --- a/field_test.go +++ b/field_test.go @@ -115,7 +115,7 @@ func TestField_InRealOneOf(t *testing.T) { func TestField_HasPresence(t *testing.T) { t.Parallel() - f := &field{} + f := dummyField() f.addType(&repT{scalarT: &scalarT{}}) assert.False(t, f.HasPresence()) @@ -137,19 +137,19 @@ func TestField_Optional(t *testing.T) { optLabel := descriptor.FieldDescriptorProto_LABEL_OPTIONAL f := &field{msg: &msg{parent: dummyFile()}} - assert.False(t, f.Optional()) + assert.False(t, f.HasOptionalKeyword()) f.desc = &descriptor.FieldDescriptorProto{Label: &optLabel} - assert.False(t, f.Optional()) + assert.False(t, f.HasOptionalKeyword()) f = dummyField() - assert.False(t, f.Optional()) + assert.False(t, f.HasOptionalKeyword()) f = dummyOneOfField(false) - assert.False(t, f.Optional()) + assert.False(t, f.HasOptionalKeyword()) f = dummyOneOfField(true) - assert.True(t, f.Optional()) + assert.True(t, f.HasOptionalKeyword()) } func TestField_Type(t *testing.T) { diff --git a/field_type.go b/field_type.go index b3dd779..3f33875 100644 --- a/field_type.go +++ b/field_type.go @@ -83,7 +83,7 @@ func (s *scalarT) Element() FieldTypeElem { return nil } func (s *scalarT) Key() FieldTypeElem { return nil } func (s *scalarT) IsOptional() bool { - return s.fld.Optional() + return s.fld.HasOptionalKeyword() } func (s *scalarT) IsRequired() bool { diff --git a/init_option.go b/init_option.go index 4f22d9d..5c92c2f 100644 --- a/init_option.go +++ b/init_option.go @@ -1,7 +1,6 @@ package pgs import ( - "google.golang.org/protobuf/types/pluginpb" "io" "os" @@ -49,11 +48,10 @@ func BiDirectional() InitOption { return func(g *Generator) { g.workflow = &onceWorkflow{workflow: &standardWorkflow{BiDi: true}} } } -// SupportProto3Optional adds support for proto3 field presence. -// See: https://github.com/protocolbuffers/protobuf/blob/master/docs/implementing_proto3_presence.md#signaling-that-your-code-generator-supports-proto3-optional -func SupportProto3Optional() InitOption { +// SupportedFeatures allows defining protoc features to enable / disable. +// See: https://github.com/protocolbuffers/protobuf/blob/v3.17.0/docs/implementing_proto3_presence.md#signaling-that-your-code-generator-supports-proto3-optional +func SupportedFeatures(feat *uint64) InitOption { return func(g *Generator) { - p3Optional := uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) - g.persister.SetSupportedField(&p3Optional) + g.persister.SetSupportedFeatures(feat) } } diff --git a/message.go b/message.go index 0f5daa3..13a2f88 100644 --- a/message.go +++ b/message.go @@ -30,7 +30,7 @@ type Message interface { OneOfFields() []Field // SyntheticOneOfFields returns only the fields contained within synthetic OneOf blocks. - // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + // See: https://github.com/protocolbuffers/protobuf/blob/v3.17.0/docs/field_presence.md SyntheticOneOfFields() []Field // OneOfs returns the OneOfs contained within this Message. @@ -38,7 +38,7 @@ type Message interface { // RealOneOfs returns the OneOfs contained within this Message. // This excludes synthetic OneOfs. - // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + // See: https://github.com/protocolbuffers/protobuf/blob/v3.17.0/docs/field_presence.md RealOneOfs() []OneOf // Extensions returns all of the Extensions applied to this Message. diff --git a/oneof.go b/oneof.go index 212d14c..34970af 100644 --- a/oneof.go +++ b/oneof.go @@ -20,7 +20,7 @@ type OneOf interface { Fields() []Field // IsSynthetic returns true if this is a proto3 synthetic oneof. - // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md + // See: https://github.com/protocolbuffers/protobuf/blob/v3.17.0/docs/field_presence.md IsSynthetic() bool setMessage(m Message) diff --git a/persister.go b/persister.go index 98a565c..21d48b7 100644 --- a/persister.go +++ b/persister.go @@ -13,7 +13,7 @@ import ( type persister interface { SetDebugger(d Debugger) SetFS(fs afero.Fs) - SetSupportedField(f *uint64) + SetSupportedFeatures(f *uint64) AddPostProcessor(proc ...PostProcessor) Persist(a ...Artifact) *plugin_go.CodeGeneratorResponse } @@ -21,21 +21,21 @@ type persister interface { type stdPersister struct { Debugger - fs afero.Fs - procs []PostProcessor - supportedField *uint64 + fs afero.Fs + procs []PostProcessor + supportedFeatures *uint64 } func newPersister() *stdPersister { return &stdPersister{fs: afero.NewOsFs()} } func (p *stdPersister) SetDebugger(d Debugger) { p.Debugger = d } func (p *stdPersister) SetFS(fs afero.Fs) { p.fs = fs } -func (p *stdPersister) SetSupportedField(f *uint64) { p.supportedField = f } +func (p *stdPersister) SetSupportedFeatures(f *uint64) { p.supportedFeatures = f } func (p *stdPersister) AddPostProcessor(proc ...PostProcessor) { p.procs = append(p.procs, proc...) } func (p *stdPersister) Persist(arts ...Artifact) *plugin_go.CodeGeneratorResponse { resp := new(plugin_go.CodeGeneratorResponse) - resp.SupportedFeatures = p.supportedField + resp.SupportedFeatures = p.supportedFeatures for _, a := range arts { switch a := a.(type) { diff --git a/proto.go b/proto.go index f615e7c..a3b78df 100644 --- a/proto.go +++ b/proto.go @@ -16,7 +16,7 @@ const ( // Proto3 syntax permits the use of "optional" field presence. Non optional fields default to the zero // value of that particular type if not defined. // Most of the field types in the generated go structs are value types. - // See: https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md#presence-in-proto3-apis + // See: https://github.com/protocolbuffers/protobuf/blob/v3.17.0/docs/field_presence.md#presence-in-proto3-apis Proto3 Syntax = "proto3" ) From 7af882f7f9c9484817568bfa6dc2e76f66f05e8b Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Sat, 7 Aug 2021 19:19:50 +0100 Subject: [PATCH 09/10] pgs (field presence): nit/fixes - revert travis coverage for older protoc versions - revert backwards incompatible change for isOptional - nit/style fixes Signed-off-by: Sarthak Gupta --- .travis.yml | 2 ++ field.go | 9 +++++---- field_test.go | 3 +-- field_type.go | 2 +- field_type_test.go | 2 +- protoc-gen-debug/main.go | 4 +++- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0991525..51e345b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,8 @@ go_import_path: github.com/lyft/protoc-gen-star env: matrix: + - PROTOC_VER="3.5.0" + - PROTOC_VER="3.6.0" - PROTOC_VER="3.16.0" before_install: diff --git a/field.go b/field.go index 445d7f7..38bc1b4 100644 --- a/field.go +++ b/field.go @@ -87,11 +87,12 @@ func (f *field) HasPresence() bool { return true } - if f.HasOptionalKeyword() || - (f.Syntax() == Proto2 && !f.Type().IsRepeated() && !f.Type().IsMap()) { - return true + if !f.Type().IsRepeated() && !f.Type().IsMap() { + if f.Syntax() == Proto2 { + return true + } + return f.HasOptionalKeyword() } - return false } diff --git a/field_test.go b/field_test.go index 98c792c..a243cbc 100644 --- a/field_test.go +++ b/field_test.go @@ -122,7 +122,6 @@ func TestField_HasPresence(t *testing.T) { f.addType(&mapT{repT: &repT{scalarT: &scalarT{}}}) assert.False(t, f.HasPresence()) - f.msg = dummyMsg() f.addType(&scalarT{}) assert.False(t, f.HasPresence()) @@ -131,7 +130,7 @@ func TestField_HasPresence(t *testing.T) { assert.True(t, f.HasPresence()) } -func TestField_Optional(t *testing.T) { +func TestField_HasOptionalKeyword(t *testing.T) { t.Parallel() optLabel := descriptor.FieldDescriptorProto_LABEL_OPTIONAL diff --git a/field_type.go b/field_type.go index 3f33875..771341c 100644 --- a/field_type.go +++ b/field_type.go @@ -83,7 +83,7 @@ func (s *scalarT) Element() FieldTypeElem { return nil } func (s *scalarT) Key() FieldTypeElem { return nil } func (s *scalarT) IsOptional() bool { - return s.fld.HasOptionalKeyword() + return !s.fld.Syntax().SupportsRequiredPrefix() || s.ProtoLabel() == Optional } func (s *scalarT) IsRequired() bool { diff --git a/field_type_test.go b/field_type_test.go index 75f2553..66a043d 100644 --- a/field_type_test.go +++ b/field_type_test.go @@ -113,7 +113,7 @@ func TestScalarT_IsNotOptional(t *testing.T) { f := dummyField() f.addType(s) - assert.False(t, s.IsOptional()) + assert.True(t, s.IsOptional()) fl := dummyFile() fl.desc.Syntax = nil diff --git a/protoc-gen-debug/main.go b/protoc-gen-debug/main.go index 27159a7..289738d 100644 --- a/protoc-gen-debug/main.go +++ b/protoc-gen-debug/main.go @@ -5,13 +5,14 @@ package main import ( "bytes" - "google.golang.org/protobuf/types/pluginpb" "io" "io/ioutil" "log" "os" "path/filepath" + "google.golang.org/protobuf/types/pluginpb" + "github.com/golang/protobuf/proto" plugin_go "github.com/golang/protobuf/protoc-gen-go/plugin" ) @@ -42,6 +43,7 @@ func main() { log.Fatal("unable to write request to disk: ", err) } + // protoc-gen-debug supports proto3 field presence for testing purposes var supportedFeatures = uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) if data, err = proto.Marshal(&plugin_go.CodeGeneratorResponse{ SupportedFeatures: &supportedFeatures, From 3eb97391c0192f3d471c5e07f8f3f7199bf1e991 Mon Sep 17 00:00:00 2001 From: Sarthak Gupta Date: Sat, 7 Aug 2021 21:31:46 +0100 Subject: [PATCH 10/10] pgs (field presence): split proto3 tests This change splits the testing harness for proto3 field presence, so we can retain functional test coverage for older protoc versions that do not support proto3 field presence. IE >v3.15.0 Signed-off-by: Sarthak Gupta --- .travis.yml | 2 +- Makefile | 17 +++ lang/go/Makefile | 13 ++ lang/go/testdata/names/types/proto3.proto | 22 --- lang/go/testdata/presence/types/params | 0 lang/go/testdata/presence/types/proto3.proto | 67 +++++++++ lang/go/type_name_p2_presence_test.go | 121 +++++++++++++++++ lang/go/type_name_p3_presence_test.go | 90 +++++++++++++ lang/go/type_name_test.go | 135 +------------------ 9 files changed, 311 insertions(+), 156 deletions(-) create mode 100644 lang/go/testdata/presence/types/params create mode 100644 lang/go/testdata/presence/types/proto3.proto create mode 100644 lang/go/type_name_p2_presence_test.go create mode 100644 lang/go/type_name_p3_presence_test.go diff --git a/.travis.yml b/.travis.yml index 51e345b..f05c01d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ env: matrix: - PROTOC_VER="3.5.0" - PROTOC_VER="3.6.0" - - PROTOC_VER="3.16.0" + - PROTOC_VER="3.17.0" before_install: - mkdir -p $GOPATH/bin diff --git a/Makefile b/Makefile index 7a6d4b9..c679944 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,6 @@ # the name of this package PKG := $(shell go list .) +PROTOC_VER := $(shell protoc --version | cut -d' ' -f2) .PHONY: bootstrap bootstrap: testdata # set up the project for development @@ -16,15 +17,27 @@ lint: # lints the package for common code smells .PHONY: quick quick: testdata # runs all tests without the race detector or coverage +ifeq ($(PROTOC_VER), 3.17.0) + go test $(PKGS) --tags=proto3_presence +else go test $(PKGS) +endif .PHONY: tests tests: testdata # runs all tests against the package with race detection and coverage percentage +ifeq ($(PROTOC_VER), 3.17.0) + go test -race -cover ./... --tags=proto3_presence +else go test -race -cover ./... +endif .PHONY: cover cover: testdata # runs all tests against the package, generating a coverage report and opening it in the browser +ifeq ($(PROTOC_VER), 3.17.0) + go test -race -covermode=atomic -coverprofile=cover.out ./... --tags=proto3_presence || true +else go test -race -covermode=atomic -coverprofile=cover.out ./... || true +endif go tool cover -html cover.out -o cover.html open cover.html @@ -76,6 +89,10 @@ testdata-go: protoc-gen-go bin/protoc-gen-debug # generate go-specific testdata testdata-names \ testdata-packages \ testdata-outputs +ifeq ($(PROTOC_VER), 3.17.0) + cd lang/go && $(MAKE) \ + testdata-presence +endif vendor: # install project dependencies which glide || (curl https://glide.sh/get | sh) diff --git a/lang/go/Makefile b/lang/go/Makefile index cf308ad..565dc2c 100644 --- a/lang/go/Makefile +++ b/lang/go/Makefile @@ -38,5 +38,18 @@ testdata-outputs: ../../bin/protoc-gen-debug cd -; \ done +testdata-presence: ../../bin/protoc-gen-debug + cd testdata/presence && \ + set -e; for subdir in `find . -mindepth 1 -maxdepth 1 -type d`; do \ + cd $$subdir; \ + params=`cat params`; \ + protoc -I . -I .. \ + --plugin=protoc-gen-debug=../../../../../bin/protoc-gen-debug \ + --debug_out=".:." \ + --go_out="$$params:." \ + `find . -name "*.proto"`; \ + cd -; \ + done + ../../bin/protoc-gen-debug: cd ../.. && $(MAKE) bin/protoc-gen-debug diff --git a/lang/go/testdata/names/types/proto3.proto b/lang/go/testdata/names/types/proto3.proto index 2a565f5..a797dd9 100644 --- a/lang/go/testdata/names/types/proto3.proto +++ b/lang/go/testdata/names/types/proto3.proto @@ -42,26 +42,4 @@ message Proto3 { enum Enum {VALUE = 0;} message Message {} - - message Optional { - optional double double = 1; - optional float float = 2; - optional int64 int64 = 3; - optional sfixed64 sfixed64 = 4; - optional sint64 sint64 = 5; - optional uint64 uint64 = 6; - optional fixed64 fixed64 = 7; - optional int32 int32 = 8; - optional sfixed32 sfixed32 = 9; - optional sint32 sint32 = 10; - optional uint32 uint32 = 11; - optional fixed32 fixed32 = 12; - optional bool bool = 13; - optional string string = 14; - optional bytes bytes = 15; - optional Enum enum = 16; - optional google.protobuf.Syntax ext_enum = 17; - optional Optional msg = 18; - optional google.protobuf.Duration ext_msg = 19; - } } diff --git a/lang/go/testdata/presence/types/params b/lang/go/testdata/presence/types/params new file mode 100644 index 0000000..e69de29 diff --git a/lang/go/testdata/presence/types/proto3.proto b/lang/go/testdata/presence/types/proto3.proto new file mode 100644 index 0000000..2a565f5 --- /dev/null +++ b/lang/go/testdata/presence/types/proto3.proto @@ -0,0 +1,67 @@ +syntax="proto3"; +package names.types; +option go_package = "example.com/foo/bar"; + +import "google/protobuf/duration.proto"; +import "google/protobuf/type.proto"; + +message Proto3 { + double double = 1; + float float = 2; + int64 int64 = 3; + sfixed64 sfixed64 = 4; + sint64 sint64 = 5; + uint64 uint64 = 6; + fixed64 fixed64 = 7; + int32 int32 = 8; + sfixed32 sfixed32 = 9; + sint32 sint32 = 10; + uint32 uint32 = 11; + fixed32 fixed32 = 12; + bool bool = 13; + string string = 14; + bytes bytes = 15; + + Enum enum = 16; + google.protobuf.Syntax ext_enum = 17; + Message msg = 18; + google.protobuf.Duration ext_msg = 19; + + repeated double repeated_scalar = 20; + repeated Enum repeated_enum = 21; + repeated google.protobuf.Syntax repeated_ext_enum = 22; + repeated Message repeated_msg = 23; + repeated google.protobuf.Duration repeated_ext_msg = 24; + + map map_scalar = 25; + map map_enum = 26; + map map_ext_enum = 27; + map map_msg = 28; + map map_ext_msg = 29; + + enum Enum {VALUE = 0;} + + message Message {} + + message Optional { + optional double double = 1; + optional float float = 2; + optional int64 int64 = 3; + optional sfixed64 sfixed64 = 4; + optional sint64 sint64 = 5; + optional uint64 uint64 = 6; + optional fixed64 fixed64 = 7; + optional int32 int32 = 8; + optional sfixed32 sfixed32 = 9; + optional sint32 sint32 = 10; + optional uint32 uint32 = 11; + optional fixed32 fixed32 = 12; + optional bool bool = 13; + optional string string = 14; + optional bytes bytes = 15; + optional Enum enum = 16; + optional google.protobuf.Syntax ext_enum = 17; + optional Optional msg = 18; + optional google.protobuf.Duration ext_msg = 19; + } +} diff --git a/lang/go/type_name_p2_presence_test.go b/lang/go/type_name_p2_presence_test.go new file mode 100644 index 0000000..c09cd7d --- /dev/null +++ b/lang/go/type_name_p2_presence_test.go @@ -0,0 +1,121 @@ +// +build !proto3_presence + +package pgsgo + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + pgs "github.com/lyft/protoc-gen-star" +) + +func TestType(t *testing.T) { + t.Parallel() + + ast := buildGraph(t, "names", "types") + ctx := loadContext(t, "names", "types") + + tests := []struct { + field string + expected TypeName + }{ + // proto2 syntax, optional + {"Proto2.double", "*float64"}, + {"Proto2.float", "*float32"}, + {"Proto2.int64", "*int64"}, + {"Proto2.sfixed64", "*int64"}, + {"Proto2.sint64", "*int64"}, + {"Proto2.uint64", "*uint64"}, + {"Proto2.fixed64", "*uint64"}, + {"Proto2.int32", "*int32"}, + {"Proto2.sfixed32", "*int32"}, + {"Proto2.sint32", "*int32"}, + {"Proto2.uint32", "*uint32"}, + {"Proto2.fixed32", "*uint32"}, + {"Proto2.bool", "*bool"}, + {"Proto2.string", "*string"}, + {"Proto2.bytes", "[]byte"}, + {"Proto2.enum", "*Proto2_Enum"}, + {"Proto2.ext_enum", "*ptype.Syntax"}, + {"Proto2.msg", "*Proto2_Required"}, + {"Proto2.ext_msg", "*duration.Duration"}, + {"Proto2.repeated_scalar", "[]float64"}, + {"Proto2.repeated_enum", "[]Proto2_Enum"}, + {"Proto2.repeated_ext_enum", "[]ptype.Syntax"}, + {"Proto2.repeated_msg", "[]*Proto2_Required"}, + {"Proto2.repeated_ext_msg", "[]*duration.Duration"}, + {"Proto2.map_scalar", "map[string]float32"}, + {"Proto2.map_enum", "map[int32]Proto2_Enum"}, + {"Proto2.map_ext_enum", "map[uint64]ptype.Syntax"}, + {"Proto2.map_msg", "map[uint32]*Proto2_Required"}, + {"Proto2.map_ext_msg", "map[int64]*duration.Duration"}, + + // proto2 syntax, required + {"Proto2.Required.double", "*float64"}, + {"Proto2.Required.float", "*float32"}, + {"Proto2.Required.int64", "*int64"}, + {"Proto2.Required.sfixed64", "*int64"}, + {"Proto2.Required.sint64", "*int64"}, + {"Proto2.Required.uint64", "*uint64"}, + {"Proto2.Required.fixed64", "*uint64"}, + {"Proto2.Required.int32", "*int32"}, + {"Proto2.Required.sfixed32", "*int32"}, + {"Proto2.Required.sint32", "*int32"}, + {"Proto2.Required.uint32", "*uint32"}, + {"Proto2.Required.fixed32", "*uint32"}, + {"Proto2.Required.bool", "*bool"}, + {"Proto2.Required.string", "*string"}, + {"Proto2.Required.bytes", "[]byte"}, + {"Proto2.Required.enum", "*Proto2_Enum"}, + {"Proto2.Required.ext_enum", "*ptype.Syntax"}, + {"Proto2.Required.msg", "*Proto2_Required"}, + {"Proto2.Required.ext_msg", "*duration.Duration"}, + + {"Proto3.double", "float64"}, + {"Proto3.float", "float32"}, + {"Proto3.int64", "int64"}, + {"Proto3.sfixed64", "int64"}, + {"Proto3.sint64", "int64"}, + {"Proto3.uint64", "uint64"}, + {"Proto3.fixed64", "uint64"}, + {"Proto3.int32", "int32"}, + {"Proto3.sfixed32", "int32"}, + {"Proto3.sint32", "int32"}, + {"Proto3.uint32", "uint32"}, + {"Proto3.fixed32", "uint32"}, + {"Proto3.bool", "bool"}, + {"Proto3.string", "string"}, + {"Proto3.bytes", "[]byte"}, + {"Proto3.enum", "Proto3_Enum"}, + {"Proto3.ext_enum", "ptype.Syntax"}, + {"Proto3.msg", "*Proto3_Message"}, + {"Proto3.ext_msg", "*duration.Duration"}, + {"Proto3.repeated_scalar", "[]float64"}, + {"Proto3.repeated_enum", "[]Proto3_Enum"}, + {"Proto3.repeated_ext_enum", "[]ptype.Syntax"}, + {"Proto3.repeated_msg", "[]*Proto3_Message"}, + {"Proto3.repeated_ext_msg", "[]*duration.Duration"}, + {"Proto3.map_scalar", "map[string]float32"}, + {"Proto3.map_enum", "map[int32]Proto3_Enum"}, + {"Proto3.map_ext_enum", "map[uint64]ptype.Syntax"}, + {"Proto3.map_msg", "map[uint32]*Proto3_Message"}, + {"Proto3.map_ext_msg", "map[int64]*duration.Duration"}, + } + + for _, test := range tests { + tc := test + t.Run(tc.field, func(t *testing.T) { + t.Parallel() + + e, ok := ast.Lookup(".names.types." + tc.field) + require.True(t, ok, "could not find field") + + fld, ok := e.(pgs.Field) + require.True(t, ok, "entity is not a field") + + assert.Equal(t, tc.expected, ctx.Type(fld)) + }) + } +} diff --git a/lang/go/type_name_p3_presence_test.go b/lang/go/type_name_p3_presence_test.go new file mode 100644 index 0000000..2312591 --- /dev/null +++ b/lang/go/type_name_p3_presence_test.go @@ -0,0 +1,90 @@ +// +build proto3_presence + +package pgsgo + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + pgs "github.com/lyft/protoc-gen-star" +) + +func TestType(t *testing.T) { + t.Parallel() + + ast := buildGraph(t, "presence", "types") + ctx := loadContext(t, "presence", "types") + + tests := []struct { + field string + expected TypeName + }{ + {"Proto3.double", "float64"}, + {"Proto3.float", "float32"}, + {"Proto3.int64", "int64"}, + {"Proto3.sfixed64", "int64"}, + {"Proto3.sint64", "int64"}, + {"Proto3.uint64", "uint64"}, + {"Proto3.fixed64", "uint64"}, + {"Proto3.int32", "int32"}, + {"Proto3.sfixed32", "int32"}, + {"Proto3.sint32", "int32"}, + {"Proto3.uint32", "uint32"}, + {"Proto3.fixed32", "uint32"}, + {"Proto3.bool", "bool"}, + {"Proto3.string", "string"}, + {"Proto3.bytes", "[]byte"}, + {"Proto3.enum", "Proto3_Enum"}, + {"Proto3.ext_enum", "typepb.Syntax"}, + {"Proto3.msg", "*Proto3_Message"}, + {"Proto3.ext_msg", "*durationpb.Duration"}, + {"Proto3.repeated_scalar", "[]float64"}, + {"Proto3.repeated_enum", "[]Proto3_Enum"}, + {"Proto3.repeated_ext_enum", "[]typepb.Syntax"}, + {"Proto3.repeated_msg", "[]*Proto3_Message"}, + {"Proto3.repeated_ext_msg", "[]*durationpb.Duration"}, + {"Proto3.map_scalar", "map[string]float32"}, + {"Proto3.map_enum", "map[int32]Proto3_Enum"}, + {"Proto3.map_ext_enum", "map[uint64]typepb.Syntax"}, + {"Proto3.map_msg", "map[uint32]*Proto3_Message"}, + {"Proto3.map_ext_msg", "map[int64]*durationpb.Duration"}, + + // proto3 syntax optional + {"Proto3.Optional.double", "*float64"}, + {"Proto3.Optional.float", "*float32"}, + {"Proto3.Optional.int64", "*int64"}, + {"Proto3.Optional.sfixed64", "*int64"}, + {"Proto3.Optional.sint64", "*int64"}, + {"Proto3.Optional.uint64", "*uint64"}, + {"Proto3.Optional.fixed64", "*uint64"}, + {"Proto3.Optional.int32", "*int32"}, + {"Proto3.Optional.sfixed32", "*int32"}, + {"Proto3.Optional.sint32", "*int32"}, + {"Proto3.Optional.uint32", "*uint32"}, + {"Proto3.Optional.fixed32", "*uint32"}, + {"Proto3.Optional.bool", "*bool"}, + {"Proto3.Optional.string", "*string"}, + {"Proto3.Optional.bytes", "[]byte"}, + {"Proto3.Optional.enum", "*Proto3_Enum"}, + {"Proto3.Optional.ext_enum", "*typepb.Syntax"}, + {"Proto3.Optional.msg", "*Proto3_Optional"}, + {"Proto3.Optional.ext_msg", "*durationpb.Duration"}, + } + + for _, test := range tests { + tc := test + t.Run(tc.field, func(t *testing.T) { + t.Parallel() + + e, ok := ast.Lookup(".names.types." + tc.field) + require.True(t, ok, "could not find field") + + fld, ok := e.(pgs.Field) + require.True(t, ok, "entity is not a field") + + assert.Equal(t, tc.expected, ctx.Type(fld)) + }) + } +} diff --git a/lang/go/type_name_test.go b/lang/go/type_name_test.go index 66ab087..b1282ee 100644 --- a/lang/go/type_name_test.go +++ b/lang/go/type_name_test.go @@ -4,141 +4,10 @@ import ( "fmt" "testing" - pgs "github.com/lyft/protoc-gen-star" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/assert" -) - -func TestType(t *testing.T) { - t.Parallel() - - ast := buildGraph(t, "names", "types") - ctx := loadContext(t, "names", "types") - - tests := []struct { - field string - expected TypeName - }{ - // proto2 syntax, optional - {"Proto2.double", "*float64"}, - {"Proto2.float", "*float32"}, - {"Proto2.int64", "*int64"}, - {"Proto2.sfixed64", "*int64"}, - {"Proto2.sint64", "*int64"}, - {"Proto2.uint64", "*uint64"}, - {"Proto2.fixed64", "*uint64"}, - {"Proto2.int32", "*int32"}, - {"Proto2.sfixed32", "*int32"}, - {"Proto2.sint32", "*int32"}, - {"Proto2.uint32", "*uint32"}, - {"Proto2.fixed32", "*uint32"}, - {"Proto2.bool", "*bool"}, - {"Proto2.string", "*string"}, - {"Proto2.bytes", "[]byte"}, - {"Proto2.enum", "*Proto2_Enum"}, - {"Proto2.ext_enum", "*typepb.Syntax"}, - {"Proto2.msg", "*Proto2_Required"}, - {"Proto2.ext_msg", "*durationpb.Duration"}, - {"Proto2.repeated_scalar", "[]float64"}, - {"Proto2.repeated_enum", "[]Proto2_Enum"}, - {"Proto2.repeated_ext_enum", "[]typepb.Syntax"}, - {"Proto2.repeated_msg", "[]*Proto2_Required"}, - {"Proto2.repeated_ext_msg", "[]*durationpb.Duration"}, - {"Proto2.map_scalar", "map[string]float32"}, - {"Proto2.map_enum", "map[int32]Proto2_Enum"}, - {"Proto2.map_ext_enum", "map[uint64]typepb.Syntax"}, - {"Proto2.map_msg", "map[uint32]*Proto2_Required"}, - {"Proto2.map_ext_msg", "map[int64]*durationpb.Duration"}, - - // proto2 syntax, required - {"Proto2.Required.double", "*float64"}, - {"Proto2.Required.float", "*float32"}, - {"Proto2.Required.int64", "*int64"}, - {"Proto2.Required.sfixed64", "*int64"}, - {"Proto2.Required.sint64", "*int64"}, - {"Proto2.Required.uint64", "*uint64"}, - {"Proto2.Required.fixed64", "*uint64"}, - {"Proto2.Required.int32", "*int32"}, - {"Proto2.Required.sfixed32", "*int32"}, - {"Proto2.Required.sint32", "*int32"}, - {"Proto2.Required.uint32", "*uint32"}, - {"Proto2.Required.fixed32", "*uint32"}, - {"Proto2.Required.bool", "*bool"}, - {"Proto2.Required.string", "*string"}, - {"Proto2.Required.bytes", "[]byte"}, - {"Proto2.Required.enum", "*Proto2_Enum"}, - {"Proto2.Required.ext_enum", "*typepb.Syntax"}, - {"Proto2.Required.msg", "*Proto2_Required"}, - {"Proto2.Required.ext_msg", "*durationpb.Duration"}, - - {"Proto3.double", "float64"}, - {"Proto3.float", "float32"}, - {"Proto3.int64", "int64"}, - {"Proto3.sfixed64", "int64"}, - {"Proto3.sint64", "int64"}, - {"Proto3.uint64", "uint64"}, - {"Proto3.fixed64", "uint64"}, - {"Proto3.int32", "int32"}, - {"Proto3.sfixed32", "int32"}, - {"Proto3.sint32", "int32"}, - {"Proto3.uint32", "uint32"}, - {"Proto3.fixed32", "uint32"}, - {"Proto3.bool", "bool"}, - {"Proto3.string", "string"}, - {"Proto3.bytes", "[]byte"}, - {"Proto3.enum", "Proto3_Enum"}, - {"Proto3.ext_enum", "typepb.Syntax"}, - {"Proto3.msg", "*Proto3_Message"}, - {"Proto3.ext_msg", "*durationpb.Duration"}, - {"Proto3.repeated_scalar", "[]float64"}, - {"Proto3.repeated_enum", "[]Proto3_Enum"}, - {"Proto3.repeated_ext_enum", "[]typepb.Syntax"}, - {"Proto3.repeated_msg", "[]*Proto3_Message"}, - {"Proto3.repeated_ext_msg", "[]*durationpb.Duration"}, - {"Proto3.map_scalar", "map[string]float32"}, - {"Proto3.map_enum", "map[int32]Proto3_Enum"}, - {"Proto3.map_ext_enum", "map[uint64]typepb.Syntax"}, - {"Proto3.map_msg", "map[uint32]*Proto3_Message"}, - {"Proto3.map_ext_msg", "map[int64]*durationpb.Duration"}, - - // proto3 syntax optional - {"Proto3.Optional.double", "*float64"}, - {"Proto3.Optional.float", "*float32"}, - {"Proto3.Optional.int64", "*int64"}, - {"Proto3.Optional.sfixed64", "*int64"}, - {"Proto3.Optional.sint64", "*int64"}, - {"Proto3.Optional.uint64", "*uint64"}, - {"Proto3.Optional.fixed64", "*uint64"}, - {"Proto3.Optional.int32", "*int32"}, - {"Proto3.Optional.sfixed32", "*int32"}, - {"Proto3.Optional.sint32", "*int32"}, - {"Proto3.Optional.uint32", "*uint32"}, - {"Proto3.Optional.fixed32", "*uint32"}, - {"Proto3.Optional.bool", "*bool"}, - {"Proto3.Optional.string", "*string"}, - {"Proto3.Optional.bytes", "[]byte"}, - {"Proto3.Optional.enum", "*Proto3_Enum"}, - {"Proto3.Optional.ext_enum", "*typepb.Syntax"}, - {"Proto3.Optional.msg", "*Proto3_Optional"}, - {"Proto3.Optional.ext_msg", "*durationpb.Duration"}, - } - - for _, test := range tests { - tc := test - t.Run(tc.field, func(t *testing.T) { - t.Parallel() - - e, ok := ast.Lookup(".names.types." + tc.field) - require.True(t, ok, "could not find field") - fld, ok := e.(pgs.Field) - require.True(t, ok, "entity is not a field") - - assert.Equal(t, tc.expected, ctx.Type(fld)) - }) - } -} + pgs "github.com/lyft/protoc-gen-star" +) func TestTypeName(t *testing.T) { t.Parallel()