[robertvoinescu-work]

b/543020713

When a paginated method's resources field is marked deprecated, the generated code hits a deprecated warning. In this case the method should not be deprecated and we should disable the warning on the repeated field while we are using this implementation.

As a concrete example, for Apps.V1 chat the Spaces field for SearchSpacesResponse is deprecated and will be replaced with the new Results field. But while Spaces exists it is the lowest numbered repeated field so it will be used for pagination. We do not mark the enumeration methods as obsolete as what is obsolete is our implementation (using Spaces.GetEnumerator instead of the new repeated Results.Enumerator field to get the enumerator - see below), once Spaces field is removed from the proto our implementation will become the following and not throw (validated):

public scg::IEnumerator<Types.SearchSpaceResult> GetEnumerator() => Results.GetEnumerator();

Validation

• I've regenerated all libraries and the only changes present where those for Apps Chat V1 fixing the issue:

diff --git a/apis/Google.Apps.Chat.V1/Google.Apps.Chat.V1/ChatServiceClient.g.cs b/apis/Google.Apps.Chat.V1/Google.Apps.Chat.V1/ChatServiceClient.g.cs
--- a/apis/Google.Apps.Chat.V1/Google.Apps.Chat.V1/ChatServiceClient.g.cs
+++ b/apis/Google.Apps.Chat.V1/Google.Apps.Chat.V1/ChatServiceClient.g.cs
@@ -19286,7 +19286,9 @@
     public partial class SearchSpacesResponse : gaxgrpc::IPageResponse<Space>
     {
         /// <summary>Returns an enumerator that iterates through the resources in this response.</summary>
+#pragma warning disable CS0612
         public scg::IEnumerator<Space> GetEnumerator() => Spaces.GetEnumerator();
+#pragma warning restore CS0612
 
         sc::IEnumerator sc::IEnumerable.GetEnumerator() => GetEnumerator();
     }

[gemini-code-assist]

Code Review

This pull request updates the API generator to mark a paginated method and its enumerators as deprecated if its underlying resources field is deprecated. It also updates the snippet generator to wrap page iterations in obsolete warning suppressions (#pragma warning disable CS0612) when the method is deprecated. Tests, fakes, and generated snippets have been added to verify this behavior. There are no review comments to address, so I have no feedback to provide.

[efevans]

I see there's an extension method on the FieldDescriptor class similar to this except it's also checking if the containing type is deprecated:

        public static bool IsDeprecated(this FieldDescriptor desc) =>
            (desc?.GetOptions()?.Deprecated ?? false) || IsDeprecated(desc?.ContainingType);

Is that something we'll need here?

[robertvoinescu-work]

Good call out, but it's not required in this case. That would result in the following getting wrapped:

+#pragma warning disable CS0612
        sc::IEnumerator sc::IEnumerable.GetEnumerator() => GetEnumerator())
+#pragma warning restore CS0612

Although this method doesn't need to get wrapped as it doesn't reference the obsolete type.

[efevans]

I see it's being used now, did you find something that caused a change of heart?

[robertvoinescu-work]

Yes sorry forgot to update here. I was wrong totally wrong in my initial comment - I ran it and that doesn't actually get wrapped 🤦 . So I went ahead and used IsDeprecated. Thanks for pointing it out.

[amanda-tarafa]

Ideally API teams would mark this method as deprecated when they mark the list property deprecated. But I don't' think that's an easy thing for us to do, not sure if it's our decision to make. Just leaving this comment for posterity.