Skip to content

Commit 37569d0

Browse files
andrewbranchandrewbranch
authored
Convert to async function: handle type arguments to then/catch (#37463)
* Handle type arguments to then/catch * Keep single-line types on a single line
1 parent 9f296ce commit 37569d0

File tree

8 files changed

+206
-27
lines changed

8 files changed

+206
-27
lines changed

‎src/compiler/emitter.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4296,7 +4296,7 @@ namespace ts {
42964296
// JsxText will be written with its leading whitespace, so don't add more manually.
42974297
return 0;
42984298
}
4299-
if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) {
4299+
if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && (!firstChild.parent || firstChild.parent === parentNode)) {
43004300
if (preserveSourceNewlines) {
43014301
return getEffectiveLines(
43024302
includeComments => getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(
@@ -4353,7 +4353,7 @@ namespace ts {
43534353
if (lastChild === undefined) {
43544354
return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1;
43554355
}
4356-
if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild) && lastChild.parent === parentNode) {
4356+
if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild) && (!lastChild.parent || lastChild.parent === parentNode)) {
43574357
if (preserveSourceNewlines) {
43584358
return getEffectiveLines(
43594359
includeComments => getLinesBetweenPositionAndNextNonWhitespaceCharacter(

‎src/services/codefixes/convertToAsyncFunction.ts‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -336,17 +336,17 @@ namespace ts.codefix {
336336
}
337337

338338
/**
339-
* Transforms the 'x' part of `x.then(...)`, or the 'y()' part of `y.catch(...)`, where 'x' and 'y()' are Promises.
339+
* Transforms the 'x' part of `x.then(...)`, or the 'y()' part of `y().catch(...)`, where 'x' and 'y()' are Promises.
340340
*/
341341
function transformPromiseExpressionOfPropertyAccess(node: Expression, transformer: Transformer, prevArgName?: SynthBindingName): readonly Statement[] {
342342
if (shouldReturn(node, transformer)) {
343343
return [createReturn(getSynthesizedDeepClone(node))];
344344
}
345345

346-
return createVariableOrAssignmentOrExpressionStatement(prevArgName, createAwait(node));
346+
return createVariableOrAssignmentOrExpressionStatement(prevArgName, createAwait(node), /*typeAnnotation*/ undefined);
347347
}
348348

349-
function createVariableOrAssignmentOrExpressionStatement(variableName: SynthBindingName | undefined, rightHandSide: Expression): readonly Statement[] {
349+
function createVariableOrAssignmentOrExpressionStatement(variableName: SynthBindingName | undefined, rightHandSide: Expression, typeAnnotation: TypeNode | undefined): readonly Statement[] {
350350
if (!variableName || isEmptyBindingName(variableName)) {
351351
// if there's no argName to assign to, there still might be side effects
352352
return [createExpressionStatement(rightHandSide)];
@@ -363,11 +363,22 @@ namespace ts.codefix {
363363
createVariableDeclarationList([
364364
createVariableDeclaration(
365365
getSynthesizedDeepClone(getNode(variableName)),
366-
/*type*/ undefined,
366+
typeAnnotation,
367367
rightHandSide)],
368368
NodeFlags.Const))];
369369
}
370370

371+
function maybeAnnotateAndReturn(expressionToReturn: Expression | undefined, typeAnnotation: TypeNode | undefined): readonly Statement[] {
372+
if (typeAnnotation && expressionToReturn) {
373+
const name = createOptimisticUniqueName("result");
374+
return [
375+
...createVariableOrAssignmentOrExpressionStatement(createSynthIdentifier(name), expressionToReturn, typeAnnotation),
376+
createReturn(name)
377+
];
378+
}
379+
return [createReturn(expressionToReturn)];
380+
}
381+
371382
// should be kept up to date with isFixablePromiseArgument in suggestionDiagnostics.ts
372383
function getTransformationBody(func: Expression, prevArgName: SynthBindingName | undefined, argName: SynthBindingName | undefined, parent: CallExpression, transformer: Transformer): readonly Statement[] {
373384
switch (func.kind) {
@@ -382,7 +393,7 @@ namespace ts.codefix {
382393

383394
const synthCall = createCall(getSynthesizedDeepClone(func as Identifier), /*typeArguments*/ undefined, isSynthIdentifier(argName) ? [argName.identifier] : []);
384395
if (shouldReturn(parent, transformer)) {
385-
return [createReturn(synthCall)];
396+
return maybeAnnotateAndReturn(synthCall, parent.typeArguments?.[0]);
386397
}
387398

388399
const type = transformer.checker.getTypeAtLocation(func);
@@ -392,7 +403,7 @@ namespace ts.codefix {
392403
return silentFail();
393404
}
394405
const returnType = callSignatures[0].getReturnType();
395-
const varDeclOrAssignment = createVariableOrAssignmentOrExpressionStatement(prevArgName, createAwait(synthCall));
406+
const varDeclOrAssignment = createVariableOrAssignmentOrExpressionStatement(prevArgName, createAwait(synthCall), parent.typeArguments?.[0]);
396407
if (prevArgName) {
397408
prevArgName.types.push(returnType);
398409
}
@@ -409,10 +420,12 @@ namespace ts.codefix {
409420
for (const statement of funcBody.statements) {
410421
if (isReturnStatement(statement)) {
411422
seenReturnStatement = true;
412-
}
413-
414-
if (isReturnStatementWithFixablePromiseHandler(statement)) {
415-
refactoredStmts = refactoredStmts.concat(getInnerTransformationBody(transformer, [statement], prevArgName));
423+
if (isReturnStatementWithFixablePromiseHandler(statement)) {
424+
refactoredStmts = refactoredStmts.concat(getInnerTransformationBody(transformer, [statement], prevArgName));
425+
}
426+
else {
427+
refactoredStmts.push(...maybeAnnotateAndReturn(statement.expression, parent.typeArguments?.[0]));
428+
}
416429
}
417430
else {
418431
refactoredStmts.push(statement);
@@ -440,14 +453,14 @@ namespace ts.codefix {
440453
const rightHandSide = getSynthesizedDeepClone(funcBody);
441454
const possiblyAwaitedRightHandSide = !!transformer.checker.getPromisedTypeOfPromise(returnType) ? createAwait(rightHandSide) : rightHandSide;
442455
if (!shouldReturn(parent, transformer)) {
443-
const transformedStatement = createVariableOrAssignmentOrExpressionStatement(prevArgName, possiblyAwaitedRightHandSide);
456+
const transformedStatement = createVariableOrAssignmentOrExpressionStatement(prevArgName, possiblyAwaitedRightHandSide, /*typeAnnotation*/ undefined);
444457
if (prevArgName) {
445458
prevArgName.types.push(returnType);
446459
}
447460
return transformedStatement;
448461
}
449462
else {
450-
return [createReturn(possiblyAwaitedRightHandSide)];
463+
return maybeAnnotateAndReturn(possiblyAwaitedRightHandSide, parent.typeArguments?.[0]);
451464
}
452465
}
453466
}

‎src/services/suggestionDiagnostics.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ namespace ts {
133133
return !!forEachReturnStatement(body, isReturnStatementWithFixablePromiseHandler);
134134
}
135135

136-
export function isReturnStatementWithFixablePromiseHandler(node: Node): node is ReturnStatement {
136+
export function isReturnStatementWithFixablePromiseHandler(node: Node): node is ReturnStatement & { expression: CallExpression } {
137137
return isReturnStatement(node) && !!node.expression && isFixablePromiseHandler(node.expression);
138138
}
139139

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

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,21 @@ interface String { charAt: any; }
255255
interface Array<T> {}`
256256
};
257257

258-
function testConvertToAsyncFunction(caption: string, text: string, baselineFolder: string, includeLib?: boolean, expectFailure = false, onlyProvideAction = false) {
258+
type WithSkipAndOnly<T extends any[]> = ((...args: T) => void) & {
259+
skip: (...args: T) => void;
260+
only: (...args: T) => void;
261+
};
262+
263+
function createTestWrapper<T extends any[]>(fn: (it: Mocha.PendingTestFunction, ...args: T) => void): WithSkipAndOnly<T> {
264+
wrapped.skip = (...args: T) => fn(it.skip, ...args);
265+
wrapped.only = (...args: T) => fn(it.only, ...args);
266+
return wrapped;
267+
function wrapped(...args: T) {
268+
return fn(it, ...args);
269+
}
270+
}
271+
272+
function testConvertToAsyncFunction(it: Mocha.PendingTestFunction, caption: string, text: string, baselineFolder: string, includeLib?: boolean, expectFailure = false, onlyProvideAction = false) {
259273
const t = extractTest(text);
260274
const selectionRange = t.ranges.get("selection")!;
261275
if (!selectionRange) {
@@ -343,7 +357,19 @@ interface Array<T> {}`
343357
}
344358
}
345359

346-
describe("unittests:: services:: convertToAsyncFunctions", () => {
360+
const _testConvertToAsyncFunction = createTestWrapper((it, caption: string, text: string) => {
361+
testConvertToAsyncFunction(it, caption, text, "convertToAsyncFunction", /*includeLib*/ true);
362+
});
363+
364+
const _testConvertToAsyncFunctionFailed = createTestWrapper((it, caption: string, text: string) => {
365+
testConvertToAsyncFunction(it, caption, text, "convertToAsyncFunction", /*includeLib*/ true, /*expectFailure*/ true);
366+
});
367+
368+
const _testConvertToAsyncFunctionFailedSuggestion = createTestWrapper((it, caption: string, text: string) => {
369+
testConvertToAsyncFunction(it, caption, text, "convertToAsyncFunction", /*includeLib*/ true, /*expectFailure*/ true, /*onlyProvideAction*/ true);
370+
});
371+
372+
describe("unittests:: services:: convertToAsyncFunction", () => {
347373
_testConvertToAsyncFunction("convertToAsyncFunction_basic", `
348374
function [#|f|](): Promise<void>{
349375
return fetch('https://typescriptlang.org').then(result => { console.log(result) });
@@ -1352,17 +1378,54 @@ function foo() {
13521378
})
13531379
}
13541380
`);
1355-
});
13561381

1357-
function _testConvertToAsyncFunction(caption: string, text: string) {
1358-
testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", /*includeLib*/ true);
1359-
}
1382+
_testConvertToAsyncFunction("convertToAsyncFunction_thenTypeArgument1", `
1383+
type APIResponse<T> = { success: true, data: T } | { success: false };
13601384
1361-
function _testConvertToAsyncFunctionFailed(caption: string, text: string) {
1362-
testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", /*includeLib*/ true, /*expectFailure*/ true);
1363-
}
1385+
function wrapResponse<T>(response: T): APIResponse<T> {
1386+
return { success: true, data: response };
1387+
}
13641388
1365-
function _testConvertToAsyncFunctionFailedSuggestion(caption: string, text: string) {
1366-
testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", /*includeLib*/ true, /*expectFailure*/ true, /*onlyProvideAction*/ true);
1367-
}
1389+
function [#|get|]() {
1390+
return Promise.resolve(undefined!).then<APIResponse<{ email: string }>>(wrapResponse);
1391+
}
1392+
`);
1393+
1394+
_testConvertToAsyncFunction("convertToAsyncFunction_thenTypeArgument2", `
1395+
type APIResponse<T> = { success: true, data: T } | { success: false };
1396+
1397+
function wrapResponse<T>(response: T): APIResponse<T> {
1398+
return { success: true, data: response };
1399+
}
1400+
1401+
function [#|get|]() {
1402+
return Promise.resolve(undefined!).then<APIResponse<{ email: string }>>(d => wrapResponse(d));
1403+
}
1404+
`);
1405+
1406+
_testConvertToAsyncFunction("convertToAsyncFunction_thenTypeArgument3", `
1407+
type APIResponse<T> = { success: true, data: T } | { success: false };
1408+
1409+
function wrapResponse<T>(response: T): APIResponse<T> {
1410+
return { success: true, data: response };
1411+
}
1412+
1413+
function [#|get|]() {
1414+
return Promise.resolve(undefined!).then<APIResponse<{ email: string }>>(d => {
1415+
console.log(d);
1416+
return wrapResponse(d);
1417+
});
1418+
}
1419+
`);
1420+
1421+
_testConvertToAsyncFunction("convertToAsyncFunction_catchTypeArgument1", `
1422+
type APIResponse<T> = { success: true, data: T } | { success: false };
1423+
1424+
function [#|get|]() {
1425+
return Promise
1426+
.resolve<APIResponse<{ email: string }>>({ success: true, data: { email: "" } })
1427+
.catch<APIResponse<{ email: string }>>(() => ({ success: false }));
1428+
}
1429+
`);
1430+
});
13681431
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// ==ORIGINAL==
2+
3+
type APIResponse<T> = { success: true, data: T } | { success: false };
4+
5+
function /*[#|*/get/*|]*/() {
6+
return Promise
7+
.resolve<APIResponse<{ email: string }>>({ success: true, data: { email: "" } })
8+
.catch<APIResponse<{ email: string }>>(() => ({ success: false }));
9+
}
10+
11+
// ==ASYNC FUNCTION::Convert to async function==
12+
13+
type APIResponse<T> = { success: true, data: T } | { success: false };
14+
15+
async function get() {
16+
try {
17+
return Promise
18+
.resolve<APIResponse<{ email: string; }>>({ success: true, data: { email: "" } });
19+
}
20+
catch (e) {
21+
const result: APIResponse<{ email: string; }> = ({ success: false });
22+
return result;
23+
}
24+
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// ==ORIGINAL==
2+
3+
type APIResponse<T> = { success: true, data: T } | { success: false };
4+
5+
function wrapResponse<T>(response: T): APIResponse<T> {
6+
return { success: true, data: response };
7+
}
8+
9+
function /*[#|*/get/*|]*/() {
10+
return Promise.resolve(undefined!).then<APIResponse<{ email: string }>>(wrapResponse);
11+
}
12+
13+
// ==ASYNC FUNCTION::Convert to async function==
14+
15+
type APIResponse<T> = { success: true, data: T } | { success: false };
16+
17+
function wrapResponse<T>(response: T): APIResponse<T> {
18+
return { success: true, data: response };
19+
}
20+
21+
async function get() {
22+
const response = await Promise.resolve((undefined!));
23+
const result: APIResponse<{ email: string; }> = wrapResponse(response);
24+
return result;
25+
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// ==ORIGINAL==
2+
3+
type APIResponse<T> = { success: true, data: T } | { success: false };
4+
5+
function wrapResponse<T>(response: T): APIResponse<T> {
6+
return { success: true, data: response };
7+
}
8+
9+
function /*[#|*/get/*|]*/() {
10+
return Promise.resolve(undefined!).then<APIResponse<{ email: string }>>(d => wrapResponse(d));
11+
}
12+
13+
// ==ASYNC FUNCTION::Convert to async function==
14+
15+
type APIResponse<T> = { success: true, data: T } | { success: false };
16+
17+
function wrapResponse<T>(response: T): APIResponse<T> {
18+
return { success: true, data: response };
19+
}
20+
21+
async function get() {
22+
const d = await Promise.resolve((undefined!));
23+
const result: APIResponse<{ email: string; }> = wrapResponse(d);
24+
return result;
25+
}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// ==ORIGINAL==
2+
3+
type APIResponse<T> = { success: true, data: T } | { success: false };
4+
5+
function wrapResponse<T>(response: T): APIResponse<T> {
6+
return { success: true, data: response };
7+
}
8+
9+
function /*[#|*/get/*|]*/() {
10+
return Promise.resolve(undefined!).then<APIResponse<{ email: string }>>(d => {
11+
console.log(d);
12+
return wrapResponse(d);
13+
});
14+
}
15+
16+
// ==ASYNC FUNCTION::Convert to async function==
17+
18+
type APIResponse<T> = { success: true, data: T } | { success: false };
19+
20+
function wrapResponse<T>(response: T): APIResponse<T> {
21+
return { success: true, data: response };
22+
}
23+
24+
async function get() {
25+
const d = await Promise.resolve((undefined!));
26+
console.log(d);
27+
const result: APIResponse<{ email: string; }> = wrapResponse(d);
28+
return result;
29+
}

0 commit comments

Comments
 (0)