From 9bb4c40633c6bdf131b53e0a8d1ad48db09d9a54 Mon Sep 17 00:00:00 2001 From: GlassBricks <24237065+GlassBricks@users.noreply.github.com> Date: Mon, 27 Mar 2023 11:21:11 -0700 Subject: [PATCH 1/4] Don't output initializer if default value is nil --- src/transformation/visitors/function.ts | 29 +++++++++++-------- .../__snapshots__/functions.spec.ts.snap | 22 ++++++++++++++ test/unit/functions/functions.spec.ts | 15 ++++++++++ 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/transformation/visitors/function.ts b/src/transformation/visitors/function.ts index f79682223..bb46e6fbb 100644 --- a/src/transformation/visitors/function.ts +++ b/src/transformation/visitors/function.ts @@ -24,10 +24,13 @@ import { transformBindingPattern } from "./variable-declaration"; function transformParameterDefaultValueDeclaration( context: TransformationContext, parameterName: lua.Identifier, - value?: ts.Expression, + value: ts.Expression, tsOriginal?: ts.Node -): lua.Statement { - const parameterValue = value ? context.transformExpression(value) : undefined; +): lua.Statement | undefined { + const parameterValue = context.transformExpression(value); + if (lua.isNilLiteral(parameterValue)) { + return undefined; + } const assignment = lua.createAssignmentStatement(parameterName, parameterValue); const nilCondition = lua.createBinaryExpression( @@ -106,7 +109,7 @@ export function transformFunctionBodyHeader( parameters: ts.NodeArray, spreadIdentifier?: lua.Identifier ): lua.Statement[] { - const headerStatements = []; + const headerStatements: lua.Statement[] = []; // Add default parameters and object binding patterns const bindingPatternDeclarations: lua.Statement[] = []; @@ -116,9 +119,12 @@ export function transformFunctionBodyHeader( const identifier = lua.createIdentifier(`____bindingPattern${bindPatternIndex++}`); if (declaration.initializer !== undefined) { // Default binding parameter - headerStatements.push( - transformParameterDefaultValueDeclaration(context, identifier, declaration.initializer) + const initializer = transformParameterDefaultValueDeclaration( + context, + identifier, + declaration.initializer ); + if (initializer) headerStatements.push(initializer); } // Binding pattern @@ -129,13 +135,12 @@ export function transformFunctionBodyHeader( bindingPatternDeclarations.push(...precedingStatements, ...bindings); } else if (declaration.initializer !== undefined) { // Default parameter - headerStatements.push( - transformParameterDefaultValueDeclaration( - context, - transformIdentifier(context, declaration.name), - declaration.initializer - ) + const initializer = transformParameterDefaultValueDeclaration( + context, + transformIdentifier(context, declaration.name), + declaration.initializer ); + if (initializer) headerStatements.push(initializer); } } diff --git a/test/unit/functions/__snapshots__/functions.spec.ts.snap b/test/unit/functions/__snapshots__/functions.spec.ts.snap index 4e07a4367..5e5670398 100644 --- a/test/unit/functions/__snapshots__/functions.spec.ts.snap +++ b/test/unit/functions/__snapshots__/functions.spec.ts.snap @@ -1,5 +1,27 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Function default parameter with "null" 1`] = ` +"local ____exports = {} +function ____exports.__main(self) + local function foo(self, x) + return x + end + return foo(nil) +end +return ____exports" +`; + +exports[`Function default parameter with "undefined" 1`] = ` +"local ____exports = {} +function ____exports.__main(self) + local function foo(self, x) + return x + end + return foo(nil) +end +return ____exports" +`; + exports[`function.length unsupported ("5.0"): code 1`] = ` "local ____exports = {} function ____exports.__main(self) diff --git a/test/unit/functions/functions.spec.ts b/test/unit/functions/functions.spec.ts index 7bd224fba..0ac23f99a 100644 --- a/test/unit/functions/functions.spec.ts +++ b/test/unit/functions/functions.spec.ts @@ -107,6 +107,21 @@ test("Function default binding parameter maintains order", () => { `.expectToMatchJsResult(); }); +test.each(["undefined", "null"])("Function default parameter with %p", defaultValue => { + util.testFunction` + function foo(x = ${defaultValue}) { + return x; + } + return foo(); + ` + .expectToMatchJsResult() + .tap(builder => { + const lua = builder.getMainLuaCodeChunk(); + expect(lua).not.toMatch("if x == nil then"); + }) + .expectLuaToMatchSnapshot(); +}); + test("Class method call", () => { util.testFunction` class TestClass { From c530dde2971e82120089cf593f12acd0d29fc7d2 Mon Sep 17 00:00:00 2001 From: GlassBricks <24237065+GlassBricks@users.noreply.github.com> Date: Mon, 27 Mar 2023 11:30:27 -0700 Subject: [PATCH 2/4] Handle preceding statements in default parameter value --- src/transformation/visitors/function.ts | 14 +++++---- .../__snapshots__/functions.spec.ts.snap | 22 ++++++++++++++ test/unit/functions/functions.spec.ts | 29 ++++++++++++++++++- 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/transformation/visitors/function.ts b/src/transformation/visitors/function.ts index bb46e6fbb..a26f56ca9 100644 --- a/src/transformation/visitors/function.ts +++ b/src/transformation/visitors/function.ts @@ -27,11 +27,15 @@ function transformParameterDefaultValueDeclaration( value: ts.Expression, tsOriginal?: ts.Node ): lua.Statement | undefined { - const parameterValue = context.transformExpression(value); - if (lua.isNilLiteral(parameterValue)) { - return undefined; + // const parameterValue = context.transformExpression(value); + const { precedingStatements, result: parameterValue } = transformInPrecedingStatementScope(context, () => + context.transformExpression(value) + ); + const statements = [...precedingStatements]; + if (!lua.isNilLiteral(parameterValue)) { + statements.push(lua.createAssignmentStatement(parameterName, parameterValue)); } - const assignment = lua.createAssignmentStatement(parameterName, parameterValue); + if (statements.length === 0) return undefined; const nilCondition = lua.createBinaryExpression( parameterName, @@ -39,7 +43,7 @@ function transformParameterDefaultValueDeclaration( lua.SyntaxKind.EqualityOperator ); - const ifBlock = lua.createBlock([assignment]); + const ifBlock = lua.createBlock(statements, tsOriginal); return lua.createIfStatement(nilCondition, ifBlock, undefined, tsOriginal); } diff --git a/test/unit/functions/__snapshots__/functions.spec.ts.snap b/test/unit/functions/__snapshots__/functions.spec.ts.snap index 5e5670398..c89b6033a 100644 --- a/test/unit/functions/__snapshots__/functions.spec.ts.snap +++ b/test/unit/functions/__snapshots__/functions.spec.ts.snap @@ -22,6 +22,28 @@ end return ____exports" `; +exports[`Function default parameter with value "null" 1`] = ` +"local ____exports = {} +function ____exports.__main(self) + local function foo(self, x) + return x + end + return foo(nil) +end +return ____exports" +`; + +exports[`Function default parameter with value "undefined" 1`] = ` +"local ____exports = {} +function ____exports.__main(self) + local function foo(self, x) + return x + end + return foo(nil) +end +return ____exports" +`; + exports[`function.length unsupported ("5.0"): code 1`] = ` "local ____exports = {} function ____exports.__main(self) diff --git a/test/unit/functions/functions.spec.ts b/test/unit/functions/functions.spec.ts index 0ac23f99a..7bdf6b040 100644 --- a/test/unit/functions/functions.spec.ts +++ b/test/unit/functions/functions.spec.ts @@ -107,7 +107,7 @@ test("Function default binding parameter maintains order", () => { `.expectToMatchJsResult(); }); -test.each(["undefined", "null"])("Function default parameter with %p", defaultValue => { +test.each(["undefined", "null"])("Function default parameter with value %p", defaultValue => { util.testFunction` function foo(x = ${defaultValue}) { return x; @@ -122,6 +122,33 @@ test.each(["undefined", "null"])("Function default parameter with %p", defaultVa .expectLuaToMatchSnapshot(); }); +test("Function default parameter with preceding statements", () => { + util.testFunction` + let i = 1 + function foo(x = i++) { + return x; + } + return [i, foo(), i]; + `.expectToMatchJsResult(); +}); + +test("Function default parameter with nil value and preceding statements", () => { + util.testFunction` + const a = new LuaTable() + a.set("foo", "bar") + function foo(x: any = a.set("foo", "baz")) { + return x ?? "nil"; + } + return [a.get("foo"), foo(), a.get("foo")]; + ` + .withLanguageExtensions() + .tap(builder => { + const lua = builder.getMainLuaCodeChunk(); + expect(lua).not.toMatch(" x = nil"); + }) + .expectToEqual(["bar", "nil", "baz"]); +}); + test("Class method call", () => { util.testFunction` class TestClass { From 8033502c39a553c60ba190db41cd1b129f28a075 Mon Sep 17 00:00:00 2001 From: GlassBricks <24237065+GlassBricks@users.noreply.github.com> Date: Mon, 27 Mar 2023 11:46:44 -0700 Subject: [PATCH 3/4] Update test snapshots --- .../__snapshots__/functions.spec.ts.snap | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/test/unit/functions/__snapshots__/functions.spec.ts.snap b/test/unit/functions/__snapshots__/functions.spec.ts.snap index c89b6033a..83345e13b 100644 --- a/test/unit/functions/__snapshots__/functions.spec.ts.snap +++ b/test/unit/functions/__snapshots__/functions.spec.ts.snap @@ -1,27 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Function default parameter with "null" 1`] = ` -"local ____exports = {} -function ____exports.__main(self) - local function foo(self, x) - return x - end - return foo(nil) -end -return ____exports" -`; - -exports[`Function default parameter with "undefined" 1`] = ` -"local ____exports = {} -function ____exports.__main(self) - local function foo(self, x) - return x - end - return foo(nil) -end -return ____exports" -`; - exports[`Function default parameter with value "null" 1`] = ` "local ____exports = {} function ____exports.__main(self) From 25a0e6778042d7c5479f9f5e237b8640c1a52692 Mon Sep 17 00:00:00 2001 From: GlassBricks <24237065+GlassBricks@users.noreply.github.com> Date: Tue, 28 Mar 2023 16:07:42 -0700 Subject: [PATCH 4/4] Changes for PR --- src/transformation/visitors/function.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/transformation/visitors/function.ts b/src/transformation/visitors/function.ts index a26f56ca9..1c21a0f7c 100644 --- a/src/transformation/visitors/function.ts +++ b/src/transformation/visitors/function.ts @@ -27,11 +27,10 @@ function transformParameterDefaultValueDeclaration( value: ts.Expression, tsOriginal?: ts.Node ): lua.Statement | undefined { - // const parameterValue = context.transformExpression(value); - const { precedingStatements, result: parameterValue } = transformInPrecedingStatementScope(context, () => - context.transformExpression(value) + const { precedingStatements: statements, result: parameterValue } = transformInPrecedingStatementScope( + context, + () => context.transformExpression(value) ); - const statements = [...precedingStatements]; if (!lua.isNilLiteral(parameterValue)) { statements.push(lua.createAssignmentStatement(parameterName, parameterValue)); }