Skip to content

Commit f31cecc

Browse files
Copilotjorgerangel-msftCopilot
authored
Support back-compat model constructors in the C# generator (#11503)
When a previously-required model property is relaxed to optional, the generator drops the corresponding parameter from the model's initialization constructor — a source-breaking change for callers that construct the model positionally (issue #11460). This restores the previously-published public constructor via the `LastContractView` back-compat mechanism. ### Changes - **`ModelProvider.BuildConstructorsForBackCompatibility` (new override)** — For each public constructor in `LastContractView` that has no current equivalent, reconstructs it as a public overload that chains to the closest current public constructor (`: this(...)`) and assigns the extra properties in its body. - Chain target is the current public constructor whose parameters form an in-order subsequence of the previous ones (closest match preferred). - Each dropped parameter must map to a settable property with an unchanged type — matched by property name, or by `OriginalName` for codegen renames. If any dropped parameter cannot be mapped, restoration is skipped. - Kept parameters forward to the chained constructor (validation stripped, since the target validates); non-nullable reference-type extras reinstate their `AssertNotNull` check. - **`BackCompatibilityChangeCategory`** — Adds `ConstructorAddedFromLastContract` and `ConstructorAddedFromLastContractSkipped` for emitter diagnostics. ### Example Given a model where `resources` was required and is now optional, the restored overload: ```csharp public MockInputModel(string name, string resources) : this(name) { Argument.AssertNotNull(resources, nameof(resources)); Resources = resources; } ``` ### Tests Added unit tests covering the restore case (required→optional), and the negative cases where the property was removed entirely or no last contract exists. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #11460 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> Co-authored-by: Jorge Rangel <jorgerangel@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e815ae8d-8fc2-4c43-982e-1e98f84f4f54
1 parent ec2295f commit f31cecc

37 files changed

Lines changed: 1548 additions & 3 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ public MrwSerializationTypeDefinition(InputModelType inputModel, ModelProvider m
125125
protected override IReadOnlyList<MethodProvider> BuildMethodsForBackCompatibility(IEnumerable<MethodProvider> originalMethods)
126126
=> [.. originalMethods];
127127

128+
protected override IReadOnlyList<ConstructorProvider> BuildConstructorsForBackCompatibility(IEnumerable<ConstructorProvider> originalConstructors)
129+
=> [.. originalConstructors];
130+
128131
private ConstructorProvider SerializationConstructor => _serializationConstructor ??= _model.FullConstructor;
129132
private PropertyProvider[] AdditionalProperties => _additionalProperties.Value;
130133

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,75 @@ await MockHelpers.LoadMockGeneratorAsync(
205205
Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
206206
}
207207

208+
[Test]
209+
public async Task BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor()
210+
{
211+
// The last contract published a parameterless `protected BaseModel()`. The current generation
212+
// makes the discriminator required, so the abstract base's initialization constructor now takes a
213+
// parameter and the parameterless constructor is dropped. It is restored, and the generated
214+
// parameterless mocking constructor on the serialization partial is removed to avoid a duplicate.
215+
var derivedInputModel = InputFactory.Model(
216+
"derivedModel",
217+
discriminatedKind: "one",
218+
properties:
219+
[
220+
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true)
221+
]);
222+
var inputModel = InputFactory.Model(
223+
"baseModel",
224+
properties:
225+
[
226+
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true)
227+
],
228+
discriminatedModels: new Dictionary<string, InputModelType>() { { "one", derivedInputModel } });
229+
230+
await MockHelpers.LoadMockGeneratorAsync(
231+
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
232+
inputModels: () => [inputModel]);
233+
234+
var model = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders
235+
.OfType<ScmModel>().Single(t => t.Name == "BaseModel");
236+
237+
model.ProcessTypeForBackCompatibility();
238+
239+
// The model gains the restored standalone parameterless constructor.
240+
var modelContent = new TypeProviderWriter(model).Write().Content;
241+
Assert.AreEqual(Helpers.GetExpectedFromFile("Model"), modelContent);
242+
243+
// The serialization partial no longer carries the parameterless mocking constructor (avoids CS0111).
244+
var serializationContent = new TypeProviderWriter(model.SerializationProviders.Single()).Write().Content;
245+
Assert.AreEqual(Helpers.GetExpectedFromFile("Serialization"), serializationContent);
246+
}
247+
248+
[Test]
249+
public async Task BackCompat_StructParameterlessConstructorNotMovedFromSerialization()
250+
{
251+
// A struct always exposes a public parameterless constructor via its serialization (mocking)
252+
// constructor, so the last contract's parameterless constructor is already present. It must not
253+
// be moved onto the model partial, which would be pointless churn with no public API change.
254+
var inputModel = InputFactory.Model(
255+
"structModel",
256+
modelAsStruct: true,
257+
properties:
258+
[
259+
InputFactory.Property("prop", InputPrimitiveType.String, isRequired: true)
260+
]);
261+
262+
await MockHelpers.LoadMockGeneratorAsync(
263+
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
264+
inputModels: () => [inputModel]);
265+
266+
var model = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders
267+
.OfType<ScmModel>().Single(t => t.Name == "StructModel");
268+
269+
model.ProcessTypeForBackCompatibility();
270+
271+
Assert.IsFalse(model.Constructors.Any(c => c.Signature.Parameters.Count == 0),
272+
"Struct model must not gain a parameterless constructor on the model partial.");
273+
Assert.IsTrue(model.SerializationProviders.Single().Constructors.Any(c => c.Signature.Parameters.Count == 0),
274+
"Struct serialization partial must retain its parameterless constructor.");
275+
}
276+
208277
[Test]
209278
public void TestDynamicModelWithUnionAdditionalProps()
210279
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// <auto-generated/>
2+
3+
#nullable disable
4+
5+
using System;
6+
using System.Collections.Generic;
7+
8+
namespace Sample.Models
9+
{
10+
public abstract partial class BaseModel
11+
{
12+
private protected readonly global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
13+
14+
private protected BaseModel(string kind)
15+
{
16+
Kind = kind;
17+
}
18+
19+
internal BaseModel(string kind, global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> additionalBinaryDataProperties)
20+
{
21+
Kind = kind;
22+
_additionalBinaryDataProperties = additionalBinaryDataProperties;
23+
}
24+
25+
protected BaseModel() : this(default)
26+
{
27+
}
28+
29+
internal string Kind { get; set; }
30+
}
31+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// <auto-generated/>
2+
3+
#nullable disable
4+
5+
using System;
6+
using System.ClientModel.Primitives;
7+
using System.Text.Json;
8+
using Sample;
9+
10+
namespace Sample.Models
11+
{
12+
[global::System.ClientModel.Primitives.PersistableModelProxyAttribute(typeof(global::Sample.Models.UnknownBaseModel))]
13+
public abstract partial class BaseModel : global::System.ClientModel.Primitives.IJsonModel<global::Sample.Models.BaseModel>
14+
{
15+
protected virtual global::Sample.Models.BaseModel PersistableModelCreateCore(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
16+
{
17+
string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.BaseModel>)this).GetFormatFromOptions(options) : options.Format;
18+
switch (format)
19+
{
20+
case "J":
21+
using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(data, global::Sample.ModelSerializationExtensions.JsonDocumentOptions))
22+
{
23+
return global::Sample.Models.BaseModel.DeserializeBaseModel(document.RootElement, options);
24+
}
25+
default:
26+
throw new global::System.FormatException($"The model {nameof(global::Sample.Models.BaseModel)} does not support reading '{options.Format}' format.");
27+
}
28+
}
29+
30+
protected virtual global::System.BinaryData PersistableModelWriteCore(global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
31+
{
32+
string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.BaseModel>)this).GetFormatFromOptions(options) : options.Format;
33+
switch (format)
34+
{
35+
case "J":
36+
return global::System.ClientModel.Primitives.ModelReaderWriter.Write(this, options, global::Sample.SampleContext.Default);
37+
default:
38+
throw new global::System.FormatException($"The model {nameof(global::Sample.Models.BaseModel)} does not support writing '{options.Format}' format.");
39+
}
40+
}
41+
42+
global::System.BinaryData global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.BaseModel>.Write(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelWriteCore(options);
43+
44+
global::Sample.Models.BaseModel global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.BaseModel>.Create(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelCreateCore(data, options);
45+
46+
string global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.BaseModel>.GetFormatFromOptions(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => "J";
47+
48+
void global::System.ClientModel.Primitives.IJsonModel<global::Sample.Models.BaseModel>.Write(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
49+
{
50+
writer.WriteStartObject();
51+
this.JsonModelWriteCore(writer, options);
52+
writer.WriteEndObject();
53+
}
54+
55+
protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
56+
{
57+
string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.BaseModel>)this).GetFormatFromOptions(options) : options.Format;
58+
if ((format != "J"))
59+
{
60+
throw new global::System.FormatException($"The model {nameof(global::Sample.Models.BaseModel)} does not support writing '{format}' format.");
61+
}
62+
writer.WritePropertyName("kind"u8);
63+
writer.WriteStringValue(Kind);
64+
if (((options.Format != "W") && (_additionalBinaryDataProperties != null)))
65+
{
66+
foreach (var item in _additionalBinaryDataProperties)
67+
{
68+
writer.WritePropertyName(item.Key);
69+
#if NET6_0_OR_GREATER
70+
writer.WriteRawValue(item.Value);
71+
#else
72+
using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(item.Value))
73+
{
74+
global::System.Text.Json.JsonSerializer.Serialize(writer, document.RootElement);
75+
}
76+
#endif
77+
}
78+
}
79+
}
80+
81+
global::Sample.Models.BaseModel global::System.ClientModel.Primitives.IJsonModel<global::Sample.Models.BaseModel>.Create(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.JsonModelCreateCore(ref reader, options);
82+
83+
protected virtual global::Sample.Models.BaseModel JsonModelCreateCore(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
84+
{
85+
string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.BaseModel>)this).GetFormatFromOptions(options) : options.Format;
86+
if ((format != "J"))
87+
{
88+
throw new global::System.FormatException($"The model {nameof(global::Sample.Models.BaseModel)} does not support reading '{format}' format.");
89+
}
90+
using global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.ParseValue(ref reader);
91+
return global::Sample.Models.BaseModel.DeserializeBaseModel(document.RootElement, options);
92+
}
93+
94+
internal static global::Sample.Models.BaseModel DeserializeBaseModel(global::System.Text.Json.JsonElement element, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
95+
{
96+
if ((element.ValueKind == global::System.Text.Json.JsonValueKind.Null))
97+
{
98+
return null;
99+
}
100+
if (element.TryGetProperty("kind"u8, out global::System.Text.Json.JsonElement discriminator))
101+
{
102+
switch (discriminator.GetString())
103+
{
104+
case "one":
105+
return global::Sample.Models.DerivedModel.DeserializeDerivedModel(element, options);
106+
}
107+
}
108+
return global::Sample.Models.UnknownBaseModel.DeserializeUnknownBaseModel(element, options);
109+
}
110+
}
111+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
namespace Sample.Models
2+
{
3+
public abstract partial class BaseModel
4+
{
5+
/// <summary> Initializes a new instance of BaseModel. </summary>
6+
protected BaseModel()
7+
{
8+
}
9+
}
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
namespace Sample.Models
2+
{
3+
public partial struct StructModel
4+
{
5+
/// <summary> Initializes a new instance of StructModel. </summary>
6+
public StructModel()
7+
{
8+
}
9+
}
10+
}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,11 @@ public enum BackCompatibilityChangeCategory
5454

5555
/// <summary>A fixed enum member was re-added to preserve a member that existed in the last contract but is no longer produced by the current spec.</summary>
5656
EnumMemberAddedFromLastContract,
57+
58+
/// <summary>A back-compat model constructor was re-added to preserve a public constructor that existed in the last contract but is no longer produced by the current spec.</summary>
59+
ConstructorAddedFromLastContract,
60+
61+
/// <summary>A back-compat model constructor could not be reconstructed from the last contract and was skipped.</summary>
62+
ConstructorAddedFromLastContractSkipped,
5763
}
5864
}

0 commit comments

Comments
 (0)