Skip to content

Commit c5a28fc

Browse files
a-tarasyuka-tarasyuk
authored
fix(39589): add await before return promise expression (microsoft#39649)
1 parent d93590e commit c5a28fc

13 files changed

+228
-23
lines changed

‎src/services/codefixes/convertToAsyncFunction.ts‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -302,18 +302,15 @@ namespace ts.codefix {
302302
const [onFulfilled, onRejected] = node.arguments;
303303
const onFulfilledArgumentName = getArgBindingName(onFulfilled, transformer);
304304
const transformationBody = getTransformationBody(onFulfilled, prevArgName, onFulfilledArgumentName, node, transformer);
305-
306305
if (onRejected) {
307306
const onRejectedArgumentName = getArgBindingName(onRejected, transformer);
308307
const tryBlock = factory.createBlock(transformExpression(node.expression, transformer, onFulfilledArgumentName).concat(transformationBody));
309308
const transformationBody2 = getTransformationBody(onRejected, prevArgName, onRejectedArgumentName, node, transformer);
310309
const catchArg = onRejectedArgumentName ? isSynthIdentifier(onRejectedArgumentName) ? onRejectedArgumentName.identifier.text : onRejectedArgumentName.bindingPattern : "e";
311310
const catchVariableDeclaration = factory.createVariableDeclaration(catchArg);
312311
const catchClause = factory.createCatchClause(catchVariableDeclaration, factory.createBlock(transformationBody2));
313-
314312
return [factory.createTryStatement(tryBlock, catchClause, /* finallyBlock */ undefined)];
315313
}
316-
317314
return transformExpression(node.expression, transformer, onFulfilledArgumentName).concat(transformationBody);
318315
}
319316

@@ -395,19 +392,21 @@ namespace ts.codefix {
395392
case SyntaxKind.FunctionExpression:
396393
case SyntaxKind.ArrowFunction: {
397394
const funcBody = (func as FunctionExpression | ArrowFunction).body;
395+
const returnType = getLastCallSignature(transformer.checker.getTypeAtLocation(func), transformer.checker)?.getReturnType();
396+
398397
// Arrow functions with block bodies { } will enter this control flow
399398
if (isBlock(funcBody)) {
400399
let refactoredStmts: Statement[] = [];
401400
let seenReturnStatement = false;
402-
403401
for (const statement of funcBody.statements) {
404402
if (isReturnStatement(statement)) {
405403
seenReturnStatement = true;
406404
if (isReturnStatementWithFixablePromiseHandler(statement)) {
407405
refactoredStmts = refactoredStmts.concat(getInnerTransformationBody(transformer, [statement], prevArgName));
408406
}
409407
else {
410-
refactoredStmts.push(...maybeAnnotateAndReturn(statement.expression, parent.typeArguments?.[0]));
408+
const possiblyAwaitedRightHandSide = returnType && statement.expression ? getPossiblyAwaitedRightHandSide(transformer.checker, returnType, statement.expression) : statement.expression;
409+
refactoredStmts.push(...maybeAnnotateAndReturn(possiblyAwaitedRightHandSide, parent.typeArguments?.[0]));
411410
}
412411
}
413412
else {
@@ -431,19 +430,21 @@ namespace ts.codefix {
431430
return innerCbBody;
432431
}
433432

434-
const type = transformer.checker.getTypeAtLocation(func);
435-
const returnType = getLastCallSignature(type, transformer.checker)!.getReturnType();
436-
const rightHandSide = getSynthesizedDeepClone(funcBody);
437-
const possiblyAwaitedRightHandSide = !!transformer.checker.getPromisedTypeOfPromise(returnType) ? factory.createAwaitExpression(rightHandSide) : rightHandSide;
438-
if (!shouldReturn(parent, transformer)) {
439-
const transformedStatement = createVariableOrAssignmentOrExpressionStatement(prevArgName, possiblyAwaitedRightHandSide, /*typeAnnotation*/ undefined);
440-
if (prevArgName) {
441-
prevArgName.types.push(returnType);
433+
if (returnType) {
434+
const possiblyAwaitedRightHandSide = getPossiblyAwaitedRightHandSide(transformer.checker, returnType, funcBody);
435+
if (!shouldReturn(parent, transformer)) {
436+
const transformedStatement = createVariableOrAssignmentOrExpressionStatement(prevArgName, possiblyAwaitedRightHandSide, /*typeAnnotation*/ undefined);
437+
if (prevArgName) {
438+
prevArgName.types.push(returnType);
439+
}
440+
return transformedStatement;
441+
}
442+
else {
443+
return maybeAnnotateAndReturn(possiblyAwaitedRightHandSide, parent.typeArguments?.[0]);
442444
}
443-
return transformedStatement;
444445
}
445446
else {
446-
return maybeAnnotateAndReturn(possiblyAwaitedRightHandSide, parent.typeArguments?.[0]);
447+
return silentFail();
447448
}
448449
}
449450
}
@@ -454,12 +455,16 @@ namespace ts.codefix {
454455
return emptyArray;
455456
}
456457

458+
function getPossiblyAwaitedRightHandSide(checker: TypeChecker, type: Type, expr: Expression): AwaitExpression | Expression {
459+
const rightHandSide = getSynthesizedDeepClone(expr);
460+
return !!checker.getPromisedTypeOfPromise(type) ? factory.createAwaitExpression(rightHandSide) : rightHandSide;
461+
}
462+
457463
function getLastCallSignature(type: Type, checker: TypeChecker): Signature | undefined {
458464
const callSignatures = checker.getSignaturesOfType(type, SignatureKind.Call);
459465
return lastOrUndefined(callSignatures);
460466
}
461467

462-
463468
function removeReturns(stmts: readonly Statement[], prevArgName: SynthBindingName | undefined, transformer: Transformer, seenReturnStatement: boolean): readonly Statement[] {
464469
const ret: Statement[] = [];
465470
for (const stmt of stmts) {

‎src/testRunner/unittests/services/convertToAsyncFunction.ts‎

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,7 @@ function [#|f|](): Promise<string> {
902902
}
903903
`
904904
);
905-
_testConvertToAsyncFunction("convertToAsyncFunction_PromiseAllAndThen", `
905+
_testConvertToAsyncFunction("convertToAsyncFunction_PromiseAllAndThen1", `
906906
function [#|f|]() {
907907
return Promise.resolve().then(function () {
908908
return Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function () {
@@ -921,6 +921,26 @@ function [#|f|]() {
921921
})]).then(res => res.toString());
922922
});
923923
}
924+
`
925+
);
926+
927+
_testConvertToAsyncFunction("convertToAsyncFunction_PromiseAllAndThen3", `
928+
function [#|f|]() {
929+
return Promise.resolve().then(() =>
930+
Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function () {
931+
return fetch("https://github.com");
932+
}).then(res => res.toString())]));
933+
}
934+
`
935+
);
936+
937+
_testConvertToAsyncFunction("convertToAsyncFunction_PromiseAllAndThen4", `
938+
function [#|f|]() {
939+
return Promise.resolve().then(() =>
940+
Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function () {
941+
return fetch("https://github.com");
942+
})]).then(res => res.toString()));
943+
}
924944
`
925945
);
926946
_testConvertToAsyncFunction("convertToAsyncFunction_Scope1", `
@@ -1124,6 +1144,27 @@ function [#|bar|]<T>(x: T): Promise<T> {
11241144
`
11251145
);
11261146

1147+
_testConvertToAsyncFunction("convertToAsyncFunction_Return1", `
1148+
function [#|f|](p: Promise<unknown>) {
1149+
return p.catch((error: Error) => {
1150+
return Promise.reject(error);
1151+
});
1152+
}`
1153+
);
1154+
1155+
_testConvertToAsyncFunction("convertToAsyncFunction_Return2", `
1156+
function [#|f|](p: Promise<unknown>) {
1157+
return p.catch((error: Error) => Promise.reject(error));
1158+
}`
1159+
);
1160+
1161+
_testConvertToAsyncFunction("convertToAsyncFunction_Return3", `
1162+
function [#|f|](p: Promise<unknown>) {
1163+
return p.catch(function (error: Error) {
1164+
return Promise.reject(error);
1165+
});
1166+
}`
1167+
);
11271168

11281169
_testConvertToAsyncFunction("convertToAsyncFunction_LocalReturn", `
11291170
function [#|f|]() {
@@ -1352,7 +1393,7 @@ function [#|f|]() {
13521393
}
13531394
`);
13541395

1355-
_testConvertToAsyncFunction("convertToAsyncFunction_noArgs", `
1396+
_testConvertToAsyncFunction("convertToAsyncFunction_noArgs1", `
13561397
function delay(millis: number): Promise<void> {
13571398
throw "no"
13581399
}
@@ -1364,7 +1405,21 @@ function [#|main2|]() {
13641405
.then(() => { console.log("."); return delay(500); })
13651406
.then(() => { console.log("."); return delay(500); })
13661407
}
1367-
`);
1408+
`);
1409+
1410+
_testConvertToAsyncFunction("convertToAsyncFunction_noArgs2", `
1411+
function delay(millis: number): Promise<void> {
1412+
throw "no"
1413+
}
1414+
1415+
function [#|main2|]() {
1416+
console.log("Please wait. Loading.");
1417+
return delay(500)
1418+
.then(() => delay(500))
1419+
.then(() => delay(500))
1420+
.then(() => delay(500))
1421+
}
1422+
`);
13681423

13691424
_testConvertToAsyncFunction("convertToAsyncFunction_exportModifier", `
13701425
export function [#|foo|]() {

tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_PromiseAllAndThen.js renamed to tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_PromiseAllAndThen1.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ function /*[#|*/f/*|]*/() {
1212

1313
async function f() {
1414
await Promise.resolve();
15-
return Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function() {
15+
return await Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function() {
1616
return fetch("https://github.com");
1717
}).then(res => res.toString())]);
1818
}

tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_PromiseAllAndThen.ts renamed to tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_PromiseAllAndThen1.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ function /*[#|*/f/*|]*/() {
1212

1313
async function f() {
1414
await Promise.resolve();
15-
return Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function() {
15+
return await Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function() {
1616
return fetch("https://github.com");
1717
}).then(res => res.toString())]);
1818
}

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_PromiseAllAndThen3.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/f/*|]*/() {
4+
return Promise.resolve().then(() =>
5+
Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function () {
6+
return fetch("https://github.com");
7+
}).then(res => res.toString())]));
8+
}
9+
10+
// ==ASYNC FUNCTION::Convert to async function==
11+
12+
async function f() {
13+
await Promise.resolve();
14+
return await Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function() {
15+
return fetch("https://github.com");
16+
}).then(res => res.toString())]);
17+
}

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_PromiseAllAndThen3.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/f/*|]*/() {
4+
return Promise.resolve().then(() =>
5+
Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function () {
6+
return fetch("https://github.com");
7+
}).then(res => res.toString())]));
8+
}
9+
10+
// ==ASYNC FUNCTION::Convert to async function==
11+
12+
async function f() {
13+
await Promise.resolve();
14+
return await Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function() {
15+
return fetch("https://github.com");
16+
}).then(res => res.toString())]);
17+
}

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_PromiseAllAndThen4.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/f/*|]*/() {
4+
return Promise.resolve().then(() =>
5+
Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function () {
6+
return fetch("https://github.com");
7+
})]).then(res => res.toString()));
8+
}
9+
10+
// ==ASYNC FUNCTION::Convert to async function==
11+
12+
async function f() {
13+
await Promise.resolve();
14+
const res = await Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function() {
15+
return fetch("https://github.com");
16+
})]);
17+
return res.toString();
18+
}

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_PromiseAllAndThen4.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/f/*|]*/() {
4+
return Promise.resolve().then(() =>
5+
Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function () {
6+
return fetch("https://github.com");
7+
})]).then(res => res.toString()));
8+
}
9+
10+
// ==ASYNC FUNCTION::Convert to async function==
11+
12+
async function f() {
13+
await Promise.resolve();
14+
const res = await Promise.all([fetch("https://typescriptlang.org"), fetch("https://microsoft.com"), Promise.resolve().then(function() {
15+
return fetch("https://github.com");
16+
})]);
17+
return res.toString();
18+
}

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Return1.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/f/*|]*/(p: Promise<unknown>) {
4+
return p.catch((error: Error) => {
5+
return Promise.reject(error);
6+
});
7+
}
8+
// ==ASYNC FUNCTION::Convert to async function==
9+
10+
async function f(p: Promise<unknown>) {
11+
try {
12+
return p;
13+
} catch (error) {
14+
return await Promise.reject(error);
15+
}
16+
}

‎tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Return2.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/f/*|]*/(p: Promise<unknown>) {
4+
return p.catch((error: Error) => Promise.reject(error));
5+
}
6+
// ==ASYNC FUNCTION::Convert to async function==
7+
8+
async function f(p: Promise<unknown>) {
9+
try {
10+
return p;
11+
} catch (error) {
12+
return await Promise.reject(error);
13+
}
14+
}

0 commit comments

Comments
 (0)