[http-client-csharp] Model factories use intermediate discriminator for leaf models sharing an inherited discriminator

Offen
  • Stimmt etwas nicht?
#12,151 1 Kommentar 2 Reaktionen 2 zugewiesene Personen Auf GitHub ansehen

Maintainer antworten meist innerhalb von 1 Tag

@JoshLove01 arbeitet bereits daran.

Seit 12.10.2026.

  • #12157 von @copilot-swe-agent — ohne Merge geschlossen

Bewertung

Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Anfängerfreundlichkeit
22/100
Issue-Typ
Bug
Klarheit
Größtenteils klar
Aktivitätsstatus
Aktiv
Tech-Stack
csharp
Bereich
devtools

Rechercherichtung

Start in Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs at EnsureDiscriminatorValueExpression, which only searches the direct base's canonical properties, and in ModelFactoryProvider.cs at GetDiscriminatorExpression, which walks ancestors. Run the focused NUnit management-generator tests for the instant and range query factories. Done means the leaf factories pass their own discriminator value, separate-discriminator hierarchies such as root kind and nested nestedKind still pass, and customization mappings are preserved.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

bug emitter:client:csharp

Summary

@typespec/http-client-csharp selects an intermediate model's discriminator value in model factories when three inheritance levels share one discriminator property. The concrete leaf models have the correct DiscriminatorValue, but their DiscriminatorValueExpression is null. This needs property-specific discriminator resolution in the upstream generator rather than a generated-SDK patch.

Observed in Azure SDK regeneration PR https://github.com/Azure/azure-sdk-for-net/pull/63730, review comment https://github.com/Azure/azure-sdk-for-net/pull/63730#discussion_r4237098529.

Minimal generator-input reproduction

Using the existing management-generator NUnit infrastructure and upstream InputFactory/ModelFactoryProvider, construct:

var discriminatorEnum = InputFactory.StringEnum(
    "AlertsManagementType",
    [("PrometheusEnrichmentItem", "PrometheusEnrichmentItem"),
     ("PrometheusInstantQuery", "PrometheusInstantQuery"),
     ("PrometheusRangeQuery", "PrometheusRangeQuery")],
    access: "internal", isExtensible: true, clientNamespace: "Samples.Models");
var instantQuery = InputFactory.Model(
    "PrometheusInstantQuery", properties: [], discriminatedKind: "PrometheusInstantQuery");
var rangeQuery = InputFactory.Model(
    "PrometheusRangeQuery", properties: [], discriminatedKind: "PrometheusRangeQuery");
var prometheusItem = InputFactory.Model(
    "PrometheusEnrichmentItem", properties: [], discriminatedKind: "PrometheusEnrichmentItem",
    derivedModels: [instantQuery, rangeQuery]);
var enrichmentItem = InputFactory.Model(
    "AlertEnrichmentItem",
    properties: [InputFactory.Property("alertEnrichmentType", discriminatorEnum,
        isRequired: true, isDiscriminator: true)],
    derivedModels: [prometheusItem]);

The hierarchy has one discriminator property declared on the root; neither the intermediate nor leaf models declares another discriminator.

The unmodified upstream factory body, before management constructor repair, is:

return new PrometheusInstantQuery(
    AlertsManagementType.PrometheusEnrichmentItem,
    additionalBinaryDataProperties: null);

The range-query factory likewise uses PrometheusEnrichmentItem.

Expected: the respective AlertsManagementType.PrometheusInstantQuery / AlertsManagementType.PrometheusRangeQuery value.

Impact in the SDK

Both generated factory constructors forward the supplied discriminator unchanged. Serialization writes "type":"PrometheusEnrichmentItem", while polymorphic readers dispatch concrete queries only for "PrometheusInstantQuery" and "PrometheusRangeQuery". Reading through the base type falls through to UnknownPrometheusEnrichmentItem, losing the concrete query type.

This impact was validated by inspecting the generated constructor/writer/reader paths; no SDK runtime round-trip test was run for this report.

Root-cause evidence

The focused test reports:

Model DiscriminatorValue DiscriminatorValueExpression Declared discriminator properties
AlertEnrichmentItem null null AlertEnrichmentType
PrometheusEnrichmentItem PrometheusEnrichmentItem correct enum expression none
PrometheusInstantQuery PrometheusInstantQuery null none
PrometheusRangeQuery PrometheusRangeQuery null none

In the installed upstream generator:

  • ModelProvider.EnsureDiscriminatorValueExpression searches only BaseModelProvider.CanonicalView.Properties for the discriminator. The direct parent of each leaf declares no discriminator, so the leaf expression is null.
  • ModelFactoryProvider.GetDiscriminatorExpression recursively walks base models until model.BaseModelProvider.DiscriminatorProperty == property, then returns that level's expression. Here it returns the intermediate model's value rather than the leaf's.

Source locations (the same direct-parent/ancestor-selection logic is also present on current TypeSpec main):

Validation and provenance

  • Reproduced using @typespec/http-client-csharp 1.0.0-alpha.20260918.2, in Azure SDK for .NET main revision e9bdd3ebf344345a7ae2dd1828d0577d33fe1651.
  • Added four local NUnit cases: instant/range factories × visitor-time/write-time management repair. All four fail the expected leaf-discriminator assertion. The upstream bodies are already incorrect before either repair path.
  • A controlled comparison replacing only the management repair helper with its pre-PR #63719 revision, retaining identical current dependencies and test expectations, also fails all four cases: old repair emits typed default, current repair emits the intermediate value. This is not a full historical-checkout comparison.
  • Therefore https://github.com/Azure/azure-sdk-for-net/pull/63719 exposed the intermediate non-null value but did not introduce the underlying concrete-type failure.

Suggested direction and regression coverage

Resolve discriminator values for the specific property, distinguishing a shared discriminator across inheritance levels from separate discriminators declared at different levels. For the shared property, use the most-derived applicable value and the declaring property's enum type/mapping. Preserve customization mappings and unknown-model behavior.

Simply using the leaf's existing DiscriminatorValueExpression is insufficient because it is null here. Conversely, always using the leaf's value would break hierarchies with separate discriminators (e.g. root kind and intermediate nestedKind). Please cover both cases, including upstream factory generation and constructor initialization. Management constructor repair has analogous ancestor-selection logic and will need alignment once upstream resolution is corrected.

- by copilot

Vorherrschende Sprache
Java
Sterne
5.9k
Forks
421
Ø Merge
2 T. 3 Std.
Gemergte PRs (30 T.)
176

Entwicklungsumgebung

In Codespaces öffnen

Startet den Dev-Container des Projekts im Browser, mit Ihrem eigenen GitHub-Konto.

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus microsoft/typespec

Alle Issues in microsoft/typespec

Ähnliche Issues

Weitere Issues zu Java

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.