reflect/prototype: add NewFileFromDescriptorProto constructor

Implement NewFileFromDescriptorProto, which constructs a
protoreflect.FileDescriptor from a provided descriptor.FileDescriptorProto.

Some other minor changes:
* Allow calling Find and Range methods on nil *protoregistry.Files to match
the behavior of maps where index operations are permitted, but store panics.
* Switch protoregistry test to be protoregistry_test to avoid cyclic dependency.

Change-Id: I5536901ef5096014a3421e63bc4a9dfcad335ea4
Reviewed-on: https://go-review.googlesource.com/132455
Reviewed-by: Damien Neil <dneil@google.com>

reflect/protoregistry/registry.go

@@ -188,6 +188,9 @@ fileLoop:
 //
 // This return (nil, NotFound) if not found.
 func (r *Files) FindDescriptorByName(name protoreflect.FullName) (protoreflect.Descriptor, error) {
+	if r == nil {
+		return nil, NotFound
+	}
 	pkg := name
 	root := &r.filesByPackage
 	for len(pkg) > 0 {
@@ -222,6 +225,9 @@ func (r *Files) RangeFiles(f func(protoreflect.FileDescriptor) bool) {
 // match before iterating over files with general prefix match.
 // The iteration order is undefined within exact matches or prefix matches.
 func (r *Files) RangeFilesByPackage(pkg protoreflect.FullName, f func(protoreflect.FileDescriptor) bool) {
+	if r == nil {
+		return
+	}
 	if strings.HasSuffix(string(pkg), ".") {
 		return // avoid edge case where splitPrefix allows trailing dot
 	}
@@ -255,6 +261,9 @@ func rangeFiles(fs *filesByPackage, f func(protoreflect.FileDescriptor) bool) bo
 // RangeFilesByPath iterates over all registered files filtered by
 // the given proto path. The iteration order is undefined.
 func (r *Files) RangeFilesByPath(path string, f func(protoreflect.FileDescriptor) bool) {
+	if r == nil {
+		return
+	}
 	for _, fd := range r.filesByPath[path] { // TODO: iterate non-deterministically
 		if !f(fd) {
 			return

reflect/protoregistry/registry_test.go

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package protoregistry
+package protoregistry_test
 
 import (
 	"fmt"
@@ -13,6 +13,7 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 
 	pref "google.golang.org/proto/reflect/protoreflect"
+	preg "google.golang.org/proto/reflect/protoregistry"
 	ptype "google.golang.org/proto/reflect/prototype"
 )
 
@@ -252,7 +253,7 @@ func TestFiles(t *testing.T) {
 	})
 	for _, tt := range tests {
 		t.Run("", func(t *testing.T) {
-			var files Files
+			var files preg.Files
 			for i, tc := range tt.files {
 				fd, err := ptype.NewFile(tc.inFile)
 				if err != nil {

reflect/prototype/protofile.go

@@ -24,8 +24,6 @@ import (
 // and initialized. This architectural approach keeps the literal representation
 // smaller, which then keeps the generated code size smaller.
 
-// TODO: Support initializing File from a google.protobuf.FileDescriptor?
-
 // TODO: Instead of a top-down construction approach where internal references
 // to message types use placeholder types, we could add a Reference method
 // on Message and Enum that creates a MessageDescriptor or EnumDescriptor

reflect/prototype/protofile_desc.go

@@ -0,0 +1,390 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package prototype
+
+import (
+	"fmt"
+	"math"
+	"strconv"
+	"strings"
+
+	descriptorV1 "github.com/golang/protobuf/protoc-gen-go/descriptor"
+
+	"google.golang.org/proto/internal/encoding/text"
+	"google.golang.org/proto/internal/errors"
+	"google.golang.org/proto/reflect/protoreflect"
+	"google.golang.org/proto/reflect/protoregistry"
+)
+
+// TODO: Should we be responsible for validating other parts of the descriptor
+// that we don't directly use?
+//
+// For example:
+//	* That field numbers don't overlap with reserved numbers.
+//	* That field names don't overlap with reserved names.
+//	* That enum numbers don't overlap with reserved numbers.
+//	* That enum names don't overlap with reserved names.
+//	* That "extendee" is not set for a message field.
+//	* That "oneof_index" is not set for an extension field.
+//	* That "json_name" is not set for an extension field. Maybe, maybe not.
+//	* That "type_name" is not set on a field for non-enums and non-messages.
+//	* That "weak" is not set for an extension field (double check this).
+
+// TODO: Store the input file descriptor to implement:
+//	* protoreflect.Descriptor.DescriptorProto
+//	* protoreflect.Descriptor.DescriptorOptions
+
+// TODO: Should we return a File instead of protoreflect.FileDescriptor?
+// This would allow users to mutate the File before converting it.
+// However, this will complicate future work for validation since File may now
+// diverge from the stored descriptor proto (see above TODO).
+
+// NewFileFromDescriptorProto creates a new protoreflect.FileDescriptor from
+// the provided descriptor message. The file must represent a valid proto file
+// according to protobuf semantics.
+//
+// Any import files, enum types, or message types referenced in the file are
+// 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.
+//
+// The caller must relinquish full ownership of the input fd and must not
+// access or mutate any fields.
+func NewFileFromDescriptorProto(fd *descriptorV1.FileDescriptorProto, r *protoregistry.Files) (protoreflect.FileDescriptor, error) {
+	var f File
+	switch fd.GetSyntax() {
+	case "proto2":
+		f.Syntax = protoreflect.Proto2
+	case "proto3":
+		f.Syntax = protoreflect.Proto3
+	default:
+		return nil, errors.New("invalid syntax: %v", fd.GetSyntax())
+	}
+	f.Path = fd.GetName()
+	f.Package = protoreflect.FullName(fd.GetPackage())
+
+	f.Imports = make([]protoreflect.FileImport, len(fd.GetDependency()))
+	for _, i := range fd.GetPublicDependency() {
+		if int(i) >= len(f.Imports) || f.Imports[i].IsPublic {
+			return nil, errors.New("invalid or duplicate public import index: %d", i)
+		}
+		f.Imports[i].IsPublic = true
+	}
+	for _, i := range fd.GetWeakDependency() {
+		if int(i) >= len(f.Imports) || f.Imports[i].IsWeak {
+			return nil, errors.New("invalid or duplicate weak import index: %d", i)
+		}
+		f.Imports[i].IsWeak = true
+	}
+	for i, path := range fd.GetDependency() {
+		var n int
+		imp := &f.Imports[i]
+		r.RangeFilesByPath(path, func(fd protoreflect.FileDescriptor) bool {
+			imp.FileDescriptor = fd
+			n++
+			return true
+		})
+		if n > 1 {
+			return nil, errors.New("duplicate files for import %q", path)
+		}
+		if imp.IsWeak || imp.FileDescriptor == nil {
+			imp.FileDescriptor = PlaceholderFile(path, "")
+		}
+	}
+
+	var err error
+	f.Messages, err = messagesFromDescriptorProto(fd.GetMessageType(), r)
+	if err != nil {
+		return nil, err
+	}
+	f.Enums, err = enumsFromDescriptorProto(fd.GetEnumType(), r)
+	if err != nil {
+		return nil, err
+	}
+	f.Extensions, err = extensionsFromDescriptorProto(fd.GetExtension(), r)
+	if err != nil {
+		return nil, err
+	}
+	f.Services, err = servicesFromDescriptorProto(fd.GetService(), r)
+	if err != nil {
+		return nil, err
+	}
+
+	return NewFile(&f)
+}
+
+func messagesFromDescriptorProto(mds []*descriptorV1.DescriptorProto, r *protoregistry.Files) (ms []Message, err error) {
+	for _, md := range mds {
+		var m Message
+		m.Name = protoreflect.Name(md.GetName())
+		m.IsMapEntry = md.GetOptions().GetMapEntry()
+		for _, fd := range md.GetField() {
+			var f Field
+			f.Name = protoreflect.Name(fd.GetName())
+			f.Number = protoreflect.FieldNumber(fd.GetNumber())
+			f.Cardinality = protoreflect.Cardinality(fd.GetLabel())
+			f.Kind = protoreflect.Kind(fd.GetType())
+			f.JSONName = fd.GetJsonName()
+			f.IsPacked = fd.GetOptions().GetPacked()
+			f.IsWeak = fd.GetOptions().GetWeak()
+			if fd.DefaultValue != nil {
+				f.Default, err = parseDefault(fd.GetDefaultValue(), f.Kind)
+				if err != nil {
+					return nil, err
+				}
+			}
+			if fd.OneofIndex != nil {
+				i := int(fd.GetOneofIndex())
+				if i >= len(md.GetOneofDecl()) {
+					return nil, errors.New("invalid oneof index: %d", i)
+				}
+				f.OneofName = protoreflect.Name(md.GetOneofDecl()[i].GetName())
+			}
+			switch f.Kind {
+			case protoreflect.EnumKind:
+				f.EnumType, err = findEnumDescriptor(fd.GetTypeName(), r)
+				if err != nil {
+					return nil, err
+				}
+				if f.IsWeak && !f.EnumType.IsPlaceholder() {
+					f.EnumType = PlaceholderEnum(f.EnumType.FullName())
+				}
+			case protoreflect.MessageKind, protoreflect.GroupKind:
+				f.MessageType, err = findMessageDescriptor(fd.GetTypeName(), r)
+				if err != nil {
+					return nil, err
+				}
+				if f.IsWeak && !f.MessageType.IsPlaceholder() {
+					f.MessageType = PlaceholderMessage(f.MessageType.FullName())
+				}
+			}
+			m.Fields = append(m.Fields, f)
+		}
+		for _, od := range md.GetOneofDecl() {
+			m.Oneofs = append(m.Oneofs, Oneof{Name: protoreflect.Name(od.GetName())})
+		}
+		for _, xr := range md.GetExtensionRange() {
+			m.ExtensionRanges = append(m.ExtensionRanges, [2]protoreflect.FieldNumber{
+				protoreflect.FieldNumber(xr.GetStart()),
+				protoreflect.FieldNumber(xr.GetEnd()),
+			})
+		}
+
+		m.Messages, err = messagesFromDescriptorProto(md.GetNestedType(), r)
+		if err != nil {
+			return nil, err
+		}
+		m.Enums, err = enumsFromDescriptorProto(md.GetEnumType(), r)
+		if err != nil {
+			return nil, err
+		}
+		m.Extensions, err = extensionsFromDescriptorProto(md.GetExtension(), r)
+		if err != nil {
+			return nil, err
+		}
+
+		ms = append(ms, m)
+	}
+	return ms, nil
+}
+
+func enumsFromDescriptorProto(eds []*descriptorV1.EnumDescriptorProto, r *protoregistry.Files) (es []Enum, err error) {
+	for _, ed := range eds {
+		var e Enum
+		e.Name = protoreflect.Name(ed.GetName())
+		for _, vd := range ed.GetValue() {
+			e.Values = append(e.Values, EnumValue{
+				Name:   protoreflect.Name(vd.GetName()),
+				Number: protoreflect.EnumNumber(vd.GetNumber()),
+			})
+		}
+		es = append(es, e)
+	}
+	return es, nil
+}
+
+func extensionsFromDescriptorProto(xds []*descriptorV1.FieldDescriptorProto, r *protoregistry.Files) (xs []Extension, err error) {
+	for _, xd := range xds {
+		var x Extension
+		x.Name = protoreflect.Name(xd.GetName())
+		x.Number = protoreflect.FieldNumber(xd.GetNumber())
+		x.Cardinality = protoreflect.Cardinality(xd.GetLabel())
+		x.Kind = protoreflect.Kind(xd.GetType())
+		x.IsPacked = xd.GetOptions().GetPacked()
+		if xd.DefaultValue != nil {
+			x.Default, err = parseDefault(xd.GetDefaultValue(), x.Kind)
+			if err != nil {
+				return nil, err
+			}
+		}
+		switch x.Kind {
+		case protoreflect.EnumKind:
+			x.EnumType, err = findEnumDescriptor(xd.GetTypeName(), r)
+			if err != nil {
+				return nil, err
+			}
+		case protoreflect.MessageKind, protoreflect.GroupKind:
+			x.MessageType, err = findMessageDescriptor(xd.GetTypeName(), r)
+			if err != nil {
+				return nil, err
+			}
+		}
+		x.ExtendedType, err = findMessageDescriptor(xd.GetExtendee(), r)
+		if err != nil {
+			return nil, err
+		}
+		xs = append(xs, x)
+	}
+	return xs, nil
+}
+
+func servicesFromDescriptorProto(sds []*descriptorV1.ServiceDescriptorProto, r *protoregistry.Files) (ss []Service, err error) {
+	for _, sd := range sds {
+		var s Service
+		s.Name = protoreflect.Name(sd.GetName())
+		for _, md := range sd.GetMethod() {
+			var m Method
+			m.Name = protoreflect.Name(md.GetName())
+			m.InputType, err = findMessageDescriptor(md.GetInputType(), r)
+			if err != nil {
+				return nil, err
+			}
+			m.OutputType, err = findMessageDescriptor(md.GetOutputType(), r)
+			if err != nil {
+				return nil, err
+			}
+			m.IsStreamingClient = md.GetClientStreaming()
+			m.IsStreamingServer = md.GetServerStreaming()
+			s.Methods = append(s.Methods, m)
+		}
+		ss = append(ss, s)
+	}
+	return ss, nil
+}
+
+// TODO: Should we allow relative names? The protoc compiler has emitted
+// absolute names for some time now. Requiring absolute names as an input
+// simplifies our implementation as we won't need to implement C++'s namespace
+// scoping rules.
+
+func findMessageDescriptor(s string, r *protoregistry.Files) (protoreflect.MessageDescriptor, error) {
+	if !strings.HasPrefix(s, ".") {
+		return nil, errors.New("identifier name must be fully qualified with a leading dot: %v", s)
+	}
+	name := protoreflect.FullName(strings.TrimPrefix(s, "."))
+	switch m, err := r.FindDescriptorByName(name); {
+	case err == nil:
+		m, ok := m.(protoreflect.MessageDescriptor)
+		if !ok {
+			return nil, errors.New("resolved wrong type: got %v, want message", typeName(m))
+		}
+		return m, nil
+	case err == protoregistry.NotFound:
+		return PlaceholderMessage(name), nil
+	default:
+		return nil, err
+	}
+}
+
+func findEnumDescriptor(s string, r *protoregistry.Files) (protoreflect.EnumDescriptor, error) {
+	if !strings.HasPrefix(s, ".") {
+		return nil, errors.New("identifier name must be fully qualified with a leading dot: %v", s)
+	}
+	name := protoreflect.FullName(strings.TrimPrefix(s, "."))
+	switch e, err := r.FindDescriptorByName(name); {
+	case err == nil:
+		e, ok := e.(protoreflect.EnumDescriptor)
+		if !ok {
+			return nil, errors.New("resolved wrong type: got %T, want enum", typeName(e))
+		}
+		return e, nil
+	case err == protoregistry.NotFound:
+		return PlaceholderEnum(name), nil
+	default:
+		return nil, err
+	}
+}
+
+func typeName(t protoreflect.Descriptor) string {
+	switch t.(type) {
+	case protoreflect.EnumType:
+		return "enum"
+	case protoreflect.MessageType:
+		return "message"
+	default:
+		return fmt.Sprintf("%T", t)
+	}
+}
+
+func parseDefault(s string, k protoreflect.Kind) (protoreflect.Value, error) {
+	switch k {
+	case protoreflect.BoolKind:
+		switch s {
+		case "true":
+			return protoreflect.ValueOf(true), nil
+		case "false":
+			return protoreflect.ValueOf(false), nil
+		}
+	case protoreflect.EnumKind:
+		// For enums, we are supposed to return a protoreflect.EnumNumber type.
+		// However, default values record the name instead of the number.
+		// We are unable to resolve the name into a number without additional
+		// type information. Thus, we temporarily return the name identifier
+		// for now and rely on logic in defaultValue.lazyInit to resolve it.
+		return protoreflect.ValueOf(s), nil
+	case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind:
+		v, err := strconv.ParseInt(s, 0, 32)
+		if err == nil {
+			return protoreflect.ValueOf(int32(v)), nil
+		}
+	case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind:
+		v, err := strconv.ParseInt(s, 0, 64)
+		if err == nil {
+			return protoreflect.ValueOf(int64(v)), nil
+		}
+	case protoreflect.Uint32Kind, protoreflect.Fixed32Kind:
+		v, err := strconv.ParseUint(s, 0, 32)
+		if err == nil {
+			return protoreflect.ValueOf(uint64(v)), nil
+		}
+	case protoreflect.Uint64Kind, protoreflect.Fixed64Kind:
+		v, err := strconv.ParseUint(s, 0, 64)
+		if err == nil {
+			return protoreflect.ValueOf(uint64(v)), nil
+		}
+	case protoreflect.FloatKind, protoreflect.DoubleKind:
+		var v float64
+		var err error
+		switch s {
+		case "nan":
+			v = math.NaN()
+		case "inf":
+			v = math.Inf(+1)
+		case "-inf":
+			v = math.Inf(-1)
+		default:
+			v, err = strconv.ParseFloat(s, 64)
+		}
+		if err == nil {
+			if k == protoreflect.FloatKind {
+				return protoreflect.ValueOf(float32(v)), nil
+			}
+			return protoreflect.ValueOf(float64(v)), nil
+		}
+	case protoreflect.StringKind, protoreflect.BytesKind:
+		// String values use the same escaping as the text format,
+		// however they lack the surrounding double quotes.
+		// TODO: Export unmarshalString in the text package to avoid this hack.
+		v, err := text.Unmarshal([]byte(`["` + s + `"]:0`))
+		if err == nil && len(v.Message()) == 1 {
+			s := v.Message()[0][0].String()
+			if k == protoreflect.StringKind {
+				return protoreflect.ValueOf(s), nil
+			}
+			return protoreflect.ValueOf([]byte(s)), nil
+		}
+	}
+	return protoreflect.Null, errors.New("invalid default value for %v: %q", k, s)
+}

reflect/prototype/protofile_type.go

@@ -427,7 +427,21 @@ func (p *defaultValue) lazyInit(t pref.FieldDescriptor, v pref.Value) pref.Value
 	p.once.Do(func() {
 		p.val = v
 		if !v.IsNull() {
-			if t.Kind() == pref.BytesKind {
+			switch t.Kind() {
+			case pref.EnumKind:
+				// Treat a string value as an identifier referencing some enum
+				// value by name and extract the enum number.
+				// If this fails, validateMessage will later detect that the
+				// default value for an enum value is the wrong type.
+				if s, ok := v.Interface().(string); ok {
+					v := t.EnumType().Values().ByName(pref.Name(s))
+					if v != nil {
+						p.val = pref.ValueOf(v.Number())
+					}
+				}
+			case pref.BytesKind:
+				// Store a copy of the default bytes, so that we can detect
+				// accidental mutations of the original value.
 				if b, ok := v.Interface().([]byte); ok && len(b) > 0 {
 					p.buf = append([]byte(nil), b...)
 				}

reflect/prototype/type_test.go

@@ -8,9 +8,11 @@ import (
 	"reflect"
 	"strconv"
 	"strings"
-	"sync"
 	"testing"
 
+	protoV1 "github.com/golang/protobuf/proto"
+	descriptorV1 "github.com/golang/protobuf/protoc-gen-go/descriptor"
+
 	pref "google.golang.org/proto/reflect/protoreflect"
 )
 
@@ -61,8 +63,10 @@ func TestDescriptors(t *testing.T) {
 	}
 }
 
+// TODO: Test NewFileFromDescriptorProto with imported files.
+
 func TestFile(t *testing.T) {
-	f := &File{
+	f1 := &File{
 		Syntax:  pref.Proto2,
 		Path:    "path/to/file.proto",
 		Package: "test",
@@ -159,7 +163,7 @@ func TestFile(t *testing.T) {
 			Number:       1000,
 			Cardinality:  pref.Repeated,
 			Kind:         pref.MessageKind,
-			IsPacked:     false,
+			IsPacked:     true,
 			MessageType:  PlaceholderMessage("test.C"),
 			ExtendedType: PlaceholderMessage("test.B"),
 		}},
@@ -174,12 +178,159 @@ func TestFile(t *testing.T) {
 			}},
 		}},
 	}
-
-	fd, err := NewFile(f)
+	fd1, err := NewFile(f1)
 	if err != nil {
 		t.Fatalf("NewFile() error: %v", err)
 	}
 
+	f2 := &descriptorV1.FileDescriptorProto{
+		Syntax:  protoV1.String("proto2"),
+		Name:    protoV1.String("path/to/file.proto"),
+		Package: protoV1.String("test"),
+		MessageType: []*descriptorV1.DescriptorProto{{
+			Name:    protoV1.String("A"),
+			Options: &descriptorV1.MessageOptions{MapEntry: protoV1.Bool(true)},
+			Field: []*descriptorV1.FieldDescriptorProto{{
+				Name:   protoV1.String("key"),
+				Number: protoV1.Int32(1),
+				Label:  descriptorV1.FieldDescriptorProto_Label(pref.Optional).Enum(),
+				Type:   descriptorV1.FieldDescriptorProto_Type(pref.StringKind).Enum(),
+			}, {
+				Name:     protoV1.String("value"),
+				Number:   protoV1.Int32(2),
+				Label:    descriptorV1.FieldDescriptorProto_Label(pref.Optional).Enum(),
+				Type:     descriptorV1.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
+				TypeName: protoV1.String(".test.B"),
+			}},
+		}, {
+			Name: protoV1.String("B"),
+			Field: []*descriptorV1.FieldDescriptorProto{{
+				Name:         protoV1.String("field_one"),
+				Number:       protoV1.Int32(1),
+				Label:        descriptorV1.FieldDescriptorProto_Label(pref.Optional).Enum(),
+				Type:         descriptorV1.FieldDescriptorProto_Type(pref.StringKind).Enum(),
+				DefaultValue: protoV1.String("hello"),
+				OneofIndex:   protoV1.Int32(0),
+			}, {
+				Name:         protoV1.String("field_two"),
+				JsonName:     protoV1.String("Field2"),
+				Number:       protoV1.Int32(2),
+				Label:        descriptorV1.FieldDescriptorProto_Label(pref.Optional).Enum(),
+				Type:         descriptorV1.FieldDescriptorProto_Type(pref.EnumKind).Enum(),
+				DefaultValue: protoV1.String("BAR"),
+				TypeName:     protoV1.String(".test.E1"),
+				OneofIndex:   protoV1.Int32(1),
+			}, {
+				Name:       protoV1.String("field_three"),
+				Number:     protoV1.Int32(3),
+				Label:      descriptorV1.FieldDescriptorProto_Label(pref.Optional).Enum(),
+				Type:       descriptorV1.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
+				TypeName:   protoV1.String(".test.C"),
+				OneofIndex: protoV1.Int32(1),
+			}, {
+				Name:     protoV1.String("field_four"),
+				JsonName: protoV1.String("Field4"),
+				Number:   protoV1.Int32(4),
+				Label:    descriptorV1.FieldDescriptorProto_Label(pref.Repeated).Enum(),
+				Type:     descriptorV1.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
+				TypeName: protoV1.String(".test.A"),
+			}, {
+				Name:    protoV1.String("field_five"),
+				Number:  protoV1.Int32(5),
+				Label:   descriptorV1.FieldDescriptorProto_Label(pref.Repeated).Enum(),
+				Type:    descriptorV1.FieldDescriptorProto_Type(pref.Int32Kind).Enum(),
+				Options: &descriptorV1.FieldOptions{Packed: protoV1.Bool(true)},
+			}, {
+				Name:   protoV1.String("field_six"),
+				Number: protoV1.Int32(6),
+				Label:  descriptorV1.FieldDescriptorProto_Label(pref.Required).Enum(),
+				Type:   descriptorV1.FieldDescriptorProto_Type(pref.StringKind).Enum(),
+			}},
+			OneofDecl: []*descriptorV1.OneofDescriptorProto{
+				{Name: protoV1.String("O1")},
+				{Name: protoV1.String("O2")},
+			},
+			ExtensionRange: []*descriptorV1.DescriptorProto_ExtensionRange{
+				{Start: protoV1.Int32(1000), End: protoV1.Int32(2000)},
+			},
+		}, {
+			Name: protoV1.String("C"),
+			NestedType: []*descriptorV1.DescriptorProto{{
+				Name: protoV1.String("A"),
+				Field: []*descriptorV1.FieldDescriptorProto{{
+					Name:   protoV1.String("F"),
+					Number: protoV1.Int32(1),
+					Label:  descriptorV1.FieldDescriptorProto_Label(pref.Required).Enum(),
+					Type:   descriptorV1.FieldDescriptorProto_Type(pref.BytesKind).Enum(),
+				}},
+			}},
+			EnumType: []*descriptorV1.EnumDescriptorProto{{
+				Name: protoV1.String("E1"),
+				Value: []*descriptorV1.EnumValueDescriptorProto{
+					{Name: protoV1.String("FOO"), Number: protoV1.Int32(0)},
+					{Name: protoV1.String("BAR"), Number: protoV1.Int32(1)},
+				},
+			}},
+			Extension: []*descriptorV1.FieldDescriptorProto{{
+				Name:     protoV1.String("X"),
+				Number:   protoV1.Int32(1000),
+				Label:    descriptorV1.FieldDescriptorProto_Label(pref.Repeated).Enum(),
+				Type:     descriptorV1.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
+				TypeName: protoV1.String(".test.C"),
+				Extendee: protoV1.String(".test.B"),
+			}},
+		}},
+		EnumType: []*descriptorV1.EnumDescriptorProto{{
+			Name: protoV1.String("E1"),
+			Value: []*descriptorV1.EnumValueDescriptorProto{
+				{Name: protoV1.String("FOO"), Number: protoV1.Int32(0)},
+				{Name: protoV1.String("BAR"), Number: protoV1.Int32(1)},
+			},
+		}},
+		Extension: []*descriptorV1.FieldDescriptorProto{{
+			Name:     protoV1.String("X"),
+			Number:   protoV1.Int32(1000),
+			Label:    descriptorV1.FieldDescriptorProto_Label(pref.Repeated).Enum(),
+			Type:     descriptorV1.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
+			Options:  &descriptorV1.FieldOptions{Packed: protoV1.Bool(true)},
+			TypeName: protoV1.String(".test.C"),
+			Extendee: protoV1.String(".test.B"),
+		}},
+		Service: []*descriptorV1.ServiceDescriptorProto{{
+			Name: protoV1.String("S"),
+			Method: []*descriptorV1.MethodDescriptorProto{{
+				Name:            protoV1.String("M"),
+				InputType:       protoV1.String(".test.A"),
+				OutputType:      protoV1.String(".test.C.A"),
+				ClientStreaming: protoV1.Bool(true),
+				ServerStreaming: protoV1.Bool(true),
+			}},
+		}},
+	}
+	fd2, err := NewFileFromDescriptorProto(f2, nil)
+	if err != nil {
+		t.Fatalf("NewFileFromDescriptorProto() error: %v", err)
+	}
+
+	tests := []struct {
+		name string
+		desc pref.FileDescriptor
+	}{
+		{"NewFile", fd1},
+		{"NewFileFromDescriptorProto", fd2},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Run("Accessors", func(t *testing.T) {
+				// Run sub-tests in parallel to induce potential races.
+				t.Run("", func(t *testing.T) { t.Parallel(); testFileAccessors(t, tt.desc) })
+				t.Run("", func(t *testing.T) { t.Parallel(); testFileAccessors(t, tt.desc) })
+			})
+		})
+	}
+}
+
+func testFileAccessors(t *testing.T, fd pref.FileDescriptor) {
 	// Represent the descriptor as a map where each key is an accessor method
 	// and the value is either the wanted tail value or another accessor map.
 	type M = map[string]interface{}
@@ -359,7 +510,7 @@ func TestFile(t *testing.T) {
 				"Number":       pref.FieldNumber(1000),
 				"Cardinality":  pref.Repeated,
 				"Kind":         pref.MessageKind,
-				"IsPacked":     false,
+				"IsPacked":     true,
 				"MessageType":  M{"FullName": pref.FullName("test.C"), "IsPlaceholder": false},
 				"ExtendedType": M{"FullName": pref.FullName("test.B"), "IsPlaceholder": false},
 			},
@@ -413,18 +564,7 @@ func TestFile(t *testing.T) {
 		"DescriptorByName:test.S.M":         M{"FullName": pref.FullName("test.S.M")},
 		"DescriptorByName:test.M":           nil,
 	}
-
-	// Concurrently explore the file tree to induce races.
-	const numGoRoutines = 2
-	var wg sync.WaitGroup
-	defer wg.Wait()
-	for i := 0; i < numGoRoutines; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			checkAccessors(t, "", reflect.ValueOf(fd), want)
-		}()
-	}
+	checkAccessors(t, "", reflect.ValueOf(fd), want)
 }
 
 func checkAccessors(t *testing.T, p string, rv reflect.Value, want map[string]interface{}) {

reflect/prototype/validate.go

@@ -10,6 +10,22 @@ import (
 
 // TODO: This is important to prevent users from creating invalid types,
 // but is not functionality needed now.
+//
+// Things to verify:
+//	* Weak fields are only used if flags.Proto1Legacy is set
+//	* Weak fields can only reference singular messages
+//	(check if this the case for oneof fields)
+//	* FieldDescriptor.MessageType cannot reference a remote type when the
+//	remote name is a type within the local file.
+//	* Default enum identifiers resolve to a declared number.
+//	* Default values are only allowed in proto2.
+//	* Default strings are valid UTF-8? Note that protoc does not check this.
+//	* Field extensions are only valid in proto2, except when extending the
+//	descriptor options.
+//	* Remote enum and message types are actually found in imported files.
+//	* Placeholder messages and types may only be for weak fields.
+//	* Placeholder full names must be valid.
+//	* The name of each descriptor must be valid.
 
 func validateFile(t pref.FileDescriptor) error {
 	return nil