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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from May 11, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
944055a
expose Options property on descriptors
jtattermusch Apr 28, 2020
9720ae4
adjust CustomOptionsTest.cs
jtattermusch Apr 28, 2020
f4ff220
add a few more tests
jtattermusch Apr 28, 2020
062c2dc
add back GetOption methods, but mark them as obsolete
jtattermusch Apr 28, 2020
402ce8b
add back GetOption tests
jtattermusch Apr 28, 2020
673245f
remove unnecessary cast
jtattermusch Apr 29, 2020
ff999c9
clarify docs
jtattermusch Apr 29, 2020
0688883
address review comments
jtattermusch May 6, 2020
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
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 11 additions & 1 deletion csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs
View file
Open in desktop
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 GetOptions() method.")]
public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber);

/// <summary>
Copy link
Contributor

@JamesNK JamesNK Apr 28, 2020 •
edited

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 (i.e. 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 EnumOptions GetOptions() => Proto.Options?.Clone();

/// <summary>
/// Gets a single value enum option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the GetOptions() method.")]
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 GetOptions() method.")]
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
View file
Open in desktop
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 GetOptions() method.")]
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 (i.e. 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 GetOptions() => Proto.Options?.Clone();

/// <summary>
/// Gets a single value enum value option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the GetOptions() method.")]
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 GetOptions() method.")]
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
View file
Open in desktop
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 GetOptions() method.")]
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 (i.e. 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 GetOptions() => Proto.Options?.Clone();

/// <summary>
/// Gets a single value field option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the GetOptions() method.")]
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 GetOptions() method.")]
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
View file
Open in desktop
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 GetOptions() method.")]
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 (i.e. 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 GetOptions() => Proto.Options?.Clone();

/// <summary>
/// Gets a single value file option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the GetOptions() method.")]
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 GetOptions() method.")]
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
View file
Open in desktop
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 GetOptions() method.")]
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 (i.e. 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 GetOptions() => Proto.Options?.Clone();

/// <summary>
/// Gets a single value message option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the GetOptions() method.")]
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 GetOptions() method.")]
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
View file
Open in desktop
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 GetOptions() method.")]
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 (i.e. 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 GetOptions() => Proto.Options?.Clone();

/// <summary>
/// Gets a single value method option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the GetOptions() method.")]
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 GetOptions() method.")]
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
View file
Open in desktop
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 GetOptions method.")]
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 (i.e. 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 GetOptions() => proto.Options?.Clone();

/// <summary>
/// Gets a single value oneof option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the GetOptions() method.")]
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 GetOptions() method.")]
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
View file
Open in desktop
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 GetOptions() method.")]
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 (i.e. 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 GetOptions() => Proto.Options?.Clone();

/// <summary>
/// Gets a single value service option for this descriptor
/// </summary>
[Obsolete("GetOption is obsolete. Use the GetOptions() method.")]
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 GetOptions() method.")]
public RepeatedField<T> GetOption<T>(RepeatedExtension<ServiceOptions, T> extension)
{
return Proto.Options.GetExtension(extension).Clone();
Expand Down