diff options
author | David Symonds <dsymonds@golang.org> | 2012-11-07 11:42:56 +1100 |
---|---|---|
committer | David Symonds <dsymonds@golang.org> | 2012-11-07 11:42:56 +1100 |
commit | 6eaeef1e28adc576df0c3e2396e9d132d6273ea2 (patch) | |
tree | d8e4bc32d862a176b6e69f58b18339d9418d2515 | |
parent | 2bba1b298b2bf3139277f4d033014fe81320a3ba (diff) | |
download | protobuf-6eaeef1e28adc576df0c3e2396e9d132d6273ea2.tar.gz |
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
-rw-r--r-- | proto/clone.go | 7 | ||||
-rw-r--r-- | proto/clone_test.go | 3 | ||||
-rw-r--r-- | proto/equal.go | 8 | ||||
-rw-r--r-- | proto/equal_test.go | 17 | ||||
-rw-r--r-- | protoc-gen-go/generator/generator.go | 46 | ||||
-rw-r--r-- | protoc-gen-go/testdata/my_test/test.pb.go | 7 | ||||
-rw-r--r-- | protoc-gen-go/testdata/my_test/test.pb.go.golden | 7 |
7 files changed, 73 insertions, 22 deletions
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 @@ func copyStruct(out, in reflect.Value) { 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 @@ var cloneTestMessage = &pb.MyMessage{ 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 @@ func equalStruct(v1, v2 reflect.Value) bool { } } - 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 @@ var EqualTests = []struct { {"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 @@ type Descriptor struct { 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 @@ func (g *Generator) buildNestedDescriptors(descs []*Descriptor) { // 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 @@ func (g *Generator) generateMessage(message *Descriptor) { 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 @@ func (g *Generator) generateMessage(message *Descriptor) { 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 @@ func (g *Generator) generateMessage(message *Descriptor) { } // 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 @@ func (g *Generator) generateMessage(message *Descriptor) { // 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 @@ func (g *Generator) generateMessage(message *Descriptor) { 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 @@ func (this *Request) GetReset_() int32 { } 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 @@ func (this *Request) GetReset_() int32 { } 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 { |