diff --git a/internal/filedesc/build.go b/internal/filedesc/build.go index 52cbdf14..427e26e6 100644 --- a/internal/filedesc/build.go +++ b/internal/filedesc/build.go @@ -6,8 +6,6 @@ package filedesc import ( - "log" - "google.golang.org/protobuf/internal/encoding/wire" "google.golang.org/protobuf/internal/fieldnum" "google.golang.org/protobuf/reflect/protoreflect" @@ -107,7 +105,7 @@ func (db DescBuilder) Build() (out struct { out.Services = fd.allServices if err := db.FileRegistry.Register(fd); err != nil { - CheckRegistryError(err) + panic(err) } return out } @@ -152,13 +150,3 @@ func (db *DescBuilder) unmarshalCounts(b []byte, isFile bool) { } } } - -// CheckRegistryError handles registration errors. -// It is a variable so that its behavior can be replaced in another source file. -var CheckRegistryError = func(err error) { - log.Printf(""+ - "WARNING: %v\n"+ - "A future release will panic on registration conflicts.\n"+ - // TODO: Add a URL pointing to documentation on how to resolve conflicts. - "\n", err) -} diff --git a/internal/filetype/build.go b/internal/filetype/build.go index 34280404..fa1bb9ac 100644 --- a/internal/filetype/build.go +++ b/internal/filetype/build.go @@ -155,7 +155,7 @@ func (tb TypeBuilder) Build() (out struct { // Register enum types. if err := tb.TypeRegistry.Register(&out.Enums[i]); err != nil { - fdesc.CheckRegistryError(err) + panic(err) } } } @@ -183,7 +183,7 @@ func (tb TypeBuilder) Build() (out struct { // Register message types. if err := tb.TypeRegistry.Register(&out.Messages[i]); err != nil { - fdesc.CheckRegistryError(err) + panic(err) } } @@ -251,7 +251,7 @@ func (tb TypeBuilder) Build() (out struct { // Register extension types. if err := tb.TypeRegistry.Register(&out.Extensions[i]); err != nil { - fdesc.CheckRegistryError(err) + panic(err) } } } diff --git a/reflect/protoregistry/registry.go b/reflect/protoregistry/registry.go index 33552f18..4abfc100 100644 --- a/reflect/protoregistry/registry.go +++ b/reflect/protoregistry/registry.go @@ -17,6 +17,7 @@ package protoregistry import ( "fmt" + "log" "reflect" "strings" @@ -24,6 +25,18 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" ) +// ignoreConflict reports whether to ignore a registration conflict +// given the descriptor being registered and the error. +// It is a variable so that the behavior is easily overridden in another file. +var ignoreConflict = func(d protoreflect.Descriptor, err error) bool { + log.Printf(""+ + "WARNING: %v\n"+ + "A future release will panic on registration conflicts.\n"+ + // TODO: Add a URL pointing to documentation on how to resolve conflicts. + "\n", err) + return true +} + // GlobalFiles is a global registry of file descriptors. var GlobalFiles *Files = new(Files) @@ -92,25 +105,38 @@ func (r *Files) registerFile(fd protoreflect.FileDescriptor) error { path := fd.Path() if prev := r.filesByPath[path]; prev != nil { err := errors.New("file %q is already registered", fd.Path()) - return amendErrorWithCaller(err, prev, fd) + err = amendErrorWithCaller(err, prev, fd) + if r == GlobalFiles && ignoreConflict(fd, err) { + err = nil + } + return err } for name := fd.Package(); name != ""; name = name.Parent() { switch prev := r.descsByName[name]; prev.(type) { case nil, *packageDescriptor: default: - err := errors.New("file %q has a name conflict over %v", fd.Path(), name) - return amendErrorWithCaller(err, prev, fd) + err := errors.New("file %q has a package name conflict over %v", fd.Path(), name) + err = amendErrorWithCaller(err, prev, fd) + if r == GlobalFiles && ignoreConflict(fd, err) { + err = nil + } + return err } } var err error + var hasConflict bool rangeTopLevelDescriptors(fd, func(d protoreflect.Descriptor) { if prev := r.descsByName[d.FullName()]; prev != nil { + hasConflict = true err = errors.New("file %q has a name conflict over %v", fd.Path(), d.FullName()) err = amendErrorWithCaller(err, prev, fd) + if r == GlobalFiles && ignoreConflict(d, err) { + err = nil + } } }) - if err != nil { + if hasConflict { return err } @@ -291,6 +317,7 @@ func rangeTopLevelDescriptors(fd protoreflect.FileDescriptor, f func(protoreflec // Type is an interface satisfied by protoreflect.EnumType, // protoreflect.MessageType, or protoreflect.ExtensionType. type Type interface { + protoreflect.Descriptor GoType() reflect.Type } @@ -413,9 +440,13 @@ typeLoop: panic(fmt.Sprintf("invalid type: %T", t)) } if prev := r.typesByName[name]; prev != nil { + err := errors.New("%v %v is already registered", typeName(typ), name) + err = amendErrorWithCaller(err, prev, typ) + if r == GlobalTypes && ignoreConflict(typ, err) { + err = nil + } if firstErr == nil { - err := errors.New("%v %v is already registered", typeName(typ), name) - firstErr = amendErrorWithCaller(err, prev, typ) + firstErr = err } continue typeLoop } @@ -425,9 +456,13 @@ typeLoop: field := xt.Number() message := xt.ContainingMessage().FullName() if prev := r.extensionsByMessage[message][field]; prev != nil { + err := errors.New("extension number %d is already registered on message %v", field, message) + err = amendErrorWithCaller(err, prev, typ) + if r == GlobalTypes && ignoreConflict(typ, err) { + err = nil + } if firstErr == nil { - err := errors.New("extension number %d is already registered on message %v", field, message) - firstErr = amendErrorWithCaller(err, prev, typ) + firstErr = err } continue typeLoop } diff --git a/reflect/protoregistry/registry_test.go b/reflect/protoregistry/registry_test.go index 3d876515..63dd1baa 100644 --- a/reflect/protoregistry/registry_test.go +++ b/reflect/protoregistry/registry_test.go @@ -129,7 +129,7 @@ func TestFiles(t *testing.T) { inFile: mustMakeFile(`syntax:"proto2" name:"test2a.proto" enum_type:[{name:"foo" value:[{name:"VALUE" number:0}]}]`), }, { inFile: mustMakeFile(`syntax:"proto2" name:"test2b.proto" package:"foo.bar.baz"`), - wantErr: `file "test2b.proto" has a name conflict over foo`, + wantErr: `file "test2b.proto" has a package name conflict over foo`, }}, }, { // Test when new enum conflicts with existing enum in same package.