diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 5c175146e34..63691a72c8b 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -1,5 +1,7 @@ /* @internal */ namespace ts { + const visitedNestedConvertibleFunctions = createMap(); + export function computeSuggestionDiagnostics(sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): DiagnosticWithLocation[] { program.getSemanticDiagnostics(sourceFile, cancellationToken); const diags: DiagnosticWithLocation[] = []; @@ -13,6 +15,7 @@ namespace ts { const isJsFile = isSourceFileJS(sourceFile); + visitedNestedConvertibleFunctions.clear(); check(sourceFile); if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) { @@ -114,17 +117,22 @@ namespace ts { } function addConvertToAsyncFunctionDiagnostics(node: FunctionLikeDeclaration, checker: TypeChecker, diags: Push): void { - if (!isAsyncFunction(node) && - node.body && - isBlock(node.body) && - hasReturnStatementWithPromiseHandler(node.body) && - returnsPromise(node, checker)) { + // need to check function before checking map so that deeper levels of nested callbacks are checked + if (isConvertibleFunction(node, checker) && !visitedNestedConvertibleFunctions.has(getKeyFromNode(node))) { diags.push(createDiagnosticForNode( !node.name && isVariableDeclaration(node.parent) && isIdentifier(node.parent.name) ? node.parent.name : node, Diagnostics.This_may_be_converted_to_an_async_function)); } } + function isConvertibleFunction(node: FunctionLikeDeclaration, checker: TypeChecker) { + return !isAsyncFunction(node) && + node.body && + isBlock(node.body) && + hasReturnStatementWithPromiseHandler(node.body) && + returnsPromise(node, checker); + } + function returnsPromise(node: FunctionLikeDeclaration, checker: TypeChecker): boolean { const functionType = checker.getTypeAtLocation(node); const callSignatures = checker.getSignaturesOfType(functionType, SignatureKind.Call); @@ -169,14 +177,20 @@ namespace ts { // should be kept up to date with getTransformationBody in convertToAsyncFunction.ts function isFixablePromiseArgument(arg: Expression): boolean { switch (arg.kind) { - case SyntaxKind.NullKeyword: - case SyntaxKind.Identifier: // identifier includes undefined case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: case SyntaxKind.ArrowFunction: + visitedNestedConvertibleFunctions.set(getKeyFromNode(arg as FunctionLikeDeclaration), true); + /* falls through */ + case SyntaxKind.NullKeyword: + case SyntaxKind.Identifier: // identifier includes undefined return true; default: return false; } } + + function getKeyFromNode(exp: FunctionLikeDeclaration) { + return `${exp.pos.toString()}:${exp.end.toString()}`; + } } diff --git a/src/testRunner/unittests/services/convertToAsyncFunction.ts b/src/testRunner/unittests/services/convertToAsyncFunction.ts index b89e684183b..8c426effa4c 100644 --- a/src/testRunner/unittests/services/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/services/convertToAsyncFunction.ts @@ -255,7 +255,7 @@ interface String { charAt: any; } interface Array {}` }; - function testConvertToAsyncFunction(caption: string, text: string, baselineFolder: string, includeLib?: boolean, expectFailure = false) { + function testConvertToAsyncFunction(caption: string, text: string, baselineFolder: string, includeLib?: boolean, expectFailure = false, onlyProvideAction = false) { const t = extractTest(text); const selectionRange = t.ranges.get("selection")!; if (!selectionRange) { @@ -307,7 +307,7 @@ interface Array {}` const actions = codefix.getFixes(context); const action = find(actions, action => action.description === Diagnostics.Convert_to_async_function.message); - if (expectFailure) { + if (expectFailure && !onlyProvideAction) { assert.isNotTrue(action && action.changes.length > 0); return; } @@ -1151,25 +1151,25 @@ function [#|f|]() { _testConvertToAsyncFunction("convertToAsyncFunction_simpleFunctionExpression", ` const [#|foo|] = function () { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); -} +} `); _testConvertToAsyncFunction("convertToAsyncFunction_simpleFunctionExpressionWithName", ` const foo = function [#|f|]() { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); -} +} `); _testConvertToAsyncFunction("convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern", ` const { length } = [#|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); -} + return Promise.resolve().then(x => 1).catch(x => "a").then(x => !!x); +} `); _testConvertToAsyncFunction("convertToAsyncFunction_bindingPattern", ` @@ -1178,7 +1178,7 @@ function [#|f|]() { } function res({ status, trailer }){ console.log(status); -} +} `); _testConvertToAsyncFunction("convertToAsyncFunction_bindingPatternNameCollision", ` @@ -1188,7 +1188,7 @@ function [#|f|]() { } function res({ status, trailer }){ console.log(status); -} +} `); _testConvertToAsyncFunctionFailed("convertToAsyncFunction_thenArgumentNotFunction", ` @@ -1209,7 +1209,7 @@ function [#|f|]() { } function res(result) { return Promise.resolve().then(x => console.log(result)); -} +} `); _testConvertToAsyncFunction("convertToAsyncFunction_callbackReturnsPromise", ` @@ -1241,7 +1241,7 @@ function [#|f|]() { return Promise.resolve(1) .then(x => Promise.reject(x)) .catch(err => console.log(err)); -} +} `); _testConvertToAsyncFunction("convertToAsyncFunction_nestedPromises", ` @@ -1266,6 +1266,22 @@ _testConvertToAsyncFunction("convertToAsyncFunction_exportModifier", ` export function [#|foo|]() { return fetch('https://typescriptlang.org').then(s => console.log(s)); } +`); + +_testConvertToAsyncFunction("convertToAsyncFunction_OutermostOnlySuccess", ` +function [#|foo|]() { + return fetch('a').then(() => { + return fetch('b').then(() => 'c'); + }) +} +`); + +_testConvertToAsyncFunctionFailedSuggestion("convertToAsyncFunction_OutermostOnlyFailure", ` +function foo() { + return fetch('a').then([#|() => {|] + return fetch('b').then(() => 'c'); + }) +} `); }); @@ -1276,4 +1292,8 @@ export function [#|foo|]() { function _testConvertToAsyncFunctionFailed(caption: string, text: string) { testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", /*includeLib*/ true, /*expectFailure*/ true); } + + function _testConvertToAsyncFunctionFailedSuggestion(caption: string, text: string) { + testConvertToAsyncFunction(caption, text, "convertToAsyncFunction", /*includeLib*/ true, /*expectFailure*/ true, /*onlyProvideAction*/ true); + } } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlyFailure.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlyFailure.ts new file mode 100644 index 00000000000..8c3d9745fe8 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlyFailure.ts @@ -0,0 +1,16 @@ +// ==ORIGINAL== + +function foo() { + return fetch('a').then(/*[#|*/() => {/*|]*/ + return fetch('b').then(() => 'c'); + }) +} + +// ==ASYNC FUNCTION::Convert to async function== + +function foo() { + return fetch('a').then(async () => { + await fetch('b'); + return 'c'; + }) +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlySuccess.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlySuccess.js new file mode 100644 index 00000000000..d710dd925f8 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlySuccess.js @@ -0,0 +1,15 @@ +// ==ORIGINAL== + +function /*[#|*/foo/*|]*/() { + return fetch('a').then(() => { + return fetch('b').then(() => 'c'); + }) +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function foo() { + await fetch('a'); + await fetch('b'); + return 'c'; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlySuccess.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlySuccess.ts new file mode 100644 index 00000000000..d710dd925f8 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_OutermostOnlySuccess.ts @@ -0,0 +1,15 @@ +// ==ORIGINAL== + +function /*[#|*/foo/*|]*/() { + return fetch('a').then(() => { + return fetch('b').then(() => 'c'); + }) +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function foo() { + await fetch('a'); + await fetch('b'); + return 'c'; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.js index f06ce44e78b..45852e51aee 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.js @@ -5,7 +5,7 @@ function /*[#|*/f/*|]*/() { } function res({ status, trailer }){ console.log(status); -} +} // ==ASYNC FUNCTION::Convert to async function== @@ -15,4 +15,4 @@ async function f() { } 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 index f06ce44e78b..45852e51aee 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPattern.ts @@ -5,7 +5,7 @@ function /*[#|*/f/*|]*/() { } function res({ status, trailer }){ console.log(status); -} +} // ==ASYNC FUNCTION::Convert to async function== @@ -15,4 +15,4 @@ async function f() { } 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 index 6813472966a..40488dbe73c 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.js @@ -6,7 +6,7 @@ function /*[#|*/f/*|]*/() { } function res({ status, trailer }){ console.log(status); -} +} // ==ASYNC FUNCTION::Convert to async function== @@ -17,4 +17,4 @@ async function f() { } 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 index 6813472966a..40488dbe73c 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_bindingPatternNameCollision.ts @@ -6,7 +6,7 @@ function /*[#|*/f/*|]*/() { } function res({ status, trailer }){ console.log(status); -} +} // ==ASYNC FUNCTION::Convert to async function== @@ -17,4 +17,4 @@ async function f() { } function res({ status, trailer }){ console.log(status); -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_callbackReturnsRejectedPromiseInTryBlock.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_callbackReturnsRejectedPromiseInTryBlock.js index 2cfd6ba951e..bbbcee9d86b 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_callbackReturnsRejectedPromiseInTryBlock.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_callbackReturnsRejectedPromiseInTryBlock.js @@ -4,7 +4,7 @@ function /*[#|*/f/*|]*/() { return Promise.resolve(1) .then(x => Promise.reject(x)) .catch(err => console.log(err)); -} +} // ==ASYNC FUNCTION::Convert to async function== @@ -16,4 +16,4 @@ async function f() { catch (err) { return console.log(err); } -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_callbackReturnsRejectedPromiseInTryBlock.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_callbackReturnsRejectedPromiseInTryBlock.ts index 2cfd6ba951e..bbbcee9d86b 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_callbackReturnsRejectedPromiseInTryBlock.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_callbackReturnsRejectedPromiseInTryBlock.ts @@ -4,7 +4,7 @@ function /*[#|*/f/*|]*/() { return Promise.resolve(1) .then(x => Promise.reject(x)) .catch(err => console.log(err)); -} +} // ==ASYNC FUNCTION::Convert to async function== @@ -16,4 +16,4 @@ async function f() { catch (err) { return console.log(err); } -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.js index 2600adec16b..fa5d3babf72 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.js @@ -1,8 +1,8 @@ // ==ORIGINAL== function /*[#|*/f/*|]*/() { - return Promise.resolve().then(x => 1).catch(x => "a").then(x => !!x); -} + return Promise.resolve().then(x => 1).catch(x => "a").then(x => !!x); +} // ==ASYNC FUNCTION::Convert to async function== @@ -15,5 +15,5 @@ async function f() { catch (x_1) { x_2 = "a"; } - return !!x_2; -} + return !!x_2; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.ts index 5c4daf076a0..15ec7ce93cf 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_catchBlockUniqueParams.ts @@ -1,8 +1,8 @@ // ==ORIGINAL== function /*[#|*/f/*|]*/() { - return Promise.resolve().then(x => 1).catch(x => "a").then(x => !!x); -} + return Promise.resolve().then(x => 1).catch(x => "a").then(x => !!x); +} // ==ASYNC FUNCTION::Convert to async function== @@ -15,5 +15,5 @@ async function f() { catch (x_1) { x_2 = "a"; } - return !!x_2; -} + return !!x_2; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_runEffectfulContinuation.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_runEffectfulContinuation.js index 4753c67cdae..ffbf5c4e835 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_runEffectfulContinuation.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_runEffectfulContinuation.js @@ -5,7 +5,7 @@ function /*[#|*/f/*|]*/() { } function res(result) { return Promise.resolve().then(x => console.log(result)); -} +} // ==ASYNC FUNCTION::Convert to async function== @@ -16,4 +16,4 @@ async function f() { } function res(result) { return Promise.resolve().then(x => console.log(result)); -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_runEffectfulContinuation.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_runEffectfulContinuation.ts index 4753c67cdae..ffbf5c4e835 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_runEffectfulContinuation.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_runEffectfulContinuation.ts @@ -5,7 +5,7 @@ function /*[#|*/f/*|]*/() { } function res(result) { return Promise.resolve().then(x => console.log(result)); -} +} // ==ASYNC FUNCTION::Convert to async function== @@ -16,4 +16,4 @@ async function f() { } function res(result) { return Promise.resolve().then(x => console.log(result)); -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpression.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpression.js index a92497ca937..d3696bcf039 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpression.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpression.js @@ -2,11 +2,11 @@ const /*[#|*/foo/*|]*/ = function () { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); -} +} // ==ASYNC FUNCTION::Convert to async function== const foo = async function () { const result = await fetch('https://typescriptlang.org'); console.log(result); -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpression.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpression.ts index a92497ca937..d3696bcf039 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpression.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpression.ts @@ -2,11 +2,11 @@ const /*[#|*/foo/*|]*/ = function () { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); -} +} // ==ASYNC FUNCTION::Convert to async function== const foo = async function () { const result = await fetch('https://typescriptlang.org'); console.log(result); -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern.js index b34c369046f..9f108e9e2a4 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern.js @@ -2,11 +2,11 @@ const { length } = /*[#|*/function/*|]*/ () { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); -} +} // ==ASYNC FUNCTION::Convert to async function== const { length } = async function () { const result = await fetch('https://typescriptlang.org'); console.log(result); -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern.ts index b34c369046f..9f108e9e2a4 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionAssignedToBindingPattern.ts @@ -2,11 +2,11 @@ const { length } = /*[#|*/function/*|]*/ () { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); -} +} // ==ASYNC FUNCTION::Convert to async function== const { length } = async function () { const result = await fetch('https://typescriptlang.org'); console.log(result); -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionWithName.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionWithName.js index 8316ee98b5b..155f28eae83 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionWithName.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionWithName.js @@ -2,11 +2,11 @@ const foo = function /*[#|*/f/*|]*/() { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); -} +} // ==ASYNC FUNCTION::Convert to async function== const foo = async function f() { const result = await fetch('https://typescriptlang.org'); console.log(result); -} +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionWithName.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionWithName.ts index 8316ee98b5b..155f28eae83 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionWithName.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_simpleFunctionExpressionWithName.ts @@ -2,11 +2,11 @@ const foo = function /*[#|*/f/*|]*/() { return fetch('https://typescriptlang.org').then(result => { console.log(result) }); -} +} // ==ASYNC FUNCTION::Convert to async function== const foo = async function f() { const result = await fetch('https://typescriptlang.org'); console.log(result); -} +}