From 21d187aa53794eb29c1b5d94dac44987433e283a Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 19 Aug 2024 16:19:41 +0200 Subject: [PATCH] compiler/protogen: support -experimental_strip_nonfunctional_codegen This flag is used by the (Google-internal) editions codegen test. Having the functionality in the open source repository reduces our maintenance burden (fewer/smaller patches). Change-Id: Idb9c95e9b2cb8922584bcb7ca13a8ddef4347af0 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/606735 LUCI-TryBot-Result: Go LUCI Reviewed-by: Lasse Folger --- cmd/protoc-gen-go/internal_gengo/main.go | 29 ++++++-- cmd/protoc-gen-go/main.go | 8 ++- compiler/protogen/protogen.go | 85 +++++++++++++++++------- 3 files changed, 90 insertions(+), 32 deletions(-) diff --git a/cmd/protoc-gen-go/internal_gengo/main.go b/cmd/protoc-gen-go/internal_gengo/main.go index 5402dfd3..1a3a3426 100644 --- a/cmd/protoc-gen-go/internal_gengo/main.go +++ b/cmd/protoc-gen-go/internal_gengo/main.go @@ -76,11 +76,14 @@ func GenerateFile(gen *protogen.Plugin, file *protogen.File) *protogen.Generated g := gen.NewGeneratedFile(filename, file.GoImportPath) f := newFileInfo(file) - genStandaloneComments(g, f, int32(genid.FileDescriptorProto_Syntax_field_number)) - genGeneratedHeader(gen, g, f) - genStandaloneComments(g, f, int32(genid.FileDescriptorProto_Package_field_number)) + var packageDoc protogen.Comments + if !gen.InternalStripForEditionsDiff() { + genStandaloneComments(g, f, int32(genid.FileDescriptorProto_Syntax_field_number)) + genGeneratedHeader(gen, g, f) + genStandaloneComments(g, f, int32(genid.FileDescriptorProto_Package_field_number)) - packageDoc := genPackageKnownComment(f) + packageDoc = genPackageKnownComment(f) + } g.P(packageDoc, "package ", f.GoPackageName) g.P() @@ -106,7 +109,23 @@ func GenerateFile(gen *protogen.Plugin, file *protogen.File) *protogen.Generated } genExtensions(g, f) - genReflectFileDescriptor(gen, g, f) + // The descriptor contains a lot of information about the syntax which is + // quite different between the proto2/3 version of a file and the equivalent + // editions version. For example, when a proto3 file is translated from + // proto3 to editions every field in that file that is marked optional in + // proto3 will have a features.field_presence option set. + // Another problem is that the descriptor contains implementation details + // that are not relevant for the semantic. For example, proto3 optional + // fields are implemented as oneof fields with one case. The descriptor + // contains different information about oneofs. If the file is translated + // to editions it no longer is treated as a oneof with one case and thus + // none of the oneof specific information is generated. + // To be able to compare the descriptor before and after translation of the + // associated proto file, we would need to trim many parts. This would lead + // to a brittle implementation in case the translation ever changes. + if !g.InternalStripForEditionsDiff() { + genReflectFileDescriptor(gen, g, f) + } return g } diff --git a/cmd/protoc-gen-go/main.go b/cmd/protoc-gen-go/main.go index 431fbb95..f3b5acb5 100644 --- a/cmd/protoc-gen-go/main.go +++ b/cmd/protoc-gen-go/main.go @@ -35,11 +35,13 @@ func main() { } var ( - flags flag.FlagSet - plugins = flags.String("plugins", "", "deprecated option") + flags flag.FlagSet + plugins = flags.String("plugins", "", "deprecated option") + experimentalStripNonFunctionalCodegen = flags.Bool("experimental_strip_nonfunctional_codegen", false, "experimental_strip_nonfunctional_codegen true means that the plugin will not emit certain parts of the generated code in order to make it possible to compare a proto2/proto3 file with its equivalent (according to proto spec) editions file. Primarily, this is the encoded descriptor.") ) protogen.Options{ - ParamFunc: flags.Set, + ParamFunc: flags.Set, + InternalStripForEditionsDiff: experimentalStripNonFunctionalCodegen, }.Run(func(gen *protogen.Plugin) error { if *plugins != "" { return errors.New("protoc-gen-go: plugins are not supported; use 'protoc --go-grpc_out=...' to generate gRPC\n\n" + diff --git a/compiler/protogen/protogen.go b/compiler/protogen/protogen.go index 3c116441..966728ed 100644 --- a/compiler/protogen/protogen.go +++ b/compiler/protogen/protogen.go @@ -152,6 +152,18 @@ type Options struct { // imported by a generated file. It returns the import path to use // for this package. ImportRewriteFunc func(GoImportPath) GoImportPath + + // StripForEditionsDiff true means that the plugin will not emit certain + // parts of the generated code in order to make it possible to compare a + // proto2/proto3 file with its equivalent (according to proto spec) + // editions file. Primarily, this is the encoded descriptor. + // + // This must be a registered flag that is initialized by ParamFunc. It will + // be used by Options.New after it has parsed the flags. + // + // This struct field is for internal use by Go Protobuf only. Do not use it, + // we might remove it at any time. + InternalStripForEditionsDiff *bool } // New returns a new Plugin. @@ -344,6 +356,15 @@ func (opts Options) New(req *pluginpb.CodeGeneratorRequest) (*Plugin, error) { return gen, nil } +// InternalStripForEditionsDiff returns whether or not to strip non-functional +// codegen for editions diff testing. +// +// This function is for internal use by Go Protobuf only. Do not use it, we +// might remove it at any time. +func (gen *Plugin) InternalStripForEditionsDiff() bool { + return gen.opts.InternalStripForEditionsDiff != nil && *gen.opts.InternalStripForEditionsDiff +} + // Error records an error in code generation. The generator will report the // error back to protoc and will not produce output. func (gen *Plugin) Error(err error) { @@ -538,7 +559,7 @@ func newEnum(gen *Plugin, f *File, parent *Message, desc protoreflect.EnumDescri Desc: desc, GoIdent: newGoIdent(f, desc), Location: loc, - Comments: makeCommentSet(f.Desc.SourceLocations().ByDescriptor(desc)), + Comments: makeCommentSet(gen, f.Desc.SourceLocations().ByDescriptor(desc)), } gen.enumsByName[desc.FullName()] = enum for i, vds := 0, enum.Desc.Values(); i < vds.Len(); i++ { @@ -575,7 +596,7 @@ func newEnumValue(gen *Plugin, f *File, message *Message, enum *Enum, desc proto GoIdent: f.GoImportPath.Ident(name), Parent: enum, Location: loc, - Comments: makeCommentSet(f.Desc.SourceLocations().ByDescriptor(desc)), + Comments: makeCommentSet(gen, f.Desc.SourceLocations().ByDescriptor(desc)), } } @@ -607,7 +628,7 @@ func newMessage(gen *Plugin, f *File, parent *Message, desc protoreflect.Message Desc: desc, GoIdent: newGoIdent(f, desc), Location: loc, - Comments: makeCommentSet(f.Desc.SourceLocations().ByDescriptor(desc)), + Comments: makeCommentSet(gen, f.Desc.SourceLocations().ByDescriptor(desc)), } gen.messagesByName[desc.FullName()] = message for i, eds := 0, desc.Enums(); i < eds.Len(); i++ { @@ -777,7 +798,7 @@ func newField(gen *Plugin, f *File, message *Message, desc protoreflect.FieldDes }, Parent: message, Location: loc, - Comments: makeCommentSet(f.Desc.SourceLocations().ByDescriptor(desc)), + Comments: makeCommentSet(gen, f.Desc.SourceLocations().ByDescriptor(desc)), } return field } @@ -844,7 +865,7 @@ func newOneof(gen *Plugin, f *File, message *Message, desc protoreflect.OneofDes GoName: parentPrefix + camelCased, }, Location: loc, - Comments: makeCommentSet(f.Desc.SourceLocations().ByDescriptor(desc)), + Comments: makeCommentSet(gen, f.Desc.SourceLocations().ByDescriptor(desc)), } } @@ -869,7 +890,7 @@ func newService(gen *Plugin, f *File, desc protoreflect.ServiceDescriptor) *Serv Desc: desc, GoName: strs.GoCamelCase(string(desc.Name())), Location: loc, - Comments: makeCommentSet(f.Desc.SourceLocations().ByDescriptor(desc)), + Comments: makeCommentSet(gen, f.Desc.SourceLocations().ByDescriptor(desc)), } for i, mds := 0, desc.Methods(); i < mds.Len(); i++ { service.Methods = append(service.Methods, newMethod(gen, f, service, mds.Get(i))) @@ -899,7 +920,7 @@ func newMethod(gen *Plugin, f *File, service *Service, desc protoreflect.MethodD GoName: strs.GoCamelCase(string(desc.Name())), Parent: service, Location: loc, - Comments: makeCommentSet(f.Desc.SourceLocations().ByDescriptor(desc)), + Comments: makeCommentSet(gen, f.Desc.SourceLocations().ByDescriptor(desc)), } return method } @@ -926,28 +947,30 @@ func (method *Method) resolveDependencies(gen *Plugin) error { // A GeneratedFile is a generated file. type GeneratedFile struct { - gen *Plugin - skip bool - filename string - goImportPath GoImportPath - buf bytes.Buffer - packageNames map[GoImportPath]GoPackageName - usedPackageNames map[GoPackageName]bool - manualImports map[GoImportPath]bool - annotations map[string][]Annotation + gen *Plugin + skip bool + filename string + goImportPath GoImportPath + buf bytes.Buffer + packageNames map[GoImportPath]GoPackageName + usedPackageNames map[GoPackageName]bool + manualImports map[GoImportPath]bool + annotations map[string][]Annotation + stripForEditionsDiff bool } // NewGeneratedFile creates a new generated file with the given filename // and import path. func (gen *Plugin) NewGeneratedFile(filename string, goImportPath GoImportPath) *GeneratedFile { g := &GeneratedFile{ - gen: gen, - filename: filename, - goImportPath: goImportPath, - packageNames: make(map[GoImportPath]GoPackageName), - usedPackageNames: make(map[GoPackageName]bool), - manualImports: make(map[GoImportPath]bool), - annotations: make(map[string][]Annotation), + gen: gen, + filename: filename, + goImportPath: goImportPath, + packageNames: make(map[GoImportPath]GoPackageName), + usedPackageNames: make(map[GoPackageName]bool), + manualImports: make(map[GoImportPath]bool), + annotations: make(map[string][]Annotation), + stripForEditionsDiff: gen.InternalStripForEditionsDiff(), } // All predeclared identifiers in Go are already used. @@ -1020,6 +1043,17 @@ func (g *GeneratedFile) Unskip() { g.skip = false } +// InternalStripForEditionsDiff returns true if the plugin should not emit certain +// parts of the generated code in order to make it possible to compare a +// proto2/proto3 file with its equivalent (according to proto spec) editions +// file. Primarily, this is the encoded descriptor. +// +// This function is for internal use by Go Protobuf only. Do not use it, we +// might remove it at any time. +func (g *GeneratedFile) InternalStripForEditionsDiff() bool { + return g.stripForEditionsDiff +} + // Annotate associates a symbol in a generated Go file with a location in a // source .proto file. // @@ -1298,7 +1332,10 @@ type CommentSet struct { Trailing Comments } -func makeCommentSet(loc protoreflect.SourceLocation) CommentSet { +func makeCommentSet(gen *Plugin, loc protoreflect.SourceLocation) CommentSet { + if gen.InternalStripForEditionsDiff() { + return CommentSet{} + } var leadingDetached []Comments for _, s := range loc.LeadingDetachedComments { leadingDetached = append(leadingDetached, Comments(s))