From b4f5cd7d6da74d859ccbf8b86cd5598edc32d426 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Wed, 22 Oct 2025 10:39:56 -0700 Subject: [PATCH 01/12] minor: added long path support to prerequisite --- packages/http-client-csharp/CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/http-client-csharp/CONTRIBUTING.md b/packages/http-client-csharp/CONTRIBUTING.md index d32813e3357..53518283b9c 100644 --- a/packages/http-client-csharp/CONTRIBUTING.md +++ b/packages/http-client-csharp/CONTRIBUTING.md @@ -22,6 +22,7 @@ Before you begin, ensure you have the following installed: - [npm](https://docs.npmjs.com/downloading-and-installing-node-js-and-npm) - [Git](https://git-scm.com/) - [PowerShell](https://docs.microsoft.com/powershell/scripting/install/installing-powershell) (version 7.0 or higher, for advanced testing and code generation scenarios) +- [Long Path Support](https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=powershell#registry-setting-to-enable-long-paths) (Windows only required to avoid path length limitations during development) ## Getting Started From 428bbe2edc2e2f1e140bcb2effc3f506e7395709 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Thu, 13 Nov 2025 10:12:21 -0800 Subject: [PATCH 02/12] added multiple constructor for hierarchy building --- .../src/Providers/ConstructorType.cs | 20 ++ .../src/Providers/ModelProvider.cs | 210 +++++++++++++++++- 2 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs new file mode 100644 index 00000000000..7600fc9eb38 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +namespace Microsoft.TypeSpec.Generator.Providers +{ + /// + /// Represents the different types of constructors that can be generated for a model. + /// + public enum ConstructorType + { + /// Standard public/internal constructor + Standard, + /// Public constructor for direct instantiation (dual pattern) + PublicForInstantiation, + /// Private protected constructor for inheritance (dual pattern) + PrivateProtectedForInheritance, + /// Internal constructor for serialization + InternalForSerialization + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index ada1ec67c73..73a2e3017cc 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -19,6 +19,9 @@ namespace Microsoft.TypeSpec.Generator.Providers public class ModelProvider : TypeProvider { private const string AdditionalBinaryDataPropsFieldDescription = "Keeps track of any properties unknown to the library."; + private const string DiscriminatorParameterName = "discriminatorValue"; + private const string DiscriminatorParameterDescription = "The discriminator property."; + private readonly InputModelType _inputModel; // Note the description cannot be built from the constructor as it would lead to a circular dependency between the base // and derived models resulting in a stack overflow. @@ -522,7 +525,13 @@ protected override ConstructorProvider[] BuildConstructors() return [FullConstructor]; } - // Build the initialization constructor + // Check if this model needs the dual constructor pattern + if (ShouldHaveDualConstructorPattern()) + { + return BuildDualConstructorPattern(); + } + + // Build the standard single initialization constructor var accessibility = DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract) ? MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected : _inputModel.Usage.HasFlag(InputModelTypeUsage.Input) @@ -551,6 +560,186 @@ protected override ConstructorProvider[] BuildConstructors() return [constructor]; } + /// + /// Determines if this model should have a dual constructor pattern. + /// This is needed when the model shares the same discriminator property name as its base model, + /// indicating it's an intermediate type in a discriminated union hierarchy. + /// + private bool ShouldHaveDualConstructorPattern() + { + // Only applies to non-abstract models with a base model + if (_isAbstract || BaseModelProvider == null) + return false; + + // Check if this model has a discriminator property in the input + if (_inputModel.DiscriminatorProperty == null) + return false; + + // Check if base model has a discriminator property with the same name + if (BaseModelProvider._inputModel.DiscriminatorProperty == null) + return false; + + // If both models have discriminator properties with the same name, + // this model needs the dual constructor pattern + return string.Equals( + _inputModel.DiscriminatorProperty.Name, + BaseModelProvider._inputModel.DiscriminatorProperty.Name, + StringComparison.OrdinalIgnoreCase); + } + + /// + /// Builds the dual constructor pattern for models that share discriminator properties with their base. + /// Creates three constructors: + /// 1. Public constructor for external use + /// 2. Private protected constructor for derived models to call + /// 3. Internal constructor for serialization + /// + private ConstructorProvider[] BuildDualConstructorPattern() + { + // Build public constructor (without discriminator parameter) + var (publicParams, publicInitializer) = BuildConstructorParameters(ConstructorType.PublicForInstantiation); + var publicConstructor = new ConstructorProvider( + signature: new ConstructorSignature( + Type, + $"Initializes a new instance of {Type:C}", + MethodSignatureModifiers.Public, + publicParams, + initializer: publicInitializer), + bodyStatements: new MethodBodyStatement[] + { + GetPropertyInitializers(true, parameters: publicParams) + }, + this); + + // Build private protected constructor (with discriminator parameter) + var (protectedParams, protectedInitializer) = BuildConstructorParameters(ConstructorType.PrivateProtectedForInheritance); + var protectedConstructor = new ConstructorProvider( + signature: new ConstructorSignature( + Type, + $"Initializes a new instance of {Type:C}", + MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected, + protectedParams, + initializer: protectedInitializer), + bodyStatements: new MethodBodyStatement[] + { + GetPropertyInitializers(true, parameters: protectedParams) + }, + this); + + // Internal constructor is the full constructor + return [publicConstructor, protectedConstructor, FullConstructor]; + } + + /// + /// Builds constructor parameters based on the constructor type using a strategy pattern. + /// + private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildConstructorParameters( + ConstructorType constructorType) + { + return constructorType switch + { + ConstructorType.PublicForInstantiation => BuildPublicInstantiationParameters(), + ConstructorType.PrivateProtectedForInheritance => BuildPrivateProtectedInheritanceParameters(), + ConstructorType.InternalForSerialization => BuildConstructorParameters(false), // Use existing logic + ConstructorType.Standard => BuildConstructorParameters(true), // Use existing logic + _ => throw new ArgumentOutOfRangeException(nameof(constructorType)) + }; + } + + /// + /// Builds parameters for public instantiation constructor (filters out discriminator). + /// + private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildPublicInstantiationParameters() + { + var (standardParams, _) = BuildConstructorParameters(true); + + // Filter out discriminator parameter + var parameters = standardParams.Where(p => p.Property == null || !p.Property.IsDiscriminator).ToList(); + + // Create initializer that calls the private protected constructor + var initializer = CreatePrivateProtectedConstructorCall(parameters); + + return (parameters, initializer); + } + + /// + /// Builds parameters for private protected inheritance constructor (includes discriminator). + /// + private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildPrivateProtectedInheritanceParameters() + { + var (standardParams, standardInitializer) = BuildConstructorParameters(true); + + // Add discriminator parameter at the beginning + var parameters = PrependDiscriminatorParameter(standardParams); + + // Create initializer that passes discriminator parameter to base + var initializer = CreateBaseConstructorCallWithDiscriminatorParameter(standardInitializer, parameters.FirstOrDefault()); + + return (parameters, initializer); + } + + /// + /// Prepends the discriminator parameter to the beginning of a parameter list. + /// + private IReadOnlyList PrependDiscriminatorParameter(IReadOnlyList parameters) + { + var result = new List(); + + if (_inputModel.DiscriminatorProperty != null) + { + var discriminatorParam = new ParameterProvider( + DiscriminatorParameterName, + $"{DiscriminatorParameterDescription}", + new CSharpType(typeof(string))); + result.Add(discriminatorParam); + } + + result.AddRange(parameters); + return result; + } + + /// + /// Creates a constructor initializer that calls the private protected constructor with discriminator value. + /// + private ConstructorInitializer CreatePrivateProtectedConstructorCall(IReadOnlyList parameters) + { + var discriminatorValue = EnsureDiscriminatorValueExpression(); + var args = new List(); + + if (discriminatorValue != null) + { + args.Add(discriminatorValue); + } + + var overriddenProperties = CanonicalView.Properties.Where(p => p.BaseProperty is not null).Select(p => p.BaseProperty!).ToHashSet(); + args.AddRange(parameters.Select(p => GetExpressionForCtor(p, overriddenProperties, true))); + + return new ConstructorInitializer(false, args.ToArray()); // this(...) call + } + + /// + /// Creates a base constructor call that uses the discriminator parameter. + /// + private ConstructorInitializer? CreateBaseConstructorCallWithDiscriminatorParameter( + ConstructorInitializer? standardInitializer, + ParameterProvider? discriminatorParam) + { + if (BaseModelProvider?.Type is null || standardInitializer is null || discriminatorParam is null) + return standardInitializer; + + var originalArgs = standardInitializer.Arguments; + if (originalArgs.Count == 0) + return standardInitializer; + + var newArgs = new List + { + discriminatorParam.AsVariable() // Use parameter instead of hardcoded value + }; + newArgs.AddRange(originalArgs.Skip(1)); // Skip original discriminator value + + return new ConstructorInitializer(standardInitializer.IsBase, newArgs); + } + /// /// Builds the internal constructor for the model which contains all public properties /// as parameters. @@ -656,7 +845,24 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( } // construct the initializer using the parameters from base signature - var constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))]); + ConstructorInitializer? constructorInitializer = null; + if (baseParameters.Count > 0) + { + // Check if base model has dual constructor pattern and we should call private protected constructor + if (isPrimaryConstructor && BaseModelProvider != null && BaseModelProvider.ShouldHaveDualConstructorPattern()) + { + // Call base model's private protected constructor with discriminator value + var args = new List(); + args.Add(Literal(_inputModel.DiscriminatorValue ?? "")); + args.AddRange(baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))); + constructorInitializer = new ConstructorInitializer(true, args); + } + else + { + // Standard base constructor call + constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))]); + } + } foreach (var property in CanonicalView.Properties) { From 8c7c78176dbcff4fb49895ddc51eaf6603ba5223 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Thu, 13 Nov 2025 16:17:07 -0800 Subject: [PATCH 03/12] added unit tests --- .../ScmModelProvider/ScmModelProviderTests.cs | 1 - .../src/Providers/ModelProvider.cs | 38 ++-- .../ModelProviders/ModelProviderTests.cs | 164 ++++++++++++++++++ 3 files changed, 189 insertions(+), 14 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs index bbbccdb3af3..e0d773dd550 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs @@ -105,7 +105,6 @@ public void TestNestedDiscriminatorDynamicModel(bool discriminatedTypeIsDynamicM ]); InputModelType catModel = InputFactory.Model( "cat", - discriminatedKind: "cat", properties: [ InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 73a2e3017cc..0a1cf9bcf11 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -562,8 +562,8 @@ protected override ConstructorProvider[] BuildConstructors() /// /// Determines if this model should have a dual constructor pattern. - /// This is needed when the model shares the same discriminator property name as its base model, - /// indicating it's an intermediate type in a discriminated union hierarchy. + /// This is needed when the model shares the same discriminator property name as its base model + /// AND has derived models, indicating it's an intermediate type in a discriminated union hierarchy. /// private bool ShouldHaveDualConstructorPattern() { @@ -571,6 +571,10 @@ private bool ShouldHaveDualConstructorPattern() if (_isAbstract || BaseModelProvider == null) return false; + // Must have derived models to be considered an intermediate type + if (_inputModel.DerivedModels.Count == 0) + return false; + // Check if this model has a discriminator property in the input if (_inputModel.DiscriminatorProperty == null) return false; @@ -580,7 +584,7 @@ private bool ShouldHaveDualConstructorPattern() return false; // If both models have discriminator properties with the same name, - // this model needs the dual constructor pattern + // and this model has derived models, it needs the dual constructor pattern return string.Equals( _inputModel.DiscriminatorProperty.Name, BaseModelProvider._inputModel.DiscriminatorProperty.Name, @@ -846,21 +850,29 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( // construct the initializer using the parameters from base signature ConstructorInitializer? constructorInitializer = null; - if (baseParameters.Count > 0) + if (BaseModelProvider != null) { - // Check if base model has dual constructor pattern and we should call private protected constructor - if (isPrimaryConstructor && BaseModelProvider != null && BaseModelProvider.ShouldHaveDualConstructorPattern()) + if (baseParameters.Count > 0) { - // Call base model's private protected constructor with discriminator value - var args = new List(); - args.Add(Literal(_inputModel.DiscriminatorValue ?? "")); - args.AddRange(baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))); - constructorInitializer = new ConstructorInitializer(true, args); + // Check if base model has dual constructor pattern and we should call private protected constructor + if (isPrimaryConstructor && BaseModelProvider.ShouldHaveDualConstructorPattern()) + { + // Call base model's private protected constructor with discriminator value + var args = new List(); + args.Add(Literal(_inputModel.DiscriminatorValue ?? "")); + args.AddRange(baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))); + constructorInitializer = new ConstructorInitializer(true, args); + } + else + { + // Standard base constructor call + constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))]); + } } else { - // Standard base constructor call - constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))]); + // Even when no base parameters, we still need a base constructor call if there's a base model + constructorInitializer = new ConstructorInitializer(true, Array.Empty()); } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index d3b32196e21..4d2156fe52c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -1289,5 +1289,169 @@ public void ModelWithOptionalDiscriminatorProperty() var file = writer.Write(); Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } + + [Test] + public void TestMultiLayerDiscriminator_IntermediateWithoutDiscriminator() + { + // Test hierarchy: Pet (base, no discriminator) → Cat (intermediate, no discriminator) → Tiger (leaf, discriminator: "tiger") + // This verifies that intermediate types without discriminators properly pass parameters to base constructors + + InputModelType tigerModel = InputFactory.Model( + "tiger", + discriminatedKind: "tiger", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("stripes", InputPrimitiveType.Int32, isRequired: true) + ]); + + InputModelType catModel = InputFactory.Model( + "cat", + // No discriminatedKind - Cat is an intermediate base type without discriminator value + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("meows", InputPrimitiveType.Boolean, isRequired: true) + ], + discriminatedModels: new Dictionary() { { "tiger", tigerModel } }); + + var baseModel = InputFactory.Model( + "pet", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true) + ], + discriminatedModels: new Dictionary() { { "cat", catModel } }); + + MockHelpers.LoadMockGenerator(inputModelTypes: [baseModel, catModel, tigerModel]); + + var tigerProvider = new ModelProvider(tigerModel); + + // Verify Tiger has correct constructor inheritance + var publicConstructor = tigerProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsNotNull(publicConstructor); + + // Tiger should call base constructor with only base parameters (no discriminator from Cat since Cat doesn't have one) + var initializer = publicConstructor!.Signature.Initializer; + Assert.IsNotNull(initializer); + Assert.IsTrue(initializer!.IsBase); + + // Should have name and meows parameters from base chain (no discriminator since Cat has no discriminatedKind) + Assert.AreEqual(2, initializer.Arguments.Count); + } + + [Test] + public void TestMultiLayerDiscriminator_IntermediateWithDiscriminator() + { + // Test hierarchy: Pet (base, no discriminator) → Cat (intermediate, discriminator: "cat") → DomesticCat (leaf, discriminator: "domestic") + // This verifies that intermediate types WITH discriminators properly handle dual constructor pattern + + InputModelType domesticCatModel = InputFactory.Model( + "domesticCat", + discriminatedKind: "domestic", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("breed", InputPrimitiveType.String, isRequired: true) + ]); + + InputModelType catModel = InputFactory.Model( + "cat", + discriminatedKind: "cat", // Cat has discriminator since it's a concrete type that can be instantiated + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("meows", InputPrimitiveType.Boolean, isRequired: true) + ], + discriminatedModels: new Dictionary() { { "domestic", domesticCatModel } }); + + var baseModel = InputFactory.Model( + "pet", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true) + ], + discriminatedModels: new Dictionary() { { "cat", catModel } }); + + MockHelpers.LoadMockGenerator(inputModelTypes: [baseModel, catModel, domesticCatModel]); + + var domesticCatProvider = new ModelProvider(domesticCatModel); + + // Verify DomesticCat has correct constructor inheritance + var publicConstructor = domesticCatProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsNotNull(publicConstructor); + + // DomesticCat should call Cat's dual constructor with discriminator value since Cat has discriminatedKind + var initializer = publicConstructor!.Signature.Initializer; + Assert.IsNotNull(initializer); + Assert.IsTrue(initializer!.IsBase); + + // Should have discriminator + parameters from Cat's dual constructor pattern + Assert.GreaterOrEqual(initializer.Arguments.Count, 2); // At least discriminator + some parameters + } + + [Test] + public void TestMultiLayerDiscriminator_ThreeLayers() + { + // Test hierarchy: Animal (base) → Pet (discriminator: "pet") → Cat (discriminator: "cat") → DomesticCat (discriminator: "domestic") + // This tests a deep hierarchy with multiple discriminator levels + + InputModelType domesticCatModel = InputFactory.Model( + "domesticCat", + discriminatedKind: "domestic", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("breed", InputPrimitiveType.String, isRequired: true) + ]); + + InputModelType catModel = InputFactory.Model( + "cat", + discriminatedKind: "cat", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("meows", InputPrimitiveType.Boolean, isRequired: true) + ], + discriminatedModels: new Dictionary() { { "domestic", domesticCatModel } }); + + InputModelType petModel = InputFactory.Model( + "pet", + discriminatedKind: "pet", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true) + ], + discriminatedModels: new Dictionary() { { "cat", catModel } }); + + var animalModel = InputFactory.Model( + "animal", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), + InputFactory.Property("species", InputPrimitiveType.String, isRequired: true) + ], + discriminatedModels: new Dictionary() { { "pet", petModel } }); + + MockHelpers.LoadMockGenerator(inputModelTypes: [animalModel, petModel, catModel, domesticCatModel]); + + var domesticCatProvider = new ModelProvider(domesticCatModel); + + // Verify the deep inheritance chain works correctly + Assert.IsNotNull(domesticCatProvider.BaseModelProvider); + Assert.IsNotNull(domesticCatProvider.BaseModelProvider!.BaseModelProvider); + Assert.IsNotNull(domesticCatProvider.BaseModelProvider!.BaseModelProvider!.BaseModelProvider); + + // Verify constructor initialization works through the chain + var publicConstructor = domesticCatProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsNotNull(publicConstructor); + + var initializer = publicConstructor!.Signature.Initializer; + Assert.IsNotNull(initializer); + Assert.IsTrue(initializer!.IsBase); + } } } From 8a2f7724f318adcbefdfe9277bcb4dba98d9783f Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Thu, 13 Nov 2025 18:42:54 -0800 Subject: [PATCH 04/12] minor: comment clean --- .../src/Providers/ConstructorType.cs | 4 ++-- .../src/Providers/ModelProvider.cs | 4 ++-- .../ModelProviders/ModelProviderTests.cs | 16 ++-------------- 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs index 7600fc9eb38..3559ef57cbf 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs @@ -10,9 +10,9 @@ public enum ConstructorType { /// Standard public/internal constructor Standard, - /// Public constructor for direct instantiation (dual pattern) + /// Public constructor for direct instantiation PublicForInstantiation, - /// Private protected constructor for inheritance (dual pattern) + /// Private protected constructor for inheritance PrivateProtectedForInheritance, /// Internal constructor for serialization InternalForSerialization diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 0a1cf9bcf11..4a087379809 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -644,8 +644,8 @@ private ConstructorProvider[] BuildDualConstructorPattern() { ConstructorType.PublicForInstantiation => BuildPublicInstantiationParameters(), ConstructorType.PrivateProtectedForInheritance => BuildPrivateProtectedInheritanceParameters(), - ConstructorType.InternalForSerialization => BuildConstructorParameters(false), // Use existing logic - ConstructorType.Standard => BuildConstructorParameters(true), // Use existing logic + ConstructorType.InternalForSerialization => BuildConstructorParameters(false), + ConstructorType.Standard => BuildConstructorParameters(true), _ => throw new ArgumentOutOfRangeException(nameof(constructorType)) }; } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index 4d2156fe52c..cb559198ff5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -1294,7 +1294,6 @@ public void ModelWithOptionalDiscriminatorProperty() public void TestMultiLayerDiscriminator_IntermediateWithoutDiscriminator() { // Test hierarchy: Pet (base, no discriminator) → Cat (intermediate, no discriminator) → Tiger (leaf, discriminator: "tiger") - // This verifies that intermediate types without discriminators properly pass parameters to base constructors InputModelType tigerModel = InputFactory.Model( "tiger", @@ -1307,7 +1306,6 @@ public void TestMultiLayerDiscriminator_IntermediateWithoutDiscriminator() InputModelType catModel = InputFactory.Model( "cat", - // No discriminatedKind - Cat is an intermediate base type without discriminator value properties: [ InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), @@ -1328,16 +1326,13 @@ public void TestMultiLayerDiscriminator_IntermediateWithoutDiscriminator() var tigerProvider = new ModelProvider(tigerModel); - // Verify Tiger has correct constructor inheritance var publicConstructor = tigerProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); Assert.IsNotNull(publicConstructor); - // Tiger should call base constructor with only base parameters (no discriminator from Cat since Cat doesn't have one) var initializer = publicConstructor!.Signature.Initializer; Assert.IsNotNull(initializer); Assert.IsTrue(initializer!.IsBase); - // Should have name and meows parameters from base chain (no discriminator since Cat has no discriminatedKind) Assert.AreEqual(2, initializer.Arguments.Count); } @@ -1345,7 +1340,6 @@ public void TestMultiLayerDiscriminator_IntermediateWithoutDiscriminator() public void TestMultiLayerDiscriminator_IntermediateWithDiscriminator() { // Test hierarchy: Pet (base, no discriminator) → Cat (intermediate, discriminator: "cat") → DomesticCat (leaf, discriminator: "domestic") - // This verifies that intermediate types WITH discriminators properly handle dual constructor pattern InputModelType domesticCatModel = InputFactory.Model( "domesticCat", @@ -1358,7 +1352,7 @@ public void TestMultiLayerDiscriminator_IntermediateWithDiscriminator() InputModelType catModel = InputFactory.Model( "cat", - discriminatedKind: "cat", // Cat has discriminator since it's a concrete type that can be instantiated + discriminatedKind: "cat", properties: [ InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true), @@ -1379,24 +1373,20 @@ public void TestMultiLayerDiscriminator_IntermediateWithDiscriminator() var domesticCatProvider = new ModelProvider(domesticCatModel); - // Verify DomesticCat has correct constructor inheritance var publicConstructor = domesticCatProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); Assert.IsNotNull(publicConstructor); - // DomesticCat should call Cat's dual constructor with discriminator value since Cat has discriminatedKind var initializer = publicConstructor!.Signature.Initializer; Assert.IsNotNull(initializer); Assert.IsTrue(initializer!.IsBase); - // Should have discriminator + parameters from Cat's dual constructor pattern - Assert.GreaterOrEqual(initializer.Arguments.Count, 2); // At least discriminator + some parameters + Assert.GreaterOrEqual(initializer.Arguments.Count, 2); } [Test] public void TestMultiLayerDiscriminator_ThreeLayers() { // Test hierarchy: Animal (base) → Pet (discriminator: "pet") → Cat (discriminator: "cat") → DomesticCat (discriminator: "domestic") - // This tests a deep hierarchy with multiple discriminator levels InputModelType domesticCatModel = InputFactory.Model( "domesticCat", @@ -1440,12 +1430,10 @@ public void TestMultiLayerDiscriminator_ThreeLayers() var domesticCatProvider = new ModelProvider(domesticCatModel); - // Verify the deep inheritance chain works correctly Assert.IsNotNull(domesticCatProvider.BaseModelProvider); Assert.IsNotNull(domesticCatProvider.BaseModelProvider!.BaseModelProvider); Assert.IsNotNull(domesticCatProvider.BaseModelProvider!.BaseModelProvider!.BaseModelProvider); - // Verify constructor initialization works through the chain var publicConstructor = domesticCatProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); Assert.IsNotNull(publicConstructor); From cc14fac967213fddb7b6dad6f703fa91fc2e3882 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Fri, 14 Nov 2025 09:35:00 -0800 Subject: [PATCH 05/12] minor: comment clean and func rename --- .../src/Providers/ModelProvider.cs | 53 +++++++++---------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 4a087379809..f75f6c3d1f6 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -21,7 +21,6 @@ public class ModelProvider : TypeProvider private const string AdditionalBinaryDataPropsFieldDescription = "Keeps track of any properties unknown to the library."; private const string DiscriminatorParameterName = "discriminatorValue"; private const string DiscriminatorParameterDescription = "The discriminator property."; - private readonly InputModelType _inputModel; // Note the description cannot be built from the constructor as it would lead to a circular dependency between the base // and derived models resulting in a stack overflow. @@ -601,7 +600,7 @@ private bool ShouldHaveDualConstructorPattern() private ConstructorProvider[] BuildDualConstructorPattern() { // Build public constructor (without discriminator parameter) - var (publicParams, publicInitializer) = BuildConstructorParameters(ConstructorType.PublicForInstantiation); + var (publicParams, publicInitializer) = BuildConstructorParametersByType(ConstructorType.PublicForInstantiation); var publicConstructor = new ConstructorProvider( signature: new ConstructorSignature( Type, @@ -616,7 +615,7 @@ private ConstructorProvider[] BuildDualConstructorPattern() this); // Build private protected constructor (with discriminator parameter) - var (protectedParams, protectedInitializer) = BuildConstructorParameters(ConstructorType.PrivateProtectedForInheritance); + var (protectedParams, protectedInitializer) = BuildConstructorParametersByType(ConstructorType.PrivateProtectedForInheritance); var protectedConstructor = new ConstructorProvider( signature: new ConstructorSignature( Type, @@ -637,7 +636,7 @@ private ConstructorProvider[] BuildDualConstructorPattern() /// /// Builds constructor parameters based on the constructor type using a strategy pattern. /// - private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildConstructorParameters( + private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildConstructorParametersByType( ConstructorType constructorType) { return constructorType switch @@ -657,15 +656,32 @@ private ConstructorProvider[] BuildDualConstructorPattern() { var (standardParams, _) = BuildConstructorParameters(true); - // Filter out discriminator parameter var parameters = standardParams.Where(p => p.Property == null || !p.Property.IsDiscriminator).ToList(); - // Create initializer that calls the private protected constructor var initializer = CreatePrivateProtectedConstructorCall(parameters); return (parameters, initializer); } + /// + /// Creates a constructor initializer that calls the private protected constructor with discriminator value. + /// + private ConstructorInitializer CreatePrivateProtectedConstructorCall(IReadOnlyList parameters) + { + var discriminatorValue = EnsureDiscriminatorValueExpression(); + var args = new List(); + + if (discriminatorValue != null) + { + args.Add(discriminatorValue); + } + + var overriddenProperties = CanonicalView.Properties.Where(p => p.BaseProperty is not null).Select(p => p.BaseProperty!).ToHashSet(); + args.AddRange(parameters.Select(p => GetExpressionForCtor(p, overriddenProperties, true))); + + return new ConstructorInitializer(false, args.ToArray()); + } + /// /// Builds parameters for private protected inheritance constructor (includes discriminator). /// @@ -673,10 +689,8 @@ private ConstructorProvider[] BuildDualConstructorPattern() { var (standardParams, standardInitializer) = BuildConstructorParameters(true); - // Add discriminator parameter at the beginning var parameters = PrependDiscriminatorParameter(standardParams); - // Create initializer that passes discriminator parameter to base var initializer = CreateBaseConstructorCallWithDiscriminatorParameter(standardInitializer, parameters.FirstOrDefault()); return (parameters, initializer); @@ -702,25 +716,6 @@ private IReadOnlyList PrependDiscriminatorParameter(IReadOnly return result; } - /// - /// Creates a constructor initializer that calls the private protected constructor with discriminator value. - /// - private ConstructorInitializer CreatePrivateProtectedConstructorCall(IReadOnlyList parameters) - { - var discriminatorValue = EnsureDiscriminatorValueExpression(); - var args = new List(); - - if (discriminatorValue != null) - { - args.Add(discriminatorValue); - } - - var overriddenProperties = CanonicalView.Properties.Where(p => p.BaseProperty is not null).Select(p => p.BaseProperty!).ToHashSet(); - args.AddRange(parameters.Select(p => GetExpressionForCtor(p, overriddenProperties, true))); - - return new ConstructorInitializer(false, args.ToArray()); // this(...) call - } - /// /// Creates a base constructor call that uses the discriminator parameter. /// @@ -737,9 +732,9 @@ private ConstructorInitializer CreatePrivateProtectedConstructorCall(IReadOnlyLi var newArgs = new List { - discriminatorParam.AsVariable() // Use parameter instead of hardcoded value + discriminatorParam.AsVariable() }; - newArgs.AddRange(originalArgs.Skip(1)); // Skip original discriminator value + newArgs.AddRange(originalArgs.Skip(1)); return new ConstructorInitializer(standardInitializer.IsBase, newArgs); } From 192b13e1279214841b94e606bbd91e35fa51e804 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Fri, 14 Nov 2025 09:46:36 -0800 Subject: [PATCH 06/12] restructure --- .../src/Providers/ConstructorType.cs | 20 ------------------ .../src/Providers/ModelProvider.cs | 21 ++----------------- 2 files changed, 2 insertions(+), 39 deletions(-) delete mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs deleted file mode 100644 index 3559ef57cbf..00000000000 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ConstructorType.cs +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -namespace Microsoft.TypeSpec.Generator.Providers -{ - /// - /// Represents the different types of constructors that can be generated for a model. - /// - public enum ConstructorType - { - /// Standard public/internal constructor - Standard, - /// Public constructor for direct instantiation - PublicForInstantiation, - /// Private protected constructor for inheritance - PrivateProtectedForInheritance, - /// Internal constructor for serialization - InternalForSerialization - } -} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index f75f6c3d1f6..096ec18e709 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -600,7 +600,7 @@ private bool ShouldHaveDualConstructorPattern() private ConstructorProvider[] BuildDualConstructorPattern() { // Build public constructor (without discriminator parameter) - var (publicParams, publicInitializer) = BuildConstructorParametersByType(ConstructorType.PublicForInstantiation); + var (publicParams, publicInitializer) = BuildPublicInstantiationParameters(); var publicConstructor = new ConstructorProvider( signature: new ConstructorSignature( Type, @@ -615,7 +615,7 @@ private ConstructorProvider[] BuildDualConstructorPattern() this); // Build private protected constructor (with discriminator parameter) - var (protectedParams, protectedInitializer) = BuildConstructorParametersByType(ConstructorType.PrivateProtectedForInheritance); + var (protectedParams, protectedInitializer) = BuildPrivateProtectedInheritanceParameters(); var protectedConstructor = new ConstructorProvider( signature: new ConstructorSignature( Type, @@ -633,22 +633,6 @@ private ConstructorProvider[] BuildDualConstructorPattern() return [publicConstructor, protectedConstructor, FullConstructor]; } - /// - /// Builds constructor parameters based on the constructor type using a strategy pattern. - /// - private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildConstructorParametersByType( - ConstructorType constructorType) - { - return constructorType switch - { - ConstructorType.PublicForInstantiation => BuildPublicInstantiationParameters(), - ConstructorType.PrivateProtectedForInheritance => BuildPrivateProtectedInheritanceParameters(), - ConstructorType.InternalForSerialization => BuildConstructorParameters(false), - ConstructorType.Standard => BuildConstructorParameters(true), - _ => throw new ArgumentOutOfRangeException(nameof(constructorType)) - }; - } - /// /// Builds parameters for public instantiation constructor (filters out discriminator). /// @@ -746,7 +730,6 @@ private IReadOnlyList PrependDiscriminatorParameter(IReadOnly private ConstructorProvider BuildFullConstructor() { var (ctorParameters, ctorInitializer) = BuildConstructorParameters(false); - return new ConstructorProvider( signature: new ConstructorSignature( Type, From 876185739f1afa362cc0d012b7656357638a5aa7 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Fri, 14 Nov 2025 15:39:22 -0800 Subject: [PATCH 07/12] removed prepend logic and ellborated tests --- .../src/Providers/ModelProvider.cs | 46 ++++---- .../ModelProviders/ModelProviderTests.cs | 109 +++++++++++++++++- 2 files changed, 125 insertions(+), 30 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 096ec18e709..4629d21306e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -568,19 +568,26 @@ private bool ShouldHaveDualConstructorPattern() { // Only applies to non-abstract models with a base model if (_isAbstract || BaseModelProvider == null) + { return false; - + } // Must have derived models to be considered an intermediate type if (_inputModel.DerivedModels.Count == 0) + { return false; + } // Check if this model has a discriminator property in the input if (_inputModel.DiscriminatorProperty == null) + { return false; + } // Check if base model has a discriminator property with the same name if (BaseModelProvider._inputModel.DiscriminatorProperty == null) + { return false; + } // If both models have discriminator properties with the same name, // and this model has derived models, it needs the dual constructor pattern @@ -671,35 +678,13 @@ private ConstructorInitializer CreatePrivateProtectedConstructorCall(IReadOnlyLi /// private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildPrivateProtectedInheritanceParameters() { - var (standardParams, standardInitializer) = BuildConstructorParameters(true); - - var parameters = PrependDiscriminatorParameter(standardParams); + var (parameters, standardInitializer) = BuildConstructorParameters(true, includeDiscriminatorParameter: true); var initializer = CreateBaseConstructorCallWithDiscriminatorParameter(standardInitializer, parameters.FirstOrDefault()); return (parameters, initializer); } - /// - /// Prepends the discriminator parameter to the beginning of a parameter list. - /// - private IReadOnlyList PrependDiscriminatorParameter(IReadOnlyList parameters) - { - var result = new List(); - - if (_inputModel.DiscriminatorProperty != null) - { - var discriminatorParam = new ParameterProvider( - DiscriminatorParameterName, - $"{DiscriminatorParameterDescription}", - new CSharpType(typeof(string))); - result.Add(discriminatorParam); - } - - result.AddRange(parameters); - return result; - } - /// /// Creates a base constructor call that uses the discriminator parameter. /// @@ -795,7 +780,7 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( } private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildConstructorParameters( - bool isPrimaryConstructor) + bool isPrimaryConstructor, bool includeDiscriminatorParameter = false) { var baseParameters = new List(); var constructorParameters = new List(); @@ -868,9 +853,18 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( ? baseParameters : baseParameters.Where(p => p.Property is null - || (p.Property.IsDiscriminator && !overriddenProperties.Contains(p.Property) && !isPrimaryConstructor) + || (p.Property.IsDiscriminator && !overriddenProperties.Contains(p.Property) && (!isPrimaryConstructor || includeDiscriminatorParameter)) || (!p.Property.IsDiscriminator && !overriddenProperties.Contains(p.Property)))); + if (includeDiscriminatorParameter && _inputModel.DiscriminatorProperty != null) + { + var discriminatorParam = new ParameterProvider( + DiscriminatorParameterName, + $"{DiscriminatorParameterDescription}", + new CSharpType(typeof(string))); + constructorParameters.Insert(0, discriminatorParam); + } + if (!isPrimaryConstructor) { foreach (var property in AdditionalPropertyProperties) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index cb559198ff5..e1fcd533ff3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -1326,14 +1326,44 @@ public void TestMultiLayerDiscriminator_IntermediateWithoutDiscriminator() var tigerProvider = new ModelProvider(tigerModel); + Assert.AreEqual(2, tigerProvider.Constructors.Count); + var publicConstructor = tigerProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); Assert.IsNotNull(publicConstructor); + Assert.AreEqual(MethodSignatureModifiers.Public, publicConstructor!.Signature.Modifiers); + + // Tiger's public constructor should have parameters: name (from Pet), meows (from Cat), stripes (from Tiger) + var publicParams = publicConstructor.Signature.Parameters; + Assert.AreEqual(3, publicParams.Count); + Assert.AreEqual("name", publicParams[0].Name); + Assert.AreEqual(typeof(string), publicParams[0].Type.FrameworkType); + Assert.AreEqual("meows", publicParams[1].Name); + Assert.AreEqual(typeof(bool), publicParams[1].Type.FrameworkType); + Assert.AreEqual("stripes", publicParams[2].Name); + Assert.AreEqual(typeof(int), publicParams[2].Type.FrameworkType); - var initializer = publicConstructor!.Signature.Initializer; + // Tiger should call base constructor with only base parameters (no discriminator from Cat since Cat doesn't have one) + var initializer = publicConstructor.Signature.Initializer; Assert.IsNotNull(initializer); Assert.IsTrue(initializer!.IsBase); + // Should have name and meows parameters from base chain (no discriminator since Cat has no discriminatedKind) Assert.AreEqual(2, initializer.Arguments.Count); + Assert.AreEqual("name", initializer.Arguments[0].ToDisplayString()); + Assert.AreEqual("meows", initializer.Arguments[1].ToDisplayString()); + + // Verify internal (serialization) constructor signature and parameters + var internalConstructor = tigerProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal)); + Assert.IsNotNull(internalConstructor); + Assert.AreEqual(MethodSignatureModifiers.Internal, internalConstructor!.Signature.Modifiers); + + // Internal constructor should have all parameters including serialization params + var internalParams = internalConstructor.Signature.Parameters; + Assert.IsTrue(internalParams.Count >= 4); + + var internalInitializer = internalConstructor.Signature.Initializer; + Assert.IsNotNull(internalInitializer); + Assert.IsTrue(internalInitializer!.IsBase); } [Test] @@ -1372,15 +1402,45 @@ public void TestMultiLayerDiscriminator_IntermediateWithDiscriminator() MockHelpers.LoadMockGenerator(inputModelTypes: [baseModel, catModel, domesticCatModel]); var domesticCatProvider = new ModelProvider(domesticCatModel); + var catProvider = new ModelProvider(catModel); + + Assert.AreEqual(2, domesticCatProvider.Constructors.Count); var publicConstructor = domesticCatProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); Assert.IsNotNull(publicConstructor); + Assert.AreEqual(MethodSignatureModifiers.Public, publicConstructor!.Signature.Modifiers); - var initializer = publicConstructor!.Signature.Initializer; + // DomesticCat's public constructor should have parameters: name (from Pet), meows (from Cat), breed (from DomesticCat) + var publicParams = publicConstructor.Signature.Parameters; + Assert.AreEqual(3, publicParams.Count); + Assert.AreEqual("name", publicParams[0].Name); + Assert.AreEqual(typeof(string), publicParams[0].Type.FrameworkType); + Assert.AreEqual("meows", publicParams[1].Name); + Assert.AreEqual(typeof(bool), publicParams[1].Type.FrameworkType); + Assert.AreEqual("breed", publicParams[2].Name); + Assert.AreEqual(typeof(string), publicParams[2].Type.FrameworkType); + + // DomesticCat should call Cat's dual constructor with discriminator value since Cat has discriminatedKind + var initializer = publicConstructor.Signature.Initializer; Assert.IsNotNull(initializer); Assert.IsTrue(initializer!.IsBase); - Assert.GreaterOrEqual(initializer.Arguments.Count, 2); + // Should have discriminator + parameters from Cat's dual constructor pattern + Assert.AreEqual(3, initializer.Arguments.Count); // discriminator "domestic" + name + meows + Assert.AreEqual("\"domestic\"", initializer.Arguments[0].ToDisplayString()); + Assert.AreEqual("name", initializer.Arguments[1].ToDisplayString()); + Assert.AreEqual("meows", initializer.Arguments[2].ToDisplayString()); + + // Verify Cat also has dual constructor pattern (public and protected) + Assert.AreEqual(3, catProvider.Constructors.Count); + + var internalConstructor = domesticCatProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal)); + Assert.IsNotNull(internalConstructor); + Assert.AreEqual(MethodSignatureModifiers.Internal, internalConstructor!.Signature.Modifiers); + + var internalInitializer = internalConstructor.Signature.Initializer; + Assert.IsNotNull(internalInitializer); + Assert.IsTrue(internalInitializer!.IsBase); } [Test] @@ -1429,17 +1489,58 @@ public void TestMultiLayerDiscriminator_ThreeLayers() MockHelpers.LoadMockGenerator(inputModelTypes: [animalModel, petModel, catModel, domesticCatModel]); var domesticCatProvider = new ModelProvider(domesticCatModel); + var catProvider = new ModelProvider(catModel); + var petProvider = new ModelProvider(petModel); + var animalProvider = new ModelProvider(animalModel); Assert.IsNotNull(domesticCatProvider.BaseModelProvider); Assert.IsNotNull(domesticCatProvider.BaseModelProvider!.BaseModelProvider); Assert.IsNotNull(domesticCatProvider.BaseModelProvider!.BaseModelProvider!.BaseModelProvider); + Assert.AreEqual(2, domesticCatProvider.Constructors.Count); // public, internal (leaf type) + Assert.AreEqual(3, catProvider.Constructors.Count); // public, protected with discriminator, internal + Assert.AreEqual(3, petProvider.Constructors.Count); // public, protected with discriminator, internal + Assert.AreEqual(2, animalProvider.Constructors.Count); // public, internal (base type) + + // Verify DomesticCat's public constructor parameters: species (from Animal), name (from Pet), meows (from Cat), breed (from DomesticCat) var publicConstructor = domesticCatProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); Assert.IsNotNull(publicConstructor); + Assert.AreEqual(MethodSignatureModifiers.Public, publicConstructor!.Signature.Modifiers); + + var publicParams = publicConstructor.Signature.Parameters; + Assert.AreEqual(4, publicParams.Count); + Assert.AreEqual("species", publicParams[0].Name); + Assert.AreEqual(typeof(string), publicParams[0].Type.FrameworkType); + Assert.AreEqual("name", publicParams[1].Name); + Assert.AreEqual(typeof(string), publicParams[1].Type.FrameworkType); + Assert.AreEqual("meows", publicParams[2].Name); + Assert.AreEqual(typeof(bool), publicParams[2].Type.FrameworkType); + Assert.AreEqual("breed", publicParams[3].Name); + Assert.AreEqual(typeof(string), publicParams[3].Type.FrameworkType); - var initializer = publicConstructor!.Signature.Initializer; + var initializer = publicConstructor.Signature.Initializer; Assert.IsNotNull(initializer); Assert.IsTrue(initializer!.IsBase); + + Assert.AreEqual(4, initializer.Arguments.Count); // discriminator "domestic" + species + name + meows + Assert.AreEqual("\"domestic\"", initializer.Arguments[0].ToDisplayString()); + Assert.AreEqual("species", initializer.Arguments[1].ToDisplayString()); + Assert.AreEqual("name", initializer.Arguments[2].ToDisplayString()); + Assert.AreEqual("meows", initializer.Arguments[3].ToDisplayString()); + + // Verify Cat's protected constructor exists and has correct signature + var catProtectedConstructor = catProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)); + Assert.IsNotNull(catProtectedConstructor); + Assert.IsTrue(catProtectedConstructor!.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)); + + // Cat's protected constructor should have: discriminator + species + name + meows + var catProtectedParams = catProtectedConstructor.Signature.Parameters; + Assert.AreEqual(4, catProtectedParams.Count); + Assert.AreEqual("discriminatorValue", catProtectedParams[0].Name); + Assert.AreEqual(typeof(string), catProtectedParams[0].Type.FrameworkType); + Assert.AreEqual("species", catProtectedParams[1].Name); + Assert.AreEqual("name", catProtectedParams[2].Name); + Assert.AreEqual("meows", catProtectedParams[3].Name); } } } From fa1ae2fc45b4702f61ba794eb09f7e15d849bc23 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 24 Nov 2025 13:35:58 -0800 Subject: [PATCH 08/12] refsctored and fixed ci failures --- .../src/Providers/ModelProvider.cs | 140 ++++-------------- 1 file changed, 28 insertions(+), 112 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 4629d21306e..4303ed96775 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -54,6 +54,7 @@ protected override FormattableString BuildDescription() } private readonly bool _isAbstract; + private readonly bool _isMultiLevelDiscriminator; private readonly CSharpType _additionalBinaryDataPropsFieldType = typeof(IDictionary); private readonly Type _additionalPropsUnknownType = typeof(BinaryData); @@ -68,6 +69,7 @@ public ModelProvider(InputModelType inputModel) : base(inputModel) { _inputModel = inputModel; _isAbstract = _inputModel.DiscriminatorProperty is not null && _inputModel.DiscriminatorValue is null; + _isMultiLevelDiscriminator = ComputeIsMultiLevelDiscriminator(); if (_inputModel.BaseModel is not null) { @@ -524,12 +526,6 @@ protected override ConstructorProvider[] BuildConstructors() return [FullConstructor]; } - // Check if this model needs the dual constructor pattern - if (ShouldHaveDualConstructorPattern()) - { - return BuildDualConstructorPattern(); - } - // Build the standard single initialization constructor var accessibility = DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract) ? MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected @@ -551,12 +547,22 @@ protected override ConstructorProvider[] BuildConstructors() }, this); + var constructors = new List { constructor }; + + // Add FullConstructor if parameters are different if (!constructorParameters.SequenceEqual(FullConstructor.Signature.Parameters)) { - return [constructor, FullConstructor]; + constructors.Add(FullConstructor); + } + + // For multi-level discriminators, add one additional private protected constructor + if (_isMultiLevelDiscriminator) + { + var protectedConstructor = BuildProtectedInheritanceConstructor(); + constructors.Add(protectedConstructor); } - return [constructor]; + return [.. constructors]; } /// @@ -564,10 +570,10 @@ protected override ConstructorProvider[] BuildConstructors() /// This is needed when the model shares the same discriminator property name as its base model /// AND has derived models, indicating it's an intermediate type in a discriminated union hierarchy. /// - private bool ShouldHaveDualConstructorPattern() + private bool ComputeIsMultiLevelDiscriminator() { // Only applies to non-abstract models with a base model - if (_isAbstract || BaseModelProvider == null) + if (_isAbstract || _inputModel.BaseModel == null) { return false; } @@ -584,7 +590,7 @@ private bool ShouldHaveDualConstructorPattern() } // Check if base model has a discriminator property with the same name - if (BaseModelProvider._inputModel.DiscriminatorProperty == null) + if (_inputModel.BaseModel.DiscriminatorProperty == null) { return false; } @@ -593,119 +599,30 @@ private bool ShouldHaveDualConstructorPattern() // and this model has derived models, it needs the dual constructor pattern return string.Equals( _inputModel.DiscriminatorProperty.Name, - BaseModelProvider._inputModel.DiscriminatorProperty.Name, + _inputModel.BaseModel.DiscriminatorProperty.Name, StringComparison.OrdinalIgnoreCase); } /// - /// Builds the dual constructor pattern for models that share discriminator properties with their base. - /// Creates three constructors: - /// 1. Public constructor for external use - /// 2. Private protected constructor for derived models to call - /// 3. Internal constructor for serialization + /// Builds a private protected constructor for multi-level discriminator inheritance. + /// This allows derived models to call this constructor with their discriminator value. /// - private ConstructorProvider[] BuildDualConstructorPattern() + private ConstructorProvider BuildProtectedInheritanceConstructor() { - // Build public constructor (without discriminator parameter) - var (publicParams, publicInitializer) = BuildPublicInstantiationParameters(); - var publicConstructor = new ConstructorProvider( - signature: new ConstructorSignature( - Type, - $"Initializes a new instance of {Type:C}", - MethodSignatureModifiers.Public, - publicParams, - initializer: publicInitializer), - bodyStatements: new MethodBodyStatement[] - { - GetPropertyInitializers(true, parameters: publicParams) - }, - this); + var (parameters, initializer) = BuildConstructorParameters(true, includeDiscriminatorParameter: true); - // Build private protected constructor (with discriminator parameter) - var (protectedParams, protectedInitializer) = BuildPrivateProtectedInheritanceParameters(); - var protectedConstructor = new ConstructorProvider( + return new ConstructorProvider( signature: new ConstructorSignature( Type, $"Initializes a new instance of {Type:C}", MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected, - protectedParams, - initializer: protectedInitializer), + parameters, + initializer: initializer), bodyStatements: new MethodBodyStatement[] { - GetPropertyInitializers(true, parameters: protectedParams) + GetPropertyInitializers(true, parameters: parameters) }, this); - - // Internal constructor is the full constructor - return [publicConstructor, protectedConstructor, FullConstructor]; - } - - /// - /// Builds parameters for public instantiation constructor (filters out discriminator). - /// - private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildPublicInstantiationParameters() - { - var (standardParams, _) = BuildConstructorParameters(true); - - var parameters = standardParams.Where(p => p.Property == null || !p.Property.IsDiscriminator).ToList(); - - var initializer = CreatePrivateProtectedConstructorCall(parameters); - - return (parameters, initializer); - } - - /// - /// Creates a constructor initializer that calls the private protected constructor with discriminator value. - /// - private ConstructorInitializer CreatePrivateProtectedConstructorCall(IReadOnlyList parameters) - { - var discriminatorValue = EnsureDiscriminatorValueExpression(); - var args = new List(); - - if (discriminatorValue != null) - { - args.Add(discriminatorValue); - } - - var overriddenProperties = CanonicalView.Properties.Where(p => p.BaseProperty is not null).Select(p => p.BaseProperty!).ToHashSet(); - args.AddRange(parameters.Select(p => GetExpressionForCtor(p, overriddenProperties, true))); - - return new ConstructorInitializer(false, args.ToArray()); - } - - /// - /// Builds parameters for private protected inheritance constructor (includes discriminator). - /// - private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildPrivateProtectedInheritanceParameters() - { - var (parameters, standardInitializer) = BuildConstructorParameters(true, includeDiscriminatorParameter: true); - - var initializer = CreateBaseConstructorCallWithDiscriminatorParameter(standardInitializer, parameters.FirstOrDefault()); - - return (parameters, initializer); - } - - /// - /// Creates a base constructor call that uses the discriminator parameter. - /// - private ConstructorInitializer? CreateBaseConstructorCallWithDiscriminatorParameter( - ConstructorInitializer? standardInitializer, - ParameterProvider? discriminatorParam) - { - if (BaseModelProvider?.Type is null || standardInitializer is null || discriminatorParam is null) - return standardInitializer; - - var originalArgs = standardInitializer.Arguments; - if (originalArgs.Count == 0) - return standardInitializer; - - var newArgs = new List - { - discriminatorParam.AsVariable() - }; - newArgs.AddRange(originalArgs.Skip(1)); - - return new ConstructorInitializer(standardInitializer.IsBase, newArgs); } /// @@ -818,7 +735,7 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( if (baseParameters.Count > 0) { // Check if base model has dual constructor pattern and we should call private protected constructor - if (isPrimaryConstructor && BaseModelProvider.ShouldHaveDualConstructorPattern()) + if (isPrimaryConstructor && BaseModelProvider._isMultiLevelDiscriminator) { // Call base model's private protected constructor with discriminator value var args = new List(); @@ -853,8 +770,7 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( ? baseParameters : baseParameters.Where(p => p.Property is null - || (p.Property.IsDiscriminator && !overriddenProperties.Contains(p.Property) && (!isPrimaryConstructor || includeDiscriminatorParameter)) - || (!p.Property.IsDiscriminator && !overriddenProperties.Contains(p.Property)))); + || (!overriddenProperties.Contains(p.Property!) && (!p.Property.IsDiscriminator || !isPrimaryConstructor || includeDiscriminatorParameter)))); if (includeDiscriminatorParameter && _inputModel.DiscriminatorProperty != null) { From 62a0470b6f69fe7b9bf60155f95e606049203dc2 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 24 Nov 2025 21:57:09 -0800 Subject: [PATCH 09/12] renamed var --- .../src/Providers/ModelProvider.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 4303ed96775..d5353ee32e4 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -697,14 +697,14 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( } private (IReadOnlyList Parameters, ConstructorInitializer? Initializer) BuildConstructorParameters( - bool isPrimaryConstructor, bool includeDiscriminatorParameter = false) + bool isInitializationConstructor, bool includeDiscriminatorParameter = false) { var baseParameters = new List(); var constructorParameters = new List(); IEnumerable baseProperties = []; IEnumerable baseFields = []; - if (isPrimaryConstructor) + if (isInitializationConstructor) { baseProperties = GetAllBasePropertiesForConstructorInitialization(); baseFields = GetAllBaseFieldsForConstructorInitialization(); @@ -719,13 +719,13 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( // add the base parameters, if any foreach (var property in baseProperties) { - AddInitializationParameterForCtor(baseParameters, Type.IsStruct, isPrimaryConstructor, property); + AddInitializationParameterForCtor(baseParameters, Type.IsStruct, isInitializationConstructor, property); } // add the base fields, if any foreach (var field in baseFields) { - AddInitializationParameterForCtor(baseParameters, Type.IsStruct, isPrimaryConstructor, field: field); + AddInitializationParameterForCtor(baseParameters, Type.IsStruct, isInitializationConstructor, field: field); } // construct the initializer using the parameters from base signature @@ -735,18 +735,18 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( if (baseParameters.Count > 0) { // Check if base model has dual constructor pattern and we should call private protected constructor - if (isPrimaryConstructor && BaseModelProvider._isMultiLevelDiscriminator) + if (isInitializationConstructor && BaseModelProvider._isMultiLevelDiscriminator) { // Call base model's private protected constructor with discriminator value var args = new List(); args.Add(Literal(_inputModel.DiscriminatorValue ?? "")); - args.AddRange(baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))); + args.AddRange(baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isInitializationConstructor))); constructorInitializer = new ConstructorInitializer(true, args); } else { // Standard base constructor call - constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))]); + constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isInitializationConstructor))]); } } else @@ -758,19 +758,19 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( foreach (var property in CanonicalView.Properties) { - AddInitializationParameterForCtor(constructorParameters, Type.IsStruct, isPrimaryConstructor, property); + AddInitializationParameterForCtor(constructorParameters, Type.IsStruct, isInitializationConstructor, property); } foreach (var field in CanonicalView.Fields) { - AddInitializationParameterForCtor(constructorParameters, Type.IsStruct, isPrimaryConstructor, field: field); + AddInitializationParameterForCtor(constructorParameters, Type.IsStruct, isInitializationConstructor, field: field); } constructorParameters.InsertRange(0, _inputModel.IsUnknownDiscriminatorModel ? baseParameters : baseParameters.Where(p => p.Property is null - || (!overriddenProperties.Contains(p.Property!) && (!p.Property.IsDiscriminator || !isPrimaryConstructor || includeDiscriminatorParameter)))); + || (!overriddenProperties.Contains(p.Property!) && (!p.Property.IsDiscriminator || !isInitializationConstructor || includeDiscriminatorParameter)))); if (includeDiscriminatorParameter && _inputModel.DiscriminatorProperty != null) { @@ -781,7 +781,7 @@ p.Property is null constructorParameters.Insert(0, discriminatorParam); } - if (!isPrimaryConstructor) + if (!isInitializationConstructor) { foreach (var property in AdditionalPropertyProperties) { From 25433f85f9bc3864d2477a870e24c60d34a02bc2 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 24 Nov 2025 22:05:56 -0800 Subject: [PATCH 10/12] minor equal changes --- .../src/Providers/ModelProvider.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index d5353ee32e4..c027a4daf35 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -597,10 +597,8 @@ private bool ComputeIsMultiLevelDiscriminator() // If both models have discriminator properties with the same name, // and this model has derived models, it needs the dual constructor pattern - return string.Equals( - _inputModel.DiscriminatorProperty.Name, - _inputModel.BaseModel.DiscriminatorProperty.Name, - StringComparison.OrdinalIgnoreCase); + return _inputModel.DiscriminatorProperty.Name == + _inputModel.BaseModel.DiscriminatorProperty.Name; } /// From 0393d126d356bef479f53c807e9fc5fa7aaaa56e Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 25 Nov 2025 09:12:27 -0800 Subject: [PATCH 11/12] refsctored --- .../src/Providers/ModelProvider.cs | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index c027a4daf35..788ba3824c7 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -644,7 +644,7 @@ private ConstructorProvider BuildFullConstructor() this); } - private IEnumerable GetAllBasePropertiesForConstructorInitialization() + private IEnumerable GetAllBasePropertiesForConstructorInitialization(bool includeAllHierarchyDiscriminator = false) { var properties = new Stack>(); var modelProvider = BaseModelProvider; @@ -656,9 +656,8 @@ private IEnumerable GetAllBasePropertiesForConstructorInitiali { if (property.IsDiscriminator) { - // In the case of nested discriminators, we only need to include the direct base discriminator property, - // as this is the only one that will be initialized in this model's constructor. - if (isDirectBase) + // In the case of nested discriminators, include discriminator property based on the parameter + if (isDirectBase || includeAllHierarchyDiscriminator) { properties.Peek().Add(property); } @@ -704,7 +703,7 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( if (isInitializationConstructor) { - baseProperties = GetAllBasePropertiesForConstructorInitialization(); + baseProperties = GetAllBasePropertiesForConstructorInitialization(includeDiscriminatorParameter); baseFields = GetAllBaseFieldsForConstructorInitialization(); } else if (BaseModelProvider?.FullConstructor.Signature != null) @@ -738,7 +737,8 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( // Call base model's private protected constructor with discriminator value var args = new List(); args.Add(Literal(_inputModel.DiscriminatorValue ?? "")); - args.AddRange(baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isInitializationConstructor))); + var filteredParams = baseParameters.Where(p => p.Property is null || !p.Property.IsDiscriminator).ToList(); + args.AddRange(filteredParams.Select(p => GetExpressionForCtor(p, overriddenProperties, isInitializationConstructor))); constructorInitializer = new ConstructorInitializer(true, args); } else @@ -770,8 +770,17 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( p.Property is null || (!overriddenProperties.Contains(p.Property!) && (!p.Property.IsDiscriminator || !isInitializationConstructor || includeDiscriminatorParameter)))); + // Replace any discriminator property parameters with the standard discriminatorValue parameter if (includeDiscriminatorParameter && _inputModel.DiscriminatorProperty != null) { + // Remove any discriminator property parameters from the hierarchy + var discriminatorParams = constructorParameters.Where(p => p.Property?.IsDiscriminator == true).ToList(); + foreach (var param in discriminatorParams) + { + constructorParameters.Remove(param); + } + + // Add the standard discriminatorValue parameter at the beginning var discriminatorParam = new ParameterProvider( DiscriminatorParameterName, $"{DiscriminatorParameterDescription}", From 925a2b0fabd9aa0c667e0f0eeb96d05aea6adc13 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 25 Nov 2025 17:01:09 -0800 Subject: [PATCH 12/12] removed hardcoded discriminator --- .../src/Providers/ModelProvider.cs | 20 ------------------- .../ModelProviders/ModelProviderTests.cs | 2 +- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 788ba3824c7..9ed1df69d43 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -19,8 +19,6 @@ namespace Microsoft.TypeSpec.Generator.Providers public class ModelProvider : TypeProvider { private const string AdditionalBinaryDataPropsFieldDescription = "Keeps track of any properties unknown to the library."; - private const string DiscriminatorParameterName = "discriminatorValue"; - private const string DiscriminatorParameterDescription = "The discriminator property."; private readonly InputModelType _inputModel; // Note the description cannot be built from the constructor as it would lead to a circular dependency between the base // and derived models resulting in a stack overflow. @@ -770,24 +768,6 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( p.Property is null || (!overriddenProperties.Contains(p.Property!) && (!p.Property.IsDiscriminator || !isInitializationConstructor || includeDiscriminatorParameter)))); - // Replace any discriminator property parameters with the standard discriminatorValue parameter - if (includeDiscriminatorParameter && _inputModel.DiscriminatorProperty != null) - { - // Remove any discriminator property parameters from the hierarchy - var discriminatorParams = constructorParameters.Where(p => p.Property?.IsDiscriminator == true).ToList(); - foreach (var param in discriminatorParams) - { - constructorParameters.Remove(param); - } - - // Add the standard discriminatorValue parameter at the beginning - var discriminatorParam = new ParameterProvider( - DiscriminatorParameterName, - $"{DiscriminatorParameterDescription}", - new CSharpType(typeof(string))); - constructorParameters.Insert(0, discriminatorParam); - } - if (!isInitializationConstructor) { foreach (var property in AdditionalPropertyProperties) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index e1fcd533ff3..234654be7eb 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -1536,7 +1536,7 @@ public void TestMultiLayerDiscriminator_ThreeLayers() // Cat's protected constructor should have: discriminator + species + name + meows var catProtectedParams = catProtectedConstructor.Signature.Parameters; Assert.AreEqual(4, catProtectedParams.Count); - Assert.AreEqual("discriminatorValue", catProtectedParams[0].Name); + Assert.AreEqual("kind", catProtectedParams[0].Name); Assert.AreEqual(typeof(string), catProtectedParams[0].Type.FrameworkType); Assert.AreEqual("species", catProtectedParams[1].Name); Assert.AreEqual("name", catProtectedParams[2].Name);