Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C#: Mark GetOption API as obsolete and expose the "GetOptions()" method on descriptors instead #7434

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
150 changes: 88 additions & 62 deletions csharp/src/Google.Protobuf.Test/Reflection/CustomOptionsTest.cs

Large diffs are not rendered by default.

12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs
Expand Up @@ -128,12 +128,21 @@ public EnumValueDescriptor FindValueByName(string name)
/// <summary>
/// The (possibly empty) set of custom options for this enum.
/// </summary>
[Obsolete("CustomOptions are obsolete. Use GetOption")]
[Obsolete("CustomOptions are obsolete. Use the Options property.")]
public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber);

/// <summary>
Copy link
Contributor

@JamesNK JamesNK Apr 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can null ever be returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved the docs.

/// The <c>EnumOptions</c>, defined in <c>descriptor.proto</c>.
/// If the options message is not present (=there are no options), <c>null</c> is returned.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// If the options message is not present (=there are no options), <c>null</c> is returned.
/// If the options message is not present (i.e. there are no options), <c>null</c> is returned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Ditto in all options, of course.)

/// Custom options can be retrieved as extensions of the returned message.
/// NOTE: A defensive copy is created each time this property is retrieved.
/// </summary>
public EnumOptions Options => Proto.Options?.Clone();

/// <summary>
/// Gets a single value enum option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public T GetOption<T>(Extension<EnumOptions, T> extension)
{
var value = Proto.Options.GetExtension(extension);
Expand All @@ -143,6 +152,7 @@ public T GetOption<T>(Extension<EnumOptions, T> extension)
/// <summary>
/// Gets a repeated value enum option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public RepeatedField<T> GetOption<T>(RepeatedExtension<EnumOptions, T> extension)
{
return Proto.Options.GetExtension(extension).Clone();
Expand Down
12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs
Expand Up @@ -73,12 +73,21 @@ public sealed class EnumValueDescriptor : DescriptorBase
/// <summary>
/// The (possibly empty) set of custom options for this enum value.
/// </summary>
[Obsolete("CustomOptions are obsolete. Use GetOption")]
[Obsolete("CustomOptions are obsolete. Use the Options property")]
public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber);

/// <summary>
/// The <c>EnumValueOptions</c>, defined in <c>descriptor.proto</c>.
/// If the options message is not present (=there are no options), <c>null</c> is returned.
/// Custom options can be retrieved as extensions of the returned message.
/// NOTE: A defensive copy is created each time this property is retrieved.
/// </summary>
public EnumValueOptions Options => Proto.Options?.Clone();

/// <summary>
/// Gets a single value enum value option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public T GetOption<T>(Extension<EnumValueOptions, T> extension)
{
var value = Proto.Options.GetExtension(extension);
Expand All @@ -88,6 +97,7 @@ public T GetOption<T>(Extension<EnumValueOptions, T> extension)
/// <summary>
/// Gets a repeated value enum value option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public RepeatedField<T> GetOption<T>(RepeatedExtension<EnumValueOptions, T> extension)
{
return Proto.Options.GetExtension(extension).Clone();
Expand Down
12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs
Expand Up @@ -304,12 +304,21 @@ public MessageDescriptor ExtendeeType
/// <summary>
/// The (possibly empty) set of custom options for this field.
/// </summary>
[Obsolete("CustomOptions are obsolete. Use GetOption")]
[Obsolete("CustomOptions are obsolete. Use the Options property.")]
public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber);

/// <summary>
/// The <c>FieldOptions</c>, defined in <c>descriptor.proto</c>.
/// If the options message is not present (=there are no options), <c>null</c> is returned.
/// Custom options can be retrieved as extensions of the returned message.
/// NOTE: A defensive copy is created each time this property is retrieved.
/// </summary>
public FieldOptions Options => Proto.Options?.Clone();

/// <summary>
/// Gets a single value field option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public T GetOption<T>(Extension<FieldOptions, T> extension)
{
var value = Proto.Options.GetExtension(extension);
Expand All @@ -319,6 +328,7 @@ public T GetOption<T>(Extension<FieldOptions, T> extension)
/// <summary>
/// Gets a repeated value field option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public RepeatedField<T> GetOption<T>(RepeatedExtension<FieldOptions, T> extension)
{
return Proto.Options.GetExtension(extension).Clone();
Expand Down
12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs
Expand Up @@ -547,12 +547,21 @@ public override string ToString()
/// <summary>
/// The (possibly empty) set of custom options for this file.
/// </summary>
[Obsolete("CustomOptions are obsolete. Use GetOption")]
[Obsolete("CustomOptions are obsolete. Use the Options property.")]
public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber);

/// <summary>
/// The <c>FileOptions</c>, defined in <c>descriptor.proto</c>.
/// If the options message is not present (=there are no options), <c>null</c> is returned.
/// Custom options can be retrieved as extensions of the returned message.
/// NOTE: A defensive copy is created each time this property is retrieved.
/// </summary>
public FileOptions Options => Proto.Options?.Clone();

/// <summary>
/// Gets a single value file option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public T GetOption<T>(Extension<FileOptions, T> extension)
{
var value = Proto.Options.GetExtension(extension);
Expand All @@ -562,6 +571,7 @@ public T GetOption<T>(Extension<FileOptions, T> extension)
/// <summary>
/// Gets a repeated value file option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public RepeatedField<T> GetOption<T>(RepeatedExtension<FileOptions, T> extension)
{
return Proto.Options.GetExtension(extension).Clone();
Expand Down
12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs
Expand Up @@ -287,12 +287,21 @@ internal bool IsExtensionsInitialized(IMessage message)
/// <summary>
/// The (possibly empty) set of custom options for this message.
/// </summary>
[Obsolete("CustomOptions are obsolete. Use GetOption")]
[Obsolete("CustomOptions are obsolete. Use the Options property")]
public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber);

/// <summary>
/// The <c>MessageOptions</c>, defined in <c>descriptor.proto</c>.
/// If the options message is not present (=there are no options), <c>null</c> is returned.
/// Custom options can be retrieved as extensions of the returned message.
/// NOTE: A defensive copy is created each time this property is retrieved.
/// </summary>
public MessageOptions Options => Proto.Options?.Clone();

/// <summary>
/// Gets a single value message option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public T GetOption<T>(Extension<MessageOptions, T> extension)
{
var value = Proto.Options.GetExtension(extension);
Expand All @@ -302,6 +311,7 @@ public T GetOption<T>(Extension<MessageOptions, T> extension)
/// <summary>
/// Gets a repeated value message option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public Collections.RepeatedField<T> GetOption<T>(RepeatedExtension<MessageOptions, T> extension)
{
return Proto.Options.GetExtension(extension).Clone();
Expand Down
12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs
Expand Up @@ -73,12 +73,21 @@ public sealed class MethodDescriptor : DescriptorBase
/// <summary>
/// The (possibly empty) set of custom options for this method.
/// </summary>
[Obsolete("CustomOptions are obsolete. Use GetOption")]
[Obsolete("CustomOptions are obsolete. Use the Options property.")]
public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber);

/// <summary>
/// The <c>MethodOptions</c>, defined in <c>descriptor.proto</c>.
/// If the options message is not present (=there are no options), <c>null</c> is returned.
/// Custom options can be retrieved as extensions of the returned message.
/// NOTE: A defensive copy is created each time this property is retrieved.
/// </summary>
public MethodOptions Options => Proto.Options?.Clone();

/// <summary>
/// Gets a single value method option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public T GetOption<T>(Extension<MethodOptions, T> extension)
{
var value = Proto.Options.GetExtension(extension);
Expand All @@ -88,6 +97,7 @@ public T GetOption<T>(Extension<MethodOptions, T> extension)
/// <summary>
/// Gets a repeated value method option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public RepeatedField<T> GetOption<T>(RepeatedExtension<MethodOptions, T> extension)
{
return Proto.Options.GetExtension(extension).Clone();
Expand Down
12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs
Expand Up @@ -117,12 +117,21 @@ public MessageDescriptor ContainingType
/// <summary>
/// The (possibly empty) set of custom options for this oneof.
/// </summary>
[Obsolete("CustomOptions are obsolete. Use GetOption")]
[Obsolete("CustomOptions are obsolete. Use the Options property.")]
public CustomOptions CustomOptions => new CustomOptions(proto.Options?._extensions?.ValuesByNumber);

/// <summary>
/// The <c>OneofOptions</c>, defined in <c>descriptor.proto</c>.
/// If the options message is not present (=there are no options), <c>null</c> is returned.
/// Custom options can be retrieved as extensions of the returned message.
/// NOTE: A defensive copy is created each time this property is retrieved.
/// </summary>
public OneofOptions Options => proto.Options?.Clone();

/// <summary>
/// Gets a single value oneof option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public T GetOption<T>(Extension<OneofOptions, T> extension)
{
var value = proto.Options.GetExtension(extension);
Expand All @@ -132,6 +141,7 @@ public T GetOption<T>(Extension<OneofOptions, T> extension)
/// <summary>
/// Gets a repeated value oneof option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public RepeatedField<T> GetOption<T>(RepeatedExtension<OneofOptions, T> extension)
{
return proto.Options.GetExtension(extension).Clone();
Expand Down
12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs
Expand Up @@ -94,12 +94,21 @@ public MethodDescriptor FindMethodByName(String name)
/// <summary>
/// The (possibly empty) set of custom options for this service.
/// </summary>
[Obsolete("CustomOptions are obsolete. Use GetOption")]
[Obsolete("CustomOptions are obsolete. Use the Options property.")]
public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber);

/// <summary>
/// The <c>ServiceOptions</c>, defined in <c>descriptor.proto</c>.
/// If the options message is not present (=there are no options), <c>null</c> is returned.
/// Custom options can be retrieved as extensions of the returned message.
/// NOTE: A defensive copy is created each time this property is retrieved.
/// </summary>
public ServiceOptions Options => Proto.Options?.Clone();

/// <summary>
/// Gets a single value service option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public T GetOption<T>(Extension<ServiceOptions, T> extension)
{
var value = Proto.Options.GetExtension(extension);
Expand All @@ -109,6 +118,7 @@ public T GetOption<T>(Extension<ServiceOptions, T> extension)
/// <summary>
/// Gets a repeated value service option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the Options property.")]
public RepeatedField<T> GetOption<T>(RepeatedExtension<ServiceOptions, T> extension)
{
return Proto.Options.GetExtension(extension).Clone();
Expand Down