reflect/protoregistry: add conflict override

The ignoreConflict function provides the ability to ignore certain conflicts.
By default, all conflicts are ignored with a log message produced instead.

Change-Id: I67fe56eef492e12421e5c8cb8d618dc2a46c82ed
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186658
Reviewed-by: Damien Neil <dneil@google.com>

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)
-}

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)
 			}
 		}
 	}

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)
+			err := errors.New("file %q has a package name conflict over %v", fd.Path(), name)
-			return amendErrorWithCaller(err, prev, fd)
+			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 = err
-					firstErr = amendErrorWithCaller(err, prev, typ)
 				}
 				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 = err
-						firstErr = amendErrorWithCaller(err, prev, typ)
 					}
 					continue typeLoop
 				}

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.