From 81e78823f68f03f84ed8b60c5045d2d1eb5e6e44 Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Mon, 15 Jun 2026 12:27:39 -0500 Subject: [PATCH] feat: generalize back compat --- .../src/Providers/ClientProvider.cs | 163 +++-------------- .../src/Providers/TypeProvider.cs | 114 +++++++++++- .../src/Utilities/BackCompatHelper.cs | 173 ++++++++++++++++++ ...patibilityKeepsUnpublishedParameterName.cs | 18 ++ .../TestClient.cs | 10 + ...patibilityRestoresPreviousParameterName.cs | 19 ++ .../TestClient.cs | 10 + ...viderInheritsParameterReorderBackCompat.cs | 14 ++ .../CustomReorderType.cs | 7 + ...oviderReorderPreservesValueTypeDefaults.cs | 14 ++ .../CustomReorderValueTypeDefaultsType.cs | 13 ++ .../TestClient.cs | 15 ++ .../test/Providers/TypeProviderTests.cs | 148 +++++++++++++++ .../backward-compatibility-extensibility.md | 170 +++++++++++++++++ 14 files changed, 749 insertions(+), 139 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName/TestClient.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat/CustomReorderType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults/CustomReorderValueTypeDefaultsType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/FindPreviousParameterNameLooksUpLastContract/TestClient.cs create mode 100644 packages/http-client-csharp/generator/docs/backward-compatibility-extensibility.md diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index 75cd23026da..0b8e6d6888e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1261,62 +1261,51 @@ protected override ScmMethodProvider[] BuildMethods() protected sealed override IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) { - List materializedMethods = [.. originalMethods]; - if (LastContractView?.Methods == null || LastContractView.Methods.Count == 0) { - return materializedMethods; + return [.. originalMethods]; } - var currentMethodSignatures = BuildCurrentMethodSignatures(materializedMethods); - - ProcessBackCompatForParameterReordering(materializedMethods, currentMethodSignatures); - ProcessBackCompatForNewOptionalParameters(materializedMethods, currentMethodSignatures); + // Snapshot the current signatures before the base reorders any of them in place, so we + // can fix up convenience method bodies that call reordered protocol methods afterwards. + var originalSignatures = new Dictionary(ReferenceEqualityComparer.Instance); + foreach (var method in originalMethods) + { + originalSignatures.TryAdd(method, method.Signature); + } - return materializedMethods; - } + List result = [.. base.BuildMethodsForBackCompatibility(originalMethods)]; - private void ProcessBackCompatForParameterReordering( - IList materializedMethods, - Dictionary currentMethodSignatures) - { + // Determine which existing methods had their parameters reordered by the base and fix up + // convenience method bodies that call the reordered protocol methods. var updatedSignatureToOriginal = new Dictionary(MethodSignature.MethodSignatureComparer); var methodsWithReorderedParams = new List(); - - foreach (var previousMethod in LastContractView!.Methods) + foreach (var (method, originalSignature) in originalSignatures) { - if (!ShouldProcessMethodForBackCompat(previousMethod.Signature, currentMethodSignatures)) + if (method.Signature.Name.Equals(originalSignature.Name) + && !MethodSignatureHelper.HaveSameParametersInSameOrder(method.Signature, originalSignature)) { - continue; - } - - var methodToUpdate = FindMethodWithSameParametersButDifferentOrder( - previousMethod.Signature, - currentMethodSignatures); - - if (methodToUpdate != null && TryReorderCurrentMethodParameters( - methodToUpdate, - previousMethod.Signature, - updatedSignatureToOriginal)) - { - methodsWithReorderedParams.Add(methodToUpdate); - CodeModelGenerator.Instance.Emitter.Debug( - $"Reordered parameters of '{Name}.{methodToUpdate.Signature.Name}' to match last contract.", - BackCompatibilityChangeCategory.MethodParameterReordering); + updatedSignatureToOriginal.TryAdd(method.Signature, originalSignature); + methodsWithReorderedParams.Add(method); } } if (methodsWithReorderedParams.Count > 0) { - UpdateConvenienceMethodsForBackCompat(materializedMethods, methodsWithReorderedParams, updatedSignatureToOriginal); + UpdateConvenienceMethodsForBackCompat(result, methodsWithReorderedParams, updatedSignatureToOriginal); } + + // Add hidden overloads for methods that gained new optional non-body parameters. + ProcessBackCompatForNewOptionalParameters(result, BuildCurrentMethodSignatureMap(result)); + + return result; } - private Dictionary BuildCurrentMethodSignatures(IEnumerable originalMethods) + private Dictionary BuildCurrentMethodSignatureMap(IEnumerable methods) { var allMethods = CustomCodeView?.Methods != null - ? originalMethods.Concat(CustomCodeView.Methods) - : originalMethods; + ? methods.Concat(CustomCodeView.Methods) + : methods; var result = new Dictionary(MethodSignature.MethodSignatureComparer); foreach (var method in allMethods) @@ -1326,86 +1315,6 @@ private Dictionary BuildCurrentMethodSignatures return result; } - private static bool ShouldProcessMethodForBackCompat( - MethodSignature previousSignature, - Dictionary currentMethodSignatures) - { - if (currentMethodSignatures.ContainsKey(previousSignature)) - { - return false; - } - - var modifiers = previousSignature.Modifiers; - return modifiers.HasFlag(MethodSignatureModifiers.Public) || - modifiers.HasFlag(MethodSignatureModifiers.Protected); - } - - private static MethodProvider? FindMethodWithSameParametersButDifferentOrder( - MethodSignature previousSignature, - Dictionary currentMethodSignatures) - { - foreach (var kvp in currentMethodSignatures) - { - var currentSignature = kvp.Key; - if (currentSignature.Name.Equals(previousSignature.Name) - && currentSignature.ReturnType?.AreNamesEqual(previousSignature.ReturnType) == true - && MethodSignatureHelper.ContainsSameParameters(previousSignature, currentSignature)) - { - return kvp.Value; - } - } - - return null; - } - - private bool TryReorderCurrentMethodParameters( - MethodProvider methodToUpdate, - MethodSignature previousSignature, - Dictionary updatedSignatureToOriginal) - { - var currentSignature = methodToUpdate.Signature; - // Early exit: Check if parameters are already in the same order - if (MethodSignatureHelper.HaveSameParametersInSameOrder(currentSignature, previousSignature)) - { - return false; - } - - var parametersByName = currentSignature.Parameters.ToDictionary(p => p.Name.ToVariableName()); - var reorderedParameters = new List(currentSignature.Parameters.Count); - - foreach (var previousParam in previousSignature.Parameters) - { - if (parametersByName.TryGetValue(previousParam.Name, out var matchingParam)) - { - reorderedParameters.Add(matchingParam); - } - } - - if (reorderedParameters.Count != currentSignature.Parameters.Count) - { - return false; - } - - var updatedSignature = new MethodSignature( - currentSignature.Name, - currentSignature.Description, - currentSignature.Modifiers, - currentSignature.ReturnType, - currentSignature.ReturnDescription, - reorderedParameters, - currentSignature.Attributes, - currentSignature.GenericArguments, - currentSignature.GenericParameterConstraints, - currentSignature.ExplicitInterface, - currentSignature.NonDocumentComment); - updatedSignatureToOriginal.TryAdd(updatedSignature, currentSignature); - - UpdateXmlDocProviderForParamReorder(methodToUpdate.XmlDocs, updatedSignature); - methodToUpdate.Update(signature: updatedSignature, xmlDocProvider: methodToUpdate.XmlDocs); - - return true; - } - private ParameterProvider BuildClientEndpointParameter() { _inputEndpointParam = _inputClient.Parameters @@ -1743,28 +1652,6 @@ private static void ReorderMethodInvocationArguments( } } - private static void UpdateXmlDocProviderForParamReorder( - XmlDocProvider xmlDocs, - MethodSignature updatedSignature) - { - var paramDocsByName = xmlDocs.Parameters.ToDictionary(s => s.Parameter.Name); - var reorderedParamDocs = new List(updatedSignature.Parameters.Count); - - foreach (var param in updatedSignature.Parameters) - { - if (paramDocsByName.TryGetValue(param.Name, out var paramDoc)) - { - reorderedParamDocs.Add(paramDoc); - } - } - - if (reorderedParamDocs.Count == xmlDocs.Parameters.Count && - !reorderedParamDocs.SequenceEqual(xmlDocs.Parameters)) - { - xmlDocs.Update(parameters: reorderedParamDocs); - } - } - private void ProcessBackCompatForNewOptionalParameters( List methods, Dictionary currentMethodSignatures) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs index 7a3047886b6..5383f14abbc 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs @@ -723,8 +723,120 @@ internal void ProcessTypeForBackCompatibility() protected internal virtual IReadOnlyList? BuildEnumValuesForBackCompatibility(IReadOnlyList originalEnumValues) => null; + /// + /// Returns this type's methods with backward compatibility applied against + /// . The default implementation restores the previous + /// parameter order on a current method when it matches a last-contract method by name and + /// return type with the same parameter set but in a different order. Reordering is done in + /// place, so a method's body (which references its parameters by object) remains valid. + /// Override and call base to extend this behavior; override without calling + /// base to replace it. + /// protected internal virtual IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) - => [.. originalMethods]; + { + var methods = new List(originalMethods); + + if (LastContractView?.Methods is not { Count: > 0 } previousMethods) + { + return methods; + } + + var currentMethodSignatures = BuildCurrentMethodSignatureMap(methods); + + foreach (var previousMethod in previousMethods) + { + if (!BackCompatHelper.ShouldApplyMethodBackCompatibility(previousMethod.Signature, currentMethodSignatures)) + { + continue; + } + + var methodToReorder = BackCompatHelper.FindMethodWithSameParametersDifferentOrder(previousMethod.Signature, currentMethodSignatures); + if (methodToReorder != null && BackCompatHelper.TryRestorePreviousParameterOrder(methodToReorder, previousMethod.Signature)) + { + CodeModelGenerator.Instance.Emitter.Debug( + $"Reordered parameters of '{Name}.{methodToReorder.Signature.Name}' to match last contract.", + BackCompatibilityChangeCategory.MethodParameterReordering); + } + } + + RestorePreviousParameterNames(methods); + + return methods; + } + + /// + /// Restores previously-published parameter names on this type's public/protected methods so a + /// generator or spec rename does not source-break callers using named arguments. The lookup is + /// keyed on each parameter's original (spec) name — retained on its source + /// — so only spec-derived parameters are + /// considered; hand-built parameters (no input parameter) are left unchanged. The rename is + /// applied in place via , which also rewrites the cached + /// variable/argument declarations so the method body and XML docs follow automatically. + /// + private void RestorePreviousParameterNames(IReadOnlyList methods) + { + foreach (var method in methods) + { + var modifiers = method.Signature.Modifiers; + if (!modifiers.HasFlag(MethodSignatureModifiers.Public) && !modifiers.HasFlag(MethodSignatureModifiers.Protected)) + { + continue; + } + + foreach (var parameter in method.Signature.Parameters) + { + var inputParameter = parameter.InputParameter; + if (inputParameter is null) + { + continue; + } + + if (!string.Equals(parameter.Name, inputParameter.Name, StringComparison.Ordinal)) + { + continue; + } + + var originalName = inputParameter.OriginalName; + if (string.IsNullOrEmpty(originalName)) + { + continue; + } + + var preservedName = BackCompatHelper.FindPreviousParameterName(LastContractView, originalName, method.Signature.Name); + + // The lookup matches the previous parameter case-insensitively (so a casing-only + // spec change still finds it), but the decision to rename is case-sensitive: a + // casing-only difference must still restore the previously-published spelling. + if (preservedName is null || string.Equals(preservedName, parameter.Name, StringComparison.Ordinal)) + { + continue; + } + + CodeModelGenerator.Instance.Emitter.Debug( + $"Preserved parameter name '{preservedName}' on '{Name}.{method.Signature.Name}' from last contract (instead of '{parameter.Name}').", + BackCompatibilityChangeCategory.ParameterNamePreserved); + parameter.Update(name: preservedName); + } + } + } + + /// + /// Builds a lookup of the type's current method signatures (including custom code methods) + /// used to match against last-contract methods. + /// + private Dictionary BuildCurrentMethodSignatureMap(IEnumerable methods) + { + var allMethods = CustomCodeView?.Methods != null + ? methods.Concat(CustomCodeView.Methods) + : methods; + + var result = new Dictionary(MethodSignature.MethodSignatureComparer); + foreach (var method in allMethods) + { + result.TryAdd(method.Signature, method); + } + return result; + } protected internal virtual IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) => [.. originalConstructors]; 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 new file mode 100644 index 00000000000..ba783444ac2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs @@ -0,0 +1,173 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.TypeSpec.Generator.Input.Extensions; +using Microsoft.TypeSpec.Generator.Primitives; +using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Statements; + +namespace Microsoft.TypeSpec.Generator.Utilities +{ + /// + /// Shared helpers for applying backward compatibility to generated methods by comparing the + /// current generation against a type's last contract. + /// + internal static class BackCompatHelper + { + /// + /// Returns true when a last-contract method should be considered for back compatibility: + /// it is public or protected and has no exact match among the current signatures. + /// + public static bool ShouldApplyMethodBackCompatibility( + MethodSignature previousSignature, + Dictionary currentMethodSignatures) + { + if (currentMethodSignatures.ContainsKey(previousSignature)) + { + return false; + } + + var modifiers = previousSignature.Modifiers; + return modifiers.HasFlag(MethodSignatureModifiers.Public) || + modifiers.HasFlag(MethodSignatureModifiers.Protected); + } + + /// + /// Finds the current method that has the same parameter set as + /// (matched by name and return type) but in a different order, or null when there is none. + /// + public static MethodProvider? FindMethodWithSameParametersDifferentOrder( + MethodSignature previousSignature, + Dictionary currentMethodSignatures) + { + foreach (var kvp in currentMethodSignatures) + { + var currentSignature = kvp.Key; + if (currentSignature.Name.Equals(previousSignature.Name) + && currentSignature.ReturnType?.AreNamesEqual(previousSignature.ReturnType) == true + && MethodSignatureHelper.ContainsSameParameters(previousSignature, currentSignature)) + { + return kvp.Value; + } + } + + return null; + } + + /// + /// Returns the previously-published name of a parameter whose original (spec) name is + /// , looked up in . When + /// is supplied, the search is scoped to last-contract methods + /// whose name matches it (allowing for a sync/async pair) so a parameter name shared across + /// methods cannot cross-match. Returns null when no match exists. + /// + public static string? FindPreviousParameterName( + TypeProvider? lastContractView, + string originalName, + string? methodName = null) + { + var lastContractMethods = lastContractView?.Methods; + if (lastContractMethods is null || lastContractMethods.Count == 0) + { + return null; + } + + IEnumerable scopedMethods = lastContractMethods; + if (methodName != null) + { + scopedMethods = lastContractMethods.Where(m => + string.Equals(m.Signature.Name, methodName, StringComparison.OrdinalIgnoreCase) || + string.Equals(m.Signature.Name, methodName + "Async", StringComparison.OrdinalIgnoreCase)); + } + + return scopedMethods + .SelectMany(method => method.Signature.Parameters) + .FirstOrDefault(p => string.Equals(p.Name, originalName, StringComparison.OrdinalIgnoreCase)) + ?.Name; + } + + /// + /// Restores the previous parameter order on in place + /// (updating XML docs). Returns true when a change was made. + /// + public static bool TryRestorePreviousParameterOrder( + MethodProvider methodToReorder, + MethodSignature previousSignature) + { + var currentSignature = methodToReorder.Signature; + if (MethodSignatureHelper.HaveSameParametersInSameOrder(currentSignature, previousSignature)) + { + return false; + } + + var parametersByName = currentSignature.Parameters.ToDictionary(p => p.Name.ToVariableName()); + var reorderedParameters = new List(currentSignature.Parameters.Count); + + foreach (var previousParam in previousSignature.Parameters) + { + if (parametersByName.TryGetValue(previousParam.Name, out var matchingParam)) + { + reorderedParameters.Add(matchingParam); + } + } + + if (reorderedParameters.Count != currentSignature.Parameters.Count) + { + return false; + } + + foreach (var previousParam in previousSignature.Parameters) + { + if (parametersByName.TryGetValue(previousParam.Name, out var matchingParam) + && matchingParam.DefaultValue is not null + && previousParam.DefaultValue is not null) + { + matchingParam.Update(defaultValue: previousParam.DefaultValue); + } + } + + var updatedSignature = new MethodSignature( + currentSignature.Name, + currentSignature.Description, + currentSignature.Modifiers, + currentSignature.ReturnType, + currentSignature.ReturnDescription, + reorderedParameters, + currentSignature.Attributes, + currentSignature.GenericArguments, + currentSignature.GenericParameterConstraints, + currentSignature.ExplicitInterface, + currentSignature.NonDocumentComment); + + UpdateXmlDocProviderForParamReorder(methodToReorder.XmlDocs, updatedSignature); + methodToReorder.Update(signature: updatedSignature, xmlDocProvider: methodToReorder.XmlDocs); + + return true; + } + + private static void UpdateXmlDocProviderForParamReorder( + XmlDocProvider xmlDocs, + MethodSignature updatedSignature) + { + var paramDocsByName = xmlDocs.Parameters.ToDictionary(s => s.Parameter.Name); + var reorderedParamDocs = new List(updatedSignature.Parameters.Count); + + foreach (var param in updatedSignature.Parameters) + { + if (paramDocsByName.TryGetValue(param.Name, out var paramDoc)) + { + reorderedParamDocs.Add(paramDoc); + } + } + + if (reorderedParamDocs.Count == xmlDocs.Parameters.Count && + !reorderedParamDocs.SequenceEqual(xmlDocs.Parameters)) + { + xmlDocs.Update(parameters: reorderedParamDocs); + } + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName.cs new file mode 100644 index 00000000000..a1fadf5dd08 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName.cs @@ -0,0 +1,18 @@ +// + +#nullable disable + +using Sample; + +namespace Test +{ + public partial class TestClient + { + public string Foo(string brandNewParam) + { + global::Sample.Argument.AssertNotNullOrEmpty(brandNewParam, nameof(brandNewParam)); + + return brandNewParam; + } + } +} 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 new file mode 100644 index 00000000000..afe96ffaaf7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs @@ -0,0 +1,10 @@ +namespace Test +{ + /// + /// Previously-published contract that does not contain the "brandNewParam" parameter. + /// + public class TestClient + { + public string Foo(string oldParam) { return null; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName.cs new file mode 100644 index 00000000000..c611ba3d45d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName.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/BuildMethodsForBackCompatibilityRestoresPreviousParameterName/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName/TestClient.cs new file mode 100644 index 00000000000..c413df7e7f8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName/TestClient.cs @@ -0,0 +1,10 @@ +namespace Test +{ + /// + /// Previously-published contract: the parameter was published as "oldParam". + /// + public class TestClient + { + public string Foo(string oldParam) { return null; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat.cs new file mode 100644 index 00000000000..a4c2bd4cf62 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +namespace Test +{ + public partial class CustomReorderType + { + public string Foo(string first, int second) + { + return null; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat/CustomReorderType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat/CustomReorderType.cs new file mode 100644 index 00000000000..0c0a8860a69 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat/CustomReorderType.cs @@ -0,0 +1,7 @@ +namespace Test +{ + public class CustomReorderType + { + public string Foo(string first, int second) => null; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults.cs new file mode 100644 index 00000000000..f41eb686ec3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +namespace Test +{ + public partial class CustomReorderValueTypeDefaultsType + { + public string Foo(int count = 0, bool flag = false) + { + return null; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults/CustomReorderValueTypeDefaultsType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults/CustomReorderValueTypeDefaultsType.cs new file mode 100644 index 00000000000..d45afd10149 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults/CustomReorderValueTypeDefaultsType.cs @@ -0,0 +1,13 @@ +namespace Test +{ + /// + /// Previous contract whose Foo method declares value-type optional parameters with literal + /// defaults (count = 0, flag = false) in the order (count, flag). The current generation + /// declares them in a different order with `default`-keyword defaults; restoring the order must + /// also preserve the previously published literal default representation. + /// + public class CustomReorderValueTypeDefaultsType + { + public string Foo(int count = 0, bool flag = false) => null; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/FindPreviousParameterNameLooksUpLastContract/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/FindPreviousParameterNameLooksUpLastContract/TestClient.cs new file mode 100644 index 00000000000..2c11ceaee1e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/FindPreviousParameterNameLooksUpLastContract/TestClient.cs @@ -0,0 +1,15 @@ +namespace Test +{ + /// + /// Represents the previously-published contract for a type whose parameters may have since + /// been renamed by the generator. + /// + public class TestClient + { + public void Foo(string oldParam) { } + + public void BarAsync(string oldAsyncParam) { } + + public void Other(string otherParam) { } + } +} 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 e597456fef9..57a50776b4e 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 @@ -12,6 +12,7 @@ using Microsoft.TypeSpec.Generator.Snippets; using Microsoft.TypeSpec.Generator.Statements; using Microsoft.TypeSpec.Generator.Tests.Common; +using Microsoft.TypeSpec.Generator.Utilities; using NUnit.Framework; namespace Microsoft.TypeSpec.Generator.Tests.Providers @@ -57,6 +58,153 @@ public async Task TestLoadLastContractView() Assert.AreEqual("p1", signature.Parameters[0].Name); } + // Validates that a custom TypeProvider inherits the parameter-reordering back-compat behavior + // from the base TypeProvider. + [Test] + public async Task CustomTypeProviderInheritsParameterReorderBackCompat() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Current generation declares Foo(second, first) — the reverse of the last contract's + // Foo(first, second). + var first = new ParameterProvider("first", $"", new CSharpType(typeof(string))); + var second = new ParameterProvider("second", $"", new CSharpType(typeof(int))); + var currentFoo = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [second, first]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "CustomReorderType", ns: "Test", methods: [currentFoo]); + + // The custom type does not override BuildMethodsForBackCompatibility; the base default + // restores the previous parameter order in place. + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + System.IO.File.WriteAllText(System.IO.Path.Combine(System.IO.Path.GetTempPath(), "valdefault_actual.cs"), actual); + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // Validates that when restoring a previous parameter order, the previously published default + // value representation is preserved (e.g. a value-type parameter keeps its literal `= 0` + // rather than flipping to `= default`). + [Test] + public async Task CustomTypeProviderReorderPreservesValueTypeDefaults() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Current generation declares Foo(flag, count) with `default`-keyword defaults — the + // reverse of the last contract's Foo(int count = 0, bool flag = false). + var flag = new ParameterProvider("flag", $"", new CSharpType(typeof(bool)), defaultValue: Snippet.Default); + var count = new ParameterProvider("count", $"", new CSharpType(typeof(int)), defaultValue: Snippet.Default); + var currentFoo = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [flag, count]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "CustomReorderValueTypeDefaultsType", ns: "Test", methods: [currentFoo]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + System.IO.File.WriteAllText(System.IO.Path.Combine(System.IO.Path.GetTempPath(), "valdefault_actual.cs"), actual); + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // Validates the shared lookup that any provider can use to restore a previously-published + // parameter name from its last contract. + [Test] + public async Task FindPreviousParameterNameLooksUpLastContract() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var lastContractView = new TestTypeProvider(name: "TestClient").LastContractView; + + // The last contract published Foo(string oldParam); scoped to that method it is found. + Assert.AreEqual("oldParam", BackCompatHelper.FindPreviousParameterName(lastContractView, "oldParam", "Foo")); + + // The exact casing from the contract is returned even when the lookup name differs only in casing. + Assert.AreEqual("oldParam", BackCompatHelper.FindPreviousParameterName(lastContractView, "oldparam", "Foo")); + + // The lookup is scoped: "oldParam" is not published on Other. + Assert.IsNull(BackCompatHelper.FindPreviousParameterName(lastContractView, "oldParam", "Other")); + + // An unscoped lookup finds the parameter on any last-contract method. + Assert.AreEqual("oldParam", BackCompatHelper.FindPreviousParameterName(lastContractView, "oldParam")); + + // A parameter that does not exist in the last contract returns null. + Assert.IsNull(BackCompatHelper.FindPreviousParameterName(lastContractView, "missing", "Foo")); + + // A sync method name matches its async counterpart (BarAsync) in the contract. + Assert.AreEqual("oldAsyncParam", BackCompatHelper.FindPreviousParameterName(lastContractView, "oldAsyncParam", "Bar")); + } + + // Validates that the lookup returns null when there is no last contract. + [Test] + public void FindPreviousParameterNameReturnsNullWithoutLastContract() + { + var typeProvider = new TestTypeProvider(name: "TestClient"); + Assert.IsNull(typeProvider.LastContractView); + Assert.IsNull(BackCompatHelper.FindPreviousParameterName(typeProvider.LastContractView, "oldParam", "Foo")); + } + + // Validates that the base BuildMethodsForBackCompatibility default automatically restores a + // previously-published parameter name on any derived type — keyed on the parameter's spec + // original name — and that the rename propagates into the already-built method body. + [Test] + public async Task BuildMethodsForBackCompatibilityRestoresPreviousParameterName() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // A spec parameter "oldParam" that the generator renamed to "newParam". + var inputParameter = InputFactory.QueryParameter("oldParam", InputPrimitiveType.String, isRequired: true); + inputParameter.Update(name: "newParam"); + + var parameter = new ParameterProvider(inputParameter); + var fooMethod = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [parameter]), + new MethodBodyStatement[] + { + // Passing the parameter as an argument to another method exercises propagation + // of the restored name into invocation arguments, not just the return. + Snippet.This.Invoke("Validate", parameter).Terminate(), + Snippet.Return(parameter), + }, + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "TestClient", methods: [fooMethod]); + + // The default has no override here; the base restores the previously-published name and + // the rename propagates into the already-built body. + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + System.IO.File.WriteAllText(System.IO.Path.Combine(System.IO.Path.GetTempPath(), "paramname_restore_actual.cs"), actual); + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // Validates that a parameter whose spec name is not in the last contract keeps its current name. + [Test] + public async Task BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // "brandNewParam" has no counterpart in the last contract (TestClient.Foo(oldParam)). + var inputParameter = InputFactory.QueryParameter("brandNewParam", InputPrimitiveType.String, isRequired: true); + var parameter = new ParameterProvider(inputParameter); + var fooMethod = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [parameter]), + Snippet.Return(parameter), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "TestClient", methods: [fooMethod]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + System.IO.File.WriteAllText(System.IO.Path.Combine(System.IO.Path.GetTempPath(), "paramname_keep_actual.cs"), actual); + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + [Test] public async Task LastContractViewLoadedForRenamedType() { diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility-extensibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility-extensibility.md new file mode 100644 index 00000000000..2a39dcca305 --- /dev/null +++ b/packages/http-client-csharp/generator/docs/backward-compatibility-extensibility.md @@ -0,0 +1,170 @@ +# Backward Compatibility Extensibility + +## Problem + +The generator preserves backward compatibility by diffing the current generation against +the last released contract and adjusting members so existing code keeps compiling. Today +this logic is private to each built-in provider (`ClientProvider`, `ModelProvider`, +`ModelFactoryProvider`, `ApiVersionEnumProvider`), so downstream providers that derive +directly from `TypeProvider` — such as the management generator's `ResourceClientProvider` — +cannot reuse it. + +## Solution + +Move the generic algorithms into `TypeProvider` as the **default implementations** of its +back-compat hooks. Any provider then consumes them by inheriting, and customizes by +overriding and calling `base`. On by default; opt out by overriding to return the input +unchanged. + +## Two API families (split by when they run) + +| Family | Shape | Caller / timing | +|---|---|---| +| **`Build…ForBackCompatibility`** | `(IReadOnlyList) → IReadOnlyList` | Framework calls, **after** visitors | +| **`Get…ForBackCompatibility`** | `(member) → value` | **You** call, **while building** a member (before serialization) | + +```csharp +public partial class TypeProvider +{ + public TypeProvider? LastContractView { get; } // already public + + // FAMILY 1 — framework calls after visitors; default applies the standard algorithm. + // Override + call base to extend; override without base to replace. + // Members produced here are NOT re-visited by visitors. + protected internal virtual IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable methods); + protected internal virtual IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable constructors); + protected internal virtual IReadOnlyList? BuildEnumValuesForBackCompatibility(IReadOnlyList enumValues); + + // Materialization seam used by the FAMILY 1 method default. The base builds the synthesized + // overload as a plain MethodProvider whose body delegates to the current method, then hands + // it here. Default returns it unchanged; override to return a provider-specific subtype or + // to apply adjustments (e.g. analyzer suppressions). + protected virtual MethodProvider BuildBackCompatibilityOverload(MethodProvider overload); + + // FAMILY 2 — you call while building; returns the value to use (non-mutating), + // scoped to this type's LastContractView. + protected virtual CSharpType GetPropertyTypeForBackCompatibility(PropertyProvider property); +} +``` + +> The parameter/signature comparison primitives used internally by the default algorithm +> (`MethodSignatureHelper`) are shared source compiled into each generator assembly and remain +> `internal`; downstream providers do not need them because the materialization seam hands them a +> fully-built `MethodProvider`. + +**Defaults:** +- `BuildMethodsForBackCompatibility` — restores previous parameter order; adds a hidden + `[EditorBrowsable(Never)]` overload (delegating to the current method) when new optional + non-body parameters were added. +- `BuildConstructorsForBackCompatibility` — keeps an abstract base type's constructor public + when the last contract exposed a matching public one. +- `BuildEnumValuesForBackCompatibility` — preserves enum members removed since the last contract. +- `GetPropertyTypeForBackCompatibility` — returns the previously published property type when + it changed, else the current type. + +## Pipeline + +```mermaid +flowchart TD + A["1 · EnsureBuilt()
build members + bodies"] --> B["2 · visitors run
(transform members)"] + B --> C["3 · ProcessTypeForBackCompatibility()"] + C --> D["4 · writers emit"] + + A -. "you call" .-> G["Get…ForBackCompatibility
(property type, …)"] + C -. "framework calls" .-> F["Build…ForBackCompatibility
(methods, ctors, enum values)"] + + style G fill:#e8f0fe,stroke:#4285f4 + style F fill:#e6f4ea,stroke:#34a853 +``` + +- **`Get…` is build-time (step 1)** because its value feeds the body/serialization built in + the same step. Running it later would desync a property's declared type from its already- + generated serialization. +- **`Build…` is post-visitor (step 3)** because its transforms are safe to apply last + (reorder is cosmetic, new overloads get a fresh body, modifier/enum changes are + declaration-only) and shouldn't be rewritten by visitors. Members it produces are not + re-visited. + +A `Build…` hook only acts when `LastContractView` has those members and a real diff exists, +so enabling defaults everywhere is safe. + +## Built-in providers after consolidation + +| Provider | Change | +|---|---| +| `ClientProvider` | Inherits the base `BuildMethodsForBackCompatibility` algorithm; keeps a slim override only for SCM-specific concerns (re-typing the synthesized overload as `ScmMethodProvider` + AZC0002 via `BuildBackCompatibilityOverload`, and fixing convenience-method bodies that call reordered protocol methods). | +| `ModelProvider` | Ctor logic → base hook; property-type logic → `GetPropertyTypeForBackCompatibility` (called from `BuildProperties`). | +| `ModelFactoryProvider` | **Keeps its override** (multi-strategy factory logic differs). | +| `ApiVersionEnumProvider` | Enum logic → base hook; **inherits**. | + +## Downstream usage (management generator) + +`ResourceClientProvider` derives from `TypeProvider`, so it inherits all the defaults. + +### 1. Standard method back-compat — free, no code + +Resource operation methods automatically get parameter-order restoration and hidden +overloads for newly added optional parameters: + +```csharp +public partial class ResourceClientProvider : TypeProvider +{ + // No override needed. +} +``` + +### 2. Materialize the synthesized overload as a provider-specific type + +Override the seam to return your own `MethodProvider` subtype or to apply adjustments. The +base has already built the hidden, delegating overload — you only re-wrap or tweak it. (This +is exactly how `ClientProvider` returns an `ScmMethodProvider` and adds its AZC0002 +suppression.) + +```csharp +public partial class ResourceClientProvider : TypeProvider +{ + protected override MethodProvider BuildBackCompatibilityOverload(MethodProvider overload) + { + // overload.Signature is hidden ([EditorBrowsable(Never)], defaults stripped) and + // overload.BodyStatements already delegates to the current method. + return new MyResourceMethodProvider(overload.Signature, overload.BodyStatements!, this, overload.XmlDocs); + } +} +``` + +### 3. Extend or filter the produced method list + +Override `BuildMethodsForBackCompatibility`, call `base` to keep the standard shims, then add +or filter your own: + +```csharp +public partial class ResourceClientProvider : TypeProvider +{ + protected override IReadOnlyList BuildMethodsForBackCompatibility( + IEnumerable methods) + { + var result = base.BuildMethodsForBackCompatibility(methods); // standard shims first + // ...inspect LastContractView and add/adjust resource-specific methods... + return result; + } +} +``` + +The default synthesized overload delegates to the current method, so it works even for +management LRO methods (it forwards to the method that wraps the result in `ArmOperation`). + +### 4. Preserve a property type while building it (build-time) + +Call the getter while constructing the property, before serialization is generated: + +```csharp +var property = TypeFactory.CreateProperty(inputProperty, this); +property.Type = GetPropertyTypeForBackCompatibility(property); // previous type wins if it changed +``` + +## Deferred + +Build-time **parameter-name preservation** (`top`→`maxCount`, page-size casing, +`@@clientName` renames) and **content-type-before-body ordering** stay in `RestClientProvider` +for now. When exposed, they join the `Get…` family as +`GetParameterNameForBackCompatibility(ParameterProvider, MethodProvider)`.