From d1b705b8d8d886678c95b513e13f83ab84c270c2 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 23 Apr 2020 17:06:56 -0700 Subject: [PATCH 1/3] Disable convert to async for `.then` with both fulfillment and rejection handlers --- src/services/suggestionDiagnostics.ts | 24 ++++++++++++++++++- .../services/convertToAsyncFunction.ts | 8 +++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index a80bd58c6d7d8..a69de96ee8ffe 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -156,7 +156,29 @@ namespace ts { } function isPromiseHandler(node: Node): node is CallExpression { - return isCallExpression(node) && (hasPropertyAccessExpressionWithName(node, "then") || hasPropertyAccessExpressionWithName(node, "catch")); + return isCallExpression(node) && ( + hasPropertyAccessExpressionWithName(node, "then") && hasZeroOrOneNonNullArguments(node) || + hasPropertyAccessExpressionWithName(node, "catch")); + } + + function hasZeroOrOneNonNullArguments(node: CallExpression) { + if (node.arguments.length < 2) return true; + let seenArg = false; + for (const arg of node.arguments) { + switch (arg.kind) { + case SyntaxKind.NullKeyword: + continue; + case SyntaxKind.Identifier: + if ((arg as Identifier).escapedText === "undefined") { + continue; + } + // falls through + default: + if (seenArg) return false; + seenArg = true; + } + } + return true; } // should be kept up to date with getTransformationBody in convertToAsyncFunction.ts diff --git a/src/testRunner/unittests/services/convertToAsyncFunction.ts b/src/testRunner/unittests/services/convertToAsyncFunction.ts index 2a36231607561..b19de1f150b32 100644 --- a/src/testRunner/unittests/services/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/services/convertToAsyncFunction.ts @@ -558,13 +558,13 @@ function [#|f|]():void{ } ` ); - _testConvertToAsyncFunction("convertToAsyncFunction_Rej", ` + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_Rej", ` function [#|f|]():Promise { return fetch('https://typescriptlang.org').then(result => { console.log(result); }, rejection => { console.log("rejected:", rejection); }); } ` ); - _testConvertToAsyncFunction("convertToAsyncFunction_RejRef", ` + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_RejRef", ` function [#|f|]():Promise { return fetch('https://typescriptlang.org').then(res, rej); } @@ -576,7 +576,7 @@ function rej(err){ } ` ); - _testConvertToAsyncFunction("convertToAsyncFunction_RejNoBrackets", ` + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_RejNoBrackets", ` function [#|f|]():Promise { return fetch('https://typescriptlang.org').then(result => console.log(result), rejection => console.log("rejected:", rejection)); } @@ -1238,7 +1238,7 @@ function [#|f|]() { } `); - _testConvertToAsyncFunction("convertToAsyncFunction_ResRejNoArgsArrow", ` + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_ResRejNoArgsArrow", ` function [#|f|]() { return Promise.resolve().then(() => 1, () => "a"); } From 555cfd205ba3d2743bd001d2e6eafe0a103d4c73 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 23 Apr 2020 17:08:40 -0700 Subject: [PATCH 2/3] Delete baselines --- .../convertToAsyncFunction_Rej.ts | 17 ----------- .../convertToAsyncFunction_RejNoBrackets.ts | 17 ----------- .../convertToAsyncFunction_RejRef.ts | 29 ------------------- ...onvertToAsyncFunction_ResRejNoArgsArrow.js | 17 ----------- ...onvertToAsyncFunction_ResRejNoArgsArrow.ts | 17 ----------- 5 files changed, 97 deletions(-) delete mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Rej.ts delete mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejNoBrackets.ts delete mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts delete mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.js delete mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.ts diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Rej.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Rej.ts deleted file mode 100644 index bf3ee9c420cd6..0000000000000 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Rej.ts +++ /dev/null @@ -1,17 +0,0 @@ -// ==ORIGINAL== - -function /*[#|*/f/*|]*/():Promise { - return fetch('https://typescriptlang.org').then(result => { console.log(result); }, rejection => { console.log("rejected:", rejection); }); -} - -// ==ASYNC FUNCTION::Convert to async function== - -async function f():Promise { - try { - const result = await fetch('https://typescriptlang.org'); - console.log(result); - } - catch (rejection) { - console.log("rejected:", rejection); - } -} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejNoBrackets.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejNoBrackets.ts deleted file mode 100644 index 8169745f5bde9..0000000000000 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejNoBrackets.ts +++ /dev/null @@ -1,17 +0,0 @@ -// ==ORIGINAL== - -function /*[#|*/f/*|]*/():Promise { - return fetch('https://typescriptlang.org').then(result => console.log(result), rejection => console.log("rejected:", rejection)); -} - -// ==ASYNC FUNCTION::Convert to async function== - -async function f():Promise { - try { - const result = await fetch('https://typescriptlang.org'); - return console.log(result); - } - catch (rejection) { - return console.log("rejected:", rejection); - } -} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts deleted file mode 100644 index 77fadff7ee993..0000000000000 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts +++ /dev/null @@ -1,29 +0,0 @@ -// ==ORIGINAL== - -function /*[#|*/f/*|]*/():Promise { - return fetch('https://typescriptlang.org').then(res, rej); -} -function res(result){ - console.log(result); -} -function rej(err){ - console.log(err); -} - -// ==ASYNC FUNCTION::Convert to async function== - -async function f():Promise { - try { - const result = await fetch('https://typescriptlang.org'); - return res(result); - } - catch (err) { - return rej(err); - } -} -function res(result){ - console.log(result); -} -function rej(err){ - console.log(err); -} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.js deleted file mode 100644 index 0b4a430f5058f..0000000000000 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.js +++ /dev/null @@ -1,17 +0,0 @@ -// ==ORIGINAL== - - function /*[#|*/f/*|]*/() { - return Promise.resolve().then(() => 1, () => "a"); - } - -// ==ASYNC FUNCTION::Convert to async function== - - async function f() { - try { - await Promise.resolve(); - return 1; - } - catch (e) { - return "a"; - } - } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.ts deleted file mode 100644 index 0b4a430f5058f..0000000000000 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.ts +++ /dev/null @@ -1,17 +0,0 @@ -// ==ORIGINAL== - - function /*[#|*/f/*|]*/() { - return Promise.resolve().then(() => 1, () => "a"); - } - -// ==ASYNC FUNCTION::Convert to async function== - - async function f() { - try { - await Promise.resolve(); - return 1; - } - catch (e) { - return "a"; - } - } From e38f8fe3f81cdb7f1e4cf252e47b90b598e54565 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 24 Apr 2020 09:16:45 -0700 Subject: [PATCH 3/3] Also disable refactor for 3+ arguments --- src/services/suggestionDiagnostics.ts | 25 ++++++------------- .../services/convertToAsyncFunction.ts | 5 ++++ 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index a69de96ee8ffe..09607d8ab8a00 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -157,28 +157,17 @@ namespace ts { function isPromiseHandler(node: Node): node is CallExpression { return isCallExpression(node) && ( - hasPropertyAccessExpressionWithName(node, "then") && hasZeroOrOneNonNullArguments(node) || + hasPropertyAccessExpressionWithName(node, "then") && hasSupportedNumberOfArguments(node) || hasPropertyAccessExpressionWithName(node, "catch")); } - function hasZeroOrOneNonNullArguments(node: CallExpression) { + function hasSupportedNumberOfArguments(node: CallExpression) { + if (node.arguments.length > 2) return false; if (node.arguments.length < 2) return true; - let seenArg = false; - for (const arg of node.arguments) { - switch (arg.kind) { - case SyntaxKind.NullKeyword: - continue; - case SyntaxKind.Identifier: - if ((arg as Identifier).escapedText === "undefined") { - continue; - } - // falls through - default: - if (seenArg) return false; - seenArg = true; - } - } - return true; + return some(node.arguments, arg => { + return arg.kind === SyntaxKind.NullKeyword || + isIdentifier(arg) && arg.text === "undefined"; + }); } // should be kept up to date with getTransformationBody in convertToAsyncFunction.ts diff --git a/src/testRunner/unittests/services/convertToAsyncFunction.ts b/src/testRunner/unittests/services/convertToAsyncFunction.ts index b19de1f150b32..999e1580a98c3 100644 --- a/src/testRunner/unittests/services/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/services/convertToAsyncFunction.ts @@ -1436,5 +1436,10 @@ function [#|get|]() { .catch>(() => ({ success: false })); } `); + + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_threeArguments", ` +function [#|f|]() { + return Promise.resolve().then(undefined, undefined, () => 1); +}`); }); }