Skip to content

Commit 21b004c

Browse files
authored
Hierarchy building (#8999)
This pull request introduces a new dual constructor pattern for intermediate models in discriminated union hierarchies, ensuring correct instantiation and inheritance when models share discriminator property names with their base models. It adds logic to detect when this pattern is needed, generates the appropriate constructors, and updates constructor initialization to support multi-layer discriminator scenarios. Several new tests are included to validate these changes. **Constructor Pattern Enhancements** * Added a new `ConstructorType` enum to define different constructor generation strategies, supporting public, private protected, and internal constructors for various use cases. * Implemented logic in `ModelProvider` to detect when a model should use the dual constructor pattern (i.e., when it shares a discriminator property name with its base and has derived models). * Added methods to generate three constructors for intermediate discriminated models: a public constructor (for external use), a private protected constructor (for inheritance), and an internal constructor (for serialization). **Constructor Initialization Logic** * Updated constructor initialization to correctly call the appropriate base constructor, including passing discriminator values when required for intermediate models using the dual constructor pattern. **Testing Multi-Layer Discriminator Hierarchies** * Added comprehensive tests to cover multi-layer discriminator scenarios, including cases with and without discriminator properties at intermediate layers, and verifying correct constructor argument initialization. **Minor Cleanup** * Removed an unused `discriminatedKind` argument in a test setup for clarity. Addresses: Azure/azure-sdk-for-net#51958
1 parent 1530d5a commit 21b004c

File tree

3 files changed

+365
-20
lines changed

3 files changed

+365
-20
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ public void TestNestedDiscriminatorDynamicModel(bool discriminatedTypeIsDynamicM
105105
]);
106106
InputModelType catModel = InputFactory.Model(
107107
"cat",
108-
discriminatedKind: "cat",
109108
properties:
110109
[
111110
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),

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

Lines changed: 112 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ protected override FormattableString BuildDescription()
5252
}
5353

5454
private readonly bool _isAbstract;
55+
private readonly bool _isMultiLevelDiscriminator;
5556

5657
private readonly CSharpType _additionalBinaryDataPropsFieldType = typeof(IDictionary<string, BinaryData>);
5758
private readonly Type _additionalPropsUnknownType = typeof(BinaryData);
@@ -66,6 +67,7 @@ public ModelProvider(InputModelType inputModel) : base(inputModel)
6667
{
6768
_inputModel = inputModel;
6869
_isAbstract = _inputModel.DiscriminatorProperty is not null && _inputModel.DiscriminatorValue is null;
70+
_isMultiLevelDiscriminator = ComputeIsMultiLevelDiscriminator();
6971

7072
if (_inputModel.BaseModel is not null)
7173
{
@@ -522,7 +524,7 @@ protected internal override ConstructorProvider[] BuildConstructors()
522524
return [FullConstructor];
523525
}
524526

525-
// Build the initialization constructor
527+
// Build the standard single initialization constructor
526528
var accessibility = DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)
527529
? MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected
528530
: _inputModel.Usage.HasFlag(InputModelTypeUsage.Input)
@@ -543,12 +545,80 @@ protected internal override ConstructorProvider[] BuildConstructors()
543545
},
544546
this);
545547

548+
var constructors = new List<ConstructorProvider> { constructor };
549+
550+
// Add FullConstructor if parameters are different
546551
if (!constructorParameters.SequenceEqual(FullConstructor.Signature.Parameters))
547552
{
548-
return [constructor, FullConstructor];
553+
constructors.Add(FullConstructor);
554+
}
555+
556+
// For multi-level discriminators, add one additional private protected constructor
557+
if (_isMultiLevelDiscriminator)
558+
{
559+
var protectedConstructor = BuildProtectedInheritanceConstructor();
560+
constructors.Add(protectedConstructor);
549561
}
550562

551-
return [constructor];
563+
return [.. constructors];
564+
}
565+
566+
/// <summary>
567+
/// Determines if this model should have a dual constructor pattern.
568+
/// This is needed when the model shares the same discriminator property name as its base model
569+
/// AND has derived models, indicating it's an intermediate type in a discriminated union hierarchy.
570+
/// </summary>
571+
private bool ComputeIsMultiLevelDiscriminator()
572+
{
573+
// Only applies to non-abstract models with a base model
574+
if (_isAbstract || _inputModel.BaseModel == null)
575+
{
576+
return false;
577+
}
578+
// Must have derived models to be considered an intermediate type
579+
if (_inputModel.DerivedModels.Count == 0)
580+
{
581+
return false;
582+
}
583+
584+
// Check if this model has a discriminator property in the input
585+
if (_inputModel.DiscriminatorProperty == null)
586+
{
587+
return false;
588+
}
589+
590+
// Check if base model has a discriminator property with the same name
591+
if (_inputModel.BaseModel.DiscriminatorProperty == null)
592+
{
593+
return false;
594+
}
595+
596+
// If both models have discriminator properties with the same name,
597+
// and this model has derived models, it needs the dual constructor pattern
598+
return _inputModel.DiscriminatorProperty.Name ==
599+
_inputModel.BaseModel.DiscriminatorProperty.Name;
600+
}
601+
602+
/// <summary>
603+
/// Builds a private protected constructor for multi-level discriminator inheritance.
604+
/// This allows derived models to call this constructor with their discriminator value.
605+
/// </summary>
606+
private ConstructorProvider BuildProtectedInheritanceConstructor()
607+
{
608+
var (parameters, initializer) = BuildConstructorParameters(true, includeDiscriminatorParameter: true);
609+
610+
return new ConstructorProvider(
611+
signature: new ConstructorSignature(
612+
Type,
613+
$"Initializes a new instance of {Type:C}",
614+
MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected,
615+
parameters,
616+
initializer: initializer),
617+
bodyStatements: new MethodBodyStatement[]
618+
{
619+
GetPropertyInitializers(true, parameters: parameters)
620+
},
621+
this);
552622
}
553623

554624
/// <summary>
@@ -558,7 +628,6 @@ protected internal override ConstructorProvider[] BuildConstructors()
558628
private ConstructorProvider BuildFullConstructor()
559629
{
560630
var (ctorParameters, ctorInitializer) = BuildConstructorParameters(false);
561-
562631
return new ConstructorProvider(
563632
signature: new ConstructorSignature(
564633
Type,
@@ -573,7 +642,7 @@ private ConstructorProvider BuildFullConstructor()
573642
this);
574643
}
575644

576-
private IEnumerable<PropertyProvider> GetAllBasePropertiesForConstructorInitialization()
645+
private IEnumerable<PropertyProvider> GetAllBasePropertiesForConstructorInitialization(bool includeAllHierarchyDiscriminator = false)
577646
{
578647
var properties = new Stack<List<PropertyProvider>>();
579648
var modelProvider = BaseModelProvider;
@@ -585,9 +654,8 @@ private IEnumerable<PropertyProvider> GetAllBasePropertiesForConstructorInitiali
585654
{
586655
if (property.IsDiscriminator)
587656
{
588-
// In the case of nested discriminators, we only need to include the direct base discriminator property,
589-
// as this is the only one that will be initialized in this model's constructor.
590-
if (isDirectBase)
657+
// In the case of nested discriminators, include discriminator property based on the parameter
658+
if (isDirectBase || includeAllHierarchyDiscriminator)
591659
{
592660
properties.Peek().Add(property);
593661
}
@@ -624,16 +692,16 @@ private IEnumerable<FieldProvider> GetAllBaseFieldsForConstructorInitialization(
624692
}
625693

626694
private (IReadOnlyList<ParameterProvider> Parameters, ConstructorInitializer? Initializer) BuildConstructorParameters(
627-
bool isPrimaryConstructor)
695+
bool isInitializationConstructor, bool includeDiscriminatorParameter = false)
628696
{
629697
var baseParameters = new List<ParameterProvider>();
630698
var constructorParameters = new List<ParameterProvider>();
631699
IEnumerable<PropertyProvider> baseProperties = [];
632700
IEnumerable<FieldProvider> baseFields = [];
633701

634-
if (isPrimaryConstructor)
702+
if (isInitializationConstructor)
635703
{
636-
baseProperties = GetAllBasePropertiesForConstructorInitialization();
704+
baseProperties = GetAllBasePropertiesForConstructorInitialization(includeDiscriminatorParameter);
637705
baseFields = GetAllBaseFieldsForConstructorInitialization();
638706
}
639707
else if (BaseModelProvider?.FullConstructor.Signature != null)
@@ -646,36 +714,61 @@ private IEnumerable<FieldProvider> GetAllBaseFieldsForConstructorInitialization(
646714
// add the base parameters, if any
647715
foreach (var property in baseProperties)
648716
{
649-
AddInitializationParameterForCtor(baseParameters, Type.IsStruct, isPrimaryConstructor, property);
717+
AddInitializationParameterForCtor(baseParameters, Type.IsStruct, isInitializationConstructor, property);
650718
}
651719

652720
// add the base fields, if any
653721
foreach (var field in baseFields)
654722
{
655-
AddInitializationParameterForCtor(baseParameters, Type.IsStruct, isPrimaryConstructor, field: field);
723+
AddInitializationParameterForCtor(baseParameters, Type.IsStruct, isInitializationConstructor, field: field);
656724
}
657725

658726
// construct the initializer using the parameters from base signature
659-
var constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isPrimaryConstructor))]);
727+
ConstructorInitializer? constructorInitializer = null;
728+
if (BaseModelProvider != null)
729+
{
730+
if (baseParameters.Count > 0)
731+
{
732+
// Check if base model has dual constructor pattern and we should call private protected constructor
733+
if (isInitializationConstructor && BaseModelProvider._isMultiLevelDiscriminator)
734+
{
735+
// Call base model's private protected constructor with discriminator value
736+
var args = new List<ValueExpression>();
737+
args.Add(Literal(_inputModel.DiscriminatorValue ?? ""));
738+
var filteredParams = baseParameters.Where(p => p.Property is null || !p.Property.IsDiscriminator).ToList();
739+
args.AddRange(filteredParams.Select(p => GetExpressionForCtor(p, overriddenProperties, isInitializationConstructor)));
740+
constructorInitializer = new ConstructorInitializer(true, args);
741+
}
742+
else
743+
{
744+
// Standard base constructor call
745+
constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isInitializationConstructor))]);
746+
}
747+
}
748+
else
749+
{
750+
// Even when no base parameters, we still need a base constructor call if there's a base model
751+
constructorInitializer = new ConstructorInitializer(true, Array.Empty<ValueExpression>());
752+
}
753+
}
660754

661755
foreach (var property in CanonicalView.Properties)
662756
{
663-
AddInitializationParameterForCtor(constructorParameters, Type.IsStruct, isPrimaryConstructor, property);
757+
AddInitializationParameterForCtor(constructorParameters, Type.IsStruct, isInitializationConstructor, property);
664758
}
665759

666760
foreach (var field in CanonicalView.Fields)
667761
{
668-
AddInitializationParameterForCtor(constructorParameters, Type.IsStruct, isPrimaryConstructor, field: field);
762+
AddInitializationParameterForCtor(constructorParameters, Type.IsStruct, isInitializationConstructor, field: field);
669763
}
670764

671765
constructorParameters.InsertRange(0, _inputModel.IsUnknownDiscriminatorModel
672766
? baseParameters
673767
: baseParameters.Where(p =>
674768
p.Property is null
675-
|| (p.Property.IsDiscriminator && !overriddenProperties.Contains(p.Property) && !isPrimaryConstructor)
676-
|| (!p.Property.IsDiscriminator && !overriddenProperties.Contains(p.Property))));
769+
|| (!overriddenProperties.Contains(p.Property!) && (!p.Property.IsDiscriminator || !isInitializationConstructor || includeDiscriminatorParameter))));
677770

678-
if (!isPrimaryConstructor)
771+
if (!isInitializationConstructor)
679772
{
680773
foreach (var property in AdditionalPropertyProperties)
681774
{

0 commit comments

Comments
 (0)