Add backcompat support for top parameter (#9645)

Copilot · JoshLove-msft · web-flow · commit 3dca795381aa · 2026-02-09T22:04:09.000Z
## Plan: Add backward compatibility support for `top` parameter conversion Based on PR #9505 which added conversion from `top` to `maxCount`, implement similar backward compatibility as was done for `maxPageSize` recasing. ### Changes Required: - [x] Understand the issue and existing code - PR #9505 added conversion from "top" to "maxCount" in RestClientProvider.cs (lines 1016-1021) - Similar backward compatibility exists for "maxPageSize" using GetCorrectedPageSizeName (lines 874-895) - Need to add similar backward compatibility for "top" parameter - [x] Implement backward compatibility for top parameter - Created reusable `GetCorrectedParameterName` helper method that takes originalName, updatedName, and client - Both paging parameter corrections (page size and maxCount) call this helper directly - Preserves parameter name if it exists in previous version (LastContractView) - Applies normalization or conversion if no backward compatibility needed - Consolidated both parameter corrections under single InputPagingServiceMethod check - Made updatedName parameter non-nullable since it's always provided by callers - Moved page size normalization logic inside the parameter match check for efficiency - [x] Add/update tests - Updated test `TopParameterPreservedWhenExistsInLastContractView` to validate LastContractView scenario - Created test data file representing previous contract with "top" parameter - Test validates that "top" is preserved when it exists in LastContractView - All 6 tests passing including test for "top" to "maxCount" conversion when no LastContractView - All PageSizeParameter tests pass (6/6) - [x] Run tests and validate changes - Successfully built C# client generator - All ListPageableTests pass (6/6) - All PageSizeParameter tests pass (6/6) - Verified backward compatibility scenario works correctly - Verified no breaking changes for existing code - [x] Update documentation - Updated backward-compatibility.md with the new top parameter conversion scenario - Added Table of Contents entries for the new subsection - Documented both backward compatibility case (preserves "top") and standardization case (converts to "maxCount") - Renamed "Parameter Name Casing" to "Parameter Naming" to better reflect the section content - [x] Refactor to eliminate code duplication - Created single reusable `GetCorrectedParameterName` helper method - Removed wrapper methods `GetCorrectedPageSizeName` and `GetCorrectedMaxCountName` - Both paging parameters now call the helper directly with appropriate arguments - Maintains same functionality with minimal, maintainable code - [x] Improve code organization - Consolidated paging parameter name corrections under single InputPagingServiceMethod check - Split out specific parameter name matching into separate if blocks within - Improved readability and maintainability - [x] Simplify method signature - Made updatedName parameter non-nullable since all callers provide non-null values - Removed unnecessary null coalesce operator - Simplified return statement and comment - [x] Optimize parameter name correction logic - Moved page size normalization check inside the parameter match condition - Only performs normalization when actually needed (when we find a matching parameter) - More efficient as it avoids unnecessary work on every method call  <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Enable backcompat support for `top` parameter</issue_title> > <issue_description>#9505 added a conversion from top to maxCount. We should add similar back compat support as we added for the `maxPageSize` recasing.</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details>  - Fixes #9643  --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com> Co-authored-by: jolov <jolov@microsoft.com>
diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs
@@ -871,27 +871,25 @@ private static bool TryGetSpecialHeaderParam(InputParameter inputParameter, [Not
                 : null;
         }
 
-        private static string GetCorrectedPageSizeName(string originalName, ClientProvider client)
+        private static void UpdateParameterNameWithBackCompat(InputParameter inputParameter, string proposedName, ClientProvider client)
         {
-            // Check if parameter exists in LastContractView for backward compatibility
+            // Check if the original parameter name exists in LastContractView for backward compatibility
             var existingParam = client.LastContractView?.Methods
                 ?.SelectMany(method => method.Signature.Parameters)
-                .FirstOrDefault(parameter => string.Equals(parameter.Name, originalName, StringComparison.OrdinalIgnoreCase))
+                .FirstOrDefault(p => string.Equals(p.Name, inputParameter.Name, StringComparison.OrdinalIgnoreCase))
                 ?.Name;
 
             if (existingParam != null)
             {
-                return existingParam;
+                // Preserve the exact name (including casing) from the previous contract for backward compatibility
+                proposedName = existingParam;
             }
 
-            // Normalize badly-cased "maxpagesize" variants to Camel Case
-            if (string.Equals(originalName, MaxPageSizeParameterName, StringComparison.OrdinalIgnoreCase))
+            // Use the updated name
+            if (!string.Equals(inputParameter.Name, proposedName, StringComparison.Ordinal))
             {
-                return MaxPageSizeParameterName;
+                inputParameter.Update(name: proposedName);
             }
-
-            // Keep original name for all other cases
-            return originalName;
         }
 
         private static bool ShouldUpdateReinjectedParameter(InputParameter inputParameter, InputPagingServiceMethod? pagingServiceMethod)
@@ -962,12 +960,6 @@ internal static List<ParameterProvider> GetMethodParameters(
 
             var pageSizeParameterName = GetPageSizeParameterName(serviceMethod as InputPagingServiceMethod);
 
-            string? correctedPageSizeName = null;
-            if (pageSizeParameterName != null)
-            {
-                correctedPageSizeName = GetCorrectedPageSizeName(pageSizeParameterName, client);
-            }
-
             ModelProvider? spreadSource = null;
             if (methodType == ScmMethodKind.Convenience)
             {
@@ -1013,19 +1005,24 @@ internal static List<ParameterProvider> GetMethodParameters(
                     continue;
                 }
 
-                // For paging operations, rename "top" parameter to "maxCount"
-                if (serviceMethod is InputPagingServiceMethod &&
-                    string.Equals(inputParam.Name, TopParameterName, StringComparison.OrdinalIgnoreCase))
+                // For paging operations, handle parameter name corrections with backward compatibility
+                if (serviceMethod is InputPagingServiceMethod)
                 {
-                    inputParam.Update(name: MaxCountParameterName);
-                }
+                    // Rename "top" parameter to "maxCount" (with backward compatibility)
+                    if (string.Equals(inputParam.Name, TopParameterName, StringComparison.OrdinalIgnoreCase))
+                    {
+                        UpdateParameterNameWithBackCompat(inputParam, MaxCountParameterName, client);
+                    }
 
-                // For paging operations, ensure page size parameter uses the correct casing
-                if (correctedPageSizeName != null && string.Equals(inputParam.Name, pageSizeParameterName, StringComparison.OrdinalIgnoreCase))
-                {
-                    if (!string.Equals(inputParam.Name, correctedPageSizeName, StringComparison.Ordinal))
+                    // Ensure page size parameter uses the correct casing (with backward compatibility)
+                    if (pageSizeParameterName != null && string.Equals(inputParam.Name, pageSizeParameterName, StringComparison.OrdinalIgnoreCase))
                     {
-                        inputParam.Update(name: correctedPageSizeName);
+                        var updatedPageSizeParameterName = pageSizeParameterName.Equals(MaxPageSizeParameterName, StringComparison.OrdinalIgnoreCase)
+                            ? MaxPageSizeParameterName
+                            : pageSizeParameterName;
+                        // For page size parameters, normalize badly-cased "maxpagesize" variants to proper camelCase, but always
+                        // respect backcompat.
+                        UpdateParameterNameWithBackCompat(inputParam, updatedPageSizeParameterName, client);
                     }
                 }
 
diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/ListPageableTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/ListPageableTests.cs
@@ -1,10 +1,14 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
+using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
 using Microsoft.TypeSpec.Generator.ClientModel.Providers;
 using Microsoft.TypeSpec.Generator.Input;
 using Microsoft.TypeSpec.Generator.Primitives;
+using Microsoft.TypeSpec.Generator.Providers;
 using Microsoft.TypeSpec.Generator.Tests.Common;
 using NUnit.Framework;
 
@@ -69,6 +73,67 @@ public void TopParameterRenamedToMaxCountInPagingOperation()
             Assert.Contains("maxCount", parameterNames, "Should contain 'maxCount' parameter");
             Assert.IsFalse(parameterNames.Contains("top"), "Should not contain 'top' parameter after renaming");
         }
+
+        [Test]
+        public async Task TopParameterPreservedWhenExistsInLastContractView()
+        {
+            // This test verifies that when a "top" parameter exists in LastContractView,
+            // it is preserved for backward compatibility instead of being converted to "maxCount"
+            var topParameter = InputFactory.QueryParameter("top", InputPrimitiveType.Int32, isRequired: false, serializedName: "top");
+
+            List<InputParameter> parameters = [topParameter];
+            List<InputMethodParameter> methodParameters =
+            [
+                InputFactory.MethodParameter("top", InputPrimitiveType.Int32, isRequired: false,
+                    location: InputRequestLocation.Query, serializedName: "top"),
+            ];
+
+            var inputModel = InputFactory.Model("Item", properties:
+            [
+                InputFactory.Property("id", InputPrimitiveType.String, isRequired: true),
+            ]);
+
+            var pagingMetadata = new InputPagingServiceMetadata(
+                ["items"],
+                new InputNextLink(null, ["nextLink"], InputResponseLocation.Body, []),
+                null,
+                null);
+
+            var response = InputFactory.OperationResponse(
+                [200],
+                InputFactory.Model(
+                    "PagedItems",
+                    properties: [
+                        InputFactory.Property("items", InputFactory.Array(inputModel)),
+                        InputFactory.Property("nextLink", InputPrimitiveType.Url)
+                    ]));
+
+            var operation = InputFactory.Operation("getItems", responses: [response], parameters: parameters);
+            var inputServiceMethod = InputFactory.PagingServiceMethod(
+                "getItems",
+                operation,
+                pagingMetadata: pagingMetadata,
+                parameters: methodParameters);
+
+            var client = InputFactory.Client("testClient", methods: [inputServiceMethod]);
+
+            var generator = await MockHelpers.LoadMockGeneratorAsync(
+                clients: () => [client],
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+
+            var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType<ClientProvider>().FirstOrDefault();
+            Assert.IsNotNull(clientProvider);
+            Assert.IsNotNull(clientProvider!.LastContractView);
+
+            var methodParams = RestClientProvider.GetMethodParameters(inputServiceMethod, ScmMethodKind.Convenience, clientProvider!);
+
+            var topParam = methodParams.FirstOrDefault(p =>
+                string.Equals(p.Name, "top", StringComparison.Ordinal));
+
+            Assert.IsNotNull(topParam, "Top parameter should be present in method parameters");
+            Assert.AreEqual("top", topParam!.Name,
+                "Parameter name should be 'top' (from LastContractView), not 'maxCount' (conversion should be prevented)");
+        }
         
         [Test]
         public void NoNextLinkOrContinuationTokenOfT()
diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/TestData/ListPageableTests/TopParameterPreservedWhenExistsInLastContractView/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/TestData/ListPageableTests/TopParameterPreservedWhenExistsInLastContractView/TestClient.cs
@@ -0,0 +1,15 @@
+#nullable disable
+
+using System.ClientModel;
+using System.ClientModel.Primitives;
+using System.Threading.Tasks;
+
+namespace Sample
+{
+    public partial class TestClient
+    {
+        // This represents the previous contract with top parameter
+        public virtual Task<ClientResult> GetItemsAsync(int? top, CancellationToken cancellationToken = default) { return null; }
+        public virtual ClientResult GetItems(int? top, CancellationToken cancellationToken = default) { return null; }
+    }
+}
diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md
@@ -11,7 +11,9 @@
   - [API Version Enum](#api-version-enum)
   - [Non-abstract Base Models](#non-abstract-base-models)
   - [Model Constructors](#model-constructors)
-  - [Parameter Name Casing](#parameter-name-casing)
+  - [Parameter Naming](#parameter-naming)
+    - [Page Size Parameter Casing Correction](#scenario-page-size-parameter-casing-correction)
+    - [Top Parameter Conversion to MaxCount](#scenario-top-parameter-conversion-to-maxcount)
 
 ## Overview
 
@@ -361,9 +363,9 @@ public abstract partial class SearchIndexerDataIdentity
 - The modifier is changed from `private protected` to `public`
 - No additional constructors are generated; only the accessibility is adjusted
 
-### Parameter Name Casing
+### Parameter Naming
 
-The generator maintains backward compatibility for parameter names to ensure that existing code continues to compile when parameter name casing is corrected or standardized.
+The generator maintains backward compatibility for parameter names to ensure that existing code continues to compile when parameter names are corrected, standardized, or converted to follow naming conventions.
 
 #### Scenario: Page Size Parameter Casing Correction
 
@@ -475,3 +477,116 @@ public virtual AsyncPageable<Item> GetItemsAsync(int? maxPageSize = null, Cancel
 - **Case 2 (Normalization)**: If the parameter does NOT exist in LastContractView, badly-cased variants are normalized to proper camelCase
 - The HTTP query parameter always uses the original serialized name from the spec (e.g., `maxpagesize`)
 - Existing client code continues to compile without changes
+
+#### Scenario: Top Parameter Conversion to MaxCount
+
+**Description:** For paging operations, the generator converts the `top` parameter to `maxCount` to follow standard naming conventions. However, backward compatibility is maintained in two ways:
+
+1. **If the `top` parameter exists in LastContractView**: The generator preserves the `top` parameter name (with its exact casing) to maintain backward compatibility
+2. **If the `top` parameter does NOT exist in LastContractView**: The generator converts `top` to the standardized `maxCount` parameter name
+
+This commonly occurs when:
+
+- Migrating from an older API version or generator that used `top` for pagination limits
+- TypeSpec defines paging operations with a `top` parameter
+- The generator needs to standardize on `maxCount` while maintaining backward compatibility
+
+**Example:**
+
+**Case 1: Top parameter exists in LastContractView - preserved for backward compatibility**
+
+Previous version had `top` parameter:
+
+```csharp
+public virtual AsyncPageable<Item> GetItemsAsync(int? top = null, CancellationToken cancellationToken = default)
+{
+    HttpMessage CreateRequest()
+    {
+        var message = pipeline.CreateMessage();
+        var request = message.Request;
+        request.Method = RequestMethod.Get;
+        var uri = new RequestUriBuilder();
+        uri.Reset(endpoint);
+        uri.AppendPath("/items", false);
+        if (top != null)
+        {
+            uri.AppendQuery("top", top.Value, true);  // Serialized name from spec
+        }
+        // ...
+    }
+    // ...
+}
+```
+
+Current TypeSpec still defines `top` parameter:
+
+```typespec
+op getItems(@query top?: int32): Page<Item>;
+```
+
+**Generated Compatibility Result:**
+
+The generator detects the `top` parameter in LastContractView and preserves it exactly to maintain backward compatibility:
+
+```csharp
+public virtual AsyncPageable<Item> GetItemsAsync(int? top = null, CancellationToken cancellationToken = default)
+{
+    HttpMessage CreateRequest()
+    {
+        var message = pipeline.CreateMessage();
+        var request = message.Request;
+        request.Method = RequestMethod.Get;
+        var uri = new RequestUriBuilder();
+        uri.Reset(endpoint);
+        uri.AppendPath("/items", false);
+        if (top != null)
+        {
+            uri.AppendQuery("top", top.Value, true);  // Still uses "top" for backward compatibility
+        }
+        // ...
+    }
+    // ...
+}
+```
+
+**Case 2: Top parameter does NOT exist in LastContractView - converted to maxCount**
+
+New paging operation with `top` parameter (no previous version):
+
+```typespec
+op getItems(@query top?: int32): Page<Item>;
+```
+
+**Generated Result:**
+
+The generator converts the parameter name to standardized `maxCount` since there's no previous version to maintain compatibility with:
+
+```csharp
+public virtual AsyncPageable<Item> GetItemsAsync(int? maxCount = null, CancellationToken cancellationToken = default)
+{
+    HttpMessage CreateRequest()
+    {
+        var message = pipeline.CreateMessage();
+        var request = message.Request;
+        request.Method = RequestMethod.Get;
+        var uri = new RequestUriBuilder();
+        uri.Reset(endpoint);
+        uri.AppendPath("/items", false);
+        if (maxCount != null)
+        {
+            uri.AppendQuery("top", maxCount.Value, true);  // Serialized name still uses "top" from spec
+        }
+        // ...
+    }
+    // ...
+}
+```
+
+**Key Points:**
+
+- **Case 1 (Backward compatibility)**: If `top` parameter exists in LastContractView, its exact name and casing are preserved
+- **Case 2 (Standardization)**: If `top` parameter does NOT exist in LastContractView, it is converted to `maxCount` for consistency
+- The HTTP query parameter always uses the original serialized name from the spec (e.g., `top`)
+- This conversion is specific to paging operations only
+- Existing client code with `top` continues to compile without changes
+- New code benefits from the standardized `maxCount` naming convention
