From c83f7698501eca9486499acc1593fe87e1257fa5 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Fri, 15 Jan 2021 03:02:48 +0200 Subject: [PATCH] fix(41405): allow using property access as reference to function (#41429) --- .../codefixes/convertToAsyncFunction.ts | 16 ++-- src/services/suggestionDiagnostics.ts | 28 ++++--- .../services/convertToAsyncFunction.ts | 84 +++++++++++++++++++ .../convertToAsyncFunction_ResRef1.ts | 25 ++++++ .../convertToAsyncFunction_ResRef2.ts | 21 +++++ .../convertToAsyncFunction_ResRef3.ts | 19 +++++ ...nvertToAsyncFunction_ResRefNoReturnVal1.ts | 25 ++++++ 7 files changed, 202 insertions(+), 16 deletions(-) create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef1.ts create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef2.ts create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef3.ts create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRefNoReturnVal1.ts diff --git a/src/services/codefixes/convertToAsyncFunction.ts b/src/services/codefixes/convertToAsyncFunction.ts index fb064d024fc..3fc61d8b45d 100644 --- a/src/services/codefixes/convertToAsyncFunction.ts +++ b/src/services/codefixes/convertToAsyncFunction.ts @@ -65,7 +65,7 @@ namespace ts.codefix { const isInJavascript = isInJSFile(functionToConvert); const setOfExpressionsToReturn = getAllPromiseExpressionsToReturn(functionToConvert, checker); const functionToConvertRenamed = renameCollidingVarNames(functionToConvert, checker, synthNamesMap); - const returnStatements = functionToConvertRenamed.body && isBlock(functionToConvertRenamed.body) ? getReturnStatementsWithPromiseHandlers(functionToConvertRenamed.body) : emptyArray; + const returnStatements = functionToConvertRenamed.body && isBlock(functionToConvertRenamed.body) ? getReturnStatementsWithPromiseHandlers(functionToConvertRenamed.body, checker) : emptyArray; const transformer: Transformer = { checker, synthNamesMap, setOfExpressionsToReturn, isInJSFile: isInJavascript }; if (!returnStatements.length) { return; @@ -90,10 +90,10 @@ namespace ts.codefix { } } - function getReturnStatementsWithPromiseHandlers(body: Block): readonly ReturnStatement[] { + function getReturnStatementsWithPromiseHandlers(body: Block, checker: TypeChecker): readonly ReturnStatement[] { const res: ReturnStatement[] = []; forEachReturnStatement(body, ret => { - if (isReturnStatementWithFixablePromiseHandler(ret)) res.push(ret); + if (isReturnStatementWithFixablePromiseHandler(ret, checker)) res.push(ret); }); return res; } @@ -374,13 +374,14 @@ namespace ts.codefix { case SyntaxKind.NullKeyword: // do not produce a transformed statement for a null argument break; + case SyntaxKind.PropertyAccessExpression: case SyntaxKind.Identifier: // identifier includes undefined if (!argName) { // undefined was argument passed to promise handler break; } - const synthCall = factory.createCallExpression(getSynthesizedDeepClone(func as Identifier), /*typeArguments*/ undefined, isSynthIdentifier(argName) ? [argName.identifier] : []); + const synthCall = factory.createCallExpression(getSynthesizedDeepClone(func as Identifier | PropertyAccessExpression), /*typeArguments*/ undefined, isSynthIdentifier(argName) ? [argName.identifier] : []); if (shouldReturn(parent, transformer)) { return maybeAnnotateAndReturn(synthCall, parent.typeArguments?.[0]); } @@ -410,7 +411,7 @@ namespace ts.codefix { for (const statement of funcBody.statements) { if (isReturnStatement(statement)) { seenReturnStatement = true; - if (isReturnStatementWithFixablePromiseHandler(statement)) { + if (isReturnStatementWithFixablePromiseHandler(statement, transformer.checker)) { refactoredStmts = refactoredStmts.concat(getInnerTransformationBody(transformer, [statement], prevArgName)); } else { @@ -432,7 +433,7 @@ namespace ts.codefix { seenReturnStatement); } else { - const innerRetStmts = isFixablePromiseHandler(funcBody) ? [factory.createReturnStatement(funcBody)] : emptyArray; + const innerRetStmts = isFixablePromiseHandler(funcBody, transformer.checker) ? [factory.createReturnStatement(funcBody)] : emptyArray; const innerCbBody = getInnerTransformationBody(transformer, innerRetStmts, prevArgName); if (innerCbBody.length > 0) { @@ -536,6 +537,9 @@ namespace ts.codefix { else if (isIdentifier(funcNode)) { name = getMapEntryOrDefault(funcNode); } + else if (isPropertyAccessExpression(funcNode) && isIdentifier(funcNode.name)) { + name = getMapEntryOrDefault(funcNode.name); + } // return undefined argName when arg is null or undefined // eslint-disable-next-line no-in-operator diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 8d557d4edcb..a002f8533e2 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -114,7 +114,7 @@ namespace ts { return !isAsyncFunction(node) && node.body && isBlock(node.body) && - hasReturnStatementWithPromiseHandler(node.body) && + hasReturnStatementWithPromiseHandler(node.body, checker) && returnsPromise(node, checker); } @@ -129,25 +129,25 @@ namespace ts { return isBinaryExpression(commonJsModuleIndicator) ? commonJsModuleIndicator.left : commonJsModuleIndicator; } - function hasReturnStatementWithPromiseHandler(body: Block): boolean { - return !!forEachReturnStatement(body, isReturnStatementWithFixablePromiseHandler); + function hasReturnStatementWithPromiseHandler(body: Block, checker: TypeChecker): boolean { + return !!forEachReturnStatement(body, statement => isReturnStatementWithFixablePromiseHandler(statement, checker)); } - export function isReturnStatementWithFixablePromiseHandler(node: Node): node is ReturnStatement & { expression: CallExpression } { - return isReturnStatement(node) && !!node.expression && isFixablePromiseHandler(node.expression); + export function isReturnStatementWithFixablePromiseHandler(node: Node, checker: TypeChecker): node is ReturnStatement & { expression: CallExpression } { + return isReturnStatement(node) && !!node.expression && isFixablePromiseHandler(node.expression, checker); } // Should be kept up to date with transformExpression in convertToAsyncFunction.ts - export function isFixablePromiseHandler(node: Node): boolean { + export function isFixablePromiseHandler(node: Node, checker: TypeChecker): boolean { // ensure outermost call exists and is a promise handler - if (!isPromiseHandler(node) || !node.arguments.every(isFixablePromiseArgument)) { + if (!isPromiseHandler(node) || !node.arguments.every(arg => isFixablePromiseArgument(arg, checker))) { return false; } // ensure all chained calls are valid let currentNode = node.expression; while (isPromiseHandler(currentNode) || isPropertyAccessExpression(currentNode)) { - if (isCallExpression(currentNode) && !currentNode.arguments.every(isFixablePromiseArgument)) { + if (isCallExpression(currentNode) && !currentNode.arguments.every(arg => isFixablePromiseArgument(arg, checker))) { return false; } currentNode = currentNode.expression; @@ -171,7 +171,7 @@ namespace ts { } // should be kept up to date with getTransformationBody in convertToAsyncFunction.ts - function isFixablePromiseArgument(arg: Expression): boolean { + function isFixablePromiseArgument(arg: Expression, checker: TypeChecker): boolean { switch (arg.kind) { case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: @@ -179,8 +179,16 @@ namespace ts { visitedNestedConvertibleFunctions.set(getKeyFromNode(arg as FunctionLikeDeclaration), true); // falls through case SyntaxKind.NullKeyword: - case SyntaxKind.Identifier: // identifier includes undefined return true; + case SyntaxKind.Identifier: + case SyntaxKind.PropertyAccessExpression: { + const symbol = checker.getSymbolAtLocation(arg); + if (!symbol) { + return false; + } + return checker.isUndefinedSymbol(symbol) || + some(skipAlias(symbol, checker).declarations, d => isFunctionLike(d) || hasInitializer(d) && !!d.initializer && isFunctionLike(d.initializer)); + } default: return false; } diff --git a/src/testRunner/unittests/services/convertToAsyncFunction.ts b/src/testRunner/unittests/services/convertToAsyncFunction.ts index 5b09aba9549..0a36a9ebfcf 100644 --- a/src/testRunner/unittests/services/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/services/convertToAsyncFunction.ts @@ -539,6 +539,7 @@ function [#|f|]():Promise { } ` ); + _testConvertToAsyncFunction("convertToAsyncFunction_NoRes3", ` function [#|f|]():Promise { return fetch('https://typescriptlang.org').catch(rej => console.log(rej)); @@ -600,6 +601,7 @@ function [#|f|]():Promise { } ` ); + _testConvertToAsyncFunction("convertToAsyncFunction_ResRef", ` function [#|f|]():Promise { return fetch('https://typescriptlang.org').then(res); @@ -609,6 +611,75 @@ function res(result){ } ` ); + + _testConvertToAsyncFunction("convertToAsyncFunction_ResRef1", ` +class Foo { + public [#|method|](): Promise { + return fetch('a').then(this.foo); + } + + private foo(res) { + return res.ok; + } +} + `); + + _testConvertToAsyncFunction("convertToAsyncFunction_ResRef2", ` +class Foo { + public [#|method|](): Promise { + return fetch('a').then(this.foo); + } + + private foo = res => res; +} + `); + + _testConvertToAsyncFunction("convertToAsyncFunction_ResRef3", ` +const res = (result) => { + return result.ok; +} +function [#|f|](): Promise { + return fetch('https://typescriptlang.org').then(res); +} + ` + ); + + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestionResRef1", ` +const res = 1; +function [#|f|]() { + return fetch('https://typescriptlang.org').then(res); +} +` + ); + + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestionResRef2", ` +class Foo { + private foo = 1; + public [#|method|](): Promise { + return fetch('a').then(this.foo); + } +} +` + ); + + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestionResRef3", ` +const res = undefined; +function [#|f|]() { + return fetch('https://typescriptlang.org').then(res); +} +` + ); + + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestionResRef4", ` +class Foo { + private foo = undefined; + public [#|method|](): Promise { + return fetch('a').then(this.foo); + } +} +` + ); + _testConvertToAsyncFunction("convertToAsyncFunction_ResRefNoReturnVal", ` function [#|f|]():Promise { return fetch('https://typescriptlang.org').then(res); @@ -618,6 +689,19 @@ function res(result){ } ` ); + + _testConvertToAsyncFunction("convertToAsyncFunction_ResRefNoReturnVal1", ` +class Foo { + public [#|method|](): Promise { + return fetch('a').then(this.foo); + } + + private foo(res) { + console.log(res); + } +} + `); + _testConvertToAsyncFunction("convertToAsyncFunction_NoBrackets", ` function [#|f|]():Promise { return fetch('https://typescriptlang.org').then(result => console.log(result)); diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef1.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef1.ts new file mode 100644 index 00000000000..b9041b7588e --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef1.ts @@ -0,0 +1,25 @@ +// ==ORIGINAL== + +class Foo { + public /*[#|*/method/*|]*/(): Promise { + return fetch('a').then(this.foo); + } + + private foo(res) { + return res.ok; + } +} + +// ==ASYNC FUNCTION::Convert to async function== + +class Foo { + public async method(): Promise { + const res = await fetch('a'); + return this.foo(res); + } + + private foo(res) { + return res.ok; + } +} + \ No newline at end of file diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef2.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef2.ts new file mode 100644 index 00000000000..e67729fdcf3 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef2.ts @@ -0,0 +1,21 @@ +// ==ORIGINAL== + +class Foo { + public /*[#|*/method/*|]*/(): Promise { + return fetch('a').then(this.foo); + } + + private foo = res => res; +} + +// ==ASYNC FUNCTION::Convert to async function== + +class Foo { + public async method(): Promise { + const res = await fetch('a'); + return this.foo(res); + } + + private foo = res => res; +} + \ No newline at end of file diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef3.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef3.ts new file mode 100644 index 00000000000..07a6b17cf2f --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRef3.ts @@ -0,0 +1,19 @@ +// ==ORIGINAL== + +const res = (result) => { + return result.ok; +} +function /*[#|*/f/*|]*/(): Promise { + return fetch('https://typescriptlang.org').then(res); +} + +// ==ASYNC FUNCTION::Convert to async function== + +const res = (result) => { + return result.ok; +} +async function f(): Promise { + const result = await fetch('https://typescriptlang.org'); + return res(result); +} + \ No newline at end of file diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRefNoReturnVal1.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRefNoReturnVal1.ts new file mode 100644 index 00000000000..23c41b28bda --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRefNoReturnVal1.ts @@ -0,0 +1,25 @@ +// ==ORIGINAL== + +class Foo { + public /*[#|*/method/*|]*/(): Promise { + return fetch('a').then(this.foo); + } + + private foo(res) { + console.log(res); + } +} + +// ==ASYNC FUNCTION::Convert to async function== + +class Foo { + public async method(): Promise { + const res = await fetch('a'); + return this.foo(res); + } + + private foo(res) { + console.log(res); + } +} + \ No newline at end of file