microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-07-05 21:23:37 -05:00
Code Issues Packages Projects Releases 212 Wiki Activity

Correctly track thisContainer for this-property-assignments in JS nested containers (#22779)

Browse Source 
* Track thisContainer for this-property-assignments in JS

Previously it would update on every block, not just those that could
bind `this`.

In addition, if the constructor symbol still can't be found, then no
binding happens. This is usually OK because people don't add new
properties in methods too often.

* Update additional baselines

* Add lib:dom to new test

* Address PR comments

* Correct new name for saveThisParentContainer
This commit is contained in:
Nathan Shively-Sanders
2018-03-22 09:54:43 -07:00
committed by GitHub
parent f3a1f165c2
commit 4462c159b1
12 changed files with 302 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/compiler/binder.ts
View File

@@ -118,7 +118,7 @@ namespace ts {
let languageVersion: ScriptTarget;
let parent: Node;
let container: Node;
let containerContainer: Node; // Container one level up
let thisParentContainer: Node; // Container one level up
let blockScopeContainer: Node;
let lastContainer: Node;
let seenThisKeyword: boolean;
@@ -186,7 +186,7 @@ namespace ts {
languageVersion = undefined;
parent = undefined;
container = undefined;
containerContainer = undefined;
thisParentContainer = undefined;
blockScopeContainer = undefined;
lastContainer = undefined;
seenThisKeyword = false;
@@ -479,11 +479,9 @@ namespace ts {
// and block-container. Then after we pop out of processing the children, we restore
// these saved values.
const saveContainer = container;
const saveContainerContainer = containerContainer;
const saveThisParentContainer = thisParentContainer;
const savedBlockScopeContainer = blockScopeContainer;
containerContainer = container;
// Depending on what kind of node this is, we may have to adjust the current container
// and block-container. If the current node is a container, then it is automatically
// considered the current block-container as well. Also, for containers that we know
@@ -502,6 +500,9 @@ namespace ts {
// for it. We must clear this so we don't accidentally move any stale data forward from
// a previous compilation.
if (containerFlags & ContainerFlags.IsContainer) {
if (node.kind !== SyntaxKind.ArrowFunction) {
thisParentContainer = container;
}
container = blockScopeContainer = node;
if (containerFlags & ContainerFlags.HasLocals) {
container.locals = createSymbolTable();
@@ -571,7 +572,7 @@ namespace ts {
}
container = saveContainer;
containerContainer = saveContainerContainer;
thisParentContainer = saveThisParentContainer;
blockScopeContainer = savedBlockScopeContainer;
}
@@ -2338,14 +2339,16 @@ namespace ts {
if (isBinaryExpression(thisContainer.parent) && thisContainer.parent.operatorToken.kind === SyntaxKind.EqualsToken) {
const l = thisContainer.parent.left;
if (isPropertyAccessEntityNameExpression(l) && isPrototypeAccess(l.expression)) {
constructorSymbol = getJSInitializerSymbolFromName(l.expression.expression, containerContainer);
constructorSymbol = getJSInitializerSymbolFromName(l.expression.expression, thisParentContainer);
}
}
// Declare a 'member' if the container is an ES5 class or ES6 constructor
constructorSymbol.members = constructorSymbol.members || createSymbolTable();
// It's acceptable for multiple 'this' assignments of the same identifier to occur
declareSymbol(constructorSymbol.members, constructorSymbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property);
if (constructorSymbol) {
// Declare a 'member' if the container is an ES5 class or ES6 constructor
constructorSymbol.members = constructorSymbol.members || createSymbolTable();
// It's acceptable for multiple 'this' assignments of the same identifier to occur
declareSymbol(constructorSymbol.members, constructorSymbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property);
}
break;
case SyntaxKind.Constructor:

35
tests/baselines/reference/typeFromPropertyAssignment20.symbols Normal file
View File

@@ -0,0 +1,35 @@
=== tests/cases/conformance/salsa/bluebird.js ===
!function outer (f) { return f }(
>outer : Symbol(outer, Decl(bluebird.js, 0, 1))
>f : Symbol(f, Decl(bluebird.js, 0, 17))
>f : Symbol(f, Decl(bluebird.js, 0, 17))
function inner () {
>inner : Symbol(inner, Decl(bluebird.js, 0, 33))
function Async() {
>Async : Symbol(Async, Decl(bluebird.js, 1, 23))
this._trampolineEnabled = true;
>_trampolineEnabled : Symbol(Async._trampolineEnabled, Decl(bluebird.js, 2, 26), Decl(bluebird.js, 7, 20))
}
Async.prototype.disableTrampolineIfNecessary = function dtin(b) {
>Async.prototype : Symbol(Async.disableTrampolineIfNecessary, Decl(bluebird.js, 4, 9))
>Async : Symbol(Async, Decl(bluebird.js, 1, 23))
>prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --))
>disableTrampolineIfNecessary : Symbol(Async.disableTrampolineIfNecessary, Decl(bluebird.js, 4, 9))
>dtin : Symbol(dtin, Decl(bluebird.js, 6, 54))
>b : Symbol(b, Decl(bluebird.js, 6, 69))
if (b) {
>b : Symbol(b, Decl(bluebird.js, 6, 69))
this._trampolineEnabled = false;
>this._trampolineEnabled : Symbol(Async._trampolineEnabled, Decl(bluebird.js, 2, 26), Decl(bluebird.js, 7, 20))
>this : Symbol(Async, Decl(bluebird.js, 1, 23))
>_trampolineEnabled : Symbol(Async._trampolineEnabled, Decl(bluebird.js, 2, 26), Decl(bluebird.js, 7, 20))
}
};
})

48
tests/baselines/reference/typeFromPropertyAssignment20.types Normal file
View File

@@ -0,0 +1,48 @@
=== tests/cases/conformance/salsa/bluebird.js ===
!function outer (f) { return f }(
>!function outer (f) { return f }( function inner () { function Async() { this._trampolineEnabled = true; } Async.prototype.disableTrampolineIfNecessary = function dtin(b) { if (b) { this._trampolineEnabled = false; } }; }) : boolean
>function outer (f) { return f }( function inner () { function Async() { this._trampolineEnabled = true; } Async.prototype.disableTrampolineIfNecessary = function dtin(b) { if (b) { this._trampolineEnabled = false; } }; }) : () => void
>function outer (f) { return f } : (f: () => void) => () => void
>outer : (f: () => void) => () => void
>f : () => void
>f : () => void
function inner () {
>function inner () { function Async() { this._trampolineEnabled = true; } Async.prototype.disableTrampolineIfNecessary = function dtin(b) { if (b) { this._trampolineEnabled = false; } }; } : () => void
>inner : () => void
function Async() {
>Async : () => void
this._trampolineEnabled = true;
>this._trampolineEnabled = true : true
>this._trampolineEnabled : any
>this : any
>_trampolineEnabled : any
>true : true
}
Async.prototype.disableTrampolineIfNecessary = function dtin(b) {
>Async.prototype.disableTrampolineIfNecessary = function dtin(b) { if (b) { this._trampolineEnabled = false; } } : (b: any) => void
>Async.prototype.disableTrampolineIfNecessary : any
>Async.prototype : any
>Async : () => void
>prototype : any
>disableTrampolineIfNecessary : any
>function dtin(b) { if (b) { this._trampolineEnabled = false; } } : (b: any) => void
>dtin : (b: any) => void
>b : any
if (b) {
>b : any
this._trampolineEnabled = false;
>this._trampolineEnabled = false : false
>this._trampolineEnabled : boolean
>this : { _trampolineEnabled: boolean; disableTrampolineIfNecessary: (b: any) => void; }
>_trampolineEnabled : boolean
>false : false
}
};
})

14
tests/baselines/reference/typeFromPropertyAssignment21.errors.txt Normal file
View File

@@ -0,0 +1,14 @@
tests/cases/conformance/salsa/chrome-devtools-DOMExtension.js(2,17): error TS2339: Property 'removeChildren' does not exist on type 'Event'.
tests/cases/conformance/salsa/chrome-devtools-DOMExtension.js(3,10): error TS2339: Property 'textContent' does not exist on type 'Event'.
==== tests/cases/conformance/salsa/chrome-devtools-DOMExtension.js (2 errors) ====
// Extend that DOM! (doesn't work, but shouldn't crash)
Event.prototype.removeChildren = function () {
~~~~~~~~~~~~~~
!!! error TS2339: Property 'removeChildren' does not exist on type 'Event'.
this.textContent = 'nope, not going to happen'
~~~~~~~~~~~
!!! error TS2339: Property 'textContent' does not exist on type 'Event'.
}

11
tests/baselines/reference/typeFromPropertyAssignment21.symbols Normal file
View File

@@ -0,0 +1,11 @@
=== tests/cases/conformance/salsa/chrome-devtools-DOMExtension.js ===
// Extend that DOM! (doesn't work, but shouldn't crash)
Event.prototype.removeChildren = function () {
>Event.prototype : Symbol(prototype, Decl(lib.dom.d.ts, --, --))
>Event : Symbol(Event, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --))
>prototype : Symbol(prototype, Decl(lib.dom.d.ts, --, --))
this.textContent = 'nope, not going to happen'
>this : Symbol(Event, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --))
}

19
tests/baselines/reference/typeFromPropertyAssignment21.types Normal file
View File

@@ -0,0 +1,19 @@
=== tests/cases/conformance/salsa/chrome-devtools-DOMExtension.js ===
// Extend that DOM! (doesn't work, but shouldn't crash)
Event.prototype.removeChildren = function () {
>Event.prototype.removeChildren = function () { this.textContent = 'nope, not going to happen'} : () => void
>Event.prototype.removeChildren : any
>Event.prototype : Event
>Event : { new (typeArg: string, eventInitDict?: EventInit): Event; prototype: Event; readonly AT_TARGET: number; readonly BUBBLING_PHASE: number; readonly CAPTURING_PHASE: number; readonly NONE: number; }
>prototype : Event
>removeChildren : any
>function () { this.textContent = 'nope, not going to happen'} : () => void
this.textContent = 'nope, not going to happen'
>this.textContent = 'nope, not going to happen' : "nope, not going to happen"
>this.textContent : any
>this : Event
>textContent : any
>'nope, not going to happen' : "nope, not going to happen"
}

21
tests/baselines/reference/typeFromPropertyAssignment22.errors.txt Normal file
View File

@@ -0,0 +1,21 @@
tests/cases/conformance/salsa/npm-install.js(12,1): error TS2322: Type 'string | number' is not assignable to type 'number | undefined'.
Type 'string' is not assignable to type 'number | undefined'.
==== tests/cases/conformance/salsa/npm-install.js (1 errors) ====
function Installer () {
this.args = 0
}
Installer.prototype.loadArgMetadata = function (next) {
// ArrowFunction isn't treated as a this-container
(args) => {
this.args = 'hi'
this.newProperty = 1
}
}
var i = new Installer()
i.newProperty = i.args // error, number | string =/> number | undefined
~~~~~~~~~~~~~
!!! error TS2322: Type 'string | number' is not assignable to type 'number | undefined'.
!!! error TS2322: Type 'string' is not assignable to type 'number | undefined'.

41
tests/baselines/reference/typeFromPropertyAssignment22.symbols Normal file
View File

@@ -0,0 +1,41 @@
=== tests/cases/conformance/salsa/npm-install.js ===
function Installer () {
>Installer : Symbol(Installer, Decl(npm-install.js, 0, 0))
this.args = 0
>args : Symbol(Installer.args, Decl(npm-install.js, 0, 23), Decl(npm-install.js, 5, 15))
}
Installer.prototype.loadArgMetadata = function (next) {
>Installer.prototype : Symbol(Installer.loadArgMetadata, Decl(npm-install.js, 2, 1))
>Installer : Symbol(Installer, Decl(npm-install.js, 0, 0))
>prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --))
>loadArgMetadata : Symbol(Installer.loadArgMetadata, Decl(npm-install.js, 2, 1))
>next : Symbol(next, Decl(npm-install.js, 3, 48))
// ArrowFunction isn't treated as a this-container
(args) => {
>args : Symbol(args, Decl(npm-install.js, 5, 5))
this.args = 'hi'
>this.args : Symbol(Installer.args, Decl(npm-install.js, 0, 23), Decl(npm-install.js, 5, 15))
>this : Symbol(Installer, Decl(npm-install.js, 0, 0))
>args : Symbol(Installer.args, Decl(npm-install.js, 0, 23), Decl(npm-install.js, 5, 15))
this.newProperty = 1
>this.newProperty : Symbol(Installer.newProperty, Decl(npm-install.js, 6, 24))
>this : Symbol(Installer, Decl(npm-install.js, 0, 0))
>newProperty : Symbol(Installer.newProperty, Decl(npm-install.js, 6, 24))
}
}
var i = new Installer()
>i : Symbol(i, Decl(npm-install.js, 10, 3))
>Installer : Symbol(Installer, Decl(npm-install.js, 0, 0))
i.newProperty = i.args // error, number | string =/> number | undefined
>i.newProperty : Symbol(Installer.newProperty, Decl(npm-install.js, 6, 24))
>i : Symbol(i, Decl(npm-install.js, 10, 3))
>newProperty : Symbol(Installer.newProperty, Decl(npm-install.js, 6, 24))
>i.args : Symbol(Installer.args, Decl(npm-install.js, 0, 23), Decl(npm-install.js, 5, 15))
>i : Symbol(i, Decl(npm-install.js, 10, 3))
>args : Symbol(Installer.args, Decl(npm-install.js, 0, 23), Decl(npm-install.js, 5, 15))

55
tests/baselines/reference/typeFromPropertyAssignment22.types Normal file
View File

@@ -0,0 +1,55 @@
=== tests/cases/conformance/salsa/npm-install.js ===
function Installer () {
>Installer : () => void
this.args = 0
>this.args = 0 : 0
>this.args : any
>this : any
>args : any
>0 : 0
}
Installer.prototype.loadArgMetadata = function (next) {
>Installer.prototype.loadArgMetadata = function (next) { // ArrowFunction isn't treated as a this-container (args) => { this.args = 'hi' this.newProperty = 1 }} : (next: any) => void
>Installer.prototype.loadArgMetadata : any
>Installer.prototype : any
>Installer : () => void
>prototype : any
>loadArgMetadata : any
>function (next) { // ArrowFunction isn't treated as a this-container (args) => { this.args = 'hi' this.newProperty = 1 }} : (next: any) => void
>next : any
// ArrowFunction isn't treated as a this-container
(args) => {
>(args) => { this.args = 'hi' this.newProperty = 1 } : (args: any) => void
>args : any
this.args = 'hi'
>this.args = 'hi' : "hi"
>this.args : string | number
>this : { args: string | number; loadArgMetadata: (next: any) => void; newProperty: number | undefined; }
>args : string | number
>'hi' : "hi"
this.newProperty = 1
>this.newProperty = 1 : 1
>this.newProperty : number | undefined
>this : { args: string | number; loadArgMetadata: (next: any) => void; newProperty: number | undefined; }
>newProperty : number | undefined
>1 : 1
}
}
var i = new Installer()
>i : { args: string | number; loadArgMetadata: (next: any) => void; newProperty: number | undefined; }
>new Installer() : { args: string | number; loadArgMetadata: (next: any) => void; newProperty: number | undefined; }
>Installer : () => void
i.newProperty = i.args // error, number | string =/> number | undefined
>i.newProperty = i.args : string | number
>i.newProperty : number | undefined
>i : { args: string | number; loadArgMetadata: (next: any) => void; newProperty: number | undefined; }
>newProperty : number | undefined
>i.args : string | number
>i : { args: string | number; loadArgMetadata: (next: any) => void; newProperty: number | undefined; }
>args : string | number

17
tests/cases/conformance/salsa/typeFromPropertyAssignment20.ts Normal file
View File

@@ -0,0 +1,17 @@
// @allowJs: true
// @checkJs: true
// @noEmit: true
// @Filename: bluebird.js
!function outer (f) { return f }(
function inner () {
function Async() {
this._trampolineEnabled = true;
}
Async.prototype.disableTrampolineIfNecessary = function dtin(b) {
if (b) {
this._trampolineEnabled = false;
}
};
})

10
tests/cases/conformance/salsa/typeFromPropertyAssignment21.ts Normal file
View File

@@ -0,0 +1,10 @@
// @allowJs: true
// @checkJs: true
// @noEmit: true
// @lib: dom, es5
// @Filename: chrome-devtools-DOMExtension.js
// Extend that DOM! (doesn't work, but shouldn't crash)
Event.prototype.removeChildren = function () {
this.textContent = 'nope, not going to happen'
}

17
tests/cases/conformance/salsa/typeFromPropertyAssignment22.ts Normal file
View File

@@ -0,0 +1,17 @@
// @allowJs: true
// @checkJs: true
// @noEmit: true
// @strictNullChecks: true
// @Filename: npm-install.js
function Installer () {
this.args = 0
}
Installer.prototype.loadArgMetadata = function (next) {
// ArrowFunction isn't treated as a this-container
(args) => {
this.args = 'hi'
this.newProperty = 1
}
}
var i = new Installer()
i.newProperty = i.args // error, number | string =/> number | undefined