Fix flow comments plugin issues (#10329)

* Fix issues in flow-comments to preserve comments and there order (fixes #10324)

* Add support in flow-comments for extends in class declarations (fixes #10323, #10321)

* Refactoring and cleanup of flow-comments plugin

* Fix comments preservation logic of flow-comments

* Fix flow-comments where comments are class identifier and extends keyword

packages/babel-plugin-transform-flow-comments/src/index.js

@@ -6,27 +6,73 @@ import generateCode from "@babel/generator";
 export default declare(api => {
   api.assertVersion(7);
 
-  function attachComment(path, comment) {
+  function commentFromString(comment) {
-    let attach = path.getPrevSibling();
+    return typeof comment === "string"
-    let where = "trailing";
+      ? { type: "CommentBlock", value: comment }
-    if (!attach.node) {
+      : comment;
-      attach = path.parentPath;
+  }
+
+  function attachComment({
+    ofPath,
+    toPath,
+    where = "trailing",
+    optional = false,
+    comments = generateComment(ofPath, optional),
+    keepType = false,
+  }) {
+    if (!toPath || !toPath.node) {
+      toPath = ofPath.getPrevSibling();
+      where = "trailing";
+    }
+    if (!toPath.node) {
+      toPath = ofPath.getNextSibling();
+      where = "leading";
+    }
+    if (!toPath.node) {
+      toPath = ofPath.parentPath;
       where = "inner";
     }
-    attach.addComment(where, comment);
+    if (!Array.isArray(comments)) {
-    path.remove();
+      comments = [comments];
+    }
+    comments = comments.map(commentFromString);
+    if (!keepType && ofPath && ofPath.node) {
+      // Removes the node at `ofPath` while conserving the comments attached
+      // to it.
+      const node = ofPath.node;
+      const parent = ofPath.parentPath;
+      const prev = ofPath.getPrevSibling();
+      const next = ofPath.getNextSibling();
+      const isSingleChild = !(prev.node || next.node);
+      const leading = node.leadingComments;
+      const trailing = node.trailingComments;
+
+      if (isSingleChild && leading) {
+        parent.addComments("inner", leading);
+      }
+      toPath.addComments(where, comments);
+      ofPath.remove();
+      if (isSingleChild && trailing) {
+        parent.addComments("inner", trailing);
+      }
+    } else {
+      toPath.addComments(where, comments);
+    }
   }
 
-  function wrapInFlowComment(path, parent) {
+  function wrapInFlowComment(path) {
-    attachComment(path, generateComment(path, parent));
+    attachComment({
+      ofPath: path,
+      comments: generateComment(path, path.parent.optional),
+    });
   }
 
-  function generateComment(path, parent) {
+  function generateComment(path, optional) {
     let comment = path
       .getSource()
       .replace(/\*-\//g, "*-ESCAPED/")
       .replace(/\*\//g, "*-/");
-    if (parent && parent.optional) comment = "?" + comment;
+    if (optional) comment = "?" + comment;
     if (comment[0] !== ":") comment = ":: " + comment;
     return comment;
   }
@@ -42,28 +88,32 @@ export default declare(api => {
     visitor: {
       TypeCastExpression(path) {
         const { node } = path;
-        path
+        attachComment({
-          .get("expression")
+          ofPath: path.get("typeAnnotation"),
-          .addComment("trailing", generateComment(path.get("typeAnnotation")));
+          toPath: path.get("expression"),
+          keepType: true,
+        });
         path.replaceWith(t.parenthesizedExpression(node.expression));
       },
 
       // support function a(b?) {}
       Identifier(path) {
-        if (path.parentPath.isFlow()) {
+        if (path.parentPath.isFlow()) return;
-          return;
-        }
-
         const { node } = path;
         if (node.typeAnnotation) {
-          const typeAnnotation = path.get("typeAnnotation");
+          attachComment({
-          path.addComment("trailing", generateComment(typeAnnotation, node));
+            ofPath: path.get("typeAnnotation"),
-          typeAnnotation.remove();
+            toPath: path,
+            optional: node.optional || node.typeAnnotation.optional,
+          });
           if (node.optional) {
             node.optional = false;
           }
         } else if (node.optional) {
-          path.addComment("trailing", ":: ?");
+          attachComment({
+            toPath: path,
+            comments: ":: ?",
+          });
           node.optional = false;
         }
       },
@@ -81,58 +131,51 @@ export default declare(api => {
       Function(path) {
         if (path.isDeclareFunction()) return;
         const { node } = path;
-        if (node.returnType) {
-          const returnType = path.get("returnType");
-          const typeAnnotation = returnType.get("typeAnnotation");
-          const block = path.get("body");
-          block.addComment(
-            "leading",
-            generateComment(returnType, typeAnnotation.node),
-          );
-          returnType.remove();
-        }
         if (node.typeParameters) {
-          const typeParameters = path.get("typeParameters");
+          attachComment({
-          const id = path.get("id");
+            ofPath: path.get("typeParameters"),
-          id.addComment(
+            toPath: path.get("id"),
-            "trailing",
+            optional: node.typeParameters.optional,
-            generateComment(typeParameters, typeParameters.node),
+          });
-          );
+        }
-          typeParameters.remove();
+        if (node.returnType) {
+          attachComment({
+            ofPath: path.get("returnType"),
+            toPath: path.get("body"),
+            where: "leading",
+            optional: node.returnType.typeAnnotation.optional,
+          });
         }
       },
 
       // support for `class X { foo: string }` - #4622
       ClassProperty(path) {
-        const { node, parent } = path;
+        const { node } = path;
         if (!node.value) {
-          wrapInFlowComment(path, parent);
+          wrapInFlowComment(path);
         } else if (node.typeAnnotation) {
-          const typeAnnotation = path.get("typeAnnotation");
+          attachComment({
-          path
+            ofPath: path.get("typeAnnotation"),
-            .get("key")
+            toPath: path.get("key"),
-            .addComment(
+            optional: node.typeAnnotation.optional,
-              "trailing",
+          });
-              generateComment(typeAnnotation, typeAnnotation.node),
-            );
-          typeAnnotation.remove();
         }
       },
 
       // support `export type a = {}` - #8 Error: You passed path.replaceWith() a falsy node
       ExportNamedDeclaration(path) {
-        const { node, parent } = path;
+        const { node } = path;
         if (node.exportKind !== "type" && !t.isFlow(node.declaration)) {
           return;
         }
-        wrapInFlowComment(path, parent);
+        wrapInFlowComment(path);
       },
 
       // support `import type A` and `import typeof A` #10
       ImportDeclaration(path) {
-        const { node, parent } = path;
+        const { node } = path;
         if (isTypeImport(node.importKind)) {
-          wrapInFlowComment(path, parent);
+          wrapInFlowComment(path);
           return;
         }
 
@@ -148,61 +191,88 @@ export default declare(api => {
         if (typeSpecifiers.length > 0) {
           const typeImportNode = t.cloneNode(node);
           typeImportNode.specifiers = typeSpecifiers;
+          const comment = `:: ${generateCode(typeImportNode).code}`;
 
           if (nonTypeSpecifiers.length > 0) {
-            path.addComment(
+            attachComment({ toPath: path, comments: comment });
-              "trailing",
-              `:: ${generateCode(typeImportNode).code}`,
-            );
           } else {
-            attachComment(path, `:: ${generateCode(typeImportNode).code}`);
+            attachComment({ ofPath: path, comments: comment });
           }
         }
       },
       ObjectPattern(path) {
         const { node } = path;
         if (node.typeAnnotation) {
-          const typeAnnotation = path.get("typeAnnotation");
+          attachComment({
-          path.addComment(
+            ofPath: path.get("typeAnnotation"),
-            "trailing",
+            toPath: path,
-            generateComment(typeAnnotation, typeAnnotation.node),
+            optional: node.optional || node.typeAnnotation.optional,
-          );
+          });
-          typeAnnotation.remove();
         }
       },
 
       Flow(path) {
-        const { parent } = path;
+        wrapInFlowComment(path);
-        wrapInFlowComment(path, parent);
       },
 
       Class(path) {
         const { node } = path;
-        if (node.typeParameters || node.implements) {
+        let comments = [];
-          const comments = [];
         if (node.typeParameters) {
           const typeParameters = path.get("typeParameters");
           comments.push(
-              generateComment(typeParameters, typeParameters.node).replace(
+            generateComment(typeParameters, node.typeParameters.optional),
-                /^:: /,
-                "",
-              ),
           );
+          const trailingComments = node.typeParameters.trailingComments;
+          if (trailingComments) {
+            comments.push(...trailingComments);
+          }
           typeParameters.remove();
         }
+
+        if (node.superClass) {
+          if (comments.length > 0) {
+            attachComment({
+              toPath: path.get("id"),
+              comments: comments,
+            });
+            comments = [];
+          }
+
+          if (node.superTypeParameters) {
+            const superTypeParameters = path.get("superTypeParameters");
+            comments.push(
+              generateComment(
+                superTypeParameters,
+                superTypeParameters.node.optional,
+              ),
+            );
+            superTypeParameters.remove();
+          }
+        }
+
         if (node.implements) {
           const impls = path.get("implements");
-            comments.push(
+          const comment =
             "implements " +
             impls
               .map(impl => generateComment(impl).replace(/^:: /, ""))
-                  .join(", "),
+              .join(", ");
-            );
           delete node["implements"];
+
+          if (comments.length === 1) {
+            comments[0] += ` ${comment}`;
+          } else {
+            comments.push(`:: ${comment}`);
+          }
         }
 
-          const block = path.get("body");
+        if (comments.length > 0) {
-          block.addComment("leading", ":: " + comments.join(" "));
+          attachComment({
+            toPath: path.get("body"),
+            where: "leading",
+            comments: comments,
+          });
         }
       },
     },

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/class-extends-type-parameter/input.js

@@ -0,0 +1 @@
+class Foo extends Bar<T> {}

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/class-extends-type-parameter/output.js

@@ -0,0 +1,3 @@
+class Foo extends Bar
+/*:: <T>*/
+{}

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/class-type-parameter-extends-type-parameter/input.js

@@ -0,0 +1,3 @@
+class Foo<T> /* inner */ extends Bar<R> {}
+
+/*a*/class /*b*/Baz/*c*/<T>/*d*/extends /*e*/Bar/*f*/<R>/*g*/ {/*h*/}/*i*/

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/class-type-parameter-extends-type-parameter/output.js

@@ -0,0 +1,30 @@
+class Foo
+/*:: <T>*/
+
+/* inner */
+extends Bar
+/*:: <R>*/
+{}
+/*a*/
+
+
+class
+/*b*/
+Baz
+/*c*/
+
+/*:: <T>*/
+
+/*d*/
+extends
+/*e*/
+Bar
+/*f*/
+
+/*:: <R>*/
+
+/*g*/
+{}
+/*h*/
+
+/*i*/

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/class-type-parameter-extends/input.js

@@ -0,0 +1 @@
+class Foo<T> extends Bar {}

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/class-type-parameter-extends/output.js

@@ -0,0 +1,3 @@
+class Foo
+/*:: <T>*/
+extends Bar {}

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/preserve-comments-order/input.js

@@ -0,0 +1,7 @@
+/*a*/
+type Foo = number;
+/*b*/
+var foo;
+/*c*/
+type Bar = number;
+/*d*/

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/preserve-comments-order/output.js

@@ -0,0 +1,11 @@
+/*a*/
+
+/*:: type Foo = number;*/
+
+/*b*/
+var foo;
+/*c*/
+
+/*:: type Bar = number;*/
+
+/*d*/

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/preserve-comments/input.js

@@ -0,0 +1,2 @@
+/**/
+type Foo = number;

packages/babel-plugin-transform-flow-comments/test/fixtures/flow-comments/preserve-comments/output.js

@@ -0,0 +1,3 @@
+/**/
+
+/*:: type Foo = number;*/