From 9d9ffee245290e2f9a2cee1499161a7d95324ddb Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 20 Jun 2019 07:40:02 -0600 Subject: [PATCH 1/2] updated call/table expression formatting Arguments/fields will now only be split into multiple lines if one or more has evaluation side-effects that would be beneficial for a debugger to step on. --- src/LuaAST.ts | 2 +- src/LuaPrinter.ts | 100 +++++++++++++----- .../__snapshots__/transformation.spec.ts.snap | 85 ++++++++------- test/unit/assignmentDestructuring.spec.ts | 6 +- test/unit/assignments/assignments.spec.ts | 14 +-- test/unit/console.spec.ts | 18 ++-- test/unit/objectLiteral.spec.ts | 10 +- test/unit/spreadElement.spec.ts | 8 +- 8 files changed, 150 insertions(+), 93 deletions(-) diff --git a/src/LuaAST.ts b/src/LuaAST.ts index 7616177e4..177a55d6f 100644 --- a/src/LuaAST.ts +++ b/src/LuaAST.ts @@ -677,7 +677,7 @@ export function createTableFieldExpression( tsOriginal?: ts.Node, parent?: Node ): TableFieldExpression { - const expression = createNode(SyntaxKind.TableExpression, tsOriginal, parent) as TableFieldExpression; + const expression = createNode(SyntaxKind.TableFieldExpression, tsOriginal, parent) as TableFieldExpression; setParent(value, expression); expression.value = value; setParent(key, expression); diff --git a/src/LuaPrinter.ts b/src/LuaPrinter.ts index 5b1d8a8c4..324caf431 100644 --- a/src/LuaPrinter.ts +++ b/src/LuaPrinter.ts @@ -573,17 +573,8 @@ export class LuaPrinter { chunks.push("{"); - if (expression.fields && expression.fields.length > 0) { - if (expression.fields.length === 1) { - // Inline tables with only one entry - chunks.push(this.printTableFieldExpression(expression.fields[0])); - } else { - chunks.push("\n"); - this.pushIndent(); - expression.fields.forEach(f => chunks.push(this.indent(), this.printTableFieldExpression(f), ",\n")); - this.popIndent(); - chunks.push(this.indent()); - } + if (expression.fields) { + chunks.push(...this.printExpressionList(expression.fields)); } chunks.push("}"); @@ -634,30 +625,33 @@ export class LuaPrinter { public printCallExpression(expression: tstl.CallExpression): SourceNode { const chunks = []; - const parameterChunks = - expression.params !== undefined ? expression.params.map(e => this.printExpression(e)) : []; + chunks.push(this.printExpression(expression.expression), "("); + + if (expression.params) { + chunks.push(...this.printExpressionList(expression.params)); + } - chunks.push(this.printExpression(expression.expression), "(", ...this.joinChunks(", ", parameterChunks), ")"); + chunks.push(")"); return this.createSourceNode(expression, chunks); } public printMethodCallExpression(expression: tstl.MethodCallExpression): SourceNode { - const prefix = this.printExpression(expression.prefixExpression); + const chunks = []; - const parameterChunks = - expression.params !== undefined ? expression.params.map(e => this.printExpression(e)) : []; + const prefix = this.printExpression(expression.prefixExpression); const name = this.printIdentifier(expression.name); - return this.createSourceNode(expression, [ - prefix, - ":", - name, - "(", - ...this.joinChunks(", ", parameterChunks), - ")", - ]); + chunks.push(prefix, ":", name, "("); + + if (expression.params) { + chunks.push(...this.printExpressionList(expression.params)); + } + + chunks.push(")"); + + return this.createSourceNode(expression, chunks); } public printIdentifier(expression: tstl.Identifier): SourceNode { @@ -713,6 +707,62 @@ export class LuaPrinter { return result; } + protected isSimpleExpression(expression: tstl.Expression): boolean { + switch (expression.kind) { + case tstl.SyntaxKind.CallExpression: + case tstl.SyntaxKind.MethodCallExpression: + case tstl.SyntaxKind.FunctionExpression: + return false; + + case tstl.SyntaxKind.TableExpression: + const tableExpression = expression as tstl.TableExpression; + return !tableExpression.fields || tableExpression.fields.every(e => this.isSimpleExpression(e)); + + case tstl.SyntaxKind.TableFieldExpression: + const fieldExpression = expression as tstl.TableFieldExpression; + return ( + (!fieldExpression.key || this.isSimpleExpression(fieldExpression.key)) && + this.isSimpleExpression(fieldExpression.value) + ); + + case tstl.SyntaxKind.TableIndexExpression: + const indexExpression = expression as tstl.TableIndexExpression; + return this.isSimpleExpression(indexExpression.table) && this.isSimpleExpression(indexExpression.index); + + case tstl.SyntaxKind.UnaryExpression: + return this.isSimpleExpression((expression as tstl.UnaryExpression).operand); + + case tstl.SyntaxKind.BinaryExpression: + const binaryExpression = expression as tstl.BinaryExpression; + return ( + this.isSimpleExpression(binaryExpression.left) && this.isSimpleExpression(binaryExpression.right) + ); + + case tstl.SyntaxKind.ParenthesizedExpression: + return this.isSimpleExpression((expression as tstl.ParenthesizedExpression).innerExpression); + } + return true; + } + + protected printExpressionList(expressions: tstl.Expression[]): SourceChunk[] { + const chunks: SourceChunk[] = []; + + if (expressions.every(e => this.isSimpleExpression(e))) { + chunks.push(...this.joinChunks(", ", expressions.map(e => this.printExpression(e)))); + } else { + chunks.push("\n"); + this.pushIndent(); + expressions.forEach((p, i) => { + const tail = i < expressions.length - 1 ? ",\n" : "\n"; + chunks.push(this.indent(), this.printExpression(p), tail); + }); + this.popIndent(); + chunks.push(this.indent()); + } + + return chunks; + } + // The key difference between this and SourceNode.toStringWithSourceMap() is that SourceNodes with null line/column // will not generate 'empty' mappings in the source map that point to nothing in the original TS. private buildSourceMap(sourceFile: string, sourceRoot: string, rootSourceNode: SourceNode): SourceMapGenerator { diff --git a/test/translation/__snapshots__/transformation.spec.ts.snap b/test/translation/__snapshots__/transformation.spec.ts.snap index 58e99b895..add5eab21 100644 --- a/test/translation/__snapshots__/transformation.spec.ts.snap +++ b/test/translation/__snapshots__/transformation.spec.ts.snap @@ -228,28 +228,12 @@ end" `; exports[`Transformation (forIn) 1`] = ` -"for i in pairs({ - a = 1, - b = 2, - c = 3, - d = 4, -}) do +"for i in pairs({a = 1, b = 2, c = 3, d = 4}) do end" `; exports[`Transformation (forOf) 1`] = ` -"for ____, i in ipairs({ - 1, - 2, - 3, - 4, - 5, - 6, - 7, - 8, - 9, - 10, -}) do +"for ____, i in ipairs({1, 2, 3, 4, 5, 6, 7, 8, 9, 10}) do end" `; @@ -577,9 +561,11 @@ f = function(____, x) return ({x = x}) end" exports[`Transformation (tryCatch) 1`] = ` "do - local ____TS_try, er = pcall(function() - local a = 42 - end) + local ____TS_try, er = pcall( + function() + local a = 42 + end + ) if not ____TS_try then local b = \\"fail\\" end @@ -588,9 +574,11 @@ end" exports[`Transformation (tryCatchFinally) 1`] = ` "do - local ____TS_try, er = pcall(function() - local a = 42 - end) + local ____TS_try, er = pcall( + function() + local a = 42 + end + ) if not ____TS_try then local b = \\"fail\\" end @@ -602,9 +590,11 @@ end" exports[`Transformation (tryFinally) 1`] = ` "do - pcall(function() - local a = 42 - end) + pcall( + function() + local a = 42 + end + ) do local b = \\"finally\\" end @@ -618,30 +608,47 @@ end tupleReturn(_G) noTupleReturn(_G) local a, b = tupleReturn(_G) -local c, d = table.unpack(noTupleReturn(_G)) +local c, d = table.unpack( + noTupleReturn(_G) +) a, b = tupleReturn(_G) -c, d = table.unpack(noTupleReturn(_G)) -local e = ({tupleReturn(_G)}) +c, d = table.unpack( + noTupleReturn(_G) +) +local e = ({ + tupleReturn(_G) +}) local f = noTupleReturn(_G) -e = ({tupleReturn(_G)}) +e = ({ + tupleReturn(_G) +}) f = noTupleReturn(_G) -foo(_G, ({tupleReturn(_G)})) -foo(_G, noTupleReturn(_G)) +foo( + _G, + ({ + tupleReturn(_G) + }) +) +foo( + _G, + noTupleReturn(_G) +) function tupleReturnFromVar(self) - local r = { - 1, - \\"baz\\", - } + local r = {1, \\"baz\\"} return table.unpack(r) end function tupleReturnForward(self) return tupleReturn(_G) end function tupleNoForward(self) - return ({tupleReturn(_G)}) + return ({ + tupleReturn(_G) + }) end function tupleReturnUnpack(self) - return table.unpack(tupleNoForward(_G)) + return table.unpack( + tupleNoForward(_G) + ) end" `; diff --git a/test/unit/assignmentDestructuring.spec.ts b/test/unit/assignmentDestructuring.spec.ts index 0aeca594d..908a4eac9 100644 --- a/test/unit/assignmentDestructuring.spec.ts +++ b/test/unit/assignmentDestructuring.spec.ts @@ -10,7 +10,7 @@ test("Assignment destructuring [5.1]", () => { luaTarget: tstl.LuaTarget.Lua51, luaLibImport: tstl.LuaLibImportKind.None, }); - expect(lua).toBe(`local a, b = unpack(myFunc())`); + expect(lua).toBe(`local a, b = unpack(\n myFunc()\n)`); }); test("Assignment destructuring [5.2]", () => { @@ -18,7 +18,7 @@ test("Assignment destructuring [5.2]", () => { luaTarget: tstl.LuaTarget.Lua52, luaLibImport: tstl.LuaLibImportKind.None, }); - expect(lua).toBe(`local a, b = table.unpack(myFunc())`); + expect(lua).toBe(`local a, b = table.unpack(\n myFunc()\n)`); }); test("Assignment destructuring [JIT]", () => { @@ -26,7 +26,7 @@ test("Assignment destructuring [JIT]", () => { luaTarget: tstl.LuaTarget.LuaJIT, luaLibImport: tstl.LuaLibImportKind.None, }); - expect(lua).toBe(`local a, b = unpack(myFunc())`); + expect(lua).toBe(`local a, b = unpack(\n myFunc()\n)`); }); test.each([ diff --git a/test/unit/assignments/assignments.spec.ts b/test/unit/assignments/assignments.spec.ts index 7eb8e5ecd..c81c5083c 100644 --- a/test/unit/assignments/assignments.spec.ts +++ b/test/unit/assignments/assignments.spec.ts @@ -4,10 +4,10 @@ import * as util from "../../util"; test.each([ { inp: `"abc"`, out: `"abc"` }, { inp: "3", out: "3" }, - { inp: "[1,2,3]", out: "{\n 1,\n 2,\n 3,\n}" }, + { inp: "[1,2,3]", out: "{1, 2, 3}" }, { inp: "true", out: "true" }, { inp: "false", out: "false" }, - { inp: `{a:3,b:"4"}`, out: `{\n a = 3,\n b = "4",\n}` }, + { inp: `{a:3,b:"4"}`, out: `{a = 3, b = "4"}` }, ])("Const assignment (%p)", ({ inp, out }) => { const lua = util.transpileString(`const myvar = ${inp}`); expect(lua).toBe(`local myvar = ${out}`); @@ -16,10 +16,10 @@ test.each([ test.each([ { inp: `"abc"`, out: `"abc"` }, { inp: "3", out: "3" }, - { inp: "[1,2,3]", out: "{\n 1,\n 2,\n 3,\n}" }, + { inp: "[1,2,3]", out: "{1, 2, 3}" }, { inp: "true", out: "true" }, { inp: "false", out: "false" }, - { inp: `{a:3,b:"4"}`, out: `{\n a = 3,\n b = "4",\n}` }, + { inp: `{a:3,b:"4"}`, out: `{a = 3, b = "4"}` }, ])("Let assignment (%p)", ({ inp, out }) => { const lua = util.transpileString(`let myvar = ${inp}`); expect(lua).toBe(`local myvar = ${out}`); @@ -28,10 +28,10 @@ test.each([ test.each([ { inp: `"abc"`, out: `"abc"` }, { inp: "3", out: "3" }, - { inp: "[1,2,3]", out: "{\n 1,\n 2,\n 3,\n}" }, + { inp: "[1,2,3]", out: "{1, 2, 3}" }, { inp: "true", out: "true" }, { inp: "false", out: "false" }, - { inp: `{a:3,b:"4"}`, out: `{\n a = 3,\n b = "4",\n}` }, + { inp: `{a:3,b:"4"}`, out: `{a = 3, b = "4"}` }, ])("Var assignment (%p)", ({ inp, out }) => { const lua = util.transpileString(`var myvar = ${inp}`); expect(lua).toBe(`myvar = ${out}`); @@ -95,7 +95,7 @@ test("TupleReturn Single assignment", () => { `; const lua = util.transpileString(code); - expect(lua).toBe("local a = ({abc()})\na = ({abc()})"); + expect(lua).toBe("local a = ({\n abc()\n})\na = ({\n abc()\n})"); }); test("TupleReturn interface assignment", () => { diff --git a/test/unit/console.spec.ts b/test/unit/console.spec.ts index 729a4440c..e82c23b04 100644 --- a/test/unit/console.spec.ts +++ b/test/unit/console.spec.ts @@ -5,8 +5,8 @@ const compilerOptions = { lib: ["lib.es2015.d.ts", "lib.dom.d.ts"] }; test.each([ { inp: "console.log()", expected: "print()" }, { inp: 'console.log("Hello")', expected: 'print("Hello")' }, - { inp: 'console.log("Hello %s", "there")', expected: 'print(string.format("Hello %s", "there"))' }, - { inp: 'console.log("Hello %%s", "there")', expected: 'print(string.format("Hello %%s", "there"))' }, + { inp: 'console.log("Hello %s", "there")', expected: 'print(\n string.format("Hello %s", "there")\n)' }, + { inp: 'console.log("Hello %%s", "there")', expected: 'print(\n string.format("Hello %%s", "there")\n)' }, { inp: 'console.log("Hello", "There")', expected: 'print("Hello", "There")' }, ])("console.log (%p)", ({ inp, expected }) => { expect(util.transpileString(inp, compilerOptions)).toBe(expected); @@ -15,23 +15,23 @@ test.each([ test.each([ { inp: "console.trace()", - expected: "print(debug.traceback())", + expected: "print(\n debug.traceback()\n)", }, { inp: 'console.trace("message")', - expected: 'print(debug.traceback("message"))', + expected: 'print(\n debug.traceback("message")\n)', }, { inp: 'console.trace("Hello %s", "there")', - expected: 'print(debug.traceback(string.format("Hello %s", "there")))', + expected: 'print(\n debug.traceback(\n string.format("Hello %s", "there")\n )\n)', }, { inp: 'console.trace("Hello %%s", "there")', - expected: 'print(debug.traceback(string.format("Hello %%s", "there")))', + expected: 'print(\n debug.traceback(\n string.format("Hello %%s", "there")\n )\n)', }, { inp: 'console.trace("Hello", "there")', - expected: 'print(debug.traceback("Hello", "there"))', + expected: 'print(\n debug.traceback("Hello", "there")\n)', }, ])("console.trace (%p)", ({ inp, expected }) => { expect(util.transpileString(inp, compilerOptions)).toBe(expected); @@ -48,11 +48,11 @@ test.each([ }, { inp: 'console.assert(false, "message %s", "info")', - expected: 'assert(false, string.format("message %s", "info"))', + expected: 'assert(\n false,\n string.format("message %s", "info")\n)', }, { inp: 'console.assert(false, "message %%s", "info")', - expected: 'assert(false, string.format("message %%s", "info"))', + expected: 'assert(\n false,\n string.format("message %%s", "info")\n)', }, { inp: 'console.assert(false, "message", "more")', diff --git a/test/unit/objectLiteral.spec.ts b/test/unit/objectLiteral.spec.ts index 54323e528..2d71a9fe8 100644 --- a/test/unit/objectLiteral.spec.ts +++ b/test/unit/objectLiteral.spec.ts @@ -1,11 +1,11 @@ import * as util from "../util"; test.each([ - { inp: `{a:3,b:"4"}`, out: '{\n a = 3,\n b = "4",\n}' }, - { inp: `{"a":3,b:"4"}`, out: '{\n a = 3,\n b = "4",\n}' }, - { inp: `{["a"]:3,b:"4"}`, out: '{\n a = 3,\n b = "4",\n}' }, - { inp: `{["a"+123]:3,b:"4"}`, out: '{\n ["a" .. 123] = 3,\n b = "4",\n}' }, - { inp: `{[myFunc()]:3,b:"4"}`, out: '{\n [myFunc(_G)] = 3,\n b = "4",\n}' }, + { inp: `{a:3,b:"4"}`, out: '{a = 3, b = "4"}' }, + { inp: `{"a":3,b:"4"}`, out: '{a = 3, b = "4"}' }, + { inp: `{["a"]:3,b:"4"}`, out: '{a = 3, b = "4"}' }, + { inp: `{["a"+123]:3,b:"4"}`, out: '{["a" .. 123] = 3, b = "4"}' }, + { inp: `{[myFunc()]:3,b:"4"}`, out: '{\n [myFunc(_G)] = 3,\n b = "4"\n}' }, { inp: `{x}`, out: `{x = x}` }, ])("Object Literal (%p)", ({ inp, out }) => { const lua = util.transpileString(`const myvar = ${inp};`); diff --git a/test/unit/spreadElement.spec.ts b/test/unit/spreadElement.spec.ts index 1177ff262..28201f12e 100644 --- a/test/unit/spreadElement.spec.ts +++ b/test/unit/spreadElement.spec.ts @@ -15,7 +15,7 @@ test("Spread Element Lua 5.1", () => { luaLibImport: tstl.LuaLibImportKind.None, }; const lua = util.transpileString(`[].push(...${JSON.stringify([1, 2, 3])});`, options); - expect(lua).toBe("__TS__ArrayPush({}, unpack({\n 1,\n 2,\n 3,\n}))"); + expect(lua).toBe("__TS__ArrayPush(\n {},\n unpack({1, 2, 3})\n)"); }); test("Spread Element Lua 5.2", () => { @@ -24,7 +24,7 @@ test("Spread Element Lua 5.2", () => { luaLibImport: tstl.LuaLibImportKind.None, }; const lua = util.transpileString(`[...[0, 1, 2]]`, options); - expect(lua).toBe("local ____ = {table.unpack({\n 0,\n 1,\n 2,\n})}"); + expect(lua).toBe("local ____ = {\n table.unpack({0, 1, 2})\n}"); }); test("Spread Element Lua 5.3", () => { @@ -33,7 +33,7 @@ test("Spread Element Lua 5.3", () => { luaLibImport: tstl.LuaLibImportKind.None, }; const lua = util.transpileString(`[...[0, 1, 2]]`, options); - expect(lua).toBe("local ____ = {table.unpack({\n 0,\n 1,\n 2,\n})}"); + expect(lua).toBe("local ____ = {\n table.unpack({0, 1, 2})\n}"); }); test("Spread Element Lua JIT", () => { @@ -42,7 +42,7 @@ test("Spread Element Lua JIT", () => { luaLibImport: tstl.LuaLibImportKind.None, }; const lua = util.transpileString(`[...[0, 1, 2]]`, options); - expect(lua).toBe("local ____ = {unpack({\n 0,\n 1,\n 2,\n})}"); + expect(lua).toBe("local ____ = {\n unpack({0, 1, 2})\n}"); }); test("Spread Element Iterable", () => { From 2c811ea628493db6a2e9dbef1340a5a39cc58673 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Fri, 28 Jun 2019 06:39:31 -0600 Subject: [PATCH 2/2] moved isSimpleExpression to tsHelper --- src/LuaPrinter.ts | 39 +-------------------------------------- src/TSHelper.ts | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/LuaPrinter.ts b/src/LuaPrinter.ts index 5d3f0e8b9..ebac13788 100644 --- a/src/LuaPrinter.ts +++ b/src/LuaPrinter.ts @@ -709,47 +709,10 @@ export class LuaPrinter { return result; } - protected isSimpleExpression(expression: tstl.Expression): boolean { - switch (expression.kind) { - case tstl.SyntaxKind.CallExpression: - case tstl.SyntaxKind.MethodCallExpression: - case tstl.SyntaxKind.FunctionExpression: - return false; - - case tstl.SyntaxKind.TableExpression: - const tableExpression = expression as tstl.TableExpression; - return !tableExpression.fields || tableExpression.fields.every(e => this.isSimpleExpression(e)); - - case tstl.SyntaxKind.TableFieldExpression: - const fieldExpression = expression as tstl.TableFieldExpression; - return ( - (!fieldExpression.key || this.isSimpleExpression(fieldExpression.key)) && - this.isSimpleExpression(fieldExpression.value) - ); - - case tstl.SyntaxKind.TableIndexExpression: - const indexExpression = expression as tstl.TableIndexExpression; - return this.isSimpleExpression(indexExpression.table) && this.isSimpleExpression(indexExpression.index); - - case tstl.SyntaxKind.UnaryExpression: - return this.isSimpleExpression((expression as tstl.UnaryExpression).operand); - - case tstl.SyntaxKind.BinaryExpression: - const binaryExpression = expression as tstl.BinaryExpression; - return ( - this.isSimpleExpression(binaryExpression.left) && this.isSimpleExpression(binaryExpression.right) - ); - - case tstl.SyntaxKind.ParenthesizedExpression: - return this.isSimpleExpression((expression as tstl.ParenthesizedExpression).innerExpression); - } - return true; - } - protected printExpressionList(expressions: tstl.Expression[]): SourceChunk[] { const chunks: SourceChunk[] = []; - if (expressions.every(e => this.isSimpleExpression(e))) { + if (expressions.every(e => tsHelper.isSimpleExpression(e))) { chunks.push(...this.joinChunks(", ", expressions.map(e => this.printExpression(e)))); } else { chunks.push("\n"); diff --git a/src/TSHelper.ts b/src/TSHelper.ts index aa21e8698..e5ebb14af 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -1,5 +1,6 @@ import * as ts from "typescript"; import { Decorator, DecoratorKind } from "./Decorator"; +import * as tstl from "./LuaAST"; export enum ContextType { None, @@ -835,3 +836,39 @@ export function isArrayLengthAssignment( return name === "length"; } + +// Returns true if expression contains no function calls +export function isSimpleExpression(expression: tstl.Expression): boolean { + switch (expression.kind) { + case tstl.SyntaxKind.CallExpression: + case tstl.SyntaxKind.MethodCallExpression: + case tstl.SyntaxKind.FunctionExpression: + return false; + + case tstl.SyntaxKind.TableExpression: + const tableExpression = expression as tstl.TableExpression; + return !tableExpression.fields || tableExpression.fields.every(e => isSimpleExpression(e)); + + case tstl.SyntaxKind.TableFieldExpression: + const fieldExpression = expression as tstl.TableFieldExpression; + return ( + (!fieldExpression.key || isSimpleExpression(fieldExpression.key)) && + isSimpleExpression(fieldExpression.value) + ); + + case tstl.SyntaxKind.TableIndexExpression: + const indexExpression = expression as tstl.TableIndexExpression; + return isSimpleExpression(indexExpression.table) && isSimpleExpression(indexExpression.index); + + case tstl.SyntaxKind.UnaryExpression: + return isSimpleExpression((expression as tstl.UnaryExpression).operand); + + case tstl.SyntaxKind.BinaryExpression: + const binaryExpression = expression as tstl.BinaryExpression; + return isSimpleExpression(binaryExpression.left) && isSimpleExpression(binaryExpression.right); + + case tstl.SyntaxKind.ParenthesizedExpression: + return isSimpleExpression((expression as tstl.ParenthesizedExpression).innerExpression); + } + return true; +}