Skip to content

Commit 64305ed

Browse files
weswighamweswigham
authored
Skip outer expressions when checking for super keyword in binder (microsoft#20164)
* Skip outter expressions when checking for super keyword in binder * use TransformFlags to optimize and correct super call transforms * Lint
1 parent 6f28b0a commit 64305ed

20 files changed

Lines changed: 753 additions & 43 deletions

‎src/compiler/binder.ts‎

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2740,6 +2740,9 @@ namespace ts {
27402740
case SyntaxKind.PropertyAccessExpression:
27412741
return computePropertyAccess(<PropertyAccessExpression>node, subtreeFlags);
27422742

2743+
case SyntaxKind.ElementAccessExpression:
2744+
return computeElementAccess(<ElementAccessExpression>node, subtreeFlags);
2745+
27432746
default:
27442747
return computeOther(node, kind, subtreeFlags);
27452748
}
@@ -2748,17 +2751,21 @@ namespace ts {
27482751
function computeCallExpression(node: CallExpression, subtreeFlags: TransformFlags) {
27492752
let transformFlags = subtreeFlags;
27502753
const expression = node.expression;
2751-
const expressionKind = expression.kind;
27522754

27532755
if (node.typeArguments) {
27542756
transformFlags |= TransformFlags.AssertTypeScript;
27552757
}
27562758

27572759
if (subtreeFlags & TransformFlags.ContainsSpread
2758-
|| isSuperOrSuperProperty(expression, expressionKind)) {
2760+
|| (expression.transformFlags & (TransformFlags.Super | TransformFlags.ContainsSuper))) {
27592761
// If the this node contains a SpreadExpression, or is a super call, then it is an ES6
27602762
// node.
27612763
transformFlags |= TransformFlags.AssertES2015;
2764+
// super property or element accesses could be inside lambdas, etc, and need a captured `this`,
2765+
// while super keyword for super calls (indicated by TransformFlags.Super) does not (since it can only be top-level in a constructor)
2766+
if (expression.transformFlags & TransformFlags.ContainsSuper) {
2767+
transformFlags |= TransformFlags.ContainsLexicalThis;
2768+
}
27622769
}
27632770

27642771
if (expression.kind === SyntaxKind.ImportKeyword) {
@@ -2775,21 +2782,6 @@ namespace ts {
27752782
return transformFlags & ~TransformFlags.ArrayLiteralOrCallOrNewExcludes;
27762783
}
27772784

2778-
function isSuperOrSuperProperty(node: Node, kind: SyntaxKind) {
2779-
switch (kind) {
2780-
case SyntaxKind.SuperKeyword:
2781-
return true;
2782-
2783-
case SyntaxKind.PropertyAccessExpression:
2784-
case SyntaxKind.ElementAccessExpression:
2785-
const expression = (<PropertyAccessExpression | ElementAccessExpression>node).expression;
2786-
const expressionKind = expression.kind;
2787-
return expressionKind === SyntaxKind.SuperKeyword;
2788-
}
2789-
2790-
return false;
2791-
}
2792-
27932785
function computeNewExpression(node: NewExpression, subtreeFlags: TransformFlags) {
27942786
let transformFlags = subtreeFlags;
27952787
if (node.typeArguments) {
@@ -2884,7 +2876,7 @@ namespace ts {
28842876
}
28852877

28862878
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
2887-
return transformFlags & ~TransformFlags.NodeExcludes;
2879+
return transformFlags & ~TransformFlags.OuterExpressionExcludes;
28882880
}
28892881

28902882
function computeClassDeclaration(node: ClassDeclaration, subtreeFlags: TransformFlags) {
@@ -3203,17 +3195,32 @@ namespace ts {
32033195

32043196
function computePropertyAccess(node: PropertyAccessExpression, subtreeFlags: TransformFlags) {
32053197
let transformFlags = subtreeFlags;
3206-
const expression = node.expression;
3207-
const expressionKind = expression.kind;
32083198

32093199
// If a PropertyAccessExpression starts with a super keyword, then it is
32103200
// ES6 syntax, and requires a lexical `this` binding.
3211-
if (expressionKind === SyntaxKind.SuperKeyword) {
3212-
transformFlags |= TransformFlags.ContainsLexicalThis;
3201+
if (transformFlags & TransformFlags.Super) {
3202+
transformFlags ^= TransformFlags.Super;
3203+
transformFlags |= TransformFlags.ContainsSuper;
32133204
}
32143205

32153206
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
3216-
return transformFlags & ~TransformFlags.NodeExcludes;
3207+
return transformFlags & ~TransformFlags.PropertyAccessExcludes;
3208+
}
3209+
3210+
function computeElementAccess(node: ElementAccessExpression, subtreeFlags: TransformFlags) {
3211+
let transformFlags = subtreeFlags;
3212+
const expression = node.expression;
3213+
const expressionFlags = expression.transformFlags; // We do not want to aggregate flags from the argument expression for super/this capturing
3214+
3215+
// If an ElementAccessExpression starts with a super keyword, then it is
3216+
// ES6 syntax, and requires a lexical `this` binding.
3217+
if (expressionFlags & TransformFlags.Super) {
3218+
transformFlags &= ~TransformFlags.Super;
3219+
transformFlags |= TransformFlags.ContainsSuper;
3220+
}
3221+
3222+
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
3223+
return transformFlags & ~TransformFlags.PropertyAccessExcludes;
32173224
}
32183225

32193226
function computeVariableDeclaration(node: VariableDeclaration, subtreeFlags: TransformFlags) {
@@ -3333,6 +3340,13 @@ namespace ts {
33333340
transformFlags |= TransformFlags.AssertESNext | TransformFlags.AssertES2017;
33343341
break;
33353342

3343+
case SyntaxKind.TypeAssertionExpression:
3344+
case SyntaxKind.AsExpression:
3345+
case SyntaxKind.PartiallyEmittedExpression:
3346+
// These nodes are TypeScript syntax.
3347+
transformFlags |= TransformFlags.AssertTypeScript;
3348+
excludeFlags = TransformFlags.OuterExpressionExcludes;
3349+
break;
33363350
case SyntaxKind.PublicKeyword:
33373351
case SyntaxKind.PrivateKeyword:
33383352
case SyntaxKind.ProtectedKeyword:
@@ -3341,8 +3355,6 @@ namespace ts {
33413355
case SyntaxKind.ConstKeyword:
33423356
case SyntaxKind.EnumDeclaration:
33433357
case SyntaxKind.EnumMember:
3344-
case SyntaxKind.TypeAssertionExpression:
3345-
case SyntaxKind.AsExpression:
33463358
case SyntaxKind.NonNullExpression:
33473359
case SyntaxKind.ReadonlyKeyword:
33483360
// These nodes are TypeScript syntax.
@@ -3470,7 +3482,8 @@ namespace ts {
34703482

34713483
case SyntaxKind.SuperKeyword:
34723484
// This node is ES6 syntax.
3473-
transformFlags |= TransformFlags.AssertES2015;
3485+
transformFlags |= TransformFlags.AssertES2015 | TransformFlags.Super;
3486+
excludeFlags = TransformFlags.OuterExpressionExcludes; // must be set to persist `Super`
34743487
break;
34753488

34763489
case SyntaxKind.ThisKeyword:
@@ -3627,6 +3640,15 @@ namespace ts {
36273640
case SyntaxKind.ObjectBindingPattern:
36283641
case SyntaxKind.ArrayBindingPattern:
36293642
return TransformFlags.BindingPatternExcludes;
3643+
case SyntaxKind.TypeAssertionExpression:
3644+
case SyntaxKind.AsExpression:
3645+
case SyntaxKind.PartiallyEmittedExpression:
3646+
case SyntaxKind.ParenthesizedExpression:
3647+
case SyntaxKind.SuperKeyword:
3648+
return TransformFlags.OuterExpressionExcludes;
3649+
case SyntaxKind.PropertyAccessExpression:
3650+
case SyntaxKind.ElementAccessExpression:
3651+
return TransformFlags.PropertyAccessExcludes;
36303652
default:
36313653
return TransformFlags.NodeExcludes;
36323654
}

‎src/compiler/types.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4456,6 +4456,8 @@ namespace ts {
44564456
ContainsYield = 1 << 24,
44574457
ContainsHoistedDeclarationOrCompletion = 1 << 25,
44584458
ContainsDynamicImport = 1 << 26,
4459+
Super = 1 << 27,
4460+
ContainsSuper = 1 << 28,
44594461

44604462
// Please leave this as 1 << 29.
44614463
// It is the maximum bit we can set before we outgrow the size of a v8 small integer (SMI) on an x86 system.
@@ -4476,7 +4478,9 @@ namespace ts {
44764478
// Scope Exclusions
44774479
// - Bitmasks that exclude flags from propagating out of a specific context
44784480
// into the subtree flags of their container.
4479-
NodeExcludes = TypeScript | ES2015 | DestructuringAssignment | Generator | HasComputedFlags,
4481+
OuterExpressionExcludes = TypeScript | ES2015 | DestructuringAssignment | Generator | HasComputedFlags,
4482+
PropertyAccessExcludes = OuterExpressionExcludes | Super,
4483+
NodeExcludes = PropertyAccessExcludes | ContainsSuper,
44804484
ArrowFunctionExcludes = NodeExcludes | ContainsDecorators | ContainsDefaultValueAssignments | ContainsLexicalThis | ContainsParameterPropertyAssignments | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,
44814485
FunctionExcludes = NodeExcludes | ContainsDecorators | ContainsDefaultValueAssignments | ContainsCapturedLexicalThis | ContainsLexicalThis | ContainsParameterPropertyAssignments | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,
44824486
ConstructorExcludes = NodeExcludes | ContainsDefaultValueAssignments | ContainsLexicalThis | ContainsCapturedLexicalThis | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,

‎tests/baselines/reference/decoratorOnClassMethod12.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key,
2828
};
2929
var M;
3030
(function (M) {
31-
var _this = this;
3231
var S = /** @class */ (function () {
3332
function S() {
3433
}

‎tests/baselines/reference/superAccess2.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ var __extends = (this && this.__extends) || (function () {
3535
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
3636
};
3737
})();
38-
var _this = this;
3938
var P = /** @class */ (function () {
4039
function P() {
4140
}

‎tests/baselines/reference/superAccessCastedCall.js‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
//// [superAccessCastedCall.ts]
2+
class Foo {
3+
bar(): void {}
4+
}
5+
6+
class Bar extends Foo {
7+
x: Number;
8+
9+
constructor() {
10+
super();
11+
this.x = 2;
12+
}
13+
14+
bar() {
15+
super.bar();
16+
(super.bar as any)();
17+
}
18+
}
19+
20+
let b = new Bar();
21+
b.bar()
22+
23+
//// [superAccessCastedCall.js]
24+
var __extends = (this && this.__extends) || (function () {
25+
var extendStatics = Object.setPrototypeOf ||
26+
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
27+
function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
28+
return function (d, b) {
29+
extendStatics(d, b);
30+
function __() { this.constructor = d; }
31+
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
32+
};
33+
})();
34+
var Foo = /** @class */ (function () {
35+
function Foo() {
36+
}
37+
Foo.prototype.bar = function () { };
38+
return Foo;
39+
}());
40+
var Bar = /** @class */ (function (_super) {
41+
__extends(Bar, _super);
42+
function Bar() {
43+
var _this = _super.call(this) || this;
44+
_this.x = 2;
45+
return _this;
46+
}
47+
Bar.prototype.bar = function () {
48+
_super.prototype.bar.call(this);
49+
_super.prototype.bar.call(this);
50+
};
51+
return Bar;
52+
}(Foo));
53+
var b = new Bar();
54+
b.bar();

‎tests/baselines/reference/superAccessCastedCall.symbols‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
=== tests/cases/compiler/superAccessCastedCall.ts ===
2+
class Foo {
3+
>Foo : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))
4+
5+
bar(): void {}
6+
>bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
7+
}
8+
9+
class Bar extends Foo {
10+
>Bar : Symbol(Bar, Decl(superAccessCastedCall.ts, 2, 1))
11+
>Foo : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))
12+
13+
x: Number;
14+
>x : Symbol(Bar.x, Decl(superAccessCastedCall.ts, 4, 23))
15+
>Number : Symbol(Number, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
16+
17+
constructor() {
18+
super();
19+
>super : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))
20+
21+
this.x = 2;
22+
>this.x : Symbol(Bar.x, Decl(superAccessCastedCall.ts, 4, 23))
23+
>this : Symbol(Bar, Decl(superAccessCastedCall.ts, 2, 1))
24+
>x : Symbol(Bar.x, Decl(superAccessCastedCall.ts, 4, 23))
25+
}
26+
27+
bar() {
28+
>bar : Symbol(Bar.bar, Decl(superAccessCastedCall.ts, 10, 5))
29+
30+
super.bar();
31+
>super.bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
32+
>super : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))
33+
>bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
34+
35+
(super.bar as any)();
36+
>super.bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
37+
>super : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))
38+
>bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
39+
}
40+
}
41+
42+
let b = new Bar();
43+
>b : Symbol(b, Decl(superAccessCastedCall.ts, 18, 3))
44+
>Bar : Symbol(Bar, Decl(superAccessCastedCall.ts, 2, 1))
45+
46+
b.bar()
47+
>b.bar : Symbol(Bar.bar, Decl(superAccessCastedCall.ts, 10, 5))
48+
>b : Symbol(b, Decl(superAccessCastedCall.ts, 18, 3))
49+
>bar : Symbol(Bar.bar, Decl(superAccessCastedCall.ts, 10, 5))
50+

‎tests/baselines/reference/superAccessCastedCall.types‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
=== tests/cases/compiler/superAccessCastedCall.ts ===
2+
class Foo {
3+
>Foo : Foo
4+
5+
bar(): void {}
6+
>bar : () => void
7+
}
8+
9+
class Bar extends Foo {
10+
>Bar : Bar
11+
>Foo : Foo
12+
13+
x: Number;
14+
>x : Number
15+
>Number : Number
16+
17+
constructor() {
18+
super();
19+
>super() : void
20+
>super : typeof Foo
21+
22+
this.x = 2;
23+
>this.x = 2 : 2
24+
>this.x : Number
25+
>this : this
26+
>x : Number
27+
>2 : 2
28+
}
29+
30+
bar() {
31+
>bar : () => void
32+
33+
super.bar();
34+
>super.bar() : void
35+
>super.bar : () => void
36+
>super : Foo
37+
>bar : () => void
38+
39+
(super.bar as any)();
40+
>(super.bar as any)() : any
41+
>(super.bar as any) : any
42+
>super.bar as any : any
43+
>super.bar : () => void
44+
>super : Foo
45+
>bar : () => void
46+
}
47+
}
48+
49+
let b = new Bar();
50+
>b : Bar
51+
>new Bar() : Bar
52+
>Bar : typeof Bar
53+
54+
b.bar()
55+
>b.bar() : void
56+
>b.bar : () => void
57+
>b : Bar
58+
>bar : () => void
59+

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
tests/cases/compiler/superElementAccess.ts(7,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
2+
tests/cases/compiler/superElementAccess.ts(8,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
3+
4+
5+
==== tests/cases/compiler/superElementAccess.ts (2 errors) ====
6+
class MyBase {
7+
m1(a: string) { return a; }
8+
private p1() { }
9+
m2: () => void = function () { }
10+
d1: number = 42;
11+
private d2: number = 42;
12+
get value() {return 0 }
13+
~~~~~
14+
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
15+
set value(v: number) { }
16+
~~~~~
17+
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
18+
}
19+
20+
21+
class MyDerived extends MyBase {
22+
23+
foo() {
24+
super["m1"]("hi"); // Should be allowed, method on base prototype
25+
26+
var l2 = super["m1"].bind(this); // Should be allowed, can access properties as well as invoke
27+
28+
var x: (a: string) => string = super["m1"]; // Should be allowed, can assign to var with compatible signature
29+
30+
super["m2"].bind(this); // Should error, instance property, not a public instance member function
31+
32+
super["p1"](); // Should error, private not public instance member function
33+
34+
var l1 = super["d1"]; // Should error, instance data property not a public instance member function
35+
36+
var l1 = super["d2"]; // Should error, instance data property not a public instance member function
37+
38+
super["m1"] = function (a: string) { return ""; }; // Should be allowed, we will not restrict assignment
39+
40+
super["value"] = 0; // Should error, instance data property not a public instance member function
41+
42+
var z = super["value"]; // Should error, instance data property not a public instance member function
43+
}
44+
}

0 commit comments

Comments
 (0)