Provide simple access to descriptor declarations in C#

This is primarily for access to comments, which would be expected to be available in a protoc plugin.

The implementation has two fiddly aspects:

- We use a Lazy<T> to avoid building the map before cross-linking. An alternative would be to crosslink at the end of the constructor, and remove the calls to CrossLink elsewhere. This would be generally better IMO, but deviate from the Java code.
- The casts to IReadOnlyList<DescriptorBase> are unfortunate. They'll always work, because these lists are always ReadOnlyCollection<T> for a descriptor type... but we can't use IList<DescriptorBase> as that's not covariant, and it's annoyingly fiddly to change the field to be of type ReadOnlyCollection<T>.

csharp/generate_protos.sh

@@ -43,6 +43,9 @@ $PROTOC -Isrc --csharp_out=csharp/src/Google.Protobuf \
 # Test protos
 $PROTOC -Isrc -Icsharp/protos \
     --csharp_out=csharp/src/Google.Protobuf.Test/TestProtos \
+    --descriptor_set_out=csharp/src/Google.Protobuf.Test/testprotos.pb \
+    --include_source_info \
+    --include_imports \
     csharp/protos/map_unittest_proto3.proto \
     csharp/protos/unittest_issues.proto \
     csharp/protos/unittest_custom_options_proto3.proto \

csharp/protos/unittest_proto3.proto

@@ -368,7 +368,9 @@ message FooResponse {}
 message FooClientMessage {}
 message FooServerMessage{}
 
+// This is a test service
 service TestService {
+  // This is a test method
   rpc Foo(FooRequest) returns (FooResponse);
   rpc Bar(BarRequest) returns (BarResponse);
 }
@@ -378,3 +380,31 @@ message BarRequest  {}
 message BarResponse {}
 
 message TestEmptyMessage {}
+
+// This is leading detached comment 1
+
+// This is leading detached comment 2
+
+// This is a leading comment
+message CommentMessage {
+  // Leading nested message comment
+  message NestedCommentMessage {
+    // Leading nested message field comment
+    string nested_text = 1;
+  }
+  
+  // Leading nested enum comment
+  enum NestedCommentEnum {
+    // Zero value comment
+    ZERO_VALUE = 0;
+  }
+  
+  // Leading field comment
+  string text = 1; // Trailing field comment
+}
+
+// Leading enum comment
+enum CommentEnum {
+  // Zero value comment
+  ZERO_VALUE = 0;
+}

csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj

@@ -27,4 +27,8 @@
     <TargetFrameworks>netcoreapp1.0</TargetFrameworks>
   </PropertyGroup>
 
+  <ItemGroup>
+    <EmbeddedResource Include="testprotos.pb" />
+  </ItemGroup>
+
 </Project>

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

+#region Copyright notice and license
+// Protocol Buffers - Google's data interchange format
+// Copyright 2018 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 Google.Protobuf.Reflection;
+using NUnit.Framework;
+using System.Linq;
+using System.Reflection;
+
+namespace Google.Protobuf.Test.Reflection
+{
+    // In reality this isn't a test for DescriptorDeclaration so much as the way they're loaded.
+    public class DescriptorDeclarationTest
+    {
+        static readonly FileDescriptor unitTestProto3Descriptor = LoadProtos();
+
+        [Test]
+        public void ServiceComments()
+        {
+            var service = unitTestProto3Descriptor.FindTypeByName<ServiceDescriptor>("TestService");
+            Assert.NotNull(service.Declaration);
+            Assert.AreEqual(" This is a test service\n", service.Declaration.LeadingComments);
+        }
+
+        [Test]
+        public void MethodComments()
+        {
+            var service = unitTestProto3Descriptor.FindTypeByName<ServiceDescriptor>("TestService");
+            var method = service.FindMethodByName("Foo");
+            Assert.NotNull(method.Declaration);
+            Assert.AreEqual(" This is a test method\n", method.Declaration.LeadingComments);
+        }
+
+        [Test]
+        public void MessageComments()
+        {
+            var message = unitTestProto3Descriptor.FindTypeByName<MessageDescriptor>("CommentMessage");
+            Assert.NotNull(message.Declaration);
+            Assert.AreEqual(" This is a leading comment\n", message.Declaration.LeadingComments);
+            Assert.AreEqual(new[] { " This is leading detached comment 1\n", " This is leading detached comment 2\n" },
+                message.Declaration.LeadingDetachedComments);
+        }
+
+        [Test]
+        public void EnumComments()
+        {
+            var descriptor = unitTestProto3Descriptor.FindTypeByName<EnumDescriptor>("CommentEnum");
+            Assert.NotNull(descriptor.Declaration);
+            Assert.AreEqual(" Leading enum comment\n", descriptor.Declaration.LeadingComments);
+        }
+
+        [Test]
+        public void NestedMessageComments()
+        {
+            var outer = unitTestProto3Descriptor.FindTypeByName<MessageDescriptor>("CommentMessage");
+            var nested = outer.FindDescriptor<MessageDescriptor>("NestedCommentMessage");
+            Assert.NotNull(nested.Declaration);
+            Assert.AreEqual(" Leading nested message comment\n", nested.Declaration.LeadingComments);
+        }
+
+        [Test]
+        public void NestedEnumComments()
+        {
+            var outer = unitTestProto3Descriptor.FindTypeByName<MessageDescriptor>("CommentMessage");
+            var nested = outer.FindDescriptor<EnumDescriptor>("NestedCommentEnum");
+            Assert.NotNull(nested.Declaration);
+            Assert.AreEqual(" Leading nested enum comment\n", nested.Declaration.LeadingComments);
+        }
+
+        [Test]
+        public void FieldComments()
+        {
+            var message = unitTestProto3Descriptor.FindTypeByName<MessageDescriptor>("CommentMessage");
+            var field = message.FindFieldByName("text");
+            Assert.NotNull(field.Declaration);
+            Assert.AreEqual(" Leading field comment\n", field.Declaration.LeadingComments);
+            Assert.AreEqual(" Trailing field comment\n", field.Declaration.TrailingComments);
+        }
+
+        [Test]
+        public void NestedMessageFieldComments()
+        {
+            var outer = unitTestProto3Descriptor.FindTypeByName<MessageDescriptor>("CommentMessage");
+            var nested = outer.FindDescriptor<MessageDescriptor>("NestedCommentMessage");
+            var field = nested.FindFieldByName("nested_text");
+            Assert.NotNull(field.Declaration);
+            Assert.AreEqual(" Leading nested message field comment\n", field.Declaration.LeadingComments);
+        }
+
+        [Test]
+        public void EnumValueComments()
+        {
+            var enumDescriptor = unitTestProto3Descriptor.FindTypeByName<EnumDescriptor>("CommentEnum");
+            var value = enumDescriptor.FindValueByName("ZERO_VALUE");
+            Assert.NotNull(value.Declaration);
+            Assert.AreEqual(" Zero value comment\n", value.Declaration.LeadingComments);
+        }
+
+        [Test]
+        public void NestedEnumValueComments()
+        {
+            var outer = unitTestProto3Descriptor.FindTypeByName<MessageDescriptor>("CommentMessage");
+            var nested = outer.FindDescriptor<EnumDescriptor>("NestedCommentEnum");
+            var value = nested.FindValueByName("ZERO_VALUE");
+            Assert.NotNull(value.Declaration);
+            Assert.AreEqual(" Zero value comment\n", value.Declaration.LeadingComments);
+        }
+
+        private static FileDescriptor LoadProtos()
+        {
+            var type = typeof(DescriptorDeclarationTest);
+            // TODO: Make this simpler :)
+            FileDescriptorSet descriptorSet;
+            using (var stream = type.GetTypeInfo().Assembly.GetManifestResourceStream($"Google.Protobuf.Test.testprotos.pb"))
+            {
+                descriptorSet = FileDescriptorSet.Parser.ParseFrom(stream);
+            }
+            var byteStrings = descriptorSet.File.Select(f => f.ToByteString()).ToList();
+            var descriptors = FileDescriptor.BuildFromByteStrings(byteStrings);
+            return descriptors.Single(d => d.Name == "unittest_proto3.proto");
+        }
+    }
+}

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

@@ -30,6 +30,8 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #endregion
 
+using System.Collections.Generic;
+
 namespace Google.Protobuf.Reflection
 {
     /// <summary>
@@ -37,15 +39,11 @@ namespace Google.Protobuf.Reflection
     /// </summary>
     public abstract class DescriptorBase : IDescriptor
     {
-        private readonly FileDescriptor file;
-        private readonly string fullName;
-        private readonly int index;
-
         internal DescriptorBase(FileDescriptor file, string fullName, int index)
         {
-            this.file = file;
-            this.fullName = fullName;
-            this.index = index;
+            File = file;
+            FullName = fullName;
+            Index = index;
         }
 
         /// <value>
@@ -56,10 +54,7 @@ namespace Google.Protobuf.Reflection
         /// this descriptor's type. (There can be duplicate values for different
         /// types, e.g. one enum type with index 0 and one message type with index 0.)
         /// </remarks>
-        public int Index
-        {
-            get { return index; }
-        }
+        public int Index { get; }
 
         /// <summary>
         /// Returns the name of the entity (field, message etc) being described.
@@ -69,17 +64,29 @@ namespace Google.Protobuf.Reflection
         /// <summary>
         /// The fully qualified name of the descriptor's target.
         /// </summary>
-        public string FullName
-        {
-            get { return fullName; }
-        }
+        public string FullName { get; }
 
         /// <value>
         /// The file this descriptor was declared in.
         /// </value>
-        public FileDescriptor File
-        {
-            get { return file; }
-        }
+        public FileDescriptor File { get; }
+
+        /// <summary>
+        /// The declaration information about the descriptor, or null if no declaration information
+        /// is available for this descriptor.
+        /// </summary>
+        /// <remarks>
+        /// This information is typically only available for dynamically loaded descriptors,
+        /// for example within a protoc plugin where the full descriptors, including source info,
+        /// are passed to the code by protoc.
+        /// </remarks>
+        public DescriptorDeclaration Declaration => File.GetDeclaration(this);
+
+        /// <summary>
+        /// Retrieves the list of nested descriptors corresponding to the given field number, if any.
+        /// If the field is unknown or not a nested descriptor list, return null to terminate the search.
+        /// The default implementation returns null.
+        /// </summary>
+        internal virtual IReadOnlyList<DescriptorBase> GetNestedDescriptorListForField(int fieldNumber) => null;
     }
 }
\ No newline at end of file

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

+#region Copyright notice and license
+// Protocol Buffers - Google's data interchange format
+// Copyright 2018 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;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Linq;
+using System.Text;
+using static Google.Protobuf.Reflection.SourceCodeInfo.Types;
+
+namespace Google.Protobuf.Reflection
+{
+    /// <summary>
+    /// Provides additional information about the declaration of a descriptor,
+    /// such as source location and comments.
+    /// </summary>
+    public sealed class DescriptorDeclaration
+    {
+        /// <summary>
+        /// The descriptor this declaration relates to.
+        /// </summary>
+        public IDescriptor Descriptor { get; }
+
+        /// <summary>
+        /// The start line of the declaration within the source file. This value is 1-based.
+        /// </summary>
+        public int StartLine { get; }
+        /// <summary>
+        /// The start column of the declaration within the source file. This value is 1-based.
+        /// </summary>
+        public int StartColumn { get; }
+
+        /// <summary>
+        /// // The end line of the declaration within the source file. This value is 1-based.
+        /// </summary>
+        public int EndLine { get; }
+        /// <summary>
+        /// The end column of the declaration within the source file. This value is 1-based.
+        /// </summary>
+        public int EndColumn { get; }
+
+        /// <summary>
+        /// Comments appearing before the declaration. Never null, but may be empty.
+        /// </summary>
+        public string LeadingComments { get; }
+
+        /// <summary>
+        /// Comments appearing after the declaration. Never null, but may be empty.
+        /// </summary>
+        public string TrailingComments { get; }
+
+        /// <summary>
+        /// Comments appearing before the declaration, but separated from it by blank
+        /// lines. Each string represents a paragraph of comments. The list is never null,
+        /// but may be empty. Likewise each element is never null, but may be empty.
+        /// </summary>
+        public IReadOnlyList<string> LeadingDetachedComments { get; }
+
+        private DescriptorDeclaration(IDescriptor descriptor, Location location)
+        {
+            // TODO: Validation
+            Descriptor = descriptor;
+            bool hasEndLine = location.Span.Count == 4;
+            // Lines and columns are 0-based in the proto.
+            StartLine = location.Span[0] + 1;
+            StartColumn = location.Span[1] + 1;
+            EndLine = hasEndLine ? location.Span[2] + 1 : StartLine;
+            EndColumn = location.Span[hasEndLine ? 3 : 2] + 1;
+            LeadingComments = location.LeadingComments;
+            TrailingComments = location.TrailingComments;
+            LeadingDetachedComments = new ReadOnlyCollection<string>(location.LeadingDetachedComments.ToList());
+        }
+
+        internal static DescriptorDeclaration FromProto(IDescriptor descriptor, Location location) =>
+            new DescriptorDeclaration(descriptor, location);
+    }
+}

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

@@ -72,6 +72,17 @@ namespace Google.Protobuf.Reflection
         /// </summary>
         public override string Name { get { return proto.Name; } }
 
+        internal override IReadOnlyList<DescriptorBase> GetNestedDescriptorListForField(int fieldNumber)
+        {
+            switch (fieldNumber)
+            {
+                case EnumDescriptorProto.ValueFieldNumber:
+                    return (IReadOnlyList<DescriptorBase>) Values;
+                default:
+                    return null;
+            }
+        }
+
         /// <summary>
         /// The CLR type for this enum. For generated code, this will be a CLR enum type.
         /// </summary>

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

@@ -34,7 +34,10 @@ using Google.Protobuf.WellKnownTypes;
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
+using System.Diagnostics;
 using System.Linq;
+using System.Threading;
+using static Google.Protobuf.Reflection.SourceCodeInfo.Types;
 
 namespace Google.Protobuf.Reflection
 {
@@ -55,6 +58,8 @@ namespace Google.Protobuf.Reflection
             ForceReflectionInitialization<Value.KindOneofCase>();
         }
 
+        private readonly Lazy<Dictionary<IDescriptor, DescriptorDeclaration>> declarations;
+
         private FileDescriptor(ByteString descriptorData, FileDescriptorProto proto, IEnumerable<FileDescriptor> dependencies, DescriptorPool pool, bool allowUnknownDependencies, GeneratedClrTypeInfo generatedCodeInfo)
         {
             SerializedData = descriptorData;
@@ -77,6 +82,81 @@ namespace Google.Protobuf.Reflection
             Services = DescriptorUtil.ConvertAndMakeReadOnly(proto.Service,
                                                              (service, index) =>
                                                              new ServiceDescriptor(service, this, index));
+
+            declarations = new Lazy<Dictionary<IDescriptor, DescriptorDeclaration>>(CreateDeclarationMap, LazyThreadSafetyMode.ExecutionAndPublication);
+        }
+
+        private Dictionary<IDescriptor, DescriptorDeclaration> CreateDeclarationMap()
+        {
+            var dictionary = new Dictionary<IDescriptor, DescriptorDeclaration>();
+            foreach (var location in Proto.SourceCodeInfo?.Location ?? Enumerable.Empty<Location>())
+            {
+                var descriptor = FindDescriptorForPath(location.Path);
+                if (descriptor != null)
+                {
+                    dictionary[descriptor] = DescriptorDeclaration.FromProto(descriptor, location);
+                }
+            }
+            return dictionary;
+
+            IDescriptor FindDescriptorForPath(IList<int> path)
+            {
+                // All complete declarations have an even, non-empty path length
+                // (There can be an empty path for a descriptor declaration, but that can't have any comments,
+                // so we currently ignore it.)
+                if (path.Count == 0 || (path.Count & 1) != 0)
+                {
+                    return null;
+                }
+                IReadOnlyList<DescriptorBase> topLevelList = GetNestedDescriptorListForField(path[0]);
+                DescriptorBase current = GetDescriptorFromList(topLevelList, path[1]);
+
+                for (int i = 2; current != null && i < path.Count; i += 2)
+                {
+                    var list = current.GetNestedDescriptorListForField(path[i]);
+                    current = GetDescriptorFromList(list, path[i + 1]);
+                }
+                return current;
+            }
+
+            DescriptorBase GetDescriptorFromList(IReadOnlyList<DescriptorBase> list, int index)
+            {
+                // This is fine: it may be a newer version of protobuf than we understand, with a new descriptor
+                // field.
+                if (list == null)
+                {
+                    return null;
+                }
+                // We *could* return null to silently continue, but this is basically data corruption.
+                if (index < 0 || index >= list.Count)
+                {
+                    // We don't have much extra information to give at this point unfortunately. If this becomes a problem,
+                    // we can pass in the complete path and report that and the file name.
+                    throw new InvalidProtocolBufferException($"Invalid descriptor location path: index out of range");
+                }
+                return list[index];
+            }
+
+            IReadOnlyList<DescriptorBase> GetNestedDescriptorListForField(int fieldNumber)
+            {
+                switch (fieldNumber)
+                {
+                    case FileDescriptorProto.ServiceFieldNumber:
+                        return (IReadOnlyList<DescriptorBase>) Services;
+                    case FileDescriptorProto.MessageTypeFieldNumber:
+                        return (IReadOnlyList<DescriptorBase>) MessageTypes;
+                    case FileDescriptorProto.EnumTypeFieldNumber:
+                        return (IReadOnlyList<DescriptorBase>) EnumTypes;
+                    default:
+                        return null;
+                }
+            }
+        }
+
+        internal DescriptorDeclaration GetDeclaration(IDescriptor descriptor)
+        {
+            declarations.Value.TryGetValue(descriptor, out var declaration);
+            return declaration;
         }
 
         /// <summary>

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

@@ -115,6 +115,21 @@ namespace Google.Protobuf.Reflection
         /// </summary>
         public override string Name => Proto.Name;
 
+        internal override IReadOnlyList<DescriptorBase> GetNestedDescriptorListForField(int fieldNumber)
+        {
+            switch (fieldNumber)
+            {
+                case DescriptorProto.FieldFieldNumber:
+                    return (IReadOnlyList<DescriptorBase>) fieldsInDeclarationOrder;
+                case DescriptorProto.NestedTypeFieldNumber:
+                    return (IReadOnlyList<DescriptorBase>) NestedTypes;
+                case DescriptorProto.EnumTypeFieldNumber:
+                    return (IReadOnlyList<DescriptorBase>) EnumTypes;
+                default:
+                    return null;
+            }
+        }
+
         internal DescriptorProto Proto { get; }
 
         /// <summary>

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

@@ -32,6 +32,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 
 namespace Google.Protobuf.Reflection
 {
@@ -58,6 +59,17 @@ namespace Google.Protobuf.Reflection
         /// </summary>
         public override string Name { get { return proto.Name; } }
 
+        internal override IReadOnlyList<DescriptorBase> GetNestedDescriptorListForField(int fieldNumber)
+        {
+            switch (fieldNumber)
+            {
+                case ServiceDescriptorProto.MethodFieldNumber:
+                    return (IReadOnlyList<DescriptorBase>) methods;
+                default:
+                    return null;
+            }
+        }
+
         internal ServiceDescriptorProto Proto { get { return proto; } }
 
         /// <value>