fix: make comment workarounds opt-in

BREAKING CHANGE: more comments will get deleted unless you opt into the workaround with
.  I turned this off by default because it crashes in a few
cases.

Author: jedwards1211

README.md

@@ -35,3 +35,56 @@ npx jscodeshift -t asyncify/index.js path/to/your/project/**/*.js
 | All but one nested if/else/switch branch return                    | 🚫         |
 | More than one if/else/switch branch doesn't return                 | 🚫         |
 | Return inside loop                                                 | 🚫         |
+
+## Warnings
+
+Comments can sometimes get deleted due to an impedance mismatch between `@babel` and `recast`
+ASTs. If you use the `--commentWorkarounds=true` option it will try to prevent more comments
+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 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:
+
+```js
+it('test', () => {
+  const doSomething = () => {
+    // ...
+  }
+
+  return doSomething()
+    .then(doSomething)
+    .then(doSomething)
+}
+```
+
+Gets converted to:
+
+```js
+it('test', async () => {
+  const doSomething = () => {
+    // ...
+  }
+  await doSomething(await doSomething(await doSomething()))
+})
+```
+
+This works even though it initially seems like it wouldn't and is obviously not what you want:
+
+```js
+it('test', async () => {
+  const doSomething = () => {
+    // ...
+  }
+  await doSomething()
+  await doSomething()
+  await doSomething()
+})
+```
+
+Although I could possibly fix this for cases where it's easy to determine that the function has
+no parameters, there could be cases where it's impossible to determine whether the identifier
+`doSomething` is even a function or whether it has parameters.
+
+At the moment, `.then(someIdentifier)` when `someIdentifier` is not a function is the only known
+case where the output behavior is wrong and will likely throw a `TypeError` (#15).

src/index.ts

@@ -15,6 +15,7 @@ module.exports = function index(
   })
 
   const ignoreChainsShorterThan = parseInt(options.ignoreChainsShorterThan)
+  const commentWorkarounds = options.commentWorkarounds
 
   let program: NodePath<t.Program> | undefined
   traverse(
@@ -26,7 +27,7 @@ module.exports = function index(
       },
     },
     undefined,
-    { ignoreChainsShorterThan }
+    { ignoreChainsShorterThan, commentWorkarounds }
   )
   if (!program) throw new Error('failed to find Program node')
   asyncify(program)

src/util/recastBugWorkarounds.ts

@@ -4,26 +4,6 @@ import { NodePath } from '@babel/traverse'
 export default function recastBugWorkarounds(path: NodePath<any>): void {
   const visitedComments: Set<t.Comment> = new Set()
   path.traverse({
-    exit(path: NodePath<t.Node>) {
-      const { leadingComments } = path.node
-      const anyNode = path.node as any
-      if (leadingComments) {
-        anyNode.comments = []
-        if (leadingComments) {
-          for (const comment of leadingComments) {
-            if (visitedComments.has(comment)) continue
-            visitedComments.add(comment)
-            anyNode.comments.push({
-              ...comment,
-              leading: true,
-              trailing: false,
-            })
-          }
-        }
-      } else {
-        anyNode.comments = null
-      }
-    },
     AwaitExpression(path: NodePath<t.AwaitExpression>) {
       const argument = path.get('argument')
       const { parentPath } = path
@@ -35,22 +15,46 @@ export default function recastBugWorkarounds(path: NodePath<any>): void {
       }
     },
   })
-  path.traverse({
-    exit(path: NodePath<t.Node>) {
-      const { trailingComments } = path.node
-      const anyNode = path.node as any
-      if (trailingComments) {
-        if (!anyNode.comments) anyNode.comments = []
-        for (const comment of trailingComments) {
-          if (visitedComments.has(comment)) continue
-          visitedComments.add(comment)
-          anyNode.comments.push({
-            ...comment,
-            leading: false,
-            trailing: true,
-          })
+  if (path.state.commentWorkarounds) {
+    path.traverse({
+      exit(path: NodePath<t.Node>) {
+        const { leadingComments } = path.node
+        const anyNode = path.node as any
+        if (leadingComments) {
+          anyNode.comments = []
+          if (leadingComments) {
+            for (const comment of leadingComments) {
+              if (visitedComments.has(comment)) continue
+              visitedComments.add(comment)
+              anyNode.comments.push({
+                ...comment,
+                leading: true,
+                trailing: false,
+              })
+            }
+          }
+        } else {
+          anyNode.comments = null
         }
-      }
-    },
-  })
+      },
+    })
+    path.traverse({
+      exit(path: NodePath<t.Node>) {
+        const { trailingComments } = path.node
+        const anyNode = path.node as any
+        if (trailingComments) {
+          if (!anyNode.comments) anyNode.comments = []
+          for (const comment of trailingComments) {
+            if (visitedComments.has(comment)) continue
+            visitedComments.add(comment)
+            anyNode.comments.push({
+              ...comment,
+              leading: false,
+              trailing: true,
+            })
+          }
+        }
+      },
+    })
+  }
 }

test/fixtures/bugs_BelongsToMany.create.ts

@@ -27,7 +27,9 @@ class BelongsToMany {
 }
 `
 
-export const options = {}
+export const options = {
+  commentWorkarounds: true,
+}
 
 export const expected = `
 class BelongsToMany {

test/fixtures/bugs_Sequelize.transaction.ts

@@ -27,7 +27,9 @@ class Sequelize {
 }
 `
 
-export const options = {}
+export const options = {
+  commentWorkarounds: true,
+}
 
 export const expected = `
 class Sequelize {

test/fixtures/leadingComment.ts

@@ -9,7 +9,9 @@ function foo() {
 }
 `
 
-export const options = {}
+export const options = {
+  commentWorkarounds: true,
+}
 
 export const expected = `
 async function foo() {