goprotobuf: Repeated extensions.

Also picks up a tweak to the JSON tags of the XXX_ fields.

R=r
CC=golang-dev
http://codereview.appspot.com/6175045

proto/decode.go

@@ -357,7 +357,9 @@ func (o *Buffer) unmarshalType(t reflect.Type, is_group bool, base uintptr) erro
 			iv := reflect.NewAt(st, unsafe.Pointer(base)).Interface()
 			if e, ok := iv.(extendableProto); ok && isExtensionField(e, int32(tag)) {
 				if err = o.skip(st, tag, wire); err == nil {
-					e.ExtensionMap()[int32(tag)] = Extension{enc: append([]byte(nil), o.buf[oi:o.index]...)}
+					ext := e.ExtensionMap()[int32(tag)] // may be missing
+					ext.enc = append(ext.enc, o.buf[oi:o.index]...)
+					e.ExtensionMap()[int32(tag)] = ext
 				}
 				continue
 			}

proto/extensions.go

@@ -66,6 +66,11 @@ type ExtensionDesc struct {
 	Tag           string      // protobuf tag style
 }
 
+func (ed *ExtensionDesc) repeated() bool {
+	t := reflect.TypeOf(ed.ExtensionType)
+	return t.Kind() == reflect.Slice && t.Elem().Kind() != reflect.Uint8
+}
+
 /*
 Extension represents an extension in a message.
 
@@ -192,21 +197,33 @@ func GetExtension(pb extendableProto, extension *ExtensionDesc) (interface{}, er
 
 // decodeExtension decodes an extension encoded in b.
 func decodeExtension(b []byte, extension *ExtensionDesc) (interface{}, error) {
-	// Discard wire type and field number varint. It isn't needed.
-	_, n := DecodeVarint(b)
-	o := NewBuffer(b[n:])
+	o := NewBuffer(b)
 
 	t := reflect.TypeOf(extension.ExtensionType)
+	rep := extension.repeated()
+
 	props := &Properties{}
 	props.Init(t, "irrelevant_name", extension.Tag, 0)
 
-	// t is a pointer, likely to a struct.
-	// Allocate a "field" to store the pointer itself; the
-	// struct pointer will be stored here. We pass
+	// t is a pointer to a struct, pointer to basic type or a slice.
+	// Allocate a "field" to store the pointer/slice itself; the
+	// pointer/slice will be stored here. We pass
 	// the address of this field to props.dec.
 	value := reflect.New(t).Elem()
-	if err := props.dec(o, props, value.UnsafeAddr()); err != nil {
-		return nil, err
+
+	for {
+		// Discard wire type and field number varint. It isn't needed.
+		if _, err := o.DecodeVarint(); err != nil {
+			return nil, err
+		}
+
+		if err := props.dec(o, props, value.UnsafeAddr()); err != nil {
+			return nil, err
+		}
+
+		if !rep || o.index >= len(o.buf) {
+			break
+		}
 	}
 	return value.Interface(), nil
 }
@@ -229,10 +246,6 @@ func GetExtensions(pb interface{}, es []*ExtensionDesc) (extensions []interface{
 	return
 }
 
-// TODO: (needed for repeated extensions)
-//   - ExtensionSize
-//   - AddExtension
-
 // SetExtension sets the specified extension of pb to the specified value.
 func SetExtension(pb extendableProto, extension *ExtensionDesc, value interface{}) error {
 	if err := checkExtensionTypes(pb, extension); err != nil {

proto/testdata/test.pb.go

@@ -165,7 +165,7 @@ func (x RepeatedEnum_Color) String() string {
 
 type GoEnum struct {
 	Foo              *FOO   `protobuf:"varint,1,req,name=foo,enum=testdata.FOO" json:"foo,omitempty"`
-	XXX_unrecognized []byte `json:",omitempty"`
+	XXX_unrecognized []byte `json:"-"`
 }
 
 func (this *GoEnum) Reset()         { *this = GoEnum{} }
@@ -174,7 +174,7 @@ func (this *GoEnum) String() string { return proto.CompactTextString(this) }
 type GoTestField struct {
 	Label            *string `protobuf:"bytes,1,req" json:"Label,omitempty"`
 	Type             *string `protobuf:"bytes,2,req" json:"Type,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *GoTestField) Reset()         { *this = GoTestField{} }
@@ -253,7 +253,7 @@ type GoTest struct {
 	Requiredgroup           *GoTest_RequiredGroup   `protobuf:"group,70,req,name=RequiredGroup" json:"requiredgroup,omitempty"`
 	Repeatedgroup           []*GoTest_RepeatedGroup `protobuf:"group,80,rep,name=RepeatedGroup" json:"repeatedgroup,omitempty"`
 	Optionalgroup           *GoTest_OptionalGroup   `protobuf:"group,90,opt,name=OptionalGroup" json:"optionalgroup,omitempty"`
-	XXX_unrecognized        []byte                  `json:",omitempty"`
+	XXX_unrecognized        []byte                  `json:"-"`
 }
 
 func (this *GoTest) Reset()         { *this = GoTest{} }
@@ -277,7 +277,7 @@ const Default_GoTest_F_Sint64Defaulted int64 = -64
 
 type GoTest_RequiredGroup struct {
 	RequiredField    *string `protobuf:"bytes,71,req" json:"RequiredField,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *GoTest_RequiredGroup) Reset()         { *this = GoTest_RequiredGroup{} }
@@ -285,7 +285,7 @@ func (this *GoTest_RequiredGroup) String() string { return proto.CompactTextStri
 
 type GoTest_RepeatedGroup struct {
 	RequiredField    *string `protobuf:"bytes,81,req" json:"RequiredField,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *GoTest_RepeatedGroup) Reset()         { *this = GoTest_RepeatedGroup{} }
@@ -293,7 +293,7 @@ func (this *GoTest_RepeatedGroup) String() string { return proto.CompactTextStri
 
 type GoTest_OptionalGroup struct {
 	RequiredField    *string `protobuf:"bytes,91,req" json:"RequiredField,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *GoTest_OptionalGroup) Reset()         { *this = GoTest_OptionalGroup{} }
@@ -305,7 +305,7 @@ type GoSkipTest struct {
 	SkipFixed64      *uint64               `protobuf:"fixed64,13,req,name=skip_fixed64" json:"skip_fixed64,omitempty"`
 	SkipString       *string               `protobuf:"bytes,14,req,name=skip_string" json:"skip_string,omitempty"`
 	Skipgroup        *GoSkipTest_SkipGroup `protobuf:"group,15,req,name=SkipGroup" json:"skipgroup,omitempty"`
-	XXX_unrecognized []byte                `json:",omitempty"`
+	XXX_unrecognized []byte                `json:"-"`
 }
 
 func (this *GoSkipTest) Reset()         { *this = GoSkipTest{} }
@@ -314,7 +314,7 @@ func (this *GoSkipTest) String() string { return proto.CompactTextString(this) }
 type GoSkipTest_SkipGroup struct {
 	GroupInt32       *int32  `protobuf:"varint,16,req,name=group_int32" json:"group_int32,omitempty"`
 	GroupString      *string `protobuf:"bytes,17,req,name=group_string" json:"group_string,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *GoSkipTest_SkipGroup) Reset()         { *this = GoSkipTest_SkipGroup{} }
@@ -322,7 +322,7 @@ func (this *GoSkipTest_SkipGroup) String() string { return proto.CompactTextStri
 
 type NonPackedTest struct {
 	A                []int32 `protobuf:"varint,1,rep,name=a" json:"a,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *NonPackedTest) Reset()         { *this = NonPackedTest{} }
@@ -330,7 +330,7 @@ func (this *NonPackedTest) String() string { return proto.CompactTextString(this
 
 type PackedTest struct {
 	B                []int32 `protobuf:"varint,1,rep,packed,name=b" json:"b,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *PackedTest) Reset()         { *this = PackedTest{} }
@@ -338,7 +338,7 @@ func (this *PackedTest) String() string { return proto.CompactTextString(this) }
 
 type MaxTag struct {
 	LastField        *string `protobuf:"bytes,536870911,opt,name=last_field" json:"last_field,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *MaxTag) Reset()         { *this = MaxTag{} }
@@ -348,7 +348,7 @@ type InnerMessage struct {
 	Host             *string `protobuf:"bytes,1,req,name=host" json:"host,omitempty"`
 	Port             *int32  `protobuf:"varint,2,opt,name=port,def=4000" json:"port,omitempty"`
 	Connected        *bool   `protobuf:"varint,3,opt,name=connected" json:"connected,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *InnerMessage) Reset()         { *this = InnerMessage{} }
@@ -361,7 +361,7 @@ type OtherMessage struct {
 	Value            []byte        `protobuf:"bytes,2,opt,name=value" json:"value,omitempty"`
 	Weight           *float32      `protobuf:"fixed32,3,opt,name=weight" json:"weight,omitempty"`
 	Inner            *InnerMessage `protobuf:"bytes,4,opt,name=inner" json:"inner,omitempty"`
-	XXX_unrecognized []byte        `json:",omitempty"`
+	XXX_unrecognized []byte        `json:"-"`
 }
 
 func (this *OtherMessage) Reset()         { *this = OtherMessage{} }
@@ -377,8 +377,8 @@ type MyMessage struct {
 	Bikeshed         *MyMessage_Color          `protobuf:"varint,7,opt,name=bikeshed,enum=testdata.MyMessage_Color" json:"bikeshed,omitempty"`
 	Somegroup        *MyMessage_SomeGroup      `protobuf:"group,8,opt,name=SomeGroup" json:"somegroup,omitempty"`
 	RepBytes         [][]byte                  `protobuf:"bytes,10,rep,name=rep_bytes" json:"rep_bytes,omitempty"`
-	XXX_extensions   map[int32]proto.Extension `json:",omitempty"`
-	XXX_unrecognized []byte                    `json:",omitempty"`
+	XXX_extensions   map[int32]proto.Extension `json:"-"`
+	XXX_unrecognized []byte                    `json:"-"`
 }
 
 func (this *MyMessage) Reset()         { *this = MyMessage{} }
@@ -400,7 +400,7 @@ func (this *MyMessage) ExtensionMap() map[int32]proto.Extension {
 
 type MyMessage_SomeGroup struct {
 	GroupField       *int32 `protobuf:"varint,9,opt,name=group_field" json:"group_field,omitempty"`
-	XXX_unrecognized []byte `json:",omitempty"`
+	XXX_unrecognized []byte `json:"-"`
 }
 
 func (this *MyMessage_SomeGroup) Reset()         { *this = MyMessage_SomeGroup{} }
@@ -408,7 +408,7 @@ func (this *MyMessage_SomeGroup) String() string { return proto.CompactTextStrin
 
 type Ext struct {
 	Data             *string `protobuf:"bytes,1,opt,name=data" json:"data,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *Ext) Reset()         { *this = Ext{} }
@@ -440,7 +440,7 @@ var E_Ext_Number = &proto.ExtensionDesc{
 
 type MessageList struct {
 	Message          []*MessageList_Message `protobuf:"group,1,rep" json:"message,omitempty"`
-	XXX_unrecognized []byte                 `json:",omitempty"`
+	XXX_unrecognized []byte                 `json:"-"`
 }
 
 func (this *MessageList) Reset()         { *this = MessageList{} }
@@ -449,7 +449,7 @@ func (this *MessageList) String() string { return proto.CompactTextString(this)
 type MessageList_Message struct {
 	Name             *string `protobuf:"bytes,2,req,name=name" json:"name,omitempty"`
 	Count            *int32  `protobuf:"varint,3,req,name=count" json:"count,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *MessageList_Message) Reset()         { *this = MessageList_Message{} }
@@ -458,7 +458,7 @@ func (this *MessageList_Message) String() string { return proto.CompactTextStrin
 type Strings struct {
 	StringField      *string `protobuf:"bytes,1,opt,name=string_field" json:"string_field,omitempty"`
 	BytesField       []byte  `protobuf:"bytes,2,opt,name=bytes_field" json:"bytes_field,omitempty"`
-	XXX_unrecognized []byte  `json:",omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (this *Strings) Reset()         { *this = Strings{} }
@@ -483,7 +483,7 @@ type Defaults struct {
 	F_Ninf           *float32        `protobuf:"fixed32,16,opt,def=-inf" json:"F_Ninf,omitempty"`
 	F_Nan            *float32        `protobuf:"fixed32,17,opt,def=nan" json:"F_Nan,omitempty"`
 	Sub              *SubDefaults    `protobuf:"bytes,18,opt,name=sub" json:"sub,omitempty"`
-	XXX_unrecognized []byte          `json:",omitempty"`
+	XXX_unrecognized []byte          `json:"-"`
 }
 
 func (this *Defaults) Reset()         { *this = Defaults{} }
@@ -512,7 +512,7 @@ var Default_Defaults_F_Nan float32 = float32(math.NaN())
 
 type SubDefaults struct {
 	N                *int64 `protobuf:"varint,1,opt,name=n,def=7" json:"n,omitempty"`
-	XXX_unrecognized []byte `json:",omitempty"`
+	XXX_unrecognized []byte `json:"-"`
 }
 
 func (this *SubDefaults) Reset()         { *this = SubDefaults{} }
@@ -522,12 +522,20 @@ const Default_SubDefaults_N int64 = 7
 
 type RepeatedEnum struct {
 	Color            []RepeatedEnum_Color `protobuf:"varint,1,rep,name=color,enum=testdata.RepeatedEnum_Color" json:"color,omitempty"`
-	XXX_unrecognized []byte               `json:",omitempty"`
+	XXX_unrecognized []byte               `json:"-"`
 }
 
 func (this *RepeatedEnum) Reset()         { *this = RepeatedEnum{} }
 func (this *RepeatedEnum) String() string { return proto.CompactTextString(this) }
 
+var E_Greeting = &proto.ExtensionDesc{
+	ExtendedType:  (*MyMessage)(nil),
+	ExtensionType: ([]string)(nil),
+	Field:         106,
+	Name:          "testdata.greeting",
+	Tag:           "bytes,106,rep,name=greeting",
+}
+
 func init() {
 	proto.RegisterEnum("testdata.FOO", FOO_name, FOO_value)
 	proto.RegisterEnum("testdata.GoTest_KIND", GoTest_KIND_name, GoTest_KIND_value)
@@ -537,4 +545,5 @@ func init() {
 	proto.RegisterExtension(E_Ext_More)
 	proto.RegisterExtension(E_Ext_Text)
 	proto.RegisterExtension(E_Ext_Number)
+	proto.RegisterExtension(E_Greeting)
 }

proto/testdata/test.proto

@@ -248,6 +248,10 @@ message Ext {
   optional string data = 1;
 }
 
+extend MyMessage {
+  repeated string greeting = 106;
+}
+
 message MessageList {
   repeated group Message = 1 {
     required string name = 2;

proto/text.go

@@ -410,15 +410,27 @@ func writeExtensions(w *textWriter, pv reflect.Value) {
 			continue
 		}
 
-		fmt.Fprintf(w, "[%s]:", desc.Name)
-		if !w.compact {
-			w.WriteByte(' ')
+		// Repeated extensions will appear as a slice.
+		if !desc.repeated() {
+			writeExtension(w, desc.Name, pb)
+		} else {
+			v := reflect.ValueOf(pb)
+			for i := 0; i < v.Len(); i++ {
+				writeExtension(w, desc.Name, v.Index(i).Interface())
+			}
 		}
-		writeAny(w, reflect.ValueOf(pb), nil)
-		w.WriteByte('\n')
 	}
 }
 
+func writeExtension(w *textWriter, name string, pb interface{}) {
+	fmt.Fprintf(w, "[%s]:", name)
+	if !w.compact {
+		w.WriteByte(' ')
+	}
+	writeAny(w, reflect.ValueOf(pb), nil)
+	w.WriteByte('\n')
+}
+
 func marshalText(w io.Writer, pb interface{}, compact bool) {
 	if pb == nil {
 		w.Write([]byte("<nil>"))

proto/text_parser.go

@@ -350,14 +350,33 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) *ParseError
 				return err
 			}
 
+			rep := desc.repeated()
+
 			// Read the extension structure, and set it in
 			// the value we're constructing.
-			ext := reflect.New(typ).Elem()
+			var ext reflect.Value
+			if !rep {
+				ext = reflect.New(typ).Elem()
+			} else {
+				ext = reflect.New(typ.Elem()).Elem()
+			}
 			if err := p.readAny(ext, props); err != nil {
 				return err
 			}
-			SetExtension(sv.Addr().Interface().(extendableProto),
-				desc, ext.Interface())
+			ep := sv.Addr().Interface().(extendableProto)
+			if !rep {
+				SetExtension(ep, desc, ext.Interface())
+			} else {
+				old, err := GetExtension(ep, desc)
+				var sl reflect.Value
+				if err == nil {
+					sl = reflect.ValueOf(old) // existing slice
+				} else {
+					sl = reflect.MakeSlice(typ, 0, 1)
+				}
+				sl = reflect.Append(sl, ext)
+				SetExtension(ep, desc, sl.Interface())
+			}
 		} else {
 			// This is a normal, non-extension field.
 			fi, props, ok := structFieldByName(st, tok.value)

proto/text_parser_test.go

@@ -64,6 +64,16 @@ func buildExtDataTest(text string) UnmarshalTextTest {
 	return UnmarshalTextTest{in: text, out: msg}
 }
 
+func buildExtRepStringTest(text string) UnmarshalTextTest {
+	msg := &MyMessage{
+		Count: Int32(42),
+	}
+	if err := SetExtension(msg, E_Greeting, []string{"bula", "hola"}); err != nil {
+		panic(err)
+	}
+	return UnmarshalTextTest{in: text, out: msg}
+}
+
 var unMarshalTextTests = []UnmarshalTextTest{
 	// Basic
 	{
@@ -225,6 +235,7 @@ var unMarshalTextTests = []UnmarshalTextTest{
 	buildExtStructTest(`count: 42 [testdata.Ext.more]:<data:"Hello, world!" >`),
 	buildExtStructTest(`count: 42 [testdata.Ext.more] {data:"Hello, world!"}`),
 	buildExtDataTest(`count: 42 [testdata.Ext.text]:"Hello, world!" [testdata.Ext.number]:1729`),
+	buildExtRepStringTest(`count: 42 [testdata.greeting]:"bula" [testdata.greeting]:"hola"`),
 
 	// Big all-in-one
 	{

proto/text_test.go

@@ -79,6 +79,10 @@ func newTestMessage() *pb.MyMessage {
 	if err := proto.SetExtension(msg, pb.E_Ext_More, ext); err != nil {
 		panic(err)
 	}
+	greetings := []string{"adg", "easy", "cow"}
+	if err := proto.SetExtension(msg, pb.E_Greeting, greetings); err != nil {
+		panic(err)
+	}
 
 	// Add an unknown extension. We marshal a pb.Ext, and fake the ID.
 	b, err := proto.Marshal(&pb.Ext{Data: proto.String("3G skiing")})
@@ -126,6 +130,9 @@ tag13: 4
 [testdata.Ext.more]: <
   data: "Big gobs for big rats"
 >
+[testdata.greeting]: "adg"
+[testdata.greeting]: "easy"
+[testdata.greeting]: "cow"
 /* 13 unknown bytes */
 tag201: "\t3G skiing"
 /* 3 unknown bytes */

protoc-gen-go/testdata/extension_user.proto

@@ -70,6 +70,15 @@ message LoginMessage {
   }
 }
 
+// Extend with a repeated field
+extend extension_base.BaseMessage {
+  repeated Detail detail = 17;
+}
+
+message Detail {
+  optional string color = 1;
+}
+
 // An extension of an extension
 message Announcement {
   optional string words = 1;