reflect/protodesc: add FileOptions
The FileOptions type provides the ability to specify specialized options
for how a file descriptor is constructed. It follows the same optional
arguments pattern as used in the proto package.
The resolver is not an option since it almost always necessary
when constructing a file descriptor.
Change-Id: Ib98ac6289881ad8402dd615f6c895da5899cb8d9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218940
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/compiler/protogen/protogen_test.go b/compiler/protogen/protogen_test.go
index 51dbd25..c3982c5 100644
--- a/compiler/protogen/protogen_test.go
+++ b/compiler/protogen/protogen_test.go
@@ -66,7 +66,7 @@
Name: proto.String("testdata/go_package/no_go_package_import.proto"),
Syntax: proto.String(protoreflect.Proto3.String()),
Package: proto.String("goproto.testdata"),
- Dependency: []string{"go_package/no_go_package.proto"},
+ Dependency: []string{"testdata/go_package/no_go_package.proto"},
},
},
}, nil)
diff --git a/reflect/protodesc/desc.go b/reflect/protodesc/desc.go
index f40a017..de9e82a 100644
--- a/reflect/protodesc/desc.go
+++ b/reflect/protodesc/desc.go
@@ -28,37 +28,40 @@
FindDescriptorByName(protoreflect.FullName) (protoreflect.Descriptor, error)
}
-// option is an optional argument that may be provided to NewFile.
-type option struct {
- pragma.DoNotCompare
- allowUnresolvable bool
-}
+// FileOptions configures the construction of file descriptors.
+type FileOptions struct {
+ pragma.NoUnkeyedLiterals
-// allowUnresolvable configures NewFile to permissively allow unresolvable
-// file, enum, or message dependencies. Unresolved dependencies are replaced by
-// placeholder equivalents.
-//
-// The following dependencies may be left unresolved:
-// • Resolving an imported file.
-// • Resolving the type for a message field or extension field.
-// If the kind of the field is unknown, then a placeholder is used for both
-// protoreflect.FieldDescriptor.Enum and protoreflect.FieldDescriptor.Message.
-// • Resolving the enum value set as the default for an optional enum field.
-// If unresolvable, the protoreflect.FieldDescriptor.Default is set to the
-// first enum value in the associated enum (or zero if the also enum dependency
-// is also unresolvable). The protoreflect.FieldDescriptor.DefaultEnumValue
-// is set as a placeholder.
-// • Resolving the extended message type for an extension field.
-// • Resolving the input or output message type for a service method.
-//
-// If the unresolved dependency uses a relative name, then the placeholder will
-// contain an invalid FullName with a "*." prefix, indicating that the starting
-// prefix of the full name is unknown.
-func allowUnresolvable() option {
- return option{allowUnresolvable: true}
+ // AllowUnresolvable configures New to permissively allow unresolvable
+ // file, enum, or message dependencies. Unresolved dependencies are replaced
+ // by placeholder equivalents.
+ //
+ // The following dependencies may be left unresolved:
+ // • Resolving an imported file.
+ // • Resolving the type for a message field or extension field.
+ // If the kind of the field is unknown, then a placeholder is used for both
+ // the Enum and Message accessors on the protoreflect.FieldDescriptor.
+ // • Resolving an enum value set as the default for an optional enum field.
+ // If unresolvable, the protoreflect.FieldDescriptor.Default is set to the
+ // first value in the associated enum (or zero if the also enum dependency
+ // is also unresolvable). The protoreflect.FieldDescriptor.DefaultEnumValue
+ // is populated with a placeholder.
+ // • Resolving the extended message type for an extension field.
+ // • Resolving the input or output message type for a service method.
+ //
+ // If the unresolved dependency uses a relative name,
+ // then the placeholder will contain an invalid FullName with a "*." prefix,
+ // indicating that the starting prefix of the full name is unknown.
+ AllowUnresolvable bool
}
// NewFile creates a new protoreflect.FileDescriptor from the provided
+// file descriptor message. See FileOptions.New for more information.
+func NewFile(fd *descriptorpb.FileDescriptorProto, r Resolver) (protoreflect.FileDescriptor, error) {
+ return FileOptions{}.New(fd, r)
+}
+
+// New creates a new protoreflect.FileDescriptor from the provided
// file descriptor message. The file must represent a valid proto file according
// to protobuf semantics. The returned descriptor is a deep copy of the input.
//
@@ -66,16 +69,7 @@
// resolved using the provided registry. When looking up an import file path,
// the path must be unique. The newly created file descriptor is not registered
// back into the provided file registry.
-func NewFile(fd *descriptorpb.FileDescriptorProto, r Resolver) (protoreflect.FileDescriptor, error) {
- // TODO(blocks): remove setting allowUnresolvable once naughty users are migrated.
- return newFile(fd, r, allowUnresolvable())
-}
-func newFile(fd *descriptorpb.FileDescriptorProto, r Resolver, opts ...option) (protoreflect.FileDescriptor, error) {
- // Process the options.
- var allowUnresolvable bool
- for _, o := range opts {
- allowUnresolvable = allowUnresolvable || o.allowUnresolvable
- }
+func (o FileOptions) New(fd *descriptorpb.FileDescriptorProto, r Resolver) (protoreflect.FileDescriptor, error) {
if r == nil {
r = (*protoregistry.Files)(nil) // empty resolver
}
@@ -120,7 +114,7 @@
for i, path := range fd.GetDependency() {
imp := &f.L2.Imports[i]
f, err := r.FindFileByPath(path)
- if err == protoregistry.NotFound && (allowUnresolvable || imp.IsWeak) {
+ if err == protoregistry.NotFound && (o.AllowUnresolvable || imp.IsWeak) {
f = filedesc.PlaceholderFile(path)
} else if err != nil {
return nil, errors.New("could not resolve import %q: %v", path, err)
@@ -188,7 +182,7 @@
}
// Step 2: Resolve every dependency reference not handled by step 1.
- r2 := &resolver{local: r1, remote: r, imports: imps, allowUnresolvable: allowUnresolvable}
+ r2 := &resolver{local: r1, remote: r, imports: imps, allowUnresolvable: o.AllowUnresolvable}
if err := r2.resolveMessageDependencies(f.L1.Messages.List, fd.GetMessageType()); err != nil {
return nil, err
}
diff --git a/reflect/protodesc/file_test.go b/reflect/protodesc/file_test.go
index 934b32f..744c02f 100644
--- a/reflect/protodesc/file_test.go
+++ b/reflect/protodesc/file_test.go
@@ -88,7 +88,7 @@
label string
inDeps []*descriptorpb.FileDescriptorProto
inDesc *descriptorpb.FileDescriptorProto
- inOpts []option
+ inOpts FileOptions
wantDesc *descriptorpb.FileDescriptorProto
wantErr string
}{{
@@ -121,7 +121,7 @@
package: ""
dependency: "dep.proto"
`),
- inOpts: []option{allowUnresolvable()},
+ inOpts: FileOptions{AllowUnresolvable: true},
}, {
label: "duplicate import",
inDesc: mustParseFile(`
@@ -129,7 +129,7 @@
package: ""
dependency: ["dep.proto", "dep.proto"]
`),
- inOpts: []option{allowUnresolvable()},
+ inOpts: FileOptions{AllowUnresolvable: true},
wantErr: `already imported "dep.proto"`,
}, {
label: "invalid weak import",
@@ -139,7 +139,7 @@
dependency: "dep.proto"
weak_dependency: [-23]
`),
- inOpts: []option{allowUnresolvable()},
+ inOpts: FileOptions{AllowUnresolvable: true},
wantErr: `invalid or duplicate weak import index: -23`,
}, {
label: "normal weak and public import",
@@ -150,7 +150,7 @@
weak_dependency: [0]
public_dependency: [0]
`),
- inOpts: []option{allowUnresolvable()},
+ inOpts: FileOptions{AllowUnresolvable: true},
}, {
label: "import public indirect dependency duplicate",
inDeps: []*descriptorpb.FileDescriptorProto{
@@ -306,7 +306,7 @@
enum_type: [{name:"E" value:[{name:"V0" number:0}]}]
}]
`),
- inOpts: []option{allowUnresolvable()},
+ inOpts: FileOptions{AllowUnresolvable: true},
}, {
label: "unresolved extendee",
inDesc: mustParseFile(`
@@ -342,7 +342,7 @@
method: [{name:"M" input_type:"foo.bar.input" output_type:".absolute.foo.bar.output"}]
}]
`),
- inOpts: []option{allowUnresolvable()},
+ inOpts: FileOptions{AllowUnresolvable: true},
}, {
label: "resolved but not imported",
inDeps: []*descriptorpb.FileDescriptorProto{mustParseFile(`
@@ -856,7 +856,7 @@
]
}]
`),
- inOpts: []option{allowUnresolvable()},
+ inOpts: FileOptions{AllowUnresolvable: true},
// TODO: Test field and oneof handling in validateMessageDeclarations
// TODO: Test unmarshalDefault
// TODO: Test validateExtensionDeclarations
@@ -883,7 +883,7 @@
}]
}]
`),
- inOpts: []option{allowUnresolvable()},
+ inOpts: FileOptions{AllowUnresolvable: true},
}, {
label: "service with wrong reference type",
inDeps: []*descriptorpb.FileDescriptorProto{
@@ -910,7 +910,7 @@
t.Run(tt.label, func(t *testing.T) {
r := new(protoregistry.Files)
for i, dep := range tt.inDeps {
- f, err := newFile(dep, r)
+ f, err := tt.inOpts.New(dep, r)
if err != nil {
t.Fatalf("dependency %d: unexpected NewFile() error: %v", i, err)
}
@@ -922,7 +922,7 @@
if tt.wantErr == "" && tt.wantDesc == nil {
tt.wantDesc = cloneFile(tt.inDesc)
}
- gotFile, err := newFile(tt.inDesc, r, tt.inOpts...)
+ gotFile, err := tt.inOpts.New(tt.inDesc, r)
if gotFile != nil {
gotDesc = ToFileDescriptorProto(gotFile)
}