Merge pull request #6431 from jtattermusch/backport_csharp_proto2_review

Backport review comments for C# proto2 features (for 3.9.x)
diff --git a/csharp/src/Google.Protobuf/Extension.cs b/csharp/src/Google.Protobuf/Extension.cs
index ddeb977..a96f8d2 100644
--- a/csharp/src/Google.Protobuf/Extension.cs
+++ b/csharp/src/Google.Protobuf/Extension.cs
@@ -35,7 +35,7 @@
 namespace Google.Protobuf
 {
     /// <summary>
-    /// Represents a non-generic extension definition
+    /// Represents a non-generic extension definition. This API is experimental and subject to change.
     /// </summary>
     public abstract class Extension
     {
@@ -44,9 +44,9 @@
         /// <summary>
         /// Internal use. Creates a new extension with the specified field number.
         /// </summary>
-        protected Extension(int number)
+        protected Extension(int fieldNumber)
         {
-            FieldNumber = number;
+            FieldNumber = fieldNumber;
         }
 
         internal abstract IExtensionValue CreateValue();
@@ -58,7 +58,8 @@
     }
 
     /// <summary>
-    /// Represents a type-safe extension identifier used for getting and setting single extension values in <see cref="IExtendableMessage{T}"/> instances
+    /// Represents a type-safe extension identifier used for getting and setting single extension values in <see cref="IExtendableMessage{T}"/> instances. 
+    /// This API is experimental and subject to change.
     /// </summary>
     /// <typeparam name="TTarget">The message type this field applies to</typeparam>
     /// <typeparam name="TValue">The field value type of this extension</typeparam>
@@ -69,7 +70,7 @@
         /// <summary>
         /// Creates a new extension identifier with the specified field number and codec
         /// </summary>
-        public Extension(int number, FieldCodec<TValue> codec) : base(number)
+        public Extension(int fieldNumber, FieldCodec<TValue> codec) : base(fieldNumber)
         {
             this.codec = codec;
         }
@@ -85,7 +86,8 @@
     }
 
     /// <summary>
-    /// Represents a type-safe extension identifier used for getting repeated extension values in <see cref="IExtendableMessage{T}"/> instances
+    /// Represents a type-safe extension identifier used for getting repeated extension values in <see cref="IExtendableMessage{T}"/> instances.
+    /// This API is experimental and subject to change.
     /// </summary>
     /// <typeparam name="TTarget">The message type this field applies to</typeparam>
     /// <typeparam name="TValue">The repeated field value type of this extension</typeparam>
@@ -96,7 +98,7 @@
         /// <summary>
         /// Creates a new repeated extension identifier with the specified field number and codec
         /// </summary>
-        public RepeatedExtension(int number, FieldCodec<TValue> codec) : base(number)
+        public RepeatedExtension(int fieldNumber, FieldCodec<TValue> codec) : base(fieldNumber)
         {
             this.codec = codec;
         }
diff --git a/csharp/src/Google.Protobuf/ExtensionRegistry.cs b/csharp/src/Google.Protobuf/ExtensionRegistry.cs
index ea655c1..b8eee24 100644
--- a/csharp/src/Google.Protobuf/ExtensionRegistry.cs
+++ b/csharp/src/Google.Protobuf/ExtensionRegistry.cs
@@ -38,7 +38,7 @@
 namespace Google.Protobuf
 {
     /// <summary>
-    /// Provides extensions to messages while parsing
+    /// Provides extensions to messages while parsing. This API is experimental and subject to change.
     /// </summary>
     public sealed class ExtensionRegistry : ICollection<Extension>, IDeepCloneable<ExtensionRegistry>
     {
@@ -67,9 +67,9 @@
         /// </summary>
         bool ICollection<Extension>.IsReadOnly => false;
 
-        internal bool ContainsInputField(CodedInputStream stream, Type target, out Extension extension)

-        {

-            return extensions.TryGetValue(new ObjectIntPair<Type>(target, WireFormat.GetTagFieldNumber(stream.LastTag)), out extension);

+        internal bool ContainsInputField(CodedInputStream stream, Type target, out Extension extension)
+        {
+            return extensions.TryGetValue(new ObjectIntPair<Type>(target, WireFormat.GetTagFieldNumber(stream.LastTag)), out extension);
         }
 
         /// <summary>
@@ -83,23 +83,13 @@
         }
 
         /// <summary>
-        /// Adds the specified extensions to the registry
-        /// </summary>
-        public void Add(params Extension[] newExtensions)
-        {
-            ProtoPreconditions.CheckNotNull(newExtensions, nameof(newExtensions));
-
-            Add((IEnumerable<Extension>)newExtensions);
-        }
-
-        /// <summary>
         /// Adds the specified extensions to the reigstry
         /// </summary>
-        public void Add(IEnumerable<Extension> newExtensions)
+        public void AddRange(IEnumerable<Extension> extensions)
         {
-            ProtoPreconditions.CheckNotNull(newExtensions, nameof(newExtensions));
+            ProtoPreconditions.CheckNotNull(extensions, nameof(extensions));
 
-            foreach (var extension in newExtensions)
+            foreach (var extension in extensions)
                 Add(extension);
         }
 
@@ -134,10 +124,10 @@
             if (array.Length - arrayIndex < Count)
                 throw new ArgumentException("The provided array is shorter than the number of elements in the registry");
 
-            for (int i = 0; i < array.Length; i++)

-            {

-                Extension extension = array[i];

-                extensions.Add(new ObjectIntPair<Type>(extension.TargetType, extension.FieldNumber), extension);

+            for (int i = 0; i < array.Length; i++)
+            {
+                Extension extension = array[i];
+                extensions.Add(new ObjectIntPair<Type>(extension.TargetType, extension.FieldNumber), extension);
             }
         }
 
diff --git a/csharp/src/Google.Protobuf/ExtensionSet.cs b/csharp/src/Google.Protobuf/ExtensionSet.cs
index fa81e6a..a46e499 100644
--- a/csharp/src/Google.Protobuf/ExtensionSet.cs
+++ b/csharp/src/Google.Protobuf/ExtensionSet.cs
@@ -40,11 +40,11 @@
     /// <summary>

     /// Methods for managing <see cref="ExtensionSet{TTarget}"/>s with null checking.

     /// 

-    /// Most users will not use this class directly

+    /// Most users will not use this class directly and its API is experimental and subject to change.

     /// </summary>

     public static class ExtensionSet

     {

-        private static bool GetValue<TTarget>(ref ExtensionSet<TTarget> set, Extension extension, out IExtensionValue value) where TTarget : IExtendableMessage<TTarget>

+        private static bool TryGetValue<TTarget>(ref ExtensionSet<TTarget> set, Extension extension, out IExtensionValue value) where TTarget : IExtendableMessage<TTarget>

         {

             if (set == null)

             {

@@ -60,7 +60,7 @@
         public static TValue Get<TTarget, TValue>(ref ExtensionSet<TTarget> set, Extension<TTarget, TValue> extension) where TTarget : IExtendableMessage<TTarget>

         {

             IExtensionValue value;

-            if (GetValue(ref set, extension, out value))

+            if (TryGetValue(ref set, extension, out value))

             {

                 return ((ExtensionValue<TValue>)value).GetValue();

             }

@@ -76,7 +76,7 @@
         public static RepeatedField<TValue> Get<TTarget, TValue>(ref ExtensionSet<TTarget> set, RepeatedExtension<TTarget, TValue> extension) where TTarget : IExtendableMessage<TTarget>

         {

             IExtensionValue value;

-            if (GetValue(ref set, extension, out value))

+            if (TryGetValue(ref set, extension, out value))

             {

                 return ((RepeatedExtensionValue<TValue>)value).GetValue();

             }

@@ -111,7 +111,7 @@
         }

 

         /// <summary>

-        /// Sets the value of the specified extension

+        /// Sets the value of the specified extension. This will make a new instance of ExtensionSet if the set is null.

         /// </summary>

         public static void Set<TTarget, TValue>(ref ExtensionSet<TTarget> set, Extension<TTarget, TValue> extension, TValue value) where TTarget : IExtendableMessage<TTarget>

         {

@@ -140,14 +140,7 @@
         public static bool Has<TTarget, TValue>(ref ExtensionSet<TTarget> set, Extension<TTarget, TValue> extension) where TTarget : IExtendableMessage<TTarget>

         {

             IExtensionValue value;

-            if (GetValue(ref set, extension, out value))

-            {

-                return ((ExtensionValue<TValue>)value).HasValue;

-            }

-            else 

-            {

-                return false;

-            }

+            return TryGetValue(ref set, extension, out value);

         }

 

         /// <summary>

diff --git a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs
index 4c2fe54..9b20e3d 100644
--- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs
+++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs
@@ -407,7 +407,7 @@
 
         private static void AddAllExtensions(FileDescriptor[] dependencies, GeneratedClrTypeInfo generatedInfo, ExtensionRegistry registry)
         {
-            registry.Add(dependencies.SelectMany(GetAllDependedExtensions).Concat(GetAllGeneratedExtensions(generatedInfo)).ToArray());
+            registry.AddRange(dependencies.SelectMany(GetAllDependedExtensions).Concat(GetAllGeneratedExtensions(generatedInfo)).ToArray());
         }
 
         private static IEnumerable<Extension> GetAllGeneratedExtensions(GeneratedClrTypeInfo generated)