proto: reset message by default in Unmarshal

We change Unmarshal to reset a message by default.
* We add a Merge option to UnmarshalOptions for explicit merging.
* We speed up Reset by checking for the Reset method.
* Remove TODOs in prototext and protojson about reset behavior.

Fixes golang/protobuf#890

Change-Id: Ibd8963c741053f564acf061fbdb846699942109c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/195457
Reviewed-by: Damien Neil <dneil@google.com>

encoding/protojson/decode.go

@@ -54,9 +54,6 @@ type UnmarshalOptions struct {
 // setting the fields. If it returns an error, the given message may be
 // partially set.
 func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error {
-	// TODO: Determine if we would like to have an option for merging or only
-	// have merging behavior. We should at least be consistent with textproto
-	// marshaling.
 	proto.Reset(m)
 
 	if o.Resolver == nil {

encoding/prototext/decode.go

@@ -53,10 +53,6 @@ type UnmarshalOptions struct {
 // Unmarshal reads the given []byte and populates the given proto.Message using options in
 // UnmarshalOptions object.
 func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error {
-	// Clear all fields before populating it.
-	// TODO: Determine if this needs to be consistent with protojson and binary unmarshal where
-	// behavior is to merge values into existing message. If decision is to not clear the fields
-	// ahead, code will need to be updated properly when merging nested messages.
 	proto.Reset(m)
 
 	// Parse into text.Value of message type.

internal/impl/decode.go

@@ -40,6 +40,7 @@ func newUnmarshalOptions(opts piface.UnmarshalOptions) unmarshalOptions {
 
 func (o unmarshalOptions) Options() proto.UnmarshalOptions {
 	return proto.UnmarshalOptions{
+		Merge:          true,
 		AllowPartial:   true,
 		DiscardUnknown: o.DiscardUnknown(),
 		Resolver:       o.Resolver(),

proto/decode.go

@@ -21,6 +21,11 @@ import (
 type UnmarshalOptions struct {
 	pragma.NoUnkeyedLiterals
 
+	// Merge merges the input into the destination message.
+	// The default behavior is to always reset the message before unmarshaling,
+	// unless Merge is specified.
+	Merge bool
+
 	// AllowPartial accepts input for messages that will result in missing
 	// required fields. If AllowPartial is false (the default), Unmarshal will
 	// return an error if there are any missing required fields.
@@ -49,7 +54,9 @@ func (o UnmarshalOptions) Unmarshal(b []byte, m Message) error {
 		o.Resolver = protoregistry.GlobalTypes
 	}
 
-	// TODO: Reset m?
+	if !o.Merge {
+		Reset(m)
+	}
 	err := o.unmarshalMessage(b, m.ProtoReflect())
 	if err != nil {
 		return err

proto/proto_methods.go

@@ -12,6 +12,8 @@ import (
 	"google.golang.org/protobuf/runtime/protoiface"
 )
 
+const hasProtoMethods = true
+
 func protoMethods(m protoreflect.Message) *protoiface.Methods {
 	if x, ok := m.(protoiface.Methoder); ok {
 		return x.ProtoMethods()

proto/proto_reflect.go

@@ -12,6 +12,8 @@ import (
 	"google.golang.org/protobuf/runtime/protoiface"
 )
 
+const hasProtoMethods = false
+
 func protoMethods(m protoreflect.Message) *protoiface.Methods {
 	return nil
 }

proto/reset.go

@@ -9,7 +9,10 @@ import "google.golang.org/protobuf/reflect/protoreflect"
 // Reset clears every field in the message.
 func Reset(m Message) {
 	// TODO: Document memory aliasing guarantees.
-	// TODO: Add fast-path for reset?
+	if mr, ok := m.(interface{ Reset() }); ok && hasProtoMethods {
+		mr.Reset()
+		return
+	}
 	resetMessage(m.ProtoReflect())
 }
 

runtime/protoiface/methods.go

@@ -73,6 +73,7 @@ type MarshalOptions struct {
 type UnmarshalOptions struct {
 	pragma.NoUnkeyedLiterals
 
+	Merge          bool // must be treated as true by method implementations
 	AllowPartial   bool // must be treated as true by method implementations
 	DiscardUnknown bool
 	Resolver       interface {