From 5d1e48dc9ebf930500a058738c45671fee1cfe3a Mon Sep 17 00:00:00 2001 From: ark120202 Date: Wed, 24 Jul 2019 10:30:19 +0500 Subject: [PATCH 1/4] Simplify and improve enum transform --- src/LuaTransformer.ts | 144 +++++++++++++++-------------------------- src/TSHelper.ts | 20 ------ src/TSTLErrors.ts | 7 -- test/unit/enum.spec.ts | 13 ---- 4 files changed, 53 insertions(+), 131 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 4138f2af2..98a8f64ea 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1654,115 +1654,77 @@ export class LuaTransformer { return result; } - public transformEnumDeclaration(enumDeclaration: ts.EnumDeclaration): StatementVisitResult { - const type = this.checker.getTypeAtLocation(enumDeclaration); - - // Const enums should never appear in the resulting code - if (type.symbol.getFlags() & ts.SymbolFlags.ConstEnum) { + public transformEnumDeclaration(node: ts.EnumDeclaration): StatementVisitResult { + if (ts.getCombinedModifierFlags(node) & ts.ModifierFlags.Const && !this.options.preserveConstEnums) { return undefined; } + const type = this.checker.getTypeAtLocation(node); const membersOnly = tsHelper.getCustomDecorators(type, this.checker).has(DecoratorKind.CompileMembersOnly); - const result: tstl.Statement[] = []; if (!membersOnly) { - const name = this.transformIdentifier(enumDeclaration.name); + const name = this.transformIdentifier(node.name); const table = tstl.createTableExpression(); - result.push(...this.createLocalOrExportedOrGlobalDeclaration(name, table, enumDeclaration)); + result.push(...this.createLocalOrExportedOrGlobalDeclaration(name, table, node)); } - for (const enumMember of this.computeEnumMembers(enumDeclaration)) { - const memberName = this.transformPropertyName(enumMember.name); - if (membersOnly) { - const enumSymbol = this.checker.getSymbolAtLocation(enumDeclaration.name); - const exportScope = enumSymbol ? this.getSymbolExportScope(enumSymbol) : undefined; + const enumReference = this.transformExpression(node.name); + for (const member of node.members) { + const memberName = this.transformPropertyName(member.name); - if (tstl.isIdentifier(memberName)) { - result.push( - ...this.createLocalOrExportedOrGlobalDeclaration( - memberName, - enumMember.value, - enumDeclaration, - undefined, - exportScope - ) - ); - } else { - result.push( - ...this.createLocalOrExportedOrGlobalDeclaration( - tstl.createIdentifier(enumMember.name.getText(), enumMember.name), - enumMember.value, - enumDeclaration, - undefined, - exportScope - ) - ); + let valueExpression: tstl.Expression | undefined; + const constEnumValue = this.tryGetConstEnumValue(member); + if (constEnumValue) { + valueExpression = constEnumValue; + } else if (member.initializer) { + if (ts.isIdentifier(member.initializer)) { + const symbol = this.checker.getSymbolAtLocation(member.initializer); + if ( + symbol && + symbol.valueDeclaration && + ts.isEnumMember(symbol.valueDeclaration) && + symbol.valueDeclaration.parent === node + ) { + const otherMemberName = this.transformPropertyName(symbol.valueDeclaration.name); + valueExpression = tstl.createTableIndexExpression(enumReference, otherMemberName); + } } - } else { - const enumTable = this.transformIdentifierExpression(enumDeclaration.name); - const property = tstl.createTableIndexExpression(enumTable, memberName); - result.push(tstl.createAssignmentStatement(property, enumMember.value, enumMember.original)); - const valueIndex = tstl.createTableIndexExpression(enumTable, enumMember.value); - result.push(tstl.createAssignmentStatement(valueIndex, memberName, enumMember.original)); - } - } - - return result; - } - - protected computeEnumMembers( - node: ts.EnumDeclaration - ): Array<{ name: ts.PropertyName; value: tstl.Expression; original: ts.Node }> { - let numericValue = 0; - let hasStringInitializers = false; - - const valueMap = new Map(); - - return node.members.map(member => { - let valueExpression: ExpressionVisitResult; - if (member.initializer) { - if (ts.isNumericLiteral(member.initializer)) { - numericValue = Number(member.initializer.text); - valueExpression = this.transformNumericLiteral(member.initializer); - numericValue++; - } else if (ts.isStringLiteral(member.initializer)) { - hasStringInitializers = true; - valueExpression = this.transformStringLiteral(member.initializer); - } else { - if (ts.isIdentifier(member.initializer)) { - const [isEnumMember, originalName] = tsHelper.isEnumMember(node, member.initializer); - if (isEnumMember === true && originalName !== undefined) { - if (valueMap.has(originalName)) { - valueExpression = valueMap.get(originalName)!; - } else { - throw new Error(`Expected valueMap to contain ${originalName}`); - } - } else { - valueExpression = this.transformExpression(member.initializer); - } - } else { - valueExpression = this.transformExpression(member.initializer); - } + if (!valueExpression) { + valueExpression = this.transformExpression(member.initializer); } - } else if (hasStringInitializers) { - throw TSTLErrors.HeterogeneousEnum(node); } else { - valueExpression = tstl.createNumericLiteral(numericValue); - numericValue++; + valueExpression = tstl.createNilLiteral(); } - valueMap.set(member.name, valueExpression); + if (membersOnly) { + const enumSymbol = this.checker.getSymbolAtLocation(node.name); + const exportScope = enumSymbol ? this.getSymbolExportScope(enumSymbol) : undefined; - const enumMember = { - name: member.name, - original: member, - value: valueExpression, - }; + result.push( + ...this.createLocalOrExportedOrGlobalDeclaration( + tstl.isIdentifier(memberName) + ? memberName + : tstl.createIdentifier(member.name.getText(), member.name), + valueExpression, + node, + undefined, + exportScope + ) + ); + } else { + const memberAccessor = tstl.createTableIndexExpression(enumReference, memberName); + result.push(tstl.createAssignmentStatement(memberAccessor, valueExpression, member)); - return enumMember; - }); + if (!tstl.isStringLiteral(valueExpression)) { + const reverseMemberAccessor = tstl.createTableIndexExpression(enumReference, memberAccessor); + result.push(tstl.createAssignmentStatement(reverseMemberAccessor, memberName, member)); + } + } + } + + return result; } protected transformGeneratorFunction( @@ -4327,7 +4289,7 @@ export class LuaTransformer { } private tryGetConstEnumValue( - node: ts.PropertyAccessExpression | ts.ElementAccessExpression + node: ts.EnumMember | ts.PropertyAccessExpression | ts.ElementAccessExpression ): tstl.Expression | undefined { const value = this.checker.getConstantValue(node); if (typeof value === "string") { diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 85a6a08ef..974953e5f 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -808,26 +808,6 @@ export function isStandardLibraryType(type: ts.Type, name: string | undefined, p return isStandardLibraryDeclaration(declaration, program); } -export function isEnumMember( - enumDeclaration: ts.EnumDeclaration, - value: ts.Expression -): [true, ts.PropertyName] | [false, undefined] { - if (ts.isIdentifier(value)) { - const enumMember = enumDeclaration.members.find(m => ts.isIdentifier(m.name) && m.name.text === value.text); - if (enumMember !== undefined) { - if (enumMember.initializer && ts.isIdentifier(enumMember.initializer)) { - return isEnumMember(enumDeclaration, enumMember.initializer); - } else { - return [true, enumMember.name]; - } - } else { - return [false, undefined]; - } - } else { - return [false, undefined]; - } -} - export function isWithinLiteralAssignmentStatement(node: ts.Node): boolean { if (!node.parent) { return false; diff --git a/src/TSTLErrors.ts b/src/TSTLErrors.ts index cc351f2b7..c00ffb5f5 100644 --- a/src/TSTLErrors.ts +++ b/src/TSTLErrors.ts @@ -34,13 +34,6 @@ export const InvalidInstanceOfLuaTable = (node: ts.Node) => export const ForbiddenLuaTableUseException = (description: string, node: ts.Node) => new TranspileError(`Invalid @luaTable usage: ${description}`, node); -export const HeterogeneousEnum = (node: ts.Node) => - new TranspileError( - `Invalid heterogeneous enum. Enums should either specify no member values, ` + - `or specify values (of the same type) for all members.`, - node - ); - export const InvalidDecoratorArgumentNumber = (name: string, got: number, expected: number, node: ts.Node) => new TranspileError(`${name} expects ${expected} argument(s) but got ${got}.`, node); diff --git a/test/unit/enum.spec.ts b/test/unit/enum.spec.ts index bd40f2619..1fc3f4ddb 100644 --- a/test/unit/enum.spec.ts +++ b/test/unit/enum.spec.ts @@ -1,4 +1,3 @@ -import * as TSTLErrors from "../../src/TSTLErrors"; import * as util from "../util"; test("Declare const enum", () => { @@ -54,18 +53,6 @@ test("Const enum without initializer in some values", () => { expect(util.transpileString(testCode)).toBe(`local valueOne = 4`); }); -test("Invalid heterogeneous enum", () => { - expect(() => { - util.transpileString(` - enum TestEnum { - a, - b = "ok", - c, - } - `); - }).toThrowExactError(TSTLErrors.HeterogeneousEnum(util.nodeStub)); -}); - test("String literal name in enum", () => { const code = ` enum TestEnum { From 754675392f687a6cd6aefbb32e621fce7576dcff Mon Sep 17 00:00:00 2001 From: ark120202 Date: Thu, 1 Aug 2019 11:23:31 +0500 Subject: [PATCH 2/4] Tests --- test/unit/enum.spec.ts | 69 ++++++++++++++++++++---------------- test/unit/namespaces.spec.ts | 12 +++++++ 2 files changed, 51 insertions(+), 30 deletions(-) diff --git a/test/unit/enum.spec.ts b/test/unit/enum.spec.ts index ed4ed5248..7fadf19fa 100644 --- a/test/unit/enum.spec.ts +++ b/test/unit/enum.spec.ts @@ -9,21 +9,18 @@ const serializeEnum = (identifier: string) => `(() => { return mappedTestEnum; })()`; -// TODO: Move to namespace tests? -test("in a namespace", () => { - util.testModule` - namespace Test { - export enum TestEnum { - A, - B, +describe("initializers", () => { + test("string", () => { + util.testFunction` + enum TestEnum { + A = "A", + B = "B", } - } - export const result = ${serializeEnum("Test.TestEnum")} - `.expectToMatchJsResult(); -}); + return ${serializeEnum("TestEnum")} + `.expectToMatchJsResult(); + }); -describe("initializers", () => { test("expression", () => { util.testFunction` const value = 6; @@ -36,6 +33,18 @@ describe("initializers", () => { `.expectToMatchJsResult(); }); + test("expression with side effect", () => { + util.testFunction` + let value = 0; + enum TestEnum { + A = value++, + B = A, + } + + return ${serializeEnum("TestEnum")} + `.expectToMatchJsResult(); + }); + test("inference", () => { util.testFunction` enum TestEnum { @@ -60,7 +69,7 @@ describe("initializers", () => { `.expectToMatchJsResult(); }); - test("other member reference", () => { + test("member reference", () => { util.testFunction` enum TestEnum { A, @@ -71,26 +80,38 @@ describe("initializers", () => { return ${serializeEnum("TestEnum")} `.expectToMatchJsResult(); }); + + test("string literal member reference", () => { + util.testFunction` + enum TestEnum { + ["A"], + "B" = A, + C = B, + } + + return ${serializeEnum("TestEnum")} + `.expectToMatchJsResult(); + }); }); describe("const enum", () => { const expectToBeConst: util.TapCallback = builder => expect(builder.getMainLuaCodeChunk()).not.toContain("TestEnum"); - test.each(["", "declare"])("%s without initializer", () => { - util.testFunction` - const enum TestEnum { + test.each(["", "declare "])("%swithout initializer", modifier => { + util.testModule` + ${modifier} const enum TestEnum { A, B, } - return TestEnum.A; + export const A = TestEnum.A; ` .tap(expectToBeConst) .expectToMatchJsResult(); }); - test("with initializer", () => { + test("with string initializer", () => { util.testFunction` const enum TestEnum { A = "ONE", @@ -129,15 +150,3 @@ test("enum toString", () => { return test.toString();`; expect(util.transpileAndExecute(code)).toBe(0); }); - -test("enum concat", () => { - const code = ` - enum TestEnum { - A, - B, - C, - } - let test = TestEnum.A; - return test + "_foobar";`; - expect(util.transpileAndExecute(code)).toBe("0_foobar"); -}); diff --git a/test/unit/namespaces.spec.ts b/test/unit/namespaces.spec.ts index 21272ae8e..fe17f6da4 100644 --- a/test/unit/namespaces.spec.ts +++ b/test/unit/namespaces.spec.ts @@ -119,3 +119,15 @@ test("`import =` on a namespace", () => { export const result = importedFunc(); `.expectToMatchJsResult(); }); + +test("enum in a namespace", () => { + util.testModule` + namespace Test { + export enum TestEnum { + A, + } + } + + export const result = Test.TestEnum.A; + `.expectToMatchJsResult(); +}); From 91313ae3c6a39a5497ad2abebf6756fece52b13e Mon Sep 17 00:00:00 2001 From: ark120202 Date: Thu, 1 Aug 2019 11:32:12 +0500 Subject: [PATCH 3/4] Fix enum toString test not working --- test/unit/enum.spec.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/unit/enum.spec.ts b/test/unit/enum.spec.ts index 7fadf19fa..8da16355c 100644 --- a/test/unit/enum.spec.ts +++ b/test/unit/enum.spec.ts @@ -139,14 +139,18 @@ describe("const enum", () => { }); }); -test("enum toString", () => { - const code = ` +test("toString", () => { + util.testFunction` enum TestEnum { A, B, C, } - let test = TestEnum.A; - return test.toString();`; - expect(util.transpileAndExecute(code)).toBe(0); + + function foo(value: TestEnum) { + return value.toString(); + } + + return foo(TestEnum.A); + `.expectToMatchJsResult(); }); From 1758a90cbb10b0b21cf37a1a253ded8005fe094b Mon Sep 17 00:00:00 2001 From: ark120202 Date: Fri, 2 Aug 2019 16:25:44 +0500 Subject: [PATCH 4/4] Don't generate reverse mapping for nil member values --- src/LuaTransformer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 32c278189..50bc074d5 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1790,7 +1790,7 @@ export class LuaTransformer { const memberAccessor = tstl.createTableIndexExpression(enumReference, memberName); result.push(tstl.createAssignmentStatement(memberAccessor, valueExpression, member)); - if (!tstl.isStringLiteral(valueExpression)) { + if (!tstl.isStringLiteral(valueExpression) && !tstl.isNilLiteral(valueExpression)) { const reverseMemberAccessor = tstl.createTableIndexExpression(enumReference, memberAccessor); result.push(tstl.createAssignmentStatement(reverseMemberAccessor, memberName, member)); }