Skip to content

Commit 3d7833e

Browse files
tomblindtomblind
authored
fixed usage of lua keywords as property names (#586)
* fixed usage of lua keywords as property names * fixed method calls that use lua keywords or invalid lua names * using transformElementCall for property calls with unsafe names * addressing feedback - not using element call for methods with lua builtin names - fixed shorthand assignment of declared lua builtins - banned lua keywords as identifier names in declare statements * Catching use of lua keywords as ambient identifiers * fixed outdated comment * removed lua keyword identifier check from declare statements
1 parent 3d63277 commit 3d7833e

6 files changed

Lines changed: 146 additions & 23 deletions

File tree

‎src/LuaKeywords.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export const luaKeywords: Set<string> = new Set([
2+
"and", "break", "do", "else", "elseif", "end", "false", "for", "function", "goto", "if", "in", "local", "nil",
3+
"not", "or", "repeat", "return", "then", "until", "while",
4+
]);
5+
6+
export const luaBuiltins: Set<string> = new Set([
7+
"_G", "assert", "coroutine", "debug", "error", "ipairs", "math", "pairs", "pcall", "print", "rawget", "rawset",
8+
"repeat", "require", "self", "string", "table", "tostring", "type", "unpack",
9+
]);

‎src/LuaPrinter.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as tstl from "./LuaAST";
66
import { CompilerOptions, LuaLibImportKind } from "./CompilerOptions";
77
import { LuaLib, LuaLibFeature } from "./LuaLib";
88
import { TSHelper as tsHelper } from "./TSHelper";
9+
import { luaKeywords } from "./LuaKeywords";
910

1011
type SourceChunk = string | SourceNode;
1112

@@ -559,7 +560,10 @@ export class LuaPrinter {
559560
const value = this.printExpression(expression.value);
560561

561562
if (expression.key) {
562-
if (tstl.isStringLiteral(expression.key) && tsHelper.isValidLuaIdentifier(expression.key.value)) {
563+
if (tstl.isStringLiteral(expression.key)
564+
&& tsHelper.isValidLuaIdentifier(expression.key.value)
565+
&& !luaKeywords.has(expression.key.value))
566+
{
563567
chunks.push(expression.key.value, " = ", value);
564568
} else {
565569
chunks.push("[", this.printExpression(expression.key), "] = ", value);
@@ -653,7 +657,10 @@ export class LuaPrinter {
653657
const chunks: SourceChunk[] = [];
654658

655659
chunks.push(this.printExpression(expression.table));
656-
if (tstl.isStringLiteral(expression.index) && tsHelper.isValidLuaIdentifier(expression.index.value)) {
660+
if (tstl.isStringLiteral(expression.index)
661+
&& tsHelper.isValidLuaIdentifier(expression.index.value)
662+
&& !luaKeywords.has(expression.index.value))
663+
{
657664
chunks.push(".", this.createSourceNode(expression.index, expression.index.value));
658665
} else {
659666
chunks.push("[", this.printExpression(expression.index), "]");

‎src/LuaTransformer.ts‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as tstl from "./LuaAST";
66
import { LuaLibFeature } from "./LuaLib";
77
import { ContextType, TSHelper as tsHelper } from "./TSHelper";
88
import { TSTLErrors } from "./TSTLErrors";
9+
import { luaKeywords, luaBuiltins } from "./LuaKeywords";
910

1011
export type StatementVisitResult = tstl.Statement | tstl.Statement[] | undefined;
1112
export type ExpressionVisitResult = tstl.Expression;
@@ -49,13 +50,6 @@ export interface DiagnosticsProducingTypeChecker extends ts.TypeChecker {
4950
}
5051

5152
export class LuaTransformer {
52-
public luaKeywords: Set<string> = new Set([
53-
"_G", "and", "assert", "break", "coroutine", "debug", "do", "else", "elseif", "end", "error", "false", "for",
54-
"function", "goto", "if", "ipairs", "in", "local", "math", "nil", "not", "or", "pairs", "pcall", "print",
55-
"rawget", "rawset", "repeat", "return", "require", "self", "string", "table", "then", "tostring", "type",
56-
"unpack", "until", "while",
57-
]);
58-
5953
private isStrict: boolean;
6054
private luaTarget: LuaTarget;
6155

@@ -3449,8 +3443,9 @@ export class LuaTransformer {
34493443
} else if (ts.isShorthandPropertyAssignment(element)) {
34503444
let identifier: tstl.Expression | undefined;
34513445
const valueSymbol = this.checker.getShorthandAssignmentValueSymbol(element);
3452-
if (valueSymbol !== undefined && this.symbolIds.has(valueSymbol)) {
3453-
// Ignore unknown symbols so things like NaN still get transformed properly
3446+
if (valueSymbol !== undefined
3447+
// Ignore declarations for things like NaN
3448+
&& !tsHelper.isStandardLibraryDeclaration(valueSymbol.valueDeclaration, this.program)) {
34543449
identifier = this.createIdentifierFromSymbol(valueSymbol, element.name);
34553450
} else {
34563451
identifier = this.transformIdentifierExpression(element.name);
@@ -3818,13 +3813,20 @@ export class LuaTransformer {
38183813
if (!signatureDeclaration
38193814
|| tsHelper.getDeclarationContextType(signatureDeclaration, this.checker) !== ContextType.Void)
38203815
{
3821-
// table:name()
3822-
return tstl.createMethodCallExpression(
3823-
table,
3824-
this.transformIdentifier(node.expression.name),
3825-
parameters,
3826-
node
3827-
);
3816+
if (luaKeywords.has(node.expression.name.text)
3817+
|| !tsHelper.isValidLuaIdentifier(node.expression.name.text))
3818+
{
3819+
return this.transformElementCall(node);
3820+
3821+
} else {
3822+
// table:name()
3823+
return tstl.createMethodCallExpression(
3824+
table,
3825+
this.transformIdentifier(node.expression.name),
3826+
parameters,
3827+
node
3828+
);
3829+
}
38283830
} else {
38293831
// table.name()
38303832
const callPath = tstl.createTableIndexExpression(
@@ -3839,7 +3841,7 @@ export class LuaTransformer {
38393841
}
38403842

38413843
public transformElementCall(node: ts.CallExpression): ExpressionVisitResult {
3842-
if (!ts.isElementAccessExpression(node.expression)) {
3844+
if (!ts.isElementAccessExpression(node.expression) && !ts.isPropertyAccessExpression(node.expression)) {
38433845
throw TSTLErrors.InvalidElementCall(node);
38443846
}
38453847

@@ -3862,7 +3864,10 @@ export class LuaTransformer {
38623864

38633865
// Cache left-side if it has effects
38643866
//(function() local ____TS_self = context; return ____TS_self[argument](parameters); end)()
3865-
const argument = this.transformExpression(node.expression.argumentExpression);
3867+
const argumentExpression = ts.isElementAccessExpression(node.expression)
3868+
? node.expression.argumentExpression
3869+
: ts.createStringLiteral(node.expression.name.text);
3870+
const argument = this.transformExpression(argumentExpression);
38663871
const selfIdentifier = tstl.createIdentifier("____TS_self");
38673872
const selfAssignment = tstl.createVariableDeclarationStatement(selfIdentifier, context);
38683873
const index = tstl.createTableIndexExpression(selfIdentifier, argument);
@@ -5269,11 +5274,11 @@ export class LuaTransformer {
52695274
}
52705275

52715276
protected isUnsafeName(name: string): boolean {
5272-
return this.luaKeywords.has(name) || !tsHelper.isValidLuaIdentifier(name);
5277+
return luaKeywords.has(name) || luaBuiltins.has(name) || !tsHelper.isValidLuaIdentifier(name);
52735278
}
52745279

52755280
protected hasUnsafeSymbolName(symbol: ts.Symbol): boolean {
5276-
if (this.luaKeywords.has(symbol.name)) {
5281+
if (luaKeywords.has(symbol.name) || luaBuiltins.has(symbol.name)) {
52775282
// lua keywords are only unsafe when non-ambient and not exported
52785283
const isNonAmbient = symbol.declarations.find(d => !tsHelper.isAmbient(d)) !== undefined;
52795284
return isNonAmbient && !this.isSymbolExported(symbol);
@@ -5284,6 +5289,10 @@ export class LuaTransformer {
52845289
protected hasUnsafeIdentifierName(identifier: ts.Identifier): boolean {
52855290
const symbol = this.checker.getSymbolAtLocation(identifier);
52865291
if (symbol !== undefined) {
5292+
if (luaKeywords.has(symbol.name) && symbol.declarations.find(d => !tsHelper.isAmbient(d)) === undefined) {
5293+
// Catch ambient declarations of identifiers with lua keyword names
5294+
throw TSTLErrors.InvalidAmbientLuaKeywordIdentifier(identifier);
5295+
}
52875296
return this.hasUnsafeSymbolName(symbol);
52885297
}
52895298
return false;

‎src/TSTLErrors.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,5 +189,12 @@ export class TSTLErrors {
189189
"must be moved before the identifier's use, or hoisting must be enabled.",
190190
node
191191
);
192-
}
192+
};
193+
194+
public static InvalidAmbientLuaKeywordIdentifier = (node: ts.Identifier) => {
195+
return new TranspileError(
196+
`Invalid use of lua keyword "${node.text}" as ambient identifier name.`,
197+
node
198+
);
199+
};
193200
}

‎test/unit/identifiers.spec.ts‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
import * as ts from "typescript";
12
import * as util from "../util";
3+
import { luaKeywords } from "../../src/LuaKeywords";
4+
import { TSTLErrors } from "../../src/TSTLErrors";
25

36
test.each(["$$$", "ɥɣɎɌͼƛಠ", "_̀ः٠‿"])("invalid lua identifier name (%p)", name => {
47
const code = `
@@ -8,6 +11,83 @@ test.each(["$$$", "ɥɣɎɌͼƛಠ", "_̀ः٠‿"])("invalid lua identifier nam
811
expect(util.transpileAndExecute(code)).toBe("foobar");
912
});
1013

14+
test.each([...luaKeywords.values()])("lua keyword as property name (%p)", keyword => {
15+
const code = `
16+
const x = { ${keyword}: "foobar" };
17+
return x.${keyword};`;
18+
19+
expect(util.transpileAndExecute(code)).toBe("foobar");
20+
});
21+
22+
test.each(["and", "elseif", "end", "goto", "local", "nil", "not", "or", "repeat", "then", "until"])(
23+
"destructuring lua keyword (%p)",
24+
keyword => {
25+
const code = `
26+
const { foo: ${keyword} } = { foo: "foobar" };
27+
return ${keyword};`;
28+
29+
expect(util.transpileAndExecute(code)).toBe("foobar");
30+
},
31+
);
32+
33+
test.each(["and", "elseif", "end", "goto", "local", "nil", "not", "or", "repeat", "then", "until"])(
34+
"destructuring shorthand lua keyword (%p)",
35+
keyword => {
36+
const code = `
37+
const { ${keyword} } = { ${keyword}: "foobar" };
38+
return ${keyword};`;
39+
40+
expect(util.transpileAndExecute(code)).toBe("foobar");
41+
},
42+
);
43+
44+
test.each(["$$$", "ɥɣɎɌͼƛಠ", "_̀ः٠‿", ...luaKeywords.values()])(
45+
"lua keyword or invalid identifier as method call (%p)",
46+
name => {
47+
const code = `
48+
const foo = {
49+
${name}(arg: string) { return "foo" + arg; }
50+
};
51+
return foo.${name}("bar");`;
52+
53+
expect(util.transpileAndExecute(code)).toBe("foobar");
54+
},
55+
);
56+
57+
test.each(["$$$", "ɥɣɎɌͼƛಠ", "_̀ः٠‿", ...luaKeywords.values()])(
58+
"lua keyword or invalid identifier as complex method call (%p)",
59+
name => {
60+
const code = `
61+
const foo = {
62+
${name}(arg: string) { return "foo" + arg; }
63+
};
64+
function getFoo() { return foo; }
65+
return getFoo().${name}("bar");`;
66+
67+
expect(util.transpileAndExecute(code)).toBe("foobar");
68+
},
69+
);
70+
71+
test.each([
72+
"var local: any;",
73+
"let local: any;",
74+
"const local: any;",
75+
"const foo: any, bar: any, local: any;",
76+
"class local {}",
77+
"namespace local { export const bar: any; }",
78+
"module local { export const bar: any; }",
79+
"enum local {}",
80+
"function local() {}",
81+
])("ambient identifier cannot be a lua keyword (%p)", statement => {
82+
const code = `
83+
declare ${statement}
84+
const foo = local;`;
85+
86+
expect(() => util.transpileString(code)).toThrow(
87+
TSTLErrors.InvalidAmbientLuaKeywordIdentifier(ts.createIdentifier("local")).message,
88+
);
89+
});
90+
1191
describe("lua keyword as identifier doesn't interfere with lua's value", () => {
1292
test("variable (nil)", () => {
1393
const code = `

‎test/unit/objectLiteral.spec.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,17 @@ describe("property shorthand", () => {
2929
expect(result).toBe(identifier);
3030
});
3131

32+
test("should support _G shorthand", () => {
33+
const result = util.transpileAndExecute(
34+
`return ({ _G })._G.foobar;`,
35+
undefined,
36+
`foobar = "foobar"`,
37+
"declare const _G: any;",
38+
);
39+
40+
expect(result).toBe("foobar");
41+
});
42+
3243
test("should support export property shorthand", () => {
3344
const code = `
3445
export const x = 1;

0 commit comments

Comments
 (0)