Skip to content

Simplify and improve enum transform #685

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 5 commits into from
Aug 3, 2019
+110 −163
Merged

Simplify and improve enum transform #685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d1e48d
Simplify and improve enum transform
ark120202 Jul 24, 2019
ad3401d
Merge remote-tracking branch 'upstream/master' into simplify-and-impr…
ark120202 Aug 1, 2019
7546753
Tests
ark120202 Aug 1, 2019
91313ae
Fix enum toString test not working
ark120202 Aug 1, 2019
1758a90
Don't generate reverse mapping for nil member values
ark120202 Aug 2, 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
144 changes: 53 additions & 91 deletions src/LuaTransformer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1727,115 +1727,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<ts.PropertyName, ExpressionVisitResult>();

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();
Copy link
Copy Markdown
Member

@Perryvw Perryvw Aug 1, 2019

Choose a reason for hiding this comment

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

Looking at this again, is this actually correct? In what case would an enum value be nil?

Copy link
Copy Markdown
Contributor Author

@ark120202 ark120202 Aug 2, 2019

Choose a reason for hiding this comment

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

Original transformer has the same branch: https://github.com/microsoft/TypeScript/blob/master/src/compiler/transformers/ts.ts#L2442.
I have thought it'd be used for some syntactically incorrect enums, but couldn't find any cases where TS goes there.

}

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) && !tstl.isNilLiteral(valueExpression)) {
const reverseMemberAccessor = tstl.createTableIndexExpression(enumReference, memberAccessor);
result.push(tstl.createAssignmentStatement(reverseMemberAccessor, memberName, member));
}
}
}

return result;
}

protected transformGeneratorFunction(
Expand Down Expand Up @@ -4707,7 +4669,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") {
Expand Down
20 changes: 0 additions & 20 deletions src/TSHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,26 +871,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;
Expand Down
7 changes: 0 additions & 7 deletions src/TSTLErrors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,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);

Expand Down
90 changes: 45 additions & 45 deletions test/unit/enum.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as TSTLErrors from "../../src/TSTLErrors";
import * as util from "../util";

// TODO: string.toString()
Expand All @@ -10,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;
Expand All @@ -37,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 {
Expand All @@ -61,7 +69,7 @@ describe("initializers", () => {
`.expectToMatchJsResult();
});

test("other member reference", () => {
test("member reference", () => {
util.testFunction`
enum TestEnum {
A,
Expand All @@ -72,38 +80,38 @@ describe("initializers", () => {
return ${serializeEnum("TestEnum")}
`.expectToMatchJsResult();
});
});

test("invalid heterogeneous enum", () => {
util.testFunction`
enum TestEnum {
A,
B = "B",
C,
}
`
.disableSemanticCheck()
.expectToHaveDiagnosticOfError(TSTLErrors.HeterogeneousEnum(util.nodeStub));
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",
Expand Down Expand Up @@ -131,26 +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);
});

test("enum concat", () => {
const code = `
enum TestEnum {
A,
B,
C,
function foo(value: TestEnum) {
return value.toString();
}
let test = TestEnum.A;
return test + "_foobar";`;
expect(util.transpileAndExecute(code)).toBe("0_foobar");

return foo(TestEnum.A);
`.expectToMatchJsResult();
});
12 changes: 12 additions & 0 deletions test/unit/namespaces.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});