feat: generalize back compat

Author: jorgerangel-msft

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<MethodProvider> BuildMethodsForBackCompatibility(IEnumerable<MethodProvider> originalMethods)
         {
-            List<MethodProvider> 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<MethodProvider, MethodSignature>(ReferenceEqualityComparer.Instance);
+            foreach (var method in originalMethods)
+            {
+                originalSignatures.TryAdd(method, method.Signature);
+            }
 
-            return materializedMethods;
-        }
+            List<MethodProvider> result = [.. base.BuildMethodsForBackCompatibility(originalMethods)];
 
-        private void ProcessBackCompatForParameterReordering(
-            IList<MethodProvider> materializedMethods,
-            Dictionary<MethodSignature, MethodProvider> 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, MethodSignature>(MethodSignature.MethodSignatureComparer);
             var methodsWithReorderedParams = new List<MethodProvider>();
-
-            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<MethodSignature, MethodProvider> BuildCurrentMethodSignatures(IEnumerable<MethodProvider> originalMethods)
+        private Dictionary<MethodSignature, MethodProvider> BuildCurrentMethodSignatureMap(IEnumerable<MethodProvider> methods)
         {
             var allMethods = CustomCodeView?.Methods != null
-                ? originalMethods.Concat(CustomCodeView.Methods)
-                : originalMethods;
+                ? methods.Concat(CustomCodeView.Methods)
+                : methods;
 
             var result = new Dictionary<MethodSignature, MethodProvider>(MethodSignature.MethodSignatureComparer);
             foreach (var method in allMethods)
@@ -1326,86 +1315,6 @@ private Dictionary<MethodSignature, MethodProvider> BuildCurrentMethodSignatures
             return result;
         }
 
-        private static bool ShouldProcessMethodForBackCompat(
-            MethodSignature previousSignature,
-            Dictionary<MethodSignature, MethodProvider> 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<MethodSignature, MethodProvider> 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<MethodSignature, MethodSignature> 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<ParameterProvider>(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<XmlDocParamStatement>(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<MethodProvider> methods,
             Dictionary<MethodSignature, MethodProvider> currentMethodSignatures)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs

@@ -723,8 +723,120 @@ internal void ProcessTypeForBackCompatibility()
         protected internal virtual IReadOnlyList<EnumTypeMember>? BuildEnumValuesForBackCompatibility(IReadOnlyList<EnumTypeMember> originalEnumValues)
             => null;
 
+        /// <summary>
+        /// Returns this type's methods with backward compatibility applied against
+        /// <see cref="LastContractView"/>. 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 <c>base</c> to extend this behavior; override without calling
+        /// <c>base</c> to replace it.
+        /// </summary>
         protected internal virtual IReadOnlyList<MethodProvider> BuildMethodsForBackCompatibility(IEnumerable<MethodProvider> originalMethods)
-            => [.. originalMethods];
+        {
+            var methods = new List<MethodProvider>(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;
+        }
+
+        /// <summary>
+        /// 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
+        /// <see cref="ParameterProvider.InputParameter"/> — so only spec-derived parameters are
+        /// considered; hand-built parameters (no input parameter) are left unchanged. The rename is
+        /// applied in place via <see cref="ParameterProvider.Update"/>, which also rewrites the cached
+        /// variable/argument declarations so the method body and XML docs follow automatically.
+        /// </summary>
+        private void RestorePreviousParameterNames(IReadOnlyList<MethodProvider> 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);
+                }
+            }
+        }
+
+        /// <summary>
+        /// Builds a lookup of the type's current method signatures (including custom code methods)
+        /// used to match against last-contract methods.
+        /// </summary>
+        private Dictionary<MethodSignature, MethodProvider> BuildCurrentMethodSignatureMap(IEnumerable<MethodProvider> methods)
+        {
+            var allMethods = CustomCodeView?.Methods != null
+                ? methods.Concat(CustomCodeView.Methods)
+                : methods;
+
+            var result = new Dictionary<MethodSignature, MethodProvider>(MethodSignature.MethodSignatureComparer);
+            foreach (var method in allMethods)
+            {
+                result.TryAdd(method.Signature, method);
+            }
+            return result;
+        }
 
         protected internal virtual IReadOnlyList<ConstructorProvider> BuildConstructorsForBackCompatibility(IEnumerable<ConstructorProvider> originalConstructors)
             => [.. originalConstructors];