Skip to content

Commit 9ba642d

Browse files
[http-client-csharp] Build discriminated base description from discriminated base type, not abstract modifier (#11412)
The discriminator "Please note this is the abstract base class..." description was only emitted when a model carried the `Abstract` declaration modifier. Whether a discriminated base type is modeled as abstract is an implementation detail — downstream emitters (e.g. provisioning) may not declare it abstract, causing the derived-classes note to be dropped. ### Changes - **`ModelProvider`**: Added `IsDiscriminatedBaseType` helper — a model with a discriminator property but no discriminator value. - **`BuildDescription`**: Gates the derived-classes note on `IsDiscriminatedBaseType` instead of the `Abstract` modifier, decoupling documentation from abstractness. - **`BuildDeclarationModifiers`**: Reuses the same helper for the abstract decision, keeping the base emitter's behavior unchanged. - **Tests**: Added `DiscriminatedBaseDescriptionIsBuiltEvenWhenNotAbstract`, using a `ModelProvider` subclass that strips `Abstract` to mimic a downstream emitter and asserting the note is still built. ```csharp private bool IsDiscriminatedBaseType => _inputModel.DiscriminatorProperty is not null && _inputModel.DiscriminatorValue is null; ``` <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #11397 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
1 parent 1f7b35c commit 9ba642d

2 files changed

Lines changed: 37 additions & 4 deletions

File tree

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,13 @@ protected override FormattableString BuildDescription()
2727
{
2828
var description = DocHelpers.GetFormattableDescription(_inputModel.Summary, _inputModel.Doc) ??
2929
$"The {Name}.";
30-
if (DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract))
30+
if (_isDiscriminatedBaseType)
3131
{
3232
_derivedModels = BuildDerivedModels();
3333
var publicDerivedModels = _derivedModels.Where(m => m.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public)).ToList();
34-
var derivedClassesDescription =
35-
"Please note this is the abstract base class. The derived classes available for instantiation are: ";
34+
var derivedClassesDescription = DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)
35+
? "Please note this is the abstract base class. The derived classes available for instantiation are: "
36+
: "Please note this is the base class. The derived classes available for instantiation are: ";
3637
bool addComma = publicDerivedModels.Count > 2;
3738
for (int i = 0; i < publicDerivedModels.Count; i++)
3839
{
@@ -65,11 +66,15 @@ protected override FormattableString BuildDescription()
6566
private ModelProvider? _baseModelProvider;
6667
private ConstructorProvider? _fullConstructor;
6768
internal PropertyProvider? DiscriminatorProperty { get; private set; }
69+
70+
private readonly bool _isDiscriminatedBaseType;
71+
6872
private ValueExpression DiscriminatorLiteral => Literal(_inputModel.DiscriminatorValue ?? "");
6973

7074
public ModelProvider(InputModelType inputModel) : base(inputModel)
7175
{
7276
_inputModel = inputModel;
77+
_isDiscriminatedBaseType = inputModel.DiscriminatorProperty is not null && inputModel.DiscriminatorValue is null;
7378
_isMultiLevelDiscriminator = ComputeIsMultiLevelDiscriminator();
7479
_useObjectAdditionalProperties = new Lazy<bool>(ShouldUseObjectAdditionalProperties);
7580
}
@@ -324,7 +329,7 @@ protected override TypeSignatureModifiers BuildDeclarationModifiers()
324329
declarationModifiers |= TypeSignatureModifiers.Internal;
325330
}
326331

327-
if (_inputModel.DiscriminatorProperty is not null && _inputModel.DiscriminatorValue is null)
332+
if (_isDiscriminatedBaseType)
328333
{
329334
declarationModifiers |= TypeSignatureModifiers.Abstract;
330335
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,34 @@ public void BaseShouldBeAbstract(InputModelType inputModel, string expectedSumma
120120
Assert.AreEqual(expectedSummary, baseModel.XmlDocs.Summary!.ToDisplayString());
121121
}
122122

123+
[TestCase(true, "Please note this is the abstract base class. The derived classes available for instantiation are: <see cref=\"Sample.Models.Cat\"/>, <see cref=\"Sample.Models.Dog\"/>, and <see cref=\"Sample.Models.AnotherAnimal\"/>.")]
124+
[TestCase(false, "Please note this is the base class. The derived classes available for instantiation are: <see cref=\"Sample.Models.Cat\"/>, <see cref=\"Sample.Models.Dog\"/>, and <see cref=\"Sample.Models.AnotherAnimal\"/>.")]
125+
public void DiscriminatedBaseDescriptionReflectsAbstractness(bool isAbstract, string expectedDescription)
126+
{
127+
MockHelpers.LoadMockGenerator();
128+
// When not abstract, simulate a downstream emitter that does not model the discriminated base type as abstract.
129+
var baseModel = isAbstract
130+
? CodeModelGenerator.Instance.TypeFactory.CreateModel(_baseModel)!
131+
: new NonAbstractModelProvider(_baseModel);
132+
Assert.AreEqual(isAbstract, baseModel.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract));
133+
134+
// The discriminated base description should reference derived models regardless of abstractness.
135+
Assert.IsNotNull(baseModel.XmlDocs.Summary);
136+
StringAssert.Contains(expectedDescription, baseModel.XmlDocs.Summary!.ToDisplayString());
137+
}
138+
139+
private class NonAbstractModelProvider : ModelProvider
140+
{
141+
public NonAbstractModelProvider(InputModelType inputModel) : base(inputModel)
142+
{
143+
}
144+
145+
protected override TypeSignatureModifiers BuildDeclarationModifiers()
146+
{
147+
return base.BuildDeclarationModifiers() & ~TypeSignatureModifiers.Abstract;
148+
}
149+
}
150+
123151
[Test]
124152
public void DiscriminatorPropertyShouldBeInternal()
125153
{

0 commit comments

Comments
 (0)