Skip to content

class field initialization order #700

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.

Sign up for GitHub

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
Perryvw merged 4 commits into from
Aug 12, 2019
Merged

class field initialization order #700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29107c7
fixed order of class property initialization
tomblind Aug 11, 2019
2452eb8
fixed constructor field initialization to falsey values
tomblind Aug 11, 2019
84c73c1
using new test builder
tomblind Aug 11, 2019
e9f8523
updated comment
tomblind Aug 11, 2019
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
103 changes: 53 additions & 50 deletions src/LuaTransformer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1237,55 +1237,29 @@ export class LuaTransformer {
return undefined;
}

const bodyWithFieldInitializers: tstl.Statement[] = this.transformClassInstanceFields(
classDeclaration,
instanceFields
);

// Check for field declarations in constructor
const constructorFieldsDeclarations = statement.parameters.filter(p => p.modifiers !== undefined);

// Add in instance field declarations
for (const declaration of constructorFieldsDeclarations) {
const declarationName = this.transformIdentifier(declaration.name as ts.Identifier);
if (declaration.initializer) {
// self.declarationName = declarationName or initializer
const assignment = tstl.createAssignmentStatement(
tstl.createTableIndexExpression(
this.createSelfIdentifier(),
tstl.createStringLiteral(declarationName.text)
),
tstl.createBinaryExpression(
declarationName,
this.transformExpression(declaration.initializer),
tstl.SyntaxKind.OrOperator
)
);
bodyWithFieldInitializers.push(assignment);
} else {
// self.declarationName = declarationName
const assignment = tstl.createAssignmentStatement(
tstl.createTableIndexExpression(
this.createSelfIdentifier(),
tstl.createStringLiteral(declarationName.text)
),
declarationName
);
bodyWithFieldInitializers.push(assignment);
}
}

// function className.constructor(self, params) ... end
// Transform body
const [body, scope] = this.transformFunctionBodyStatements(statement.body);

const [params, dotsLiteral, restParamName] = this.transformParameters(
statement.parameters,
this.createSelfIdentifier()
);

const [body] = this.transformFunctionBody(statement.parameters, statement.body, restParamName);
// Make sure default parameters are assigned before fields are initialized
const bodyWithFieldInitializers = this.transformFunctionBodyHeader(scope, statement.parameters, restParamName);

// If there are field initializers and the first statement is a super call, hoist the super call to the top
if (bodyWithFieldInitializers.length > 0 && statement.body && statement.body.statements.length > 0) {
// Check for field declarations in constructor
const constructorFieldsDeclarations = statement.parameters.filter(p => p.modifiers !== undefined);

const classInstanceFields = this.transformClassInstanceFields(classDeclaration, instanceFields);

// If there are field initializers and the first statement is a super call,
// move super call between default assignments and initializers
if (
(constructorFieldsDeclarations.length > 0 || classInstanceFields.length > 0) &&
statement.body &&
statement.body.statements.length > 0
) {
const firstStatement = statement.body.statements[0];
if (
ts.isExpressionStatement(firstStatement) &&
Expand All @@ -1294,11 +1268,27 @@ export class LuaTransformer {
) {
const superCall = body.shift();
if (superCall) {
bodyWithFieldInitializers.unshift(superCall);
bodyWithFieldInitializers.push(superCall);
Copy link
Member

@Perryvw Perryvw Aug 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks strange in combination with the comment on line 1256.

}
}
}

// Add in instance field declarations
for (const declaration of constructorFieldsDeclarations) {
const declarationName = this.transformIdentifier(declaration.name as ts.Identifier);
// self.declarationName = declarationName
const assignment = tstl.createAssignmentStatement(
tstl.createTableIndexExpression(
this.createSelfIdentifier(),
tstl.createStringLiteral(declarationName.text)
),
declarationName
);
bodyWithFieldInitializers.push(assignment);
}

bodyWithFieldInitializers.push(...classInstanceFields);

bodyWithFieldInitializers.push(...body);

const block: tstl.Block = tstl.createBlock(bodyWithFieldInitializers);
Expand Down Expand Up @@ -1490,16 +1480,19 @@ export class LuaTransformer {
);
}

protected transformFunctionBody(
parameters: ts.NodeArray<ts.ParameterDeclaration>,
body: ts.Block,
spreadIdentifier?: tstl.Identifier
): [tstl.Statement[], Scope] {
protected transformFunctionBodyStatements(body: ts.Block): [tstl.Statement[], Scope] {
this.pushScope(ScopeType.Function);
const bodyStatements = this.performHoisting(this.transformStatements(body.statements));
const scope = this.popScope();
return [bodyStatements, scope];
}

const headerStatements = [];
protected transformFunctionBodyHeader(
bodyScope: Scope,
parameters: ts.NodeArray<ts.ParameterDeclaration>,
spreadIdentifier?: tstl.Identifier
): tstl.Statement[] {
const headerStatements: tstl.Statement[] = [];

// Add default parameters and object binding patterns
const bindingPatternDeclarations: tstl.Statement[] = [];
Expand Down Expand Up @@ -1530,14 +1523,24 @@ export class LuaTransformer {
}

// Push spread operator here
if (spreadIdentifier && this.isRestParameterReferenced(spreadIdentifier, scope)) {
if (spreadIdentifier && this.isRestParameterReferenced(spreadIdentifier, bodyScope)) {
const spreadTable = this.wrapInTable(tstl.createDotsLiteral());
headerStatements.push(tstl.createVariableDeclarationStatement(spreadIdentifier, spreadTable));
}

// Binding pattern statements need to be after spread table is declared
headerStatements.push(...bindingPatternDeclarations);

return headerStatements;
}

protected transformFunctionBody(
parameters: ts.NodeArray<ts.ParameterDeclaration>,
body: ts.Block,
spreadIdentifier?: tstl.Identifier
): [tstl.Statement[], Scope] {
const [bodyStatements, scope] = this.transformFunctionBodyStatements(body);
const headerStatements = this.transformFunctionBodyHeader(scope, parameters, spreadIdentifier);
return [headerStatements.concat(bodyStatements), scope];
}

Expand Down
19 changes: 19 additions & 0 deletions test/unit/classes/classes.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,25 @@ test("ClassConstructorAssignmentDefault", () => {
expect(result).toBe(3);
});

test("ClassConstructorPropertyInitiailizationOrder", () => {
util.testFunction`
class Test {
public foo = this.bar;
constructor(public bar: string) {}
}
return (new Test("baz")).foo;
`.expectToMatchJsResult();
});

test("ClassConstructorPropertyInitiailizationFalsey", () => {
util.testFunction`
class Test {
constructor(public foo = true) {}
}
return (new Test(false)).foo;
`.expectToMatchJsResult();
});

test("ClassNewNoBrackets", () => {
const result = util.transpileAndExecute(
`class a {
Expand Down