From 74d0aef84c56fcbc996cb1cda3c295a981dd4cf8 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sat, 2 Oct 2021 15:51:02 +0200 Subject: [PATCH 1/9] Make await throw if awaited thing is a rejected promise --- src/lualib/Await.ts | 3 +++ test/unit/builtins/async-await.spec.ts | 35 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/lualib/Await.ts b/src/lualib/Await.ts index 61f383e96..fbecfecf0 100644 --- a/src/lualib/Await.ts +++ b/src/lualib/Await.ts @@ -48,5 +48,8 @@ function __TS__AsyncAwaiter(this: void, generator: (this: void) => void) { } function __TS__Await(this: void, thing: unknown) { + if (thing instanceof __TS__Promise && thing.state === __TS__PromiseState.Rejected) { + throw thing.rejectionReason; + } return coroutine.yield(thing); } diff --git a/test/unit/builtins/async-await.spec.ts b/test/unit/builtins/async-await.spec.ts index 0a9c3513e..90d319d00 100644 --- a/test/unit/builtins/async-await.spec.ts +++ b/test/unit/builtins/async-await.spec.ts @@ -369,3 +369,38 @@ test("async function can forward varargs", () => { .setTsHeader(promiseTestLib) .expectToEqual(["resolved", "A", "B", "C"]); }); + +// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1105 +describe("try/catch in async function", () => { + test("await inside try/catch returns inside async function", () => { + util.testModule` + export let result = 0; + async function foo(): Promise { + try { + return await new Promise(resolve => resolve(4)); + } catch { + throw "an error occurred in the async function" + } + } + foo().then(value => { + result = value; + }); + `.expectToEqual({ result: 4 }); + }); + + test("await inside try/catch throws inside async function", () => { + util.testModule` + export let reason = ""; + async function foo(): Promise { + try { + return await new Promise((resolve, reject) => reject("test error")); + } catch (e) { + throw "an error occurred in the async function: " + e; + } + } + foo().catch(e => { + reason = e; + }); + `.expectToEqual({ reason: "an error occurred in the async function: test error" }); + }); +}); From 9bf79f74496c1ef65750a87f24b9b7b67999b57f Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sun, 10 Oct 2021 20:32:33 +0200 Subject: [PATCH 2/9] fix almost all tests --- src/lualib/Await.ts | 42 +++++++++++----- src/transformation/visitors/async-await.ts | 3 +- src/transformation/visitors/errors.ts | 43 ++++++++--------- src/transformation/visitors/return.ts | 56 ++++++++++++++++------ test/unit/builtins/async-await.spec.ts | 34 ++++++++++++- 5 files changed, 125 insertions(+), 53 deletions(-) diff --git a/src/lualib/Await.ts b/src/lualib/Await.ts index fbecfecf0..c0bab3c45 100644 --- a/src/lualib/Await.ts +++ b/src/lualib/Await.ts @@ -14,6 +14,8 @@ // }; // +type ErrorHandler = (this: void, error: unknown) => unknown; + // eslint-disable-next-line @typescript-eslint/promise-function-async function __TS__AsyncAwaiter(this: void, generator: (this: void) => void) { return new Promise((resolve, reject) => { @@ -23,33 +25,47 @@ function __TS__AsyncAwaiter(this: void, generator: (this: void) => void) { function adopt(value: unknown) { return value instanceof __TS__Promise ? value : Promise.resolve(value); } - function fulfilled(value) { - const [success, resultOrError] = coroutine.resume(asyncCoroutine, value); + function fulfilled(value: unknown) { + const [success, errorOrErrorHandler, resultOrError] = coroutine.resume(asyncCoroutine, value); if (success) { - step(resultOrError); + step(resultOrError, errorOrErrorHandler); } else { reject(resultOrError); } } - function step(result: unknown) { + function rejected(handler: ErrorHandler | undefined) { + if (handler) { + return (value: unknown) => { + const [success, valueOrError] = pcall(handler, value); + if (success) { + step(valueOrError, handler); + } else { + reject(valueOrError); + } + }; + } else { + // If no catch clause, just reject + return value => { + reject(value) + }; + } + } + function step(result: unknown, errorHandler: ErrorHandler | undefined) { if (coroutine.status(asyncCoroutine) === "dead") { resolve(result); } else { - adopt(result).then(fulfilled, reason => reject(reason)); + adopt(result).then(fulfilled, rejected(errorHandler)); } } - const [success, resultOrError] = coroutine.resume(asyncCoroutine); + const [success, errorOrErrorHandler, resultOrError] = coroutine.resume(asyncCoroutine); if (success) { - step(resultOrError); + step(resultOrError, errorOrErrorHandler); } else { - reject(resultOrError); + reject(errorOrErrorHandler); } }); } -function __TS__Await(this: void, thing: unknown) { - if (thing instanceof __TS__Promise && thing.state === __TS__PromiseState.Rejected) { - throw thing.rejectionReason; - } - return coroutine.yield(thing); +function __TS__Await(this: void, errorHandler: ErrorHandler, thing: unknown) { + return coroutine.yield(errorHandler, thing); } diff --git a/src/transformation/visitors/async-await.ts b/src/transformation/visitors/async-await.ts index c6dcc86dc..6b43d4c84 100644 --- a/src/transformation/visitors/async-await.ts +++ b/src/transformation/visitors/async-await.ts @@ -16,7 +16,8 @@ export const transformAwaitExpression: FunctionVisitor = (no } const expression = context.transformExpression(node.expression); - return transformLuaLibFunction(context, LuaLibFeature.Await, node, expression); + const catchIdentifier = lua.createIdentifier("____catch"); + return transformLuaLibFunction(context, LuaLibFeature.Await, node, catchIdentifier, expression); }; export function isAsyncFunction(declaration: ts.FunctionLikeDeclaration): boolean { diff --git a/src/transformation/visitors/errors.ts b/src/transformation/visitors/errors.ts index 0f067c939..b68ec317b 100644 --- a/src/transformation/visitors/errors.ts +++ b/src/transformation/visitors/errors.ts @@ -6,6 +6,7 @@ import { findScope, ScopeType } from "../utils/scope"; import { transformScopeBlock } from "./block"; import { transformIdentifier } from "./identifier"; import { isInMultiReturnFunction } from "./language-extensions/multi"; +import { createReturnStatement } from "./return"; export const transformTryStatement: FunctionVisitor = (statement, context) => { const [tryBlock, tryScope] = transformScopeBlock(context, statement.tryBlock, ScopeType.Try); @@ -15,7 +16,7 @@ export const transformTryStatement: FunctionVisitor = (statemen const result: lua.Statement[] = []; - let returnedIdentifier: lua.Identifier | undefined; + const returnedIdentifier = lua.createIdentifier("____hasReturned"); let returnCondition: lua.Expression | undefined; const pCall = lua.createIdentifier("pcall"); @@ -23,16 +24,14 @@ export const transformTryStatement: FunctionVisitor = (statemen if (statement.catchClause && statement.catchClause.block.statements.length > 0) { // try with catch - let [catchBlock, catchScope] = transformScopeBlock(context, statement.catchClause.block, ScopeType.Catch); - if (statement.catchClause.variableDeclaration) { - // Replace ____returned with catch variable - returnedIdentifier = transformIdentifier( - context, - statement.catchClause.variableDeclaration.name as ts.Identifier - ); - } else if (tryScope.functionReturned || catchScope.functionReturned) { - returnedIdentifier = lua.createIdentifier("____returned"); - } + const [catchBlock, catchScope] = transformScopeBlock(context, statement.catchClause.block, ScopeType.Catch); + + const catchParameter = statement.catchClause.variableDeclaration ? transformIdentifier(context, statement.catchClause.variableDeclaration.name as ts.Identifier) : undefined; + const catchParameters = () => catchParameter ? [lua.cloneIdentifier(catchParameter)] : []; + + const catchIdentifier = lua.createIdentifier("____catch"); + const catchFunction = lua.createFunctionExpression(catchBlock, catchParameters()); + result.push(lua.createVariableDeclarationStatement(catchIdentifier, catchFunction)); const tryReturnIdentifiers = [tryResultIdentifier]; // ____try if (returnedIdentifier) { @@ -44,20 +43,18 @@ export const transformTryStatement: FunctionVisitor = (statemen } result.push(lua.createVariableDeclarationStatement(tryReturnIdentifiers, tryCall)); - if ((tryScope.functionReturned || catchScope.functionReturned) && returnedIdentifier) { - // Wrap catch in function if try or catch has return - const catchCall = lua.createCallExpression(lua.createFunctionExpression(catchBlock), []); - const catchAssign = lua.createAssignmentStatement( - [lua.cloneIdentifier(returnedIdentifier), lua.cloneIdentifier(returnValueIdentifier)], - catchCall - ); - catchBlock = lua.createBlock([catchAssign]); - } + // Wrap catch in function if try or catch has return + const catchCall = lua.createCallExpression(catchIdentifier, [lua.cloneIdentifier(returnedIdentifier)]); + const catchAssign = lua.createAssignmentStatement( + [lua.cloneIdentifier(returnedIdentifier), lua.cloneIdentifier(returnValueIdentifier)], + catchCall + ); + const notTryCondition = lua.createUnaryExpression(tryResultIdentifier, lua.SyntaxKind.NotOperator); - result.push(lua.createIfStatement(notTryCondition, catchBlock)); + result.push(lua.createIfStatement(notTryCondition, lua.createBlock([catchAssign]))); } else if (tryScope.functionReturned) { // try with return, but no catch - returnedIdentifier = lua.createIdentifier("____returned"); + // returnedIdentifier = lua.createIdentifier("____returned"); const returnedVariables = [tryResultIdentifier, returnedIdentifier, returnValueIdentifier]; result.push(lua.createVariableDeclarationStatement(returnedVariables, tryCall)); @@ -94,7 +91,7 @@ export const transformTryStatement: FunctionVisitor = (statemen returnValues.push(lua.cloneIdentifier(returnValueIdentifier)); } - const returnStatement = lua.createReturnStatement(returnValues); + const returnStatement = createReturnStatement(context, returnValues, statement); const ifReturnedStatement = lua.createIfStatement(returnCondition, lua.createBlock([returnStatement])); result.push(ifReturnedStatement); } diff --git a/src/transformation/visitors/return.ts b/src/transformation/visitors/return.ts index 2ed0ebe3e..a0b60264b 100644 --- a/src/transformation/visitors/return.ts +++ b/src/transformation/visitors/return.ts @@ -14,6 +14,7 @@ import { canBeMultiReturnType, } from "./language-extensions/multi"; import { invalidMultiFunctionReturnType } from "../utils/diagnostics"; +import { findFirstNodeAbove } from "../utils/typescript"; function transformExpressionsInReturn( context: TransformationContext, @@ -59,17 +60,6 @@ export function transformExpressionBodyToReturnStatement( } export const transformReturnStatement: FunctionVisitor = (statement, context) => { - // Bubble up explicit return flag and check if we're inside a try/catch block - let insideTryCatch = false; - for (const scope of walkScopesUp(context)) { - scope.functionReturned = true; - - if (scope.type === ScopeType.Function) { - break; - } - - insideTryCatch = insideTryCatch || scope.type === ScopeType.Try || scope.type === ScopeType.Catch; - } let results: lua.Expression[]; @@ -80,15 +70,51 @@ export const transformReturnStatement: FunctionVisitor = (st validateAssignment(context, statement, expressionType, returnType); } - results = transformExpressionsInReturn(context, statement.expression, insideTryCatch); + results = transformExpressionsInReturn(context, statement.expression, isInTryCatch(context)); } else { // Empty return results = []; } - if (insideTryCatch) { + return createReturnStatement(context, results, statement); +}; + +export function createReturnStatement(context: TransformationContext, values: lua.Expression[], node: ts.Node): lua.ReturnStatement { + const results = [...values]; + + if (isInTryCatch(context)) { + // Bubble up explicit return flag and check if we're inside a try/catch block results.unshift(lua.createBooleanLiteral(true)); + } else if (isInAsyncFunction(node)) { + // Add nil error handler in async function and not in try + results.unshift(lua.createNilLiteral()); } - return lua.createReturnStatement(results, statement); -}; + return lua.createReturnStatement(results, node); +} + +function isInAsyncFunction(node: ts.Node): boolean { + // Check if node is in function declaration with `async` + const declaration = findFirstNodeAbove(node, ts.isFunctionLike); + if (!declaration) { + return false; + } + + return declaration.modifiers?.some(m => m.kind === ts.SyntaxKind.AsyncKeyword) ?? false; +} + +function isInTryCatch(context: TransformationContext): boolean { + // Check if context is in a try or catch + let insideTryCatch = false; + for (const scope of walkScopesUp(context)) { + scope.functionReturned = true; + + if (scope.type === ScopeType.Function) { + break; + } + + insideTryCatch = insideTryCatch || scope.type === ScopeType.Try || scope.type === ScopeType.Catch; + } + + return insideTryCatch; +} diff --git a/test/unit/builtins/async-await.spec.ts b/test/unit/builtins/async-await.spec.ts index 90d319d00..e210cf9c2 100644 --- a/test/unit/builtins/async-await.spec.ts +++ b/test/unit/builtins/async-await.spec.ts @@ -161,6 +161,19 @@ test.each(["async function abc() {", "const abc = async () => {"])( } ); +test("can make inline async functions", () => { + util.testFunction` + const foo = async function() { return "foo"; }; + const bar = async function() { return await foo(); }; + + const { state, value } = bar() as any; + return { state, value }; + `.debug().expectToEqual({ + state: 1, // __TS__PromiseState.Fulfilled + value: "foo", + }); +}); + test("can make async lambdas with expression body", () => { util.testFunction` const foo = async () => "foo"; @@ -168,7 +181,7 @@ test("can make async lambdas with expression body", () => { const { state, value } = bar() as any; return { state, value }; - `.expectToEqual({ + `.debug().expectToEqual({ state: 1, // __TS__PromiseState.Fulfilled value: "foo", }); @@ -403,4 +416,23 @@ describe("try/catch in async function", () => { }); `.expectToEqual({ reason: "an error occurred in the async function: test error" }); }); + + test("await inside try/catch deferred rejection uses catch clause", () => { + util.testModule` + export let reason = ""; + let reject: (reason: string) => void; + + async function foo(): Promise { + try { + return await new Promise((res, rej) => { reject = rej; }); + } catch (e) { + throw "an error occurred in the async function: " + e; + } + } + foo().catch(e => { + reason = e; + }); + reject("test error"); + `.expectToEqual({ reason: "an error occurred in the async function: test error" }); + }); }); From 9bb9bea3a49418aa9a7d34179fb2c704ee8f35fe Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 11 Oct 2021 22:08:30 +0200 Subject: [PATCH 3/9] Also fix lambas in async --- src/transformation/visitors/return.ts | 2 +- test/unit/builtins/async-await.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/transformation/visitors/return.ts b/src/transformation/visitors/return.ts index a0b60264b..de2df655e 100644 --- a/src/transformation/visitors/return.ts +++ b/src/transformation/visitors/return.ts @@ -56,7 +56,7 @@ export function transformExpressionBodyToReturnStatement( node: ts.Expression ): lua.Statement { const expressions = transformExpressionsInReturn(context, node, false); - return lua.createReturnStatement(expressions, node); + return createReturnStatement(context, expressions, node); } export const transformReturnStatement: FunctionVisitor = (statement, context) => { diff --git a/test/unit/builtins/async-await.spec.ts b/test/unit/builtins/async-await.spec.ts index e210cf9c2..668551bd0 100644 --- a/test/unit/builtins/async-await.spec.ts +++ b/test/unit/builtins/async-await.spec.ts @@ -168,7 +168,7 @@ test("can make inline async functions", () => { const { state, value } = bar() as any; return { state, value }; - `.debug().expectToEqual({ + `.expectToEqual({ state: 1, // __TS__PromiseState.Fulfilled value: "foo", }); @@ -181,7 +181,7 @@ test("can make async lambdas with expression body", () => { const { state, value } = bar() as any; return { state, value }; - `.debug().expectToEqual({ + `.expectToEqual({ state: 1, // __TS__PromiseState.Fulfilled value: "foo", }); From 86f7ce7f6bf22e146fca2cd706b06fafafe98d66 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 11 Oct 2021 22:18:32 +0200 Subject: [PATCH 4/9] Fix bug in try/catch adding extra return twice --- src/transformation/visitors/errors.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/transformation/visitors/errors.ts b/src/transformation/visitors/errors.ts index b68ec317b..05601d8c8 100644 --- a/src/transformation/visitors/errors.ts +++ b/src/transformation/visitors/errors.ts @@ -2,7 +2,7 @@ import * as ts from "typescript"; import * as lua from "../../LuaAST"; import { FunctionVisitor } from "../context"; import { createUnpackCall } from "../utils/lua-ast"; -import { findScope, ScopeType } from "../utils/scope"; +import { ScopeType } from "../utils/scope"; import { transformScopeBlock } from "./block"; import { transformIdentifier } from "./identifier"; import { isInMultiReturnFunction } from "./language-extensions/multi"; @@ -26,8 +26,10 @@ export const transformTryStatement: FunctionVisitor = (statemen // try with catch const [catchBlock, catchScope] = transformScopeBlock(context, statement.catchClause.block, ScopeType.Catch); - const catchParameter = statement.catchClause.variableDeclaration ? transformIdentifier(context, statement.catchClause.variableDeclaration.name as ts.Identifier) : undefined; - const catchParameters = () => catchParameter ? [lua.cloneIdentifier(catchParameter)] : []; + const catchParameter = statement.catchClause.variableDeclaration + ? transformIdentifier(context, statement.catchClause.variableDeclaration.name as ts.Identifier) + : undefined; + const catchParameters = () => (catchParameter ? [lua.cloneIdentifier(catchParameter)] : []); const catchIdentifier = lua.createIdentifier("____catch"); const catchFunction = lua.createFunctionExpression(catchBlock, catchParameters()); @@ -74,16 +76,7 @@ export const transformTryStatement: FunctionVisitor = (statemen } if (returnCondition && returnedIdentifier) { - // With catch clause: - // if ____returned then return ____returnValue end - // No catch clause: - // if ____try and ____returned then return ____returnValue end const returnValues: lua.Expression[] = []; - const parentTryCatch = findScope(context, ScopeType.Function | ScopeType.Try | ScopeType.Catch); - if (parentTryCatch && parentTryCatch.type !== ScopeType.Function) { - // Nested try/catch needs to prefix a 'true' return value - returnValues.push(lua.createBooleanLiteral(true)); - } if (isInMultiReturnFunction(context, statement)) { returnValues.push(createUnpackCall(context, lua.cloneIdentifier(returnValueIdentifier))); From 29666e52958c22a2f06ebc821b6dd40486fc5c0f Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 11 Oct 2021 22:20:27 +0200 Subject: [PATCH 5/9] Fix prettier --- src/lualib/Await.ts | 2 +- src/transformation/visitors/return.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/lualib/Await.ts b/src/lualib/Await.ts index c0bab3c45..0bbfdd09f 100644 --- a/src/lualib/Await.ts +++ b/src/lualib/Await.ts @@ -46,7 +46,7 @@ function __TS__AsyncAwaiter(this: void, generator: (this: void) => void) { } else { // If no catch clause, just reject return value => { - reject(value) + reject(value); }; } } diff --git a/src/transformation/visitors/return.ts b/src/transformation/visitors/return.ts index de2df655e..6a5b1bbb8 100644 --- a/src/transformation/visitors/return.ts +++ b/src/transformation/visitors/return.ts @@ -60,7 +60,6 @@ export function transformExpressionBodyToReturnStatement( } export const transformReturnStatement: FunctionVisitor = (statement, context) => { - let results: lua.Expression[]; if (statement.expression) { @@ -79,7 +78,11 @@ export const transformReturnStatement: FunctionVisitor = (st return createReturnStatement(context, results, statement); }; -export function createReturnStatement(context: TransformationContext, values: lua.Expression[], node: ts.Node): lua.ReturnStatement { +export function createReturnStatement( + context: TransformationContext, + values: lua.Expression[], + node: ts.Node +): lua.ReturnStatement { const results = [...values]; if (isInTryCatch(context)) { From 3084fa729f0833ac05114e61425bbf3ab712fa3f Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sun, 19 Sep 2021 22:45:32 +0200 Subject: [PATCH 6/9] Add failing tests --- test/unit/builtins/async-await.spec.ts | 40 +++++++++++++++++--------- test/unit/functions/generators.spec.ts | 16 +++++++++++ test/util.ts | 11 +++++++ 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/test/unit/builtins/async-await.spec.ts b/test/unit/builtins/async-await.spec.ts index 668551bd0..9c9e97dac 100644 --- a/test/unit/builtins/async-await.spec.ts +++ b/test/unit/builtins/async-await.spec.ts @@ -385,8 +385,9 @@ test("async function can forward varargs", () => { // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1105 describe("try/catch in async function", () => { - test("await inside try/catch returns inside async function", () => { - util.testModule` + util.testEachVersion( + "await inside try/catch returns inside async function", + () => util.testModule` export let result = 0; async function foo(): Promise { try { @@ -398,11 +399,14 @@ describe("try/catch in async function", () => { foo().then(value => { result = value; }); - `.expectToEqual({ result: 4 }); - }); - - test("await inside try/catch throws inside async function", () => { - util.testModule` + `, + // Cannot execute LuaJIT with test runner + util.expectEachVersionExceptJit(builder => builder.expectToEqual({ result: 4 })) + ); + + util.testEachVersion( + "await inside try/catch throws inside async function", + () => util.testModule` export let reason = ""; async function foo(): Promise { try { @@ -414,11 +418,16 @@ describe("try/catch in async function", () => { foo().catch(e => { reason = e; }); - `.expectToEqual({ reason: "an error occurred in the async function: test error" }); - }); - - test("await inside try/catch deferred rejection uses catch clause", () => { - util.testModule` + `, + util.expectEachVersionExceptJit(builder => + builder.expectToEqual({ reason: "an error occurred in the async function: test error" }) + ) + ); + + util.testEachVersion( + "await inside try/catch deferred rejection uses catch clause", + () => + util.testModule` export let reason = ""; let reject: (reason: string) => void; @@ -433,6 +442,9 @@ describe("try/catch in async function", () => { reason = e; }); reject("test error"); - `.expectToEqual({ reason: "an error occurred in the async function: test error" }); - }); + `, + util.expectEachVersionExceptJit(builder => + builder.expectToEqual({ reason: "an error occurred in the async function: test error" }) + ) + ); }); diff --git a/test/unit/functions/generators.spec.ts b/test/unit/functions/generators.spec.ts index 7065aecf6..f1ef23349 100644 --- a/test/unit/functions/generators.spec.ts +++ b/test/unit/functions/generators.spec.ts @@ -147,3 +147,19 @@ test("hoisting", () => { } `.expectToMatchJsResult(); }); + +util.testEachVersion( + "generator yield inside try/catch", + () => util.testFunction` + function* generator() { + try { + yield 4; + } catch { + throw "something went wrong"; + } + } + return generator().next(); + `, + // Cannot execute LuaJIT with test runner + util.expectEachVersionExceptJit(builder => builder.expectToMatchJsResult()) +); diff --git a/test/util.ts b/test/util.ts index 7752496a6..37da6716e 100644 --- a/test/util.ts +++ b/test/util.ts @@ -65,6 +65,17 @@ export function testEachVersion( } } +export function expectEachVersionExceptJit(expectation: (builder: T) => void): Record void) | boolean> { + return { + [tstl.LuaTarget.Universal]: expectation, + [tstl.LuaTarget.Lua51]: expectation, + [tstl.LuaTarget.Lua52]: expectation, + [tstl.LuaTarget.Lua53]: expectation, + [tstl.LuaTarget.Lua54]: expectation, + [tstl.LuaTarget.LuaJIT]: false, // Exclude JIT + } +} + const memoize: MethodDecorator = (_target, _propertyKey, descriptor) => { const originalFunction = descriptor.value as any; const memoized = new WeakMap(); From 93ef06c0f7bc9c6b4eabd6485330cc842670fdf4 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Fri, 22 Oct 2021 20:49:51 +0200 Subject: [PATCH 7/9] Add diagnostic to prevent 5.1 users from trying to use try/catch in async function --- src/transformation/utils/typescript/nodes.ts | 11 +++++++++ src/transformation/visitors/errors.ts | 8 ++++++ src/transformation/visitors/return.ts | 12 +-------- test/unit/builtins/async-await.spec.ts | 26 ++++++++++++++------ test/util.ts | 6 +++-- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/transformation/utils/typescript/nodes.ts b/src/transformation/utils/typescript/nodes.ts index 3007c4d7d..8e1fd6405 100644 --- a/src/transformation/utils/typescript/nodes.ts +++ b/src/transformation/utils/typescript/nodes.ts @@ -1,4 +1,5 @@ import * as ts from "typescript"; +import { findFirstNodeAbove } from "."; import { TransformationContext } from "../../context"; export function isAssignmentPattern(node: ts.Node): node is ts.AssignmentPattern { @@ -25,6 +26,16 @@ export function isInDestructingAssignment(node: ts.Node): boolean { ); } +export function isInAsyncFunction(node: ts.Node): boolean { + // Check if node is in function declaration with `async` + const declaration = findFirstNodeAbove(node, ts.isFunctionLike); + if (!declaration) { + return false; + } + + return declaration.modifiers?.some(m => m.kind === ts.SyntaxKind.AsyncKeyword) ?? false; +} + /** * Quite hacky, avoid unless absolutely necessary! */ diff --git a/src/transformation/visitors/errors.ts b/src/transformation/visitors/errors.ts index 05601d8c8..49602bcab 100644 --- a/src/transformation/visitors/errors.ts +++ b/src/transformation/visitors/errors.ts @@ -1,8 +1,11 @@ import * as ts from "typescript"; +import { LuaTarget } from "../.."; import * as lua from "../../LuaAST"; import { FunctionVisitor } from "../context"; +import { unsupportedForTarget } from "../utils/diagnostics"; import { createUnpackCall } from "../utils/lua-ast"; import { ScopeType } from "../utils/scope"; +import { isInAsyncFunction } from "../utils/typescript"; import { transformScopeBlock } from "./block"; import { transformIdentifier } from "./identifier"; import { isInMultiReturnFunction } from "./language-extensions/multi"; @@ -11,6 +14,11 @@ import { createReturnStatement } from "./return"; export const transformTryStatement: FunctionVisitor = (statement, context) => { const [tryBlock, tryScope] = transformScopeBlock(context, statement.tryBlock, ScopeType.Try); + if (context.options.luaTarget === LuaTarget.Lua51 && isInAsyncFunction(statement)) { + context.diagnostics.push(unsupportedForTarget(statement, "try/catch inside async functions", LuaTarget.Lua51)); + return tryBlock.statements; + } + const tryResultIdentifier = lua.createIdentifier("____try"); const returnValueIdentifier = lua.createIdentifier("____returnValue"); diff --git a/src/transformation/visitors/return.ts b/src/transformation/visitors/return.ts index 6a5b1bbb8..1bd1bb0fe 100644 --- a/src/transformation/visitors/return.ts +++ b/src/transformation/visitors/return.ts @@ -14,7 +14,7 @@ import { canBeMultiReturnType, } from "./language-extensions/multi"; import { invalidMultiFunctionReturnType } from "../utils/diagnostics"; -import { findFirstNodeAbove } from "../utils/typescript"; +import { isInAsyncFunction } from "../utils/typescript"; function transformExpressionsInReturn( context: TransformationContext, @@ -96,16 +96,6 @@ export function createReturnStatement( return lua.createReturnStatement(results, node); } -function isInAsyncFunction(node: ts.Node): boolean { - // Check if node is in function declaration with `async` - const declaration = findFirstNodeAbove(node, ts.isFunctionLike); - if (!declaration) { - return false; - } - - return declaration.modifiers?.some(m => m.kind === ts.SyntaxKind.AsyncKeyword) ?? false; -} - function isInTryCatch(context: TransformationContext): boolean { // Check if context is in a try or catch let insideTryCatch = false; diff --git a/test/unit/builtins/async-await.spec.ts b/test/unit/builtins/async-await.spec.ts index 9c9e97dac..c08514236 100644 --- a/test/unit/builtins/async-await.spec.ts +++ b/test/unit/builtins/async-await.spec.ts @@ -1,5 +1,6 @@ import { ModuleKind, ScriptTarget } from "typescript"; -import { awaitMustBeInAsyncFunction } from "../../../src/transformation/utils/diagnostics"; +import { LuaTarget } from "../../../src"; +import { awaitMustBeInAsyncFunction, unsupportedForTarget } from "../../../src/transformation/utils/diagnostics"; import * as util from "../../util"; const promiseTestLib = ` @@ -401,7 +402,10 @@ describe("try/catch in async function", () => { }); `, // Cannot execute LuaJIT with test runner - util.expectEachVersionExceptJit(builder => builder.expectToEqual({ result: 4 })) + { + ...util.expectEachVersionExceptJit(builder => builder.expectToEqual({ result: 4 })), + [LuaTarget.Lua51]: builder => builder.expectToHaveDiagnostics([unsupportedForTarget.code]), + } ); util.testEachVersion( @@ -419,9 +423,12 @@ describe("try/catch in async function", () => { reason = e; }); `, - util.expectEachVersionExceptJit(builder => - builder.expectToEqual({ reason: "an error occurred in the async function: test error" }) - ) + { + ...util.expectEachVersionExceptJit(builder => + builder.expectToEqual({ reason: "an error occurred in the async function: test error" }) + ), + [LuaTarget.Lua51]: builder => builder.expectToHaveDiagnostics([unsupportedForTarget.code]), + } ); util.testEachVersion( @@ -443,8 +450,11 @@ describe("try/catch in async function", () => { }); reject("test error"); `, - util.expectEachVersionExceptJit(builder => - builder.expectToEqual({ reason: "an error occurred in the async function: test error" }) - ) + { + ...util.expectEachVersionExceptJit(builder => + builder.expectToEqual({ reason: "an error occurred in the async function: test error" }) + ), + [LuaTarget.Lua51]: builder => builder.expectToHaveDiagnostics([unsupportedForTarget.code]), + } ); }); diff --git a/test/util.ts b/test/util.ts index 37da6716e..7d5ea6b95 100644 --- a/test/util.ts +++ b/test/util.ts @@ -65,7 +65,9 @@ export function testEachVersion( } } -export function expectEachVersionExceptJit(expectation: (builder: T) => void): Record void) | boolean> { +export function expectEachVersionExceptJit( + expectation: (builder: T) => void +): Record void) | boolean> { return { [tstl.LuaTarget.Universal]: expectation, [tstl.LuaTarget.Lua51]: expectation, @@ -73,7 +75,7 @@ export function expectEachVersionExceptJit(expectation: (builder: T) => void) [tstl.LuaTarget.Lua53]: expectation, [tstl.LuaTarget.Lua54]: expectation, [tstl.LuaTarget.LuaJIT]: false, // Exclude JIT - } + }; } const memoize: MethodDecorator = (_target, _propertyKey, descriptor) => { From e5dde956c1bef59c722560b0309f288a73cf7f52 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sat, 23 Oct 2021 20:57:18 +0200 Subject: [PATCH 8/9] Diagnostic for try/catch in generator or async function in 5.1 --- src/transformation/utils/typescript/nodes.ts | 10 ++++++++++ src/transformation/visitors/errors.ts | 9 ++++++++- test/unit/functions/generators.spec.ts | 7 ++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/transformation/utils/typescript/nodes.ts b/src/transformation/utils/typescript/nodes.ts index 8e1fd6405..ef6cec08f 100644 --- a/src/transformation/utils/typescript/nodes.ts +++ b/src/transformation/utils/typescript/nodes.ts @@ -36,6 +36,16 @@ export function isInAsyncFunction(node: ts.Node): boolean { return declaration.modifiers?.some(m => m.kind === ts.SyntaxKind.AsyncKeyword) ?? false; } +export function isInGeneratorFunction(node: ts.Node): boolean { + // Check if node is in function declaration with `async` + const declaration = findFirstNodeAbove(node, ts.isFunctionDeclaration); + if (!declaration) { + return false; + } + + return declaration.asteriskToken !== undefined; +} + /** * Quite hacky, avoid unless absolutely necessary! */ diff --git a/src/transformation/visitors/errors.ts b/src/transformation/visitors/errors.ts index 49602bcab..2b58428b2 100644 --- a/src/transformation/visitors/errors.ts +++ b/src/transformation/visitors/errors.ts @@ -5,7 +5,7 @@ import { FunctionVisitor } from "../context"; import { unsupportedForTarget } from "../utils/diagnostics"; import { createUnpackCall } from "../utils/lua-ast"; import { ScopeType } from "../utils/scope"; -import { isInAsyncFunction } from "../utils/typescript"; +import { isInAsyncFunction, isInGeneratorFunction } from "../utils/typescript"; import { transformScopeBlock } from "./block"; import { transformIdentifier } from "./identifier"; import { isInMultiReturnFunction } from "./language-extensions/multi"; @@ -19,6 +19,13 @@ export const transformTryStatement: FunctionVisitor = (statemen return tryBlock.statements; } + if (context.options.luaTarget === LuaTarget.Lua51 && isInGeneratorFunction(statement)) { + context.diagnostics.push( + unsupportedForTarget(statement, "try/catch inside generator functions", LuaTarget.Lua51) + ); + return tryBlock.statements; + } + const tryResultIdentifier = lua.createIdentifier("____try"); const returnValueIdentifier = lua.createIdentifier("____returnValue"); diff --git a/test/unit/functions/generators.spec.ts b/test/unit/functions/generators.spec.ts index f1ef23349..03b7f3bfd 100644 --- a/test/unit/functions/generators.spec.ts +++ b/test/unit/functions/generators.spec.ts @@ -1,3 +1,5 @@ +import { LuaTarget } from "../../../src/CompilerOptions"; +import { unsupportedForTarget } from "../../../src/transformation/utils/diagnostics"; import * as util from "../../util"; test("generator parameters", () => { @@ -161,5 +163,8 @@ util.testEachVersion( return generator().next(); `, // Cannot execute LuaJIT with test runner - util.expectEachVersionExceptJit(builder => builder.expectToMatchJsResult()) + { + ...util.expectEachVersionExceptJit(builder => builder.expectToMatchJsResult()), + [LuaTarget.Lua51]: builder => builder.expectToHaveDiagnostics([unsupportedForTarget.code]), + } ); From a75f9be6562e82f7b76b6596b84d41ca25fc7ad8 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 25 Oct 2021 21:54:35 +0200 Subject: [PATCH 9/9] Fix missing case in promise then --- src/lualib/Promise.ts | 21 ++++++++++++--- test/unit/builtins/promise.spec.ts | 42 ++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/lualib/Promise.ts b/src/lualib/Promise.ts index c8fdea117..76f119c38 100644 --- a/src/lualib/Promise.ts +++ b/src/lualib/Promise.ts @@ -70,13 +70,16 @@ class __TS__Promise implements Promise { onFulfilled?: FulfillCallback, onRejected?: RejectCallback ): Promise { - const { promise, resolve, reject } = __TS__PromiseDeferred(); + const { promise, resolve, reject } = __TS__PromiseDeferred(); + + const isFulfilled = this.state === __TS__PromiseState.Fulfilled; + const isRejected = this.state === __TS__PromiseState.Rejected; if (onFulfilled) { const internalCallback = this.createPromiseResolvingCallback(onFulfilled, resolve, reject); this.fulfilledCallbacks.push(internalCallback); - if (this.state === __TS__PromiseState.Fulfilled) { + if (isFulfilled) { // If promise already resolved, immediately call callback internalCallback(this.value); } @@ -89,13 +92,23 @@ class __TS__Promise implements Promise { const internalCallback = this.createPromiseResolvingCallback(onRejected, resolve, reject); this.rejectedCallbacks.push(internalCallback); - if (this.state === __TS__PromiseState.Rejected) { + if (isRejected) { // If promise already rejected, immediately call callback internalCallback(this.rejectionReason); } } - return promise; + if (isFulfilled) { + // If promise already resolved, also resolve returned promise + resolve(this.value); + } + + if (isRejected) { + // If promise already rejected, also reject returned promise + reject(this.rejectionReason); + } + + return promise as Promise; } // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch public catch(onRejected?: (reason: any) => TResult | PromiseLike): Promise { diff --git a/test/unit/builtins/promise.spec.ts b/test/unit/builtins/promise.spec.ts index 4a22f7514..60e30dcf1 100644 --- a/test/unit/builtins/promise.spec.ts +++ b/test/unit/builtins/promise.spec.ts @@ -709,6 +709,48 @@ test("promise is instanceof promise", () => { util.testExpression`Promise.resolve(4) instanceof Promise`.expectToMatchJsResult(); }); +test("chained then on resolved promise", () => { + util.testFunction` + Promise.resolve("result1").then(undefined, () => {}).then(value => log(value)); + Promise.resolve("result2").then(value => "then1", () => {}).then(value => log(value)); + Promise.resolve("result3").then(value => undefined, () => {}).then(value => log(value ?? "undefined")); + Promise.resolve("result4").then(value => "then2").then(value => [value, "then3"]).then(([v1, v2]) => log(v1, v2)); + + return allLogs; + ` + .setTsHeader(promiseTestLib) + .expectToEqual(["result1", "then1", "undefined", "then2", "then3"]); +}); + +test("chained catch on rejected promise", () => { + util.testFunction` + Promise.reject("reason1").then(() => {}).then(v => log("resolved", v), reason => log("rejected", reason)); + Promise.reject("reason2").then(() => {}, () => "reason3").then(v => log("resolved", v)); + Promise.reject("reason4").then(() => {}, () => undefined).then(v => log("resolved", v ?? "undefined")); + + return allLogs; + ` + .setTsHeader(promiseTestLib) + .expectToEqual(["rejected", "reason1", "resolved", "reason3", "resolved", "undefined"]); +}); + +// Issue 2 from https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1105 +test("catch after then catches rejected promise", () => { + util.testFunction` + Promise.reject('test error') + .then(result => { + log("then", result); + }) + .catch(e => { + log("catch", e); + }) + + return allLogs; + ` + .setTsHeader(promiseTestLib) + .expectToEqual(["catch", "test error"]); +}); + describe("Promise.all", () => { test("resolves once all arguments are resolved", () => { util.testFunction`