Merge pull request #528 from jmarais/mergeaa810b6

Mergeaa810b6

README

@@ -1,6 +1,6 @@
 Protocol Buffers for Go with Gadgets
 
-GoGoProtobuf http://github.com/gogo/protobuf extends 
+GoGoProtobuf http://github.com/gogo/protobuf extends
 GoProtobuf http://github.com/golang/protobuf
 
 Copyright (c) 2013, The GoGo Authors. All rights reserved.
@@ -169,16 +169,13 @@ Consider file test.proto, containing
 ```proto
 	syntax = "proto2";
 	package example;
-	
+
 	enum FOO { X = 17; };
-	
+
 	message Test {
 	  required string label = 1;
 	  optional int32 type = 2 [default=77];
 	  repeated int64 reps = 3;
-	  optional group OptionalGroup = 4 {
-	    required string RequiredField = 5;
-	  }
 	}
 ```
 
@@ -195,13 +192,10 @@ To create and play with a Test object from the example package,
 	)
 
 	func main() {
-		test := &example.Test {
+		test := &example.Test{
 			Label: proto.String("hello"),
 			Type:  proto.Int32(17),
 			Reps:  []int64{1, 2, 3},
-			Optionalgroup: &example.Test_OptionalGroup {
-				RequiredField: proto.String("good bye"),
-			},
 		}
 		data, err := proto.Marshal(test)
 		if err != nil {

proto/all_test.go

@@ -2343,6 +2343,28 @@ func TestDeterministicErrorOnCustomMarshaler(t *testing.T) {
 	}
 }
 
+func TestRequired(t *testing.T) {
+	// The F_BoolRequired field appears after all of the required fields.
+	// It should still be handled even after multiple required field violations.
+	m := &GoTest{F_BoolRequired: Bool(true)}
+	got, err := Marshal(m)
+	if _, ok := err.(*RequiredNotSetError); !ok {
+		t.Errorf("Marshal() = %v, want RequiredNotSetError error", err)
+	}
+	if want := []byte{0x50, 0x01}; !bytes.Equal(got, want) {
+		t.Errorf("Marshal() = %x, want %x", got, want)
+	}
+
+	m = new(GoTest)
+	err = Unmarshal(got, m)
+	if _, ok := err.(*RequiredNotSetError); !ok {
+		t.Errorf("Marshal() = %v, want RequiredNotSetError error", err)
+	}
+	if !m.GetF_BoolRequired() {
+		t.Error("m.F_BoolRequired = false, want true")
+	}
+}
+
 // Benchmarks
 
 func testMsg() *GoTest {

proto/table_marshal.go

@@ -279,11 +279,13 @@ func (u *marshalInfo) marshal(b []byte, ptr pointer, deterministic bool) ([]byte
 		b = append(b, s...)
 	}
 	for _, f := range u.fields {
-		if f.required && errLater == nil {
+		if f.required {
 			if f.isPointer && ptr.offset(f.field).getPointer().isNil() {
 				// Required field is not set.
 				// We record the error but keep going, to give a complete marshaling.
-				errLater = &RequiredNotSetError{f.name}
+				if errLater == nil {
+					errLater = &RequiredNotSetError{f.name}
+				}
 				continue
 			}
 		}
@@ -2825,7 +2827,7 @@ func (u *marshalInfo) appendMessageSet(b []byte, ext *XXX_InternalExtensions, de
 		p := toAddrPointer(&v, ei.isptr)
 		b, err = ei.marshaler(b, p, 3<<3|WireBytes, deterministic)
 		b = append(b, 1<<3|WireEndGroup)
-		if nerr.Merge(err) {
+		if !nerr.Merge(err) {
 			return b, err
 		}
 	}

proto/table_unmarshal.go

@@ -177,10 +177,12 @@ func (u *unmarshalInfo) unmarshal(m pointer, b []byte) error {
 				reqMask |= f.reqMask
 				continue
 			}
-			if r, ok := err.(*RequiredNotSetError); ok && errLater == nil {
+			if r, ok := err.(*RequiredNotSetError); ok {
 				// Remember this error, but keep parsing. We need to produce
 				// a full parse even if a required field is missing.
-				errLater = r
+				if errLater == nil {
+					errLater = r
+				}
 				reqMask |= f.reqMask
 				continue
 			}