From d3292becccfd612bfac0926212d254dc314b9628 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 23 Apr 2024 10:40:43 -0700 Subject: [PATCH 1/5] Recover from bad op is --- packages/compiler/src/core/checker.ts | 64 ++++++++++++++++----------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 7390a622f3..65f5cb7f1d 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -1819,7 +1819,11 @@ export function createChecker(program: Program): Checker { } if (mapper === undefined && inInterface) { - compilerAssert(parentInterface, "Operation in interface should already have been checked."); + compilerAssert( + parentInterface, + "Operation in interface should already have been checked.", + node.parent + ); } checkTemplateDeclaration(node, mapper); @@ -1837,32 +1841,42 @@ export function createChecker(program: Program): Checker { if (node.signature.kind === SyntaxKind.OperationSignatureReference) { // Attempt to resolve the operation const baseOperation = checkOperationIs(node, node.signature.baseOperation, mapper); - if (!baseOperation) { - return errorType; - } - sourceOperation = baseOperation; - const parameterModelSym = getOrCreateAugmentedSymbolTable(symbol!.metatypeMembers!).get( - "parameters" - )!; - // Reference the same return type and create the parameters type - const clone = initializeClone(baseOperation.parameters, { - properties: createRekeyableMap(), - }); + if (baseOperation) { + sourceOperation = baseOperation; + const parameterModelSym = getOrCreateAugmentedSymbolTable(symbol!.metatypeMembers!).get( + "parameters" + )!; + // Reference the same return type and create the parameters type + const clone = initializeClone(baseOperation.parameters, { + properties: createRekeyableMap(), + }); - clone.properties = createRekeyableMap( - Array.from(baseOperation.parameters.properties.entries()).map(([key, prop]) => [ - key, - cloneTypeForSymbol(getMemberSymbol(parameterModelSym, prop.name)!, prop, { - model: clone, - sourceProperty: prop, - }), - ]) - ); - parameters = finishType(clone); - returnType = baseOperation.returnType; + clone.properties = createRekeyableMap( + Array.from(baseOperation.parameters.properties.entries()).map(([key, prop]) => [ + key, + cloneTypeForSymbol(getMemberSymbol(parameterModelSym, prop.name)!, prop, { + model: clone, + sourceProperty: prop, + }), + ]) + ); + parameters = finishType(clone); + returnType = baseOperation.returnType; - // Copy decorators from the base operation, inserting the base decorators first - decorators = [...baseOperation.decorators]; + // Copy decorators from the base operation, inserting the base decorators first + decorators = [...baseOperation.decorators]; + } else { + // If we can't resolve the signature we return an empty model. + parameters = createAndFinishType({ + kind: "Model", + name: "", + decorators: [], + properties: createRekeyableMap(), + derivedModels: [], + sourceModels: [], + }); + returnType = voidType; + } } else { parameters = getTypeForNode(node.signature.parameters, mapper) as Model; returnType = getTypeForNode(node.signature.returnType, mapper); From 83a2c6539d8275a0eb099b8bf4c25426ecd272b2 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 23 Apr 2024 10:54:43 -0700 Subject: [PATCH 2/5] add test --- .../compiler/test/checker/operations.test.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/compiler/test/checker/operations.test.ts b/packages/compiler/test/checker/operations.test.ts index a8d8a65642..859126a19a 100644 --- a/packages/compiler/test/checker/operations.test.ts +++ b/packages/compiler/test/checker/operations.test.ts @@ -259,6 +259,37 @@ describe("compiler: operations", () => { ok(gammaTargets.has(newFoo)); }); + // Regression test for https://github.com/microsoft/typespec/issues/3199 + it("produce an empty interface operation in template when op is reference is invalid", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + @test op test is IFace.Action; + + interface IFace { + Action is string; + } + ` + ); + + const [{ test }, diagnostics] = await testHost.compileAndDiagnose("./main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "is-operation", + message: "Operation can only reuse the signature of another operation.", + }, + { + code: "is-operation", + message: "Operation can only reuse the signature of another operation.", + }, + ]); + strictEqual(test.kind, "Operation"); + strictEqual(test.parameters.name, ""); + strictEqual(test.parameters.properties.size, 0); + strictEqual(test.returnType.kind, "Intrinsic"); + strictEqual((test.returnType as IntrinsicType).name, "void"); + }); + it("emit diagnostic when operation is referencing itself as signature", async () => { testHost.addTypeSpecFile( "main.tsp", From a199bd44ade49c966e4a02762baa670c7a4eb01b Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 23 Apr 2024 10:55:26 -0700 Subject: [PATCH 3/5] Create recover-bad-op-is-2024-3-23-17-46-20.md --- .chronus/changes/recover-bad-op-is-2024-3-23-17-46-20.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/recover-bad-op-is-2024-3-23-17-46-20.md diff --git a/.chronus/changes/recover-bad-op-is-2024-3-23-17-46-20.md b/.chronus/changes/recover-bad-op-is-2024-3-23-17-46-20.md new file mode 100644 index 0000000000..553de9ab5d --- /dev/null +++ b/.chronus/changes/recover-bad-op-is-2024-3-23-17-46-20.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@typespec/compiler" +--- + +Fix compiler crash when using an invalid `is` target in an interface operation template From 89a6f5c297c8c094ca318e8f06297a135ef6314b Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 23 Apr 2024 10:56:31 -0700 Subject: [PATCH 4/5] . --- packages/compiler/src/core/checker.ts | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 65f5cb7f1d..0a57dd3e69 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -1807,7 +1807,7 @@ export function createChecker(program: Program): Checker { node: OperationStatementNode, mapper: TypeMapper | undefined, parentInterface?: Interface - ): Operation | ErrorType { + ): Operation { const inInterface = node.parent?.kind === SyntaxKind.InterfaceStatement; const symbol = inInterface ? getSymbolForMember(node) : node.symbol; const links = symbol && getSymbolLinks(symbol); @@ -4158,19 +4158,17 @@ export function createChecker(program: Program): Checker { for (const opNode of node.operations) { const opType = checkOperation(opNode, mapper, interfaceType); - if (opType.kind === "Operation") { - if (ownMembers.has(opType.name)) { - reportCheckerDiagnostic( - createDiagnostic({ - code: "interface-duplicate", - format: { name: opType.name }, - target: opNode, - }) - ); - continue; - } - ownMembers.set(opType.name, opType); + if (ownMembers.has(opType.name)) { + reportCheckerDiagnostic( + createDiagnostic({ + code: "interface-duplicate", + format: { name: opType.name }, + target: opNode, + }) + ); + continue; } + ownMembers.set(opType.name, opType); } return ownMembers; } From ce251512e0b3cc7b2bbd62a2f953c0d5bea70e97 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 23 Apr 2024 11:15:40 -0700 Subject: [PATCH 5/5] fix --- packages/compiler/test/checker/operations.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/compiler/test/checker/operations.test.ts b/packages/compiler/test/checker/operations.test.ts index 859126a19a..fd04995c92 100644 --- a/packages/compiler/test/checker/operations.test.ts +++ b/packages/compiler/test/checker/operations.test.ts @@ -338,10 +338,6 @@ describe("compiler: operations", () => { code: "circular-op-signature", message: "Operation 'foo' recursively references itself.", }, - { - code: "circular-op-signature", - message: "Operation 'bar' recursively references itself.", - }, ]); }); @@ -361,10 +357,6 @@ describe("compiler: operations", () => { code: "circular-op-signature", message: "Operation 'foo' recursively references itself.", }, - { - code: "circular-op-signature", - message: "Operation 'bar' recursively references itself.", - }, ]); });