Skip to content

Commit 4c73b2e

Browse files
andrewbranchandrewbranch
committed
Basic support for binding patterns in async/await code fix
1 parent 33c3ce9 commit 4c73b2e

File tree

2 files changed

+80
-29
lines changed

2 files changed

+80
-29
lines changed

‎src/services/codefixes/convertToAsyncFunction.ts

Lines changed: 72 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ namespace ts.codefix {
1414
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, err) => convertToAsyncFunction(changes, err.file, err.start, context.program.getTypeChecker(), context)),
1515
});
1616

17+
type SynthBindingName = SynthBindingPattern | SynthIdentifier;
18+
19+
interface SynthBindingPattern {
20+
readonly elements: ReadonlyArray<SynthBindingName>;
21+
readonly bindingPattern: BindingPattern;
22+
readonly types: Type[];
23+
}
24+
1725
interface SynthIdentifier {
1826
readonly identifier: Identifier;
1927
readonly types: Type[];
@@ -266,7 +274,7 @@ namespace ts.codefix {
266274

267275
// dispatch function to recursively build the refactoring
268276
// should be kept up to date with isFixablePromiseHandler in suggestionDiagnostics.ts
269-
function transformExpression(node: Expression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
277+
function transformExpression(node: Expression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthBindingName): ReadonlyArray<Statement> {
270278
if (!node) {
271279
return emptyArray;
272280
}
@@ -277,7 +285,7 @@ namespace ts.codefix {
277285
if (isCallExpression(node) && hasPropertyAccessExpressionWithName(node, "then") && nodeType && !!transformer.checker.getPromisedTypeOfPromise(nodeType)) {
278286
return transformThen(node, transformer, outermostParent, prevArgName);
279287
}
280-
else if (isCallExpression(node) && hasPropertyAccessExpressionWithName(node, "catch") && nodeType && !!transformer.checker.getPromisedTypeOfPromise(nodeType)) {
288+
else if (isCallExpression(node) && hasPropertyAccessExpressionWithName(node, "catch") && nodeType && !!transformer.checker.getPromisedTypeOfPromise(nodeType) && (!prevArgName || "identifier" in prevArgName)) {
281289
return transformCatch(node, transformer, prevArgName);
282290
}
283291
else if (isPropertyAccessExpression(node)) {
@@ -293,7 +301,7 @@ namespace ts.codefix {
293301

294302
function transformCatch(node: CallExpression, transformer: Transformer, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
295303
const func = node.arguments[0];
296-
const argName = getArgName(func, transformer);
304+
const argName = getArgBindingName(func, transformer);
297305
const shouldReturn = transformer.setOfExpressionsToReturn.get(getNodeId(node).toString());
298306

299307
/*
@@ -319,8 +327,9 @@ namespace ts.codefix {
319327
const tryBlock = createBlock(transformExpression(node.expression, transformer, node, prevArgName));
320328

321329
const transformationBody = getTransformationBody(func, prevArgName, argName, node, transformer);
322-
const catchArg = argName ? argName.identifier.text : "e";
323-
const catchClause = createCatchClause(catchArg, createBlock(transformationBody));
330+
const catchArg = argName ? "identifier" in argName ? argName.identifier.text : argName.bindingPattern : "e";
331+
const catchVariableDeclaration = createVariableDeclaration(catchArg);
332+
const catchClause = createCatchClause(catchVariableDeclaration, createBlock(transformationBody));
324333

325334
/*
326335
In order to avoid an implicit any, we will synthesize a type for the declaration using the unions of the types of both paths (try block and catch block)
@@ -338,44 +347,54 @@ namespace ts.codefix {
338347
return varDeclList ? [varDeclList, tryStatement] : [tryStatement];
339348
}
340349

350+
function getIdentifierTextsFromBindingName(bindingName: BindingName): ReadonlyArray<string> {
351+
if (isIdentifier(bindingName)) return [bindingName.text];
352+
return flatMap(bindingName.elements, element => {
353+
if (isOmittedExpression(element)) return [];
354+
return getIdentifierTextsFromBindingName(element.name);
355+
});
356+
}
357+
341358
function createUniqueSynthName(prevArgName: SynthIdentifier) {
342359
const renamedPrevArg = createOptimisticUniqueName(prevArgName.identifier.text);
343360
const newSynthName = { identifier: renamedPrevArg, types: [], numberOfAssignmentsOriginal: 0 };
344361
return newSynthName;
345362
}
346363

347-
function transformThen(node: CallExpression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
364+
function transformThen(node: CallExpression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthBindingName): ReadonlyArray<Statement> {
348365
const [res, rej] = node.arguments;
349366

350367
if (!res) {
351368
return transformExpression(node.expression, transformer, outermostParent);
352369
}
353370

354-
const argNameRes = getArgName(res, transformer);
371+
const argNameRes = getArgBindingName(res, transformer);
355372
const transformationBody = getTransformationBody(res, prevArgName, argNameRes, node, transformer);
356373

357374
if (rej) {
358-
const argNameRej = getArgName(rej, transformer);
375+
const argNameRej = getArgBindingName(rej, transformer);
359376

360377
const tryBlock = createBlock(transformExpression(node.expression, transformer, node, argNameRes).concat(transformationBody));
361378

362379
const transformationBody2 = getTransformationBody(rej, prevArgName, argNameRej, node, transformer);
363380

364-
const catchArg = argNameRej ? argNameRej.identifier.text : "e";
365-
const catchClause = createCatchClause(catchArg, createBlock(transformationBody2));
381+
const catchArg = argNameRej ? "identifier" in argNameRej ? argNameRej.identifier.text : argNameRej.bindingPattern : "e";
382+
const catchVariableDeclaration = createVariableDeclaration(catchArg);
383+
const catchClause = createCatchClause(catchVariableDeclaration, createBlock(transformationBody2));
366384

367385
return [createTry(tryBlock, catchClause, /* finallyBlock */ undefined)];
368386
}
369387

370388
return transformExpression(node.expression, transformer, node, argNameRes).concat(transformationBody);
371389
}
372390

373-
function getFlagOfIdentifier(node: Identifier, constIdentifiers: ReadonlyArray<Identifier>): NodeFlags {
374-
const inArr: boolean = constIdentifiers.some(elem => elem.text === node.text);
391+
function getFlagOfBindingName(bindingName: SynthBindingName, constIdentifiers: ReadonlyArray<Identifier>): NodeFlags {
392+
const identifiers = getIdentifierTextsFromBindingName(getNode(bindingName));
393+
const inArr: boolean = constIdentifiers.some(elem => contains(identifiers, elem.text));
375394
return inArr ? NodeFlags.Const : NodeFlags.Let;
376395
}
377396

378-
function transformPromiseCall(node: Expression, transformer: Transformer, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
397+
function transformPromiseCall(node: Expression, transformer: Transformer, prevArgName?: SynthBindingName): ReadonlyArray<Statement> {
379398
const shouldReturn = transformer.setOfExpressionsToReturn.get(getNodeId(node).toString());
380399
// the identifier is empty when the handler (.then()) ignores the argument - In this situation we do not need to save the result of the promise returning call
381400
const originalNodeParent = node.original ? node.original.parent : node.parent;
@@ -389,23 +408,23 @@ namespace ts.codefix {
389408
return [createReturn(getSynthesizedDeepClone(node))];
390409
}
391410

392-
function createTransformedStatement(prevArgName: SynthIdentifier | undefined, rightHandSide: Expression, transformer: Transformer): ReadonlyArray<Statement> {
393-
if (!prevArgName || prevArgName.identifier.text.length === 0) {
411+
function createTransformedStatement(prevArgName: SynthBindingName | undefined, rightHandSide: Expression, transformer: Transformer): ReadonlyArray<Statement> {
412+
if (!prevArgName || isEmpty(prevArgName)) {
394413
// if there's no argName to assign to, there still might be side effects
395414
return [createStatement(rightHandSide)];
396415
}
397416

398-
if (prevArgName.types.length < prevArgName.numberOfAssignmentsOriginal) {
417+
if ("identifier" in prevArgName && prevArgName.types.length < prevArgName.numberOfAssignmentsOriginal) {
399418
// if the variable has already been declared, we don't need "let" or "const"
400419
return [createStatement(createAssignment(getSynthesizedDeepClone(prevArgName.identifier), rightHandSide))];
401420
}
402421

403422
return [createVariableStatement(/*modifiers*/ undefined,
404-
(createVariableDeclarationList([createVariableDeclaration(getSynthesizedDeepClone(prevArgName.identifier), /*type*/ undefined, rightHandSide)], getFlagOfIdentifier(prevArgName.identifier, transformer.constIdentifiers))))];
423+
(createVariableDeclarationList([createVariableDeclaration(getSynthesizedDeepClone(getNode(prevArgName)), /*type*/ undefined, rightHandSide)], getFlagOfBindingName(prevArgName, transformer.constIdentifiers))))];
405424
}
406425

407426
// should be kept up to date with isFixablePromiseArgument in suggestionDiagnostics.ts
408-
function getTransformationBody(func: Expression, prevArgName: SynthIdentifier | undefined, argName: SynthIdentifier | undefined, parent: CallExpression, transformer: Transformer): ReadonlyArray<Statement> {
427+
function getTransformationBody(func: Expression, prevArgName: SynthBindingName | undefined, argName: SynthBindingName | undefined, parent: CallExpression, transformer: Transformer): ReadonlyArray<Statement> {
409428

410429
const shouldReturn = transformer.setOfExpressionsToReturn.get(getNodeId(parent).toString());
411430
switch (func.kind) {
@@ -418,7 +437,7 @@ namespace ts.codefix {
418437
break;
419438
}
420439

421-
const synthCall = createCall(getSynthesizedDeepClone(func as Identifier), /*typeArguments*/ undefined, [argName.identifier]);
440+
const synthCall = createCall(getSynthesizedDeepClone(func as Identifier), /*typeArguments*/ undefined, "identifier" in argName ? [argName.identifier] : []);
422441
if (shouldReturn) {
423442
return [createReturn(synthCall)];
424443
}
@@ -461,7 +480,7 @@ namespace ts.codefix {
461480
return shouldReturn ? refactoredStmts.map(s => getSynthesizedDeepClone(s)) :
462481
removeReturns(
463482
refactoredStmts,
464-
prevArgName === undefined ? undefined : prevArgName.identifier,
483+
prevArgName,
465484
transformer,
466485
seenReturnStatement);
467486
}
@@ -503,7 +522,7 @@ namespace ts.codefix {
503522
}
504523

505524

506-
function removeReturns(stmts: ReadonlyArray<Statement>, prevArgName: Identifier | undefined, transformer: Transformer, seenReturnStatement: boolean): ReadonlyArray<Statement> {
525+
function removeReturns(stmts: ReadonlyArray<Statement>, prevArgName: SynthBindingName | undefined, transformer: Transformer, seenReturnStatement: boolean): ReadonlyArray<Statement> {
507526
const ret: Statement[] = [];
508527
for (const stmt of stmts) {
509528
if (isReturnStatement(stmt)) {
@@ -514,7 +533,7 @@ namespace ts.codefix {
514533
}
515534
else {
516535
ret.push(createVariableStatement(/*modifiers*/ undefined,
517-
(createVariableDeclarationList([createVariableDeclaration(prevArgName, /*type*/ undefined, possiblyAwaitedExpression)], getFlagOfIdentifier(prevArgName, transformer.constIdentifiers)))));
536+
(createVariableDeclarationList([createVariableDeclaration(getNode(prevArgName), /*type*/ undefined, possiblyAwaitedExpression)], getFlagOfBindingName(prevArgName, transformer.constIdentifiers)))));
518537
}
519538
}
520539
}
@@ -526,14 +545,14 @@ namespace ts.codefix {
526545
// if block has no return statement, need to define prevArgName as undefined to prevent undeclared variables
527546
if (!seenReturnStatement && prevArgName !== undefined) {
528547
ret.push(createVariableStatement(/*modifiers*/ undefined,
529-
(createVariableDeclarationList([createVariableDeclaration(prevArgName, /*type*/ undefined, createIdentifier("undefined"))], getFlagOfIdentifier(prevArgName, transformer.constIdentifiers)))));
548+
(createVariableDeclarationList([createVariableDeclaration(getNode(prevArgName), /*type*/ undefined, createIdentifier("undefined"))], getFlagOfBindingName(prevArgName, transformer.constIdentifiers)))));
530549
}
531550

532551
return ret;
533552
}
534553

535554

536-
function getInnerTransformationBody(transformer: Transformer, innerRetStmts: ReadonlyArray<Node>, prevArgName?: SynthIdentifier) {
555+
function getInnerTransformationBody(transformer: Transformer, innerRetStmts: ReadonlyArray<Node>, prevArgName?: SynthBindingName) {
537556

538557
let innerCbBody: Statement[] = [];
539558
for (const stmt of innerRetStmts) {
@@ -553,30 +572,40 @@ namespace ts.codefix {
553572
return innerCbBody;
554573
}
555574

556-
function getArgName(funcNode: Expression, transformer: Transformer): SynthIdentifier | undefined {
575+
function getArgBindingName(funcNode: Expression, transformer: Transformer): SynthBindingName | undefined {
557576

558577
const numberOfAssignmentsOriginal = 0;
559578
const types: Type[] = [];
560579

561-
let name: SynthIdentifier | undefined;
580+
let name: SynthBindingName | undefined;
562581

563582
if (isFunctionLikeDeclaration(funcNode)) {
564583
if (funcNode.parameters.length > 0) {
565-
const param = funcNode.parameters[0].name as Identifier;
566-
name = getMapEntryOrDefault(param);
584+
const param = funcNode.parameters[0].name;
585+
name = getMappedBindingNameOrDefault(param);
567586
}
568587
}
569588
else if (isIdentifier(funcNode)) {
570589
name = getMapEntryOrDefault(funcNode);
571590
}
572591

573592
// return undefined argName when arg is null or undefined
574-
if (!name || name.identifier.text === "undefined") {
593+
if (!name || "identifier" in name && name.identifier.text === "undefined") {
575594
return undefined;
576595
}
577596

578597
return name;
579598

599+
function getMappedBindingNameOrDefault(bindingName: BindingName): SynthBindingName {
600+
if (isIdentifier(bindingName)) return getMapEntryOrDefault(bindingName);
601+
const elements = flatMap(bindingName.elements, element => {
602+
if (isOmittedExpression(element)) return [];
603+
return [getMappedBindingNameOrDefault(element.name)];
604+
});
605+
606+
return { elements, bindingPattern: bindingName, types: [] };
607+
}
608+
580609
function getMapEntryOrDefault(identifier: Identifier): SynthIdentifier {
581610
const originalNode = getOriginalNode(identifier);
582611
const symbol = getSymbol(originalNode);
@@ -597,4 +626,18 @@ namespace ts.codefix {
597626
return node.original ? node.original : node;
598627
}
599628
}
629+
630+
function isEmpty(bindingName: SynthBindingName | undefined): boolean {
631+
if (!bindingName) {
632+
return true;
633+
}
634+
if ("identifier" in bindingName) {
635+
return !bindingName.identifier.text;
636+
}
637+
return every(bindingName.elements, isEmpty);
638+
}
639+
640+
function getNode(bindingName: SynthBindingName) {
641+
return "identifier" in bindingName ? bindingName.identifier : bindingName.bindingPattern;
642+
}
600643
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,14 @@ interface Array<T> {}`
347347
_testConvertToAsyncFunction("convertToAsyncFunction_basic", `
348348
function [#|f|](): Promise<void>{
349349
return fetch('https://typescriptlang.org').then(result => { console.log(result) });
350+
}`);
351+
_testConvertToAsyncFunction("convertToAsyncFunction_arrayBindingPattern", `
352+
function [#|f|](): Promise<void>{
353+
return fetch('https://typescriptlang.org').then(([result]) => { console.log(result) });
354+
}`);
355+
_testConvertToAsyncFunction("convertToAsyncFunction_objectBindingPattern", `
356+
function [#|f|](): Promise<void>{
357+
return fetch('https://typescriptlang.org').then(({ result }) => { console.log(result) });
350358
}`);
351359
_testConvertToAsyncFunction("convertToAsyncFunction_basicNoReturnTypeAnnotation", `
352360
function [#|f|]() {

0 commit comments

Comments
 (0)