Browse Source

goprotobuf: Match C++ for text format of unknown fields.

R=r
CC=golang-dev
https://codereview.appspot.com/7982043
David Symonds 13 years ago
parent
commit
2037090d1d
6 changed files with 34 additions and 11 deletions
  1. 4 1
      proto/properties.go
  2. 1 0
      proto/size_test.go
  3. 16 0
      proto/testdata/test.proto
  4. 6 4
      proto/text.go
  5. 4 3
      proto/text_parser.go
  6. 3 3
      proto/text_test.go

+ 4 - 1
proto/properties.go

@@ -515,10 +515,13 @@ func getPropertiesLocked(t reflect.Type) *StructProperties {
 	prop.unrecField = invalidField
 	prop.Prop = make([]*Properties, t.NumField())
 	prop.order = make([]int, t.NumField())
+
 	for i := 0; i < t.NumField(); i++ {
 		f := t.Field(i)
 		p := new(Properties)
-		p.init(f.Type, f.Name, f.Tag.Get("protobuf"), &f, false)
+		name := f.Name
+		p.init(f.Type, name, f.Tag.Get("protobuf"), &f, false)
+
 		if f.Name == "XXX_extensions" { // special case
 			p.enc = (*Buffer).enc_map
 			p.dec = nil // not needed

+ 1 - 0
proto/size_test.go

@@ -54,6 +54,7 @@ func init() {
 
 	// Force messageWithExtension3 to have the extension encoded.
 	Marshal(messageWithExtension3)
+
 }
 
 var SizeTests = []struct {

+ 16 - 0
proto/testdata/test.proto

@@ -334,3 +334,19 @@ message MoreRepeated {
   repeated int32 ints_packed = 4 [packed=true];
   repeated string strings = 5;
 }
+
+// GroupOld and GroupNew have the same wire format.
+// GroupNew has a new field inside a group.
+
+message GroupOld {
+  optional group G = 1 {
+    optional int32 x = 2;
+  }
+}
+
+message GroupNew {
+  optional group G = 1 {
+    optional int32 x = 2;
+    optional int32 y = 3;
+  }
+}

+ 6 - 4
proto/text.go

@@ -183,8 +183,11 @@ func writeStruct(w *textWriter, sv reflect.Value) error {
 	sprops := GetProperties(st)
 	for i := 0; i < sv.NumField(); i++ {
 		fv := sv.Field(i)
-		if name := st.Field(i).Name; strings.HasPrefix(name, "XXX_") {
-			// There's only two XXX_ fields:
+		props := sprops.Prop[i]
+		name := st.Field(i).Name
+
+		if strings.HasPrefix(name, "XXX_") {
+			// There are two XXX_ fields:
 			//   XXX_unrecognized []byte
 			//   XXX_extensions   map[int32]proto.Extension
 			// The first is handled here;
@@ -196,7 +199,6 @@ func writeStruct(w *textWriter, sv reflect.Value) error {
 			}
 			continue
 		}
-		props := sprops.Prop[i]
 		if fv.Kind() == reflect.Ptr && fv.IsNil() {
 			// Field not filled in. This could be an optional field or
 			// a required field that wasn't filled in. Either way, there
@@ -464,7 +466,7 @@ func writeUnknownStruct(w *textWriter, data []byte) (err error) {
 			}
 			continue
 		}
-		if _, err := fmt.Fprintf(w, "tag%d", tag); err != nil {
+		if _, err := fmt.Fprint(w, tag); err != nil {
 			return err
 		}
 		if wire != WireStartGroup {

+ 4 - 3
proto/text_parser.go

@@ -505,8 +505,11 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) *ParseError
 				return p.errorf("unknown field name %q in %v", tok.value, st)
 			}
 
+			dst := sv.Field(fi)
+			isDstNil := isNil(dst)
+
 			// Check that it's not already set if it's not a repeated field.
-			if !props.Repeated && !isNil(sv.Field(fi)) {
+			if !props.Repeated && !isDstNil {
 				return p.errorf("non-repeated field %q was repeated", tok.value)
 			}
 
@@ -514,8 +517,6 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) *ParseError
 				return err
 			}
 
-			dst := sv.Field(fi)
-
 			// Parse into the field.
 			if err := p.readAny(dst, props); err != nil {
 				return err

+ 3 - 3
proto/text_test.go

@@ -128,7 +128,7 @@ SomeGroup {
   group_field: 8
 }
 /* 2 unknown bytes */
-tag13: 4
+13: 4
 [testdata.Ext.more]: <
   data: "Big gobs for big rats"
 >
@@ -136,9 +136,9 @@ tag13: 4
 [testdata.greeting]: "easy"
 [testdata.greeting]: "cow"
 /* 13 unknown bytes */
-tag201: "\t3G skiing"
+201: "\t3G skiing"
 /* 3 unknown bytes */
-tag202: 19
+202: 19
 `
 
 func TestMarshalText(t *testing.T) {