From 29107c7a3c632746b8a84936ea5d45f666dc979a Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sun, 11 Aug 2019 07:11:29 -0600 Subject: [PATCH 1/4] fixed order of class property initialization fixes #697 --- src/LuaTransformer.ts | 7 +++---- test/unit/classes/classes.spec.ts | 12 ++++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 50bc074d5..3fac0673d 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1237,10 +1237,7 @@ export class LuaTransformer { return undefined; } - const bodyWithFieldInitializers: tstl.Statement[] = this.transformClassInstanceFields( - classDeclaration, - instanceFields - ); + const bodyWithFieldInitializers: tstl.Statement[] = []; // Check for field declarations in constructor const constructorFieldsDeclarations = statement.parameters.filter(p => p.modifiers !== undefined); @@ -1275,6 +1272,8 @@ export class LuaTransformer { } } + bodyWithFieldInitializers.push(...this.transformClassInstanceFields(classDeclaration, instanceFields)); + // function className.constructor(self, params) ... end const [params, dotsLiteral, restParamName] = this.transformParameters( diff --git a/test/unit/classes/classes.spec.ts b/test/unit/classes/classes.spec.ts index 3f156c93a..fbf3a53e2 100644 --- a/test/unit/classes/classes.spec.ts +++ b/test/unit/classes/classes.spec.ts @@ -88,6 +88,18 @@ test("ClassConstructorAssignmentDefault", () => { expect(result).toBe(3); }); +test("ClassConstructorPropertyInitiailizationOrder", () => { + const result = util.transpileAndExecute( + `class Test { + public foo = this.bar; + constructor(public bar: string) {} + } + return (new Test("baz")).foo;` + ); + + expect(result).toBe("baz"); +}); + test("ClassNewNoBrackets", () => { const result = util.transpileAndExecute( `class a { From 2452eb8e1770296d768686386fe27478ff1d8c61 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sun, 11 Aug 2019 07:44:48 -0600 Subject: [PATCH 2/4] fixed constructor field initialization to falsey values --- src/LuaTransformer.ts | 99 ++++++++++++++++--------------- test/unit/classes/classes.spec.ts | 11 ++++ 2 files changed, 62 insertions(+), 48 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 3fac0673d..fef6b5bed 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1237,54 +1237,28 @@ export class LuaTransformer { return undefined; } - const bodyWithFieldInitializers: tstl.Statement[] = []; - - // 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); - } - } - - bodyWithFieldInitializers.push(...this.transformClassInstanceFields(classDeclaration, instanceFields)); - - // 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); + + // 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, hoist the super call to the top - if (bodyWithFieldInitializers.length > 0 && statement.body && statement.body.statements.length > 0) { + if ( + (constructorFieldsDeclarations.length > 0 || classInstanceFields.length > 0) && + statement.body && + statement.body.statements.length > 0 + ) { const firstStatement = statement.body.statements[0]; if ( ts.isExpressionStatement(firstStatement) && @@ -1293,11 +1267,27 @@ export class LuaTransformer { ) { const superCall = body.shift(); if (superCall) { - bodyWithFieldInitializers.unshift(superCall); + bodyWithFieldInitializers.push(superCall); } } } + // 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); @@ -1489,16 +1479,19 @@ export class LuaTransformer { ); } - protected transformFunctionBody( - parameters: ts.NodeArray, - 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, + spreadIdentifier?: tstl.Identifier + ): tstl.Statement[] { + const headerStatements: tstl.Statement[] = []; // Add default parameters and object binding patterns const bindingPatternDeclarations: tstl.Statement[] = []; @@ -1529,7 +1522,7 @@ 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)); } @@ -1537,6 +1530,16 @@ export class LuaTransformer { // Binding pattern statements need to be after spread table is declared headerStatements.push(...bindingPatternDeclarations); + return headerStatements; + } + + protected transformFunctionBody( + parameters: ts.NodeArray, + 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]; } diff --git a/test/unit/classes/classes.spec.ts b/test/unit/classes/classes.spec.ts index fbf3a53e2..199bdf2cc 100644 --- a/test/unit/classes/classes.spec.ts +++ b/test/unit/classes/classes.spec.ts @@ -100,6 +100,17 @@ test("ClassConstructorPropertyInitiailizationOrder", () => { expect(result).toBe("baz"); }); +test("ClassConstructorPropertyInitiailizationFalsey", () => { + const result = util.transpileAndExecute( + `class Test { + constructor(public foo = true) {} + } + return (new Test(false)).foo;` + ); + + expect(result).toBe(false); +}); + test("ClassNewNoBrackets", () => { const result = util.transpileAndExecute( `class a { From 84c73c1b3190e66a1d0f54435310c3483158145d Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sun, 11 Aug 2019 13:27:06 -0600 Subject: [PATCH 3/4] using new test builder --- test/unit/classes/classes.spec.ts | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/test/unit/classes/classes.spec.ts b/test/unit/classes/classes.spec.ts index 199bdf2cc..343ad75ad 100644 --- a/test/unit/classes/classes.spec.ts +++ b/test/unit/classes/classes.spec.ts @@ -89,26 +89,22 @@ test("ClassConstructorAssignmentDefault", () => { }); test("ClassConstructorPropertyInitiailizationOrder", () => { - const result = util.transpileAndExecute( - `class Test { + util.testFunction` + class Test { public foo = this.bar; constructor(public bar: string) {} } - return (new Test("baz")).foo;` - ); - - expect(result).toBe("baz"); + return (new Test("baz")).foo; + `.expectToMatchJsResult(); }); test("ClassConstructorPropertyInitiailizationFalsey", () => { - const result = util.transpileAndExecute( - `class Test { + util.testFunction` + class Test { constructor(public foo = true) {} } - return (new Test(false)).foo;` - ); - - expect(result).toBe(false); + return (new Test(false)).foo; + `.expectToMatchJsResult(); }); test("ClassNewNoBrackets", () => { From e9f85238e43a8d3c80a8073d573e8d47b26e9d25 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sun, 11 Aug 2019 13:27:19 -0600 Subject: [PATCH 4/4] updated comment --- src/LuaTransformer.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index fef6b5bed..d8aee1a01 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1253,7 +1253,8 @@ export class LuaTransformer { const classInstanceFields = this.transformClassInstanceFields(classDeclaration, instanceFields); - // If there are field initializers and the first statement is a super call, hoist the super call to the top + // 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 &&