From 0055da60015a72ba43151f47141308aec423eee7 Mon Sep 17 00:00:00 2001 From: Changqing Jing Date: Mon, 31 Jul 2023 14:18:13 +0800 Subject: [PATCH 1/3] bug fix 2727: class member function type is assigned to a normal function type --- src/compiler.ts | 4 +- src/types.ts | 28 +++++----- .../class-member-function-as-parameter.json | 8 +++ .../class-member-function-as-parameter.ts | 52 +++++++++++++++++++ 4 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 tests/compiler/class-member-function-as-parameter.json create mode 100644 tests/compiler/class-member-function-as-parameter.ts diff --git a/src/compiler.ts b/src/compiler.ts index a2b313603b..43ffd7ba72 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6611,7 +6611,9 @@ export class Compiler extends DiagnosticEmitter { if (!overrideInstance.is(CommonFlags.Compiled)) continue; // errored let overrideType = overrideInstance.type; let originalType = instance.type; - if (!overrideType.isAssignableTo(originalType)) { + + assert(originalType.getSignature() != null && overrideType.getSignature() != null); + if (!(overrideType.getSignature() as Signature).isAssignableTo(originalType.getSignature() as Signature, true)) { this.error( DiagnosticCode.Type_0_is_not_assignable_to_type_1, overrideInstance.identifierNode.range, overrideType.toString(), originalType.toString() diff --git a/src/types.ts b/src/types.ts index 4aef00a5b2..37effad4de 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1051,21 +1051,23 @@ export class Signature { isAssignableTo(target: Signature, checkCompatibleOverride: bool = false): bool { let thisThisType = this.thisType; let targetThisType = target.thisType; - if (checkCompatibleOverride) { - // check kind of `this` type - if (thisThisType) { - if (!targetThisType || !thisThisType.canExtendOrImplement(targetThisType)) { + + if ( + (thisThisType == null && targetThisType != null) || + (thisThisType != null && targetThisType == null) + ) { + return false; + }else if(thisThisType != null && targetThisType != null){ + if(checkCompatibleOverride){ + // check kind of `this` type + if(!thisThisType.canExtendOrImplement(targetThisType)){ + return false; + } + }else{ + // check `this` type (invariant) + if(!targetThisType.isAssignableTo(thisThisType)){ return false; } - } else if (targetThisType) { - return false; - } - } else { - // check `this` type (invariant) - if (thisThisType) { - if (targetThisType != targetThisType) return false; - } else if (targetThisType) { - return false; } } diff --git a/tests/compiler/class-member-function-as-parameter.json b/tests/compiler/class-member-function-as-parameter.json new file mode 100644 index 0000000000..60bb912d81 --- /dev/null +++ b/tests/compiler/class-member-function-as-parameter.json @@ -0,0 +1,8 @@ +{ + "asc_flags": [], + "stderr": [ + "TS2322: Type '(this: class-member-function-as-parameter/C, i32) => i32' is not assignable to type '(i32) => i32'.", + "TS2322: Type '() => void' is not assignable to type '(this: class-member-function-as-parameter/B) => void'.", + "EOF" + ] +} diff --git a/tests/compiler/class-member-function-as-parameter.ts b/tests/compiler/class-member-function-as-parameter.ts new file mode 100644 index 0000000000..2bbce497bf --- /dev/null +++ b/tests/compiler/class-member-function-as-parameter.ts @@ -0,0 +1,52 @@ +class C { + aa: i32 = 1; + callback(a: i32): i32 { + return this.aa + a + 3; + } +} + +function expectCallback(c1: (arg0: i32) => i32): i32 { + return c1(4); +} + +export function fut(): i32 { + const c1 = new C(); + return expectCallback(c1.callback); +} + +fut(); + +class A { + foo(): void { + console.log("A"); + } +} + +class B extends A { + foo(): void { + console.log("B"); + } +} + +function foo(): void { + console.log("nothing"); +} + +function consume(callback: (this: B) => void): void { + const b = new B(); + callback.call(b); +} + +export function testNull(): void { + consume(foo); // This should (and does) error; this is fine. +} + +export function testA(): void { + const a = new A(); + consume(a.foo); // This shouldn't error +} + +testNull(); +testA(); + +ERROR("EOF"); \ No newline at end of file From 75e3d5e43cee65d01974073748534f110206538c Mon Sep 17 00:00:00 2001 From: Changqing Jing Date: Tue, 1 Aug 2023 09:50:10 +0800 Subject: [PATCH 2/3] fix according to PR comments --- src/compiler.ts | 11 ++-- src/types.ts | 23 +++----- .../class-member-function-as-parameter.json | 4 +- .../class-member-function-as-parameter.ts | 59 ++++--------------- 4 files changed, 25 insertions(+), 72 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index 43ffd7ba72..3dfdcabd6e 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6609,19 +6609,18 @@ export class Compiler extends DiagnosticEmitter { for (let i = 0, k = overrideInstances.length; i < k; ++i) { let overrideInstance = overrideInstances[i]; if (!overrideInstance.is(CommonFlags.Compiled)) continue; // errored - let overrideType = overrideInstance.type; - let originalType = instance.type; - assert(originalType.getSignature() != null && overrideType.getSignature() != null); - if (!(overrideType.getSignature() as Signature).isAssignableTo(originalType.getSignature() as Signature, true)) { + const overrideSignature = overrideInstance.signature; + const originalSignature = instance.signature; + + if (!overrideSignature.isAssignableTo(originalSignature, true)) { this.error( DiagnosticCode.Type_0_is_not_assignable_to_type_1, - overrideInstance.identifierNode.range, overrideType.toString(), originalType.toString() + overrideInstance.identifierNode.range, overrideSignature.toString(), originalSignature.toString() ); continue; } // TODO: additional optional parameters are not permitted by `isAssignableTo` yet - let overrideSignature = overrideInstance.signature; let overrideParameterTypes = overrideSignature.parameterTypes; let overrideNumParameters = overrideParameterTypes.length; let paramExprs = new Array(1 + overrideNumParameters); diff --git a/src/types.ts b/src/types.ts index 37effad4de..fdaa22f278 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1052,23 +1052,14 @@ export class Signature { let thisThisType = this.thisType; let targetThisType = target.thisType; - if ( - (thisThisType == null && targetThisType != null) || - (thisThisType != null && targetThisType == null) - ) { - return false; - }else if(thisThisType != null && targetThisType != null){ - if(checkCompatibleOverride){ - // check kind of `this` type - if(!thisThisType.canExtendOrImplement(targetThisType)){ - return false; - } - }else{ - // check `this` type (invariant) - if(!targetThisType.isAssignableTo(thisThisType)){ - return false; - } + if (thisThisType != null && targetThisType != null){ + const compatibleThisType = checkCompatibleOverride ? thisThisType.canExtendOrImplement(targetThisType) + : targetThisType.isAssignableTo(thisThisType); + if (!compatibleThisType) { + return false; } + }else if (thisThisType || targetThisType){ + return false; } // check rest parameter diff --git a/tests/compiler/class-member-function-as-parameter.json b/tests/compiler/class-member-function-as-parameter.json index 60bb912d81..38ccfe0479 100644 --- a/tests/compiler/class-member-function-as-parameter.json +++ b/tests/compiler/class-member-function-as-parameter.json @@ -1,8 +1,8 @@ { "asc_flags": [], "stderr": [ - "TS2322: Type '(this: class-member-function-as-parameter/C, i32) => i32' is not assignable to type '(i32) => i32'.", - "TS2322: Type '() => void' is not assignable to type '(this: class-member-function-as-parameter/B) => void'.", + "TS2322: Type '() => void' is not assignable to type '(this: class-member-function-as-parameter/A) => void'.", + "TS2322: Type '(this: class-member-function-as-parameter/B) => void' is not assignable to type '(this: class-member-function-as-parameter/A) => void'.", "EOF" ] } diff --git a/tests/compiler/class-member-function-as-parameter.ts b/tests/compiler/class-member-function-as-parameter.ts index 2bbce497bf..ec5b591a64 100644 --- a/tests/compiler/class-member-function-as-parameter.ts +++ b/tests/compiler/class-member-function-as-parameter.ts @@ -1,52 +1,15 @@ -class C { - aa: i32 = 1; - callback(a: i32): i32 { - return this.aa + a + 3; - } -} - -function expectCallback(c1: (arg0: i32) => i32): i32 { - return c1(4); -} - -export function fut(): i32 { - const c1 = new C(); - return expectCallback(c1.callback); -} - -fut(); - class A { - foo(): void { - console.log("A"); - } + foo(): void { } } - class B extends A { - foo(): void { - console.log("B"); - } -} - -function foo(): void { - console.log("nothing"); -} - -function consume(callback: (this: B) => void): void { - const b = new B(); - callback.call(b); -} - -export function testNull(): void { - consume(foo); // This should (and does) error; this is fine. -} - -export function testA(): void { - const a = new A(); - consume(a.foo); // This shouldn't error -} - -testNull(); -testA(); - + foo(): void { } +} +function foo(): void { } +function consumeA(callback: (this: A) => void): void { } +function consumeB(callback: (this: B) => void): void { } +const a = new A(); +const b = new B(); +consumeB(a.foo); // shouldn't error +consumeA(foo); // should error +consumeA(b.foo); // should error ERROR("EOF"); \ No newline at end of file From b74c7de7cea28c0dee0e9479fcc406d3f16e0e09 Mon Sep 17 00:00:00 2001 From: Changqing Jing Date: Wed, 2 Aug 2023 13:38:07 +0800 Subject: [PATCH 3/3] Refine format according to code review --- src/compiler.ts | 4 ++-- src/types.ts | 11 +++++------ tests/compiler/class-member-function-as-parameter.ts | 5 +++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index 3dfdcabd6e..6955383478 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6610,8 +6610,8 @@ export class Compiler extends DiagnosticEmitter { let overrideInstance = overrideInstances[i]; if (!overrideInstance.is(CommonFlags.Compiled)) continue; // errored - const overrideSignature = overrideInstance.signature; - const originalSignature = instance.signature; + let overrideSignature = overrideInstance.signature; + let originalSignature = instance.signature; if (!overrideSignature.isAssignableTo(originalSignature, true)) { this.error( diff --git a/src/types.ts b/src/types.ts index fdaa22f278..675e9c7ee7 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1052,13 +1052,12 @@ export class Signature { let thisThisType = this.thisType; let targetThisType = target.thisType; - if (thisThisType != null && targetThisType != null){ - const compatibleThisType = checkCompatibleOverride ? thisThisType.canExtendOrImplement(targetThisType) + if (thisThisType && targetThisType) { + const compatibleThisType = checkCompatibleOverride + ? thisThisType.canExtendOrImplement(targetThisType) : targetThisType.isAssignableTo(thisThisType); - if (!compatibleThisType) { - return false; - } - }else if (thisThisType || targetThisType){ + if (!compatibleThisType) return false; + } else if (thisThisType || targetThisType) { return false; } diff --git a/tests/compiler/class-member-function-as-parameter.ts b/tests/compiler/class-member-function-as-parameter.ts index ec5b591a64..611f0fbf03 100644 --- a/tests/compiler/class-member-function-as-parameter.ts +++ b/tests/compiler/class-member-function-as-parameter.ts @@ -1,14 +1,19 @@ class A { foo(): void { } } + class B extends A { foo(): void { } } + function foo(): void { } + function consumeA(callback: (this: A) => void): void { } function consumeB(callback: (this: B) => void): void { } + const a = new A(); const b = new B(); + consumeB(a.foo); // shouldn't error consumeA(foo); // should error consumeA(b.foo); // should error