Skip to content

Commit dffc2b0

Browse files
fix(http-client-csharp): forward discriminator to base ctor for external base models (#11002)
## Problem Discriminated subtypes whose base model is marked as an external type (via the TCGC `@alternateType` decorator) did not forward the discriminator value in the generated base constructor call. When the base was external, it became a bare `TypeProvider` that cannot serve as a `BaseModelProvider`, so no `: base(...)` initializer was emitted and the discriminator literal was dropped. ## Fix Route external `InputModelType`s through `SystemObjectModelProvider` (a real `ModelProvider`) so the existing constructor / discriminator-forwarding logic applies: - `TypeFactory.CreateModel`: external models now map to a `SystemObjectModelProvider` wrapping the resolved framework/referenced type, handled **before** the overridable `CreateModelCore` so all generators (including the Scm client generator, whose `ScmTypeFactory.CreateModelCore` override previously bypassed it) share the behavior. - `OutputLibrary.BuildModels`: external bases mapped to `SystemObjectModelProvider` are excluded from emission (the external type already exists in a referenced assembly). ## Result For a discriminated base `Animal` marked external, derived types now emit: ```csharp public partial class Pet : <ExternalBase> { internal Pet(string name, bool trained) : base("pet", name) { ... } } public partial class Dog : Pet { internal Dog(string name, bool trained, string breed) : base("dog", name, trained) { ... } } ``` and the external base type itself is not generated. ## Validation - Added unit + e2e tests in `SystemObjectModelProviderTests`. - Sample-TypeSpec end-to-end run confirmed the discriminator is forwarded through the real Scm emitter and the external base is excluded. - Full suites pass: `Microsoft.TypeSpec.Generator.Tests` (1536) and `Microsoft.TypeSpec.Generator.ClientModel.Tests` (1429); C# formatting clean. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c38a789 commit dffc2b0

4 files changed

Lines changed: 249 additions & 3 deletions

File tree

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ScmTypeFactoryTests.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
using System.Linq;
88
using Microsoft.TypeSpec.Generator.ClientModel.Providers;
99
using Microsoft.TypeSpec.Generator.Input;
10+
using Microsoft.TypeSpec.Generator.Primitives;
11+
using Microsoft.TypeSpec.Generator.Providers;
1012
using Microsoft.TypeSpec.Generator.Tests.Common;
1113
using NUnit.Framework;
1214

@@ -144,6 +146,53 @@ public void TestCreateSerializations_ReturnsBothMrwAndMultipart_WhenJsonAndMpfdU
144146
"Expected a multipart serialization provider for a model with MultipartFormData usage.");
145147
}
146148

149+
// ScmTypeFactory overrides CreateModelCore to return ScmModelProvider. External-type
150+
// handling lives in the (non-overridable) base TypeFactory.CreateModel, so it must still
151+
// apply here. This guards against regressing the fix by re-introducing external handling
152+
// into CreateModelCore only, which would silently drop the discriminator for the Scm path.
153+
[Test]
154+
public void ExternalBaseModel_MapsToSystemObjectModelProvider_AndForwardsDiscriminator_ThroughScmTypeFactory()
155+
{
156+
var baseModel = InputFactory.Model(
157+
"Animal",
158+
properties:
159+
[
160+
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
161+
InputFactory.Property("name", InputPrimitiveType.String, isRequired: true),
162+
],
163+
external: new InputExternalTypeMetadata("System.Exception", null, null));
164+
var derivedModel = InputFactory.Model(
165+
"Pet",
166+
baseModel: baseModel,
167+
discriminatedKind: "pet",
168+
properties:
169+
[
170+
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
171+
InputFactory.Property("trained", InputPrimitiveType.Boolean, isRequired: true),
172+
]);
173+
174+
MockHelpers.LoadMockGenerator(inputModels: () => [baseModel, derivedModel]);
175+
176+
// The external base maps to a SystemObjectModelProvider even though ScmTypeFactory
177+
// overrides CreateModelCore.
178+
var baseProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(baseModel);
179+
Assert.IsInstanceOf<SystemObjectModelProvider>(baseProvider);
180+
181+
// The derived model is a regular ScmModelProvider whose base is the SystemObjectModelProvider.
182+
var derivedProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(derivedModel) as ModelProvider;
183+
Assert.IsNotNull(derivedProvider);
184+
Assert.IsInstanceOf<ScmModelProvider>(derivedProvider);
185+
Assert.IsInstanceOf<SystemObjectModelProvider>(derivedProvider!.BaseModelProvider);
186+
187+
// Some constructor forwards the discriminator literal to the external base.
188+
var forwardsDiscriminator = derivedProvider.Constructors.Any(
189+
c => c.Signature.Initializer is { IsBase: true } init &&
190+
init.Arguments.Any(a => a.ToDisplayString() == "\"pet\""));
191+
Assert.IsTrue(
192+
forwardsDiscriminator,
193+
"Expected a base constructor call forwarding the discriminator value \"pet\" to the external base.");
194+
}
195+
147196
private static InputModelProperty FilePartProperty(string name)
148197
=> InputFactory.Property(
149198
name,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ private static TypeProvider[] BuildModels()
5252
foreach (var inputModel in input.Models)
5353
{
5454
var outputModel = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel);
55-
if (outputModel != null)
55+
if (outputModel != null && outputModel is not SystemObjectModelProvider)
5656
{
5757
models.Add(outputModel);
5858
var unknownVariant = inputModel.DiscriminatedSubtypes.Values.FirstOrDefault(m => m.IsUnknownDiscriminatorModel);

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,13 @@ protected internal TypeFactory()
181181
// (e.g., when BuildBaseModelProvider triggers CreateModel for all input models).
182182
InputTypeToModelProvider[model] = null;
183183

184-
modelProvider = CreateModelCore(model);
184+
// A model marked as external maps to a type that already exists in a framework or
185+
// referenced assembly, so it must not be generated. Represent it with a
186+
// SystemObjectModelProvider so that generated models deriving from it still get a
187+
// ModelProvider base (enabling constructor chaining and discriminator forwarding)
188+
// rather than a bare TypeProvider that cannot serve as a base model. This is handled
189+
// here, before the overridable CreateModelCore, so all generators share the behavior.
190+
modelProvider = CreateExternalModel(model) ?? CreateModelCore(model);
185191

186192
foreach (var visitor in Visitors)
187193
{
@@ -203,7 +209,25 @@ protected internal TypeFactory()
203209
return modelProvider;
204210
}
205211

206-
protected virtual ModelProvider? CreateModelCore(InputModelType model) => new ModelProvider(model);
212+
protected virtual ModelProvider? CreateModelCore(InputModelType model)
213+
=> new ModelProvider(model);
214+
215+
/// <summary>
216+
/// Maps an external <see cref="InputModelType"/> (one marked via <c>@alternateType</c>) to a
217+
/// <see cref="SystemObjectModelProvider"/> that wraps the resolved framework/referenced type.
218+
/// Returns <c>null</c> when the model is not external or the external type cannot be resolved,
219+
/// in which case normal model creation proceeds.
220+
/// </summary>
221+
private SystemObjectModelProvider? CreateExternalModel(InputModelType model)
222+
{
223+
if (model.External == null)
224+
{
225+
return null;
226+
}
227+
228+
var externalType = CreateExternalType(model.External);
229+
return externalType != null ? new SystemObjectModelProvider(externalType, model) : null;
230+
}
207231

208232
/// <summary>
209233
/// Factory method for creating the <see cref="ModelFactoryProvider"/> that emits the

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

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,5 +370,178 @@ public void Constructor_ThrowsOnNullSystemType()
370370
var inputModel = InputFactory.Model("Resource", properties: []);
371371
Assert.Throws<ArgumentNullException>(() => new SystemObjectModelProvider(null!, inputModel));
372372
}
373+
374+
// -------------------------------------------------------------------
375+
// 9. A derived discriminated model forwards its discriminator value to a
376+
// SystemObjectModelProvider base constructor. This is impossible with
377+
// SystemObjectTypeProvider because it cannot serve as a BaseModelProvider.
378+
// -------------------------------------------------------------------
379+
380+
[Test]
381+
public void DerivedDiscriminatedModel_ForwardsDiscriminatorToSystemObjectModelProviderBase()
382+
{
383+
var baseInputModel = InputFactory.Model(
384+
"BaseModel",
385+
properties:
386+
[
387+
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
388+
InputFactory.Property("name", InputPrimitiveType.String, isRequired: true),
389+
]);
390+
var derivedInputModel = InputFactory.Model(
391+
"DerivedModel",
392+
baseModel: baseInputModel,
393+
discriminatedKind: "one",
394+
properties:
395+
[
396+
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
397+
InputFactory.Property("color", InputPrimitiveType.String, isRequired: true),
398+
]);
399+
400+
var systemType = CreateSystemCSharpType("BaseModelData", "TestFramework");
401+
MockHelpers.LoadMockGenerator(
402+
inputModelTypes: [baseInputModel, derivedInputModel],
403+
createModelCore: (model) =>
404+
model.Name == "BaseModel"
405+
? new SystemObjectModelProvider(systemType, model)
406+
: new ModelProvider(model));
407+
408+
var derivedProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(derivedInputModel) as ModelProvider;
409+
Assert.IsNotNull(derivedProvider);
410+
Assert.IsInstanceOf<SystemObjectModelProvider>(derivedProvider!.BaseModelProvider);
411+
412+
var publicCtor = derivedProvider.Constructors.FirstOrDefault(
413+
c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
414+
Assert.IsNotNull(publicCtor);
415+
416+
var initializer = publicCtor!.Signature.Initializer;
417+
Assert.IsNotNull(initializer);
418+
Assert.IsTrue(initializer!.IsBase);
419+
420+
// The base constructor call must forward the discriminator literal "one".
421+
Assert.IsTrue(
422+
initializer.Arguments.Any(a => a.ToDisplayString() == "\"one\""),
423+
"Expected the base constructor call to forward the discriminator value \"one\". " +
424+
"Actual arguments: " + string.Join(", ", initializer.Arguments.Select(a => a.ToDisplayString())));
425+
}
426+
427+
// -------------------------------------------------------------------
428+
// 10. End-to-end: a base model marked external (External metadata) is mapped to a
429+
// SystemObjectModelProvider by the default factory, is not emitted, and a derived
430+
// discriminated model forwards its discriminator value to the external base.
431+
// -------------------------------------------------------------------
432+
433+
[Test]
434+
public void ExternalBaseModel_MapsToSystemObjectModelProvider_AndForwardsDiscriminator()
435+
{
436+
var baseInputModel = InputFactory.Model(
437+
"BaseModel",
438+
properties:
439+
[
440+
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
441+
InputFactory.Property("name", InputPrimitiveType.String, isRequired: true),
442+
],
443+
external: new InputExternalTypeMetadata("System.Exception", null, null));
444+
var derivedInputModel = InputFactory.Model(
445+
"DerivedModel",
446+
baseModel: baseInputModel,
447+
discriminatedKind: "one",
448+
properties:
449+
[
450+
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
451+
InputFactory.Property("color", InputPrimitiveType.String, isRequired: true),
452+
]);
453+
454+
// No createModelCore override: the default (real) CreateModelCore must perform the mapping.
455+
var mockGenerator = MockHelpers.LoadMockGenerator(
456+
inputModelTypes: [baseInputModel, derivedInputModel]);
457+
458+
// The external base maps to a SystemObjectModelProvider rather than a generated model.
459+
var baseProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(baseInputModel);
460+
Assert.IsInstanceOf<SystemObjectModelProvider>(baseProvider);
461+
462+
// The derived model uses it as its base model provider.
463+
var derivedProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(derivedInputModel) as ModelProvider;
464+
Assert.IsNotNull(derivedProvider);
465+
Assert.IsInstanceOf<SystemObjectModelProvider>(derivedProvider!.BaseModelProvider);
466+
467+
// The derived constructor forwards the discriminator value to the external base.
468+
var publicCtor = derivedProvider.Constructors.FirstOrDefault(
469+
c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
470+
Assert.IsNotNull(publicCtor);
471+
var initializer = publicCtor!.Signature.Initializer;
472+
Assert.IsNotNull(initializer);
473+
Assert.IsTrue(initializer!.IsBase);
474+
Assert.IsTrue(
475+
initializer.Arguments.Any(a => a.ToDisplayString() == "\"one\""),
476+
"Expected the base constructor call to forward the discriminator value \"one\". " +
477+
"Actual arguments: " + string.Join(", ", initializer.Arguments.Select(a => a.ToDisplayString())));
478+
479+
// The external base is not emitted as a generated type.
480+
Assert.IsFalse(
481+
CodeModelGenerator.Instance.OutputLibrary.TypeProviders.Any(t => t is SystemObjectModelProvider),
482+
"External base models should not be emitted as generated types.");
483+
}
484+
485+
// -------------------------------------------------------------------
486+
// 11. Fallback: an external model whose type cannot be resolved is generated
487+
// normally (as a regular ModelProvider) rather than being dropped.
488+
// -------------------------------------------------------------------
489+
490+
[Test]
491+
public void ExternalModel_ThatCannotBeResolved_FallsBackToNormalGeneration()
492+
{
493+
var inputModel = InputFactory.Model(
494+
"Widget",
495+
properties: [InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)],
496+
// Not a real framework type and no package metadata, so resolution fails.
497+
external: new InputExternalTypeMetadata("Some.Unresolvable.ExternalType", null, null));
498+
499+
MockHelpers.LoadMockGenerator(inputModelTypes: [inputModel]);
500+
501+
var provider = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel);
502+
503+
// Unresolvable external metadata: no SystemObjectModelProvider mapping; generate normally.
504+
Assert.IsNotNull(provider);
505+
Assert.IsNotInstanceOf<SystemObjectModelProvider>(provider);
506+
507+
// And the model is still emitted as a generated type.
508+
Assert.IsTrue(
509+
CodeModelGenerator.Instance.OutputLibrary.TypeProviders.Any(t => t == provider),
510+
"An external model that cannot be resolved should still be generated.");
511+
}
512+
513+
// -------------------------------------------------------------------
514+
// 12. A property typed as an external model resolves to the external type, and the
515+
// external model itself is not generated.
516+
// -------------------------------------------------------------------
517+
518+
[Test]
519+
public void PropertyTypedAsExternalModel_ResolvesToExternalType_AndExternalModelIsNotGenerated()
520+
{
521+
var externalModel = InputFactory.Model(
522+
"ExternalThing",
523+
properties: [InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)],
524+
external: new InputExternalTypeMetadata("System.Exception", null, null));
525+
var containerModel = InputFactory.Model(
526+
"Container",
527+
properties: [InputFactory.Property("thing", externalModel, isRequired: true)]);
528+
529+
MockHelpers.LoadMockGenerator(inputModelTypes: [externalModel, containerModel]);
530+
531+
// The external model maps to a SystemObjectModelProvider and is not emitted.
532+
var externalProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(externalModel);
533+
Assert.IsInstanceOf<SystemObjectModelProvider>(externalProvider);
534+
Assert.IsFalse(
535+
CodeModelGenerator.Instance.OutputLibrary.TypeProviders.Any(t => t is SystemObjectModelProvider),
536+
"External models must not be emitted as generated types.");
537+
538+
// A property typed as the external model resolves to the external framework type.
539+
var container = CodeModelGenerator.Instance.TypeFactory.CreateModel(containerModel) as ModelProvider;
540+
Assert.IsNotNull(container);
541+
var thingProperty = container!.Properties.FirstOrDefault(p => p.Name == "Thing");
542+
Assert.IsNotNull(thingProperty);
543+
Assert.AreEqual("Exception", thingProperty!.Type.Name);
544+
Assert.AreEqual("System", thingProperty.Type.Namespace);
545+
}
373546
}
374547
}

0 commit comments

Comments
 (0)