fix: preserve non-function handler behavior

fix #15

Author: jedwards1211

README.md

@@ -43,7 +43,7 @@ ASTs. If you use the `--commentWorkarounds=true` option it will try to prevent m
 from getting deleted but it sometimes causes an assertion to fail in `recast`.
 
 There are a few edge cases where `asyncify` produces funky output. It's intended to not break
-any existing behavior (at the moment there are a few bugs that do, which I need to fix)
+any existing behavior (I know of no cases where it does, and I have fixed several such issues)
 but sometimes the output will be be semantically wrong even if it behaves
 correctly. For example, I've seen a case where doing an async operation several times in a row:
 

src/util/canDefinitelyInvoke.ts

@@ -0,0 +1,21 @@
+import * as t from '@babel/types'
+import { NodePath } from '@babel/traverse'
+
+export default function canDefinitelyInvoke<T extends t.Node>(
+  expr: NodePath<T>
+): boolean {
+  if (expr.isIdentifier()) {
+    let target: NodePath<any> | undefined = expr
+    while (target) {
+      if (target.isIdentifier()) {
+        target = target.scope.getBinding(target.node.name)?.path
+      } else if (target.isVariableDeclarator()) {
+        target = (target as NodePath<t.VariableDeclarator>).get('init')
+      } else {
+        break
+      }
+    }
+    return target ? target.isFunction() : false
+  }
+  return expr.isFunction()
+}

src/util/unwindCatch.ts

@@ -12,6 +12,7 @@ import mergeCatchIntoTryFinally from './mergeCatchIntoFinally'
 import { awaited } from './builders'
 import convertConditionalReturns from './convertConditionalReturns'
 import findNode from './findNode'
+import canDefinitelyInvoke from './canDefinitelyInvoke'
 
 export default function unwindCatch(
   handler: NodePath<t.Expression>
@@ -34,6 +35,9 @@ export default function unwindCatch(
   }
 
   if (!handler.isFunction()) {
+    if (!canDefinitelyInvoke(handler)) {
+      return getPreceedingLink(link)
+    }
     const callee = handler.node
     ;[handler] = handler.replaceWith(
       t.arrowFunctionExpression(

src/util/unwindFinally.ts

@@ -10,6 +10,7 @@ import convertBodyToBlockStatement from './convertBodyToBlockStatement'
 import convertConditionalReturns from './convertConditionalReturns'
 import mergeStatementsIntoTryFinally from './mergeStatementsIntoTryFinally'
 import findNode from './findNode'
+import canDefinitelyInvoke from './canDefinitelyInvoke'
 
 export default function unwindFinally(
   handler: NodePath<t.Expression>
@@ -23,6 +24,9 @@ export default function unwindFinally(
   }
 
   if (!handler.isFunction()) {
+    if (!canDefinitelyInvoke(handler)) {
+      return preceedingLink
+    }
     const callee = handler.node
     ;[handler] = handler.replaceWith(
       t.arrowFunctionExpression([], t.callExpression(callee, []))

src/util/unwindThen.ts

@@ -11,6 +11,7 @@ import prependBodyStatement from './prependBodyStatement'
 import replaceLink from './replaceLink'
 import convertConditionalReturns from './convertConditionalReturns'
 import findNode from './findNode'
+import canDefinitelyInvoke from './canDefinitelyInvoke'
 
 export function unwindThen(
   handler: NodePath<t.Expression>
@@ -59,8 +60,11 @@ export function unwindThen(
       preceedingLink.node
     )
   }
-  return findNode(
-    replaceLink(link, t.callExpression(handler.node, [preceeding])) as any,
-    preceedingLink.node
-  )
+  if (canDefinitelyInvoke(handler)) {
+    return findNode(
+      replaceLink(link, t.callExpression(handler.node, [preceeding])) as any,
+      preceedingLink.node
+    )
+  }
+  return preceedingLink
 }

test/fixtures/catchExpressionBody_Returned.ts

@@ -1,4 +1,5 @@
 export const input = `
+const handleError = () => {}
 function foo() {
   return baz.catch(handleError)
 }
@@ -7,6 +8,7 @@ function foo() {
 export const options = {}
 
 export const expected = `
+const handleError = () => {}
 async function foo() {
   try {
     return await baz

test/fixtures/catch_handlersThatCantBeUnwound.ts

@@ -39,37 +39,37 @@ export const options = {}
 
 export const expected = `
 async function foo() {
-  return String(await bar.catch(baz => {
+  return bar.catch(baz => {
     switch (baz) {
       case 2: return
     }
     console.log('test')
-  }))
+  }).then(String)
 }
 async function bar() {
-  return String(await a.catch(b => {
+  return a.catch(b => {
     for (const i of [1, 2, 3]) {
       return
     }
     console.log('test')
-  }))
+  }).then(String)
 }
 async function qux() {
-  return String(await a.catch(b => {
+  return a.catch(b => {
     while (i) {
       return
     }
     console.log('test')
-  }))
+  }).then(String)
 }
 async function baz() {
-  return String(await a.catch(b => {
+  return a.catch(b => {
     if (a) {
       if (b) {
         return
       }
     }
     console.log('test')
-  }))
+  }).then(String)
 }
 `

test/fixtures/finallyIdentifier_Returned.ts

@@ -1,4 +1,6 @@
 export const input = `
+function cleanup() {
+}
 async function foo() {
   return await baz.finally(cleanup)
 }
@@ -7,6 +9,8 @@ async function foo() {
 export const options = {}
 
 export const expected = `
+function cleanup() {
+}
 async function foo() {
   try {
     return await baz

test/fixtures/finally_handlersThatCantBeUnwound.ts

@@ -39,37 +39,37 @@ export const options = {}
 
 export const expected = `
 async function foo() {
-  return String(await bar.finally(async () => {
+  return bar.finally(async () => {
     switch (baz) {
       case 2: return
     }
     console.log('test')
-  }))
+  }).then(String)
 }
 async function bar() {
-  return String(await a.finally(async () => {
+  return a.finally(async () => {
     for (const i of [1, 2, 3]) {
       return
     }
     console.log('test')
-  }))
+  }).then(String)
 }
 async function qux() {
-  return String(await a.finally(async () => {
+  return a.finally(async () => {
     while (i) {
       return
     }
     console.log('test')
-  }))
+  }).then(String)
 }
 async function baz() {
-  return String(await a.finally(async () => {
+  return a.finally(async () => {
     if (a) {
       if (b) {
         return
       }
     }
     console.log('test')
-  }))
+  }).then(String)
 }
 `

test/fixtures/thenExpression_Unconsumed.ts

@@ -0,0 +1,15 @@
+export const input = `
+async function foo() {
+  const handler = 2
+  await baz.then(makeHandler(5))
+}
+`
+
+export const options = {}
+
+export const expected = `
+async function foo() {
+  const handler = 2
+  await baz.then(makeHandler(5))
+}
+`

test/fixtures/then_handlersThatCantBeUnwound.ts

@@ -39,45 +39,37 @@ export const options = {}
 
 export const expected = `
 async function foo() {
-  return String(
-    await bar.then(async baz => {
-      switch (baz) {
-        case 2: return
-      }
-      console.log('test')
-    })
-  )
+  return bar.then(async baz => {
+    switch (baz) {
+      case 2: return
+    }
+    console.log('test')
+  }).then(String)
 }
 async function bar() {
-  return String(
-    await a.then(async b => {
-      for (const i of [1, 2, 3]) {
-        return
-      }
-      console.log('test')
-    })
-  )
+  return a.then(async b => {
+    for (const i of [1, 2, 3]) {
+      return
+    }
+    console.log('test')
+  }).then(String)
 }
 async function qux() {
-  return String(
-    await a.then(async b => {
-      while (i) {
-        return
-      }
-      console.log('test')
-    })
-  )
+  return a.then(async b => {
+    while (i) {
+      return
+    }
+    console.log('test')
+  }).then(String)
 }
 async function baz() {
-  return String(
-    await a.then(async b => {
-      if (a) {
-        if (b) {
-          return
-        }
+  return a.then(async b => {
+    if (a) {
+      if (b) {
+        return
       }
-      console.log('test')
-    })
-  )
+    }
+    console.log('test')
+  }).then(String)
 }
 `