From b6d5faaec1eae826db2dfc56d89c0384cd3cb0ef Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 21:01:47 +0000 Subject: [PATCH 01/13] Initial plan From c20d2ae17c08edb07374fb5b051aeaa942128f8e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 21:18:46 +0000 Subject: [PATCH 02/13] Add back-compat overload for new optional non-body parameters Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/bb773994-13fe-4a5a-971a-b594a6d82388 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 225 ++++++++++++++++- .../ClientProviders/ClientProviderTests.cs | 228 ++++++++++++++++++ .../TestClient.cs | 21 ++ .../TestClient.cs | 21 ++ .../TestClient.cs | 21 ++ .../TestClient.cs | 21 ++ .../BackCompatibilityChangeCategory.cs | 3 + .../src/EmitterRpc/Emitter.cs | 1 + .../generator/docs/backward-compatibility.md | 50 ++++ 9 files changed, 588 insertions(+), 3 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded/TestClient.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload/TestClient.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded/TestClient.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload/TestClient.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index 2287b15c23d..2dcdf9cdf8e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1261,7 +1261,8 @@ protected sealed override IReadOnlyList BuildMethodsForBackCompa return [.. originalMethods]; } - var currentMethodSignatures = BuildCurrentMethodSignatures(originalMethods); + var materializedMethods = originalMethods as IList ?? [.. originalMethods]; + var currentMethodSignatures = BuildCurrentMethodSignatures(materializedMethods); var updatedSignatureToOriginal = new Dictionary(MethodSignature.MethodSignatureComparer); var methodsWithReorderedParams = new List(); @@ -1290,10 +1291,14 @@ protected sealed override IReadOnlyList BuildMethodsForBackCompa if (methodsWithReorderedParams.Count > 0) { - UpdateConvenienceMethodsForBackCompat(originalMethods, methodsWithReorderedParams, updatedSignatureToOriginal); + UpdateConvenienceMethodsForBackCompat(materializedMethods, methodsWithReorderedParams, updatedSignatureToOriginal); } - return [.. originalMethods]; + // Add back-compat overloads for methods that have gained one or more new optional non-body parameter(s) + // relative to the last contract. See https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter + var resultMethods = new List(materializedMethods); + AddBackCompatOverloadsForNewOptionalParameters(resultMethods); + return resultMethods; } private Dictionary BuildCurrentMethodSignatures(IEnumerable originalMethods) @@ -1748,5 +1753,219 @@ private static void UpdateXmlDocProviderForParamReorder( xmlDocs.Update(parameters: reorderedParamDocs); } } + + /// + /// For each public/protected method on the last contract that does not have an exact match in the + /// current contract, attempts to find a corresponding current method whose parameter list is the + /// previous method's parameter list (in the same order) plus one or more additional optional + /// non-body parameters. When such a current method is found, a hidden back-compat overload that + /// matches the previous signature is added; its body simply delegates to the current method, + /// passing default values for the new parameters. + /// + /// Per https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter + /// this back-compat behavior is intentionally restricted to non-body parameters. + /// + private void AddBackCompatOverloadsForNewOptionalParameters(List methods) + { + if (LastContractView?.Methods == null || LastContractView.Methods.Count == 0) + { + return; + } + + var existingSignatures = new HashSet(MethodSignature.MethodSignatureComparer); + foreach (var m in methods) + { + existingSignatures.Add(m.Signature); + } + if (CustomCodeView?.Methods != null) + { + foreach (var m in CustomCodeView.Methods) + { + existingSignatures.Add(m.Signature); + } + } + + // Group current methods by name (using their post-reorder signatures). + var currentMethodsByName = new Dictionary>(StringComparer.Ordinal); + foreach (var method in methods) + { + if (!currentMethodsByName.TryGetValue(method.Signature.Name, out var list)) + { + list = []; + currentMethodsByName[method.Signature.Name] = list; + } + list.Add(method); + } + + var newMethods = new List(); + + foreach (var previousMethod in LastContractView.Methods) + { + var previousSignature = previousMethod.Signature; + + // Only process public/protected methods. + if (!previousSignature.Modifiers.HasFlag(MethodSignatureModifiers.Public) && + !previousSignature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)) + { + continue; + } + + // Skip if the current contract already contains an exact-matching signature. + if (existingSignatures.Contains(previousSignature)) + { + continue; + } + + if (!currentMethodsByName.TryGetValue(previousSignature.Name, out var candidates)) + { + continue; + } + + MethodProvider? matchedCurrent = null; + foreach (var candidate in candidates) + { + if (HasNewOptionalNonBodyParametersOnly(previousSignature, candidate.Signature)) + { + matchedCurrent = candidate; + break; + } + } + + if (matchedCurrent == null) + { + continue; + } + + var overload = BuildBackCompatOverloadForNewOptionalParameters(previousMethod, matchedCurrent); + if (overload == null) + { + continue; + } + + if (existingSignatures.Add(overload.Signature)) + { + newMethods.Add(overload); + CodeModelGenerator.Instance.Emitter.Debug( + $"Added back-compat overload for '{Name}.{previousSignature.Name}' to handle new optional parameter(s) introduced relative to the last contract.", + BackCompatibilityChangeCategory.MethodNewOptionalParameterOverloadAdded); + } + } + + methods.AddRange(newMethods); + } + + /// + /// Returns true when contains all parameters of + /// in the same order, and the additional parameters are + /// all optional and none of them are body parameters. + /// + private static bool HasNewOptionalNonBodyParametersOnly( + MethodSignature previousSignature, + MethodSignature currentSignature) + { + if (currentSignature.Parameters.Count <= previousSignature.Parameters.Count) + { + return false; + } + + if (!ReturnTypesMatch(previousSignature.ReturnType, currentSignature.ReturnType)) + { + return false; + } + + // Walk current parameters and ensure previous parameters appear in the same relative order + // (matched by variable name and type), with every "extra" parameter being optional and non-body. + int previousIndex = 0; + for (int currentIndex = 0; currentIndex < currentSignature.Parameters.Count; currentIndex++) + { + var currentParam = currentSignature.Parameters[currentIndex]; + + if (previousIndex < previousSignature.Parameters.Count) + { + var previousParam = previousSignature.Parameters[previousIndex]; + if (currentParam.Name.ToVariableName() == previousParam.Name.ToVariableName() && + currentParam.Type.AreNamesEqual(previousParam.Type)) + { + previousIndex++; + continue; + } + } + + // current parameter is "new" relative to the previous contract + if (currentParam.DefaultValue is null) + { + return false; + } + + if (currentParam.Location == ParameterLocation.Body) + { + return false; + } + } + + // All previous parameters must have been matched. + return previousIndex == previousSignature.Parameters.Count; + } + + private static bool ReturnTypesMatch(CSharpType? a, CSharpType? b) + { + if (ReferenceEquals(a, b)) + { + return true; + } + if (a is null || b is null) + { + return false; + } + return a.AreNamesEqual(b); + } + + private MethodProvider? BuildBackCompatOverloadForNewOptionalParameters( + MethodProvider previousMethod, + MethodProvider currentMethod) + { + var previousSignature = previousMethod.Signature; + var currentSignature = currentMethod.Signature; + + var previousParamsByName = new Dictionary(StringComparer.Ordinal); + foreach (var p in previousSignature.Parameters) + { + previousParamsByName[p.Name.ToVariableName()] = p; + } + + var arguments = new List(currentSignature.Parameters.Count); + foreach (var currentParam in currentSignature.Parameters) + { + if (previousParamsByName.TryGetValue(currentParam.Name.ToVariableName(), out var prevParam)) + { + arguments.Add(prevParam); + } + else + { + arguments.Add(currentParam.DefaultValue ?? Default); + } + } + + // Always invoke without async/await: the back-compat overload simply delegates to the current + // method. For async methods, returning the Task directly is sufficient and keeps the back-compat + // overload itself non-async (its signature was loaded from the previous DLL contract and does + // not carry the async modifier). Constructing InvokeMethodExpression directly leaves + // CallAsAsync / AddConfigureAwaitFalse at their default false value. + var invocation = new InvokeMethodExpression(This, currentSignature, arguments); + + MethodBodyStatement body = previousSignature.ReturnType is null + ? invocation.Terminate() + : Return(invocation); + + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature(previousSignature, hideMethod: true); + var kind = (currentMethod as ScmMethodProvider)?.Kind ?? ScmMethodKind.Convenience; + return new ScmMethodProvider( + backCompatSignature, + body, + this, + kind, + xmlDocProvider: previousMethod.XmlDocs, + serviceMethod: (currentMethod as ScmMethodProvider)?.ServiceMethod); + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs index 4af2c5add7f..5465ff6c5d7 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs @@ -3149,6 +3149,234 @@ public async Task BackCompatibility_DuplicateMethodSignatureDoesNotThrow() Assert.DoesNotThrow(() => processMethod?.Invoke(clientProvider, null)); } + // Last contract has GetData(string param1, int param2, CancellationToken) (and async). + // The current TypeSpec adds a new optional non-body (header) parameter `param3`. + // Expected: a hidden back-compat overload matching the previous signature is added that + // delegates to the new method, passing default for the new parameter. + [Test] + public async Task BackCompatibility_NewOptionalNonBodyParameterAdded() + { + var param1 = InputFactory.BodyParameter("param1", InputPrimitiveType.String, isRequired: true); + var param2 = InputFactory.QueryParameter("param2", InputPrimitiveType.Int32, isRequired: true); + var param3 = InputFactory.HeaderParameter("param3", InputPrimitiveType.Boolean, isRequired: false); + + var operation = InputFactory.Operation( + "GetData", + parameters: [param1, param2, param3], + responses: [InputFactory.OperationResponse([200], bodytype: InputPrimitiveType.String)]); + + List methodParameters = + [ + InputFactory.MethodParameter("param1", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: true), + InputFactory.MethodParameter("param2", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("param3", InputPrimitiveType.Boolean, location: InputRequestLocation.Header, isRequired: false), + ]; + + var method = InputFactory.BasicServiceMethod("GetData", operation, parameters: [.. methodParameters]); + var client = InputFactory.Client(TestClientName, methods: [method]); + + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType().FirstOrDefault(); + Assert.IsNotNull(clientProvider); + Assert.IsNotNull(clientProvider!.LastContractView); + + var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + processMethod?.Invoke(clientProvider, null); + + var allGetDataConvenienceMethods = clientProvider!.Methods + .Where(m => m.Signature.Name.StartsWith("GetData") && m is ScmMethodProvider { Kind: ScmMethodKind.Convenience }) + .ToList(); + + // Expect 4 convenience methods: current sync/async (with param3) + back-compat sync/async (without param3) + Assert.AreEqual(4, allGetDataConvenienceMethods.Count); + + var backCompatSync = allGetDataConvenienceMethods + .FirstOrDefault(m => m.Signature.Name == "GetData" && m.Signature.Parameters.All(p => p.Name != "param3")); + var backCompatAsync = allGetDataConvenienceMethods + .FirstOrDefault(m => m.Signature.Name == "GetDataAsync" && m.Signature.Parameters.All(p => p.Name != "param3")); + Assert.IsNotNull(backCompatSync, "Expected a back-compat sync overload without param3."); + Assert.IsNotNull(backCompatAsync, "Expected a back-compat async overload without param3."); + + // The back-compat overloads should be marked with [EditorBrowsable(EditorBrowsableState.Never)] + // and should not have default values on the parameters. + foreach (var backCompat in new[] { backCompatSync!, backCompatAsync! }) + { + Assert.AreEqual(1, backCompat.Signature.Attributes.Count); + Assert.AreEqual( + "[global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)]\n", + backCompat.Signature.Attributes[0].ToDisplayString()); + foreach (var p in backCompat.Signature.Parameters) + { + Assert.IsNull(p.DefaultValue); + } + } + + // Validate the back-compat sync body delegates to the current sync method with default for param3. + var syncBody = backCompatSync!.BodyStatements!.ToDisplayString(); + Assert.AreEqual("return this.GetData(param2, param1, default, cancellationToken);\n", syncBody); + + // Validate the back-compat async body delegates to the current async method (no await; returns Task directly). + var asyncBody = backCompatAsync!.BodyStatements!.ToDisplayString(); + Assert.AreEqual("return this.GetDataAsync(param2, param1, default, cancellationToken);\n", asyncBody); + } + + // The current TypeSpec adds two new optional non-body parameters relative to the last contract. + // Expected: a single back-compat overload matching the previous signature is added that + // delegates to the new method, passing default for both new parameters. + [Test] + public async Task BackCompatibility_MultipleNewOptionalNonBodyParametersAdded() + { + var param1 = InputFactory.BodyParameter("param1", InputPrimitiveType.String, isRequired: true); + var param2 = InputFactory.QueryParameter("param2", InputPrimitiveType.Int32, isRequired: true); + var param3 = InputFactory.HeaderParameter("param3", InputPrimitiveType.Boolean, isRequired: false); + var param4 = InputFactory.QueryParameter("param4", InputPrimitiveType.String, isRequired: false); + + var operation = InputFactory.Operation( + "GetData", + parameters: [param1, param2, param3, param4], + responses: [InputFactory.OperationResponse([200], bodytype: InputPrimitiveType.String)]); + + List methodParameters = + [ + InputFactory.MethodParameter("param1", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: true), + InputFactory.MethodParameter("param2", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("param3", InputPrimitiveType.Boolean, location: InputRequestLocation.Header, isRequired: false), + InputFactory.MethodParameter("param4", InputPrimitiveType.String, location: InputRequestLocation.Query, isRequired: false), + ]; + + var method = InputFactory.BasicServiceMethod("GetData", operation, parameters: [.. methodParameters]); + var client = InputFactory.Client(TestClientName, methods: [method]); + + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType().FirstOrDefault(); + Assert.IsNotNull(clientProvider); + Assert.IsNotNull(clientProvider!.LastContractView); + + var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + processMethod?.Invoke(clientProvider, null); + + var convenienceMethods = clientProvider!.Methods + .Where(m => m.Signature.Name.StartsWith("GetData") && m is ScmMethodProvider { Kind: ScmMethodKind.Convenience }) + .ToList(); + + Assert.AreEqual(4, convenienceMethods.Count); + + // Each back-compat overload should have only the previous-contract parameters. + var backCompatSync = convenienceMethods + .FirstOrDefault(m => m.Signature.Name == "GetData" + && m.Signature.Parameters.All(p => p.Name != "param3" && p.Name != "param4")); + var backCompatAsync = convenienceMethods + .FirstOrDefault(m => m.Signature.Name == "GetDataAsync" + && m.Signature.Parameters.All(p => p.Name != "param3" && p.Name != "param4")); + Assert.IsNotNull(backCompatSync); + Assert.IsNotNull(backCompatAsync); + + // The back-compat overloads should pass `default` for both new parameters. + Assert.AreEqual( + "return this.GetData(param2, param1, default, default, cancellationToken);\n", + backCompatSync!.BodyStatements!.ToDisplayString()); + Assert.AreEqual( + "return this.GetDataAsync(param2, param1, default, default, cancellationToken);\n", + backCompatAsync!.BodyStatements!.ToDisplayString()); + } + + // When the new parameter is a body parameter, the back-compat overload should NOT be added. + // See https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter + [Test] + public async Task BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload() + { + // Last contract had a GetData(int param2, CancellationToken) method (no body parameter). + // Current method adds an optional body parameter `param1`. + var param1 = InputFactory.BodyParameter("param1", InputPrimitiveType.String, isRequired: false); + var param2 = InputFactory.QueryParameter("param2", InputPrimitiveType.Int32, isRequired: true); + + var operation = InputFactory.Operation( + "GetData", + parameters: [param1, param2], + responses: [InputFactory.OperationResponse([200], bodytype: InputPrimitiveType.String)]); + + List methodParameters = + [ + InputFactory.MethodParameter("param1", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: false), + InputFactory.MethodParameter("param2", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + ]; + + var method = InputFactory.BasicServiceMethod("GetData", operation, parameters: [.. methodParameters]); + var client = InputFactory.Client(TestClientName, methods: [method]); + + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType().FirstOrDefault(); + Assert.IsNotNull(clientProvider); + Assert.IsNotNull(clientProvider!.LastContractView); + + var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + processMethod?.Invoke(clientProvider, null); + + var convenienceMethods = clientProvider!.Methods + .Where(m => m.Signature.Name.StartsWith("GetData") && m is ScmMethodProvider { Kind: ScmMethodKind.Convenience }) + .ToList(); + + // Only the current sync/async pair: no back-compat overload because the new parameter is a body parameter. + Assert.AreEqual(2, convenienceMethods.Count); + foreach (var m in convenienceMethods) + { + Assert.IsTrue(m.Signature.Parameters.Any(p => p.Name == "param1")); + } + } + + // When the new parameter is required (not optional), the back-compat overload should NOT be added, + // even if it is non-body. Adding such an overload would require us to invent a value for a required + // parameter, which would be unsafe. + [Test] + public async Task BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload() + { + var param1 = InputFactory.BodyParameter("param1", InputPrimitiveType.String, isRequired: true); + var param2 = InputFactory.QueryParameter("param2", InputPrimitiveType.Int32, isRequired: true); + var param3 = InputFactory.HeaderParameter("param3", InputPrimitiveType.Boolean, isRequired: true); + + var operation = InputFactory.Operation( + "GetData", + parameters: [param1, param2, param3], + responses: [InputFactory.OperationResponse([200], bodytype: InputPrimitiveType.String)]); + + List methodParameters = + [ + InputFactory.MethodParameter("param1", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: true), + InputFactory.MethodParameter("param2", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("param3", InputPrimitiveType.Boolean, location: InputRequestLocation.Header, isRequired: true), + ]; + + var method = InputFactory.BasicServiceMethod("GetData", operation, parameters: [.. methodParameters]); + var client = InputFactory.Client(TestClientName, methods: [method]); + + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType().FirstOrDefault(); + Assert.IsNotNull(clientProvider); + Assert.IsNotNull(clientProvider!.LastContractView); + + var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + processMethod?.Invoke(clientProvider, null); + + var convenienceMethods = clientProvider!.Methods + .Where(m => m.Signature.Name.StartsWith("GetData") && m is ScmMethodProvider { Kind: ScmMethodKind.Convenience }) + .ToList(); + + // Only the current sync/async pair: no back-compat overload because the new parameter is required. + Assert.AreEqual(2, convenienceMethods.Count); + } + [Test] public void ServerTemplateWithBasePathOnly_DoesNotDuplicateBasePath() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded/TestClient.cs new file mode 100644 index 00000000000..18ff51338f3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded/TestClient.cs @@ -0,0 +1,21 @@ +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload/TestClient.cs new file mode 100644 index 00000000000..0b21ee524da --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload/TestClient.cs @@ -0,0 +1,21 @@ +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + public virtual ClientResult GetData(int param2, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public virtual Task> GetDataAsync(int param2, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded/TestClient.cs new file mode 100644 index 00000000000..18ff51338f3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded/TestClient.cs @@ -0,0 +1,21 @@ +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload/TestClient.cs new file mode 100644 index 00000000000..18ff51338f3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload/TestClient.cs @@ -0,0 +1,21 @@ +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs index dbd213f608f..369662795a2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs @@ -39,5 +39,8 @@ public enum BackCompatibilityChangeCategory /// A back-compat model factory method could not be created and was skipped. ModelFactoryMethodSkipped, + + /// A back-compat overload of a client method was added because new optional non-body parameter(s) were introduced relative to the last contract. + MethodNewOptionalParameterOverloadAdded, } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs index 7fed8e03981..8cfca5f86eb 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs @@ -180,6 +180,7 @@ public void WriteBufferedMessages() BackCompatibilityChangeCategory.ModelFactoryMethodReplaced => "Model Factory Method Replaced For Back-Compat", BackCompatibilityChangeCategory.ModelFactoryMethodAdded => "Model Factory Method Added For Back-Compat", BackCompatibilityChangeCategory.ModelFactoryMethodSkipped => "Model Factory Method Back-Compat Skipped", + BackCompatibilityChangeCategory.MethodNewOptionalParameterOverloadAdded => "Method Back-Compat Overload Added For New Optional Parameter", _ => category.ToString(), }; diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index 3dda7eceeed..6997b65a842 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -21,6 +21,8 @@ - [Method Parameter Name Preserved from Last Contract](#scenario-method-parameter-name-preserved-from-last-contract) - [Content-Type Parameter Ordering](#content-type-parameter-ordering) - [Content-Type Before Body Preserved from Last Contract](#scenario-content-type-before-body-preserved-from-last-contract) + - [Service Method Evolution](#service-method-evolution) + - [New Optional Non-Body Parameter Added to a Service Method](#scenario-new-optional-non-body-parameter-added-to-a-service-method) ## Overview @@ -784,3 +786,51 @@ public virtual ClientResult UpdateSkillDefaultVersion(string skillId, string con // contentType stays before content for backward compatibility } ``` + +### Service Method Evolution + +#### Scenario: New Optional Non-Body Parameter Added to a Service Method + +**Description:** When the current TypeSpec adds one or more new optional non-body parameters (e.g. query, header, path) to an existing service method, the generator emits a hidden back-compat overload that matches the previous contract's signature and delegates to the new method, passing `default` for the new parameter(s). + +This follows the guidance in [Service-Driven Evolution: A method gets a new optional parameter](https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter). The behavior is **intentionally restricted to non-body parameters** because adding a body parameter typically reflects a schema change and is handled differently. + +**Rules:** + +- The previous method's parameters must appear, in the same relative order and matching by name and type, within the current method's parameters. +- Every "extra" parameter on the current method must be optional (i.e. have a default value). +- No "extra" parameter on the current method may be a body parameter; if any extra parameter is a body parameter, no back-compat overload is added. +- The back-compat overload is hidden via `[EditorBrowsable(EditorBrowsableState.Never)]` and has all default values stripped from its parameters to avoid ambiguous call sites. + +**Example:** + +Previous version of the client: + +```csharp +public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken = default); +public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken = default); +``` + +Current TypeSpec adds an optional header parameter `param3`: + +```typespec +op getData(@query param2: int32, @body param1: string, @header param3?: boolean): string; +``` + +**Generated Compatibility Methods:** + +```csharp +[EditorBrowsable(EditorBrowsableState.Never)] +public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken) +{ + return this.GetData(param2, param1, default, cancellationToken); +} + +[EditorBrowsable(EditorBrowsableState.Never)] +public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken) +{ + return this.GetDataAsync(param2, param1, default, cancellationToken); +} +``` + +The same logic applies when more than one new optional non-body parameter is added — the back-compat overload simply passes `default` for each new parameter when delegating to the current method. From 81e56f3d60ea27ebdafa8b98eba1a04b0e1ba406 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 21:38:21 +0000 Subject: [PATCH 03/13] Refactor: extract reorder helper, simplify comments, reuse signature dict Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/98e4708e-374c-4348-8b5b-30e4c877bd56 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 60 ++++++++----------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index 2dcdf9cdf8e..8caa5de4c55 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1263,10 +1263,23 @@ protected sealed override IReadOnlyList BuildMethodsForBackCompa var materializedMethods = originalMethods as IList ?? [.. originalMethods]; var currentMethodSignatures = BuildCurrentMethodSignatures(materializedMethods); + + ApplyParameterReorderingForBackCompat(materializedMethods, currentMethodSignatures); + + // Add back-compat overloads for methods that have gained one or more new optional non-body parameter(s). + var resultMethods = new List(materializedMethods); + AddBackCompatOverloadsForNewOptionalParameters(resultMethods, currentMethodSignatures); + return resultMethods; + } + + private void ApplyParameterReorderingForBackCompat( + IList materializedMethods, + Dictionary currentMethodSignatures) + { var updatedSignatureToOriginal = new Dictionary(MethodSignature.MethodSignatureComparer); var methodsWithReorderedParams = new List(); - foreach (var previousMethod in LastContractView.Methods) + foreach (var previousMethod in LastContractView!.Methods) { if (!ShouldProcessMethodForBackCompat(previousMethod.Signature, currentMethodSignatures)) { @@ -1293,12 +1306,6 @@ protected sealed override IReadOnlyList BuildMethodsForBackCompa { UpdateConvenienceMethodsForBackCompat(materializedMethods, methodsWithReorderedParams, updatedSignatureToOriginal); } - - // Add back-compat overloads for methods that have gained one or more new optional non-body parameter(s) - // relative to the last contract. See https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter - var resultMethods = new List(materializedMethods); - AddBackCompatOverloadsForNewOptionalParameters(resultMethods); - return resultMethods; } private Dictionary BuildCurrentMethodSignatures(IEnumerable originalMethods) @@ -1754,40 +1761,20 @@ private static void UpdateXmlDocProviderForParamReorder( } } - /// - /// For each public/protected method on the last contract that does not have an exact match in the - /// current contract, attempts to find a corresponding current method whose parameter list is the - /// previous method's parameter list (in the same order) plus one or more additional optional - /// non-body parameters. When such a current method is found, a hidden back-compat overload that - /// matches the previous signature is added; its body simply delegates to the current method, - /// passing default values for the new parameters. - /// - /// Per https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter - /// this back-compat behavior is intentionally restricted to non-body parameters. - /// - private void AddBackCompatOverloadsForNewOptionalParameters(List methods) + private void AddBackCompatOverloadsForNewOptionalParameters( + List methods, + Dictionary currentMethodSignatures) { if (LastContractView?.Methods == null || LastContractView.Methods.Count == 0) { return; } - var existingSignatures = new HashSet(MethodSignature.MethodSignatureComparer); - foreach (var m in methods) - { - existingSignatures.Add(m.Signature); - } - if (CustomCodeView?.Methods != null) - { - foreach (var m in CustomCodeView.Methods) - { - existingSignatures.Add(m.Signature); - } - } - - // Group current methods by name (using their post-reorder signatures). + // Group existing (current + custom) methods by name so we can quickly find candidates + // that share the previous method's name. Reuses the dictionary built once in + // BuildMethodsForBackCompatibility to avoid recomputing signatures. var currentMethodsByName = new Dictionary>(StringComparer.Ordinal); - foreach (var method in methods) + foreach (var method in currentMethodSignatures.Values) { if (!currentMethodsByName.TryGetValue(method.Signature.Name, out var list)) { @@ -1797,6 +1784,7 @@ private void AddBackCompatOverloadsForNewOptionalParameters(List list.Add(method); } + var addedSignatures = new HashSet(MethodSignature.MethodSignatureComparer); var newMethods = new List(); foreach (var previousMethod in LastContractView.Methods) @@ -1811,7 +1799,7 @@ private void AddBackCompatOverloadsForNewOptionalParameters(List } // Skip if the current contract already contains an exact-matching signature. - if (existingSignatures.Contains(previousSignature)) + if (currentMethodSignatures.ContainsKey(previousSignature)) { continue; } @@ -1842,7 +1830,7 @@ private void AddBackCompatOverloadsForNewOptionalParameters(List continue; } - if (existingSignatures.Add(overload.Signature)) + if (!currentMethodSignatures.ContainsKey(overload.Signature) && addedSignatures.Add(overload.Signature)) { newMethods.Add(overload); CodeModelGenerator.Instance.Emitter.Debug( From 779d6630d1a94e3006826ed65d018933e8ad560d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:56:50 +0000 Subject: [PATCH 04/13] Rename docs section to "Client Methods" and fix example with body param Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/eaa4071b-877d-4406-a1ca-63af4e8c07c5 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../generator/docs/backward-compatibility.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index 6997b65a842..ce91b36377e 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -787,7 +787,7 @@ public virtual ClientResult UpdateSkillDefaultVersion(string skillId, string con } ``` -### Service Method Evolution +### Client Methods #### Scenario: New Optional Non-Body Parameter Added to a Service Method @@ -807,29 +807,29 @@ This follows the guidance in [Service-Driven Evolution: A method gets a new opti Previous version of the client: ```csharp -public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken = default); -public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken = default); +public virtual ClientResult GetData(int p1, BinaryContent body, RequestOptions options = null); +public virtual Task GetDataAsync(int p1, BinaryContent body, RequestOptions options = null); ``` -Current TypeSpec adds an optional header parameter `param3`: +Current TypeSpec adds an optional query parameter `p2`: ```typespec -op getData(@query param2: int32, @body param1: string, @header param3?: boolean): string; +op getData(@query p1: int32, @body body: SampleModel, @query p2?: boolean): string; ``` **Generated Compatibility Methods:** ```csharp [EditorBrowsable(EditorBrowsableState.Never)] -public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken) +public virtual ClientResult GetData(int p1, BinaryContent body, RequestOptions options) { - return this.GetData(param2, param1, default, cancellationToken); + return this.GetData(p1, body, default, options); } [EditorBrowsable(EditorBrowsableState.Never)] -public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken) +public virtual Task GetDataAsync(int p1, BinaryContent body, RequestOptions options) { - return this.GetDataAsync(param2, param1, default, cancellationToken); + return this.GetDataAsync(p1, body, default, options); } ``` From eb3aca66ba457e1d36e1b9ee2d7760321893b0af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 17:31:31 +0000 Subject: [PATCH 05/13] Update TOC anchor to match renamed Client Methods section Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/fdf69480-deaf-4b00-8d8a-e88ad22e8313 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../http-client-csharp/generator/docs/backward-compatibility.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index ce91b36377e..33c5cf2a591 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -21,7 +21,7 @@ - [Method Parameter Name Preserved from Last Contract](#scenario-method-parameter-name-preserved-from-last-contract) - [Content-Type Parameter Ordering](#content-type-parameter-ordering) - [Content-Type Before Body Preserved from Last Contract](#scenario-content-type-before-body-preserved-from-last-contract) - - [Service Method Evolution](#service-method-evolution) + - [Client Methods](#client-methods) - [New Optional Non-Body Parameter Added to a Service Method](#scenario-new-optional-non-body-parameter-added-to-a-service-method) ## Overview From 12265c0efd0aad8fc65694e82402361f3e524be2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 18:01:02 +0000 Subject: [PATCH 06/13] Show full generated client with inline comments in docs example Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/df3eba50-228a-4ff8-88e1-756b943c65dc Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../generator/docs/backward-compatibility.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index 33c5cf2a591..edf735029e4 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -817,15 +817,32 @@ Current TypeSpec adds an optional query parameter `p2`: op getData(@query p1: int32, @body body: SampleModel, @query p2?: boolean): string; ``` -**Generated Compatibility Methods:** +**Generated Client:** ```csharp +// Current method generated from the updated TypeSpec — includes the new optional `p2` parameter. +public virtual ClientResult GetData(int p1, BinaryContent body, bool? p2 = null, RequestOptions options = null) +{ + // ... implementation ... +} + +// Back-compat overload matching the previous contract's signature. Hidden from IntelliSense and +// delegates to the current method, passing `default` for the new `p2` parameter. Default values +// are stripped from this signature to avoid ambiguous call sites with the current method above. [EditorBrowsable(EditorBrowsableState.Never)] public virtual ClientResult GetData(int p1, BinaryContent body, RequestOptions options) { return this.GetData(p1, body, default, options); } +// Current async method generated from the updated TypeSpec — includes the new optional `p2` parameter. +public virtual Task GetDataAsync(int p1, BinaryContent body, bool? p2 = null, RequestOptions options = null) +{ + // ... implementation ... +} + +// Back-compat async overload matching the previous contract's signature. Hidden from IntelliSense +// and delegates to the current async method without `await` so this method itself is non-async. [EditorBrowsable(EditorBrowsableState.Never)] public virtual Task GetDataAsync(int p1, BinaryContent body, RequestOptions options) { From a9a100832885d65779e784d0ef5f4e80d9bfeaed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 20:27:44 +0000 Subject: [PATCH 07/13] Address review: refactor ClientProvider, rename enum, fix test param naming Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/1d3d7484-11d8-4289-866b-9b8fa8f1d4a9 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 88 +++++++++++-------- .../ClientProviders/ClientProviderTests.cs | 26 +++--- .../TestClient.cs | 4 +- .../TestClient.cs | 4 +- .../BackCompatibilityChangeCategory.cs | 2 +- .../src/EmitterRpc/Emitter.cs | 2 +- .../generator/docs/backward-compatibility.md | 25 +++--- 7 files changed, 81 insertions(+), 70 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index 8caa5de4c55..2f51dbf3d24 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1256,23 +1256,22 @@ protected override ScmMethodProvider[] BuildMethods() protected sealed override IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) { + List materializedMethods = [.. originalMethods]; + if (LastContractView?.Methods == null || LastContractView.Methods.Count == 0) { - return [.. originalMethods]; + return materializedMethods; } - var materializedMethods = originalMethods as IList ?? [.. originalMethods]; var currentMethodSignatures = BuildCurrentMethodSignatures(materializedMethods); - ApplyParameterReorderingForBackCompat(materializedMethods, currentMethodSignatures); + ProcessBackCompatForParameterReordering(materializedMethods, currentMethodSignatures); + ProcessBackCompatForNewOptionalParameters(materializedMethods, currentMethodSignatures); - // Add back-compat overloads for methods that have gained one or more new optional non-body parameter(s). - var resultMethods = new List(materializedMethods); - AddBackCompatOverloadsForNewOptionalParameters(resultMethods, currentMethodSignatures); - return resultMethods; + return materializedMethods; } - private void ApplyParameterReorderingForBackCompat( + private void ProcessBackCompatForParameterReordering( IList materializedMethods, Dictionary currentMethodSignatures) { @@ -1761,21 +1760,22 @@ private static void UpdateXmlDocProviderForParamReorder( } } - private void AddBackCompatOverloadsForNewOptionalParameters( + private void ProcessBackCompatForNewOptionalParameters( List methods, Dictionary currentMethodSignatures) { - if (LastContractView?.Methods == null || LastContractView.Methods.Count == 0) - { - return; - } - // Group existing (current + custom) methods by name so we can quickly find candidates // that share the previous method's name. Reuses the dictionary built once in // BuildMethodsForBackCompatibility to avoid recomputing signatures. var currentMethodsByName = new Dictionary>(StringComparer.Ordinal); foreach (var method in currentMethodSignatures.Values) { + // Skip CreateRequest methods: they are internal helpers and never need a back-compat overload. + if (method is ScmMethodProvider { Kind: ScmMethodKind.CreateRequest }) + { + continue; + } + if (!currentMethodsByName.TryGetValue(method.Signature.Name, out var list)) { list = []; @@ -1787,7 +1787,7 @@ private void AddBackCompatOverloadsForNewOptionalParameters( var addedSignatures = new HashSet(MethodSignature.MethodSignatureComparer); var newMethods = new List(); - foreach (var previousMethod in LastContractView.Methods) + foreach (var previousMethod in LastContractView!.Methods) { var previousSignature = previousMethod.Signature; @@ -1819,12 +1819,18 @@ private void AddBackCompatOverloadsForNewOptionalParameters( } } - if (matchedCurrent == null) + if (matchedCurrent is not ScmMethodProvider matchedScmMethod) { continue; } - var overload = BuildBackCompatOverloadForNewOptionalParameters(previousMethod, matchedCurrent); + // Only generate back-compat overloads for Convenience or Protocol methods. + if (matchedScmMethod.Kind != ScmMethodKind.Convenience && matchedScmMethod.Kind != ScmMethodKind.Protocol) + { + continue; + } + + var overload = BuildBackCompatOverloadForNewOptionalParameters(previousMethod, matchedScmMethod); if (overload == null) { continue; @@ -1835,18 +1841,15 @@ private void AddBackCompatOverloadsForNewOptionalParameters( newMethods.Add(overload); CodeModelGenerator.Instance.Emitter.Debug( $"Added back-compat overload for '{Name}.{previousSignature.Name}' to handle new optional parameter(s) introduced relative to the last contract.", - BackCompatibilityChangeCategory.MethodNewOptionalParameterOverloadAdded); + BackCompatibilityChangeCategory.SvcMethodNewOptionalParameterOverloadAdded); } } methods.AddRange(newMethods); } - /// - /// Returns true when contains all parameters of - /// in the same order, and the additional parameters are - /// all optional and none of them are body parameters. - /// + // Returns true when currentSignature contains all parameters of previousSignature in the same + // relative order, every "extra" parameter is optional, and none of the extras are body parameters. private static bool HasNewOptionalNonBodyParametersOnly( MethodSignature previousSignature, MethodSignature currentSignature) @@ -1871,6 +1874,10 @@ private static bool HasNewOptionalNonBodyParametersOnly( if (previousIndex < previousSignature.Parameters.Count) { var previousParam = previousSignature.Parameters[previousIndex]; + // Use AreNamesEqual (rather than the default equality) because the previous + // parameter type was loaded from the last contract assembly and may carry + // different metadata (e.g. nullability annotations) than the freshly generated + // current parameter type even when the two represent the same logical type. if (currentParam.Name.ToVariableName() == previousParam.Name.ToVariableName() && currentParam.Type.AreNamesEqual(previousParam.Type)) { @@ -1879,7 +1886,9 @@ private static bool HasNewOptionalNonBodyParametersOnly( } } - // current parameter is "new" relative to the previous contract + // currentParam is "new" relative to the previous contract. Adding a back-compat overload + // requires us to supply a value when delegating to the current method, so the parameter + // must already provide a default value we can pass through. if (currentParam.DefaultValue is null) { return false; @@ -1895,6 +1904,9 @@ private static bool HasNewOptionalNonBodyParametersOnly( return previousIndex == previousSignature.Parameters.Count; } + // Returns true when the return types are considered equal across the previous and current + // contracts. Compares by name (ignoring nullability) because the previous type was loaded + // from the last contract assembly and may carry different metadata than the current one. private static bool ReturnTypesMatch(CSharpType? a, CSharpType? b) { if (ReferenceEquals(a, b)) @@ -1908,17 +1920,17 @@ private static bool ReturnTypesMatch(CSharpType? a, CSharpType? b) return a.AreNamesEqual(b); } - private MethodProvider? BuildBackCompatOverloadForNewOptionalParameters( + private ScmMethodProvider? BuildBackCompatOverloadForNewOptionalParameters( MethodProvider previousMethod, - MethodProvider currentMethod) + ScmMethodProvider currentMethod) { var previousSignature = previousMethod.Signature; var currentSignature = currentMethod.Signature; - var previousParamsByName = new Dictionary(StringComparer.Ordinal); + var previousParamsByName = new Dictionary(); foreach (var p in previousSignature.Parameters) { - previousParamsByName[p.Name.ToVariableName()] = p; + previousParamsByName.TryAdd(p.Name.ToVariableName(), p); } var arguments = new List(currentSignature.Parameters.Count); @@ -1934,11 +1946,10 @@ private static bool ReturnTypesMatch(CSharpType? a, CSharpType? b) } } - // Always invoke without async/await: the back-compat overload simply delegates to the current - // method. For async methods, returning the Task directly is sufficient and keeps the back-compat - // overload itself non-async (its signature was loaded from the previous DLL contract and does - // not carry the async modifier). Constructing InvokeMethodExpression directly leaves - // CallAsAsync / AddConfigureAwaitFalse at their default false value. + // Delegate to the current method without async/await: returning the Task directly is + // sufficient and avoids state-machine overhead. We construct InvokeMethodExpression + // directly so CallAsAsync stays false even when the current async method's signature + // carries the Async modifier. var invocation = new InvokeMethodExpression(This, currentSignature, arguments); MethodBodyStatement body = previousSignature.ReturnType is null @@ -1946,14 +1957,13 @@ private static bool ReturnTypesMatch(CSharpType? a, CSharpType? b) : Return(invocation); var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature(previousSignature, hideMethod: true); - var kind = (currentMethod as ScmMethodProvider)?.Kind ?? ScmMethodKind.Convenience; return new ScmMethodProvider( - backCompatSignature, - body, - this, - kind, + signature: backCompatSignature, + bodyStatements: body, + enclosingType: this, + methodKind: currentMethod.Kind, xmlDocProvider: previousMethod.XmlDocs, - serviceMethod: (currentMethod as ScmMethodProvider)?.ServiceMethod); + serviceMethod: currentMethod.ServiceMethod); } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs index 5465ff6c5d7..342423b05e5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs @@ -3149,15 +3149,15 @@ public async Task BackCompatibility_DuplicateMethodSignatureDoesNotThrow() Assert.DoesNotThrow(() => processMethod?.Invoke(clientProvider, null)); } - // Last contract has GetData(string param1, int param2, CancellationToken) (and async). + // Last contract has GetData(int param1, string param2, CancellationToken) (and async). // The current TypeSpec adds a new optional non-body (header) parameter `param3`. // Expected: a hidden back-compat overload matching the previous signature is added that // delegates to the new method, passing default for the new parameter. [Test] public async Task BackCompatibility_NewOptionalNonBodyParameterAdded() { - var param1 = InputFactory.BodyParameter("param1", InputPrimitiveType.String, isRequired: true); - var param2 = InputFactory.QueryParameter("param2", InputPrimitiveType.Int32, isRequired: true); + var param1 = InputFactory.QueryParameter("param1", InputPrimitiveType.Int32, isRequired: true); + var param2 = InputFactory.BodyParameter("param2", InputPrimitiveType.String, isRequired: true); var param3 = InputFactory.HeaderParameter("param3", InputPrimitiveType.Boolean, isRequired: false); var operation = InputFactory.Operation( @@ -3167,8 +3167,8 @@ public async Task BackCompatibility_NewOptionalNonBodyParameterAdded() List methodParameters = [ - InputFactory.MethodParameter("param1", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: true), - InputFactory.MethodParameter("param2", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("param1", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("param2", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: true), InputFactory.MethodParameter("param3", InputPrimitiveType.Boolean, location: InputRequestLocation.Header, isRequired: false), ]; @@ -3216,11 +3216,11 @@ public async Task BackCompatibility_NewOptionalNonBodyParameterAdded() // Validate the back-compat sync body delegates to the current sync method with default for param3. var syncBody = backCompatSync!.BodyStatements!.ToDisplayString(); - Assert.AreEqual("return this.GetData(param2, param1, default, cancellationToken);\n", syncBody); + Assert.AreEqual("return this.GetData(param1, param2, default, cancellationToken);\n", syncBody); // Validate the back-compat async body delegates to the current async method (no await; returns Task directly). var asyncBody = backCompatAsync!.BodyStatements!.ToDisplayString(); - Assert.AreEqual("return this.GetDataAsync(param2, param1, default, cancellationToken);\n", asyncBody); + Assert.AreEqual("return this.GetDataAsync(param1, param2, default, cancellationToken);\n", asyncBody); } // The current TypeSpec adds two new optional non-body parameters relative to the last contract. @@ -3229,8 +3229,8 @@ public async Task BackCompatibility_NewOptionalNonBodyParameterAdded() [Test] public async Task BackCompatibility_MultipleNewOptionalNonBodyParametersAdded() { - var param1 = InputFactory.BodyParameter("param1", InputPrimitiveType.String, isRequired: true); - var param2 = InputFactory.QueryParameter("param2", InputPrimitiveType.Int32, isRequired: true); + var param1 = InputFactory.QueryParameter("param1", InputPrimitiveType.Int32, isRequired: true); + var param2 = InputFactory.BodyParameter("param2", InputPrimitiveType.String, isRequired: true); var param3 = InputFactory.HeaderParameter("param3", InputPrimitiveType.Boolean, isRequired: false); var param4 = InputFactory.QueryParameter("param4", InputPrimitiveType.String, isRequired: false); @@ -3241,8 +3241,8 @@ public async Task BackCompatibility_MultipleNewOptionalNonBodyParametersAdded() List methodParameters = [ - InputFactory.MethodParameter("param1", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: true), - InputFactory.MethodParameter("param2", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("param1", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("param2", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: true), InputFactory.MethodParameter("param3", InputPrimitiveType.Boolean, location: InputRequestLocation.Header, isRequired: false), InputFactory.MethodParameter("param4", InputPrimitiveType.String, location: InputRequestLocation.Query, isRequired: false), ]; @@ -3279,10 +3279,10 @@ public async Task BackCompatibility_MultipleNewOptionalNonBodyParametersAdded() // The back-compat overloads should pass `default` for both new parameters. Assert.AreEqual( - "return this.GetData(param2, param1, default, default, cancellationToken);\n", + "return this.GetData(param1, param2, default, default, cancellationToken);\n", backCompatSync!.BodyStatements!.ToDisplayString()); Assert.AreEqual( - "return this.GetDataAsync(param2, param1, default, default, cancellationToken);\n", + "return this.GetDataAsync(param1, param2, default, default, cancellationToken);\n", backCompatAsync!.BodyStatements!.ToDisplayString()); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded/TestClient.cs index 18ff51338f3..5bca61e2cea 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded/TestClient.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded/TestClient.cs @@ -8,12 +8,12 @@ namespace Sample { public partial class TestClient { - public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken = default) + public virtual ClientResult GetData(int param1, string param2, CancellationToken cancellationToken = default) { throw new NotImplementedException(); } - public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken = default) + public virtual Task> GetDataAsync(int param1, string param2, CancellationToken cancellationToken = default) { throw new NotImplementedException(); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded/TestClient.cs index 18ff51338f3..5bca61e2cea 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded/TestClient.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded/TestClient.cs @@ -8,12 +8,12 @@ namespace Sample { public partial class TestClient { - public virtual ClientResult GetData(int param2, string param1, CancellationToken cancellationToken = default) + public virtual ClientResult GetData(int param1, string param2, CancellationToken cancellationToken = default) { throw new NotImplementedException(); } - public virtual Task> GetDataAsync(int param2, string param1, CancellationToken cancellationToken = default) + public virtual Task> GetDataAsync(int param1, string param2, CancellationToken cancellationToken = default) { throw new NotImplementedException(); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs index 369662795a2..5b635fe8b9a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs @@ -41,6 +41,6 @@ public enum BackCompatibilityChangeCategory ModelFactoryMethodSkipped, /// A back-compat overload of a client method was added because new optional non-body parameter(s) were introduced relative to the last contract. - MethodNewOptionalParameterOverloadAdded, + SvcMethodNewOptionalParameterOverloadAdded, } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs index 8cfca5f86eb..9b83da66b33 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs @@ -180,7 +180,7 @@ public void WriteBufferedMessages() BackCompatibilityChangeCategory.ModelFactoryMethodReplaced => "Model Factory Method Replaced For Back-Compat", BackCompatibilityChangeCategory.ModelFactoryMethodAdded => "Model Factory Method Added For Back-Compat", BackCompatibilityChangeCategory.ModelFactoryMethodSkipped => "Model Factory Method Back-Compat Skipped", - BackCompatibilityChangeCategory.MethodNewOptionalParameterOverloadAdded => "Method Back-Compat Overload Added For New Optional Parameter", + BackCompatibilityChangeCategory.SvcMethodNewOptionalParameterOverloadAdded => "Method Back-Compat Overload Added For New Optional Parameter", _ => category.ToString(), }; diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index edf735029e4..2f4b4c0ce04 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -819,30 +819,29 @@ op getData(@query p1: int32, @body body: SampleModel, @query p2?: boolean): stri **Generated Client:** +The generated client includes the current methods (with the new optional `p2` parameter) and the hidden back-compat overloads that match the previous contract's signature. Required parameters come first, followed by optional parameters, with `RequestOptions` last — matching the [Azure SDK parameter ordering guidelines](https://azure.github.io/azure-sdk/dotnet_implementation.html#parameter-presence-and-ordering). + ```csharp -// Current method generated from the updated TypeSpec — includes the new optional `p2` parameter. +// Current sync method generated from the updated TypeSpec. public virtual ClientResult GetData(int p1, BinaryContent body, bool? p2 = null, RequestOptions options = null) { // ... implementation ... } -// Back-compat overload matching the previous contract's signature. Hidden from IntelliSense and -// delegates to the current method, passing `default` for the new `p2` parameter. Default values -// are stripped from this signature to avoid ambiguous call sites with the current method above. -[EditorBrowsable(EditorBrowsableState.Never)] -public virtual ClientResult GetData(int p1, BinaryContent body, RequestOptions options) +// Current async method generated from the updated TypeSpec. +public virtual Task GetDataAsync(int p1, BinaryContent body, bool? p2 = null, RequestOptions options = null) { - return this.GetData(p1, body, default, options); + // ... implementation ... } -// Current async method generated from the updated TypeSpec — includes the new optional `p2` parameter. -public virtual Task GetDataAsync(int p1, BinaryContent body, bool? p2 = null, RequestOptions options = null) +// Back-compat sync overload matching the previous contract's signature. +[EditorBrowsable(EditorBrowsableState.Never)] +public virtual ClientResult GetData(int p1, BinaryContent body, RequestOptions options) { - // ... implementation ... + return this.GetData(p1, body, default, options); } -// Back-compat async overload matching the previous contract's signature. Hidden from IntelliSense -// and delegates to the current async method without `await` so this method itself is non-async. +// Back-compat async overload matching the previous contract's signature. [EditorBrowsable(EditorBrowsableState.Never)] public virtual Task GetDataAsync(int p1, BinaryContent body, RequestOptions options) { @@ -850,4 +849,6 @@ public virtual Task GetDataAsync(int p1, BinaryContent body, Reque } ``` +The back-compat overloads are hidden from IntelliSense via `[EditorBrowsable(EditorBrowsableState.Never)]`, have all default values stripped to avoid ambiguous call sites with the current methods, and delegate to the current method passing `default` for each new parameter. Async overloads return the `Task` directly without `await` so the back-compat method itself remains non-async, avoiding unnecessary state-machine overhead for a simple delegation. + The same logic applies when more than one new optional non-body parameter is added — the back-compat overload simply passes `default` for each new parameter when delegating to the current method. From 502a6ddfbbd41c5b048b4a18d3dbfb2b4debcebb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 20:34:46 +0000 Subject: [PATCH 08/13] Use TestData for back-compat tests; add model body test Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/1d3d7484-11d8-4289-866b-9b8fa8f1d4a9 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ClientProviders/ClientProviderTests.cs | 126 ++++++++---------- ...ltipleNewOptionalNonBodyParametersAdded.cs | 95 +++++++++++++ ...dyParameterDoesNotAddBackCompatOverload.cs | 74 ++++++++++ ...bility_NewOptionalNonBodyParameterAdded.cs | 95 +++++++++++++ ...ionalNonBodyParameterAddedWithModelBody.cs | 94 +++++++++++++ .../TestClient.cs | 21 +++ ...edParameterDoesNotAddBackCompatOverload.cs | 82 ++++++++++++ 7 files changed, 516 insertions(+), 71 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody/TestClient.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs index 342423b05e5..0c07bb51d50 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs @@ -3186,41 +3186,9 @@ public async Task BackCompatibility_NewOptionalNonBodyParameterAdded() var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var allGetDataConvenienceMethods = clientProvider!.Methods - .Where(m => m.Signature.Name.StartsWith("GetData") && m is ScmMethodProvider { Kind: ScmMethodKind.Convenience }) - .ToList(); - - // Expect 4 convenience methods: current sync/async (with param3) + back-compat sync/async (without param3) - Assert.AreEqual(4, allGetDataConvenienceMethods.Count); - - var backCompatSync = allGetDataConvenienceMethods - .FirstOrDefault(m => m.Signature.Name == "GetData" && m.Signature.Parameters.All(p => p.Name != "param3")); - var backCompatAsync = allGetDataConvenienceMethods - .FirstOrDefault(m => m.Signature.Name == "GetDataAsync" && m.Signature.Parameters.All(p => p.Name != "param3")); - Assert.IsNotNull(backCompatSync, "Expected a back-compat sync overload without param3."); - Assert.IsNotNull(backCompatAsync, "Expected a back-compat async overload without param3."); - - // The back-compat overloads should be marked with [EditorBrowsable(EditorBrowsableState.Never)] - // and should not have default values on the parameters. - foreach (var backCompat in new[] { backCompatSync!, backCompatAsync! }) - { - Assert.AreEqual(1, backCompat.Signature.Attributes.Count); - Assert.AreEqual( - "[global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)]\n", - backCompat.Signature.Attributes[0].ToDisplayString()); - foreach (var p in backCompat.Signature.Parameters) - { - Assert.IsNull(p.DefaultValue); - } - } - - // Validate the back-compat sync body delegates to the current sync method with default for param3. - var syncBody = backCompatSync!.BodyStatements!.ToDisplayString(); - Assert.AreEqual("return this.GetData(param1, param2, default, cancellationToken);\n", syncBody); - - // Validate the back-compat async body delegates to the current async method (no await; returns Task directly). - var asyncBody = backCompatAsync!.BodyStatements!.ToDisplayString(); - Assert.AreEqual("return this.GetDataAsync(param1, param2, default, cancellationToken);\n", asyncBody); + var writer = new TypeProviderWriter(clientProvider!); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } // The current TypeSpec adds two new optional non-body parameters relative to the last contract. @@ -3261,29 +3229,55 @@ public async Task BackCompatibility_MultipleNewOptionalNonBodyParametersAdded() var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var convenienceMethods = clientProvider!.Methods - .Where(m => m.Signature.Name.StartsWith("GetData") && m is ScmMethodProvider { Kind: ScmMethodKind.Convenience }) - .ToList(); + var writer = new TypeProviderWriter(clientProvider!); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } - Assert.AreEqual(4, convenienceMethods.Count); + // Last contract has GetData(int param1, ModelType body, CancellationToken) (and async). + // The current TypeSpec adds a new optional non-body (header) parameter `param3`. + // Expected: a hidden back-compat overload matching the previous signature is added even when + // the method has a model-typed request body. + [Test] + public async Task BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody() + { + var bodyModel = InputFactory.Model( + "SampleModel", + properties: [InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)]); - // Each back-compat overload should have only the previous-contract parameters. - var backCompatSync = convenienceMethods - .FirstOrDefault(m => m.Signature.Name == "GetData" - && m.Signature.Parameters.All(p => p.Name != "param3" && p.Name != "param4")); - var backCompatAsync = convenienceMethods - .FirstOrDefault(m => m.Signature.Name == "GetDataAsync" - && m.Signature.Parameters.All(p => p.Name != "param3" && p.Name != "param4")); - Assert.IsNotNull(backCompatSync); - Assert.IsNotNull(backCompatAsync); + var param1 = InputFactory.QueryParameter("param1", InputPrimitiveType.Int32, isRequired: true); + var param2 = InputFactory.BodyParameter("body", bodyModel, isRequired: true); + var param3 = InputFactory.HeaderParameter("param3", InputPrimitiveType.Boolean, isRequired: false); - // The back-compat overloads should pass `default` for both new parameters. - Assert.AreEqual( - "return this.GetData(param1, param2, default, default, cancellationToken);\n", - backCompatSync!.BodyStatements!.ToDisplayString()); - Assert.AreEqual( - "return this.GetDataAsync(param1, param2, default, default, cancellationToken);\n", - backCompatAsync!.BodyStatements!.ToDisplayString()); + var operation = InputFactory.Operation( + "GetData", + parameters: [param1, param2, param3], + responses: [InputFactory.OperationResponse([200], bodytype: InputPrimitiveType.String)]); + + List methodParameters = + [ + InputFactory.MethodParameter("param1", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("body", bodyModel, location: InputRequestLocation.Body, isRequired: true), + InputFactory.MethodParameter("param3", InputPrimitiveType.Boolean, location: InputRequestLocation.Header, isRequired: false), + ]; + + var method = InputFactory.BasicServiceMethod("GetData", operation, parameters: [.. methodParameters]); + var client = InputFactory.Client(TestClientName, methods: [method]); + + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType().FirstOrDefault(); + Assert.IsNotNull(clientProvider); + Assert.IsNotNull(clientProvider!.LastContractView); + + var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + processMethod?.Invoke(clientProvider, null); + + var writer = new TypeProviderWriter(clientProvider!); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } // When the new parameter is a body parameter, the back-compat overload should NOT be added. @@ -3321,16 +3315,9 @@ public async Task BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompat var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var convenienceMethods = clientProvider!.Methods - .Where(m => m.Signature.Name.StartsWith("GetData") && m is ScmMethodProvider { Kind: ScmMethodKind.Convenience }) - .ToList(); - - // Only the current sync/async pair: no back-compat overload because the new parameter is a body parameter. - Assert.AreEqual(2, convenienceMethods.Count); - foreach (var m in convenienceMethods) - { - Assert.IsTrue(m.Signature.Parameters.Any(p => p.Name == "param1")); - } + var writer = new TypeProviderWriter(clientProvider!); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } // When the new parameter is required (not optional), the back-compat overload should NOT be added, @@ -3369,12 +3356,9 @@ public async Task BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOver var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var convenienceMethods = clientProvider!.Methods - .Where(m => m.Signature.Name.StartsWith("GetData") && m is ScmMethodProvider { Kind: ScmMethodKind.Convenience }) - .ToList(); - - // Only the current sync/async pair: no back-compat overload because the new parameter is required. - Assert.AreEqual(2, convenienceMethods.Count); + var writer = new TypeProviderWriter(clientProvider!); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs new file mode 100644 index 00000000000..249961418fe --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs @@ -0,0 +1,95 @@ +// + +#nullable disable + +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.ComponentModel; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + private readonly global::System.Uri _endpoint; + + protected TestClient() + { + } + + public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) + { + } + + internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) + { + global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); + + options ??= new global::Sample.TestClientOptions(); + + _endpoint = endpoint; + if ((authenticationPolicy != null)) + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); + } + else + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); + } + } + + public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) + { + } + + public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } + + public virtual global::System.ClientModel.ClientResult GetData(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, string param4 = default, global::System.ClientModel.Primitives.RequestOptions options = null) + { + global::Sample.Argument.AssertNotNull(content, nameof(content)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param1, content, param3, param4, options); + return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); + } + + public virtual async global::System.Threading.Tasks.Task GetDataAsync(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, string param4 = default, global::System.ClientModel.Primitives.RequestOptions options = null) + { + global::Sample.Argument.AssertNotNull(content, nameof(content)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param1, content, param3, param4, options); + return global::System.ClientModel.ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, options).ConfigureAwait(false)); + } + + public virtual global::System.ClientModel.ClientResult GetData(int param1, string param2, bool? param3 = default, string param4 = default, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNullOrEmpty(param2, nameof(param2)); + + using global::System.ClientModel.BinaryContent content = global::System.ClientModel.BinaryContent.Create(global::System.BinaryData.FromString(param2)); + global::System.ClientModel.ClientResult result = this.GetData(param1, content, param3, param4, cancellationToken.ToRequestOptions()); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + public virtual async global::System.Threading.Tasks.Task> GetDataAsync(int param1, string param2, bool? param3 = default, string param4 = default, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNullOrEmpty(param2, nameof(param2)); + + using global::System.ClientModel.BinaryContent content = global::System.ClientModel.BinaryContent.Create(global::System.BinaryData.FromString(param2)); + global::System.ClientModel.ClientResult result = await this.GetDataAsync(param1, content, param3, param4, cancellationToken.ToRequestOptions()).ConfigureAwait(false); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public virtual global::System.ClientModel.ClientResult GetData(int param1, string param2, global::System.Threading.CancellationToken cancellationToken) + { + return this.GetData(param1, param2, default, default, cancellationToken); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public virtual global::System.Threading.Tasks.Task> GetDataAsync(int param1, string param2, global::System.Threading.CancellationToken cancellationToken) + { + return this.GetDataAsync(param1, param2, default, default, cancellationToken); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload.cs new file mode 100644 index 00000000000..92dcfd0515e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload.cs @@ -0,0 +1,74 @@ +// + +#nullable disable + +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + private readonly global::System.Uri _endpoint; + + protected TestClient() + { + } + + public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) + { + } + + internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) + { + global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); + + options ??= new global::Sample.TestClientOptions(); + + _endpoint = endpoint; + if ((authenticationPolicy != null)) + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); + } + else + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); + } + } + + public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) + { + } + + public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } + + public virtual global::System.ClientModel.ClientResult GetData(int param2, global::System.ClientModel.BinaryContent content, global::System.ClientModel.Primitives.RequestOptions options = null) + { + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param2, content, options); + return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); + } + + public virtual async global::System.Threading.Tasks.Task GetDataAsync(int param2, global::System.ClientModel.BinaryContent content, global::System.ClientModel.Primitives.RequestOptions options = null) + { + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param2, content, options); + return global::System.ClientModel.ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, options).ConfigureAwait(false)); + } + + public virtual global::System.ClientModel.ClientResult GetData(int param2, string param1 = default, global::System.Threading.CancellationToken cancellationToken = default) + { + using global::System.ClientModel.BinaryContent content = global::System.ClientModel.BinaryContent.Create(global::System.BinaryData.FromString(param1)); + global::System.ClientModel.ClientResult result = this.GetData(param2, content, cancellationToken.ToRequestOptions()); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + public virtual async global::System.Threading.Tasks.Task> GetDataAsync(int param2, string param1 = default, global::System.Threading.CancellationToken cancellationToken = default) + { + using global::System.ClientModel.BinaryContent content = global::System.ClientModel.BinaryContent.Create(global::System.BinaryData.FromString(param1)); + global::System.ClientModel.ClientResult result = await this.GetDataAsync(param2, content, cancellationToken.ToRequestOptions()).ConfigureAwait(false); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs new file mode 100644 index 00000000000..d6eb9d364eb --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs @@ -0,0 +1,95 @@ +// + +#nullable disable + +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.ComponentModel; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + private readonly global::System.Uri _endpoint; + + protected TestClient() + { + } + + public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) + { + } + + internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) + { + global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); + + options ??= new global::Sample.TestClientOptions(); + + _endpoint = endpoint; + if ((authenticationPolicy != null)) + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); + } + else + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); + } + } + + public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) + { + } + + public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } + + public virtual global::System.ClientModel.ClientResult GetData(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, global::System.ClientModel.Primitives.RequestOptions options = null) + { + global::Sample.Argument.AssertNotNull(content, nameof(content)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param1, content, param3, options); + return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); + } + + public virtual async global::System.Threading.Tasks.Task GetDataAsync(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, global::System.ClientModel.Primitives.RequestOptions options = null) + { + global::Sample.Argument.AssertNotNull(content, nameof(content)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param1, content, param3, options); + return global::System.ClientModel.ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, options).ConfigureAwait(false)); + } + + public virtual global::System.ClientModel.ClientResult GetData(int param1, string param2, bool? param3 = default, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNullOrEmpty(param2, nameof(param2)); + + using global::System.ClientModel.BinaryContent content = global::System.ClientModel.BinaryContent.Create(global::System.BinaryData.FromString(param2)); + global::System.ClientModel.ClientResult result = this.GetData(param1, content, param3, cancellationToken.ToRequestOptions()); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + public virtual async global::System.Threading.Tasks.Task> GetDataAsync(int param1, string param2, bool? param3 = default, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNullOrEmpty(param2, nameof(param2)); + + using global::System.ClientModel.BinaryContent content = global::System.ClientModel.BinaryContent.Create(global::System.BinaryData.FromString(param2)); + global::System.ClientModel.ClientResult result = await this.GetDataAsync(param1, content, param3, cancellationToken.ToRequestOptions()).ConfigureAwait(false); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public virtual global::System.ClientModel.ClientResult GetData(int param1, string param2, global::System.Threading.CancellationToken cancellationToken) + { + return this.GetData(param1, param2, default, cancellationToken); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public virtual global::System.Threading.Tasks.Task> GetDataAsync(int param1, string param2, global::System.Threading.CancellationToken cancellationToken) + { + return this.GetDataAsync(param1, param2, default, cancellationToken); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs new file mode 100644 index 00000000000..37615aa19e7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs @@ -0,0 +1,94 @@ +// + +#nullable disable + +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.ComponentModel; +using System.Threading; +using System.Threading.Tasks; +using Sample.Models; + +namespace Sample +{ + public partial class TestClient + { + private readonly global::System.Uri _endpoint; + + protected TestClient() + { + } + + public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) + { + } + + internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) + { + global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); + + options ??= new global::Sample.TestClientOptions(); + + _endpoint = endpoint; + if ((authenticationPolicy != null)) + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); + } + else + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); + } + } + + public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) + { + } + + public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } + + public virtual global::System.ClientModel.ClientResult GetData(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, global::System.ClientModel.Primitives.RequestOptions options = null) + { + global::Sample.Argument.AssertNotNull(content, nameof(content)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param1, content, param3, options); + return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); + } + + public virtual async global::System.Threading.Tasks.Task GetDataAsync(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, global::System.ClientModel.Primitives.RequestOptions options = null) + { + global::Sample.Argument.AssertNotNull(content, nameof(content)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param1, content, param3, options); + return global::System.ClientModel.ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, options).ConfigureAwait(false)); + } + + public virtual global::System.ClientModel.ClientResult GetData(int param1, global::Sample.Models.SampleModel body, bool? param3 = default, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNull(body, nameof(body)); + + global::System.ClientModel.ClientResult result = this.GetData(param1, body, param3, cancellationToken.ToRequestOptions()); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + public virtual async global::System.Threading.Tasks.Task> GetDataAsync(int param1, global::Sample.Models.SampleModel body, bool? param3 = default, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNull(body, nameof(body)); + + global::System.ClientModel.ClientResult result = await this.GetDataAsync(param1, body, param3, cancellationToken.ToRequestOptions()).ConfigureAwait(false); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public virtual global::System.ClientModel.ClientResult GetData(int param1, global::Sample.Models.SampleModel body, global::System.Threading.CancellationToken cancellationToken) + { + return this.GetData(param1, body, default, cancellationToken); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public virtual global::System.Threading.Tasks.Task> GetDataAsync(int param1, global::Sample.Models.SampleModel body, global::System.Threading.CancellationToken cancellationToken) + { + return this.GetDataAsync(param1, body, default, cancellationToken); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody/TestClient.cs new file mode 100644 index 00000000000..d4332ecd679 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody/TestClient.cs @@ -0,0 +1,21 @@ +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + public virtual ClientResult GetData(int param1, global::Sample.Models.SampleModel body, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public virtual Task> GetDataAsync(int param1, global::Sample.Models.SampleModel body, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.cs new file mode 100644 index 00000000000..895fe8cccf5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.cs @@ -0,0 +1,82 @@ +// + +#nullable disable + +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + private readonly global::System.Uri _endpoint; + + protected TestClient() + { + } + + public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) + { + } + + internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) + { + global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); + + options ??= new global::Sample.TestClientOptions(); + + _endpoint = endpoint; + if ((authenticationPolicy != null)) + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); + } + else + { + Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); + } + } + + public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) + { + } + + public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } + + public virtual global::System.ClientModel.ClientResult GetData(int param2, bool param3, global::System.ClientModel.BinaryContent content, global::System.ClientModel.Primitives.RequestOptions options = null) + { + global::Sample.Argument.AssertNotNull(content, nameof(content)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param2, param3, content, options); + return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); + } + + public virtual async global::System.Threading.Tasks.Task GetDataAsync(int param2, bool param3, global::System.ClientModel.BinaryContent content, global::System.ClientModel.Primitives.RequestOptions options = null) + { + global::Sample.Argument.AssertNotNull(content, nameof(content)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param2, param3, content, options); + return global::System.ClientModel.ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, options).ConfigureAwait(false)); + } + + public virtual global::System.ClientModel.ClientResult GetData(int param2, bool param3, string param1, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNullOrEmpty(param1, nameof(param1)); + + using global::System.ClientModel.BinaryContent content = global::System.ClientModel.BinaryContent.Create(global::System.BinaryData.FromString(param1)); + global::System.ClientModel.ClientResult result = this.GetData(param2, param3, content, cancellationToken.ToRequestOptions()); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + public virtual async global::System.Threading.Tasks.Task> GetDataAsync(int param2, bool param3, string param1, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNullOrEmpty(param1, nameof(param1)); + + using global::System.ClientModel.BinaryContent content = global::System.ClientModel.BinaryContent.Create(global::System.BinaryData.FromString(param1)); + global::System.ClientModel.ClientResult result = await this.GetDataAsync(param2, param3, content, cancellationToken.ToRequestOptions()).ConfigureAwait(false); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + } +} From 82a37419b3422c46e7081a2b989d1add278ded9f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 20:44:10 +0000 Subject: [PATCH 09/13] Clarify why optional check matters: required new params are breaking Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/af254bc1-2d6e-4c6f-bd54-8eebe1cdba56 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index 2f51dbf3d24..dd73e73d6e8 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1886,9 +1886,11 @@ private static bool HasNewOptionalNonBodyParametersOnly( } } - // currentParam is "new" relative to the previous contract. Adding a back-compat overload - // requires us to supply a value when delegating to the current method, so the parameter - // must already provide a default value we can pass through. + // currentParam is "new" relative to the previous contract. Per the Service-Driven + // Evolution guidance, we only emit a back-compat overload when the added parameter is + // *optional* in the current contract. A parameter without a default value is required, + // and adding a required parameter is itself a breaking change we should surface rather + // than silently bridge with a back-compat overload. if (currentParam.DefaultValue is null) { return false; From 9a2fe89e1c3337e2952b7ef54c23ec5dffc6fcb8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 20:47:40 +0000 Subject: [PATCH 10/13] Use This.Invoke(string, args) snippet overload to delegate without async Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/dcc6b3ed-492d-4ad0-8452-da4e8ccfe12a Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index dd73e73d6e8..a358624f181 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1949,10 +1949,10 @@ private static bool ReturnTypesMatch(CSharpType? a, CSharpType? b) } // Delegate to the current method without async/await: returning the Task directly is - // sufficient and avoids state-machine overhead. We construct InvokeMethodExpression - // directly so CallAsAsync stays false even when the current async method's signature - // carries the Async modifier. - var invocation = new InvokeMethodExpression(This, currentSignature, arguments); + // sufficient and avoids state-machine overhead. We use the name-based Invoke overload so + // CallAsAsync stays false even when the current async method's signature carries the + // Async modifier (the MethodSignature-based overloads auto-set CallAsAsync). + var invocation = This.Invoke(currentSignature.Name, arguments); MethodBodyStatement body = previousSignature.ReturnType is null ? invocation.Terminate() From a1b4f973e5e2bfb27814c32aae696d9ee39839a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:35:46 +0000 Subject: [PATCH 11/13] Address PR feedback: simplify code, slim test fixtures, add path/header test, refine docs Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/1c242ed5-2826-4b96-be51-1161c90ff229 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 85 +++++-------------- .../ClientProviders/ClientProviderTests.cs | 56 ++++++++++-- ...ltipleNewOptionalNonBodyParametersAdded.cs | 33 ------- ...dyParameterDoesNotAddBackCompatOverload.cs | 33 ------- ...bility_NewOptionalNonBodyParameterAdded.cs | 33 ------- ...ionalNonBodyParameterAddedWithModelBody.cs | 34 -------- ...rameterAddedWithPathAndHeaderParameters.cs | 63 ++++++++++++++ .../TestClient.cs | 21 +++++ ...edParameterDoesNotAddBackCompatOverload.cs | 33 ------- .../generator/docs/backward-compatibility.md | 10 +-- 10 files changed, 158 insertions(+), 243 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters/TestClient.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index a358624f181..93e7c201630 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1764,13 +1764,9 @@ private void ProcessBackCompatForNewOptionalParameters( List methods, Dictionary currentMethodSignatures) { - // Group existing (current + custom) methods by name so we can quickly find candidates - // that share the previous method's name. Reuses the dictionary built once in - // BuildMethodsForBackCompatibility to avoid recomputing signatures. var currentMethodsByName = new Dictionary>(StringComparer.Ordinal); foreach (var method in currentMethodSignatures.Values) { - // Skip CreateRequest methods: they are internal helpers and never need a back-compat overload. if (method is ScmMethodProvider { Kind: ScmMethodKind.CreateRequest }) { continue; @@ -1784,68 +1780,49 @@ private void ProcessBackCompatForNewOptionalParameters( list.Add(method); } - var addedSignatures = new HashSet(MethodSignature.MethodSignatureComparer); - var newMethods = new List(); - foreach (var previousMethod in LastContractView!.Methods) { var previousSignature = previousMethod.Signature; - // Only process public/protected methods. if (!previousSignature.Modifiers.HasFlag(MethodSignatureModifiers.Public) && !previousSignature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)) { continue; } - // Skip if the current contract already contains an exact-matching signature. - if (currentMethodSignatures.ContainsKey(previousSignature)) - { - continue; - } - - if (!currentMethodsByName.TryGetValue(previousSignature.Name, out var candidates)) + if (currentMethodSignatures.ContainsKey(previousSignature) || + !currentMethodsByName.TryGetValue(previousSignature.Name, out var candidates)) { continue; } - MethodProvider? matchedCurrent = null; + ScmMethodProvider? matchedCurrent = null; foreach (var candidate in candidates) { - if (HasNewOptionalNonBodyParametersOnly(previousSignature, candidate.Signature)) + if (candidate is ScmMethodProvider { Kind: ScmMethodKind.Convenience or ScmMethodKind.Protocol } scmCandidate && + HasNewOptionalNonBodyParametersOnly(previousSignature, scmCandidate.Signature)) { - matchedCurrent = candidate; + matchedCurrent = scmCandidate; break; } } - if (matchedCurrent is not ScmMethodProvider matchedScmMethod) + if (matchedCurrent is null) { continue; } - // Only generate back-compat overloads for Convenience or Protocol methods. - if (matchedScmMethod.Kind != ScmMethodKind.Convenience && matchedScmMethod.Kind != ScmMethodKind.Protocol) + var overload = BuildBackCompatOverloadForNewOptionalParameters(previousMethod, matchedCurrent); + if (overload == null || !currentMethodSignatures.TryAdd(overload.Signature, overload)) { continue; } - var overload = BuildBackCompatOverloadForNewOptionalParameters(previousMethod, matchedScmMethod); - if (overload == null) - { - continue; - } - - if (!currentMethodSignatures.ContainsKey(overload.Signature) && addedSignatures.Add(overload.Signature)) - { - newMethods.Add(overload); - CodeModelGenerator.Instance.Emitter.Debug( - $"Added back-compat overload for '{Name}.{previousSignature.Name}' to handle new optional parameter(s) introduced relative to the last contract.", - BackCompatibilityChangeCategory.SvcMethodNewOptionalParameterOverloadAdded); - } + methods.Add(overload); + CodeModelGenerator.Instance.Emitter.Debug( + $"Added back-compat overload for '{Name}.{previousSignature.Name}' to handle new optional parameter(s) introduced relative to the last contract.", + BackCompatibilityChangeCategory.SvcMethodNewOptionalParameterOverloadAdded); } - - methods.AddRange(newMethods); } // Returns true when currentSignature contains all parameters of previousSignature in the same @@ -1859,7 +1836,9 @@ private static bool HasNewOptionalNonBodyParametersOnly( return false; } - if (!ReturnTypesMatch(previousSignature.ReturnType, currentSignature.ReturnType)) + if (previousSignature.ReturnType is null + ? currentSignature.ReturnType is not null + : !previousSignature.ReturnType.AreNamesEqual(currentSignature.ReturnType)) { return false; } @@ -1874,10 +1853,6 @@ private static bool HasNewOptionalNonBodyParametersOnly( if (previousIndex < previousSignature.Parameters.Count) { var previousParam = previousSignature.Parameters[previousIndex]; - // Use AreNamesEqual (rather than the default equality) because the previous - // parameter type was loaded from the last contract assembly and may carry - // different metadata (e.g. nullability annotations) than the freshly generated - // current parameter type even when the two represent the same logical type. if (currentParam.Name.ToVariableName() == previousParam.Name.ToVariableName() && currentParam.Type.AreNamesEqual(previousParam.Type)) { @@ -1886,11 +1861,9 @@ private static bool HasNewOptionalNonBodyParametersOnly( } } - // currentParam is "new" relative to the previous contract. Per the Service-Driven - // Evolution guidance, we only emit a back-compat overload when the added parameter is - // *optional* in the current contract. A parameter without a default value is required, - // and adding a required parameter is itself a breaking change we should surface rather - // than silently bridge with a back-compat overload. + // Per the Service-Driven Evolution guidance, we only emit a back-compat overload when + // the added parameter is *optional*. A parameter without a default is required, and + // adding a required parameter is itself a breaking change. if (currentParam.DefaultValue is null) { return false; @@ -1906,22 +1879,6 @@ private static bool HasNewOptionalNonBodyParametersOnly( return previousIndex == previousSignature.Parameters.Count; } - // Returns true when the return types are considered equal across the previous and current - // contracts. Compares by name (ignoring nullability) because the previous type was loaded - // from the last contract assembly and may carry different metadata than the current one. - private static bool ReturnTypesMatch(CSharpType? a, CSharpType? b) - { - if (ReferenceEquals(a, b)) - { - return true; - } - if (a is null || b is null) - { - return false; - } - return a.AreNamesEqual(b); - } - private ScmMethodProvider? BuildBackCompatOverloadForNewOptionalParameters( MethodProvider previousMethod, ScmMethodProvider currentMethod) @@ -1948,10 +1905,6 @@ private static bool ReturnTypesMatch(CSharpType? a, CSharpType? b) } } - // Delegate to the current method without async/await: returning the Task directly is - // sufficient and avoids state-machine overhead. We use the name-based Invoke overload so - // CallAsAsync stays false even when the current async method's signature carries the - // Async modifier (the MethodSignature-based overloads auto-set CallAsAsync). var invocation = This.Invoke(currentSignature.Name, arguments); MethodBodyStatement body = previousSignature.ReturnType is null diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs index 0c07bb51d50..48275b97be9 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs @@ -3186,7 +3186,7 @@ public async Task BackCompatibility_NewOptionalNonBodyParameterAdded() var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var writer = new TypeProviderWriter(clientProvider!); + var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(clientProvider!, name => name == "GetData" || name == "GetDataAsync")); var file = writer.Write(); Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } @@ -3229,7 +3229,7 @@ public async Task BackCompatibility_MultipleNewOptionalNonBodyParametersAdded() var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var writer = new TypeProviderWriter(clientProvider!); + var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(clientProvider!, name => name == "GetData" || name == "GetDataAsync")); var file = writer.Write(); Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } @@ -3275,7 +3275,7 @@ public async Task BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBod var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var writer = new TypeProviderWriter(clientProvider!); + var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(clientProvider!, name => name == "GetData" || name == "GetDataAsync")); var file = writer.Write(); Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } @@ -3315,7 +3315,7 @@ public async Task BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompat var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var writer = new TypeProviderWriter(clientProvider!); + var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(clientProvider!, name => name == "GetData" || name == "GetDataAsync")); var file = writer.Write(); Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } @@ -3356,7 +3356,53 @@ public async Task BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOver var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); processMethod?.Invoke(clientProvider, null); - var writer = new TypeProviderWriter(clientProvider!); + var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(clientProvider!, name => name == "GetData" || name == "GetDataAsync")); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + // Last contract has GetData(string itemId, int filter, CancellationToken) (and async) where + // itemId is a path parameter, filter is a required query, and a required header is present. + // The current TypeSpec adds a new optional query parameter `sort`. + // Expected: a hidden back-compat overload matching the previous signature is added even when + // the operation mixes path, query, and header parameters. + [Test] + public async Task BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters() + { + var itemId = InputFactory.PathParameter("itemId", InputPrimitiveType.String, isRequired: true); + var filter = InputFactory.QueryParameter("filter", InputPrimitiveType.Int32, isRequired: true); + var region = InputFactory.HeaderParameter("region", InputPrimitiveType.String, isRequired: true); + var sort = InputFactory.QueryParameter("sort", InputPrimitiveType.String, isRequired: false); + + var operation = InputFactory.Operation( + "GetData", + path: "/items/{itemId}", + parameters: [itemId, filter, region, sort], + responses: [InputFactory.OperationResponse([200], bodytype: InputPrimitiveType.String)]); + + List methodParameters = + [ + InputFactory.MethodParameter("itemId", InputPrimitiveType.String, location: InputRequestLocation.Path, isRequired: true), + InputFactory.MethodParameter("filter", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("region", InputPrimitiveType.String, location: InputRequestLocation.Header, isRequired: true), + InputFactory.MethodParameter("sort", InputPrimitiveType.String, location: InputRequestLocation.Query, isRequired: false), + ]; + + var method = InputFactory.BasicServiceMethod("GetData", operation, parameters: [.. methodParameters]); + var client = InputFactory.Client(TestClientName, methods: [method]); + + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType().FirstOrDefault(); + Assert.IsNotNull(clientProvider); + Assert.IsNotNull(clientProvider!.LastContractView); + + var processMethod = typeof(ClientProvider).GetMethod("ProcessTypeForBackCompatibility", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + processMethod?.Invoke(clientProvider, null); + + var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(clientProvider!, name => name == "GetData" || name == "GetDataAsync")); var file = writer.Write(); Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs index 249961418fe..3ea718fb2dd 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs @@ -13,39 +13,6 @@ namespace Sample { public partial class TestClient { - private readonly global::System.Uri _endpoint; - - protected TestClient() - { - } - - public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) - { - } - - internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) - { - global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); - - options ??= new global::Sample.TestClientOptions(); - - _endpoint = endpoint; - if ((authenticationPolicy != null)) - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); - } - else - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); - } - } - - public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) - { - } - - public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } - public virtual global::System.ClientModel.ClientResult GetData(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, string param4 = default, global::System.ClientModel.Primitives.RequestOptions options = null) { global::Sample.Argument.AssertNotNull(content, nameof(content)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload.cs index 92dcfd0515e..bc72ecb5894 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload.cs @@ -12,39 +12,6 @@ namespace Sample { public partial class TestClient { - private readonly global::System.Uri _endpoint; - - protected TestClient() - { - } - - public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) - { - } - - internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) - { - global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); - - options ??= new global::Sample.TestClientOptions(); - - _endpoint = endpoint; - if ((authenticationPolicy != null)) - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); - } - else - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); - } - } - - public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) - { - } - - public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } - public virtual global::System.ClientModel.ClientResult GetData(int param2, global::System.ClientModel.BinaryContent content, global::System.ClientModel.Primitives.RequestOptions options = null) { using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(param2, content, options); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs index d6eb9d364eb..d3453a0b8d3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs @@ -13,39 +13,6 @@ namespace Sample { public partial class TestClient { - private readonly global::System.Uri _endpoint; - - protected TestClient() - { - } - - public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) - { - } - - internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) - { - global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); - - options ??= new global::Sample.TestClientOptions(); - - _endpoint = endpoint; - if ((authenticationPolicy != null)) - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); - } - else - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); - } - } - - public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) - { - } - - public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } - public virtual global::System.ClientModel.ClientResult GetData(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, global::System.ClientModel.Primitives.RequestOptions options = null) { global::Sample.Argument.AssertNotNull(content, nameof(content)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs index 37615aa19e7..bcda9712c9a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs @@ -2,7 +2,6 @@ #nullable disable -using System; using System.ClientModel; using System.ClientModel.Primitives; using System.ComponentModel; @@ -14,39 +13,6 @@ namespace Sample { public partial class TestClient { - private readonly global::System.Uri _endpoint; - - protected TestClient() - { - } - - public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) - { - } - - internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) - { - global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); - - options ??= new global::Sample.TestClientOptions(); - - _endpoint = endpoint; - if ((authenticationPolicy != null)) - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); - } - else - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); - } - } - - public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) - { - } - - public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } - public virtual global::System.ClientModel.ClientResult GetData(int param1, global::System.ClientModel.BinaryContent content, bool? param3 = default, global::System.ClientModel.Primitives.RequestOptions options = null) { global::Sample.Argument.AssertNotNull(content, nameof(content)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters.cs new file mode 100644 index 00000000000..3d32328ae88 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters.cs @@ -0,0 +1,63 @@ +// + +#nullable disable + +using System.ClientModel; +using System.ClientModel.Primitives; +using System.ComponentModel; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + public virtual global::System.ClientModel.ClientResult GetData(string itemId, int filter, string region, string sort, global::System.ClientModel.Primitives.RequestOptions options) + { + global::Sample.Argument.AssertNotNullOrEmpty(itemId, nameof(itemId)); + global::Sample.Argument.AssertNotNullOrEmpty(region, nameof(region)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(itemId, filter, region, sort, options); + return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); + } + + public virtual async global::System.Threading.Tasks.Task GetDataAsync(string itemId, int filter, string region, string sort, global::System.ClientModel.Primitives.RequestOptions options) + { + global::Sample.Argument.AssertNotNullOrEmpty(itemId, nameof(itemId)); + global::Sample.Argument.AssertNotNullOrEmpty(region, nameof(region)); + + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(itemId, filter, region, sort, options); + return global::System.ClientModel.ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, options).ConfigureAwait(false)); + } + + public virtual global::System.ClientModel.ClientResult GetData(string itemId, int filter, string region, string sort = default, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNullOrEmpty(itemId, nameof(itemId)); + global::Sample.Argument.AssertNotNullOrEmpty(region, nameof(region)); + + global::System.ClientModel.ClientResult result = this.GetData(itemId, filter, region, sort, cancellationToken.ToRequestOptions()); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + public virtual async global::System.Threading.Tasks.Task> GetDataAsync(string itemId, int filter, string region, string sort = default, global::System.Threading.CancellationToken cancellationToken = default) + { + global::Sample.Argument.AssertNotNullOrEmpty(itemId, nameof(itemId)); + global::Sample.Argument.AssertNotNullOrEmpty(region, nameof(region)); + + global::System.ClientModel.ClientResult result = await this.GetDataAsync(itemId, filter, region, sort, cancellationToken.ToRequestOptions()).ConfigureAwait(false); + return global::System.ClientModel.ClientResult.FromValue(result.GetRawResponse().Content.ToObjectFromJson(), result.GetRawResponse()); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public virtual global::System.ClientModel.ClientResult GetData(string itemId, int filter, string region, global::System.Threading.CancellationToken cancellationToken) + { + return this.GetData(itemId, filter, region, default, cancellationToken); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public virtual global::System.Threading.Tasks.Task> GetDataAsync(string itemId, int filter, string region, global::System.Threading.CancellationToken cancellationToken) + { + return this.GetDataAsync(itemId, filter, region, default, cancellationToken); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters/TestClient.cs new file mode 100644 index 00000000000..461df2c8993 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters/TestClient.cs @@ -0,0 +1,21 @@ +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + public virtual ClientResult GetData(string itemId, int filter, string region, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public virtual Task> GetDataAsync(string itemId, int filter, string region, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.cs index 895fe8cccf5..df446a2081b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.cs @@ -12,39 +12,6 @@ namespace Sample { public partial class TestClient { - private readonly global::System.Uri _endpoint; - - protected TestClient() - { - } - - public TestClient(global::System.Uri endpoint) : this(endpoint, new global::Sample.TestClientOptions()) - { - } - - internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy authenticationPolicy, global::System.Uri endpoint, global::Sample.TestClientOptions options) - { - global::Sample.Argument.AssertNotNull(endpoint, nameof(endpoint)); - - options ??= new global::Sample.TestClientOptions(); - - _endpoint = endpoint; - if ((authenticationPolicy != null)) - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly), authenticationPolicy }, Array.Empty()); - } - else - { - Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty()); - } - } - - public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options) - { - } - - public global::System.ClientModel.Primitives.ClientPipeline Pipeline { get; } - public virtual global::System.ClientModel.ClientResult GetData(int param2, bool param3, global::System.ClientModel.BinaryContent content, global::System.ClientModel.Primitives.RequestOptions options = null) { global::Sample.Argument.AssertNotNull(content, nameof(content)); diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index 2f4b4c0ce04..e60b9e85ab1 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -823,13 +823,13 @@ The generated client includes the current methods (with the new optional `p2` pa ```csharp // Current sync method generated from the updated TypeSpec. -public virtual ClientResult GetData(int p1, BinaryContent body, bool? p2 = null, RequestOptions options = null) +public virtual ClientResult GetData(int p1, BinaryContent body, bool? p2 = default, RequestOptions options = null) { // ... implementation ... } // Current async method generated from the updated TypeSpec. -public virtual Task GetDataAsync(int p1, BinaryContent body, bool? p2 = null, RequestOptions options = null) +public virtual async Task GetDataAsync(int p1, BinaryContent body, bool? p2 = default, RequestOptions options = null) { // ... implementation ... } @@ -838,17 +838,15 @@ public virtual Task GetDataAsync(int p1, BinaryContent body, bool? [EditorBrowsable(EditorBrowsableState.Never)] public virtual ClientResult GetData(int p1, BinaryContent body, RequestOptions options) { - return this.GetData(p1, body, default, options); + return this.GetData(p1: p1, body: body, p2: default, options: options); } // Back-compat async overload matching the previous contract's signature. [EditorBrowsable(EditorBrowsableState.Never)] public virtual Task GetDataAsync(int p1, BinaryContent body, RequestOptions options) { - return this.GetDataAsync(p1, body, default, options); + return this.GetDataAsync(p1: p1, body: body, p2: default, options: options); } ``` The back-compat overloads are hidden from IntelliSense via `[EditorBrowsable(EditorBrowsableState.Never)]`, have all default values stripped to avoid ambiguous call sites with the current methods, and delegate to the current method passing `default` for each new parameter. Async overloads return the `Task` directly without `await` so the back-compat method itself remains non-async, avoiding unnecessary state-machine overhead for a simple delegation. - -The same logic applies when more than one new optional non-body parameter is added — the back-compat overload simply passes `default` for each new parameter when delegating to the current method. From 73705debf2e626d529a1b657596fdf055cd29ae5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:04:19 +0000 Subject: [PATCH 12/13] Use named arguments in back-compat overload body, simplify return, drop unused comparer, trim docs Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/47365c54-329e-421a-ba22-e8bcb81a5ec1 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 18 ++++++------------ ...ultipleNewOptionalNonBodyParametersAdded.cs | 4 ++-- ...ibility_NewOptionalNonBodyParameterAdded.cs | 4 ++-- ...tionalNonBodyParameterAddedWithModelBody.cs | 4 ++-- ...arameterAddedWithPathAndHeaderParameters.cs | 4 ++-- .../generator/docs/backward-compatibility.md | 6 ++---- 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index 93e7c201630..b46d25fa85f 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1764,7 +1764,7 @@ private void ProcessBackCompatForNewOptionalParameters( List methods, Dictionary currentMethodSignatures) { - var currentMethodsByName = new Dictionary>(StringComparer.Ordinal); + var currentMethodsByName = new Dictionary>(); foreach (var method in currentMethodSignatures.Values) { if (method is ScmMethodProvider { Kind: ScmMethodKind.CreateRequest }) @@ -1895,21 +1895,15 @@ private static bool HasNewOptionalNonBodyParametersOnly( var arguments = new List(currentSignature.Parameters.Count); foreach (var currentParam in currentSignature.Parameters) { - if (previousParamsByName.TryGetValue(currentParam.Name.ToVariableName(), out var prevParam)) - { - arguments.Add(prevParam); - } - else - { - arguments.Add(currentParam.DefaultValue ?? Default); - } + ValueExpression value = previousParamsByName.TryGetValue(currentParam.Name.ToVariableName(), out var prevParam) + ? prevParam + : (currentParam.DefaultValue ?? Default); + arguments.Add(PositionalReference(currentParam.Name, value)); } var invocation = This.Invoke(currentSignature.Name, arguments); - MethodBodyStatement body = previousSignature.ReturnType is null - ? invocation.Terminate() - : Return(invocation); + MethodBodyStatement body = Return(invocation); var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature(previousSignature, hideMethod: true); return new ScmMethodProvider( diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs index 3ea718fb2dd..f1caf7a33e4 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_MultipleNewOptionalNonBodyParametersAdded.cs @@ -50,13 +50,13 @@ public partial class TestClient [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] public virtual global::System.ClientModel.ClientResult GetData(int param1, string param2, global::System.Threading.CancellationToken cancellationToken) { - return this.GetData(param1, param2, default, default, cancellationToken); + return this.GetData(param1: param1, param2: param2, param3: default, param4: default, cancellationToken: cancellationToken); } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] public virtual global::System.Threading.Tasks.Task> GetDataAsync(int param1, string param2, global::System.Threading.CancellationToken cancellationToken) { - return this.GetDataAsync(param1, param2, default, default, cancellationToken); + return this.GetDataAsync(param1: param1, param2: param2, param3: default, param4: default, cancellationToken: cancellationToken); } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs index d3453a0b8d3..3050a4a5b27 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAdded.cs @@ -50,13 +50,13 @@ public partial class TestClient [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] public virtual global::System.ClientModel.ClientResult GetData(int param1, string param2, global::System.Threading.CancellationToken cancellationToken) { - return this.GetData(param1, param2, default, cancellationToken); + return this.GetData(param1: param1, param2: param2, param3: default, cancellationToken: cancellationToken); } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] public virtual global::System.Threading.Tasks.Task> GetDataAsync(int param1, string param2, global::System.Threading.CancellationToken cancellationToken) { - return this.GetDataAsync(param1, param2, default, cancellationToken); + return this.GetDataAsync(param1: param1, param2: param2, param3: default, cancellationToken: cancellationToken); } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs index bcda9712c9a..7abd57c6eeb 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithModelBody.cs @@ -48,13 +48,13 @@ public partial class TestClient [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] public virtual global::System.ClientModel.ClientResult GetData(int param1, global::Sample.Models.SampleModel body, global::System.Threading.CancellationToken cancellationToken) { - return this.GetData(param1, body, default, cancellationToken); + return this.GetData(param1: param1, body: body, param3: default, cancellationToken: cancellationToken); } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] public virtual global::System.Threading.Tasks.Task> GetDataAsync(int param1, global::Sample.Models.SampleModel body, global::System.Threading.CancellationToken cancellationToken) { - return this.GetDataAsync(param1, body, default, cancellationToken); + return this.GetDataAsync(param1: param1, body: body, param3: default, cancellationToken: cancellationToken); } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters.cs index 3d32328ae88..ecd5a27c74a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/BackCompatibility_NewOptionalNonBodyParameterAddedWithPathAndHeaderParameters.cs @@ -51,13 +51,13 @@ public partial class TestClient [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] public virtual global::System.ClientModel.ClientResult GetData(string itemId, int filter, string region, global::System.Threading.CancellationToken cancellationToken) { - return this.GetData(itemId, filter, region, default, cancellationToken); + return this.GetData(itemId: itemId, filter: filter, region: region, sort: default, cancellationToken: cancellationToken); } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] public virtual global::System.Threading.Tasks.Task> GetDataAsync(string itemId, int filter, string region, global::System.Threading.CancellationToken cancellationToken) { - return this.GetDataAsync(itemId, filter, region, default, cancellationToken); + return this.GetDataAsync(itemId: itemId, filter: filter, region: region, sort: default, cancellationToken: cancellationToken); } } } diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index e60b9e85ab1..58033ebdc03 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -791,9 +791,7 @@ public virtual ClientResult UpdateSkillDefaultVersion(string skillId, string con #### Scenario: New Optional Non-Body Parameter Added to a Service Method -**Description:** When the current TypeSpec adds one or more new optional non-body parameters (e.g. query, header, path) to an existing service method, the generator emits a hidden back-compat overload that matches the previous contract's signature and delegates to the new method, passing `default` for the new parameter(s). - -This follows the guidance in [Service-Driven Evolution: A method gets a new optional parameter](https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter). The behavior is **intentionally restricted to non-body parameters** because adding a body parameter typically reflects a schema change and is handled differently. +**Description:** When the current TypeSpec adds one or more new optional non-body parameters (e.g. query, header, path) to an existing service method, the generator emits a hidden back-compat overload that matches the previous contract's signature and delegates to the new method, passing `default` for the new parameter(s). The behavior is **intentionally restricted to non-body parameters** because adding a body parameter typically reflects a schema change and is handled differently. **Rules:** @@ -849,4 +847,4 @@ public virtual Task GetDataAsync(int p1, BinaryContent body, Reque } ``` -The back-compat overloads are hidden from IntelliSense via `[EditorBrowsable(EditorBrowsableState.Never)]`, have all default values stripped to avoid ambiguous call sites with the current methods, and delegate to the current method passing `default` for each new parameter. Async overloads return the `Task` directly without `await` so the back-compat method itself remains non-async, avoiding unnecessary state-machine overhead for a simple delegation. +The back-compat overloads are hidden from IntelliSense via `[EditorBrowsable(EditorBrowsableState.Never)]`, have all default values stripped to avoid ambiguous call sites with the current methods, and delegate to the current method passing `default` for each new parameter. From 1ca179b8ed5d3140595432c95c333b2ec02d315a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:30:11 +0000 Subject: [PATCH 13/13] Inline invocation/body and drop ToVariableName in back-compat overload Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/637c9d8b-3cd8-40eb-aa73-3c87be64c020 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index b46d25fa85f..e8abf90fbe5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1889,26 +1889,21 @@ private static bool HasNewOptionalNonBodyParametersOnly( var previousParamsByName = new Dictionary(); foreach (var p in previousSignature.Parameters) { - previousParamsByName.TryAdd(p.Name.ToVariableName(), p); + previousParamsByName.TryAdd(p.Name, p); } var arguments = new List(currentSignature.Parameters.Count); foreach (var currentParam in currentSignature.Parameters) { - ValueExpression value = previousParamsByName.TryGetValue(currentParam.Name.ToVariableName(), out var prevParam) + ValueExpression value = previousParamsByName.TryGetValue(currentParam.Name, out var prevParam) ? prevParam : (currentParam.DefaultValue ?? Default); arguments.Add(PositionalReference(currentParam.Name, value)); } - var invocation = This.Invoke(currentSignature.Name, arguments); - - MethodBodyStatement body = Return(invocation); - - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature(previousSignature, hideMethod: true); return new ScmMethodProvider( - signature: backCompatSignature, - bodyStatements: body, + signature: MethodSignatureHelper.BuildBackCompatMethodSignature(previousSignature, hideMethod: true), + bodyStatements: Return(This.Invoke(currentSignature.Name, arguments)), enclosingType: this, methodKind: currentMethod.Kind, xmlDocProvider: previousMethod.XmlDocs,