From 7e4d72d0d600d27ecb5ac3e94e9ea48f6be210e9 Mon Sep 17 00:00:00 2001 From: jolov Date: Tue, 16 Jun 2026 17:29:28 -0700 Subject: [PATCH 1/3] fix(http-client-csharp): forward discriminator to base ctor for external base models External base models marked via @alternateType now map to a SystemObjectModelProvider (a ModelProvider) in TypeFactory.CreateModel, so derived discriminated types emit the correct base constructor call with the discriminator value, and the external base type itself is not generated. Handling this in CreateModel (rather than the overridable CreateModelCore) ensures all generators, including the Scm client generator, share the behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...-base-discriminator-2026-06-16-17-20-40.md | 7 ++ .../src/OutputLibrary.cs | 2 +- .../src/TypeFactory.cs | 28 ++++- .../SystemObjectModelProviderTests.cs | 111 ++++++++++++++++++ 4 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 .chronus/changes/joshlove-msft-csharp-external-base-discriminator-2026-06-16-17-20-40.md diff --git a/.chronus/changes/joshlove-msft-csharp-external-base-discriminator-2026-06-16-17-20-40.md b/.chronus/changes/joshlove-msft-csharp-external-base-discriminator-2026-06-16-17-20-40.md new file mode 100644 index 00000000000..818694e0ec9 --- /dev/null +++ b/.chronus/changes/joshlove-msft-csharp-external-base-discriminator-2026-06-16-17-20-40.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-client-csharp" +--- + +Forward the discriminator value to the base constructor when a discriminated base model is marked as an external type. External base models now map to a model provider so derived types emit the correct `: base("")` call and the external base itself is not generated. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/OutputLibrary.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/OutputLibrary.cs index 72a17dc74c7..7ee3ab7b5d3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/OutputLibrary.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/OutputLibrary.cs @@ -52,7 +52,7 @@ private static TypeProvider[] BuildModels() foreach (var inputModel in input.Models) { var outputModel = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel); - if (outputModel != null) + if (outputModel != null && outputModel is not SystemObjectModelProvider) { models.Add(outputModel); var unknownVariant = inputModel.DiscriminatedSubtypes.Values.FirstOrDefault(m => m.IsUnknownDiscriminatorModel); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs index c207c3e130b..1f509b16156 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs @@ -181,7 +181,13 @@ protected internal TypeFactory() // (e.g., when BuildBaseModelProvider triggers CreateModel for all input models). InputTypeToModelProvider[model] = null; - modelProvider = CreateModelCore(model); + // A model marked as external maps to a type that already exists in a framework or + // referenced assembly, so it must not be generated. Represent it with a + // SystemObjectModelProvider so that generated models deriving from it still get a + // ModelProvider base (enabling constructor chaining and discriminator forwarding) + // rather than a bare TypeProvider that cannot serve as a base model. This is handled + // here, before the overridable CreateModelCore, so all generators share the behavior. + modelProvider = CreateExternalModel(model) ?? CreateModelCore(model); foreach (var visitor in Visitors) { @@ -203,7 +209,25 @@ protected internal TypeFactory() return modelProvider; } - protected virtual ModelProvider? CreateModelCore(InputModelType model) => new ModelProvider(model); + protected virtual ModelProvider? CreateModelCore(InputModelType model) + => new ModelProvider(model); + + /// + /// Maps an external (one marked via @alternateType) to a + /// that wraps the resolved framework/referenced type. + /// Returns null when the model is not external or the external type cannot be resolved, + /// in which case normal model creation proceeds. + /// + private SystemObjectModelProvider? CreateExternalModel(InputModelType model) + { + if (model.External == null) + { + return null; + } + + var externalType = CreateExternalType(model.External); + return externalType != null ? new SystemObjectModelProvider(externalType, model) : null; + } /// /// Factory method for creating the that emits the diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/SystemObjectModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/SystemObjectModelProviderTests.cs index f58ce631ff7..12ef3f7e632 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/SystemObjectModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/SystemObjectModelProviderTests.cs @@ -370,5 +370,116 @@ public void Constructor_ThrowsOnNullSystemType() var inputModel = InputFactory.Model("Resource", properties: []); Assert.Throws(() => new SystemObjectModelProvider(null!, inputModel)); } + + // ------------------------------------------------------------------- + // 9. A derived discriminated model forwards its discriminator value to a + // SystemObjectModelProvider base constructor. This is impossible with + // SystemObjectTypeProvider because it cannot serve as a BaseModelProvider. + // ------------------------------------------------------------------- + + [Test] + public void DerivedDiscriminatedModel_ForwardsDiscriminatorToSystemObjectModelProviderBase() + { + var baseInputModel = InputFactory.Model( + "BaseModel", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + ]); + var derivedInputModel = InputFactory.Model( + "DerivedModel", + baseModel: baseInputModel, + discriminatedKind: "one", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("color", InputPrimitiveType.String, isRequired: true), + ]); + + var systemType = CreateSystemCSharpType("BaseModelData", "TestFramework"); + MockHelpers.LoadMockGenerator( + inputModelTypes: [baseInputModel, derivedInputModel], + createModelCore: (model) => + model.Name == "BaseModel" + ? new SystemObjectModelProvider(systemType, model) + : new ModelProvider(model)); + + var derivedProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(derivedInputModel) as ModelProvider; + Assert.IsNotNull(derivedProvider); + Assert.IsInstanceOf(derivedProvider!.BaseModelProvider); + + var publicCtor = derivedProvider.Constructors.FirstOrDefault( + c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsNotNull(publicCtor); + + var initializer = publicCtor!.Signature.Initializer; + Assert.IsNotNull(initializer); + Assert.IsTrue(initializer!.IsBase); + + // The base constructor call must forward the discriminator literal "one". + Assert.IsTrue( + initializer.Arguments.Any(a => a.ToDisplayString() == "\"one\""), + "Expected the base constructor call to forward the discriminator value \"one\". " + + "Actual arguments: " + string.Join(", ", initializer.Arguments.Select(a => a.ToDisplayString()))); + } + + // ------------------------------------------------------------------- + // 10. End-to-end: a base model marked external (External metadata) is mapped to a + // SystemObjectModelProvider by the default factory, is not emitted, and a derived + // discriminated model forwards its discriminator value to the external base. + // ------------------------------------------------------------------- + + [Test] + public void ExternalBaseModel_MapsToSystemObjectModelProvider_AndForwardsDiscriminator() + { + var baseInputModel = InputFactory.Model( + "BaseModel", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + ], + external: new InputExternalTypeMetadata("System.Exception", null, null)); + var derivedInputModel = InputFactory.Model( + "DerivedModel", + baseModel: baseInputModel, + discriminatedKind: "one", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("color", InputPrimitiveType.String, isRequired: true), + ]); + + // No createModelCore override: the default (real) CreateModelCore must perform the mapping. + var mockGenerator = MockHelpers.LoadMockGenerator( + inputModelTypes: [baseInputModel, derivedInputModel]); + + // The external base maps to a SystemObjectModelProvider rather than a generated model. + var baseProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(baseInputModel); + Assert.IsInstanceOf(baseProvider); + + // The derived model uses it as its base model provider. + var derivedProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(derivedInputModel) as ModelProvider; + Assert.IsNotNull(derivedProvider); + Assert.IsInstanceOf(derivedProvider!.BaseModelProvider); + + // The derived constructor forwards the discriminator value to the external base. + var publicCtor = derivedProvider.Constructors.FirstOrDefault( + c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsNotNull(publicCtor); + var initializer = publicCtor!.Signature.Initializer; + Assert.IsNotNull(initializer); + Assert.IsTrue(initializer!.IsBase); + Assert.IsTrue( + initializer.Arguments.Any(a => a.ToDisplayString() == "\"one\""), + "Expected the base constructor call to forward the discriminator value \"one\". " + + "Actual arguments: " + string.Join(", ", initializer.Arguments.Select(a => a.ToDisplayString()))); + + // The external base is not emitted as a generated type. + Assert.IsFalse( + CodeModelGenerator.Instance.OutputLibrary.TypeProviders.Any(t => t is SystemObjectModelProvider), + "External base models should not be emitted as generated types."); + } } } From dabef4a7c080413ac2230a2eca2c52e77ad029b8 Mon Sep 17 00:00:00 2001 From: jolov Date: Tue, 16 Jun 2026 17:31:28 -0700 Subject: [PATCH 2/3] Remove changeset Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...harp-external-base-discriminator-2026-06-16-17-20-40.md | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 .chronus/changes/joshlove-msft-csharp-external-base-discriminator-2026-06-16-17-20-40.md diff --git a/.chronus/changes/joshlove-msft-csharp-external-base-discriminator-2026-06-16-17-20-40.md b/.chronus/changes/joshlove-msft-csharp-external-base-discriminator-2026-06-16-17-20-40.md deleted file mode 100644 index 818694e0ec9..00000000000 --- a/.chronus/changes/joshlove-msft-csharp-external-base-discriminator-2026-06-16-17-20-40.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -changeKind: fix -packages: - - "@typespec/http-client-csharp" ---- - -Forward the discriminator value to the base constructor when a discriminated base model is marked as an external type. External base models now map to a model provider so derived types emit the correct `: base("")` call and the external base itself is not generated. From f6b56c297a5015e9704bd55d88413ad4ebaaf8e1 Mon Sep 17 00:00:00 2001 From: jolov Date: Tue, 16 Jun 2026 21:50:20 -0700 Subject: [PATCH 3/3] test(http-client-csharp): add coverage for external base discriminator forwarding - ScmTypeFactoryTests: verify an external base model maps to SystemObjectModelProvider and the discriminator is forwarded through the Scm factory (which overrides CreateModelCore) - guards against regressing the fix back into CreateModelCore only. - SystemObjectModelProviderTests: external model that cannot be resolved falls back to normal generation; a property typed as an external model resolves to the external type while the external model itself is not generated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/ScmTypeFactoryTests.cs | 49 +++++++++++++++ .../SystemObjectModelProviderTests.cs | 62 +++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ScmTypeFactoryTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ScmTypeFactoryTests.cs index e37473bbd3c..98f1b9a5fd0 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ScmTypeFactoryTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ScmTypeFactoryTests.cs @@ -7,6 +7,8 @@ using System.Linq; using Microsoft.TypeSpec.Generator.ClientModel.Providers; using Microsoft.TypeSpec.Generator.Input; +using Microsoft.TypeSpec.Generator.Primitives; +using Microsoft.TypeSpec.Generator.Providers; using Microsoft.TypeSpec.Generator.Tests.Common; using NUnit.Framework; @@ -144,6 +146,53 @@ public void TestCreateSerializations_ReturnsBothMrwAndMultipart_WhenJsonAndMpfdU "Expected a multipart serialization provider for a model with MultipartFormData usage."); } + // ScmTypeFactory overrides CreateModelCore to return ScmModelProvider. External-type + // handling lives in the (non-overridable) base TypeFactory.CreateModel, so it must still + // apply here. This guards against regressing the fix by re-introducing external handling + // into CreateModelCore only, which would silently drop the discriminator for the Scm path. + [Test] + public void ExternalBaseModel_MapsToSystemObjectModelProvider_AndForwardsDiscriminator_ThroughScmTypeFactory() + { + var baseModel = InputFactory.Model( + "Animal", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + ], + external: new InputExternalTypeMetadata("System.Exception", null, null)); + var derivedModel = InputFactory.Model( + "Pet", + baseModel: baseModel, + discriminatedKind: "pet", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("trained", InputPrimitiveType.Boolean, isRequired: true), + ]); + + MockHelpers.LoadMockGenerator(inputModels: () => [baseModel, derivedModel]); + + // The external base maps to a SystemObjectModelProvider even though ScmTypeFactory + // overrides CreateModelCore. + var baseProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(baseModel); + Assert.IsInstanceOf(baseProvider); + + // The derived model is a regular ScmModelProvider whose base is the SystemObjectModelProvider. + var derivedProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(derivedModel) as ModelProvider; + Assert.IsNotNull(derivedProvider); + Assert.IsInstanceOf(derivedProvider); + Assert.IsInstanceOf(derivedProvider!.BaseModelProvider); + + // Some constructor forwards the discriminator literal to the external base. + var forwardsDiscriminator = derivedProvider.Constructors.Any( + c => c.Signature.Initializer is { IsBase: true } init && + init.Arguments.Any(a => a.ToDisplayString() == "\"pet\"")); + Assert.IsTrue( + forwardsDiscriminator, + "Expected a base constructor call forwarding the discriminator value \"pet\" to the external base."); + } + private static InputModelProperty FilePartProperty(string name) => InputFactory.Property( name, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/SystemObjectModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/SystemObjectModelProviderTests.cs index 12ef3f7e632..8e36226503a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/SystemObjectModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/SystemObjectModelProviderTests.cs @@ -481,5 +481,67 @@ public void ExternalBaseModel_MapsToSystemObjectModelProvider_AndForwardsDiscrim CodeModelGenerator.Instance.OutputLibrary.TypeProviders.Any(t => t is SystemObjectModelProvider), "External base models should not be emitted as generated types."); } + + // ------------------------------------------------------------------- + // 11. Fallback: an external model whose type cannot be resolved is generated + // normally (as a regular ModelProvider) rather than being dropped. + // ------------------------------------------------------------------- + + [Test] + public void ExternalModel_ThatCannotBeResolved_FallsBackToNormalGeneration() + { + var inputModel = InputFactory.Model( + "Widget", + properties: [InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)], + // Not a real framework type and no package metadata, so resolution fails. + external: new InputExternalTypeMetadata("Some.Unresolvable.ExternalType", null, null)); + + MockHelpers.LoadMockGenerator(inputModelTypes: [inputModel]); + + var provider = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel); + + // Unresolvable external metadata: no SystemObjectModelProvider mapping; generate normally. + Assert.IsNotNull(provider); + Assert.IsNotInstanceOf(provider); + + // And the model is still emitted as a generated type. + Assert.IsTrue( + CodeModelGenerator.Instance.OutputLibrary.TypeProviders.Any(t => t == provider), + "An external model that cannot be resolved should still be generated."); + } + + // ------------------------------------------------------------------- + // 12. A property typed as an external model resolves to the external type, and the + // external model itself is not generated. + // ------------------------------------------------------------------- + + [Test] + public void PropertyTypedAsExternalModel_ResolvesToExternalType_AndExternalModelIsNotGenerated() + { + var externalModel = InputFactory.Model( + "ExternalThing", + properties: [InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)], + external: new InputExternalTypeMetadata("System.Exception", null, null)); + var containerModel = InputFactory.Model( + "Container", + properties: [InputFactory.Property("thing", externalModel, isRequired: true)]); + + MockHelpers.LoadMockGenerator(inputModelTypes: [externalModel, containerModel]); + + // The external model maps to a SystemObjectModelProvider and is not emitted. + var externalProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(externalModel); + Assert.IsInstanceOf(externalProvider); + Assert.IsFalse( + CodeModelGenerator.Instance.OutputLibrary.TypeProviders.Any(t => t is SystemObjectModelProvider), + "External models must not be emitted as generated types."); + + // A property typed as the external model resolves to the external framework type. + var container = CodeModelGenerator.Instance.TypeFactory.CreateModel(containerModel) as ModelProvider; + Assert.IsNotNull(container); + var thingProperty = container!.Properties.FirstOrDefault(p => p.Name == "Thing"); + Assert.IsNotNull(thingProperty); + Assert.AreEqual("Exception", thingProperty!.Type.Name); + Assert.AreEqual("System", thingProperty.Type.Namespace); + } } }