From 2187ba1f848ebfbb5ab2f0aeef2abca8167eb5ba Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 2 Apr 2020 14:12:19 -0800 Subject: [PATCH] Fix variable name collisions (#37761) --- .../codefixes/convertToAsyncFunction.ts | 20 +------------------ .../services/convertToAsyncFunction.ts | 9 +++++++++ ...ToAsyncFunction_InnerPromiseRetBinding1.ts | 8 ++++---- ...tToAsyncFunction_ParameterNameCollision.ts | 20 +++++++++++++++++++ ...onvertToAsyncFunction_basicWithComments.ts | 2 +- 5 files changed, 35 insertions(+), 24 deletions(-) create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ParameterNameCollision.ts diff --git a/src/services/codefixes/convertToAsyncFunction.ts b/src/services/codefixes/convertToAsyncFunction.ts index 11f97bbb7f2..b0f0ee7de84 100644 --- a/src/services/codefixes/convertToAsyncFunction.ts +++ b/src/services/codefixes/convertToAsyncFunction.ts @@ -36,11 +36,6 @@ namespace ts.codefix { hasBeenDeclared: boolean; } - interface SymbolAndIdentifier { - readonly identifier: Identifier; - readonly symbol: Symbol; - } - interface Transformer { readonly checker: TypeChecker; readonly synthNamesMap: Map; // keys are the symbol id of the identifier @@ -139,12 +134,6 @@ namespace ts.codefix { return !!(nodeType && checker.getPromisedTypeOfPromise(nodeType)); } - function isParameterOfPromiseCallback(node: Node, checker: TypeChecker): node is ParameterDeclaration { - return isParameter(node) && ( - isPromiseReturningCallExpression(node.parent.parent, checker, "then") || - isPromiseReturningCallExpression(node.parent.parent, checker, "catch")); - } - function isPromiseTypedExpression(node: Node, checker: TypeChecker): node is Expression { if (!isExpression(node)) return false; return !!checker.getPromisedTypeOfPromise(checker.getTypeAtLocation(node)); @@ -160,7 +149,6 @@ namespace ts.codefix { It then checks for any collisions and renames them through getSynthesizedDeepClone */ function renameCollidingVarNames(nodeToRename: FunctionLikeDeclaration, checker: TypeChecker, synthNamesMap: Map, sourceFile: SourceFile): FunctionLikeDeclaration { - const variableNames: SymbolAndIdentifier[] = []; const identsToRenameMap = createMap(); // key is the symbol id const collidingSymbolMap = createMultiMap(); forEachChild(nodeToRename, function visit(node: Node) { @@ -188,7 +176,6 @@ namespace ts.codefix { const ident = firstParameter && isParameter(firstParameter.valueDeclaration) && tryCast(firstParameter.valueDeclaration.name, isIdentifier) || createOptimisticUniqueName("result"); const synthName = getNewNameIfConflict(ident, collidingSymbolMap); synthNamesMap.set(symbolIdString, synthName); - variableNames.push({ identifier: synthName.identifier, symbol }); collidingSymbolMap.add(ident.text, symbol); } // We only care about identifiers that are parameters, variable declarations, or binding elements @@ -201,17 +188,12 @@ namespace ts.codefix { const newName = getNewNameIfConflict(node, collidingSymbolMap); identsToRenameMap.set(symbolIdString, newName.identifier); synthNamesMap.set(symbolIdString, newName); - variableNames.push({ identifier: newName.identifier, symbol }); collidingSymbolMap.add(originalName, symbol); } else { const identifier = getSynthesizedDeepClone(node); - identsToRenameMap.set(symbolIdString, identifier); synthNamesMap.set(symbolIdString, createSynthIdentifier(identifier)); - if (isParameterOfPromiseCallback(node.parent, checker) || isVariableDeclaration(node.parent)) { - variableNames.push({ identifier, symbol }); - collidingSymbolMap.add(originalName, symbol); - } + collidingSymbolMap.add(originalName, symbol); } } } diff --git a/src/testRunner/unittests/services/convertToAsyncFunction.ts b/src/testRunner/unittests/services/convertToAsyncFunction.ts index b0df94c7dba..2a362316075 100644 --- a/src/testRunner/unittests/services/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/services/convertToAsyncFunction.ts @@ -1113,7 +1113,16 @@ function rej(reject): a{ ` ); + _testConvertToAsyncFunction("convertToAsyncFunction_ParameterNameCollision", ` +async function foo(x: T): Promise { + return x; +} +function [#|bar|](x: T): Promise { + return foo(x).then(foo) +} +` + ); _testConvertToAsyncFunction("convertToAsyncFunction_LocalReturn", ` diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_InnerPromiseRetBinding1.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_InnerPromiseRetBinding1.ts index 7f1559ff7cb..aa683f7fd34 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_InnerPromiseRetBinding1.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_InnerPromiseRetBinding1.ts @@ -12,13 +12,13 @@ function /*[#|*/innerPromise/*|]*/(): Promise { async function innerPromise(): Promise { const resp = await fetch("https://typescriptlang.org"); - let blob: any; + let blob_1: any; try { const { blob } = await resp.blob(); - blob = blob.byteOffset; + blob_1 = blob.byteOffset; } catch ({ message }) { - blob = 'Error ' + message; + blob_1 = 'Error ' + message; } - return blob.toString(); + return blob_1.toString(); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ParameterNameCollision.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ParameterNameCollision.ts new file mode 100644 index 00000000000..eae9e505e7d --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ParameterNameCollision.ts @@ -0,0 +1,20 @@ +// ==ORIGINAL== + +async function foo(x: T): Promise { + return x; +} + +function /*[#|*/bar/*|]*/(x: T): Promise { + return foo(x).then(foo) +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function foo(x: T): Promise { + return x; +} + +async function bar(x: T): Promise { + const x_1 = await foo(x); + return foo(x_1); +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_basicWithComments.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_basicWithComments.ts index 670a8e9d9df..913295e63dc 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_basicWithComments.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_basicWithComments.ts @@ -14,6 +14,6 @@ async function f(): Promise{ // a /*b*/ const result /*g*/ = await fetch(/*d*/ 'https://typescriptlang.org' /*e*/); - console.log(result); /*k*/ + console.log(/*i*/ result /*j*/); /*k*/ // m } \ No newline at end of file