Skip to content

Commit d53b3ac

Browse files
sandersnsandersn
authored
Merge pull request #15032 from Microsoft/fix-anon-class-this-reference-caching
Do not walk up control flow containers for `this`
2 parents 8a599f6 + 6e149dd commit d53b3ac

8 files changed

+116
-24
lines changed

‎src/compiler/checker.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -11067,7 +11067,7 @@ namespace ts {
1106711067
else if (flow.flags & FlowFlags.Start) {
1106811068
// Check if we should continue with the control flow of the containing function.
1106911069
const container = (<FlowStart>flow).container;
11070-
if (container && container !== flowContainer && reference.kind !== SyntaxKind.PropertyAccessExpression) {
11070+
if (container && container !== flowContainer && reference.kind !== SyntaxKind.PropertyAccessExpression && reference.kind !== SyntaxKind.ThisKeyword) {
1107111071
flow = container.flowNode;
1107211072
continue;
1107311073
}

‎tests/baselines/reference/anonymousClassExpression2.errors.txt

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
tests/cases/compiler/anonymousClassExpression2.ts(13,18): error TS2339: Property 'methodA' does not exist on type 'B'.
2+
3+
4+
==== tests/cases/compiler/anonymousClassExpression2.ts (1 errors) ====
5+
// Fixes #14860
6+
// note: repros with `while (0);` too
7+
// but it's less inscrutable and more obvious to put it *inside* the loop
8+
while (0) {
9+
class A {
10+
methodA() {
11+
this; //note: a this reference of some kind is required to trigger the bug
12+
}
13+
}
14+
15+
class B {
16+
methodB() {
17+
this.methodA; // error
18+
~~~~~~~
19+
!!! error TS2339: Property 'methodA' does not exist on type 'B'.
20+
this.methodB; // ok
21+
}
22+
}
23+
}
24+

‎tests/baselines/reference/anonymousClassExpression2.js

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
//// [anonymousClassExpression2.ts]
2+
// Fixes #14860
3+
// note: repros with `while (0);` too
4+
// but it's less inscrutable and more obvious to put it *inside* the loop
5+
while (0) {
6+
class A {
7+
methodA() {
8+
this; //note: a this reference of some kind is required to trigger the bug
9+
}
10+
}
11+
12+
class B {
13+
methodB() {
14+
this.methodA; // error
15+
this.methodB; // ok
16+
}
17+
}
18+
}
19+
20+
21+
//// [anonymousClassExpression2.js]
22+
// Fixes #14860
23+
// note: repros with `while (0);` too
24+
// but it's less inscrutable and more obvious to put it *inside* the loop
25+
while (0) {
26+
var A = (function () {
27+
function A() {
28+
}
29+
A.prototype.methodA = function () {
30+
this; //note: a this reference of some kind is required to trigger the bug
31+
};
32+
return A;
33+
}());
34+
var B = (function () {
35+
function B() {
36+
}
37+
B.prototype.methodB = function () {
38+
this.methodA; // error
39+
this.methodB; // ok
40+
};
41+
return B;
42+
}());
43+
}

‎tests/baselines/reference/narrowedConstInMethod.js

+8-5
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
//// [narrowedConstInMethod.ts]
2+
// Fixes #10501, possibly null 'x'
23
function f() {
34
const x: string | null = <any>{};
45
if (x !== null) {
56
return {
6-
bar() { return x.length; } // Error: possibly null x
7+
bar() { return x.length; } // ok
78
};
89
}
910
}
@@ -12,17 +13,19 @@ function f2() {
1213
const x: string | null = <any>{};
1314
if (x !== null) {
1415
return class {
15-
bar() { return x.length; } // Error: possibly null x
16+
bar() { return x.length; } // ok
1617
};
1718
}
18-
}
19+
}
20+
1921

2022
//// [narrowedConstInMethod.js]
23+
// Fixes #10501, possibly null 'x'
2124
function f() {
2225
var x = {};
2326
if (x !== null) {
2427
return {
25-
bar: function () { return x.length; } // Error: possibly null x
28+
bar: function () { return x.length; } // ok
2629
};
2730
}
2831
}
@@ -32,7 +35,7 @@ function f2() {
3235
return (function () {
3336
function class_1() {
3437
}
35-
class_1.prototype.bar = function () { return x.length; }; // Error: possibly null x
38+
class_1.prototype.bar = function () { return x.length; }; // ok
3639
return class_1;
3740
}());
3841
}

‎tests/baselines/reference/narrowedConstInMethod.symbols

+13-11
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,42 @@
11
=== tests/cases/compiler/narrowedConstInMethod.ts ===
2+
// Fixes #10501, possibly null 'x'
23
function f() {
34
>f : Symbol(f, Decl(narrowedConstInMethod.ts, 0, 0))
45

56
const x: string | null = <any>{};
6-
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 1, 9))
7+
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 2, 9))
78

89
if (x !== null) {
9-
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 1, 9))
10+
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 2, 9))
1011

1112
return {
12-
bar() { return x.length; } // Error: possibly null x
13-
>bar : Symbol(bar, Decl(narrowedConstInMethod.ts, 3, 16))
13+
bar() { return x.length; } // ok
14+
>bar : Symbol(bar, Decl(narrowedConstInMethod.ts, 4, 16))
1415
>x.length : Symbol(String.length, Decl(lib.d.ts, --, --))
15-
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 1, 9))
16+
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 2, 9))
1617
>length : Symbol(String.length, Decl(lib.d.ts, --, --))
1718

1819
};
1920
}
2021
}
2122

2223
function f2() {
23-
>f2 : Symbol(f2, Decl(narrowedConstInMethod.ts, 7, 1))
24+
>f2 : Symbol(f2, Decl(narrowedConstInMethod.ts, 8, 1))
2425

2526
const x: string | null = <any>{};
26-
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 10, 9))
27+
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 11, 9))
2728

2829
if (x !== null) {
29-
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 10, 9))
30+
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 11, 9))
3031

3132
return class {
32-
bar() { return x.length; } // Error: possibly null x
33-
>bar : Symbol((Anonymous class).bar, Decl(narrowedConstInMethod.ts, 12, 22))
33+
bar() { return x.length; } // ok
34+
>bar : Symbol((Anonymous class).bar, Decl(narrowedConstInMethod.ts, 13, 22))
3435
>x.length : Symbol(String.length, Decl(lib.d.ts, --, --))
35-
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 10, 9))
36+
>x : Symbol(x, Decl(narrowedConstInMethod.ts, 11, 9))
3637
>length : Symbol(String.length, Decl(lib.d.ts, --, --))
3738

3839
};
3940
}
4041
}
42+

‎tests/baselines/reference/narrowedConstInMethod.types

+6-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
=== tests/cases/compiler/narrowedConstInMethod.ts ===
2+
// Fixes #10501, possibly null 'x'
23
function f() {
34
>f : () => { bar(): number; } | undefined
45

@@ -14,9 +15,9 @@ function f() {
1415
>null : null
1516

1617
return {
17-
>{ bar() { return x.length; } // Error: possibly null x } : { bar(): number; }
18+
>{ bar() { return x.length; } // ok } : { bar(): number; }
1819

19-
bar() { return x.length; } // Error: possibly null x
20+
bar() { return x.length; } // ok
2021
>bar : () => number
2122
>x.length : number
2223
>x : string
@@ -41,9 +42,9 @@ function f2() {
4142
>null : null
4243

4344
return class {
44-
>class { bar() { return x.length; } // Error: possibly null x } : typeof (Anonymous class)
45+
>class { bar() { return x.length; } // ok } : typeof (Anonymous class)
4546

46-
bar() { return x.length; } // Error: possibly null x
47+
bar() { return x.length; } // ok
4748
>bar : () => number
4849
>x.length : number
4950
>x : string
@@ -52,3 +53,4 @@ function f2() {
5253
};
5354
}
5455
}
56+

‎tests/cases/compiler/anonymousClassExpression2.ts

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Fixes #14860
2+
// note: repros with `while (0);` too
3+
// but it's less inscrutable and more obvious to put it *inside* the loop
4+
while (0) {
5+
class A {
6+
methodA() {
7+
this; //note: a this reference of some kind is required to trigger the bug
8+
}
9+
}
10+
11+
class B {
12+
methodB() {
13+
this.methodA; // error
14+
this.methodB; // ok
15+
}
16+
}
17+
}

‎tests/cases/compiler/narrowedConstInMethod.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
// @strictNullChecks: true
22

3+
// Fixes #10501, possibly null 'x'
34
function f() {
45
const x: string | null = <any>{};
56
if (x !== null) {
67
return {
7-
bar() { return x.length; } // Error: possibly null x
8+
bar() { return x.length; } // ok
89
};
910
}
1011
}
@@ -13,7 +14,7 @@ function f2() {
1314
const x: string | null = <any>{};
1415
if (x !== null) {
1516
return class {
16-
bar() { return x.length; } // Error: possibly null x
17+
bar() { return x.length; } // ok
1718
};
1819
}
19-
}
20+
}

0 commit comments

Comments
 (0)