Merge pull request #55 from misberner/mi/fix-clone-unknownfields

clone: Take unknown fields into account

conformance/internal/conformance/clonevt_test.go

@@ -4,7 +4,9 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/structpb"
 	"google.golang.org/protobuf/types/known/wrapperspb"
@@ -97,3 +99,29 @@ func TestCloneVT3(t *testing.T) {
 	require.False(t, clone.EqualVT(msg), "cloned message unchanged after mutation")
 	require.True(t, orig.EqualVT(msg), "mutating cloned %T mutated original:\nmsg = %+v\nafter clone = %+v\n", msg, msg, orig)
 }
+
+func TestCloneVT_UnknownFields(t *testing.T) {
+	msg := &TestAllTypesProto3{
+		OptionalInt32: 42,
+	}
+
+	const unknownFieldNumber = 1337
+	require.Nil(t, msg.ProtoReflect().Descriptor().Fields().ByNumber(unknownFieldNumber),
+		"if this assertion fails, please change the above constant to a field number not used in the proto")
+	data, err := msg.MarshalVT()
+	require.NoError(t, err)
+
+	data = protowire.AppendTag(data, unknownFieldNumber, protowire.BytesType)
+	data = protowire.AppendString(data, "foo bar baz")
+
+	unmarshaled := new(TestAllTypesProto3)
+	require.NoError(t, unmarshaled.UnmarshalVT(data), "unmarshaling should succeed")
+
+	cloned := unmarshaled.CloneVT()
+	assert.Truef(t, proto.Equal(unmarshaled, cloned), "expected %T to be equal:\nunmarshaled = %+v\ncloned = %+v\n", unmarshaled, unmarshaled, cloned)
+
+	protoCloned := proto.Clone(unmarshaled).(*TestAllTypesProto3)
+	require.True(t, proto.Equal(unmarshaled, protoCloned), "proto.Clone is misbehaving")
+
+	assert.Truef(t, proto.Equal(cloned, protoCloned), "expected %T to be equal:\ncloned = %+v\nprotoCloned = %+v\n", cloned, cloned, protoCloned)
+}

features/clone/clone.go

@@ -144,7 +144,7 @@ func (p *clone) cloneField(lhsBase, rhsBase string, allFieldsNullable bool, fiel
 func (p *clone) generateCloneMethodsForMessage(proto3 bool, message *protogen.Message) {
 	ccTypeName := message.GoIdent.GoName
 	p.P(`func (m *`, ccTypeName, `) `, cloneName, `() *`, ccTypeName, ` {`)
-	p.body(!proto3, ccTypeName, message.Fields)
+	p.body(!proto3, ccTypeName, message.Fields, true)
 	p.P(`}`)
 	p.P()
 	p.P(`func (m *`, ccTypeName, `) `, cloneGenericName, `() `, protoPkg.Ident("Message"), ` {`)
@@ -156,7 +156,7 @@ func (p *clone) generateCloneMethodsForMessage(proto3 bool, message *protogen.Me
 // body generates the code for the actual cloning logic of a structure containing the given fields.
 // In practice, those can be the fields of a message, or of a oneof struct.
 // The object to be cloned is assumed to be called "m".
-func (p *clone) body(allFieldsNullable bool, ccTypeName string, fields []*protogen.Field) {
+func (p *clone) body(allFieldsNullable bool, ccTypeName string, fields []*protogen.Field, cloneUnknownFields bool) {
 	// The method body for a message or a oneof wrapper always starts with a nil check.
 	p.P(`if m == nil {`)
 	// We use an explicitly typed nil to avoid returning the nil interface in the oneof wrapper
@@ -200,6 +200,14 @@ func (p *clone) body(allFieldsNullable bool, ccTypeName string, fields []*protog
 		p.cloneField("r", "m", allFieldsNullable, field)
 	}
 
+	if cloneUnknownFields {
+		// Clone unknown fields, if any
+		p.P(`if len(m.unknownFields) > 0 {`)
+		p.P(`r.unknownFields = make([]byte, len(m.unknownFields))`)
+		p.P(`copy(r.unknownFields, m.unknownFields)`)
+		p.P(`}`)
+	}
+
 	p.P(`return r`)
 }
 
@@ -214,7 +222,7 @@ func (p *clone) generateCloneMethodsForOneof(field *protogen.Field) {
 	fieldInOneof := *field
 	fieldInOneof.Oneof = nil
 	// If we have a scalar field in a oneof, that field is never nullable, even when using proto2
-	p.body(false, ccTypeName, []*protogen.Field{&fieldInOneof})
+	p.body(false, ccTypeName, []*protogen.Field{&fieldInOneof}, false)
 	p.P(`}`)
 	p.P()
 }