Leave trailing comments after handling a possible trailing com… (#10445)

* Leave trailing comments aftre handling a possible trailing comma * perf
babel · Sep 23, 2019 · 3069747 · 3069747
1 parent 962015f
commit 3069747
Show file tree

Hide file tree

Showing 5 changed files with 507 additions and 10 deletions.
diff --git a/packages/babel-parser/src/parser/comments.js b/packages/babel-parser/src/parser/comments.js
@@ -38,7 +38,19 @@ export default class CommentsParser extends BaseParser {
     this.state.leadingComments.push(comment);
   }
 
-  adjustCommentsAfterTrailingComma(node: Node, elements: Node[]) {
+  adjustCommentsAfterTrailingComma(
+    node: Node,
+    elements: Node[],
+    // When the current node is followed by a token which hasn't a respective AST node, we
+    // need to take all the trailing comments to prevent them from being attached to an
+    // unrelated node. e.g. in
+    //     var { x } /* cmt */ = { y }
+    // we don't want /* cmt */ to be attached to { y }.
+    // On the other hand, in
+    //     fn(x) [new line] /* cmt */ [new line] y
+    // /* cmt */ is both a trailing comment of fn(x) and a leading comment of y
+    takeAllComments?: boolean,
+  ) {
     if (this.state.leadingComments.length === 0) {
       return;
     }
@@ -59,17 +71,24 @@ export default class CommentsParser extends BaseParser {
     }
 
     const newTrailingComments = [];
-    while (this.state.leadingComments.length) {
-      const leadingComment = this.state.leadingComments.shift();
+    for (let i = 0; i < this.state.leadingComments.length; i++) {
+      const leadingComment = this.state.leadingComments[i];
       if (leadingComment.end < node.end) {
         newTrailingComments.push(leadingComment);
+
+        // Perf: we don't need to splice if we are going to reset the array anyway
+        if (!takeAllComments) {
+          this.state.leadingComments.splice(i, 1);
+          i--;
+        }
       } else {
         if (node.trailingComments === undefined) {
           node.trailingComments = [];
         }
         node.trailingComments.push(leadingComment);
       }
     }
+    if (takeAllComments) this.state.leadingComments = [];
 
     if (newTrailingComments.length > 0) {
       lastElement.trailingComments = newTrailingComments;
@@ -130,16 +149,20 @@ export default class CommentsParser extends BaseParser {
     if (firstChild) {
       switch (node.type) {
         case "ObjectExpression":
-        case "ObjectPattern":
           this.adjustCommentsAfterTrailingComma(node, node.properties);
           break;
+        case "ObjectPattern":
+          this.adjustCommentsAfterTrailingComma(node, node.properties, true);
+          break;
         case "CallExpression":
           this.adjustCommentsAfterTrailingComma(node, node.arguments);
           break;
         case "ArrayExpression":
-        case "ArrayPattern":
           this.adjustCommentsAfterTrailingComma(node, node.elements);
           break;
+        case "ArrayPattern":
+          this.adjustCommentsAfterTrailingComma(node, node.elements, true);
+          break;
       }
     } else if (
       this.state.commentPreviousNode &&
@@ -148,9 +171,11 @@ export default class CommentsParser extends BaseParser {
         (this.state.commentPreviousNode.type === "ExportSpecifier" &&
           node.type !== "ExportSpecifier"))
     ) {
-      this.adjustCommentsAfterTrailingComma(node, [
-        this.state.commentPreviousNode,
-      ]);
+      this.adjustCommentsAfterTrailingComma(
+        node,
+        [this.state.commentPreviousNode],
+        true,
+      );
     }
 
     if (lastChild) {

diff --git a/...s/babel-parser/test/fixtures/comments/basic/switch-function-call-no-semicolon/output.json b/...s/babel-parser/test/fixtures/comments/basic/switch-function-call-no-semicolon/output.json
@@ -250,7 +250,25 @@
                 "label": null
               }
             ],
-            "test": null
+            "test": null,
+            "leadingComments": [
+              {
+                "type": "CommentLine",
+                "value": " comment",
+                "start": 47,
+                "end": 57,
+                "loc": {
+                  "start": {
+                    "line": 4,
+                    "column": 4
+                  },
+                  "end": {
+                    "line": 4,
+                    "column": 14
+                  }
+                }
+              }
+            ]
           }
         ]
       }

diff --git a/packages/babel-parser/test/fixtures/comments/regression/10230/output.json b/packages/babel-parser/test/fixtures/comments/regression/10230/output.json
@@ -215,7 +215,25 @@
             "identifierName": "B"
           },
           "name": "B"
-        }
+        },
+        "leadingComments": [
+          {
+            "type": "CommentLine",
+            "value": " Two",
+            "start": 27,
+            "end": 33,
+            "loc": {
+              "start": {
+                "line": 6,
+                "column": 0
+              },
+              "end": {
+                "line": 6,
+                "column": 6
+              }
+            }
+          }
+        ]
       }
     ],
     "directives": []

diff --git a/packages/babel-parser/test/fixtures/comments/regression/10432/input.js b/packages/babel-parser/test/fixtures/comments/regression/10432/input.js
@@ -0,0 +1,5 @@
+const socket = socketClient(address)
+/* istanbul ignore next */
+socket.on('connect', function () {
+  debug('Connected to ' + address)
+})