Skip to content

Commit e7fb18e

Browse files
andrewbranchandrewbranch
committed
Handle case where you have to add a destructuring after a try/catch block
1 parent 92f0ac7 commit e7fb18e

5 files changed

Lines changed: 155 additions & 25 deletions

File tree

‎src/services/codefixes/convertToAsyncFunction.ts‎

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ namespace ts.codefix {
285285
if (isCallExpression(node) && hasPropertyAccessExpressionWithName(node, "then") && nodeType && !!transformer.checker.getPromisedTypeOfPromise(nodeType)) {
286286
return transformThen(node, transformer, outermostParent, prevArgName);
287287
}
288-
else if (isCallExpression(node) && hasPropertyAccessExpressionWithName(node, "catch") && nodeType && !!transformer.checker.getPromisedTypeOfPromise(nodeType) && (!prevArgName || "identifier" in prevArgName)) {
288+
else if (isCallExpression(node) && hasPropertyAccessExpressionWithName(node, "catch") && nodeType && !!transformer.checker.getPromisedTypeOfPromise(nodeType)) {
289289
return transformCatch(node, transformer, prevArgName);
290290
}
291291
else if (isPropertyAccessExpression(node)) {
@@ -299,52 +299,67 @@ namespace ts.codefix {
299299
return emptyArray;
300300
}
301301

302-
function transformCatch(node: CallExpression, transformer: Transformer, prevArgName?: SynthIdentifier): ReadonlyArray<Statement> {
302+
function transformCatch(node: CallExpression, transformer: Transformer, prevArgName?: SynthBindingName): ReadonlyArray<Statement> {
303303
const func = node.arguments[0];
304304
const argName = getArgBindingName(func, transformer);
305305
const shouldReturn = transformer.setOfExpressionsToReturn.get(getNodeId(node).toString());
306+
let possibleNameForVarDecl: SynthIdentifier | undefined;
306307

307308
/*
308309
If there is another call in the chain after the .catch() we are transforming, we will need to save the result of both paths (try block and catch block)
309310
To do this, we will need to synthesize a variable that we were not aware of while we were adding identifiers to the synthNamesMap
310311
We will use the prevArgName and then update the synthNamesMap with a new variable name for the next transformation step
311312
*/
312313
if (prevArgName && !shouldReturn) {
313-
prevArgName.numberOfAssignmentsOriginal = 2; // Try block and catch block
314-
transformer.synthNamesMap.forEach((val, key) => {
315-
if (val.identifier.text === prevArgName.identifier.text) {
316-
const newSynthName = createUniqueSynthName(prevArgName);
317-
transformer.synthNamesMap.set(key, newSynthName);
318-
}
319-
});
314+
if (isSynthIdentifier(prevArgName)) {
315+
possibleNameForVarDecl = prevArgName;
316+
transformer.synthNamesMap.forEach((val, key) => {
317+
if (val.identifier.text === prevArgName.identifier.text) {
318+
const newSynthName = createUniqueSynthName(prevArgName);
319+
transformer.synthNamesMap.set(key, newSynthName);
320+
}
321+
});
322+
}
323+
else {
324+
possibleNameForVarDecl = createUniqueSynthName({
325+
identifier: createOptimisticUniqueName("result"),
326+
types: prevArgName.types,
327+
numberOfAssignmentsOriginal: 0
328+
});
329+
}
320330

331+
possibleNameForVarDecl.numberOfAssignmentsOriginal = 2; // Try block and catch block
321332
// update the constIdentifiers list
322-
if (transformer.constIdentifiers.some(elem => elem.text === prevArgName.identifier.text)) {
323-
transformer.constIdentifiers.push(createUniqueSynthName(prevArgName).identifier);
333+
if (transformer.constIdentifiers.some(elem => elem.text === possibleNameForVarDecl!.identifier.text)) {
334+
transformer.constIdentifiers.push(createUniqueSynthName(possibleNameForVarDecl).identifier);
324335
}
325336
}
326337

327-
const tryBlock = createBlock(transformExpression(node.expression, transformer, node, prevArgName));
338+
const tryBlock = createBlock(transformExpression(node.expression, transformer, node, possibleNameForVarDecl));
328339

329-
const transformationBody = getTransformationBody(func, prevArgName, argName, node, transformer);
330-
const catchArg = argName ? "identifier" in argName ? argName.identifier.text : argName.bindingPattern : "e";
340+
const transformationBody = getTransformationBody(func, possibleNameForVarDecl, argName, node, transformer);
341+
const catchArg = argName ? isSynthIdentifier(argName) ? argName.identifier.text : argName.bindingPattern : "e";
331342
const catchVariableDeclaration = createVariableDeclaration(catchArg);
332343
const catchClause = createCatchClause(catchVariableDeclaration, createBlock(transformationBody));
333344

334345
/*
335346
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)
336347
*/
337-
let varDeclList;
338-
if (prevArgName && !shouldReturn) {
339-
const typeArray: Type[] = prevArgName.types;
348+
let varDeclList: VariableStatement | undefined;
349+
let varDeclIdentifier: Identifier | undefined;
350+
if (possibleNameForVarDecl && !shouldReturn) {
351+
varDeclIdentifier = getSynthesizedDeepClone(possibleNameForVarDecl.identifier);
352+
const typeArray: Type[] = possibleNameForVarDecl.types;
340353
const unionType = transformer.checker.getUnionType(typeArray, UnionReduction.Subtype);
341354
const unionTypeNode = transformer.isInJSFile ? undefined : transformer.checker.typeToTypeNode(unionType);
342-
const varDecl = [createVariableDeclaration(getSynthesizedDeepClone(prevArgName.identifier), unionTypeNode)];
355+
const varDecl = [createVariableDeclaration(varDeclIdentifier, unionTypeNode)];
343356
varDeclList = createVariableStatement(/*modifiers*/ undefined, createVariableDeclarationList(varDecl, NodeFlags.Let));
344357
}
345358

346359
const tryStatement = createTry(tryBlock, catchClause, /*finallyBlock*/ undefined);
347-
return varDeclList ? [varDeclList, tryStatement] : [tryStatement];
360+
const destructuredResult = prevArgName && varDeclIdentifier && isSynthBindingPattern(prevArgName)
361+
&& createVariableStatement(/* modifiers */ undefined, createVariableDeclarationList([createVariableDeclaration(getSynthesizedDeepCloneWithRenames(prevArgName.bindingPattern), /* type */ undefined, varDeclIdentifier)], NodeFlags.Const));
362+
return compact([varDeclList, tryStatement, destructuredResult]);
348363
}
349364

350365
function getIdentifierTextsFromBindingName(bindingName: BindingName): ReadonlyArray<string> {
@@ -355,7 +370,7 @@ namespace ts.codefix {
355370
});
356371
}
357372

358-
function createUniqueSynthName(prevArgName: SynthIdentifier) {
373+
function createUniqueSynthName(prevArgName: SynthIdentifier): SynthIdentifier {
359374
const renamedPrevArg = createOptimisticUniqueName(prevArgName.identifier.text);
360375
const newSynthName = { identifier: renamedPrevArg, types: [], numberOfAssignmentsOriginal: 0 };
361376
return newSynthName;
@@ -378,7 +393,7 @@ namespace ts.codefix {
378393

379394
const transformationBody2 = getTransformationBody(rej, prevArgName, argNameRej, node, transformer);
380395

381-
const catchArg = argNameRej ? "identifier" in argNameRej ? argNameRej.identifier.text : argNameRej.bindingPattern : "e";
396+
const catchArg = argNameRej ? isSynthIdentifier(argNameRej) ? argNameRej.identifier.text : argNameRej.bindingPattern : "e";
382397
const catchVariableDeclaration = createVariableDeclaration(catchArg);
383398
const catchClause = createCatchClause(catchVariableDeclaration, createBlock(transformationBody2));
384399

@@ -414,7 +429,7 @@ namespace ts.codefix {
414429
return [createStatement(rightHandSide)];
415430
}
416431

417-
if ("identifier" in prevArgName && prevArgName.types.length < prevArgName.numberOfAssignmentsOriginal) {
432+
if (isSynthIdentifier(prevArgName) && prevArgName.types.length < prevArgName.numberOfAssignmentsOriginal) {
418433
// if the variable has already been declared, we don't need "let" or "const"
419434
return [createStatement(createAssignment(getSynthesizedDeepClone(prevArgName.identifier), rightHandSide))];
420435
}
@@ -437,7 +452,7 @@ namespace ts.codefix {
437452
break;
438453
}
439454

440-
const synthCall = createCall(getSynthesizedDeepClone(func as Identifier), /*typeArguments*/ undefined, "identifier" in argName ? [argName.identifier] : []);
455+
const synthCall = createCall(getSynthesizedDeepClone(func as Identifier), /*typeArguments*/ undefined, isSynthIdentifier(argName) ? [argName.identifier] : []);
441456
if (shouldReturn) {
442457
return [createReturn(synthCall)];
443458
}
@@ -631,13 +646,21 @@ namespace ts.codefix {
631646
if (!bindingName) {
632647
return true;
633648
}
634-
if ("identifier" in bindingName) {
649+
if (isSynthIdentifier(bindingName)) {
635650
return !bindingName.identifier.text;
636651
}
637652
return every(bindingName.elements, isEmpty);
638653
}
639654

640655
function getNode(bindingName: SynthBindingName) {
641-
return "identifier" in bindingName ? bindingName.identifier : bindingName.bindingPattern;
656+
return isSynthIdentifier(bindingName) ? bindingName.identifier : bindingName.bindingPattern;
657+
}
658+
659+
function isSynthIdentifier(bindingName: SynthBindingName): bindingName is SynthIdentifier {
660+
return "identifier" in bindingName;
661+
}
662+
663+
function isSynthBindingPattern(bindingName: SynthBindingName): bindingName is SynthBindingPattern {
664+
return "elements" in bindingName;
642665
}
643666
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,39 @@ function [#|innerPromise|](): Promise<string> {
622622
`
623623
);
624624

625+
_testConvertToAsyncFunction("convertToAsyncFunction_InnerPromiseRetBinding1", `
626+
function [#|innerPromise|](): Promise<string> {
627+
return fetch("https://typescriptlang.org").then(resp => {
628+
return resp.blob().then(({ blob }) => blob.byteOffset).catch(({ message }) => 'Error ' + message);
629+
}).then(blob => {
630+
return blob.toString();
631+
});
632+
}
633+
`
634+
);
635+
636+
_testConvertToAsyncFunction("convertToAsyncFunction_InnerPromiseRetBinding2", `
637+
function [#|innerPromise|](): Promise<string> {
638+
return fetch("https://typescriptlang.org").then(resp => {
639+
return resp.blob().then(blob => blob.byteOffset).catch(err => 'Error');
640+
}).then(({ x }) => {
641+
return x.toString();
642+
});
643+
}
644+
`
645+
);
646+
647+
_testConvertToAsyncFunction("convertToAsyncFunction_InnerPromiseRetBinding3", `
648+
function [#|innerPromise|](): Promise<string> {
649+
return fetch("https://typescriptlang.org").then(resp => {
650+
return resp.blob().then(({ blob }) => blob.byteOffset).catch(({ message }) => 'Error ' + message);
651+
}).then(([x, y]) => {
652+
return (x || y).toString();
653+
});
654+
}
655+
`
656+
);
657+
625658
_testConvertToAsyncFunctionFailed("convertToAsyncFunction_VarReturn01", `
626659
function [#|f|]() {
627660
let blob = fetch("https://typescriptlang.org").then(resp => console.log(resp));

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/innerPromise/*|]*/(): Promise<string> {
4+
return fetch("https://typescriptlang.org").then(resp => {
5+
return resp.blob().then(({ blob }) => blob.byteOffset).catch(({ message }) => 'Error ' + message);
6+
}).then(blob => {
7+
return blob.toString();
8+
});
9+
}
10+
11+
// ==ASYNC FUNCTION::Convert to async function==
12+
13+
async function innerPromise(): Promise<string> {
14+
const resp = await fetch("https://typescriptlang.org");
15+
let blob: any;
16+
try {
17+
const { blob } = await resp.blob();
18+
blob = blob.byteOffset;
19+
}
20+
catch ({ message }) {
21+
blob = 'Error ' + message;
22+
}
23+
return blob.toString();
24+
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/innerPromise/*|]*/(): Promise<string> {
4+
return fetch("https://typescriptlang.org").then(resp => {
5+
return resp.blob().then(blob => blob.byteOffset).catch(err => 'Error');
6+
}).then(({ x }) => {
7+
return x.toString();
8+
});
9+
}
10+
11+
// ==ASYNC FUNCTION::Convert to async function==
12+
13+
async function innerPromise(): Promise<string> {
14+
const resp = await fetch("https://typescriptlang.org");
15+
let result: any;
16+
try {
17+
const blob = await resp.blob();
18+
result = blob.byteOffset;
19+
}
20+
catch (err) {
21+
result = 'Error';
22+
}
23+
const { x } = result;
24+
return x.toString();
25+
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// ==ORIGINAL==
2+
3+
function /*[#|*/innerPromise/*|]*/(): Promise<string> {
4+
return fetch("https://typescriptlang.org").then(resp => {
5+
return resp.blob().then(({ blob }) => blob.byteOffset).catch(({ message }) => 'Error ' + message);
6+
}).then(([x, y]) => {
7+
return (x || y).toString();
8+
});
9+
}
10+
11+
// ==ASYNC FUNCTION::Convert to async function==
12+
13+
async function innerPromise(): Promise<string> {
14+
const resp = await fetch("https://typescriptlang.org");
15+
let result: any;
16+
try {
17+
const { blob } = await resp.blob();
18+
result = blob.byteOffset;
19+
}
20+
catch ({ message }) {
21+
result = 'Error ' + message;
22+
}
23+
const [x, y] = result;
24+
return (x || y).toString();
25+
}

0 commit comments

Comments
 (0)