Support ToString in RepeatedField and MapField.

This changes how we approach JSON formatting in general - instead of looking  at the field a value came from, we just look at the type of the value. It's possible this *could* be slightly inefficient, but if we start caring about JSON performance deeply, we'll probably want to rewrite all of this anyway. It's definitely simpler this way.

When we support dynamic messages, we'll need to modify JsonFormatter to handle enum values, as they won't come be "real" .NET enums at that point. It shouldn't be hard to do though.

5 files changed, 203 insertions, 115 deletions

diff --git a/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs b/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs
index 29c4c2a9..ba82c0e8 100644
--- a/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs
+++ b/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs
@@ -562,6 +562,20 @@ namespace Google.Protobuf.Collections
             Assert.IsFalse(values.Contains(null));
         }
 
+        [Test]
+        public void ToString_StringToString()
+        {
+            var map = new MapField<string, string> { { "foo", "bar" }, { "x", "y" } };
+            Assert.AreEqual("{ \"foo\": \"bar\", \"x\": \"y\" }", map.ToString());
+        }
+
+        [Test]
+        public void ToString_UnsupportedKeyType()
+        {
+            var map = new MapField<byte, string> { { 10, "foo" } };
+            Assert.Throws<ArgumentException>(() => map.ToString());
+        }
+
         private static KeyValuePair<TKey, TValue> NewKeyValuePair<TKey, TValue>(TKey key, TValue value)
         {
             return new KeyValuePair<TKey, TValue>(key, value);
diff --git a/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs b/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs
index 8c804fdd..8ed54cfb 100644
--- a/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs
+++ b/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs
@@ -37,6 +37,7 @@ using System.IO;
 using System.Linq;
 using System.Text;
 using Google.Protobuf.TestProtos;
+using Google.Protobuf.WellKnownTypes;
 using NUnit.Framework;
 
 namespace Google.Protobuf.Collections
@@ -599,5 +600,61 @@ namespace Google.Protobuf.Collections
             list.Insert(1, "middle");
             CollectionAssert.AreEqual(new[] { "first", "middle", "second" }, list);
         }
+
+        [Test]
+        public void ToString_Integers()
+        {
+            var list = new RepeatedField<int> { 5, 10, 20 };
+            var text = list.ToString();
+            Assert.AreEqual("[ 5, 10, 20 ]", text);
+        }
+
+        [Test]
+        public void ToString_Strings()
+        {
+            var list = new RepeatedField<string> { "x", "y", "z" };
+            var text = list.ToString();
+            Assert.AreEqual("[ \"x\", \"y\", \"z\" ]", text);
+        }
+
+        [Test]
+        public void ToString_Messages()
+        {
+            var list = new RepeatedField<TestAllTypes> { new TestAllTypes { SingleDouble = 1.5 }, new TestAllTypes { SingleInt32 = 10 } };
+            var text = list.ToString();
+            Assert.AreEqual("[ { \"singleDouble\": 1.5 }, { \"singleInt32\": 10 } ]", text);
+        }
+
+        [Test]
+        public void ToString_Empty()
+        {
+            var list = new RepeatedField<TestAllTypes> { };
+            var text = list.ToString();
+            Assert.AreEqual("[ ]", text);
+        }
+
+        [Test]
+        public void ToString_InvalidElementType()
+        {
+            var list = new RepeatedField<decimal> { 15m };
+            Assert.Throws<ArgumentException>(() => list.ToString());
+        }
+
+        [Test]
+        public void ToString_Timestamp()
+        {
+            var list = new RepeatedField<Timestamp> { Timestamp.FromDateTime(new DateTime(2015, 10, 1, 12, 34, 56, DateTimeKind.Utc)) };
+            var text = list.ToString();
+            Assert.AreEqual("[ \"2015-10-01T12:34:56Z\" ]", text);
+        }
+
+        [Test]
+        public void ToString_Struct()
+        {
+            var message = new Struct { Fields = { { "foo", new Value { NumberValue = 20 } } } };
+            var list = new RepeatedField<Struct> { message };
+            var text = list.ToString();
+            Assert.AreEqual(text, "[ { \"foo\": 20 } ]", message.ToString());
+        }
     }
 }
diff --git a/csharp/src/Google.Protobuf/Collections/MapField.cs b/csharp/src/Google.Protobuf/Collections/MapField.cs
index 0fa63bef..c0ed28ae 100644
--- a/csharp/src/Google.Protobuf/Collections/MapField.cs
+++ b/csharp/src/Google.Protobuf/Collections/MapField.cs
@@ -35,6 +35,7 @@ using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text;
 using Google.Protobuf.Compatibility;
 
 namespace Google.Protobuf.Collections
@@ -45,10 +46,17 @@ namespace Google.Protobuf.Collections
     /// <typeparam name="TKey">Key type in the map. Must be a type supported by Protocol Buffer map keys.</typeparam>
     /// <typeparam name="TValue">Value type in the map. Must be a type supported by Protocol Buffers.</typeparam>
     /// <remarks>
+    /// <para>
     /// This implementation preserves insertion order for simplicity of testing
     /// code using maps fields. Overwriting an existing entry does not change the
     /// position of that entry within the map. Equality is not order-sensitive.
     /// For string keys, the equality comparison is provided by <see cref="StringComparer.Ordinal" />.
+    /// </para>
+    /// <para>
+    /// This implementation does not generally prohibit the use of key/value types which are not
+    /// supported by Protocol Buffers (e.g. using a key type of <code>byte</code>) but nor does it guarantee
+    /// that all operations will work in such cases.
+    /// </para>
     /// </remarks>
     public sealed class MapField<TKey, TValue> : IDeepCloneable<MapField<TKey, TValue>>, IDictionary<TKey, TValue>, IEquatable<MapField<TKey, TValue>>, IDictionary
     {
@@ -482,6 +490,17 @@ namespace Google.Protobuf.Collections
             return size;
         }
 
+        /// <summary>
+        /// Returns a string representation of this repeated field, in the same
+        /// way as it would be represented by the default JSON formatter.
+        /// </summary>
+        public override string ToString()
+        {
+            var builder = new StringBuilder();
+            JsonFormatter.Default.WriteDictionary(builder, this);
+            return builder.ToString();
+        }
+
         #region IDictionary explicit interface implementation
         void IDictionary.Add(object key, object value)
         {
diff --git a/csharp/src/Google.Protobuf/Collections/RepeatedField.cs b/csharp/src/Google.Protobuf/Collections/RepeatedField.cs
index d9ced6ec..e3f65afe 100644
--- a/csharp/src/Google.Protobuf/Collections/RepeatedField.cs
+++ b/csharp/src/Google.Protobuf/Collections/RepeatedField.cs
@@ -33,6 +33,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Text;
 using Google.Protobuf.Compatibility;
 
 namespace Google.Protobuf.Collections
@@ -41,6 +42,10 @@ namespace Google.Protobuf.Collections
     /// The contents of a repeated field: essentially, a collection with some extra
     /// restrictions (no null values) and capabilities (deep cloning).
     /// </summary>
+    /// <remarks>
+    /// This implementation does not generally prohibit the use of types which are not
+    /// supported by Protocol Buffers but nor does it guarantee that all operations will work in such cases.
+    /// </remarks>
     /// <typeparam name="T">The element type of the repeated field.</typeparam>
     public sealed class RepeatedField<T> : IList<T>, IList, IDeepCloneable<RepeatedField<T>>, IEquatable<RepeatedField<T>>
     {
@@ -465,6 +470,17 @@ namespace Google.Protobuf.Collections
         }
 
         /// <summary>
+        /// Returns a string representation of this repeated field, in the same
+        /// way as it would be represented by the default JSON formatter.
+        /// </summary>
+        public override string ToString()
+        {
+            var builder = new StringBuilder();
+            JsonFormatter.Default.WriteList(builder, this);
+            return builder.ToString();
+        }
+
+        /// <summary>
         /// Gets or sets the item at the specified index.
         /// </summary>
         /// <value>
diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs
index 12bbdfdd..3f9bd478 100644
--- a/csharp/src/Google.Protobuf/JsonFormatter.cs
+++ b/csharp/src/Google.Protobuf/JsonFormatter.cs
@@ -170,7 +170,7 @@ namespace Google.Protobuf
                     continue;
                 }
                 // Omit awkward (single) values such as unknown enum values
-                if (!field.IsRepeated && !field.IsMap && !CanWriteSingleValue(accessor.Descriptor, value))
+                if (!field.IsRepeated && !field.IsMap && !CanWriteSingleValue(value))
                 {
                     continue;
                 }
@@ -182,7 +182,7 @@ namespace Google.Protobuf
                 }
                 WriteString(builder, ToCamelCase(accessor.Descriptor.Name));
                 builder.Append(": ");
-                WriteValue(builder, accessor, value);
+                WriteValue(builder, value);
                 first = false;
             }            
             builder.Append(first ? "}" : " }");
@@ -291,93 +291,81 @@ namespace Google.Protobuf
                     throw new ArgumentException("Invalid field type");
             }
         }
-
-        private void WriteValue(StringBuilder builder, IFieldAccessor accessor, object value)
+        
+        private void WriteValue(StringBuilder builder, object value)
         {
-            if (accessor.Descriptor.IsMap)
+            if (value == null)
             {
-                WriteDictionary(builder, accessor, (IDictionary) value);
+                WriteNull(builder);
             }
-            else if (accessor.Descriptor.IsRepeated)
+            else if (value is bool)
             {
-                WriteList(builder, accessor, (IList) value);
+                builder.Append((bool) value ? "true" : "false");
             }
-            else
+            else if (value is ByteString)
             {
-                WriteSingleValue(builder, accessor.Descriptor, value);
+                // Nothing in Base64 needs escaping
+                builder.Append('"');
+                builder.Append(((ByteString) value).ToBase64());
+                builder.Append('"');
             }
-        }
-
-        private void WriteSingleValue(StringBuilder builder, FieldDescriptor descriptor, object value)
-        {
-            switch (descriptor.FieldType)
+            else if (value is string)
             {
-                case FieldType.Bool:
-                    builder.Append((bool) value ? "true" : "false");
-                    break;
-                case FieldType.Bytes:
-                    // Nothing in Base64 needs escaping
+                WriteString(builder, (string) value);
+            }
+            else if (value is IDictionary)
+            {
+                WriteDictionary(builder, (IDictionary) value);
+            }
+            else if (value is IList)
+            {
+                WriteList(builder, (IList) value);
+            }
+            else if (value is int || value is uint)
+            {
+                IFormattable formattable = (IFormattable) value;
+                builder.Append(formattable.ToString("d", CultureInfo.InvariantCulture));
+            }
+            else if (value is long || value is ulong)
+            {
+                builder.Append('"');
+                IFormattable formattable = (IFormattable) value;
+                builder.Append(formattable.ToString("d", CultureInfo.InvariantCulture));
+                builder.Append('"');
+            }
+            else if (value is System.Enum)
+            {
+                WriteString(builder, value.ToString());
+            }
+            else if (value is float || value is double)
+            {
+                string text = ((IFormattable) value).ToString("r", CultureInfo.InvariantCulture);
+                if (text == "NaN" || text == "Infinity" || text == "-Infinity")
+                {
                     builder.Append('"');
-                    builder.Append(((ByteString) value).ToBase64());
+                    builder.Append(text);
                     builder.Append('"');
-                    break;
-                case FieldType.String:
-                    WriteString(builder, (string) value);
-                    break;
-                case FieldType.Fixed32:
-                case FieldType.UInt32:
-                case FieldType.SInt32:
-                case FieldType.Int32:
-                case FieldType.SFixed32:
-                    {
-                        IFormattable formattable = (IFormattable) value;
-                        builder.Append(formattable.ToString("d", CultureInfo.InvariantCulture));
-                        break;
-                    }
-                case FieldType.Enum:
-                    EnumValueDescriptor enumValue = descriptor.EnumType.FindValueByNumber((int) value);
-                    // We will already have validated that this is a known value.
-                    WriteString(builder, enumValue.Name);
-                    break;
-                case FieldType.Fixed64:
-                case FieldType.UInt64:
-                case FieldType.SFixed64:
-                case FieldType.Int64:
-                case FieldType.SInt64:
-                    {
-                        builder.Append('"');
-                        IFormattable formattable = (IFormattable) value;
-                        builder.Append(formattable.ToString("d", CultureInfo.InvariantCulture));
-                        builder.Append('"');
-                        break;
-                    }
-                case FieldType.Double:
-                case FieldType.Float:
-                    string text = ((IFormattable) value).ToString("r", CultureInfo.InvariantCulture);
-                    if (text == "NaN" || text == "Infinity" || text == "-Infinity")
-                    {
-                        builder.Append('"');
-                        builder.Append(text);
-                        builder.Append('"');
-                    }
-                    else
-                    {
-                        builder.Append(text);
-                    }
-                    break;
-                case FieldType.Message:
-                case FieldType.Group: // Never expect to get this, but...
-                    if (descriptor.MessageType.IsWellKnownType)
-                    {
-                        WriteWellKnownTypeValue(builder, descriptor.MessageType, value, true);
-                    }
-                    else
-                    {
-                        WriteMessage(builder, (IMessage) value);
-                    }
-                    break;
-                default:
-                    throw new ArgumentException("Invalid field type: " + descriptor.FieldType);
+                }
+                else
+                {
+                    builder.Append(text);
+                }
+            }
+            else if (value is IMessage)
+            {
+                IMessage message = (IMessage) value;
+                if (message.Descriptor.IsWellKnownType)
+                {
+                    WriteWellKnownTypeValue(builder, message.Descriptor, value, true);
+                }
+                else
+                {
+                    WriteMessage(builder, (IMessage) value);
+                }
+            }
+            else
+            {
+                throw new ArgumentException("Unable to format value of type " + value.GetType());
             }
         }
 
@@ -398,7 +386,7 @@ namespace Google.Protobuf
             // so we can write it as if we were unconditionally writing the Value field for the wrapper type.
             if (descriptor.File == Int32Value.Descriptor.File)
             {
-                WriteSingleValue(builder, descriptor.FindFieldByNumber(1), value);
+                WriteValue(builder, value);
                 return;
             }
             if (descriptor.FullName == Timestamp.Descriptor.FullName)
@@ -424,7 +412,7 @@ namespace Google.Protobuf
             if (descriptor.FullName == ListValue.Descriptor.FullName)
             {
                 var fieldAccessor = descriptor.Fields[ListValue.ValuesFieldNumber].Accessor;
-                WriteList(builder, fieldAccessor, (IList) fieldAccessor.GetValue((IMessage) value));
+                WriteList(builder, (IList) fieldAccessor.GetValue((IMessage) value));
                 return;
             }
             if (descriptor.FullName == Value.Descriptor.FullName)
@@ -565,7 +553,7 @@ namespace Google.Protobuf
                 case Value.BoolValueFieldNumber:
                 case Value.StringValueFieldNumber:
                 case Value.NumberValueFieldNumber:
-                    WriteSingleValue(builder, specifiedField, value);
+                    WriteValue(builder, value);
                     return;
                 case Value.StructValueFieldNumber:
                 case Value.ListValueFieldNumber:
@@ -581,13 +569,13 @@ namespace Google.Protobuf
             }
         }
 
-        private void WriteList(StringBuilder builder, IFieldAccessor accessor, IList list)
+        internal void WriteList(StringBuilder builder, IList list)
         {
             builder.Append("[ ");
             bool first = true;
             foreach (var value in list)
             {
-                if (!CanWriteSingleValue(accessor.Descriptor, value))
+                if (!CanWriteSingleValue(value))
                 {
                     continue;
                 }
@@ -595,22 +583,20 @@ namespace Google.Protobuf
                 {
                     builder.Append(", ");
                 }
-                WriteSingleValue(builder, accessor.Descriptor, value);
+                WriteValue(builder, value);
                 first = false;
             }
             builder.Append(first ? "]" : " ]");
         }
 
-        private void WriteDictionary(StringBuilder builder, IFieldAccessor accessor, IDictionary dictionary)
+        internal void WriteDictionary(StringBuilder builder, IDictionary dictionary)
         {
             builder.Append("{ ");
             bool first = true;
-            FieldDescriptor keyType = accessor.Descriptor.MessageType.FindFieldByNumber(1);
-            FieldDescriptor valueType = accessor.Descriptor.MessageType.FindFieldByNumber(2);
             // This will box each pair. Could use IDictionaryEnumerator, but that's ugly in terms of disposal.
             foreach (DictionaryEntry pair in dictionary)
             {
-                if (!CanWriteSingleValue(valueType, pair.Value))
+                if (!CanWriteSingleValue(pair.Value))
                 {
                     continue;
                 }
@@ -619,32 +605,29 @@ namespace Google.Protobuf
                     builder.Append(", ");
                 }
                 string keyText;
-                switch (keyType.FieldType)
+                if (pair.Key is string)
                 {
-                    case FieldType.String:
-                        keyText = (string) pair.Key;
-                        break;
-                    case FieldType.Bool:
-                        keyText = (bool) pair.Key ? "true" : "false";
-                        break;
-                    case FieldType.Fixed32:
-                    case FieldType.Fixed64:
-                    case FieldType.SFixed32:
-                    case FieldType.SFixed64:
-                    case FieldType.Int32:
-                    case FieldType.Int64:
-                    case FieldType.SInt32:
-                    case FieldType.SInt64:
-                    case FieldType.UInt32:
-                    case FieldType.UInt64:
-                        keyText = ((IFormattable) pair.Key).ToString("d", CultureInfo.InvariantCulture);
-                        break;
-                    default:
-                        throw new ArgumentException("Invalid key type: " + keyType.FieldType);
+                    keyText = (string) pair.Key;
+                }
+                else if (pair.Key is bool)
+                {
+                    keyText = (bool) pair.Key ? "true" : "false";
+                }
+                else if (pair.Key is int || pair.Key is uint | pair.Key is long || pair.Key is ulong)
+                {
+                    keyText = ((IFormattable) pair.Key).ToString("d", CultureInfo.InvariantCulture);
+                }
+                else
+                {
+                    if (pair.Key == null)
+                    {
+                        throw new ArgumentException("Dictionary has entry with null key");
+                    }
+                    throw new ArgumentException("Unhandled dictionary key type: " + pair.Key.GetType());
                 }
                 WriteString(builder, keyText);
                 builder.Append(": ");
-                WriteSingleValue(builder, valueType, pair.Value);
+                WriteValue(builder, pair.Value);
                 first = false;
             }
             builder.Append(first ? "}" : " }");
@@ -655,12 +638,11 @@ namespace Google.Protobuf
         /// Currently only relevant for enums, where unknown values can't be represented.
         /// For repeated/map fields, this always returns true.
         /// </summary>
-        private bool CanWriteSingleValue(FieldDescriptor descriptor, object value)
+        private bool CanWriteSingleValue(object value)
         {
-            if (descriptor.FieldType == FieldType.Enum)
+            if (value is System.Enum)
             {
-                EnumValueDescriptor enumValue = descriptor.EnumType.FindValueByNumber((int) value);
-                return enumValue != null;
+                return System.Enum.IsDefined(value.GetType(), value);
             }
             return true;
         }