Merge pull request #130 from howardjohn/oneof/fix

Fix marshalling of empty oneOf messages

Author: vmg

conformance/internal/conformance/oneof_test.go

@@ -0,0 +1,16 @@
+package conformance
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
+)
+
+func TestEmptyOneoff(t *testing.T) {
+	// Regression test for https://github.com/planetscale/vtprotobuf/issues/61
+	msg := &TestAllTypesProto3{OneofField: &TestAllTypesProto3_OneofNestedMessage{}}
+	upstream, _ := proto.Marshal(msg)
+	vt, _ := msg.MarshalVTStrict()
+	require.Equal(t, upstream, vt)
+}

conformance/internal/conformance/test_messages_proto2_vtproto.pb.go

@@ -12,7 +12,6 @@ import (
 	"strings"
 
 	"github.com/planetscale/vtprotobuf/generator"
-
 	"google.golang.org/protobuf/compiler/protogen"
 	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/reflect/protoreflect"
@@ -520,7 +519,14 @@ func (p *marshal) field(oneof bool, numGen *counter, field *protogen.Field) {
 	default:
 		panic("not implemented")
 	}
-	if repeated || nullable {
+	// Empty protobufs should emit a message or compatibility with Golang protobuf;
+	// See https://github.com/planetscale/vtprotobuf/issues/61
+	if oneof && field.Desc.Kind() == protoreflect.MessageKind && !field.Desc.IsMap() && !field.Desc.IsList() {
+		p.P("} else {")
+		p.P("i = protohelpers.EncodeVarint(dAtA, i, 0)")
+		p.encodeKey(fieldNumber, wireType)
+		p.P("}")
+	} else if repeated || nullable {
 		p.P(`}`)
 	}
 }
@@ -676,7 +682,7 @@ func (p *marshal) message(message *protogen.Message) {
 	p.P(`}`)
 	p.P()
 
-	//Generate MarshalToVT methods for oneof fields
+	// Generate MarshalToVT methods for oneof fields
 	for _, field := range message.Fields {
 		if field.Oneof == nil || field.Oneof.Desc.IsSynthetic() {
 			continue
@@ -709,7 +715,6 @@ func (p *marshal) marshalBackwardSize(varInt bool) {
 	if varInt {
 		p.encodeVarint(`size`)
 	}
-
 }
 
 func (p *marshal) marshalBackward(varName string, varInt bool, message *protogen.Message) {

conformance/internal/conformance/test_messages_proto3_vtproto.pb.go

@@ -8,11 +8,10 @@ package size
 import (
 	"strconv"
 
+	"github.com/planetscale/vtprotobuf/generator"
 	"google.golang.org/protobuf/compiler/protogen"
 	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/reflect/protoreflect"
-
-	"github.com/planetscale/vtprotobuf/generator"
 )
 
 func init() {
@@ -266,7 +265,12 @@ func (p *size) field(oneof bool, field *protogen.Field, sizeName string) {
 	default:
 		panic("not implemented")
 	}
-	if repeated || nullable {
+	// Empty protobufs should emit a message or compatibility with Golang protobuf;
+	// See https://github.com/planetscale/vtprotobuf/issues/61
+	// Size is always 3 so just hardcode that here
+	if oneof && field.Desc.Kind() == protoreflect.MessageKind && !field.Desc.IsMap() && !field.Desc.IsList() {
+		p.P("} else { n += 3 }")
+	} else if repeated || nullable {
 		p.P(`}`)
 	}
 }
@@ -310,8 +314,6 @@ func (p *size) message(message *protogen.Message) {
 				}
 				p.P(`}`)
 			} else {
-				//if _, ok := oneofs[fieldname]; !ok {
-				//oneofs[fieldname] = struct{}{}
 				p.P(`if vtmsg, ok := m.`, fieldname, `.(interface{ SizeVT() int }); ok {`)
 				p.P(`n+=vtmsg.`, sizeName, `()`)
 				p.P(`}`)