From 8baab7784547579734c0327818624df736d4fb60 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 26 Apr 2021 21:43:36 +0200 Subject: [PATCH 1/3] Made switch default case always last --- src/transformation/visitors/switch.ts | 20 +- test/unit/__snapshots__/switch.spec.ts.snap | 53 +++ test/unit/conditionals.spec.ts | 297 ----------------- test/unit/switch.spec.ts | 350 ++++++++++++++++++++ 4 files changed, 419 insertions(+), 301 deletions(-) create mode 100644 test/unit/__snapshots__/switch.spec.ts.snap create mode 100644 test/unit/switch.spec.ts diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 99ff3df7d..1dd974bd2 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -37,13 +37,25 @@ export const transformSwitchStatement: FunctionVisitor = (st statements.push(concatenatedIf); } - const hasDefaultCase = statement.caseBlock.clauses.some(ts.isDefaultClause); + const defaultCase = statement.caseBlock.clauses.find(ts.isDefaultClause); + const hasDefaultCase = defaultCase !== undefined; + statements.push(lua.createGotoStatement(`${switchName}_${hasDefaultCase ? "case_default" : "end"}`)); - for (const [index, clause] of statement.caseBlock.clauses.entries()) { - const labelName = `${switchName}_case_${ts.isCaseClause(clause) ? index : "default"}`; + // Handle all non-default cases + for (const [index, clause] of caseClauses.entries()) { + const labelName = `${switchName}_case_${index}`; + statements.push(lua.createLabelStatement(labelName)); + + const caseStatements = context.transformStatements(clause.statements); + statements.push(lua.createDoStatement(caseStatements)); + } + + // Always handle default case last + if (defaultCase !== undefined) { + const labelName = `${switchName}_case_default`; statements.push(lua.createLabelStatement(labelName)); - statements.push(lua.createDoStatement(context.transformStatements(clause.statements))); + statements.push(lua.createDoStatement(context.transformStatements(defaultCase.statements))); } statements.push(lua.createLabelStatement(`${switchName}_end`)); diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap new file mode 100644 index 000000000..e7d003fa5 --- /dev/null +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -0,0 +1,53 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`array 1`] = ` +"local ____exports = {} +function ____exports.__main(self) + local result = -1 + local ____switch3 = 2 + if ____switch3 == 0 then + goto ____switch3_case_0 + elseif ____switch3 == 1 then + goto ____switch3_case_1 + elseif ____switch3 == 2 then + goto ____switch3_case_2 + end + goto ____switch3_end + ::____switch3_case_0:: + do + do + result = 200 + goto ____switch3_end + end + end + ::____switch3_case_1:: + do + do + result = 100 + goto ____switch3_end + end + end + ::____switch3_case_2:: + do + do + result = 1 + goto ____switch3_end + end + end + ::____switch3_end:: + return result +end +return ____exports" +`; + +exports[`switch not allowed in 5.1: code 1`] = ` +"local ____exports = {} +function ____exports.__main(self) + local ____switch3 = \\"abc\\" + goto ____switch3_end + ::____switch3_end:: +end +return ____exports" +`; + +exports[`switch not allowed in 5.1: diagnostics 1`] = `"main.ts(2,9): error TSTL: Switch statements is/are not supported for target Lua 5.1."`; diff --git a/test/unit/conditionals.spec.ts b/test/unit/conditionals.spec.ts index 1df76d6a1..c9717d0c0 100644 --- a/test/unit/conditionals.spec.ts +++ b/test/unit/conditionals.spec.ts @@ -1,5 +1,4 @@ import * as tstl from "../../src"; -import { unsupportedForTarget } from "../../src/transformation/utils/diagnostics"; import * as util from "../util"; test.each([0, 1])("if (%p)", inp => { @@ -52,302 +51,6 @@ test.each([0, 1, 2, 3])("ifelseifelse (%p)", inp => { `.expectToMatchJsResult(); }); -test.each([0, 1, 2, 3])("switch (%p)", inp => { - util.testFunction` - let result: number = -1; - - switch (${inp}) { - case 0: - result = 0; - break; - case 1: - result = 1; - break; - case 2: - result = 2; - break; - } - return result; - `.expectToMatchJsResult(); -}); - -test.each([0, 1, 2, 3])("switchdefault (%p)", inp => { - util.testFunction` - let result: number = -1; - - switch (${inp}) { - case 0: - result = 0; - break; - case 1: - result = 1; - break; - case 2: - result = 2; - break; - default: - result = -2; - break; - } - return result; - `.expectToMatchJsResult(); -}); - -test.each([0, 0, 2, 3, 4, 5, 7])("switchfallthrough (%p)", inp => { - util.testFunction` - let result: number = -1; - - switch (${inp}) { - case 0: - result = 0; - case 1: - result = 1; - break; - case 2: - result = 2; - case 3: - case 4: - result = 4; - break; - case 5: - result = 5; - case 6: - result += 10; - break; - case 7: - result = 7; - default: - result = -2; - break; - } - - return result; - `.expectToMatchJsResult(); -}); - -test.each([0, 1, 2, 3])("nestedSwitch (%p)", inp => { - util.testFunction` - let result: number = -1; - - switch (${inp} as number) { - case 0: - result = 0; - break; - case 1: - switch(${inp} as number) { - case 0: - result = 0; - break; - case 1: - result = 1; - break; - default: - result = -3; - break; - } - break; - case 2: - result = 2; - break; - default: - result = -2; - break; - } - return result; - `.expectToMatchJsResult(); -}); - -test("switch cases scope", () => { - util.testFunction` - switch (0 as number) { - case 0: - let foo: number | undefined = 1; - case 1: - foo = 2; - case 2: - return foo; - } - `.expectToMatchJsResult(); -}); - -test("variable in nested scope does not interfere with case scope", () => { - util.testFunction` - let foo: number = 0; - switch (foo) { - case 0: { - let foo = 1; - } - - case 1: - return foo; - } - `.expectToMatchJsResult(); -}); - -test("switch using variable re-declared in cases", () => { - util.testFunction` - let foo: number = 0; - switch (foo) { - case 0: - let foo = true; - case 1: - return foo; - } - `.expectToMatchJsResult(); -}); - -test.each([0, 1, 2])("switch with block statement scope (%p)", inp => { - util.testFunction` - let result: number = -1; - - switch (${inp}) { - case 0: { - let x = 0; - result = 0; - break; - } - case 1: { - let x = 1; - result = x; - } - case 2: { - let x = 2; - result = x; - break; - } - } - return result; - `.expectToMatchJsResult(); -}); - -test.each([0, 1, 2, 3])("switchReturn (%p)", inp => { - util.testFunction` - switch (${inp}) { - case 0: - return 0; - break; - case 1: - return 1; - case 2: - return 2; - break; - } - - return -1; - `.expectToMatchJsResult(); -}); - -test.each([0, 1, 2, 3])("switchWithBrackets (%p)", inp => { - util.testFunction` - let result: number = -1; - - switch (${inp}) { - case 0: { - result = 0; - break; - } - case 1: { - result = 1; - break; - } - case 2: { - result = 2; - break; - } - } - return result; - `.expectToMatchJsResult(); -}); - -test.each([0, 1, 2, 3])("switchWithBracketsBreakInConditional (%p)", inp => { - util.testFunction` - let result: number = -1; - - switch (${inp}) { - case 0: { - result = 0; - break; - } - case 1: { - result = 1; - - if (result == 1) break; - } - case 2: { - result = 2; - break; - } - } - return result; - `.expectToMatchJsResult(); -}); - -test.each([0, 1, 2, 3])("switchWithBracketsBreakInInternalLoop (%p)", inp => { - util.testFunction` - let result: number = -1; - - switch (${inp} as number) { - case 0: { - result = 0; - - for (let i = 0; i < 5; i++) { - result++; - - if (i >= 2) { - break; - } - } - } - case 1: { - result++; - break; - } - case 2: { - result = 2; - break; - } - } - return result; - `.expectToMatchJsResult(); -}); - -test("switch uses elseif", () => { - test("array", () => { - util.testFunction` - let result: number = -1; - - switch (2 as number) { - case 0: { - result = 200; - break; - } - - case 1: { - result = 100; - break; - } - - case 2: { - result = 1; - break; - } - } - - return result; - ` - .expectLuaToMatchSnapshot() - .expectToMatchJsResult(); - }); -}); - -test("switch not allowed in 5.1", () => { - util.testFunction` - switch ("abc") {} - ` - .setOptions({ luaTarget: tstl.LuaTarget.Lua51 }) - .expectDiagnosticsToMatchSnapshot([unsupportedForTarget.code]); -}); - test.each([ { input: "true ? 'a' : 'b'" }, { input: "false ? 'a' : 'b'" }, diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts new file mode 100644 index 000000000..366b78ab1 --- /dev/null +++ b/test/unit/switch.spec.ts @@ -0,0 +1,350 @@ +import * as tstl from "../../src"; +import { unsupportedForTarget } from "../../src/transformation/utils/diagnostics"; +import * as util from "../util"; + +test.each([0, 1, 2, 3])("switch (%p)", inp => { + util.testFunction` + let result: number = -1; + + switch (${inp}) { + case 0: + result = 0; + break; + case 1: + result = 1; + break; + case 2: + result = 2; + break; + } + return result; + `.expectToMatchJsResult(); +}); + +test.each([0, 1, 2, 3])("switchdefault (%p)", inp => { + util.testFunction` + let result: number = -1; + + switch (${inp}) { + case 0: + result = 0; + break; + case 1: + result = 1; + break; + case 2: + result = 2; + break; + default: + result = -2; + break; + } + return result; + `.expectToMatchJsResult(); +}); + +test.each([0, 0, 2, 3, 4, 5, 7])("switchfallthrough (%p)", inp => { + util.testFunction` + let result: number = -1; + + switch (${inp}) { + case 0: + result = 0; + case 1: + result = 1; + break; + case 2: + result = 2; + case 3: + case 4: + result = 4; + break; + case 5: + result = 5; + case 6: + result += 10; + break; + case 7: + result = 7; + default: + result = -2; + break; + } + + return result; + `.expectToMatchJsResult(); +}); + +test.each([0, 1, 2, 3])("nestedSwitch (%p)", inp => { + util.testFunction` + let result: number = -1; + + switch (${inp} as number) { + case 0: + result = 0; + break; + case 1: + switch(${inp} as number) { + case 0: + result = 0; + break; + case 1: + result = 1; + break; + default: + result = -3; + break; + } + break; + case 2: + result = 2; + break; + default: + result = -2; + break; + } + return result; + `.expectToMatchJsResult(); +}); + +test("switch cases scope", () => { + util.testFunction` + switch (0 as number) { + case 0: + let foo: number | undefined = 1; + case 1: + foo = 2; + case 2: + return foo; + } + `.expectToMatchJsResult(); +}); + +test("variable in nested scope does not interfere with case scope", () => { + util.testFunction` + let foo: number = 0; + switch (foo) { + case 0: { + let foo = 1; + } + + case 1: + return foo; + } + `.expectToMatchJsResult(); +}); + +test("switch using variable re-declared in cases", () => { + util.testFunction` + let foo: number = 0; + switch (foo) { + case 0: + let foo = true; + case 1: + return foo; + } + `.expectToMatchJsResult(); +}); + +test.each([0, 1, 2])("switch with block statement scope (%p)", inp => { + util.testFunction` + let result: number = -1; + + switch (${inp}) { + case 0: { + let x = 0; + result = 0; + break; + } + case 1: { + let x = 1; + result = x; + } + case 2: { + let x = 2; + result = x; + break; + } + } + return result; + `.expectToMatchJsResult(); +}); + +test.each([0, 1, 2, 3])("switchReturn (%p)", inp => { + util.testFunction` + switch (${inp}) { + case 0: + return 0; + break; + case 1: + return 1; + case 2: + return 2; + break; + } + + return -1; + `.expectToMatchJsResult(); +}); + +test.each([0, 1, 2, 3])("switchWithBrackets (%p)", inp => { + util.testFunction` + let result: number = -1; + + switch (${inp}) { + case 0: { + result = 0; + break; + } + case 1: { + result = 1; + break; + } + case 2: { + result = 2; + break; + } + } + return result; + `.expectToMatchJsResult(); +}); + +test.each([0, 1, 2, 3])("switchWithBracketsBreakInConditional (%p)", inp => { + util.testFunction` + let result: number = -1; + + switch (${inp}) { + case 0: { + result = 0; + break; + } + case 1: { + result = 1; + + if (result == 1) break; + } + case 2: { + result = 2; + break; + } + } + return result; + `.expectToMatchJsResult(); +}); + +test.each([0, 1, 2, 3])("switchWithBracketsBreakInInternalLoop (%p)", inp => { + util.testFunction` + let result: number = -1; + + switch (${inp} as number) { + case 0: { + result = 0; + + for (let i = 0; i < 5; i++) { + result++; + + if (i >= 2) { + break; + } + } + } + case 1: { + result++; + break; + } + case 2: { + result = 2; + break; + } + } + return result; + `.expectToMatchJsResult(); +}); + +test("switch uses elseif", () => { + test("array", () => { + util.testFunction` + let result: number = -1; + + switch (2 as number) { + case 0: { + result = 200; + break; + } + + case 1: { + result = 100; + break; + } + + case 2: { + result = 1; + break; + } + } + + return result; + ` + .expectLuaToMatchSnapshot() + .expectToMatchJsResult(); + }); +}); + +test("switch not allowed in 5.1", () => { + util.testFunction` + switch ("abc") {} + ` + .setOptions({ luaTarget: tstl.LuaTarget.Lua51 }) + .expectDiagnosticsToMatchSnapshot([unsupportedForTarget.code]); +}); + +// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/967 +test("switch default case not last - first", () => { + util.testFunction` + switch (3 as number) { + default: + return "wrong"; + case 3: + return "right"; + } + `.expectToMatchJsResult(); +}); + +test("switch default case not last - second", () => { + util.testFunction` + switch (3 as number) { + case 4: + return "also wrong"; + default: + return "wrong"; + case 3: + return "right"; + } + `.expectToMatchJsResult(); +}); + +test("switch fallthrough enters default", () => { + util.testFunction` + const out = []; + switch (3 as number) { + case 3: + out.push("3"); + default: + out.push("default"); + } + return out; + `.expectToMatchJsResult(); +}); + +test("switch fallthrough does not enter earlier", () => { + util.testFunction` + const out = []; + switch (3 as number) { + default: + out.push("default"); + case 3: + out.push("3"); + } + return out; + `.debug().expectToMatchJsResult(); +}); From cba7a2f8cad8e474bcf33c426dcab1f354608399 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 26 Apr 2021 22:01:12 +0200 Subject: [PATCH 2/3] Actually fixed switch statement this time --- src/transformation/visitors/switch.ts | 29 ++++------ .../__snapshots__/conditionals.spec.ts.snap | 53 ------------------- test/unit/switch.spec.ts | 4 +- 3 files changed, 12 insertions(+), 74 deletions(-) delete mode 100644 test/unit/__snapshots__/conditionals.spec.ts.snap diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 1dd974bd2..a82ff0bf0 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -18,10 +18,13 @@ export const transformSwitchStatement: FunctionVisitor = (st let statements: lua.Statement[] = []; - const caseClauses = statement.caseBlock.clauses.filter(ts.isCaseClause); - // Starting from the back, concatenating ifs into one big if/elseif statement - const concatenatedIf = caseClauses.reduceRight((previousCondition, clause, index) => { + const concatenatedIf = statement.caseBlock.clauses.reduceRight((previousCondition, clause, index) => { + if (ts.isDefaultClause(clause)) { + // Skip default clause here (needs to be included to ensure index lines up with index later) + return previousCondition; + } + // If the clause condition holds, go to the correct label const condition = lua.createBinaryExpression( switchVariable, @@ -37,25 +40,13 @@ export const transformSwitchStatement: FunctionVisitor = (st statements.push(concatenatedIf); } - const defaultCase = statement.caseBlock.clauses.find(ts.isDefaultClause); - const hasDefaultCase = defaultCase !== undefined; - + const hasDefaultCase = statement.caseBlock.clauses.some(ts.isDefaultClause); statements.push(lua.createGotoStatement(`${switchName}_${hasDefaultCase ? "case_default" : "end"}`)); - // Handle all non-default cases - for (const [index, clause] of caseClauses.entries()) { - const labelName = `${switchName}_case_${index}`; - statements.push(lua.createLabelStatement(labelName)); - - const caseStatements = context.transformStatements(clause.statements); - statements.push(lua.createDoStatement(caseStatements)); - } - - // Always handle default case last - if (defaultCase !== undefined) { - const labelName = `${switchName}_case_default`; + for (const [index, clause] of statement.caseBlock.clauses.entries()) { + const labelName = `${switchName}_case_${ts.isCaseClause(clause) ? index : "default"}`; statements.push(lua.createLabelStatement(labelName)); - statements.push(lua.createDoStatement(context.transformStatements(defaultCase.statements))); + statements.push(lua.createDoStatement(context.transformStatements(clause.statements))); } statements.push(lua.createLabelStatement(`${switchName}_end`)); diff --git a/test/unit/__snapshots__/conditionals.spec.ts.snap b/test/unit/__snapshots__/conditionals.spec.ts.snap deleted file mode 100644 index e7d003fa5..000000000 --- a/test/unit/__snapshots__/conditionals.spec.ts.snap +++ /dev/null @@ -1,53 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`array 1`] = ` -"local ____exports = {} -function ____exports.__main(self) - local result = -1 - local ____switch3 = 2 - if ____switch3 == 0 then - goto ____switch3_case_0 - elseif ____switch3 == 1 then - goto ____switch3_case_1 - elseif ____switch3 == 2 then - goto ____switch3_case_2 - end - goto ____switch3_end - ::____switch3_case_0:: - do - do - result = 200 - goto ____switch3_end - end - end - ::____switch3_case_1:: - do - do - result = 100 - goto ____switch3_end - end - end - ::____switch3_case_2:: - do - do - result = 1 - goto ____switch3_end - end - end - ::____switch3_end:: - return result -end -return ____exports" -`; - -exports[`switch not allowed in 5.1: code 1`] = ` -"local ____exports = {} -function ____exports.__main(self) - local ____switch3 = \\"abc\\" - goto ____switch3_end - ::____switch3_end:: -end -return ____exports" -`; - -exports[`switch not allowed in 5.1: diagnostics 1`] = `"main.ts(2,9): error TSTL: Switch statements is/are not supported for target Lua 5.1."`; diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 366b78ab1..07f2903bb 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -336,7 +336,7 @@ test("switch fallthrough enters default", () => { `.expectToMatchJsResult(); }); -test("switch fallthrough does not enter earlier", () => { +test("switch fallthrough does not enter earlier default", () => { util.testFunction` const out = []; switch (3 as number) { @@ -346,5 +346,5 @@ test("switch fallthrough does not enter earlier", () => { out.push("3"); } return out; - `.debug().expectToMatchJsResult(); + `.expectToMatchJsResult(); }); From 5e9cbffbc724c5410878be82b98b9a75bdc7a058 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 26 Apr 2021 22:04:08 +0200 Subject: [PATCH 3/3] Added extra default fallthrough test --- test/unit/switch.spec.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 07f2903bb..dd3e2a09c 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -348,3 +348,16 @@ test("switch fallthrough does not enter earlier default", () => { return out; `.expectToMatchJsResult(); }); + +test("switch fallthrough stops after default", () => { + util.testFunction` + const out = []; + switch (4 as number) { + default: + out.push("default"); + case 3: + out.push("3"); + } + return out; + `.expectToMatchJsResult(); +});