diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs index 33ad107cb95..f0717c007ff 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs @@ -167,7 +167,7 @@ public static void RestorePreviousParameterNames( for (int i = 0; i < currentParameters.Count; i++) { var parameter = currentParameters[i]; - string? preservedName; + string? preservedName = null; var inputParameter = parameter.InputParameter; if (inputParameter is not null) @@ -178,26 +178,22 @@ public static void RestorePreviousParameterNames( } var originalName = inputParameter.OriginalName; - if (string.IsNullOrEmpty(originalName)) + if (!string.IsNullOrEmpty(originalName)) { - continue; + preservedName = FindPreviousParameterName(lastContractView, originalName, method.Signature.Name); } - - preservedName = FindPreviousParameterName(lastContractView, originalName, method.Signature.Name); } - else + + // Fall back to a positional match for synthesized parameters + if (string.IsNullOrEmpty(preservedName)) { - // Positional fallback for synthesized parameters (e.g. model factory methods). if (!matchingPreviousResolved) { matchingPrevious = FindMethodWithSameSignatureIgnoringNames(previousMethods, method.Signature); matchingPreviousResolved = true; } - var previousParameters = matchingPrevious?.Signature.Parameters; - preservedName = previousParameters is not null && previousParameters.Count == currentParameters.Count - ? previousParameters[i].Name - : null; + preservedName = matchingPrevious?.Signature.Parameters[i].Name; } // A casing-only difference is still a source-breaking rename for named arguments, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs index afe96ffaaf7..cb01fdee067 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs @@ -1,10 +1,11 @@ namespace Test { /// - /// Previously-published contract that does not contain the "brandNewParam" parameter. + /// Previously-published contract whose Foo overload has a different signature (int, not string), + /// so the current Foo(string brandNewParam) has no last-contract method to match positionally. /// public class TestClient { - public string Foo(string oldParam) { return null; } + public string Foo(int oldParam) { return null; } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresRenamedParameterBySignatureMatch.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresRenamedParameterBySignatureMatch.cs new file mode 100644 index 00000000000..c611ba3d45d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresRenamedParameterBySignatureMatch.cs @@ -0,0 +1,19 @@ +// + +#nullable disable + +using Sample; + +namespace Test +{ + public partial class TestClient + { + public string Foo(string oldParam) + { + global::Sample.Argument.AssertNotNullOrEmpty(oldParam, nameof(oldParam)); + + this.Validate(oldParam); + return oldParam; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresRenamedParameterBySignatureMatch/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresRenamedParameterBySignatureMatch/TestClient.cs new file mode 100644 index 00000000000..9d1f30eca74 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresRenamedParameterBySignatureMatch/TestClient.cs @@ -0,0 +1,11 @@ +namespace Test +{ + /// + /// Previously-published contract whose Foo parameter is named "oldParam"; the current generator + /// emits the same signature but names the parameter "newParam". + /// + public class TestClient + { + public string Foo(string oldParam) { return null; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index 2843dc6d709..4143130ff2a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs @@ -675,13 +675,14 @@ public async Task BuildMethodsForBackCompatibilityRestoresPreviousParameterName( Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); } - // Validates that a parameter whose spec name is not in the last contract keeps its current name. + // Validates that a parameter is not positionally restored when no last-contract method matches + // the current method's signature (here the last-contract Foo takes an int, the current takes a string). [Test] public async Task BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName() { await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - // "brandNewParam" has no counterpart in the last contract (TestClient.Foo(oldParam)). + // "brandNewParam" has no counterpart in the last contract (TestClient.Foo(int oldParam)). var inputParameter = InputFactory.QueryParameter("brandNewParam", InputPrimitiveType.String, isRequired: true); var parameter = new ParameterProvider(inputParameter); var fooMethod = new MethodProvider( @@ -697,6 +698,36 @@ public async Task BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName( Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); } + // Validates the positional fallback: when a spec parameter's previously-published name matches + // neither its current name nor its spec original name (e.g. a rename by a different generator), + // it is restored from the last-contract method that matches by signature (name and parameter types). + [Test] + public async Task BuildMethodsForBackCompatibilityRestoresRenamedParameterBySignatureMatch() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // The spec name equals the current name ("newParam"), so it is not discoverable by spec name; + // only a signature match against the last-contract Foo(string oldParam) can restore "oldParam". + var inputParameter = InputFactory.QueryParameter("newParam", InputPrimitiveType.String, isRequired: true); + + var parameter = new ParameterProvider(inputParameter); + var fooMethod = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [parameter]), + new MethodBodyStatement[] + { + Snippet.This.Invoke("Validate", parameter).Terminate(), + Snippet.Return(parameter), + }, + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "TestClient", methods: [fooMethod]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + // When the restored name is used both as an argument (AsArgument -> _asArgument) and // as a variable (-> _asVariable), materializing both cached expressions before the rename, the // rename must keep them sharing one declaration. Otherwise the writer renames the two