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)