From b484370dcc2c4d65625974ff27413976799ddb8b Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Tue, 18 Sep 2018 09:34:08 -0700 Subject: [PATCH 1/2] Clean up for type precision and clarity --- .../codefixes/convertToAsyncFunction.ts | 20 +++++++++++-------- .../unittests/convertToAsyncFunction.ts | 6 ++++++ .../convertToAsyncFunction_NoCatch.js | 15 ++++++++++++++ .../convertToAsyncFunction_NoCatch.ts | 15 ++++++++++++++ 4 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.js create mode 100644 tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.ts diff --git a/src/services/codefixes/convertToAsyncFunction.ts b/src/services/codefixes/convertToAsyncFunction.ts index 9998569b58540..01aaaae65caaa 100644 --- a/src/services/codefixes/convertToAsyncFunction.ts +++ b/src/services/codefixes/convertToAsyncFunction.ts @@ -372,7 +372,7 @@ namespace ts.codefix { // the identifier is empty when the handler (.then()) ignores the argument - In this situation we do not need to save the result of the promise returning call const originalNodeParent = node.original ? node.original.parent : node.parent; if (prevArgName && !shouldReturn && (!originalNodeParent || isPropertyAccessExpression(originalNodeParent))) { - return createTransformedStatement(prevArgName, createAwait(node), transformer).concat(); // hack to make the types match + return createTransformedStatement(prevArgName, createAwait(node), transformer); } else if (!prevArgName && !shouldReturn && (!originalNodeParent || isPropertyAccessExpression(originalNodeParent))) { return [createStatement(createAwait(node))]; @@ -381,7 +381,7 @@ namespace ts.codefix { return [createReturn(getSynthesizedDeepClone(node))]; } - function createTransformedStatement(prevArgName: SynthIdentifier | undefined, rightHandSide: Expression, transformer: Transformer): NodeArray { + function createTransformedStatement(prevArgName: SynthIdentifier | undefined, rightHandSide: Expression, transformer: Transformer): MutableNodeArray { if (!prevArgName || prevArgName.identifier.text.length === 0) { // if there's no argName to assign to, there still might be side effects return createNodeArray([createStatement(rightHandSide)]); @@ -405,7 +405,10 @@ namespace ts.codefix { // do not produce a transformed statement for a null argument break; case SyntaxKind.Identifier: // identifier includes undefined - if (!argName) break; + if (!argName) { + // undefined was argument passed to promise handler + break; + } const synthCall = createCall(getSynthesizedDeepClone(func) as Identifier, /*typeArguments*/ undefined, argName ? [argName.identifier] : []); if (shouldReturn) { @@ -533,7 +536,7 @@ namespace ts.codefix { return innerCbBody; } - function getArgName(funcNode: Node, transformer: Transformer): SynthIdentifier | undefined { + function getArgName(funcNode: Expression, transformer: Transformer): SynthIdentifier | undefined { const numberOfAssignmentsOriginal = 0; const types: Type[] = []; @@ -543,20 +546,21 @@ namespace ts.codefix { if (isFunctionLikeDeclaration(funcNode)) { if (funcNode.parameters.length > 0) { const param = funcNode.parameters[0].name as Identifier; - name = getMapEntryIfExists(param); + name = getMapEntryOrDefault(param); } } else if (isIdentifier(funcNode)) { - name = getMapEntryIfExists(funcNode); + name = getMapEntryOrDefault(funcNode); } - if (!name || name.identifier === undefined || name.identifier.text === "undefined") { + // return undefined argName when arg is null or undefined + if (!name || name.identifier.text === "undefined") { return undefined; } return name; - function getMapEntryIfExists(identifier: Identifier): SynthIdentifier { + function getMapEntryOrDefault(identifier: Identifier): SynthIdentifier { const originalNode = getOriginalNode(identifier); const symbol = getSymbol(originalNode); diff --git a/src/testRunner/unittests/convertToAsyncFunction.ts b/src/testRunner/unittests/convertToAsyncFunction.ts index 5eb49eaa4453c..fe2d99e196189 100644 --- a/src/testRunner/unittests/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/convertToAsyncFunction.ts @@ -487,6 +487,12 @@ function [#|f|]():Promise { function [#|f|]() { return fetch('https://typescriptlang.org').then(undefined, rejection => console.log("rejected:", rejection)); } +` + ); + _testConvertToAsyncFunction("convertToAsyncFunction_NoCatch", ` +function [#|f|]() { + return fetch('https://typescriptlang.org').then(x => x.statusText).catch(undefined); +} ` ); _testConvertToAsyncFunctionFailed("convertToAsyncFunction_NoSuggestion", ` diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.js new file mode 100644 index 0000000000000..25061a4dfb5a1 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.js @@ -0,0 +1,15 @@ +// ==ORIGINAL== + +function /*[#|*/f/*|]*/() { + return fetch('https://typescriptlang.org').then(x => x.statusText).catch(undefined); +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function f() { + try { + const x = await fetch('https://typescriptlang.org'); + return x.statusText; + } + catch (e) { } +} diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.ts new file mode 100644 index 0000000000000..25061a4dfb5a1 --- /dev/null +++ b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.ts @@ -0,0 +1,15 @@ +// ==ORIGINAL== + +function /*[#|*/f/*|]*/() { + return fetch('https://typescriptlang.org').then(x => x.statusText).catch(undefined); +} + +// ==ASYNC FUNCTION::Convert to async function== + +async function f() { + try { + const x = await fetch('https://typescriptlang.org'); + return x.statusText; + } + catch (e) { } +} From b850b3b88fe3fcc16c8c9a4aab3fcb789dcc0095 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Tue, 18 Sep 2018 10:26:12 -0700 Subject: [PATCH 2/2] Update test name --- src/testRunner/unittests/convertToAsyncFunction.ts | 2 +- ...tion_NoCatch.js => convertToAsyncFunction_NoCatchHandler.js} | 0 ...tion_NoCatch.ts => convertToAsyncFunction_NoCatchHandler.ts} | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename tests/baselines/reference/convertToAsyncFunction/{convertToAsyncFunction_NoCatch.js => convertToAsyncFunction_NoCatchHandler.js} (100%) rename tests/baselines/reference/convertToAsyncFunction/{convertToAsyncFunction_NoCatch.ts => convertToAsyncFunction_NoCatchHandler.ts} (100%) diff --git a/src/testRunner/unittests/convertToAsyncFunction.ts b/src/testRunner/unittests/convertToAsyncFunction.ts index fe2d99e196189..88ea3d04aae6f 100644 --- a/src/testRunner/unittests/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/convertToAsyncFunction.ts @@ -489,7 +489,7 @@ function [#|f|]() { } ` ); - _testConvertToAsyncFunction("convertToAsyncFunction_NoCatch", ` + _testConvertToAsyncFunction("convertToAsyncFunction_NoCatchHandler", ` function [#|f|]() { return fetch('https://typescriptlang.org').then(x => x.statusText).catch(undefined); } diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.js b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatchHandler.js similarity index 100% rename from tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.js rename to tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatchHandler.js diff --git a/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.ts b/tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatchHandler.ts similarity index 100% rename from tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatch.ts rename to tests/baselines/reference/convertToAsyncFunction/convertToAsyncFunction_NoCatchHandler.ts