proto: distinguish between invalid and empty messages in Equal
The v1 proto.Equal function treats (*Message)(nil) and new(Message)
as being different, while v2 proto.Equal treated them as equal since
a typed nil pointer is functionally an empty message since the
protobuf data model has no concept of presence as a first-class
property of messages.
Unfortunately, a significant amount of code depends on this distinction
that it would be difficult to migrate users from v1 to v2 unless we
preserved similar semantics in the v2 proto.Equal.
Also, double down on these semantics for protocmp.Transform.
Fixes #965
Change-Id: I21e78ba6251401a0ac0ccf495188093973cd7f3f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213238
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/proto/decode_test.go b/proto/decode_test.go
index 8140d79..02f07d4 100644
--- a/proto/decode_test.go
+++ b/proto/decode_test.go
@@ -40,7 +40,7 @@
for i := range wire {
wire[i] = 0
}
- if !proto.Equal(got, want) {
+ if !proto.Equal(got, want) && got.ProtoReflect().IsValid() && want.ProtoReflect().IsValid() {
t.Errorf("Unmarshal returned unexpected result; got:\n%v\nwant:\n%v", marshalText(got), marshalText(want))
}
})
diff --git a/proto/encode_test.go b/proto/encode_test.go
index 8f02d0c..fd03468 100644
--- a/proto/encode_test.go
+++ b/proto/encode_test.go
@@ -45,7 +45,7 @@
t.Errorf("Unmarshal error: %v\nMessage:\n%v", err, marshalText(want))
return
}
- if !proto.Equal(got, want) {
+ if !proto.Equal(got, want) && got.ProtoReflect().IsValid() && want.ProtoReflect().IsValid() {
t.Errorf("Unmarshal returned unexpected result; got:\n%v\nwant:\n%v", marshalText(got), marshalText(want))
}
})
@@ -81,7 +81,7 @@
t.Errorf("Unmarshal error: %v\nMessage:\n%v", err, marshalText(want))
return
}
- if !proto.Equal(got, want) {
+ if !proto.Equal(got, want) && got.ProtoReflect().IsValid() && want.ProtoReflect().IsValid() {
t.Errorf("Unmarshal returned unexpected result; got:\n%v\nwant:\n%v", marshalText(got), marshalText(want))
}
})
diff --git a/proto/equal.go b/proto/equal.go
index 7b513bd..32ba946 100644
--- a/proto/equal.go
+++ b/proto/equal.go
@@ -19,7 +19,8 @@
//
// Two messages are equal if they belong to the same message descriptor,
// have the same set of populated known and extension field values,
-// and the same set of unknown fields values.
+// and the same set of unknown fields values. If either of the top-level
+// messages are invalid, then Equal reports true only if both are invalid.
//
// Scalar values are compared with the equivalent of the == operator in Go,
// except bytes values which are compared using bytes.Equal and
@@ -32,7 +33,12 @@
if x == nil || y == nil {
return x == nil && y == nil
}
- return equalMessage(x.ProtoReflect(), y.ProtoReflect())
+ mx := x.ProtoReflect()
+ my := y.ProtoReflect()
+ if !mx.IsValid() || !my.IsValid() {
+ return !mx.IsValid() && !my.IsValid()
+ }
+ return equalMessage(mx, my)
}
// equalMessage compares two messages.
diff --git a/proto/equal_test.go b/proto/equal_test.go
index add1687..da3ff2c 100644
--- a/proto/equal_test.go
+++ b/proto/equal_test.go
@@ -30,6 +30,14 @@
eq: false,
}, {
x: (*testpb.TestAllTypes)(nil),
+ y: (*testpb.TestAllTypes)(nil),
+ eq: true,
+ }, {
+ x: new(testpb.TestAllTypes),
+ y: (*testpb.TestAllTypes)(nil),
+ eq: false,
+ }, {
+ x: new(testpb.TestAllTypes),
y: new(testpb.TestAllTypes),
eq: true,
},
diff --git a/testing/protocmp/xform.go b/testing/protocmp/xform.go
index e7c0fb9..daba15c 100644
--- a/testing/protocmp/xform.go
+++ b/testing/protocmp/xform.go
@@ -153,18 +153,12 @@
}
return false
- }, cmp.Transformer("protocmp.Transform", func(m interface{}) Message {
- if m == nil {
+ }, cmp.Transformer("protocmp.Transform", func(v interface{}) Message {
+ m := protoimpl.X.MessageOf(v)
+ if m == nil || !m.IsValid() {
return nil
}
-
- // TODO: Should typed nil messages result in a nil Message?
- // For now, do so as it is easier to remove this check than to add it.
- if v := reflect.ValueOf(m); v.Kind() == reflect.Ptr && v.IsNil() {
- return nil
- }
-
- return transformMessage(protoimpl.X.MessageOf(m))
+ return transformMessage(m)
}))
}
diff --git a/testing/protocmp/xform_test.go b/testing/protocmp/xform_test.go
index b7cc496..189af9e 100644
--- a/testing/protocmp/xform_test.go
+++ b/testing/protocmp/xform_test.go
@@ -22,7 +22,6 @@
}
func TestTransform(t *testing.T) {
- t.Skip()
tests := []struct {
in proto.Message
want Message
