Skip to content

Commit b6684e0

Browse files
Preserve accessible serialization constructors during back compat (#11798)
## Summary - Preserve an accessible parameterless constructor that already exists on a generated serialization partial during API compatibility processing. - Avoid removing a visitor-customized constructor and synthesizing a replacement that delegates to a different overload. - Cover both the accessible-constructor preservation path and the existing internal-constructor replacement path. This addresses the constructor churn observed in openai/openai-dotnet#1341 when ApiCompatVersion is enabled. ## Validation - `npm run build` - `eng/scripts/Generate.ps1` - `npm test` - `npm run cop` - Focused constructor compatibility tests --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00636c74-c4f7-434a-8a70-fb0c3d221328
1 parent 68ef6ba commit b6684e0

3 files changed

Lines changed: 105 additions & 7 deletions

File tree

  • packages/http-client-csharp/generator

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
using System.Linq;
77
using System.Text.Json.Serialization;
88
using System.Threading.Tasks;
9+
using Microsoft.TypeSpec.Generator.Expressions;
910
using Microsoft.TypeSpec.Generator.Input;
1011
using Microsoft.TypeSpec.Generator.Primitives;
12+
using Microsoft.TypeSpec.Generator.Snippets;
1113
using Microsoft.TypeSpec.Generator.Tests.Common;
1214
using NUnit.Framework;
1315
using ScmModel = Microsoft.TypeSpec.Generator.ClientModel.Providers.ScmModelProvider;
@@ -260,6 +262,85 @@ await MockHelpers.LoadMockGeneratorAsync(
260262
Assert.AreEqual(Helpers.GetExpectedFromFile("Serialization"), serializationContent);
261263
}
262264

265+
[Test]
266+
public async Task BackCompat_AccessibleParameterlessSerializationConstructorIsPreserved()
267+
{
268+
var inputModel = InputFactory.Model(
269+
"mockInputModel",
270+
usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json,
271+
properties:
272+
[
273+
InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)
274+
]);
275+
276+
await MockHelpers.LoadMockGeneratorAsync(
277+
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
278+
inputModels: () => [inputModel]);
279+
280+
var model = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders
281+
.OfType<ScmModel>().Single(t => t.Name == "MockInputModel");
282+
var serializationProvider = model.SerializationProviders.Single();
283+
var serializationConstructor = serializationProvider.Constructors
284+
.Single(c => c.Signature.Parameters.Count == 0);
285+
var originalSignature = serializationConstructor.Signature;
286+
ValueExpression[] fullConstructorArguments =
287+
[.. model.FullConstructor.Signature.Parameters.Select(_ => Snippet.Default)];
288+
var fullConstructorInitializer = new ConstructorInitializer(
289+
IsBase: false,
290+
fullConstructorArguments);
291+
292+
// Simulate a visitor making the serialization constructor public and routing it through
293+
// the full constructor.
294+
serializationConstructor.Update(
295+
signature: new ConstructorSignature(
296+
originalSignature.Type,
297+
originalSignature.Description,
298+
MethodSignatureModifiers.Public,
299+
originalSignature.Parameters,
300+
originalSignature.Attributes,
301+
fullConstructorInitializer));
302+
303+
model.ProcessTypeForBackCompatibility();
304+
305+
Assert.IsFalse(model.Constructors.Any(c => c.Signature.Parameters.Count == 0),
306+
"An accessible parameterless constructor already exists on the serialization partial.");
307+
var preservedConstructor = serializationProvider.Constructors
308+
.Single(c => c.Signature.Parameters.Count == 0);
309+
Assert.AreSame(serializationConstructor, preservedConstructor);
310+
Assert.AreSame(fullConstructorInitializer, preservedConstructor.Signature.Initializer);
311+
}
312+
313+
[Test]
314+
public async Task BackCompat_InaccessibleParameterlessSerializationConstructorIsReplaced()
315+
{
316+
var inputModel = InputFactory.Model(
317+
"mockInputModel",
318+
usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json,
319+
properties:
320+
[
321+
InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)
322+
]);
323+
324+
await MockHelpers.LoadMockGeneratorAsync(
325+
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(
326+
method: nameof(BackCompat_AccessibleParameterlessSerializationConstructorIsPreserved)),
327+
inputModels: () => [inputModel]);
328+
329+
var model = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders
330+
.OfType<ScmModel>().Single(t => t.Name == "MockInputModel");
331+
var serializationProvider = model.SerializationProviders.Single();
332+
Assert.IsTrue(serializationProvider.Constructors.Any(c =>
333+
c.Signature.Parameters.Count == 0
334+
&& c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal)));
335+
336+
model.ProcessTypeForBackCompatibility();
337+
338+
Assert.IsFalse(serializationProvider.Constructors.Any(c => c.Signature.Parameters.Count == 0));
339+
Assert.IsTrue(model.Constructors.Any(c =>
340+
c.Signature.Parameters.Count == 0
341+
&& c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)));
342+
}
343+
263344
[Test]
264345
public async Task BackCompat_StructParameterlessConstructorNotMovedFromSerialization()
265346
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
namespace Sample.Models
2+
{
3+
public partial class MockInputModel
4+
{
5+
public MockInputModel()
6+
{
7+
}
8+
}
9+
}

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -847,15 +847,23 @@ protected internal override IReadOnlyList<ConstructorProvider> BuildConstructors
847847

848848
// A previously published accessible parameterless constructor is dropped when the current
849849
// generation makes a property required. Restore it and drop the generated mocking constructor
850-
// so it is not a duplicate. An accessible parameterless constructor (generated or custom code)
851-
// counts as already present; an inaccessible generated mocking constructor does not. A struct
852-
// always exposes a public parameterless constructor via its serialization (mocking)
853-
// constructor, so there is nothing to restore on the model partial.
850+
// so it is not a duplicate. An accessible parameterless constructor on any generated partial
851+
// or in custom code counts as already present. A struct always exposes a public parameterless
852+
// constructor via its serialization partial, so there is nothing to restore on the model partial.
854853
if (previousParameters.Count == 0)
855854
{
856-
if (!Type.IsStruct
857-
&& !constructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers))
858-
&& !candidateConstructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers)))
855+
if (Type.IsStruct)
856+
{
857+
continue;
858+
}
859+
860+
var hasAccessibleParameterlessSerializationConstructor = SerializationProviders
861+
.SelectMany(p => p.Constructors)
862+
.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers));
863+
864+
if (!constructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers))
865+
&& !candidateConstructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers))
866+
&& !hasAccessibleParameterlessSerializationConstructor)
859867
{
860868
var parameterlessConstructor = BuildBackCompatParameterlessConstructor(previousConstructor, candidateConstructors);
861869
RemoveGeneratedMockingConstructor(constructors);

0 commit comments

Comments
 (0)