Skip to content

Commit

Permalink
Report error when having a circular template constraint (microsoft#2932)
Browse files Browse the repository at this point in the history
  • Loading branch information
timotheeguerin authored and markcowl committed Mar 5, 2024
1 parent 53db348 commit 221bc8b
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@typespec/compiler"
---

Report error when having a circular template constraint e.g. `model Example<T extends T>`
17 changes: 17 additions & 0 deletions packages/compiler/src/core/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,7 @@ export function createChecker(program: Program): Checker {
| AliasStatementNode
| InterfaceStatementNode
| OperationStatementNode
| TemplateParameterDeclarationNode
| UnionStatementNode
): number {
const symbol =
Expand Down Expand Up @@ -750,6 +751,19 @@ export function createChecker(program: Program): Checker {
const grandParentNode = parentNode.parent;
const links = getSymbolLinks(node.symbol);

if (pendingResolutions.has(getNodeSymId(node), ResolutionKind.Constraint)) {
if (mapper === undefined) {
reportCheckerDiagnostic(
createDiagnostic({
code: "circular-constraint",
format: { typeName: node.id.sv },
target: node.constraint!,
})
);
}
return errorType;
}

let type: TemplateParameter | undefined = links.declaredType as TemplateParameter;
if (type === undefined) {
if (grandParentNode) {
Expand All @@ -770,7 +784,9 @@ export function createChecker(program: Program): Checker {
});

if (node.constraint) {
pendingResolutions.start(getNodeSymId(node), ResolutionKind.Constraint);
type.constraint = getTypeOrValueTypeForNode(node.constraint);
pendingResolutions.finish(getNodeSymId(node), ResolutionKind.Constraint);
}
if (node.default) {
type.default = checkTemplateParameterDefault(
Expand Down Expand Up @@ -6330,6 +6346,7 @@ const _assertReflectionNameToKind: Record<string, Type["kind"]> = ReflectionName
enum ResolutionKind {
Type,
BaseType,
Constraint,
}

class PendingResolutions {
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler/src/core/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,12 @@ const diagnostics = {
default: paramMessage`Type '${"typeName"}' recursively references itself as a base type.`,
},
},
"circular-constraint": {
severity: "error",
messages: {
default: paramMessage`Type parameter '${"typeName"}' has a circular constraint.`,
},
},
"circular-op-signature": {
severity: "error",
messages: {
Expand Down
30 changes: 30 additions & 0 deletions packages/compiler/test/checker/templates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,36 @@ describe("compiler: templates", () => {
`);
});

it("emits diagnostic when constraint reference itself", async () => {
testHost.addTypeSpecFile("main.tsp", `model Test<A extends A> {}`);

const diagnostics = await testHost.diagnose("main.tsp");
expectDiagnostics(diagnostics, {
code: "circular-constraint",
message: "Type parameter 'A' has a circular constraint.",
});
});

it("emits diagnostic when constraint reference other parameter in circular constraint", async () => {
testHost.addTypeSpecFile("main.tsp", `model Test<A extends B, B extends A> {}`);

const diagnostics = await testHost.diagnose("main.tsp");
expectDiagnostics(diagnostics, {
code: "circular-constraint",
message: "Type parameter 'A' has a circular constraint.",
});
});

it("emits diagnostic when constraint reference itself inside an expression", async () => {
testHost.addTypeSpecFile("main.tsp", `model Test<A extends {name: A}> {}`);

const diagnostics = await testHost.diagnose("main.tsp");
expectDiagnostics(diagnostics, {
code: "circular-constraint",
message: "Type parameter 'A' has a circular constraint.",
});
});

it("emit diagnostics if template default is not assignable to constraint", async () => {
const { source, pos, end } = extractSquiggles(`
model A<T extends string = ~~~123~~~> { a: T }
Expand Down

0 comments on commit 221bc8b

Please sign in to comment.