Add Support for More Back Compat Model Ctor Scenarios (#11614)

This PR introduces more covered scenarios for the back compat support
for model ctors.

fixes: #11588

Author: jorgerangel-msft

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

@@ -802,13 +802,21 @@ protected internal override IReadOnlyList<ConstructorProvider> BuildConstructors
                 return base.BuildConstructorsForBackCompatibility(originalConstructors);
             }
 
-            var constructors = new List<ConstructorProvider>(base.BuildConstructorsForBackCompatibility(originalConstructors));
-            var restorablePropertyLookup = BuildRestorablePropertyLookup();
+            var originalConstructorList = originalConstructors as IReadOnlyList<ConstructorProvider> ?? [.. originalConstructors];
             IReadOnlyList<ConstructorProvider> candidateConstructors = CustomCodeView?.Constructors is { Count: > 0 } customConstructors
-                ? [.. constructors, .. customConstructors]
-                : constructors;
+                ? [.. originalConstructorList, .. customConstructors]
+                : originalConstructorList;
 
-            foreach (var previousConstructor in previousConstructors)
+            var restorablePreviousConstructors = previousConstructors
+                .Where(c => !BackCompatHelper.IsConstructorRemovalAcceptedInBaseline(this, c.Signature))
+                .ToList();
+
+            RestorePreviousConstructorParameterNames(originalConstructorList, candidateConstructors, restorablePreviousConstructors);
+
+            var constructors = new List<ConstructorProvider>(base.BuildConstructorsForBackCompatibility(originalConstructorList));
+            var restorablePropertyLookup = BuildRestorablePropertyLookup();
+
+            foreach (var previousConstructor in restorablePreviousConstructors)
             {
                 if (!MethodSignatureHelper.IsPublicApi(previousConstructor.Signature.Modifiers))
                 {
@@ -817,19 +825,17 @@ protected internal override IReadOnlyList<ConstructorProvider> BuildConstructors
 
                 var previousParameters = previousConstructor.Signature.Parameters;
 
-                if (BackCompatHelper.IsConstructorRemovalAcceptedInBaseline(this, previousConstructor.Signature))
-                {
-                    continue;
-                }
-
                 // A previously published accessible parameterless constructor is dropped when the current
                 // generation makes a property required. Restore it and drop the generated mocking constructor
                 // so it is not a duplicate. An accessible parameterless constructor (generated or custom code)
-                // counts as already present; an inaccessible generated mocking constructor does not.
-                if (!Type.IsStruct && previousParameters.Count == 0)
-                {
-                    if (!constructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers))
-                        && !CanonicalView.Constructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers)))
+                // counts as already present; an inaccessible generated mocking constructor does not. A struct
+                // always exposes a public parameterless constructor via its serialization (mocking)
+                // constructor, so there is nothing to restore on the model partial.
+                if (previousParameters.Count == 0)
+                {
+                    if (!Type.IsStruct
+                        && !constructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers))
+                        && !candidateConstructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers)))
                     {
                         var parameterlessConstructor = BuildBackCompatParameterlessConstructor(previousConstructor, candidateConstructors);
                         RemoveGeneratedMockingConstructor(constructors);
@@ -843,9 +849,9 @@ protected internal override IReadOnlyList<ConstructorProvider> BuildConstructors
                 }
 
                 // If a constructor with the same parameters already exists - either still generated or
-                // supplied by custom code (which lives in the canonical view) - there is nothing to restore.
+                // supplied by custom code - there is nothing to restore.
                 if (constructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters))
-                    || CanonicalView.Constructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters)))
+                    || candidateConstructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters)))
                 {
                     continue;
                 }
@@ -868,6 +874,70 @@ protected internal override IReadOnlyList<ConstructorProvider> BuildConstructors
             return constructors;
         }
 
+        private void RestorePreviousConstructorParameterNames(
+            IReadOnlyList<ConstructorProvider> currentConstructors,
+            IReadOnlyList<ConstructorProvider> candidateConstructors,
+            IReadOnlyList<ConstructorProvider> previousConstructors)
+        {
+            const MethodSignatureModifiers privateProtected = MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected;
+            foreach (var previousConstructor in previousConstructors)
+            {
+                if (!MethodSignatureHelper.IsPublicApi(previousConstructor.Signature.Modifiers))
+                {
+                    continue;
+                }
+
+                var previousParameters = previousConstructor.Signature.Parameters;
+
+                // A generated or custom constructor that already matches the previous signature (types and
+                // names) satisfies the contract; renaming another constructor into it would collide.
+                if (candidateConstructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters)))
+                {
+                    continue;
+                }
+
+                var currentConstructor = currentConstructors.FirstOrDefault(c =>
+                    (MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers)
+                        || (c.Signature.Modifiers & privateProtected) == privateProtected)
+                    && MethodSignatureBase.SignatureComparer.Equals(c.Signature, previousConstructor.Signature));
+                if (currentConstructor is null)
+                {
+                    continue;
+                }
+
+                var currentParameters = currentConstructor.Signature.Parameters;
+
+                // A swap or rotation keeps every previous name, so realign the existing parameter objects
+                // to the previous order - renaming positionally would mis-bind a caller's named argument to
+                // the wrong property. Otherwise restore names positionally where the types line up.
+                var currentByName = currentParameters.ToDictionary(p => p.Name);
+                IReadOnlyList<ParameterProvider> restoredParameters = previousParameters.All(p => currentByName.ContainsKey(p.Name))
+                    ? [.. previousParameters.Select(p => currentByName[p.Name])]
+                    : currentParameters;
+                if (!restoredParameters.Select((p, i) => p.Type.AreNamesEqual(previousParameters[i].Type)).All(match => match))
+                {
+                    continue;
+                }
+
+                for (int i = 0; i < restoredParameters.Count; i++)
+                {
+                    var restoredName = previousParameters[i].Name;
+                    if (string.Equals(restoredParameters[i].Name, restoredName, StringComparison.Ordinal))
+                    {
+                        continue;
+                    }
+
+                    CodeModelGenerator.Instance.Emitter.Debug(
+                        $"Preserved parameter name '{restoredName}' at position {i} on constructor '{Name}' from last contract (instead of '{restoredParameters[i].Name}').",
+                        BackCompatibilityChangeCategory.ParameterNamePreserved);
+                    restoredParameters[i].Update(name: restoredName);
+                }
+
+                currentConstructor.Signature.Update(parameters: [.. restoredParameters]);
+                currentConstructor.Update(signature: currentConstructor.Signature);
+            }
+        }
+
         private bool TryBuildRestoredConstructor(
             ConstructorProvider previousConstructor,
             IReadOnlyList<ConstructorProvider> currentConstructors,
@@ -878,7 +948,9 @@ private bool TryBuildRestoredConstructor(
             var previousParameters = previousConstructor.Signature.Parameters;
 
             // Find the public constructor to chain to: its parameters must form an in-order subsequence of
-            // the previous constructor's parameters. Prefer the closest one.
+            // the previous constructor's parameters. Prefer the closest one. Without a chaining target the
+            // constructor is not restored - a standalone constructor would bypass the current constructor's
+            // initialization (e.g. the implicit base() call and inherited get-only properties).
             ConstructorProvider? targetConstructor = null;
             foreach (var candidate in currentConstructors)
             {
@@ -898,7 +970,7 @@ private bool TryBuildRestoredConstructor(
                 }
             }
 
-            if (targetConstructor == null)
+            if (targetConstructor is null)
             {
                 return false;
             }
@@ -925,23 +997,23 @@ private bool TryBuildRestoredConstructor(
                     continue;
                 }
 
-                if (!restorablePropertyLookup.TryGetValue(previousParameter.Name, out var property)
-                    || !property.Type.AreNamesEqual(previousParameter.Type))
+                // Each extra parameter (not consumed by the chain target) must map to a settable, wire-backed
+                // property assigned in the restored constructor's body.
+                var property = restorablePropertyLookup.TryGetValue(previousParameter.Name, out var chained)
+                    && chained.Type.AreNamesEqual(previousParameter.Type)
+                    ? chained
+                    : null;
+
+                if (property is null || extraAssignments.Any(a => a.Property == property))
                 {
                     return false;
                 }
 
-                var restoredParameter = PartialMethodCustomization.CloneParameterWithName(
-                    property.AsParameter,
-                    previousParameter.Name,
-                    removeDefault: true);
-
+                var restoredParameter = PartialMethodCustomization.CloneParameterWithName(property.AsParameter, previousParameter.Name, removeDefault: true);
                 restoredParameters.Add(restoredParameter);
                 extraAssignments.Add((property, restoredParameter));
             }
 
-            // Every target parameter must be consumed and at least one extra property must be assigned,
-            // otherwise the restored constructor would be redundant or would produce an invalid chained call.
             if (targetIndex != targetParameters.Count || extraAssignments.Count == 0)
             {
                 return false;

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

@@ -1075,6 +1075,11 @@ protected internal virtual IReadOnlyList<ConstructorProvider> BuildConstructorsF
                     continue;
                 }
 
+                if (BackCompatHelper.IsConstructorRemovalAcceptedInBaseline(this, previousConstructor.Signature))
+                {
+                    continue;
+                }
+
                 // Find a matching constructor in the current version by parameter signature
                 for (int i = 0; i < constructors.Count; i++)
                 {