Collect comments around parentheses in expressions (#13803)

Co-authored-by: Brian Ng <bng412@gmail.com>
babel · Oct 5, 2021 · 64f14b0 · 64f14b0
1 parent 7bbdf09
commit 64f14b0
Show file tree

Hide file tree

Showing 16 changed files with 508 additions and 21 deletions.
diff --git a/.../regression/comment-before-parentheses-return-arg-createParenthesizedExpressions/input.js b/.../regression/comment-before-parentheses-return-arg-createParenthesizedExpressions/input.js
@@ -0,0 +1,17 @@
+function assertElement(assertFn, shouldBeElement, opt_message) {
+  return /** @type {!Ele	ment} */ (
+	  assertType_(
+	    assertFn,
+		  shouldBeElement,
+		  isElement(shouldBeElement),
+		  'Element expected',
+		  opt_message
+	  )
+	);
+}
+
+const slot = /** @type {!HTMLSlotElement} */ (e.target);
+
+assertElement(
+  /** @type {Element} */ (el),
+);
diff --git a/...ression/comment-before-parentheses-return-arg-createParenthesizedExpressions/options.json b/...ression/comment-before-parentheses-return-arg-createParenthesizedExpressions/options.json
@@ -0,0 +1,4 @@
+{
+  "retainLines": true,
+  "createParenthesizedExpressions": true
+}
diff --git a/...regression/comment-before-parentheses-return-arg-createParenthesizedExpressions/output.js b/...regression/comment-before-parentheses-return-arg-createParenthesizedExpressions/output.js
@@ -0,0 +1,16 @@
+function assertElement(assertFn, shouldBeElement, opt_message) {
+  return (/** @type {!Ele	ment} */
+    assertType_(
+    assertFn,
+    shouldBeElement,
+    isElement(shouldBeElement),
+    'Element expected',
+    opt_message));
+
+
+}
+
+const slot = /** @type {!HTMLSlotElement} */e.target;
+
+assertElement(
+/** @type {Element} */el);
diff --git a/...s/babel-generator/test/fixtures/regression/comment-before-parentheses-return-arg/input.js b/...s/babel-generator/test/fixtures/regression/comment-before-parentheses-return-arg/input.js
@@ -0,0 +1,17 @@
+function assertElement(assertFn, shouldBeElement, opt_message) {
+  return /** @type {!Ele	ment} */ (
+	  assertType_(
+	    assertFn,
+		  shouldBeElement,
+		  isElement(shouldBeElement),
+		  'Element expected',
+		  opt_message
+	  )
+	);
+}
+
+const slot = /** @type {!HTMLSlotElement} */ (e.target);
+
+assertElement(
+  /** @type {Element} */ (el),
+);
diff --git a/.../babel-generator/test/fixtures/regression/comment-before-parentheses-return-arg/output.js b/.../babel-generator/test/fixtures/regression/comment-before-parentheses-return-arg/output.js
@@ -0,0 +1,13 @@
+function assertElement(assertFn, shouldBeElement, opt_message) {
+  return (
+    /** @type {!Ele	ment} */
+    assertType_(assertFn, shouldBeElement, isElement(shouldBeElement), 'Element expected', opt_message)
+  );
+}
+
+const slot =
+/** @type {!HTMLSlotElement} */
+e.target;
+assertElement(
+/** @type {Element} */
+el);
diff --git a/packages/babel-parser/src/parser/comments.js b/packages/babel-parser/src/parser/comments.js
@@ -26,6 +26,7 @@ export type CommentWhitespace = {
   trailingNode: Node | null,
   containingNode: Node | null,
 };
+
 /**
  * Merge comments with node's trailingComments or assign comments to be
  * trailingComments. New comments will be placed before old comments
@@ -42,6 +43,22 @@ function setTrailingComments(node: Node, comments: Array<Comment>) {
   }
 }
 
+/**
+ * Merge comments with node's leadingComments or assign comments to be
+ * leadingComments. New comments will be placed before old comments
+ * because the commentStack is enumerated reversely.
+ *
+ * @param {Node} node
+ * @param {Array<Comment>} comments
+ */
+function setLeadingComments(node: Node, comments: Array<Comment>) {
+  if (node.leadingComments === undefined) {
+    node.leadingComments = comments;
+  } else {
+    node.leadingComments.unshift(...comments);
+  }
+}
+
 /**
  * Merge comments with node's innerComments or assign comments to be
  * innerComments. New comments will be placed before old comments
@@ -50,10 +67,10 @@ function setTrailingComments(node: Node, comments: Array<Comment>) {
  * @param {Node} node
  * @param {Array<Comment>} comments
  */
-export function setInnerComments(node: Node, comments: Array<Comment> | void) {
+export function setInnerComments(node: Node, comments: Array<Comment>) {
   if (node.innerComments === undefined) {
     node.innerComments = comments;
-  } else if (comments !== undefined) {
+  } else {
     node.innerComments.unshift(...comments);
   }
 }
@@ -149,7 +166,7 @@ export default class CommentsParser extends BaseParser {
         setTrailingComments(commentWS.leadingNode, comments);
       }
       if (commentWS.trailingNode !== null) {
-        commentWS.trailingNode.leadingComments = comments;
+        setLeadingComments(commentWS.trailingNode, comments);
       }
     } else {
       /*:: invariant(commentWS.containingNode !== null) */
@@ -238,4 +255,36 @@ export default class CommentsParser extends BaseParser {
       commentWS.leadingNode = null;
     }
   }
+
+  /**
+   * Attach a node to the comment whitespaces right before/after
+   * the given range.
+   *
+   * This is used to properly attach comments around parenthesized
+   * expressions as leading/trailing comments of the inner expression.
+   *
+   * @param {Node} node
+   * @param {number} start
+   * @param {number} end
+   */
+  takeSurroundingComments(node: Node, start: number, end: number) {
+    const { commentStack } = this.state;
+    const commentStackLength = commentStack.length;
+    if (commentStackLength === 0) return;
+    let i = commentStackLength - 1;
+
+    for (; i >= 0; i--) {
+      const commentWS = commentStack[i];
+      const commentEnd = commentWS.end;
+      const commentStart = commentWS.start;
+
+      if (commentStart === end) {
+        commentWS.leadingNode = node;
+      } else if (commentEnd === start) {
+        commentWS.trailingNode = node;
+      } else if (commentEnd < start) {
+        break;
+      }
+    }
+  }
 }
diff --git a/packages/babel-parser/src/parser/expression.js b/packages/babel-parser/src/parser/expression.js
@@ -1023,9 +1023,13 @@ export default class ExpressionParser extends LValParser {
       call.extra?.trailingComma,
     );
     // mark inner comments of `async()` as inner comments of `async () =>`
-    setInnerComments(node, call.innerComments);
+    if (call.innerComments) {
+      setInnerComments(node, call.innerComments);
+    }
     // mark trailing comments of `async` to be inner comments
-    setInnerComments(node, call.callee.trailingComments);
+    if (call.callee.trailingComments) {
+      setInnerComments(node, call.callee.trailingComments);
+    }
     return node;
   }
 
@@ -1738,6 +1742,9 @@ export default class ExpressionParser extends LValParser {
     if (!this.options.createParenthesizedExpressions) {
       this.addExtra(val, "parenthesized", true);
       this.addExtra(val, "parenStart", startPos);
+
+      this.takeSurroundingComments(val, startPos, this.state.lastTokEnd);
+
       return val;
     }
 

diff --git a/packages/babel-parser/test/fixtures/comments/basic/nested-parentheses/input.js b/packages/babel-parser/test/fixtures/comments/basic/nested-parentheses/input.js
@@ -0,0 +1 @@
+1 + /* 0 */(/* 1 */(/* 2 */(/* 3 */i/* 4 */)/* 5 */)/* 6 */)/* 7 */;
diff --git a/packages/babel-parser/test/fixtures/comments/basic/nested-parentheses/output.json b/packages/babel-parser/test/fixtures/comments/basic/nested-parentheses/output.json
@@ -0,0 +1,128 @@
+{
+  "type": "File",
+  "start":0,"end":68,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":68}},
+  "program": {
+    "type": "Program",
+    "start":0,"end":68,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":68}},
+    "sourceType": "script",
+    "interpreter": null,
+    "body": [
+      {
+        "type": "ExpressionStatement",
+        "start":0,"end":68,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":68}},
+        "expression": {
+          "type": "BinaryExpression",
+          "start":0,"end":60,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":60}},
+          "left": {
+            "type": "NumericLiteral",
+            "start":0,"end":1,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":1}},
+            "extra": {
+              "rawValue": 1,
+              "raw": "1"
+            },
+            "value": 1
+          },
+          "operator": "+",
+          "right": {
+            "type": "Identifier",
+            "start":35,"end":36,"loc":{"start":{"line":1,"column":35},"end":{"line":1,"column":36},"identifierName":"i"},
+            "name": "i",
+            "extra": {
+              "parenthesized": true,
+              "parenStart": 11
+            },
+            "trailingComments": [
+              {
+                "type": "CommentBlock",
+                "value": " 4 ",
+                "start":36,"end":43,"loc":{"start":{"line":1,"column":36},"end":{"line":1,"column":43}}
+              },
+              {
+                "type": "CommentBlock",
+                "value": " 5 ",
+                "start":44,"end":51,"loc":{"start":{"line":1,"column":44},"end":{"line":1,"column":51}}
+              },
+              {
+                "type": "CommentBlock",
+                "value": " 6 ",
+                "start":52,"end":59,"loc":{"start":{"line":1,"column":52},"end":{"line":1,"column":59}}
+              }
+            ],
+            "leadingComments": [
+              {
+                "type": "CommentBlock",
+                "value": " 0 ",
+                "start":4,"end":11,"loc":{"start":{"line":1,"column":4},"end":{"line":1,"column":11}}
+              },
+              {
+                "type": "CommentBlock",
+                "value": " 1 ",
+                "start":12,"end":19,"loc":{"start":{"line":1,"column":12},"end":{"line":1,"column":19}}
+              },
+              {
+                "type": "CommentBlock",
+                "value": " 2 ",
+                "start":20,"end":27,"loc":{"start":{"line":1,"column":20},"end":{"line":1,"column":27}}
+              },
+              {
+                "type": "CommentBlock",
+                "value": " 3 ",
+                "start":28,"end":35,"loc":{"start":{"line":1,"column":28},"end":{"line":1,"column":35}}
+              }
+            ]
+          },
+          "trailingComments": [
+            {
+              "type": "CommentBlock",
+              "value": " 7 ",
+              "start":60,"end":67,"loc":{"start":{"line":1,"column":60},"end":{"line":1,"column":67}}
+            }
+          ]
+        }
+      }
+    ],
+    "directives": []
+  },
+  "comments": [
+    {
+      "type": "CommentBlock",
+      "value": " 0 ",
+      "start":4,"end":11,"loc":{"start":{"line":1,"column":4},"end":{"line":1,"column":11}}
+    },
+    {
+      "type": "CommentBlock",
+      "value": " 1 ",
+      "start":12,"end":19,"loc":{"start":{"line":1,"column":12},"end":{"line":1,"column":19}}
+    },
+    {
+      "type": "CommentBlock",
+      "value": " 2 ",
+      "start":20,"end":27,"loc":{"start":{"line":1,"column":20},"end":{"line":1,"column":27}}
+    },
+    {
+      "type": "CommentBlock",
+      "value": " 3 ",
+      "start":28,"end":35,"loc":{"start":{"line":1,"column":28},"end":{"line":1,"column":35}}
+    },
+    {
+      "type": "CommentBlock",
+      "value": " 4 ",
+      "start":36,"end":43,"loc":{"start":{"line":1,"column":36},"end":{"line":1,"column":43}}
+    },
+    {
+      "type": "CommentBlock",
+      "value": " 5 ",
+      "start":44,"end":51,"loc":{"start":{"line":1,"column":44},"end":{"line":1,"column":51}}
+    },
+    {
+      "type": "CommentBlock",
+      "value": " 6 ",
+      "start":52,"end":59,"loc":{"start":{"line":1,"column":52},"end":{"line":1,"column":59}}
+    },
+    {
+      "type": "CommentBlock",
+      "value": " 7 ",
+      "start":60,"end":67,"loc":{"start":{"line":1,"column":60},"end":{"line":1,"column":67}}
+    }
+  ]
+}
diff --git a/packages/babel-parser/test/fixtures/comments/basic/sequence-expression/output.json b/packages/babel-parser/test/fixtures/comments/basic/sequence-expression/output.json
@@ -24,22 +24,15 @@
             "start":77,"end":84,"loc":{"start":{"line":1,"column":77},"end":{"line":1,"column":84}}
           }
         ],
-        "innerComments": [
-          {
-            "type": "CommentBlock",
-            "value": " 2 ",
-            "start":9,"end":16,"loc":{"start":{"line":1,"column":9},"end":{"line":1,"column":16}}
-          },
-          {
-            "type": "CommentBlock",
-            "value": " 8 ",
-            "start":68,"end":75,"loc":{"start":{"line":1,"column":68},"end":{"line":1,"column":75}}
-          }
-        ],
         "expression": {
           "type": "SequenceExpression",
           "start":27,"end":47,"loc":{"start":{"line":1,"column":27},"end":{"line":1,"column":47}},
           "leadingComments": [
+            {
+              "type": "CommentBlock",
+              "value": " 2 ",
+              "start":9,"end":16,"loc":{"start":{"line":1,"column":9},"end":{"line":1,"column":16}}
+            },
             {
               "type": "CommentBlock",
               "value": " 3 ",
@@ -51,6 +44,11 @@
               "type": "CommentBlock",
               "value": " 7 ",
               "start":58,"end":65,"loc":{"start":{"line":1,"column":58},"end":{"line":1,"column":65}}
+            },
+            {
+              "type": "CommentBlock",
+              "value": " 8 ",
+              "start":68,"end":75,"loc":{"start":{"line":1,"column":68},"end":{"line":1,"column":75}}
             }
           ],
           "extra": {

diff --git a/packages/babel-parser/test/fixtures/comments/regression/13750/input.js b/packages/babel-parser/test/fixtures/comments/regression/13750/input.js
@@ -0,0 +1,5 @@
+if (args[0] === 1  || /* istanbul ignore next */ (args[0] === 2  || args[0] === 3)) {
+  output = args[0] + 10;
+} else {
+  output = 20;
+}