From ff0bed2835b25e1a364d490146da45ca3eb497ce Mon Sep 17 00:00:00 2001 From: jolov Date: Wed, 26 Aug 2026 21:45:02 -0700 Subject: [PATCH 1/8] feat(compiler): add `extends` base type clause for unions A named union can now declare a base type with `extends`. Every variant must be assignable to that base type, and the resolved type is exposed on the type graph as `Union.baseType` so emitters can represent the union with a polymorphic base type in languages without native unions. `extends` on a union is purely a constraint: it doesn't create any inheritance relationship, the base type doesn't become a variant, it doesn't make the union extensible and it has no interaction with `@discriminator`. Fixes #2737 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4 --- .../union-extends-base-type-2026-8-26.md | 28 ++ grammars/typespec.json | 24 + packages/compiler/src/core/checker.ts | 109 +++++ packages/compiler/src/core/parser.ts | 12 + packages/compiler/src/core/semantic-walker.ts | 3 + packages/compiler/src/core/types.ts | 20 + .../compiler/src/experimental/mutators.ts | 1 + .../src/formatter/print/comment-handler.ts | 26 ++ .../compiler/src/formatter/print/printer.ts | 10 +- packages/compiler/src/server/completion.ts | 12 +- packages/compiler/src/server/tmlanguage.ts | 18 +- packages/compiler/test/checker/union.test.ts | 433 +++++++++++++++++- .../test/experimental/mutator.test.ts | 12 + .../compiler/test/formatter/formatter.test.ts | 103 +++++ .../test/formatter/scenarios/inputs/union.tsp | 7 + .../formatter/scenarios/outputs/union.tsp | 9 + packages/compiler/test/parser.test.ts | 17 +- .../compiler/test/semantic-walker.test.ts | 18 + .../compiler/test/server/colorization.test.ts | 35 ++ .../compiler/test/server/completion.test.ts | 36 ++ packages/spec/src/spec.emu.html | 5 +- .../docs/docs/language-basics/unions.md | 71 +++ 22 files changed, 1001 insertions(+), 8 deletions(-) create mode 100644 .chronus/changes/union-extends-base-type-2026-8-26.md diff --git a/.chronus/changes/union-extends-base-type-2026-8-26.md b/.chronus/changes/union-extends-base-type-2026-8-26.md new file mode 100644 index 00000000000..07441da5037 --- /dev/null +++ b/.chronus/changes/union-extends-base-type-2026-8-26.md @@ -0,0 +1,28 @@ +--- +changeKind: feature +packages: + - "@typespec/compiler" +--- + +Add support for an `extends` clause on union statements to constrain every variant to a common base type. + +```tsp +model PetBase { + name: string; +} +model Cat extends PetBase { + toy: string; +} +model Dog extends PetBase { + food: string; +} + +union Pet extends PetBase { + cat: Cat, + dog: Dog, +} +``` + +The base type is exposed on the type graph as `Union.baseType`, giving emitters an easy way to know that all the variants of a union share a common base type. A diagnostic is reported on any variant that isn't assignable to the base type. + +`extends` on a union is purely a constraint: it doesn't imply any subtyping relationship, it doesn't make the union extensible and it has no interaction with `@discriminator`. diff --git a/grammars/typespec.json b/grammars/typespec.json index c29ce411b12..1c9c41f6bdf 100644 --- a/grammars/typespec.json +++ b/grammars/typespec.json @@ -1378,6 +1378,24 @@ } ] }, + "union-extends": { + "name": "meta.union-extends.typespec", + "begin": "\\b(extends)\\b", + "beginCaptures": { + "1": { + "name": "keyword.other.tsp" + } + }, + "end": "((?=\\{)|(?=;|@|\\)|\\}|\\b(?:extern|internal)\\b|\\b(?:namespace|model|op|using|import|enum|alias|union|interface|dec|fn)\\b))", + "patterns": [ + { + "include": "#expression" + }, + { + "include": "#punctuation-comma" + } + ] + }, "union-statement": { "name": "meta.union-statement.typespec", "begin": "(?:(internal)\\s+)?\\b(union)\\b\\s+(\\b[_$[:alpha:]][_$[:alnum:]]*\\b|`(?:[^`\\\\]|\\\\.)*`)", @@ -1397,6 +1415,12 @@ { "include": "#token" }, + { + "include": "#type-parameters" + }, + { + "include": "#union-extends" + }, { "include": "#union-body" } diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 9d989e33d61..2372ad81099 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -1399,6 +1399,8 @@ export function createChecker(program: Program, resolver: NameResolver): Checker ); case SyntaxKind.InterfaceStatement: return checkDeprecatedNode(node); + case SyntaxKind.UnionStatement: + return checkDeprecatedNode(node); case SyntaxKind.IntersectionExpression: case SyntaxKind.UnionExpression: case SyntaxKind.ModelProperty: @@ -7786,6 +7788,10 @@ export function createChecker(program: Program, resolver: NameResolver): Checker }); linkType(ctx, links, unionType); + if (node.extends) { + unionType.baseType = checkUnionBaseType(ctx, node, unionType, node.extends); + } + unionType.decorators = checkDecorators(ctx, unionType, node); checkUnionVariants(ctx, unionType, node, variants); @@ -7823,9 +7829,112 @@ export function createChecker(program: Program, resolver: NameResolver): Checker continue; } variants.set(variantType.name as string, variantType); + checkUnionVariantAgainstBaseType(ctx, parentUnion, variantNode, variantType); + } + } + + /** + * Validate that a union variant satisfies the constraint declared by the union `extends` clause. + * Skipped inside of an uninstantiated template declaration where variant types are still + * unresolved template parameters. Each instantiation is checked instead. + */ + function checkUnionVariantAgainstBaseType( + ctx: CheckContext, + parentUnion: Union, + variantNode: UnionVariantNode, + variantType: UnionVariant, + ) { + const baseType = parentUnion.baseType; + if (baseType === undefined || ctx.hasFlags(CheckFlags.InTemplateDeclaration)) { + return; + } + if (isErrorType(variantType.type)) { + return; + } + checkTypeAssignable(variantType.type, baseType, variantNode.value); + } + + /** + * Resolve the type referenced by a union `extends` clause. + * + * The resulting type is only a constraint on the union variants: it doesn't create any + * inheritance relationship, it doesn't add anything to the union and it doesn't make the + * union extensible. + */ + function checkUnionBaseType( + ctx: CheckContext, + union: UnionStatementNode, + unionType: Union, + extendsRef: Expression, + ): Type | undefined { + const unionSymId = getNodeSym(union); + pendingResolutions.start(unionSymId, ResolutionKind.BaseType); + + try { + const target = resolver.getNodeLinks(extendsRef).resolvedSymbol; + if (target && pendingResolutions.has(target, ResolutionKind.BaseType)) { + if (ctx.mapper === undefined) { + reportCheckerDiagnostic( + createDiagnostic({ + code: "circular-base-type", + format: { typeName: target.name }, + target: target, + }), + ); + } + return undefined; + } + + const baseType = getTypeForNode(extendsRef, ctx); + if (isErrorType(baseType)) { + // Should already have reported an error when resolving the expression. + return undefined; + } + + // `extends` accepts an arbitrary expression so, unlike `model`/`scalar`, the union can + // also reference itself through a union expression (e.g. `union a extends a | string` or + // `union a extends b` with `alias b = a | string`). Those don't go through a symbol that + // `pendingResolutions` can observe so they are detected on the resolved type instead. + if (unionExpressionReferences(baseType, unionType)) { + if (ctx.mapper === undefined) { + reportCheckerDiagnostic( + createDiagnostic({ + code: "circular-base-type", + format: { typeName: union.id.sv }, + target: extendsRef, + }), + ); + } + return undefined; + } + return baseType; + } finally { + pendingResolutions.finish(unionSymId, ResolutionKind.BaseType); } } + /** + * Check whether `target` is reachable from `type` through union expressions only. + * + * Traversal deliberately stops at anything else (named unions, models, arrays, ...): a union + * referencing itself from those positions builds a perfectly valid cyclic type graph, exactly + * like `model Foo { foo: Foo }` does, and must not be reported. Only union expressions are + * followed, which is a finite syntactic structure, so this always terminates. + */ + function unionExpressionReferences(type: Type, target: Union): boolean { + if (type === target) { + return true; + } + if (type.kind === "Union" && type.expression) { + for (const variant of type.variants.values()) { + if (unionExpressionReferences(variant.type, target)) { + return true; + } + } + } + return false; + } + function checkUnionVariant(ctx: CheckContext, variantNode: UnionVariantNode): UnionVariant { const links = getSymbolLinksForMember(variantNode); if (links && links.declaredType && ctx.mapper === undefined) { diff --git a/packages/compiler/src/core/parser.ts b/packages/compiler/src/core/parser.ts index 749ec169d17..e75940ecba9 100644 --- a/packages/compiler/src/core/parser.ts +++ b/packages/compiler/src/core/parser.ts @@ -704,6 +704,9 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa const { items: templateParameters, range: templateParametersRange } = parseTemplateParameterList(); + expectTokenIsOneOf(Token.OpenBrace, Token.ExtendsKeyword); + + const optionalExtends = parseOptionalUnionExtends(); const { items: options } = parseList(ListKind.UnionVariants, parseUnionVariant); return { @@ -711,6 +714,7 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa id, templateParameters, templateParametersRange, + extends: optionalExtends, decorators, modifiers, modifierFlags: modifiersToFlags(modifiers), @@ -719,6 +723,13 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa }; } + function parseOptionalUnionExtends() { + if (parseOptional(Token.ExtendsKeyword)) { + return parseExpression(); + } + return undefined; + } + function parseIdOrValueForVariant(): Expression { const nextToken = token(); @@ -3072,6 +3083,7 @@ export function visitChildren(node: Node, cb: NodeCallback): T | undefined visitEach(cb, node.decorators) || visitNode(cb, node.id) || visitEach(cb, node.templateParameters) || + visitNode(cb, node.extends) || visitEach(cb, node.options) ); case SyntaxKind.UnionVariant: diff --git a/packages/compiler/src/core/semantic-walker.ts b/packages/compiler/src/core/semantic-walker.ts index d4e127a032f..e13bf3bd602 100644 --- a/packages/compiler/src/core/semantic-walker.ts +++ b/packages/compiler/src/core/semantic-walker.ts @@ -335,6 +335,9 @@ function navigateUnionType(type: Union, context: NavigationContext) { return; } if (context.emit("union", type) === ListenerFlow.NoRecursion) return; + if (type.baseType) { + navigateTypeInternal(type.baseType, context); + } for (const variant of type.variants.values()) { navigateUnionTypeVariant(variant, context); } diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index 92a0d21f665..4a1d779dc89 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -675,6 +675,19 @@ export interface Union extends BaseType, DecoratedType, TemplatedTypeBase { expression: boolean; + /** + * Type declared with the `extends` clause of a union statement. Every variant of the + * union is guaranteed to be assignable to this type. + * + * This is only set for named unions declared with an `extends` clause. It documents a + * constraint: it does **not** imply a subclassing relationship, it does **not** mean the + * union is extensible, and it has no interaction with `@discriminator`. + * + * Emitters should not require this to be present: a union with the same variants and no + * `extends` clause should ideally be handled the same way. + */ + baseType?: Type; + /** * Late-bound symbol of this interface type. * @internal @@ -1600,6 +1613,13 @@ export interface InterfaceStatementNode extends BaseNode, DeclarationNode, Templ export interface UnionStatementNode extends BaseNode, DeclarationNode, TemplateDeclarationNode { readonly kind: SyntaxKind.UnionStatement; readonly options: readonly UnionVariantNode[]; + /** + * Type that every variant of this union must be assignable to. + * + * This is a constraint only, it does not imply any subtyping relationship between + * the union and the base type beyond the one that already exists structurally. + */ + readonly extends?: Expression; readonly decorators: readonly DecoratorExpressionNode[]; readonly parent?: TypeSpecScriptNode | NamespaceStatementNode; } diff --git a/packages/compiler/src/experimental/mutators.ts b/packages/compiler/src/experimental/mutators.ts index 150621d5dbb..783381b6645 100644 --- a/packages/compiler/src/experimental/mutators.ts +++ b/packages/compiler/src/experimental/mutators.ts @@ -701,6 +701,7 @@ function createMutatorEngine( break; case "Union": mutateSubMap(root, "variants", mutating, newMutators); + mutateProperty(root, "baseType", mutating, newMutators); break; case "UnionVariant": mutateProperty(root, "type", mutating, newMutators); diff --git a/packages/compiler/src/formatter/print/comment-handler.ts b/packages/compiler/src/formatter/print/comment-handler.ts index b1138fdaa91..fa4678aa080 100644 --- a/packages/compiler/src/formatter/print/comment-handler.ts +++ b/packages/compiler/src/formatter/print/comment-handler.ts @@ -19,6 +19,7 @@ export const commentHandler: Printer["handleComments"] = { addEmptyInterfaceComment, addEmptyModelComment, addEmptyScalarComment, + addEmptyUnionComment, addCommentBetweenAnnotationsAndNode, handleOnlyComments, ].some((x) => x({ comment, text, options, ast: ast as TypeSpecScriptNode, isLastComment })), @@ -153,6 +154,31 @@ function addEmptyScalarComment({ comment }: CommentContext) { return false; } +/** + * When a comment is on an empty union make sure it gets added as a dangling comment on it and not on the identifier. + * + * @example + * + * union Foo extends Bar { + * // My comment + * } + */ +function addEmptyUnionComment({ comment }: CommentContext) { + const { precedingNode, enclosingNode } = comment; + + if ( + enclosingNode && + enclosingNode.kind === SyntaxKind.UnionStatement && + enclosingNode.options.length === 0 && + precedingNode && + (precedingNode === enclosingNode.id || precedingNode === enclosingNode.extends) + ) { + util.addDanglingComment(enclosingNode, comment, undefined); + return true; + } + return false; +} + function handleOnlyComments({ comment, ast, isLastComment }: CommentContext) { const { enclosingNode } = comment; if (ast?.statements?.length === 0) { diff --git a/packages/compiler/src/formatter/print/printer.ts b/packages/compiler/src/formatter/print/printer.ts index 51491a656ab..da3e9580d1b 100644 --- a/packages/compiler/src/formatter/print/printer.ts +++ b/packages/compiler/src/formatter/print/printer.ts @@ -742,12 +742,14 @@ export function printUnionStatement( const id = path.call(print, "id"); const { decorators } = printDecorators(path, options, print, { tryInline: false }); const generic = printTemplateParameters(path, options, print, "templateParameters"); + const heritage = printHeritageClause(path, print, "extends", "extends"); return [ decorators, printModifiers(path, options, print), "union ", id, generic, + heritage, " ", printUnionVariantsBlock(path, options, print), ]; @@ -759,11 +761,15 @@ export function printUnionVariantsBlock( print: PrettierChildPrint, ) { const node = path.node; - if (node.options.length === 0) { + const nodeHasComments = hasComments(node, CommentCheckFlags.Dangling); + if (node.options.length === 0 && !nodeHasComments) { return "{}"; } - const body = joinMembersInBlock(path, "options", options, print, ",", hardline); + const body = [joinMembersInBlock(path, "options", options, print, ",", hardline)]; + if (nodeHasComments) { + body.push(printDanglingComments(path, options, { sameIndent: true })); + } return group(["{", indent(body), hardline, "}"]); } diff --git a/packages/compiler/src/server/completion.ts b/packages/compiler/src/server/completion.ts index 0c0294f3b57..2d596830069 100644 --- a/packages/compiler/src/server/completion.ts +++ b/packages/compiler/src/server/completion.ts @@ -91,6 +91,7 @@ function addCompletionByLookingBackward( n.kind === SyntaxKind.ScalarStatement || n.kind === SyntaxKind.OperationStatement || n.kind === SyntaxKind.InterfaceStatement || + n.kind === SyntaxKind.UnionStatement || n.kind === SyntaxKind.TemplateParameterDeclaration, true /*includeSelf*/, ); @@ -112,12 +113,14 @@ function addCompletionByLookingBackwardNode( [SyntaxKind.ScalarStatement]: "scalarHeader", [SyntaxKind.OperationStatement]: "operationHeader", [SyntaxKind.InterfaceStatement]: "interfaceHeader", + [SyntaxKind.UnionStatement]: "unionHeader", }; switch (preNode?.kind) { case SyntaxKind.ModelStatement: case SyntaxKind.ScalarStatement: case SyntaxKind.OperationStatement: case SyntaxKind.InterfaceStatement: + case SyntaxKind.UnionStatement: const idEndPos = preNode.templateParametersRange.end >= 0 ? preNode.templateParametersRange.end @@ -195,6 +198,7 @@ interface KeywordArea { templateParameter?: boolean; operationHeader?: boolean; interfaceHeader?: boolean; + unionHeader?: boolean; } const keywords = [ @@ -218,7 +222,13 @@ const keywords = [ // On model `model Foo ...` [ "extends", - { modelHeader: true, scalarHeader: true, templateParameter: true, interfaceHeader: true }, + { + modelHeader: true, + scalarHeader: true, + templateParameter: true, + interfaceHeader: true, + unionHeader: true, + }, ], ["is", { modelHeader: true, operationHeader: true }], diff --git a/packages/compiler/src/server/tmlanguage.ts b/packages/compiler/src/server/tmlanguage.ts index 92b268fef58..f766da9433b 100644 --- a/packages/compiler/src/server/tmlanguage.ts +++ b/packages/compiler/src/server/tmlanguage.ts @@ -695,6 +695,17 @@ const unionBody: BeginEndRule = { patterns: [namedUnionVariant, token, directive, decorator, expression, punctuationComma], }; +const unionExtends: BeginEndRule = { + key: "union-extends", + scope: meta, + begin: "\\b(extends)\\b", + beginCaptures: { + "1": { scope: "keyword.other.tsp" }, + }, + end: `((?=\\{)|${universalEndExceptComma})`, + patterns: [expression, punctuationComma], +}; + const unionStatement: BeginEndRule = { key: "union-statement", scope: meta, @@ -705,7 +716,12 @@ const unionStatement: BeginEndRule = { "3": { scope: "entity.name.type.tsp" }, }, end: `(?<=\\})|${universalEnd}`, - patterns: [token, unionBody], + patterns: [ + token, + typeParameters, + unionExtends, // before unionBody or `extends` will look like a type name + unionBody, + ], }; const aliasAssignment: BeginEndRule = { diff --git a/packages/compiler/test/checker/union.test.ts b/packages/compiler/test/checker/union.test.ts index db34d307aaf..d756682db4d 100644 --- a/packages/compiler/test/checker/union.test.ts +++ b/packages/compiler/test/checker/union.test.ts @@ -1,7 +1,13 @@ import { ok, strictEqual } from "assert"; import { describe, it } from "vitest"; -import type { Model, Union, UnionVariant } from "../../src/core/types.js"; -import { expectTypeEquals, mockFile, t } from "../../src/testing/index.js"; +import type { Model, Scalar, Union, UnionVariant } from "../../src/core/types.js"; +import { + expectDiagnosticEmpty, + expectDiagnostics, + expectTypeEquals, + mockFile, + t, +} from "../../src/testing/index.js"; import { Tester } from "../tester.js"; describe("declarations", () => { @@ -72,6 +78,429 @@ describe("declarations", () => { }); }); +describe("extends", () => { + it("sets baseType on the union type", async () => { + const { Pet, PetBase } = await Tester.compile(t.code` + model ${t.model("PetBase")} { name: string } + model Cat extends PetBase { toy: string } + model Dog extends PetBase { food: string } + + union ${t.union("Pet")} extends PetBase { + cat: Cat, + dog: Dog, + } + `); + + expectTypeEquals(Pet.baseType, PetBase); + }); + + it("baseType is undefined when there is no extends clause", async () => { + const { Pet } = await Tester.compile(t.code` + model Cat { name: string } + union ${t.union("Pet")} { cat: Cat } + `); + + strictEqual(Pet.baseType, undefined); + }); + + it("does not add the base type as a variant", async () => { + const { Pet } = await Tester.compile(t.code` + model PetBase { name: string } + model Cat extends PetBase { toy: string } + + union ${t.union("Pet")} extends PetBase { cat: Cat } + `); + + strictEqual(Pet.variants.size, 1); + ok(Pet.variants.has("cat")); + }); + + it("accepts variants that structurally satisfy the base type without extending it", async () => { + // Per the design, `extends` is an assignability constraint, not a nominal one. + const diagnostics = await Tester.diagnose(` + model PetBase { name: string } + model Cat { name: string, toy: string } + model Dog extends PetBase { food: string } + + union Pet extends PetBase { + cat: Cat, + dog: Dog, + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("accepts the base type itself as a variant", async () => { + const diagnostics = await Tester.diagnose(` + model PetBase { name: string } + model Cat extends PetBase { toy: string } + + union Pet extends PetBase { + cat: Cat, + base: PetBase, + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("emits a diagnostic on the variant that doesn't satisfy the constraint", async () => { + const diagnostics = await Tester.diagnose(` + model PetBase { name: string } + model Cat extends PetBase { toy: string } + model Rock { hardness: int32 } + + union Pet extends PetBase { + cat: Cat, + rock: Rock, + } + `); + expectDiagnostics(diagnostics, { + code: "unassignable", + message: /Type 'Rock' is not assignable to type 'PetBase'/, + }); + }); + + it("emits one diagnostic per offending variant", async () => { + const diagnostics = await Tester.diagnose(` + model PetBase { name: string } + model Rock { hardness: int32 } + model Tree { height: int32 } + + union Pet extends PetBase { + rock: Rock, + tree: Tree, + } + `); + expectDiagnostics(diagnostics, [ + { code: "unassignable", message: /Type 'Rock' is not assignable to type 'PetBase'/ }, + { code: "unassignable", message: /Type 'Tree' is not assignable to type 'PetBase'/ }, + ]); + }); + + it("works with unnamed variants", async () => { + const diagnostics = await Tester.diagnose(` + union Status extends string { + "start", + "stop", + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("emits a diagnostic for an unnamed variant that doesn't satisfy the constraint", async () => { + const diagnostics = await Tester.diagnose(` + union Status extends string { + "start", + 123, + } + `); + expectDiagnostics(diagnostics, { + code: "unassignable", + message: "Type '123' is not assignable to type 'string'", + }); + }); + + it("supports composing unions declared with extends", async () => { + const diagnostics = await Tester.diagnose(` + union OperationStatus extends string { + "Running", + "Succeeded", + "Failure", + } + + union ServiceOperationStatus extends string { + OperationStatus, + "NotStarted", + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("supports a scalar base type", async () => { + const { Status, string: stringType } = await Tester.compile(t.code` + union ${t.union("Status")} extends ${t.scalar("string")} { + "start", + "stop", + } + `); + expectTypeEquals(Status.baseType, stringType); + }); + + it("supports a union expression as the base type", async () => { + const { Foo } = await Tester.compile(t.code` + union ${t.union("Foo")} extends string | int32 { + a: string, + b: int32, + } + `); + strictEqual(Foo.baseType?.kind, "Union"); + }); + + it("emits a diagnostic when a variant doesn't satisfy a union expression base type", async () => { + const diagnostics = await Tester.diagnose(` + union Foo extends string | int32 { + a: string, + b: boolean, + } + `); + expectDiagnostics(diagnostics, { + code: "unassignable", + message: "Type 'boolean' is not assignable to type 'string | int32'", + }); + }); + + it("supports an intersection as the base type", async () => { + const diagnostics = await Tester.diagnose(` + model A { a: string } + model B { b: string } + model AB { a: string, b: string, c: string } + + union Foo extends A & B { + ab: AB, + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("supports a templated base type reference", async () => { + const diagnostics = await Tester.diagnose(` + model Wrapper { value: T } + + union Foo extends Wrapper { + a: Wrapper, + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + describe("templates", () => { + it("checks the constraint on instantiation", async () => { + const diagnostics = await Tester.diagnose(` + union Foo extends string { + value: T, + } + + alias Bad = Foo; + `); + expectDiagnostics(diagnostics, { + code: "unassignable", + message: "Type 'int32' is not assignable to type 'string'", + }); + }); + + it("does not report on a valid instantiation", async () => { + const diagnostics = await Tester.diagnose(` + union Foo extends string { + value: T, + } + + alias Good = Foo<"abc">; + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("does not report on the uninstantiated template declaration", async () => { + const diagnostics = await Tester.diagnose(` + union Foo extends string { + value: T, + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("supports a template parameter as the base type", async () => { + const { Foo, string: stringType } = await Tester.compile(t.code` + union Template extends T { + value: string, + } + + alias ${t.union("Foo")} = Template<${t.scalar("string")}>; + `); + expectTypeEquals(Foo.baseType, stringType); + }); + + it("emits a diagnostic when a variant doesn't satisfy a template parameter base type", async () => { + const diagnostics = await Tester.diagnose(` + union Template extends T { + value: string, + } + + alias Bad = Template; + `); + expectDiagnostics(diagnostics, { + code: "unassignable", + message: "Type 'string' is not assignable to type 'int32'", + }); + }); + }); + + describe("circular references", () => { + it("reports a diagnostic when a union extends itself", async () => { + const diagnostics = await Tester.diagnose(`union a extends a { x: string }`); + expectDiagnostics(diagnostics, { + code: "circular-base-type", + message: "Type 'a' recursively references itself as a base type.", + }); + }); + + it("reports a diagnostic when a union extends itself via another union", async () => { + const diagnostics = await Tester.diagnose(` + union a extends b { x: string } + union b extends a { x: string } + `); + expectDiagnostics(diagnostics, { + code: "circular-base-type", + message: "Type 'a' recursively references itself as a base type.", + }); + }); + + it("reports a diagnostic when a union extends itself via an alias", async () => { + const diagnostics = await Tester.diagnose(` + union a extends b { x: string } + alias b = a; + `); + expectDiagnostics(diagnostics, { + code: "circular-base-type", + message: "Type 'a' recursively references itself as a base type.", + }); + }); + + it("reports a diagnostic when a union references itself in a union expression", async () => { + const diagnostics = await Tester.diagnose(`union a extends a | string { x: string }`); + expectDiagnostics(diagnostics, { + code: "circular-base-type", + message: "Type 'a' recursively references itself as a base type.", + }); + }); + + it("reports a diagnostic when a union references itself in a nested union expression", async () => { + const diagnostics = await Tester.diagnose( + `union a extends string | (int32 | a) { x: string }`, + ); + expectDiagnostics(diagnostics, { + code: "circular-base-type", + message: "Type 'a' recursively references itself as a base type.", + }); + }); + + it("reports a diagnostic when a union references itself in a union expression via an alias", async () => { + const diagnostics = await Tester.diagnose(` + union a extends b { x: string } + alias b = a | string; + `); + expectDiagnostics(diagnostics, { + code: "circular-base-type", + message: "Type 'a' recursively references itself as a base type.", + }); + }); + + it("doesn't set a base type when a circular reference is reported", async () => { + const [{ a }, diagnostics] = await Tester.compileAndDiagnose( + t.code`union ${t.union("a")} extends a | string { x: string }`, + ); + expectDiagnostics(diagnostics, { code: "circular-base-type" }); + strictEqual(a.baseType, undefined); + }); + + it("allows a union to reference itself from a model reachable from the base type", async () => { + // Cyclic type graphs are legal in TypeSpec (e.g. `model Foo { foo: Foo }`) so this must + // not be reported as a circular base type. + const [{ a }, diagnostics] = await Tester.compileAndDiagnose(t.code` + model Box { inner: a } + union ${t.union("a")} extends Box { x: Box } + `); + expectDiagnosticEmpty(diagnostics); + strictEqual(a.baseType?.kind, "Model"); + }); + }); + + it("doesn't cascade errors when the base type cannot be resolved", async () => { + const diagnostics = await Tester.diagnose(` + union Foo extends NotDefined { + a: string, + } + `); + expectDiagnostics(diagnostics, { + code: "invalid-ref", + message: "Unknown identifier NotDefined", + }); + }); + + it("reports a diagnostic when the base type is a value", async () => { + const diagnostics = await Tester.diagnose(` + union Foo extends #{ a: 1 } { + a: string, + } + `); + expectDiagnostics(diagnostics, [{ code: "value-in-type" }]); + }); + + describe("deprecation", () => { + it("reports the deprecation of the base type", async () => { + const diagnostics = await Tester.diagnose(` + #deprecated "Use NewBase instead" + model Base {} + + union Foo extends Base { + a: {}, + } + `); + expectDiagnostics(diagnostics, [ + { code: "deprecated", message: "Deprecated: Use NewBase instead" }, + ]); + }); + + it("doesn't report the deprecation of the base type when the union is deprecated", async () => { + // Same mitigation as `model Foo extends Base`: a deprecated declaration is allowed to + // reference deprecated types without adding noise. + const diagnostics = await Tester.diagnose(` + #deprecated "Use NewBase instead" + model Base {} + + #deprecated "Use NewFoo instead" + union Foo extends Base { + a: {}, + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("doesn't copy the deprecation of the base type onto the union", async () => { + // `extends` on a union is a constraint, not inheritance, so the deprecation must not + // propagate to the union the way it does for `scalar`. + const diagnostics = await Tester.diagnose(` + #deprecated "Use NewBase instead" + model Base {} + + union Foo extends Base { + a: {}, + } + + model Usage { + foo: Foo, + } + `); + expectDiagnostics(diagnostics, [ + { code: "deprecated", message: "Deprecated: Use NewBase instead" }, + ]); + }); + }); + + it("keeps a per-instantiation base type", async () => { + const { Foo, Bar } = await Tester.compile(t.code` + union Template extends T { + value: T, + } + + alias ${t.union("Foo")} = Template; + alias ${t.union("Bar")} = Template; + `); + + strictEqual((Foo.baseType as Scalar).name, "string"); + strictEqual((Bar.baseType as Scalar).name, "int32"); + }); +}); + describe("expressions", () => { it("reduces union expressions and gives them symbol keys", async () => { const { Foo } = await Tester.compile(t.code` diff --git a/packages/compiler/test/experimental/mutator.test.ts b/packages/compiler/test/experimental/mutator.test.ts index 7302674365a..30849334f96 100644 --- a/packages/compiler/test/experimental/mutator.test.ts +++ b/packages/compiler/test/experimental/mutator.test.ts @@ -490,6 +490,18 @@ describe("global graph mutation", () => { const barProp: any = MutatedB.properties.get("bar"); expectTypeEquals(MutatedA, barProp.type.values[0]); }); + + it("mutate union base type", async () => { + const type = await globalMutate(` + model PetBase { name: string } + model Cat extends PetBase { toy: string } + union Pet extends PetBase { cat: Cat }; + `); + + const MutatedPetBase = type.models.get("PetBase")!; + const MutatedPet = type.unions.get("Pet")!; + expectTypeEquals(MutatedPet.baseType!, MutatedPetBase); + }); }); describe("decorators", () => { diff --git a/packages/compiler/test/formatter/formatter.test.ts b/packages/compiler/test/formatter/formatter.test.ts index dce7cb8fe2c..7d18a5f3eec 100644 --- a/packages/compiler/test/formatter/formatter.test.ts +++ b/packages/compiler/test/formatter/formatter.test.ts @@ -1975,6 +1975,109 @@ union Foo { }); }); + describe("extends", () => { + it("formats a union extends clause", async () => { + await assertFormat({ + code: ` +union Foo extends Bar { a: A, b: B} +`, + expected: ` +union Foo extends Bar { + a: A, + b: B, +} +`, + }); + }); + + it("formats a templated union with an extends clause", async () => { + await assertFormat({ + code: ` +union Foo extends Bar { a: T} +`, + expected: ` +union Foo extends Bar { + a: T, +} +`, + }); + }); + + it("formats decorators and modifiers with an extends clause", async () => { + await assertFormat({ + code: ` +@doc("foo") internal union Foo extends string { "a", "b"} +`, + expected: ` +@doc("foo") +internal union Foo extends string { + "a", + "b", +} +`, + }); + }); + + it("breaks a long extends clause onto a new line", async () => { + await assertFormat({ + code: ` +union ThisIsAVeryLongUnionName extends ThisIsAVeryLongBaseTypeName { a: A } +`, + expected: ` +union ThisIsAVeryLongUnionName + extends ThisIsAVeryLongBaseTypeName { + a: A, +} +`, + }); + }); + + it("keeps a dangling comment inside an empty union body with an extends clause", async () => { + await assertFormat({ + code: ` +union Foo extends Bar { + // dangling +} +`, + expected: ` +union Foo extends Bar { + // dangling +} +`, + }); + }); + + it("keeps a comment between the base type and the body", async () => { + await assertFormat({ + code: ` +union Foo +// before extends +extends Bar {} +`, + expected: ` +union Foo extends Bar { + // before extends +} +`, + }); + }); + }); + + it("keeps a dangling comment inside an empty union body", async () => { + await assertFormat({ + code: ` +union Foo { + // dangling +} +`, + expected: ` +union Foo { + // dangling +} +`, + }); + }); + // Regression test for https://github.com/microsoft/typespec/issues/11009 it("does not add a blank line or extra indent for a union used as a template argument", async () => { await assertFormat({ diff --git a/packages/compiler/test/formatter/scenarios/inputs/union.tsp b/packages/compiler/test/formatter/scenarios/inputs/union.tsp index ca0d82c8a8c..59c1227a6d4 100644 --- a/packages/compiler/test/formatter/scenarios/inputs/union.tsp +++ b/packages/compiler/test/formatter/scenarios/inputs/union.tsp @@ -1,2 +1,9 @@ union Foo { x: int32 } + +union Constrained extends + Base { a: A, b: B } + +union Empty extends Base { + // dangling +} \ No newline at end of file diff --git a/packages/compiler/test/formatter/scenarios/outputs/union.tsp b/packages/compiler/test/formatter/scenarios/outputs/union.tsp index 9b5593c8231..487b9d5dfab 100644 --- a/packages/compiler/test/formatter/scenarios/outputs/union.tsp +++ b/packages/compiler/test/formatter/scenarios/outputs/union.tsp @@ -1,3 +1,12 @@ union Foo { x: int32, } + +union Constrained extends Base { + a: A, + b: B, +} + +union Empty extends Base { + // dangling +} diff --git a/packages/compiler/test/parser.test.ts b/packages/compiler/test/parser.test.ts index 2d3157811a4..70762b3aba2 100644 --- a/packages/compiler/test/parser.test.ts +++ b/packages/compiler/test/parser.test.ts @@ -270,8 +270,23 @@ describe("union declarations", () => { `union A { string, int32 }`, `union A { B, C }`, `union A { "hi", \`bye\` }`, + "union A extends B { x: C }", + "union A extends B { x: C }", + "union A extends B | C { x: B }", + "union A extends B & C { x: B }", + "union A extends string { `hi` }", + "union A extends B[] { x: B[] }", + "union A extends { name: string } { x: B }", + "union A extends T { x: T }", + "union A extends string { x: T }", + "@myDec union A extends B { @myDec a: B }", + ]); + parseErrorEach([ + ['union A { @myDec "x" x: number, y: string }', [/';' expected/]], + ["union A extends { x: B }", [/'{' expected/]], + ["union A extends;", [/Expression expected/]], + ["union A is B { x: B }", [/'{', or 'extends' expected/]], ]); - parseErrorEach([['union A { @myDec "x" x: number, y: string }', [/';' expected/]]]); }); describe("const statements", () => { diff --git a/packages/compiler/test/semantic-walker.test.ts b/packages/compiler/test/semantic-walker.test.ts index 2e2e3cadd6d..7a78c438d4a 100644 --- a/packages/compiler/test/semantic-walker.test.ts +++ b/packages/compiler/test/semantic-walker.test.ts @@ -504,6 +504,24 @@ it("finds unions", async () => { strictEqual(result.unionVariants[0].name!, "x"); }); +it("navigates the base type of a union declared with extends", async () => { + const result = await runNavigator(` + model PetBase { name: string } + model Cat extends PetBase { toy: string } + + union Pet extends PetBase { + cat: Cat; + } + `); + + strictEqual(result.unions.length, 1); + strictEqual(result.unions[0].name!, "Pet"); + ok( + result.models.some((x) => x.name === "PetBase"), + "PetBase should be navigated as the base type of the union", + ); +}); + it("finds tuples", async () => { const result = await runNavigator(` model ContainsTuple { diff --git a/packages/compiler/test/server/colorization.test.ts b/packages/compiler/test/server/colorization.test.ts index 374d563fd7e..5a56a804a67 100644 --- a/packages/compiler/test/server/colorization.test.ts +++ b/packages/compiler/test/server/colorization.test.ts @@ -1091,6 +1091,41 @@ function testColorization(description: string, tokenize: Tokenize) { Token.punctuation.closeBrace, ]); }); + + it("union with extends", async () => { + const tokens = await tokenize("union Foo extends Bar { a: A }"); + deepStrictEqual(tokens, [ + Token.keywords.union, + Token.identifiers.type("Foo"), + Token.keywords.extends, + Token.identifiers.type("Bar"), + Token.punctuation.openBrace, + Token.identifiers.variable("a"), + Token.operators.typeAnnotation, + Token.identifiers.type("A"), + Token.punctuation.closeBrace, + ]); + }); + + it("templated union with extends", async () => { + const tokens = await tokenize("union Foo extends Bar { a: T }"); + deepStrictEqual(tokens, [ + Token.keywords.union, + Token.identifiers.type("Foo"), + Token.punctuation.typeParameters.begin, + Token.identifiers.type("T"), + Token.keywords.extends, + Token.identifiers.type("string"), + Token.punctuation.typeParameters.end, + Token.keywords.extends, + Token.identifiers.type("Bar"), + Token.punctuation.openBrace, + Token.identifiers.variable("a"), + Token.operators.typeAnnotation, + Token.identifiers.type("T"), + Token.punctuation.closeBrace, + ]); + }); }); describe("namespaces", () => { diff --git a/packages/compiler/test/server/completion.test.ts b/packages/compiler/test/server/completion.test.ts index 7ecfe5b1a18..99968bc08fb 100644 --- a/packages/compiler/test/server/completion.test.ts +++ b/packages/compiler/test/server/completion.test.ts @@ -140,6 +140,22 @@ describe("completes for keywords", () => { [`interface I {┆}`, []], [`interface I`, []], + [`union U ┆`, ["extends"]], + [`union U ┆ `, ["extends"]], + [`union U \n┆\n`, ["extends"]], + [`union U ┆{}`, ["extends"]], + [`union U ┆ {}`, ["extends"]], + [`union U ┆ \nscalar S2`, ["extends"]], + [`model M1{}; union U ┆ M1`, ["extends"]], + [`model M1{}; union U e┆x M1`, ["extends"]], + [`union U ┆\n`, ["extends"]], + [`union U┆ \n`, ["extends"]], + [`union U ┆ {}`, ["extends"]], + [`union U ex┆`, ["extends"]], + [`union U ex┆tends`, ["extends"]], + [`union U {┆}`, []], + [`union U {}`, []], + [`scalar S`, ["extends"]], [`scalar S`, ["extends"]], [`model M`, ["extends"]], @@ -1321,6 +1337,26 @@ describe("identifiers", () => { ]); }); + it("completes types in a union extends clause", async () => { + const completions = await complete( + ` + namespace N { + model A {} + union B extends ┆ + } + `, + ); + + check(completions, [ + { + label: "A", + insertText: "A", + kind: CompletionItemKind.Class, + documentation: { kind: MarkupKind.Markdown, value: "```typespec\nmodel N.A\n```" }, + }, + ]); + }); + it("completes using statements", async () => { const completions = await complete( ` diff --git a/packages/spec/src/spec.emu.html b/packages/spec/src/spec.emu.html index 5230dacc946..bc3a8e27a18 100644 --- a/packages/spec/src/spec.emu.html +++ b/packages/spec/src/spec.emu.html @@ -387,7 +387,10 @@

Syntactic Grammar

UnionStatement : - DirectiveList? DecoratorList? `union` Identifier TemplateParameters? `{` UnionBody? `}` + DirectiveList? DecoratorList? `union` Identifier TemplateParameters? UnionExtends? `{` UnionBody? `}` + +UnionExtends : + `extends` Expression UnionBody : UnionVariantList `;`? diff --git a/website/src/content/docs/docs/language-basics/unions.md b/website/src/content/docs/docs/language-basics/unions.md index 8a0bbfbbc84..13250a58a98 100644 --- a/website/src/content/docs/docs/language-basics/unions.md +++ b/website/src/content/docs/docs/language-basics/unions.md @@ -35,3 +35,74 @@ union Breed { ``` The above example is equivalent to the `Breed` alias mentioned earlier, with the difference that emitters can recognize `Breed` as a named entity and also identify the `beagle`, `shepherd`, and `retriever` names for the options. This format also allows the application of [decorators](./decorators.md) to each of the options. + +## Constraining a union with `extends` + +A named union can declare a base type with the `extends` keyword. Every variant of the union must be [assignable](./type-relations.md) to that base type, otherwise a diagnostic is reported on the offending variant. + +```typespec +model Dog { + name: string; +} +model Beagle extends Dog { + huntingSkill: string; +} +model GermanShepherd extends Dog { + guardingSkill: string; +} + +union Breed extends Dog { + beagle: Beagle, + shepherd: GermanShepherd, +} +``` + +This serves two purposes: + +- It prevents a common class of mistake where an unrelated type is accidentally added to a union. +- It records the common base type in the type graph, which makes it easy for emitters to represent the union with a polymorphic base type in languages that don't support unions natively. + +The base type does **not** become a variant of the union. `Breed` above still has exactly two variants. + +`extends` is a constraint, not a declaration of inheritance. A variant only needs to be assignable to the base type, it doesn't have to explicitly `extends` it: + +```typespec +model Dog { + name: string; +} +model Beagle { + name: string; + huntingSkill: string; +} + +// Ok: `Beagle` is assignable to `Dog` even though it doesn't explicitly extend it. +union Breed extends Dog { + beagle: Beagle, +} +``` + +Any type expression can be used as the base type, including scalars, unions and templates. + +```typespec +union OperationStatus extends string { + "Running", + "Succeeded", + "Failed", +} +``` + +:::caution +`extends` on a union does not mean the union is extensible. `union Foo extends string { "a", "b" }` and `union Foo { "a", "b" }` describe exactly the same set of values, and emitters should treat them the same way. To allow additional values, add a variant for them explicitly: + +```typespec +union OperationStatus extends string { + "Running", + "Succeeded", + "Failed", + string, +} +``` + +::: + +`extends` also has no interaction with the [`@discriminator`](../standard-library/built-in-decorators.md#@discriminator) decorator. From cf859a2d03cea0db7f78aff79d9808b0a3e0d480 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 27 Aug 2026 12:53:08 -0700 Subject: [PATCH 2/8] feat(compiler): add `@strictExtends` to require union variants to extend the base type The `extends` clause of a union is a structural constraint: any variant with a compatible shape satisfies it. Emitters targeting languages without native unions represent such a union with a polymorphic base type, which requires each variant to actually derive from the base type. `@strictExtends` turns that into a compile time error. It only adds a constraint when the base type is a model, since assignability between scalars is already nominal. A variant that is itself a union satisfies the constraint when all of its own variants do, so unions can still be composed. Implements the opt-in decorator proposed in #2737 and tracked by #3900. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4 --- .../changes/union-strict-extends-2026-8-26.md | 29 ++ packages/compiler/generated-defs/TypeSpec.ts | 34 +++ packages/compiler/lib/std/decorators.tsp | 31 +++ packages/compiler/src/core/messages.ts | 12 + packages/compiler/src/lib/decorators.ts | 78 +++++- packages/compiler/src/lib/tsp-index.ts | 2 + .../test/decorators/decorators.test.ts | 253 ++++++++++++++++++ .../docs/docs/language-basics/unions.md | 48 ++++ .../standard-library/built-in-decorators.md | 41 +++ 9 files changed, 527 insertions(+), 1 deletion(-) create mode 100644 .chronus/changes/union-strict-extends-2026-8-26.md diff --git a/.chronus/changes/union-strict-extends-2026-8-26.md b/.chronus/changes/union-strict-extends-2026-8-26.md new file mode 100644 index 00000000000..75e58deb866 --- /dev/null +++ b/.chronus/changes/union-strict-extends-2026-8-26.md @@ -0,0 +1,29 @@ +--- +changeKind: feature +packages: + - "@typespec/compiler" +--- + +Add `@strictExtends` to require every variant of a union to explicitly extend the base type declared by the union `extends` clause. + +By default the `extends` clause of a union is a structural constraint: any variant with a compatible shape satisfies it. Emitters targeting languages without native unions represent such a union with a polymorphic base type, which requires each variant to actually derive from the base type. + +```tsp +model Pet { + name: string; +} +model Cat extends Pet { + meow: boolean; +} +model Rock { + name: string; +} + +@strictExtends +union Pets extends Pet { + cat: Cat, // ok: `Cat` extends `Pet` + rock: Rock, // error: `Rock` has the same shape as `Pet` but doesn't extend it +} +``` + +`@strictExtends` only adds a constraint when the base type is a model: assignability between scalars is already nominal in TypeSpec. A variant that is itself a union satisfies the constraint when all of its own variants do, so unions can still be composed. diff --git a/packages/compiler/generated-defs/TypeSpec.ts b/packages/compiler/generated-defs/TypeSpec.ts index 0fa7f3838e6..5a0af1b1f6a 100644 --- a/packages/compiler/generated-defs/TypeSpec.ts +++ b/packages/compiler/generated-defs/TypeSpec.ts @@ -632,6 +632,39 @@ export type DiscriminatedDecorator = ( options?: DiscriminatedOptions, ) => DecoratorValidatorCallbacks | void; +/** + * Require every variant of a union to explicitly extend the base type declared by the union + * `extends` clause. + * + * By default a union `extends` clause is a structural constraint: any variant with a compatible + * shape satisfies it. Emitters targeting languages without native unions represent such a union + * with a polymorphic base type, which requires each variant to actually derive from that base + * type. `@strictExtends` turns that requirement into a compile time error. + * + * This only adds a constraint when the base type is a model: assignability between scalars is + * already nominal in TypeSpec so nothing needs to be enforced for them. + * + * A variant that is itself a union satisfies the constraint when all of its own variants do, + * which allows composing unions. + * + * @example + * ```typespec + * model Pet {} + * model Cat extends Pet {} + * model Rock {} + * + * @strictExtends + * union Pets extends Pet { + * cat: Cat, // ok: `Cat` extends `Pet` + * rock: Rock, // error: `Rock` has the same shape as `Pet` but doesn't extend it + * } + * ``` + */ +export type StrictExtendsDecorator = ( + context: DecoratorContext, + target: Union, +) => DecoratorValidatorCallbacks | void; + /** * Specify the property to be used to discriminate this type. * @@ -1191,6 +1224,7 @@ export type TypeSpecDecorators = { overload: OverloadDecorator; encodedName: EncodedNameDecorator; discriminated: DiscriminatedDecorator; + strictExtends: StrictExtendsDecorator; discriminator: DiscriminatorDecorator; example: ExampleDecorator; opExample: OpExampleDecorator; diff --git a/packages/compiler/lib/std/decorators.tsp b/packages/compiler/lib/std/decorators.tsp index fed9f685237..a639c29f15d 100644 --- a/packages/compiler/lib/std/decorators.tsp +++ b/packages/compiler/lib/std/decorators.tsp @@ -453,6 +453,37 @@ model DiscriminatedOptions { */ extern dec discriminated(target: Union, options?: valueof DiscriminatedOptions); +/** + * Require every variant of a union to explicitly extend the base type declared by the union + * `extends` clause. + * + * By default a union `extends` clause is a structural constraint: any variant with a compatible + * shape satisfies it. Emitters targeting languages without native unions represent such a union + * with a polymorphic base type, which requires each variant to actually derive from that base + * type. `@strictExtends` turns that requirement into a compile time error. + * + * This only adds a constraint when the base type is a model: assignability between scalars is + * already nominal in TypeSpec so nothing needs to be enforced for them. + * + * A variant that is itself a union satisfies the constraint when all of its own variants do, + * which allows composing unions. + * + * @example + * + * ```typespec + * model Pet {} + * model Cat extends Pet {} + * model Rock {} + * + * @strictExtends + * union Pets extends Pet { + * cat: Cat, // ok: `Cat` extends `Pet` + * rock: Rock, // error: `Rock` has the same shape as `Pet` but doesn't extend it + * } + * ``` + */ +extern dec strictExtends(target: Union); + /** * Specify the property to be used to discriminate this type. * @param propertyName The property name to use for discrimination diff --git a/packages/compiler/src/core/messages.ts b/packages/compiler/src/core/messages.ts index c91a2528634..78712d9d345 100644 --- a/packages/compiler/src/core/messages.ts +++ b/packages/compiler/src/core/messages.ts @@ -536,6 +536,18 @@ const diagnostics = { default: paramMessage`Union already has a variant named ${"name"}`, }, }, + "strict-extends-no-base-type": { + severity: "error", + messages: { + default: "@strictExtends can only be used on a union declaring a base type with `extends`.", + }, + }, + "strict-extends-variant": { + severity: "error", + messages: { + default: paramMessage`Variant of type '${"variantType"}' must explicitly extend '${"baseType"}' as required by @strictExtends.`, + }, + }, "enum-member-duplicate": { severity: "error", messages: { diff --git a/packages/compiler/src/lib/decorators.ts b/packages/compiler/src/lib/decorators.ts index 3d7f4ba788a..fa9406d3f5a 100644 --- a/packages/compiler/src/lib/decorators.ts +++ b/packages/compiler/src/lib/decorators.ts @@ -27,6 +27,7 @@ import type { PatternDecorator, ReturnsDocDecorator, SecretDecorator, + StrictExtendsDecorator, SummaryDecorator, TagDecorator, WithOptionalPropertiesDecorator, @@ -71,7 +72,7 @@ import { parseMimeType } from "../core/mime-type.js"; import type { Numeric } from "../core/numeric.js"; import { isNumeric } from "../core/numeric.js"; import type { Program } from "../core/program.js"; -import { isArrayModelType, isValue } from "../core/type-utils.js"; +import { isArrayModelType, isErrorType, isValue } from "../core/type-utils.js"; import type { AugmentDecoratorStatementNode, DecoratorContext, @@ -1328,6 +1329,81 @@ export const $discriminator: DiscriminatorDecorator = ( setDiscriminator(context.program, entity, { propertyName }); }; +// -- @strictExtends -------------------------------------------------------------------------- + +export const $strictExtends: StrictExtendsDecorator = ( + context: DecoratorContext, + entity: Union, +) => { + const baseType = entity.baseType; + if (baseType === undefined) { + reportDiagnostic(context.program, { + code: "strict-extends-no-base-type", + target: context.decoratorTarget, + }); + return; + } + + // Only a model base type needs the extra check: assignability between scalars is already + // nominal in TypeSpec, so an unrelated scalar can never satisfy the `extends` clause anyway. + if (baseType.kind !== "Model") { + return; + } + + for (const variant of entity.variants.values()) { + if (isErrorType(variant.type)) { + continue; + } + if (!derivesFromModel(variant.type, baseType, new Set())) { + reportDiagnostic(context.program, { + code: "strict-extends-variant", + format: { + variantType: getTypeName(variant.type), + baseType: getTypeName(baseType), + }, + target: variant.node ?? entity, + }); + } + } +}; + +/** + * Check whether `type` explicitly derives from `baseType` through `extends` declarations. + * + * A union variant that is itself a union derives from the base type when every one of its own + * variants does, which is what makes composing unions work. `path` guards against a union + * reaching itself, which is a legal (if unsatisfiable here) type graph. + */ +function derivesFromModel(type: Type, baseType: Model, path: Set): boolean { + switch (type.kind) { + case "Model": + for (let current: Model | undefined = type; current; current = current.baseModel) { + if (current === baseType) { + return true; + } + } + return false; + case "Union": { + if (path.has(type) || type.variants.size === 0) { + return false; + } + path.add(type); + try { + for (const variant of type.variants.values()) { + if (!derivesFromModel(variant.type, baseType, path)) { + return false; + } + } + return true; + } finally { + path.delete(type); + } + } + default: + return false; + } +} + export interface Example extends ExampleOptions { readonly value: Value; } diff --git a/packages/compiler/src/lib/tsp-index.ts b/packages/compiler/src/lib/tsp-index.ts index 0d9c9e0684e..bfbf46fe060 100644 --- a/packages/compiler/src/lib/tsp-index.ts +++ b/packages/compiler/src/lib/tsp-index.ts @@ -27,6 +27,7 @@ import { $returnsDoc, $secret, $service, + $strictExtends, $summary, $tag, $withOptionalProperties, @@ -105,6 +106,7 @@ export const $decorators = { encodedName: $encodedName, discriminated: discriminatedDecorator, discriminator: $discriminator, + strictExtends: $strictExtends, example: $example, opExample: $opExample, inspectType: $inspectType, diff --git a/packages/compiler/test/decorators/decorators.test.ts b/packages/compiler/test/decorators/decorators.test.ts index b7a72b91a68..1312a57b417 100644 --- a/packages/compiler/test/decorators/decorators.test.ts +++ b/packages/compiler/test/decorators/decorators.test.ts @@ -1308,6 +1308,259 @@ describe("@discriminated", () => { }); }); +describe("@strictExtends", () => { + it("emit error if the union has no `extends` clause", async () => { + const diagnostics = await Tester.diagnose(` + @strictExtends + union Pets {} + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-no-base-type", + message: "@strictExtends can only be used on a union declaring a base type with `extends`.", + }); + }); + + it("accepts variants extending the base type", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + model Dog extends Pet { bark: boolean } + + @strictExtends + union Pets extends Pet { + cat: Cat, + dog: Dog, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("accepts the base type itself as a variant", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + + @strictExtends + union Pets extends Pet { + pet: Pet, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("accepts variants extending the base type transitively", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + model Tiger extends Cat { stripes: int32 } + + @strictExtends + union Pets extends Pet { + tiger: Tiger, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("emit error for a variant that only structurally satisfies the base type", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Rock { name: string } + + @strictExtends + union Pets extends Pet { + rock: Rock, + } + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-variant", + message: "Variant of type 'Rock' must explicitly extend 'Pet' as required by @strictExtends.", + }); + }); + + it("emit error for an anonymous model variant", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + + @strictExtends + union Pets extends Pet { + rock: { name: string }, + } + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-variant", + }); + }); + + it("reports every offending variant", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + model Rock { name: string } + model Tree { name: string } + + @strictExtends + union Pets extends Pet { + cat: Cat, + rock: Rock, + tree: Tree, + } + `); + + expectDiagnostics(diagnostics, [ + { code: "strict-extends-variant" }, + { code: "strict-extends-variant" }, + ]); + }); + + it("accepts a union variant when all of its own variants extend the base type", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + model Tiger extends Cat { stripes: int32 } + model Dog extends Pet { bark: boolean } + + union Cats extends Cat { + cat: Cat, + tiger: Tiger, + } + + @strictExtends + union Pets extends Pet { + Cats, + dog: Dog, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("emit error when a union variant contains a type not extending the base type", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + model Rock { name: string } + + union Others { + cat: Cat, + rock: Rock, + } + + @strictExtends + union Pets extends Pet { + Others, + } + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-variant", + }); + }); + + it("emit error for an empty union variant", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + + union Empty {} + + @strictExtends + union Pets extends Pet { + Empty, + } + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-variant", + }); + }); + + it("accepts the same union appearing as multiple variants", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + + union Cats extends Pet { + cat: Cat, + } + + @strictExtends + union Pets extends Pet { + a: Cats, + b: Cats, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("adds no constraint when the base type is a scalar", async () => { + const diagnostics = await Tester.diagnose(` + scalar myString extends string; + + @strictExtends + union Values extends string { + a: myString, + b: "literal", + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("is not applied to an uninstantiated template declaration", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + + @strictExtends + union Pets extends Pet { + value: T, + } + + alias Instance = Pets; + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("emit error on a template instantiation with an offending argument", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Rock { name: string } + + @strictExtends + union Pets extends Pet { + value: T, + } + + alias Instance = Pets; + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-variant", + }); + }); + + it("reports both diagnostics for a variant that also fails the base type constraint", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Rock { size: int32 } + + @strictExtends + union Pets extends Pet { + rock: Rock, + } + `); + + expectDiagnostics(diagnostics, [{ code: "unassignable" }, { code: "strict-extends-variant" }]); + }); +}); + describe("@encodedName", () => { it("emit error if passing invalid mime type", async () => { const diagnostics = await Tester.diagnose(` diff --git a/website/src/content/docs/docs/language-basics/unions.md b/website/src/content/docs/docs/language-basics/unions.md index 13250a58a98..1ec59807182 100644 --- a/website/src/content/docs/docs/language-basics/unions.md +++ b/website/src/content/docs/docs/language-basics/unions.md @@ -81,6 +81,54 @@ union Breed extends Dog { } ``` +### Requiring explicit inheritance with `@strictExtends` + +Emitters targeting languages without native unions represent a union with an `extends` clause as a polymorphic base type. That representation requires every variant to actually derive from the base type, which structural assignability alone doesn't guarantee. + +Apply [`@strictExtends`](../standard-library/built-in-decorators.md#@strictExtends) to turn that requirement into a compile time error: + +```typespec +model Dog { + name: string; +} +model Beagle { + name: string; + huntingSkill: string; +} + +@strictExtends +union Breed extends Dog { + beagle: Beagle, // error: `Beagle` is assignable to `Dog` but doesn't extend it. +} +``` + +`@strictExtends` only adds a constraint when the base type is a model: assignability between scalars is already nominal in TypeSpec, so `@strictExtends` on `union Foo extends string {...}` changes nothing. + +A variant that is itself a union satisfies the constraint when all of its own variants do, so unions can still be composed: + +```typespec +model Pet { + name: string; +} +model Cat extends Pet { + meow: boolean; +} +model Dog extends Pet { + bark: boolean; +} + +union Cats extends Pet { + cat: Cat, +} + +// Ok: every variant of `Cats` extends `Pet`. +@strictExtends +union Pets extends Pet { + Cats, + dog: Dog, +} +``` + Any type expression can be used as the base type, including scalars, unions and templates. ```typespec diff --git a/website/src/content/docs/docs/standard-library/built-in-decorators.md b/website/src/content/docs/docs/standard-library/built-in-decorators.md index 220f260e175..713244b75be 100644 --- a/website/src/content/docs/docs/standard-library/built-in-decorators.md +++ b/website/src/content/docs/docs/standard-library/built-in-decorators.md @@ -1198,6 +1198,47 @@ namespace PetStore; ``` +### `@strictExtends` {#@strictExtends} + +Require every variant of a union to explicitly extend the base type declared by the union +`extends` clause. + +By default a union `extends` clause is a structural constraint: any variant with a compatible +shape satisfies it. Emitters targeting languages without native unions represent such a union +with a polymorphic base type, which requires each variant to actually derive from that base +type. `@strictExtends` turns that requirement into a compile time error. + +This only adds a constraint when the base type is a model: assignability between scalars is +already nominal in TypeSpec so nothing needs to be enforced for them. + +A variant that is itself a union satisfies the constraint when all of its own variants do, +which allows composing unions. +```typespec +@strictExtends +``` + +#### Target + +`Union` + +#### Parameters +None + +#### Examples + +```typespec +model Pet {} +model Cat extends Pet {} +model Rock {} + +@strictExtends +union Pets extends Pet { + cat: Cat, // ok: `Cat` extends `Pet` + rock: Rock, // error: `Rock` has the same shape as `Pet` but doesn't extend it +} +``` + + ### `@summary` {#@summary} Typically a short, single-line description. From 358fc120d2e74b202ea17ab8b16179c3a38c4ea1 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 27 Aug 2026 14:15:58 -0700 Subject: [PATCH 3/8] fix(compiler): fix stack overflow when checking assignability of recursive types `isTypeAssignableToInternal` created a brand new relation cache for every nested call instead of forwarding the one it was given, so the "in progress" entry seeded by `areModelsRelated` only survived a single level and mutually recursive models recursed forever. Unions were never seeded at all, so any union reaching itself did the same. Forwarding the cache alone is not enough: the cache stored only the `Related` result and dropped the errors, and `areModelsRelated` turns a result with no errors back into `Related.true`. The cache now stores the errors alongside the result. A purely cyclic union describes an empty set of values, so it is vacuously assignable to anything, and the dual seed is used on the target side where being assignable to a union only requires one variant to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4 --- ...ix-relation-checker-recursion-2026-8-26.md | 14 ++ .../src/core/type-relation-checker.ts | 80 +++++++---- .../compiler/test/checker/relation.test.ts | 136 ++++++++++++++++++ 3 files changed, 204 insertions(+), 26 deletions(-) create mode 100644 .chronus/changes/fix-relation-checker-recursion-2026-8-26.md diff --git a/.chronus/changes/fix-relation-checker-recursion-2026-8-26.md b/.chronus/changes/fix-relation-checker-recursion-2026-8-26.md new file mode 100644 index 00000000000..8cf511c9563 --- /dev/null +++ b/.chronus/changes/fix-relation-checker-recursion-2026-8-26.md @@ -0,0 +1,14 @@ +--- +changeKind: fix +packages: + - "@typespec/compiler" +--- + +Fix a stack overflow when checking assignability of mutually recursive types + +Checking whether a type was assignable to another one could recurse forever and crash the compiler with `RangeError: Maximum call stack size exceeded`. Two cases were affected: + +- mutually recursive models, such as `model A { b: B }` / `model B { a: A }` +- any union reaching itself, such as `union Foo { self: Foo }` + +The relation cache is now shared for the whole check instead of being recreated at every level, and unions seed it before walking their variants, so a cycle coming back to the same pair of types resolves instead of recursing. diff --git a/packages/compiler/src/core/type-relation-checker.ts b/packages/compiler/src/core/type-relation-checker.ts index e02e0c33c46..47c5e651bac 100644 --- a/packages/compiler/src/core/type-relation-checker.ts +++ b/packages/compiler/src/core/type-relation-checker.ts @@ -87,6 +87,28 @@ interface TypeRelationError { skipIfFirst?: boolean; } +/** + * Result of a relation check. + * + * The errors are cached together with the result: returning a cached `Related.false` without its + * errors would make callers like {@link areModelsRelated} see a failure with nothing to report and + * turn it back into a success. + */ +type RelationResult = [Related, readonly TypeRelationError[]]; + +/** + * Cache of the relation between two entities for the duration of a single top level check. + * + * It doubles as the guard that makes recursive types terminate: a relation is seeded with + * {@link Related.maybe} before its members are walked, so a cycle coming back to the same pair + * resolves optimistically instead of recursing forever. + */ +type RelationCache = MultiKeyMap<[Entity | IndeterminateEntity, Entity], RelationResult>; + +function createRelationCache(): RelationCache { + return new MultiKeyMap<[Entity | IndeterminateEntity, Entity], RelationResult>(); +} + /** * Mapping from the reflection models to Type["kind"] value */ @@ -130,7 +152,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source, target, diagnosticTarget, - new MultiKeyMap<[Entity, Entity], Related>(), + createRelationCache(), ); return [related === Related.true, convertErrorsToDiagnostics(errors, diagnosticTarget)]; } @@ -218,7 +240,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source, target, diagnosticTarget, - new MultiKeyMap<[Entity, Entity], Related>(), + createRelationCache(), ); return [related === Related.true, convertErrorsToDiagnostics(errors, diagnosticTarget)]; } @@ -227,27 +249,22 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Entity | IndeterminateEntity, target: Entity, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity | IndeterminateEntity, Entity], Related>, - ): [Related, readonly TypeRelationError[]] { + relationCache: RelationCache, + ): RelationResult { const cached = relationCache.get([source, target]); if (cached !== undefined) { - return [cached, []]; + return cached; } - const [result, diagnostics] = isTypeAssignableToWorker( - source, - target, - diagnosticTarget, - new MultiKeyMap<[Entity, Entity], Related>(), - ); + const result = isTypeAssignableToWorker(source, target, diagnosticTarget, relationCache); relationCache.set([source, target], result); - return [result, diagnostics]; + return result; } function isTypeAssignableToWorker( source: Entity | IndeterminateEntity, target: Entity, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { if ( "kind" in source && @@ -298,6 +315,9 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T } if (source.kind === "Union") { + // Seed the relation before walking the variants: a union can reach itself + // (`union Foo { self: Foo }`) and would otherwise recurse forever. + relationCache.set([source, target], [Related.maybe, []]); for (const variant of source.variants.values()) { const [variantAssignable] = isTypeAssignableToInternal( variant.type, @@ -366,7 +386,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T indeterminate: IndeterminateEntity, target: Type | MixedParameterConstraint, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { const [typeRelated, typeDiagnostics] = isTypeAssignableToInternal( indeterminate.type, @@ -398,7 +418,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Entity, target: Type, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { if (!isValue(source)) { return [Related.false, [createUnassignableDiagnostic(source, target, diagnosticTarget)]]; @@ -411,7 +431,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Entity, target: MixedParameterConstraint, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { if ("entityKind" in source && source.entityKind === "MixedParameterConstraint") { if (source.type && target.type) { @@ -471,7 +491,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Value, target: Type, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { return isTypeAssignableToInternal(source.type, target, diagnosticTarget, relationCache); } @@ -549,7 +569,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Type, target: FunctionType, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { if (source.kind !== "FunctionType") { return [Related.false, [createUnassignableDiagnostic(source, target, diagnosticTarget)]]; @@ -599,7 +619,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T sourceParameters: readonly MixedFunctionParameter[], targetParameters: readonly MixedFunctionParameter[], diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { const queue = sourceParameters.slice(); const errors: TypeRelationError[] = []; @@ -836,9 +856,9 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Model, target: Model, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { - relationCache.set([source, target], Related.maybe); + relationCache.set([source, target], [Related.maybe, []]); const errors: TypeRelationError[] = []; const remainingProperties = new Map(source.properties); @@ -952,7 +972,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T properties: Map, indexerConstaint: Type, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Type, Type], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { for (const prop of properties.values()) { const [related, diagnostics] = isTypeAssignableToInternal( @@ -974,7 +994,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Model, target: Model & { indexer: ModelIndexer }, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { if (source.indexer === undefined || source.indexer.key !== target.indexer.key) { return [ @@ -1003,7 +1023,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Tuple, target: ArrayModelType, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { const minItems = getMinItems(program, target); const maxItems = getMaxItems(program, target); @@ -1051,7 +1071,7 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Tuple | ArrayValue, target: Tuple, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, readonly TypeRelationError[]] { if (source.values.length !== target.values.length) { return [ @@ -1085,11 +1105,19 @@ export function createTypeRelationChecker(program: Program, checker: Checker): T source: Type, target: Union, diagnosticTarget: Entity | Node, - relationCache: MultiKeyMap<[Entity, Entity], Related>, + relationCache: RelationCache, ): [Related, TypeRelationError[]] { if (source.kind === "UnionVariant" && source.union === target) { return [Related.true, []]; } + // Seed the relation before walking the variants: a union can reach itself + // (`union Foo { self: Foo }`) and would otherwise recurse forever. Being assignable to a union + // means being assignable to at least one of its variants, so coming back to the same pair + // brings no new information and is resolved as unrelated. + relationCache.set( + [source, target], + [Related.false, [createUnassignableDiagnostic(source, target, diagnosticTarget)]], + ); for (const option of target.variants.values()) { const [related] = isTypeAssignableToInternal( source, diff --git a/packages/compiler/test/checker/relation.test.ts b/packages/compiler/test/checker/relation.test.ts index 5e7daf968ed..33fc185dcb8 100644 --- a/packages/compiler/test/checker/relation.test.ts +++ b/packages/compiler/test/checker/relation.test.ts @@ -1713,6 +1713,142 @@ describe("compiler: checker: type relations", () => { await expectTypeAssignable({ source, target }); }); }); + + describe("recursive types", () => { + it("model referencing itself", async () => { + await expectTypeAssignable({ + source: "Source", + target: "Target", + commonCode: ` + model Source { self: Source } + model Target { self: Target } + `, + }); + }); + + it("mutually recursive models", async () => { + await expectTypeAssignable({ + source: "SourceA", + target: "TargetA", + commonCode: ` + model SourceA { b: SourceB } + model SourceB { a: SourceA } + model TargetA { b: TargetB } + model TargetB { a: TargetA } + `, + }); + }); + + it("mutually recursive models with a mismatch deep in the cycle", async () => { + await expectTypeNotAssignable( + { + source: "SourceA", + target: "TargetA", + commonCode: ` + model SourceA { b: SourceB } + model SourceB { a: SourceA, extra: string } + model TargetA { b: TargetB } + model TargetB { a: TargetA, extra: int32 } + `, + }, + { code: "unassignable" }, + ); + }); + + it("union referencing itself", async () => { + // A union whose only variant is itself describes an empty set of values, so like `never` + // it is vacuously assignable to anything. What matters here is that it terminates. + await expectTypeAssignable({ + source: "Loop", + target: "Target", + commonCode: ` + union Loop { self: Loop } + model Target { name: string } + `, + }); + }); + + it("mutually recursive unions", async () => { + await expectTypeAssignable({ + source: "LoopA", + target: "Target", + commonCode: ` + union LoopA { b: LoopB } + union LoopB { a: LoopA } + model Target { name: string } + `, + }); + }); + + it("union referencing itself alongside an unassignable variant", async () => { + await expectTypeNotAssignable( + { + source: "Loop", + target: "Target", + commonCode: ` + union Loop { self: Loop, other: string } + model Target { name: string } + `, + }, + { code: "unassignable" }, + ); + }); + + it("model with a recursive union property", async () => { + await expectTypeAssignable({ + source: "Source", + target: "Target", + commonCode: ` + union Loop { self: Loop, name: string } + model Source { value: Loop } + model Target { value: Loop } + `, + }); + }); + + it("recursive union as the target", async () => { + await expectTypeAssignable({ + source: "string", + target: "Loop", + commonCode: `union Loop { self: Loop, s: string }`, + }); + }); + + it("recursive union as the target without a matching variant", async () => { + await expectTypeNotAssignable( + { + source: "string", + target: "Loop", + commonCode: `union Loop { self: Loop }`, + }, + { code: "unassignable" }, + ); + }); + + it("mutually recursive unions on both sides", async () => { + await expectTypeAssignable({ + source: "SourceA", + target: "TargetA", + commonCode: ` + union SourceA { b: SourceB } + union SourceB { a: SourceA, s: string } + union TargetA { b: TargetB } + union TargetB { a: TargetA, s: string } + `, + }); + }); + + it("model with a recursive array property", async () => { + await expectTypeAssignable({ + source: "Source", + target: "Target", + commonCode: ` + model Source { items: Source[] } + model Target { items: Target[] } + `, + }); + }); + }); }); }); From b955e41c79fa231ddc449f285017d0e6546f297d Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 27 Aug 2026 14:29:54 -0700 Subject: [PATCH 4/8] feat(compiler): harden @strictExtends validation Address the review findings on the first `@strictExtends` implementation. - Validate from `onTargetFinish` instead of during decorator application so a decorator applied later cannot invalidate the guarantee. - Report an error when the base type is not a model, a scalar or an enum instead of silently accepting every variant. - Handle scalars, string/number/boolean literals and enum members instead of only models, so a literal that is structurally assignable to a custom scalar is rejected. - Treat `never`, an empty union and a union cycle as satisfying the constraint so composition is closed: a union that `@strictExtends` accepts on its own is always usable as a variant of another one. - Memoize the walk, which was exponential on a union graph reachable through many paths. Results that relied on short circuiting a cycle are not memoized because they are only valid for that walk. - Name the offending leaf and point the diagnostic at the variant type expression instead of the whole variant. - Stay silent when the base type failed to resolve, and when the variant is not even assignable to the base type, since the checker already reported both. - Export `$strictExtends` from the package entry point like the other built in decorators. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4 --- .../changes/union-strict-extends-2026-8-26.md | 2 +- packages/compiler/generated-defs/TypeSpec.ts | 7 +- packages/compiler/lib/std/decorators.tsp | 7 +- packages/compiler/src/core/messages.ts | 9 +- packages/compiler/src/index.ts | 1 + packages/compiler/src/lib/decorators.ts | 205 +++++++++++--- .../test/decorators/decorators.test.ts | 253 +++++++++++++++++- .../docs/docs/language-basics/unions.md | 4 +- .../standard-library/built-in-decorators.md | 7 +- 9 files changed, 434 insertions(+), 61 deletions(-) diff --git a/.chronus/changes/union-strict-extends-2026-8-26.md b/.chronus/changes/union-strict-extends-2026-8-26.md index 75e58deb866..850095e37dd 100644 --- a/.chronus/changes/union-strict-extends-2026-8-26.md +++ b/.chronus/changes/union-strict-extends-2026-8-26.md @@ -26,4 +26,4 @@ union Pets extends Pet { } ``` -`@strictExtends` only adds a constraint when the base type is a model: assignability between scalars is already nominal in TypeSpec. A variant that is itself a union satisfies the constraint when all of its own variants do, so unions can still be composed. +`@strictExtends` can only be used when the base type is a model, a scalar or an enum, since those are the only types that can be explicitly extended. A variant that is itself a union satisfies the constraint when all of its own variants do, so unions can still be composed. diff --git a/packages/compiler/generated-defs/TypeSpec.ts b/packages/compiler/generated-defs/TypeSpec.ts index 5a0af1b1f6a..3a935899bcf 100644 --- a/packages/compiler/generated-defs/TypeSpec.ts +++ b/packages/compiler/generated-defs/TypeSpec.ts @@ -641,11 +641,12 @@ export type DiscriminatedDecorator = ( * with a polymorphic base type, which requires each variant to actually derive from that base * type. `@strictExtends` turns that requirement into a compile time error. * - * This only adds a constraint when the base type is a model: assignability between scalars is - * already nominal in TypeSpec so nothing needs to be enforced for them. + * This only adds a constraint when the base type is a model, scalar or enum. Any other base + * type (a union, tuple, `unknown`, ...) cannot be explicitly extended and is an error. * * A variant that is itself a union satisfies the constraint when all of its own variants do, - * which allows composing unions. + * which allows composing unions. `never`, an empty union and a self referencing union describe + * no value at all so they satisfy the constraint vacuously. * * @example * ```typespec diff --git a/packages/compiler/lib/std/decorators.tsp b/packages/compiler/lib/std/decorators.tsp index a639c29f15d..c9a5252594a 100644 --- a/packages/compiler/lib/std/decorators.tsp +++ b/packages/compiler/lib/std/decorators.tsp @@ -462,11 +462,12 @@ extern dec discriminated(target: Union, options?: valueof DiscriminatedOptions); * with a polymorphic base type, which requires each variant to actually derive from that base * type. `@strictExtends` turns that requirement into a compile time error. * - * This only adds a constraint when the base type is a model: assignability between scalars is - * already nominal in TypeSpec so nothing needs to be enforced for them. + * This only adds a constraint when the base type is a model, scalar or enum. Any other base + * type (a union, tuple, `unknown`, ...) cannot be explicitly extended and is an error. * * A variant that is itself a union satisfies the constraint when all of its own variants do, - * which allows composing unions. + * which allows composing unions. `never`, an empty union and a self referencing union describe + * no value at all so they satisfy the constraint vacuously. * * @example * diff --git a/packages/compiler/src/core/messages.ts b/packages/compiler/src/core/messages.ts index 78712d9d345..aba1a692489 100644 --- a/packages/compiler/src/core/messages.ts +++ b/packages/compiler/src/core/messages.ts @@ -542,10 +542,17 @@ const diagnostics = { default: "@strictExtends can only be used on a union declaring a base type with `extends`.", }, }, + "strict-extends-invalid-base-type": { + severity: "error", + messages: { + default: paramMessage`@strictExtends cannot be used with the base type '${"baseType"}': only a model, scalar or enum base type can be explicitly extended.`, + }, + }, "strict-extends-variant": { severity: "error", messages: { - default: paramMessage`Variant of type '${"variantType"}' must explicitly extend '${"baseType"}' as required by @strictExtends.`, + default: paramMessage`Variant of type '${"variantType"}' must be or extend '${"baseType"}' as required by @strictExtends.`, + nested: paramMessage`Variant of type '${"variantType"}' includes '${"offendingType"}' which must be or extend '${"baseType"}' as required by @strictExtends.`, }, }, "enum-member-duplicate": { diff --git a/packages/compiler/src/index.ts b/packages/compiler/src/index.ts index 58419bc29de..a3c59a760e2 100644 --- a/packages/compiler/src/index.ts +++ b/packages/compiler/src/index.ts @@ -145,6 +145,7 @@ export { $returnTypeVisibility, $secret, $service, + $strictExtends, $summary, $tag, $visibility, diff --git a/packages/compiler/src/lib/decorators.ts b/packages/compiler/src/lib/decorators.ts index fa9406d3f5a..23ebab21b57 100644 --- a/packages/compiler/src/lib/decorators.ts +++ b/packages/compiler/src/lib/decorators.ts @@ -67,16 +67,18 @@ import { setMinValue, setMinValueExclusive, } from "../core/intrinsic-type-state.js"; -import { reportDiagnostic } from "../core/messages.js"; +import { createDiagnostic, reportDiagnostic } from "../core/messages.js"; import { parseMimeType } from "../core/mime-type.js"; import type { Numeric } from "../core/numeric.js"; import { isNumeric } from "../core/numeric.js"; import type { Program } from "../core/program.js"; -import { isArrayModelType, isErrorType, isValue } from "../core/type-utils.js"; +import { isArrayModelType, isErrorType, isNeverType, isValue } from "../core/type-utils.js"; import type { AugmentDecoratorStatementNode, + BooleanLiteral, DecoratorContext, DecoratorExpressionNode, + Diagnostic, DiagnosticTarget, Enum, EnumValue, @@ -85,11 +87,13 @@ import type { ModelProperty, Namespace, Node, + NumericLiteral, ObjectValue, Operation, Scalar, ScalarValue, StdTypeName, + StringLiteral, Type, Union, UnionVariant, @@ -1331,77 +1335,198 @@ export const $discriminator: DiscriminatorDecorator = ( // -- @strictExtends -------------------------------------------------------------------------- +/** Base types that a variant can nominally derive from. */ +type StrictExtendsBase = Model | Scalar | Enum; + export const $strictExtends: StrictExtendsDecorator = ( context: DecoratorContext, entity: Union, ) => { + // Validate once every decorator on the union has been applied: another decorator could + // otherwise change the variants after this one ran and silently break the guarantee. + return { + onTargetFinish: () => validateStrictExtends(context, entity), + }; +}; + +function validateStrictExtends(context: DecoratorContext, entity: Union): Diagnostic[] { + const { program } = context; const baseType = entity.baseType; if (baseType === undefined) { - reportDiagnostic(context.program, { - code: "strict-extends-no-base-type", - target: context.decoratorTarget, - }); - return; + // A base type that failed to resolve, or a circular one, was already reported by the checker. + if (entity.node?.kind === SyntaxKind.UnionStatement && entity.node.extends !== undefined) { + return []; + } + return [ + createDiagnostic({ + code: "strict-extends-no-base-type", + target: context.decoratorTarget, + }), + ]; } - // Only a model base type needs the extra check: assignability between scalars is already - // nominal in TypeSpec, so an unrelated scalar can never satisfy the `extends` clause anyway. - if (baseType.kind !== "Model") { - return; + if (!isStrictExtendsBase(baseType)) { + return [ + createDiagnostic({ + code: "strict-extends-invalid-base-type", + format: { baseType: getTypeName(baseType) }, + target: context.decoratorTarget, + }), + ]; } + const diagnostics: Diagnostic[] = []; + const state: DerivationState = { + program, + base: baseType, + memo: new Map(), + path: new Set(), + cycleHits: 0, + }; for (const variant of entity.variants.values()) { - if (isErrorType(variant.type)) { + // The checker already reported a variant that isn't even assignable to the base type. + if (!program.checker.isTypeAssignableTo(variant.type, baseType, variant.type)[0]) { continue; } - if (!derivesFromModel(variant.type, baseType, new Set())) { - reportDiagnostic(context.program, { - code: "strict-extends-variant", - format: { - variantType: getTypeName(variant.type), - baseType: getTypeName(baseType), - }, - target: variant.node ?? entity, - }); + const offending = findTypeNotDerivedFrom(variant.type, state); + if (offending === undefined) { + continue; } + const target = variant.node?.value ?? variant.node ?? entity; + diagnostics.push( + offending === variant.type + ? createDiagnostic({ + code: "strict-extends-variant", + format: { variantType: getTypeName(variant.type), baseType: getTypeName(baseType) }, + target, + }) + : createDiagnostic({ + code: "strict-extends-variant", + messageId: "nested", + format: { + variantType: getTypeName(variant.type), + offendingType: getTypeName(offending), + baseType: getTypeName(baseType), + }, + target, + }), + ); } -}; + return diagnostics; +} + +function isStrictExtendsBase(type: Type): type is StrictExtendsBase { + return type.kind === "Model" || type.kind === "Scalar" || type.kind === "Enum"; +} + +interface DerivationState { + readonly program: Program; + readonly base: StrictExtendsBase; + /** Settled results, keyed by type. `undefined` means the type derives from the base type. */ + readonly memo: Map; + /** Unions currently being walked, to detect cycles. */ + readonly path: Set; + /** Number of times a cycle was short circuited, used to know what is safe to memoize. */ + cycleHits: number; +} /** - * Check whether `type` explicitly derives from `baseType` through `extends` declarations. + * Find a type reachable from `type` that doesn't explicitly derive from the base type, or + * `undefined` when the whole type satisfies the constraint. * - * A union variant that is itself a union derives from the base type when every one of its own - * variants does, which is what makes composing unions work. `path` guards against a union - * reaching itself, which is a legal (if unsatisfiable here) type graph. + * A variant that is itself a union satisfies the constraint when all of its own variants do, + * which is what makes composing unions work. An empty union, a union cycle and `never` are all + * uninhabited so they satisfy it vacuously, exactly like the assignability rules do. */ -function derivesFromModel(type: Type, baseType: Model, path: Set): boolean { +function findTypeNotDerivedFrom(type: Type, state: DerivationState): Type | undefined { + if (type === state.base || isErrorType(type) || isNeverType(type)) { + return undefined; + } + const memoized = state.memo.get(type); + if (memoized !== undefined || state.memo.has(type)) { + return memoized; + } + + let result: Type | undefined = type; + let memoizable = true; switch (type.kind) { case "Model": - for (let current: Model | undefined = type; current; current = current.baseModel) { - if (current === baseType) { - return true; + if (state.base.kind === "Model") { + for (let current: Model | undefined = type; current; current = current.baseModel) { + if (current === state.base) { + result = undefined; + break; + } } } - return false; + break; + case "Scalar": + if (state.base.kind === "Scalar" && derivesFromScalar(type, state.base)) { + result = undefined; + } + break; + case "String": + case "Number": + case "Boolean": + // A literal nominally is its standard scalar type, and nothing else. + if (state.base === literalStdScalar(type, state.program)) { + result = undefined; + } + break; + case "EnumMember": + if (type.enum === state.base) { + result = undefined; + } + break; case "Union": { - if (path.has(type) || type.variants.size === 0) { - return false; + if (state.path.has(type)) { + // A union reaching itself contributes no value of its own. + state.cycleHits++; + return undefined; } - path.add(type); + const cycleHitsBefore = state.cycleHits; + state.path.add(type); try { + result = undefined; for (const variant of type.variants.values()) { - if (!derivesFromModel(variant.type, baseType, path)) { - return false; + const offending = findTypeNotDerivedFrom(variant.type, state); + if (offending !== undefined) { + result = offending; + break; } } - return true; } finally { - path.delete(type); + state.path.delete(type); } + // A result that relied on short circuiting a cycle is only valid for this walk: the same + // union reached from outside the cycle can have a different answer. + memoizable = state.cycleHits === cycleHitsBefore; + break; + } + } + + if (memoizable) { + state.memo.set(type, result); + } + return result; +} + +function derivesFromScalar(type: Scalar, base: Scalar): boolean { + for (let current: Scalar | undefined = type; current; current = current.baseScalar) { + if (current === base) { + return true; } - default: - return false; } + return false; +} + +function literalStdScalar( + type: StringLiteral | NumericLiteral | BooleanLiteral, + program: Program, +): Scalar { + const name: StdTypeName = + type.kind === "String" ? "string" : type.kind === "Number" ? "numeric" : "boolean"; + return program.checker.getStdType(name); } export interface Example extends ExampleOptions { diff --git a/packages/compiler/test/decorators/decorators.test.ts b/packages/compiler/test/decorators/decorators.test.ts index 1312a57b417..e1260958c0f 100644 --- a/packages/compiler/test/decorators/decorators.test.ts +++ b/packages/compiler/test/decorators/decorators.test.ts @@ -1321,6 +1321,15 @@ describe("@strictExtends", () => { }); }); + it("doesn't emit a cascading error when the base type failed to resolve", async () => { + const diagnostics = await Tester.diagnose(` + @strictExtends + union Pets extends NotDefined {} + `); + + expectDiagnostics(diagnostics, { code: "invalid-ref" }); + }); + it("accepts variants extending the base type", async () => { const diagnostics = await Tester.diagnose(` model Pet { name: string } @@ -1378,7 +1387,7 @@ describe("@strictExtends", () => { expectDiagnostics(diagnostics, { code: "strict-extends-variant", - message: "Variant of type 'Rock' must explicitly extend 'Pet' as required by @strictExtends.", + message: "Variant of type 'Rock' must be or extend 'Pet' as required by @strictExtends.", }); }); @@ -1440,7 +1449,7 @@ describe("@strictExtends", () => { expectDiagnosticEmpty(diagnostics); }); - it("emit error when a union variant contains a type not extending the base type", async () => { + it("names the offending type when a union variant contains it", async () => { const diagnostics = await Tester.diagnose(` model Pet { name: string } model Cat extends Pet { meow: boolean } @@ -1459,10 +1468,12 @@ describe("@strictExtends", () => { expectDiagnostics(diagnostics, { code: "strict-extends-variant", + message: + "Variant of type 'Others' includes 'Rock' which must be or extend 'Pet' as required by @strictExtends.", }); }); - it("emit error for an empty union variant", async () => { + it("accepts an empty union variant", async () => { const diagnostics = await Tester.diagnose(` model Pet { name: string } @@ -1474,11 +1485,104 @@ describe("@strictExtends", () => { } `); + expectDiagnosticEmpty(diagnostics); + }); + + it("accepts a union that a `@strictExtends` union accepts as a variant", async () => { + // Composition must be closed: if `Empty` satisfies `@strictExtends` on its own then using it + // as a variant of another `@strictExtends` union has to be valid too. + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + + @strictExtends + union Empty extends Pet {} + + @strictExtends + union Pets extends Pet { + Empty, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("accepts a self referencing union variant", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + + union Loop { + self: Loop, + } + + @strictExtends + union Pets extends Pet { + Loop, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("emit error for a union cycle that also reaches an offending type", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Rock { name: string } + + union Loop { + self: Loop, + rock: Rock, + } + + @strictExtends + union Pets extends Pet { + Loop, + } + `); + expectDiagnostics(diagnostics, { code: "strict-extends-variant", + message: + "Variant of type 'Loop' includes 'Rock' which must be or extend 'Pet' as required by @strictExtends.", }); }); + it("reports every variant of a cycle that reaches an offending type", async () => { + // `C` is only satisfied while the walk from `A` has `A` on its path. That answer must not be + // reused for `C` itself, which does reach `Rock` through `A` and `B`. + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Rock { name: string } + + union A { b: B } + union B { c: C, rock: Rock } + union C { a: A } + + @strictExtends + union Pets extends Pet { + x: A, + y: C, + } + `); + + expectDiagnostics(diagnostics, [ + { code: "strict-extends-variant" }, + { code: "strict-extends-variant" }, + ]); + }); + + it("accepts a `never` variant", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + + @strictExtends + union Pets extends Pet { + nothing: never, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + it("accepts the same union appearing as multiple variants", async () => { const diagnostics = await Tester.diagnose(` model Pet { name: string } @@ -1498,20 +1602,92 @@ describe("@strictExtends", () => { expectDiagnosticEmpty(diagnostics); }); - it("adds no constraint when the base type is a scalar", async () => { + it("accepts scalars extending a scalar base type", async () => { const diagnostics = await Tester.diagnose(` scalar myString extends string; + scalar myOtherString extends myString; @strictExtends - union Values extends string { + union Values extends myString { a: myString, - b: "literal", + b: myOtherString, } `); expectDiagnosticEmpty(diagnostics); }); + it("accepts literals when the base type is the matching standard scalar", async () => { + const diagnostics = await Tester.diagnose(` + @strictExtends + union Values extends string { + a: "literal", + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("emit error for a literal assignable to a custom scalar base type", async () => { + const diagnostics = await Tester.diagnose(` + scalar myString extends string; + + @strictExtends + union Values extends myString { + a: "literal", + } + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-variant", + message: `Variant of type '"literal"' must be or extend 'myString' as required by @strictExtends.`, + }); + }); + + it("accepts enum members of an enum base type", async () => { + const diagnostics = await Tester.diagnose(` + enum Direction { up, down } + + @strictExtends + union Directions extends Direction { + all: Direction, + up: Direction.up, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); + + it("emit error when the base type cannot be extended", async () => { + const diagnostics = await Tester.diagnose(` + @strictExtends + union Values extends unknown { + a: string, + } + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-invalid-base-type", + message: + "@strictExtends cannot be used with the base type 'unknown': only a model, scalar or enum base type can be explicitly extended.", + }); + }); + + it("emit error when the base type is a union", async () => { + const diagnostics = await Tester.diagnose(` + union Base { a: string, b: int32 } + + @strictExtends + union Values extends Base { + a: string, + } + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-invalid-base-type", + }); + }); + it("is not applied to an uninstantiated template declaration", async () => { const diagnostics = await Tester.diagnose(` model Pet { name: string } @@ -1546,7 +1722,7 @@ describe("@strictExtends", () => { }); }); - it("reports both diagnostics for a variant that also fails the base type constraint", async () => { + it("doesn't report a variant that already failed the base type constraint", async () => { const diagnostics = await Tester.diagnose(` model Pet { name: string } model Rock { size: int32 } @@ -1557,7 +1733,68 @@ describe("@strictExtends", () => { } `); - expectDiagnostics(diagnostics, [{ code: "unassignable" }, { code: "strict-extends-variant" }]); + expectDiagnostics(diagnostics, { code: "unassignable" }); + }); + + it("validates the variants after every decorator has been applied", async () => { + const host = await createTestHost(); + host.addJsFile("mutate.js", { + namespace: "Test", + $replaceVariants(ctx: any, target: any, replacement: any) { + for (const variant of target.variants.values()) { + variant.type = replacement; + } + }, + }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./mutate.js"; + + namespace Test { + extern dec replaceVariants(target: TypeSpec.Reflection.Union, replacement: unknown); + } + + using Test; + + model Pet { name: string } + model Cat extends Pet { name: string } + model Rock { name: string } + + @replaceVariants(Rock) + @strictExtends + union Pets extends Pet { + cat: Cat, + } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, { code: "strict-extends-variant" }); + }); + + it("checks a union graph reachable through many paths only once", async () => { + // Each union references the previous one twice, so a non memoized walk would visit 2^depth + // types. + const depth = 30; + const unions = [`union U0 { cat: Cat }`]; + for (let i = 1; i <= depth; i++) { + unions.push(`union U${i} { a: U${i - 1}, b: U${i - 1} }`); + } + + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + + ${unions.join("\n")} + + @strictExtends + union Pets extends Pet { + all: U${depth}, + } + `); + + expectDiagnosticEmpty(diagnostics); }); }); diff --git a/website/src/content/docs/docs/language-basics/unions.md b/website/src/content/docs/docs/language-basics/unions.md index 1ec59807182..70e0d71f01c 100644 --- a/website/src/content/docs/docs/language-basics/unions.md +++ b/website/src/content/docs/docs/language-basics/unions.md @@ -102,9 +102,9 @@ union Breed extends Dog { } ``` -`@strictExtends` only adds a constraint when the base type is a model: assignability between scalars is already nominal in TypeSpec, so `@strictExtends` on `union Foo extends string {...}` changes nothing. +`@strictExtends` can only be used when the base type is a model, a scalar or an enum, since those are the only types that can be explicitly extended. Using it on a union whose base type is, for example, another union or `unknown` is an error. -A variant that is itself a union satisfies the constraint when all of its own variants do, so unions can still be composed: +A variant that is itself a union satisfies the constraint when all of its own variants do, so unions can still be composed. `never`, an empty union and a self referencing union describe no value at all, so they satisfy the constraint vacuously. ```typespec model Pet { diff --git a/website/src/content/docs/docs/standard-library/built-in-decorators.md b/website/src/content/docs/docs/standard-library/built-in-decorators.md index 713244b75be..85f95ee3f0c 100644 --- a/website/src/content/docs/docs/standard-library/built-in-decorators.md +++ b/website/src/content/docs/docs/standard-library/built-in-decorators.md @@ -1208,11 +1208,12 @@ shape satisfies it. Emitters targeting languages without native unions represent with a polymorphic base type, which requires each variant to actually derive from that base type. `@strictExtends` turns that requirement into a compile time error. -This only adds a constraint when the base type is a model: assignability between scalars is -already nominal in TypeSpec so nothing needs to be enforced for them. +This only adds a constraint when the base type is a model, scalar or enum. Any other base +type (a union, tuple, `unknown`, ...) cannot be explicitly extended and is an error. A variant that is itself a union satisfies the constraint when all of its own variants do, -which allows composing unions. +which allows composing unions. `never`, an empty union and a self referencing union describe +no value at all so they satisfy the constraint vacuously. ```typespec @strictExtends ``` From d4d9d0fd1ac16b113a95092562c11e1e4d0e9b31 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 27 Aug 2026 14:56:13 -0700 Subject: [PATCH 5/8] fix(compiler): make @strictExtends validation order and mutation safe Follow up on a second adversarial review pass. - Validate from `onGraphFinish` instead of `onTargetFinish`. A union that is part of a cycle is still being built when the decorated union finishes, so validating earlier made the result depend on declaration order: moving the offending variant before the back edge changed an accepted program into a rejected one. - Stop assuming a variant that isn't assignable to the base type was already reported by the checker. The checker validates the variants a union was declared with, so a decorator that replaces one afterwards would bypass the validation entirely. A variant that satisfies neither constraint now reports both. - Replace the memoized walk with a plain reachability walk. A union carries no value of its own, so the question is only whether anything offending is reachable through its variants, which makes cycles fall out naturally. The previous cycle guard disabled memoization for every ancestor of a cycle and was exponential on a cyclic graph reachable through many paths: 8s at depth 24, instant now at depth 30. - Validate a union once even when the decorator is applied more than once, directly or through `@@strictExtends`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4 --- packages/compiler/src/lib/decorators.ts | 159 ++++++++---------- .../test/decorators/decorators.test.ts | 127 +++++++++++++- 2 files changed, 194 insertions(+), 92 deletions(-) diff --git a/packages/compiler/src/lib/decorators.ts b/packages/compiler/src/lib/decorators.ts index 23ebab21b57..d241080f390 100644 --- a/packages/compiler/src/lib/decorators.ts +++ b/packages/compiler/src/lib/decorators.ts @@ -1335,17 +1335,29 @@ export const $discriminator: DiscriminatorDecorator = ( // -- @strictExtends -------------------------------------------------------------------------- -/** Base types that a variant can nominally derive from. */ +/** Base types that a union variant can nominally derive from. */ type StrictExtendsBase = Model | Scalar | Enum; +const [hasStrictExtends, markStrictExtends] = useStateSet( + createStateSymbol("strictExtends"), +); + export const $strictExtends: StrictExtendsDecorator = ( context: DecoratorContext, entity: Union, ) => { - // Validate once every decorator on the union has been applied: another decorator could - // otherwise change the variants after this one ran and silently break the guarantee. + // The decorator is idempotent, so applying it twice, or once directly and once with `@@`, + // should not report the same problem twice. + if (hasStrictExtends(context.program, entity)) { + return; + } + markStrictExtends(context.program, entity); + + // Validate once the whole type graph is checked. Validating earlier would look at a union that + // is still being built when the type graph is cyclic, and would let a decorator applied after + // this one change the variants and silently break the guarantee. return { - onTargetFinish: () => validateStrictExtends(context, entity), + onGraphFinish: () => validateStrictExtends(context, entity), }; }; @@ -1376,18 +1388,8 @@ function validateStrictExtends(context: DecoratorContext, entity: Union): Diagno } const diagnostics: Diagnostic[] = []; - const state: DerivationState = { - program, - base: baseType, - memo: new Map(), - path: new Set(), - cycleHits: 0, - }; + const state: DerivationState = { program, base: baseType, clean: new Set() }; for (const variant of entity.variants.values()) { - // The checker already reported a variant that isn't even assignable to the base type. - if (!program.checker.isTypeAssignableTo(variant.type, baseType, variant.type)[0]) { - continue; - } const offending = findTypeNotDerivedFrom(variant.type, state); if (offending === undefined) { continue; @@ -1422,102 +1424,83 @@ function isStrictExtendsBase(type: Type): type is StrictExtendsBase { interface DerivationState { readonly program: Program; readonly base: StrictExtendsBase; - /** Settled results, keyed by type. `undefined` means the type derives from the base type. */ - readonly memo: Map; - /** Unions currently being walked, to detect cycles. */ - readonly path: Set; - /** Number of times a cycle was short circuited, used to know what is safe to memoize. */ - cycleHits: number; + /** Types already proven to have nothing offending reachable from them. */ + readonly clean: Set; } /** * Find a type reachable from `type` that doesn't explicitly derive from the base type, or * `undefined` when the whole type satisfies the constraint. * - * A variant that is itself a union satisfies the constraint when all of its own variants do, - * which is what makes composing unions work. An empty union, a union cycle and `never` are all - * uninhabited so they satisfy it vacuously, exactly like the assignability rules do. + * A union carries no value of its own, so it satisfies the constraint when every type reachable + * through its variants does. That makes composing unions work, and makes `never`, an empty union + * and a union cycle satisfy it vacuously, exactly like the assignability rules do. */ function findTypeNotDerivedFrom(type: Type, state: DerivationState): Type | undefined { - if (type === state.base || isErrorType(type) || isNeverType(type)) { + const visited = new Set(); + const offending = findTypeNotDerivedFromWorker(type, state, visited); + if (offending === undefined) { + // Nothing offending is reachable from any of the types just walked, which doesn't depend on + // where the walk started, so the whole set can be reused. + for (const seen of visited) { + state.clean.add(seen); + } + } + return offending; +} + +function findTypeNotDerivedFromWorker( + type: Type, + state: DerivationState, + visited: Set, +): Type | undefined { + // Reaching a type again, including through a cycle, brings nothing new. + if (visited.has(type) || state.clean.has(type)) { return undefined; } - const memoized = state.memo.get(type); - if (memoized !== undefined || state.memo.has(type)) { - return memoized; + visited.add(type); + + if (type.kind === "Union") { + for (const variant of type.variants.values()) { + const offending = findTypeNotDerivedFromWorker(variant.type, state, visited); + if (offending !== undefined) { + return offending; + } + } + return undefined; } - let result: Type | undefined = type; - let memoizable = true; + return derivesFromBase(type, state) ? undefined : type; +} + +function derivesFromBase(type: Type, state: DerivationState): boolean { + const { base } = state; + if (type === base || isErrorType(type) || isNeverType(type)) { + return true; + } switch (type.kind) { case "Model": - if (state.base.kind === "Model") { - for (let current: Model | undefined = type; current; current = current.baseModel) { - if (current === state.base) { - result = undefined; - break; - } - } + if (base.kind !== "Model") return false; + for (let current: Model | undefined = type; current; current = current.baseModel) { + if (current === base) return true; } - break; + return false; case "Scalar": - if (state.base.kind === "Scalar" && derivesFromScalar(type, state.base)) { - result = undefined; + if (base.kind !== "Scalar") return false; + for (let current: Scalar | undefined = type; current; current = current.baseScalar) { + if (current === base) return true; } - break; + return false; case "String": case "Number": case "Boolean": // A literal nominally is its standard scalar type, and nothing else. - if (state.base === literalStdScalar(type, state.program)) { - result = undefined; - } - break; + return base === literalStdScalar(type, state.program); case "EnumMember": - if (type.enum === state.base) { - result = undefined; - } - break; - case "Union": { - if (state.path.has(type)) { - // A union reaching itself contributes no value of its own. - state.cycleHits++; - return undefined; - } - const cycleHitsBefore = state.cycleHits; - state.path.add(type); - try { - result = undefined; - for (const variant of type.variants.values()) { - const offending = findTypeNotDerivedFrom(variant.type, state); - if (offending !== undefined) { - result = offending; - break; - } - } - } finally { - state.path.delete(type); - } - // A result that relied on short circuiting a cycle is only valid for this walk: the same - // union reached from outside the cycle can have a different answer. - memoizable = state.cycleHits === cycleHitsBefore; - break; - } - } - - if (memoizable) { - state.memo.set(type, result); - } - return result; -} - -function derivesFromScalar(type: Scalar, base: Scalar): boolean { - for (let current: Scalar | undefined = type; current; current = current.baseScalar) { - if (current === base) { - return true; - } + return type.enum === base; + default: + return false; } - return false; } function literalStdScalar( diff --git a/packages/compiler/test/decorators/decorators.test.ts b/packages/compiler/test/decorators/decorators.test.ts index e1260958c0f..1196ebddaf6 100644 --- a/packages/compiler/test/decorators/decorators.test.ts +++ b/packages/compiler/test/decorators/decorators.test.ts @@ -1722,7 +1722,7 @@ describe("@strictExtends", () => { }); }); - it("doesn't report a variant that already failed the base type constraint", async () => { + it("reports both diagnostics for a variant that also fails the base type constraint", async () => { const diagnostics = await Tester.diagnose(` model Pet { name: string } model Rock { size: int32 } @@ -1733,7 +1733,63 @@ describe("@strictExtends", () => { } `); - expectDiagnostics(diagnostics, { code: "unassignable" }); + expectDiagnostics(diagnostics, [{ code: "unassignable" }, { code: "strict-extends-variant" }]); + }); + + it("reports the union only once when the decorator is applied twice", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Rock { name: string } + + @strictExtends + @strictExtends + union Pets extends Pet { + rock: Rock, + } + `); + + expectDiagnostics(diagnostics, { code: "strict-extends-variant" }); + }); + + it("reports the union only once when it is also augmented", async () => { + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Rock { name: string } + + @strictExtends + union Pets extends Pet { + rock: Rock, + } + + @@strictExtends(Pets); + `); + + expectDiagnostics(diagnostics, { code: "strict-extends-variant" }); + }); + + it("doesn't depend on the declaration order of a cyclic union", async () => { + // `A` is still being built when `B` finishes, so validating any earlier than the whole type + // graph would see `A` as an empty union and accept it. + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Rock { name: string } + + union A { + b: B, + rock: Rock, + } + + @strictExtends + union B extends Pet { + a: A, + } + `); + + expectDiagnostics(diagnostics, { + code: "strict-extends-variant", + message: + "Variant of type 'A' includes 'Rock' which must be or extend 'Pet' as required by @strictExtends.", + }); }); it("validates the variants after every decorator has been applied", async () => { @@ -1773,9 +1829,48 @@ describe("@strictExtends", () => { expectDiagnostics(diagnostics, { code: "strict-extends-variant" }); }); + it("validates a variant a decorator replaced with a type unrelated to the base type", async () => { + // The checker validated the original variant, so a variant that isn't even assignable to the + // base type can't be assumed to have been reported already. + const host = await createTestHost(); + host.addJsFile("mutate.js", { + namespace: "Test", + $replaceVariants(ctx: any, target: any, replacement: any) { + for (const variant of target.variants.values()) { + variant.type = replacement; + } + }, + }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./mutate.js"; + + namespace Test { + extern dec replaceVariants(target: TypeSpec.Reflection.Union, replacement: unknown); + } + + using Test; + + model Pet { name: string } + model Cat extends Pet { name: string } + model Bad { size: int32 } + + @replaceVariants(Bad) + @strictExtends + union Pets extends Pet { + cat: Cat, + } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, { code: "strict-extends-variant" }); + }); + it("checks a union graph reachable through many paths only once", async () => { - // Each union references the previous one twice, so a non memoized walk would visit 2^depth - // types. + // Each union references the previous one twice, so a walk that doesn't remember what it + // already visited would visit 2^depth types. const depth = 30; const unions = [`union U0 { cat: Cat }`]; for (let i = 1; i <= depth; i++) { @@ -1796,6 +1891,30 @@ describe("@strictExtends", () => { expectDiagnosticEmpty(diagnostics); }); + + it("checks a cyclic union graph reachable through many paths only once", async () => { + // Same shape, but the leaf union is part of a cycle: reaching a cycle must not disable + // reusing what the walk already knows. + const depth = 30; + const unions = [`union U0 { self: U0, cat: Cat }`]; + for (let i = 1; i <= depth; i++) { + unions.push(`union U${i} { a: U${i - 1}, b: U${i - 1} }`); + } + + const diagnostics = await Tester.diagnose(` + model Pet { name: string } + model Cat extends Pet { meow: boolean } + + ${unions.join("\n")} + + @strictExtends + union Pets extends Pet { + all: U${depth}, + } + `); + + expectDiagnosticEmpty(diagnostics); + }); }); describe("@encodedName", () => { From 2e8760521a2e67ac02d6f953bcf7164c9a81055e Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 27 Aug 2026 15:14:59 -0700 Subject: [PATCH 6/8] fix(compiler): validate @strictExtends unions created after checking Graph finish validators are only drained once, at the end of the checking stage, so a `@strictExtends` union created after that - for example a clone a mutator produces during `$onValidate` - registered a validator that was never run, and was silently never validated. Keep deferring to graph finish while the program is being checked, which is what makes the result independent of declaration order for cyclic unions, and validate at target finish otherwise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4 --- packages/compiler/src/lib/decorators.ts | 31 +++++++--- .../test/decorators/decorators.test.ts | 61 +++++++++++++++++++ 2 files changed, 85 insertions(+), 7 deletions(-) diff --git a/packages/compiler/src/lib/decorators.ts b/packages/compiler/src/lib/decorators.ts index d241080f390..0680e455b77 100644 --- a/packages/compiler/src/lib/decorators.ts +++ b/packages/compiler/src/lib/decorators.ts @@ -71,7 +71,7 @@ import { createDiagnostic, reportDiagnostic } from "../core/messages.js"; import { parseMimeType } from "../core/mime-type.js"; import type { Numeric } from "../core/numeric.js"; import { isNumeric } from "../core/numeric.js"; -import type { Program } from "../core/program.js"; +import type { CompilationStage, Program } from "../core/program.js"; import { isArrayModelType, isErrorType, isNeverType, isValue } from "../core/type-utils.js"; import type { AugmentDecoratorStatementNode, @@ -1353,14 +1353,31 @@ export const $strictExtends: StrictExtendsDecorator = ( } markStrictExtends(context.program, entity); - // Validate once the whole type graph is checked. Validating earlier would look at a union that - // is still being built when the type graph is cyclic, and would let a decorator applied after - // this one change the variants and silently break the guarantee. - return { - onGraphFinish: () => validateStrictExtends(context, entity), - }; + const validate = () => validateStrictExtends(context, entity); + + // While the program is being checked, validate once the whole type graph is checked. Validating + // earlier would look at a union that is still being built when the type graph is cyclic, and + // would let a decorator applied after this one change the variants and silently break the + // guarantee. + // Graph finish validators are only run at the end of the checking stage, so a union created + // after that, for example a clone a mutator produced during `$onValidate`, has to be validated + // as soon as it is finished or it would never be validated at all. + return isAfterCheckingStage(context.program) + ? { onTargetFinish: validate } + : { onGraphFinish: validate }; }; +/** Stages that run after the checker ran, and so after graph finish validators were run. */ +const afterCheckingStages: ReadonlySet = new Set([ + "validating", + "linting", + "emitting", +]); + +function isAfterCheckingStage(program: Program): boolean { + return afterCheckingStages.has(program.currentStage); +} + function validateStrictExtends(context: DecoratorContext, entity: Union): Diagnostic[] { const { program } = context; const baseType = entity.baseType; diff --git a/packages/compiler/test/decorators/decorators.test.ts b/packages/compiler/test/decorators/decorators.test.ts index 1196ebddaf6..a700d68d14d 100644 --- a/packages/compiler/test/decorators/decorators.test.ts +++ b/packages/compiler/test/decorators/decorators.test.ts @@ -1,5 +1,6 @@ import { deepStrictEqual, ok, strictEqual } from "assert"; import { describe, expect, it } from "vitest"; +import { MutatorFlow, mutateSubgraph } from "../../src/experimental/mutators.js"; import { getDiscriminatedUnion, isSecret } from "../../src/index.js"; import { getDoc, @@ -1868,6 +1869,66 @@ describe("@strictExtends", () => { expectDiagnostics(diagnostics, { code: "strict-extends-variant" }); }); + it("validates a union a mutator cloned after the program was checked", async () => { + // Graph finish validators only run at the end of the checking stage, so a union created after + // that has to be validated when it is finished instead. + const diagnostics = await diagnoseMutatedStrictUnion("Rock"); + expectDiagnostics(diagnostics, { code: "strict-extends-variant" }); + }); + + it("doesn't report a union a mutator cloned after the program was checked when it is valid", async () => { + const diagnostics = await diagnoseMutatedStrictUnion("Dog"); + expectDiagnosticEmpty(diagnostics); + }); + + /** + * Compile a `@strictExtends` union and then, during the validation stage, mutate a clone of it so + * every variant has the given type. + */ + async function diagnoseMutatedStrictUnion(replacementName: string) { + const host = await createTestHost(); + host.addJsFile("mutate.js", { + $onValidate(program: any) { + const globalNs = program.getGlobalNamespaceType(); + const replacement = globalNs.models.get(replacementName); + mutateSubgraph( + program, + [ + { + name: "replace-variant", + Union: { mutate() {} }, + UnionVariant: { + filter: () => MutatorFlow.DoNotRecur, + mutate(_source: any, clone: any) { + clone.type = replacement; + }, + }, + } as any, + ], + globalNs.unions.get("Pets"), + ); + }, + }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./mutate.js"; + + model Pet { name: string } + model Cat extends Pet { name: string } + model Dog extends Pet { name: string } + model Rock { name: string } + + @strictExtends + union Pets extends Pet { + cat: Cat, + } + `, + ); + + return host.diagnose("main.tsp"); + } + it("checks a union graph reachable through many paths only once", async () => { // Each union references the previous one twice, so a walk that doesn't remember what it // already visited would visit 2^depth types. From 75a1042b1439b65b66050b5ba7f5f4af874330f6 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 27 Aug 2026 15:30:02 -0700 Subject: [PATCH 7/8] fix(compiler): run onGraphFinish validators for types created after checking `postCheckValidators` is drained once, at the end of `checkProgram`, so a decorator applied to a type created after that - a clone a mutator produced during `$onValidate` for example - registered an `onGraphFinish` validator that was never run. The checker now runs those validators as soon as the type is finished, since there is no graph finish left to wait for. `@strictExtends` relies on this: it validates at graph finish so the result doesn't depend on the declaration order of a cyclic union, and a post-check clone would otherwise silently never be validated. Using `program.currentStage` to detect this in the decorator instead is not enough: the stage stays `"checking"` when the checker reported errors, which is exactly the state a language server program is left in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4 --- ...nish-validator-after-checking-2026-8-26.md | 7 ++++ packages/compiler/src/core/checker.ts | 14 ++++++- packages/compiler/src/core/types.ts | 1 + packages/compiler/src/lib/decorators.ts | 31 ++++---------- .../compiler/test/checker/decorators.test.ts | 25 +++++++++++ .../test/decorators/decorators.test.ts | 42 +++++++++++++++++++ 6 files changed, 95 insertions(+), 25 deletions(-) create mode 100644 .chronus/changes/fix-graph-finish-validator-after-checking-2026-8-26.md diff --git a/.chronus/changes/fix-graph-finish-validator-after-checking-2026-8-26.md b/.chronus/changes/fix-graph-finish-validator-after-checking-2026-8-26.md new file mode 100644 index 00000000000..c36d6ec2b79 --- /dev/null +++ b/.chronus/changes/fix-graph-finish-validator-after-checking-2026-8-26.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/compiler" +--- + +Run the `onGraphFinish` validator of a decorator applied to a type created after the type graph was checked, a clone a mutator produced during `$onValidate` for example. Those validators used to be registered but never run. diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 2372ad81099..7cf65d14d11 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -539,6 +539,11 @@ export function createChecker(program: Program, resolver: NameResolver): Checker const pendingResolutions = new PendingResolutions(); const spreadResolutionAncestors = new Map>(); const postCheckValidators: ValidatorFn[] = []; + /** + * Whether {@link postCheckValidators} was already run. Types created after that, with a mutator + * for example, have no graph finish left to wait for. + */ + let postCheckValidatorsRan = false; const typespecNamespaceBinding = resolver.symbols.global.exports!.get("TypeSpec"); if (typespecNamespaceBinding) { @@ -4900,6 +4905,7 @@ export function createChecker(program: Program, resolver: NameResolver): Checker internalDecoratorValidation(); assertNoPendingResolutions(); runPostValidators(postCheckValidators); + postCheckValidatorsRan = true; } function assertNoPendingResolutions() { @@ -8170,7 +8176,13 @@ export function createChecker(program: Program, resolver: NameResolver): Checker postSelfValidators.push(validators.onTargetFinish); } if (validators?.onGraphFinish) { - postCheckValidators.push(validators.onGraphFinish); + if (postCheckValidatorsRan) { + // The type graph was already checked so the validator would never run. Run it as soon as + // the type is finished instead. + postSelfValidators.push(validators.onGraphFinish); + } else { + postCheckValidators.push(validators.onGraphFinish); + } } } return postSelfValidators; diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index 4a1d779dc89..05060409faf 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -66,6 +66,7 @@ export interface DecoratorValidatorCallbacks { /** * Run validation after everything is checked in the type graph. Useful when trying to get an overall view of the program. * @note This is meant for validation which means the type graph should be treated as readonly in this function. + * @note For a type created after the type graph was checked, with a mutator for example, there is no graph finish left to wait for and this runs as soon as the type is finished. */ readonly onGraphFinish?: ValidatorFn; } diff --git a/packages/compiler/src/lib/decorators.ts b/packages/compiler/src/lib/decorators.ts index 0680e455b77..ec9afb21c1b 100644 --- a/packages/compiler/src/lib/decorators.ts +++ b/packages/compiler/src/lib/decorators.ts @@ -71,7 +71,7 @@ import { createDiagnostic, reportDiagnostic } from "../core/messages.js"; import { parseMimeType } from "../core/mime-type.js"; import type { Numeric } from "../core/numeric.js"; import { isNumeric } from "../core/numeric.js"; -import type { CompilationStage, Program } from "../core/program.js"; +import type { Program } from "../core/program.js"; import { isArrayModelType, isErrorType, isNeverType, isValue } from "../core/type-utils.js"; import type { AugmentDecoratorStatementNode, @@ -1353,31 +1353,14 @@ export const $strictExtends: StrictExtendsDecorator = ( } markStrictExtends(context.program, entity); - const validate = () => validateStrictExtends(context, entity); - - // While the program is being checked, validate once the whole type graph is checked. Validating - // earlier would look at a union that is still being built when the type graph is cyclic, and - // would let a decorator applied after this one change the variants and silently break the - // guarantee. - // Graph finish validators are only run at the end of the checking stage, so a union created - // after that, for example a clone a mutator produced during `$onValidate`, has to be validated - // as soon as it is finished or it would never be validated at all. - return isAfterCheckingStage(context.program) - ? { onTargetFinish: validate } - : { onGraphFinish: validate }; + // Validate once the whole type graph is checked. Validating earlier would look at a union that is + // still being built when the type graph is cyclic, and would let a decorator applied after this + // one change the variants and silently break the guarantee. + // For a union created after the type graph was checked, a clone a mutator produced during + // `$onValidate` for example, the checker runs this as soon as the union is finished instead. + return { onGraphFinish: () => validateStrictExtends(context, entity) }; }; -/** Stages that run after the checker ran, and so after graph finish validators were run. */ -const afterCheckingStages: ReadonlySet = new Set([ - "validating", - "linting", - "emitting", -]); - -function isAfterCheckingStage(program: Program): boolean { - return afterCheckingStages.has(program.currentStage); -} - function validateStrictExtends(context: DecoratorContext, entity: Union): Diagnostic[] { const { program } = context; const baseType = entity.baseType; diff --git a/packages/compiler/test/checker/decorators.test.ts b/packages/compiler/test/checker/decorators.test.ts index 0dad4ab7b20..3b15fe12ae4 100644 --- a/packages/compiler/test/checker/decorators.test.ts +++ b/packages/compiler/test/checker/decorators.test.ts @@ -5,6 +5,7 @@ import { Numeric } from "../../src/core/numeric.js"; import type { DecoratorContext, DecoratorFunction, Model } from "../../src/index.js"; import { setTypeSpecNamespace } from "../../src/index.js"; import { expectDiagnostics, mockFile, t } from "../../src/testing/index.js"; +import { $ } from "../../src/typekit/index.js"; import { Tester } from "../tester.js"; const DecTester = Tester.files({ @@ -977,4 +978,28 @@ describe("validators", () => { `validate(B)`, ]); }); + + it("post apply validator when the type is finished if the graph was already checked", async () => { + // There is no graph finish left to wait for, so a validator that is only registered then would + // never run at all. + const order: string[] = []; + const tester = await testerForDecorator((_: DecoratorContext, target: Model) => { + order.push(`apply(${target.name})`); + return { + onGraphFinish: () => { + order.push(`validate(${target.name})`); + return []; + }, + }; + }); + const { A, program } = await tester.compile(t.code` + @myDecorator + model ${t.model("A")} {} + `); + deepStrictEqual(order, [`apply(A)`, `validate(A)`]); + + $(program).type.finishType($(program).type.clone(A)); + + deepStrictEqual(order, [`apply(A)`, `validate(A)`, `apply(A)`, `validate(A)`]); + }); }); diff --git a/packages/compiler/test/decorators/decorators.test.ts b/packages/compiler/test/decorators/decorators.test.ts index a700d68d14d..bf569b294a4 100644 --- a/packages/compiler/test/decorators/decorators.test.ts +++ b/packages/compiler/test/decorators/decorators.test.ts @@ -21,6 +21,8 @@ import { } from "../../src/lib/decorators.js"; import { expectDiagnosticEmpty, expectDiagnostics, t } from "../../src/testing/index.js"; import { createTestHost } from "../../src/testing/test-host.js"; +import { $ } from "../../src/typekit/index.js"; +import { createRekeyableMap } from "../../src/utils/misc.js"; import { Tester } from "../tester.js"; describe("dev comment /** */", () => { @@ -1929,6 +1931,46 @@ describe("@strictExtends", () => { return host.diagnose("main.tsp"); } + it("validates a union cloned after a compilation the checker reported errors for", async () => { + // The compilation stops after the checker reported errors, but the program is still used by + // tooling like the language server, and the type graph was already checked. + const host = await createTestHost(); + host.addTypeSpecFile( + "main.tsp", + ` + model Broken { value: Missing } + + model Pet { name: string } + model Cat extends Pet { name: string } + model Rock { name: string } + + @strictExtends + union Pets extends Pet { + cat: Cat, + } + `, + ); + await host.diagnose("main.tsp"); + + const program = host.program; + const globalNs = program.getGlobalNamespaceType(); + const rock = globalNs.models.get("Rock")!; + const clone = $(program).type.clone(globalNs.unions.get("Pets")!); + clone.variants = createRekeyableMap( + [...clone.variants].map(([key, variant]) => { + const variantClone = $(program).type.clone(variant); + variantClone.type = rock; + return [key, variantClone]; + }), + ); + $(program).type.finishType(clone); + + expectDiagnostics(program.diagnostics, [ + { code: "invalid-ref" }, + { code: "strict-extends-variant" }, + ]); + }); + it("checks a union graph reachable through many paths only once", async () => { // Each union references the previous one twice, so a walk that doesn't remember what it // already visited would visit 2^depth types. From 8e8cddffb67c6ccaa0a353bfd25c4b89ffa3fa14 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 27 Aug 2026 15:35:55 -0700 Subject: [PATCH 8/8] docs(compiler): note the graph finish validator limitation for post-check types Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4 --- packages/compiler/src/core/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index 05060409faf..250fda2a9df 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -66,7 +66,7 @@ export interface DecoratorValidatorCallbacks { /** * Run validation after everything is checked in the type graph. Useful when trying to get an overall view of the program. * @note This is meant for validation which means the type graph should be treated as readonly in this function. - * @note For a type created after the type graph was checked, with a mutator for example, there is no graph finish left to wait for and this runs as soon as the type is finished. + * @note For a type created after the type graph was checked, with a mutator for example, there is no graph finish left to wait for and this runs as soon as the type is finished. It then only sees the graph as it exists at that point, so it cannot observe types or relationships attached to it afterwards. */ readonly onGraphFinish?: ValidatorFn; }