diff --git a/desc/protoparse/linker.go b/desc/protoparse/linker.go index 45893e25..ab729333 100644 --- a/desc/protoparse/linker.go +++ b/desc/protoparse/linker.go @@ -213,7 +213,10 @@ func addEnumToPool(r *parseResult, pool map[string]proto.Message, errs *errorHan return err } for _, evd := range ed.Value { - vfqn := fqn + "." + evd.GetName() + // protobuf name-scoping rules for enum values follow C++ scoping rules: + // the enum value name is a symbol in the *parent* scope (the one + // enclosing the enum). + vfqn := prefix + evd.GetName() if err := addToPool(r, pool, errs, vfqn, evd); err != nil { return err } @@ -238,7 +241,16 @@ func addServiceToPool(r *parseResult, pool map[string]proto.Message, errs *error func addToPool(r *parseResult, pool map[string]proto.Message, errs *errorHandler, fqn string, dsc proto.Message) error { if d, ok := pool[fqn]; ok { node := r.nodes[dsc] - if err := errs.handleErrorWithPos(node.Start(), "duplicate symbol %s: already defined as %s", fqn, descriptorType(d)); err != nil { + _, additionIsEnumVal := dsc.(*dpb.EnumValueDescriptorProto) + _, existingIsEnumVal := d.(*dpb.EnumValueDescriptorProto) + // because of weird scoping for enum values, provide more context in error message + // if this conflict is with an enum value + var suffix string + if additionIsEnumVal || existingIsEnumVal { + suffix = "; protobuf uses C++ scoping rules for enum values, so they exist in the scope enclosing the enum" + } + // TODO: also include the source location for the conflicting symbol + if err := errs.handleErrorWithPos(node.Start(), "duplicate symbol %s: already defined as %s%s", fqn, descriptorType(d), suffix); err != nil { return err } } diff --git a/desc/protoparse/linker_test.go b/desc/protoparse/linker_test.go index 0773cb88..b9ab9705 100644 --- a/desc/protoparse/linker_test.go +++ b/desc/protoparse/linker_test.go @@ -111,6 +111,12 @@ func TestLinkerValidation(t *testing.T) { }, `foo.proto:1:8: cycle found in imports: "foo.proto" -> "foo2.proto" -> "foo.proto"`, }, + { + map[string]string{ + "foo.proto": "enum foo { bar = 1; baz = 2; } enum fu { bar = 1; baz = 2; }", + }, + `foo.proto:1:42: duplicate symbol bar: already defined as enum value; protobuf uses C++ scoping rules for enum values, so they exist in the scope enclosing the enum`, + }, { map[string]string{ "foo.proto": "message foo {} enum foo { V = 0; }", diff --git a/desc/protoparse/reporting_test.go b/desc/protoparse/reporting_test.go index 4c8b95b1..f8a870c6 100644 --- a/desc/protoparse/reporting_test.go +++ b/desc/protoparse/reporting_test.go @@ -104,7 +104,7 @@ func TestErrorReporting(t *testing.T) { `, }, expectedErrs: []string{ - "test.proto:8:49: duplicate symbol Bar.BAZ: already defined as enum value", + "test.proto:8:49: duplicate symbol BAZ: already defined as enum value; protobuf uses C++ scoping rules for enum values, so they exist in the scope enclosing the enum", "test.proto:10:41: duplicate symbol Bar: already defined as enum", "test.proto:12:49: duplicate symbol Bar.Foo: already defined as method", "test.proto:12:58: method Bar.Foo: unknown request type Frob",