Support type cast comments for acorn parser (#17491)

Co-authored-by: fisker <lionkay@gmail.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

changelog_unreleased/javascript/17491.md

@@ -0,0 +1,15 @@
+#### Support type cast comments for acorn parser (#17491 by @ArnaudBarre)
+
+This was previously only supported by the Babel parser.
+
+<!-- prettier-ignore -->
+```js
+// Input
+/** @type {MyType} */ (x).foo;
+
+// Prettier stable (--parser=acorn)
+/** @type {MyType} */ x.foo;
+
+// Prettier main (--parser=acorn)
+/** @type {MyType} */ (x).foo;
+```

src/language-js/clean.js

@@ -3,7 +3,6 @@ import {
   isNumericLiteral,
   isStringLiteral,
 } from "./utils/index.js";
-import isBlockComment from "./utils/is-block-comment.js";
 
 const ignoredProperties = new Set([
   "range",
@@ -27,7 +26,7 @@ const removeTemplateElementsValue = (node) => {
   }
 };
 
-function clean(original, cloned, parent) {
+function clean(original, cloned) {
   if (original.type === "Program") {
     delete cloned.sourceType;
   }
@@ -173,28 +172,12 @@ function clean(original, cloned, parent) {
   ) {
     removeTemplateElementsValue(cloned.quasi);
   }
+
+  // TODO: Only delete value when there is leading comment which is exactly
+  // `/* GraphQL */` or `/* HTML */`
+  // Also see ./embed.js
   if (original.type === "TemplateLiteral") {
-    // This checks for a leading comment that is exactly `/* GraphQL */`
-    // In order to be in line with other implementations of this comment tag
-    // we will not trim the comment value and we will expect exactly one space on
-    // either side of the GraphQL string
-    // Also see ./embed.js
-    const hasLanguageComment = original.leadingComments?.some(
-      (comment) =>
-        isBlockComment(comment) &&
-        ["GraphQL", "HTML"].some(
-          (languageName) => comment.value === ` ${languageName} `,
-        ),
-    );
-    if (
-      hasLanguageComment ||
-      (parent.type === "CallExpression" && parent.callee.name === "graphql") ||
-      // TODO: check parser
-      // `flow` and `typescript` don't have `leadingComments`
-      !original.leadingComments
-    ) {
-      removeTemplateElementsValue(cloned);
-    }
+    removeTemplateElementsValue(cloned);
   }
 
   // We print `(a?.b!).c` as `(a?.b)!.c`, but `typescript` parse them differently

src/language-js/parse/acorn.js

@@ -17,6 +17,7 @@ const parseOptions = {
   allowSuperOutsideMethod: true,
   locations: true,
   ranges: true,
+  preserveParens: true,
 };
 
 function createParseError(error) {

src/language-js/parse/babel.js

@@ -27,6 +27,7 @@ const parseOptions = {
   errorRecovery: true,
   createParenthesizedExpressions: true,
   createImportExpressions: true,
+  attachComment: false,
   plugins: [
     // When adding a plugin, please add a test in `tests/format/js/babel-plugins`,
     // To remove plugins, remove it here and run `yarn test tests/format/js/babel-plugins` to verify
@@ -151,7 +152,7 @@ function createParse({ isExpression = false, optionsCombinations }) {
       ast = wrapBabelExpression(ast, { text, rootMarker: options.rootMarker });
     }
 
-    return postprocess(ast, { parser: "babel", text });
+    return postprocess(ast, { text });
   };
 }
 

src/language-js/parse/postprocess/index.js

@@ -51,22 +51,41 @@ function postprocess(ast, options) {
     comments.unshift(interpreter);
   }
 
-  // Keep Babel's non-standard ParenthesizedExpression nodes only if they have Closure-style type cast comments.
-  if (parser === "babel") {
-    const startOffsetsOfTypeCastedNodes = new Set();
+  if (isNonEmptyArray(ast.comments)) {
+    let followingComment;
+    for (let i = ast.comments.length - 1; i >= 0; i--) {
+      const comment = ast.comments[i];
 
-    // Comments might be attached not directly to ParenthesizedExpression but to its ancestor.
-    // E.g.: /** @type {Foo} */ (foo).bar();
-    // Let's use the fact that those ancestors and ParenthesizedExpression have the same start offset.
-
-    ast = visitNode(ast, (node) => {
-      if (node.leadingComments?.some(isTypeCastComment)) {
-        startOffsetsOfTypeCastedNodes.add(locStart(node));
+      if (
+        followingComment &&
+        locEnd(comment) === locStart(followingComment) &&
+        isBlockComment(comment) &&
+        isBlockComment(followingComment) &&
+        isIndentableBlockComment(comment) &&
+        isIndentableBlockComment(followingComment)
+      ) {
+        ast.comments.splice(i + 1, 1);
+        comment.value += "*//*" + followingComment.value;
+        comment.range = [locStart(comment), locEnd(followingComment)];
       }
-    });
 
-    ast = visitNode(ast, (node) => {
-      if (node.type === "ParenthesizedExpression") {
+      /* c8 ignore next 3 */
+      if (!isLineComment(comment) && !isBlockComment(comment)) {
+        throw new TypeError(`Unknown comment type: "${comment.type}".`);
+      }
+
+      /* c8 ignore next 3 */
+      if (process.env.NODE_ENV !== "production") {
+        assertComment(comment, text);
+      }
+
+      followingComment = comment;
+    }
+  }
+
+  ast = visitNode(ast, (node) => {
+    switch (node.type) {
+      case "ParenthesizedExpression": {
         const { expression } = node;
 
         // Align range with `flow`
@@ -75,17 +94,24 @@ function postprocess(ast, options) {
           return expression;
         }
 
-        const start = locStart(node);
-        if (!startOffsetsOfTypeCastedNodes.has(start)) {
+        // Keep ParenthesizedExpression nodes only if they have Closure-style type cast comments.
+        const nodeStart = locStart(node);
+        const previousComment = ast.comments.findLast(
+          (comment) => locEnd(comment) <= nodeStart,
+        );
+        const keepTypeCast =
+          previousComment &&
+          isTypeCastComment(previousComment) &&
+          // check that there are only white spaces between the comment and the parenthesis
+          text.slice(locEnd(previousComment), nodeStart).trim().length === 0;
+
+        if (!keepTypeCast) {
           expression.extra = { ...expression.extra, parenthesized: true };
           return expression;
         }
+        break;
       }
-    });
-  }
 
-  ast = visitNode(ast, (node) => {
-    switch (node.type) {
       case "LogicalExpression":
         // We remove unneeded parens around same-operator LogicalExpressions
         if (isUnbalancedLogicalTree(node)) {
@@ -152,37 +178,6 @@ function postprocess(ast, options) {
     }
   });
 
-  if (isNonEmptyArray(ast.comments)) {
-    let followingComment;
-    for (let i = ast.comments.length - 1; i >= 0; i--) {
-      const comment = ast.comments[i];
-
-      if (
-        followingComment &&
-        locEnd(comment) === locStart(followingComment) &&
-        isBlockComment(comment) &&
-        isBlockComment(followingComment) &&
-        isIndentableBlockComment(comment) &&
-        isIndentableBlockComment(followingComment)
-      ) {
-        ast.comments.splice(i + 1, 1);
-        comment.value += "*//*" + followingComment.value;
-        comment.range = [locStart(comment), locEnd(followingComment)];
-      }
-
-      if (!isLineComment(comment) && !isBlockComment(comment)) {
-        throw new TypeError(`Unknown comment type: "${comment.type}".`);
-      }
-
-      /* c8 ignore next 3 */
-      if (process.env.NODE_ENV !== "production") {
-        assertComment(comment, text);
-      }
-
-      followingComment = comment;
-    }
-  }
-
   // In `typescript`/`espree`/`flow`, `Program` doesn't count whitespace and comments
   // See https://github.com/eslint/espree/issues/488
   if (ast.type === "Program") {

tests/config/run-format-test.js

@@ -63,7 +63,7 @@ const espreeDisabledTests = new Set(
     "comments-closure-typecast",
   ].map((directory) => path.join(__dirname, "../format/js", directory)),
 );
-const acornDisabledTests = espreeDisabledTests;
+const acornDisabledTests = new Set();
 const meriyahDisabledTests = new Set([
   ...espreeDisabledTests,
   ...[

tests/format/js/comments-closure-typecast/__snapshots__/format.test.js.snap

@@ -215,7 +215,7 @@ var a =
    * bla bla bla
    */
   //2
-  (window["s"]).toString();
+  window["s"].toString();
 console.log(a.foo());
 
 ================================================================================
@@ -361,14 +361,14 @@ const prop = /** @type {Object} */(myObject.property).someProp;
 const test = /** @type (function (*): ?|undefined) */
       (goog.partial(NewThing.onTemplateChange, rationaleField, typeField));
 
-const test = /** @type (function (*): ?|undefined) */ (goog.partial(NewThing.onTemplateChange, rationaleField, typeField));
+const foo1 = /** @type (function (*): ?|undefined) */ (goog.partial(NewThing.onTemplateChange, rationaleField, typeField));
 
 const model = /** @type {?{getIndex: Function}} */ (model);
 
 const foo = /** @type {string} */
   (bar);
 
-const test = /** @type (function (*): ?|undefined) */ (foo);
+const foo2 = /** @type (function (*): ?|undefined) */ (foo);
 
 =====================================output=====================================
 /** @type {Object} */ (myObject.property).someProp = true;
@@ -380,7 +380,7 @@ const test =
   /** @type (function (*): ?|undefined) */
   (goog.partial(NewThing.onTemplateChange, rationaleField, typeField));
 
-const test = /** @type (function (*): ?|undefined) */ (
+const foo1 = /** @type (function (*): ?|undefined) */ (
   goog.partial(NewThing.onTemplateChange, rationaleField, typeField)
 );
 
@@ -388,7 +388,7 @@ const model = /** @type {?{getIndex: Function}} */ (model);
 
 const foo = /** @type {string} */ (bar);
 
-const test = /** @type (function (*): ?|undefined) */ (foo);
+const foo2 = /** @type (function (*): ?|undefined) */ (foo);
 
 ================================================================================
 `;
@@ -405,7 +405,7 @@ function jsdocCastInReturn() {
   return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ (fooBarBaz);
 }
 
-const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
+const myLongVariableFoo1 = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
       (fooBarBaz);
 
 function jsdocCastInReturn() {
@@ -413,7 +413,7 @@ function jsdocCastInReturn() {
     (fooBarBaz));
 }
 
-const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
+const myLongVariableFoo2 = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
       (fooBarBaz);
 
 function jsdocCastInReturn() {
@@ -431,7 +431,7 @@ function jsdocCastInReturn() {
   );
 }
 
-const myLongVariableName =
+const myLongVariableFoo1 =
   /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
   (fooBarBaz);
 
@@ -442,7 +442,7 @@ function jsdocCastInReturn() {
   );
 }
 
-const myLongVariableName =
+const myLongVariableFoo2 =
   /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
   (fooBarBaz);
 

tests/format/js/comments-closure-typecast/issue-4124.js

@@ -6,11 +6,11 @@ const prop = /** @type {Object} */(myObject.property).someProp;
 const test = /** @type (function (*): ?|undefined) */
       (goog.partial(NewThing.onTemplateChange, rationaleField, typeField));
 
-const test = /** @type (function (*): ?|undefined) */ (goog.partial(NewThing.onTemplateChange, rationaleField, typeField));
+const foo1 = /** @type (function (*): ?|undefined) */ (goog.partial(NewThing.onTemplateChange, rationaleField, typeField));
 
 const model = /** @type {?{getIndex: Function}} */ (model);
 
 const foo = /** @type {string} */
   (bar);
 
-const test = /** @type (function (*): ?|undefined) */ (foo);
+const foo2 = /** @type (function (*): ?|undefined) */ (foo);

tests/format/js/comments-closure-typecast/issue-8045.js

@@ -4,7 +4,7 @@ function jsdocCastInReturn() {
   return /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */ (fooBarBaz);
 }
 
-const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
+const myLongVariableFoo1 = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
       (fooBarBaz);
 
 function jsdocCastInReturn() {
@@ -12,7 +12,7 @@ function jsdocCastInReturn() {
     (fooBarBaz));
 }
 
-const myLongVariableName = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
+const myLongVariableFoo2 = /** @type {ThisIsAVeryLongTypeThatShouldTriggerLineWrapping} */
       (fooBarBaz);
 
 function jsdocCastInReturn() {