From 11177315dad1ad97b3653cb996d429a5552df5cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 19:54:42 +0000 Subject: [PATCH 01/17] Initial plan From 87e3d31d3efffbb171bbf2e43ab30c42b3fca199 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:32:49 +0000 Subject: [PATCH 02/17] fix(http-client-csharp): preserve factory parameter optionality Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ModelFactoryProvider.cs | 78 ++++++++++--- .../src/Shared/MethodSignatureHelper.cs | 62 +++++++++-- .../ModelFactoryProviderTests.cs | 103 ++++++++++++++++-- .../SampleNamespaceModelFactory.cs | 21 ++++ .../SampleNamespaceModelFactory.cs | 21 ++++ .../SampleNamespaceModelFactory.cs | 20 ++++ .../SampleNamespaceModelFactory.cs | 20 ++++ ...criminatorReturnTypeOverloadIsGenerated.cs | 2 +- .../test/Shared/MethodSignatureHelperTests.cs | 48 ++++++++ 9 files changed, 342 insertions(+), 33 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index a8079006dd4..2efc189fda4 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -164,29 +164,44 @@ protected internal sealed override IReadOnlyList BuildMethodsFor foreach (var currentOverload in currentOverloads) { // If the parameter ordering is the only difference, just use the previous method - if (MethodSignatureHelper.ContainsSameParameters(previousMethod.Signature, currentOverload) - && TryBuildCompatibleMethodForPreviousContract(previousMethod, currentOverload, false, out MethodProvider? replacedMethod)) + if (MethodSignatureHelper.ContainsSameParameters(previousMethod.Signature, currentOverload)) { - factoryMethods.Add(replacedMethod); - var factoryMethodToRemove = factoryMethods .FirstOrDefault(m => MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); - if (factoryMethodToRemove != null) + var coexistingOverloads = GetCurrentOverloadSignatures( + factoryMethods, + previousMethod.Signature.Name, + factoryMethodToRemove); + if (TryBuildCompatibleMethodForPreviousContract( + previousMethod, + currentOverload, + false, + coexistingOverloads, + out MethodProvider? replacedMethod)) { - factoryMethods.Remove(factoryMethodToRemove); - } + factoryMethods.Add(replacedMethod); - CodeModelGenerator.Instance.Emitter.Debug( - $"Replaced model factory method '{Name}.{currentOverload.Name}' with previous parameter order from last contract.", - BackCompatibilityChangeCategory.ModelFactoryMethodReplaced); + if (factoryMethodToRemove != null) + { + factoryMethods.Remove(factoryMethodToRemove); + } - foundCompatibleOverload = true; - break; + CodeModelGenerator.Instance.Emitter.Debug( + $"Replaced model factory method '{Name}.{currentOverload.Name}' with previous parameter order from last contract.", + BackCompatibilityChangeCategory.ModelFactoryMethodReplaced); + foundCompatibleOverload = true; + break; + } } - if (TryBuildCompatibleMethodForPreviousContract(previousMethod, currentOverload, true, out replacedMethod)) + if (TryBuildCompatibleMethodForPreviousContract( + previousMethod, + currentOverload, + true, + GetCurrentOverloadSignatures(factoryMethods, previousMethod.Signature.Name), + out var hiddenMethod)) { - factoryMethods.Add(replacedMethod); + factoryMethods.Add(hiddenMethod); CodeModelGenerator.Instance.Emitter.Debug( $"Added back-compat overload for model factory method '{Name}.{previousMethod.Signature.Name}' delegating to '{currentOverload.Name}'.", BackCompatibilityChangeCategory.ModelFactoryMethodAdded); @@ -201,7 +216,12 @@ protected internal sealed override IReadOnlyList BuildMethodsFor } // If no compatible overload found, try to add the previous method by instantiating the model directly. - if (TryBuildCompatibleMethodForPreviousContract(previousMethod, null, true, out var builtMethod)) + if (TryBuildCompatibleMethodForPreviousContract( + previousMethod, + null, + true, + GetCurrentOverloadSignatures(factoryMethods, previousMethod.Signature.Name), + out var builtMethod)) { factoryMethods.Add(builtMethod); CodeModelGenerator.Instance.Emitter.Debug( @@ -219,6 +239,23 @@ protected internal sealed override IReadOnlyList BuildMethodsFor return [.. factoryMethods]; } + private IReadOnlyList GetCurrentOverloadSignatures( + IEnumerable factoryMethods, + string methodName, + MethodProvider? methodToExclude = null) + { + var signatures = factoryMethods + .Where(m => !ReferenceEquals(m, methodToExclude) && m.Signature.Name == methodName) + .Select(m => m.Signature) + .ToList(); + signatures.AddRange( + CustomCodeView?.Methods + .Where(m => m.Signature.Name == methodName) + .Select(m => m.Signature) + ?? []); + return signatures; + } + internal static IReadOnlyList GetUnavailableSignatureTypes(MethodSignature signature) { var unavailableTypes = new HashSet(StringComparer.Ordinal); @@ -310,6 +347,7 @@ private bool TryBuildCompatibleMethodForPreviousContract( MethodProvider previousMethod, MethodSignature? currentMethodSignature, bool hideMethod, + IReadOnlyList currentOverloadSignatures, [NotNullWhen(true)] out MethodProvider? builtMethod) { builtMethod = null; @@ -345,7 +383,10 @@ private bool TryBuildCompatibleMethodForPreviousContract( { var callToOverload = Return(new InvokeMethodExpression(null, currentMethodSignature, arguments)); builtMethod = new MethodProvider( - MethodSignatureHelper.BuildBackCompatMethodSignature(previousMethod.Signature, hideMethod), + MethodSignatureHelper.BuildBackCompatMethodSignature( + previousMethod.Signature, + hideMethod, + currentMethodSignatures: currentOverloadSignatures), callToOverload, this, previousMethod.XmlDocs); @@ -356,7 +397,10 @@ private bool TryBuildCompatibleMethodForPreviousContract( MethodBodyStatements body = ConstructMethodBody(previousMethod.Signature, modelToInstantiate); builtMethod = new MethodProvider( - MethodSignatureHelper.BuildBackCompatMethodSignature(previousMethod.Signature, hideMethod), + MethodSignatureHelper.BuildBackCompatMethodSignature( + previousMethod.Signature, + hideMethod, + currentMethodSignatures: currentOverloadSignatures), body, this, previousMethod.XmlDocs); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index 1baac80ce24..e2a5e54b835 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -57,15 +57,23 @@ internal static bool HaveSameParametersInSameOrder(MethodSignature method1, Meth return true; } - internal static MethodSignature BuildBackCompatMethodSignature(MethodSignature previousMethodSignature, bool hideMethod, bool shouldNotBeAsync = false) + internal static MethodSignature BuildBackCompatMethodSignature( + MethodSignature previousMethodSignature, + bool hideMethod, + bool shouldNotBeAsync = false, + IReadOnlyList? currentMethodSignatures = null) { - if (hideMethod) + // Require parameters through the first positional type difference with every competing + // overload. If one signature is only a type-prefix of the other, require one beyond the + // shared prefix so calls that omit trailing arguments bind only one overload. + int requiredParameterCount = currentMethodSignatures is not null + ? GetMinimumRequiredParameterCount(previousMethodSignature, currentMethodSignatures) + : hideMethod + ? previousMethodSignature.Parameters.Count + : 0; + for (int i = 0; i < requiredParameterCount; i++) { - // make all parameter required to avoid ambiguous call sites if necessary - foreach (var param in previousMethodSignature.Parameters) - { - param.DefaultValue = null; - } + previousMethodSignature.Parameters[i].DefaultValue = null; } var modifiers = shouldNotBeAsync @@ -85,6 +93,46 @@ internal static MethodSignature BuildBackCompatMethodSignature(MethodSignature p Attributes: attributes); } + private static int GetMinimumRequiredParameterCount( + MethodSignature previousMethodSignature, + IReadOnlyList currentMethodSignatures) + { + int requiredParameterCount = 0; + foreach (var currentMethodSignature in currentMethodSignatures) + { + if (currentMethodSignature.Name == previousMethodSignature.Name) + { + requiredParameterCount = Math.Max( + requiredParameterCount, + GetMinimumRequiredParameterCount(previousMethodSignature, currentMethodSignature)); + } + } + + return requiredParameterCount; + } + + private static int GetMinimumRequiredParameterCount( + MethodSignature previousMethodSignature, + MethodSignature currentMethodSignature) + { + int sharedParameterCount = Math.Min( + previousMethodSignature.Parameters.Count, + currentMethodSignature.Parameters.Count); + for (int i = 0; i < sharedParameterCount; i++) + { + var previousType = previousMethodSignature.Parameters[i].Type; + var currentType = currentMethodSignature.Parameters[i].Type; + if (!previousType.AreNamesEqual(currentType) + || previousType.IsNullable != currentType.IsNullable + && (previousType.IsValueType || currentType.IsValueType)) + { + return i + 1; + } + } + + return Math.Min(previousMethodSignature.Parameters.Count, sharedParameterCount + 1); + } + private sealed class ParameterProviderVariableNameComparer : IEqualityComparer { public bool Equals(ParameterProvider? x, ParameterProvider? y) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index 3fd14bf42b4..4a8cc3e3df3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -257,10 +257,9 @@ public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder() Assert.AreEqual("modelProp", parameters[0].Name); Assert.AreEqual("stringProp", parameters[1].Name); Assert.AreEqual("listProp", parameters[2].Name); - foreach (var param in parameters) - { - Assert.IsNull(param.DefaultValue); - } + Assert.IsNull(parameters[0].DefaultValue); + Assert.IsNotNull(parameters[1].DefaultValue); + Assert.IsNotNull(parameters[2].DefaultValue); // validate the previous method body uses named arguments to ensure correct mapping // even though the parameter order differs between the previous and current methods @@ -272,6 +271,79 @@ public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder() result); } + [Test] + public async Task BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters() + { + var compatibilityModel = GetCompatibilityModel(includeCount: true); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [compatibilityModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync())).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var backwardCompatibilityMethod = modelFactory.Methods + .Single(m => m.Signature.Name == "CompatibilityModel" && m.Signature.Parameters.Count == 4); + var parameters = backwardCompatibilityMethod.Signature.Parameters; + + Assert.IsNull(parameters[0].DefaultValue); + Assert.IsNull(parameters[1].DefaultValue); + Assert.IsNull(parameters[2].DefaultValue); + Assert.IsNotNull(parameters[3].DefaultValue); + } + + [Test] + public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix() + { + var compatibilityModel = GetCompatibilityModel(includeCount: true); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [compatibilityModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync())).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var backwardCompatibilityMethod = modelFactory.Methods + .Single(m => m.Signature.Name == "CompatibilityModel" && m.Signature.Parameters.Count == 4); + var parameters = backwardCompatibilityMethod.Signature.Parameters; + + Assert.IsNull(parameters[0].DefaultValue); + Assert.IsNull(parameters[1].DefaultValue); + Assert.IsNull(parameters[2].DefaultValue); + Assert.IsNotNull(parameters[3].DefaultValue); + } + + [Test] + public async Task BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix() + { + var compatibilityModel = GetCompatibilityModel(includeCount: false); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [compatibilityModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Custom"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var backwardCompatibilityMethod = modelFactory.Methods.Single(m => m.Signature.Name == "CompatibilityModel"); + var parameters = backwardCompatibilityMethod.Signature.Parameters; + + CollectionAssert.AreEqual( + new[] { "id", "name", "enabled", "description", "kind" }, + parameters.Select(p => p.Name)); + Assert.IsNull(parameters[0].DefaultValue); + Assert.IsNull(parameters[1].DefaultValue); + Assert.IsNull(parameters[2].DefaultValue); + Assert.IsNotNull(parameters[3].DefaultValue); + Assert.IsNotNull(parameters[4].DefaultValue); + } + // This test validates that only the previous model factory methods are generated when only the parameter ordering is changed // in the current library version. [Test] @@ -374,10 +446,7 @@ public async Task BackCompatibility_NoCurrentOverloadFound() var parameters = backwardCompatibilityMethod!.Signature.Parameters; Assert.AreEqual(1, parameters.Count); Assert.AreEqual("stringProp", parameters[0].Name); - foreach (var param in parameters) - { - Assert.IsNull(param.DefaultValue); - } + Assert.IsNotNull(parameters[0].DefaultValue); var attributes = backwardCompatibilityMethod!.Signature.Attributes; Assert.AreEqual(1, attributes.Count); var printedAttribute = attributes[0].ToDisplayString(); @@ -1250,6 +1319,24 @@ public async Task BackCompatibility_BackCompatMethodCanBeMutatedByVisitor() Assert.IsNotNull(renamed, "The visitor's rename of the back-compat method was not applied."); } + private static InputModelType GetCompatibilityModel(bool includeCount) + { + List properties = + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Name", InputPrimitiveType.String), + InputFactory.Property("Kind", InputPrimitiveType.String), + InputFactory.Property("Enabled", new InputNullableType(InputPrimitiveType.Boolean)), + InputFactory.Property("Description", InputPrimitiveType.String), + ]; + if (includeCount) + { + properties.Add(InputFactory.Property("Count", new InputNullableType(InputPrimitiveType.Int32))); + } + + return InputFactory.Model("CompatibilityModel", properties: properties); + } + private static InputModelType[] GetTestModels() { InputType additionalPropertiesUnknown = InputPrimitiveType.Any; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..8c266785202 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)/SampleNamespaceModelFactory.cs @@ -0,0 +1,21 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id = default, + string name = default, + string kind = default, + bool? enabled = default, + string description = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..a56d75fb22c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,21 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id = default, + string name = default, + bool? enabled = default, + string description = default, + string kind = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..bd6cf0cd7f7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix/SampleNamespaceModelFactory.cs @@ -0,0 +1,20 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id = default, + string name = default, + bool? enabled = default, + string description = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..cd06f2e0679 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters/SampleNamespaceModelFactory.cs @@ -0,0 +1,20 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id, + string name, + bool? enabled, + string description = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_UnknownDiscriminatorReturnTypeOverloadIsGenerated.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_UnknownDiscriminatorReturnTypeOverloadIsGenerated.cs index c9aa1d5716c..5220d6b97f2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_UnknownDiscriminatorReturnTypeOverloadIsGenerated.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_UnknownDiscriminatorReturnTypeOverloadIsGenerated.cs @@ -20,7 +20,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.UnknownAbstractModel UnknownAbstractModel(string prop1, string kind) + public static global::Sample.Models.UnknownAbstractModel UnknownAbstractModel(string prop1 = default, string kind = default) { return new global::Sample.Models.UnknownAbstractModel(kind, prop1, additionalBinaryDataProperties: null); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index 8646fac594f..48a50604b5d 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -349,6 +349,54 @@ public void BuildBackCompatMethodSignature_HideMethodTrue_WithMultipleParameters } } + [Test] + public void BuildBackCompatMethodSignature_UsesMinimumPrefixAcrossCurrentOverloads() + { + var previousSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), + new ParameterProvider("param2", $"", typeof(string), defaultValue: Default), + new ParameterProvider("param3", $"", typeof(bool), defaultValue: Default), + new ParameterProvider("param4", $"", typeof(string), defaultValue: Default)); + var currentSignature1 = CreateMethodSignature("TestMethod", + new ParameterProvider("param3", $"", typeof(bool), defaultValue: Default), + new ParameterProvider("param1", $"", typeof(string), defaultValue: Default)); + var currentSignature2 = CreateMethodSignature("TestMethod", + new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), + new ParameterProvider("param2", $"", typeof(string), defaultValue: Default), + new ParameterProvider("other", $"", typeof(int), defaultValue: Default)); + + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + previousSignature, + hideMethod: true, + currentMethodSignatures: [currentSignature1, currentSignature2]); + + Assert.IsNull(backCompatSignature.Parameters[0].DefaultValue); + Assert.IsNull(backCompatSignature.Parameters[1].DefaultValue); + Assert.IsNull(backCompatSignature.Parameters[2].DefaultValue); + Assert.IsNotNull(backCompatSignature.Parameters[3].DefaultValue); + } + + [Test] + public void BuildBackCompatMethodSignature_OnlyValueTypeNullabilityDistinguishesOverloads() + { + var previousSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("param1", $"", new CSharpType(typeof(string), isNullable: true), defaultValue: Default), + new ParameterProvider("param2", $"", new CSharpType(typeof(int), isNullable: true), defaultValue: Default), + new ParameterProvider("param3", $"", typeof(bool), defaultValue: Default)); + var currentSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), + new ParameterProvider("param2", $"", typeof(int), defaultValue: Default)); + + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + previousSignature, + hideMethod: true, + currentMethodSignatures: [currentSignature]); + + Assert.IsNull(backCompatSignature.Parameters[0].DefaultValue); + Assert.IsNull(backCompatSignature.Parameters[1].DefaultValue); + Assert.IsNotNull(backCompatSignature.Parameters[2].DefaultValue); + } + private static MethodSignature CreateMethodSignature( string name, params ParameterProvider[] parameters) From b1d8764f204e79d32da82427e129d7c068e6b3ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:38:46 +0000 Subject: [PATCH 03/17] refactor(http-client-csharp): clarify overload type comparison Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Shared/MethodSignatureHelper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index e2a5e54b835..5fa6a5243ab 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -123,8 +123,8 @@ private static int GetMinimumRequiredParameterCount( var previousType = previousMethodSignature.Parameters[i].Type; var currentType = currentMethodSignature.Parameters[i].Type; if (!previousType.AreNamesEqual(currentType) - || previousType.IsNullable != currentType.IsNullable - && (previousType.IsValueType || currentType.IsValueType)) + || (previousType.IsNullable != currentType.IsNullable + && (previousType.IsValueType || currentType.IsValueType))) { return i + 1; } From 52eb9bddd357ff7d6a021be61a06c7357fce0582 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:00:30 +0000 Subject: [PATCH 04/17] test(http-client-csharp): snapshot model factory compatibility cases Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Shared/MethodSignatureHelper.cs | 27 +------------- .../ModelFactoryProviderTests.cs | 37 ++++--------------- ...eredCustomOverloadRequiresMinimumPrefix.cs | 22 +++++++++++ .../SampleNamespaceModelFactory.cs | 0 ...yOptionalParametersRequireMinimumPrefix.cs | 30 +++++++++++++++ .../SampleNamespaceModelFactory.cs | 0 ...etersPreserveOptionalTrailingParameters.cs | 30 +++++++++++++++ 7 files changed, 91 insertions(+), 55 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix => BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last)}/SampleNamespaceModelFactory.cs (100%) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters => BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters(Last)}/SampleNamespaceModelFactory.cs (100%) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index 5fa6a5243ab..d57fc4abd50 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -27,7 +27,7 @@ internal static bool ContainsSameParameters(MethodSignature method1, MethodSigna return false; } - HashSet method1Parameters = new(method1.Parameters, new ParameterProviderVariableNameComparer()); + HashSet method1Parameters = new(method1.Parameters); foreach (var method2Param in method2.Parameters) { if (!method1Parameters.Contains(method2Param)) @@ -132,30 +132,5 @@ private static int GetMinimumRequiredParameterCount( return Math.Min(previousMethodSignature.Parameters.Count, sharedParameterCount + 1); } - - private sealed class ParameterProviderVariableNameComparer : IEqualityComparer - { - public bool Equals(ParameterProvider? x, ParameterProvider? y) - { - if (ReferenceEquals(x, y)) - { - return true; - } - - if (x is null || y is null) - { - return false; - } - - return x.Type.AreNamesEqual(y.Type) - && x.Name.ToVariableName() == y.Name.ToVariableName() - && x.Attributes.SequenceEqual(y.Attributes); - } - - public int GetHashCode(ParameterProvider obj) - { - return HashCode.Combine(obj.Name.ToVariableName()); - } - } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index 4a8cc3e3df3..1049e98b081 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -279,19 +279,13 @@ public async Task BackCompatibility_ReorderedRequiredParametersPreserveOptionalT _instance = (await MockHelpers.LoadMockGeneratorAsync( inputNamespaceName: "Sample.Namespace", inputModelTypes: [compatibilityModel], - lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync())).Object; + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; var modelFactory = _instance.OutputLibrary.ModelFactory.Value; modelFactory.ProcessTypeForBackCompatibility(); - var backwardCompatibilityMethod = modelFactory.Methods - .Single(m => m.Signature.Name == "CompatibilityModel" && m.Signature.Parameters.Count == 4); - var parameters = backwardCompatibilityMethod.Signature.Parameters; - - Assert.IsNull(parameters[0].DefaultValue); - Assert.IsNull(parameters[1].DefaultValue); - Assert.IsNull(parameters[2].DefaultValue); - Assert.IsNotNull(parameters[3].DefaultValue); + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } [Test] @@ -302,19 +296,13 @@ public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireMinim _instance = (await MockHelpers.LoadMockGeneratorAsync( inputNamespaceName: "Sample.Namespace", inputModelTypes: [compatibilityModel], - lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync())).Object; + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; var modelFactory = _instance.OutputLibrary.ModelFactory.Value; modelFactory.ProcessTypeForBackCompatibility(); - var backwardCompatibilityMethod = modelFactory.Methods - .Single(m => m.Signature.Name == "CompatibilityModel" && m.Signature.Parameters.Count == 4); - var parameters = backwardCompatibilityMethod.Signature.Parameters; - - Assert.IsNull(parameters[0].DefaultValue); - Assert.IsNull(parameters[1].DefaultValue); - Assert.IsNull(parameters[2].DefaultValue); - Assert.IsNotNull(parameters[3].DefaultValue); + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } [Test] @@ -331,17 +319,8 @@ public async Task BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix var modelFactory = _instance.OutputLibrary.ModelFactory.Value; modelFactory.ProcessTypeForBackCompatibility(); - var backwardCompatibilityMethod = modelFactory.Methods.Single(m => m.Signature.Name == "CompatibilityModel"); - var parameters = backwardCompatibilityMethod.Signature.Parameters; - - CollectionAssert.AreEqual( - new[] { "id", "name", "enabled", "description", "kind" }, - parameters.Select(p => p.Name)); - Assert.IsNull(parameters[0].DefaultValue); - Assert.IsNull(parameters[1].DefaultValue); - Assert.IsNull(parameters[2].DefaultValue); - Assert.IsNotNull(parameters[3].DefaultValue); - Assert.IsNotNull(parameters[4].DefaultValue); + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } // This test validates that only the previous model factory methods are generated when only the parameter ordering is changed diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs new file mode 100644 index 00000000000..6f0e0d593dd --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs @@ -0,0 +1,22 @@ +// + +#nullable disable + +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default, string kind = default) + { + return new global::Sample.Models.CompatibilityModel( + id, + name, + kind, + enabled, + description, + additionalBinaryDataProperties: null); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs new file mode 100644 index 00000000000..6ab6f396661 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs @@ -0,0 +1,30 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string kind = default, bool? enabled = default, string description = default, int? count = default) + { + return new global::Sample.Models.CompatibilityModel( + id, + name, + kind, + enabled, + description, + count, + additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default) + { + return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters(Last)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters(Last)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs new file mode 100644 index 00000000000..6ab6f396661 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs @@ -0,0 +1,30 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string kind = default, bool? enabled = default, string description = default, int? count = default) + { + return new global::Sample.Models.CompatibilityModel( + id, + name, + kind, + enabled, + description, + count, + additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default) + { + return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); + } + } +} From 1320ecc9bd15fbedc3aae01dc5859f90243a43c3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:28:25 +0000 Subject: [PATCH 05/17] fix(http-client-csharp): filter model factory overload applicability Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Shared/MethodSignatureHelper.cs | 44 +++++++---- .../ModelFactoryProviderTests.cs | 7 +- ...eredCustomOverloadRequiresMinimumPrefix.cs | 2 +- ...yOptionalParametersRequireMinimumPrefix.cs | 2 +- ...etersPreserveOptionalTrailingParameters.cs | 2 +- .../test/Shared/MethodSignatureHelperTests.cs | 77 +++++++++++++++---- ...ithNamedNullAndImplicitArgumentsCompile.cs | 59 ++++++++++++++ 7 files changed, 159 insertions(+), 34 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/TestData/MethodSignatureHelperTests/BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index d57fc4abd50..0b30bb9f6ea 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -63,9 +63,10 @@ internal static MethodSignature BuildBackCompatMethodSignature( bool shouldNotBeAsync = false, IReadOnlyList? currentMethodSignatures = null) { - // Require parameters through the first positional type difference with every competing - // overload. If one signature is only a type-prefix of the other, require one beyond the - // shared prefix so calls that omit trailing arguments bind only one overload. + // Parameter type differences do not reliably disambiguate overloads because named + // arguments, null literals, and implicit conversions can still make both candidates + // applicable. When a current overload can be called with an argument count supported + // by the previous signature, require all previous parameters. int requiredParameterCount = currentMethodSignatures is not null ? GetMinimumRequiredParameterCount(previousMethodSignature, currentMethodSignatures) : hideMethod @@ -115,22 +116,37 @@ private static int GetMinimumRequiredParameterCount( MethodSignature previousMethodSignature, MethodSignature currentMethodSignature) { - int sharedParameterCount = Math.Min( - previousMethodSignature.Parameters.Count, - currentMethodSignature.Parameters.Count); - for (int i = 0; i < sharedParameterCount; i++) + if (currentMethodSignature.Parameters.Any(p => p.IsRef || p.IsOut)) { - var previousType = previousMethodSignature.Parameters[i].Type; - var currentType = currentMethodSignature.Parameters[i].Type; - if (!previousType.AreNamesEqual(currentType) - || (previousType.IsNullable != currentType.IsNullable - && (previousType.IsValueType || currentType.IsValueType))) + return 0; + } + + int previousMinimumArgumentCount = GetMinimumArgumentCount(previousMethodSignature); + int currentMinimumArgumentCount = GetMinimumArgumentCount(currentMethodSignature); + int currentMaximumArgumentCount = currentMethodSignature.Parameters.Any(p => p.IsParams) + ? int.MaxValue + : currentMethodSignature.Parameters.Count; + + return Math.Max(previousMinimumArgumentCount, currentMinimumArgumentCount) <= + Math.Min(previousMethodSignature.Parameters.Count, currentMaximumArgumentCount) + ? previousMethodSignature.Parameters.Count + : 0; + } + + private static int GetMinimumArgumentCount(MethodSignature methodSignature) + { + int count = 0; + foreach (var parameter in methodSignature.Parameters) + { + if (parameter.DefaultValue is not null || parameter.IsParams) { - return i + 1; + break; } + + count++; } - return Math.Min(previousMethodSignature.Parameters.Count, sharedParameterCount + 1); + return count; } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index 1049e98b081..4adbd2cef68 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -257,9 +257,10 @@ public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder() Assert.AreEqual("modelProp", parameters[0].Name); Assert.AreEqual("stringProp", parameters[1].Name); Assert.AreEqual("listProp", parameters[2].Name); - Assert.IsNull(parameters[0].DefaultValue); - Assert.IsNotNull(parameters[1].DefaultValue); - Assert.IsNotNull(parameters[2].DefaultValue); + foreach (var parameter in parameters) + { + Assert.IsNull(parameter.DefaultValue); + } // validate the previous method body uses named arguments to ensure correct mapping // even though the parameter order differs between the previous and current methods diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs index 6f0e0d593dd..564bb978b1a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs @@ -8,7 +8,7 @@ namespace Sample.Namespace { public static partial class SampleNamespaceModelFactory { - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default, string kind = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description, string kind) { return new global::Sample.Models.CompatibilityModel( id, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs index 6ab6f396661..05ad40d1411 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs @@ -22,7 +22,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description) { return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs index 6ab6f396661..05ad40d1411 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs @@ -22,7 +22,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description) { return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index 48a50604b5d..35ccf5bf69e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -3,10 +3,15 @@ using System; using System.Collections.Generic; +using System.IO; +using System.Linq; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; using Microsoft.TypeSpec.Generator.Statements; +using Microsoft.TypeSpec.Generator.Tests.Common; using NUnit.Framework; using static Microsoft.TypeSpec.Generator.Snippets.Snippet; @@ -350,7 +355,7 @@ public void BuildBackCompatMethodSignature_HideMethodTrue_WithMultipleParameters } [Test] - public void BuildBackCompatMethodSignature_UsesMinimumPrefixAcrossCurrentOverloads() + public void BuildBackCompatMethodSignature_RequiresAllParametersForPotentiallyApplicableOverload() { var previousSignature = CreateMethodSignature("TestMethod", new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), @@ -370,31 +375,75 @@ public void BuildBackCompatMethodSignature_UsesMinimumPrefixAcrossCurrentOverloa hideMethod: true, currentMethodSignatures: [currentSignature1, currentSignature2]); - Assert.IsNull(backCompatSignature.Parameters[0].DefaultValue); - Assert.IsNull(backCompatSignature.Parameters[1].DefaultValue); - Assert.IsNull(backCompatSignature.Parameters[2].DefaultValue); - Assert.IsNotNull(backCompatSignature.Parameters[3].DefaultValue); + foreach (var parameter in backCompatSignature.Parameters) + { + Assert.IsNull(parameter.DefaultValue); + } } [Test] - public void BuildBackCompatMethodSignature_OnlyValueTypeNullabilityDistinguishesOverloads() + public void BuildBackCompatMethodSignature_PreservesDefaultsForInapplicableArgumentCount() { var previousSignature = CreateMethodSignature("TestMethod", - new ParameterProvider("param1", $"", new CSharpType(typeof(string), isNullable: true), defaultValue: Default), - new ParameterProvider("param2", $"", new CSharpType(typeof(int), isNullable: true), defaultValue: Default), - new ParameterProvider("param3", $"", typeof(bool), defaultValue: Default)); + new ParameterProvider("value", $"", typeof(string), defaultValue: Default)); var currentSignature = CreateMethodSignature("TestMethod", - new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), - new ParameterProvider("param2", $"", typeof(int), defaultValue: Default)); + new ParameterProvider("first", $"", typeof(bool)), + new ParameterProvider("second", $"", typeof(int))); var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature]); - Assert.IsNull(backCompatSignature.Parameters[0].DefaultValue); - Assert.IsNull(backCompatSignature.Parameters[1].DefaultValue); - Assert.IsNotNull(backCompatSignature.Parameters[2].DefaultValue); + Assert.IsNotNull(backCompatSignature.Parameters[0].DefaultValue); + } + + [Test] + public void BuildBackCompatMethodSignature_PreservesDefaultsForRefOutOverload() + { + var previousSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("value", $"", typeof(string), defaultValue: Default)); + var currentSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("value", $"", typeof(string), isRef: true)); + + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + previousSignature, + hideMethod: true, + currentMethodSignatures: [currentSignature]); + + Assert.IsNotNull(backCompatSignature.Parameters[0].DefaultValue); + } + + [Test] + public void BuildBackCompatMethodSignature_PreservesDefaultsForInapplicableParamsOverload() + { + var previousSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("value", $"", typeof(string), defaultValue: Default)); + var currentSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("first", $"", typeof(bool)), + new ParameterProvider("second", $"", typeof(int)), + new ParameterProvider("rest", $"", typeof(int[]), isParams: true)); + + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + previousSignature, + hideMethod: true, + currentMethodSignatures: [currentSignature]); + + Assert.IsNotNull(backCompatSignature.Parameters[0].DefaultValue); + } + + [Test] + public void BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile() + { + var compilation = CSharpCompilation.Create( + "Consumer", + [CSharpSyntaxTree.ParseText(Helpers.GetExpectedFromFile())], + ((string)AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES")!) + .Split(Path.PathSeparator) + .Select(path => MetadataReference.CreateFromFile(path)), + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + + Assert.IsEmpty(compilation.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error)); } private static MethodSignature CreateMethodSignature( diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/TestData/MethodSignatureHelperTests/BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/TestData/MethodSignatureHelperTests/BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile.cs new file mode 100644 index 00000000000..7d10f53e6b1 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/TestData/MethodSignatureHelperTests/BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile.cs @@ -0,0 +1,59 @@ +#nullable disable + +public class Consumer +{ + public static void NamedAndNull(string id = null, string name = null, string kind = null, bool? enabled = null, string description = null) + { + } + + public static void NamedAndNull(string id, string name, bool? enabled, string description, string kind) + { + } + + public static void Implicit(NewTarget value, string optional = null) + { + } + + public static void Implicit(OldTarget value, string optional) + { + } + + public static void Inapplicable(string value = null) + { + } + + public static void Inapplicable(bool first, int second, params int[] rest) + { + } + + public static void ByRef(string value = null) + { + } + + public static void ByRef(ref string value) + { + } + + public static void Call() + { + NamedAndNull(id: "id", name: "name", kind: "kind"); + NamedAndNull("id", "name", null); + Implicit(new Source()); + Inapplicable(); + ByRef(); + } +} + +public class OldTarget +{ +} + +public class NewTarget +{ +} + +public class Source +{ + public static implicit operator OldTarget(Source value) => new OldTarget(); + public static implicit operator NewTarget(Source value) => new NewTarget(); +} From b08c9d8c59d8d14df2ca010839be856583d2c775 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:33:24 +0000 Subject: [PATCH 06/17] fix(http-client-csharp): retain custom factory overloads Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ModelFactoryProvider.cs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index 2efc189fda4..2dab31021a6 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -102,7 +102,8 @@ protected internal sealed override IReadOnlyList BuildMethodsFor return [.. originalMethods]; } - List factoryMethods = [.. originalMethods]; + IReadOnlyList customFactoryMethods = CustomCodeView?.Methods ?? []; + List factoryMethods = [.. originalMethods, .. customFactoryMethods]; // Preserve the original parameter names on current factory methods when the only // change between the previous and current contract is a parameter rename. This @@ -110,7 +111,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor // property is renamed via @@clientName, spec rename, or naming-rule change). BackCompatHelper.RestorePreviousParameterNames(this, factoryMethods); - HashSet currentMethodSignatures = new List([.. factoryMethods, .. CustomCodeView?.Methods ?? []]) + HashSet currentMethodSignatures = factoryMethods .Select(m => m.Signature) .ToHashSet(MethodSignature.MethodSignatureComparer); @@ -167,7 +168,9 @@ protected internal sealed override IReadOnlyList BuildMethodsFor if (MethodSignatureHelper.ContainsSameParameters(previousMethod.Signature, currentOverload)) { var factoryMethodToRemove = factoryMethods - .FirstOrDefault(m => MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); + .FirstOrDefault(m => + !customFactoryMethods.Contains(m) && + MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); var coexistingOverloads = GetCurrentOverloadSignatures( factoryMethods, previousMethod.Signature.Name, @@ -236,7 +239,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor } } - return [.. factoryMethods]; + return [.. factoryMethods.Where(m => !customFactoryMethods.Contains(m))]; } private IReadOnlyList GetCurrentOverloadSignatures( @@ -248,11 +251,6 @@ private IReadOnlyList GetCurrentOverloadSignatures( .Where(m => !ReferenceEquals(m, methodToExclude) && m.Signature.Name == methodName) .Select(m => m.Signature) .ToList(); - signatures.AddRange( - CustomCodeView?.Methods - .Where(m => m.Signature.Name == methodName) - .Select(m => m.Signature) - ?? []); return signatures; } From 9c58f4772276b57d4fdef164f906df3be5cb388f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:51:52 +0000 Subject: [PATCH 07/17] fix(http-client-csharp): restore parameter name comparer Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Shared/MethodSignatureHelper.cs | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index 0b30bb9f6ea..01bd6796621 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -27,7 +27,7 @@ internal static bool ContainsSameParameters(MethodSignature method1, MethodSigna return false; } - HashSet method1Parameters = new(method1.Parameters); + HashSet method1Parameters = new(method1.Parameters, new ParameterProviderVariableNameComparer()); foreach (var method2Param in method2.Parameters) { if (!method1Parameters.Contains(method2Param)) @@ -148,5 +148,30 @@ private static int GetMinimumArgumentCount(MethodSignature methodSignature) return count; } + + private sealed class ParameterProviderVariableNameComparer : IEqualityComparer + { + public bool Equals(ParameterProvider? x, ParameterProvider? y) + { + if (ReferenceEquals(x, y)) + { + return true; + } + + if (x is null || y is null) + { + return false; + } + + return x.Type.AreNamesEqual(y.Type) + && x.Name.ToVariableName() == y.Name.ToVariableName() + && x.Attributes.SequenceEqual(y.Attributes); + } + + public int GetHashCode(ParameterProvider obj) + { + return HashCode.Combine(obj.Name.ToVariableName()); + } + } } } From 94cc7819e5c37366d78fded3e9027e6f95ea90f7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:03:48 +0000 Subject: [PATCH 08/17] fix(http-client-csharp): streamline factory overload analysis Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ModelFactoryProvider.cs | 38 ++++++------ .../test/Shared/MethodSignatureHelperTests.cs | 17 ------ ...ithNamedNullAndImplicitArgumentsCompile.cs | 59 ------------------- 3 files changed, 20 insertions(+), 94 deletions(-) delete mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/TestData/MethodSignatureHelperTests/BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index 2dab31021a6..4bcd519717b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -103,7 +103,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor } IReadOnlyList customFactoryMethods = CustomCodeView?.Methods ?? []; - List factoryMethods = [.. originalMethods, .. customFactoryMethods]; + List factoryMethods = [.. originalMethods]; // Preserve the original parameter names on current factory methods when the only // change between the previous and current contract is a parameter rename. This @@ -112,6 +112,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor BackCompatHelper.RestorePreviousParameterNames(this, factoryMethods); HashSet currentMethodSignatures = factoryMethods + .Concat(customFactoryMethods) .Select(m => m.Signature) .ToHashSet(MethodSignature.MethodSignatureComparer); @@ -141,20 +142,21 @@ protected internal sealed override IReadOnlyList BuildMethodsFor List currentOverloads = []; bool foundCompatibleOverload = false; + var currentOverloadSignatures = GetCurrentOverloadSignatures( + factoryMethods, + customFactoryMethods, + previousMethod.Signature.Name); // Attempt to find an updated method in the current contract to call - foreach (var currentMethodSignature in currentMethodSignatures) + foreach (var currentMethodSignature in currentOverloadSignatures) { - if (currentMethodSignature.Name.Equals(previousMethod.Signature.Name)) + if (MethodSignatureHelper.HaveSameParametersInSameOrder(currentMethodSignature, previousMethod.Signature)) { - if (MethodSignatureHelper.HaveSameParametersInSameOrder(currentMethodSignature, previousMethod.Signature)) - { - foundCompatibleOverload = true; - break; - } - - currentOverloads.Add(currentMethodSignature); + foundCompatibleOverload = true; + break; } + + currentOverloads.Add(currentMethodSignature); } if (foundCompatibleOverload) @@ -168,11 +170,10 @@ protected internal sealed override IReadOnlyList BuildMethodsFor if (MethodSignatureHelper.ContainsSameParameters(previousMethod.Signature, currentOverload)) { var factoryMethodToRemove = factoryMethods - .FirstOrDefault(m => - !customFactoryMethods.Contains(m) && - MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); + .FirstOrDefault(m => MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); var coexistingOverloads = GetCurrentOverloadSignatures( factoryMethods, + customFactoryMethods, previousMethod.Signature.Name, factoryMethodToRemove); if (TryBuildCompatibleMethodForPreviousContract( @@ -201,7 +202,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor previousMethod, currentOverload, true, - GetCurrentOverloadSignatures(factoryMethods, previousMethod.Signature.Name), + currentOverloadSignatures, out var hiddenMethod)) { factoryMethods.Add(hiddenMethod); @@ -223,7 +224,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor previousMethod, null, true, - GetCurrentOverloadSignatures(factoryMethods, previousMethod.Signature.Name), + currentOverloadSignatures, out var builtMethod)) { factoryMethods.Add(builtMethod); @@ -239,19 +240,20 @@ protected internal sealed override IReadOnlyList BuildMethodsFor } } - return [.. factoryMethods.Where(m => !customFactoryMethods.Contains(m))]; + return [.. factoryMethods]; } private IReadOnlyList GetCurrentOverloadSignatures( IEnumerable factoryMethods, + IEnumerable customFactoryMethods, string methodName, MethodProvider? methodToExclude = null) { - var signatures = factoryMethods + return factoryMethods + .Concat(customFactoryMethods) .Where(m => !ReferenceEquals(m, methodToExclude) && m.Signature.Name == methodName) .Select(m => m.Signature) .ToList(); - return signatures; } internal static IReadOnlyList GetUnavailableSignatureTypes(MethodSignature signature) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index 35ccf5bf69e..1f4a21c1761 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -3,11 +3,8 @@ using System; using System.Collections.Generic; -using System.IO; using System.Linq; using System.Threading.Tasks; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; using Microsoft.TypeSpec.Generator.Statements; @@ -432,20 +429,6 @@ public void BuildBackCompatMethodSignature_PreservesDefaultsForInapplicableParam Assert.IsNotNull(backCompatSignature.Parameters[0].DefaultValue); } - [Test] - public void BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile() - { - var compilation = CSharpCompilation.Create( - "Consumer", - [CSharpSyntaxTree.ParseText(Helpers.GetExpectedFromFile())], - ((string)AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES")!) - .Split(Path.PathSeparator) - .Select(path => MetadataReference.CreateFromFile(path)), - new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); - - Assert.IsEmpty(compilation.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error)); - } - private static MethodSignature CreateMethodSignature( string name, params ParameterProvider[] parameters) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/TestData/MethodSignatureHelperTests/BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/TestData/MethodSignatureHelperTests/BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile.cs deleted file mode 100644 index 7d10f53e6b1..00000000000 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/TestData/MethodSignatureHelperTests/BuildBackCompatMethodSignature_ConsumerCallsWithNamedNullAndImplicitArgumentsCompile.cs +++ /dev/null @@ -1,59 +0,0 @@ -#nullable disable - -public class Consumer -{ - public static void NamedAndNull(string id = null, string name = null, string kind = null, bool? enabled = null, string description = null) - { - } - - public static void NamedAndNull(string id, string name, bool? enabled, string description, string kind) - { - } - - public static void Implicit(NewTarget value, string optional = null) - { - } - - public static void Implicit(OldTarget value, string optional) - { - } - - public static void Inapplicable(string value = null) - { - } - - public static void Inapplicable(bool first, int second, params int[] rest) - { - } - - public static void ByRef(string value = null) - { - } - - public static void ByRef(ref string value) - { - } - - public static void Call() - { - NamedAndNull(id: "id", name: "name", kind: "kind"); - NamedAndNull("id", "name", null); - Implicit(new Source()); - Inapplicable(); - ByRef(); - } -} - -public class OldTarget -{ -} - -public class NewTarget -{ -} - -public class Source -{ - public static implicit operator OldTarget(Source value) => new OldTarget(); - public static implicit operator NewTarget(Source value) => new NewTarget(); -} From 684de9a612510f16be958fcac6bf55c3383bb889 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:28:09 +0000 Subject: [PATCH 09/17] refactor(http-client-csharp): clarify model factory overload analysis Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ModelFactoryProvider.cs | 16 ++++----- .../src/Shared/MethodSignatureHelper.cs | 34 +++++++++++++------ .../test/Shared/MethodSignatureHelperTests.cs | 31 ++++++++++++++--- 3 files changed, 56 insertions(+), 25 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index 4bcd519717b..5fac7640457 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -143,8 +143,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor List currentOverloads = []; bool foundCompatibleOverload = false; var currentOverloadSignatures = GetCurrentOverloadSignatures( - factoryMethods, - customFactoryMethods, + factoryMethods.Concat(customFactoryMethods), previousMethod.Signature.Name); // Attempt to find an updated method in the current contract to call @@ -172,8 +171,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor var factoryMethodToRemove = factoryMethods .FirstOrDefault(m => MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); var coexistingOverloads = GetCurrentOverloadSignatures( - factoryMethods, - customFactoryMethods, + factoryMethods.Concat(customFactoryMethods), previousMethod.Signature.Name, factoryMethodToRemove); if (TryBuildCompatibleMethodForPreviousContract( @@ -244,13 +242,11 @@ protected internal sealed override IReadOnlyList BuildMethodsFor } private IReadOnlyList GetCurrentOverloadSignatures( - IEnumerable factoryMethods, - IEnumerable customFactoryMethods, + IEnumerable methods, string methodName, MethodProvider? methodToExclude = null) { - return factoryMethods - .Concat(customFactoryMethods) + return methods .Where(m => !ReferenceEquals(m, methodToExclude) && m.Signature.Name == methodName) .Select(m => m.Signature) .ToList(); @@ -383,7 +379,7 @@ private bool TryBuildCompatibleMethodForPreviousContract( { var callToOverload = Return(new InvokeMethodExpression(null, currentMethodSignature, arguments)); builtMethod = new MethodProvider( - MethodSignatureHelper.BuildBackCompatMethodSignature( + MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( previousMethod.Signature, hideMethod, currentMethodSignatures: currentOverloadSignatures), @@ -397,7 +393,7 @@ private bool TryBuildCompatibleMethodForPreviousContract( MethodBodyStatements body = ConstructMethodBody(previousMethod.Signature, modelToInstantiate); builtMethod = new MethodProvider( - MethodSignatureHelper.BuildBackCompatMethodSignature( + MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( previousMethod.Signature, hideMethod, currentMethodSignatures: currentOverloadSignatures), diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index 01bd6796621..61dd7165455 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -60,18 +60,30 @@ internal static bool HaveSameParametersInSameOrder(MethodSignature method1, Meth internal static MethodSignature BuildBackCompatMethodSignature( MethodSignature previousMethodSignature, bool hideMethod, - bool shouldNotBeAsync = false, - IReadOnlyList? currentMethodSignatures = null) + bool shouldNotBeAsync = false) + => BuildBackCompatMethodSignature( + previousMethodSignature, + hideMethod, + shouldNotBeAsync, + hideMethod ? previousMethodSignature.Parameters.Count : 0); + + internal static MethodSignature BuildBackCompatMethodSignatureWithOverloadAnalysis( + MethodSignature previousMethodSignature, + bool hideMethod, + IReadOnlyList currentMethodSignatures, + bool shouldNotBeAsync = false) + => BuildBackCompatMethodSignature( + previousMethodSignature, + hideMethod, + shouldNotBeAsync, + GetMinimumRequiredParameterCount(previousMethodSignature, currentMethodSignatures)); + + private static MethodSignature BuildBackCompatMethodSignature( + MethodSignature previousMethodSignature, + bool hideMethod, + bool shouldNotBeAsync, + int requiredParameterCount) { - // Parameter type differences do not reliably disambiguate overloads because named - // arguments, null literals, and implicit conversions can still make both candidates - // applicable. When a current overload can be called with an argument count supported - // by the previous signature, require all previous parameters. - int requiredParameterCount = currentMethodSignatures is not null - ? GetMinimumRequiredParameterCount(previousMethodSignature, currentMethodSignatures) - : hideMethod - ? previousMethodSignature.Parameters.Count - : 0; for (int i = 0; i < requiredParameterCount; i++) { previousMethodSignature.Parameters[i].DefaultValue = null; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index 1f4a21c1761..8757d65218c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -367,7 +367,7 @@ public void BuildBackCompatMethodSignature_RequiresAllParametersForPotentiallyAp new ParameterProvider("param2", $"", typeof(string), defaultValue: Default), new ParameterProvider("other", $"", typeof(int), defaultValue: Default)); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature1, currentSignature2]); @@ -376,6 +376,29 @@ public void BuildBackCompatMethodSignature_RequiresAllParametersForPotentiallyAp { Assert.IsNull(parameter.DefaultValue); } + Assert.IsTrue(backCompatSignature.Attributes.Any(a => a.Type.Equals(typeof(System.ComponentModel.EditorBrowsableAttribute)))); + } + + [Test] + public void BuildBackCompatMethodSignatureWithOverloadAnalysis_HideMethodFalse_RemovesDefaultsWithoutEditorBrowsable() + { + var previousSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), + new ParameterProvider("param2", $"", typeof(string), defaultValue: Default)); + var currentSignature = CreateMethodSignature("TestMethod", + new ParameterProvider("param2", $"", typeof(string), defaultValue: Default), + new ParameterProvider("param1", $"", typeof(string), defaultValue: Default)); + + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( + previousSignature, + hideMethod: false, + currentMethodSignatures: [currentSignature]); + + foreach (var parameter in backCompatSignature.Parameters) + { + Assert.IsNull(parameter.DefaultValue); + } + Assert.IsFalse(backCompatSignature.Attributes.Any(a => a.Type.Equals(typeof(System.ComponentModel.EditorBrowsableAttribute)))); } [Test] @@ -387,7 +410,7 @@ public void BuildBackCompatMethodSignature_PreservesDefaultsForInapplicableArgum new ParameterProvider("first", $"", typeof(bool)), new ParameterProvider("second", $"", typeof(int))); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature]); @@ -403,7 +426,7 @@ public void BuildBackCompatMethodSignature_PreservesDefaultsForRefOutOverload() var currentSignature = CreateMethodSignature("TestMethod", new ParameterProvider("value", $"", typeof(string), isRef: true)); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature]); @@ -421,7 +444,7 @@ public void BuildBackCompatMethodSignature_PreservesDefaultsForInapplicableParam new ParameterProvider("second", $"", typeof(int)), new ParameterProvider("rest", $"", typeof(int[]), isParams: true)); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature]); From d952b09a5e0f20f2d47a8f0436ab09231ccef7bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:55:43 +0000 Subject: [PATCH 10/17] fix(http-client-csharp): stabilize model factory overload analysis Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ModelFactoryProvider.cs | 63 +++++++++++++++++-- .../src/Shared/MethodSignatureHelper.cs | 2 +- .../ModelFactoryProviderTests.cs | 6 +- .../SampleNamespaceModelFactory.cs | 0 .../SampleNamespaceModelFactory.cs | 0 ...redCustomOverloadRequiresAllParameters.cs} | 0 .../SampleNamespaceModelFactory.cs | 0 ...OptionalParametersRequireAllParameters.cs} | 0 .../SampleNamespaceModelFactory.cs | 0 ...RequiredParametersRequireAllParameters.cs} | 0 .../test/Shared/MethodSignatureHelperTests.cs | 12 ++-- 11 files changed, 67 insertions(+), 16 deletions(-) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom) => BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Custom)}/SampleNamespaceModelFactory.cs (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last) => BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Last)}/SampleNamespaceModelFactory.cs (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs => BackCompatibility_ReorderedCustomOverloadRequiresAllParameters.cs} (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last) => BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters(Last)}/SampleNamespaceModelFactory.cs (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs => BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters.cs} (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters(Last) => BackCompatibility_ReorderedRequiredParametersRequireAllParameters(Last)}/SampleNamespaceModelFactory.cs (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs => BackCompatibility_ReorderedRequiredParametersRequireAllParameters.cs} (100%) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index 5fac7640457..3bac0ab56a4 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -116,6 +116,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor .Select(m => m.Signature) .ToHashSet(MethodSignature.MethodSignatureComparer); + var compatiblePreviousMethods = new List(); foreach (var previousMethod in LastContractView.Methods) { if (!MethodSignatureHelper.IsPublicApi(previousMethod.Signature.Modifiers) || @@ -140,11 +141,21 @@ protected internal sealed override IReadOnlyList BuildMethodsFor continue; } + compatiblePreviousMethods.Add(previousMethod); + } + + foreach (var previousMethod in compatiblePreviousMethods) + { List currentOverloads = []; bool foundCompatibleOverload = false; var currentOverloadSignatures = GetCurrentOverloadSignatures( factoryMethods.Concat(customFactoryMethods), previousMethod.Signature.Name); + var compatibilityOverloadSignatures = GetCompatibilityOverloadSignatures( + currentOverloadSignatures, + compatiblePreviousMethods, + previousMethod); + bool previousMethodHasOptionalParameters = previousMethod.Signature.Parameters.Any(p => p.DefaultValue is not null); // Attempt to find an updated method in the current contract to call foreach (var currentMethodSignature in currentOverloadSignatures) @@ -174,11 +185,15 @@ protected internal sealed override IReadOnlyList BuildMethodsFor factoryMethods.Concat(customFactoryMethods), previousMethod.Signature.Name, factoryMethodToRemove); + var coexistingCompatibilityOverloads = GetCompatibilityOverloadSignatures( + coexistingOverloads, + compatiblePreviousMethods, + previousMethod); if (TryBuildCompatibleMethodForPreviousContract( previousMethod, currentOverload, false, - coexistingOverloads, + coexistingCompatibilityOverloads, out MethodProvider? replacedMethod)) { factoryMethods.Add(replacedMethod); @@ -188,6 +203,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor factoryMethods.Remove(factoryMethodToRemove); } + ReportPotentialCompatibilitySourceBreak(previousMethod, replacedMethod, previousMethodHasOptionalParameters); CodeModelGenerator.Instance.Emitter.Debug( $"Replaced model factory method '{Name}.{currentOverload.Name}' with previous parameter order from last contract.", BackCompatibilityChangeCategory.ModelFactoryMethodReplaced); @@ -200,10 +216,11 @@ protected internal sealed override IReadOnlyList BuildMethodsFor previousMethod, currentOverload, true, - currentOverloadSignatures, + compatibilityOverloadSignatures, out var hiddenMethod)) { factoryMethods.Add(hiddenMethod); + ReportPotentialCompatibilitySourceBreak(previousMethod, hiddenMethod, previousMethodHasOptionalParameters); CodeModelGenerator.Instance.Emitter.Debug( $"Added back-compat overload for model factory method '{Name}.{previousMethod.Signature.Name}' delegating to '{currentOverload.Name}'.", BackCompatibilityChangeCategory.ModelFactoryMethodAdded); @@ -222,10 +239,11 @@ protected internal sealed override IReadOnlyList BuildMethodsFor previousMethod, null, true, - currentOverloadSignatures, + compatibilityOverloadSignatures, out var builtMethod)) { factoryMethods.Add(builtMethod); + ReportPotentialCompatibilitySourceBreak(previousMethod, builtMethod, previousMethodHasOptionalParameters); CodeModelGenerator.Instance.Emitter.Debug( $"Added back-compat model factory method '{Name}.{previousMethod.Signature.Name}' from last contract.", BackCompatibilityChangeCategory.ModelFactoryMethodAdded); @@ -241,17 +259,50 @@ protected internal sealed override IReadOnlyList BuildMethodsFor return [.. factoryMethods]; } + private static IReadOnlyList GetCompatibilityOverloadSignatures( + IReadOnlyList currentOverloadSignatures, + IReadOnlyList compatiblePreviousMethods, + MethodProvider previousMethod) + { + HashSet overloadSignatures = new(MethodSignature.MethodSignatureComparer); + foreach (var compatiblePreviousMethod in compatiblePreviousMethods) + { + if (!ReferenceEquals(compatiblePreviousMethod, previousMethod)) + { + overloadSignatures.Add(compatiblePreviousMethod.Signature); + } + } + + overloadSignatures.UnionWith(currentOverloadSignatures); + return [.. overloadSignatures]; + } + private IReadOnlyList GetCurrentOverloadSignatures( IEnumerable methods, string methodName, MethodProvider? methodToExclude = null) { return methods - .Where(m => !ReferenceEquals(m, methodToExclude) && m.Signature.Name == methodName) + .Where(m => (methodToExclude is null || !ReferenceEquals(m, methodToExclude)) && m.Signature.Name == methodName) .Select(m => m.Signature) .ToList(); } + private void ReportPotentialCompatibilitySourceBreak( + MethodProvider previousMethod, + MethodProvider compatibilityMethod, + bool previousMethodHasOptionalParameters) + { + if (!previousMethodHasOptionalParameters || compatibilityMethod.Signature.Parameters.Any(p => p.DefaultValue is not null)) + { + return; + } + + CodeModelGenerator.Instance.Emitter.Info( + $"Back-compat model factory method '{Name}.{previousMethod.Signature.Name}' requires all parameters because of coexisting overloads; some calls accepted by the last contract may no longer compile.", + BackCompatibilityChangeCategory.ModelFactoryMethodAdded); + } + internal static IReadOnlyList GetUnavailableSignatureTypes(MethodSignature signature) { var unavailableTypes = new HashSet(StringComparer.Ordinal); @@ -379,7 +430,7 @@ private bool TryBuildCompatibleMethodForPreviousContract( { var callToOverload = Return(new InvokeMethodExpression(null, currentMethodSignature, arguments)); builtMethod = new MethodProvider( - MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( + MethodSignatureHelper.BuildBackCompatMethodSignature( previousMethod.Signature, hideMethod, currentMethodSignatures: currentOverloadSignatures), @@ -393,7 +444,7 @@ private bool TryBuildCompatibleMethodForPreviousContract( MethodBodyStatements body = ConstructMethodBody(previousMethod.Signature, modelToInstantiate); builtMethod = new MethodProvider( - MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( + MethodSignatureHelper.BuildBackCompatMethodSignature( previousMethod.Signature, hideMethod, currentMethodSignatures: currentOverloadSignatures), diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index 61dd7165455..e4a16583155 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -67,7 +67,7 @@ internal static MethodSignature BuildBackCompatMethodSignature( shouldNotBeAsync, hideMethod ? previousMethodSignature.Parameters.Count : 0); - internal static MethodSignature BuildBackCompatMethodSignatureWithOverloadAnalysis( + internal static MethodSignature BuildBackCompatMethodSignature( MethodSignature previousMethodSignature, bool hideMethod, IReadOnlyList currentMethodSignatures, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index 4adbd2cef68..2b71766a3b8 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -273,7 +273,7 @@ public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder() } [Test] - public async Task BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters() + public async Task BackCompatibility_ReorderedRequiredParametersRequireAllParameters() { var compatibilityModel = GetCompatibilityModel(includeCount: true); @@ -290,7 +290,7 @@ public async Task BackCompatibility_ReorderedRequiredParametersPreserveOptionalT } [Test] - public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix() + public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters() { var compatibilityModel = GetCompatibilityModel(includeCount: true); @@ -307,7 +307,7 @@ public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireMinim } [Test] - public async Task BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix() + public async Task BackCompatibility_ReorderedCustomOverloadRequiresAllParameters() { var compatibilityModel = GetCompatibilityModel(includeCount: false); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Custom)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Custom)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Last)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Last)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last)/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters(Last)/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveOptionalTrailingParameters.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index 8757d65218c..816c469347c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -367,7 +367,7 @@ public void BuildBackCompatMethodSignature_RequiresAllParametersForPotentiallyAp new ParameterProvider("param2", $"", typeof(string), defaultValue: Default), new ParameterProvider("other", $"", typeof(int), defaultValue: Default)); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature1, currentSignature2]); @@ -380,7 +380,7 @@ public void BuildBackCompatMethodSignature_RequiresAllParametersForPotentiallyAp } [Test] - public void BuildBackCompatMethodSignatureWithOverloadAnalysis_HideMethodFalse_RemovesDefaultsWithoutEditorBrowsable() + public void BuildBackCompatMethodSignature_WithOverloads_HideMethodFalse_RemovesDefaultsWithoutEditorBrowsable() { var previousSignature = CreateMethodSignature("TestMethod", new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), @@ -389,7 +389,7 @@ public void BuildBackCompatMethodSignatureWithOverloadAnalysis_HideMethodFalse_R new ParameterProvider("param2", $"", typeof(string), defaultValue: Default), new ParameterProvider("param1", $"", typeof(string), defaultValue: Default)); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( previousSignature, hideMethod: false, currentMethodSignatures: [currentSignature]); @@ -410,7 +410,7 @@ public void BuildBackCompatMethodSignature_PreservesDefaultsForInapplicableArgum new ParameterProvider("first", $"", typeof(bool)), new ParameterProvider("second", $"", typeof(int))); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature]); @@ -426,7 +426,7 @@ public void BuildBackCompatMethodSignature_PreservesDefaultsForRefOutOverload() var currentSignature = CreateMethodSignature("TestMethod", new ParameterProvider("value", $"", typeof(string), isRef: true)); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature]); @@ -444,7 +444,7 @@ public void BuildBackCompatMethodSignature_PreservesDefaultsForInapplicableParam new ParameterProvider("second", $"", typeof(int)), new ParameterProvider("rest", $"", typeof(int[]), isParams: true)); - var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignatureWithOverloadAnalysis( + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( previousSignature, hideMethod: true, currentMethodSignatures: [currentSignature]); From 4ecb8fa6a75e3b91fdad79bd3b8853fc56f40175 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 23:03:15 +0000 Subject: [PATCH 11/17] test(http-client-csharp): clarify overload analysis coverage Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelFactoryProviderTests.cs | 17 ----------- .../SampleNamespaceModelFactory.cs | 20 ------------- ...dRequiredParametersRequireAllParameters.cs | 30 ------------------- .../test/Shared/MethodSignatureHelperTests.cs | 28 ++++++++--------- 4 files changed, 14 insertions(+), 81 deletions(-) delete mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs delete mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index 2b71766a3b8..0554d3ba7f0 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -272,23 +272,6 @@ public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder() result); } - [Test] - public async Task BackCompatibility_ReorderedRequiredParametersRequireAllParameters() - { - var compatibilityModel = GetCompatibilityModel(includeCount: true); - - _instance = (await MockHelpers.LoadMockGeneratorAsync( - inputNamespaceName: "Sample.Namespace", - inputModelTypes: [compatibilityModel], - lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; - - var modelFactory = _instance.OutputLibrary.ModelFactory.Value; - modelFactory.ProcessTypeForBackCompatibility(); - - var content = new TypeProviderWriter(modelFactory).Write().Content; - Assert.AreEqual(Helpers.GetExpectedFromFile(), content); - } - [Test] public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs deleted file mode 100644 index cd06f2e0679..00000000000 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs +++ /dev/null @@ -1,20 +0,0 @@ -using Sample.Models; - -namespace Sample.Namespace -{ - public static partial class SampleNamespaceModelFactory - { - public static CompatibilityModel CompatibilityModel( - string id, - string name, - bool? enabled, - string description = default) - { } - } -} - -namespace Sample.Models -{ - public partial class CompatibilityModel - { } -} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters.cs deleted file mode 100644 index 05ad40d1411..00000000000 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersRequireAllParameters.cs +++ /dev/null @@ -1,30 +0,0 @@ -// - -#nullable disable - -using System.ComponentModel; -using Sample.Models; - -namespace Sample.Namespace -{ - public static partial class SampleNamespaceModelFactory - { - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string kind = default, bool? enabled = default, string description = default, int? count = default) - { - return new global::Sample.Models.CompatibilityModel( - id, - name, - kind, - enabled, - description, - count, - additionalBinaryDataProperties: null); - } - - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description) - { - return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); - } - } -} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index 816c469347c..1eca30b5b89 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -352,25 +352,25 @@ public void BuildBackCompatMethodSignature_HideMethodTrue_WithMultipleParameters } [Test] - public void BuildBackCompatMethodSignature_RequiresAllParametersForPotentiallyApplicableOverload() + public void BuildBackCompatMethodSignature_AllOptionalFactoryOverloadsRequireAllParameters() { - var previousSignature = CreateMethodSignature("TestMethod", - new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), - new ParameterProvider("param2", $"", typeof(string), defaultValue: Default), - new ParameterProvider("param3", $"", typeof(bool), defaultValue: Default), - new ParameterProvider("param4", $"", typeof(string), defaultValue: Default)); - var currentSignature1 = CreateMethodSignature("TestMethod", - new ParameterProvider("param3", $"", typeof(bool), defaultValue: Default), - new ParameterProvider("param1", $"", typeof(string), defaultValue: Default)); - var currentSignature2 = CreateMethodSignature("TestMethod", - new ParameterProvider("param1", $"", typeof(string), defaultValue: Default), - new ParameterProvider("param2", $"", typeof(string), defaultValue: Default), - new ParameterProvider("other", $"", typeof(int), defaultValue: Default)); + var previousSignature = CreateMethodSignature("CompatibilityModel", + new ParameterProvider("id", $"", typeof(string), defaultValue: Default), + new ParameterProvider("name", $"", typeof(string), defaultValue: Default), + new ParameterProvider("enabled", $"", typeof(bool?), defaultValue: Default), + new ParameterProvider("description", $"", typeof(string), defaultValue: Default)); + var currentSignature = CreateMethodSignature("CompatibilityModel", + new ParameterProvider("id", $"", typeof(string), defaultValue: Default), + new ParameterProvider("name", $"", typeof(string), defaultValue: Default), + new ParameterProvider("kind", $"", typeof(string), defaultValue: Default), + new ParameterProvider("enabled", $"", typeof(bool?), defaultValue: Default), + new ParameterProvider("description", $"", typeof(string), defaultValue: Default), + new ParameterProvider("count", $"", typeof(int?), defaultValue: Default)); var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( previousSignature, hideMethod: true, - currentMethodSignatures: [currentSignature1, currentSignature2]); + currentMethodSignatures: [currentSignature]); foreach (var parameter in backCompatSignature.Parameters) { From c4d1cde04859042daaeb9bb03d68fac2c692d9a7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:21:34 +0000 Subject: [PATCH 12/17] fix(http-client-csharp): streamline back-compat factory analysis Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ModelFactoryProvider.cs | 40 ++++++------------- .../ModelFactoryProviderTests.cs | 20 ++++++++-- .../SampleNamespaceModelFactory.cs | 23 +++++++++++ .../SampleNamespaceModelFactory.cs | 20 ++++++++++ ...loadsPreserveTrailingOptionalParameters.cs | 18 +++++++++ 5 files changed, 89 insertions(+), 32 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters(Custom)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index 3bac0ab56a4..d3ba35f34c5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -111,10 +111,12 @@ protected internal sealed override IReadOnlyList BuildMethodsFor // property is renamed via @@clientName, spec rename, or naming-rule change). BackCompatHelper.RestorePreviousParameterNames(this, factoryMethods); - HashSet currentMethodSignatures = factoryMethods + var allFactoryMethods = factoryMethods .Concat(customFactoryMethods) - .Select(m => m.Signature) - .ToHashSet(MethodSignature.MethodSignatureComparer); + .ToList(); + HashSet currentMethodSignatures = allFactoryMethods + .Select(m => m.Signature) + .ToHashSet(MethodSignature.MethodSignatureComparer); var compatiblePreviousMethods = new List(); foreach (var previousMethod in LastContractView.Methods) @@ -149,13 +151,8 @@ protected internal sealed override IReadOnlyList BuildMethodsFor List currentOverloads = []; bool foundCompatibleOverload = false; var currentOverloadSignatures = GetCurrentOverloadSignatures( - factoryMethods.Concat(customFactoryMethods), + allFactoryMethods, previousMethod.Signature.Name); - var compatibilityOverloadSignatures = GetCompatibilityOverloadSignatures( - currentOverloadSignatures, - compatiblePreviousMethods, - previousMethod); - bool previousMethodHasOptionalParameters = previousMethod.Signature.Parameters.Any(p => p.DefaultValue is not null); // Attempt to find an updated method in the current contract to call foreach (var currentMethodSignature in currentOverloadSignatures) @@ -174,6 +171,11 @@ protected internal sealed override IReadOnlyList BuildMethodsFor continue; } + var compatibilityOverloadSignatures = GetCompatibilityOverloadSignatures( + currentOverloadSignatures, + compatiblePreviousMethods, + previousMethod); + foreach (var currentOverload in currentOverloads) { // If the parameter ordering is the only difference, just use the previous method @@ -182,7 +184,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor var factoryMethodToRemove = factoryMethods .FirstOrDefault(m => MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); var coexistingOverloads = GetCurrentOverloadSignatures( - factoryMethods.Concat(customFactoryMethods), + allFactoryMethods, previousMethod.Signature.Name, factoryMethodToRemove); var coexistingCompatibilityOverloads = GetCompatibilityOverloadSignatures( @@ -203,7 +205,6 @@ protected internal sealed override IReadOnlyList BuildMethodsFor factoryMethods.Remove(factoryMethodToRemove); } - ReportPotentialCompatibilitySourceBreak(previousMethod, replacedMethod, previousMethodHasOptionalParameters); CodeModelGenerator.Instance.Emitter.Debug( $"Replaced model factory method '{Name}.{currentOverload.Name}' with previous parameter order from last contract.", BackCompatibilityChangeCategory.ModelFactoryMethodReplaced); @@ -220,7 +221,6 @@ protected internal sealed override IReadOnlyList BuildMethodsFor out var hiddenMethod)) { factoryMethods.Add(hiddenMethod); - ReportPotentialCompatibilitySourceBreak(previousMethod, hiddenMethod, previousMethodHasOptionalParameters); CodeModelGenerator.Instance.Emitter.Debug( $"Added back-compat overload for model factory method '{Name}.{previousMethod.Signature.Name}' delegating to '{currentOverload.Name}'.", BackCompatibilityChangeCategory.ModelFactoryMethodAdded); @@ -243,7 +243,6 @@ protected internal sealed override IReadOnlyList BuildMethodsFor out var builtMethod)) { factoryMethods.Add(builtMethod); - ReportPotentialCompatibilitySourceBreak(previousMethod, builtMethod, previousMethodHasOptionalParameters); CodeModelGenerator.Instance.Emitter.Debug( $"Added back-compat model factory method '{Name}.{previousMethod.Signature.Name}' from last contract.", BackCompatibilityChangeCategory.ModelFactoryMethodAdded); @@ -288,21 +287,6 @@ private IReadOnlyList GetCurrentOverloadSignatures( .ToList(); } - private void ReportPotentialCompatibilitySourceBreak( - MethodProvider previousMethod, - MethodProvider compatibilityMethod, - bool previousMethodHasOptionalParameters) - { - if (!previousMethodHasOptionalParameters || compatibilityMethod.Signature.Parameters.Any(p => p.DefaultValue is not null)) - { - return; - } - - CodeModelGenerator.Instance.Emitter.Info( - $"Back-compat model factory method '{Name}.{previousMethod.Signature.Name}' requires all parameters because of coexisting overloads; some calls accepted by the last contract may no longer compile.", - BackCompatibilityChangeCategory.ModelFactoryMethodAdded); - } - internal static IReadOnlyList GetUnavailableSignatureTypes(MethodSignature signature) { var unavailableTypes = new HashSet(StringComparer.Ordinal); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index 0554d3ba7f0..b8469a83ddb 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -257,10 +257,6 @@ public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder() Assert.AreEqual("modelProp", parameters[0].Name); Assert.AreEqual("stringProp", parameters[1].Name); Assert.AreEqual("listProp", parameters[2].Name); - foreach (var parameter in parameters) - { - Assert.IsNull(parameter.DefaultValue); - } // validate the previous method body uses named arguments to ensure correct mapping // even though the parameter order differs between the previous and current methods @@ -307,6 +303,22 @@ public async Task BackCompatibility_ReorderedCustomOverloadRequiresAllParameters Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } + [Test] + public async Task BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters() + { + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [GetCompatibilityModel(includeCount: false)], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Custom"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + // This test validates that only the previous model factory methods are generated when only the parameter ordering is changed // in the current library version. [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters(Custom)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters(Custom)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..19c503d2962 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters(Custom)/SampleNamespaceModelFactory.cs @@ -0,0 +1,23 @@ +using Microsoft.TypeSpec.Generator.Customizations; +using Sample.Models; + +namespace Sample.Namespace +{ + [CodeGenSuppress("CompatibilityModel", typeof(string), typeof(string), typeof(string), typeof(bool?), typeof(string))] + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id, + string name, + bool? enabled, + string description, + string kind) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..bdfe257c627 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,20 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id, + string name, + bool? enabled = default, + string description = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters.cs new file mode 100644 index 00000000000..70b749e4a2e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsPreserveTrailingOptionalParameters.cs @@ -0,0 +1,18 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled = default, string description = default) + { + return CompatibilityModel(id: id, name: name, enabled: enabled, description: description, kind: default); + } + } +} From 890dd3ecf26f60033e7e3f51c23c755dfceb46ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 20:12:09 +0000 Subject: [PATCH 13/17] fix(http-client-csharp): preserve factory trailing defaults Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Shared/MethodSignatureHelper.cs | 4 ++- .../ModelFactoryProviderTests.cs | 17 +++++++++++ .../SampleNamespaceModelFactory.cs | 20 +++++++++++++ ...etersPreserveTrailingOptionalParameters.cs | 30 +++++++++++++++++++ .../test/Shared/MethodSignatureHelperTests.cs | 27 +++++++++++++++++ 5 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index e4a16583155..27e679d3cb2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -141,7 +141,9 @@ private static int GetMinimumRequiredParameterCount( return Math.Max(previousMinimumArgumentCount, currentMinimumArgumentCount) <= Math.Min(previousMethodSignature.Parameters.Count, currentMaximumArgumentCount) - ? previousMethodSignature.Parameters.Count + ? previousMinimumArgumentCount == 0 + ? previousMethodSignature.Parameters.Count + : previousMinimumArgumentCount : 0; } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index b8469a83ddb..c79fd92efaf 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -285,6 +285,23 @@ public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireAllPa Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } + [Test] + public async Task BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters() + { + var compatibilityModel = GetCompatibilityModel(includeCount: true); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [compatibilityModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + [Test] public async Task BackCompatibility_ReorderedCustomOverloadRequiresAllParameters() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..bdfe257c627 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,20 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id, + string name, + bool? enabled = default, + string description = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs new file mode 100644 index 00000000000..f3dd404ef54 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs @@ -0,0 +1,30 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string kind = default, bool? enabled = default, string description = default, int? count = default) + { + return new global::Sample.Models.CompatibilityModel( + id, + name, + kind, + enabled, + description, + count, + additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled = default, string description = default) + { + return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index 1eca30b5b89..af1ef0a9d62 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -379,6 +379,33 @@ public void BuildBackCompatMethodSignature_AllOptionalFactoryOverloadsRequireAll Assert.IsTrue(backCompatSignature.Attributes.Any(a => a.Type.Equals(typeof(System.ComponentModel.EditorBrowsableAttribute)))); } + [Test] + public void BuildBackCompatMethodSignature_RequiredFactoryParametersPreserveTrailingDefaults() + { + var previousSignature = CreateMethodSignature("CompatibilityModel", + new ParameterProvider("id", $"", typeof(string)), + new ParameterProvider("name", $"", typeof(string)), + new ParameterProvider("enabled", $"", typeof(bool?), defaultValue: Default), + new ParameterProvider("description", $"", typeof(string), defaultValue: Default)); + var currentSignature = CreateMethodSignature("CompatibilityModel", + new ParameterProvider("id", $"", typeof(string), defaultValue: Default), + new ParameterProvider("name", $"", typeof(string), defaultValue: Default), + new ParameterProvider("kind", $"", typeof(string), defaultValue: Default), + new ParameterProvider("enabled", $"", typeof(bool?), defaultValue: Default), + new ParameterProvider("description", $"", typeof(string), defaultValue: Default), + new ParameterProvider("count", $"", typeof(int?), defaultValue: Default)); + + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + previousSignature, + hideMethod: true, + currentMethodSignatures: [currentSignature]); + + Assert.IsNull(backCompatSignature.Parameters[0].DefaultValue); + Assert.IsNull(backCompatSignature.Parameters[1].DefaultValue); + Assert.IsNotNull(backCompatSignature.Parameters[2].DefaultValue); + Assert.IsNotNull(backCompatSignature.Parameters[3].DefaultValue); + } + [Test] public void BuildBackCompatMethodSignature_WithOverloads_HideMethodFalse_RemovesDefaultsWithoutEditorBrowsable() { From 55b24679e29f14ace966c3a8cc4d59ba4ee8f9b3 Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Tue, 18 Aug 2026 16:54:30 -0500 Subject: [PATCH 14/17] fix(http-client-csharp): preserve factory trailing optional parameters The model factory back-compat analysis promoted every parameter to required whenever the previous signature was fully optional, and it removed a current overload even when that exact signature was still part of the last contract. Regenerating Azure.ResourceManager.AppService reproduced both: SiteContainerData lost the GA parameter optionality and one of its overloads disappeared. Require only the parameter prefix up to and including the first position whose type differs from the competing overload, so trailing parameters keep the optionality they were published with, and never replace a current overload that still matches a previously shipped signature. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 098ae4f6-52c2-41dc-a7aa-88f561b153e5 --- .../src/Providers/ModelFactoryProvider.cs | 18 ++++-- .../src/Shared/MethodSignatureHelper.cs | 31 ++++++++-- .../ModelFactoryProviderTests.cs | 57 ++++++++++++++++++- .../SampleNamespaceModelFactory.cs | 18 ++++++ ...onalPrefixOverloadRequiresAllParameters.cs | 23 ++++++++ .../SampleNamespaceModelFactory.cs | 0 .../SampleNamespaceModelFactory.cs | 0 ...redCustomOverloadRequiresMinimumPrefix.cs} | 2 +- .../SampleNamespaceModelFactory.cs | 0 ...OptionalParametersRequireMinimumPrefix.cs} | 2 +- .../SampleNamespaceModelFactory.cs | 30 ++++++++++ ...eredOverloadKeptWhenStillInLastContract.cs | 23 ++++++++ ...etersPreserveTrailingOptionalParameters.cs | 2 +- .../test/Shared/MethodSignatureHelperTests.cs | 15 +++-- 14 files changed, 200 insertions(+), 21 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Custom) => BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)}/SampleNamespaceModelFactory.cs (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Last) => BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)}/SampleNamespaceModelFactory.cs (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedCustomOverloadRequiresAllParameters.cs => BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs} (92%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters(Last) => BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last)}/SampleNamespaceModelFactory.cs (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/{BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters.cs => BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs} (97%) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index d3ba35f34c5..0c4435e2a57 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -119,10 +119,19 @@ protected internal sealed override IReadOnlyList BuildMethodsFor .ToHashSet(MethodSignature.MethodSignatureComparer); var compatiblePreviousMethods = new List(); + HashSet previousPublicSignatures = new(MethodSignature.MethodSignatureComparer); foreach (var previousMethod in LastContractView.Methods) { - if (!MethodSignatureHelper.IsPublicApi(previousMethod.Signature.Modifiers) || - currentMethodSignatures.Contains(previousMethod.Signature)) + if (!MethodSignatureHelper.IsPublicApi(previousMethod.Signature.Modifiers)) + { + continue; + } + + // Record every public previous signature, including the ones skipped below, because + // a current overload that still matches one of them must never be removed. + previousPublicSignatures.Add(previousMethod.Signature); + + if (currentMethodSignatures.Contains(previousMethod.Signature)) { continue; } @@ -178,8 +187,9 @@ protected internal sealed override IReadOnlyList BuildMethodsFor foreach (var currentOverload in currentOverloads) { - // If the parameter ordering is the only difference, just use the previous method - if (MethodSignatureHelper.ContainsSameParameters(previousMethod.Signature, currentOverload)) + // If the parameter ordering is the only difference, just use the previous method. + if (MethodSignatureHelper.ContainsSameParameters(previousMethod.Signature, currentOverload) + && !previousPublicSignatures.Contains(currentOverload)) { var factoryMethodToRemove = factoryMethods .FirstOrDefault(m => MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index 27e679d3cb2..89409316daa 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -139,12 +139,31 @@ private static int GetMinimumRequiredParameterCount( ? int.MaxValue : currentMethodSignature.Parameters.Count; - return Math.Max(previousMinimumArgumentCount, currentMinimumArgumentCount) <= - Math.Min(previousMethodSignature.Parameters.Count, currentMaximumArgumentCount) - ? previousMinimumArgumentCount == 0 - ? previousMethodSignature.Parameters.Count - : previousMinimumArgumentCount - : 0; + // No argument count can reach both overloads, so the previous signature is already + // unambiguous and keeps the optionality it was published with. + if (Math.Max(previousMinimumArgumentCount, currentMinimumArgumentCount) > + Math.Min(previousMethodSignature.Parameters.Count, currentMaximumArgumentCount)) + { + return 0; + } + + // Require only the prefix up to and including the first position whose parameter type + // differs. Any call supplying that many arguments can no longer bind to the current + // overload, so every trailing parameter keeps the optionality it had previously. + int overlappingParameterCount = Math.Min( + previousMethodSignature.Parameters.Count, + currentMethodSignature.Parameters.Count); + for (int i = 0; i < overlappingParameterCount; i++) + { + if (!previousMethodSignature.Parameters[i].Type.AreNamesEqual(currentMethodSignature.Parameters[i].Type)) + { + return Math.Max(i + 1, previousMinimumArgumentCount); + } + } + + // The shorter signature is a positional prefix of the other, so no argument count + // distinguishes them. Fall back to requiring every parameter. + return previousMethodSignature.Parameters.Count; } private static int GetMinimumArgumentCount(MethodSignature methodSignature) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index c79fd92efaf..d2a2cbd2c55 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -269,7 +269,7 @@ public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder() } [Test] - public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters() + public async Task BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix() { var compatibilityModel = GetCompatibilityModel(includeCount: true); @@ -303,7 +303,7 @@ public async Task BackCompatibility_ReorderedRequiredParametersPreserveTrailingO } [Test] - public async Task BackCompatibility_ReorderedCustomOverloadRequiresAllParameters() + public async Task BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix() { var compatibilityModel = GetCompatibilityModel(includeCount: false); @@ -336,6 +336,59 @@ public async Task BackCompatibility_CustomOverloadsPreserveTrailingOptionalParam Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } + // Mirrors the reported Azure.ResourceManager.AppService regression: the last contract + // contains both the overload matching today's natural parameter order and a previously + // shipped overload with the discriminating parameter moved to the end. The first overload + // must be kept (removing it would be breaking) and the second must only require the + // parameter prefix needed to disambiguate it, preserving its trailing optional parameters. + [Test] + public async Task BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Kind", InputPrimitiveType.String), + InputFactory.Property("Image", InputPrimitiveType.String), + InputFactory.Property("IsMain", new InputNullableType(InputPrimitiveType.Boolean)), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + // A previous signature that is a positional prefix of the current overload cannot be + // disambiguated by argument count, so every parameter must become required. C# then prefers + // the compatibility overload for an exact argument list because it needs no optional defaults. + [Test] + public async Task BackCompatibility_PositionalPrefixOverloadRequiresAllParameters() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Name", InputPrimitiveType.String), + InputFactory.Property("Count", new InputNullableType(InputPrimitiveType.Int32)), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + // This test validates that only the previous model factory methods are generated when only the parameter ordering is changed // in the current library version. [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..f40d4dfc446 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,18 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id = default, + string name = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} \ No newline at end of file diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs new file mode 100644 index 00000000000..2bc28c06a72 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs @@ -0,0 +1,23 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, int? count = default) + { + return new global::Sample.Models.CompatibilityModel(id, name, count, additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name) + { + return CompatibilityModel(id: id, name: name, count: default); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Custom)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Custom)/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Custom)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters(Last)/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs similarity index 92% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs index 564bb978b1a..6f0e0d593dd 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresAllParameters.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedCustomOverloadRequiresMinimumPrefix.cs @@ -8,7 +8,7 @@ namespace Sample.Namespace { public static partial class SampleNamespaceModelFactory { - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description, string kind) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default, string kind = default) { return new global::Sample.Models.CompatibilityModel( id, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last)/SampleNamespaceModelFactory.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix(Last)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs similarity index 97% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs index 05ad40d1411..6ab6f396661 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs @@ -22,7 +22,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default) { return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..784c2492912 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,30 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + // Still matches the current contract's natural parameter order, so this overload must be + // preserved rather than replaced by the reordered overload below. + public static CompatibilityModel CompatibilityModel( + string id = default, + string kind = default, + string image = default, + bool? isMain = default) + { } + + // The previously shipped overload with 'kind' moved to the end. + public static CompatibilityModel CompatibilityModel( + string id = default, + string image = default, + bool? isMain = default, + string kind = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs new file mode 100644 index 00000000000..ec2102cb147 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs @@ -0,0 +1,23 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string kind = default, string image = default, bool? isMain = default) + { + return new global::Sample.Models.CompatibilityModel(id, kind, image, isMain, additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string image, bool? isMain, string kind = default) + { + return new global::Sample.Models.CompatibilityModel(id, kind, image, isMain, additionalBinaryDataProperties: null); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs index f3dd404ef54..6ab6f396661 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs @@ -22,7 +22,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled = default, string description = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default) { return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index af1ef0a9d62..efbdd31468d 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -352,7 +352,7 @@ public void BuildBackCompatMethodSignature_HideMethodTrue_WithMultipleParameters } [Test] - public void BuildBackCompatMethodSignature_AllOptionalFactoryOverloadsRequireAllParameters() + public void BuildBackCompatMethodSignature_AllOptionalFactoryOverloadsRequireMinimumPrefix() { var previousSignature = CreateMethodSignature("CompatibilityModel", new ParameterProvider("id", $"", typeof(string), defaultValue: Default), @@ -372,10 +372,13 @@ public void BuildBackCompatMethodSignature_AllOptionalFactoryOverloadsRequireAll hideMethod: true, currentMethodSignatures: [currentSignature]); - foreach (var parameter in backCompatSignature.Parameters) - { - Assert.IsNull(parameter.DefaultValue); - } + // 'enabled' is the first position whose type differs from the current overload + // ('bool?' versus 'kind'), so supplying three arguments already disambiguates the + // call and 'description' keeps the optionality it was published with. + Assert.IsNull(backCompatSignature.Parameters[0].DefaultValue); + Assert.IsNull(backCompatSignature.Parameters[1].DefaultValue); + Assert.IsNull(backCompatSignature.Parameters[2].DefaultValue); + Assert.IsNotNull(backCompatSignature.Parameters[3].DefaultValue); Assert.IsTrue(backCompatSignature.Attributes.Any(a => a.Type.Equals(typeof(System.ComponentModel.EditorBrowsableAttribute)))); } @@ -402,7 +405,7 @@ public void BuildBackCompatMethodSignature_RequiredFactoryParametersPreserveTrai Assert.IsNull(backCompatSignature.Parameters[0].DefaultValue); Assert.IsNull(backCompatSignature.Parameters[1].DefaultValue); - Assert.IsNotNull(backCompatSignature.Parameters[2].DefaultValue); + Assert.IsNull(backCompatSignature.Parameters[2].DefaultValue); Assert.IsNotNull(backCompatSignature.Parameters[3].DefaultValue); } From 4dd3d003e91223446967b8100b8988d027f2b7d9 Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Wed, 19 Aug 2026 11:24:51 -0500 Subject: [PATCH 15/17] more fixes --- .../src/Providers/ModelFactoryProvider.cs | 17 +++++++ .../src/Shared/MethodSignatureHelper.cs | 49 +++++++++++++------ .../ModelFactoryProviderTests.cs | 48 ++++++++++++++++++ .../SampleNamespaceModelFactory.cs | 23 +++++++++ ...sitorAddedOverloadRequiresMinimumPrefix.cs | 30 ++++++++++++ 5 files changed, 151 insertions(+), 16 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index 0c4435e2a57..a0f38625ee5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -133,6 +133,23 @@ protected internal sealed override IReadOnlyList BuildMethodsFor if (currentMethodSignatures.Contains(previousMethod.Signature)) { + // A method with this signature is already present, but it may have been restored + // from the last contract by a library visitor before this pass ran. + var restoredOverload = factoryMethods.FirstOrDefault(m => + MethodSignature.MethodSignatureComparer.Equals(m.Signature, previousMethod.Signature) + && m.Signature.Attributes.Any(a => + a.Type is { IsFrameworkType: true } + && a.Type.FrameworkType == typeof(System.ComponentModel.EditorBrowsableAttribute))); + if (restoredOverload is not null) + { + MethodSignatureHelper.RequireMinimumParameterPrefix( + restoredOverload.Signature, + GetCurrentOverloadSignatures( + allFactoryMethods, + restoredOverload.Signature.Name, + restoredOverload)); + } + continue; } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index 89409316daa..e5516bcadba 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -61,34 +61,51 @@ internal static MethodSignature BuildBackCompatMethodSignature( MethodSignature previousMethodSignature, bool hideMethod, bool shouldNotBeAsync = false) - => BuildBackCompatMethodSignature( - previousMethodSignature, - hideMethod, - shouldNotBeAsync, - hideMethod ? previousMethodSignature.Parameters.Count : 0); + { + if (hideMethod) + { + RequireMinimumParameterPrefix(previousMethodSignature); + } + + return CreateBackCompatSignature(previousMethodSignature, hideMethod, shouldNotBeAsync); + } internal static MethodSignature BuildBackCompatMethodSignature( MethodSignature previousMethodSignature, bool hideMethod, IReadOnlyList currentMethodSignatures, bool shouldNotBeAsync = false) - => BuildBackCompatMethodSignature( - previousMethodSignature, - hideMethod, - shouldNotBeAsync, - GetMinimumRequiredParameterCount(previousMethodSignature, currentMethodSignatures)); + { + RequireMinimumParameterPrefix(previousMethodSignature, currentMethodSignatures); - private static MethodSignature BuildBackCompatMethodSignature( - MethodSignature previousMethodSignature, - bool hideMethod, - bool shouldNotBeAsync, - int requiredParameterCount) + return CreateBackCompatSignature(previousMethodSignature, hideMethod, shouldNotBeAsync); + } + + /// + /// Removes the default values from the leading parameters of so it + /// can no longer be called with fewer arguments than the prefix that distinguishes it from + /// . When no overloads are supplied there is nothing to + /// compare against and every parameter becomes required. + /// + internal static void RequireMinimumParameterPrefix( + MethodSignature signature, + IReadOnlyList? currentMethodSignatures = null) { + int requiredParameterCount = currentMethodSignatures is null + ? signature.Parameters.Count + : GetMinimumRequiredParameterCount(signature, currentMethodSignatures); + for (int i = 0; i < requiredParameterCount; i++) { - previousMethodSignature.Parameters[i].DefaultValue = null; + signature.Parameters[i].DefaultValue = null; } + } + private static MethodSignature CreateBackCompatSignature( + MethodSignature previousMethodSignature, + bool hideMethod, + bool shouldNotBeAsync) + { var modifiers = shouldNotBeAsync ? previousMethodSignature.Modifiers & ~MethodSignatureModifiers.Async : previousMethodSignature.Modifiers; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index d2a2cbd2c55..308414e8fea 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -3,12 +3,15 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Linq; using System.Threading.Tasks; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Input.Extensions; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Snippets; +using Microsoft.TypeSpec.Generator.Statements; using Microsoft.TypeSpec.Generator.Tests.Common; using NUnit.Framework; @@ -389,6 +392,51 @@ public async Task BackCompatibility_PositionalPrefixOverloadRequiresAllParameter Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } + // The management generator's ModelFactoryVisitor restores last-contract methods verbatim during + // the visitor pass, which runs before back-compatibility processing. Those overloads keep every + // optional default they shipped with, so they can make a previously valid call ambiguous against + // the current method. Back-compatibility processing must reshape them just like the overloads it + // synthesizes itself. The restored overload is added directly here to model that visitor. + [Test] + public async Task BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Name", InputPrimitiveType.String), + InputFactory.Property("Image", InputPrimitiveType.String), + InputFactory.Property("TargetPort", InputPrimitiveType.String), + InputFactory.Property("IsMain", new InputNullableType(InputPrimitiveType.Boolean)), + InputFactory.Property("Kind", InputPrimitiveType.String), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + + var previous = modelFactory.LastContractView!.Methods[0]; + var restored = new MethodProvider( + new MethodSignature( + previous.Signature.Name, + previous.Signature.Description, + previous.Signature.Modifiers, + previous.Signature.ReturnType, + previous.Signature.ReturnDescription, + previous.Signature.Parameters, + [.. previous.Signature.Attributes, new AttributeStatement(typeof(EditorBrowsableAttribute), Snippet.FrameworkEnumValue(EditorBrowsableState.Never))]), + Snippet.Throw(Snippet.Null), + modelFactory); + modelFactory.Update(methods: [.. modelFactory.Methods, restored]); + + modelFactory.ProcessTypeForBackCompatibility(); + + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + // This test validates that only the previous model factory methods are generated when only the parameter ordering is changed // in the current library version. [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..4b9a5266e90 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,23 @@ +using Sample.Models; + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + // Previously shipped overload with 'kind' earlier in the parameter list, all optional. + public static CompatibilityModel CompatibilityModel( + string id = default, + string name = default, + string kind = default, + string image = default, + string targetPort = default, + bool? isMain = default) + { } + } +} \ No newline at end of file diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix.cs new file mode 100644 index 00000000000..11f6e1a8f0a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix.cs @@ -0,0 +1,30 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string image = default, string targetPort = default, bool? isMain = default, string kind = default) + { + return new global::Sample.Models.CompatibilityModel( + id, + name, + image, + targetPort, + isMain, + kind, + additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, string kind, string image, string targetPort, bool? isMain = default) + { + throw null; + } + } +} From 94550ea748f5998aef5ca4f7f586eea905d462db Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Wed, 19 Aug 2026 12:26:29 -0500 Subject: [PATCH 16/17] test(http-client-csharp): cover multiple previous factory overloads Two last-contract overloads can compete with the same current method and with each other, and each needs its own required prefix. Pin both branches of the analysis: an overload that is a positional prefix of the current method becomes fully required because no argument count distinguishes it, while a reordered overload only requires the prefix up to its first differing parameter type and keeps its trailing optional parameters. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 098ae4f6-52c2-41dc-a7aa-88f561b153e5 --- .../ModelFactoryProviderTests.cs | 28 +++++++++++++++ .../SampleNamespaceModelFactory.cs | 31 ++++++++++++++++ ...iousOverloadsRequireIndependentPrefixes.cs | 35 +++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index 308414e8fea..4ebd724bcaf 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -437,6 +437,34 @@ public async Task BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix() Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } + // Two previous overloads compete with the same current method and with each other, so each + // needs its own prefix. The first is a positional prefix of the current method, so no argument + // count distinguishes them and it must become fully required; the second moved a parameter + // earlier, so a shorter prefix is enough and its trailing parameter stays optional. + [Test] + public async Task BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Name", InputPrimitiveType.String), + InputFactory.Property("Count", new InputNullableType(InputPrimitiveType.Int32)), + InputFactory.Property("Flag", new InputNullableType(InputPrimitiveType.Boolean)), + InputFactory.Property("Kind", InputPrimitiveType.String), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + // This test validates that only the previous model factory methods are generated when only the parameter ordering is changed // in the current library version. [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..0d70a20542e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,31 @@ +using Sample.Models; + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + // Positional prefix of the current method (differs only past 'count'), so no argument + // count distinguishes the two and every parameter must become required. + public static CompatibilityModel CompatibilityModel( + string id = default, + string name = default, + int? count = default, + string kind = default) + { } + + // Reordered: 'kind' moved ahead of 'count', so supplying three arguments already + // disambiguates it and 'count' keeps the optionality it shipped with. + public static CompatibilityModel CompatibilityModel( + string id = default, + string name = default, + string kind = default, + int? count = default) + { } + } +} \ No newline at end of file diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes.cs new file mode 100644 index 00000000000..f27d09c6d7a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes.cs @@ -0,0 +1,35 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, int? count = default, bool? flag = default, string kind = default) + { + return new global::Sample.Models.CompatibilityModel( + id, + name, + count, + flag, + kind, + additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, int? count, string kind) + { + return CompatibilityModel(id: id, name: name, count: count, flag: default, kind: kind); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, string kind, int? count = default) + { + return CompatibilityModel(id: id, name: name, count: count, flag: default, kind: kind); + } + } +} From fd369e53586b181be65db0c447fb90b6d76c8cec Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Thu, 20 Aug 2026 14:41:07 -0500 Subject: [PATCH 17/17] pr feedback --- .../src/Providers/ModelFactoryProvider.cs | 99 +++++----- .../src/Shared/MethodSignatureHelper.cs | 65 ++++--- .../ModelFactoryProviderTests.cs | 174 ++++++++++++++++-- ...y_AbstractReturnTypeOverloadIsGenerated.cs | 4 +- .../SampleNamespaceModelFactory.cs | 22 +++ ...eviousOverloadKeepsPublishedOptionality.cs | 23 +++ .../SampleNamespaceModelFactory.cs | 21 +++ ...eviousOverloadsKeepPublishedOptionality.cs | 29 +++ .../SampleNamespaceModelFactory.cs | 21 +++ ...erloadsKeepPublishedOptionalityReversed.cs | 29 +++ ...iousOverloadsRequireIndependentPrefixes.cs | 6 +- .../SampleNamespaceModelFactory.cs | 19 ++ ...onalPrefixOverloadRequiresAllParameters.cs | 4 +- .../SampleNamespaceModelFactory.cs | 25 +++ ...yOptionalParametersRequireMinimumPrefix.cs | 4 +- ...eredOverloadKeptWhenStillInLastContract.cs | 2 +- ...etersPreserveTrailingOptionalParameters.cs | 4 +- ...sitorAddedOverloadRequiresMinimumPrefix.cs | 4 +- .../test/Shared/MethodSignatureHelperTests.cs | 30 +++ 19 files changed, 481 insertions(+), 104 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_NewOverloadIsConstrainedToPreservePublishedNamedArguments(Last)/SampleNamespaceModelFactory.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PublishedOverloadBoundariesArePreserved(Last)/SampleNamespaceModelFactory.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index a0f38625ee5..6ec325a7e1e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -114,12 +114,14 @@ protected internal sealed override IReadOnlyList BuildMethodsFor var allFactoryMethods = factoryMethods .Concat(customFactoryMethods) .ToList(); - HashSet currentMethodSignatures = allFactoryMethods + List currentMethodSignatures = allFactoryMethods .Select(m => m.Signature) - .ToHashSet(MethodSignature.MethodSignatureComparer); + .ToList(); var compatiblePreviousMethods = new List(); - HashSet previousPublicSignatures = new(MethodSignature.MethodSignatureComparer); + List previousPublicSignatures = []; + List preservedPreviousSignatures = []; + foreach (var previousMethod in LastContractView.Methods) { if (!MethodSignatureHelper.IsPublicApi(previousMethod.Signature.Modifiers)) @@ -131,23 +133,22 @@ protected internal sealed override IReadOnlyList BuildMethodsFor // a current overload that still matches one of them must never be removed. previousPublicSignatures.Add(previousMethod.Signature); - if (currentMethodSignatures.Contains(previousMethod.Signature)) + if (currentMethodSignatures.Any(current => + MethodSignature.MethodSignatureComparer.Equals(current, previousMethod.Signature))) { - // A method with this signature is already present, but it may have been restored - // from the last contract by a library visitor before this pass ran. - var restoredOverload = factoryMethods.FirstOrDefault(m => - MethodSignature.MethodSignatureComparer.Equals(m.Signature, previousMethod.Signature) - && m.Signature.Attributes.Any(a => - a.Type is { IsFrameworkType: true } - && a.Type.FrameworkType == typeof(System.ComponentModel.EditorBrowsableAttribute))); - if (restoredOverload is not null) + preservedPreviousSignatures.Add(previousMethod.Signature); + + // The current model shape may have regenerated a previously published signature + // with different defaults. Restore its published required/optional boundary. + var matchingCurrentMethod = factoryMethods.FirstOrDefault(m => + MethodSignature.MethodSignatureComparer.Equals(m.Signature, previousMethod.Signature)); + if (matchingCurrentMethod is not null) { - MethodSignatureHelper.RequireMinimumParameterPrefix( - restoredOverload.Signature, - GetCurrentOverloadSignatures( - allFactoryMethods, - restoredOverload.Signature.Name, - restoredOverload)); + for (int i = 0; i < previousMethod.Signature.Parameters.Count; i++) + { + matchingCurrentMethod.Signature.Parameters[i].DefaultValue = + previousMethod.Signature.Parameters[i].DefaultValue; + } } continue; @@ -170,6 +171,27 @@ protected internal sealed override IReadOnlyList BuildMethodsFor } compatiblePreviousMethods.Add(previousMethod); + preservedPreviousSignatures.Add(previousMethod.Signature); + } + + // Preserve every published signature as-is and constrain only newly generated overloads. + // Unlike a compatibility signature, a new overload has no existing callers whose minimum + // argument count must be retained. + foreach (var currentMethod in factoryMethods) + { + if (previousPublicSignatures.Any(previous => + MethodSignature.MethodSignatureComparer.Equals(previous, currentMethod.Signature))) + { + continue; + } + + var previousOverloads = preservedPreviousSignatures + .Where(signature => signature.Name == currentMethod.Signature.Name) + .ToList(); + MethodSignatureHelper.RequireMinimumParameterPrefix( + currentMethod.Signature, + previousOverloads, + preservePublishedMinimumArgumentCount: false); } foreach (var previousMethod in compatiblePreviousMethods) @@ -197,27 +219,29 @@ protected internal sealed override IReadOnlyList BuildMethodsFor continue; } - var compatibilityOverloadSignatures = GetCompatibilityOverloadSignatures( - currentOverloadSignatures, - compatiblePreviousMethods, - previousMethod); + // Generated overloads were constrained above, so only immutable custom overloads can + // still force a compatibility signature to change its published optionality. + var compatibilityOverloadSignatures = customFactoryMethods + .Select(method => method.Signature) + .Where(signature => + signature.Name == previousMethod.Signature.Name + && !previousPublicSignatures.Any(previous => + MethodSignature.MethodSignatureComparer.Equals(previous, signature))) + .ToList(); foreach (var currentOverload in currentOverloads) { // If the parameter ordering is the only difference, just use the previous method. if (MethodSignatureHelper.ContainsSameParameters(previousMethod.Signature, currentOverload) - && !previousPublicSignatures.Contains(currentOverload)) + && !previousPublicSignatures.Any(previous => + MethodSignature.MethodSignatureComparer.Equals(previous, currentOverload))) { var factoryMethodToRemove = factoryMethods .FirstOrDefault(m => MethodSignature.MethodSignatureComparer.Equals(m.Signature, currentOverload)); - var coexistingOverloads = GetCurrentOverloadSignatures( + var coexistingCompatibilityOverloads = GetCurrentOverloadSignatures( allFactoryMethods, previousMethod.Signature.Name, factoryMethodToRemove); - var coexistingCompatibilityOverloads = GetCompatibilityOverloadSignatures( - coexistingOverloads, - compatiblePreviousMethods, - previousMethod); if (TryBuildCompatibleMethodForPreviousContract( previousMethod, currentOverload, @@ -281,28 +305,9 @@ protected internal sealed override IReadOnlyList BuildMethodsFor BackCompatibilityChangeCategory.ModelFactoryMethodSkipped); } } - return [.. factoryMethods]; } - private static IReadOnlyList GetCompatibilityOverloadSignatures( - IReadOnlyList currentOverloadSignatures, - IReadOnlyList compatiblePreviousMethods, - MethodProvider previousMethod) - { - HashSet overloadSignatures = new(MethodSignature.MethodSignatureComparer); - foreach (var compatiblePreviousMethod in compatiblePreviousMethods) - { - if (!ReferenceEquals(compatiblePreviousMethod, previousMethod)) - { - overloadSignatures.Add(compatiblePreviousMethod.Signature); - } - } - - overloadSignatures.UnionWith(currentOverloadSignatures); - return [.. overloadSignatures]; - } - private IReadOnlyList GetCurrentOverloadSignatures( IEnumerable methods, string methodName, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index e5516bcadba..6f2e694eac1 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -89,11 +89,15 @@ internal static MethodSignature BuildBackCompatMethodSignature( /// internal static void RequireMinimumParameterPrefix( MethodSignature signature, - IReadOnlyList? currentMethodSignatures = null) + IReadOnlyList? currentMethodSignatures = null, + bool preservePublishedMinimumArgumentCount = true) { int requiredParameterCount = currentMethodSignatures is null ? signature.Parameters.Count - : GetMinimumRequiredParameterCount(signature, currentMethodSignatures); + : GetMinimumRequiredParameterCount( + signature, + currentMethodSignatures, + preservePublishedMinimumArgumentCount); for (int i = 0; i < requiredParameterCount; i++) { @@ -124,17 +128,21 @@ private static MethodSignature CreateBackCompatSignature( } private static int GetMinimumRequiredParameterCount( - MethodSignature previousMethodSignature, - IReadOnlyList currentMethodSignatures) + MethodSignature targetMethodSignature, + IReadOnlyList competingMethodSignatures, + bool preservePublishedMinimumArgumentCount) { int requiredParameterCount = 0; - foreach (var currentMethodSignature in currentMethodSignatures) + foreach (var competingMethodSignature in competingMethodSignatures) { - if (currentMethodSignature.Name == previousMethodSignature.Name) + if (competingMethodSignature.Name == targetMethodSignature.Name) { requiredParameterCount = Math.Max( requiredParameterCount, - GetMinimumRequiredParameterCount(previousMethodSignature, currentMethodSignature)); + GetMinimumRequiredParameterCount( + targetMethodSignature, + competingMethodSignature, + preservePublishedMinimumArgumentCount)); } } @@ -142,45 +150,54 @@ private static int GetMinimumRequiredParameterCount( } private static int GetMinimumRequiredParameterCount( - MethodSignature previousMethodSignature, - MethodSignature currentMethodSignature) + MethodSignature targetMethodSignature, + MethodSignature competingMethodSignature, + bool preservePublishedMinimumArgumentCount) { - if (currentMethodSignature.Parameters.Any(p => p.IsRef || p.IsOut)) + if (competingMethodSignature.Parameters.Any(p => p.IsRef || p.IsOut)) { return 0; } - int previousMinimumArgumentCount = GetMinimumArgumentCount(previousMethodSignature); - int currentMinimumArgumentCount = GetMinimumArgumentCount(currentMethodSignature); - int currentMaximumArgumentCount = currentMethodSignature.Parameters.Any(p => p.IsParams) + int targetMinimumArgumentCount = GetMinimumArgumentCount(targetMethodSignature); + int competingMinimumArgumentCount = GetMinimumArgumentCount(competingMethodSignature); + int competingMaximumArgumentCount = competingMethodSignature.Parameters.Any(p => p.IsParams) ? int.MaxValue - : currentMethodSignature.Parameters.Count; + : competingMethodSignature.Parameters.Count; + + // No argument count can reach both overloads, so the target needs no additional + // required parameters. + if (Math.Max(targetMinimumArgumentCount, competingMinimumArgumentCount) > + Math.Min(targetMethodSignature.Parameters.Count, competingMaximumArgumentCount)) + { + return 0; + } - // No argument count can reach both overloads, so the previous signature is already - // unambiguous and keeps the optionality it was published with. - if (Math.Max(previousMinimumArgumentCount, currentMinimumArgumentCount) > - Math.Min(previousMethodSignature.Parameters.Count, currentMaximumArgumentCount)) + // When the target is a published signature, do not raise its minimum argument count to + // address overlap with a competitor that cannot apply to its shorter calls. + if (preservePublishedMinimumArgumentCount && + competingMinimumArgumentCount > targetMinimumArgumentCount) { return 0; } // Require only the prefix up to and including the first position whose parameter type - // differs. Any call supplying that many arguments can no longer bind to the current + // differs. Any call supplying that many arguments can no longer bind to the competing // overload, so every trailing parameter keeps the optionality it had previously. int overlappingParameterCount = Math.Min( - previousMethodSignature.Parameters.Count, - currentMethodSignature.Parameters.Count); + targetMethodSignature.Parameters.Count, + competingMethodSignature.Parameters.Count); for (int i = 0; i < overlappingParameterCount; i++) { - if (!previousMethodSignature.Parameters[i].Type.AreNamesEqual(currentMethodSignature.Parameters[i].Type)) + if (!targetMethodSignature.Parameters[i].Type.AreNamesEqual(competingMethodSignature.Parameters[i].Type)) { - return Math.Max(i + 1, previousMinimumArgumentCount); + return Math.Max(i + 1, targetMinimumArgumentCount); } } // The shorter signature is a positional prefix of the other, so no argument count // distinguishes them. Fall back to requiring every parameter. - return previousMethodSignature.Parameters.Count; + return targetMethodSignature.Parameters.Count; } private static int GetMinimumArgumentCount(MethodSignature methodSignature) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index 4ebd724bcaf..c53235d8bec 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -201,7 +201,7 @@ public async Task BackCompatibility_NewModelPropertyAdded() Assert.AreEqual("listProp", parameters[2].Name); foreach (var param in parameters) { - Assert.IsNull(param.DefaultValue); + Assert.IsNotNull(param.DefaultValue); } var currentParameters = currentOverloadMethod!.Signature.Parameters; @@ -212,7 +212,7 @@ public async Task BackCompatibility_NewModelPropertyAdded() Assert.AreEqual("dictProp", currentParameters[3].Name); foreach (var param in currentParameters) { - Assert.IsNotNull(param.DefaultValue); + Assert.IsNull(param.DefaultValue); } Assert.IsTrue(parameters[0].Type.AreNamesEqual(currentParameters[0].Type)); @@ -339,11 +339,8 @@ public async Task BackCompatibility_CustomOverloadsPreserveTrailingOptionalParam Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } - // Mirrors the reported Azure.ResourceManager.AppService regression: the last contract - // contains both the overload matching today's natural parameter order and a previously - // shipped overload with the discriminating parameter moved to the end. The first overload - // must be kept (removing it would be breaking) and the second must only require the - // parameter prefix needed to disambiguate it, preserving its trailing optional parameters. + // Mirrors the reported Azure.ResourceManager.AppService regression: both overloads already + // existed in the last contract, so each must retain its published optionality. [Test] public async Task BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract() { @@ -367,9 +364,8 @@ public async Task BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } - // A previous signature that is a positional prefix of the current overload cannot be - // disambiguated by argument count, so every parameter must become required. C# then prefers - // the compatibility overload for an exact argument list because it needs no optional defaults. + // A previous signature that is a positional prefix of a new current overload cannot be + // disambiguated by argument count, so every parameter on the new overload must be required. [Test] public async Task BackCompatibility_PositionalPrefixOverloadRequiresAllParameters() { @@ -393,10 +389,8 @@ public async Task BackCompatibility_PositionalPrefixOverloadRequiresAllParameter } // The management generator's ModelFactoryVisitor restores last-contract methods verbatim during - // the visitor pass, which runs before back-compatibility processing. Those overloads keep every - // optional default they shipped with, so they can make a previously valid call ambiguous against - // the current method. Back-compatibility processing must reshape them just like the overloads it - // synthesizes itself. The restored overload is added directly here to model that visitor. + // the visitor pass, which runs before back-compatibility processing. The restored overload must + // keep its published defaults while the new generated overload acquires the required prefix. [Test] public async Task BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix() { @@ -437,10 +431,8 @@ public async Task BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix() Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } - // Two previous overloads compete with the same current method and with each other, so each - // needs its own prefix. The first is a positional prefix of the current method, so no argument - // count distinguishes them and it must become fully required; the second moved a parameter - // earlier, so a shorter prefix is enough and its trailing parameter stays optional. + // Two previous overloads compete with the same new current method. Both retain their published + // defaults while the new overload acquires the longest prefix needed to avoid both. [Test] public async Task BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes() { @@ -465,6 +457,148 @@ public async Task BackCompatibility_MultiplePreviousOverloadsRequireIndependentP Assert.AreEqual(Helpers.GetExpectedFromFile(), content); } + // Overloads that shipped together in the published contract already coexisted there, so they + // are not new competitors for one another; only a surviving current or custom overload can + // introduce ambiguity that was not already present. Here 'id, count' is a positional prefix + // of the wider overload, so treating them as competitors would make the wider one fully + // required and break the previously valid call 'CompatibilityModel("i", 1, "e")'. + [Test] + public async Task BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Description", InputPrimitiveType.String), + InputFactory.Property("Name", InputPrimitiveType.String), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + // Same last-contract overloads as above but declared in the opposite order. Signatures are + // mutated in place, so this pins that the result does not depend on declaration order. + [Test] + public async Task BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Description", InputPrimitiveType.String), + InputFactory.Property("Name", InputPrimitiveType.String), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + // An all-required previous overload is only callable at exactly its own argument count, so it + // cannot be reached by shorter calls to a coexisting all-optional overload. Promoting the + // all-optional overload against it would raise its published minimum argument count and break + // previously valid low-arity calls, so the published optionality must be preserved. This + // mirrors the shipped Azure.ResourceManager.AppService 'SiteConfigProperties' shape. + [Test] + public async Task BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Name", InputPrimitiveType.String), + InputFactory.Property("Count", new InputNullableType(InputPrimitiveType.Int32)), + InputFactory.Property("Kind", InputPrimitiveType.String), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + // A current generated overload can have the same signature as a previously published overload + // while acquiring different defaults from the current model shape. Preserve the published + // required boundary on that overload and the published optionality on its reordered companion + // rather than swapping their callability. + [Test] + public async Task BackCompatibility_PublishedOverloadBoundariesArePreserved() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Allow", new InputNullableType(InputPrimitiveType.Boolean)), + InputFactory.Property("Kind", InputPrimitiveType.String), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var currentOverload = modelFactory.Methods.Single(m => + m.Signature.Name == "CompatibilityModel" + && m.Signature.Parameters[1].Name == "allow"); + var compatibilityOverload = modelFactory.Methods.Single(m => + m.Signature.Name == "CompatibilityModel" + && m.Signature.Parameters[1].Name == "kind"); + + Assert.IsTrue(currentOverload.Signature.Parameters.All(p => p.DefaultValue is null)); + Assert.IsTrue(compatibilityOverload.Signature.Parameters.All(p => p.DefaultValue is not null)); + } + + // The newly generated overload did not exist in the previous contract, so it can acquire the + // required prefix needed for disambiguation. The previous overload must remain fully optional + // so calls using its unique parameter names continue to compile. + [Test] + public async Task BackCompatibility_NewOverloadIsConstrainedToPreservePublishedNamedArguments() + { + InputModelType model = InputFactory.Model("CompatibilityModel", properties: + [ + InputFactory.Property("Id", InputPrimitiveType.String), + InputFactory.Property("Properties", new InputNullableType(InputPrimitiveType.Int32)), + ]); + + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: [model], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last"))).Object; + + var modelFactory = _instance.OutputLibrary.ModelFactory.Value; + modelFactory.ProcessTypeForBackCompatibility(); + + var currentOverload = modelFactory.Methods.Single(m => + m.Signature.Name == "CompatibilityModel" + && m.Signature.Parameters.Count == 2); + var compatibilityOverload = modelFactory.Methods.Single(m => + m.Signature.Name == "CompatibilityModel" + && m.Signature.Parameters.Count == 3); + + Assert.IsTrue(currentOverload.Signature.Parameters.All(p => p.DefaultValue is null)); + Assert.IsTrue(compatibilityOverload.Signature.Parameters.All(p => p.DefaultValue is not null)); + } + // This test validates that only the previous model factory methods are generated when only the parameter ordering is changed // in the current library version. [Test] @@ -860,9 +994,11 @@ public async Task BackCompatibility_NewPropertyAddedWithRenamedParam() Assert.AreEqual("listProp", parameters[2].Name); foreach (var param in parameters) { - Assert.IsNull(param.DefaultValue); + Assert.IsNotNull(param.DefaultValue); } + Assert.IsTrue(currentParameters.All(p => p.DefaultValue is null)); + // The backcompat overload's body instantiates the model directly because the previous // parameter names (oldStringProp, oldModelProp) do not match any current property name. // For unmatched parameters the generator falls back to passing `default` to the diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AbstractReturnTypeOverloadIsGenerated.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AbstractReturnTypeOverloadIsGenerated.cs index 8c46eba00bb..be14011abf7 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AbstractReturnTypeOverloadIsGenerated.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AbstractReturnTypeOverloadIsGenerated.cs @@ -9,7 +9,7 @@ namespace Sample.Namespace { public static partial class SampleNamespaceModelFactory { - public static global::Sample.Models.AbstractModel AbstractModel(string kind = default, string prop1 = default, string prop2 = default) + public static global::Sample.Models.AbstractModel AbstractModel(string kind, string prop1, string prop2) { return new global::Sample.Models.UnknownAbstractModel(kind, prop1, prop2, additionalBinaryDataProperties: null); } @@ -20,7 +20,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.AbstractModel AbstractModel(string prop1, string kind) + public static global::Sample.Models.AbstractModel AbstractModel(string prop1 = default, string kind = default) { return AbstractModel(kind: kind, prop1: prop1, prop2: default); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..69433192e28 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,22 @@ +using Sample.Models; + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + // An all-required overload. It is only callable with exactly four arguments, so it can never + // be reached by a shorter call to the all-optional overload below. + public static CompatibilityModel CompatibilityModel(string id, string name, bool? flag, string kind) + { } + + // The all-optional overload. Its published minimum is zero arguments and must stay that way. + public static CompatibilityModel CompatibilityModel(string id = default, string name = default, int? count = default, string kind = default) + { } + } +} \ No newline at end of file diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality.cs new file mode 100644 index 00000000000..4b3ffa11f1c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AllRequiredPreviousOverloadKeepsPublishedOptionality.cs @@ -0,0 +1,23 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, int? count = default, string kind = default) + { + return new global::Sample.Models.CompatibilityModel(id, name, count, kind, additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? flag, string kind) + { + return new global::Sample.Models.CompatibilityModel(id, name, default, kind, additionalBinaryDataProperties: null); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..f50561547c2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,21 @@ +using Sample.Models; + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + // A compatibility shim published by an earlier version. + public static CompatibilityModel CompatibilityModel(string id, int? count = default) + { } + // The published overload. 'id, count' is a positional prefix of it, so the two only + // coexisted safely because both shipped with their trailing parameters optional. + public static CompatibilityModel CompatibilityModel(string id, int? count = default, string extra = default, string other = default) + { } + } +} \ No newline at end of file diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality.cs new file mode 100644 index 00000000000..b2b8eb3d04f --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionality.cs @@ -0,0 +1,29 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string description, string name = default) + { + return new global::Sample.Models.CompatibilityModel(id, description, name, additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, int? count = default) + { + return new global::Sample.Models.CompatibilityModel(id, default, default, additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, int? count = default, string extra = default, string other = default) + { + return new global::Sample.Models.CompatibilityModel(id, default, default, additionalBinaryDataProperties: null); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..4db67ddeab5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,21 @@ +using Sample.Models; + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + // The published overload. 'id, count' is a positional prefix of it, so the two only + // coexisted safely because both shipped with their trailing parameters optional. + public static CompatibilityModel CompatibilityModel(string id, int? count = default, string extra = default, string other = default) + { } + // A compatibility shim published by an earlier version. + public static CompatibilityModel CompatibilityModel(string id, int? count = default) + { } + } +} \ No newline at end of file diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed.cs new file mode 100644 index 00000000000..6e688e2d9fc --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsKeepPublishedOptionalityReversed.cs @@ -0,0 +1,29 @@ +// + +#nullable disable + +using System.ComponentModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string description, string name = default) + { + return new global::Sample.Models.CompatibilityModel(id, description, name, additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, int? count = default, string extra = default, string other = default) + { + return new global::Sample.Models.CompatibilityModel(id, default, default, additionalBinaryDataProperties: null); + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, int? count = default) + { + return new global::Sample.Models.CompatibilityModel(id, default, default, additionalBinaryDataProperties: null); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes.cs index f27d09c6d7a..ef249a93cfc 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireIndependentPrefixes.cs @@ -9,7 +9,7 @@ namespace Sample.Namespace { public static partial class SampleNamespaceModelFactory { - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, int? count = default, bool? flag = default, string kind = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, int? count, bool? flag, string kind = default) { return new global::Sample.Models.CompatibilityModel( id, @@ -21,13 +21,13 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, int? count, string kind) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, int? count = default, string kind = default) { return CompatibilityModel(id: id, name: name, count: count, flag: default, kind: kind); } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, string kind, int? count = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string kind = default, int? count = default) { return CompatibilityModel(id: id, name: name, count: count, flag: default, kind: kind); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_NewOverloadIsConstrainedToPreservePublishedNamedArguments(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_NewOverloadIsConstrainedToPreservePublishedNamedArguments(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..5bb87f8129a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_NewOverloadIsConstrainedToPreservePublishedNamedArguments(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,19 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id = default, + string unit = default, + int? properties = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs index 2bc28c06a72..8ebf6150303 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs @@ -9,13 +9,13 @@ namespace Sample.Namespace { public static partial class SampleNamespaceModelFactory { - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, int? count = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, int? count) { return new global::Sample.Models.CompatibilityModel(id, name, count, additionalBinaryDataProperties: null); } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default) { return CompatibilityModel(id: id, name: name, count: default); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PublishedOverloadBoundariesArePreserved(Last)/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PublishedOverloadBoundariesArePreserved(Last)/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..a3e9ac74f23 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PublishedOverloadBoundariesArePreserved(Last)/SampleNamespaceModelFactory.cs @@ -0,0 +1,25 @@ +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static CompatibilityModel CompatibilityModel( + string id, + bool? allow, + string kind) + { } + + public static CompatibilityModel CompatibilityModel( + string id = default, + string kind = default, + bool? allow = default) + { } + } +} + +namespace Sample.Models +{ + public partial class CompatibilityModel + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs index 6ab6f396661..ca3a3c10eb9 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireMinimumPrefix.cs @@ -9,7 +9,7 @@ namespace Sample.Namespace { public static partial class SampleNamespaceModelFactory { - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string kind = default, bool? enabled = default, string description = default, int? count = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, string kind, bool? enabled = default, string description = default, int? count = default) { return new global::Sample.Models.CompatibilityModel( id, @@ -22,7 +22,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, bool? enabled = default, string description = default) { return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs index ec2102cb147..c636faa9f68 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs @@ -15,7 +15,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string image, bool? isMain, string kind = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string image = default, bool? isMain = default, string kind = default) { return new global::Sample.Models.CompatibilityModel(id, kind, image, isMain, additionalBinaryDataProperties: null); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs index 6ab6f396661..aa880309e32 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedRequiredParametersPreserveTrailingOptionalParameters.cs @@ -9,7 +9,7 @@ namespace Sample.Namespace { public static partial class SampleNamespaceModelFactory { - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string kind = default, bool? enabled = default, string description = default, int? count = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, string kind, bool? enabled = default, string description = default, int? count = default) { return new global::Sample.Models.CompatibilityModel( id, @@ -22,7 +22,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled, string description = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, bool? enabled = default, string description = default) { return CompatibilityModel(id: id, name: name, kind: default, enabled: enabled, description: description, count: default); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix.cs index 11f6e1a8f0a..32bf918ce17 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_VisitorAddedOverloadRequiresMinimumPrefix.cs @@ -9,7 +9,7 @@ namespace Sample.Namespace { public static partial class SampleNamespaceModelFactory { - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string image = default, string targetPort = default, bool? isMain = default, string kind = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, string image, string targetPort, bool? isMain, string kind = default) { return new global::Sample.Models.CompatibilityModel( id, @@ -22,7 +22,7 @@ public static partial class SampleNamespaceModelFactory } [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] - public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id, string name, string kind, string image, string targetPort, bool? isMain = default) + public static global::Sample.Models.CompatibilityModel CompatibilityModel(string id = default, string name = default, string kind = default, string image = default, string targetPort = default, bool? isMain = default) { throw null; } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs index efbdd31468d..5614331a7a5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Shared/MethodSignatureHelperTests.cs @@ -409,6 +409,36 @@ public void BuildBackCompatMethodSignature_RequiredFactoryParametersPreserveTrai Assert.IsNotNull(backCompatSignature.Parameters[3].DefaultValue); } + // A competitor that cannot be called with as few arguments as the previous signature only + // overlaps it at higher argument counts. Promotion raises the previous signature's minimum + // callable argument count, so it would break previously valid calls that were never + // ambiguous. Keep the published optionality instead. + [Test] + public void BuildBackCompatMethodSignature_AllRequiredCompetitorPreservesPublishedOptionality() + { + var previousSignature = CreateMethodSignature("CompatibilityModel", + new ParameterProvider("id", $"", typeof(string), defaultValue: Default), + new ParameterProvider("name", $"", typeof(string), defaultValue: Default), + new ParameterProvider("count", $"", typeof(int?), defaultValue: Default), + new ParameterProvider("kind", $"", typeof(string), defaultValue: Default)); + // Every parameter is required, so this overload is only callable with exactly three + // arguments and can never be reached by a shorter call. + var currentSignature = CreateMethodSignature("CompatibilityModel", + new ParameterProvider("id", $"", typeof(string)), + new ParameterProvider("flag", $"", typeof(bool?)), + new ParameterProvider("count", $"", typeof(int?))); + + var backCompatSignature = MethodSignatureHelper.BuildBackCompatMethodSignature( + previousSignature, + hideMethod: true, + currentMethodSignatures: [currentSignature]); + + foreach (var parameter in backCompatSignature.Parameters) + { + Assert.IsNotNull(parameter.DefaultValue); + } + } + [Test] public void BuildBackCompatMethodSignature_WithOverloads_HideMethodFalse_RemovesDefaultsWithoutEditorBrowsable() {