TypeScriptToLua · tomblind · May 26, 2019 · May 25, 2019 · May 25, 2019 · May 25, 2019
diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts
@@ -264,11 +264,7 @@ export class LuaTransformer {
                         exportedIdentifier = this.transformIdentifier(specifier.propertyName);
                      } else {
                         const exportedSymbol = this.checker.getExportSpecifierLocalTargetSymbol(specifier);
-                        if (exportedSymbol !== undefined) {
-                            exportedIdentifier = this.createIdentifierFromSymbol(exportedSymbol, specifier.name);
-                        } else {
-                            exportedIdentifier = this.transformIdentifier(specifier.name);
-                        }
+                        exportedIdentifier = this.createShorthandIdentifier(exportedSymbol, specifier.name);
                     }
 
                     return tstl.createAssignmentStatement(
@@ -483,10 +479,13 @@ export class LuaTransformer {
         }
 
         let className: tstl.Identifier;
+        let classNameText: string;
         if (nameOverride !== undefined) {
             className = nameOverride;
+            classNameText = nameOverride.text;
         } else if (statement.name !== undefined) {
             className = this.transformIdentifier(statement.name);
+            classNameText = statement.name.text;
         } else {
             throw TSTLErrors.MissingClassName(statement);
         }
@@ -593,6 +592,7 @@ export class LuaTransformer {
                 statement,
                 className,
                 localClassName,
+                classNameText,
                 extendsType
             );
             result.push(...classCreationMethods);
@@ -727,6 +727,7 @@ export class LuaTransformer {
         statement: ts.ClassLikeDeclarationBase,
         className: tstl.Identifier,
         localClassName: tstl.Identifier,
+        classNameText: string,
         extendsType?: ts.Type
     ): tstl.Statement[]
     {
@@ -752,7 +753,7 @@ export class LuaTransformer {
         result.push(
             tstl.createAssignmentStatement(
                 tstl.createTableIndexExpression(tstl.cloneIdentifier(localClassName), tstl.createStringLiteral("name")),
-                tstl.createStringLiteral(localClassName.text),
+                tstl.createStringLiteral(classNameText),
                 statement
             )
         );
@@ -3440,15 +3441,8 @@ export class LuaTransformer {
                 properties.push(tstl.createTableFieldExpression(expression, name, element));
 
             } else if (ts.isShorthandPropertyAssignment(element)) {
-                let identifier: tstl.Expression | undefined;
                 const valueSymbol = this.checker.getShorthandAssignmentValueSymbol(element);
-                if (valueSymbol !== undefined
-                    // Ignore declarations for things like NaN
-                    && !tsHelper.isStandardLibraryDeclaration(valueSymbol.valueDeclaration, this.program)) {
-                    identifier = this.createIdentifierFromSymbol(valueSymbol, element.name);
-                } else {
-                    identifier = this.transformIdentifierExpression(element.name);
-                }
+                let identifier = this.createShorthandIdentifier(valueSymbol, element.name);
                 if (tstl.isIdentifier(identifier) && valueSymbol !== undefined && this.isSymbolExported(valueSymbol)) {
                     identifier = this.createExportedIdentifier(identifier);
                 }
@@ -5265,34 +5259,62 @@ export class LuaTransformer {
         return tstl.createBinaryExpression(expression, tstl.createNumericLiteral(1), tstl.SyntaxKind.AdditionOperator);
     }
 
-    protected createIdentifierFromSymbol(symbol: ts.Symbol, tsOriginal?: ts.Node): tstl.Identifier {
-        const name = this.hasUnsafeSymbolName(symbol)
-            ? this.createSafeName(symbol.name)
-            : symbol.name;
-        return tstl.createIdentifier(name, tsOriginal, this.symbolIds.get(symbol));
+    protected createShorthandIdentifier(
+        valueSymbol: ts.Symbol | undefined,
+        propertyIdentifier: ts.Identifier
+    ): tstl.Expression
+    {
+        let name: string;
+        if (valueSymbol !== undefined) {
+            name = this.hasUnsafeSymbolName(valueSymbol, propertyIdentifier)
+                ? this.createSafeName(valueSymbol.name)
+                : valueSymbol.name;
+
+        } else {
+            const propertyName = this.getIdentifierText(propertyIdentifier);
+            if (luaKeywords.has(propertyName) || !tsHelper.isValidLuaIdentifier(propertyName)) {
+                // Catch ambient declarations of identifiers with bad names
+                throw TSTLErrors.InvalidAmbientIdentifierName(propertyIdentifier);
+            }
+            name = this.hasUnsafeIdentifierName(propertyIdentifier)
+                ? this.createSafeName(propertyName)
+                : propertyName;
+        }
+
+        const identifier = this.transformIdentifierExpression(ts.createIdentifier(name));
+        tstl.setNodeOriginal(identifier, propertyIdentifier);
+        if (valueSymbol !== undefined && tstl.isIdentifier(identifier)) {
+            identifier.symbolId = this.symbolIds.get(valueSymbol);
+        }
+        return identifier;
     }
 
     protected isUnsafeName(name: string): boolean {
         return luaKeywords.has(name) || luaBuiltins.has(name) || !tsHelper.isValidLuaIdentifier(name);
     }
 
-    protected hasUnsafeSymbolName(symbol: ts.Symbol): boolean {
-        if (luaKeywords.has(symbol.name) || luaBuiltins.has(symbol.name)) {
-            // lua keywords are only unsafe when non-ambient and not exported
-            const isNonAmbient = symbol.declarations.find(d => !tsHelper.isAmbient(d)) !== undefined;
-            return isNonAmbient && !this.isSymbolExported(symbol);
+    protected hasUnsafeSymbolName(symbol: ts.Symbol, tsOriginal?: ts.Identifier): boolean {
+        const isLuaKeyword = luaKeywords.has(symbol.name);
+        const isInvalidIdentifier = !tsHelper.isValidLuaIdentifier(symbol.name);
+        const isAmbient = symbol.declarations.some(d => tsHelper.isAmbient(d));
+        if ((isLuaKeyword || isInvalidIdentifier) && isAmbient) {
+            // Catch ambient declarations of identifiers with bad names
+            throw TSTLErrors.InvalidAmbientIdentifierName(tsOriginal || ts.createIdentifier(symbol.name));
+        }
+
+        if (this.isUnsafeName(symbol.name)) {
+            // only unsafe when non-ambient and not exported
+            return !isAmbient && !this.isSymbolExported(symbol);
         }
-        return this.isUnsafeName(symbol.name);
+        return false;
     }
 
     protected hasUnsafeIdentifierName(identifier: ts.Identifier): boolean {
         const symbol = this.checker.getSymbolAtLocation(identifier);
         if (symbol !== undefined) {
-            if (luaKeywords.has(symbol.name) && symbol.declarations.find(d => !tsHelper.isAmbient(d)) === undefined) {
-                // Catch ambient declarations of identifiers with lua keyword names
-                throw TSTLErrors.InvalidAmbientLuaKeywordIdentifier(identifier);
-            }
-            return this.hasUnsafeSymbolName(symbol);
+            return this.hasUnsafeSymbolName(symbol, identifier);
+        } else if (luaKeywords.has(identifier.text) || !tsHelper.isValidLuaIdentifier(identifier.text)) {
+            throw TSTLErrors.InvalidAmbientIdentifierName(identifier);
         }
         return false;
     }
@@ -5556,7 +5578,7 @@ export class LuaTransformer {
         declaration: ts.ClassLikeDeclaration
     ): tstl.AssignmentStatement | undefined {
         const className = declaration.name !== undefined
-            ? this.transformIdentifier(declaration.name)
+            ? this.addExportToIdentifier(this.transformIdentifier(declaration.name))
             : tstl.createAnonymousIdentifier();
 
         const decorators = declaration.decorators;
@@ -5585,3 +5607,4 @@ export class LuaTransformer {
         );
     }
 }
+
diff --git a/src/TSTLErrors.ts b/src/TSTLErrors.ts
@@ -195,9 +195,9 @@ export class TSTLErrors {
         return new TranspileError(`Unsupported object destructuring in for...of statement.`, node);
     };
 
-    public static InvalidAmbientLuaKeywordIdentifier = (node: ts.Identifier) => {
+    public static InvalidAmbientIdentifierName = (node: ts.Identifier) => {
         return new TranspileError(
-            `Invalid use of lua keyword "${node.text}" as ambient identifier name.`,
+            `Invalid ambient identifier name "${node.text}". Ambient identifiers must be valid lua identifiers.`,
             node
         );
     };

diff --git a/test/unit/classDecorator.spec.ts b/test/unit/classDecorator.spec.ts
@@ -188,3 +188,16 @@ test("Throws error if decorator function has void context", () => {
         TSTLErrors.InvalidDecoratorContext(util.nodeStub),
     );
 });
+
+test("Exported class decorator", () => {
+    const code = `
+        function decorator<T extends any>(c: T): T {
+            c.bar = "foobar";
+            return c;
+        }
+
+        @decorator
+        export class Foo {}`;
+
+    expect(util.transpileExecuteAndReturnExport(code, "Foo.bar")).toBe("foobar");
+});
diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts
@@ -3,7 +3,24 @@ import * as util from "../util";
 import { luaKeywords } from "../../src/LuaKeywords";
 import { TSTLErrors } from "../../src/TSTLErrors";
 
-test.each(["$$$", "ɥɣɎɌͼƛಠ", "_̀ः٠‿"])("invalid lua identifier name (%p)", name => {
+const invalidLuaCharNames = ["$$$", "ɥɣɎɌͼƛಠ", "_̀ः٠‿"];
+const validTsInvalidLuaKeywordNames = [
+    "and",
+    "elseif",
+    "end",
+    "goto",
+    "local",
+    "nil",
+    "not",
+    "or",
+    "repeat",
+    "then",
+    "until",
+];
+const invalidLuaNames = [...invalidLuaCharNames, ...luaKeywords.values()];
+const validTsInvalidLuaNames = [...invalidLuaCharNames, ...validTsInvalidLuaKeywordNames];
+
+test.each(validTsInvalidLuaNames)("invalid lua identifier name (%p)", name => {
     const code = `
         const ${name} = "foobar";
         return ${name};`;
@@ -19,42 +36,33 @@ test.each([...luaKeywords.values()])("lua keyword as property name (%p)", keywor
     expect(util.transpileAndExecute(code)).toBe("foobar");
 });
 
-test.each(["and", "elseif", "end", "goto", "local", "nil", "not", "or", "repeat", "then", "until"])(
-    "destructuring lua keyword (%p)",
-    keyword => {
-        const code = `
+test.each(validTsInvalidLuaKeywordNames)("destructuring lua keyword (%p)", keyword => {
+    const code = `
             const { foo: ${keyword} } = { foo: "foobar" };
             return ${keyword};`;
 
-        expect(util.transpileAndExecute(code)).toBe("foobar");
-    },
-);
+    expect(util.transpileAndExecute(code)).toBe("foobar");
+});
 
-test.each(["and", "elseif", "end", "goto", "local", "nil", "not", "or", "repeat", "then", "until"])(
-    "destructuring shorthand lua keyword (%p)",
-    keyword => {
-        const code = `
+test.each(validTsInvalidLuaKeywordNames)("destructuring shorthand lua keyword (%p)", keyword => {
+    const code = `
             const { ${keyword} } = { ${keyword}: "foobar" };
             return ${keyword};`;
 
-        expect(util.transpileAndExecute(code)).toBe("foobar");
-    },
-);
+    expect(util.transpileAndExecute(code)).toBe("foobar");
+});
 
-test.each(["$$$", "ɥɣɎɌͼƛಠ", "_̀ः٠‿", ...luaKeywords.values()])(
-    "lua keyword or invalid identifier as method call (%p)",
-    name => {
-        const code = `
+test.each(invalidLuaNames)("lua keyword or invalid identifier as method call (%p)", name => {
+    const code = `
         const foo = {
             ${name}(arg: string) { return "foo" + arg; }
         };
         return foo.${name}("bar");`;
 
-        expect(util.transpileAndExecute(code)).toBe("foobar");
-    },
-);
+    expect(util.transpileAndExecute(code)).toBe("foobar");
+});
 
-test.each(["$$$", "ɥɣɎɌͼƛಠ", "_̀ः٠‿", ...luaKeywords.values()])(
+test.each(invalidLuaNames)(
     "lua keyword or invalid identifier as complex method call (%p)",
     name => {
         const code = `
@@ -84,10 +92,107 @@ test.each([
         const foo = local;`;
 
     expect(() => util.transpileString(code)).toThrow(
-        TSTLErrors.InvalidAmbientLuaKeywordIdentifier(ts.createIdentifier("local")).message,
+        TSTLErrors.InvalidAmbientIdentifierName(ts.createIdentifier("local")).message,
     );
 });
 
+test.each([
+    "var $$$: any;",
+    "let $$$: any;",
+    "const $$$: any;",
+    "const foo: any, bar: any, $$$: any;",
+    "class $$$ {}",
+    "namespace $$$ { export const bar: any; }",
+    "module $$$ { export const bar: any; }",
+    "enum $$$ {}",
+    "function $$$() {}",
+])("ambient identifier must be a valid lua identifier (%p)", statement => {
+    const code = `
+        declare ${statement}
+        const foo = $$$;`;
+
+    expect(() => util.transpileString(code)).toThrow(
+        TSTLErrors.InvalidAmbientIdentifierName(ts.createIdentifier("$$$")).message,
+    );
+});
+
+test.each(validTsInvalidLuaNames)(
+    "ambient identifier must be a valid lua identifier (object literal shorthand) (%p)",
+    name => {
+        const code = `
+        declare var ${name}: any;
+        const foo = { ${name} };`;
+
+        expect(() => util.transpileString(code)).toThrow(
+            TSTLErrors.InvalidAmbientIdentifierName(ts.createIdentifier(name)).message,
+        );
+    },
+);
+
+test.each(validTsInvalidLuaNames)(
+    "undeclared identifier must be a valid lua identifier (%p)",
+    name => {
+        expect(() => util.transpileString(`const foo = ${name};`)).toThrow(
+            TSTLErrors.InvalidAmbientIdentifierName(ts.createIdentifier(name)).message,
+        );
+    },
+);
+
+test.each(validTsInvalidLuaNames)(
+    "undeclared identifier must be a valid lua identifier (object literal shorthand) (%p)",
+    name => {
+        expect(() => util.transpileString(`const foo = { ${name} };`)).toThrow(
+            TSTLErrors.InvalidAmbientIdentifierName(ts.createIdentifier(name)).message,
+        );
+    },
+);
+
+test.each(validTsInvalidLuaNames)(
+    "exported values with invalid lua identifier names (%p)",
+    name => {
+        const code = `export const ${name} = "foobar";`;
+        const lua = util.transpileString(code);
+        expect(lua.indexOf(`"${name}"`)).toBeGreaterThanOrEqual(0);
+        expect(util.executeLua(`return (function() ${lua} end)()["${name}"]`)).toBe("foobar");
+    },
+);
+
+test.each(validTsInvalidLuaNames)("class with invalid lua name has correct name property", name => {
+    const code = `
+        class ${name} {}
+        return ${name}.name;`;
+
+    expect(util.transpileAndExecute(code)).toBe(name);
+});
+
+test.each(validTsInvalidLuaNames)("decorated class with invalid lua name", name => {
+    const code = `
+        function decorator<T extends any>(c: T): T {
+            c.bar = "foobar";
+            return c;
+        }
+
+        @decorator
+        class ${name} {}
+        return (${name} as any).bar;`;
+
+    expect(util.transpileAndExecute(code)).toBe("foobar");
+});
+
+test.each(validTsInvalidLuaNames)("exported decorated class with invalid lua name", name => {
+    const code = `
+        function decorator<T extends any>(c: T): T {
+            c.bar = "foobar";
+            return c;
+        }
+
+        @decorator
+        export class ${name} {}`;
+
+    const lua = util.transpileString(code);
+    expect(util.executeLua(`return (function() ${lua} end)()["${name}"].bar`)).toBe("foobar");
+});
+
 describe("lua keyword as identifier doesn't interfere with lua's value", () => {
     test("variable (nil)", () => {
         const code = `