Skip to content

Commit

Permalink
Recover from bad op is reference in template (#3216)
Browse files Browse the repository at this point in the history
fix #3199
  • Loading branch information
timotheeguerin authored Apr 23, 2024
1 parent 65409be commit fe51f05
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 46 deletions.
8 changes: 8 additions & 0 deletions .chronus/changes/recover-bad-op-is-2024-3-23-17-46-20.md
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"
---

Fix compiler crash when using an invalid `is` target in an interface operation template
88 changes: 50 additions & 38 deletions packages/compiler/src/core/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -4144,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;
}
Expand Down
39 changes: 31 additions & 8 deletions packages/compiler/test/checker/operations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<int32>;
interface IFace {
Action<T> 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",
Expand Down Expand Up @@ -307,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.",
},
]);
});

Expand All @@ -330,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.",
},
]);
});

Expand Down

0 comments on commit fe51f05

Please sign in to comment.