From 77a4b0ae24ea1b18ab7d36b5b3fa7e4c2b7467c3 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 30 Mar 2020 21:29:40 +0200 Subject: [PATCH 1/4] Fixed incorrect escaping of fields --- src/transformation/visitors/call.ts | 3 +- .../visitors/class/members/constructor.ts | 22 +++++++------ test/unit/identifiers.spec.ts | 31 +++++++++++++++++++ 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index bd8099afa..24c4864d1 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -10,7 +10,6 @@ import { LuaLibFeature, transformLuaLibFunction } from "../utils/lualib"; import { isValidLuaIdentifier } from "../utils/safe-names"; import { isArrayType, isExpressionWithEvaluationEffect, isInDestructingAssignment } from "../utils/typescript"; import { transformElementAccessArgument } from "./access"; -import { transformIdentifier } from "./identifier"; import { transformLuaTableCallExpression } from "./lua-table"; export type PropertyCallExpression = ts.CallExpression & { expression: ts.PropertyAccessExpression }; @@ -117,7 +116,7 @@ export function transformContextualCallExpression( return lua.createMethodCallExpression( table, - transformIdentifier(context, left.name), + lua.createIdentifier(left.name.text, left.name), transformedArguments, node ); diff --git a/src/transformation/visitors/class/members/constructor.ts b/src/transformation/visitors/class/members/constructor.ts index d0c611465..642a49a7c 100644 --- a/src/transformation/visitors/class/members/constructor.ts +++ b/src/transformation/visitors/class/members/constructor.ts @@ -1,11 +1,11 @@ import * as ts from "typescript"; import * as lua from "../../../../LuaAST"; -import { createSelfIdentifier } from "../../../utils/lua-ast"; -import { transformParameters, transformFunctionBodyStatements, transformFunctionBodyHeader } from "../../function"; import { TransformationContext } from "../../../context"; +import { createSelfIdentifier } from "../../../utils/lua-ast"; +import { popScope, pushScope, ScopeType } from "../../../utils/scope"; +import { transformFunctionBodyHeader, transformFunctionBodyStatements, transformParameters } from "../../function"; import { transformIdentifier } from "../../identifier"; import { transformClassInstanceFields } from "./fields"; -import { pushScope, ScopeType, popScope } from "../../../utils/scope"; export function createConstructorName(className: lua.Identifier): lua.TableIndexExpression { return lua.createTableIndexExpression( @@ -66,13 +66,15 @@ export function transformConstructorDeclaration( // Add in instance field declarations for (const declaration of constructorFieldsDeclarations) { - const declarationName = transformIdentifier(context, declaration.name as ts.Identifier); - // self.declarationName = declarationName - const assignment = lua.createAssignmentStatement( - lua.createTableIndexExpression(createSelfIdentifier(), lua.createStringLiteral(declarationName.text)), - declarationName - ); - bodyWithFieldInitializers.push(assignment); + // TypeScript error: A parameter property may not be declared using a binding pattern + if (ts.isIdentifier(declaration.name)) { + // self.declarationName = declarationName + const assignment = lua.createAssignmentStatement( + lua.createTableIndexExpression(createSelfIdentifier(), lua.createStringLiteral(declaration.name.text)), + transformIdentifier(context, declaration.name as ts.Identifier) + ); + bodyWithFieldInitializers.push(assignment); + } } bodyWithFieldInitializers.push(...classInstanceFields); diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts index 84583ccd7..9c48235b2 100644 --- a/test/unit/identifiers.spec.ts +++ b/test/unit/identifiers.spec.ts @@ -757,3 +757,34 @@ test("exported variable with lua keyword as name is not renamed", () => { expect(util.transpileExecuteAndReturnExport(code, "print")).toBe("foobar"); }); + +// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/846 +test("lua keyword as class method", () => { + util.testModule` + class MyClass { + error() { return "Error!"; } + } + export const result = new MyClass().error(); + `.expectToMatchJsResult(); +}); + +// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/833 +test("lua keyword as object method", () => { + util.testModule` + const obj = { error: () => "Error!" }; + export const result = obj.error(); + `.expectToMatchJsResult(); +}); + +// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/789 +test("lua keyword as in constructor assignment", () => { + util.testModule` + class A { + constructor(public error: string){} + } + + export const result = new A("42").error; + ` + .debug() + .expectToMatchJsResult(); +}); From 12a8373f23d6ff0c5cb24ad661f10352712d0510 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 30 Mar 2020 21:38:25 +0200 Subject: [PATCH 2/4] Removed forgotten debug() and moved comment around --- src/transformation/visitors/class/members/constructor.ts | 2 +- test/unit/identifiers.spec.ts | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/transformation/visitors/class/members/constructor.ts b/src/transformation/visitors/class/members/constructor.ts index 642a49a7c..d8d1964c3 100644 --- a/src/transformation/visitors/class/members/constructor.ts +++ b/src/transformation/visitors/class/members/constructor.ts @@ -66,7 +66,6 @@ export function transformConstructorDeclaration( // Add in instance field declarations for (const declaration of constructorFieldsDeclarations) { - // TypeScript error: A parameter property may not be declared using a binding pattern if (ts.isIdentifier(declaration.name)) { // self.declarationName = declarationName const assignment = lua.createAssignmentStatement( @@ -75,6 +74,7 @@ export function transformConstructorDeclaration( ); bodyWithFieldInitializers.push(assignment); } + // else { TypeScript error: A parameter property may not be declared using a binding pattern } } bodyWithFieldInitializers.push(...classInstanceFields); diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts index 9c48235b2..400f778ae 100644 --- a/test/unit/identifiers.spec.ts +++ b/test/unit/identifiers.spec.ts @@ -784,7 +784,5 @@ test("lua keyword as in constructor assignment", () => { } export const result = new A("42").error; - ` - .debug() - .expectToMatchJsResult(); + `.expectToMatchJsResult(); }); From d9afa54e0b4f045053eec5e89540b9d01bf5e9f5 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Tue, 31 Mar 2020 23:16:32 +0200 Subject: [PATCH 3/4] Updated test for invalid syntax identifiers --- .../visitors/class/members/constructor.ts | 2 +- test/unit/identifiers.spec.ts | 52 ++++++++++--------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/transformation/visitors/class/members/constructor.ts b/src/transformation/visitors/class/members/constructor.ts index d8d1964c3..fe25a21b2 100644 --- a/src/transformation/visitors/class/members/constructor.ts +++ b/src/transformation/visitors/class/members/constructor.ts @@ -70,7 +70,7 @@ export function transformConstructorDeclaration( // self.declarationName = declarationName const assignment = lua.createAssignmentStatement( lua.createTableIndexExpression(createSelfIdentifier(), lua.createStringLiteral(declaration.name.text)), - transformIdentifier(context, declaration.name as ts.Identifier) + transformIdentifier(context, declaration.name) ); bodyWithFieldInitializers.push(assignment); } diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts index 400f778ae..7773bfe07 100644 --- a/test/unit/identifiers.spec.ts +++ b/test/unit/identifiers.spec.ts @@ -758,31 +758,33 @@ test("exported variable with lua keyword as name is not renamed", () => { expect(util.transpileExecuteAndReturnExport(code, "print")).toBe("foobar"); }); -// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/846 -test("lua keyword as class method", () => { - util.testModule` - class MyClass { - error() { return "Error!"; } - } - export const result = new MyClass().error(); - `.expectToMatchJsResult(); -}); +describe.each(["error", ...invalidLuaCharNames])("risky variable name %s", variableName => { + // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/846 + test("as class method", () => { + util.testModule` + class MyClass { + ${variableName}() { return "Error!"; } + } + export const result = new MyClass().${variableName}(); + `.expectToMatchJsResult(); + }); -// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/833 -test("lua keyword as object method", () => { - util.testModule` - const obj = { error: () => "Error!" }; - export const result = obj.error(); - `.expectToMatchJsResult(); -}); + // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/833 + test("as object method", () => { + util.testModule` + const obj = { ${variableName}: () => "Error!" }; + export const result = obj.${variableName}(); + `.expectToMatchJsResult(); + }); -// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/789 -test("lua keyword as in constructor assignment", () => { - util.testModule` - class A { - constructor(public error: string){} - } - - export const result = new A("42").error; - `.expectToMatchJsResult(); + // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/789 + test("in constructor assignment", () => { + util.testModule` + class A { + constructor(public ${variableName}: string){} + } + + export const result = new A("42").${variableName}; + `.debug().expectToMatchJsResult(); + }); }); From f7ba6adc08e7d2916fdd8195e6a3bf99f1aa3776 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Tue, 31 Mar 2020 23:30:28 +0200 Subject: [PATCH 4/4] Reverted test changes --- test/unit/identifiers.spec.ts | 52 +++++++++++++++++------------------ 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts index 7773bfe07..27046c81b 100644 --- a/test/unit/identifiers.spec.ts +++ b/test/unit/identifiers.spec.ts @@ -758,33 +758,31 @@ test("exported variable with lua keyword as name is not renamed", () => { expect(util.transpileExecuteAndReturnExport(code, "print")).toBe("foobar"); }); -describe.each(["error", ...invalidLuaCharNames])("risky variable name %s", variableName => { - // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/846 - test("as class method", () => { - util.testModule` - class MyClass { - ${variableName}() { return "Error!"; } - } - export const result = new MyClass().${variableName}(); - `.expectToMatchJsResult(); - }); +// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/846 +test("lua built-in as class method", () => { + util.testModule` + class MyClass { + error() { return "Error!"; } + } + export const result = new MyClass().error(); + `.expectToMatchJsResult(); +}); - // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/833 - test("as object method", () => { - util.testModule` - const obj = { ${variableName}: () => "Error!" }; - export const result = obj.${variableName}(); - `.expectToMatchJsResult(); - }); +// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/833 +test("lua built-in as object method", () => { + util.testModule` + const obj = { error: () => "Error!" }; + export const result = obj.error(); + `.expectToMatchJsResult(); +}); - // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/789 - test("in constructor assignment", () => { - util.testModule` - class A { - constructor(public ${variableName}: string){} - } - - export const result = new A("42").${variableName}; - `.debug().expectToMatchJsResult(); - }); +// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/789 +test("lua built-in as in constructor assignment", () => { + util.testModule` + class A { + constructor(public error: string){} + } + + export const result = new A("42").error; + `.expectToMatchJsResult(); });