Lots more tests for FieldCodec, MapField, RepeatedField

... and some implementation changes to go with them.

7 files changed, 516 insertions, 18 deletions

diff --git a/csharp/src/ProtocolBuffers.Test/Collections/MapFieldTest.cs b/csharp/src/ProtocolBuffers.Test/Collections/MapFieldTest.cs
index 75f8ff2a..d43bed3e 100644
--- a/csharp/src/ProtocolBuffers.Test/Collections/MapFieldTest.cs
+++ b/csharp/src/ProtocolBuffers.Test/Collections/MapFieldTest.cs
@@ -34,6 +34,8 @@ using System;
 using System.Collections.Generic;
 using Google.Protobuf.TestProtos;
 using NUnit.Framework;
+using System.Collections;
+using System.Linq;
 
 namespace Google.Protobuf.Collections
 {
@@ -54,6 +56,18 @@ namespace Google.Protobuf.Collections
         }
 
         [Test]
+        public void Freeze_Idempotent()
+        {
+            var message = new ForeignMessage { C = 20 };
+            var map = new MapField<string, ForeignMessage> { { "x", message } };
+            Assert.IsFalse(map.IsFrozen);
+            map.Freeze();
+            Assert.IsTrue(message.IsFrozen);
+            map.Freeze();
+            Assert.IsTrue(message.IsFrozen);
+        }
+
+        [Test]
         public void Freeze_PreventsMutation()
         {
             var map = new MapField<string, string>();
@@ -188,6 +202,15 @@ namespace Google.Protobuf.Collections
         }
 
         [Test]
+        public void Equality_Simple()
+        {
+            var map = new MapField<string, string>();
+            EqualityTester.AssertEquality(map, map);
+            EqualityTester.AssertInequality(map, null);
+            Assert.IsFalse(map.Equals(new object()));
+        }
+
+        [Test]
         public void EqualityIsValueSensitive()
         {
             // Note: Without some care, it's a little easier than one might
@@ -287,7 +310,8 @@ namespace Google.Protobuf.Collections
             Assert.IsFalse(map.Remove("missing"));
             Assert.AreEqual(1, map.Count);
             Assert.IsTrue(map.Remove("foo"));
-            Assert.AreEqual(0, map.Count);           
+            Assert.AreEqual(0, map.Count);
+            Assert.Throws<ArgumentNullException>(() => map.Remove(null));
         }
 
         [Test]
@@ -346,6 +370,164 @@ namespace Google.Protobuf.Collections
             Assert.AreEqual("z", map["x"]);
         }
 
+        [Test]
+        public void GetEnumerator_NonGeneric()
+        {
+            IEnumerable map = new MapField<string, string> { { "x", "y" } };
+            CollectionAssert.AreEqual(new[] { new KeyValuePair<string, string>("x", "y") },
+                map.Cast<object>().ToList());
+        }
+
+        // Test for the explicitly-implemented non-generic IDictionary interface
+        [Test]
+        public void IDictionary_GetEnumerator()
+        {
+            IDictionary map = new MapField<string, string> { { "x", "y" } };
+            var enumerator = map.GetEnumerator();
+
+            // Commented assertions show an ideal situation - it looks like
+            // the LinkedList enumerator doesn't throw when you ask for the current entry
+            // at an inappropriate time; fixing this would be more work than it's worth.
+            // Assert.Throws<InvalidOperationException>(() => enumerator.Current.GetHashCode());
+            Assert.IsTrue(enumerator.MoveNext());
+            Assert.AreEqual("x", enumerator.Key);
+            Assert.AreEqual("y", enumerator.Value);
+            Assert.AreEqual(new DictionaryEntry("x", "y"), enumerator.Current);
+            Assert.AreEqual(new DictionaryEntry("x", "y"), enumerator.Entry);
+            Assert.IsFalse(enumerator.MoveNext());
+            // Assert.Throws<InvalidOperationException>(() => enumerator.Current.GetHashCode());
+            enumerator.Reset();
+            // Assert.Throws<InvalidOperationException>(() => enumerator.Current.GetHashCode());
+            Assert.IsTrue(enumerator.MoveNext());
+            Assert.AreEqual("x", enumerator.Key); // Assume the rest are okay
+        }
+
+        [Test]
+        public void IDictionary_Add()
+        {
+            var map = new MapField<string, string> { { "x", "y" } };
+            IDictionary dictionary = map;
+            dictionary.Add("a", "b");
+            Assert.AreEqual("b", map["a"]);
+            Assert.Throws<ArgumentException>(() => dictionary.Add("a", "duplicate"));
+            Assert.Throws<InvalidCastException>(() => dictionary.Add(new object(), "key is bad"));
+            Assert.Throws<InvalidCastException>(() => dictionary.Add("value is bad", new object()));
+        }
+
+        [Test]
+        public void IDictionary_Contains()
+        {
+            var map = new MapField<string, string> { { "x", "y" } };
+            IDictionary dictionary = map;
+
+            Assert.IsFalse(dictionary.Contains("a"));
+            Assert.IsFalse(dictionary.Contains(5));
+            // Surprising, but IDictionary.Contains is only about keys.
+            Assert.IsFalse(dictionary.Contains(new DictionaryEntry("x", "y")));
+            Assert.IsTrue(dictionary.Contains("x"));
+        }
+
+        [Test]
+        public void IDictionary_Remove()
+        {
+            var map = new MapField<string, string> { { "x", "y" } };
+            IDictionary dictionary = map;
+            dictionary.Remove("a");
+            Assert.AreEqual(1, dictionary.Count);
+            dictionary.Remove(5);
+            Assert.AreEqual(1, dictionary.Count);
+            dictionary.Remove(new DictionaryEntry("x", "y"));
+            Assert.AreEqual(1, dictionary.Count);
+            dictionary.Remove("x");
+            Assert.AreEqual(0, dictionary.Count);
+            Assert.Throws<ArgumentNullException>(() => dictionary.Remove(null));
+
+            map.Freeze();
+            // Call should fail even though it clearly doesn't contain 5 as a key.
+            Assert.Throws<InvalidOperationException>(() => dictionary.Remove(5));
+        }
+
+        [Test]
+        public void IDictionary_CopyTo()
+        {
+            var map = new MapField<string, string> { { "x", "y" } };
+            IDictionary dictionary = map;
+            var array = new DictionaryEntry[3];
+            dictionary.CopyTo(array, 1);
+            CollectionAssert.AreEqual(new[] { default(DictionaryEntry), new DictionaryEntry("x", "y"), default(DictionaryEntry) },
+                array);
+            var objectArray = new object[3];
+            dictionary.CopyTo(objectArray, 1);
+            CollectionAssert.AreEqual(new object[] { null, new DictionaryEntry("x", "y"), null },
+                objectArray);
+        }
+
+        [Test]
+        public void IDictionary_IsFixedSize()
+        {
+            var map = new MapField<string, string> { { "x", "y" } };
+            IDictionary dictionary = map;
+            Assert.IsFalse(dictionary.IsFixedSize);
+            map.Freeze();
+            Assert.IsTrue(dictionary.IsFixedSize);
+        }
+
+        [Test]
+        public void IDictionary_Keys()
+        {
+            IDictionary dictionary = new MapField<string, string> { { "x", "y" } };
+            CollectionAssert.AreEqual(new[] { "x" }, dictionary.Keys);
+        }
+
+        [Test]
+        public void IDictionary_Values()
+        {
+            IDictionary dictionary = new MapField<string, string> { { "x", "y" } };
+            CollectionAssert.AreEqual(new[] { "y" }, dictionary.Values);
+        }
+
+        [Test]
+        public void IDictionary_IsSynchronized()
+        {
+            IDictionary dictionary = new MapField<string, string> { { "x", "y" } };
+            Assert.IsFalse(dictionary.IsSynchronized);
+        }
+
+        [Test]
+        public void IDictionary_SyncRoot()
+        {
+            IDictionary dictionary = new MapField<string, string> { { "x", "y" } };
+            Assert.AreSame(dictionary, dictionary.SyncRoot);
+        }
+
+        [Test]
+        public void IDictionary_Indexer_Get()
+        {
+            IDictionary dictionary = new MapField<string, string> { { "x", "y" } };
+            Assert.AreEqual("y", dictionary["x"]);
+            Assert.IsNull(dictionary["a"]);
+            Assert.IsNull(dictionary[5]);
+            Assert.Throws<ArgumentNullException>(() => dictionary[null].GetHashCode());
+        }
+
+        [Test]
+        public void IDictionary_Indexer_Set()
+        {
+            var map = new MapField<string, string> { { "x", "y" } };
+            IDictionary dictionary = map;
+            map["a"] = "b";
+            Assert.AreEqual("b", map["a"]);
+            map["a"] = "c";
+            Assert.AreEqual("c", map["a"]);
+            Assert.Throws<InvalidCastException>(() => dictionary[5] = "x");
+            Assert.Throws<InvalidCastException>(() => dictionary["x"] = 5);
+            Assert.Throws<ArgumentNullException>(() => dictionary[null] = "z");
+            Assert.Throws<ArgumentNullException>(() => dictionary["x"] = null);
+            map.Freeze();
+            // Note: Not InvalidOperationException.
+            Assert.Throws<NotSupportedException>(() => dictionary["a"] = "c");
+        }
+
         private static KeyValuePair<TKey, TValue> NewKeyValuePair<TKey, TValue>(TKey key, TValue value)
         {
             return new KeyValuePair<TKey, TValue>(key, value);
diff --git a/csharp/src/ProtocolBuffers.Test/Collections/RepeatedFieldTest.cs b/csharp/src/ProtocolBuffers.Test/Collections/RepeatedFieldTest.cs
index 988801b7..6eff8683 100644
--- a/csharp/src/ProtocolBuffers.Test/Collections/RepeatedFieldTest.cs
+++ b/csharp/src/ProtocolBuffers.Test/Collections/RepeatedFieldTest.cs
@@ -31,9 +31,11 @@
 #endregion
 
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Text;
 using Google.Protobuf.TestProtos;
 using NUnit.Framework;
 
@@ -83,6 +85,115 @@ namespace Google.Protobuf.Collections
         }
 
         [Test]
+        public void RemoveAt_Valid()
+        {
+            var list = new RepeatedField<string> { "first", "second", "third" };
+            list.RemoveAt(1);
+            CollectionAssert.AreEqual(new[] { "first", "third" }, list);
+            // Just check that these don't throw...
+            list.RemoveAt(list.Count - 1); // Now the count will be 1...
+            list.RemoveAt(0);
+            Assert.AreEqual(0, list.Count);
+        }
+
+        [Test]
+        public void RemoveAt_Invalid()
+        {
+            var list = new RepeatedField<string> { "first", "second", "third" };
+            Assert.Throws<ArgumentOutOfRangeException>(() => list.RemoveAt(-1));
+            Assert.Throws<ArgumentOutOfRangeException>(() => list.RemoveAt(3));
+        }
+
+        [Test]
+        public void Insert_Valid()
+        {
+            var list = new RepeatedField<string> { "first", "second" };
+            list.Insert(1, "middle");
+            CollectionAssert.AreEqual(new[] { "first", "middle", "second" }, list);
+            list.Insert(3, "end");
+            CollectionAssert.AreEqual(new[] { "first", "middle", "second", "end" }, list);
+            list.Insert(0, "start");
+            CollectionAssert.AreEqual(new[] { "start", "first", "middle", "second", "end" }, list);
+        }
+
+        [Test]
+        public void Insert_Invalid()
+        {
+            var list = new RepeatedField<string> { "first", "second" };
+            Assert.Throws<ArgumentOutOfRangeException>(() => list.Insert(-1, "foo"));
+            Assert.Throws<ArgumentOutOfRangeException>(() => list.Insert(3, "foo"));
+            Assert.Throws<ArgumentNullException>(() => list.Insert(0, null));
+        }
+
+        [Test]
+        public void Equals_RepeatedField()
+        {
+            var list = new RepeatedField<string> { "first", "second" };
+            Assert.IsFalse(list.Equals((RepeatedField<string>) null));
+            Assert.IsTrue(list.Equals(list));
+            Assert.IsFalse(list.Equals(new RepeatedField<string> { "first", "third" }));
+            Assert.IsFalse(list.Equals(new RepeatedField<string> { "first" }));
+            Assert.IsTrue(list.Equals(new RepeatedField<string> { "first", "second" }));
+        }
+
+        [Test]
+        public void Equals_Object()
+        {
+            var list = new RepeatedField<string> { "first", "second" };
+            Assert.IsFalse(list.Equals((object) null));
+            Assert.IsTrue(list.Equals((object) list));
+            Assert.IsFalse(list.Equals((object) new RepeatedField<string> { "first", "third" }));
+            Assert.IsFalse(list.Equals((object) new RepeatedField<string> { "first" }));
+            Assert.IsTrue(list.Equals((object) new RepeatedField<string> { "first", "second" }));
+            Assert.IsFalse(list.Equals(new object()));
+        }
+
+        [Test]
+        public void GetEnumerator_GenericInterface()
+        {
+            IEnumerable<string> list = new RepeatedField<string> { "first", "second" };
+            // Select gets rid of the optimizations in ToList...
+            CollectionAssert.AreEqual(new[] { "first", "second" }, list.Select(x => x).ToList());
+        }
+
+        [Test]
+        public void GetEnumerator_NonGenericInterface()
+        {
+            IEnumerable list = new RepeatedField<string> { "first", "second" };
+            CollectionAssert.AreEqual(new[] { "first", "second" }, list.Cast<object>().ToList());
+        }
+
+        [Test]
+        public void CopyTo()
+        {
+            var list = new RepeatedField<string> { "first", "second" };
+            string[] stringArray = new string[4];
+            list.CopyTo(stringArray, 1);
+            CollectionAssert.AreEqual(new[] { null, "first", "second", null }, stringArray);
+        }
+
+        [Test]
+        public void Indexer_Get()
+        {
+            var list = new RepeatedField<string> { "first", "second" };
+            Assert.AreEqual("first", list[0]);
+            Assert.AreEqual("second", list[1]);
+            Assert.Throws<ArgumentOutOfRangeException>(() => list[-1].GetHashCode());
+            Assert.Throws<ArgumentOutOfRangeException>(() => list[2].GetHashCode());
+        }
+
+        [Test]
+        public void Indexer_Set()
+        {
+            var list = new RepeatedField<string> { "first", "second" };
+            list[0] = "changed";
+            Assert.AreEqual("changed", list[0]);
+            Assert.Throws<ArgumentNullException>(() => list[0] = null);
+            Assert.Throws<ArgumentOutOfRangeException>(() => list[-1] = "bad");
+            Assert.Throws<ArgumentOutOfRangeException>(() => list[2] = "bad");
+        }
+
+        [Test]
         public void Freeze_FreezesElements()
         {
             var list = new RepeatedField<TestAllTypes> { new TestAllTypes() };
@@ -125,6 +236,27 @@ namespace Google.Protobuf.Collections
         }
 
         [Test]
+        public void Enumerator()
+        {
+            var list = new RepeatedField<string> { "first", "second" };
+            using (var enumerator = list.GetEnumerator())
+            {
+                Assert.Throws<InvalidOperationException>(() => enumerator.Current.GetHashCode());
+                Assert.IsTrue(enumerator.MoveNext());
+                Assert.AreEqual("first", enumerator.Current);
+                Assert.IsTrue(enumerator.MoveNext());
+                Assert.AreEqual("second", enumerator.Current);
+                Assert.IsFalse(enumerator.MoveNext());
+                Assert.Throws<InvalidOperationException>(() => enumerator.Current.GetHashCode());
+                Assert.IsFalse(enumerator.MoveNext());
+                enumerator.Reset();
+                Assert.Throws<InvalidOperationException>(() => enumerator.Current.GetHashCode());
+                Assert.IsTrue(enumerator.MoveNext());
+                Assert.AreEqual("first", enumerator.Current);
+            }
+        }
+
+        [Test]
         public void AddEntriesFrom_PackedInt32()
         {
             uint packedTag = WireFormat.MakeTag(10, WireFormat.WireType.LengthDelimited);
@@ -309,6 +441,42 @@ namespace Google.Protobuf.Collections
             Assert.IsTrue(input.IsAtEnd);
         }
 
+        [Test]
+        public void CalculateSize_VariableSizeNonPacked()
+        {
+            var list = new RepeatedField<int> { 1, 500, 1 };
+            var tag = WireFormat.MakeTag(1, WireFormat.WireType.Varint);
+            // 2 bytes for the first entry, 3 bytes for the second, 2 bytes for the third
+            Assert.AreEqual(7, list.CalculateSize(FieldCodec.ForInt32(tag)));
+        }
+
+        [Test]
+        public void CalculateSize_FixedSizeNonPacked()
+        {
+            var list = new RepeatedField<int> { 1, 500, 1 };
+            var tag = WireFormat.MakeTag(1, WireFormat.WireType.Fixed32);
+            // 5 bytes for the each entry
+            Assert.AreEqual(15, list.CalculateSize(FieldCodec.ForSFixed32(tag)));
+        }
+
+        [Test]
+        public void CalculateSize_VariableSizePacked()
+        {
+            var list = new RepeatedField<int> { 1, 500, 1};
+            var tag = WireFormat.MakeTag(1, WireFormat.WireType.LengthDelimited);
+            // 1 byte for the tag, 1 byte for the length,
+            // 1 byte for the first entry, 2 bytes for the second, 1 byte for the third
+            Assert.AreEqual(6, list.CalculateSize(FieldCodec.ForInt32(tag)));
+        }
+
+        [Test]
+        public void CalculateSize_FixedSizePacked()
+        {
+            var list = new RepeatedField<int> { 1, 500, 1 };
+            var tag = WireFormat.MakeTag(1, WireFormat.WireType.LengthDelimited);
+            // 1 byte for the tag, 1 byte for the length, 4 bytes per entry
+            Assert.AreEqual(14, list.CalculateSize(FieldCodec.ForSFixed32(tag)));
+        }
 
         [Test]
         public void TestNegativeEnumArray()
@@ -378,5 +546,100 @@ namespace Google.Protobuf.Collections
             Assert.AreEqual(((SampleEnum)(-4)), values[4]);
             Assert.AreEqual(((SampleEnum)(-5)), values[5]);
         }
+
+        // Fairly perfunctory tests for the non-generic IList implementation
+        [Test]
+        public void IList_Indexer()
+        {
+            var field = new RepeatedField<string> { "first", "second" };
+            IList list = field;
+            Assert.AreEqual("first", list[0]);
+            list[1] = "changed";
+            Assert.AreEqual("changed", field[1]);
+        }
+
+        [Test]
+        public void IList_Contains()
+        {
+            IList list = new RepeatedField<string> { "first", "second" };
+            Assert.IsTrue(list.Contains("second"));
+            Assert.IsFalse(list.Contains("third"));
+            Assert.IsFalse(list.Contains(new object()));
+        }
+
+        [Test]
+        public void IList_Add()
+        {
+            IList list = new RepeatedField<string> { "first", "second" };
+            list.Add("third");
+            CollectionAssert.AreEqual(new[] { "first", "second", "third" }, list);
+        }
+
+        [Test]
+        public void IList_Remove()
+        {
+            IList list = new RepeatedField<string> { "first", "second" };
+            list.Remove("third"); // No-op, no exception
+            list.Remove(new object()); // No-op, no exception
+            list.Remove("first");
+            CollectionAssert.AreEqual(new[] { "second" }, list);
+        }
+
+        [Test]
+        public void IList_IsFixedSize()
+        {
+            var field = new RepeatedField<string> { "first", "second" };
+            IList list = field;
+            Assert.IsFalse(list.IsFixedSize);
+            field.Freeze();
+            Assert.IsTrue(list.IsFixedSize);
+        }
+
+        [Test]
+        public void IList_IndexOf()
+        {
+            IList list = new RepeatedField<string> { "first", "second" };
+            Assert.AreEqual(1, list.IndexOf("second"));
+            Assert.AreEqual(-1, list.IndexOf("third"));
+            Assert.AreEqual(-1, list.IndexOf(new object()));
+        }
+
+        [Test]
+        public void IList_SyncRoot()
+        {
+            IList list = new RepeatedField<string> { "first", "second" };
+            Assert.AreSame(list, list.SyncRoot);
+        }
+
+        [Test]
+        public void IList_CopyTo()
+        {
+            IList list = new RepeatedField<string> { "first", "second" };
+            string[] stringArray = new string[4];
+            list.CopyTo(stringArray, 1);
+            CollectionAssert.AreEqual(new[] { null, "first",  "second", null }, stringArray);
+
+            object[] objectArray = new object[4];
+            list.CopyTo(objectArray, 1);
+            CollectionAssert.AreEqual(new[] { null, "first", "second", null }, objectArray);
+
+            Assert.Throws<ArrayTypeMismatchException>(() => list.CopyTo(new StringBuilder[4], 1));
+            Assert.Throws<ArrayTypeMismatchException>(() => list.CopyTo(new int[4], 1));
+        }
+
+        [Test]
+        public void IList_IsSynchronized()
+        {
+            IList list = new RepeatedField<string> { "first", "second" };
+            Assert.IsFalse(list.IsSynchronized);
+        }
+
+        [Test]
+        public void IList_Insert()
+        {
+            IList list = new RepeatedField<string> { "first", "second" };
+            list.Insert(1, "middle");
+            CollectionAssert.AreEqual(new[] { "first", "middle", "second" }, list);
+        }
     }
 }
diff --git a/csharp/src/ProtocolBuffers.Test/EqualityTester.cs b/csharp/src/ProtocolBuffers.Test/EqualityTester.cs
index b372443b..a669baba 100644
--- a/csharp/src/ProtocolBuffers.Test/EqualityTester.cs
+++ b/csharp/src/ProtocolBuffers.Test/EqualityTester.cs
@@ -45,15 +45,20 @@ namespace Google.Protobuf
         public static void AssertEquality<T>(T first, T second) where T : IEquatable<T>
         {
             Assert.IsTrue(first.Equals(second));
+            Assert.IsTrue(first.Equals((object) second));
             Assert.AreEqual(first.GetHashCode(), second.GetHashCode());
         }
 
         public static void AssertInequality<T>(T first, T second) where T : IEquatable<T>
         {
             Assert.IsFalse(first.Equals(second));
+            Assert.IsFalse(first.Equals((object) second));
             // While this isn't a requirement, the chances of this test failing due to
             // coincidence rather than a bug are very small.
-            Assert.AreNotEqual(first.GetHashCode(), second.GetHashCode());
+            if (first != null && second != null)
+            {
+                Assert.AreNotEqual(first.GetHashCode(), second.GetHashCode());
+            }
         }
     }
 }
diff --git a/csharp/src/ProtocolBuffers.Test/FieldCodecTest.cs b/csharp/src/ProtocolBuffers.Test/FieldCodecTest.cs
index a14040d1..c6ed2a21 100644
--- a/csharp/src/ProtocolBuffers.Test/FieldCodecTest.cs
+++ b/csharp/src/ProtocolBuffers.Test/FieldCodecTest.cs
@@ -86,12 +86,22 @@ namespace Google.Protobuf
             codec.TestDefaultValue();
         }
 
+        [Test, TestCaseSource("Codecs")]
+        public void FixedSize(ICodecTestData codec)
+        {
+            codec.TestFixedSize();
+        }
+
+        // This is ugly, but it means we can have a non-generic interface.
+        // It feels like NUnit should support this better, but I don't know
+        // of any better ways right now.
         public interface ICodecTestData
         {
             void TestRoundTripRaw();
             void TestRoundTripWithTag();
             void TestCalculateSizeWithTag();
             void TestDefaultValue();
+            void TestFixedSize();
         }
 
         public class FieldCodecTestData<T> : ICodecTestData
@@ -169,6 +179,11 @@ namespace Google.Protobuf
                 }
             }
 
+            public void TestFixedSize()
+            {
+                Assert.AreEqual(name.Contains("Fixed"), codec.FixedSize != 0);
+            }
+
             public override string ToString()
             {
                 return name;
diff --git a/csharp/src/ProtocolBuffers/Collections/MapField.cs b/csharp/src/ProtocolBuffers/Collections/MapField.cs
index 779ff061..0f379eaa 100644
--- a/csharp/src/ProtocolBuffers/Collections/MapField.cs
+++ b/csharp/src/ProtocolBuffers/Collections/MapField.cs
@@ -367,11 +367,13 @@ namespace Google.Protobuf.Collections
 
         IDictionaryEnumerator IDictionary.GetEnumerator()
         {
-            throw new NotImplementedException();
+            return new DictionaryEnumerator(GetEnumerator());
         }
 
         void IDictionary.Remove(object key)
         {
+            ThrowHelper.ThrowIfNull(key, "key");
+            this.CheckMutable();
             if (!(key is TKey))
             {
                 return;
@@ -381,7 +383,9 @@ namespace Google.Protobuf.Collections
 
         void ICollection.CopyTo(Array array, int index)
         {
-            throw new NotImplementedException();
+            // This is ugly and slow as heck, but with any luck it will never be used anyway.
+            ICollection temp = this.Select(pair => new DictionaryEntry(pair.Key, pair.Value)).ToList();
+            temp.CopyTo(array, index);
         }
 
         bool IDictionary.IsFixedSize { get { return IsFrozen; } }
@@ -392,12 +396,13 @@ namespace Google.Protobuf.Collections
 
         bool ICollection.IsSynchronized { get { return false; } }
 
-        object ICollection.SyncRoot { get { return null; } }
+        object ICollection.SyncRoot { get { return this; } }
 
         object IDictionary.this[object key]
         {
             get
             {
+                ThrowHelper.ThrowIfNull(key, "key");
                 if (!(key is TKey))
                 {
                     return null;
@@ -407,10 +412,42 @@ namespace Google.Protobuf.Collections
                 return value;
             }
 
-            set { this[(TKey)key] = (TValue)value; }
+            set
+            {
+                if (frozen)
+                {
+                    throw new NotSupportedException("Dictionary is frozen");
+                }
+                this[(TKey)key] = (TValue)value;
+            }
         }
         #endregion
 
+        private class DictionaryEnumerator : IDictionaryEnumerator
+        {
+            private readonly IEnumerator<KeyValuePair<TKey, TValue>> enumerator;
+
+            internal DictionaryEnumerator(IEnumerator<KeyValuePair<TKey, TValue>> enumerator)
+            {
+                this.enumerator = enumerator;
+            }
+
+            public bool MoveNext()
+            {
+                return enumerator.MoveNext();
+            }
+
+            public void Reset()
+            {
+                enumerator.Reset();
+            }
+
+            public object Current { get { return Entry; } }
+            public DictionaryEntry Entry { get { return new DictionaryEntry(Key, Value); } }
+            public object Key { get { return enumerator.Current.Key; } }
+            public object Value { get { return enumerator.Current.Value; } }
+        }
+
         /// <summary>
         /// A codec for a specific map field. This contains all the information required to encoded and
         /// decode the nested messages.
diff --git a/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs b/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs
index ebc711de..b6b52cb9 100644
--- a/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs
+++ b/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs
@@ -376,6 +376,7 @@ namespace Google.Protobuf.Collections
             this.CheckMutable();
             EnsureSize(count + 1);
             Array.Copy(array, index, array, index + 1, count - index);
+            array[index] = item;
             count++;
         }
 
@@ -421,18 +422,12 @@ namespace Google.Protobuf.Collections
 
         void ICollection.CopyTo(Array array, int index)
         {
-            ThrowHelper.ThrowIfNull(array, "array");
-            T[] strongArray = array as T[];
-            if (strongArray == null)
-            {
-                throw new ArgumentException("Array is of incorrect type", "array");
-            }
-            CopyTo(strongArray, index);
+            Array.Copy(this.array, 0, array, index, count);
         }
 
         bool ICollection.IsSynchronized { get { return false; } }
 
-        object ICollection.SyncRoot { get { return null; } }
+        object ICollection.SyncRoot { get { return this; } }
 
         object IList.this[int index]
         {
@@ -490,6 +485,7 @@ namespace Google.Protobuf.Collections
             {
                 if (index + 1 >= field.Count)
                 {
+                    index = field.Count;
                     return false;
                 }
                 index++;
diff --git a/csharp/src/ProtocolBuffers/FieldCodec.cs b/csharp/src/ProtocolBuffers/FieldCodec.cs
index 2cebc1bb..c72a3e7b 100644
--- a/csharp/src/ProtocolBuffers/FieldCodec.cs
+++ b/csharp/src/ProtocolBuffers/FieldCodec.cs
@@ -68,12 +68,12 @@ namespace Google.Protobuf
 
         public static FieldCodec<uint> ForFixed32(uint tag)
         {
-            return new FieldCodec<uint>(input => input.ReadFixed32(), (output, value) => output.WriteFixed32(value), CodedOutputStream.ComputeFixed32Size, tag);
+            return new FieldCodec<uint>(input => input.ReadFixed32(), (output, value) => output.WriteFixed32(value), 4, tag);
         }
 
         public static FieldCodec<int> ForSFixed32(uint tag)
         {
-            return new FieldCodec<int>(input => input.ReadSFixed32(), (output, value) => output.WriteSFixed32(value), CodedOutputStream.ComputeSFixed32Size, tag);
+            return new FieldCodec<int>(input => input.ReadSFixed32(), (output, value) => output.WriteSFixed32(value), 4, tag);
         }
 
         public static FieldCodec<uint> ForUInt32(uint tag)
@@ -93,12 +93,12 @@ namespace Google.Protobuf
 
         public static FieldCodec<ulong> ForFixed64(uint tag)
         {
-            return new FieldCodec<ulong>(input => input.ReadFixed64(), (output, value) => output.WriteFixed64(value), CodedOutputStream.ComputeFixed64Size, tag);
+            return new FieldCodec<ulong>(input => input.ReadFixed64(), (output, value) => output.WriteFixed64(value), 8, tag);
         }
 
         public static FieldCodec<long> ForSFixed64(uint tag)
         {
-            return new FieldCodec<long>(input => input.ReadSFixed64(), (output, value) => output.WriteSFixed64(value), CodedOutputStream.ComputeSFixed64Size, tag);
+            return new FieldCodec<long>(input => input.ReadSFixed64(), (output, value) => output.WriteSFixed64(value), 8, tag);
         }
 
         public static FieldCodec<ulong> ForUInt64(uint tag)