-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Error when accessing this before super #4814
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
Changes from all commits
55254be
e73a6f8
07f8ddf
e0b6f7a
6e4574f
3be5161
70a6fe0
9109595
cfd1a76
7b6dc92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6577,6 +6577,21 @@ namespace ts { | |
| let container = getThisContainer(node, /* includeArrowFunctions */ true); | ||
| let needToCaptureLexicalThis = false; | ||
|
|
||
|
|
||
| if (container.kind === SyntaxKind.Constructor) { | ||
| // Keep track of whether we have seen "super" before encounter "this" so that | ||
| // we can report appropriate error later in checkConstructorDeclaration | ||
| // We have to do the check here to make sure we won't give false error when | ||
| // "this" is used in arrow functions | ||
| // For example: | ||
| // constructor() { | ||
| // (()=>this); // No Error | ||
| // super(); | ||
| // } | ||
| let nodeLinks = getNodeLinks(container); | ||
| nodeLinks.flags |= NodeCheckFlags.HasSeenThisCall; | ||
| } | ||
|
|
||
| // Now skip arrow functions to get the "real" owner of 'this'. | ||
| if (container.kind === SyntaxKind.ArrowFunction) { | ||
| container = getThisContainer(container, /* includeArrowFunctions */ false); | ||
|
|
@@ -9285,6 +9300,14 @@ namespace ts { | |
|
|
||
| let signature = getResolvedSignature(node); | ||
| if (node.expression.kind === SyntaxKind.SuperKeyword) { | ||
| let containgFunction = getContainingFunction(node.expression); | ||
|
|
||
| if (containgFunction && containgFunction.kind === SyntaxKind.Constructor) { | ||
| let nodeLinks = getNodeLinks(containgFunction); | ||
| if (!(nodeLinks.flags & NodeCheckFlags.HasSeenThisCall)) { | ||
| nodeLinks.flags |= NodeCheckFlags.HasSeenSuperBeforeThis; | ||
| } | ||
| } | ||
| return voidType; | ||
| } | ||
| if (node.kind === SyntaxKind.NewExpression) { | ||
|
|
@@ -10794,8 +10817,25 @@ namespace ts { | |
| let containingClassSymbol = getSymbolOfNode(containingClassDecl); | ||
| let containingClassInstanceType = <InterfaceType>getDeclaredTypeOfSymbol(containingClassSymbol); | ||
| let baseConstructorType = getBaseConstructorTypeOfClass(containingClassInstanceType); | ||
| let statements = (<Block>node.body).statements; | ||
| let superCallStatement: ExpressionStatement; | ||
| let isSuperCallFirstStatment: boolean; | ||
|
|
||
| for (let statement of statements) { | ||
| if (statement.kind === SyntaxKind.ExpressionStatement && isSuperCallExpression((<ExpressionStatement>statement).expression)) { | ||
| superCallStatement = <ExpressionStatement>statement; | ||
| if (isSuperCallFirstStatment === undefined) { | ||
| isSuperCallFirstStatment = true; | ||
| } | ||
| } | ||
| else if (isSuperCallFirstStatment === undefined && !isPrologueDirective(statement)) { | ||
| isSuperCallFirstStatment = false; | ||
| } | ||
| } | ||
|
|
||
| if (containsSuperCall(node.body)) { | ||
| // The main different between looping through each statement in constructor and calling containsSuperCall is that, | ||
| // containsSuperCall will consider "super" inside computed-property for inner class declaration | ||
| if (superCallStatement || containsSuperCall(node.body)) { | ||
| if (baseConstructorType === nullType) { | ||
| error(node, Diagnostics.A_constructor_cannot_contain_a_super_call_when_its_class_extends_null); | ||
| } | ||
|
|
@@ -10812,25 +10852,18 @@ namespace ts { | |
| // Skip past any prologue directives to find the first statement | ||
| // to ensure that it was a super call. | ||
| if (superCallShouldBeFirst) { | ||
| let statements = (<Block>node.body).statements; | ||
| let superCallStatement: ExpressionStatement; | ||
| for (let statement of statements) { | ||
| if (statement.kind === SyntaxKind.ExpressionStatement && isSuperCallExpression((<ExpressionStatement>statement).expression)) { | ||
| superCallStatement = <ExpressionStatement>statement; | ||
| break; | ||
| } | ||
| if (!isPrologueDirective(statement)) { | ||
| break; | ||
| } | ||
| } | ||
| if (!superCallStatement) { | ||
| error(node, Diagnostics.A_super_call_must_be_the_first_statement_in_the_constructor_when_a_class_contains_initialized_properties_or_has_parameter_properties); | ||
| if (!isSuperCallFirstStatment) { | ||
| error(superCallStatement, Diagnostics.A_super_call_must_be_the_first_statement_in_the_constructor_when_a_class_contains_initialized_properties_or_has_parameter_properties); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this our requirement? In ES6 it is legal to have statements before There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The check our requirement instead of the ES6. In particular, the error is used in the following case: class B {}
class C extends B {
public x = 10; // without this we won't give an error
constructor() {
var x = 10;
super();
}
} |
||
| } | ||
| else { | ||
| // In such a required super call, it is a compile-time error for argument expressions to reference this. | ||
| markThisReferencesAsErrors(superCallStatement.expression); | ||
| } | ||
| } | ||
| else if (!(getNodeCheckFlags(node) & NodeCheckFlags.HasSeenSuperBeforeThis)) { | ||
| // In ES6, super inside constructor of class-declaration has to precede "this" accessing | ||
| error(superCallStatement, Diagnostics.super_must_be_called_before_accessing_this_in_the_constructor_of_a_derived_class); | ||
| } | ||
| } | ||
| else if (baseConstructorType !== nullType) { | ||
| error(node, Diagnostics.Constructors_for_derived_classes_must_contain_a_super_call); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| //// [checkSuperCallBeforeThisAccessing1.ts] | ||
| class Based { } | ||
| class Derived extends Based { | ||
| public x: number; | ||
| constructor() { | ||
| super(); | ||
| this; | ||
| this.x = 10; | ||
| var that = this; | ||
| } | ||
| } | ||
|
|
||
| //// [checkSuperCallBeforeThisAccessing1.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 Based = (function () { | ||
| function Based() { | ||
| } | ||
| return Based; | ||
| })(); | ||
| var Derived = (function (_super) { | ||
| __extends(Derived, _super); | ||
| function Derived() { | ||
| _super.call(this); | ||
| this; | ||
| this.x = 10; | ||
| var that = this; | ||
| } | ||
| return Derived; | ||
| })(Based); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| === tests/cases/compiler/checkSuperCallBeforeThisAccessing1.ts === | ||
| class Based { } | ||
| >Based : Symbol(Based, Decl(checkSuperCallBeforeThisAccessing1.ts, 0, 0)) | ||
|
|
||
| class Derived extends Based { | ||
| >Derived : Symbol(Derived, Decl(checkSuperCallBeforeThisAccessing1.ts, 0, 15)) | ||
| >Based : Symbol(Based, Decl(checkSuperCallBeforeThisAccessing1.ts, 0, 0)) | ||
|
|
||
| public x: number; | ||
| >x : Symbol(x, Decl(checkSuperCallBeforeThisAccessing1.ts, 1, 29)) | ||
|
|
||
| constructor() { | ||
| super(); | ||
| >super : Symbol(Based, Decl(checkSuperCallBeforeThisAccessing1.ts, 0, 0)) | ||
|
|
||
| this; | ||
| >this : Symbol(Derived, Decl(checkSuperCallBeforeThisAccessing1.ts, 0, 15)) | ||
|
|
||
| this.x = 10; | ||
| >this.x : Symbol(x, Decl(checkSuperCallBeforeThisAccessing1.ts, 1, 29)) | ||
| >this : Symbol(Derived, Decl(checkSuperCallBeforeThisAccessing1.ts, 0, 15)) | ||
| >x : Symbol(x, Decl(checkSuperCallBeforeThisAccessing1.ts, 1, 29)) | ||
|
|
||
| var that = this; | ||
| >that : Symbol(that, Decl(checkSuperCallBeforeThisAccessing1.ts, 7, 11)) | ||
| >this : Symbol(Derived, Decl(checkSuperCallBeforeThisAccessing1.ts, 0, 15)) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| === tests/cases/compiler/checkSuperCallBeforeThisAccessing1.ts === | ||
| class Based { } | ||
| >Based : Based | ||
|
|
||
| class Derived extends Based { | ||
| >Derived : Derived | ||
| >Based : Based | ||
|
|
||
| public x: number; | ||
| >x : number | ||
|
|
||
| constructor() { | ||
| super(); | ||
| >super() : void | ||
| >super : typeof Based | ||
|
|
||
| this; | ||
| >this : this | ||
|
|
||
| this.x = 10; | ||
| >this.x = 10 : number | ||
| >this.x : number | ||
| >this : this | ||
| >x : number | ||
| >10 : number | ||
|
|
||
| var that = this; | ||
| >that : this | ||
| >this : this | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| tests/cases/compiler/checkSuperCallBeforeThisAccessing2.ts(6,9): error TS17006: 'super' must be called before accessing 'this' in the constructor of a derived class. | ||
|
|
||
|
|
||
| ==== tests/cases/compiler/checkSuperCallBeforeThisAccessing2.ts (1 errors) ==== | ||
| class Based { } | ||
| class Derived extends Based { | ||
| public x: number; | ||
| constructor() { | ||
| this.x = 100; | ||
| super(); | ||
| ~~~~~~~~ | ||
| !!! error TS17006: 'super' must be called before accessing 'this' in the constructor of a derived class. | ||
| this.x = 10; | ||
| var that = this; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| //// [checkSuperCallBeforeThisAccessing2.ts] | ||
| class Based { } | ||
| class Derived extends Based { | ||
| public x: number; | ||
| constructor() { | ||
| this.x = 100; | ||
| super(); | ||
| this.x = 10; | ||
| var that = this; | ||
| } | ||
| } | ||
|
|
||
| //// [checkSuperCallBeforeThisAccessing2.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 Based = (function () { | ||
| function Based() { | ||
| } | ||
| return Based; | ||
| })(); | ||
| var Derived = (function (_super) { | ||
| __extends(Derived, _super); | ||
| function Derived() { | ||
| this.x = 100; | ||
| _super.call(this); | ||
| this.x = 10; | ||
| var that = this; | ||
| } | ||
| return Derived; | ||
| })(Based); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| //// [checkSuperCallBeforeThisAccessing3.ts] | ||
| class Based { } | ||
| class Derived extends Based { | ||
| public x: number; | ||
| constructor() { | ||
| class innver { | ||
| public y: boolean; | ||
| constructor() { | ||
| this.y = true; | ||
| } | ||
| } | ||
| super(); | ||
| this.x = 10; | ||
| var that = this; | ||
| } | ||
| } | ||
|
|
||
| //// [checkSuperCallBeforeThisAccessing3.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 Based = (function () { | ||
| function Based() { | ||
| } | ||
| return Based; | ||
| })(); | ||
| var Derived = (function (_super) { | ||
| __extends(Derived, _super); | ||
| function Derived() { | ||
| var innver = (function () { | ||
| function innver() { | ||
| this.y = true; | ||
| } | ||
| return innver; | ||
| })(); | ||
| _super.call(this); | ||
| this.x = 10; | ||
| var that = this; | ||
| } | ||
| return Derived; | ||
| })(Based); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| === tests/cases/compiler/checkSuperCallBeforeThisAccessing3.ts === | ||
| class Based { } | ||
| >Based : Symbol(Based, Decl(checkSuperCallBeforeThisAccessing3.ts, 0, 0)) | ||
|
|
||
| class Derived extends Based { | ||
| >Derived : Symbol(Derived, Decl(checkSuperCallBeforeThisAccessing3.ts, 0, 15)) | ||
| >Based : Symbol(Based, Decl(checkSuperCallBeforeThisAccessing3.ts, 0, 0)) | ||
|
|
||
| public x: number; | ||
| >x : Symbol(x, Decl(checkSuperCallBeforeThisAccessing3.ts, 1, 29)) | ||
|
|
||
| constructor() { | ||
| class innver { | ||
| >innver : Symbol(innver, Decl(checkSuperCallBeforeThisAccessing3.ts, 3, 19)) | ||
|
|
||
| public y: boolean; | ||
| >y : Symbol(y, Decl(checkSuperCallBeforeThisAccessing3.ts, 4, 22)) | ||
|
|
||
| constructor() { | ||
| this.y = true; | ||
| >this.y : Symbol(y, Decl(checkSuperCallBeforeThisAccessing3.ts, 4, 22)) | ||
| >this : Symbol(innver, Decl(checkSuperCallBeforeThisAccessing3.ts, 3, 19)) | ||
| >y : Symbol(y, Decl(checkSuperCallBeforeThisAccessing3.ts, 4, 22)) | ||
| } | ||
| } | ||
| super(); | ||
| >super : Symbol(Based, Decl(checkSuperCallBeforeThisAccessing3.ts, 0, 0)) | ||
|
|
||
| this.x = 10; | ||
| >this.x : Symbol(x, Decl(checkSuperCallBeforeThisAccessing3.ts, 1, 29)) | ||
| >this : Symbol(Derived, Decl(checkSuperCallBeforeThisAccessing3.ts, 0, 15)) | ||
| >x : Symbol(x, Decl(checkSuperCallBeforeThisAccessing3.ts, 1, 29)) | ||
|
|
||
| var that = this; | ||
| >that : Symbol(that, Decl(checkSuperCallBeforeThisAccessing3.ts, 12, 11)) | ||
| >this : Symbol(Derived, Decl(checkSuperCallBeforeThisAccessing3.ts, 0, 15)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we special case
thisbeforesuper, or should we consider a more general purpose approach to TDZ that encompasses boththisbeforesuperand use before declaration forletandconst? I imagine there may be more cases like this in the future.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you though I think that will need to be a separate change. Here is a issue #5232