Fix equality of messages to include unknown fields

src/ProtocolBuffers.Test/AbstractMessageTest.cs

@@ -210,6 +210,17 @@ namespace Google.ProtocolBuffers {
       CheckNotEqual(d, f);
 
       CheckNotEqual(e, f);
+
+      // Deserializing into the TestEmptyMessage such that every field is an UnknownFieldSet.Field
+      TestEmptyMessage eUnknownFields = TestEmptyMessage.ParseFrom(e.ToByteArray());
+      TestEmptyMessage fUnknownFields = TestEmptyMessage.ParseFrom(f.ToByteArray());
+      CheckNotEqual(eUnknownFields, fUnknownFields);
+      CheckEqualsIsConsistent(eUnknownFields);
+      CheckEqualsIsConsistent(fUnknownFields);
+
+      // Subseqent reconstitutions should be identical
+      TestEmptyMessage eUnknownFields2 = TestEmptyMessage.ParseFrom(e.ToByteArray());
+      CheckEqualsIsConsistent(eUnknownFields, eUnknownFields2);
     }
     
     /// <summary>
@@ -221,9 +232,16 @@ namespace Google.ProtocolBuffers {
       
       // Object should be equal to a dynamic copy of itself.
       DynamicMessage dynamic = DynamicMessage.CreateBuilder(message).Build();
-      Assert.AreEqual(message, dynamic);
-      Assert.AreEqual(dynamic, message);
-      Assert.AreEqual(dynamic.GetHashCode(), message.GetHashCode());
+      CheckEqualsIsConsistent(message, dynamic);
+    }
+
+    /// <summary>
+    /// Asserts that the given protos are equal and have the same hash code.
+    /// </summary>
+    private static void CheckEqualsIsConsistent(IMessage message1, IMessage message2) {
+      Assert.AreEqual(message1, message2);
+      Assert.AreEqual(message2, message1);
+      Assert.AreEqual(message2.GetHashCode(), message1.GetHashCode());
     }
 
     /// <summary>

src/ProtocolBuffers.Test/UnknownFieldSetTest.cs

@@ -326,5 +326,72 @@ namespace Google.ProtocolBuffers {
       Assert.AreEqual(1, field.VarintList.Count);
       Assert.AreEqual(0x7FFFFFFFFFFFFFFFUL, field.VarintList[0]);
     }
+
+    [Test]
+    public void EqualsAndHashCode() {
+      UnknownField fixed32Field = UnknownField.CreateBuilder().AddFixed32(1).Build();
+      UnknownField fixed64Field = UnknownField.CreateBuilder().AddFixed64(1).Build();
+      UnknownField varIntField = UnknownField.CreateBuilder().AddVarint(1).Build();
+      UnknownField lengthDelimitedField = UnknownField.CreateBuilder().AddLengthDelimited(ByteString.Empty).Build();
+      UnknownField groupField = UnknownField.CreateBuilder().AddGroup(unknownFields).Build();
+
+      UnknownFieldSet a = UnknownFieldSet.CreateBuilder().AddField(1, fixed32Field).Build();
+      UnknownFieldSet b = UnknownFieldSet.CreateBuilder().AddField(1, fixed64Field).Build();
+      UnknownFieldSet c = UnknownFieldSet.CreateBuilder().AddField(1, varIntField).Build();
+      UnknownFieldSet d = UnknownFieldSet.CreateBuilder().AddField(1, lengthDelimitedField).Build();
+      UnknownFieldSet e = UnknownFieldSet.CreateBuilder().AddField(1, groupField).Build();
+
+      CheckEqualsIsConsistent(a);
+      CheckEqualsIsConsistent(b);
+      CheckEqualsIsConsistent(c);
+      CheckEqualsIsConsistent(d);
+      CheckEqualsIsConsistent(e);
+
+      CheckNotEqual(a, b);
+      CheckNotEqual(a, c);
+      CheckNotEqual(a, d);
+      CheckNotEqual(a, e);
+      CheckNotEqual(b, c);
+      CheckNotEqual(b, d);
+      CheckNotEqual(b, e);
+      CheckNotEqual(c, d);
+      CheckNotEqual(c, e);
+      CheckNotEqual(d, e);
+    }
+
+    /// <summary>
+    /// Asserts that the given field sets are not equal and have different
+    /// hash codes.
+    /// </summary>
+    /// <remarks>
+    /// It's valid for non-equal objects to have the same hash code, so
+    /// this test is stricter than it needs to be. However, this should happen
+    /// relatively rarely.
+    /// </remarks>
+    /// <param name="s1"></param>
+    /// <param name="s2"></param>
+    private static void CheckNotEqual(UnknownFieldSet s1, UnknownFieldSet s2) {
+      String equalsError = string.Format("{0} should not be equal to {1}", s1, s2);
+      Assert.IsFalse(s1.Equals(s2), equalsError);
+      Assert.IsFalse(s2.Equals(s1), equalsError);
+
+      Assert.IsFalse(s1.GetHashCode() == s2.GetHashCode(),
+          string.Format("{0} should have a different hash code from {1}", s1, s2));
+          
+    }
+
+    /**
+     * Asserts that the given field sets are equal and have identical hash codes.
+     */
+    private static void CheckEqualsIsConsistent(UnknownFieldSet set) {
+      // Object should be equal to itself.
+      Assert.AreEqual(set, set);
+
+      // Object should be equal to a copy of itself.
+      UnknownFieldSet copy = UnknownFieldSet.CreateBuilder(set).Build();
+      Assert.AreEqual(set, copy);
+      Assert.AreEqual(copy, set);
+      Assert.AreEqual(set.GetHashCode(), copy.GetHashCode());
+    }
   }
 }

src/ProtocolBuffers/AbstractMessage.cs

@@ -213,13 +213,14 @@ namespace Google.ProtocolBuffers {
       if (otherMessage == null || otherMessage.DescriptorForType != DescriptorForType) {
         return false;
       }
-      return Dictionaries.Equals(AllFields, otherMessage.AllFields);
+      return Dictionaries.Equals(AllFields, otherMessage.AllFields) && UnknownFields.Equals(otherMessage.UnknownFields);
     }
 
     public override int GetHashCode() {
       int hash = 41;
       hash = (19 * hash) + DescriptorForType.GetHashCode();
       hash = (53 * hash) + Dictionaries.GetHashCode(AllFields);
+      hash = (29 * hash) + UnknownFields.GetHashCode();
       return hash;
     }
   }

src/ProtocolBuffers/Collections/Dictionaries.cs

@@ -36,8 +36,7 @@ using System.Collections.Generic;
 namespace Google.ProtocolBuffers.Collections {
 
   /// <summary>
-  /// Non-generic class with generic methods which proxy to the non-generic methods
-  /// in the generic class.
+  /// Utility class for dictionaries.
   /// </summary>
   public static class Dictionaries {
 
@@ -60,27 +59,12 @@ namespace Google.ProtocolBuffers.Collections {
         IEnumerable leftEnumerable = leftEntry.Value as IEnumerable;
         IEnumerable rightEnumerable = rightValue as IEnumerable;
         if (leftEnumerable == null || rightEnumerable == null) {
-          if (!object.Equals(leftEntry.Value, rightValue)) {
+          if (!Equals(leftEntry.Value, rightValue)) {
             return false;
           }
         } else {
-          IEnumerator leftEnumerator = leftEnumerable.GetEnumerator();
-          try {
-            foreach (object rightObject in rightEnumerable) {
-              if (!leftEnumerator.MoveNext()) {
-                return false;
-              }
-              if (!object.Equals(leftEnumerator.Current, rightObject)) {
-                return false;
-              }
-            }
-            if (leftEnumerator.MoveNext()) {
-              return false;
-            }
-          } finally {
-            if (leftEnumerator is IDisposable) {
-              ((IDisposable)leftEnumerator).Dispose();
-            }
+          if (!Enumerables.Equals(leftEnumerable, rightEnumerable)) {
+            return false;
           }
         }
       }

src/ProtocolBuffers/Collections/Enumerables.cs

+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc.  All rights reserved.
+// http://github.com/jskeet/dotnet-protobufs/
+// Original C++/Java/Python code:
+// http://code.google.com/p/protobuf/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+using System;
+using System.Collections;
+
+namespace Google.ProtocolBuffers.Collections {
+  /// <summary>
+  /// Utility class for IEnumerable (and potentially the generic version in the future).
+  /// </summary>
+  public static class Enumerables {
+    public static bool Equals(IEnumerable left, IEnumerable right) {
+      IEnumerator leftEnumerator = left.GetEnumerator();
+      try {
+        foreach (object rightObject in right) {
+          if (!leftEnumerator.MoveNext()) {
+            return false;
+          }
+          if (!Equals(leftEnumerator.Current, rightObject)) {
+            return false;
+          }
+        }
+        if (leftEnumerator.MoveNext()) {
+          return false;
+        }
+      } finally {
+        IDisposable leftEnumeratorDisposable = leftEnumerator as IDisposable;
+        if (leftEnumeratorDisposable != null) {
+          leftEnumeratorDisposable.Dispose();
+        }
+      }
+      return true;
+    }
+  }
+}

src/ProtocolBuffers/Collections/Lists.cs

@@ -45,6 +45,33 @@ namespace Google.ProtocolBuffers.Collections {
     public static IList<T> AsReadOnly<T>(IList<T> list) {
       return Lists<T>.AsReadOnly(list);
     }
+
+    public static bool Equals<T>(IList<T> left, IList<T> right) {
+      if (left == right) {
+        return true;
+      }
+      if (left == null || right == null) {
+        return false;
+      }
+      if (left.Count != right.Count) {
+        return false;
+      }
+      IEqualityComparer<T> comparer = EqualityComparer<T>.Default;
+      for (int i = 0; i < left.Count; i++) {
+        if (!comparer.Equals(left[i], right[i])) {
+          return false;
+        }
+      }
+      return true;
+    }
+
+    public static int GetHashCode<T>(IList<T> list) {
+      int hash = 31;
+      foreach (T element in list) {
+        hash = hash * 29 + element.GetHashCode();
+      }
+      return hash;
+    }
   }
 
   /// <summary>

src/ProtocolBuffers/Collections/PopsicleList.cs

+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc.  All rights reserved.
+// http://github.com/jskeet/dotnet-protobufs/
+// Original C++/Java/Python code:
+// http://code.google.com/p/protobuf/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 using System;
 using System.Collections.Generic;
-using System.Text;
 using System.Collections;
 
 namespace Google.ProtocolBuffers.Collections {

src/ProtocolBuffers/ProtocolBuffers.csproj

@@ -41,6 +41,7 @@
     <Compile Include="AbstractBuilder.cs" />
     <Compile Include="AbstractMessage.cs" />
     <Compile Include="ByteString.cs" />
+    <Compile Include="Collections\Enumerables.cs" />
     <Compile Include="Collections\PopsicleList.cs" />
     <Compile Include="Delegates.cs" />
     <Compile Include="CodedInputStream.cs" />

src/ProtocolBuffers/UnknownField.cs

@@ -113,6 +113,29 @@ namespace Google.ProtocolBuffers {
       get { return groupList; }
     }
 
+    public override bool Equals(object other) {
+      if (ReferenceEquals(this, other)) {
+        return true;
+      }
+      UnknownField otherField = other as UnknownField;
+      return otherField != null
+        && Lists.Equals(varintList, otherField.varintList)
+        && Lists.Equals(fixed32List, otherField.fixed32List)
+        && Lists.Equals(fixed64List, otherField.fixed64List)
+        && Lists.Equals(lengthDelimitedList, otherField.lengthDelimitedList)
+        && Lists.Equals(groupList, otherField.groupList);
+    }
+
+    public override int GetHashCode() {
+      int hash = 43;
+      hash = hash * 47 + Lists.GetHashCode(varintList);
+      hash = hash * 47 + Lists.GetHashCode(fixed32List);
+      hash = hash * 47 + Lists.GetHashCode(fixed64List);
+      hash = hash * 47 + Lists.GetHashCode(lengthDelimitedList);
+      hash = hash * 47 + Lists.GetHashCode(groupList);
+      return hash;
+    }
+
     /// <summary>
     /// Constructs a new Builder.
     /// </summary>

src/ProtocolBuffers/UnknownFieldSet.cs

@@ -194,6 +194,18 @@ namespace Google.ProtocolBuffers {
       }
     }
 
+    public override bool Equals(object other) {
+      if (ReferenceEquals(this, other)) {
+        return true;
+      }
+      UnknownFieldSet otherSet = other as UnknownFieldSet;
+      return otherSet != null && Dictionaries.Equals(fields, otherSet.fields);
+    }
+
+    public override int GetHashCode() {
+      return Dictionaries.GetHashCode(fields);
+    }
+
     /// <summary>
     /// Parses an UnknownFieldSet from the given input.
     /// </summary>
@@ -236,8 +248,8 @@ namespace Google.ProtocolBuffers {
       // Optimization:  We keep around a builder for the last field that was
       // modified so that we can efficiently add to it multiple times in a
       // row (important when parsing an unknown repeated field).
-      int lastFieldNumber;
-      UnknownField.Builder lastField;
+      private int lastFieldNumber;
+      private UnknownField.Builder lastField;
 
       internal Builder() {
       }