From 8cd35f3a14a9edc0fe1485f5c0be93c8e100d2d6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 24 Mar 2020 15:29:05 -0700 Subject: [PATCH 1/3] Handle two-argument .thens correctly --- .../codefixes/convertToAsyncFunction.ts | 57 +++++++++++++++---- .../services/convertToAsyncFunction.ts | 29 ++++++++++ .../convertToAsyncFunction_CatchAndRej.ts | 6 +- .../convertToAsyncFunction_CatchAndRejRef.ts | 5 +- .../convertToAsyncFunction_Rej.ts | 6 +- .../convertToAsyncFunction_RejNoBrackets.ts | 5 +- .../convertToAsyncFunction_RejRef.ts | 5 +- ...onvertToAsyncFunction_ResRejNoArgsArrow.js | 2 +- ...onvertToAsyncFunction_ResRejNoArgsArrow.ts | 2 +- .../convertToAsyncFunction_twoArgumentThen.js | 22 +++++++ .../convertToAsyncFunction_twoArgumentThen.ts | 22 +++++++ ...tion_twoArgumentThen_onRejectedNoReturn.js | 22 +++++++ ...tion_twoArgumentThen_onRejectedNoReturn.ts | 22 +++++++ ...unction_twoArgumentThen_onRejectedThrow.js | 24 ++++++++ ...unction_twoArgumentThen_onRejectedThrow.ts | 24 ++++++++ 15 files changed, 230 insertions(+), 23 deletions(-) create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.js create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.ts create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.js create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.ts create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.js create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.ts diff --git a/src/services/codefixes/convertToAsyncFunction.ts b/src/services/codefixes/convertToAsyncFunction.ts index 11f97bbb7f297..83951b1a8b575 100644 --- a/src/services/codefixes/convertToAsyncFunction.ts +++ b/src/services/codefixes/convertToAsyncFunction.ts @@ -282,7 +282,7 @@ namespace ts.codefix { // We are about to write a 'let' variable declaration, but `transformExpression` for both // the try block and catch block will assign to this name. Setting this flag indicates // that future assignments should be written as `name = value` instead of `const name = value`. - possibleNameForVarDecl.hasBeenDeclared = true; + setHasBeenDeclared(possibleNameForVarDecl); } const tryBlock = createBlock(transformExpression(node.expression, transformer, possibleNameForVarDecl)); @@ -319,22 +319,43 @@ namespace ts.codefix { function transformThen(node: CallExpression, transformer: Transformer, prevArgName?: SynthBindingName): readonly Statement[] { const [onFulfilled, onRejected] = node.arguments; const onFulfilledArgumentName = getArgBindingName(onFulfilled, transformer); - const transformationBody = getTransformationBody(onFulfilled, prevArgName, onFulfilledArgumentName, node, transformer); - if (onRejected) { + // From: + // x().then(onFulfilledArg => { /* on fulfilled */ }, onRejectedArg => { /* on rejected */ }) + // + // To: + // let onFulfilledArg; + // try { + // onFulfilledArg = await x(); + // } catch (onRejectedArg) { + // /* on rejected */ + // return; + // } + // /* on fulfilled */ const onRejectedArgumentName = getArgBindingName(onRejected, transformer); - const tryBlock = createBlock(transformExpression(node.expression, transformer, onFulfilledArgumentName).concat(transformationBody)); - const transformationBody2 = getTransformationBody(onRejected, prevArgName, onRejectedArgumentName, node, transformer); + const onFulfilledArgType = onFulfilledArgumentName && !transformer.isInJSFile ? transformer.checker.typeToTypeNode(transformer.checker.getTypeAtLocation(getNode(onFulfilledArgumentName))) : undefined; + const onFulfilledArgVariableDeclaration = createVariableOrAssignmentOrExpressionStatement(onFulfilledArgumentName, /*rightHandSide*/ undefined, onFulfilledArgType); + const onFulfilledBody = getTransformationBody(onFulfilled, prevArgName, onFulfilledArgumentName, node, transformer); + const tryBlock = createBlock(transformExpression(node.expression, transformer, onFulfilledArgumentName)); + const onRejectedBody = ensureReturnOrThrow(getTransformationBody(onRejected, prevArgName, onRejectedArgumentName, node, transformer)); const catchArg = onRejectedArgumentName ? isSynthIdentifier(onRejectedArgumentName) ? onRejectedArgumentName.identifier.text : onRejectedArgumentName.bindingPattern : "e"; const catchVariableDeclaration = createVariableDeclaration(catchArg); - const catchClause = createCatchClause(catchVariableDeclaration, createBlock(transformationBody2)); - - return [createTry(tryBlock, catchClause, /* finallyBlock */ undefined)]; + const catchClause = createCatchClause(catchVariableDeclaration, createBlock(onRejectedBody)); + return onFulfilledArgVariableDeclaration.concat(createTry(tryBlock, catchClause, /* finallyBlock */ undefined), onFulfilledBody); } + const transformationBody = getTransformationBody(onFulfilled, prevArgName, onFulfilledArgumentName, node, transformer); return transformExpression(node.expression, transformer, onFulfilledArgumentName).concat(transformationBody); } + function ensureReturnOrThrow(statements: readonly Statement[]): readonly Statement[] { + const lastStatement = lastOrUndefined(statements); + if (lastStatement?.kind !== SyntaxKind.ReturnStatement && lastStatement?.kind !== SyntaxKind.ThrowStatement) { + return statements.concat(createReturn()); + } + return statements; + } + /** * Transforms the 'x' part of `x.then(...)`, or the 'y()' part of `y().catch(...)`, where 'x' and 'y()' are Promises. */ @@ -346,17 +367,19 @@ namespace ts.codefix { return createVariableOrAssignmentOrExpressionStatement(prevArgName, createAwait(node), /*typeAnnotation*/ undefined); } - function createVariableOrAssignmentOrExpressionStatement(variableName: SynthBindingName | undefined, rightHandSide: Expression, typeAnnotation: TypeNode | undefined): readonly Statement[] { + function createVariableOrAssignmentOrExpressionStatement(variableName: SynthBindingName | undefined, rightHandSide: Expression | undefined, typeAnnotation: TypeNode | undefined): readonly Statement[] { if (!variableName || isEmptyBindingName(variableName)) { // if there's no argName to assign to, there still might be side effects - return [createExpressionStatement(rightHandSide)]; + return rightHandSide ? [createExpressionStatement(rightHandSide)] : emptyArray; } if (isSynthIdentifier(variableName) && variableName.hasBeenDeclared) { // if the variable has already been declared, we don't need "let" or "const" + Debug.assertIsDefined(rightHandSide); return [createExpressionStatement(createAssignment(getSynthesizedDeepClone(variableName.identifier), rightHandSide))]; } + setHasBeenDeclared(variableName); return [ createVariableStatement( /*modifiers*/ undefined, @@ -365,7 +388,7 @@ namespace ts.codefix { getSynthesizedDeepClone(getNode(variableName)), typeAnnotation, rightHandSide)], - NodeFlags.Const))]; + rightHandSide ? NodeFlags.Const : NodeFlags.Let))]; } function maybeAnnotateAndReturn(expressionToReturn: Expression | undefined, typeAnnotation: TypeNode | undefined): readonly Statement[] { @@ -589,6 +612,18 @@ namespace ts.codefix { return every(bindingName.elements, isEmptyBindingName); } + function setHasBeenDeclared(bindingName: SynthBindingName) { + if (isSynthIdentifier(bindingName)) { + setIdentifierHasBeenDeclared(bindingName); + } + else { + bindingName.elements.forEach(setHasBeenDeclared); + } + function setIdentifierHasBeenDeclared(identifier: SynthIdentifier) { + identifier.hasBeenDeclared = true; + } + } + function getNode(bindingName: SynthBindingName) { return isSynthIdentifier(bindingName) ? bindingName.identifier : bindingName.bindingPattern; } diff --git a/src/testRunner/unittests/services/convertToAsyncFunction.ts b/src/testRunner/unittests/services/convertToAsyncFunction.ts index b0df94c7dba9e..86968a2258aa6 100644 --- a/src/testRunner/unittests/services/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/services/convertToAsyncFunction.ts @@ -1426,6 +1426,35 @@ function [#|get|]() { .resolve>({ success: true, data: { email: "" } }) .catch>(() => ({ success: false })); } +`); + + _testConvertToAsyncFunction("convertToAsyncFunction_twoArgumentThen", ` +function [#|fSync|]() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => null); +}; +`); + + _testConvertToAsyncFunction("convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn", ` +function [#|fSync|]() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => {}); +}; +`); + + _testConvertToAsyncFunction("convertToAsyncFunction_twoArgumentThen_onRejectedThrow", ` +function [#|fSync|]() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => { + throw new Error('Thrown from onRejected'); + }); +}; `); }); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRej.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRej.ts index 247191aa1fb0a..b51571619616d 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRej.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRej.ts @@ -7,13 +7,15 @@ function /*[#|*/f/*|]*/():Promise { async function f():Promise { try { + let result: Response; try { - const result = await fetch('https://typescriptlang.org'); - console.log(result); + result = await fetch('https://typescriptlang.org'); } catch (rejection) { console.log("rejected:", rejection); + return; } + console.log(result); } catch (err) { console.log(err); diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRejRef.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRejRef.ts index 883da342a6195..abfdbd6413abd 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRejRef.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRejRef.ts @@ -16,13 +16,14 @@ function catch_err(err){ async function f():Promise { try { + let result: any; try { - const result = await fetch('https://typescriptlang.org'); - return res(result); + result = await fetch('https://typescriptlang.org'); } catch (rejection) { return rej(rejection); } + return res(result); } catch (err) { return catch_err(err); diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Rej.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Rej.ts index bf3ee9c420cd6..3d91e26a06ddd 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Rej.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_Rej.ts @@ -7,11 +7,13 @@ function /*[#|*/f/*|]*/():Promise { // ==ASYNC FUNCTION::Convert to async function== async function f():Promise { + let result: Response; try { - const result = await fetch('https://typescriptlang.org'); - console.log(result); + result = await fetch('https://typescriptlang.org'); } catch (rejection) { console.log("rejected:", rejection); + return; } + console.log(result); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejNoBrackets.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejNoBrackets.ts index 8169745f5bde9..596890ab100e2 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejNoBrackets.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejNoBrackets.ts @@ -7,11 +7,12 @@ function /*[#|*/f/*|]*/():Promise { // ==ASYNC FUNCTION::Convert to async function== async function f():Promise { + let result: Response; try { - const result = await fetch('https://typescriptlang.org'); - return console.log(result); + result = await fetch('https://typescriptlang.org'); } catch (rejection) { return console.log("rejected:", rejection); } + return console.log(result); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts index 77fadff7ee993..0103d3c645cdf 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts @@ -13,13 +13,14 @@ function rej(err){ // ==ASYNC FUNCTION::Convert to async function== async function f():Promise { + let result: any; try { - const result = await fetch('https://typescriptlang.org'); - return res(result); + result = await fetch('https://typescriptlang.org'); } catch (err) { return rej(err); } + return res(result); } function res(result){ console.log(result); diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.js index 0b4a430f5058f..5390b2be40015 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.js @@ -9,9 +9,9 @@ async function f() { try { await Promise.resolve(); - return 1; } catch (e) { return "a"; } + return 1; } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.ts index 0b4a430f5058f..5390b2be40015 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_ResRejNoArgsArrow.ts @@ -9,9 +9,9 @@ async function f() { try { await Promise.resolve(); - return 1; } catch (e) { return "a"; } + return 1; } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.js new file mode 100644 index 0000000000000..00728e2f7c233 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.js @@ -0,0 +1,22 @@ +// ==ORIGINAL== + +function /*[#|*/fSync/*|]*/() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => null); +}; + +// ==ASYNC FUNCTION::Convert to async function== + +async function fSync() { + let x; + try { + x = await Promise.resolve(0); + } + catch (e) { + return null; + } + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); +}; diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.ts new file mode 100644 index 0000000000000..7b87f30eac044 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.ts @@ -0,0 +1,22 @@ +// ==ORIGINAL== + +function /*[#|*/fSync/*|]*/() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => null); +}; + +// ==ASYNC FUNCTION::Convert to async function== + +async function fSync() { + let x: number; + try { + x = await Promise.resolve(0); + } + catch (e) { + return null; + } + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); +}; diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.js new file mode 100644 index 0000000000000..b2efd1a97e8c0 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.js @@ -0,0 +1,22 @@ +// ==ORIGINAL== + +function /*[#|*/fSync/*|]*/() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => {}); +}; + +// ==ASYNC FUNCTION::Convert to async function== + +async function fSync() { + let x; + try { + x = await Promise.resolve(0); + } + catch (e) { + return; + } + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); +}; diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.ts new file mode 100644 index 0000000000000..0045bdd3db8d2 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.ts @@ -0,0 +1,22 @@ +// ==ORIGINAL== + +function /*[#|*/fSync/*|]*/() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => {}); +}; + +// ==ASYNC FUNCTION::Convert to async function== + +async function fSync() { + let x: number; + try { + x = await Promise.resolve(0); + } + catch (e) { + return; + } + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); +}; diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.js new file mode 100644 index 0000000000000..126700f27a1c8 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.js @@ -0,0 +1,24 @@ +// ==ORIGINAL== + +function /*[#|*/fSync/*|]*/() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => { + throw new Error('Thrown from onRejected'); + }); +}; + +// ==ASYNC FUNCTION::Convert to async function== + +async function fSync() { + let x; + try { + x = await Promise.resolve(0); + } + catch (e) { + throw new Error('Thrown from onRejected'); + } + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); +}; diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.ts new file mode 100644 index 0000000000000..e9c94e8c29598 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.ts @@ -0,0 +1,24 @@ +// ==ORIGINAL== + +function /*[#|*/fSync/*|]*/() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => { + throw new Error('Thrown from onRejected'); + }); +}; + +// ==ASYNC FUNCTION::Convert to async function== + +async function fSync() { + let x: number; + try { + x = await Promise.resolve(0); + } + catch (e) { + throw new Error('Thrown from onRejected'); + } + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); +}; From 43d379b07ef3197ea45fc407bdcd4ff6f8aa824b Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 24 Mar 2020 15:37:11 -0700 Subject: [PATCH 2/3] =?UTF-8?q?Don=E2=80=99t=20write=20an=20explicit=20`an?= =?UTF-8?q?y`=20annotation=20so=20control=20flow=20typing=20can=20work?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/services/codefixes/convertToAsyncFunction.ts | 5 +++-- .../convertToAsyncFunction_CatchAndRejRef.ts | 2 +- .../convertToAsyncFunction/convertToAsyncFunction_RejRef.ts | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/services/codefixes/convertToAsyncFunction.ts b/src/services/codefixes/convertToAsyncFunction.ts index 83951b1a8b575..2a5301db4af58 100644 --- a/src/services/codefixes/convertToAsyncFunction.ts +++ b/src/services/codefixes/convertToAsyncFunction.ts @@ -333,8 +333,9 @@ namespace ts.codefix { // } // /* on fulfilled */ const onRejectedArgumentName = getArgBindingName(onRejected, transformer); - const onFulfilledArgType = onFulfilledArgumentName && !transformer.isInJSFile ? transformer.checker.typeToTypeNode(transformer.checker.getTypeAtLocation(getNode(onFulfilledArgumentName))) : undefined; - const onFulfilledArgVariableDeclaration = createVariableOrAssignmentOrExpressionStatement(onFulfilledArgumentName, /*rightHandSide*/ undefined, onFulfilledArgType); + const onFulfilledArgType = onFulfilledArgumentName && !transformer.isInJSFile && transformer.checker.getTypeAtLocation(getNode(onFulfilledArgumentName)); + const onFulfilledArgTypeNode = onFulfilledArgType && !(onFulfilledArgType.flags & TypeFlags.Any) ? transformer.checker.typeToTypeNode(onFulfilledArgType) : undefined; + const onFulfilledArgVariableDeclaration = createVariableOrAssignmentOrExpressionStatement(onFulfilledArgumentName, /*rightHandSide*/ undefined, onFulfilledArgTypeNode); const onFulfilledBody = getTransformationBody(onFulfilled, prevArgName, onFulfilledArgumentName, node, transformer); const tryBlock = createBlock(transformExpression(node.expression, transformer, onFulfilledArgumentName)); const onRejectedBody = ensureReturnOrThrow(getTransformationBody(onRejected, prevArgName, onRejectedArgumentName, node, transformer)); diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRejRef.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRejRef.ts index abfdbd6413abd..c337f1366c108 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRejRef.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_CatchAndRejRef.ts @@ -16,7 +16,7 @@ function catch_err(err){ async function f():Promise { try { - let result: any; + let result; try { result = await fetch('https://typescriptlang.org'); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts index 0103d3c645cdf..21e910f88d73d 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_RejRef.ts @@ -13,7 +13,7 @@ function rej(err){ // ==ASYNC FUNCTION::Convert to async function== async function f():Promise { - let result: any; + let result; try { result = await fetch('https://typescriptlang.org'); } From cc7bc713c44409e2d7c183b39dbc7970b29e5b7d Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 25 Mar 2020 10:41:38 -0700 Subject: [PATCH 3/3] Disable refactor when two-argument `then` has another chained `then` --- src/services/suggestionDiagnostics.ts | 12 +++++++++--- .../services/convertToAsyncFunction.ts | 19 ++++++++++++++++--- .../convertToAsyncFunction_twoArgumentThen.js | 4 ++-- .../convertToAsyncFunction_twoArgumentThen.ts | 4 ++-- ...tion_twoArgumentThen_onRejectedNoReturn.js | 4 ++-- ...tion_twoArgumentThen_onRejectedNoReturn.ts | 4 ++-- ...unction_twoArgumentThen_onRejectedThrow.js | 4 ++-- ...unction_twoArgumentThen_onRejectedThrow.ts | 4 ++-- 8 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index a80bd58c6d7d8..fd4df4a948ba3 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -145,20 +145,26 @@ namespace ts { } // ensure all chained calls are valid - let currentNode = node.expression; + let seenThen = node.expression.name.escapedText === "then"; + let currentNode: Node = node.expression; while (isPromiseHandler(currentNode) || isPropertyAccessExpression(currentNode)) { - if (isCallExpression(currentNode) && !currentNode.arguments.every(isFixablePromiseArgument)) { + if (isCallExpression(currentNode) && (!currentNode.arguments.every(isFixablePromiseArgument) || isThenCallWithTwoArguments(currentNode) && seenThen)) { return false; } + seenThen = seenThen || isPromiseHandler(currentNode) && currentNode.expression.name.escapedText === "then"; currentNode = currentNode.expression; } return true; } - function isPromiseHandler(node: Node): node is CallExpression { + function isPromiseHandler(node: Node): node is CallExpression & { expression: PropertyAccessExpression } { return isCallExpression(node) && (hasPropertyAccessExpressionWithName(node, "then") || hasPropertyAccessExpressionWithName(node, "catch")); } + function isThenCallWithTwoArguments(node: CallExpression) { + return hasPropertyAccessExpressionWithName(node, "then") && node.arguments.length > 1; + } + // should be kept up to date with getTransformationBody in convertToAsyncFunction.ts function isFixablePromiseArgument(arg: Expression): boolean { switch (arg.kind) { diff --git a/src/testRunner/unittests/services/convertToAsyncFunction.ts b/src/testRunner/unittests/services/convertToAsyncFunction.ts index 86968a2258aa6..f4149978800e8 100644 --- a/src/testRunner/unittests/services/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/services/convertToAsyncFunction.ts @@ -1434,7 +1434,7 @@ function [#|fSync|]() { console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); }, () => null); -}; +} `); _testConvertToAsyncFunction("convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn", ` @@ -1443,7 +1443,7 @@ function [#|fSync|]() { console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); }, () => {}); -}; +} `); _testConvertToAsyncFunction("convertToAsyncFunction_twoArgumentThen_onRejectedThrow", ` @@ -1454,7 +1454,20 @@ function [#|fSync|]() { }, () => { throw new Error('Thrown from onRejected'); }); -}; +} +`); + + _testConvertToAsyncFunctionFailed("convertToAsyncFunction_twoArgumentThen_chainedThen", ` +function [#|fSync|]() { + return Promise.resolve(0).then(x => { + console.log(x); // note: added to illustrate refactor + throw new Error('Failure!'); + }, () => { + throw new Error('Thrown from onRejected'); + }).then(() => { + return 0; + }); +} `); }); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.js index 00728e2f7c233..377cd84b0bd1e 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.js @@ -5,7 +5,7 @@ function /*[#|*/fSync/*|]*/() { console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); }, () => null); -}; +} // ==ASYNC FUNCTION::Convert to async function== @@ -19,4 +19,4 @@ async function fSync() { } console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); -}; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.ts index 7b87f30eac044..600491ba4e5b2 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen.ts @@ -5,7 +5,7 @@ function /*[#|*/fSync/*|]*/() { console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); }, () => null); -}; +} // ==ASYNC FUNCTION::Convert to async function== @@ -19,4 +19,4 @@ async function fSync() { } console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); -}; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.js index b2efd1a97e8c0..53ee0ae1011bc 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.js @@ -5,7 +5,7 @@ function /*[#|*/fSync/*|]*/() { console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); }, () => {}); -}; +} // ==ASYNC FUNCTION::Convert to async function== @@ -19,4 +19,4 @@ async function fSync() { } console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); -}; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.ts index 0045bdd3db8d2..e0d0303389ae4 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedNoReturn.ts @@ -5,7 +5,7 @@ function /*[#|*/fSync/*|]*/() { console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); }, () => {}); -}; +} // ==ASYNC FUNCTION::Convert to async function== @@ -19,4 +19,4 @@ async function fSync() { } console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); -}; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.js index 126700f27a1c8..3ff811f873a91 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.js +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.js @@ -7,7 +7,7 @@ function /*[#|*/fSync/*|]*/() { }, () => { throw new Error('Thrown from onRejected'); }); -}; +} // ==ASYNC FUNCTION::Convert to async function== @@ -21,4 +21,4 @@ async function fSync() { } console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); -}; +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.ts index e9c94e8c29598..d2f3e86a1980a 100644 --- a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.ts +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_twoArgumentThen_onRejectedThrow.ts @@ -7,7 +7,7 @@ function /*[#|*/fSync/*|]*/() { }, () => { throw new Error('Thrown from onRejected'); }); -}; +} // ==ASYNC FUNCTION::Convert to async function== @@ -21,4 +21,4 @@ async function fSync() { } console.log(x); // note: added to illustrate refactor throw new Error('Failure!'); -}; +}