compiler/protogen: move name mangling logic to protogen
The name mangling logic should be unified in a single place
rather than being split between compiler/protogen and cmd/protoc-gen-go.
Move it over compiler/protogen.
Change-Id: Iea65e0b3fba45e0c95c76e3fc1f061e0fa8f84d7
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/191117
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/cmd/protoc-gen-go/internal_gengo/main.go b/cmd/protoc-gen-go/internal_gengo/main.go
index 2a200a6..0bb1f4f 100644
--- a/cmd/protoc-gen-go/internal_gengo/main.go
+++ b/cmd/protoc-gen-go/internal_gengo/main.go
@@ -402,19 +402,6 @@
}
}
-// enumLegacyName returns the name used by the v1 proto package.
-//
-// Confusingly, this is <proto_package>.<go_ident>. This probably should have
-// been the full name of the proto enum type instead, but changing it at this
-// point would require thought.
-func enumLegacyName(enum *protogen.Enum) string {
- fdesc := enum.Desc.ParentFile()
- if fdesc.Package() == "" {
- return enum.GoIdent.GoName
- }
- return string(fdesc.Package()) + "." + enum.GoIdent.GoName
-}
-
func genMessage(gen *protogen.Plugin, g *protogen.GeneratedFile, f *fileInfo, message *protogen.Message) {
if message.Desc.IsMapEntry() {
return
@@ -501,7 +488,7 @@
}
ss := []string{fmt.Sprintf(" Types that are assignable to %s:\n", oneof.GoName)}
for _, field := range oneof.Fields {
- ss = append(ss, "\t*"+fieldOneofType(field).GoName+"\n")
+ ss = append(ss, "\t*"+field.GoIdent.GoName+"\n")
}
leadingComments += protogen.Comments(strings.Join(ss, ""))
g.P(leadingComments,
@@ -691,7 +678,7 @@
g.P("}")
case field.Oneof != nil:
g.P(leadingComments, "func (x *", message.GoIdent, ") Get", field.GoName, "() ", goType, " {")
- g.P("if x, ok := x.Get", field.Oneof.GoName, "().(*", fieldOneofType(field), "); ok {")
+ g.P("if x, ok := x.Get", field.Oneof.GoName, "().(*", field.GoIdent, "); ok {")
g.P("return x.", field.GoName)
g.P("}")
g.P("return ", defaultValue)
@@ -793,7 +780,14 @@
func fieldProtobufTagValue(field *protogen.Field) string {
var enumName string
if field.Desc.Kind() == protoreflect.EnumKind {
- enumName = enumLegacyName(field.Enum)
+ // For historical reasons, the name used in the tag is neither
+ // the protobuf full name nor the fully qualified Go identifier,
+ // but an odd mix of both.
+ enumName = field.Enum.GoIdent.GoName
+ protoPkg := string(field.Enum.Desc.ParentFile().Package())
+ if protoPkg != "" {
+ enumName = protoPkg + "." + enumName
+ }
}
return tag.Marshal(field.Desc, enumName)
}
@@ -891,7 +885,7 @@
leadingComments = appendDeprecationSuffix(leadingComments,
extension.Desc.Options().(*descriptorpb.FieldOptions).GetDeprecated())
g.P(leadingComments,
- extensionVar(f.File, extension), " = &", extensionTypesVarName(f), "[", allExtensionsByPtr[extension], "]",
+ "E_", extension.GoIdent, " = &", extensionTypesVarName(f), "[", allExtensionsByPtr[extension], "]",
trailingComment(extension.Comments.Trailing))
}
g.P(")")
@@ -899,16 +893,6 @@
}
}
-// extensionVar returns the var holding the ExtensionDesc for an extension.
-func extensionVar(f *protogen.File, extension *protogen.Extension) protogen.GoIdent {
- name := "E_"
- if extension.Parent != nil {
- name += extension.Parent.GoIdent.GoName + "_"
- }
- name += extension.GoName
- return f.GoImportPath.Ident(name)
-}
-
// genOneofWrapperTypes generates the oneof wrapper types and
// associates the types with the parent message type.
func genOneofWrapperTypes(gen *protogen.Plugin, g *protogen.GeneratedFile, f *fileInfo, message *protogen.Message) {
@@ -919,10 +903,9 @@
g.P("}")
g.P()
for _, field := range oneof.Fields {
- name := fieldOneofType(field)
- g.Annotate(name.GoName, field.Location)
- g.Annotate(name.GoName+"."+field.GoName, field.Location)
- g.P("type ", name, " struct {")
+ g.Annotate(field.GoIdent.GoName, field.Location)
+ g.Annotate(field.GoIdent.GoName+"."+field.GoName, field.Location)
+ g.P("type ", field.GoIdent, " struct {")
goType, _ := fieldGoType(g, f, field)
tags := structTags{
{"protobuf", fieldProtobufTagValue(field)},
@@ -936,7 +919,7 @@
g.P()
}
for _, field := range oneof.Fields {
- g.P("func (*", fieldOneofType(field), ") ", ifName, "() {}")
+ g.P("func (*", field.GoIdent, ") ", ifName, "() {}")
g.P()
}
}
@@ -945,39 +928,7 @@
// oneofInterfaceName returns the name of the interface type implemented by
// the oneof field value types.
func oneofInterfaceName(oneof *protogen.Oneof) string {
- return fmt.Sprintf("is%s_%s", oneof.Parent.GoIdent.GoName, oneof.GoName)
-}
-
-// fieldOneofType returns the wrapper type used to represent a field in a oneof.
-func fieldOneofType(field *protogen.Field) protogen.GoIdent {
- ident := protogen.GoIdent{
- GoImportPath: field.Parent.GoIdent.GoImportPath,
- GoName: field.Parent.GoIdent.GoName + "_" + field.GoName,
- }
- // Check for collisions with nested messages or enums.
- //
- // This conflict resolution is incomplete: Among other things, it
- // does not consider collisions with other oneof field types.
- //
- // TODO: Consider dropping this entirely. Detecting conflicts and
- // producing an error is almost certainly better than permuting
- // field and type names in mostly unpredictable ways.
-Loop:
- for {
- for _, message := range field.Parent.Messages {
- if message.GoIdent == ident {
- ident.GoName += "_"
- continue Loop
- }
- }
- for _, enum := range field.Parent.Enums {
- if enum.GoIdent == ident {
- ident.GoName += "_"
- continue Loop
- }
- }
- return ident
- }
+ return "is" + oneof.GoIdent.GoName
}
var jsonIgnoreTags = structTags{{"json", "-"}}
diff --git a/cmd/protoc-gen-go/internal_gengo/reflect.go b/cmd/protoc-gen-go/internal_gengo/reflect.go
index 63bbbd0..c0f2dfd 100644
--- a/cmd/protoc-gen-go/internal_gengo/reflect.go
+++ b/cmd/protoc-gen-go/internal_gengo/reflect.go
@@ -192,7 +192,7 @@
g.P(typesVar, "[", idx, "].OneofWrappers = []interface{} {")
for _, oneof := range message.Oneofs {
for _, field := range oneof.Fields {
- g.P("(*", fieldOneofType(field), ")(nil),")
+ g.P("(*", field.GoIdent, ")(nil),")
}
}
g.P("}")
diff --git a/compiler/protogen/protogen.go b/compiler/protogen/protogen.go
index 0620d3a..ec03704 100644
--- a/compiler/protogen/protogen.go
+++ b/compiler/protogen/protogen.go
@@ -556,7 +556,7 @@
// A top-level enum value's name is: EnumName_ValueName
// An enum value contained in a message is: MessageName_ValueName
//
- // Enum value names are not camelcased.
+ // For historical reasons, enum value names are not camel-cased.
parentIdent := enum.GoIdent
if message != nil {
parentIdent = message.GoIdent
@@ -661,15 +661,39 @@
usedNames["Get"+name] = hasGetter
return name
}
- seenOneofs := make(map[int]bool)
for _, field := range message.Fields {
field.GoName = makeNameUnique(field.GoName, true)
+ field.GoIdent.GoName = message.GoIdent.GoName + "_" + field.GoName
+ if field.Oneof != nil && field.Oneof.Fields[0] == field {
+ // Make the name for a oneof unique as well. For historical reasons,
+ // this assumes that a getter method is not generated for oneofs.
+ // This is incorrect, but fixing it breaks existing code.
+ field.Oneof.GoName = makeNameUnique(field.Oneof.GoName, false)
+ field.Oneof.GoIdent.GoName = message.GoIdent.GoName + "_" + field.Oneof.GoName
+ }
+ }
+
+ // Oneof field name conflict resolution.
+ //
+ // This conflict resolution is incomplete as it does not consider collisions
+ // with other oneof field types, but fixing it breaks existing code.
+ for _, field := range message.Fields {
if field.Oneof != nil {
- if !seenOneofs[field.Oneof.Desc.Index()] {
- // If this is a field in a oneof that we haven't seen before,
- // make the name for that oneof unique as well.
- field.Oneof.GoName = makeNameUnique(field.Oneof.GoName, false)
- seenOneofs[field.Oneof.Desc.Index()] = true
+ Loop:
+ for {
+ for _, nestedMessage := range message.Messages {
+ if nestedMessage.GoIdent == field.GoIdent {
+ field.GoIdent.GoName += "_"
+ continue Loop
+ }
+ }
+ for _, nestedEnum := range message.Enums {
+ if nestedEnum.GoIdent == field.GoIdent {
+ field.GoIdent.GoName += "_"
+ continue Loop
+ }
+ }
+ break Loop
}
}
}
@@ -703,7 +727,13 @@
// GoName is the base name of this field's Go field and methods.
// For code generated by protoc-gen-go, this means a field named
// '{{GoName}}' and a getter method named 'Get{{GoName}}'.
- GoName string
+ GoName string // e.g., "FieldName"
+
+ // GoIdent is the base name of a top-level declaration for this field.
+ // For code generated by protoc-gen-go, this means a wrapper type named
+ // '{{GoIdent}}' for members fields of a oneof, and a variable named
+ // 'E_{{GoIdent}}' for extension fields.
+ GoIdent GoIdent // e.g., "MessageName_FieldName"
Parent *Message // message in which this field is declared; nil if top-level extension
Oneof *Oneof // containing oneof; nil if not part of a oneof
@@ -726,9 +756,18 @@
default:
loc = message.Location.appendPath(fieldnum.DescriptorProto_Field, int32(desc.Index()))
}
+ camelCased := camelCase(string(desc.Name()))
+ var parentPrefix string
+ if message != nil {
+ parentPrefix = message.GoIdent.GoName + "_"
+ }
field := &Field{
- Desc: desc,
- GoName: camelCase(string(desc.Name())),
+ Desc: desc,
+ GoName: camelCased,
+ GoIdent: GoIdent{
+ GoImportPath: f.GoImportPath,
+ GoName: parentPrefix + camelCased,
+ },
Parent: message,
Location: loc,
Comments: f.comments[newPathKey(loc.Path)],
@@ -769,7 +808,13 @@
type Oneof struct {
Desc protoreflect.OneofDescriptor
- GoName string // Go field name of this oneof
+ // GoName is the base name of this oneof's Go field and methods.
+ // For code generated by protoc-gen-go, this means a field named
+ // '{{GoName}}' and a getter method named 'Get{{GoName}}'.
+ GoName string // e.g., "OneofName"
+
+ // GoIdent is the base name of a top-level declaration for this oneof.
+ GoIdent GoIdent // e.g., "MessageName_OneofName"
Parent *Message // message in which this oneof is declared
@@ -781,10 +826,16 @@
func newOneof(gen *Plugin, f *File, message *Message, desc protoreflect.OneofDescriptor) *Oneof {
loc := message.Location.appendPath(fieldnum.DescriptorProto_OneofDecl, int32(desc.Index()))
+ camelCased := camelCase(string(desc.Name()))
+ parentPrefix := message.GoIdent.GoName + "_"
return &Oneof{
- Desc: desc,
- Parent: message,
- GoName: camelCase(string(desc.Name())),
+ Desc: desc,
+ Parent: message,
+ GoName: camelCased,
+ GoIdent: GoIdent{
+ GoImportPath: f.GoImportPath,
+ GoName: parentPrefix + camelCased,
+ },
Location: loc,
Comments: f.comments[newPathKey(loc.Path)],
}
