diff --git a/src/services/codefixes/convertToAsyncFunction.ts b/src/services/codefixes/convertToAsyncFunction.ts index 7f5174cf67b8b..8bd8341264a96 100644 --- a/src/services/codefixes/convertToAsyncFunction.ts +++ b/src/services/codefixes/convertToAsyncFunction.ts @@ -83,19 +83,14 @@ namespace ts.codefix { } for (const statement of returnStatements) { - if (isCallExpression(statement)) { - startTransformation(statement, statement); - } - else { - forEachChild(statement, function visit(node: Node) { - if (isCallExpression(node)) { - startTransformation(node, statement); - } - else if (!isFunctionLike(node)) { - forEachChild(node, visit); - } - }); - } + forEachChild(statement, function visit(node) { + if (isCallExpression(node)) { + startTransformation(node, statement); + } + else if (!isFunctionLike(node)) { + forEachChild(node, visit); + } + }); } } @@ -167,6 +162,7 @@ namespace ts.codefix { function renameCollidingVarNames(nodeToRename: FunctionLikeDeclaration, checker: TypeChecker, synthNamesMap: Map, context: CodeFixContextBase, setOfAllExpressionsToReturn: Map, originalType: Map, allVarNames: SymbolAndIdentifier[]): FunctionLikeDeclaration { const identsToRenameMap: Map = createMap(); // key is the symbol id + const collidingSymbolMap: Map = createMap(); forEachChild(nodeToRename, function visit(node: Node) { if (!isIdentifier(node)) { forEachChild(node, visit); @@ -184,19 +180,25 @@ namespace ts.codefix { // if the identifier refers to a function we want to add the new synthesized variable for the declaration (ex. blob in let blob = res(arg)) // Note - the choice of the last call signature is arbitrary if (lastCallSignature && lastCallSignature.parameters.length && !synthNamesMap.has(symbolIdString)) { - const synthName = getNewNameIfConflict(createIdentifier(lastCallSignature.parameters[0].name), allVarNames); + const firstParameter = lastCallSignature.parameters[0]; + const ident = isParameter(firstParameter.valueDeclaration) && tryCast(firstParameter.valueDeclaration.name, isIdentifier) || createOptimisticUniqueName("result"); + const synthName = getNewNameIfConflict(ident, collidingSymbolMap); synthNamesMap.set(symbolIdString, synthName); allVarNames.push({ identifier: synthName.identifier, symbol }); + addNameToFrequencyMap(collidingSymbolMap, ident.text, symbol); } // we only care about identifiers that are parameters and declarations (don't care about other uses) else if (node.parent && (isParameter(node.parent) || isVariableDeclaration(node.parent))) { + const originalName = node.text; + const collidingSymbols = collidingSymbolMap.get(originalName); // if the identifier name conflicts with a different identifier that we've already seen - if (allVarNames.some(ident => ident.identifier.text === node.text && ident.symbol !== symbol)) { - const newName = getNewNameIfConflict(node, allVarNames); + if (collidingSymbols && collidingSymbols.some(prevSymbol => prevSymbol !== symbol)) { + const newName = getNewNameIfConflict(node, collidingSymbolMap); identsToRenameMap.set(symbolIdString, newName.identifier); synthNamesMap.set(symbolIdString, newName); allVarNames.push({ identifier: newName.identifier, symbol }); + addNameToFrequencyMap(collidingSymbolMap, originalName, symbol); } else { const identifier = getSynthesizedDeepClone(node); @@ -204,6 +206,7 @@ namespace ts.codefix { synthNamesMap.set(symbolIdString, { identifier, types: [], numberOfAssignmentsOriginal: allVarNames.filter(elem => elem.identifier.text === node.text).length/*, numberOfAssignmentsSynthesized: 0*/ }); if ((isParameter(node.parent) && isExpressionOrCallOnTypePromise(node.parent.parent)) || isVariableDeclaration(node.parent)) { allVarNames.push({ identifier, symbol }); + addNameToFrequencyMap(collidingSymbolMap, originalName, symbol); } } } @@ -246,8 +249,17 @@ namespace ts.codefix { } - function getNewNameIfConflict(name: Identifier, allVarNames: SymbolAndIdentifier[]): SynthIdentifier { - const numVarsSameName = allVarNames.filter(elem => elem.identifier.text === name.text).length; + function addNameToFrequencyMap(renamedVarNameFrequencyMap: Map, originalName: string, symbol: Symbol) { + if (renamedVarNameFrequencyMap.has(originalName)) { + renamedVarNameFrequencyMap.get(originalName)!.push(symbol); + } + else { + renamedVarNameFrequencyMap.set(originalName, [symbol]); + } + } + + function getNewNameIfConflict(name: Identifier, originalNames: Map): SynthIdentifier { + const numVarsSameName = (originalNames.get(name.text) || []).length; const numberOfAssignmentsOriginal = 0; const identifier = numVarsSameName === 0 ? name : createIdentifier(name.text + "_" + numVarsSameName); return { identifier, types: [], numberOfAssignmentsOriginal }; @@ -294,13 +306,14 @@ namespace ts.codefix { prevArgName.numberOfAssignmentsOriginal = 2; // Try block and catch block transformer.synthNamesMap.forEach((val, key) => { if (val.identifier.text === prevArgName.identifier.text) { - transformer.synthNamesMap.set(key, getNewNameIfConflict(prevArgName.identifier, transformer.allVarNames)); + const newSynthName = createUniqueSynthName(prevArgName); + transformer.synthNamesMap.set(key, newSynthName); } }); // update the constIdentifiers list if (transformer.constIdentifiers.some(elem => elem.text === prevArgName.identifier.text)) { - transformer.constIdentifiers.push(getNewNameIfConflict(prevArgName.identifier, transformer.allVarNames).identifier); + transformer.constIdentifiers.push(createUniqueSynthName(prevArgName).identifier); } } @@ -326,6 +339,12 @@ namespace ts.codefix { return varDeclList ? [varDeclList, tryStatement] : [tryStatement]; } + function createUniqueSynthName(prevArgName: SynthIdentifier) { + const renamedPrevArg = createOptimisticUniqueName(prevArgName.identifier.text); + const newSynthName = { identifier: renamedPrevArg, types: [], numberOfAssignmentsOriginal: 0 }; + return newSynthName; + } + function transformThen(node: CallExpression, transformer: Transformer, outermostParent: CallExpression, prevArgName?: SynthIdentifier): Statement[] { const [res, rej] = node.arguments; @@ -348,11 +367,8 @@ namespace ts.codefix { return [createTry(tryBlock, catchClause, /* finallyBlock */ undefined) as Statement]; } - else { - return transformExpression(node.expression, transformer, node, argNameRes).concat(transformationBody); - } - return []; + return transformExpression(node.expression, transformer, node, argNameRes).concat(transformationBody); } function getFlagOfIdentifier(node: Identifier, constIdentifiers: Identifier[]): NodeFlags { @@ -419,8 +435,13 @@ namespace ts.codefix { // Arrow functions with block bodies { } will enter this control flow if (isFunctionLikeDeclaration(func) && func.body && isBlock(func.body) && func.body.statements) { let refactoredStmts: Statement[] = []; + let seenReturnStatement = false; for (const statement of func.body.statements) { + if (isReturnStatement(statement)) { + seenReturnStatement = true; + } + if (getReturnStatementsWithPromiseHandlers(statement).length) { refactoredStmts = refactoredStmts.concat(getInnerTransformationBody(transformer, [statement], prevArgName)); } @@ -430,7 +451,7 @@ namespace ts.codefix { } return shouldReturn ? getSynthesizedDeepClones(createNodeArray(refactoredStmts)) : - removeReturns(createNodeArray(refactoredStmts), prevArgName!.identifier, transformer.constIdentifiers); + removeReturns(createNodeArray(refactoredStmts), prevArgName!.identifier, transformer.constIdentifiers, seenReturnStatement); } else { const funcBody = (func).body; @@ -443,7 +464,7 @@ namespace ts.codefix { if (hasPrevArgName && !shouldReturn) { const type = transformer.checker.getTypeAtLocation(func); - const returnType = getLastCallSignature(type, transformer.checker).getReturnType(); + const returnType = getLastCallSignature(type, transformer.checker)!.getReturnType(); const varDeclOrAssignment = createVariableDeclarationOrAssignment(prevArgName!, getSynthesizedDeepClone(funcBody) as Expression, transformer); prevArgName!.types.push(returnType); return varDeclOrAssignment; @@ -460,13 +481,13 @@ namespace ts.codefix { return createNodeArray([]); } - function getLastCallSignature(type: Type, checker: TypeChecker): Signature { - const callSignatures = type && checker.getSignaturesOfType(type, SignatureKind.Call); - return callSignatures && callSignatures[callSignatures.length - 1]; + function getLastCallSignature(type: Type, checker: TypeChecker): Signature | undefined { + const callSignatures = checker.getSignaturesOfType(type, SignatureKind.Call); + return lastOrUndefined(callSignatures); } - function removeReturns(stmts: NodeArray, prevArgName: Identifier, constIdentifiers: Identifier[]): NodeArray { + function removeReturns(stmts: NodeArray, prevArgName: Identifier, constIdentifiers: Identifier[], seenReturnStatement: boolean): NodeArray { const ret: Statement[] = []; for (const stmt of stmts) { if (isReturnStatement(stmt)) { @@ -480,6 +501,12 @@ namespace ts.codefix { } } + // if block has no return statement, need to define prevArgName as undefined to prevent undeclared variables + if (!seenReturnStatement) { + ret.push(createVariableStatement(/*modifiers*/ undefined, + (createVariableDeclarationList([createVariableDeclaration(prevArgName, /*type*/ undefined, createIdentifier("undefined"))], getFlagOfIdentifier(prevArgName, constIdentifiers))))); + } + return createNodeArray(ret); } @@ -517,9 +544,6 @@ namespace ts.codefix { name = getMapEntryIfExists(param); } } - else if (isCallExpression(funcNode) && funcNode.arguments.length > 0 && isIdentifier(funcNode.arguments[0])) { - name = { identifier: funcNode.arguments[0] as Identifier, types, numberOfAssignmentsOriginal }; - } else if (isIdentifier(funcNode)) { name = getMapEntryIfExists(funcNode); } diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 271f0601186d0..9bcbdd650327c 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -141,8 +141,8 @@ namespace ts { } /** @internal */ - export function getReturnStatementsWithPromiseHandlers(node: Node): Node[] { - const returnStatements: Node[] = []; + export function getReturnStatementsWithPromiseHandlers(node: Node): ReturnStatement[] { + const returnStatements: ReturnStatement[] = []; if (isFunctionLike(node)) { forEachChild(node, visit); } @@ -155,14 +155,8 @@ namespace ts { return; } - if (isReturnStatement(child)) { - forEachChild(child, addHandlers); - } - - function addHandlers(returnChild: Node) { - if (isFixablePromiseHandler(returnChild)) { - returnStatements.push(child as ReturnStatement); - } + if (isReturnStatement(child) && child.expression && isFixablePromiseHandler(child.expression)) { + returnStatements.push(child); } forEachChild(child, visit); diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 7f0d06b480032..a2eb2f0270c53 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1664,11 +1664,10 @@ namespace ts { return clone; } - export function getSynthesizedDeepCloneWithRenames(node: T, includeTrivia = true, renameMap?: Map, checker?: TypeChecker, callback?: (originalNode: Node, clone: Node) => any): T { - + export function getSynthesizedDeepCloneWithRenames(node: T, includeTrivia = true, renameMap?: Map, checker?: TypeChecker, callback?: (originalNode: Node, clone: Node) => any): T { let clone; - if (node && isIdentifier(node!) && renameMap && checker) { - const symbol = checker.getSymbolAtLocation(node!); + if (isIdentifier(node) && renameMap && checker) { + const symbol = checker.getSymbolAtLocation(node); const renameInfo = symbol && renameMap.get(String(getSymbolId(symbol))); if (renameInfo) { @@ -1677,11 +1676,11 @@ namespace ts { } if (!clone) { - clone = node && getSynthesizedDeepCloneWorker(node as NonNullable, renameMap, checker, callback); + clone = getSynthesizedDeepCloneWorker(node as NonNullable, renameMap, checker, callback); } if (clone && !includeTrivia) suppressLeadingAndTrailingTrivia(clone); - if (callback && node) callback(node!, clone); + if (callback && clone) callback(node, clone); return clone as T; } diff --git a/src/testRunner/unittests/convertToAsyncFunction.ts b/src/testRunner/unittests/convertToAsyncFunction.ts index 9f675f1c89a33..b6e8cddb0072c 100644 --- a/src/testRunner/unittests/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/convertToAsyncFunction.ts @@ -751,7 +751,7 @@ function [#|f|](): Promise { } return x.then(resp => { var blob = resp.blob().then(blob => blob.byteOffset).catch(err => 'Error'); - return fetch("https://micorosft.com").then(res => console.log("Another one!")); + return fetch("https://microsoft.com").then(res => console.log("Another one!")); }); } ` @@ -1132,6 +1132,31 @@ function [#|f|]() { const [#|foo|] = function () { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); } +`); + + _testConvertToAsyncFunction("convertToAsyncFunction_catchBlockUniqueParams", ` +function [#|f|]() { + return Promise.resolve().then(x => 1).catch(x => "a").then(x => !!x); +} +`); + + _testConvertToAsyncFunction("convertToAsyncFunction_bindingPattern", ` +function [#|f|]() { + return fetch('https://typescriptlang.org').then(res); +} +function res({ status, trailer }){ + console.log(status); +} +`); + + _testConvertToAsyncFunction("convertToAsyncFunction_bindingPatternNameCollision", ` +function [#|f|]() { + const result = 'https://typescriptlang.org'; + return fetch(result).then(res); +} +function res({ status, trailer }){ + console.log(status); +} `); _testConvertToAsyncFunctionFailed("convertToAsyncFunction_thenArgumentNotFunction", ` @@ -1146,7 +1171,6 @@ function [#|f|]() { } `); - }); function _testConvertToAsyncFunction(caption: string, text: string) { diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_InnerVarNameConflict.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_InnerVarNameConflict.ts index 3570a90a0b166..72e4a66fb55f4 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_InnerVarNameConflict.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_InnerVarNameConflict.ts @@ -13,5 +13,6 @@ function /*[#|*/f/*|]*/(): Promise { async function f(): Promise { const resp = await fetch("https://typescriptlang.org"); var blob = resp.blob().then(blob_1 => blob_1.byteOffset).catch(err => 'Error'); - return blob_1.toString(); + const blob_2 = undefined; + return blob_2.toString(); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_MultipleReturns2.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_MultipleReturns2.ts index 6569c1fb0ef99..59a02875d84c3 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_MultipleReturns2.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_MultipleReturns2.ts @@ -7,7 +7,7 @@ function /*[#|*/f/*|]*/(): Promise { } return x.then(resp => { var blob = resp.blob().then(blob => blob.byteOffset).catch(err => 'Error'); - return fetch("https://micorosft.com").then(res => console.log("Another one!")); + return fetch("https://microsoft.com").then(res => console.log("Another one!")); }); } @@ -21,6 +21,6 @@ async function f(): Promise { } const resp = await x; var blob = resp.blob().then(blob_1 => blob_1.byteOffset).catch(err => 'Error'); - const res_1 = await fetch("https://micorosft.com"); + const res_2 = await fetch("https://microsoft.com"); return console.log("Another one!"); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.js new file mode 100644 index 0000000000000..f06ce44e78b8a --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.js @@ -0,0 +1,18 @@ +// ==ORIGINAL== + +function /*[#|*/f/*|]*/() { + return fetch('https://typescriptlang.org').then(res); +} +function res({ status, trailer }){ + console.log(status); +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function f() { + const result = await fetch('https://typescriptlang.org'); + return res(result); +} +function res({ status, trailer }){ + console.log(status); +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.ts new file mode 100644 index 0000000000000..f06ce44e78b8a --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.ts @@ -0,0 +1,18 @@ +// ==ORIGINAL== + +function /*[#|*/f/*|]*/() { + return fetch('https://typescriptlang.org').then(res); +} +function res({ status, trailer }){ + console.log(status); +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function f() { + const result = await fetch('https://typescriptlang.org'); + return res(result); +} +function res({ status, trailer }){ + console.log(status); +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.js new file mode 100644 index 0000000000000..6813472966a51 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.js @@ -0,0 +1,20 @@ +// ==ORIGINAL== + +function /*[#|*/f/*|]*/() { + const result = 'https://typescriptlang.org'; + return fetch(result).then(res); +} +function res({ status, trailer }){ + console.log(status); +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function f() { + const result = 'https://typescriptlang.org'; + const result_1 = await fetch(result); + return res(result_1); +} +function res({ status, trailer }){ + console.log(status); +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.ts new file mode 100644 index 0000000000000..6813472966a51 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.ts @@ -0,0 +1,20 @@ +// ==ORIGINAL== + +function /*[#|*/f/*|]*/() { + const result = 'https://typescriptlang.org'; + return fetch(result).then(res); +} +function res({ status, trailer }){ + console.log(status); +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function f() { + const result = 'https://typescriptlang.org'; + const result_1 = await fetch(result); + return res(result_1); +} +function res({ status, trailer }){ + console.log(status); +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.js new file mode 100644 index 0000000000000..2600adec16b1a --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.js @@ -0,0 +1,19 @@ +// ==ORIGINAL== + +function /*[#|*/f/*|]*/() { + return Promise.resolve().then(x => 1).catch(x => "a").then(x => !!x); +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function f() { + let x_2; + try { + const x = await Promise.resolve(); + x_2 = 1; + } + catch (x_1) { + x_2 = "a"; + } + return !!x_2; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.ts new file mode 100644 index 0000000000000..5c4daf076a035 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.ts @@ -0,0 +1,19 @@ +// ==ORIGINAL== + +function /*[#|*/f/*|]*/() { + return Promise.resolve().then(x => 1).catch(x => "a").then(x => !!x); +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function f() { + let x_2: string | number; + try { + const x = await Promise.resolve(); + x_2 = 1; + } + catch (x_1) { + x_2 = "a"; + } + return !!x_2; +}