Skip to content

Commit 90c08c2

Browse files
author
Kanchalai Tanglertsampan
committed
Port PR#6860 lexically check calling super before this
Update baselines add baselines Update baseline Port PR microsoft#6860 lexically check calling super before this Check using "super" before "this" lexically instead of using the NodeCheckFlags Remove "type-checking" way of checking if super is used before this. Instead check using whether super occurs before this syntactically Refactor the code Dive down to get super call Address PR Address PR about tests Add a flag so we don't repeatedly finding super call rename function Move tests into correct location Address PR: report error on super call instead of entire constructor node remove marge mark
1 parent 20f7b18 commit 90c08c2

31 files changed

Lines changed: 747 additions & 30 deletions

‎src/compiler/checker.ts‎

Lines changed: 74 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7328,17 +7328,76 @@ namespace ts {
73287328
}
73297329
}
73307330

7331+
function isSuperCallExpression(n: Node): boolean {
7332+
return n.kind === SyntaxKind.CallExpression && (<CallExpression>n).expression.kind === SyntaxKind.SuperKeyword;
7333+
}
7334+
7335+
function findFirstSuperCall(n: Node): Node {
7336+
if (isSuperCallExpression(n)) {
7337+
return n;
7338+
}
7339+
else if (isFunctionLike(n)) {
7340+
return undefined;
7341+
}
7342+
return forEachChild(n, findFirstSuperCall);
7343+
}
7344+
7345+
/**
7346+
* Return a cached result if super-statement is already found.
7347+
* Otherwise, find a super statement in a given constructor function and cache the result in the node-links of the constructor
7348+
*
7349+
* @param constructor constructor-function to look for super statement
7350+
*/
7351+
function getSuperCallInConstructor(constructor: ConstructorDeclaration): ExpressionStatement {
7352+
const links = getNodeLinks(constructor);
7353+
7354+
// Only trying to find super-call if we haven't yet tried to find one. Once we try, we will record the result
7355+
if (links.hasSuperCall === undefined) {
7356+
links.superCall = <ExpressionStatement>findFirstSuperCall(constructor.body);
7357+
links.hasSuperCall = links.superCall ? true : false;
7358+
}
7359+
return links.superCall;
7360+
}
7361+
7362+
/**
7363+
* Check if the given class-declaration extends null then return true.
7364+
* Otherwise, return false
7365+
* @param classDecl a class declaration to check if it extends null
7366+
*/
7367+
function classDeclarationExtendsNull(classDecl: ClassDeclaration): boolean {
7368+
const classSymbol = getSymbolOfNode(classDecl);
7369+
const classInstanceType = <InterfaceType>getDeclaredTypeOfSymbol(classSymbol);
7370+
const baseConstructorType = getBaseConstructorTypeOfClass(classInstanceType);
7371+
7372+
return baseConstructorType === nullType;
7373+
}
7374+
73317375
function checkThisExpression(node: Node): Type {
73327376
// Stop at the first arrow function so that we can
73337377
// tell whether 'this' needs to be captured.
73347378
let container = getThisContainer(node, /* includeArrowFunctions */ true);
73357379
let needToCaptureLexicalThis = false;
73367380

73377381
if (container.kind === SyntaxKind.Constructor) {
7338-
const baseTypeNode = getClassExtendsHeritageClauseElement(<ClassLikeDeclaration>container.parent);
7339-
if (baseTypeNode && !(getNodeCheckFlags(container) & NodeCheckFlags.HasSeenSuperCall)) {
7340-
// In ES6, super inside constructor of class-declaration has to precede "this" accessing
7341-
error(node, Diagnostics.super_must_be_called_before_accessing_this_in_the_constructor_of_a_derived_class);
7382+
const containingClassDecl = <ClassDeclaration>container.parent;
7383+
const baseTypeNode = getClassExtendsHeritageClauseElement(containingClassDecl);
7384+
7385+
// If a containing class does not have extends clause or the class extends null
7386+
// skip checking whether super statement is called before "this" accessing.
7387+
if (baseTypeNode && !classDeclarationExtendsNull(containingClassDecl)) {
7388+
const superCall = getSuperCallInConstructor(<ConstructorDeclaration>container);
7389+
7390+
// We should give an error in the following cases:
7391+
// - No super-call
7392+
// - "this" is accessing before super-call.
7393+
// i.e super(this)
7394+
// this.x; super();
7395+
// We want to make sure that super-call is done before accessing "this" so that
7396+
// "this" is not accessed as a parameter of the super-call.
7397+
if (!superCall || superCall.end > node.pos) {
7398+
// In ES6, super inside constructor of class-declaration has to precede "this" accessing
7399+
error(node, Diagnostics.super_must_be_called_before_accessing_this_in_the_constructor_of_a_derived_class);
7400+
}
73427401
}
73437402
}
73447403

@@ -10287,14 +10346,11 @@ namespace ts {
1028710346
checkGrammarTypeArguments(node, node.typeArguments) || checkGrammarArguments(node, node.arguments);
1028810347

1028910348
const signature = getResolvedSignature(node);
10290-
if (node.expression.kind === SyntaxKind.SuperKeyword) {
10291-
const containingFunction = getContainingFunction(node.expression);
1029210349

10293-
if (containingFunction && containingFunction.kind === SyntaxKind.Constructor) {
10294-
getNodeLinks(containingFunction).flags |= NodeCheckFlags.HasSeenSuperCall;
10295-
}
10350+
if (node.expression.kind === SyntaxKind.SuperKeyword) {
1029610351
return voidType;
1029710352
}
10353+
1029810354
if (node.kind === SyntaxKind.NewExpression) {
1029910355
const declaration = signature.declaration;
1030010356

@@ -11875,17 +11931,15 @@ namespace ts {
1187511931
}
1187611932

1187711933
// TS 1.0 spec (April 2014): 8.3.2
11878-
// Constructors of classes with no extends clause may not contain super calls, whereas
11879-
// constructors of derived classes must contain at least one super call somewhere in their function body.
11934+
// Constructors of classes with no extends clause and constructors of classes that extends null may not contain super calls,
11935+
// whereas constructors of derived classes must contain at least one super call somewhere in their function body.
1188011936
const containingClassDecl = <ClassDeclaration>node.parent;
1188111937
if (getClassExtendsHeritageClauseElement(containingClassDecl)) {
11882-
const containingClassSymbol = getSymbolOfNode(containingClassDecl);
11883-
const containingClassInstanceType = <InterfaceType>getDeclaredTypeOfSymbol(containingClassSymbol);
11884-
const baseConstructorType = getBaseConstructorTypeOfClass(containingClassInstanceType);
11885-
11886-
if (containsSuperCall(node.body)) {
11887-
if (baseConstructorType === nullType) {
11888-
error(node, Diagnostics.A_constructor_cannot_contain_a_super_call_when_its_class_extends_null);
11938+
const classExtendsNull = classDeclarationExtendsNull(containingClassDecl);
11939+
const superCall = getSuperCallInConstructor(node);
11940+
if (superCall) {
11941+
if (classExtendsNull) {
11942+
error(superCall, Diagnostics.A_constructor_cannot_contain_a_super_call_when_its_class_extends_null);
1188911943
}
1189011944

1189111945
// The first statement in the body of a constructor (excluding prologue directives) must be a super call
@@ -11902,6 +11956,7 @@ namespace ts {
1190211956
if (superCallShouldBeFirst) {
1190311957
const statements = (<Block>node.body).statements;
1190411958
let superCallStatement: ExpressionStatement;
11959+
1190511960
for (const statement of statements) {
1190611961
if (statement.kind === SyntaxKind.ExpressionStatement && isSuperCallExpression((<ExpressionStatement>statement).expression)) {
1190711962
superCallStatement = <ExpressionStatement>statement;
@@ -11916,7 +11971,7 @@ namespace ts {
1191611971
}
1191711972
}
1191811973
}
11919-
else if (baseConstructorType !== nullType) {
11974+
else if (!classExtendsNull) {
1192011975
error(node, Diagnostics.Constructors_for_derived_classes_must_contain_a_super_call);
1192111976
}
1192211977
}

‎src/compiler/types.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,6 @@ namespace ts {
430430
IntrinsicElement = IntrinsicNamedElement | IntrinsicIndexedElement,
431431
}
432432

433-
434433
/* @internal */
435434
export const enum RelationComparisonResult {
436435
Succeeded = 1, // Should be truthy
@@ -2041,10 +2040,9 @@ namespace ts {
20412040
LoopWithCapturedBlockScopedBinding = 0x00010000, // Loop that contains block scoped variable captured in closure
20422041
CapturedBlockScopedBinding = 0x00020000, // Block-scoped binding that is captured in some function
20432042
BlockScopedBindingInLoop = 0x00040000, // Block-scoped binding with declaration nested inside iteration statement
2044-
HasSeenSuperCall = 0x00080000, // Set during the binding when encounter 'super'
2045-
ClassWithBodyScopedClassBinding = 0x00100000, // Decorated class that contains a binding to itself inside of the class body.
2046-
BodyScopedClassBinding = 0x00200000, // Binding to a decorated class inside of the class's body.
2047-
NeedsLoopOutParameter = 0x00400000, // Block scoped binding whose value should be explicitly copied outside of the converted loop
2043+
ClassWithBodyScopedClassBinding = 0x00080000, // Decorated class that contains a binding to itself inside of the class body.
2044+
BodyScopedClassBinding = 0x00100000, // Binding to a decorated class inside of the class's body.
2045+
NeedsLoopOutParameter = 0x00200000, // Block scoped binding whose value should be explicitly copied outside of the converted loop
20482046
}
20492047

20502048
/* @internal */
@@ -2064,6 +2062,8 @@ namespace ts {
20642062
importOnRightSide?: Symbol; // for import declarations - import that appear on the right side
20652063
jsxFlags?: JsxFlags; // flags for knowing what kind of element/attributes we're dealing with
20662064
resolvedJsxType?: Type; // resolved element attributes type of a JSX openinglike element
2065+
hasSuperCall?: boolean; // recorded result when we try to find super-call. We only try to find one if this flag is undefined, indicating that we haven't made an attempt.
2066+
superCall?: ExpressionStatement; // Cached first super-call found in the constructor. Used in checking whether super is called before this-accessing
20672067
}
20682068

20692069
export const enum TypeFlags {

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
1-
tests/cases/compiler/classExtendsNull.ts(2,5): error TS17005: A constructor cannot contain a 'super' call when its class extends 'null'
1+
tests/cases/compiler/classExtendsNull.ts(3,9): error TS17005: A constructor cannot contain a 'super' call when its class extends 'null'
22

33

44
==== tests/cases/compiler/classExtendsNull.ts (1 errors) ====
55
class C extends null {
66
constructor() {
7-
~~~~~~~~~~~~~~~
87
super();
9-
~~~~~~~~~~~~~~~~
8+
~~~~~~~
9+
!!! error TS17005: A constructor cannot contain a 'super' call when its class extends 'null'
1010
return Object.create(null);
11-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1211
}
13-
~~~~~
14-
!!! error TS17005: A constructor cannot contain a 'super' call when its class extends 'null'
1512
}
1613

1714
class D extends null {

‎tests/baselines/reference/superCallBeforeThisAccessing1.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
//// [superCallBeforeThisAccessing1.ts]
2+
declare var Factory: any
3+
4+
class Base {
5+
constructor(c) { }
6+
}
7+
class D extends Base {
8+
private _t;
9+
constructor() {
10+
super(i);
11+
var s = {
12+
t: this._t
13+
}
14+
var i = Factory.create(s);
15+
}
16+
}
17+
18+
19+
//// [superCallBeforeThisAccessing1.js]
20+
var __extends = (this && this.__extends) || function (d, b) {
21+
for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
22+
function __() { this.constructor = d; }
23+
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
24+
};
25+
var Base = (function () {
26+
function Base(c) {
27+
}
28+
return Base;
29+
}());
30+
var D = (function (_super) {
31+
__extends(D, _super);
32+
function D() {
33+
_super.call(this, i);
34+
var s = {
35+
t: this._t
36+
};
37+
var i = Factory.create(s);
38+
}
39+
return D;
40+
}(Base));

‎tests/baselines/reference/superCallBeforeThisAccessing1.symbols‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
=== tests/cases/conformance/es6/classDeclaration/superCallBeforeThisAccessing1.ts ===
2+
declare var Factory: any
3+
>Factory : Symbol(Factory, Decl(superCallBeforeThisAccessing1.ts, 0, 11))
4+
5+
class Base {
6+
>Base : Symbol(Base, Decl(superCallBeforeThisAccessing1.ts, 0, 24))
7+
8+
constructor(c) { }
9+
>c : Symbol(c, Decl(superCallBeforeThisAccessing1.ts, 3, 16))
10+
}
11+
class D extends Base {
12+
>D : Symbol(D, Decl(superCallBeforeThisAccessing1.ts, 4, 1))
13+
>Base : Symbol(Base, Decl(superCallBeforeThisAccessing1.ts, 0, 24))
14+
15+
private _t;
16+
>_t : Symbol(_t, Decl(superCallBeforeThisAccessing1.ts, 5, 22))
17+
18+
constructor() {
19+
super(i);
20+
>super : Symbol(Base, Decl(superCallBeforeThisAccessing1.ts, 0, 24))
21+
>i : Symbol(i, Decl(superCallBeforeThisAccessing1.ts, 12, 11))
22+
23+
var s = {
24+
>s : Symbol(s, Decl(superCallBeforeThisAccessing1.ts, 9, 11))
25+
26+
t: this._t
27+
>t : Symbol(t, Decl(superCallBeforeThisAccessing1.ts, 9, 17))
28+
>this._t : Symbol(_t, Decl(superCallBeforeThisAccessing1.ts, 5, 22))
29+
>this : Symbol(D, Decl(superCallBeforeThisAccessing1.ts, 4, 1))
30+
>_t : Symbol(_t, Decl(superCallBeforeThisAccessing1.ts, 5, 22))
31+
}
32+
var i = Factory.create(s);
33+
>i : Symbol(i, Decl(superCallBeforeThisAccessing1.ts, 12, 11))
34+
>Factory : Symbol(Factory, Decl(superCallBeforeThisAccessing1.ts, 0, 11))
35+
>s : Symbol(s, Decl(superCallBeforeThisAccessing1.ts, 9, 11))
36+
}
37+
}
38+

‎tests/baselines/reference/superCallBeforeThisAccessing1.types‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
=== tests/cases/conformance/es6/classDeclaration/superCallBeforeThisAccessing1.ts ===
2+
declare var Factory: any
3+
>Factory : any
4+
5+
class Base {
6+
>Base : Base
7+
8+
constructor(c) { }
9+
>c : any
10+
}
11+
class D extends Base {
12+
>D : D
13+
>Base : Base
14+
15+
private _t;
16+
>_t : any
17+
18+
constructor() {
19+
super(i);
20+
>super(i) : void
21+
>super : typeof Base
22+
>i : any
23+
24+
var s = {
25+
>s : { t: any; }
26+
>{ t: this._t } : { t: any; }
27+
28+
t: this._t
29+
>t : any
30+
>this._t : any
31+
>this : this
32+
>_t : any
33+
}
34+
var i = Factory.create(s);
35+
>i : any
36+
>Factory.create(s) : any
37+
>Factory.create : any
38+
>Factory : any
39+
>create : any
40+
>s : { t: any; }
41+
}
42+
}
43+

‎tests/baselines/reference/superCallBeforeThisAccessing2.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//// [superCallBeforeThisAccessing2.ts]
2+
class Base {
3+
constructor(c) { }
4+
}
5+
class D extends Base {
6+
private _t;
7+
constructor() {
8+
super(() => { this._t }); // no error. only check when this is directly accessing in constructor
9+
}
10+
}
11+
12+
13+
//// [superCallBeforeThisAccessing2.js]
14+
var __extends = (this && this.__extends) || function (d, b) {
15+
for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
16+
function __() { this.constructor = d; }
17+
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
18+
};
19+
var Base = (function () {
20+
function Base(c) {
21+
}
22+
return Base;
23+
}());
24+
var D = (function (_super) {
25+
__extends(D, _super);
26+
function D() {
27+
var _this = this;
28+
_super.call(this, function () { _this._t; }); // no error. only check when this is directly accessing in constructor
29+
}
30+
return D;
31+
}(Base));

‎tests/baselines/reference/superCallBeforeThisAccessing2.symbols‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
=== tests/cases/conformance/es6/classDeclaration/superCallBeforeThisAccessing2.ts ===
2+
class Base {
3+
>Base : Symbol(Base, Decl(superCallBeforeThisAccessing2.ts, 0, 0))
4+
5+
constructor(c) { }
6+
>c : Symbol(c, Decl(superCallBeforeThisAccessing2.ts, 1, 16))
7+
}
8+
class D extends Base {
9+
>D : Symbol(D, Decl(superCallBeforeThisAccessing2.ts, 2, 1))
10+
>Base : Symbol(Base, Decl(superCallBeforeThisAccessing2.ts, 0, 0))
11+
12+
private _t;
13+
>_t : Symbol(_t, Decl(superCallBeforeThisAccessing2.ts, 3, 22))
14+
15+
constructor() {
16+
super(() => { this._t }); // no error. only check when this is directly accessing in constructor
17+
>super : Symbol(Base, Decl(superCallBeforeThisAccessing2.ts, 0, 0))
18+
>this._t : Symbol(_t, Decl(superCallBeforeThisAccessing2.ts, 3, 22))
19+
>this : Symbol(D, Decl(superCallBeforeThisAccessing2.ts, 2, 1))
20+
>_t : Symbol(_t, Decl(superCallBeforeThisAccessing2.ts, 3, 22))
21+
}
22+
}
23+

‎tests/baselines/reference/superCallBeforeThisAccessing2.types‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
=== tests/cases/conformance/es6/classDeclaration/superCallBeforeThisAccessing2.ts ===
2+
class Base {
3+
>Base : Base
4+
5+
constructor(c) { }
6+
>c : any
7+
}
8+
class D extends Base {
9+
>D : D
10+
>Base : Base
11+
12+
private _t;
13+
>_t : any
14+
15+
constructor() {
16+
super(() => { this._t }); // no error. only check when this is directly accessing in constructor
17+
>super(() => { this._t }) : void
18+
>super : typeof Base
19+
>() => { this._t } : () => void
20+
>this._t : any
21+
>this : this
22+
>_t : any
23+
}
24+
}
25+

0 commit comments

Comments
 (0)