goprotobuf: Don't treat groups as messages.
They no longer get XXX_unrecognized fields, nor satisfy the proto.Message interface.
R=nigeltao, r
CC=golang-dev
http://codereview.appspot.com/6829043
diff --git a/proto/clone.go b/proto/clone.go
index b23f46b..71c0f52 100644
--- a/proto/clone.go
+++ b/proto/clone.go
@@ -66,7 +66,12 @@
copyExtension(emOut.ExtensionMap(), emIn.ExtensionMap())
}
- uin := in.FieldByName("XXX_unrecognized").Bytes()
+ // Groups don't have XXX_unrecognized fields.
+ uf := in.FieldByName("XXX_unrecognized")
+ if !uf.IsValid() {
+ return
+ }
+ uin := uf.Bytes()
if len(uin) > 0 {
out.FieldByName("XXX_unrecognized").SetBytes(append([]byte(nil), uin...))
}
diff --git a/proto/clone_test.go b/proto/clone_test.go
index e6edcf3..5005542 100644
--- a/proto/clone_test.go
+++ b/proto/clone_test.go
@@ -53,6 +53,9 @@
Value: []byte("some bytes"),
},
},
+ Somegroup: &pb.MyMessage_SomeGroup{
+ GroupField: proto.Int32(6),
+ },
RepBytes: [][]byte{[]byte("sham"), []byte("wow")},
}
diff --git a/proto/equal.go b/proto/equal.go
index 8c26e75..11ea6ed 100644
--- a/proto/equal.go
+++ b/proto/equal.go
@@ -126,7 +126,13 @@
}
}
- u1 := v1.FieldByName("XXX_unrecognized").Bytes()
+ // Groups don't have XXX_unrecognized.
+ uf := v1.FieldByName("XXX_unrecognized")
+ if !uf.IsValid() {
+ return true
+ }
+
+ u1 := uf.Bytes()
u2 := v2.FieldByName("XXX_unrecognized").Bytes()
if !bytes.Equal(u1, u2) {
return false
diff --git a/proto/equal_test.go b/proto/equal_test.go
index 0096aa9..574e52e 100644
--- a/proto/equal_test.go
+++ b/proto/equal_test.go
@@ -138,6 +138,23 @@
{"int32 extension vs. itself", messageWithInt32Extension1, messageWithInt32Extension1, true},
{"int32 extension vs. a different int32", messageWithInt32Extension1, messageWithInt32Extension2, false},
+
+ {
+ "message with group",
+ &pb.MyMessage{
+ Count: Int32(1),
+ Somegroup: &pb.MyMessage_SomeGroup{
+ GroupField: Int32(5),
+ },
+ },
+ &pb.MyMessage{
+ Count: Int32(1),
+ Somegroup: &pb.MyMessage_SomeGroup{
+ GroupField: Int32(5),
+ },
+ },
+ true,
+ },
}
func TestEqual(t *testing.T) {
diff --git a/protoc-gen-go/generator/generator.go b/protoc-gen-go/generator/generator.go
index bb51c79..7bf7b39 100644
--- a/protoc-gen-go/generator/generator.go
+++ b/protoc-gen-go/generator/generator.go
@@ -104,6 +104,7 @@
ext []*ExtensionDescriptor // Extensions, if any.
typename []string // Cached typename vector.
index int // If a top-level message, the index into message_type.
+ group bool
}
// TypeName returns the elements of the dotted type name.
@@ -647,7 +648,23 @@
// Construct the Descriptor and add it to the slice
func addDescriptor(sl []*Descriptor, desc *descriptor.DescriptorProto, parent *Descriptor, file *descriptor.FileDescriptorProto, index int) []*Descriptor {
- d := &Descriptor{common{file}, desc, parent, nil, nil, nil, index}
+ d := &Descriptor{common{file}, desc, parent, nil, nil, nil, index, false}
+
+ // The only way to distinguish a group from a message is whether
+ // the containing message has a TYPE_GROUP field that matches.
+ if parent != nil {
+ parts := d.TypeName()
+ if file.Package != nil {
+ parts = append([]string{*file.Package}, parts...)
+ }
+ exp := "." + strings.Join(parts, ".")
+ for _, field := range parent.Field {
+ if field.GetType() == descriptor.FieldDescriptorProto_TYPE_GROUP && field.GetTypeName() == exp {
+ d.group = true
+ break
+ }
+ }
+ }
d.ext = make([]*ExtensionDescriptor, len(desc.Extension))
for i, field := range desc.Extension {
@@ -1303,6 +1320,7 @@
fieldNames := make(map[*descriptor.FieldDescriptorProto]string)
g.P("type ", ccTypeName, " struct {")
g.In()
+
for _, field := range message.Field {
fieldname := CamelCase(*field.Name)
for usedNames[fieldname] {
@@ -1312,21 +1330,25 @@
fieldNames[field] = fieldname
typename, wiretype := g.GoType(message, field)
jsonName := *field.Name
- tag := fmt.Sprintf("`protobuf:%s json:%q`", g.goTag(field, wiretype), jsonName+",omitempty")
- g.P(fieldname, "\t", typename, "\t", tag)
+ tag := fmt.Sprintf("protobuf:%s json:%q", g.goTag(field, wiretype), jsonName+",omitempty")
+ g.P(fieldname, "\t", typename, "\t`", tag, "`")
g.RecordTypeUse(field.GetTypeName())
}
if len(message.ExtensionRange) > 0 {
g.P("XXX_extensions\t\tmap[int32]", g.Pkg["proto"], ".Extension `json:\"-\"`")
}
- g.P("XXX_unrecognized\t[]byte `json:\"-\"`")
+ if !message.group {
+ g.P("XXX_unrecognized\t[]byte `json:\"-\"`")
+ }
g.Out()
g.P("}")
// Reset, String and ProtoMessage methods.
g.P("func (this *", ccTypeName, ") Reset() { *this = ", ccTypeName, "{} }")
- g.P("func (this *", ccTypeName, ") String() string { return ", g.Pkg["proto"], ".CompactTextString(this) }")
- g.P("func (*", ccTypeName, ") ProtoMessage() {}")
+ if !message.group {
+ g.P("func (this *", ccTypeName, ") String() string { return ", g.Pkg["proto"], ".CompactTextString(this) }")
+ g.P("func (*", ccTypeName, ") ProtoMessage() {}")
+ }
// Extension support methods
var hasExtensions, isMessageSet bool
@@ -1443,7 +1465,8 @@
}
// Only export getter symbols for basic types,
- // and for messages, groups and enums in the same package.
+ // and for messages and enums in the same package.
+ // Groups are not exported.
// Foreign types can't be hoisted through a public import because
// the importer may not already be importing the defining .proto.
// As an example, imagine we have an import tree like this:
@@ -1453,8 +1476,9 @@
// because A is not importing C already.
var getter, genType bool
switch *field.Type {
- case descriptor.FieldDescriptorProto_TYPE_GROUP, descriptor.FieldDescriptorProto_TYPE_MESSAGE,
- descriptor.FieldDescriptorProto_TYPE_ENUM:
+ case descriptor.FieldDescriptorProto_TYPE_GROUP:
+ getter = false
+ case descriptor.FieldDescriptorProto_TYPE_MESSAGE, descriptor.FieldDescriptorProto_TYPE_ENUM:
// Only export getter if its return type is in this package.
getter = g.ObjectNamed(field.GetTypeName()).PackageName() == message.PackageName()
genType = true
@@ -1522,7 +1546,9 @@
g.P()
}
- g.file.addExport(message, &messageSymbol{ccTypeName, hasExtensions, isMessageSet, getters})
+ if !message.group {
+ g.file.addExport(message, &messageSymbol{ccTypeName, hasExtensions, isMessageSet, getters})
+ }
for _, ext := range message.ext {
g.generateExtension(ext)
diff --git a/protoc-gen-go/testdata/my_test/test.pb.go b/protoc-gen-go/testdata/my_test/test.pb.go
index b6a93da..4a465f7 100644
--- a/protoc-gen-go/testdata/my_test/test.pb.go
+++ b/protoc-gen-go/testdata/my_test/test.pb.go
@@ -219,13 +219,10 @@
}
type Request_SomeGroup struct {
- GroupField *int32 `protobuf:"varint,9,opt,name=group_field" json:"group_field,omitempty"`
- XXX_unrecognized []byte `json:"-"`
+ GroupField *int32 `protobuf:"varint,9,opt,name=group_field" json:"group_field,omitempty"`
}
-func (this *Request_SomeGroup) Reset() { *this = Request_SomeGroup{} }
-func (this *Request_SomeGroup) String() string { return proto.CompactTextString(this) }
-func (*Request_SomeGroup) ProtoMessage() {}
+func (this *Request_SomeGroup) Reset() { *this = Request_SomeGroup{} }
func (this *Request_SomeGroup) GetGroupField() int32 {
if this != nil && this.GroupField != nil {
diff --git a/protoc-gen-go/testdata/my_test/test.pb.go.golden b/protoc-gen-go/testdata/my_test/test.pb.go.golden
index b6a93da..4a465f7 100644
--- a/protoc-gen-go/testdata/my_test/test.pb.go.golden
+++ b/protoc-gen-go/testdata/my_test/test.pb.go.golden
@@ -219,13 +219,10 @@
}
type Request_SomeGroup struct {
- GroupField *int32 `protobuf:"varint,9,opt,name=group_field" json:"group_field,omitempty"`
- XXX_unrecognized []byte `json:"-"`
+ GroupField *int32 `protobuf:"varint,9,opt,name=group_field" json:"group_field,omitempty"`
}
-func (this *Request_SomeGroup) Reset() { *this = Request_SomeGroup{} }
-func (this *Request_SomeGroup) String() string { return proto.CompactTextString(this) }
-func (*Request_SomeGroup) ProtoMessage() {}
+func (this *Request_SomeGroup) Reset() { *this = Request_SomeGroup{} }
func (this *Request_SomeGroup) GetGroupField() int32 {
if this != nil && this.GroupField != nil {