Skip to content

Skip outer expressions when checking for super keyword in binder #20164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 48 additions & 26 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2734,6 +2734,9 @@ namespace ts {
case SyntaxKind.PropertyAccessExpression:
return computePropertyAccess(<PropertyAccessExpression>node, subtreeFlags);

case SyntaxKind.ElementAccessExpression:
return computeElementAccess(<ElementAccessExpression>node, subtreeFlags);

default:
return computeOther(node, kind, subtreeFlags);
}
Expand All @@ -2742,17 +2745,21 @@ namespace ts {
function computeCallExpression(node: CallExpression, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
const expression = node.expression;
const expressionKind = expression.kind;

if (node.typeArguments) {
transformFlags |= TransformFlags.AssertTypeScript;
}

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

if (expression.kind === SyntaxKind.ImportKeyword) {
Expand All @@ -2769,21 +2776,6 @@ namespace ts {
return transformFlags & ~TransformFlags.ArrayLiteralOrCallOrNewExcludes;
}

function isSuperOrSuperProperty(node: Node, kind: SyntaxKind) {
switch (kind) {
case SyntaxKind.SuperKeyword:
return true;

case SyntaxKind.PropertyAccessExpression:
case SyntaxKind.ElementAccessExpression:
const expression = (<PropertyAccessExpression | ElementAccessExpression>node).expression;
const expressionKind = expression.kind;
return expressionKind === SyntaxKind.SuperKeyword;
}

return false;
}

function computeNewExpression(node: NewExpression, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
if (node.typeArguments) {
Expand Down Expand Up @@ -2878,7 +2870,7 @@ namespace ts {
}

node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
return transformFlags & ~TransformFlags.NodeExcludes;
return transformFlags & ~TransformFlags.OuterExpressionExcludes;
}

function computeClassDeclaration(node: ClassDeclaration, subtreeFlags: TransformFlags) {
Expand Down Expand Up @@ -3197,17 +3189,32 @@ namespace ts {

function computePropertyAccess(node: PropertyAccessExpression, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
const expression = node.expression;
const expressionKind = expression.kind;

// If a PropertyAccessExpression starts with a super keyword, then it is
// ES6 syntax, and requires a lexical `this` binding.
if (expressionKind === SyntaxKind.SuperKeyword) {
transformFlags |= TransformFlags.ContainsLexicalThis;
if (transformFlags & TransformFlags.Super) {
transformFlags ^= TransformFlags.Super;
transformFlags |= TransformFlags.ContainsSuper;
}

node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
return transformFlags & ~TransformFlags.NodeExcludes;
return transformFlags & ~TransformFlags.PropertyAccessExcludes;
}

function computeElementAccess(node: ElementAccessExpression, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
const expression = node.expression;
const expressionFlags = expression.transformFlags; // We do not want to aggregate flags from the argument expression for super/this capturing

// If an ElementAccessExpression starts with a super keyword, then it is
// ES6 syntax, and requires a lexical `this` binding.
if (expressionFlags & TransformFlags.Super) {
transformFlags &= ~TransformFlags.Super;
transformFlags |= TransformFlags.ContainsSuper;
}

node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
return transformFlags & ~TransformFlags.PropertyAccessExcludes;
}

function computeVariableDeclaration(node: VariableDeclaration, subtreeFlags: TransformFlags) {
Expand Down Expand Up @@ -3327,6 +3334,13 @@ namespace ts {
transformFlags |= TransformFlags.AssertESNext | TransformFlags.AssertES2017;
break;

case SyntaxKind.TypeAssertionExpression:
case SyntaxKind.AsExpression:
case SyntaxKind.PartiallyEmittedExpression:
// These nodes are TypeScript syntax.
transformFlags |= TransformFlags.AssertTypeScript;
excludeFlags = TransformFlags.OuterExpressionExcludes;
break;
case SyntaxKind.PublicKeyword:
case SyntaxKind.PrivateKeyword:
case SyntaxKind.ProtectedKeyword:
Expand All @@ -3335,8 +3349,6 @@ namespace ts {
case SyntaxKind.ConstKeyword:
case SyntaxKind.EnumDeclaration:
case SyntaxKind.EnumMember:
case SyntaxKind.TypeAssertionExpression:
case SyntaxKind.AsExpression:
case SyntaxKind.NonNullExpression:
case SyntaxKind.ReadonlyKeyword:
// These nodes are TypeScript syntax.
Expand Down Expand Up @@ -3464,7 +3476,8 @@ namespace ts {

case SyntaxKind.SuperKeyword:
// This node is ES6 syntax.
transformFlags |= TransformFlags.AssertES2015;
transformFlags |= TransformFlags.AssertES2015 | TransformFlags.Super;
excludeFlags = TransformFlags.OuterExpressionExcludes; // must be set to persist `Super`
break;

case SyntaxKind.ThisKeyword:
Expand Down Expand Up @@ -3621,6 +3634,15 @@ namespace ts {
case SyntaxKind.ObjectBindingPattern:
case SyntaxKind.ArrayBindingPattern:
return TransformFlags.BindingPatternExcludes;
case SyntaxKind.TypeAssertionExpression:
case SyntaxKind.AsExpression:
case SyntaxKind.PartiallyEmittedExpression:
case SyntaxKind.ParenthesizedExpression:
case SyntaxKind.SuperKeyword:
return TransformFlags.OuterExpressionExcludes;
case SyntaxKind.PropertyAccessExpression:
case SyntaxKind.ElementAccessExpression:
return TransformFlags.PropertyAccessExcludes;
default:
return TransformFlags.NodeExcludes;
}
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4448,6 +4448,8 @@ namespace ts {
ContainsYield = 1 << 24,
ContainsHoistedDeclarationOrCompletion = 1 << 25,
ContainsDynamicImport = 1 << 26,
Super = 1 << 27,
ContainsSuper = 1 << 28,

// Please leave this as 1 << 29.
// It is the maximum bit we can set before we outgrow the size of a v8 small integer (SMI) on an x86 system.
Expand All @@ -4468,7 +4470,9 @@ namespace ts {
// Scope Exclusions
// - Bitmasks that exclude flags from propagating out of a specific context
// into the subtree flags of their container.
NodeExcludes = TypeScript | ES2015 | DestructuringAssignment | Generator | HasComputedFlags,
OuterExpressionExcludes = TypeScript | ES2015 | DestructuringAssignment | Generator | HasComputedFlags,
PropertyAccessExcludes = OuterExpressionExcludes | Super,
NodeExcludes = PropertyAccessExcludes | ContainsSuper,
ArrowFunctionExcludes = NodeExcludes | ContainsDecorators | ContainsDefaultValueAssignments | ContainsLexicalThis | ContainsParameterPropertyAssignments | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,
FunctionExcludes = NodeExcludes | ContainsDecorators | ContainsDefaultValueAssignments | ContainsCapturedLexicalThis | ContainsLexicalThis | ContainsParameterPropertyAssignments | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,
ConstructorExcludes = NodeExcludes | ContainsDefaultValueAssignments | ContainsLexicalThis | ContainsCapturedLexicalThis | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,
Expand Down
1 change: 0 additions & 1 deletion tests/baselines/reference/decoratorOnClassMethod12.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key,
};
var M;
(function (M) {
var _this = this;
var S = /** @class */ (function () {
function S() {
}
Expand Down
1 change: 0 additions & 1 deletion tests/baselines/reference/superAccess2.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ var __extends = (this && this.__extends) || (function () {
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
})();
var _this = this;
var P = /** @class */ (function () {
function P() {
}
Expand Down
54 changes: 54 additions & 0 deletions tests/baselines/reference/superAccessCastedCall.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//// [superAccessCastedCall.ts]
class Foo {
bar(): void {}
}

class Bar extends Foo {
x: Number;

constructor() {
super();
this.x = 2;
}

bar() {
super.bar();
(super.bar as any)();
}
}

let b = new Bar();
b.bar()

//// [superAccessCastedCall.js]
var __extends = (this && this.__extends) || (function () {
var extendStatics = Object.setPrototypeOf ||
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
return function (d, b) {
extendStatics(d, b);
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
})();
var Foo = /** @class */ (function () {
function Foo() {
}
Foo.prototype.bar = function () { };
return Foo;
}());
var Bar = /** @class */ (function (_super) {
__extends(Bar, _super);
function Bar() {
var _this = _super.call(this) || this;
_this.x = 2;
return _this;
}
Bar.prototype.bar = function () {
_super.prototype.bar.call(this);
_super.prototype.bar.call(this);
};
return Bar;
}(Foo));
var b = new Bar();
b.bar();
50 changes: 50 additions & 0 deletions tests/baselines/reference/superAccessCastedCall.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
=== tests/cases/compiler/superAccessCastedCall.ts ===
class Foo {
>Foo : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))

bar(): void {}
>bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
}

class Bar extends Foo {
>Bar : Symbol(Bar, Decl(superAccessCastedCall.ts, 2, 1))
>Foo : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))

x: Number;
>x : Symbol(Bar.x, Decl(superAccessCastedCall.ts, 4, 23))
>Number : Symbol(Number, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))

constructor() {
super();
>super : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))

this.x = 2;
>this.x : Symbol(Bar.x, Decl(superAccessCastedCall.ts, 4, 23))
>this : Symbol(Bar, Decl(superAccessCastedCall.ts, 2, 1))
>x : Symbol(Bar.x, Decl(superAccessCastedCall.ts, 4, 23))
}

bar() {
>bar : Symbol(Bar.bar, Decl(superAccessCastedCall.ts, 10, 5))

super.bar();
>super.bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
>super : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))
>bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))

(super.bar as any)();
>super.bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
>super : Symbol(Foo, Decl(superAccessCastedCall.ts, 0, 0))
>bar : Symbol(Foo.bar, Decl(superAccessCastedCall.ts, 0, 11))
}
}

let b = new Bar();
>b : Symbol(b, Decl(superAccessCastedCall.ts, 18, 3))
>Bar : Symbol(Bar, Decl(superAccessCastedCall.ts, 2, 1))

b.bar()
>b.bar : Symbol(Bar.bar, Decl(superAccessCastedCall.ts, 10, 5))
>b : Symbol(b, Decl(superAccessCastedCall.ts, 18, 3))
>bar : Symbol(Bar.bar, Decl(superAccessCastedCall.ts, 10, 5))

59 changes: 59 additions & 0 deletions tests/baselines/reference/superAccessCastedCall.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
=== tests/cases/compiler/superAccessCastedCall.ts ===
class Foo {
>Foo : Foo

bar(): void {}
>bar : () => void
}

class Bar extends Foo {
>Bar : Bar
>Foo : Foo

x: Number;
>x : Number
>Number : Number

constructor() {
super();
>super() : void
>super : typeof Foo

this.x = 2;
>this.x = 2 : 2
>this.x : Number
>this : this
>x : Number
>2 : 2
}

bar() {
>bar : () => void

super.bar();
>super.bar() : void
>super.bar : () => void
>super : Foo
>bar : () => void

(super.bar as any)();
>(super.bar as any)() : any
>(super.bar as any) : any
>super.bar as any : any
>super.bar : () => void
>super : Foo
>bar : () => void
}
}

let b = new Bar();
>b : Bar
>new Bar() : Bar
>Bar : typeof Bar

b.bar()
>b.bar() : void
>b.bar : () => void
>b : Bar
>bar : () => void

44 changes: 44 additions & 0 deletions tests/baselines/reference/superElementAccess.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
tests/cases/compiler/superElementAccess.ts(7,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/superElementAccess.ts(8,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.


==== tests/cases/compiler/superElementAccess.ts (2 errors) ====
class MyBase {
m1(a: string) { return a; }
private p1() { }
m2: () => void = function () { }
d1: number = 42;
private d2: number = 42;
get value() {return 0 }
~~~~~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
set value(v: number) { }
~~~~~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
}


class MyDerived extends MyBase {

foo() {
super["m1"]("hi"); // Should be allowed, method on base prototype

var l2 = super["m1"].bind(this); // Should be allowed, can access properties as well as invoke

var x: (a: string) => string = super["m1"]; // Should be allowed, can assign to var with compatible signature

super["m2"].bind(this); // Should error, instance property, not a public instance member function

super["p1"](); // Should error, private not public instance member function

var l1 = super["d1"]; // Should error, instance data property not a public instance member function

var l1 = super["d2"]; // Should error, instance data property not a public instance member function

super["m1"] = function (a: string) { return ""; }; // Should be allowed, we will not restrict assignment

super["value"] = 0; // Should error, instance data property not a public instance member function

var z = super["value"]; // Should error, instance data property not a public instance member function
}
}
Loading