Skip to content

Commit

Permalink
Avoid circular reference in this-property assignments (#37827)
Browse files Browse the repository at this point in the history
* 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 #35099

* Add test case + rename function

* Use isMatchingReference
  • Loading branch information
sandersn authored Apr 10, 2020
1 parent 795a5c8 commit eb105ef
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7635,6 +7635,9 @@ namespace ts {
}
return anyType;
}
if (containsSameNamedThisProperty(expression.left, expression.right)) {
return anyType;
}
const type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right));
if (type.flags & TypeFlags.Object &&
kind === AssignmentDeclarationKind.ModuleExports &&
Expand Down Expand Up @@ -7673,6 +7676,12 @@ namespace ts {
return type;
}

function containsSameNamedThisProperty(thisProperty: Expression, expression: Expression) {
return isPropertyAccessExpression(thisProperty)
&& thisProperty.expression.kind === SyntaxKind.ThisKeyword
&& forEachChildRecursively(expression, n => isMatchingReference(thisProperty, n));
}

function isDeclarationInConstructor(expression: Expression) {
const thisContainer = getThisContainer(expression, /*includeArrowFunctions*/ false);
// Properties defined in a constructor (or base constructor, or javascript constructor function) don't get undefined added.
Expand Down
28 changes: 28 additions & 0 deletions tests/baselines/reference/thisPropertyAssignmentCircular.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//// [thisPropertyAssignmentCircular.js]
export class Foo {
constructor() {
this.foo = "Hello";
}
slicey() {
this.foo = this.foo.slice();
}
m() {
this.foo
}
}

/** @class */
function C() {
this.x = 0;
this.x = function() { this.x.toString(); }
}




//// [thisPropertyAssignmentCircular.d.ts]
export class Foo {
foo: string;
slicey(): void;
m(): void;
}
51 changes: 51 additions & 0 deletions tests/baselines/reference/thisPropertyAssignmentCircular.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
=== tests/cases/conformance/salsa/thisPropertyAssignmentCircular.js ===
export class Foo {
>Foo : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))

constructor() {
this.foo = "Hello";
>this.foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
>this : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
>foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
}
slicey() {
>slicey : Symbol(Foo.slicey, Decl(thisPropertyAssignmentCircular.js, 3, 5))

this.foo = this.foo.slice();
>this.foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
>this : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
>foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
>this.foo.slice : Symbol(String.slice, Decl(lib.es5.d.ts, --, --))
>this.foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
>this : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
>foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
>slice : Symbol(String.slice, Decl(lib.es5.d.ts, --, --))
}
m() {
>m : Symbol(Foo.m, Decl(thisPropertyAssignmentCircular.js, 6, 5))

this.foo
>this.foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
>this : Symbol(Foo, Decl(thisPropertyAssignmentCircular.js, 0, 0))
>foo : Symbol(Foo.foo, Decl(thisPropertyAssignmentCircular.js, 1, 19), Decl(thisPropertyAssignmentCircular.js, 4, 14))
}
}

/** @class */
function C() {
>C : Symbol(C, Decl(thisPropertyAssignmentCircular.js, 10, 1))

this.x = 0;
>this.x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
>this : Symbol(C, Decl(thisPropertyAssignmentCircular.js, 10, 1))
>x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))

this.x = function() { this.x.toString(); }
>this.x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
>this : Symbol(C, Decl(thisPropertyAssignmentCircular.js, 10, 1))
>x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
>this.x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
>this : Symbol(C, Decl(thisPropertyAssignmentCircular.js, 10, 1))
>x : Symbol(C.x, Decl(thisPropertyAssignmentCircular.js, 13, 14), Decl(thisPropertyAssignmentCircular.js, 14, 15))
}

62 changes: 62 additions & 0 deletions tests/baselines/reference/thisPropertyAssignmentCircular.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
=== tests/cases/conformance/salsa/thisPropertyAssignmentCircular.js ===
export class Foo {
>Foo : Foo

constructor() {
this.foo = "Hello";
>this.foo = "Hello" : "Hello"
>this.foo : string
>this : this
>foo : string
>"Hello" : "Hello"
}
slicey() {
>slicey : () => void

this.foo = this.foo.slice();
>this.foo = this.foo.slice() : string
>this.foo : string
>this : this
>foo : string
>this.foo.slice() : string
>this.foo.slice : (start?: number, end?: number) => string
>this.foo : string
>this : this
>foo : string
>slice : (start?: number, end?: number) => string
}
m() {
>m : () => void

this.foo
>this.foo : string
>this : this
>foo : string
}
}

/** @class */
function C() {
>C : typeof C

this.x = 0;
>this.x = 0 : 0
>this.x : any
>this : this
>x : any
>0 : 0

this.x = function() { this.x.toString(); }
>this.x = function() { this.x.toString(); } : () => void
>this.x : any
>this : this
>x : any
>function() { this.x.toString(); } : () => void
>this.x.toString() : any
>this.x.toString : any
>this.x : any
>this : this
>x : any
>toString : any
}

22 changes: 22 additions & 0 deletions tests/cases/conformance/salsa/thisPropertyAssignmentCircular.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// @allowJs: true
// @checkJs: true
// @declaration: true
// @emitDeclarationOnly: true
// @filename: thisPropertyAssignmentCircular.js
export class Foo {
constructor() {
this.foo = "Hello";
}
slicey() {
this.foo = this.foo.slice();
}
m() {
this.foo
}
}

/** @class */
function C() {
this.x = 0;
this.x = function() { this.x.toString(); }
}

0 comments on commit eb105ef

Please sign in to comment.