Skip to content
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

Allowed non-this, non-super code before super call in derived classes with property initializers #29374

Merged
merged 61 commits into from
Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
6845db9
Allowed non-this, non-super code before super call in derived classes
Jan 11, 2019
dbd3a2a
Used new isNewThisScope utility and dummy prop in baselines
Jan 11, 2019
5aa79cd
Accounted for parameters and get/set accessor bodies
Jan 11, 2019
02516cc
Accounted for class extensions
Jan 12, 2019
00b4a2b
(node|statement)RefersToSuperOrThis wasn't checking root level scope …
Jan 12, 2019
6d06b21
Better 'references' name and comments; accounted for more method edge…
Jan 13, 2019
ccd52fc
Limited super calls to root-level statements in constructor bodies
Jan 15, 2019
da71ed9
Merge branch 'master'
Jan 15, 2019
b4a9ac2
Fixed error number following 'master' merge
Jan 15, 2019
0d91548
Added decorator test cases
Jan 21, 2019
e4dca14
Again allowed arrow functions
Jan 21, 2019
c8fc681
Merge branch 'master'
Jul 16, 2019
cf52812
Accepted new baselines
Jul 16, 2019
a39761d
Added allowance for (super())
Jul 19, 2019
afc58e1
Reworked emitter transforms for ES this binding semantics
Sep 2, 2019
a48d1ea
Used skipOuterExpressions when diving for super()s; fix prop ordering
Sep 2, 2019
efbe416
Allow direct var _this = super() when no pre-super() statements; lint…
Sep 2, 2019
429347d
Always with the TSLint
Sep 2, 2019
03da7f7
One last touchup: skipOuterExpressions in es2015 transformer
Sep 2, 2019
142704e
Merge branch 'master'
Oct 21, 2019
e71bee1
Fixed new lint complaint in utilities.ts
Oct 21, 2019
4cef299
Again added a falls-through; it'd be swell if I could run linting loc…
Oct 21, 2019
660feb2
This time I think I got it
Oct 21, 2019
e425e9b
Well at least the error is a different one
Oct 21, 2019
041c58a
Merge branch 'master'
JoshuaKGoldberg Feb 15, 2020
74e6972
Undid irrelevant whitespace changes
JoshuaKGoldberg Feb 16, 2020
093b1b7
Mostly addressed private/field issues
JoshuaKGoldberg Feb 28, 2020
7d99293
Merge branch 'master' of https://github.com/microsoft/TypeScript into…
orta Mar 3, 2020
c09a156
Accepted derivedClassSuperProperties baseline
JoshuaKGoldberg Mar 4, 2020
25c5c10
Merge branch 'master'
JoshuaKGoldberg Mar 5, 2020
39207d2
Lint fix, lovely
JoshuaKGoldberg Mar 5, 2020
25aff6c
Remove now-unnecesary comment
JoshuaKGoldberg Mar 6, 2020
f33707f
Merge branch 'master'
Dec 2, 2020
34ad5b3
First round of feedback
Dec 2, 2020
9022a17
Moved prologue statements to start of statements
Dec 2, 2020
cd82cc5
Added consideration for super statements in loops and the like
Dec 2, 2020
2629491
Ordering and a _this_1 test
Dec 2, 2020
8039315
Missed the one change I needed...
Dec 2, 2020
eb7388a
Merge branch 'master'
JoshuaKGoldberg Jan 8, 2021
39ffad6
First round of feedback corrections
JoshuaKGoldberg Jan 11, 2021
26fc18c
Feedback round two: statements
JoshuaKGoldberg Jan 11, 2021
0448f54
Feedback: used more direct statements
JoshuaKGoldberg Jan 11, 2021
70e741b
Fixed classFields emit to not duplicate temp variables
JoshuaKGoldberg Jan 11, 2021
d5ad9c3
Refactored es2015 helper to be less overloaded
JoshuaKGoldberg Jan 11, 2021
fe2ac08
Accounted for parentheses
JoshuaKGoldberg Jan 11, 2021
360ad2e
Simpler feedback: -1, and emptyArray
JoshuaKGoldberg Jan 11, 2021
23780a1
Next feedback: superStatementIndex
JoshuaKGoldberg Jan 11, 2021
4b0bd82
Feedback: simplified to no longer create slice arrays
JoshuaKGoldberg Jan 12, 2021
6319af5
Adjusted for default and rest parameters
Jan 14, 2021
e434962
Added test case for commas
Jan 14, 2021
bacc434
Corrected comment ranges
Jan 21, 2021
b250f5f
Merge branch 'master' into non-this-super-before-super
rbuckton Jan 25, 2021
44d7a9a
Handled comments after super, with tests
Jan 27, 2021
f16ccd4
Merge branch 'master'
Jan 27, 2021
44a164e
Merge branch 'master'
JoshuaKGoldberg Feb 24, 2021
af16920
Merge branch 'master'
May 28, 2021
47fc125
Merge branch 'main'
Sep 11, 2021
ffdc7f8
Merge branch 'main'
Oct 18, 2021
dea7150
Merge branch 'main'
JoshuaKGoldberg Jan 13, 2022
39d9901
Fixed Bad/Late super baselines
JoshuaKGoldberg Jan 13, 2022
1b3dd6d
Remove unused param and unnecessary baseline comments
Jan 13, 2022
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
62 changes: 46 additions & 16 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29242,33 +29242,41 @@ namespace ts {
error(superCall, Diagnostics.A_constructor_cannot_contain_a_super_call_when_its_class_extends_null);
}

// The first statement in the body of a constructor (excluding prologue directives) must be a super call
// A super call must be root-level (excluding prologue directives) in a constructor
// if both of the following are true:
// - The containing class is a derived class.
// - The constructor declares parameter properties
// or the containing class declares instance member variables with initializers.
const superCallShouldBeFirst =
const superCallShouldBeRootLevel =
some((<ClassDeclaration>node.parent).members, isInstancePropertyWithInitializerOrPrivateIdentifierProperty) ||
some(node.parameters, p => hasModifier(p, ModifierFlags.ParameterPropertyModifier));

// Skip past any prologue directives to find the first statement
// to ensure that it was a super call.
if (superCallShouldBeFirst) {
const statements = node.body!.statements;
let superCallStatement: ExpressionStatement | undefined;
if (superCallShouldBeRootLevel) {
// Until we have better flow analysis, it is an error to place the super call within any kind of block or conditional
// See GH #8277
if (!superCallIsRootLevelInConstructor(superCall, node.body!)) {
error(superCall, Diagnostics.A_super_call_must_be_a_root_level_statement_within_a_constructor_of_a_derived_class_that_contains_initialized_properties_parameter_properties_or_private_identifiers);
}
// Skip past any prologue directives to check statements for referring to 'super' or 'this' before a super call
else {
let superCallStatement: ExpressionStatement | undefined;

for (const statement of statements) {
if (statement.kind === SyntaxKind.ExpressionStatement && isSuperCall((<ExpressionStatement>statement).expression)) {
superCallStatement = <ExpressionStatement>statement;
break;
for (const statement of node.body!.statements) {
if (isExpressionStatement(statement) && isSuperCall(skipOuterExpressions(statement.expression))) {
superCallStatement = statement;
break;
}
if (!isPrologueDirective(statement) && nodeImmediatelyReferencesSuperOrThis(statement)) {
break;
}
}
if (!isPrologueDirective(statement)) {
break;

// Until we have better flow analysis, it is an error to place the super call within any kind of block or conditional
// See GH #8277
if (superCallStatement === undefined) {
error(node, Diagnostics.A_super_call_must_be_the_first_statement_in_the_constructor_to_refer_to_super_or_this_when_a_derived_class_contains_initialized_properties_parameter_properties_or_private_identifiers);
}
}
if (!superCallStatement) {
error(node, Diagnostics.A_super_call_must_be_the_first_statement_in_the_constructor_when_a_class_contains_initialized_properties_parameter_properties_or_private_identifiers);
}
}
}
else if (!classExtendsNull) {
Expand All @@ -29277,6 +29285,28 @@ namespace ts {
}
}

function superCallIsRootLevelInConstructor(superCall: Node, body: Block) {
let expressionParent = superCall.parent;

while (isParenthesizedExpression(expressionParent)) {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
expressionParent = expressionParent.parent;
}

return expressionParent.parent === body;
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
}

function nodeImmediatelyReferencesSuperOrThis(node: Node): boolean {
if (node.kind === SyntaxKind.SuperKeyword || node.kind === SyntaxKind.ThisKeyword) {
return true;
}

if (isNewOrDelayedThisScope(node)) {
return false;
}

return !!forEachChild(node, nodeImmediatelyReferencesSuperOrThis);
}

function checkAccessorDeclaration(node: AccessorDeclaration) {
if (produceDiagnostics) {
// Grammar checking accessors
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,7 @@
"category": "Error",
"code": 2375
},
"A 'super' call must be the first statement in the constructor when a class contains initialized properties, parameter properties, or private identifiers.": {
"A 'super' call must be the first statement in the constructor to refer to 'super' or 'this' when a derived class contains initialized properties, parameter properties, or private identifiers.": {
"category": "Error",
"code": 2376
},
Expand Down Expand Up @@ -2903,6 +2903,10 @@
"category": "Error",
"code": 2784
},
"A 'super' call must be a root-level statement within a constructor of a derived class that contains initialized properties, parameter properties, or private identifiers.": {
"category": "Error",
"code": 2785
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
124 changes: 104 additions & 20 deletions src/compiler/transformers/classFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -631,10 +631,27 @@ namespace ts {

resumeLexicalEnvironment();

let indexOfFirstStatement = 0;
const needsSyntheticConstructor = !constructor && isDerivedClass;
let indexOfFirstStatementAfterSuper = 0;
let indexAfterLastPrologueStatement = 0;
let foundSuperStatement = false;
let statements: Statement[] = [];

if (!constructor && isDerivedClass) {
if (constructor) {
const initialDirectivesAndSuperCall = addPrologueDirectivesAndInitialSuperCall(constructor, statements, visitor);
indexOfFirstStatementAfterSuper = initialDirectivesAndSuperCall.indexOfFirstStatementAfterSuper;
indexAfterLastPrologueStatement = initialDirectivesAndSuperCall.indexAfterLastPrologueStatement;
foundSuperStatement = initialDirectivesAndSuperCall.foundSuperStatement;

// Add existing statements before any super call
statements.splice(
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
indexAfterLastPrologueStatement,
0,
...visitNodes(constructor.body!.statements, visitor, isStatement, indexAfterLastPrologueStatement, indexOfFirstStatementAfterSuper - indexAfterLastPrologueStatement - (foundSuperStatement ? 1 : 0)),
);
}

if (needsSyntheticConstructor) {
// Add a synthetic `super` call:
//
// super(...arguments);
Expand All @@ -650,10 +667,9 @@ namespace ts {
);
}

if (constructor) {
indexOfFirstStatement = addPrologueDirectivesAndInitialSuperCall(constructor, statements, visitor);
}
// Add the property initializers. Transforms this:
// Add the property initializers.
//
// If we don't useDefineForClassFields, we directly convert to exressions. Transforms this:
//
// public x = 1;
//
Expand All @@ -663,23 +679,57 @@ namespace ts {
// this.x = 1;
// }
//
if (constructor?.body) {
let afterParameterProperties = findIndex(constructor.body.statements, s => !isParameterPropertyDeclaration(getOriginalNode(s), constructor), indexOfFirstStatement);
if (afterParameterProperties === -1) {
afterParameterProperties = constructor.body.statements.length;
// If we do useDefineForClassFields, they'll be converted elsewhere.
// We instead *remove* them from the transformed output at this stage.
let parameterPropertyDeclarationCount = 0;
if (constructor && constructor.body) {
if (useDefineForClassFields) {
statements = statements.filter(statement => !isParameterPropertyDeclaration(getOriginalNode(statement), constructor));
}
if (afterParameterProperties > indexOfFirstStatement) {
if (!useDefineForClassFields) {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
addRange(statements, visitNodes(constructor.body.statements, visitor, isStatement, indexOfFirstStatement, afterParameterProperties - indexOfFirstStatement));
else {
for (const statement of constructor.body.statements) {
if (isParameterPropertyDeclaration(getOriginalNode(statement), constructor)) {
parameterPropertyDeclarationCount++;
}
}
if (parameterPropertyDeclarationCount > 0) {
const parameterProperties = visitNodes(constructor.body.statements, visitor, isStatement, indexOfFirstStatementAfterSuper, parameterPropertyDeclarationCount);

// If there was a super() call found, add parameter properties immediately after it
if (foundSuperStatement) {
addRange(statements, parameterProperties);
}
// If a synthetic super() call was added, add them just after it
else if (needsSyntheticConstructor) {
statements = addRange(
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
[statements[0]],
[
...parameterProperties,
...statements.slice(1),
]
);
}
// Since there wasn't a super() call, add them to the top of the constructor
else {
statements = addRange(
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
[],
[
...parameterProperties,
...statements,
]
);
}

indexOfFirstStatementAfterSuper += parameterPropertyDeclarationCount;
}
indexOfFirstStatement = afterParameterProperties;
}
}
addPropertyStatements(statements, properties, createThis());

// Add existing statements, skipping the initial super call.
addPropertyStatements(statements, properties, createThis(), getPropertyStatementInsertionIndex(needsSyntheticConstructor, foundSuperStatement, useDefineForClassFields ? 0 : parameterPropertyDeclarationCount));

// Add existing statements after the initial super call
if (constructor) {
addRange(statements, visitNodes(constructor.body!.statements, visitor, isStatement, indexOfFirstStatement));
addRange(statements, visitNodes(constructor.body!.statements, visitBodyStatement, isStatement, indexOfFirstStatementAfterSuper));
}

statements = mergeLexicalEnvironment(statements, endLexicalEnvironment());
Expand All @@ -694,6 +744,33 @@ namespace ts {
),
/*location*/ constructor ? constructor.body : undefined
);

function visitBodyStatement(statement: Node) {
if (useDefineForClassFields && isParameterPropertyDeclaration(getOriginalNode(statement), constructor!)) {
return undefined;
}

return visitor(statement);
}
}

/**
* Finds the statement to insert class properties into a class constructor.
*
* @param needsSyntheticConstructor Whether the constructor was synthesized.
* @param foundSuperStatement Whether a super() statement was found in the (non-synthesized) constructor.
* @param parameterPropertyDeclarationCount How many parameter properties were created in the (non-synthesized) constructor.
*/
function getPropertyStatementInsertionIndex(needsSyntheticConstructor: boolean, foundSuperStatement: boolean, parameterPropertyDeclarationCount: number) {
if (needsSyntheticConstructor) {
return 1;
}

if (!foundSuperStatement) {
return parameterPropertyDeclarationCount;
}

return undefined;
}

/**
Expand All @@ -702,8 +779,9 @@ namespace ts {
* @param properties An array of property declarations to transform.
* @param receiver The receiver on which each property should be assigned.
*/
function addPropertyStatements(statements: Statement[], properties: readonly PropertyDeclaration[], receiver: LeftHandSideExpression) {
for (const property of properties) {
function addPropertyStatements(statements: Statement[], properties: readonly PropertyDeclaration[], receiver: LeftHandSideExpression, insertionIndex?: number) {
for (let i = 0; i < properties.length; i += 1) {
const property = properties[i];
const expression = transformProperty(property, receiver);
if (!expression) {
continue;
Expand All @@ -712,7 +790,13 @@ namespace ts {
setSourceMapRange(statement, moveRangePastModifiers(property));
setCommentRange(statement, property);
setOriginalNode(statement, property);
statements.push(statement);

if (insertionIndex !== undefined) {
statements.splice(i + insertionIndex, 0, statement);
}
else {
statements.push(statement);
}
}
}

Expand Down
Loading