Tidy up reflection in advance of attempting to implement DynamicMessage.

There are corner cases where MessageDescriptor.{ClrType,Parser} will return null, and these are now documented. However, normally they *should* be implemented, even for descriptors of for dynamic messages. Ditto FieldDescriptor.Accessor.
We'll still need a fair amount of work to implement dynamic messages, but this change means that the public API will be remain intact.

Additionally, this change starts making use of C# 6 features in the files that it touches. This is far from exhaustive, and later PRs will have more.

Generated code changes coming in the next commit.

csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs

@@ -63,7 +63,7 @@ namespace Google.Protobuf.Reflection
             Assert.AreEqual(UnittestImportProto3Reflection.Descriptor, file.Dependencies[0]);
 
             MessageDescriptor messageType = TestAllTypes.Descriptor;
-            Assert.AreSame(typeof(TestAllTypes), messageType.GeneratedType);
+            Assert.AreSame(typeof(TestAllTypes), messageType.ClrType);
             Assert.AreSame(TestAllTypes.Parser, messageType.Parser);
             Assert.AreEqual(messageType, file.MessageTypes[0]);
             Assert.AreEqual(messageType, file.FindTypeByName<MessageDescriptor>("TestAllTypes"));
@@ -227,18 +227,12 @@ namespace Google.Protobuf.Reflection
         }
 
         [Test]
-        public void ConstructionWithoutGeneratedCodeInfo()
+        public void MapEntryMessageDescriptor()
         {
-            var data = UnittestIssuesReflection.Descriptor.Proto.ToByteArray();
-            var newDescriptor = Google.Protobuf.Reflection.FileDescriptor.InternalBuildGeneratedFileFrom(data, new Reflection.FileDescriptor[] { }, null);
-
-            // We should still be able to get at a field...
-            var messageDescriptor = newDescriptor.FindTypeByName<MessageDescriptor>("ItemField");
-            var fieldDescriptor = messageDescriptor.FindFieldByName("item");
-            // But there shouldn't be an accessor (or a generated type for the message, or parser)
-            Assert.IsNull(fieldDescriptor.Accessor);
-            Assert.IsNull(messageDescriptor.GeneratedType);
-            Assert.IsNull(messageDescriptor.Parser);
+            var descriptor = MapWellKnownTypes.Descriptor.NestedTypes[0];
+            Assert.IsNull(descriptor.Parser);
+            Assert.IsNull(descriptor.ClrType);
+            Assert.IsNull(descriptor.Fields[1].Accessor);
         }
 
         // From TestFieldOrdering:

csharp/src/Google.Protobuf/JsonFormatter.cs

@@ -388,8 +388,7 @@ namespace Google.Protobuf
             // If it's the message form, we can extract the value first, which *will* be the (possibly boxed) native value,
             // and then proceed, writing it as if we were definitely in a field. (We never need to wrap it in an extra string...
             // WriteValue will do the right thing.)
-            // TODO: Detect this differently when we have dynamic messages.
-            if (descriptor.File == Int32Value.Descriptor.File)
+            if (descriptor.IsWrapperType)
             {
                 if (value is IMessage)
                 {

csharp/src/Google.Protobuf/JsonParser.cs

@@ -291,8 +291,7 @@ namespace Google.Protobuf
             {
                 // Parse wrapper types as their constituent types.
                 // TODO: What does this mean for null?
-                // TODO: Detect this differently when we have dynamic messages, and put it in one place...
-                if (field.MessageType.IsWellKnownType && field.MessageType.File == Int32Value.Descriptor.File)
+                if (field.MessageType.IsWrapperType)
                 {
                     field = field.MessageType.Fields[WrappersReflection.WrapperValueFieldNumber];
                     fieldType = field.FieldType;

csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs

@@ -43,13 +43,13 @@ namespace Google.Protobuf.Reflection
         private readonly EnumDescriptorProto proto;
         private readonly MessageDescriptor containingType;
         private readonly IList<EnumValueDescriptor> values;
-        private readonly Type generatedType;
+        private readonly Type clrType;
 
         internal EnumDescriptor(EnumDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index, Type generatedType)
             : base(file, file.ComputeFullName(parent, proto.Name), index)
         {
             this.proto = proto;
-            this.generatedType = generatedType;
+            this.clrType = generatedType;
             containingType = parent;
 
             if (proto.Value.Count == 0)
@@ -73,9 +73,9 @@ namespace Google.Protobuf.Reflection
         public override string Name { get { return proto.Name; } }
 
         /// <summary>
-        /// The generated type for this enum, or <c>null</c> if the descriptor does not represent a generated type.
+        /// The CLR type for this enum. For generated code, this will be a CLR enum type.
         /// </summary>
-        public Type GeneratedType { get { return generatedType; } }
+        public Type ClrType { get { return clrType; } }
 
         /// <value>
         /// If this is a nested type, get the outer descriptor, otherwise null.

csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs

@@ -62,8 +62,7 @@ namespace Google.Protobuf.Reflection
 
             if (FieldNumber <= 0)
             {
-                throw new DescriptorValidationException(this,
-                                                        "Field numbers must be positive integers.");
+                throw new DescriptorValidationException(this, "Field numbers must be positive integers.");
             }
             containingType = parent;
             // OneofIndex "defaults" to -1 due to a hack in FieldDescriptor.OnConstruction.
@@ -72,7 +71,7 @@ namespace Google.Protobuf.Reflection
                 if (proto.OneofIndex < 0 || proto.OneofIndex >= parent.Proto.OneofDecl.Count)
                 {
                     throw new DescriptorValidationException(this,
-                        "FieldDescriptorProto.oneof_index is out of range for type " + parent.Name);
+                        $"FieldDescriptorProto.oneof_index is out of range for type {parent.Name}");
                 }
                 containingOneof = parent.Oneofs[proto.OneofIndex];
             }
@@ -94,13 +93,22 @@ namespace Google.Protobuf.Reflection
         internal FieldDescriptorProto Proto { get { return proto; } }
 
         /// <summary>
-        /// Returns the accessor for this field, or <c>null</c> if this descriptor does
-        /// not support reflective access.
+        /// Returns the accessor for this field.
         /// </summary>
         /// <remarks>
+        /// <para>
         /// While a <see cref="FieldDescriptor"/> describes the field, it does not provide
         /// any way of obtaining or changing the value of the field within a specific message;
         /// that is the responsibility of the accessor.
+        /// </para>
+        /// <para>
+        /// The value returned by this property will be non-null for all regular fields. However,
+        /// if a message containing a map field is introspected, the list of nested messages will include
+        /// an auto-generated nested key/value pair message for the field. This is not represented in any
+        /// generated type, and the value of the map field itself is represented by a dictionary in the
+        /// reflection API. There are never instances of those "hidden" messages, so no accessor is provided
+        /// and this property will return null.
+        /// </para>
         /// </remarks>
         public IFieldAccessor Accessor { get { return accessor; } }
         
@@ -281,7 +289,7 @@ namespace Google.Protobuf.Reflection
                     }
                     else
                     {
-                        throw new DescriptorValidationException(this, "\"" + Proto.TypeName + "\" is not a type.");
+                        throw new DescriptorValidationException(this, $"\"{Proto.TypeName}\" is not a type.");
                     }
                 }
 
@@ -289,8 +297,7 @@ namespace Google.Protobuf.Reflection
                 {
                     if (!(typeDescriptor is MessageDescriptor))
                     {
-                        throw new DescriptorValidationException(this,
-                                                                "\"" + Proto.TypeName + "\" is not a message type.");
+                        throw new DescriptorValidationException(this, $"\"{Proto.TypeName}\" is not a message type.");
                     }
                     messageType = (MessageDescriptor) typeDescriptor;
 
@@ -303,7 +310,7 @@ namespace Google.Protobuf.Reflection
                 {
                     if (!(typeDescriptor is EnumDescriptor))
                     {
-                        throw new DescriptorValidationException(this, "\"" + Proto.TypeName + "\" is not an enum type.");
+                        throw new DescriptorValidationException(this, $"\"{Proto.TypeName}\" is not an enum type.");
                     }
                     enumType = (EnumDescriptor) typeDescriptor;
                 }
@@ -333,14 +340,16 @@ namespace Google.Protobuf.Reflection
 
         private IFieldAccessor CreateAccessor(string propertyName)
         {
-            if (containingType.GeneratedType == null || propertyName == null)
+            // If we're given no property name, that's because we really don't want an accessor.
+            // (At the moment, that means it's a map entry message...)
+            if (propertyName == null)
             {
                 return null;
             }
-            var property = containingType.GeneratedType.GetProperty(propertyName);
+            var property = containingType.ClrType.GetProperty(propertyName);
             if (property == null)
             {
-                throw new DescriptorValidationException(this, "Property " + propertyName + " not found in " + containingType.GeneratedType);
+                throw new DescriptorValidationException(this, $"Property {propertyName} not found in {containingType.ClrType}");
             }
             return IsMap ? new MapFieldAccessor(property, this)
                 : IsRepeated ? new RepeatedFieldAccessor(property, this)

csharp/src/Google.Protobuf/Reflection/GeneratedCodeInfo.cs

+#region Copyright notice and license
+// Protocol Buffers - Google's data interchange format
+// Copyright 2015 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// 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.
+#endregion
 using System;
 
 namespace Google.Protobuf.Reflection
@@ -20,31 +51,31 @@ namespace Google.Protobuf.Reflection
         /// <summary>
         /// Irrelevant for file descriptors; the parser for message descriptors.
         /// </summary>
-        public MessageParser Parser { get; private set; }
+        public MessageParser Parser { get; }
 
         /// <summary>
         /// Irrelevant for file descriptors; the CLR property names (in message descriptor field order)
         /// for fields in the message for message descriptors.
         /// </summary>
-        public string[] PropertyNames { get; private set; }
+        public string[] PropertyNames { get; }
 
         /// <summary>
         /// Irrelevant for file descriptors; the CLR property "base" names (in message descriptor oneof order)
         /// for oneofs in the message for message descriptors. It is expected that for a oneof name of "Foo",
         /// there will be a "FooCase" property and a "ClearFoo" method.
         /// </summary>
-        public string[] OneofNames { get; private set; }
+        public string[] OneofNames { get; }
 
         /// <summary>
         /// The reflection information for types within this file/message descriptor. Elements may be null
         /// if there is no corresponding generated type, e.g. for map entry types.
         /// </summary>
-        public GeneratedCodeInfo[] NestedTypes { get; private set; }
+        public GeneratedCodeInfo[] NestedTypes { get; }
 
         /// <summary>
         /// The CLR types for enums within this file/message descriptor.
         /// </summary>
-        public Type[] NestedEnums { get; private set; }
+        public Type[] NestedEnums { get; }
 
         /// <summary>
         /// Creates a GeneratedCodeInfo for a message descriptor, with nested types, nested enums, the CLR type, property names and oneof names.

csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs

@@ -86,8 +86,7 @@ namespace Google.Protobuf.Reflection
         /// in a particular message.
         /// </summary>
         /// <value>
-        /// The accessor used for reflective access, or <c>null</c> if reflection is not
-        /// supported by this descriptor.
+        /// The accessor used for reflective access.
         /// </value>
         public OneofAccessor Accessor { get { return accessor; } }
 
@@ -106,19 +105,15 @@ namespace Google.Protobuf.Reflection
 
         private OneofAccessor CreateAccessor(string clrName)
         {
-            if (containingType.GeneratedType == null || clrName == null)
-            {
-                return null;
-            }
-            var caseProperty = containingType.GeneratedType.GetProperty(clrName + "Case");
+            var caseProperty = containingType.ClrType.GetProperty(clrName + "Case");
             if (caseProperty == null)
             {
-                throw new DescriptorValidationException(this, "Property " + clrName + "Case not found in " + containingType.GeneratedType);
+                throw new DescriptorValidationException(this, $"Property {clrName}Case not found in {containingType.ClrType}");
             }
-            var clearMethod = containingType.GeneratedType.GetMethod("Clear" + clrName);
+            var clearMethod = containingType.ClrType.GetMethod("Clear" + clrName);
             if (clearMethod == null)
             {
-                throw new DescriptorValidationException(this, "Method Clear" + clrName + " not found in " + containingType.GeneratedType);
+                throw new DescriptorValidationException(this, $"Method Clear{clrName} not found in {containingType.ClrType}");
             }
 
             return new OneofAccessor(caseProperty, clearMethod, this);

src/google/protobuf/compiler/csharp/csharp_reflection_class.cc

@@ -166,7 +166,7 @@ void ReflectionClassGenerator::WriteDescriptor(io::Printer* printer) {
   // -----------------------------------------------------------------
   // Invoke InternalBuildGeneratedFileFrom() to build the file.
   printer->Print(
-      "descriptor = pbr::FileDescriptor.InternalBuildGeneratedFileFrom(descriptorData,\n");
+      "descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData,\n");
   printer->Print("    new pbr::FileDescriptor[] { ");
   for (int i = 0; i < file_->dependency_count(); i++) {
     // descriptor.proto is special: we don't allow access to the generated code, but there's