Merge pull request microsoft#12893 from Microsoft/capturedThisFix

DanielRosenwasser · web-flow · commit 8e94d84953a0 · 2016-12-14T13:31:08.000-08:00
Avoid replacing last statement of derived constructors if 'this' is referenced.
diff --git a/src/compiler/transformers/es2015.ts b/src/compiler/transformers/es2015.ts
@@ -1024,8 +1024,11 @@ namespace ts {
                 }
             }
 
-            // Return the result if we have an immediate super() call on the last statement.
-            if (superCallExpression && statementOffset === ctorStatements.length - 1) {
+            // Return the result if we have an immediate super() call on the last statement,
+            // but only if the constructor itself doesn't use 'this' elsewhere.
+            if (superCallExpression
+                && statementOffset === ctorStatements.length - 1
+                && !(ctor.transformFlags & (TransformFlags.ContainsLexicalThis | TransformFlags.ContainsCapturedLexicalThis))) {
                 const returnStatement = createReturn(superCallExpression);
 
                 if (superCallExpression.kind !== SyntaxKind.BinaryExpression
diff --git a/tests/baselines/reference/arrowFunctionContexts.js b/tests/baselines/reference/arrowFunctionContexts.js
@@ -116,7 +116,8 @@ var Base = (function () {
 var Derived = (function (_super) {
     __extends(Derived, _super);
     function Derived() {
-        return _super.call(this, function () { return _this; }) || this;
+        var _this = _super.call(this, function () { return _this; }) || this;
+        return _this;
     }
     return Derived;
 }(Base));
@@ -157,7 +158,8 @@ var M2;
     var Derived = (function (_super) {
         __extends(Derived, _super);
         function Derived() {
-            return _super.call(this, function () { return _this; }) || this;
+            var _this = _super.call(this, function () { return _this; }) || this;
+            return _this;
         }
         return Derived;
     }(Base));
diff --git a/tests/baselines/reference/captureSuperPropertyAccessInSuperCall01.errors.txt b/tests/baselines/reference/captureSuperPropertyAccessInSuperCall01.errors.txt
@@ -0,0 +1,17 @@
+tests/cases/compiler/captureSuperPropertyAccessInSuperCall01.ts(9,24): error TS17011: 'super' must be called before accessing a property of 'super' in the constructor of a derived class.
+
+
+==== tests/cases/compiler/captureSuperPropertyAccessInSuperCall01.ts (1 errors) ====
+    class A {
+    	constructor(f: () => string) {
+    	}
+    	public blah(): string { return ""; }
+    }
+    
+    class B extends A {
+    	constructor() {
+    		super(() => { return super.blah(); })
+    		                     ~~~~~
+!!! error TS17011: 'super' must be called before accessing a property of 'super' in the constructor of a derived class.
+    	}
+    }
diff --git a/tests/baselines/reference/captureSuperPropertyAccessInSuperCall01.js b/tests/baselines/reference/captureSuperPropertyAccessInSuperCall01.js
@@ -0,0 +1,33 @@
+//// [captureSuperPropertyAccessInSuperCall01.ts]
+class A {
+	constructor(f: () => string) {
+	}
+	public blah(): string { return ""; }
+}
+
+class B extends A {
+	constructor() {
+		super(() => { return super.blah(); })
+	}
+}
+
+//// [captureSuperPropertyAccessInSuperCall01.js]
+var __extends = (this && this.__extends) || function (d, b) {
+    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
+    function __() { this.constructor = d; }
+    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
+};
+var A = (function () {
+    function A(f) {
+    }
+    A.prototype.blah = function () { return ""; };
+    return A;
+}());
+var B = (function (_super) {
+    __extends(B, _super);
+    function B() {
+        var _this = _super.call(this, function () { return _super.blah.call(_this); }) || this;
+        return _this;
+    }
+    return B;
+}(A));
diff --git a/tests/baselines/reference/captureThisInSuperCall.js b/tests/baselines/reference/captureThisInSuperCall.js
@@ -22,7 +22,8 @@ var A = (function () {
 var B = (function (_super) {
     __extends(B, _super);
     function B() {
-        return _super.call(this, { test: function () { return _this.someMethod(); } }) || this;
+        var _this = _super.call(this, { test: function () { return _this.someMethod(); } }) || this;
+        return _this;
     }
     B.prototype.someMethod = function () { };
     return B;
diff --git a/tests/baselines/reference/checkSuperCallBeforeThisAccessing5.js b/tests/baselines/reference/checkSuperCallBeforeThisAccessing5.js
@@ -25,7 +25,8 @@ var Based = (function () {
 var Derived = (function (_super) {
     __extends(Derived, _super);
     function Derived() {
-        return _super.call(this, this.x) || this;
+        var _this = _super.call(this, _this.x) || this;
+        return _this;
     }
     return Derived;
 }(Based));
diff --git a/tests/baselines/reference/checkSuperCallBeforeThisAccessing7.js b/tests/baselines/reference/checkSuperCallBeforeThisAccessing7.js
@@ -23,7 +23,8 @@ var Base = (function () {
 var Super = (function (_super) {
     __extends(Super, _super);
     function Super() {
-        return _super.call(this, (function () { return _this; })) || this;
+        var _this = _super.call(this, (function () { return _this; })) || this;
+        return _this;
     }
     return Super;
 }(Base));
diff --git a/tests/baselines/reference/derivedClassSuperCallsWithThisArg.js b/tests/baselines/reference/derivedClassSuperCallsWithThisArg.js
@@ -42,7 +42,8 @@ var Base = (function () {
 var Derived = (function (_super) {
     __extends(Derived, _super);
     function Derived() {
-        return _super.call(this, _this) || this;
+        var _this = _super.call(this, _this) || this;
+        return _this;
     }
     return Derived;
 }(Base));
diff --git a/tests/baselines/reference/superCallBeforeThisAccessing2.js b/tests/baselines/reference/superCallBeforeThisAccessing2.js
@@ -24,7 +24,8 @@ var Base = (function () {
 var D = (function (_super) {
     __extends(D, _super);
     function D() {
-        return _super.call(this, function () { _this._t; }) || this;
+        var _this = _super.call(this, function () { _this._t; }) || this;
+        return _this;
     }
     return D;
 }(Base));
diff --git a/tests/baselines/reference/superCallBeforeThisAccessing6.js b/tests/baselines/reference/superCallBeforeThisAccessing6.js
@@ -24,7 +24,8 @@ var Base = (function () {
 var D = (function (_super) {
     __extends(D, _super);
     function D() {
-        return _super.call(this, this) || this;
+        var _this = _super.call(this, _this) || this;
+        return _this;
     }
     return D;
 }(Base));
diff --git a/tests/baselines/reference/superPropertyAccessInSuperCall01.errors.txt b/tests/baselines/reference/superPropertyAccessInSuperCall01.errors.txt
@@ -0,0 +1,17 @@
+tests/cases/compiler/superPropertyAccessInSuperCall01.ts(9,9): error TS17011: 'super' must be called before accessing a property of 'super' in the constructor of a derived class.
+
+
+==== tests/cases/compiler/superPropertyAccessInSuperCall01.ts (1 errors) ====
+    class A {
+    	constructor(f: string) {
+    	}
+    	public blah(): string { return ""; }
+    }
+    
+    class B extends A {
+    	constructor() {
+    		super(super.blah())
+    		      ~~~~~
+!!! error TS17011: 'super' must be called before accessing a property of 'super' in the constructor of a derived class.
+    	}
+    }
diff --git a/tests/baselines/reference/superPropertyAccessInSuperCall01.js b/tests/baselines/reference/superPropertyAccessInSuperCall01.js
@@ -0,0 +1,33 @@
+//// [superPropertyAccessInSuperCall01.ts]
+class A {
+	constructor(f: string) {
+	}
+	public blah(): string { return ""; }
+}
+
+class B extends A {
+	constructor() {
+		super(super.blah())
+	}
+}
+
+//// [superPropertyAccessInSuperCall01.js]
+var __extends = (this && this.__extends) || function (d, b) {
+    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
+    function __() { this.constructor = d; }
+    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
+};
+var A = (function () {
+    function A(f) {
+    }
+    A.prototype.blah = function () { return ""; };
+    return A;
+}());
+var B = (function (_super) {
+    __extends(B, _super);
+    function B() {
+        var _this = _super.call(this, _super.blah.call(_this)) || this;
+        return _this;
+    }
+    return B;
+}(A));
diff --git a/tests/baselines/reference/superPropertyInConstructorBeforeSuperCall.js b/tests/baselines/reference/superPropertyInConstructorBeforeSuperCall.js
@@ -40,7 +40,8 @@ var C1 = (function (_super) {
 var C2 = (function (_super) {
     __extends(C2, _super);
     function C2() {
-        return _super.call(this, _super.x.call(_this)) || this;
+        var _this = _super.call(this, _super.x.call(_this)) || this;
+        return _this;
     }
     return C2;
 }(B));
diff --git a/tests/baselines/reference/thisInInvalidContexts.js b/tests/baselines/reference/thisInInvalidContexts.js
@@ -70,7 +70,8 @@ var ClassWithNoInitializer = (function (_super) {
     __extends(ClassWithNoInitializer, _super);
     //'this' in optional super call
     function ClassWithNoInitializer() {
-        return _super.call(this, _this) || this;
+        var _this = _super.call(this, _this) || this;
+        return _this;
     }
     return ClassWithNoInitializer;
 }(BaseErrClass));
diff --git a/tests/baselines/reference/thisInInvalidContextsExternalModule.js b/tests/baselines/reference/thisInInvalidContextsExternalModule.js
@@ -71,7 +71,8 @@ var ClassWithNoInitializer = (function (_super) {
     __extends(ClassWithNoInitializer, _super);
     //'this' in optional super call
     function ClassWithNoInitializer() {
-        return _super.call(this, _this) || this;
+        var _this = _super.call(this, _this) || this;
+        return _this;
     }
     return ClassWithNoInitializer;
 }(BaseErrClass));
diff --git a/tests/baselines/reference/thisInSuperCall.js b/tests/baselines/reference/thisInSuperCall.js
@@ -36,7 +36,8 @@ var Base = (function () {
 var Foo = (function (_super) {
     __extends(Foo, _super);
     function Foo() {
-        return _super.call(this, _this) || this;
+        var _this = _super.call(this, _this) || this;
+        return _this;
     }
     return Foo;
 }(Base));
diff --git a/tests/baselines/reference/thisInSuperCall2.js b/tests/baselines/reference/thisInSuperCall2.js
@@ -33,7 +33,8 @@ var Base = (function () {
 var Foo = (function (_super) {
     __extends(Foo, _super);
     function Foo() {
-        return _super.call(this, _this) || this;
+        var _this = _super.call(this, _this) || this;
+        return _this;
     }
     return Foo;
 }(Base));
diff --git a/tests/baselines/reference/validUseOfThisInSuper.js b/tests/baselines/reference/validUseOfThisInSuper.js
@@ -24,7 +24,8 @@ var Base = (function () {
 var Super = (function (_super) {
     __extends(Super, _super);
     function Super() {
-        return _super.call(this, (function () { return _this; })()) || this;
+        var _this = _super.call(this, (function () { return _this; })()) || this;
+        return _this;
     }
     return Super;
 }(Base));
diff --git a/tests/cases/compiler/captureSuperPropertyAccessInSuperCall01.ts b/tests/cases/compiler/captureSuperPropertyAccessInSuperCall01.ts
@@ -0,0 +1,11 @@
+class A {
+	constructor(f: () => string) {
+	}
+	public blah(): string { return ""; }
+}
+
+class B extends A {
+	constructor() {
+		super(() => { return super.blah(); })
+	}
+}
diff --git a/tests/cases/compiler/superPropertyAccessInSuperCall01.ts b/tests/cases/compiler/superPropertyAccessInSuperCall01.ts
@@ -0,0 +1,11 @@
+class A {
+	constructor(f: string) {
+	}
+	public blah(): string { return ""; }
+}
+
+class B extends A {
+	constructor() {
+		super(super.blah())
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -1024,8 +1024,11 @@ namespace ts {`
`1024`	`1024`	`}`
`1025`	`1025`	`}`
`1026`	`1026`
`1027`		`- // Return the result if we have an immediate super() call on the last statement.`
`1028`		`- if (superCallExpression && statementOffset === ctorStatements.length - 1) {`
	`1027`	`+ // Return the result if we have an immediate super() call on the last statement,`
	`1028`	`+ // but only if the constructor itself doesn't use 'this' elsewhere.`
	`1029`	`+ if (superCallExpression`
	`1030`	`+ && statementOffset === ctorStatements.length - 1`
	`1031`	`+ && !(ctor.transformFlags & (TransformFlags.ContainsLexicalThis \| TransformFlags.ContainsCapturedLexicalThis))) {`
`1029`	`1032`	`const returnStatement = createReturn(superCallExpression);`
`1030`	`1033`
`1031`	`1034`	`if (superCallExpression.kind !== SyntaxKind.BinaryExpression`