From cdb77735690769e70cf823761f2a94779405a005 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 14 May 2019 16:05:06 -0700 Subject: [PATCH] encoding: switch ordering of Unmarshal arguments While it is general convention that the receiver being mutated is the first argument, the standard library specifically goes against this convention when it comes to the Unmarshal function. Switch the ordering of the Unmarshal function to match the Go stdlib. Change-Id: I893346680233ef9fec7104415a54a0a7ae353378 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/177258 Reviewed-by: Damien Neil --- encoding/bench_test.go | 4 ++-- encoding/jsonpb/jsonpb.go | 14 +++++++++----- encoding/protojson/bench_test.go | 2 +- encoding/protojson/decode.go | 6 +++--- encoding/protojson/decode_test.go | 2 +- encoding/prototext/decode.go | 6 +++--- encoding/prototext/decode_test.go | 2 +- encoding/prototext/other_test.go | 2 +- encoding/textpb/textpb.go | 14 +++++++++----- internal/cmd/conformance/main.go | 2 +- 10 files changed, 31 insertions(+), 23 deletions(-) diff --git a/encoding/bench_test.go b/encoding/bench_test.go index 5a541304..ab0fbbda 100644 --- a/encoding/bench_test.go +++ b/encoding/bench_test.go @@ -166,7 +166,7 @@ func BenchmarkTextDecode(b *testing.B) { if *benchV1 { err = protoV1.UnmarshalText(string(in), m) } else { - err = prototext.Unmarshal(m, in) + err = prototext.Unmarshal(in, m) } if err != nil { b.Fatal(err) @@ -203,7 +203,7 @@ func BenchmarkJSONDecode(b *testing.B) { if *benchV1 { err = jsonpbV1.UnmarshalString(string(out), m) } else { - err = protojson.Unmarshal(m, out) + err = protojson.Unmarshal(out, m) } if err != nil { b.Fatal(err) diff --git a/encoding/jsonpb/jsonpb.go b/encoding/jsonpb/jsonpb.go index 43c28b5b..4cf24d67 100644 --- a/encoding/jsonpb/jsonpb.go +++ b/encoding/jsonpb/jsonpb.go @@ -1,13 +1,17 @@ // Package jsonpb is deprecated. package jsonpb -import "google.golang.org/protobuf/encoding/protojson" - -var ( - Marshal = protojson.Marshal - Unmarshal = protojson.Unmarshal +import ( + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" ) +var Marshal = protojson.Marshal + +func Unmarshal(m proto.Message, b []byte) error { + return protojson.Unmarshal(b, m) +} + type ( MarshalOptions = protojson.MarshalOptions UnmarshalOptions = protojson.UnmarshalOptions diff --git a/encoding/protojson/bench_test.go b/encoding/protojson/bench_test.go index 595c9089..70603afa 100644 --- a/encoding/protojson/bench_test.go +++ b/encoding/protojson/bench_test.go @@ -15,7 +15,7 @@ func BenchmarkUnmarshal_Duration(b *testing.B) { input := []byte(`"-123456789.123456789s"`) for i := 0; i < b.N; i++ { - err := protojson.Unmarshal(&knownpb.Duration{}, input) + err := protojson.Unmarshal(input, &knownpb.Duration{}) if err != nil { b.Fatal(err) } diff --git a/encoding/protojson/decode.go b/encoding/protojson/decode.go index b6c056ec..99e1a8b3 100644 --- a/encoding/protojson/decode.go +++ b/encoding/protojson/decode.go @@ -21,8 +21,8 @@ import ( ) // Unmarshal reads the given []byte into the given proto.Message. -func Unmarshal(m proto.Message, b []byte) error { - return UnmarshalOptions{}.Unmarshal(m, b) +func Unmarshal(b []byte, m proto.Message) error { + return UnmarshalOptions{}.Unmarshal(b, m) } // UnmarshalOptions is a configurable JSON format parser. @@ -48,7 +48,7 @@ type UnmarshalOptions struct { // options in UnmarshalOptions object. It will clear the message first before // setting the fields. If it returns an error, the given message may be // partially set. -func (o UnmarshalOptions) Unmarshal(m proto.Message, b []byte) error { +func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error { mr := m.ProtoReflect() // 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 diff --git a/encoding/protojson/decode_test.go b/encoding/protojson/decode_test.go index 75b28d31..ba8f10bc 100644 --- a/encoding/protojson/decode_test.go +++ b/encoding/protojson/decode_test.go @@ -2596,7 +2596,7 @@ func TestUnmarshal(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.desc, func(t *testing.T) { - err := tt.umo.Unmarshal(tt.inputMessage, []byte(tt.inputText)) + err := tt.umo.Unmarshal([]byte(tt.inputText), tt.inputMessage) if err != nil && !tt.wantErr { t.Errorf("Unmarshal() returned error: %v\n\n", err) } diff --git a/encoding/prototext/decode.go b/encoding/prototext/decode.go index f9263bdd..efc4c7af 100644 --- a/encoding/prototext/decode.go +++ b/encoding/prototext/decode.go @@ -20,8 +20,8 @@ import ( ) // Unmarshal reads the given []byte into the given proto.Message. -func Unmarshal(m proto.Message, b []byte) error { - return UnmarshalOptions{}.Unmarshal(m, b) +func Unmarshal(b []byte, m proto.Message) error { + return UnmarshalOptions{}.Unmarshal(b, m) } // UnmarshalOptions is a configurable textproto format unmarshaler. @@ -41,7 +41,7 @@ type UnmarshalOptions struct { // Unmarshal reads the given []byte and populates the given proto.Message using options in // UnmarshalOptions object. -func (o UnmarshalOptions) Unmarshal(m proto.Message, b []byte) error { +func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error { var nerr errors.NonFatal mr := m.ProtoReflect() diff --git a/encoding/prototext/decode_test.go b/encoding/prototext/decode_test.go index 4837117b..20fcc947 100644 --- a/encoding/prototext/decode_test.go +++ b/encoding/prototext/decode_test.go @@ -1523,7 +1523,7 @@ type_url: "pb2.Nested" for _, tt := range tests { tt := tt t.Run(tt.desc, func(t *testing.T) { - err := tt.umo.Unmarshal(tt.inputMessage, []byte(tt.inputText)) + err := tt.umo.Unmarshal([]byte(tt.inputText), tt.inputMessage) if err != nil && !tt.wantErr { t.Errorf("Unmarshal() returned error: %v\n\n", err) } diff --git a/encoding/prototext/other_test.go b/encoding/prototext/other_test.go index 4819f7d3..82620998 100644 --- a/encoding/prototext/other_test.go +++ b/encoding/prototext/other_test.go @@ -226,7 +226,7 @@ func TestRoundTrip(t *testing.T) { } gotMessage := new(pb2.KnownTypes) - err = prototext.UnmarshalOptions{Resolver: tt.resolver}.Unmarshal(gotMessage, b) + err = prototext.UnmarshalOptions{Resolver: tt.resolver}.Unmarshal(b, gotMessage) if err != nil { t.Errorf("Unmarshal() returned error: %v\n\n", err) } diff --git a/encoding/textpb/textpb.go b/encoding/textpb/textpb.go index ba2c9ead..2c54d659 100644 --- a/encoding/textpb/textpb.go +++ b/encoding/textpb/textpb.go @@ -1,13 +1,17 @@ // Package textpb is deprecated. package textpb -import "google.golang.org/protobuf/encoding/prototext" - -var ( - Marshal = prototext.Marshal - Unmarshal = prototext.Unmarshal +import ( + "google.golang.org/protobuf/encoding/prototext" + "google.golang.org/protobuf/proto" ) +var Marshal = prototext.Marshal + +func Unmarshal(m proto.Message, b []byte) error { + return prototext.Unmarshal(b, m) +} + type ( MarshalOptions = prototext.MarshalOptions UnmarshalOptions = prototext.UnmarshalOptions diff --git a/internal/cmd/conformance/main.go b/internal/cmd/conformance/main.go index 52623761..ea54a828 100644 --- a/internal/cmd/conformance/main.go +++ b/internal/cmd/conformance/main.go @@ -71,7 +71,7 @@ func handle(req *pb.ConformanceRequest) *pb.ConformanceResponse { case *pb.ConformanceRequest_JsonPayload: err = protojson.UnmarshalOptions{ DiscardUnknown: req.TestCategory == pb.TestCategory_JSON_IGNORE_UNKNOWN_PARSING_TEST, - }.Unmarshal(msg, []byte(p.JsonPayload)) + }.Unmarshal([]byte(p.JsonPayload), msg) default: return &pb.ConformanceResponse{ Result: &pb.ConformanceResponse_RuntimeError{