Skip to content

Commit 2dbd643

Browse files
committed
Type-check the this parameter of (override) signatures correctly
In most cases, a function type is assignable to another if the target `this` parameter is a subtype of the original `this` parameter. However, this check wasn't performed correctly in `Signature#isAssignableTo`. Also, the reverse is true when checking whether override signatures are compatible, since the `this` parameter refers to the subclass that's overriding the parent class's method. At least, that's how I think it works... Fixes AssemblyScript#2727. Closes AssemblyScript#2728.
1 parent 382aabe commit 2dbd643

File tree

4 files changed

+35
-7
lines changed

4 files changed

+35
-7
lines changed

src/compiler.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6609,17 +6609,17 @@ export class Compiler extends DiagnosticEmitter {
66096609
for (let i = 0, k = overrideInstances.length; i < k; ++i) {
66106610
let overrideInstance = overrideInstances[i];
66116611
if (!overrideInstance.is(CommonFlags.Compiled)) continue; // errored
6612-
let overrideType = overrideInstance.type;
6613-
let originalType = instance.type;
6614-
if (!overrideType.isAssignableTo(originalType)) {
6612+
let overrideSignature = overrideInstance.signature;
6613+
let originalSignature = instance.signature;
6614+
if (!overrideSignature.isAssignableTo(originalSignature, true)) {
66156615
this.error(
66166616
DiagnosticCode.Type_0_is_not_assignable_to_type_1,
6617-
overrideInstance.identifierNode.range, overrideType.toString(), originalType.toString()
6617+
overrideInstance.identifierNode.range,
6618+
overrideSignature.toString(), originalSignature.toString()
66186619
);
66196620
continue;
66206621
}
66216622
// TODO: additional optional parameters are not permitted by `isAssignableTo` yet
6622-
let overrideSignature = overrideInstance.signature;
66236623
let overrideParameterTypes = overrideSignature.parameterTypes;
66246624
let overrideNumParameters = overrideParameterTypes.length;
66256625
let paramExprs = new Array<ExpressionRef>(1 + overrideNumParameters);

src/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,9 +1061,9 @@ export class Signature {
10611061
return false;
10621062
}
10631063
} else {
1064-
// check `this` type (invariant)
1064+
// check `this` type (contravariant)
10651065
if (thisThisType) {
1066-
if (targetThisType != targetThisType) return false;
1066+
if (!targetThisType || !targetThisType.isAssignableTo(thisThisType)) return false;
10671067
} else if (targetThisType) {
10681068
return false;
10691069
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"stderr": [
3+
"TS2322: Type '() => void' is not assignable to type '(this: class-method-as-parameter/A) => void'.",
4+
"TS2322: Type '(this: class-method-as-parameter/B) => void' is not assignable to type '(this: class-method-as-parameter/A) => void'.",
5+
"EOF"
6+
]
7+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
class A {
2+
foo(): void {}
3+
}
4+
5+
class B extends A {
6+
foo(): void {}
7+
}
8+
9+
function foo(): void {}
10+
11+
function consumeA(callback: (this: A) => void): void {}
12+
function consumeB(callback: (this: B) => void): void {}
13+
14+
const a = new A();
15+
const b = new B();
16+
17+
consumeB(a.foo); // shouldn't error
18+
consumeA(foo); // should error
19+
consumeA(b.foo); // should error
20+
21+
ERROR("EOF");

0 commit comments

Comments
 (0)