Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit eb105ef

Browse filesBrowse files
sandersnsandersn
authored
Avoid circular reference in this-property assignments (microsoft#37827)
* Avoid circular reference in this-property assignments To do this, don't check this-property assigments that have the this-property of the lhs appearing somewhere on the rhs: ```js class C { m() { this.x = 12 this.x = this.x + this.y } } ``` I tried suppressing the circularity error, but because we cache the first type discovered for a property, this still results in an implicit any for `x` in the previous example. It just doesn't have an error. Fixes microsoft#35099 * Add test case + rename function * Use isMatchingReference
1 parent 795a5c8 commit eb105ef
Copy full SHA for eb105ef

5 files changed

+172Lines changed: 172 additions & 0 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/compiler/checker.ts‎

Copy file name to clipboardExpand all lines: src/compiler/checker.ts
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7635,6 +7635,9 @@ namespace ts {
76357635
}
76367636
return anyType;
76377637
}
7638+
if (containsSameNamedThisProperty(expression.left, expression.right)) {
7639+
return anyType;
7640+
}
76387641
const type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right));
76397642
if (type.flags & TypeFlags.Object &&
76407643
kind === AssignmentDeclarationKind.ModuleExports &&
@@ -7673,6 +7676,12 @@ namespace ts {
76737676
return type;
76747677
}
76757678

7679+
function containsSameNamedThisProperty(thisProperty: Expression, expression: Expression) {
7680+
return isPropertyAccessExpression(thisProperty)
7681+
&& thisProperty.expression.kind === SyntaxKind.ThisKeyword
7682+
&& forEachChildRecursively(expression, n => isMatchingReference(thisProperty, n));
7683+
}
7684+
76767685
function isDeclarationInConstructor(expression: Expression) {
76777686
const thisContainer = getThisContainer(expression, /*includeArrowFunctions*/ false);
76787687
// Properties defined in a constructor (or base constructor, or javascript constructor function) don't get undefined added.
Collapse file

‎tests/baselines/reference/thisPropertyAssignmentCircular.js‎

Copy file name to clipboard
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//// [thisPropertyAssignmentCircular.js]
2+
export class Foo {
3+
constructor() {
4+
this.foo = "Hello";
5+
}
6+
slicey() {
7+
this.foo = this.foo.slice();
8+
}
9+
m() {
10+
this.foo
11+
}
12+
}
13+
14+
/** @class */
15+
function C() {
16+
this.x = 0;
17+
this.x = function() { this.x.toString(); }
18+
}
19+
20+
21+
22+
23+
//// [thisPropertyAssignmentCircular.d.ts]
24+
export class Foo {
25+
foo: string;
26+
slicey(): void;
27+
m(): void;
28+
}
Collapse file

‎tests/baselines/reference/thisPropertyAssignmentCircular.symbols‎

Copy file name to clipboard
+51Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
=== tests/cases/conformance/salsa/thisPropertyAssignmentCircular.js ===
2+
export class Foo {
3+
>Foo : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
4+
5+
constructor() {
6+
this.foo = "Hello";
7+
>this.foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
8+
>this : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
9+
>foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
10+
}
11+
slicey() {
12+
>slicey : Symbol(Foo.slicey, Decl(thisPropertyAssignmentCircular.js, 3, 5))
13+
14+
this.foo = this.foo.slice();
15+
>this.foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
16+
>this : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
17+
>foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
18+
>this.foo.slice : Symbol(String.slice, Decl(lib.es5.d.ts, --, --))
19+
>this.foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
20+
>this : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
21+
>foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
22+
>slice : Symbol(String.slice, Decl(lib.es5.d.ts, --, --))
23+
}
24+
m() {
25+
>m : Symbol(Foo.m, Decl(thisPropertyAssignmentCircular.js, 6, 5))
26+
27+
this.foo
28+
>this.foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
29+
>this : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
30+
>foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
31+
}
32+
}
33+
34+
/** @class */
35+
function C() {
36+
>C : Symbol(C, Decl(thisPropertyAssignmentCircular.js, 10, 1))
37+
38+
this.x = 0;
39+
>this.x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
40+
>this : Symbol(C, Decl(thisPropertyAssignmentCircular.js, 10, 1))
41+
>x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
42+
43+
this.x = function() { this.x.toString(); }
44+
>this.x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
45+
>this : Symbol(C, Decl(thisPropertyAssignmentCircular.js, 10, 1))
46+
>x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
47+
>this.x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
48+
>this : Symbol(C, Decl(thisPropertyAssignmentCircular.js, 10, 1))
49+
>x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
50+
}
51+
Collapse file

‎tests/baselines/reference/thisPropertyAssignmentCircular.types‎

Copy file name to clipboard
+62Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
=== tests/cases/conformance/salsa/thisPropertyAssignmentCircular.js ===
2+
export class Foo {
3+
>Foo : Foo
4+
5+
constructor() {
6+
this.foo = "Hello";
7+
>this.foo = "Hello" : "Hello"
8+
>this.foo : string
9+
>this : this
10+
>foo : string
11+
>"Hello" : "Hello"
12+
}
13+
slicey() {
14+
>slicey : () => void
15+
16+
this.foo = this.foo.slice();
17+
>this.foo = this.foo.slice() : string
18+
>this.foo : string
19+
>this : this
20+
>foo : string
21+
>this.foo.slice() : string
22+
>this.foo.slice : (start?: number, end?: number) => string
23+
>this.foo : string
24+
>this : this
25+
>foo : string
26+
>slice : (start?: number, end?: number) => string
27+
}
28+
m() {
29+
>m : () => void
30+
31+
this.foo
32+
>this.foo : string
33+
>this : this
34+
>foo : string
35+
}
36+
}
37+
38+
/** @class */
39+
function C() {
40+
>C : typeof C
41+
42+
this.x = 0;
43+
>this.x = 0 : 0
44+
>this.x : any
45+
>this : this
46+
>x : any
47+
>0 : 0
48+
49+
this.x = function() { this.x.toString(); }
50+
>this.x = function() { this.x.toString(); } : () => void
51+
>this.x : any
52+
>this : this
53+
>x : any
54+
>function() { this.x.toString(); } : () => void
55+
>this.x.toString() : any
56+
>this.x.toString : any
57+
>this.x : any
58+
>this : this
59+
>x : any
60+
>toString : any
61+
}
62+
Collapse file

‎tests/cases/conformance/salsa/thisPropertyAssignmentCircular.ts‎

Copy file name to clipboard
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// @allowJs: true
2+
// @checkJs: true
3+
// @declaration: true
4+
// @emitDeclarationOnly: true
5+
// @filename: thisPropertyAssignmentCircular.js
6+
export class Foo {
7+
constructor() {
8+
this.foo = "Hello";
9+
}
10+
slicey() {
11+
this.foo = this.foo.slice();
12+
}
13+
m() {
14+
this.foo
15+
}
16+
}
17+
18+
/** @class */
19+
function C() {
20+
this.x = 0;
21+
this.x = function() { this.x.toString(); }
22+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.