reflect/protoreflect: add MessageDescriptor.ExtensionRangeOptions

Considerable thought was given to whether a seperate ExtensionRanges interface
should be made that encapsulates FieldNumbers with an Options method.
However, I decided against this design for the following reasons:
* Extension ranges share syntax with reserved numbers and fields.
Why is extension ranges so special that it can have options, while the other
two currently do not? How do we know that those other two won't grow options
in the future? If they do, then those APIs can be expanded in the same way as
how extension range options is being expanded here today.
* Extension range options stand out like a sore thumb compared to the other
eight options. The other options correspond with a named declaration and have
a full protobuf name that they are associated with. Extension range options
is the only options that is not correlated with a full name.
* Extension range options are an extremely rarely used feature and
it seems unfortunate complicating the common case with additional structure.

Change-Id: Ib284a0b798c57dc264febe304692eee5b9c8e91b
Reviewed-on: https://go-review.googlesource.com/c/153018
Reviewed-by: Damien Neil <dneil@google.com>
This commit is contained in:
Joe Tsai 2018-12-06 12:10:41 -08:00 committed by Joe Tsai
parent a4cbffe4bc
commit 381f04c102
10 changed files with 97 additions and 53 deletions

View File

@ -5,22 +5,28 @@
package typefmt
import (
"reflect"
"testing"
pref "github.com/golang/protobuf/v2/reflect/protoreflect"
)
// TestDescriptorAccessors tests that descriptorAccessors is up-to-date.
func TestDescriptorAccessors(t *testing.T) {
ignore := map[string]bool{
"DefaultEnumValue": true,
"DescriptorByName": true,
"ProtoType": true,
}
rt := reflect.TypeOf((*pref.Descriptor)(nil)).Elem()
for i := 0; i < rt.NumMethod(); i++ {
ignore[rt.Method(i).Name] = true
"Parent": true,
"Index": true,
"Syntax": true,
"Name": true,
"FullName": true,
"IsPlaceholder": true,
"ProtoInternal": true,
"ProtoType": true,
"DescriptorByName": true, // specific to FileDescriptor
"DefaultEnumValue": true, // specific to FieldDescriptor
// TODO: These should be removed or handled.
"DescriptorProto": true,
"ExtensionRangeOptions": true,
"Options": true,
}
for rt, m := range descriptorAccessors {

View File

@ -173,11 +173,11 @@ func messagesFromDescriptorProto(mds []*descriptorpb.DescriptorProto, syntax pro
})
}
for _, xr := range md.GetExtensionRange() {
// TODO: Extension range options.
m.ExtensionRanges = append(m.ExtensionRanges, [2]protoreflect.FieldNumber{
protoreflect.FieldNumber(xr.GetStart()),
protoreflect.FieldNumber(xr.GetEnd()),
})
m.ExtensionRangeOptions = append(m.ExtensionRangeOptions, xr.GetOptions())
}
m.Messages, err = messagesFromDescriptorProto(md.GetNestedType(), syntax, r)

View File

@ -235,6 +235,12 @@ type MessageDescriptor interface {
// ExtensionRanges is the field ranges used for extension fields.
// In Proto3, it is always an empty ranges.
ExtensionRanges() FieldRanges
// ExtensionRangeOptions returns the ith extension range options,
// which is a google.protobuf.ExtensionRangeOptions protobuf message.
// The caller must not modify the returned message.
//
// This method may return a nil interface value if no options are present.
ExtensionRangeOptions(i int) ProtoMessage
// Messages is a list of nested message declarations.
Messages() MessageDescriptors

View File

@ -45,8 +45,8 @@ func (t placeholderFile) Options() pref.ProtoMessage { retur
func (t placeholderFile) Path() string { return t.path }
func (t placeholderFile) Package() pref.FullName { return t.FullName() }
func (t placeholderFile) Imports() pref.FileImports { return &emptyFiles }
func (t placeholderFile) Messages() pref.MessageDescriptors { return &emptyMessages }
func (t placeholderFile) Enums() pref.EnumDescriptors { return &emptyEnums }
func (t placeholderFile) Messages() pref.MessageDescriptors { return &emptyMessages }
func (t placeholderFile) Extensions() pref.ExtensionDescriptors { return &emptyExtensions }
func (t placeholderFile) Services() pref.ServiceDescriptors { return &emptyServices }
func (t placeholderFile) DescriptorByName(pref.FullName) pref.Descriptor { return nil }
@ -57,17 +57,18 @@ type placeholderMessage struct {
placeholderName
}
func (t placeholderMessage) Options() pref.ProtoMessage { return optionTypes.Message }
func (t placeholderMessage) IsMapEntry() bool { return false }
func (t placeholderMessage) Fields() pref.FieldDescriptors { return &emptyFields }
func (t placeholderMessage) Oneofs() pref.OneofDescriptors { return &emptyOneofs }
func (t placeholderMessage) RequiredNumbers() pref.FieldNumbers { return &emptyNumbers }
func (t placeholderMessage) ExtensionRanges() pref.FieldRanges { return &emptyRanges }
func (t placeholderMessage) Messages() pref.MessageDescriptors { return &emptyMessages }
func (t placeholderMessage) Enums() pref.EnumDescriptors { return &emptyEnums }
func (t placeholderMessage) Extensions() pref.ExtensionDescriptors { return &emptyExtensions }
func (t placeholderMessage) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, t) }
func (t placeholderMessage) ProtoType(pref.MessageDescriptor) {}
func (t placeholderMessage) Options() pref.ProtoMessage { return optionTypes.Message }
func (t placeholderMessage) IsMapEntry() bool { return false }
func (t placeholderMessage) Fields() pref.FieldDescriptors { return &emptyFields }
func (t placeholderMessage) Oneofs() pref.OneofDescriptors { return &emptyOneofs }
func (t placeholderMessage) RequiredNumbers() pref.FieldNumbers { return &emptyNumbers }
func (t placeholderMessage) ExtensionRanges() pref.FieldRanges { return &emptyRanges }
func (t placeholderMessage) ExtensionRangeOptions(int) pref.ProtoMessage { panic("out of bounds") }
func (t placeholderMessage) Enums() pref.EnumDescriptors { return &emptyEnums }
func (t placeholderMessage) Messages() pref.MessageDescriptors { return &emptyMessages }
func (t placeholderMessage) Extensions() pref.ExtensionDescriptors { return &emptyExtensions }
func (t placeholderMessage) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, t) }
func (t placeholderMessage) ProtoType(pref.MessageDescriptor) {}
type placeholderEnum struct {
placeholderName

View File

@ -33,15 +33,16 @@ import "github.com/golang/protobuf/v2/reflect/protoreflect"
// File is a constructor for protoreflect.FileDescriptor.
type File struct {
Syntax protoreflect.Syntax
Path string
Package protoreflect.FullName
Imports []protoreflect.FileImport
Messages []Message
Syntax protoreflect.Syntax
Path string
Package protoreflect.FullName
Imports []protoreflect.FileImport
Options protoreflect.ProtoMessage
Enums []Enum
Messages []Message
Extensions []Extension
Services []Service
Options protoreflect.ProtoMessage
*fileMeta
}
@ -94,14 +95,16 @@ func visitMessages(d interface {
// Message is a constructor for protoreflect.MessageDescriptor.
type Message struct {
Name protoreflect.Name
Fields []Field
Oneofs []Oneof
ExtensionRanges [][2]protoreflect.FieldNumber
Messages []Message
Enums []Enum
Extensions []Extension
Options protoreflect.ProtoMessage
Name protoreflect.Name
Fields []Field
Oneofs []Oneof
ExtensionRanges [][2]protoreflect.FieldNumber
ExtensionRangeOptions []protoreflect.ProtoMessage
Options protoreflect.ProtoMessage
Enums []Enum
Messages []Message
Extensions []Extension
*messageMeta
}

View File

@ -73,8 +73,8 @@ func (t fileDesc) Options() pref.ProtoMessage { return alt
func (t fileDesc) Path() string { return t.f.Path }
func (t fileDesc) Package() pref.FullName { return t.f.Package }
func (t fileDesc) Imports() pref.FileImports { return (*fileImports)(&t.f.Imports) }
func (t fileDesc) Messages() pref.MessageDescriptors { return t.f.ms.lazyInit(t, t.f.Messages) }
func (t fileDesc) Enums() pref.EnumDescriptors { return t.f.es.lazyInit(t, t.f.Enums) }
func (t fileDesc) Messages() pref.MessageDescriptors { return t.f.ms.lazyInit(t, t.f.Messages) }
func (t fileDesc) Extensions() pref.ExtensionDescriptors { return t.f.xs.lazyInit(t, t.f.Extensions) }
func (t fileDesc) Services() pref.ServiceDescriptors { return t.f.ss.lazyInit(t, t.f.Services) }
func (t fileDesc) DescriptorByName(s pref.FullName) pref.Descriptor { return t.f.ds.lookup(t, s) }
@ -183,13 +183,30 @@ func (t messageDesc) Fields() pref.FieldDescriptors { return t.m.fs.lazy
func (t messageDesc) Oneofs() pref.OneofDescriptors { return t.m.os.lazyInit(t, t.m.Oneofs) }
func (t messageDesc) RequiredNumbers() pref.FieldNumbers { return t.m.ns.lazyInit(t.m.Fields) }
func (t messageDesc) ExtensionRanges() pref.FieldRanges { return (*ranges)(&t.m.ExtensionRanges) }
func (t messageDesc) Messages() pref.MessageDescriptors { return t.m.ms.lazyInit(t, t.m.Messages) }
func (t messageDesc) ExtensionRangeOptions(i int) pref.ProtoMessage {
return extensionRangeOptions(i, len(t.m.ExtensionRanges), t.m.ExtensionRangeOptions)
}
func (t messageDesc) Enums() pref.EnumDescriptors { return t.m.es.lazyInit(t, t.m.Enums) }
func (t messageDesc) Messages() pref.MessageDescriptors { return t.m.ms.lazyInit(t, t.m.Messages) }
func (t messageDesc) Extensions() pref.ExtensionDescriptors { return t.m.xs.lazyInit(t, t.m.Extensions) }
func (t messageDesc) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, t) }
func (t messageDesc) ProtoType(pref.MessageDescriptor) {}
func (t messageDesc) ProtoInternal(pragma.DoNotImplement) {}
func extensionRangeOptions(i, n int, ms []pref.ProtoMessage) pref.ProtoMessage {
if i < 0 || i >= n {
panic("out of bounds")
}
var m pref.ProtoMessage
if i < len(ms) {
m = ms[i]
}
if m == nil {
m = optionTypes.ExtensionRange
}
return m
}
type messageOptions struct {
once sync.Once
isMapEntry bool

View File

@ -15,12 +15,13 @@ import (
// StandaloneMessage is a constructor for a protoreflect.MessageDescriptor
// that does not have a parent and has no child declarations.
type StandaloneMessage struct {
Syntax protoreflect.Syntax
FullName protoreflect.FullName
Fields []Field
Oneofs []Oneof
ExtensionRanges [][2]protoreflect.FieldNumber
Options protoreflect.ProtoMessage
Syntax protoreflect.Syntax
FullName protoreflect.FullName
Fields []Field
Oneofs []Oneof
ExtensionRanges [][2]protoreflect.FieldNumber
ExtensionRangeOptions []protoreflect.ProtoMessage
Options protoreflect.ProtoMessage
fields fieldsMeta
oneofs oneofsMeta

View File

@ -24,13 +24,16 @@ func (t standaloneMessage) DescriptorProto() (pref.Message, bool) { return nil,
func (t standaloneMessage) Options() pref.ProtoMessage {
return altOptions(t.m.Options, optionTypes.Message)
}
func (t standaloneMessage) IsMapEntry() bool { return t.m.options.lazyInit(t).isMapEntry }
func (t standaloneMessage) Fields() pref.FieldDescriptors { return t.m.fields.lazyInit(t, t.m.Fields) }
func (t standaloneMessage) Oneofs() pref.OneofDescriptors { return t.m.oneofs.lazyInit(t, t.m.Oneofs) }
func (t standaloneMessage) RequiredNumbers() pref.FieldNumbers { return t.m.nums.lazyInit(t.m.Fields) }
func (t standaloneMessage) ExtensionRanges() pref.FieldRanges { return (*ranges)(&t.m.ExtensionRanges) }
func (t standaloneMessage) Messages() pref.MessageDescriptors { return &emptyMessages }
func (t standaloneMessage) IsMapEntry() bool { return t.m.options.lazyInit(t).isMapEntry }
func (t standaloneMessage) Fields() pref.FieldDescriptors { return t.m.fields.lazyInit(t, t.m.Fields) }
func (t standaloneMessage) Oneofs() pref.OneofDescriptors { return t.m.oneofs.lazyInit(t, t.m.Oneofs) }
func (t standaloneMessage) RequiredNumbers() pref.FieldNumbers { return t.m.nums.lazyInit(t.m.Fields) }
func (t standaloneMessage) ExtensionRanges() pref.FieldRanges { return (*ranges)(&t.m.ExtensionRanges) }
func (t standaloneMessage) ExtensionRangeOptions(i int) pref.ProtoMessage {
return extensionRangeOptions(i, len(t.m.ExtensionRanges), t.m.ExtensionRangeOptions)
}
func (t standaloneMessage) Enums() pref.EnumDescriptors { return &emptyEnums }
func (t standaloneMessage) Messages() pref.MessageDescriptors { return &emptyMessages }
func (t standaloneMessage) Extensions() pref.ExtensionDescriptors { return &emptyExtensions }
func (t standaloneMessage) Format(s fmt.State, r rune) { pfmt.FormatDesc(s, r, t) }
func (t standaloneMessage) ProtoType(pref.MessageDescriptor) {}

View File

@ -110,6 +110,10 @@ func TestFile(t *testing.T) {
{Name: "O2"}, // "test.B.O2"
},
ExtensionRanges: [][2]pref.FieldNumber{{1000, 2000}, {3000, 3001}},
ExtensionRangeOptions: []pref.ProtoMessage{
0: (*descriptorpb.ExtensionRangeOptions)(nil),
1: new(descriptorpb.ExtensionRangeOptions),
},
}, {
Name: "C", // "test.C"
Messages: []ptype.Message{{
@ -250,7 +254,7 @@ func TestFile(t *testing.T) {
},
ExtensionRange: []*descriptorpb.DescriptorProto_ExtensionRange{
{Start: scalar.Int32(1000), End: scalar.Int32(2000)},
{Start: scalar.Int32(3000), End: scalar.Int32(3001)},
{Start: scalar.Int32(3000), End: scalar.Int32(3001), Options: new(descriptorpb.ExtensionRangeOptions)},
},
}, {
Name: scalar.String("C"),
@ -502,6 +506,8 @@ func testFileAccessors(t *testing.T, fd pref.FileDescriptor) {
"Has:2000": false,
"Has:3000": true,
},
"ExtensionRangeOptions:0": (*descriptorpb.ExtensionRangeOptions)(nil),
"ExtensionRangeOptions:1": new(descriptorpb.ExtensionRangeOptions),
},
"Get:2": M{
"Name": pref.Name("C"),
@ -669,7 +675,7 @@ func checkAccessors(t *testing.T, p string, rv reflect.Value, want map[string]in
}
if want := v; !reflect.DeepEqual(got, want) {
t.Errorf("%v = %v, want %v", p, got, want)
t.Errorf("%v = %T(%v), want %T(%v)", p, got, got, want, want)
}
}
}

View File

@ -27,6 +27,7 @@ import (
// * 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 validateFile(t pref.FileDescriptor) error {
return nil