diff --git a/internal/cmd/pbdump/pbdump.go b/internal/cmd/pbdump/pbdump.go index 8bc17530..98a36ebd 100644 --- a/internal/cmd/pbdump/pbdump.go +++ b/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)) } diff --git a/internal/cmd/pbdump/pbdump_test.go b/internal/cmd/pbdump/pbdump_test.go index a3f69982..567cea0e 100644 --- a/internal/cmd/pbdump/pbdump_test.go +++ b/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) } diff --git a/internal/encoding/pack/pack_test.go b/internal/encoding/pack/pack_test.go index d6704e9a..9d1239a2 100644 --- a/internal/encoding/pack/pack_test.go +++ b/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) diff --git a/internal/filedesc/desc_list.go b/internal/filedesc/desc_list.go index bbded603..cd92236b 100644 --- a/internal/filedesc/desc_list.go +++ b/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 diff --git a/internal/filedesc/desc_test.go b/internal/filedesc/desc_test.go index 5bea386c..66ce5183 100644 --- a/internal/filedesc/desc_test.go +++ b/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 diff --git a/reflect/protodesc/desc.go b/reflect/protodesc/desc.go index 012748bc..f8a24beb 100644 --- a/reflect/protodesc/desc.go +++ b/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()) } diff --git a/reflect/protodesc/desc_init.go b/reflect/protodesc/desc_init.go index 9d8246ee..84c76c47 100644 --- a/reflect/protodesc/desc_init.go +++ b/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? diff --git a/reflect/protodesc/desc_resolve.go b/reflect/protodesc/desc_resolve.go index 25e2c3bf..83f0ba00 100644 --- a/reflect/protodesc/desc_resolve.go +++ b/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 } diff --git a/reflect/protodesc/desc_validate.go b/reflect/protodesc/desc_validate.go index 53db7af6..7f5c8783 100644 --- a/reflect/protodesc/desc_validate.go +++ b/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.GetExtendee() != "" { - return errors.New("message field may not have extendee") + if fd.Extendee != nil { + return errors.New("message field %q may not have extendee: %q", f.FullName(), fd.GetExtendee()) + } + 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] - if xd.OneofIndex != nil { - return errors.New("extension may not have oneof_index") + // 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 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()) + } + 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()) + } } - _ = x } 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) +} diff --git a/reflect/protodesc/file_test.go b/reflect/protodesc/file_test.go index 45910fcf..26bc174c 100644 --- a/reflect/protodesc/file_test.go +++ 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) + } + } +} diff --git a/reflect/protodesc/protodesc_test.go b/reflect/protodesc/protodesc_test.go deleted file mode 100644 index dabda29b..00000000 --- a/reflect/protodesc/protodesc_test.go +++ /dev/null @@ -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) - } - }) - } -} diff --git a/reflect/protoregistry/registry_test.go b/reflect/protoregistry/registry_test.go index d6bfa593..d59910d3 100644 --- a/reflect/protoregistry/registry_test.go +++ b/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"