From cb57f17aba3d6d5a5e1cb9e2e9ba4d78f4736b7b Mon Sep 17 00:00:00 2001
From: Benjamin Lichtman <belichtm@microsoft.com>
Date: Mon, 31 Dec 2018 16:25:26 -0800
Subject: [PATCH] Simplify approach

---
 .../codefixes/convertToAsyncFunction.ts       | 10 ++++----
 src/services/suggestionDiagnostics.ts         | 25 +++++++++----------
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/services/codefixes/convertToAsyncFunction.ts b/src/services/codefixes/convertToAsyncFunction.ts
index daf1d89b39a..aae1d205de3 100644
--- a/src/services/codefixes/convertToAsyncFunction.ts
+++ b/src/services/codefixes/convertToAsyncFunction.ts
@@ -60,7 +60,7 @@ namespace ts.codefix {
         const setOfExpressionsToReturn = getAllPromiseExpressionsToReturn(functionToConvert, checker);
         const functionToConvertRenamed = renameCollidingVarNames(functionToConvert, checker, synthNamesMap, context, setOfExpressionsToReturn, originalTypeMap, allVarNames);
         const constIdentifiers = getConstIdentifiers(synthNamesMap);
-        const returnStatements = functionToConvertRenamed.body && isBlock(functionToConvertRenamed.body) ? getReturnStatementsWithPromiseHandlers(functionToConvertRenamed.body, checker) : emptyArray;
+        const returnStatements = functionToConvertRenamed.body && isBlock(functionToConvertRenamed.body) ? getReturnStatementsWithPromiseHandlers(functionToConvertRenamed.body) : emptyArray;
         const transformer: Transformer = { checker, synthNamesMap, allVarNames, setOfExpressionsToReturn, constIdentifiers, originalTypeMap, isInJSFile: isInJavascript };
 
         if (!returnStatements.length) {
@@ -87,10 +87,10 @@ namespace ts.codefix {
         }
     }
 
-    function getReturnStatementsWithPromiseHandlers(body: Block, checker: TypeChecker): ReadonlyArray<ReturnStatement> {
+    function getReturnStatementsWithPromiseHandlers(body: Block): ReadonlyArray<ReturnStatement> {
         const res: ReturnStatement[] = [];
         forEachReturnStatement(body, ret => {
-            if (isReturnStatementWithFixablePromiseHandler(ret, checker)) res.push(ret);
+            if (isReturnStatementWithFixablePromiseHandler(ret)) res.push(ret);
         });
         return res;
     }
@@ -450,7 +450,7 @@ namespace ts.codefix {
                             seenReturnStatement = true;
                         }
 
-                        if (isReturnStatementWithFixablePromiseHandler(statement, transformer.checker)) {
+                        if (isReturnStatementWithFixablePromiseHandler(statement)) {
                             refactoredStmts = refactoredStmts.concat(getInnerTransformationBody(transformer, [statement], prevArgName));
                         }
                         else {
@@ -466,7 +466,7 @@ namespace ts.codefix {
                             seenReturnStatement);
                 }
                 else {
-                    const innerRetStmts = isFixablePromiseHandler(funcBody, transformer.checker) ? [createReturn(funcBody)] : emptyArray;
+                    const innerRetStmts = isFixablePromiseHandler(funcBody) ? [createReturn(funcBody)] : emptyArray;
                     const innerCbBody = getInnerTransformationBody(transformer, innerRetStmts, prevArgName);
 
                     if (innerCbBody.length > 0) {
diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts
index b6783d79bae..63691a72c8b 100644
--- a/src/services/suggestionDiagnostics.ts
+++ b/src/services/suggestionDiagnostics.ts
@@ -117,7 +117,8 @@ namespace ts {
     }
 
     function addConvertToAsyncFunctionDiagnostics(node: FunctionLikeDeclaration, checker: TypeChecker, diags: Push<DiagnosticWithLocation>): void {
-        if (!visitedNestedConvertibleFunctions.has(getKeyFromNode(node)) && isConvertibleFunction(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));
@@ -128,7 +129,7 @@ namespace ts {
         return !isAsyncFunction(node) &&
             node.body &&
             isBlock(node.body) &&
-            hasReturnStatementWithPromiseHandler(node.body, checker) &&
+            hasReturnStatementWithPromiseHandler(node.body) &&
             returnsPromise(node, checker);
     }
 
@@ -143,25 +144,25 @@ namespace ts {
         return isBinaryExpression(commonJsModuleIndicator) ? commonJsModuleIndicator.left : commonJsModuleIndicator;
     }
 
-    function hasReturnStatementWithPromiseHandler(body: Block, checker: TypeChecker): boolean {
-        return !!forEachReturnStatement(body, stmt => isReturnStatementWithFixablePromiseHandler(stmt, checker));
+    function hasReturnStatementWithPromiseHandler(body: Block): boolean {
+        return !!forEachReturnStatement(body, isReturnStatementWithFixablePromiseHandler);
     }
 
-    export function isReturnStatementWithFixablePromiseHandler(node: Node, checker: TypeChecker): node is ReturnStatement {
-        return isReturnStatement(node) && !!node.expression && isFixablePromiseHandler(node.expression, checker);
+    export function isReturnStatementWithFixablePromiseHandler(node: Node): node is ReturnStatement {
+        return isReturnStatement(node) && !!node.expression && isFixablePromiseHandler(node.expression);
     }
 
     // Should be kept up to date with transformExpression in convertToAsyncFunction.ts
-    export function isFixablePromiseHandler(node: Node, checker: TypeChecker): boolean {
+    export function isFixablePromiseHandler(node: Node): boolean {
         // ensure outermost call exists and is a promise handler
-        if (!isPromiseHandler(node) || !node.arguments.every((arg) => isFixablePromiseArgument(arg, checker))) {
+        if (!isPromiseHandler(node) || !node.arguments.every(isFixablePromiseArgument)) {
             return false;
         }
 
         // ensure all chained calls are valid
         let currentNode = node.expression;
         while (isPromiseHandler(currentNode) || isPropertyAccessExpression(currentNode)) {
-            if (isCallExpression(currentNode) && !currentNode.arguments.every(arg => isFixablePromiseArgument(arg, checker))) {
+            if (isCallExpression(currentNode) && !currentNode.arguments.every(isFixablePromiseArgument)) {
                 return false;
             }
             currentNode = currentNode.expression;
@@ -174,14 +175,12 @@ namespace ts {
     }
 
     // should be kept up to date with getTransformationBody in convertToAsyncFunction.ts
-    function isFixablePromiseArgument(arg: Expression, checker: TypeChecker): boolean {
+    function isFixablePromiseArgument(arg: Expression): boolean {
         switch (arg.kind) {
             case SyntaxKind.FunctionDeclaration:
             case SyntaxKind.FunctionExpression:
             case SyntaxKind.ArrowFunction:
-                if (isConvertibleFunction(arg as FunctionLikeDeclaration, checker)) {
-                    visitedNestedConvertibleFunctions.set(getKeyFromNode(arg as FunctionLikeDeclaration), true);
-                }
+                visitedNestedConvertibleFunctions.set(getKeyFromNode(arg as FunctionLikeDeclaration), true);
                 /* falls through */
             case SyntaxKind.NullKeyword:
             case SyntaxKind.Identifier: // identifier includes undefined