cmd/protoc-gen-go: avoid referencing remote enum values by name

The Go generator has historically always prefixed an enum value
with the name of the enum type, when it was unnecessary to do so.

For example:
	enum Status {
		STATUS_FAILED = 0;
		STATUS_PASSED = 1;
	}
would be generated as:
	type Status int32
	const (
		Status_STATUS_FAILED Status = 0
		Status_STATUS_PASSED Status = 1
	)

It is common for the enum values to be manually prefixed by the
enum type since protobuf enums use C++ namespace rules where
enum types and enum values are in the same namespace scope.
Thus, having the Go generator add a prefix is redundant.
See https://github.com/golang/protobuf/issues/513.

Some custom Go generators like protoc-gen-gogo allow removing
the prefix with the gogoproto.goproto_enum_prefix feature.
However, this leads to interoperability issues between
protoc-gen-go and protoc-gen-gogo, where the enum value names
cannot be accurately inferred.

Avoid this problem by just hard-coding the enum value number
for values declared in other packages. This provides benefits
in interoperability at the small cost of enum values possibly
being stale if their value were ever changed in a remote package.
However, this would only occur with use of proto2 enums and
default values, which seems to be an exceptionally rare situation.

Before:
	Default_MyMessage_MyField = remotepb.FooEnum_FOO_ENUM
After:
	Default_MyMessage_MyField = remotepb.FooEnum(4) // remotepb.FooEnum_FOO_ENUM

Before:
	func (x *MyMessage) GetField() remotepb.FooEnum {
		...
		return remotepb.FooEnum_FOO_ZERO
	}
After:
	func (x *MyMessage) GetField() remotepb.FooEnum {
		...
		return remotepb.FooEnum(0) // always 0 for proto3 and often 0 for proto2
	}

Change-Id: I3a06cd553f2eaf6124666f6c36c196d500d35718
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/319649
Trust: Joe Tsai <joetsai@digital-static.net>
Reviewed-by: Damien Neil <dneil@google.com>
This commit is contained in:
Joe Tsai 2021-05-12 21:40:36 -07:00
parent 50a85913fb
commit acaef6adb7
9 changed files with 1410 additions and 1313 deletions

View File

@ -447,7 +447,15 @@ func genMessageDefaultDecls(g *protogen.GeneratedFile, f *fileInfo, m *messageIn
case protoreflect.EnumKind:
idx := field.Desc.DefaultEnumValue().Index()
val := field.Enum.Values[idx]
consts = append(consts, fmt.Sprintf("%s = %s", name, g.QualifiedGoIdent(val.GoIdent)))
if val.GoIdent.GoImportPath == f.GoImportPath {
consts = append(consts, fmt.Sprintf("%s = %s", name, g.QualifiedGoIdent(val.GoIdent)))
} else {
// If the enum value is declared in a different Go package,
// reference it by number since the name may not be correct.
// See https://github.com/golang/protobuf/issues/513.
consts = append(consts, fmt.Sprintf("%s = %s(%d) // %s",
name, g.QualifiedGoIdent(field.Enum.GoIdent), val.Desc.Number(), g.QualifiedGoIdent(val.GoIdent)))
}
case protoreflect.FloatKind, protoreflect.DoubleKind:
if f := defVal.Float(); math.IsNaN(f) || math.IsInf(f, 0) {
var fn, arg string
@ -550,7 +558,7 @@ func genMessageGetterMethods(g *protogen.GeneratedFile, f *fileInfo, m *messageI
// Getter for message field.
goType, pointer := fieldGoType(g, f, field)
defaultValue := fieldDefaultValue(g, m, field)
defaultValue := fieldDefaultValue(g, f, m, field)
g.Annotate(m.GoIdent.GoName+".Get"+field.GoName, field.Location)
leadingComments := appendDeprecationSuffix("",
field.Desc.Options().(*descriptorpb.FieldOptions).GetDeprecated())
@ -672,7 +680,7 @@ func fieldProtobufTagValue(field *protogen.Field) string {
return tag.Marshal(field.Desc, enumName)
}
func fieldDefaultValue(g *protogen.GeneratedFile, m *messageInfo, field *protogen.Field) string {
func fieldDefaultValue(g *protogen.GeneratedFile, f *fileInfo, m *messageInfo, field *protogen.Field) string {
if field.Desc.IsList() {
return "nil"
}
@ -691,7 +699,15 @@ func fieldDefaultValue(g *protogen.GeneratedFile, m *messageInfo, field *protoge
case protoreflect.MessageKind, protoreflect.GroupKind, protoreflect.BytesKind:
return "nil"
case protoreflect.EnumKind:
return g.QualifiedGoIdent(field.Enum.Values[0].GoIdent)
val := field.Enum.Values[0]
if val.GoIdent.GoImportPath == f.GoImportPath {
return g.QualifiedGoIdent(val.GoIdent)
} else {
// If the enum value is declared in a different Go package,
// reference it by number since the name may not be correct.
// See https://github.com/golang/protobuf/issues/513.
return g.QualifiedGoIdent(field.Enum.GoIdent) + "(" + strconv.FormatInt(int64(val.Desc.Number()), 32) + ")"
}
default:
return "0"
}

View File

@ -106,7 +106,7 @@ func (x *Public) GetE() sub.E {
if x != nil && x.E != nil {
return *x.E
}
return sub.E_ZERO
return sub.E(0)
}
func (x *Public) GetLocal() *Local {

View File

@ -67,7 +67,7 @@ func (x *Local) GetE() sub.E {
if x != nil && x.E != nil {
return *x.E
}
return sub.E_ZERO
return sub.E(0)
}
var File_cmd_protoc_gen_go_testdata_import_public_b_proto protoreflect.FileDescriptor

View File

@ -1130,7 +1130,7 @@ func (x *TestAllTypesProto3) GetOneofNullValue() structpb.NullValue {
if x, ok := x.GetOneofField().(*TestAllTypesProto3_OneofNullValue); ok {
return x.OneofNullValue
}
return structpb.NullValue_NULL_VALUE
return structpb.NullValue(0)
}
func (x *TestAllTypesProto3) GetOptionalBoolWrapper() *wrapperspb.BoolValue {
@ -1305,7 +1305,7 @@ func (x *TestAllTypesProto3) GetOptionalNullValue() structpb.NullValue {
if x != nil {
return x.OptionalNullValue
}
return structpb.NullValue_NULL_VALUE
return structpb.NullValue(0)
}
func (x *TestAllTypesProto3) GetRepeatedDuration() []*durationpb.Duration {

View File

@ -251,7 +251,7 @@ func (x *TestFieldTrack) GetOptionalEnum() test.TestAllTypes_NestedEnum {
if x != nil && x.OptionalEnum != nil {
return *x.OptionalEnum
}
return test.TestAllTypes_FOO
return test.TestAllTypes_NestedEnum(0)
}
//go:nointerface

File diff suppressed because it is too large Load Diff

View File

@ -6,6 +6,7 @@ syntax = "proto2";
package goproto.proto.test;
import "google/protobuf/descriptor.proto";
import "internal/testprotos/test/test_import.proto";
import public "internal/testprotos/test/test_public.proto";
import weak "internal/testprotos/test/weak1/test_weak.proto";
@ -381,3 +382,7 @@ service TestDeprecatedService {
message WeirdDefault {
optional bytes weird_default = 1 [default = "hello, \"world!\"\ndead\xde\xad\xbe\xefbeef`"];
}
message RemoteDefault {
optional google.protobuf.FieldDescriptorProto.Type type = 1 [default = TYPE_ENUM];
}

View File

@ -1384,7 +1384,7 @@ func (x *KnownTypes) GetOptNull() structpb.NullValue {
if x != nil && x.OptNull != nil {
return *x.OptNull
}
return structpb.NullValue_NULL_VALUE
return structpb.NullValue(0)
}
func (x *KnownTypes) GetOptEmpty() *emptypb.Empty {

View File

@ -172,7 +172,7 @@ func (x *Api) GetSyntax() typepb.Syntax {
if x != nil {
return x.Syntax
}
return typepb.Syntax_SYNTAX_PROTO2
return typepb.Syntax(0)
}
// Method represents a method of an API interface.
@ -275,7 +275,7 @@ func (x *Method) GetSyntax() typepb.Syntax {
if x != nil {
return x.Syntax
}
return typepb.Syntax_SYNTAX_PROTO2
return typepb.Syntax(0)
}
// Declares an API Interface to be included in this interface. The including