reflect/protodesc: enforce strict validation

Hyrum's Law dictates that if we do not prevent naughty behavior,
people will rely on it. If we do not validate that the provided
file descriptor is correct today, it will be near impossible
to add proper validation checks later on.

The logic added validates that the provided file descriptor is
correct according to the same semantics as protoc,
which was reversed engineered to derive the set of rules implemented here.
The rules are unfortunately complicated because protobuf is a language
full of many non-orthogonal features. While our logic is complicated,
it is still 1/7th the size of the equivalent C++ code!

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

internal/cmd/pbdump/pbdump.go

@@ -205,7 +205,7 @@ func (fs fields) Descriptor() (protoreflect.MessageDescriptor, error) {
 	fd, err := protodesc.NewFile(&descriptorpb.FileDescriptorProto{
 		Name:        scalar.String("dump.proto"),
 		Syntax:      scalar.String("proto2"),
-		MessageType: []*descriptorpb.DescriptorProto{fs.messageDescriptor("M")},
+		MessageType: []*descriptorpb.DescriptorProto{fs.messageDescriptor("X")},
 	}, nil)
 	if err != nil {
 		return nil, err
@@ -220,7 +220,7 @@ func (fs fields) messageDescriptor(name protoreflect.FullName) *descriptorpb.Des
 			k = protoreflect.MessageKind
 		}
 		f := &descriptorpb.FieldDescriptorProto{
-			Name:   scalar.String(fmt.Sprintf("f%d", n)),
+			Name:   scalar.String(fmt.Sprintf("x%d", n)),
 			Number: scalar.Int32(int32(n)),
 			Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
 			Type:   descriptorpb.FieldDescriptorProto_Type(k).Enum(),
@@ -234,7 +234,7 @@ func (fs fields) messageDescriptor(name protoreflect.FullName) *descriptorpb.Des
 			f.Label = descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum()
 			f.Options = &descriptorpb.FieldOptions{Packed: scalar.Bool(true)}
 		case protoreflect.MessageKind, protoreflect.GroupKind:
-			s := name.Append(protoreflect.Name(fmt.Sprintf("M%d", n)))
+			s := name.Append(protoreflect.Name(fmt.Sprintf("X%d", n)))
 			f.TypeName = scalar.String(string("." + s))
 			m.NestedType = append(m.NestedType, fs[n].sub.messageDescriptor(s))
 		}

internal/cmd/pbdump/pbdump_test.go

@@ -36,7 +36,7 @@ func TestFields(t *testing.T) {
 		wantErr  string
 	}{{
 		inFields: []fieldsKind{{pref.MessageKind, ""}},
-		wantMsg:  mustMakeMessage(`name:"M"`),
+		wantMsg:  mustMakeMessage(`name:"X"`),
 	}, {
 		inFields: []fieldsKind{{pref.MessageKind, "987654321"}},
 		wantErr:  "invalid field: 987654321",
@@ -62,27 +62,27 @@ func TestFields(t *testing.T) {
 			{pref.GroupKind, "10"},
 		},
 		wantMsg: mustMakeMessage(`
-			name: "M"
+			name: "X"
 			field: [
-				{name:"f10" number:10 label:LABEL_OPTIONAL type:TYPE_GROUP type_name:".M.M10"}
+				{name:"x10" number:10 label:LABEL_OPTIONAL type:TYPE_GROUP type_name:".X.X10"}
 			]
 			nested_type: [{
-				name: "M10"
+				name: "X10"
 				field: [
-					{name:"f20" number:20 label:LABEL_OPTIONAL type:TYPE_MESSAGE type_name:".M.M10.M20"},
-					{name:"f21" number:21 label:LABEL_OPTIONAL type:TYPE_MESSAGE type_name:".M.M10.M21"}
+					{name:"x20" number:20 label:LABEL_OPTIONAL type:TYPE_MESSAGE type_name:".X.X10.X20"},
+					{name:"x21" number:21 label:LABEL_OPTIONAL type:TYPE_MESSAGE type_name:".X.X10.X21"}
 				]
 				nested_type: [{
-					name: "M20"
+					name: "X20"
 					field:[
-						{name:"f30" number:30 label:LABEL_OPTIONAL type:TYPE_MESSAGE, type_name:".M.M10.M20.M30"},
-						{name:"f31" number:31 label:LABEL_REPEATED type:TYPE_INT32 options:{packed:true}}
+						{name:"x30" number:30 label:LABEL_OPTIONAL type:TYPE_MESSAGE, type_name:".X.X10.X20.X30"},
+						{name:"x31" number:31 label:LABEL_REPEATED type:TYPE_INT32 options:{packed:true}}
 					]
 					nested_type: [{
-						name: "M30"
+						name: "X30"
 					}]
 				}, {
-					name: "M21"
+					name: "X21"
 				}]
 			}]
 		`),
@@ -103,7 +103,7 @@ func TestFields(t *testing.T) {
 			if tt.wantErr != "" {
 				t.Errorf("all Set calls succeeded, want %v error", tt.wantErr)
 			}
-			gotMsg := fields.messageDescriptor("M")
+			gotMsg := fields.messageDescriptor("X")
 			if !proto.Equal(gotMsg, tt.wantMsg) {
 				t.Errorf("messageDescriptor() mismatch:\ngot  %v\nwant %v", gotMsg, tt.wantMsg)
 			}

internal/encoding/pack/pack_test.go

@@ -26,21 +26,22 @@ var msgDesc = func() pref.MessageDescriptor {
 		message_type: [{
 			name: "Message"
 			field: [
-				{name:"F1"  number:1  label:LABEL_REPEATED type:TYPE_BOOL     options:{packed:true}},
-				{name:"F2"  number:2  label:LABEL_REPEATED type:TYPE_INT64    options:{packed:true}},
-				{name:"F3"  number:3  label:LABEL_REPEATED type:TYPE_SINT64   options:{packed:true}},
-				{name:"F4"  number:4  label:LABEL_REPEATED type:TYPE_UINT64   options:{packed:true}},
-				{name:"F5"  number:5  label:LABEL_REPEATED type:TYPE_FIXED32  options:{packed:true}},
-				{name:"F6"  number:6  label:LABEL_REPEATED type:TYPE_SFIXED32 options:{packed:true}},
-				{name:"F7"  number:7  label:LABEL_REPEATED type:TYPE_FLOAT    options:{packed:true}},
-				{name:"F8"  number:8  label:LABEL_REPEATED type:TYPE_FIXED64  options:{packed:true}},
-				{name:"F9"  number:9  label:LABEL_REPEATED type:TYPE_SFIXED64 options:{packed:true}},
-				{name:"F10" number:10 label:LABEL_REPEATED type:TYPE_DOUBLE   options:{packed:true}},
-				{name:"F11" number:11 label:LABEL_OPTIONAL type:TYPE_STRING},
-				{name:"F12" number:12 label:LABEL_OPTIONAL type:TYPE_BYTES},
-				{name:"F13" number:13 label:LABEL_OPTIONAL type:TYPE_MESSAGE type_name:".Message"},
-				{name:"F14" number:14 label:LABEL_OPTIONAL type:TYPE_GROUP   type_name:".Message"}
+				{name:"f1"  number:1  label:LABEL_REPEATED type:TYPE_BOOL     options:{packed:true}},
+				{name:"f2"  number:2  label:LABEL_REPEATED type:TYPE_INT64    options:{packed:true}},
+				{name:"f3"  number:3  label:LABEL_REPEATED type:TYPE_SINT64   options:{packed:true}},
+				{name:"f4"  number:4  label:LABEL_REPEATED type:TYPE_UINT64   options:{packed:true}},
+				{name:"f5"  number:5  label:LABEL_REPEATED type:TYPE_FIXED32  options:{packed:true}},
+				{name:"f6"  number:6  label:LABEL_REPEATED type:TYPE_SFIXED32 options:{packed:true}},
+				{name:"f7"  number:7  label:LABEL_REPEATED type:TYPE_FLOAT    options:{packed:true}},
+				{name:"f8"  number:8  label:LABEL_REPEATED type:TYPE_FIXED64  options:{packed:true}},
+				{name:"f9"  number:9  label:LABEL_REPEATED type:TYPE_SFIXED64 options:{packed:true}},
+				{name:"f10" number:10 label:LABEL_REPEATED type:TYPE_DOUBLE   options:{packed:true}},
+				{name:"f11" number:11 label:LABEL_OPTIONAL type:TYPE_STRING},
+				{name:"f12" number:12 label:LABEL_OPTIONAL type:TYPE_BYTES},
+				{name:"f13" number:13 label:LABEL_OPTIONAL type:TYPE_MESSAGE type_name:".Message"},
+				{name:"f14" number:14 label:LABEL_OPTIONAL type:TYPE_GROUP   type_name:".Message.F14"}
 			]
+			nested_type: [{name: "F14"}]
 		}]
 	`
 	pb := new(descriptorpb.FileDescriptorProto)

internal/filedesc/desc_list.go

@@ -6,11 +6,15 @@ package filedesc
 
 import (
 	"fmt"
+	"math"
 	"sort"
 	"sync"
 
 	"google.golang.org/protobuf/internal/descfmt"
+	"google.golang.org/protobuf/internal/encoding/wire"
+	"google.golang.org/protobuf/internal/errors"
 	"google.golang.org/protobuf/internal/pragma"
+	"google.golang.org/protobuf/reflect/protoreflect"
 	pref "google.golang.org/protobuf/reflect/protoreflect"
 )
 
@@ -24,60 +28,57 @@ func (p *FileImports) ProtoInternal(pragma.DoNotImplement) {}
 type Names struct {
 	List []pref.Name
 	once sync.Once
-	has  map[pref.Name]struct{} // protected by once
+	has  map[pref.Name]int // protected by once
 }
 
-func (p *Names) Len() int            { return len(p.List) }
-func (p *Names) Get(i int) pref.Name { return p.List[i] }
-func (p *Names) Has(s pref.Name) bool {
+func (p *Names) Len() int                            { return len(p.List) }
+func (p *Names) Get(i int) pref.Name                 { return p.List[i] }
+func (p *Names) Has(s pref.Name) bool                { return p.lazyInit().has[s] > 0 }
+func (p *Names) Format(s fmt.State, r rune)          { descfmt.FormatList(s, r, p) }
+func (p *Names) ProtoInternal(pragma.DoNotImplement) {}
+func (p *Names) lazyInit() *Names {
 	p.once.Do(func() {
 		if len(p.List) > 0 {
-			p.has = make(map[pref.Name]struct{}, len(p.List))
+			p.has = make(map[pref.Name]int, len(p.List))
 			for _, s := range p.List {
-				p.has[s] = struct{}{}
+				p.has[s] = p.has[s] + 1
 			}
 		}
 	})
-	_, ok := p.has[s]
-	return ok
+	return p
+}
+
+// CheckValid reports any errors with the set of names with an error message
+// that completes the sentence: "ranges is invalid because it has ..."
+func (p *Names) CheckValid() error {
+	for s, n := range p.lazyInit().has {
+		switch {
+		case n > 1:
+			return errors.New("duplicate name: %q", s)
+		case false && !s.IsValid():
+			// NOTE: The C++ implementation does not validate the identifier.
+			// See https://github.com/protocolbuffers/protobuf/issues/6335.
+			return errors.New("invalid name: %q", s)
+		}
+	}
+	return nil
 }
-func (p *Names) Format(s fmt.State, r rune)          { descfmt.FormatList(s, r, p) }
-func (p *Names) ProtoInternal(pragma.DoNotImplement) {}
 
 type EnumRanges struct {
 	List   [][2]pref.EnumNumber // start inclusive; end inclusive
 	once   sync.Once
-	sorted [][2]pref.EnumNumber         // protected by once
-	has    map[pref.EnumNumber]struct{} // protected by once
+	sorted [][2]pref.EnumNumber // protected by once
 }
 
 func (p *EnumRanges) Len() int                     { return len(p.List) }
 func (p *EnumRanges) Get(i int) [2]pref.EnumNumber { return p.List[i] }
 func (p *EnumRanges) Has(n pref.EnumNumber) bool {
-	p.once.Do(func() {
-		for _, r := range p.List {
-			if r[0] == r[1]-0 {
-				if p.has == nil {
-					p.has = make(map[pref.EnumNumber]struct{}, len(p.List))
-				}
-				p.has[r[0]] = struct{}{}
-			} else {
-				p.sorted = append(p.sorted, r)
-			}
-		}
-		sort.Slice(p.sorted, func(i, j int) bool {
-			return p.sorted[i][0] < p.sorted[j][0]
-		})
-	})
-	if _, ok := p.has[n]; ok {
-		return true
-	}
-	for ls := p.sorted; len(ls) > 0; {
+	for ls := p.lazyInit().sorted; len(ls) > 0; {
 		i := len(ls) / 2
-		switch r := ls[i]; {
-		case n < r[0]:
+		switch r := enumRange(ls[i]); {
+		case n < r.Start():
 			ls = ls[:i] // search lower
-		case n > r[1]:
+		case n > r.End():
 			ls = ls[i+1:] // search upper
 		default:
 			return true
@@ -87,42 +88,60 @@ func (p *EnumRanges) Has(n pref.EnumNumber) bool {
 }
 func (p *EnumRanges) Format(s fmt.State, r rune)          { descfmt.FormatList(s, r, p) }
 func (p *EnumRanges) ProtoInternal(pragma.DoNotImplement) {}
+func (p *EnumRanges) lazyInit() *EnumRanges {
+	p.once.Do(func() {
+		p.sorted = append(p.sorted, p.List...)
+		sort.Slice(p.sorted, func(i, j int) bool {
+			return p.sorted[i][0] < p.sorted[j][0]
+		})
+	})
+	return p
+}
+
+// CheckValid reports any errors with the set of names with an error message
+// that completes the sentence: "ranges is invalid because it has ..."
+func (p *EnumRanges) CheckValid() error {
+	var rp enumRange
+	for i, r := range p.lazyInit().sorted {
+		r := enumRange(r)
+		switch {
+		case !(r.Start() <= r.End()):
+			return errors.New("invalid range: %v", r)
+		case !(rp.End() < r.Start()) && i > 0:
+			return errors.New("overlapping ranges: %v with %v", rp, r)
+		}
+		rp = r
+	}
+	return nil
+}
+
+type enumRange [2]protoreflect.EnumNumber
+
+func (r enumRange) Start() protoreflect.EnumNumber { return r[0] } // inclusive
+func (r enumRange) End() protoreflect.EnumNumber   { return r[1] } // inclusive
+func (r enumRange) String() string {
+	if r.Start() == r.End() {
+		return fmt.Sprintf("%d", r.Start())
+	}
+	return fmt.Sprintf("%d to %d", r.Start(), r.End())
+}
 
 type FieldRanges struct {
 	List   [][2]pref.FieldNumber // start inclusive; end exclusive
 	once   sync.Once
-	sorted [][2]pref.FieldNumber         // protected by once
-	has    map[pref.FieldNumber]struct{} // protected by once
+	sorted [][2]pref.FieldNumber // protected by once
 }
 
 func (p *FieldRanges) Len() int                      { return len(p.List) }
 func (p *FieldRanges) Get(i int) [2]pref.FieldNumber { return p.List[i] }
 func (p *FieldRanges) Has(n pref.FieldNumber) bool {
-	p.once.Do(func() {
-		for _, r := range p.List {
-			if r[0] == r[1]-1 {
-				if p.has == nil {
-					p.has = make(map[pref.FieldNumber]struct{}, len(p.List))
-				}
-				p.has[r[0]] = struct{}{}
-			} else {
-				p.sorted = append(p.sorted, r)
-			}
-		}
-		sort.Slice(p.sorted, func(i, j int) bool {
-			return p.sorted[i][0] < p.sorted[j][0]
-		})
-	})
-	if _, ok := p.has[n]; ok {
-		return true
-	}
-	for ls := p.sorted; len(ls) > 0; {
+	for ls := p.lazyInit().sorted; len(ls) > 0; {
 		i := len(ls) / 2
-		switch r := ls[i]; {
-		case n < r[0]:
+		switch r := fieldRange(ls[i]); {
+		case n < r.Start():
 			ls = ls[:i] // search lower
-		case n >= r[1]:
-			ls = ls[i+1:] // search higher
+		case n > r.End():
+			ls = ls[i+1:] // search upper
 		default:
 			return true
 		}
@@ -131,6 +150,76 @@ func (p *FieldRanges) Has(n pref.FieldNumber) bool {
 }
 func (p *FieldRanges) Format(s fmt.State, r rune)          { descfmt.FormatList(s, r, p) }
 func (p *FieldRanges) ProtoInternal(pragma.DoNotImplement) {}
+func (p *FieldRanges) lazyInit() *FieldRanges {
+	p.once.Do(func() {
+		p.sorted = append(p.sorted, p.List...)
+		sort.Slice(p.sorted, func(i, j int) bool {
+			return p.sorted[i][0] < p.sorted[j][0]
+		})
+	})
+	return p
+}
+
+// CheckValid reports any errors with the set of ranges with an error message
+// that completes the sentence: "ranges is invalid because it has ..."
+func (p *FieldRanges) CheckValid(isMessageSet bool) error {
+	var rp fieldRange
+	for i, r := range p.lazyInit().sorted {
+		r := fieldRange(r)
+		switch {
+		case !isValidFieldNumber(r.Start(), isMessageSet):
+			return errors.New("invalid field number: %d", r.Start())
+		case !isValidFieldNumber(r.End(), isMessageSet):
+			return errors.New("invalid field number: %d", r.End())
+		case !(r.Start() <= r.End()):
+			return errors.New("invalid range: %v", r)
+		case !(rp.End() < r.Start()) && i > 0:
+			return errors.New("overlapping ranges: %v with %v", rp, r)
+		}
+		rp = r
+	}
+	return nil
+}
+
+// isValidFieldNumber reports whether the field number is valid.
+// Unlike the FieldNumber.IsValid method, it allows ranges that cover the
+// reserved number range.
+func isValidFieldNumber(n protoreflect.FieldNumber, isMessageSet bool) bool {
+	if isMessageSet {
+		return wire.MinValidNumber <= n && n <= math.MaxInt32
+	}
+	return wire.MinValidNumber <= n && n <= wire.MaxValidNumber
+}
+
+// CheckOverlap reports an error if p and q overlap.
+func (p *FieldRanges) CheckOverlap(q *FieldRanges) error {
+	rps := p.lazyInit().sorted
+	rqs := q.lazyInit().sorted
+	for pi, qi := 0, 0; pi < len(rps) && qi < len(rqs); {
+		rp := fieldRange(rps[pi])
+		rq := fieldRange(rqs[qi])
+		if !(rp.End() < rq.Start() || rq.End() < rp.Start()) {
+			return errors.New("overlapping ranges: %v with %v", rp, rq)
+		}
+		if rp.Start() < rq.Start() {
+			pi++
+		} else {
+			qi++
+		}
+	}
+	return nil
+}
+
+type fieldRange [2]protoreflect.FieldNumber
+
+func (r fieldRange) Start() protoreflect.FieldNumber { return r[0] }     // inclusive
+func (r fieldRange) End() protoreflect.FieldNumber   { return r[1] - 1 } // inclusive
+func (r fieldRange) String() string {
+	if r.Start() == r.End() {
+		return fmt.Sprintf("%d", r.Start())
+	}
+	return fmt.Sprintf("%d to %d", r.Start(), r.End())
+}
 
 type FieldNumbers struct {
 	List []pref.FieldNumber

internal/filedesc/desc_test.go

@@ -39,22 +39,8 @@ func TestFile(t *testing.T) {
 		MessageType: []*descriptorpb.DescriptorProto{{
 			Name: scalar.String("A"),
 			Options: &descriptorpb.MessageOptions{
-				MapEntry:   scalar.Bool(true),
 				Deprecated: scalar.Bool(true),
 			},
-			Field: []*descriptorpb.FieldDescriptorProto{{
-				Name:    scalar.String("key"),
-				Number:  scalar.Int32(1),
-				Options: &descriptorpb.FieldOptions{Deprecated: scalar.Bool(true)},
-				Label:   descriptorpb.FieldDescriptorProto_Label(pref.Optional).Enum(),
-				Type:    descriptorpb.FieldDescriptorProto_Type(pref.StringKind).Enum(),
-			}, {
-				Name:     scalar.String("value"),
-				Number:   scalar.Int32(2),
-				Label:    descriptorpb.FieldDescriptorProto_Label(pref.Optional).Enum(),
-				Type:     descriptorpb.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
-				TypeName: scalar.String(".test.B"),
-			}},
 		}, {
 			Name: scalar.String("B"),
 			Field: []*descriptorpb.FieldDescriptorProto{{
@@ -86,7 +72,7 @@ func TestFile(t *testing.T) {
 				Number:   scalar.Int32(4),
 				Label:    descriptorpb.FieldDescriptorProto_Label(pref.Repeated).Enum(),
 				Type:     descriptorpb.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
-				TypeName: scalar.String(".test.A"),
+				TypeName: scalar.String(".test.B.FieldFourEntry"),
 			}, {
 				Name:    scalar.String("field_five"),
 				Number:  scalar.Int32(5),
@@ -119,6 +105,24 @@ func TestFile(t *testing.T) {
 				{Start: scalar.Int32(1000), End: scalar.Int32(2000)},
 				{Start: scalar.Int32(3000), End: scalar.Int32(3001), Options: new(descriptorpb.ExtensionRangeOptions)},
 			},
+			NestedType: []*descriptorpb.DescriptorProto{{
+				Name: scalar.String("FieldFourEntry"),
+				Field: []*descriptorpb.FieldDescriptorProto{{
+					Name:   scalar.String("key"),
+					Number: scalar.Int32(1),
+					Label:  descriptorpb.FieldDescriptorProto_Label(pref.Optional).Enum(),
+					Type:   descriptorpb.FieldDescriptorProto_Type(pref.StringKind).Enum(),
+				}, {
+					Name:     scalar.String("value"),
+					Number:   scalar.Int32(2),
+					Label:    descriptorpb.FieldDescriptorProto_Label(pref.Optional).Enum(),
+					Type:     descriptorpb.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
+					TypeName: scalar.String(".test.B"),
+				}},
+				Options: &descriptorpb.MessageOptions{
+					MapEntry: scalar.Bool(true),
+				},
+			}},
 		}, {
 			Name: scalar.String("C"),
 			NestedType: []*descriptorpb.DescriptorProto{{
@@ -168,9 +172,9 @@ func TestFile(t *testing.T) {
 			Name:     scalar.String("X"),
 			Number:   scalar.Int32(1000),
 			Label:    descriptorpb.FieldDescriptorProto_Label(pref.Repeated).Enum(),
-			Type:     descriptorpb.FieldDescriptorProto_Type(pref.MessageKind).Enum(),
+			Type:     descriptorpb.FieldDescriptorProto_Type(pref.EnumKind).Enum(),
 			Options:  &descriptorpb.FieldOptions{Packed: scalar.Bool(true)},
-			TypeName: scalar.String(".test.C"),
+			TypeName: scalar.String(".test.E1"),
 			Extendee: scalar.String(".test.B"),
 		}},
 		Service: []*descriptorpb.ServiceDescriptorProto{{
@@ -239,57 +243,10 @@ func testFileAccessors(t *testing.T, fd pref.FileDescriptor) {
 				"Name":          pref.Name("A"),
 				"FullName":      pref.FullName("test.A"),
 				"IsPlaceholder": false,
-				"IsMapEntry":    true,
+				"IsMapEntry":    false,
 				"Options": &descriptorpb.MessageOptions{
-					MapEntry:   scalar.Bool(true),
 					Deprecated: scalar.Bool(true),
 				},
-				"Fields": M{
-					"Len": 2,
-					"ByNumber:1": M{
-						"Parent":            M{"FullName": pref.FullName("test.A")},
-						"Index":             0,
-						"Name":              pref.Name("key"),
-						"FullName":          pref.FullName("test.A.key"),
-						"Number":            pref.FieldNumber(1),
-						"Cardinality":       pref.Optional,
-						"Kind":              pref.StringKind,
-						"Options":           &descriptorpb.FieldOptions{Deprecated: scalar.Bool(true)},
-						"HasJSONName":       false,
-						"JSONName":          "key",
-						"IsPacked":          false,
-						"IsList":            false,
-						"IsMap":             false,
-						"IsExtension":       false,
-						"IsWeak":            false,
-						"Default":           "",
-						"ContainingOneof":   nil,
-						"ContainingMessage": M{"FullName": pref.FullName("test.A")},
-						"Message":           nil,
-						"Enum":              nil,
-					},
-					"ByNumber:2": M{
-						"Parent":            M{"FullName": pref.FullName("test.A")},
-						"Index":             1,
-						"Name":              pref.Name("value"),
-						"FullName":          pref.FullName("test.A.value"),
-						"Number":            pref.FieldNumber(2),
-						"Cardinality":       pref.Optional,
-						"Kind":              pref.MessageKind,
-						"JSONName":          "value",
-						"IsPacked":          false,
-						"IsList":            false,
-						"IsMap":             false,
-						"IsExtension":       false,
-						"IsWeak":            false,
-						"Default":           nil,
-						"ContainingOneof":   nil,
-						"ContainingMessage": M{"FullName": pref.FullName("test.A")},
-						"Message":           M{"FullName": pref.FullName("test.B"), "IsPlaceholder": false},
-						"Enum":              nil,
-					},
-					"ByNumber:3": nil,
-				},
 				"Oneofs":          M{"Len": 0},
 				"RequiredNumbers": M{"Len": 0},
 				"ExtensionRanges": M{"Len": 0},
@@ -339,7 +296,7 @@ func testFileAccessors(t *testing.T, fd pref.FileDescriptor) {
 						"MapKey":      M{"Kind": pref.StringKind},
 						"MapValue":    M{"Kind": pref.MessageKind, "Message": M{"FullName": pref.FullName("test.B")}},
 						"Default":     nil,
-						"Message":     M{"FullName": pref.FullName("test.A"), "IsPlaceholder": false},
+						"Message":     M{"FullName": pref.FullName("test.B.FieldFourEntry"), "IsPlaceholder": false},
 					},
 					"ByNumber:5": M{
 						"Cardinality": pref.Repeated,
@@ -417,6 +374,56 @@ func testFileAccessors(t *testing.T, fd pref.FileDescriptor) {
 				},
 				"ExtensionRangeOptions:0": (*descriptorpb.ExtensionRangeOptions)(nil),
 				"ExtensionRangeOptions:1": new(descriptorpb.ExtensionRangeOptions),
+				"Messages": M{
+					"Get:0": M{
+						"Fields": M{
+							"Len": 2,
+							"ByNumber:1": M{
+								"Parent":            M{"FullName": pref.FullName("test.B.FieldFourEntry")},
+								"Index":             0,
+								"Name":              pref.Name("key"),
+								"FullName":          pref.FullName("test.B.FieldFourEntry.key"),
+								"Number":            pref.FieldNumber(1),
+								"Cardinality":       pref.Optional,
+								"Kind":              pref.StringKind,
+								"Options":           (*descriptorpb.FieldOptions)(nil),
+								"HasJSONName":       false,
+								"JSONName":          "key",
+								"IsPacked":          false,
+								"IsList":            false,
+								"IsMap":             false,
+								"IsExtension":       false,
+								"IsWeak":            false,
+								"Default":           "",
+								"ContainingOneof":   nil,
+								"ContainingMessage": M{"FullName": pref.FullName("test.B.FieldFourEntry")},
+								"Message":           nil,
+								"Enum":              nil,
+							},
+							"ByNumber:2": M{
+								"Parent":            M{"FullName": pref.FullName("test.B.FieldFourEntry")},
+								"Index":             1,
+								"Name":              pref.Name("value"),
+								"FullName":          pref.FullName("test.B.FieldFourEntry.value"),
+								"Number":            pref.FieldNumber(2),
+								"Cardinality":       pref.Optional,
+								"Kind":              pref.MessageKind,
+								"JSONName":          "value",
+								"IsPacked":          false,
+								"IsList":            false,
+								"IsMap":             false,
+								"IsExtension":       false,
+								"IsWeak":            false,
+								"Default":           nil,
+								"ContainingOneof":   nil,
+								"ContainingMessage": M{"FullName": pref.FullName("test.B.FieldFourEntry")},
+								"Message":           M{"FullName": pref.FullName("test.B"), "IsPlaceholder": false},
+								"Enum":              nil,
+							},
+							"ByNumber:3": nil,
+						},
+					},
+				},
 			},
 			"Get:2": M{
 				"Name":  pref.Name("C"),
@@ -475,7 +482,7 @@ func testFileAccessors(t *testing.T, fd pref.FileDescriptor) {
 				"Name":              pref.Name("X"),
 				"Number":            pref.FieldNumber(1000),
 				"Cardinality":       pref.Repeated,
-				"Kind":              pref.MessageKind,
+				"Kind":              pref.EnumKind,
 				"IsExtension":       true,
 				"IsPacked":          true,
 				"IsList":            true,
@@ -484,7 +491,7 @@ func testFileAccessors(t *testing.T, fd pref.FileDescriptor) {
 				"MapValue":          nil,
 				"ContainingOneof":   nil,
 				"ContainingMessage": M{"FullName": pref.FullName("test.B"), "IsPlaceholder": false},
-				"Message":           M{"FullName": pref.FullName("test.C"), "IsPlaceholder": false},
+				"Enum":              M{"FullName": pref.FullName("test.E1"), "IsPlaceholder": false},
 				"Options":           &descriptorpb.FieldOptions{Packed: scalar.Bool(true)},
 			},
 		},
@@ -598,22 +605,7 @@ func testFileFormat(t *testing.T, fd pref.FileDescriptor) {
 	Path:    "path/to/file.proto"
 	Package: test
 	Messages: [{
-		Name:       A
-		IsMapEntry: true
-		Fields: [{
-			Name:        key
-			Number:      1
-			Cardinality: optional
-			Kind:        string
-			JSONName:    "key"
-		}, {
-			Name:        value
-			Number:      2
-			Cardinality: optional
-			Kind:        message
-			JSONName:    "value"
-			Message:     test.B
-		}]
+		Name: A
 	}, {
 		Name: B
 		Fields: [{
@@ -680,6 +672,24 @@ func testFileFormat(t *testing.T, fd pref.FileDescriptor) {
 		ReservedRanges:  [100:200, 300]
 		RequiredNumbers: [6]
 		ExtensionRanges: [1000:2000, 3000]
+		Messages: [{
+			Name:       FieldFourEntry
+			IsMapEntry: true
+			Fields: [{
+				Name:        key
+				Number:      1
+				Cardinality: optional
+				Kind:        string
+				JSONName:    "key"
+			}, {
+				Name:        value
+				Number:      2
+				Cardinality: optional
+				Kind:        message
+				JSONName:    "value"
+				Message:     test.B
+			}]
+		}]
 	}, {
 		Name: C
 		Messages: [{
@@ -727,13 +737,13 @@ func testFileFormat(t *testing.T, fd pref.FileDescriptor) {
 		Name:        X
 		Number:      1000
 		Cardinality: repeated
-		Kind:        message
+		Kind:        enum
 		JSONName:    "X"
 		IsPacked:    true
 		IsExtension: true
 		IsList:      true
 		Extendee:    test.B
-		Message:     test.C
+		Enum:        test.E1
 	}]
 	Services: [{
 		Name: S

reflect/protodesc/desc.go

@@ -17,6 +17,9 @@ import (
 )
 
 // Resolver is the resolver used by NewFile to resolve dependencies.
+// The enums and messages provided must belong to some parent file,
+// which is also registered.
+//
 // It is implemented by protoregistry.Files.
 type Resolver interface {
 	FindFileByPath(string) (protoreflect.FileDescriptor, error)
@@ -57,7 +60,7 @@ func allowUnresolvable() option {
 // 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.
 //
-// Any import files, enum types, or message types referenced in the file are
+// Any imported 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.
@@ -83,10 +86,16 @@ func newFile(fd *descriptorpb.FileDescriptorProto, r Resolver, opts ...option) (
 	case "proto3":
 		f.L1.Syntax = protoreflect.Proto3
 	default:
-		return nil, errors.New("invalid syntax: %v", fd.GetSyntax())
+		return nil, errors.New("invalid syntax: %q", fd.GetSyntax())
 	}
 	f.L1.Path = fd.GetName()
+	if f.L1.Path == "" {
+		return nil, errors.New("file path must be populated")
+	}
 	f.L1.Package = protoreflect.FullName(fd.GetPackage())
+	if !f.L1.Package.IsValid() && f.L1.Package != "" {
+		return nil, errors.New("invalid package: %q", f.L1.Package)
+	}
 	if opts := fd.GetOptions(); opts != nil {
 		opts = clone(opts).(*descriptorpb.FileOptions)
 		f.L2.Options = func() protoreflect.ProtoMessage { return opts }
@@ -94,13 +103,13 @@ func newFile(fd *descriptorpb.FileDescriptorProto, r Resolver, opts ...option) (
 
 	f.L2.Imports = make(filedesc.FileImports, len(fd.GetDependency()))
 	for _, i := range fd.GetPublicDependency() {
-		if int(i) >= len(f.L2.Imports) || f.L2.Imports[i].IsPublic {
+		if !(0 <= i && int(i) < len(f.L2.Imports)) || f.L2.Imports[i].IsPublic {
 			return nil, errors.New("invalid or duplicate public import index: %d", i)
 		}
 		f.L2.Imports[i].IsPublic = true
 	}
 	for _, i := range fd.GetWeakDependency() {
-		if int(i) >= len(f.L2.Imports) || f.L2.Imports[i].IsWeak {
+		if !(0 <= i && int(i) < len(f.L2.Imports)) || f.L2.Imports[i].IsWeak {
 			return nil, errors.New("invalid or duplicate weak import index: %d", i)
 		}
 		f.L2.Imports[i].IsWeak = true
@@ -116,7 +125,13 @@ func newFile(fd *descriptorpb.FileDescriptorProto, r Resolver, opts ...option) (
 		}
 		imp.FileDescriptor = f
 
+		if imps[imp.Path()] {
+			return nil, errors.New("already imported %q", path)
+		}
 		imps[imp.Path()] = true
+	}
+	for i, _ := range fd.GetDependency() {
+		imp := &f.L2.Imports[i]
 		imps.importPublic(imp.Imports())
 	}
 

reflect/protodesc/desc_init.go

@@ -5,6 +5,7 @@
 package protodesc
 
 import (
+	"google.golang.org/protobuf/internal/errors"
 	"google.golang.org/protobuf/internal/filedesc"
 	"google.golang.org/protobuf/reflect/protoreflect"
 
@@ -214,7 +215,9 @@ func (r descsByName) initMethodsFromDescriptorProto(mds []*descriptorpb.MethodDe
 }
 
 func (r descsByName) makeBase(child, parent protoreflect.Descriptor, name string, idx int) (filedesc.BaseL0, error) {
-	// TODO: Verify that the name is valid?
+	if !protoreflect.Name(name).IsValid() {
+		return filedesc.BaseL0{}, errors.New("descriptor %q has an invalid nested name: %q", parent.FullName(), name)
+	}
 
 	// Derive the full name of the child.
 	// Note that enum values are a sibling to the enum parent in the namespace.
@@ -224,7 +227,9 @@ func (r descsByName) makeBase(child, parent protoreflect.Descriptor, name string
 	} else {
 		fullName = parent.FullName().Append(protoreflect.Name(name))
 	}
-	// TODO: Verify that this descriptor is not already declared?
+	if _, ok := r[fullName]; ok {
+		return filedesc.BaseL0{}, errors.New("descriptor %q already declared", fullName)
+	}
 	r[fullName] = child
 
 	// TODO: Verify that the full name does not already exist in the resolver?

reflect/protodesc/desc_resolve.go

@@ -276,7 +276,11 @@ func unmarshalDefault(s string, fd protoreflect.FieldDescriptor, allowUnresolvab
 	} else if err != nil {
 		return v, ev, err
 	}
-	// TODO: Validate that the default is not specified under proto3.
-	// TODO: Validate that the default is not specified for composite types.
+	if fd.Syntax() == protoreflect.Proto3 {
+		return v, ev, errors.New("cannot be specified under proto3 semantics")
+	}
+	if fd.Kind() == protoreflect.MessageKind || fd.Kind() == protoreflect.GroupKind || fd.Cardinality() == protoreflect.Repeated {
+		return v, ev, errors.New("cannot be specified on composite types")
+	}
 	return v, ev, nil
 }

reflect/protodesc/desc_validate.go

@@ -5,59 +5,72 @@
 package protodesc
 
 import (
+	"strings"
+	"unicode"
+
+	"google.golang.org/protobuf/internal/encoding/wire"
 	"google.golang.org/protobuf/internal/errors"
 	"google.golang.org/protobuf/internal/filedesc"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	"google.golang.org/protobuf/types/descriptorpb"
 )
 
-// TODO: Should we be responsible for validating other parts of the descriptor
-// that we don't directly use?
-//
-// For example:
-//	* That "json_name" is not set for an extension field. Maybe, maybe not.
-//	* 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).
-
-// 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.
-//	* Options are of the correct Go type (e.g. *descriptorpb.MessageOptions).
-// 	* len(ExtensionRangeOptions) <= len(ExtensionRanges)
-
 func validateEnumDeclarations(es []filedesc.Enum, eds []*descriptorpb.EnumDescriptorProto) error {
 	for i, ed := range eds {
 		e := &es[i]
-		for j, _ := range ed.GetValue() {
+		if err := e.L2.ReservedNames.CheckValid(); err != nil {
+			return errors.New("enum %q reserved names has %v", e.FullName(), err)
+		}
+		if err := e.L2.ReservedRanges.CheckValid(); err != nil {
+			return errors.New("enum %q reserved ranges has %v", e.FullName(), err)
+		}
+		if len(ed.GetValue()) == 0 {
+			return errors.New("enum %q must contain at least one value declaration", e.FullName())
+		}
+		allowAlias := ed.GetOptions().GetAllowAlias()
+		foundAlias := false
+		for i := 0; i < e.Values().Len(); i++ {
+			v1 := e.Values().Get(i)
+			if v2 := e.Values().ByNumber(v1.Number()); v1 != v2 {
+				foundAlias = true
+				if !allowAlias {
+					return errors.New("enum %q has conflicting non-aliased values on number %d: %q with %q", e.FullName(), v1.Number(), v1.Name(), v2.Name())
+				}
+			}
+		}
+		if allowAlias && !foundAlias {
+			return errors.New("enum %q allows aliases, but none were found", e.FullName())
+		}
+		if e.Syntax() == protoreflect.Proto3 {
+			if v := e.Values().Get(0); v.Number() != 0 {
+				return errors.New("enum %q using proto3 semantics must have zero number for the first value", v.FullName())
+			}
+			// Verify that value names in proto3 do not conflict if the
+			// case-insensitive prefix is removed.
+			// See protoc v3.8.0: src/google/protobuf/descriptor.cc:4991-5055
+			names := map[string]protoreflect.EnumValueDescriptor{}
+			prefix := strings.Replace(strings.ToLower(string(e.Name())), "_", "", -1)
+			for i := 0; i < e.Values().Len(); i++ {
+				v1 := e.Values().Get(i)
+				s := enumValueName(trimEnumPrefix(v1.Name(), prefix))
+				if v2, ok := names[s]; ok && v1.Number() != v2.Number() {
+					return errors.New("enum %q using proto3 semantics has conflict: %q with %q", e.FullName(), v1.Name(), v2.Name())
+				}
+				names[s] = v1
+			}
+		}
+
+		for j, vd := range ed.GetValue() {
 			v := &e.L2.Values.List[j]
+			if vd.Number == nil {
+				return errors.New("enum value %q must have a specified number", v.FullName())
+			}
 			if e.L2.ReservedNames.Has(v.Name()) {
-				return errors.New("enum %v contains value with reserved name %q", e.Name(), v.Name())
+				return errors.New("enum value %q must not use reserved name", v.FullName())
 			}
 			if e.L2.ReservedRanges.Has(v.Number()) {
-				return errors.New("enum %v contains value with reserved number %d", e.Name(), v.Number())
+				return errors.New("enum value %q must not use reserved number %d", v.FullName(), v.Number())
 			}
 		}
 	}
@@ -67,19 +80,105 @@ func validateEnumDeclarations(es []filedesc.Enum, eds []*descriptorpb.EnumDescri
 func validateMessageDeclarations(ms []filedesc.Message, mds []*descriptorpb.DescriptorProto) error {
 	for i, md := range mds {
 		m := &ms[i]
+
+		// Handle the message descriptor itself.
+		isMessageSet := md.GetOptions().GetMessageSetWireFormat()
+		if err := m.L2.ReservedNames.CheckValid(); err != nil {
+			return errors.New("message %q reserved names has %v", m.FullName(), err)
+		}
+		if err := m.L2.ReservedRanges.CheckValid(isMessageSet); err != nil {
+			return errors.New("message %q reserved ranges has %v", m.FullName(), err)
+		}
+		if err := m.L2.ExtensionRanges.CheckValid(isMessageSet); err != nil {
+			return errors.New("message %q extension ranges has %v", m.FullName(), err)
+		}
+		if err := (*filedesc.FieldRanges).CheckOverlap(&m.L2.ReservedRanges, &m.L2.ExtensionRanges); err != nil {
+			return errors.New("message %q reserved and extension ranges has %v", m.FullName(), err)
+		}
+		for i := 0; i < m.Fields().Len(); i++ {
+			f1 := m.Fields().Get(i)
+			if f2 := m.Fields().ByNumber(f1.Number()); f1 != f2 {
+				return errors.New("message %q has conflicting fields: %q with %q", m.FullName(), f1.Name(), f2.Name())
+			}
+		}
+		if isMessageSet && (m.Syntax() != protoreflect.Proto2 || m.Fields().Len() > 0 || m.ExtensionRanges().Len() == 0) {
+			return errors.New("message %q is an invalid proto1 MessageSet", m.FullName())
+		}
+		if m.Syntax() == protoreflect.Proto3 {
+			if m.ExtensionRanges().Len() > 0 {
+				return errors.New("message %q using proto3 semantics cannot have extension ranges", m.FullName())
+			}
+			// Verify that field names in proto3 do not conflict if lowercased
+			// with all underscores removed.
+			// See protoc v3.8.0: src/google/protobuf/descriptor.cc:5830-5847
+			names := map[string]protoreflect.FieldDescriptor{}
+			for i := 0; i < m.Fields().Len(); i++ {
+				f1 := m.Fields().Get(i)
+				s := strings.Replace(strings.ToLower(string(f1.Name())), "_", "", -1)
+				if f2, ok := names[s]; ok {
+					return errors.New("message %q using proto3 semantics has conflict: %q with %q", m.FullName(), f1.Name(), f2.Name())
+				}
+				names[s] = f1
+			}
+		}
+
 		for j, fd := range md.GetField() {
 			f := &m.L2.Fields.List[j]
 			if m.L2.ReservedNames.Has(f.Name()) {
-				return errors.New("%v contains field with reserved name %q", m.Name(), f.Name())
+				return errors.New("message field %q must not use reserved name", f.FullName())
+			}
+			if !f.Number().IsValid() {
+				return errors.New("message field %q has an invalid number: %d", f.FullName(), f.Number())
+			}
+			if !f.Cardinality().IsValid() {
+				return errors.New("message field %q has an invalid cardinality: %d", f.FullName(), f.Cardinality())
 			}
 			if m.L2.ReservedRanges.Has(f.Number()) {
-				return errors.New("%v contains field with reserved number %d", m.Name(), f.Number())
+				return errors.New("message field %q must not use reserved number %d", f.FullName(), f.Number())
 			}
 			if m.L2.ExtensionRanges.Has(f.Number()) {
-				return errors.New("%v contains field with number %d in extension range", m.Name(), f.Number())
+				return errors.New("message field %q with number %d in extension range", f.FullName(), f.Number())
+			}
+			if fd.Extendee != nil {
+				return errors.New("message field %q may not have extendee: %q", f.FullName(), fd.GetExtendee())
 			}
-			if fd.GetExtendee() != "" {
-				return errors.New("message field may not have extendee")
+			if f.IsWeak() && (!isOptionalMessage(f) || f.ContainingOneof() != nil) {
+				return errors.New("message field %q may only be weak for an optional message", f.FullName())
+			}
+			if f.IsPacked() && !isPackable(f) {
+				return errors.New("message field %q is not packable", f.FullName())
+			}
+			if err := checkValidGroup(f); err != nil {
+				return errors.New("message field %q is an invalid group: %v", f.FullName(), err)
+			}
+			if err := checkValidMap(f); err != nil {
+				return errors.New("message field %q is an invalid map: %v", f.FullName(), err)
+			}
+			if f.Syntax() == protoreflect.Proto3 {
+				if f.Cardinality() == protoreflect.Required {
+					return errors.New("message field %q using proto3 semantics cannot be required", f.FullName())
+				}
+				if f.Enum() != nil && f.Enum().Syntax() != protoreflect.Proto3 {
+					return errors.New("message field %q using proto3 semantics may only depend on a proto3 enum", f.FullName())
+				}
+			}
+		}
+		for j, _ := range md.GetOneofDecl() {
+			o := &m.L2.Oneofs.List[j]
+			if o.Fields().Len() == 0 {
+				return errors.New("message oneof %q must contain at least one field declaration", o.FullName())
+			}
+			if n := o.Fields().Len(); n-1 != (o.Fields().Get(n-1).Index() - o.Fields().Get(0).Index()) {
+				return errors.New("message oneof %q must have consecutively declared fields", o.FullName())
+			}
+			for i := 0; i < o.Fields().Len(); i++ {
+				f := o.Fields().Get(i)
+				if f.Cardinality() != protoreflect.Optional {
+					return errors.New("message field %q belongs in a oneof and must be optional", f.FullName())
+				}
+				if f.IsWeak() {
+					return errors.New("message field %q belongs in a oneof and must not be a weak reference", f.FullName())
+				}
 			}
 		}
 
@@ -99,10 +198,228 @@ func validateMessageDeclarations(ms []filedesc.Message, mds []*descriptorpb.Desc
 func validateExtensionDeclarations(xs []filedesc.Extension, xds []*descriptorpb.FieldDescriptorProto) error {
 	for i, xd := range xds {
 		x := &xs[i]
+		// NOTE: Avoid using the IsValid method since extensions to MessageSet
+		// may have a field number higher than normal. This check only verifies
+		// that the number is not negative or reserved. We check again later
+		// if we know that the extendee is definitely not a MessageSet.
+		if n := x.Number(); n < 0 || (wire.FirstReservedNumber <= n && n <= wire.LastReservedNumber) {
+			return errors.New("extension field %q has an invalid number: %d", x.FullName(), x.Number())
+		}
+		if !x.Cardinality().IsValid() || x.Cardinality() == protoreflect.Required {
+			return errors.New("extension field %q has an invalid cardinality: %d", x.FullName(), x.Cardinality())
+		}
+		if xd.JsonName != nil {
+			if xd.GetJsonName() != jsonName(x.Name()) {
+				return errors.New("extension field %q may not have an explicitly set JSON name: %q", x.FullName(), xd.GetJsonName())
+			}
+		}
 		if xd.OneofIndex != nil {
-			return errors.New("extension may not have oneof_index")
+			return errors.New("extension field %q may not be part of a oneof", x.FullName())
+		}
+		if md := x.ContainingMessage(); !md.IsPlaceholder() {
+			if !md.ExtensionRanges().Has(x.Number()) {
+				return errors.New("extension field %q extends %q with non-extension field number: %d", x.FullName(), md.FullName(), x.Number())
+			}
+			isMessageSet := md.Options().(*descriptorpb.MessageOptions).GetMessageSetWireFormat()
+			if isMessageSet && !isOptionalMessage(x) {
+				return errors.New("extension field %q extends MessageSet and must be an optional message", x.FullName())
+			}
+			if !isMessageSet && !x.Number().IsValid() {
+				return errors.New("extension field %q has an invalid number: %d", x.FullName(), x.Number())
+			}
+		}
+		if xd.GetOptions().GetWeak() {
+			return errors.New("extension field %q cannot be a weak reference", x.FullName())
+		}
+		if x.IsPacked() && !isPackable(x) {
+			return errors.New("extension field %q is not packable", x.FullName())
 		}
-		_ = x
+		if err := checkValidGroup(x); err != nil {
+			return errors.New("extension field %q is an invalid group: %v", x.FullName(), err)
+		}
+		if md := x.Message(); md != nil && md.IsMapEntry() {
+			return errors.New("extension field %q cannot be a map entry", x.FullName())
+		}
+		if x.Syntax() == protoreflect.Proto3 {
+			switch x.ContainingMessage().FullName() {
+			case (*descriptorpb.FileOptions)(nil).ProtoReflect().Descriptor().FullName():
+			case (*descriptorpb.EnumOptions)(nil).ProtoReflect().Descriptor().FullName():
+			case (*descriptorpb.EnumValueOptions)(nil).ProtoReflect().Descriptor().FullName():
+			case (*descriptorpb.MessageOptions)(nil).ProtoReflect().Descriptor().FullName():
+			case (*descriptorpb.FieldOptions)(nil).ProtoReflect().Descriptor().FullName():
+			case (*descriptorpb.OneofOptions)(nil).ProtoReflect().Descriptor().FullName():
+			case (*descriptorpb.ExtensionRangeOptions)(nil).ProtoReflect().Descriptor().FullName():
+			case (*descriptorpb.ServiceOptions)(nil).ProtoReflect().Descriptor().FullName():
+			case (*descriptorpb.MethodOptions)(nil).ProtoReflect().Descriptor().FullName():
+			default:
+				return errors.New("extension field %q cannot be declared in proto3 unless extended descriptor options", x.FullName())
+			}
+		}
+	}
+	return nil
+}
+
+// isOptionalMessage reports whether this is an optional message.
+// If the kind is unknown, it is assumed to be a message.
+func isOptionalMessage(fd protoreflect.FieldDescriptor) bool {
+	return (fd.Kind() == 0 || fd.Kind() == protoreflect.MessageKind) && fd.Cardinality() == protoreflect.Optional
+}
+
+// isPackable checks whether the pack option can be specified.
+func isPackable(fd protoreflect.FieldDescriptor) bool {
+	switch fd.Kind() {
+	case protoreflect.StringKind, protoreflect.BytesKind, protoreflect.MessageKind, protoreflect.GroupKind:
+		return false
+	}
+	return fd.IsList()
+}
+
+// checkValidGroup reports whether fd is a valid group according to the same
+// rules that protoc imposes.
+func checkValidGroup(fd protoreflect.FieldDescriptor) error {
+	md := fd.Message()
+	switch {
+	case fd.Kind() != protoreflect.GroupKind:
+		return nil
+	case fd.Syntax() != protoreflect.Proto2:
+		return errors.New("invalid under proto2 semantics")
+	case md == nil || md.IsPlaceholder():
+		return errors.New("message must be resolvable")
+	case fd.FullName().Parent() != md.FullName().Parent():
+		return errors.New("message and field must be declared in the same scope")
+	case !unicode.IsUpper(rune(md.Name()[0])):
+		return errors.New("message name must start with an uppercase")
+	case fd.Name() != protoreflect.Name(strings.ToLower(string(md.Name()))):
+		return errors.New("field name must be lowercased form of the message name")
 	}
 	return nil
 }
+
+// checkValidMap checks whether the field is a valid map according to the same
+// rules that protoc imposes.
+// See protoc v3.8.0: src/google/protobuf/descriptor.cc:6045-6115
+func checkValidMap(fd protoreflect.FieldDescriptor) error {
+	md := fd.Message()
+	switch {
+	case md == nil || !md.IsMapEntry():
+		return nil
+	case fd.FullName().Parent() != md.FullName().Parent():
+		return errors.New("message and field must be declared in the same scope")
+	case md.Name() != mapEntryName(fd.Name()):
+		return errors.New("incorrect implicit map entry name")
+	case fd.Cardinality() != protoreflect.Repeated:
+		return errors.New("field must be repeated")
+	case md.Fields().Len() != 2:
+		return errors.New("message must have exactly two fields")
+	case md.ExtensionRanges().Len() > 0:
+		return errors.New("message must not have any extension ranges")
+	case md.Enums().Len()+md.Messages().Len()+md.Extensions().Len() > 0:
+		return errors.New("message must not have any nested declarations")
+	}
+	kf := md.Fields().Get(0)
+	vf := md.Fields().Get(1)
+	switch {
+	case kf.Name() != "key" || kf.Number() != 1 || kf.Cardinality() != protoreflect.Optional || kf.ContainingOneof() != nil || kf.HasDefault():
+		return errors.New("invalid key field")
+	case vf.Name() != "value" || vf.Number() != 2 || vf.Cardinality() != protoreflect.Optional || vf.ContainingOneof() != nil || vf.HasDefault():
+		return errors.New("invalid value field")
+	}
+	switch kf.Kind() {
+	case protoreflect.BoolKind: // bool
+	case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: // int32
+	case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: // int64
+	case protoreflect.Uint32Kind, protoreflect.Fixed32Kind: // uint32
+	case protoreflect.Uint64Kind, protoreflect.Fixed64Kind: // uint64
+	case protoreflect.StringKind: // string
+	default:
+		return errors.New("invalid key kind: %v", kf.Kind())
+	}
+	if e := vf.Enum(); e != nil && e.Values().Len() > 0 && e.Values().Get(0).Number() != 0 {
+		return errors.New("map enum value must have zero number for the first value")
+	}
+	return nil
+}
+
+// mapEntryName derives the name of the map entry message given the field name.
+// See protoc v3.8.0: src/google/protobuf/descriptor.cc:254-276,6057
+func mapEntryName(s protoreflect.Name) protoreflect.Name {
+	var b []byte
+	upperNext := true
+	for _, c := range s {
+		switch {
+		case c == '_':
+			upperNext = true
+		case upperNext:
+			b = append(b, byte(unicode.ToUpper(c)))
+			upperNext = false
+		default:
+			b = append(b, byte(c))
+		}
+	}
+	b = append(b, "Entry"...)
+	return protoreflect.Name(b)
+}
+
+// enumValueName derives the camel-cased enum value name.
+// See protoc v3.8.0: src/google/protobuf/descriptor.cc:297-313
+func enumValueName(s protoreflect.Name) string {
+	var b []byte
+	upperNext := true
+	for _, c := range s {
+		switch {
+		case c == '_':
+			upperNext = true
+		case upperNext:
+			b = append(b, byte(unicode.ToUpper(c)))
+			upperNext = false
+		default:
+			b = append(b, byte(unicode.ToLower(c)))
+			upperNext = false
+		}
+	}
+	return string(b)
+}
+
+// trimEnumPrefix trims the enum name prefix from an enum value name,
+// where the prefix is all lowercase without underscores.
+// See protoc v3.8.0: src/google/protobuf/descriptor.cc:330-375
+func trimEnumPrefix(s protoreflect.Name, prefix string) protoreflect.Name {
+	s0 := s // original input
+	for len(s) > 0 && len(prefix) > 0 {
+		if s[0] == '_' {
+			s = s[1:]
+			continue
+		}
+		if unicode.ToLower(rune(s[0])) != rune(prefix[0]) {
+			return s0 // no prefix match
+		}
+		s, prefix = s[1:], prefix[1:]
+	}
+	if len(prefix) > 0 {
+		return s0 // no prefix match
+	}
+	s = protoreflect.Name(strings.TrimLeft(string(s), "_"))
+	if len(s) == 0 {
+		return s0 // avoid returning empty string
+	}
+	return s
+}
+
+// jsonName creates a JSON name from the protobuf short name.
+// See protoc v3.8.0: src/google/protobuf/descriptor.cc:278-295
+func jsonName(s protoreflect.Name) string {
+	var b []byte
+	var wasUnderscore bool
+	for i := 0; i < len(s); i++ { // proto identifiers are always ASCII
+		c := s[i]
+		if c != '_' {
+			isLower := 'a' <= c && c <= 'z'
+			if wasUnderscore && isLower {
+				c -= 'a' - 'A'
+			}
+			b = append(b, c)
+		}
+		wasUnderscore = c == '_'
+	}
+	return string(b)
+}

reflect/protodesc/file_test.go

@@ -11,6 +11,7 @@ import (
 
 	"google.golang.org/protobuf/encoding/prototext"
 	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/reflect/protoregistry"
 
 	"google.golang.org/protobuf/types/descriptorpb"
@@ -30,6 +31,60 @@ func cloneFile(in *descriptorpb.FileDescriptorProto) *descriptorpb.FileDescripto
 	return out
 }
 
+var (
+	proto2Enum = mustParseFile(`
+		syntax:    "proto2"
+		name:      "proto2_enum.proto"
+		package:   "test.proto2"
+		enum_type: [{name:"Enum" value:[{name:"ONE" number:1}]}]
+	`)
+	proto3Message = mustParseFile(`
+		syntax:    "proto3"
+		name:      "proto3_message.proto"
+		package:   "test.proto3"
+		message_type: [{
+			name:  "Message"
+			field: [
+				{name:"foo" number:1 label:LABEL_OPTIONAL type:TYPE_STRING},
+				{name:"bar" number:2 label:LABEL_OPTIONAL type:TYPE_STRING}
+			]
+		}]
+	`)
+	extendableMessage = mustParseFile(`
+		syntax:       "proto2"
+		name:         "extendable_message.proto"
+		package:      "test.proto2"
+		message_type: [{name:"Message" extension_range:[{start:1 end:1000}]}]
+	`)
+	importPublicFile1 = mustParseFile(`
+		syntax:            "proto3"
+		name:              "import_public1.proto"
+		dependency:        ["proto2_enum.proto", "proto3_message.proto", "extendable_message.proto"]
+		message_type:      [{name:"Public1"}]
+	`)
+	importPublicFile2 = mustParseFile(`
+		syntax:            "proto3"
+		name:              "import_public2.proto"
+		dependency:        ["import_public1.proto"]
+		public_dependency: [0]
+		message_type:      [{name:"Public2"}]
+	`)
+	importPublicFile3 = mustParseFile(`
+		syntax:            "proto3"
+		name:              "import_public3.proto"
+		dependency:        ["import_public2.proto", "extendable_message.proto"]
+		public_dependency: [0]
+		message_type:      [{name:"Public3"}]
+	`)
+	importPublicFile4 = mustParseFile(`
+		syntax:            "proto3"
+		name:              "import_public4.proto"
+		dependency:        ["import_public2.proto", "import_public3.proto", "proto2_enum.proto"]
+		public_dependency: [0, 1]
+		message_type:      [{name:"Public4"}]
+	`)
+)
+
 func TestNewFile(t *testing.T) {
 	tests := []struct {
 		label    string
@@ -39,6 +94,94 @@ func TestNewFile(t *testing.T) {
 		wantDesc *descriptorpb.FileDescriptorProto
 		wantErr  string
 	}{{
+		label:   "empty path",
+		inDesc:  mustParseFile(``),
+		wantErr: `path must be populated`,
+	}, {
+		label:  "empty package and syntax",
+		inDesc: mustParseFile(`name:"weird" package:""`),
+	}, {
+		label:   "invalid syntax",
+		inDesc:  mustParseFile(`name:"weird" syntax:"proto9"`),
+		wantErr: `invalid syntax: "proto9"`,
+	}, {
+		label:   "bad package",
+		inDesc:  mustParseFile(`name:"weird" package:"$"`),
+		wantErr: `invalid package: "$"`,
+	}, {
+		label: "unresolvable import",
+		inDesc: mustParseFile(`
+			name:       "test.proto"
+			package:    ""
+			dependency: "dep.proto"
+		`),
+		wantErr: `could not resolve import "dep.proto": not found`,
+	}, {
+		label: "unresolvable import but allowed",
+		inDesc: mustParseFile(`
+			name:       "test.proto"
+			package:    ""
+			dependency: "dep.proto"
+		`),
+		inOpts: []option{allowUnresolvable()},
+	}, {
+		label: "duplicate import",
+		inDesc: mustParseFile(`
+			name:       "test.proto"
+			package:    ""
+			dependency: ["dep.proto", "dep.proto"]
+		`),
+		inOpts:  []option{allowUnresolvable()},
+		wantErr: `already imported "dep.proto"`,
+	}, {
+		label: "invalid weak import",
+		inDesc: mustParseFile(`
+			name:            "test.proto"
+			package:         ""
+			dependency:      "dep.proto"
+			weak_dependency: [-23]
+		`),
+		inOpts:  []option{allowUnresolvable()},
+		wantErr: `invalid or duplicate weak import index: -23`,
+	}, {
+		label: "normal weak and public import",
+		inDesc: mustParseFile(`
+			name:              "test.proto"
+			package:           ""
+			dependency:        "dep.proto"
+			weak_dependency:   [0]
+			public_dependency: [0]
+		`),
+		inOpts: []option{allowUnresolvable()},
+	}, {
+		label: "import public indirect dependency duplicate",
+		inDeps: []*descriptorpb.FileDescriptorProto{
+			mustParseFile(`name:"leaf.proto"`),
+			mustParseFile(`name:"public.proto" dependency:"leaf.proto" public_dependency:0`),
+		},
+		inDesc: mustParseFile(`
+			name: "test.proto"
+			package: ""
+			dependency: ["public.proto", "leaf.proto"]
+		`),
+	}, {
+		label: "import public graph",
+		inDeps: []*descriptorpb.FileDescriptorProto{
+			cloneFile(proto2Enum),
+			cloneFile(proto3Message),
+			cloneFile(extendableMessage),
+			cloneFile(importPublicFile1),
+			cloneFile(importPublicFile2),
+			cloneFile(importPublicFile3),
+			cloneFile(importPublicFile4),
+		},
+		inDesc: mustParseFile(`
+			name:       "test.proto"
+			package:    "test.graph"
+			dependency: ["import_public4.proto"],
+		`),
+		// TODO: Test import public
+	}, {
 		label: "resolve relative reference",
 		inDesc: mustParseFile(`
 			name: "test.proto"
@@ -216,6 +359,499 @@ func TestNewFile(t *testing.T) {
 				field: [{name:"F" number:1 label:LABEL_OPTIONAL type:TYPE_MESSAGE type_name:".fizz.M.M"}]
 			}]
 		`),
+	}, {
+		label: "namespace conflict on enum value",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			enum_type: [{
+				name: "foo"
+				value: [{name:"foo" number:0}]
+			}]
+		`),
+		wantErr: `descriptor "foo" already declared`,
+	}, {
+		label: "no namespace conflict on message field",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{
+				name: "foo"
+				field: [{name:"foo" number:1 label:LABEL_OPTIONAL type:TYPE_STRING}]
+			}]
+		`),
+	}, {
+		label: "invalid name",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name: "$"}]
+		`),
+		wantErr: `descriptor "" has an invalid nested name: "$"`,
+	}, {
+		label: "invalid empty enum",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{name:"E"}]}]
+		`),
+		wantErr: `enum "M.E" must contain at least one value declaration`,
+	}, {
+		label: "invalid enum value without number",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{name:"E" value:[{name:"one"}]}]}]
+		`),
+		wantErr: `enum value "M.one" must have a specified number`,
+	}, {
+		label: "valid enum",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{name:"E" value:[{name:"one" number:1}]}]}]
+		`),
+	}, {
+		label: "invalid enum reserved names",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:          "E"
+				reserved_name: [""]
+				value: [{name:"V" number:0}]
+			}]}]
+		`),
+		// NOTE: In theory this should be an error.
+		// See https://github.com/protocolbuffers/protobuf/issues/6335.
+		/*wantErr: `enum "M.E" reserved names has invalid name: ""`,*/
+	}, {
+		label: "duplicate enum reserved names",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:          "E"
+				reserved_name: ["foo", "foo"]
+			}]}]
+		`),
+		wantErr: `enum "M.E" reserved names has duplicate name: "foo"`,
+	}, {
+		label: "valid enum reserved names",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:          "E"
+				reserved_name: ["foo", "bar"]
+				value:         [{name:"baz" number:1}]
+			}]}]
+		`),
+	}, {
+		label: "use of enum reserved names",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:          "E"
+				reserved_name: ["foo", "bar"]
+				value:         [{name:"foo" number:1}]
+			}]}]
+		`),
+		wantErr: `enum value "M.foo" must not use reserved name`,
+	}, {
+		label: "invalid enum reserved ranges",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:           "E"
+				reserved_range: [{start:5 end:4}]
+			}]}]
+		`),
+		wantErr: `enum "M.E" reserved ranges has invalid range: 5 to 4`,
+	}, {
+		label: "overlapping enum reserved ranges",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:           "E"
+				reserved_range: [{start:1 end:1000}, {start:10 end:100}]
+			}]}]
+		`),
+		wantErr: `enum "M.E" reserved ranges has overlapping ranges: 1 to 1000 with 10 to 100`,
+	}, {
+		label: "valid enum reserved names",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:           "E"
+				reserved_range: [{start:1 end:10}, {start:100 end:1000}]
+				value:          [{name:"baz" number:50}]
+			}]}]
+		`),
+	}, {
+		label: "use of enum reserved range",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:           "E"
+				reserved_range: [{start:1 end:10}, {start:100 end:1000}]
+				value:          [{name:"baz" number:500}]
+			}]}]
+		`),
+		wantErr: `enum value "M.baz" must not use reserved number 500`,
+	}, {
+		label: "unused enum alias feature",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:    "E"
+				value:   [{name:"baz" number:500}]
+				options: {allow_alias:true}
+			}]}]
+		`),
+		wantErr: `enum "M.E" allows aliases, but none were found`,
+	}, {
+		label: "enum number conflicts",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:  "E"
+				value: [{name:"foo" number:0}, {name:"bar" number:1}, {name:"baz" number:1}]
+			}]}]
+		`),
+		wantErr: `enum "M.E" has conflicting non-aliased values on number 1: "baz" with "bar"`,
+	}, {
+		label: "aliased enum numbers",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:    "E"
+				value:   [{name:"foo" number:0}, {name:"bar" number:1}, {name:"baz" number:1}]
+				options: {allow_alias:true}
+			}]}]
+		`),
+	}, {
+		label: "invalid proto3 enum",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:  "E"
+				value: [{name:"baz" number:500}]
+			}]}]
+		`),
+		wantErr: `enum "M.baz" using proto3 semantics must have zero number for the first value`,
+	}, {
+		label: "valid proto3 enum",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:  "E"
+				value: [{name:"baz" number:0}]
+			}]}]
+		`),
+	}, {
+		label: "proto3 enum name prefix conflict",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:  "E"
+				value: [{name:"e_Foo" number:0}, {name:"fOo" number:1}]
+			}]}]
+		`),
+		wantErr: `enum "M.E" using proto3 semantics has conflict: "fOo" with "e_Foo"`,
+	}, {
+		label: "proto2 enum has name prefix check",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:  "E"
+				value: [{name:"e_Foo" number:0}, {name:"fOo" number:1}]
+			}]}]
+		`),
+	}, {
+		label: "proto3 enum same name prefix with number conflict",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:  "E"
+				value: [{name:"e_Foo" number:0}, {name:"fOo" number:0}]
+			}]}]
+		`),
+		wantErr: `enum "M.E" has conflicting non-aliased values on number 0: "fOo" with "e_Foo"`,
+	}, {
+		label: "proto3 enum same name prefix with alias numbers",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" enum_type:[{
+				name:    "E"
+				value:   [{name:"e_Foo" number:0}, {name:"fOo" number:0}]
+				options: {allow_alias: true}
+			}]}]
+		`),
+	}, {
+		label: "invalid message reserved names",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:          "M"
+				reserved_name: ["$"]
+			}]}]
+		`),
+		// NOTE: In theory this should be an error.
+		// See https://github.com/protocolbuffers/protobuf/issues/6335.
+		/*wantErr: `message "M.M" reserved names has invalid name: "$"`,*/
+	}, {
+		label: "valid message reserved names",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:          "M"
+				reserved_name: ["foo", "bar"]
+				field:         [{name:"foo" number:1 label:LABEL_OPTIONAL type:TYPE_STRING}]
+			}]}]
+		`),
+		wantErr: `message field "M.M.foo" must not use reserved name`,
+	}, {
+		label: "valid message reserved names",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:          "M"
+				reserved_name: ["foo", "bar"]
+				field:         [{name:"baz" number:1 label:LABEL_OPTIONAL type:TYPE_STRING oneof_index:0}]
+				oneof_decl:    [{name:"foo"}] # not affected by reserved_name
+			}]}]
+		`),
+	}, {
+		label: "invalid reserved number",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:           "M"
+				reserved_range: [{start:1 end:1}]
+				field:          [{name:"baz" number:1 label:LABEL_OPTIONAL type:TYPE_STRING}]
+			}]}]
+		`),
+		wantErr: `message "M.M" reserved ranges has invalid field number: 0`,
+	}, {
+		label: "invalid reserved ranges",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:           "M"
+				reserved_range: [{start:2 end:2}]
+				field:          [{name:"baz" number:1 label:LABEL_OPTIONAL type:TYPE_STRING}]
+			}]}]
+		`),
+		wantErr: `message "M.M" reserved ranges has invalid range: 2 to 1`,
+	}, {
+		label: "overlapping reserved ranges",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:           "M"
+				reserved_range: [{start:1 end:10}, {start:2 end:9}]
+				field:          [{name:"baz" number:1 label:LABEL_OPTIONAL type:TYPE_STRING}]
+			}]}]
+		`),
+		wantErr: `message "M.M" reserved ranges has overlapping ranges: 1 to 9 with 2 to 8`,
+	}, {
+		label: "use of reserved message field number",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:           "M"
+				reserved_range: [{start:10 end:20}, {start:20 end:30}, {start:30 end:31}]
+				field:          [{name:"baz" number:30 label:LABEL_OPTIONAL type:TYPE_STRING}]
+			}]}]
+		`),
+		wantErr: `message field "M.M.baz" must not use reserved number 30`,
+	}, {
+		label: "invalid extension ranges",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:            "M"
+				extension_range: [{start:-500 end:2}]
+				field:           [{name:"baz" number:1 label:LABEL_OPTIONAL type:TYPE_STRING}]
+			}]}]
+		`),
+		wantErr: `message "M.M" extension ranges has invalid field number: -500`,
+	}, {
+		label: "overlapping reserved and extension ranges",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:            "M"
+				reserved_range:  [{start:15 end:20}, {start:1 end:3}, {start:7 end:10}]
+				extension_range: [{start:8 end:9}, {start:3 end:5}]
+			}]}]
+		`),
+		wantErr: `message "M.M" reserved and extension ranges has overlapping ranges: 7 to 9 with 8`,
+	}, {
+		label: "message field conflicting number",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:            "M"
+				field: [
+					{name:"one" number:1 label:LABEL_OPTIONAL type:TYPE_STRING},
+					{name:"One" number:1 label:LABEL_OPTIONAL type:TYPE_STRING}
+				]
+			}]}]
+		`),
+		wantErr: `message "M.M" has conflicting fields: "One" with "one"`,
+	}, {
+		label: "invalid MessageSet",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:    "M"
+				options: {message_set_wire_format:true}
+			}]}]
+		`),
+		wantErr: `message "M.M" is an invalid proto1 MessageSet`,
+	}, {
+		label: "valid MessageSet",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:            "M"
+				extension_range: [{start:1 end:100000}]
+				options:         {message_set_wire_format:true}
+			}]}]
+		`),
+	}, {
+		label: "invalid extension ranges in proto3",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:            "M"
+				extension_range: [{start:1 end:100000}]
+			}]}]
+		`),
+		wantErr: `message "M.M" using proto3 semantics cannot have extension ranges`,
+	}, {
+		label: "proto3 message fields conflict",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name: "M"
+				field: [
+					{name:"_b_a_z_" number:1 label:LABEL_OPTIONAL type:TYPE_STRING},
+					{name:"baz" number:2 label:LABEL_OPTIONAL type:TYPE_STRING}
+				]
+			}]}]
+		`),
+		wantErr: `proto: message "M.M" using proto3 semantics has conflict: "baz" with "_b_a_z_"`,
+	}, {
+		label: "proto3 message fields",
+		inDesc: mustParseFile(`
+			syntax:  "proto3"
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name:       "M"
+				field:      [{name:"_b_a_z_" number:1 label:LABEL_OPTIONAL type:TYPE_STRING oneof_index:0}]
+				oneof_decl: [{name:"baz"}] # proto3 name conflict logic does not include oneof
+			}]}]
+		`),
+	}, {
+		label: "proto2 message fields with no conflict",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			message_type: [{name:"M" nested_type:[{
+				name: "M"
+				field: [
+					{name:"_b_a_z_" number:1 label:LABEL_OPTIONAL type:TYPE_STRING},
+					{name:"baz" number:2 label:LABEL_OPTIONAL type:TYPE_STRING}
+				]
+			}]}]
+		`),
+		// TODO: Test field and oneof handling in validateMessageDeclarations
+		// TODO: Test unmarshalDefault
+		// TODO: Test validateExtensionDeclarations
+		// TODO: Test checkValidGroup
+		// TODO: Test checkValidMap
+	}, {
+		label: "empty service",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			service: [{name:"service"}]
+		`),
+	}, {
+		label: "service with method with unresolved",
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			service: [{
+				name: "service"
+				method: [{
+					name:"method"
+					input_type:"foo"
+					output_type:".foo.bar.baz"
+				}]
+			}]
+		`),
+		inOpts: []option{allowUnresolvable()},
+	}, {
+		label: "service with wrong reference type",
+		inDeps: []*descriptorpb.FileDescriptorProto{
+			cloneFile(proto3Message),
+			cloneFile(proto2Enum),
+		},
+		inDesc: mustParseFile(`
+			name:    "test.proto"
+			package: ""
+			dependency: ["proto2_enum.proto", "proto3_message.proto"]
+			service: [{
+				name: "service"
+				method: [{
+					name:        "method"
+					input_type:  ".test.proto2.Enum",
+					output_type: ".test.proto3.Message"
+				}]
+			}]
+		`),
+		wantErr: `service method "service.method" cannot resolve input: resolved "test.proto2.Enum", but it is not an message`,
 	}}
 
 	for _, tt := range tests {
@@ -247,3 +883,57 @@ func TestNewFile(t *testing.T) {
 		})
 	}
 }
+
+func TestName(t *testing.T) {
+	tests := []struct {
+		in            protoreflect.Name
+		enumPrefix    string
+		wantMapEntry  protoreflect.Name
+		wantEnumValue string
+		wantTrimValue protoreflect.Name
+		wantJSON      string
+	}{{
+		in:            "abc",
+		enumPrefix:    "",
+		wantMapEntry:  "AbcEntry",
+		wantEnumValue: "Abc",
+		wantTrimValue: "abc",
+		wantJSON:      "abc",
+	}, {
+		in:            "foo_baR_",
+		enumPrefix:    "foo_bar",
+		wantMapEntry:  "FooBaREntry",
+		wantEnumValue: "FooBar",
+		wantTrimValue: "foo_baR_",
+		wantJSON:      "fooBaR",
+	}, {
+		in:            "snake_caseCamelCase",
+		enumPrefix:    "snakecasecamel",
+		wantMapEntry:  "SnakeCaseCamelCaseEntry",
+		wantEnumValue: "SnakeCasecamelcase",
+		wantTrimValue: "Case",
+		wantJSON:      "snakeCaseCamelCase",
+	}, {
+		in:            "FiZz_BuZz",
+		enumPrefix:    "fizz",
+		wantMapEntry:  "FiZzBuZzEntry",
+		wantEnumValue: "FizzBuzz",
+		wantTrimValue: "BuZz",
+		wantJSON:      "FiZzBuZz",
+	}}
+
+	for _, tt := range tests {
+		if got := mapEntryName(tt.in); got != tt.wantMapEntry {
+			t.Errorf("mapEntryName(%q) = %q, want %q", tt.in, got, tt.wantMapEntry)
+		}
+		if got := enumValueName(tt.in); got != tt.wantEnumValue {
+			t.Errorf("enumValueName(%q) = %q, want %q", tt.in, got, tt.wantEnumValue)
+		}
+		if got := trimEnumPrefix(tt.in, tt.enumPrefix); got != tt.wantTrimValue {
+			t.Errorf("trimEnumPrefix(%q, %q) = %q, want %q", tt.in, tt.enumPrefix, got, tt.wantTrimValue)
+		}
+		if got := jsonName(tt.in); got != tt.wantJSON {
+			t.Errorf("jsonName(%q) = %q, want %q", tt.in, got, tt.wantJSON)
+		}
+	}
+}

reflect/protodesc/protodesc_test.go

@@ -1,636 +0,0 @@
-// Copyright 2019 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 protodesc
-
-import (
-	"strings"
-	"testing"
-
-	"google.golang.org/protobuf/internal/scalar"
-	"google.golang.org/protobuf/reflect/protoregistry"
-
-	"google.golang.org/protobuf/types/descriptorpb"
-)
-
-// Tests validation logic for malformed descriptors.
-func TestNewFile_ValidationErrors(t *testing.T) {
-	testCases := []struct {
-		name    string
-		deps    []*descriptorpb.FileDescriptorProto
-		fd      *descriptorpb.FileDescriptorProto
-		wantErr string
-	}{{
-		name: "field number reserved",
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:    scalar.String("field-number-reserved.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("BadMessage"),
-				ReservedRange: []*descriptorpb.DescriptorProto_ReservedRange{{
-					Start: scalar.Int32(3),
-					End:   scalar.Int32(4),
-				}},
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("good_field"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
-				}, {
-					Name:   scalar.String("bad_field"),
-					Number: scalar.Int32(3),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
-				}},
-			}},
-		},
-		wantErr: "reserved number 3",
-	}, {
-		name: "field name reserved",
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:    scalar.String("field-name-reserved.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name:         scalar.String("BadMessage"),
-				ReservedName: []string{"bad_field", "baz"},
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("good_field"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
-				}, {
-					Name:   scalar.String("bad_field"),
-					Number: scalar.Int32(3),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
-				}},
-			}},
-		},
-		wantErr: `reserved name "bad_field"`,
-	}, {
-		name: "normal field with extendee",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("extensible.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("ExtensibleMessage"),
-				ExtensionRange: []*descriptorpb.DescriptorProto_ExtensionRange{{
-					Start: scalar.Int32(1000),
-					End:   scalar.Int32(2000),
-				}},
-			}},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:       scalar.String("field-with-extendee.proto"),
-			Syntax:     scalar.String("proto2"),
-			Package:    scalar.String("foo"),
-			Dependency: []string{"extensible.proto"},
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("BadMessage"),
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("good_field"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
-				}, {
-					Name:     scalar.String("bad_field"),
-					Number:   scalar.Int32(3),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
-					Extendee: scalar.String(".foo.ExtensibleMessage"),
-				}},
-			}},
-		},
-		wantErr: "may not have extendee",
-	}, {
-		name: "type_name on int32 field",
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:    scalar.String("int32-with-type-name.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			EnumType: []*descriptorpb.EnumDescriptorProto{{
-				Name: scalar.String("AnEnum"),
-				Value: []*descriptorpb.EnumValueDescriptorProto{{
-					Name:   scalar.String("UNKNOWN"),
-					Number: scalar.Int32(0),
-				}},
-			}},
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("BadMessage"),
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("good_field"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
-				}, {
-					Name:     scalar.String("bad_field"),
-					Number:   scalar.Int32(3),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
-					TypeName: scalar.String("AnEnum"),
-				}},
-			}},
-		},
-		wantErr: `message field "foo.BadMessage.bad_field" cannot resolve type: target name cannot be specified for int32`,
-	}, {
-		name: "type_name on string extension",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("extensible.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("ExtensibleMessage"),
-				ExtensionRange: []*descriptorpb.DescriptorProto_ExtensionRange{{
-					Start: scalar.Int32(1000),
-					End:   scalar.Int32(2000),
-				}},
-			}},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:       scalar.String("string-ext-with-type-name.proto"),
-			Syntax:     scalar.String("proto2"),
-			Package:    scalar.String("bar"),
-			Dependency: []string{"extensible.proto"},
-			EnumType: []*descriptorpb.EnumDescriptorProto{{
-				Name: scalar.String("AnEnum"),
-				Value: []*descriptorpb.EnumValueDescriptorProto{{
-					Name:   scalar.String("UNKNOWN"),
-					Number: scalar.Int32(0),
-				}},
-			}},
-			Extension: []*descriptorpb.FieldDescriptorProto{{
-				Name:     scalar.String("my_ext"),
-				Number:   scalar.Int32(1000),
-				Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-				Type:     descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				Extendee: scalar.String(".foo.ExtensibleMessage"),
-				TypeName: scalar.String("AnEnum"),
-			}},
-		},
-		wantErr: `extension field "bar.my_ext" cannot resolve type: target name cannot be specified for string`,
-	}, {
-		name: "oneof_index on extension",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("extensible.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("ExtensibleMessage"),
-				ExtensionRange: []*descriptorpb.DescriptorProto_ExtensionRange{{
-					Start: scalar.Int32(1000),
-					End:   scalar.Int32(2000),
-				}},
-			}},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:       scalar.String("ext-with-oneof-index.proto"),
-			Syntax:     scalar.String("proto2"),
-			Package:    scalar.String("bar"),
-			Dependency: []string{"extensible.proto"},
-			Extension: []*descriptorpb.FieldDescriptorProto{{
-				Name:       scalar.String("my_ext"),
-				Number:     scalar.Int32(1000),
-				Label:      descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-				Type:       descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				Extendee:   scalar.String(".foo.ExtensibleMessage"),
-				OneofIndex: scalar.Int32(0),
-			}},
-		},
-		wantErr: "oneof_index",
-	}, {
-		name: "enum with reserved number",
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:    scalar.String("enum-with-reserved-number.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			EnumType: []*descriptorpb.EnumDescriptorProto{{
-				Name: scalar.String("AnEnum"),
-				ReservedRange: []*descriptorpb.EnumDescriptorProto_EnumReservedRange{{
-					Start: scalar.Int32(5),
-					End:   scalar.Int32(6),
-				}, {
-					Start: scalar.Int32(10),
-					End:   scalar.Int32(12),
-				}},
-				Value: []*descriptorpb.EnumValueDescriptorProto{{
-					Name:   scalar.String("UNKNOWN"),
-					Number: scalar.Int32(0),
-				}, {
-					Name:   scalar.String("FOO"),
-					Number: scalar.Int32(1),
-				}, {
-					Name:   scalar.String("BAR"),
-					Number: scalar.Int32(2),
-				}, {
-					Name:   scalar.String("BAD"),
-					Number: scalar.Int32(11),
-				}},
-			}},
-		},
-		wantErr: "reserved number 11",
-	}, {
-		name: "enum with reserved number",
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:    scalar.String("enum-with-reserved-name.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("ParentMessage"),
-				EnumType: []*descriptorpb.EnumDescriptorProto{{
-					Name:         scalar.String("AnEnum"),
-					ReservedName: []string{"ABC", "XYZ"},
-					Value: []*descriptorpb.EnumValueDescriptorProto{{
-						Name:   scalar.String("UNKNOWN"),
-						Number: scalar.Int32(0),
-					}, {
-						Name:   scalar.String("FOO"),
-						Number: scalar.Int32(1),
-					}, {
-						Name:   scalar.String("BAR"),
-						Number: scalar.Int32(2),
-					}, {
-						Name:   scalar.String("XYZ"),
-						Number: scalar.Int32(3),
-					}},
-				}},
-			}},
-		},
-		wantErr: `reserved name "XYZ"`,
-	}, {
-		name: "message dependency without import",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("foo.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("Foo"),
-			}},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:    scalar.String("message-dependency-without-import.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("bar"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("Bar"),
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("id"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				}, {
-					Name:     scalar.String("foo"),
-					Number:   scalar.Int32(2),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
-					TypeName: scalar.String(".foo.Foo"),
-				}},
-			}},
-		},
-		wantErr: `message field "bar.Bar.foo" cannot resolve type: resolved "foo.Foo", but "foo.proto" is not imported`,
-	}, {
-		name: "enum dependency without import",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("foo.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			EnumType: []*descriptorpb.EnumDescriptorProto{{
-				Name: scalar.String("Foo"),
-				Value: []*descriptorpb.EnumValueDescriptorProto{{
-					Name:   scalar.String("UNKNOWN"),
-					Number: scalar.Int32(0),
-				}},
-			}},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:    scalar.String("enum-dependency-without-import.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("bar"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("Bar"),
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("id"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				}, {
-					Name:     scalar.String("foo"),
-					Number:   scalar.Int32(2),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_ENUM.Enum(),
-					TypeName: scalar.String(".foo.Foo"),
-				}},
-			}},
-		},
-		wantErr: `message field "bar.Bar.foo" cannot resolve type: resolved "foo.Foo", but "foo.proto" is not imported`,
-	}, {
-		name: "message dependency on without import on file imported by a public import",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("foo.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("Foo"),
-			}},
-		}, {
-			Name:       scalar.String("baz.proto"),
-			Syntax:     scalar.String("proto2"),
-			Package:    scalar.String("foo"),
-			Dependency: []string{"foo.proto"},
-		}, {
-			Name:             scalar.String("old-baz.proto"),
-			Syntax:           scalar.String("proto2"),
-			Package:          scalar.String("foo"),
-			Dependency:       []string{"baz.proto"},
-			PublicDependency: []int32{0},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:       scalar.String("message-dependency-without-import.proto"),
-			Syntax:     scalar.String("proto2"),
-			Package:    scalar.String("bar"),
-			Dependency: []string{"old-baz.proto"},
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("Bar"),
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("id"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				}, {
-					Name:     scalar.String("foo"),
-					Number:   scalar.Int32(2),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
-					TypeName: scalar.String(".foo.Foo"),
-				}},
-			}},
-		},
-		wantErr: `message field "bar.Bar.foo" cannot resolve type: resolved "foo.Foo", but "foo.proto" is not imported`,
-	}}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			r := new(protoregistry.Files)
-			for _, dep := range tc.deps {
-				f, err := NewFile(dep, r)
-				if err != nil {
-					t.Fatalf("Error creating dependency: %v", err)
-				}
-				if err := r.Register(f); err != nil {
-					t.Fatalf("Error adding dependency: %v", err)
-				}
-			}
-			if _, err := NewFile(tc.fd, r); err == nil || !strings.Contains(err.Error(), tc.wantErr) {
-				t.Errorf("NewFile: got err = %v; want error containing %q", err, tc.wantErr)
-			}
-		})
-	}
-}
-
-// Sanity checks for well-formed descriptors. Most behavior with well-formed descriptors is covered
-// by other tests that rely on generated descriptors.
-func TestNewFile_ValidationOK(t *testing.T) {
-	testCases := []struct {
-		name string
-		deps []*descriptorpb.FileDescriptorProto
-		fd   *descriptorpb.FileDescriptorProto
-	}{{
-		name: "self contained file",
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:    scalar.String("self-contained.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			EnumType: []*descriptorpb.EnumDescriptorProto{{
-				Name: scalar.String("TopLevelEnum"),
-				Value: []*descriptorpb.EnumValueDescriptorProto{{
-					Name:   scalar.String("UNKNOWN"),
-					Number: scalar.Int32(0),
-				}},
-			}},
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("TopLevelMessage"),
-				EnumType: []*descriptorpb.EnumDescriptorProto{{
-					Name: scalar.String("NestedEnum"),
-					Value: []*descriptorpb.EnumValueDescriptorProto{{
-						Name:   scalar.String("UNKNOWN"),
-						Number: scalar.Int32(0),
-					}},
-				}},
-				NestedType: []*descriptorpb.DescriptorProto{{
-					Name: scalar.String("NestedMessage"),
-				}},
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("id"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				}, {
-					Name:     scalar.String("top_level_enum"),
-					Number:   scalar.Int32(2),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_ENUM.Enum(),
-					TypeName: scalar.String(".foo.TopLevelEnum"),
-				}, {
-					Name:     scalar.String("nested_enum"),
-					Number:   scalar.Int32(3),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_ENUM.Enum(),
-					TypeName: scalar.String(".foo.TopLevelMessage.NestedEnum"),
-				}, {
-					Name:     scalar.String("nested_message"),
-					Number:   scalar.Int32(4),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
-					TypeName: scalar.String(".foo.TopLevelMessage.NestedMessage"),
-				}},
-			}},
-		},
-	}, {
-		name: "external types with explicit import",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("foo.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("FooMessage"),
-			}},
-			EnumType: []*descriptorpb.EnumDescriptorProto{{
-				Name: scalar.String("BarEnum"),
-				Value: []*descriptorpb.EnumValueDescriptorProto{{
-					Name:   scalar.String("UNKNOWN"),
-					Number: scalar.Int32(0),
-				}},
-			}},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:       scalar.String("external-types-with-explicit-import.proto"),
-			Syntax:     scalar.String("proto2"),
-			Package:    scalar.String("bar"),
-			Dependency: []string{"foo.proto"},
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("Bar"),
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("id"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				}, {
-					Name:     scalar.String("foo"),
-					Number:   scalar.Int32(2),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
-					TypeName: scalar.String(".foo.FooMessage"),
-				}, {
-					Name:     scalar.String("bar"),
-					Number:   scalar.Int32(3),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_ENUM.Enum(),
-					TypeName: scalar.String(".foo.BarEnum"),
-				}},
-			}},
-		},
-	}, {
-		name: "external types with transitive public imports",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("quux.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("QuuxMessage"),
-			}},
-		}, {
-			Name:             scalar.String("foo.proto"),
-			Syntax:           scalar.String("proto2"),
-			Package:          scalar.String("foo"),
-			Dependency:       []string{"quux.proto"},
-			PublicDependency: []int32{0},
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("FooMessage"),
-			}},
-			EnumType: []*descriptorpb.EnumDescriptorProto{{
-				Name: scalar.String("BarEnum"),
-				Value: []*descriptorpb.EnumValueDescriptorProto{{
-					Name:   scalar.String("UNKNOWN"),
-					Number: scalar.Int32(0),
-				}},
-			}},
-		}, {
-			Name:             scalar.String("old-name.proto"),
-			Syntax:           scalar.String("proto2"),
-			Package:          scalar.String("foo"),
-			Dependency:       []string{"foo.proto"},
-			PublicDependency: []int32{0},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:       scalar.String("external-types-with-public-import.proto"),
-			Syntax:     scalar.String("proto2"),
-			Package:    scalar.String("bar"),
-			Dependency: []string{"old-name.proto"},
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("Bar"),
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("id"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				}, {
-					Name:     scalar.String("foo"),
-					Number:   scalar.Int32(2),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
-					TypeName: scalar.String(".foo.FooMessage"),
-				}, {
-					Name:     scalar.String("bar"),
-					Number:   scalar.Int32(3),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_ENUM.Enum(),
-					TypeName: scalar.String(".foo.BarEnum"),
-				}, {
-					Name:     scalar.String("quux"),
-					Number:   scalar.Int32(4),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
-					TypeName: scalar.String(".foo.QuuxMessage"),
-				}},
-			}},
-		},
-	}, {
-		name: "external type from weak import",
-		deps: []*descriptorpb.FileDescriptorProto{{
-			Name:    scalar.String("weak.proto"),
-			Syntax:  scalar.String("proto2"),
-			Package: scalar.String("foo"),
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("WeakMessage"),
-			}},
-		}},
-		fd: &descriptorpb.FileDescriptorProto{
-			Name:           scalar.String("external-type-from-weak-import.proto"),
-			Syntax:         scalar.String("proto2"),
-			Package:        scalar.String("bar"),
-			Dependency:     []string{"weak.proto"},
-			WeakDependency: []int32{0},
-			MessageType: []*descriptorpb.DescriptorProto{{
-				Name: scalar.String("Bar"),
-				Field: []*descriptorpb.FieldDescriptorProto{{
-					Name:   scalar.String("id"),
-					Number: scalar.Int32(1),
-					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
-				}, {
-					Name:     scalar.String("weak_message"),
-					Number:   scalar.Int32(2),
-					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-					Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
-					TypeName: scalar.String(".foo.WeakMessage"),
-					Options: &descriptorpb.FieldOptions{
-						Weak: scalar.Bool(true),
-					},
-				}},
-			}},
-		},
-	}, {
-		name: "local enum value dependency in sibling to parent message",
-		fd: mustParseFile(`
-			name: "test.proto"
-			message_type: [{
-				name: "M1"
-				field: [{
-					name:          "F"
-					number:        1
-					label:         LABEL_OPTIONAL
-					type:          TYPE_ENUM
-					type_name:     ".M2.E"
-					default_value: "V2"
-				}]
-			}, {
-				name: "M2"
-				enum_type: [{
-					name: "E"
-					value: [{name:"V1" number:1}, {name:"V2" number:2}]
-				}]
-			}]
-		`),
-	}}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			r := new(protoregistry.Files)
-			for _, dep := range tc.deps {
-				f, err := NewFile(dep, r)
-				if err != nil {
-					t.Fatalf("error creating dependency: %v", err)
-				}
-				if err := r.Register(f); err != nil {
-					t.Fatalf("error adding dependency: %v", err)
-				}
-			}
-			if _, err := NewFile(tc.fd, r); err != nil {
-				t.Errorf("unexpected NewFile error: %v", err)
-			}
-		})
-	}
-}

reflect/protoregistry/registry_test.go

@@ -70,7 +70,7 @@ func TestFiles(t *testing.T) {
 			{inFile: mustMakeFile(`syntax:"proto2" name:"foo/bar/test.proto" package:"my.test"`)},
 			{inFile: mustMakeFile(`syntax:"proto2" name:"foo/bar/test.proto" package:"foo.bar.baz"`), wantErr: "already registered"},
 			{inFile: mustMakeFile(`syntax:"proto2" name:"test2.proto" package:"my.test.package"`)},
-			{inFile: mustMakeFile(`syntax:"proto2" name:"" package:"foo.bar"`)},
+			{inFile: mustMakeFile(`syntax:"proto2" name:"weird" package:"foo.bar"`)},
 			{inFile: mustMakeFile(`syntax:"proto2" name:"foo/bar/baz/../test.proto" package:"my.test"`)},
 		},
 
@@ -90,7 +90,7 @@ func TestFiles(t *testing.T) {
 			inPkg: "foo.bar",
 			wantFiles: []file{
 				{"test1.proto", "foo.bar"},
-				{"", "foo.bar"},
+				{"weird", "foo.bar"},
 			},
 		}, {
 			inPkg: "my.test",
@@ -105,9 +105,9 @@ func TestFiles(t *testing.T) {
 		findPaths: []testFindPath{{
 			inPath: "nothing",
 		}, {
-			inPath: "",
+			inPath: "weird",
 			wantFiles: []file{
-				{"", "foo.bar"},
+				{"weird", "foo.bar"},
 			},
 		}, {
 			inPath: "foo/bar/test.proto",
@@ -120,13 +120,13 @@ func TestFiles(t *testing.T) {
 		files: []testFile{{
 			inFile: mustMakeFile(`syntax:"proto2" name:"test1a.proto" package:"foo.bar.baz"`),
 		}, {
-			inFile:  mustMakeFile(`syntax:"proto2" name:"test1b.proto" enum_type:[{name:"foo"}]`),
+			inFile:  mustMakeFile(`syntax:"proto2" name:"test1b.proto" enum_type:[{name:"foo" value:[{name:"VALUE" number:0}]}]`),
 			wantErr: `file "test1b.proto" has a name conflict over foo`,
 		}},
 	}, {
 		// Test when new package conflicts with existing enum.
 		files: []testFile{{
-			inFile: mustMakeFile(`syntax:"proto2" name:"test2a.proto" enum_type:[{name:"foo"}]`),
+			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`,
@@ -134,9 +134,9 @@ func TestFiles(t *testing.T) {
 	}, {
 		// Test when new enum conflicts with existing enum in same package.
 		files: []testFile{{
-			inFile: mustMakeFile(`syntax:"proto2" name:"test3a.proto" package:"foo" enum_type:[{name:"BAR"}]`),
+			inFile: mustMakeFile(`syntax:"proto2" name:"test3a.proto" package:"foo" enum_type:[{name:"BAR" value:[{name:"VALUE" number:0}]}]`),
 		}, {
-			inFile:  mustMakeFile(`syntax:"proto2" name:"test3b.proto" package:"foo" enum_type:[{name:"BAR"}]`),
+			inFile:  mustMakeFile(`syntax:"proto2" name:"test3b.proto" package:"foo" enum_type:[{name:"BAR" value:[{name:"VALUE2" number:0}]}]`),
 			wantErr: `file "test3b.proto" has a name conflict over foo.BAR`,
 		}},
 	}, {
@@ -207,6 +207,7 @@ func TestFiles(t *testing.T) {
 		}, {
 			// Make sure we can register without package name.
 			inFile: mustMakeFile(`
+				name:   "weird"
 				syntax: "proto2"
 				message_type: [{
 					name: "Message"