From 04300b79eb02a2c9aee2bd104e69556e44ad7686 Mon Sep 17 00:00:00 2001 From: tadelesh Date: Fri, 16 Oct 2026 12:31:35 +0800 Subject: [PATCH 1/7] feat(tcgc): convert inline discriminated unions to model hierarchies Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454 --- ...c-inline-discriminated-union-conversion.md | 7 + .../src/context.ts | 4 + .../union-model-conversion-conflict.md | 3 + .../src/interfaces.ts | 2 + .../typespec-client-generator-core/src/lib.ts | 7 + .../src/types.ts | 324 +++++++++++-- .../test/types/union.test.ts | 433 +++++++++++++++++- .../guideline.md | 32 ++ 8 files changed, 781 insertions(+), 31 deletions(-) create mode 100644 .chronus/changes/tcgc-inline-discriminated-union-conversion.md create mode 100644 packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md diff --git a/.chronus/changes/tcgc-inline-discriminated-union-conversion.md b/.chronus/changes/tcgc-inline-discriminated-union-conversion.md new file mode 100644 index 0000000000..e40c9ead07 --- /dev/null +++ b/.chronus/changes/tcgc-inline-discriminated-union-conversion.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +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. diff --git a/packages/typespec-client-generator-core/src/context.ts b/packages/typespec-client-generator-core/src/context.ts index 90a2e5c4f5..145d5507f6 100644 --- a/packages/typespec-client-generator-core/src/context.ts +++ b/packages/typespec-client-generator-core/src/context.ts @@ -71,6 +71,7 @@ export function createTCGCContext( previewStringRegex: /-preview$/, disableUsageAccessPropagationToBase: false, + convertDiscriminatedUnionsToModels: true, generateProtocolMethods: true, generateConvenienceMethods: true, __referencedTypeCache: new Map< @@ -173,6 +174,8 @@ export interface CreateSdkContextOptions { exportTCGCoutput?: boolean; // this flag is for emitter to export TCGC output as yaml file flattenUnionAsEnum?: boolean; // this flag is for emitter to decide whether tcgc should flatten union as enum enableLegacyHierarchyBuilding?: boolean; // this flag is for emitter to decide whether tcgc should respect the `@hierarchyBuilding` decorator + /** Convert eligible inline-discriminated unions to model hierarchies. Default: true. */ + convertDiscriminatedUnionsToModels?: boolean; } export async function createSdkContext< @@ -238,6 +241,7 @@ export async function createSdkContext< disableUsageAccessPropagationToBase: options?.disableUsageAccessPropagationToBase ?? false, flattenUnionAsEnum: options?.flattenUnionAsEnum ?? true, enableLegacyHierarchyBuilding: options?.enableLegacyHierarchyBuilding ?? true, + convertDiscriminatedUnionsToModels: options?.convertDiscriminatedUnionsToModels ?? true, }; if (context.options["examples-dir"]) { diff --git a/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md b/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md new file mode 100644 index 0000000000..b2231378c3 --- /dev/null +++ b/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md @@ -0,0 +1,3 @@ +An inline-discriminated union cannot be converted to a model hierarchy when its discriminator property or tag mappings conflict with another union or an existing model discriminator. TCGC preserves the union and reports this warning rather than overwriting the conflicting metadata. + +Use consistent discriminator metadata for the shared hierarchy, or set `convertDiscriminatedUnionsToModels: false` in `createSdkContext` to retain discriminated unions. diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index 06caa743b3..831ba5a762 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -66,7 +66,9 @@ export interface TCGCContext { disableUsageAccessPropagationToBase: boolean; flattenUnionAsEnum?: boolean; enableLegacyHierarchyBuilding?: boolean; + convertDiscriminatedUnionsToModels?: boolean; + __discriminatedUnionModels?: Map; __referencedTypeCache: Map; __arrayDictionaryCache: Map; __methodParameterCache: Map; diff --git a/packages/typespec-client-generator-core/src/lib.ts b/packages/typespec-client-generator-core/src/lib.ts index 1f3a5afabc..d316f696df 100644 --- a/packages/typespec-client-generator-core/src/lib.ts +++ b/packages/typespec-client-generator-core/src/lib.ts @@ -171,6 +171,13 @@ function doc(code: string) { export const $lib = createTypeSpecLibrary({ name: "@azure-tools/typespec-client-generator-core", diagnostics: { + "union-model-conversion-conflict": { + ...doc("union-model-conversion-conflict"), + severity: "warning", + messages: { + default: paramMessage`Cannot convert discriminated union '${"unionName"}' to a model hierarchy because its discriminator metadata conflicts with another union or model using the same hierarchy. The union is preserved. Set convertDiscriminatedUnionsToModels to false to retain union representations.`, + }, + }, "union-null": { ...doc("union-null"), severity: "warning", diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 5983978175..c772a32881 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -31,6 +31,7 @@ import { isErrorModel, isNeverType, isTemplateDeclaration, + navigateType, resolveEncodedName, } from "@typespec/compiler"; import { isEvents } from "@typespec/events"; @@ -559,6 +560,26 @@ export function getSdkUnionWithDiagnostics( ): [SdkType, readonly Diagnostic[]] { let retval: SdkType | undefined = context.__referencedTypeCache.get(type); const diagnostics = createDiagnosticCollector(); + initializeDiscriminatedUnionModels(context); + if ( + !retval && + context.__discriminatedUnionModels?.has(type) && + context.__discriminatedUnionModels.get(type) === undefined + ) { + diagnostics.add( + createDiagnostic({ + code: "union-model-conversion-conflict", + target: type, + format: { unionName: type.name ?? "(anonymous)" }, + }), + ); + } + const baseModel = context.__discriminatedUnionModels?.get(type); + if (baseModel) { + const model = diagnostics.pipe(getSdkModelWithDiagnostics(context, baseModel, operation)); + updateReferencedTypeMap(context, type, model); + return diagnostics.wrap(model); + } if (!retval) { const nonNullOptions = getNonNullOptions(type); @@ -692,6 +713,254 @@ export function getSdkUnionWithDiagnostics( return diagnostics.wrap(retval); } +function initializeDiscriminatedUnionModels(context: TCGCContext): void { + if (context.__discriminatedUnionModels || context.convertDiscriminatedUnionsToModels === false) { + return; + } + const models = (context.__discriminatedUnionModels = new Map()); + navigateType( + context.getMutatedGlobalNamespace(), + { + union(union) { + const base = union.baseType; + if ( + isTemplateDeclaration(union) || + isEvents(context.program, union) || + base?.kind !== "Model" || + !base.name || + isArrayModelType(base) || + base.indexer || + union.variants.size === 0 || + getAlternateType(context, union) || + walkBaseChain(base).some( + (model) => + getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), + ) + ) { + return; + } + const discriminated = $(context.program).union.getDiscriminatedUnion(union); + if ( + !discriminated || + discriminated.options.envelope !== "none" || + discriminated.defaultVariant + ) { + return; + } + for (const variant of union.variants.values()) { + if ( + typeof variant.name !== "string" || + variant.type.kind !== "Model" || + variant.type === base || + !walkBaseChain(variant.type).includes(base) || + walkBaseChain(variant.type).some( + (model) => + getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), + ) + ) { + return; + } + } + models.set(union, base); + }, + }, + {}, + ); + const conflicts = new Set(); + const candidates = [...models.entries()]; + for (const [union, base] of candidates) { + if (!base) continue; + const discriminated = $(context.program).union.getDiscriminatedUnion(union)!; + const propertyName = discriminated.options.discriminatorPropertyName; + const existingDiscriminator = getDiscriminator(context.program, base); + if (existingDiscriminator && existingDiscriminator.propertyName !== propertyName) { + conflicts.add(union); + } + for (const model of [base, ...discriminated.variants.values()]) { + if (model.kind !== "Model") continue; + const property = walkBaseChain(model) + .map((ancestor) => ancestor.properties.get(propertyName)) + .find((property) => property !== undefined); + if ( + property && + resolveEncodedName(context.program, property, "application/json") !== propertyName + ) { + conflicts.add(union); + } + } + for (const [otherUnion, otherBase] of candidates) { + const other = $(context.program).union.getDiscriminatedUnion(otherUnion)!; + if (base === otherBase && propertyName !== other.options.discriminatorPropertyName) { + conflicts.add(union); + conflicts.add(otherUnion); + } + for (const [tag, variant] of discriminated.variants) { + for (const [otherTag, otherVariant] of other.variants) { + if ( + (variant === otherVariant && + (tag !== otherTag || propertyName !== other.options.discriminatorPropertyName)) || + (base === otherBase && tag === otherTag && variant !== otherVariant) + ) { + conflicts.add(union); + conflicts.add(otherUnion); + } + } + } + } + } + // A retained union must not reference models reshaped by another conflicting conversion. + let changed = true; + while (changed) { + changed = false; + const affectedModels = new Set(); + for (const [union, base] of candidates) { + if (!conflicts.has(union)) continue; + for (const model of [base, ...[...union.variants.values()].map((variant) => variant.type)]) { + if (model?.kind !== "Model") continue; + for (const ancestor of walkBaseChain(model)) affectedModels.add(ancestor); + } + } + for (const [union, base] of candidates) { + if ( + !conflicts.has(union) && + ((base && walkBaseChain(base).some((model) => affectedModels.has(model))) || + [...union.variants.values()].some( + (variant) => + variant.type.kind === "Model" && + walkBaseChain(variant.type).some((model) => affectedModels.has(model)), + )) + ) { + conflicts.add(union); + changed = true; + } + } + } + for (const union of conflicts) { + models.set(union, undefined); + } +} + +function addUnionDiscriminatorToModel( + context: TCGCContext, + rawModel: Model, + model: SdkModelType, +): [void, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + for (const [union, base] of context.__discriminatedUnionModels ?? []) { + if (!base) continue; + const discriminated = $(context.program).union.getDiscriminatedUnion(union)!; + const propertyName = discriminated.options.discriminatorPropertyName; + if (rawModel === base) { + let property = findDiscriminatorProperty(model, propertyName); + if (!property) { + const inheritedProperty = findInheritedDiscriminatorProperty(model, propertyName); + property = createDiscriminatorProperty( + model, + inheritedProperty?.name ?? propertyName, + inheritedProperty?.type ?? getTypeSpecBuiltInType(context, "string"), + propertyName, + ); + property.serializationOptions.json = { name: propertyName }; + model.properties.unshift(property); + } + property.optional = false; + property.discriminator = true; + model.discriminatorProperty = property; + model.discriminatedSubtypes ??= {}; + for (const [tag, variant] of discriminated.variants) { + compilerAssert(variant.kind === "Model", "Converted union variants must be models.", union); + model.discriminatedSubtypes[tag] = diagnostics.pipe( + getSdkModelWithDiagnostics(context, variant), + ); + } + } else { + for (const [tag, variant] of discriminated.variants) { + if (variant !== rawModel) continue; + let property = findDiscriminatorProperty(model, propertyName); + const inheritedProperty = findInheritedDiscriminatorProperty(model, propertyName); + if (!property) { + const constant: SdkConstantType = { + kind: "constant", + value: tag, + valueType: getTypeSpecBuiltInType(context, "string"), + name: `${model.name}${pascalCase(propertyName)}`, + isGeneratedName: true, + isExactName: false, + decorators: [], + }; + property = createDiscriminatorProperty( + model, + inheritedProperty?.name ?? propertyName, + constant, + propertyName, + ); + property.serializationOptions.json = { name: propertyName }; + model.properties.unshift(property); + } + if (property.type.kind === "constant" && inheritedProperty?.type.kind === "enum") { + const value = inheritedProperty.type.values.find((value) => value.value === tag); + if (value) property.type = value; + } + property.optional = false; + property.discriminator = true; + model.discriminatorValue = tag; + } + } + } + return diagnostics.wrap(undefined); +} + +function findDiscriminatorProperty( + model: SdkModelType, + name: string, +): SdkModelPropertyType | undefined { + return model.properties.find( + (property) => + (property.__raw?.name ?? property.serializationOptions.json?.name ?? property.name) === name, + ); +} + +function findInheritedDiscriminatorProperty( + model: SdkModelType, + name: string, +): SdkModelPropertyType | undefined { + for (let parent = model.baseModel; parent; parent = parent.baseModel) { + const property = findDiscriminatorProperty(parent, name); + if (property) return property; + } + return undefined; +} + +function createDiscriminatorProperty( + model: SdkModelType, + name: string, + type: SdkType, + serializedName: string, +): SdkModelPropertyType { + return { + kind: "property", + doc: `Discriminator property for ${model.name}.`, + optional: false, + discriminator: true, + // eslint-disable-next-line @typescript-eslint/no-deprecated + serializedName, + serializationOptions: {}, + type, + name, + isGeneratedName: false, + isExactName: false, + onClient: false, + apiVersions: model.apiVersions, + isApiVersionParam: false, + // eslint-disable-next-line @typescript-eslint/no-deprecated + isMultipartFileInput: false, + flatten: false, + crossLanguageDefinitionId: `${model.crossLanguageDefinitionId}.${name}`, + decorators: [], + access: "public", + }; +} + function getEmptyUnionType( context: TCGCContext, type: Union, @@ -835,32 +1104,17 @@ function addDiscriminatorToModelType( discriminatorType = getTypeSpecBuiltInType(context, "string"); } const name = discriminatorProperty ? discriminatorProperty.name : discriminator.propertyName; - const discriminatorPropertyType: SdkModelPropertyType = { - kind: "property", - doc: `Discriminator property for ${model.name}.`, - optional: false, - discriminator: true, - // eslint-disable-next-line @typescript-eslint/no-deprecated - serializedName: discriminatorProperty + const discriminatorPropertyType = createDiscriminatorProperty( + model, + name, + discriminatorType!, + discriminatorProperty ? discriminatorProperty.serializedName // eslint-disable-line @typescript-eslint/no-deprecated : discriminator.propertyName, - serializationOptions: {}, - type: discriminatorType!, - name, - isGeneratedName: false, - isExactName: false, - onClient: false, - apiVersions: discriminatorProperty - ? getAvailableApiVersions(context, discriminatorProperty.__raw!, type) - : model.apiVersions, - isApiVersionParam: false, - // eslint-disable-next-line @typescript-eslint/no-deprecated - isMultipartFileInput: false, // discriminator property cannot be a file - flatten: false, // discriminator properties can not be flattened - crossLanguageDefinitionId: `${model.crossLanguageDefinitionId}.${name}`, - decorators: [], - access: "public", - }; + ); + discriminatorPropertyType.apiVersions = discriminatorProperty + ? getAvailableApiVersions(context, discriminatorProperty.__raw!, type) + : model.apiVersions; model.properties.splice(0, 0, discriminatorPropertyType); model.discriminatorProperty = discriminatorPropertyType; } @@ -881,6 +1135,7 @@ export function getSdkModelWithDiagnostics( operation?: Operation, ): [SdkModelType, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); + initializeDiscriminatedUnionModels(context); let sdkType = context.__referencedTypeCache.get(type) as SdkModelType | undefined; if (!sdkType) { @@ -900,6 +1155,11 @@ export function getSdkModelWithDiagnostics( serializationOptions: {}, }; updateReferencedTypeMap(context, type, sdkType); + for (const [union, base] of context.__discriminatedUnionModels ?? []) { + if (base === type) { + updateReferencedTypeMap(context, union, sdkType); + } + } // model MyModel is Record<> {} should be model with additional properties if (type.sourceModel?.kind === "Model" && type.sourceModel?.name === "Record") { @@ -978,6 +1238,7 @@ export function getSdkModelWithDiagnostics( } } diagnostics.pipe(addDiscriminatorToModelType(context, type, sdkType)); + diagnostics.pipe(addUnionDiscriminatorToModel(context, type, sdkType)); updateReferencedTypeMap(context, type, sdkType); } return diagnostics.wrap(sdkType); @@ -2095,14 +2356,16 @@ function updateTypesFromOperation( function updateAccessOverride(context: TCGCContext): [void, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); // set access for all orphan model without override - for (const sdkType of context.__referencedTypeCache.values()) { - const accessOverride = getAccessOverride(context, sdkType.__raw as any); + for (const [rawType, sdkType] of context.__referencedTypeCache) { + if (rawType.kind !== "Model" && rawType.kind !== "Enum" && rawType.kind !== "Union") continue; + const accessOverride = getAccessOverride(context, rawType); if (!sdkType.__accessSet && accessOverride === undefined) { diagnostics.pipe(updateUsageOrAccess(context, "public", sdkType)); } } - for (const sdkType of context.__referencedTypeCache.values()) { - const accessOverride = getAccessOverride(context, sdkType.__raw as any); + for (const [rawType, sdkType] of context.__referencedTypeCache) { + if (rawType.kind !== "Model" && rawType.kind !== "Enum" && rawType.kind !== "Union") continue; + const accessOverride = getAccessOverride(context, rawType); if (accessOverride) { diagnostics.pipe(updateUsageOrAccess(context, accessOverride, sdkType, { isOverride: true })); } @@ -2112,8 +2375,9 @@ function updateAccessOverride(context: TCGCContext): [void, readonly Diagnostic[ function updateUsageOverride(context: TCGCContext): [void, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - for (const sdkType of context.__referencedTypeCache.values()) { - const usageOverride = getUsageOverride(context, sdkType.__raw as any); + for (const [rawType, sdkType] of context.__referencedTypeCache) { + if (rawType.kind !== "Model" && rawType.kind !== "Enum" && rawType.kind !== "Union") continue; + const usageOverride = getUsageOverride(context, rawType); if (usageOverride) { diagnostics.pipe(updateUsageOrAccess(context, usageOverride, sdkType, { isOverride: true })); if (usageOverride & UsageFlags.Json) { diff --git a/packages/typespec-client-generator-core/test/types/union.test.ts b/packages/typespec-client-generator-core/test/types/union.test.ts index 9e01da08d6..acbb2f874b 100644 --- a/packages/typespec-client-generator-core/test/types/union.test.ts +++ b/packages/typespec-client-generator-core/test/types/union.test.ts @@ -1,10 +1,441 @@ import { expectDiagnostics } from "@typespec/compiler/testing"; import { deepStrictEqual, ok, strictEqual } from "assert"; -import { it } from "vitest"; +import { describe, it } from "vitest"; import { type SdkArrayType, type SdkMethodResponse, UsageFlags } from "../../src/interfaces.js"; import { createSdkContextForTester, SimpleTester, SimpleTesterWithService } from "../tester.js"; import { getSdkTypeHelper } from "./utils.js"; +describe("inline discriminated union conversion", () => { + it.each([false, true])( + "converts the hierarchy with injected discriminator=%s", + async (injected) => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { + ${injected ? "" : "kind: string;"} + name: string; + } + model Mammal extends PetBase { age?: int32; } + model Cat extends Mammal { ${injected ? "" : 'kind: "cat";'} meows: boolean; } + model Dog extends PetBase { ${injected ? "" : 'kind: "dog";'} barks: boolean; } + model Unrelated extends PetBase { flies: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat, dog: Dog } + @post op echo(@body pet: Pet): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((x) => x.name === "PetBase"); + ok(base); + strictEqual(context.sdkPackage.unions.length, 0); + const method = context.sdkPackage.clients[0].methods[0]; + strictEqual(method.parameters[0].type, base); + strictEqual(method.response.type, base); + strictEqual(method.operation.bodyParam?.type, base); + strictEqual(method.operation.responses[0].type, base); + strictEqual(base.discriminatorProperty?.name, "kind"); + strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "kind"); + deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), ["cat", "dog"]); + strictEqual(base.properties.filter((property) => property.discriminator).length, 1); + for (const [tag, name] of [ + ["cat", "Cat"], + ["dog", "Dog"], + ]) { + const variant = context.sdkPackage.models.find((x) => x.name === name); + ok(variant); + strictEqual(base.discriminatedSubtypes?.[tag], variant); + strictEqual(variant.discriminatorValue, tag); + const property = variant.properties.find((x) => x.discriminator); + ok(property); + strictEqual(property.type.kind, "constant"); + strictEqual(property.type.value, tag); + strictEqual(property.serializationOptions.json?.name, "kind"); + strictEqual(variant.usage, UsageFlags.Input | UsageFlags.Output | UsageFlags.Json); + } + strictEqual( + context.sdkPackage.models.some((x) => x.name === "Unrelated"), + false, + ); + }, + ); + + it("preserves unions and variant identity when conversion is disabled", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + op get(): Pet; + `); + const context = await createSdkContextForTester( + program, + {}, + { convertDiscriminatedUnionsToModels: false }, + ); + expectDiagnostics(context.diagnostics, { + code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind", + severity: "warning", + }); + const union = context.sdkPackage.unions[0]; + strictEqual(union.kind, "union"); + strictEqual(union.discriminatedOptions?.envelope, "none"); + strictEqual( + union.variantTypes[0], + context.sdkPackage.models.find((model) => model.name === "Cat"), + ); + strictEqual(context.sdkPackage.clients[0].methods[0].response.type, union); + const base = context.sdkPackage.models.find((x) => x.name === "PetBase"); + ok(base); + strictEqual(base.discriminatorProperty, undefined); + strictEqual(base.discriminatedSubtypes, undefined); + strictEqual( + context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, + undefined, + ); + const converted = await createSdkContextForTester(program); + strictEqual(converted.sdkPackage.clients[0].methods[0].response.type?.kind, "model"); + strictEqual(base.discriminatorProperty, undefined); + }); + + it("converts recursive properties, containers, and nullable uses regardless of traversal order", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; next?: Pet; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + model Holder { pet: Pet; pets: Pet[]; lookup: Record; maybe: Pet | null; } + op getCat(): Cat; + @post op echo(@body body: Holder): Holder; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((x) => x.name === "PetBase"); + const holder = context.sdkPackage.models.find((x) => x.name === "Holder"); + ok(base); + ok(holder); + strictEqual(base.properties.find((x) => x.name === "next")?.type, base); + strictEqual(holder.properties[0].type, base); + const array = holder.properties[1].type; + strictEqual(array.kind, "array"); + strictEqual(array.valueType, base); + const dict = holder.properties[2].type; + strictEqual(dict.kind, "dict"); + strictEqual(dict.valueType, base); + const nullable = holder.properties[3].type; + strictEqual(nullable.kind, "nullable"); + strictEqual(nullable.type, base); + strictEqual(context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, "cat"); + }); + + it("uses wire tags independently of client names", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { @clientName("clientKind") type: string; } + @clientName("Kitten") + model Cat extends PetBase { @clientName("clientKind") type: "feline"; } + @discriminated(#{ envelope: "none", discriminatorPropertyName: "type" }) + union Pet extends PetBase { feline: Cat } + op get(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((x) => x.name === "PetBase"); + ok(base); + strictEqual(base.discriminatorProperty?.name, "clientKind"); + strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "type"); + strictEqual(base.discriminatedSubtypes?.feline.name, "Kitten"); + strictEqual(base.discriminatedSubtypes?.feline.discriminatorValue, "feline"); + }); + + it("preserves client names when injecting a subtype discriminator", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { @clientName("clientKind") kind: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + op get(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const cat = context.sdkPackage.models.find((x) => x.name === "Cat"); + ok(cat); + const property = cat.properties.find((x) => x.discriminator); + ok(property); + strictEqual(property.name, "clientKind"); + strictEqual(property.serializationOptions.json?.name, "kind"); + }); + + it("preserves union usage and access overrides on the projected base", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @usage(Usage.input | Usage.output | Usage.json) + @access(Access.public) + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((x) => x.name === "PetBase"); + ok(base); + strictEqual(base.usage, UsageFlags.Input | UsageFlags.Output | UsageFlags.Json); + strictEqual(base.access, "public"); + strictEqual(base.discriminatedSubtypes?.cat.access, "public"); + strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "kind"); + }); + + it("preserves an inherited discriminator's client name and enum type", async () => { + const { program } = await SimpleTesterWithService.compile(` + union PetKind { cat: "cat", dog: "dog" } + model NamedEntity { @clientName("clientKind") kind: PetKind; } + model PetBase extends NamedEntity { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + op get(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + strictEqual(base.discriminatorProperty?.name, "clientKind"); + strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "kind"); + strictEqual(base.discriminatorProperty?.type.kind, "enum"); + strictEqual(base.baseModel?.discriminatorProperty, undefined); + const property = base.discriminatedSubtypes?.cat.properties.find( + (property) => property.discriminator, + ); + ok(property); + strictEqual(property.name, "clientKind"); + strictEqual(property.type.kind, "enumvalue"); + }); + + it.each([false, true])( + "preserves enum discriminator types with injected discriminator=%s", + async (injected) => { + const { program } = await SimpleTesterWithService.compile(` + union PetKind { cat: "cat", dog: "dog" } + model PetBase { kind: PetKind; } + model Cat extends PetBase { ${injected ? "" : 'kind: "cat";'} meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + op get(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + const enumType = base.discriminatorProperty?.type; + ok(enumType); + strictEqual(enumType.kind, "enum"); + strictEqual( + base.discriminatedSubtypes?.cat.properties.find((property) => property.discriminator)?.type, + enumType.values.find((value) => value.value === "cat"), + ); + }, + ); + + it("combines compatible disjoint unions using the same base", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + model Dog extends PetBase { barks: boolean; } + @discriminated(#{ envelope: "none" }) + union IndoorPet extends PetBase { cat: Cat } + @discriminated(#{ envelope: "none" }) + union OutdoorPet extends PetBase { dog: Dog } + @route("/indoor") op indoor(): IndoorPet; + @route("/outdoor") op outdoor(): OutdoorPet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((x) => x.name === "PetBase"); + ok(base); + deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), ["cat", "dog"]); + context.sdkPackage.clients[0].methods.forEach((method) => + strictEqual(method.response.type, base), + ); + strictEqual(base.properties.filter((property) => property.discriminator).length, 1); + strictEqual(context.sdkPackage.models.filter((x) => x.name === "PetBase").length, 1); + }); + + it("retains all unions connected to a conflicting hierarchy", async () => { + const { program } = await SimpleTesterWithService.compile(` + model AnimalBase { name: string; } + model PetBase extends AnimalBase { } + model Cat extends PetBase { meows: boolean; } + model Dog extends PetBase { barks: boolean; } + @discriminated(#{ envelope: "none" }) + union Pets extends PetBase { cat: Cat } + @discriminated(#{ envelope: "none" }) + union OtherPets extends PetBase { feline: Cat } + @discriminated(#{ envelope: "none" }) + union Animals extends AnimalBase { dog: Dog } + @route("/pets") op pets(): Pets; + @route("/others") op others(): OtherPets; + @route("/animals") op animals(): Animals; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics( + context.diagnostics, + Array.from({ length: 3 }, () => [ + { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, + { code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind" }, + ]).flat(), + ); + context.sdkPackage.clients[0].methods.forEach((method) => + strictEqual(method.response.type?.kind, "union"), + ); + strictEqual( + context.sdkPackage.models.find((model) => model.name === "Dog")?.discriminatorValue, + undefined, + ); + }); + + it("propagates internal operation access to the converted hierarchy", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + @access(Access.internal) + op get(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + strictEqual(base.access, "internal"); + strictEqual(base.discriminatedSubtypes?.cat.access, "internal"); + }); + + it("retains conflicting unions instead of overwriting subtype tags", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + @discriminated(#{ envelope: "none" }) + union Animal extends PetBase { feline: Cat } + @route("/pet") op pet(): Pet; + @route("/animal") op animal(): Animal; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, [ + { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, + { code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind" }, + { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, + { code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind" }, + ]); + context.sdkPackage.clients[0].methods.forEach((method) => + strictEqual(method.response.type?.kind, "union"), + ); + strictEqual( + context.sdkPackage.models.find((x) => x.name === "PetBase")?.discriminatorProperty, + undefined, + ); + strictEqual( + context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, + undefined, + ); + }); + + it.each([ + { name: "repeated payload", properties: "", variants: "cat: Cat, feline: Cat" }, + { + name: "encoded discriminator", + properties: '@encodedName("application/json", "type") kind: string;', + variants: "cat: Cat", + }, + ])("reports incompatible $name metadata", async ({ properties, variants }) => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { ${properties} name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { ${variants} } + op get(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, [ + { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, + ...Array.from({ length: variants.includes(",") ? 2 : 1 }, () => ({ + code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind", + })), + ]); + strictEqual(context.sdkPackage.clients[0].methods[0].response.type?.kind, "union"); + strictEqual( + context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, + undefined, + ); + }); + + it.each([ + { + name: "ordinary", + decorator: "", + cat: "extends PetBase", + constraint: "extends PetBase", + variants: "cat: Cat, dog: Dog", + }, + { + name: "envelope", + decorator: "@discriminated", + cat: "extends PetBase", + constraint: "extends PetBase", + variants: "cat: Cat", + }, + { + name: "structural", + decorator: '@discriminated(#{ envelope: "none" })', + cat: "", + constraint: "extends PetBase", + variants: "cat: Cat", + }, + { + name: "no base", + decorator: '@discriminated(#{ envelope: "none" })', + cat: "extends PetBase", + constraint: "", + variants: "cat: Cat", + }, + { + name: "default", + decorator: '@discriminated(#{ envelope: "none" })', + cat: "extends PetBase", + constraint: "extends PetBase", + variants: "cat: Cat, UnknownPet", + }, + { + name: "base alternative", + decorator: '@discriminated(#{ envelope: "none" })', + cat: "extends PetBase", + constraint: "extends PetBase", + variants: "cat: Cat, base: PetBase", + }, + ])("leaves $name unions unchanged", async ({ decorator, cat, constraint, variants }) => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat ${cat} { name: string; meows: boolean; } + model Dog extends PetBase { barks: boolean; } + model UnknownPet extends PetBase { kind: string; } + ${decorator} + union Pet ${constraint} { ${variants} } + op get(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics( + context.diagnostics, + Array.from({ length: variants.includes(",") ? 2 : 1 }, () => ({ + code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind", + severity: "warning" as const, + })), + ); + strictEqual(context.sdkPackage.clients[0].methods[0].response.type?.kind, "union"); + strictEqual( + context.sdkPackage.models.find((x) => x.name === "PetBase")?.discriminatorProperty, + undefined, + ); + }); +}); + it("primitive union", async function () { const { program } = await SimpleTesterWithService.compile( ` diff --git a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md index 3a0194e1a1..e45047939a 100644 --- a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md +++ b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md @@ -62,6 +62,8 @@ Use the [TCGC Playground](https://azure.github.io/typespec-azure/playground/?e=% TCGC provides flags to control the client type graph style, such as enabling or disabling convenience and protocol APIs for Java and C# emitters. Other emitters should not set `generate-convenience-methods` or `generate-protocol-methods`; TCGC reports an `unnecessary-emitter-option` warning when either option is explicitly set for another language. See the [documentation](../reference/emitter/#emitter-options) for details. +By default, `createSdkContext` converts eligible inline-discriminated unions to the existing polymorphic model representation. Emitters that use native discriminated unions, such as Rust, can opt out with `{ convertDiscriminatedUnionsToModels: false }` in the third argument to `createSdkContext`. + ## TCGC Raw Types and Helpers In order to introduce the client concept, TCGC introduces some new raw types and helper functions. @@ -241,6 +243,36 @@ For types in TypeSpec, TCGC provides several client types to represent them in a - [`SdkUnionType`](../reference/js-api/interfaces/sdkuniontype/) represents a TCGC union type. It is typically converted from a TypeSpec [`Union`](https://typespec.io/docs/language-basics/unions/) type. - When a TypeSpec union declares an `extends` constraint, `SdkUnionType.baseType` preserves that explicit constraint. TCGC does not infer `baseType` merely because all variants share a common ancestor. Usage, access, and serialization information propagate from the union to its explicit base type, including through nullable wrappers and other union constraints. This propagation does not include unrelated discriminated subtypes of a base model. +#### Inline-discriminated union conversion + +An inline-discriminated union is converted when it uses `@discriminated(#{ envelope: "none" })`, explicitly extends a named model, and every alternative is a named variant whose model nominally inherits from that base, directly or transitively: + +```tsp +model PetBase { + name: string; +} +model Cat extends PetBase { + meows: boolean; +} +model Dog extends PetBase { + barks: boolean; +} + +@discriminated(#{ envelope: "none" }) +union Pet extends PetBase { + cat: Cat, + dog: Dog, +} + +@post op echo(@body pet: Pet): Pet; +``` + +References to `Pet` resolve to the existing `PetBase` SDK model, including operation parameters/responses, model properties, containers, and nullable uses. The base's `discriminatorProperty` and `discriminatedSubtypes`, plus each variant's `discriminatorValue` and discriminator property, describe the inline tags. Missing discriminator properties are synthesized; the compiler type graph is not changed. The projected model retains the base model's name and identity rather than introducing a model named after the union. + +Ordinary unions, object-envelope unions, structural-only variants, unions without a model constraint, and unions containing defaults or the base itself as an alternative are not converted. Alternate types and legacy hierarchy rebasing retain their existing handling. Conflicting discriminator metadata produces `union-model-conversion-conflict` and leaves the affected unions unconverted. + +Compatible unions sharing the same base share its SDK model and combined subtype map; the polymorphic representation does not enforce each union's subset separately. Synthesized inline discriminator properties also appear on directly used SDK variants. Disable conversion when an emitter needs union-specific serialization or exact alternative-set information. + **Model Types:** - [`SdkModelType`](../reference/js-api/interfaces/sdkmodeltype/) represents a TCGC model type. It is typically converted from a TypeSpec [`Model`](https://typespec.io/docs/language-basics/models/) type. From d588153bc63d054e7f9cea30e591c531e225c528 Mon Sep 17 00:00:00 2001 From: tadelesh Date: Fri, 16 Oct 2026 14:05:53 +0800 Subject: [PATCH 2/7] Address discriminated union conversion review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454 --- .../union-model-conversion-conflict.md | 73 ++++- .../src/interfaces.ts | 3 + .../src/types.ts | 296 +++++++++++++----- .../test/types/union.test.ts | 125 +++++++- .../guideline.md | 2 +- 5 files changed, 408 insertions(+), 91 deletions(-) diff --git a/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md b/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md index b2231378c3..ec76ce1e58 100644 --- a/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md +++ b/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md @@ -1,3 +1,72 @@ -An inline-discriminated union cannot be converted to a model hierarchy when its discriminator property or tag mappings conflict with another union or an existing model discriminator. TCGC preserves the union and reports this warning rather than overwriting the conflicting metadata. +This diagnostic is issued when an inline-discriminated union's discriminator property or tag mappings conflict with another union or an existing model discriminator in the same hierarchy. -Use consistent discriminator metadata for the shared hierarchy, or set `convertDiscriminatedUnionsToModels: false` in `createSdkContext` to retain discriminated unions. +## Impact + +- **Area:** SDK union type conversion. Generation continues with the union representation instead of overwriting conflicting discriminator metadata. +- **Not affected:** The TypeSpec union declaration, service routes, and wire payloads are unchanged. + +## ❌ Incorrect Usage + +```typespec +@service +namespace Example; + +model PetBase { + name: string; +} + +model Cat extends PetBase { + meows: boolean; +} + +@discriminated(#{ envelope: "none" }) +union Pet extends PetBase { + cat: Cat, + feline: Cat, // one model cannot have two polymorphic discriminator values +} + +op get(): Pet; +``` + +## Diagnostic Message + +TCGC reports: + +```text +Cannot convert discriminated union 'Pet' to a model hierarchy because its discriminator metadata conflicts with another union or model using the same hierarchy. The union is preserved. Set convertDiscriminatedUnionsToModels to false to retain union representations. +``` + +## ✅ How to Fix + +Use consistent discriminator properties and tag mappings throughout the shared hierarchy. Give each alternative with a different wire tag its own model. + +```typespec +@service +namespace Example; + +model PetBase { + name: string; +} + +model Cat extends PetBase { + meows: boolean; +} + +model Dog extends PetBase { + barks: boolean; +} + +@discriminated(#{ envelope: "none" }) +union Pet extends PetBase { + cat: Cat, + dog: Dog, +} + +op get(): Pet; +``` + +Emitters that intentionally use native discriminated unions can set `convertDiscriminatedUnionsToModels: false` in `createSdkContext` instead of requesting polymorphic conversion. + +## Suppression + +This diagnostic should not be suppressed. Align the hierarchy's discriminator metadata, or disable conversion when the emitter intentionally retains discriminated unions. diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index 831ba5a762..3476a922f7 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -69,6 +69,9 @@ export interface TCGCContext { convertDiscriminatedUnionsToModels?: boolean; __discriminatedUnionModels?: Map; + __discriminatedUnionConversionDepth?: number; + __deferDiscriminatedUnionConversion?: boolean; + __pendingDiscriminatedUnionConversion?: boolean; __referencedTypeCache: Map; __arrayDictionaryCache: Map; __methodParameterCache: Map; diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index c772a32881..c5c6139f7e 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -31,7 +31,6 @@ import { isErrorModel, isNeverType, isTemplateDeclaration, - navigateType, resolveEncodedName, } from "@typespec/compiler"; import { isEvents } from "@typespec/events"; @@ -557,31 +556,23 @@ export function getSdkUnionWithDiagnostics( context: TCGCContext, type: Union, operation?: Operation, +): [SdkType, readonly Diagnostic[]] { + const [sdkType, diagnostics] = withDiscriminatedUnionConversion(context, () => + getSdkUnionWithDiagnosticsInternal(context, type, operation), + ); + return [getProjectedUnion(context, sdkType), diagnostics]; +} + +function getSdkUnionWithDiagnosticsInternal( + context: TCGCContext, + type: Union, + operation?: Operation, ): [SdkType, readonly Diagnostic[]] { let retval: SdkType | undefined = context.__referencedTypeCache.get(type); const diagnostics = createDiagnosticCollector(); - initializeDiscriminatedUnionModels(context); - if ( - !retval && - context.__discriminatedUnionModels?.has(type) && - context.__discriminatedUnionModels.get(type) === undefined - ) { - diagnostics.add( - createDiagnostic({ - code: "union-model-conversion-conflict", - target: type, - format: { unionName: type.name ?? "(anonymous)" }, - }), - ); - } - const baseModel = context.__discriminatedUnionModels?.get(type); - if (baseModel) { - const model = diagnostics.pipe(getSdkModelWithDiagnostics(context, baseModel, operation)); - updateReferencedTypeMap(context, type, model); - return diagnostics.wrap(model); - } if (!retval) { + registerDiscriminatedUnionModel(context, type); const nonNullOptions = getNonNullOptions(type); const nullOption = getNullOption(type); @@ -618,10 +609,11 @@ export function getSdkUnionWithDiagnostics( // judge if the union can be converted to enum // if language does not need flatten union as enum // filter the case that union is composed of union or enum - context.flattenUnionAsEnum || - ![...type.variants.values()].some((variant) => { - return variant.type.kind === "Union" || variant.type.kind === "Enum"; - }) + !context.__discriminatedUnionModels?.has(type) && + (context.flattenUnionAsEnum || + ![...type.variants.values()].some((variant) => { + return variant.type.kind === "Union" || variant.type.kind === "Enum"; + })) ) { const unionAsEnum = diagnostics.pipe(getUnionAsEnum(type)); if (unionAsEnum) { @@ -713,61 +705,94 @@ export function getSdkUnionWithDiagnostics( return diagnostics.wrap(retval); } -function initializeDiscriminatedUnionModels(context: TCGCContext): void { - if (context.__discriminatedUnionModels || context.convertDiscriminatedUnionsToModels === false) { +function registerDiscriminatedUnionModel(context: TCGCContext, union: Union): void { + if (context.convertDiscriminatedUnionsToModels === false) { return; } - const models = (context.__discriminatedUnionModels = new Map()); - navigateType( - context.getMutatedGlobalNamespace(), - { - union(union) { - const base = union.baseType; - if ( - isTemplateDeclaration(union) || - isEvents(context.program, union) || - base?.kind !== "Model" || - !base.name || - isArrayModelType(base) || - base.indexer || - union.variants.size === 0 || - getAlternateType(context, union) || - walkBaseChain(base).some( - (model) => - getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), - ) - ) { - return; - } - const discriminated = $(context.program).union.getDiscriminatedUnion(union); - if ( - !discriminated || - discriminated.options.envelope !== "none" || - discriminated.defaultVariant - ) { - return; - } - for (const variant of union.variants.values()) { - if ( - typeof variant.name !== "string" || - variant.type.kind !== "Model" || - variant.type === base || - !walkBaseChain(variant.type).includes(base) || - walkBaseChain(variant.type).some( - (model) => - getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), - ) - ) { - return; - } - } - models.set(union, base); - }, - }, - {}, - ); - const conflicts = new Set(); + const base = union.baseType; + if ( + isTemplateDeclaration(union) || + isEvents(context.program, union) || + base?.kind !== "Model" || + !base.name || + isArrayModelType(base) || + base.indexer || + union.variants.size === 0 || + getAlternateType(context, union) || + walkBaseChain(base).some( + (model) => getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), + ) + ) { + return; + } + const discriminated = $(context.program).union.getDiscriminatedUnion(union); + if (!discriminated || discriminated.options.envelope !== "none" || discriminated.defaultVariant) { + return; + } + for (const variant of union.variants.values()) { + if ( + typeof variant.name !== "string" || + variant.type.kind !== "Model" || + variant.type === base || + !walkBaseChain(variant.type).includes(base) || + walkBaseChain(variant.type).some( + (model) => getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), + ) + ) { + return; + } + } + (context.__discriminatedUnionModels ??= new Map()).set(union, base); + context.__pendingDiscriminatedUnionConversion = true; +} + +function withDiscriminatedUnionConversion( + context: TCGCContext, + convert: () => [T, readonly Diagnostic[]], +): [T, readonly Diagnostic[]] { + if ( + context.__deferDiscriminatedUnionConversion || + context.convertDiscriminatedUnionsToModels === false + ) { + return convert(); + } + context.__discriminatedUnionConversionDepth = + (context.__discriminatedUnionConversionDepth ?? 0) + 1; + try { + const diagnostics = createDiagnosticCollector(); + const sdkType = diagnostics.pipe(convert()); + if ( + context.__discriminatedUnionConversionDepth === 1 && + !context.__deferDiscriminatedUnionConversion + ) { + diagnostics.pipe(finalizeDiscriminatedUnionModels(context, sdkType)); + } + return diagnostics.wrap(sdkType); + } finally { + context.__discriminatedUnionConversionDepth--; + } +} + +function getProjectedUnion(context: TCGCContext, type: SdkType): SdkType { + return type.kind === "union" && type.__raw + ? (context.__referencedTypeCache.get(type.__raw) ?? type) + : type; +} + +function finalizeDiscriminatedUnionModels( + context: TCGCContext, + root?: SdkType, +): [void, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + const models = context.__discriminatedUnionModels; + if (!context.__pendingDiscriminatedUnionConversion || !models) { + return diagnostics.wrap(undefined); + } + context.__pendingDiscriminatedUnionConversion = false; const candidates = [...models.entries()]; + const conflicts = new Set( + candidates.filter(([, base]) => base === undefined).map(([union]) => union), + ); for (const [union, base] of candidates) { if (!base) continue; const discriminated = $(context.program).union.getDiscriminatedUnion(union)!; @@ -836,8 +861,86 @@ function initializeDiscriminatedUnionModels(context: TCGCContext): void { } } for (const union of conflicts) { + if (models.get(union) === undefined) continue; models.set(union, undefined); + diagnostics.add( + createDiagnostic({ + code: "union-model-conversion-conflict", + target: union, + format: { unionName: union.name ?? "(anonymous)" }, + }), + ); + diagnostics.pipe(getUnionAsEnum(union)); + } + const replacements = new Map(); + for (const [union, base] of models) { + if (!base) continue; + const sdkUnion = context.__referencedTypeCache.get(union); + const sdkBase = context.__referencedTypeCache.get(base); + compilerAssert(sdkUnion && sdkBase?.kind === "model", "Union conversion types must be cached."); + if (sdkUnion.kind !== "union") continue; + replacements.set(sdkUnion, sdkBase); + } + if (replacements.size === 0) return diagnostics.wrap(undefined); + + for (const [rawType, sdkType] of context.__referencedTypeCache) { + if (rawType.kind === "Model" && sdkType.kind === "model") { + diagnostics.pipe(addUnionDiscriminatorToModel(context, rawType, sdkType)); + } + } + + // Resolve aliases only after all handled unions have participated in conflict detection. + const seen = new Set(); + function replace(type: SdkType): SdkType { + const replacement = replacements.get(type); + if (replacement) return replacement; + if (seen.has(type)) return type; + seen.add(type); + switch (type.kind) { + case "model": + for (const property of type.properties) property.type = replace(property.type); + if (type.additionalProperties) + type.additionalProperties = replace(type.additionalProperties); + break; + case "union": + type.variantTypes = type.variantTypes.map(replace); + if (type.baseType) type.baseType = replace(type.baseType); + break; + case "nullable": + type.type = replace(type.type); + break; + case "array": + case "dict": + type.valueType = replace(type.valueType); + break; + case "tuple": + type.valueTypes = type.valueTypes.map(replace); + break; + } + return type; } + for (const [rawType, sdkType] of context.__referencedTypeCache) { + updateReferencedTypeMap(context, rawType, replace(sdkType)); + } + for (const sdkType of context.__arrayDictionaryCache.values()) replace(sdkType); + for (const property of context.__modelPropertyCache.values()) + property.type = replace(property.type); + for (const parameter of context.__methodParameterCache.values()) + parameter.type = replace(parameter.type); + for (const header of context.__responseHeaderCache.values()) header.type = replace(header.type); + for (const parameters of context.__clientParametersCache.values()) { + for (const parameter of parameters) parameter.type = replace(parameter.type); + } + if (root) replace(root); + for (const model of new Set(replacements.values())) { + diagnostics.pipe(updateUsageOrAccess(context, model.usage, model)); + if (model.__accessSet) diagnostics.pipe(updateUsageOrAccess(context, model.access, model)); + const contentTypes: string[] = []; + if (model.serializationOptions.json) contentTypes.push("application/json"); + if (model.serializationOptions.xml) contentTypes.push("application/xml"); + diagnostics.pipe(updateSerializationOptions(context, model, contentTypes)); + } + return diagnostics.wrap(undefined); } function addUnionDiscriminatorToModel( @@ -1133,9 +1236,18 @@ export function getSdkModelWithDiagnostics( context: TCGCContext, type: Model, operation?: Operation, +): [SdkModelType, readonly Diagnostic[]] { + return withDiscriminatedUnionConversion(context, () => + getSdkModelWithDiagnosticsInternal(context, type, operation), + ); +} + +function getSdkModelWithDiagnosticsInternal( + context: TCGCContext, + type: Model, + operation?: Operation, ): [SdkModelType, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - initializeDiscriminatedUnionModels(context); let sdkType = context.__referencedTypeCache.get(type) as SdkModelType | undefined; if (!sdkType) { @@ -1155,11 +1267,6 @@ export function getSdkModelWithDiagnostics( serializationOptions: {}, }; updateReferencedTypeMap(context, type, sdkType); - for (const [union, base] of context.__discriminatedUnionModels ?? []) { - if (base === type) { - updateReferencedTypeMap(context, union, sdkType); - } - } // model MyModel is Record<> {} should be model with additional properties if (type.sourceModel?.kind === "Model" && type.sourceModel?.name === "Record") { @@ -1238,7 +1345,6 @@ export function getSdkModelWithDiagnostics( } } diagnostics.pipe(addDiscriminatorToModelType(context, type, sdkType)); - diagnostics.pipe(addUnionDiscriminatorToModel(context, type, sdkType)); updateReferencedTypeMap(context, type, sdkType); } return diagnostics.wrap(sdkType); @@ -1419,6 +1525,17 @@ export function getClientTypeWithDiagnostics( context: TCGCContext, type: Type, operation?: Operation, +): [SdkType, readonly Diagnostic[]] { + const [sdkType, diagnostics] = withDiscriminatedUnionConversion(context, () => + getClientTypeWithDiagnosticsInternal(context, type, operation), + ); + return [getProjectedUnion(context, sdkType), diagnostics]; +} + +function getClientTypeWithDiagnosticsInternal( + context: TCGCContext, + type: Type, + operation?: Operation, ): [SdkType, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); let retval: SdkType | undefined; @@ -2788,6 +2905,16 @@ export function getAllReferencedTypes( } export function handleAllTypes(context: TCGCContext): [void, readonly Diagnostic[]] { + const deferred = context.__deferDiscriminatedUnionConversion; + context.__deferDiscriminatedUnionConversion = true; + try { + return handleAllTypesInternal(context); + } finally { + context.__deferDiscriminatedUnionConversion = deferred; + } +} + +function handleAllTypesInternal(context: TCGCContext): [void, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const services = new Set(); for (const client of listClients(context)) { @@ -2836,6 +2963,7 @@ export function handleAllTypes(context: TCGCContext): [void, readonly Diagnostic } // update for orphan models/enums/unions diagnostics.pipe(handleServiceOrphanTypes(context)); + diagnostics.pipe(finalizeDiscriminatedUnionModels(context)); // update access diagnostics.pipe(updateAccessOverride(context)); // update usage diff --git a/packages/typespec-client-generator-core/test/types/union.test.ts b/packages/typespec-client-generator-core/test/types/union.test.ts index acbb2f874b..61db53813b 100644 --- a/packages/typespec-client-generator-core/test/types/union.test.ts +++ b/packages/typespec-client-generator-core/test/types/union.test.ts @@ -1,7 +1,9 @@ -import { expectDiagnostics } from "@typespec/compiler/testing"; +import { expectDiagnostics, t } from "@typespec/compiler/testing"; import { deepStrictEqual, ok, strictEqual } from "assert"; import { describe, it } from "vitest"; +import { createTCGCContext } from "../../src/context.js"; import { type SdkArrayType, type SdkMethodResponse, UsageFlags } from "../../src/interfaces.js"; +import { getSdkModel, getSdkUnion } from "../../src/types.js"; import { createSdkContextForTester, SimpleTester, SimpleTesterWithService } from "../tester.js"; import { getSdkTypeHelper } from "./utils.js"; @@ -126,6 +128,88 @@ describe("inline discriminated union conversion", () => { strictEqual(context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, "cat"); }); + it("does not reshape a used model because of an unhandled union outside the service", async () => { + const { program } = await SimpleTester.compile(` + namespace Definitions { + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union UnusedPet extends PetBase { cat: Cat } + } + @service namespace TestService { + op get(): Definitions.PetBase; + } + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + strictEqual(base.discriminatorProperty, undefined); + strictEqual(base.discriminatedSubtypes, undefined); + strictEqual( + context.sdkPackage.models.some((model) => model.name === "Cat"), + false, + ); + }); + + it("updates previously handled model properties and HTTP types to the projected base", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + model Holder { pet: Pet; } + @route("/base") op getBase(): PetBase; + @route("/holder") @post op echo(@body body: Holder): Holder; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const [getBase, echo] = context.sdkPackage.clients[0].methods; + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + const holder = context.sdkPackage.models.find((model) => model.name === "Holder"); + ok(base); + ok(holder); + strictEqual(getBase.response.type, base); + strictEqual(getBase.operation.responses[0].type, base); + strictEqual(echo.parameters[0].type, holder); + strictEqual(echo.operation.bodyParam?.type, holder); + strictEqual(holder.properties[0].type, base); + strictEqual(base.discriminatedSubtypes?.cat.discriminatorValue, "cat"); + }); + + it("preserves standalone SDK union conversion after the base is already cached", async () => { + const { program, PetBase, Pet } = await SimpleTesterWithService.compile(t.code` + model ${t.model("PetBase")} { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union ${t.union("Pet")} extends PetBase { cat: Cat } + `); + const context = createTCGCContext(program, "@azure-tools/typespec-test", { + mutateNamespace: false, + }); + const base = getSdkModel(context, PetBase); + const projected = getSdkUnion(context, Pet); + strictEqual(projected, base); + strictEqual(base.discriminatedSubtypes?.cat.discriminatorValue, "cat"); + strictEqual(base.properties.filter((property) => property.discriminator).length, 1); + }); + + it("discovers instantiated union templates during normal type handling", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: T } + op get(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + strictEqual(context.sdkPackage.clients[0].methods[0].response.type, base); + strictEqual(base.discriminatedSubtypes?.cat.discriminatorValue, "cat"); + }); + it("uses wire tags independently of client names", async () => { const { program } = await SimpleTesterWithService.compile(` model PetBase { @clientName("clientKind") type: string; } @@ -257,6 +341,38 @@ describe("inline discriminated union conversion", () => { strictEqual(context.sdkPackage.models.filter((x) => x.name === "PetBase").length, 1); }); + it("propagates combined usage, access, and serialization after shared-base conversion", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + model Dog extends PetBase { barks: boolean; } + @discriminated(#{ envelope: "none" }) + union IndoorPet extends PetBase { cat: Cat } + @discriminated(#{ envelope: "none" }) + union OutdoorPet extends PetBase { dog: Dog } + @access(Access.internal) + @route("/indoor") @post + op indoor(@header contentType: "application/xml", @body body: IndoorPet): void; + @route("/outdoor") op outdoor(): OutdoorPet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), ["cat", "dog"]); + for (const variant of Object.values(base.discriminatedSubtypes ?? {})) { + strictEqual( + variant.usage, + UsageFlags.Input | UsageFlags.Output | UsageFlags.Json | UsageFlags.Xml, + ); + strictEqual(variant.access, "public"); + ok(variant.serializationOptions.json); + ok(variant.serializationOptions.xml); + } + strictEqual(context.sdkPackage.clients[0].methods[0].operation.bodyParam?.type, base); + strictEqual(context.sdkPackage.clients[0].methods[1].response.type, base); + }); + it("retains all unions connected to a conflicting hierarchy", async () => { const { program } = await SimpleTesterWithService.compile(` model AnimalBase { name: string; } @@ -315,6 +431,7 @@ describe("inline discriminated union conversion", () => { union Pet extends PetBase { cat: Cat } @discriminated(#{ envelope: "none" }) union Animal extends PetBase { feline: Cat } + @route("/base") op base(): PetBase; @route("/pet") op pet(): Pet; @route("/animal") op animal(): Animal; `); @@ -325,9 +442,9 @@ describe("inline discriminated union conversion", () => { { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, { code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind" }, ]); - context.sdkPackage.clients[0].methods.forEach((method) => - strictEqual(method.response.type?.kind, "union"), - ); + context.sdkPackage.clients[0].methods + .slice(1) + .forEach((method) => strictEqual(method.response.type?.kind, "union")); strictEqual( context.sdkPackage.models.find((x) => x.name === "PetBase")?.discriminatorProperty, undefined, diff --git a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md index e45047939a..3224122968 100644 --- a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md +++ b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md @@ -245,7 +245,7 @@ For types in TypeSpec, TCGC provides several client types to represent them in a #### Inline-discriminated union conversion -An inline-discriminated union is converted when it uses `@discriminated(#{ envelope: "none" })`, explicitly extends a named model, and every alternative is a named variant whose model nominally inherits from that base, directly or transitively: +An inline-discriminated union handled by the SDK context is converted when it uses `@discriminated(#{ envelope: "none" })`, explicitly extends a named model, and every alternative is a named variant whose model nominally inherits from that base, directly or transitively. Unhandled unions do not reshape otherwise-used models. ```tsp model PetBase { From 0dc4b0c5f736e70f000f7ca28df3f96d53a05a6a Mon Sep 17 00:00:00 2001 From: tadelesh Date: Fri, 16 Oct 2026 15:39:52 +0800 Subject: [PATCH 3/7] Simplify discriminated union conversion to direct base aliases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454 --- .../union-model-conversion-conflict.md | 4 +- .../src/interfaces.ts | 5 +- .../typespec-client-generator-core/src/lib.ts | 4 +- .../src/types.ts | 469 +++++------------- .../test/types/union.test.ts | 110 ++-- .../guideline.md | 2 +- 6 files changed, 205 insertions(+), 389 deletions(-) diff --git a/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md b/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md index ec76ce1e58..2615d1ec78 100644 --- a/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md +++ b/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md @@ -2,7 +2,7 @@ This diagnostic is issued when an inline-discriminated union's discriminator pro ## Impact -- **Area:** SDK union type conversion. Generation continues with the union representation instead of overwriting conflicting discriminator metadata. +- **Area:** SDK union type conversion. The union resolves to its declared base SDK model, but incompatible discriminator metadata prevents generating a valid polymorphic hierarchy. TCGC reports an error instead of overwriting existing discriminator metadata. - **Not affected:** The TypeSpec union declaration, service routes, and wire payloads are unchanged. ## ❌ Incorrect Usage @@ -33,7 +33,7 @@ op get(): Pet; TCGC reports: ```text -Cannot convert discriminated union 'Pet' to a model hierarchy because its discriminator metadata conflicts with another union or model using the same hierarchy. The union is preserved. Set convertDiscriminatedUnionsToModels to false to retain union representations. +Discriminated union 'Pet' has conflicting discriminator metadata for its base model hierarchy. Use consistent discriminator properties and tag mappings, or set convertDiscriminatedUnionsToModels to false to retain union representations. ``` ## ✅ How to Fix diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index 3476a922f7..930c244bbb 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -68,10 +68,7 @@ export interface TCGCContext { enableLegacyHierarchyBuilding?: boolean; convertDiscriminatedUnionsToModels?: boolean; - __discriminatedUnionModels?: Map; - __discriminatedUnionConversionDepth?: number; - __deferDiscriminatedUnionConversion?: boolean; - __pendingDiscriminatedUnionConversion?: boolean; + __discriminatedUnionModels?: Map; __referencedTypeCache: Map; __arrayDictionaryCache: Map; __methodParameterCache: Map; diff --git a/packages/typespec-client-generator-core/src/lib.ts b/packages/typespec-client-generator-core/src/lib.ts index d316f696df..b118afa781 100644 --- a/packages/typespec-client-generator-core/src/lib.ts +++ b/packages/typespec-client-generator-core/src/lib.ts @@ -173,9 +173,9 @@ export const $lib = createTypeSpecLibrary({ diagnostics: { "union-model-conversion-conflict": { ...doc("union-model-conversion-conflict"), - severity: "warning", + severity: "error", messages: { - default: paramMessage`Cannot convert discriminated union '${"unionName"}' to a model hierarchy because its discriminator metadata conflicts with another union or model using the same hierarchy. The union is preserved. Set convertDiscriminatedUnionsToModels to false to retain union representations.`, + default: paramMessage`Discriminated union '${"unionName"}' has conflicting discriminator metadata for its base model hierarchy. Use consistent discriminator properties and tag mappings, or set convertDiscriminatedUnionsToModels to false to retain union representations.`, }, }, "union-null": { diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index c5c6139f7e..6bc83ff66e 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -2,6 +2,7 @@ import { type UnionEnum, getUnionAsEnum } from "@azure-tools/typespec-azure-core import { type BooleanLiteral, type Diagnostic, + type DiscriminatedUnion, type EncodeData, type Enum, type EnumMember, @@ -556,23 +557,23 @@ export function getSdkUnionWithDiagnostics( context: TCGCContext, type: Union, operation?: Operation, -): [SdkType, readonly Diagnostic[]] { - const [sdkType, diagnostics] = withDiscriminatedUnionConversion(context, () => - getSdkUnionWithDiagnosticsInternal(context, type, operation), - ); - return [getProjectedUnion(context, sdkType), diagnostics]; -} - -function getSdkUnionWithDiagnosticsInternal( - context: TCGCContext, - type: Union, - operation?: Operation, ): [SdkType, readonly Diagnostic[]] { let retval: SdkType | undefined = context.__referencedTypeCache.get(type); const diagnostics = createDiagnosticCollector(); if (!retval) { - registerDiscriminatedUnionModel(context, type); + const discriminated = $(context.program).union.getDiscriminatedUnion(type); + if ( + context.convertDiscriminatedUnionsToModels !== false && + discriminated?.options.envelope === "none" && + !discriminated.defaultVariant && + !isEvents(context.program, type) + ) { + const model = diagnostics.pipe( + getSdkDiscriminatedUnionModel(context, discriminated, operation), + ); + if (model) return diagnostics.wrap(model); + } const nonNullOptions = getNonNullOptions(type); const nullOption = getNullOption(type); @@ -609,11 +610,10 @@ function getSdkUnionWithDiagnosticsInternal( // judge if the union can be converted to enum // if language does not need flatten union as enum // filter the case that union is composed of union or enum - !context.__discriminatedUnionModels?.has(type) && - (context.flattenUnionAsEnum || - ![...type.variants.values()].some((variant) => { - return variant.type.kind === "Union" || variant.type.kind === "Enum"; - })) + context.flattenUnionAsEnum || + ![...type.variants.values()].some((variant) => { + return variant.type.kind === "Union" || variant.type.kind === "Enum"; + }) ) { const unionAsEnum = diagnostics.pipe(getUnionAsEnum(type)); if (unionAsEnum) { @@ -651,7 +651,7 @@ function getSdkUnionWithDiagnosticsInternal( access: "public", usage: UsageFlags.None, }; - const discriminatedOptions = $(context.program).union.getDiscriminatedUnion(type)?.options; + const discriminatedOptions = discriminated?.options; if (discriminatedOptions) { const envelope = discriminatedOptions.envelope ?? "object"; retval.discriminatedOptions = { @@ -705,164 +705,66 @@ function getSdkUnionWithDiagnosticsInternal( return diagnostics.wrap(retval); } -function registerDiscriminatedUnionModel(context: TCGCContext, union: Union): void { - if (context.convertDiscriminatedUnionsToModels === false) { - return; - } +function getSdkDiscriminatedUnionModel( + context: TCGCContext, + discriminated: DiscriminatedUnion, + operation?: Operation, +): [SdkModelType | undefined, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + const union = discriminated.type; const base = union.baseType; if ( - isTemplateDeclaration(union) || - isEvents(context.program, union) || base?.kind !== "Model" || - !base.name || - isArrayModelType(base) || base.indexer || - union.variants.size === 0 || - getAlternateType(context, union) || - walkBaseChain(base).some( - (model) => getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), + discriminated.variants.size === 0 || + [...discriminated.variants.values()].some( + (variant) => + variant.kind !== "Model" || + variant === base || + !walkBaseChain(variant).includes(base) || + walkBaseChain(variant).some( + (model) => getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), + ), ) ) { - return; - } - const discriminated = $(context.program).union.getDiscriminatedUnion(union); - if (!discriminated || discriminated.options.envelope !== "none" || discriminated.defaultVariant) { - return; - } - for (const variant of union.variants.values()) { - if ( - typeof variant.name !== "string" || - variant.type.kind !== "Model" || - variant.type === base || - !walkBaseChain(variant.type).includes(base) || - walkBaseChain(variant.type).some( - (model) => getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), - ) - ) { - return; - } + return diagnostics.wrap(undefined); } - (context.__discriminatedUnionModels ??= new Map()).set(union, base); - context.__pendingDiscriminatedUnionConversion = true; -} -function withDiscriminatedUnionConversion( - context: TCGCContext, - convert: () => [T, readonly Diagnostic[]], -): [T, readonly Diagnostic[]] { + (context.__discriminatedUnionModels ??= new Map()).set(union, base); + const model = diagnostics.pipe(getSdkModelWithDiagnostics(context, base, operation)); + updateReferencedTypeMap(context, union, model); + const variants = [...discriminated.variants].map(([tag, variant]) => { + compilerAssert(variant.kind === "Model", "Inline discriminated union variants must be models."); + return { + tag, + raw: variant, + model: diagnostics.pipe(getSdkModelWithDiagnostics(context, variant, operation)), + }; + }); + const propertyName = discriminated.options.discriminatorPropertyName; if ( - context.__deferDiscriminatedUnionConversion || - context.convertDiscriminatedUnionsToModels === false - ) { - return convert(); - } - context.__discriminatedUnionConversionDepth = - (context.__discriminatedUnionConversionDepth ?? 0) + 1; - try { - const diagnostics = createDiagnosticCollector(); - const sdkType = diagnostics.pipe(convert()); - if ( - context.__discriminatedUnionConversionDepth === 1 && - !context.__deferDiscriminatedUnionConversion - ) { - diagnostics.pipe(finalizeDiscriminatedUnionModels(context, sdkType)); - } - return diagnostics.wrap(sdkType); - } finally { - context.__discriminatedUnionConversionDepth--; - } -} - -function getProjectedUnion(context: TCGCContext, type: SdkType): SdkType { - return type.kind === "union" && type.__raw - ? (context.__referencedTypeCache.get(type.__raw) ?? type) - : type; -} - -function finalizeDiscriminatedUnionModels( - context: TCGCContext, - root?: SdkType, -): [void, readonly Diagnostic[]] { - const diagnostics = createDiagnosticCollector(); - const models = context.__discriminatedUnionModels; - if (!context.__pendingDiscriminatedUnionConversion || !models) { - return diagnostics.wrap(undefined); - } - context.__pendingDiscriminatedUnionConversion = false; - const candidates = [...models.entries()]; - const conflicts = new Set( - candidates.filter(([, base]) => base === undefined).map(([union]) => union), - ); - for (const [union, base] of candidates) { - if (!base) continue; - const discriminated = $(context.program).union.getDiscriminatedUnion(union)!; - const propertyName = discriminated.options.discriminatorPropertyName; - const existingDiscriminator = getDiscriminator(context.program, base); - if (existingDiscriminator && existingDiscriminator.propertyName !== propertyName) { - conflicts.add(union); - } - for (const model of [base, ...discriminated.variants.values()]) { - if (model.kind !== "Model") continue; - const property = walkBaseChain(model) + new Set(variants.map((variant) => variant.model)).size !== variants.length || + (model.discriminatorProperty && + (model.discriminatorProperty.serializationOptions.json?.name ?? + model.discriminatorProperty.__raw?.name ?? + model.discriminatorProperty.name) !== propertyName) || + [base, ...variants.map((variant) => variant.raw)].some((raw) => { + const property = walkBaseChain(raw) .map((ancestor) => ancestor.properties.get(propertyName)) .find((property) => property !== undefined); - if ( + return ( property && resolveEncodedName(context.program, property, "application/json") !== propertyName - ) { - conflicts.add(union); - } - } - for (const [otherUnion, otherBase] of candidates) { - const other = $(context.program).union.getDiscriminatedUnion(otherUnion)!; - if (base === otherBase && propertyName !== other.options.discriminatorPropertyName) { - conflicts.add(union); - conflicts.add(otherUnion); - } - for (const [tag, variant] of discriminated.variants) { - for (const [otherTag, otherVariant] of other.variants) { - if ( - (variant === otherVariant && - (tag !== otherTag || propertyName !== other.options.discriminatorPropertyName)) || - (base === otherBase && tag === otherTag && variant !== otherVariant) - ) { - conflicts.add(union); - conflicts.add(otherUnion); - } - } - } - } - } - // A retained union must not reference models reshaped by another conflicting conversion. - let changed = true; - while (changed) { - changed = false; - const affectedModels = new Set(); - for (const [union, base] of candidates) { - if (!conflicts.has(union)) continue; - for (const model of [base, ...[...union.variants.values()].map((variant) => variant.type)]) { - if (model?.kind !== "Model") continue; - for (const ancestor of walkBaseChain(model)) affectedModels.add(ancestor); - } - } - for (const [union, base] of candidates) { - if ( - !conflicts.has(union) && - ((base && walkBaseChain(base).some((model) => affectedModels.has(model))) || - [...union.variants.values()].some( - (variant) => - variant.type.kind === "Model" && - walkBaseChain(variant.type).some((model) => affectedModels.has(model)), - )) - ) { - conflicts.add(union); - changed = true; - } - } - } - for (const union of conflicts) { - if (models.get(union) === undefined) continue; - models.set(union, undefined); + ); + }) || + variants.some( + (variant) => + (variant.model.discriminatorValue !== undefined && + variant.model.discriminatorValue !== variant.tag) || + (model.discriminatedSubtypes?.[variant.tag] !== undefined && + model.discriminatedSubtypes[variant.tag] !== variant.model), + ) + ) { diagnostics.add( createDiagnostic({ code: "union-model-conversion-conflict", @@ -870,168 +772,83 @@ function finalizeDiscriminatedUnionModels( format: { unionName: union.name ?? "(anonymous)" }, }), ); - diagnostics.pipe(getUnionAsEnum(union)); - } - const replacements = new Map(); - for (const [union, base] of models) { - if (!base) continue; - const sdkUnion = context.__referencedTypeCache.get(union); - const sdkBase = context.__referencedTypeCache.get(base); - compilerAssert(sdkUnion && sdkBase?.kind === "model", "Union conversion types must be cached."); - if (sdkUnion.kind !== "union") continue; - replacements.set(sdkUnion, sdkBase); + return diagnostics.wrap(model); } - if (replacements.size === 0) return diagnostics.wrap(undefined); - - for (const [rawType, sdkType] of context.__referencedTypeCache) { - if (rawType.kind === "Model" && sdkType.kind === "model") { - diagnostics.pipe(addUnionDiscriminatorToModel(context, rawType, sdkType)); - } - } - - // Resolve aliases only after all handled unions have participated in conflict detection. - const seen = new Set(); - function replace(type: SdkType): SdkType { - const replacement = replacements.get(type); - if (replacement) return replacement; - if (seen.has(type)) return type; - seen.add(type); - switch (type.kind) { - case "model": - for (const property of type.properties) property.type = replace(property.type); - if (type.additionalProperties) - type.additionalProperties = replace(type.additionalProperties); - break; - case "union": - type.variantTypes = type.variantTypes.map(replace); - if (type.baseType) type.baseType = replace(type.baseType); - break; - case "nullable": - type.type = replace(type.type); - break; - case "array": - case "dict": - type.valueType = replace(type.valueType); - break; - case "tuple": - type.valueTypes = type.valueTypes.map(replace); - break; - } - return type; - } - for (const [rawType, sdkType] of context.__referencedTypeCache) { - updateReferencedTypeMap(context, rawType, replace(sdkType)); - } - for (const sdkType of context.__arrayDictionaryCache.values()) replace(sdkType); - for (const property of context.__modelPropertyCache.values()) - property.type = replace(property.type); - for (const parameter of context.__methodParameterCache.values()) - parameter.type = replace(parameter.type); - for (const header of context.__responseHeaderCache.values()) header.type = replace(header.type); - for (const parameters of context.__clientParametersCache.values()) { - for (const parameter of parameters) parameter.type = replace(parameter.type); - } - if (root) replace(root); - for (const model of new Set(replacements.values())) { - diagnostics.pipe(updateUsageOrAccess(context, model.usage, model)); - if (model.__accessSet) diagnostics.pipe(updateUsageOrAccess(context, model.access, model)); - const contentTypes: string[] = []; - if (model.serializationOptions.json) contentTypes.push("application/json"); - if (model.serializationOptions.xml) contentTypes.push("application/xml"); - diagnostics.pipe(updateSerializationOptions(context, model, contentTypes)); + model.discriminatorProperty = diagnostics.pipe( + getUnionDiscriminatorProperty(context, base, model, propertyName), + ); + model.discriminatedSubtypes ??= {}; + for (const variant of variants) { + diagnostics.pipe( + getUnionDiscriminatorProperty( + context, + variant.raw, + variant.model, + propertyName, + variant.tag, + model.discriminatorProperty.type, + ), + ); + variant.model.discriminatorValue = variant.tag; + model.discriminatedSubtypes[variant.tag] = variant.model; } - return diagnostics.wrap(undefined); + diagnostics.pipe(updateUsageOrAccess(context, model.usage, model)); + if (model.__accessSet) diagnostics.pipe(updateUsageOrAccess(context, model.access, model)); + const contentTypes: string[] = []; + if (model.serializationOptions.json) contentTypes.push("application/json"); + if (model.serializationOptions.xml) contentTypes.push("application/xml"); + diagnostics.pipe(updateSerializationOptions(context, model, contentTypes)); + return diagnostics.wrap(model); } -function addUnionDiscriminatorToModel( +function getUnionDiscriminatorProperty( context: TCGCContext, rawModel: Model, model: SdkModelType, -): [void, readonly Diagnostic[]] { + name: string, + tag?: string, + discriminatorType?: SdkType, +): [SdkModelPropertyType, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - for (const [union, base] of context.__discriminatedUnionModels ?? []) { - if (!base) continue; - const discriminated = $(context.program).union.getDiscriminatedUnion(union)!; - const propertyName = discriminated.options.discriminatorPropertyName; - if (rawModel === base) { - let property = findDiscriminatorProperty(model, propertyName); - if (!property) { - const inheritedProperty = findInheritedDiscriminatorProperty(model, propertyName); - property = createDiscriminatorProperty( - model, - inheritedProperty?.name ?? propertyName, - inheritedProperty?.type ?? getTypeSpecBuiltInType(context, "string"), - propertyName, - ); - property.serializationOptions.json = { name: propertyName }; - model.properties.unshift(property); - } - property.optional = false; - property.discriminator = true; - model.discriminatorProperty = property; - model.discriminatedSubtypes ??= {}; - for (const [tag, variant] of discriminated.variants) { - compilerAssert(variant.kind === "Model", "Converted union variants must be models.", union); - model.discriminatedSubtypes[tag] = diagnostics.pipe( - getSdkModelWithDiagnostics(context, variant), - ); - } - } else { - for (const [tag, variant] of discriminated.variants) { - if (variant !== rawModel) continue; - let property = findDiscriminatorProperty(model, propertyName); - const inheritedProperty = findInheritedDiscriminatorProperty(model, propertyName); - if (!property) { - const constant: SdkConstantType = { + let property = model.properties.find( + (property) => + (property.__raw?.name ?? property.serializationOptions.json?.name ?? property.name) === name, + ); + const rawProperty = walkBaseChain(rawModel) + .map((ancestor) => ancestor.properties.get(name)) + .find((property) => property !== undefined); + const source = rawProperty + ? diagnostics.pipe(getSdkModelPropertyType(context, rawProperty)) + : undefined; + if (!property && rawModel.properties.has(name)) property = source; + if (!property) { + const type: SdkType = + tag === undefined + ? (source?.type ?? getTypeSpecBuiltInType(context, "string")) + : { kind: "constant", value: tag, valueType: getTypeSpecBuiltInType(context, "string"), - name: `${model.name}${pascalCase(propertyName)}`, + name: `${model.name}${pascalCase(name)}`, isGeneratedName: true, isExactName: false, decorators: [], }; - property = createDiscriminatorProperty( - model, - inheritedProperty?.name ?? propertyName, - constant, - propertyName, - ); - property.serializationOptions.json = { name: propertyName }; - model.properties.unshift(property); - } - if (property.type.kind === "constant" && inheritedProperty?.type.kind === "enum") { - const value = inheritedProperty.type.values.find((value) => value.value === tag); - if (value) property.type = value; - } - property.optional = false; - property.discriminator = true; - model.discriminatorValue = tag; - } - } + property = createDiscriminatorProperty(model, source?.name ?? name, type, name); + model.properties.unshift(property); } - return diagnostics.wrap(undefined); -} - -function findDiscriminatorProperty( - model: SdkModelType, - name: string, -): SdkModelPropertyType | undefined { - return model.properties.find( - (property) => - (property.__raw?.name ?? property.serializationOptions.json?.name ?? property.name) === name, - ); -} - -function findInheritedDiscriminatorProperty( - model: SdkModelType, - name: string, -): SdkModelPropertyType | undefined { - for (let parent = model.baseModel; parent; parent = parent.baseModel) { - const property = findDiscriminatorProperty(parent, name); - if (property) return property; + if ( + tag !== undefined && + property.type.kind === "constant" && + discriminatorType?.kind === "enum" + ) { + const value = discriminatorType.values.find((value) => value.value === tag); + if (value) property.type = value; } - return undefined; + property.optional = false; + property.discriminator = true; + property.serializationOptions.json ??= { name }; + return diagnostics.wrap(property); } function createDiscriminatorProperty( @@ -1236,16 +1053,6 @@ export function getSdkModelWithDiagnostics( context: TCGCContext, type: Model, operation?: Operation, -): [SdkModelType, readonly Diagnostic[]] { - return withDiscriminatedUnionConversion(context, () => - getSdkModelWithDiagnosticsInternal(context, type, operation), - ); -} - -function getSdkModelWithDiagnosticsInternal( - context: TCGCContext, - type: Model, - operation?: Operation, ): [SdkModelType, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); let sdkType = context.__referencedTypeCache.get(type) as SdkModelType | undefined; @@ -1267,6 +1074,10 @@ function getSdkModelWithDiagnosticsInternal( serializationOptions: {}, }; updateReferencedTypeMap(context, type, sdkType); + // Register aliases before traversing properties that can refer back to the union. + for (const [union, base] of context.__discriminatedUnionModels ?? []) { + if (base === type) updateReferencedTypeMap(context, union, sdkType); + } // model MyModel is Record<> {} should be model with additional properties if (type.sourceModel?.kind === "Model" && type.sourceModel?.name === "Record") { @@ -1525,17 +1336,6 @@ export function getClientTypeWithDiagnostics( context: TCGCContext, type: Type, operation?: Operation, -): [SdkType, readonly Diagnostic[]] { - const [sdkType, diagnostics] = withDiscriminatedUnionConversion(context, () => - getClientTypeWithDiagnosticsInternal(context, type, operation), - ); - return [getProjectedUnion(context, sdkType), diagnostics]; -} - -function getClientTypeWithDiagnosticsInternal( - context: TCGCContext, - type: Type, - operation?: Operation, ): [SdkType, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); let retval: SdkType | undefined; @@ -2905,16 +2705,6 @@ export function getAllReferencedTypes( } export function handleAllTypes(context: TCGCContext): [void, readonly Diagnostic[]] { - const deferred = context.__deferDiscriminatedUnionConversion; - context.__deferDiscriminatedUnionConversion = true; - try { - return handleAllTypesInternal(context); - } finally { - context.__deferDiscriminatedUnionConversion = deferred; - } -} - -function handleAllTypesInternal(context: TCGCContext): [void, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const services = new Set(); for (const client of listClients(context)) { @@ -2963,7 +2753,6 @@ function handleAllTypesInternal(context: TCGCContext): [void, readonly Diagnosti } // update for orphan models/enums/unions diagnostics.pipe(handleServiceOrphanTypes(context)); - diagnostics.pipe(finalizeDiscriminatedUnionModels(context)); // update access diagnostics.pipe(updateAccessOverride(context)); // update usage diff --git a/packages/typespec-client-generator-core/test/types/union.test.ts b/packages/typespec-client-generator-core/test/types/union.test.ts index 61db53813b..ea807f706f 100644 --- a/packages/typespec-client-generator-core/test/types/union.test.ts +++ b/packages/typespec-client-generator-core/test/types/union.test.ts @@ -128,6 +128,33 @@ describe("inline discriminated union conversion", () => { strictEqual(context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, "cat"); }); + it("reuses discriminator properties declared after recursive union properties", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { + next?: Pet; + @clientName("clientKind") kind: string; + } + model Cat extends PetBase { kind: "cat"; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + @route("/base") op getBase(): PetBase; + @route("/union") op getUnion(): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + strictEqual(base.properties.filter((property) => property.discriminator).length, 1); + strictEqual( + base.discriminatorProperty, + base.properties.find((property) => property.name === "clientKind"), + ); + strictEqual(base.properties.find((property) => property.name === "next")?.type, base); + context.sdkPackage.clients[0].methods.forEach((method) => + strictEqual(method.response.type, base), + ); + }); + it("does not reshape a used model because of an unhandled union outside the service", async () => { const { program } = await SimpleTester.compile(` namespace Definitions { @@ -272,9 +299,13 @@ describe("inline discriminated union conversion", () => { model NamedEntity { @clientName("clientKind") kind: PetKind; } model PetBase extends NamedEntity { name: string; } model Cat extends PetBase { meows: boolean; } + model Dog extends PetBase { barks: boolean; } @discriminated(#{ envelope: "none" }) union Pet extends PetBase { cat: Cat } - op get(): Pet; + @discriminated(#{ envelope: "none" }) + union OtherPet extends PetBase { dog: Dog } + @route("/cat") op get(): Pet; + @route("/dog") op getOther(): OtherPet; `); const context = await createSdkContextForTester(program); expectDiagnostics(context.diagnostics, []); @@ -290,6 +321,10 @@ describe("inline discriminated union conversion", () => { ok(property); strictEqual(property.name, "clientKind"); strictEqual(property.type.kind, "enumvalue"); + strictEqual(base.discriminatedSubtypes?.dog.discriminatorValue, "dog"); + context.sdkPackage.clients[0].methods.forEach((method) => + strictEqual(method.response.type, base), + ); }); it.each([false, true])( @@ -373,7 +408,7 @@ describe("inline discriminated union conversion", () => { strictEqual(context.sdkPackage.clients[0].methods[1].response.type, base); }); - it("retains all unions connected to a conflicting hierarchy", async () => { + it("aliases connected unions to their respective bases without rolling back valid hierarchies", async () => { const { program } = await SimpleTesterWithService.compile(` model AnimalBase { name: string; } model PetBase extends AnimalBase { } @@ -390,20 +425,20 @@ describe("inline discriminated union conversion", () => { @route("/animals") op animals(): Animals; `); const context = await createSdkContextForTester(program); - expectDiagnostics( - context.diagnostics, - Array.from({ length: 3 }, () => [ - { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, - { code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind" }, - ]).flat(), - ); - context.sdkPackage.clients[0].methods.forEach((method) => - strictEqual(method.response.type?.kind, "union"), - ); - strictEqual( - context.sdkPackage.models.find((model) => model.name === "Dog")?.discriminatorValue, - undefined, - ); + expectDiagnostics(context.diagnostics, { + code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict", + severity: "error", + }); + const petBase = context.sdkPackage.models.find((model) => model.name === "PetBase"); + const animalBase = context.sdkPackage.models.find((model) => model.name === "AnimalBase"); + ok(petBase); + ok(animalBase); + const methods = context.sdkPackage.clients[0].methods; + strictEqual(methods[0].response.type, petBase); + strictEqual(methods[1].response.type, petBase); + strictEqual(methods[2].response.type, animalBase); + strictEqual(petBase.discriminatedSubtypes?.cat.discriminatorValue, "cat"); + strictEqual(animalBase.discriminatedSubtypes?.dog.discriminatorValue, "dog"); }); it("propagates internal operation access to the converted hierarchy", async () => { @@ -423,7 +458,7 @@ describe("inline discriminated union conversion", () => { strictEqual(base.discriminatedSubtypes?.cat.access, "internal"); }); - it("retains conflicting unions instead of overwriting subtype tags", async () => { + it("keeps conflicting unions aliased to the base without overwriting subtype tags", async () => { const { program } = await SimpleTesterWithService.compile(` model PetBase { name: string; } model Cat extends PetBase { meows: boolean; } @@ -436,23 +471,17 @@ describe("inline discriminated union conversion", () => { @route("/animal") op animal(): Animal; `); const context = await createSdkContextForTester(program); - expectDiagnostics(context.diagnostics, [ - { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, - { code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind" }, - { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, - { code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind" }, - ]); - context.sdkPackage.clients[0].methods - .slice(1) - .forEach((method) => strictEqual(method.response.type?.kind, "union")); - strictEqual( - context.sdkPackage.models.find((x) => x.name === "PetBase")?.discriminatorProperty, - undefined, - ); - strictEqual( - context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, - undefined, + expectDiagnostics(context.diagnostics, { + code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict", + severity: "error", + }); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + context.sdkPackage.clients[0].methods.forEach((method) => + strictEqual(method.response.type, base), ); + deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), ["cat"]); + strictEqual(context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, "cat"); }); it.each([ @@ -471,13 +500,14 @@ describe("inline discriminated union conversion", () => { op get(): Pet; `); const context = await createSdkContextForTester(program); - expectDiagnostics(context.diagnostics, [ - { code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict" }, - ...Array.from({ length: variants.includes(",") ? 2 : 1 }, () => ({ - code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind", - })), - ]); - strictEqual(context.sdkPackage.clients[0].methods[0].response.type?.kind, "union"); + expectDiagnostics(context.diagnostics, { + code: "@azure-tools/typespec-client-generator-core/union-model-conversion-conflict", + severity: "error", + }); + strictEqual( + context.sdkPackage.clients[0].methods[0].response.type, + context.sdkPackage.models.find((model) => model.name === "PetBase"), + ); strictEqual( context.sdkPackage.models.find((x) => x.name === "Cat")?.discriminatorValue, undefined, diff --git a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md index 3224122968..736425458b 100644 --- a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md +++ b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md @@ -269,7 +269,7 @@ union Pet extends PetBase { References to `Pet` resolve to the existing `PetBase` SDK model, including operation parameters/responses, model properties, containers, and nullable uses. The base's `discriminatorProperty` and `discriminatedSubtypes`, plus each variant's `discriminatorValue` and discriminator property, describe the inline tags. Missing discriminator properties are synthesized; the compiler type graph is not changed. The projected model retains the base model's name and identity rather than introducing a model named after the union. -Ordinary unions, object-envelope unions, structural-only variants, unions without a model constraint, and unions containing defaults or the base itself as an alternative are not converted. Alternate types and legacy hierarchy rebasing retain their existing handling. Conflicting discriminator metadata produces `union-model-conversion-conflict` and leaves the affected unions unconverted. +Ordinary unions, object-envelope unions, structural-only variants, unions without a model constraint, and unions containing defaults or the base itself as an alternative are not converted. Alternate types and legacy hierarchy rebasing retain their existing handling. Eligible unions always resolve to their base SDK model. Conflicting discriminator metadata produces a `union-model-conversion-conflict` error rather than overwriting an existing hierarchy or falling back to a union representation. Compatible unions sharing the same base share its SDK model and combined subtype map; the polymorphic representation does not enforce each union's subset separately. Synthesized inline discriminator properties also appear on directly used SDK variants. Disable conversion when an emitter needs union-specific serialization or exact alternative-set information. From fe5dc2345fe991bd9c4aefa6ea150851f74a74aa Mon Sep 17 00:00:00 2001 From: tadelesh Date: Fri, 16 Oct 2026 16:48:36 +0800 Subject: [PATCH 4/7] Fix discriminated union metadata and emitter review regressions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe48cf1c-c567-4896-b51d-5ae7233ae454 --- .../changes/ts-transitive-union-dispatch.md | 7 + .../src/decorators.ts | 14 +- .../src/types.ts | 47 ++++-- .../test/tester.ts | 1 + .../test/types/union.test.ts | 147 ++++++++++++++++-- .../src/modular/helpers/type-helpers.ts | 12 +- .../modular/serialization/serialize-utils.ts | 6 +- .../models/inlineDiscriminatedUnion.md | 65 ++++++++ packages/typespec-ts/test/util/test-util.ts | 1 + .../guideline.md | 2 +- 10 files changed, 269 insertions(+), 33 deletions(-) create mode 100644 .chronus/changes/ts-transitive-union-dispatch.md create mode 100644 packages/typespec-ts/test/modular-unit/scenarios/models/inlineDiscriminatedUnion.md diff --git a/.chronus/changes/ts-transitive-union-dispatch.md b/.chronus/changes/ts-transitive-union-dispatch.md new file mode 100644 index 0000000000..ead9cc4eb0 --- /dev/null +++ b/.chronus/changes/ts-transitive-union-dispatch.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@azure-tools/typespec-ts" +--- + +Include discriminated union variants behind untagged intermediate base models in polymorphic unions and serialization dispatch. diff --git a/packages/typespec-client-generator-core/src/decorators.ts b/packages/typespec-client-generator-core/src/decorators.ts index ee471f1e96..3b95ec7560 100644 --- a/packages/typespec-client-generator-core/src/decorators.ts +++ b/packages/typespec-client-generator-core/src/decorators.ts @@ -712,7 +712,12 @@ export function getUsage(context: TCGCContext, entity: Model | Enum | Union): Us switch (entity.kind) { case "Union": const type = getSdkUnion(context, entity); - if (type.kind === "enum" || type.kind === "union" || type.kind === "nullable") { + if ( + type.kind === "model" || + type.kind === "enum" || + type.kind === "union" || + type.kind === "nullable" + ) { return type.usage; } return UsageFlags.None; @@ -771,7 +776,12 @@ export function getAccess( return getSdkEnum(context, entity).access; case "Union": { const type = getSdkUnion(context, entity); - if (type.kind === "enum" || type.kind === "union" || type.kind === "nullable") { + if ( + type.kind === "model" || + type.kind === "enum" || + type.kind === "union" || + type.kind === "nullable" + ) { return type.access; } return "public"; diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 6bc83ff66e..b1b6606008 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -720,7 +720,6 @@ function getSdkDiscriminatedUnionModel( [...discriminated.variants.values()].some( (variant) => variant.kind !== "Model" || - variant === base || !walkBaseChain(variant).includes(base) || walkBaseChain(variant).some( (model) => getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), @@ -733,17 +732,25 @@ function getSdkDiscriminatedUnionModel( (context.__discriminatedUnionModels ??= new Map()).set(union, base); const model = diagnostics.pipe(getSdkModelWithDiagnostics(context, base, operation)); updateReferencedTypeMap(context, union, model); - const variants = [...discriminated.variants].map(([tag, variant]) => { - compilerAssert(variant.kind === "Model", "Inline discriminated union variants must be models."); - return { - tag, - raw: variant, - model: diagnostics.pipe(getSdkModelWithDiagnostics(context, variant, operation)), - }; - }); + // The base is represented by the hierarchy's fallback, not by a subtype of itself. + const variants = [...discriminated.variants] + .filter(([, variant]) => variant !== base) + .map(([tag, variant]) => { + compilerAssert( + variant.kind === "Model", + "Inline discriminated union variants must be models.", + ); + return { + tag, + raw: variant, + model: diagnostics.pipe(getSdkModelWithDiagnostics(context, variant, operation)), + }; + }); const propertyName = discriminated.options.discriminatorPropertyName; + // A payload model can have only one tag. Existing hierarchies must also agree on + // the discriminator's wire name and on tags introduced by other unions. if ( - new Set(variants.map((variant) => variant.model)).size !== variants.length || + new Set(variants.map((variant) => variant.raw)).size !== variants.length || (model.discriminatorProperty && (model.discriminatorProperty.serializationOptions.json?.name ?? model.discriminatorProperty.__raw?.name ?? @@ -761,7 +768,8 @@ function getSdkDiscriminatedUnionModel( (variant) => (variant.model.discriminatorValue !== undefined && variant.model.discriminatorValue !== variant.tag) || - (model.discriminatedSubtypes?.[variant.tag] !== undefined && + (model.discriminatedSubtypes && + Object.hasOwn(model.discriminatedSubtypes, variant.tag) && model.discriminatedSubtypes[variant.tag] !== variant.model), ) ) { @@ -777,7 +785,10 @@ function getSdkDiscriminatedUnionModel( model.discriminatorProperty = diagnostics.pipe( getUnionDiscriminatorProperty(context, base, model, propertyName), ); - model.discriminatedSubtypes ??= {}; + model.discriminatedSubtypes = Object.assign( + Object.create(null) as Record, + model.discriminatedSubtypes, + ); for (const variant of variants) { diagnostics.pipe( getUnionDiscriminatorProperty( @@ -835,6 +846,7 @@ function getUnionDiscriminatorProperty( decorators: [], }; property = createDiscriminatorProperty(model, source?.name ?? name, type, name); + property.__raw = rawProperty; model.properties.unshift(property); } if ( @@ -2272,6 +2284,17 @@ function updateTypesFromOperation( function updateAccessOverride(context: TCGCContext): [void, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); + const overrides = new Map(); + for (const [rawType, sdkType] of context.__referencedTypeCache) { + if (rawType.kind !== "Model" && rawType.kind !== "Enum" && rawType.kind !== "Union") continue; + const access = getAccessOverride(context, rawType); + if (access && (access === "public" || !overrides.has(sdkType))) overrides.set(sdkType, access); + } + // Seed orphan hierarchies from all aliases before a raw base or subtype gets a + // public default. Already-inferred access still participates in conflict checks below. + for (const [sdkType, access] of overrides) { + if (!sdkType.__accessSet) diagnostics.pipe(updateUsageOrAccess(context, access, sdkType)); + } // set access for all orphan model without override for (const [rawType, sdkType] of context.__referencedTypeCache) { if (rawType.kind !== "Model" && rawType.kind !== "Enum" && rawType.kind !== "Union") continue; diff --git a/packages/typespec-client-generator-core/test/tester.ts b/packages/typespec-client-generator-core/test/tester.ts index 9bf184a683..4080a9b478 100644 --- a/packages/typespec-client-generator-core/test/tester.ts +++ b/packages/typespec-client-generator-core/test/tester.ts @@ -88,6 +88,7 @@ ${x} * XML tester with XML library support */ export const XmlTester = createTester(resolvePath(import.meta.dirname, ".."), { + features: ["union-extends"], libraries: [ "@typespec/http", "@typespec/rest", diff --git a/packages/typespec-client-generator-core/test/types/union.test.ts b/packages/typespec-client-generator-core/test/types/union.test.ts index ea807f706f..77750effaf 100644 --- a/packages/typespec-client-generator-core/test/types/union.test.ts +++ b/packages/typespec-client-generator-core/test/types/union.test.ts @@ -2,9 +2,15 @@ import { expectDiagnostics, t } from "@typespec/compiler/testing"; import { deepStrictEqual, ok, strictEqual } from "assert"; import { describe, it } from "vitest"; import { createTCGCContext } from "../../src/context.js"; +import { getAccess, getUsage } from "../../src/decorators.js"; import { type SdkArrayType, type SdkMethodResponse, UsageFlags } from "../../src/interfaces.js"; import { getSdkModel, getSdkUnion } from "../../src/types.js"; -import { createSdkContextForTester, SimpleTester, SimpleTesterWithService } from "../tester.js"; +import { + createSdkContextForTester, + SimpleTester, + SimpleTesterWithService, + XmlTesterWithBuiltInService, +} from "../tester.js"; import { getSdkTypeHelper } from "./utils.js"; describe("inline discriminated union conversion", () => { @@ -38,6 +44,11 @@ describe("inline discriminated union conversion", () => { strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "kind"); deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), ["cat", "dog"]); strictEqual(base.properties.filter((property) => property.discriminator).length, 1); + const union = context + .getMutatedGlobalNamespace() + .namespaces.get("TestService")! + .unions.get("Pet")!; + strictEqual(getUsage(context, union), base.usage); for (const [tag, name] of [ ["cat", "Cat"], ["dog", "Dog"], @@ -60,6 +71,47 @@ describe("inline discriminated union conversion", () => { }, ); + it.each([false, true])( + "allows a base alternative with other subtypes=%s", + async (withSubtype) => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { base: PetBase, ${withSubtype ? "cat: Cat" : ""} } + @post op echo(@body body: Pet): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + strictEqual(context.sdkPackage.clients[0].methods[0].parameters[0].type, base); + strictEqual(context.sdkPackage.clients[0].methods[0].response.type, base); + strictEqual(base.discriminatorValue, undefined); + deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), withSubtype ? ["cat"] : []); + }, + ); + + it.each(["__proto__", "constructor", "toString"])("preserves the wire tag %s", async (tag) => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { ${tag}: Cat } + @post op echo(@body body: Pet): Pet; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + const cat = context.sdkPackage.models.find((model) => model.name === "Cat"); + ok(base); + ok(cat); + deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), [tag]); + strictEqual(base.discriminatedSubtypes?.[tag], cat); + strictEqual(cat.discriminatorValue, tag); + strictEqual(cat.usage, UsageFlags.Input | UsageFlags.Output | UsageFlags.Json); + }); + it("preserves unions and variant identity when conversion is disabled", async () => { const { program } = await SimpleTesterWithService.compile(` model PetBase { name: string; } @@ -274,23 +326,90 @@ describe("inline discriminated union conversion", () => { strictEqual(property.serializationOptions.json?.name, "kind"); }); - it("preserves union usage and access overrides on the projected base", async () => { - const { program } = await SimpleTesterWithService.compile(` + it.each(["public", "internal"] as const)( + "preserves union usage and %s access overrides on the projected base", + async (access) => { + const { program } = await SimpleTesterWithService.compile(` model PetBase { name: string; } model Cat extends PetBase { meows: boolean; } @usage(Usage.input | Usage.output | Usage.json) - @access(Access.public) + @access(Access.${access}) + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((x) => x.name === "PetBase"); + ok(base); + strictEqual(base.usage, UsageFlags.Input | UsageFlags.Output | UsageFlags.Json); + strictEqual(base.access, access); + strictEqual(base.discriminatedSubtypes?.cat.access, access); + strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "kind"); + }, + ); + + it("still reports internal union overrides conflicting with public operation use", async () => { + const { program } = await SimpleTesterWithService.compile(` + model PetBase { name: string; } + model Cat extends PetBase { meows: boolean; } + @access(Access.internal) @discriminated(#{ envelope: "none" }) union Pet extends PetBase { cat: Cat } + op get(): Pet; `); const context = await createSdkContextForTester(program); - expectDiagnostics(context.diagnostics, []); - const base = context.sdkPackage.models.find((x) => x.name === "PetBase"); + expectDiagnostics( + context.diagnostics, + Array.from({ length: 2 }, () => ({ + code: "@azure-tools/typespec-client-generator-core/conflict-access-override", + })), + ); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); ok(base); - strictEqual(base.usage, UsageFlags.Input | UsageFlags.Output | UsageFlags.Json); strictEqual(base.access, "public"); strictEqual(base.discriminatedSubtypes?.cat.access, "public"); - strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "kind"); + }); + + it("preserves inherited discriminator XML metadata without changing the inherited SDK property", async () => { + const { program } = await XmlTesterWithBuiltInService.compile(` + model Entity { + @clientName("clientKind") + @encodedName("application/xml", "XmlKind") + @Xml.attribute + @Xml.ns("https://example.com/pets", "p") + kind: string; + } + model PetBase extends Entity { name: string; } + model Cat extends PetBase { meows: boolean; } + @discriminated(#{ envelope: "none" }) + union Pet extends PetBase { cat: Cat } + @post op echo(@header contentType: "application/xml", @body body: Pet): + { @header contentType: "application/xml"; @body body: Pet; }; + `); + const context = await createSdkContextForTester(program); + expectDiagnostics(context.diagnostics, []); + const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); + ok(base); + const inherited = base.baseModel?.properties.find((property) => property.name === "clientKind"); + const subtype = base.discriminatedSubtypes?.cat.properties.find( + (property) => property.discriminator, + ); + ok(inherited); + ok(subtype); + for (const property of [base.discriminatorProperty, subtype]) { + ok(property); + strictEqual(property.__raw, inherited.__raw); + strictEqual(property.name, "clientKind"); + strictEqual(property.serializationOptions.xml?.name, "XmlKind"); + strictEqual(property.serializationOptions.xml?.attribute, true); + deepStrictEqual(property.serializationOptions.xml?.ns, { + namespace: "https://example.com/pets", + prefix: "p", + }); + } + strictEqual(inherited.discriminator, false); + strictEqual(inherited.type.kind, "string"); + strictEqual(subtype.type.kind, "constant"); }); it("preserves an inherited discriminator's client name and enum type", async () => { @@ -456,6 +575,11 @@ describe("inline discriminated union conversion", () => { ok(base); strictEqual(base.access, "internal"); strictEqual(base.discriminatedSubtypes?.cat.access, "internal"); + const union = context + .getMutatedGlobalNamespace() + .namespaces.get("TestService")! + .unions.get("Pet")!; + strictEqual(getAccess(context, union), "internal"); }); it("keeps conflicting unions aliased to the base without overwriting subtype tags", async () => { @@ -550,13 +674,6 @@ describe("inline discriminated union conversion", () => { constraint: "extends PetBase", variants: "cat: Cat, UnknownPet", }, - { - name: "base alternative", - decorator: '@discriminated(#{ envelope: "none" })', - cat: "extends PetBase", - constraint: "extends PetBase", - variants: "cat: Cat, base: PetBase", - }, ])("leaves $name unions unchanged", async ({ decorator, cat, constraint, variants }) => { const { program } = await SimpleTesterWithService.compile(` model PetBase { name: string; } diff --git a/packages/typespec-ts/src/modular/helpers/type-helpers.ts b/packages/typespec-ts/src/modular/helpers/type-helpers.ts index 13e2cd75ec..13e6153f72 100644 --- a/packages/typespec-ts/src/modular/helpers/type-helpers.ts +++ b/packages/typespec-ts/src/modular/helpers/type-helpers.ts @@ -18,8 +18,16 @@ export function getDirectSubtypes(type: SdkModelType) { if (!type.discriminatedSubtypes) { return []; } - // Filter out subtypes that extend from other discriminated subtypes (hierarchical inheritance) - return Object.values(type.discriminatedSubtypes).filter((subtype) => subtype.baseModel === type); + const subtypes = new Set(Object.values(type.discriminatedSubtypes)); + // Untagged intermediate models do not introduce another dispatch level. + return [...subtypes].filter((subtype) => { + let parent = subtype.baseModel; + while (parent && parent !== type) { + if (subtypes.has(parent)) return false; + parent = parent.baseModel; + } + return parent === type; + }); } export function getAdditionalPropertiesType(type: SdkType | undefined): SdkType | undefined { diff --git a/packages/typespec-ts/src/modular/serialization/serialize-utils.ts b/packages/typespec-ts/src/modular/serialization/serialize-utils.ts index 75ac442464..7096e49b6d 100644 --- a/packages/typespec-ts/src/modular/serialization/serialize-utils.ts +++ b/packages/typespec-ts/src/modular/serialization/serialize-utils.ts @@ -30,7 +30,11 @@ export function getAllDiscriminatedValues( // Check if this model is a subtype of the given discriminator property if ( !!discriminatorProperty && - model.baseModel?.discriminatorProperty?.name === discriminatorProperty?.name + getAllAncestors(model).some( + (ancestor) => + ancestor.kind === "model" && + ancestor.discriminatorProperty?.name === discriminatorProperty.name, + ) ) { values.push(model.discriminatorValue); if (model.discriminatorProperty?.name === discriminatorProperty.name) { diff --git a/packages/typespec-ts/test/modular-unit/scenarios/models/inlineDiscriminatedUnion.md b/packages/typespec-ts/test/modular-unit/scenarios/models/inlineDiscriminatedUnion.md new file mode 100644 index 0000000000..3a847ad016 --- /dev/null +++ b/packages/typespec-ts/test/modular-unit/scenarios/models/inlineDiscriminatedUnion.md @@ -0,0 +1,65 @@ +# Transitive inline-discriminated union dispatch + +Variants behind an untagged intermediate model must remain in the union and in both serialization dispatchers. + +```tsp +model PetBase { + name: string; +} + +model Mammal extends PetBase { + age?: int32; +} + +model Cat extends Mammal { + meows: boolean; +} + +model Dog extends PetBase { + barks: boolean; +} + +@discriminated(#{ envelope: "none" }) +union Pet extends PetBase { + cat: Cat, + dog: Dog, +} + +@post +op echo(@body body: Pet): Pet; +``` + +```ts models alias PetBaseUnion +/** Alias for PetBaseUnion */ +export type PetBaseUnion = Cat | Dog | PetBase; +``` + +```ts models function petBaseUnionSerializer +export function petBaseUnionSerializer(item: PetBaseUnion): any { + switch (item.kind) { + case "cat": + return catSerializer(item as Cat); + + case "dog": + return dogSerializer(item as Dog); + + default: + return petBaseSerializer(item); + } +} +``` + +```ts models function petBaseUnionDeserializer +export function petBaseUnionDeserializer(item: any): PetBaseUnion { + switch (item["kind"]) { + case "cat": + return catDeserializer(item as Cat); + + case "dog": + return dogDeserializer(item as Dog); + + default: + return petBaseDeserializer(item); + } +} +``` diff --git a/packages/typespec-ts/test/util/test-util.ts b/packages/typespec-ts/test/util/test-util.ts index d29c834947..f06245dcf6 100644 --- a/packages/typespec-ts/test/util/test-util.ts +++ b/packages/typespec-ts/test/util/test-util.ts @@ -48,6 +48,7 @@ const __dirname = getDirectoryPath(fileURLToPath(import.meta.url)); * base so the libraries resolve from this package's dependencies. */ const sharedTester = createTester(resolvePath(__dirname, "..", ".."), { + features: ["union-extends"], libraries: [ "@typespec/http", "@typespec/rest", diff --git a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md index 736425458b..37afa09a1d 100644 --- a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md +++ b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md @@ -269,7 +269,7 @@ union Pet extends PetBase { References to `Pet` resolve to the existing `PetBase` SDK model, including operation parameters/responses, model properties, containers, and nullable uses. The base's `discriminatorProperty` and `discriminatedSubtypes`, plus each variant's `discriminatorValue` and discriminator property, describe the inline tags. Missing discriminator properties are synthesized; the compiler type graph is not changed. The projected model retains the base model's name and identity rather than introducing a model named after the union. -Ordinary unions, object-envelope unions, structural-only variants, unions without a model constraint, and unions containing defaults or the base itself as an alternative are not converted. Alternate types and legacy hierarchy rebasing retain their existing handling. Eligible unions always resolve to their base SDK model. Conflicting discriminator metadata produces a `union-model-conversion-conflict` error rather than overwriting an existing hierarchy or falling back to a union representation. +Ordinary unions, object-envelope unions, structural-only variants, unions without a model constraint, and unions containing defaults are not converted. An alternative using the base model itself is supported as the hierarchy's fallback and is not added to `discriminatedSubtypes`. Alternate types and legacy hierarchy rebasing retain their existing handling. Eligible unions always resolve to their base SDK model. Conflicting discriminator metadata produces a `union-model-conversion-conflict` error rather than overwriting an existing hierarchy or falling back to a union representation. Compatible unions sharing the same base share its SDK model and combined subtype map; the polymorphic representation does not enforce each union's subset separately. Synthesized inline discriminator properties also appear on directly used SDK variants. Disable conversion when an emitter needs union-specific serialization or exact alternative-set information. From 966d79e48cb0d8207610abee5e509d103a57e10d Mon Sep 17 00:00:00 2001 From: tadelesh Date: Fri, 16 Oct 2026 17:12:06 +0800 Subject: [PATCH 5/7] fix(tcgc): exclude unions containing the base from conversion 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 --- .../src/types.ts | 23 ++++++-------- .../test/types/union.test.ts | 30 +++++++++++++++---- .../guideline.md | 2 +- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index b1b6606008..5d9a551bb2 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -720,6 +720,7 @@ function getSdkDiscriminatedUnionModel( [...discriminated.variants.values()].some( (variant) => variant.kind !== "Model" || + variant === base || !walkBaseChain(variant).includes(base) || walkBaseChain(variant).some( (model) => getAlternateType(context, model) || getLegacyHierarchyBuilding(context, model), @@ -732,20 +733,14 @@ function getSdkDiscriminatedUnionModel( (context.__discriminatedUnionModels ??= new Map()).set(union, base); const model = diagnostics.pipe(getSdkModelWithDiagnostics(context, base, operation)); updateReferencedTypeMap(context, union, model); - // The base is represented by the hierarchy's fallback, not by a subtype of itself. - const variants = [...discriminated.variants] - .filter(([, variant]) => variant !== base) - .map(([tag, variant]) => { - compilerAssert( - variant.kind === "Model", - "Inline discriminated union variants must be models.", - ); - return { - tag, - raw: variant, - model: diagnostics.pipe(getSdkModelWithDiagnostics(context, variant, operation)), - }; - }); + const variants = [...discriminated.variants].map(([tag, variant]) => { + compilerAssert(variant.kind === "Model", "Inline discriminated union variants must be models."); + return { + tag, + raw: variant, + model: diagnostics.pipe(getSdkModelWithDiagnostics(context, variant, operation)), + }; + }); const propertyName = discriminated.options.discriminatorPropertyName; // A payload model can have only one tag. Existing hierarchies must also agree on // the discriminator's wire name and on tags introduced by other unions. diff --git a/packages/typespec-client-generator-core/test/types/union.test.ts b/packages/typespec-client-generator-core/test/types/union.test.ts index 77750effaf..f0f3e61409 100644 --- a/packages/typespec-client-generator-core/test/types/union.test.ts +++ b/packages/typespec-client-generator-core/test/types/union.test.ts @@ -72,7 +72,7 @@ describe("inline discriminated union conversion", () => { ); it.each([false, true])( - "allows a base alternative with other subtypes=%s", + "does not convert a union containing its base with other subtypes=%s", async (withSubtype) => { const { program } = await SimpleTesterWithService.compile(` model PetBase { name: string; } @@ -82,13 +82,33 @@ describe("inline discriminated union conversion", () => { @post op echo(@body body: Pet): Pet; `); const context = await createSdkContextForTester(program); - expectDiagnostics(context.diagnostics, []); + expectDiagnostics( + context.diagnostics, + Array.from({ length: withSubtype ? 2 : 1 }, () => ({ + code: "@azure-tools/typespec-azure-core/union-enums-invalid-kind", + severity: "warning" as const, + })), + ); const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); ok(base); - strictEqual(context.sdkPackage.clients[0].methods[0].parameters[0].type, base); - strictEqual(context.sdkPackage.clients[0].methods[0].response.type, base); + const union = context.sdkPackage.unions[0]; + ok(union); + strictEqual(union.kind, "union"); + strictEqual(union.baseType, base); + strictEqual(union.discriminatedOptions?.envelope, "none"); + strictEqual(context.sdkPackage.clients[0].methods[0].parameters[0].type, union); + strictEqual(context.sdkPackage.clients[0].methods[0].response.type, union); + deepStrictEqual(union.variantTypes, [ + base, + ...(withSubtype ? [context.sdkPackage.models.find((model) => model.name === "Cat")] : []), + ]); + strictEqual(base.discriminatorProperty, undefined); strictEqual(base.discriminatorValue, undefined); - deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), withSubtype ? ["cat"] : []); + strictEqual(base.discriminatedSubtypes, undefined); + strictEqual( + context.sdkPackage.models.find((model) => model.name === "Cat")?.discriminatorValue, + undefined, + ); }, ); diff --git a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md index 37afa09a1d..1786993479 100644 --- a/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md +++ b/website/src/content/docs/docs/libraries/typespec-client-generator-core/guideline.md @@ -269,7 +269,7 @@ union Pet extends PetBase { References to `Pet` resolve to the existing `PetBase` SDK model, including operation parameters/responses, model properties, containers, and nullable uses. The base's `discriminatorProperty` and `discriminatedSubtypes`, plus each variant's `discriminatorValue` and discriminator property, describe the inline tags. Missing discriminator properties are synthesized; the compiler type graph is not changed. The projected model retains the base model's name and identity rather than introducing a model named after the union. -Ordinary unions, object-envelope unions, structural-only variants, unions without a model constraint, and unions containing defaults are not converted. An alternative using the base model itself is supported as the hierarchy's fallback and is not added to `discriminatedSubtypes`. Alternate types and legacy hierarchy rebasing retain their existing handling. Eligible unions always resolve to their base SDK model. Conflicting discriminator metadata produces a `union-model-conversion-conflict` error rather than overwriting an existing hierarchy or falling back to a union representation. +Ordinary unions, object-envelope unions, structural-only variants, unions without a model constraint, and unions containing defaults or the base model itself as an alternative are not converted. Alternate types and legacy hierarchy rebasing retain their existing handling. Eligible unions always resolve to their base SDK model. Conflicting discriminator metadata produces a `union-model-conversion-conflict` error rather than overwriting an existing hierarchy or falling back to a union representation. Compatible unions sharing the same base share its SDK model and combined subtype map; the polymorphic representation does not enforce each union's subset separately. Synthesized inline discriminator properties also appear on directly used SDK variants. Disable conversion when an emitter needs union-specific serialization or exact alternative-set information. From 233dfe4b5aafa9dcbc61fa1c6a3ed9da23d9ac6b Mon Sep 17 00:00:00 2001 From: tadelesh Date: Fri, 16 Oct 2026 17:39:26 +0800 Subject: [PATCH 6/7] revert: remove TypeScript emitter changes from TCGC union work 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 --- .../changes/ts-transitive-union-dispatch.md | 7 -- .../src/modular/helpers/type-helpers.ts | 12 +--- .../modular/serialization/serialize-utils.ts | 6 +- .../models/inlineDiscriminatedUnion.md | 65 ------------------- packages/typespec-ts/test/util/test-util.ts | 1 - 5 files changed, 3 insertions(+), 88 deletions(-) delete mode 100644 .chronus/changes/ts-transitive-union-dispatch.md delete mode 100644 packages/typespec-ts/test/modular-unit/scenarios/models/inlineDiscriminatedUnion.md diff --git a/.chronus/changes/ts-transitive-union-dispatch.md b/.chronus/changes/ts-transitive-union-dispatch.md deleted file mode 100644 index ead9cc4eb0..0000000000 --- a/.chronus/changes/ts-transitive-union-dispatch.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -changeKind: fix -packages: - - "@azure-tools/typespec-ts" ---- - -Include discriminated union variants behind untagged intermediate base models in polymorphic unions and serialization dispatch. diff --git a/packages/typespec-ts/src/modular/helpers/type-helpers.ts b/packages/typespec-ts/src/modular/helpers/type-helpers.ts index 13e6153f72..13e2cd75ec 100644 --- a/packages/typespec-ts/src/modular/helpers/type-helpers.ts +++ b/packages/typespec-ts/src/modular/helpers/type-helpers.ts @@ -18,16 +18,8 @@ export function getDirectSubtypes(type: SdkModelType) { if (!type.discriminatedSubtypes) { return []; } - const subtypes = new Set(Object.values(type.discriminatedSubtypes)); - // Untagged intermediate models do not introduce another dispatch level. - return [...subtypes].filter((subtype) => { - let parent = subtype.baseModel; - while (parent && parent !== type) { - if (subtypes.has(parent)) return false; - parent = parent.baseModel; - } - return parent === type; - }); + // Filter out subtypes that extend from other discriminated subtypes (hierarchical inheritance) + return Object.values(type.discriminatedSubtypes).filter((subtype) => subtype.baseModel === type); } export function getAdditionalPropertiesType(type: SdkType | undefined): SdkType | undefined { diff --git a/packages/typespec-ts/src/modular/serialization/serialize-utils.ts b/packages/typespec-ts/src/modular/serialization/serialize-utils.ts index 7096e49b6d..75ac442464 100644 --- a/packages/typespec-ts/src/modular/serialization/serialize-utils.ts +++ b/packages/typespec-ts/src/modular/serialization/serialize-utils.ts @@ -30,11 +30,7 @@ export function getAllDiscriminatedValues( // Check if this model is a subtype of the given discriminator property if ( !!discriminatorProperty && - getAllAncestors(model).some( - (ancestor) => - ancestor.kind === "model" && - ancestor.discriminatorProperty?.name === discriminatorProperty.name, - ) + model.baseModel?.discriminatorProperty?.name === discriminatorProperty?.name ) { values.push(model.discriminatorValue); if (model.discriminatorProperty?.name === discriminatorProperty.name) { diff --git a/packages/typespec-ts/test/modular-unit/scenarios/models/inlineDiscriminatedUnion.md b/packages/typespec-ts/test/modular-unit/scenarios/models/inlineDiscriminatedUnion.md deleted file mode 100644 index 3a847ad016..0000000000 --- a/packages/typespec-ts/test/modular-unit/scenarios/models/inlineDiscriminatedUnion.md +++ /dev/null @@ -1,65 +0,0 @@ -# Transitive inline-discriminated union dispatch - -Variants behind an untagged intermediate model must remain in the union and in both serialization dispatchers. - -```tsp -model PetBase { - name: string; -} - -model Mammal extends PetBase { - age?: int32; -} - -model Cat extends Mammal { - meows: boolean; -} - -model Dog extends PetBase { - barks: boolean; -} - -@discriminated(#{ envelope: "none" }) -union Pet extends PetBase { - cat: Cat, - dog: Dog, -} - -@post -op echo(@body body: Pet): Pet; -``` - -```ts models alias PetBaseUnion -/** Alias for PetBaseUnion */ -export type PetBaseUnion = Cat | Dog | PetBase; -``` - -```ts models function petBaseUnionSerializer -export function petBaseUnionSerializer(item: PetBaseUnion): any { - switch (item.kind) { - case "cat": - return catSerializer(item as Cat); - - case "dog": - return dogSerializer(item as Dog); - - default: - return petBaseSerializer(item); - } -} -``` - -```ts models function petBaseUnionDeserializer -export function petBaseUnionDeserializer(item: any): PetBaseUnion { - switch (item["kind"]) { - case "cat": - return catDeserializer(item as Cat); - - case "dog": - return dogDeserializer(item as Dog); - - default: - return petBaseDeserializer(item); - } -} -``` diff --git a/packages/typespec-ts/test/util/test-util.ts b/packages/typespec-ts/test/util/test-util.ts index f06245dcf6..d29c834947 100644 --- a/packages/typespec-ts/test/util/test-util.ts +++ b/packages/typespec-ts/test/util/test-util.ts @@ -48,7 +48,6 @@ const __dirname = getDirectoryPath(fileURLToPath(import.meta.url)); * base so the libraries resolve from this package's dependencies. */ const sharedTester = createTester(resolvePath(__dirname, "..", ".."), { - features: ["union-extends"], libraries: [ "@typespec/http", "@typespec/rest", From f2d92d97d9cca509db29b6502c37a66f7df1fef7 Mon Sep 17 00:00:00 2001 From: tadelesh Date: Mon, 19 Oct 2026 15:45:05 +0800 Subject: [PATCH 7/7] fix(tcgc): address diagnostic and inheritance review comments 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 --- .../union-model-conversion-conflict.md | 2 +- .../typespec-client-generator-core/src/lib.ts | 2 +- .../test/types/union.test.ts | 64 ++++++------------- 3 files changed, 21 insertions(+), 47 deletions(-) diff --git a/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md b/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md index 2615d1ec78..8b856ef672 100644 --- a/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md +++ b/packages/typespec-client-generator-core/src/diagnostics/union-model-conversion-conflict.md @@ -33,7 +33,7 @@ op get(): Pet; TCGC reports: ```text -Discriminated union 'Pet' has conflicting discriminator metadata for its base model hierarchy. Use consistent discriminator properties and tag mappings, or set convertDiscriminatedUnionsToModels to false to retain union representations. +Discriminated union 'Pet' has conflicting discriminator metadata for its base model hierarchy. Use consistent discriminator properties and tag mappings, or set `convertDiscriminatedUnionsToModels` to `false` to retain union representations. ``` ## ✅ How to Fix diff --git a/packages/typespec-client-generator-core/src/lib.ts b/packages/typespec-client-generator-core/src/lib.ts index b118afa781..c34be610ed 100644 --- a/packages/typespec-client-generator-core/src/lib.ts +++ b/packages/typespec-client-generator-core/src/lib.ts @@ -175,7 +175,7 @@ export const $lib = createTypeSpecLibrary({ ...doc("union-model-conversion-conflict"), severity: "error", messages: { - default: paramMessage`Discriminated union '${"unionName"}' has conflicting discriminator metadata for its base model hierarchy. Use consistent discriminator properties and tag mappings, or set convertDiscriminatedUnionsToModels to false to retain union representations.`, + default: paramMessage`Discriminated union '${"unionName"}' has conflicting discriminator metadata for its base model hierarchy. Use consistent discriminator properties and tag mappings, or set \`convertDiscriminatedUnionsToModels\` to \`false\` to retain union representations.`, }, }, "union-null": { diff --git a/packages/typespec-client-generator-core/test/types/union.test.ts b/packages/typespec-client-generator-core/test/types/union.test.ts index f0f3e61409..154b41d50e 100644 --- a/packages/typespec-client-generator-core/test/types/union.test.ts +++ b/packages/typespec-client-generator-core/test/types/union.test.ts @@ -43,6 +43,11 @@ describe("inline discriminated union conversion", () => { strictEqual(base.discriminatorProperty?.name, "kind"); strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "kind"); deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), ["cat", "dog"]); + const mammal = context.sdkPackage.models.find((model) => model.name === "Mammal"); + ok(mammal); + strictEqual(mammal.baseModel, base); + strictEqual(base.discriminatedSubtypes?.cat.baseModel, mammal); + strictEqual(mammal.discriminatorProperty, undefined); strictEqual(base.properties.filter((property) => property.discriminator).length, 1); const union = context .getMutatedGlobalNamespace() @@ -112,26 +117,6 @@ describe("inline discriminated union conversion", () => { }, ); - it.each(["__proto__", "constructor", "toString"])("preserves the wire tag %s", async (tag) => { - const { program } = await SimpleTesterWithService.compile(` - model PetBase { name: string; } - model Cat extends PetBase { meows: boolean; } - @discriminated(#{ envelope: "none" }) - union Pet extends PetBase { ${tag}: Cat } - @post op echo(@body body: Pet): Pet; - `); - const context = await createSdkContextForTester(program); - expectDiagnostics(context.diagnostics, []); - const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); - const cat = context.sdkPackage.models.find((model) => model.name === "Cat"); - ok(base); - ok(cat); - deepStrictEqual(Object.keys(base.discriminatedSubtypes ?? {}), [tag]); - strictEqual(base.discriminatedSubtypes?.[tag], cat); - strictEqual(cat.discriminatorValue, tag); - strictEqual(cat.usage, UsageFlags.Input | UsageFlags.Output | UsageFlags.Json); - }); - it("preserves unions and variant identity when conversion is disabled", async () => { const { program } = await SimpleTesterWithService.compile(` model PetBase { name: string; } @@ -227,30 +212,6 @@ describe("inline discriminated union conversion", () => { ); }); - it("does not reshape a used model because of an unhandled union outside the service", async () => { - const { program } = await SimpleTester.compile(` - namespace Definitions { - model PetBase { name: string; } - model Cat extends PetBase { meows: boolean; } - @discriminated(#{ envelope: "none" }) - union UnusedPet extends PetBase { cat: Cat } - } - @service namespace TestService { - op get(): Definitions.PetBase; - } - `); - const context = await createSdkContextForTester(program); - expectDiagnostics(context.diagnostics, []); - const base = context.sdkPackage.models.find((model) => model.name === "PetBase"); - ok(base); - strictEqual(base.discriminatorProperty, undefined); - strictEqual(base.discriminatedSubtypes, undefined); - strictEqual( - context.sdkPackage.models.some((model) => model.name === "Cat"), - false, - ); - }); - it("updates previously handled model properties and HTTP types to the projected base", async () => { const { program } = await SimpleTesterWithService.compile(` model PetBase { name: string; } @@ -436,7 +397,8 @@ describe("inline discriminated union conversion", () => { const { program } = await SimpleTesterWithService.compile(` union PetKind { cat: "cat", dog: "dog" } model NamedEntity { @clientName("clientKind") kind: PetKind; } - model PetBase extends NamedEntity { name: string; } + model Entity extends NamedEntity { id: string; } + model PetBase extends Entity { name: string; } model Cat extends PetBase { meows: boolean; } model Dog extends PetBase { barks: boolean; } @discriminated(#{ envelope: "none" }) @@ -454,12 +416,24 @@ describe("inline discriminated union conversion", () => { strictEqual(base.discriminatorProperty?.serializationOptions.json?.name, "kind"); strictEqual(base.discriminatorProperty?.type.kind, "enum"); strictEqual(base.baseModel?.discriminatorProperty, undefined); + const entity = base.baseModel; + const root = entity?.baseModel; + ok(entity); + ok(root); + strictEqual(entity.name, "Entity"); + strictEqual(root.name, "NamedEntity"); + strictEqual(root.discriminatorProperty, undefined); + const inherited = root.properties.find((property) => property.name === "clientKind"); + ok(inherited); + strictEqual(base.discriminatorProperty?.__raw, inherited.__raw); + strictEqual(base.discriminatorProperty?.type, inherited.type); const property = base.discriminatedSubtypes?.cat.properties.find( (property) => property.discriminator, ); ok(property); strictEqual(property.name, "clientKind"); strictEqual(property.type.kind, "enumvalue"); + strictEqual(base.discriminatedSubtypes?.cat.baseModel, base); strictEqual(base.discriminatedSubtypes?.dog.discriminatorValue, "dog"); context.sdkPackage.clients[0].methods.forEach((method) => strictEqual(method.response.type, base),