Preserve accessible serialization constructors during back compat

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs

@@ -6,8 +6,10 @@
 using System.Linq;
 using System.Text.Json.Serialization;
 using System.Threading.Tasks;
+using Microsoft.TypeSpec.Generator.Expressions;
 using Microsoft.TypeSpec.Generator.Input;
 using Microsoft.TypeSpec.Generator.Primitives;
+using Microsoft.TypeSpec.Generator.Snippets;
 using Microsoft.TypeSpec.Generator.Tests.Common;
 using NUnit.Framework;
 using ScmModel = Microsoft.TypeSpec.Generator.ClientModel.Providers.ScmModelProvider;
@@ -260,6 +262,85 @@ await MockHelpers.LoadMockGeneratorAsync(
             Assert.AreEqual(Helpers.GetExpectedFromFile("Serialization"), serializationContent);
         }
 
+        [Test]
+        public async Task BackCompat_AccessibleParameterlessSerializationConstructorIsPreserved()
+        {
+            var inputModel = InputFactory.Model(
+                "mockInputModel",
+                usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json,
+                properties:
+                [
+                    InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)
+                ]);
+
+            await MockHelpers.LoadMockGeneratorAsync(
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
+                inputModels: () => [inputModel]);
+
+            var model = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders
+                .OfType<ScmModel>().Single(t => t.Name == "MockInputModel");
+            var serializationProvider = model.SerializationProviders.Single();
+            var serializationConstructor = serializationProvider.Constructors
+                .Single(c => c.Signature.Parameters.Count == 0);
+            var originalSignature = serializationConstructor.Signature;
+            ValueExpression[] fullConstructorArguments =
+                [.. model.FullConstructor.Signature.Parameters.Select(_ => Snippet.Default)];
+            var fullConstructorInitializer = new ConstructorInitializer(
+                IsBase: false,
+                fullConstructorArguments);
+
+            // Simulate a visitor making the serialization constructor public and routing it through
+            // the full constructor.
+            serializationConstructor.Update(
+                signature: new ConstructorSignature(
+                    originalSignature.Type,
+                    originalSignature.Description,
+                    MethodSignatureModifiers.Public,
+                    originalSignature.Parameters,
+                    originalSignature.Attributes,
+                    fullConstructorInitializer));
+
+            model.ProcessTypeForBackCompatibility();
+
+            Assert.IsFalse(model.Constructors.Any(c => c.Signature.Parameters.Count == 0),
+                "An accessible parameterless constructor already exists on the serialization partial.");
+            var preservedConstructor = serializationProvider.Constructors
+                .Single(c => c.Signature.Parameters.Count == 0);
+            Assert.AreSame(serializationConstructor, preservedConstructor);
+            Assert.AreSame(fullConstructorInitializer, preservedConstructor.Signature.Initializer);
+        }
+
+        [Test]
+        public async Task BackCompat_InaccessibleParameterlessSerializationConstructorIsReplaced()
+        {
+            var inputModel = InputFactory.Model(
+                "mockInputModel",
+                usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json,
+                properties:
+                [
+                    InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)
+                ]);
+
+            await MockHelpers.LoadMockGeneratorAsync(
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(
+                    method: nameof(BackCompat_AccessibleParameterlessSerializationConstructorIsPreserved)),
+                inputModels: () => [inputModel]);
+
+            var model = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders
+                .OfType<ScmModel>().Single(t => t.Name == "MockInputModel");
+            var serializationProvider = model.SerializationProviders.Single();
+            Assert.IsTrue(serializationProvider.Constructors.Any(c =>
+                c.Signature.Parameters.Count == 0
+                && c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal)));
+
+            model.ProcessTypeForBackCompatibility();
+
+            Assert.IsFalse(serializationProvider.Constructors.Any(c => c.Signature.Parameters.Count == 0));
+            Assert.IsTrue(model.Constructors.Any(c =>
+                c.Signature.Parameters.Count == 0
+                && c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)));
+        }
+
         [Test]
         public async Task BackCompat_StructParameterlessConstructorNotMovedFromSerialization()
         {

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_AccessibleParameterlessSerializationConstructorIsPreserved/MockInputModel.cs

@@ -0,0 +1,9 @@
+namespace Sample.Models
+{
+    public partial class MockInputModel
+    {
+        public MockInputModel()
+        {
+        }
+    }
+}

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

@@ -847,15 +847,23 @@ protected internal override IReadOnlyList<ConstructorProvider> BuildConstructors
 
                 // 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. A struct
-                // always exposes a public parameterless constructor via its serialization (mocking)
-                // constructor, so there is nothing to restore on the model partial.
+                // so it is not a duplicate. An accessible parameterless constructor on any generated partial
+                // or in custom code counts as already present. A struct always exposes a public parameterless
+                // constructor via its serialization partial, 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)))
+                    if (Type.IsStruct)
+                    {
+                        continue;
+                    }
+
+                    var hasAccessibleParameterlessSerializationConstructor = SerializationProviders
+                        .SelectMany(p => p.Constructors)
+                        .Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers));
+
+                    if (!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))
+                        && !hasAccessibleParameterlessSerializationConstructor)
                     {
                         var parameterlessConstructor = BuildBackCompatParameterlessConstructor(previousConstructor, candidateConstructors);
                         RemoveGeneratedMockingConstructor(constructors);