Skip to content

Commit c6bc569

Browse files
Use named arguments in ModelFactory backward-compat overloads (#10404)
`TryBuildMethodArgumentsForOverload` emits positional arguments when calling the current ModelFactory method from a backward-compat overload. When `@@hierarchyBuilding` reorders constructor parameters, positional args map to wrong positions, causing CS1503. ### Changes - **`ModelFactoryProvider.cs`**: Wrap matched parameters in `PositionalParameterReferenceExpression` so all arguments are named, not just the defaulted ones for new parameters. - **`ModelFactoryProviderTests.cs`**: Update existing assertion to expect named args. Add `BackCompatibility_NewPropertyAddedWithDifferentParamOrder` test covering the reordered-params-plus-new-property scenario. Before: ```csharp return PublicModel1(stringProp, modelProp, listProp, dictProp: default); ``` After: ```csharp return PublicModel1(stringProp: stringProp, modelProp: modelProp, listProp: listProp, dictProp: default); ``` --------- 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>
1 parent 3cd7c82 commit c6bc569

3 files changed

Lines changed: 75 additions & 2 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ private static bool TryBuildMethodArgumentsForOverload(
286286
}
287287
else
288288
{
289-
arguments.Add(previousParameter);
289+
arguments.Add(parameter.PositionalReference(previousParameter));
290290
}
291291
}
292292

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,54 @@ public async Task BackCompatibility_NewModelPropertyAdded()
193193
Assert.IsNotNull(body);
194194
var result = body!.ToDisplayString();
195195
Assert.AreEqual(
196-
"return PublicModel1(stringProp, modelProp, listProp, dictProp: default);\n",
196+
"return PublicModel1(stringProp: stringProp, modelProp: modelProp, listProp: listProp, dictProp: default);\n",
197+
result);
198+
}
199+
200+
// This test validates that when a new property is added AND the previous contract had a different
201+
// parameter ordering, the backward-compat overload uses named arguments to correctly call the current method.
202+
[Test]
203+
public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder()
204+
{
205+
_instance = (await MockHelpers.LoadMockGeneratorAsync(
206+
inputNamespaceName: "Sample.Namespace",
207+
inputModelTypes: ModelList,
208+
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync())).Object;
209+
210+
var modelFactory = _instance!.OutputLibrary.ModelFactory.Value;
211+
Assert.AreEqual("SampleNamespaceModelFactory", modelFactory.Name);
212+
213+
modelFactory.ProcessTypeForBackCompatibility();
214+
215+
var methods = modelFactory.Methods;
216+
// There should be an additional method for backward compatibility
217+
Assert.AreEqual(ModelList.Length - ModelList.Where(m => m.Access == "internal").Count() + 1, methods.Count);
218+
219+
var currentOverloadMethod = methods
220+
.FirstOrDefault(m => m.Signature.Name == "PublicModel1" && m.Signature.Parameters.Any(p => p.Name == "dictProp"));
221+
var backwardCompatibilityMethod = methods
222+
.FirstOrDefault(m => m.Signature.Name == "PublicModel1" && m.Signature.Parameters.All(p => p.Name != "dictProp"));
223+
Assert.IsNotNull(currentOverloadMethod);
224+
Assert.IsNotNull(backwardCompatibilityMethod);
225+
226+
// validate the signature of the backward compatibility method preserves the previous parameter order
227+
var parameters = backwardCompatibilityMethod!.Signature.Parameters;
228+
Assert.AreEqual(3, parameters.Count);
229+
Assert.AreEqual("modelProp", parameters[0].Name);
230+
Assert.AreEqual("stringProp", parameters[1].Name);
231+
Assert.AreEqual("listProp", parameters[2].Name);
232+
foreach (var param in parameters)
233+
{
234+
Assert.IsNull(param.DefaultValue);
235+
}
236+
237+
// validate the previous method body uses named arguments to ensure correct mapping
238+
// even though the parameter order differs between the previous and current methods
239+
var body = backwardCompatibilityMethod!.BodyStatements;
240+
Assert.IsNotNull(body);
241+
var result = body!.ToDisplayString();
242+
Assert.AreEqual(
243+
"return PublicModel1(stringProp: stringProp, modelProp: modelProp, listProp: listProp, dictProp: default);\n",
197244
result);
198245
}
199246

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using SampleTypeSpec;
2+
using System;
3+
using System.Collections.Generic;
4+
using System.Collections.ObjectModel;
5+
using Sample.Models;
6+
7+
namespace Sample.Namespace
8+
{
9+
public static partial class SampleNamespaceModelFactory
10+
{
11+
public static PublicModel1 PublicModel1(
12+
Thing modelProp = default,
13+
string stringProp = default,
14+
IEnumerable<string> listProp = default)
15+
{ }
16+
}
17+
}
18+
19+
namespace Sample.Models
20+
{
21+
public partial class PublicModel1
22+
{ }
23+
24+
public partial class Thing
25+
{ }
26+
}

0 commit comments

Comments
 (0)