Skip to content

[TCGC] Convert inline discriminated unions to polymorphic models by default - #5743

Open
Chenjie Shi (tadelesh) wants to merge 7 commits into
Azure:mainfrom
tadelesh:feat/convert-discriminated-unions
Open

Chenjie Shi (tadelesh) wants to merge 7 commits into
Azure:mainfrom
tadelesh:feat/convert-discriminated-unions

Conversation

@tadelesh

@tadelesh Chenjie Shi (tadelesh) commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

Implement the non-envelope discriminated-union conversion agreed in #5703.

  • Convert eligible @discriminated(#{ envelope: "none" }) unions with an explicit named model base and nominally inherited model variants to the existing polymorphic SDK model representation by default.
  • Reuse the declared base model's name and identity, populate discriminator properties/tag values/subtype mappings, and preserve recursive references, containers, nullable uses, client names, enum discriminator types, usage, access, and serialization metadata.
  • Add convertDiscriminatedUnionsToModels: false to createSdkContext options so Rust and other emitters can retain native discriminated unions.
  • Leave ordinary unions, object envelopes, SSE event unions, structural-only alternatives, unconstrained unions, defaults, and base-model alternatives unconverted. Report an error for conflicting hierarchy metadata without overwriting existing metadata or changing the base-model alias.
  • Document eligibility, opt-out behavior, and the shared-base tradeoff: compatible unions share a combined subtype map rather than preserving per-union alternative subsets.

This PR is based on current main, does not depend on the separate union-variant-name work, and does not change language emitter code, the core submodule, or the lockfile.

Companion wire-contract scenarios: microsoft/typespec#12288. Per-language emitter integration and the SDK representation of envelope unions remain separate work.

Validation

  • Built TCGC and its workspace dependencies.
  • Complete TCGC suite: 1,705 passed, 2 skipped.
  • Repository-wide pnpm format and pnpm lint passed.
  • TCGC build and tests passed with Node.js 26.0.0.
  • Chronus change-description validation and git diff --check passed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

All changed packages have been documented.

  • ✅ @azure-tools/typespec-client-generator-core
Show changes

@azure-tools/typespec-client-generator-core - feature ✏️

Convert inline-discriminated unions with explicit nominal model hierarchies to polymorphic SDK models by default. Set convertDiscriminatedUnionsToModels: false in createSdkContext to preserve union representations. Ordinary unions and object-envelope unions retain their existing behavior.

@pkg-pr-new

pkg-pr-new Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@azure-tools/typespec-client-generator-core@5743

commit: f2d92d9

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

📦 Package size report

1 package changed size, +4.74 KB (+0.0%) packed overall.

Package Packed (base → head) Δ Packed Unpacked (base → head) Δ Unpacked
@azure-tools/typespec-client-generator-core 243.98 KB → 246.94 KB +2.95 KB (+1.2%) 🔴 1.30 MB → 1.32 MB +16.30 KB (+1.2%) 🔴
13 package(s) with no notable change
Package Packed (base → head) Δ Packed Unpacked (base → head) Δ Unpacked
@azure-tools/typespec-java 20.21 MB → 20.22 MB +1.70 KB (+0.0%) 22.51 MB → 22.51 MB +1.04 KB (+0.0%)
@azure-tools/typespec-azure-resource-manager 186.58 KB → 186.64 KB +68 B (+0.0%) 1.12 MB → 1.12 MB +632 B (+0.1%)
@azure-tools/typespec-azure-rulesets 5.57 KB → 5.59 KB +20 B (+0.4%) 34.21 KB → 34.40 KB +193 B (+0.6%)
@azure-tools/typespec-ts 530.77 KB → 530.77 KB -2 B (-0.0%) 2.67 MB → 2.67 MB —
@azure-tools/azure-http-specs 143.52 KB → 143.52 KB — 1.20 MB → 1.20 MB —
@azure-tools/typespec-autorest 87.87 KB → 87.87 KB — 424.17 KB → 424.17 KB —
@azure-tools/typespec-autorest-canonical 7.42 KB → 7.42 KB — 26.00 KB → 26.00 KB —
@azure-tools/typespec-azure-core 136.55 KB → 136.55 KB — 733.30 KB → 733.30 KB —
@azure-tools/typespec-azure-examples 80.15 KB → 80.15 KB — 383.36 KB → 383.36 KB —
@azure-tools/typespec-azure-portal-core 36.26 KB → 36.26 KB — 242.62 KB → 242.62 KB —
@azure-tools/typespec-go 263.76 KB → 263.76 KB — 1.35 MB → 1.35 MB —
@azure-tools/typespec-metadata 15.91 KB → 15.91 KB — 62.26 KB → 62.26 KB —
@azure-tools/typespec-python 42.54 KB → 42.54 KB — 165.94 KB → 165.94 KB —

Packed = gzipped .tgz published to npm. Unpacked = total extracted size. 🆕 added, 🗑️ removed. Packages from the core/ submodule are not included.
🔴 grew · 🟢 shrank — only changes of at least 512 B and 0.5% are marked.

@azure-sdk-automation

Copy link
Copy Markdown
Contributor

You can try these changes here

🛝 Playground 🌐 Website

Comment thread packages/typespec-client-generator-core/src/types.ts Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454
Comment thread packages/typespec-client-generator-core/src/types.ts Outdated
Comment thread packages/typespec-client-generator-core/src/types.ts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved metadata and subtype-handling defects can produce incorrect generated SDKs.

5 open findings
What changed in this PR

Converts eligible inline-discriminated unions into TCGC’s existing polymorphic SDK models by default, supporting the design discussed in #5703.

Changes:

  • Reuses declared base models and populates discriminator metadata.
  • Adds an emitter opt-out and conflict diagnostics.
  • Adds conversion tests, documentation, and a release note.
File Description
website/​src/​content/​docs/​docs/​libraries/​typespec-client-generator-core/​guideline.md Documents eligibility, opt-out, and shared-base behavior.
packages/​typespec-client-generator-core/​test/​types/​union.test.ts Tests conversion, exclusions, metadata, and conflicts.
packages/​typespec-client-generator-core/​src/​types.ts Implements conversion and metadata propagation.
packages/​typespec-client-generator-core/​src/​lib.ts Registers the conflict diagnostic.
packages/​typespec-client-generator-core/​src/​interfaces.ts Adds conversion settings and alias tracking.
packages/​typespec-client-generator-core/​src/​diagnostics/​union-model-conversion-conflict.md Explains conflicts and remediation.
packages/​typespec-client-generator-core/​src/​context.ts Enables conversion by default with an opt-out.
.chronus/​changes/​tcgc-inline-discriminated-union-conversion.md Records the feature release note.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/typespec-client-generator-core/src/types.ts Outdated
Comment thread packages/typespec-client-generator-core/src/types.ts
Comment thread packages/typespec-client-generator-core/src/types.ts
Comment thread packages/typespec-client-generator-core/src/types.ts
Comment thread packages/typespec-client-generator-core/src/types.ts
Comment thread packages/typespec-client-generator-core/src/types.ts
Comment thread packages/typespec-client-generator-core/src/types.ts
Comment thread packages/typespec-client-generator-core/src/types.ts
Comment thread packages/typespec-client-generator-core/src/types.ts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454
tadelesh added 2 commits October 16, 2026 17:12
Preserve native union representations when the declared base is an alternative, including base-only unions, and update regression coverage and guidance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454
Keep the PR scoped to TCGC and retain the original base-alternative eligibility handling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454
@tadelesh Chenjie Shi (tadelesh) removed the emitter:typescript Issues for @azure-tools/typespec-ts emitter label Oct 16, 2026
Comment thread packages/typespec-client-generator-core/src/lib.ts Outdated
Comment thread packages/typespec-client-generator-core/test/types/union.test.ts Outdated
Comment thread packages/typespec-client-generator-core/test/types/union.test.ts Outdated
Comment thread packages/typespec-client-generator-core/test/types/union.test.ts
Apply diagnostic code formatting, remove the two requested tests, and strengthen middle-layer root discriminator coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454
Copilot AI balanced review requested due to automatic review settings October 19, 2026 07:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Recursive construction can bypass discriminator conflict detection, and synthesized properties lose exact-name metadata.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Medium severity Preserve discriminator metadata across traversal orders

packages/​typespec-client-generator-core/​src/​types.ts:752

With @discriminator("type") model PetBase { next?: Pet; type: string; }, Cat extends PetBase { type: "cat"; }, and an inline Pet extends PetBase { feline: Cat } using kind, visiting the base first reaches this guard before its discriminator is initialized (types.ts:1165). Conversion therefore misses the conflict; the later model-discriminator pass leaves both properties marked as discriminators and overwrites Cat's tag while retaining the union's subtype key. Validate the raw hierarchy metadata or defer conversion metadata changes until model initialization completes, preserving the union's base alias, and test both traversal orders.

Medium severity Preserve exact-name flag for inherited discriminators

packages/​typespec-client-generator-core/​src/​types.ts:844

For an inherited discriminator named with @@clientName(PetBase.kind, exact("client_kind")), synthesis copies the source name but resets isExactName to false. TypeScript's normalizeSdkName (packages/typespec-ts/src/utils/name-utils.ts:223–235) then normalizes the subtype property to clientKind while the base retains client_kind. Copy the source's exact-name flag and add a regression assertion for the synthesized property.

5 resolved since last review

Review effort: Balanced

Timothee Guerin (timotheeguerin) pushed a commit to timotheeguerin/typespec that referenced this pull request Oct 19, 2026
…#12294)

Fix Java handling of tagged descendants behind untagged inheritance
intermediates.

- Do not manufacture an outer discriminator value from an intermediate's
class name or dispatch to it as a tagged subtype.
- Preserve TCGC's subtype map for usage propagation, so tagged
grandchildren are emitted without exposing unrelated descendants.
- Disable shared JSON helpers when an untagged node would cause
inherited fields to be skipped.

Regression coverage compiles generated models and checks deserialization
and JSON round trips with one/two intermediate levels and sharing
enabled/disabled. Genuine nested-discriminator metadata remains covered.

Validation: 136 generator-core tests and 53 emitter tests pass; emitter
build, type-check, lint, and formatting checks pass. Paired with the
TCGC build from Azure/typespec-azure#5743, both
transitive and ancestor/descendant UNION cases generate, compile, and
round-trip successfully. The direct MODEL/UNION control produces nine
byte-identical Java source files.

This does not fix the separate pre-existing TCGC omission in the
transitive MODEL setting.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 33054d5c-f968-4c4e-a9ac-19c8fe6994c4

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib:tcgc Issues for @azure-tools/typespec-client-generator-core library meta:website TypeSpec.io updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants