JavaScript: Prefer a trailing comment when comment on between test and consequent on ternary

changelog_unreleased/javascript/pr-8554.md

@@ -0,0 +1,22 @@
+#### Prefer a trailing comment when comment on between test and consequent on ternary ([#8554](https://github.com/prettier/prettier/pull/8554) by [@sosukesuzuki](https://github.com/sosukesuzuki))
+
+<!-- prettier-ignore -->
+```js
+// Input
+test
+  // comment
+  ? foo
+  : bar;
+
+// Prettier (stable)
+test
+  ? // comment
+    foo
+  : bar;
+
+// Prettier (master)
+test
+  // comment
+  ? foo
+  : bar;
+```

src/language-js/comments.js

@@ -8,7 +8,14 @@ const {
   getNextNonSpaceNonCommentCharacterIndex,
 } = require("../common/util-shared");
 
-function handleOwnLineComment(comment, text, options, ast, isLastComment) {
+function handleOwnLineComment(
+  comment,
+  text,
+  options,
+  ast,
+  isLastComment,
+  comments
+) {
   const { precedingNode, enclosingNode, followingNode } = comment;
   return (
     handleLastFunctionArgComments(
@@ -67,7 +74,15 @@ function handleOwnLineComment(comment, text, options, ast, isLastComment) {
       comment,
       options
     ) ||
-    handleLabeledStatementComments(enclosingNode, comment)
+    handleLabeledStatementComments(enclosingNode, comment) ||
+    handleTernaryTrailingComments(
+      enclosingNode,
+      precedingNode,
+      comment,
+      comments,
+      text,
+      options
+    )
   );
 }
 
@@ -406,9 +421,7 @@ function handleConditionalExpressionComments(
 
   if (
     (!precedingNode || !isSameLineAsPrecedingNode) &&
-    enclosingNode &&
-    (enclosingNode.type === "ConditionalExpression" ||
-      enclosingNode.type === "TSConditionalType") &&
+    isTernaryExpression(enclosingNode) &&
     followingNode
   ) {
     addLeadingComment(followingNode, comment);
@@ -942,6 +955,56 @@ function handleTSMappedTypeComments(
   return false;
 }
 
+function hasQuestionInRange(text, start, end, comments, options) {
+  for (let i = start; i < end; ++i) {
+    if (text.charAt(i) === "?") {
+      let hasQuestion = true;
+      // Ignore "?" in comments
+      for (const comment of comments) {
+        const commentStart = options.locStart(comment);
+        const commentEnd = options.locEnd(comment);
+        if (commentStart < i && i < commentEnd) {
+          hasQuestion = false;
+        }
+      }
+      return hasQuestion;
+    }
+  }
+  return false;
+}
+
+function handleTernaryTrailingComments(
+  enclosingNode,
+  precedingNode,
+  comment,
+  comments,
+  text,
+  options
+) {
+  // test
+  //   // comment
+  //   ? first
+  //   : second
+  if (
+    isTernaryTest(precedingNode, enclosingNode) &&
+    // test ?
+    //   // comment
+    //   first
+    //   : second
+    !hasQuestionInRange(
+      text,
+      options.locEnd(precedingNode),
+      options.locStart(comment),
+      comments,
+      options
+    )
+  ) {
+    addTrailingComment(precedingNode, comment);
+    return true;
+  }
+  return false;
+}
+
 function isBlockComment(comment) {
   return comment.type === "Block" || comment.type === "CommentBlock";
 }
@@ -1008,6 +1071,20 @@ function getCommentChildNodes(node, options) {
   }
 }
 
+function shouldIndentComment(path) {
+  return (
+    isCommentParentTernaryTest(path) &&
+    !isTernaryExpression(path.getParentNode(2))
+  );
+}
+
+function shouldDedentComment(path) {
+  return (
+    isCommentParentTernaryTest(path) &&
+    isTernaryExpression(path.getParentNode(2))
+  );
+}
+
 function isTypeCastComment(comment) {
   return (
     isBlockComment(comment) &&
@@ -1019,6 +1096,31 @@ function isTypeCastComment(comment) {
   );
 }
 
+function isTernaryExpression(node) {
+  return (
+    node &&
+    (node.type === "ConditionalExpression" || node.type === "TSConditionalType")
+  );
+}
+
+/**
+ * Check if node matches the ternary test node
+ */
+function isTernaryTest(node, ternary) {
+  if (!node || !isTernaryExpression(ternary)) {
+    return false;
+  }
+  const testField = ternary.test ? "test" : "extendsType";
+  return node === ternary[testField];
+}
+
+function isCommentParentTernaryTest(commentPath) {
+  return isTernaryTest(
+    commentPath.getParentNode(),
+    commentPath.getParentNode(1)
+  );
+}
+
 module.exports = {
   handleOwnLineComment,
   handleEndOfLineComment,
@@ -1028,4 +1130,6 @@ module.exports = {
   isTypeCastComment,
   getGapRegex,
   getCommentChildNodes,
+  shouldIndentComment,
+  shouldDedentComment,
 };

src/language-js/printer-estree.js

@@ -5447,4 +5447,6 @@ module.exports = {
   },
   getGapRegex: handleComments.getGapRegex,
   getCommentChildNodes: handleComments.getCommentChildNodes,
+  shouldIndentComment: handleComments.shouldIndentComment,
+  shouldDedentComment: handleComments.shouldDedentComment,
 };

src/main/comments.js

@@ -9,6 +9,7 @@ const {
   indent,
   lineSuffix,
   join,
+  dedent,
   cursor,
 } = require("../document").builders;
 const {
@@ -218,7 +219,14 @@ function attach(comments, ast, text, options) {
       // If a comment exists on its own line, prefer a leading comment.
       // We also need to check if it's the first line of the file.
       if (
-        pluginHandleOwnLineComment(comment, text, options, ast, isLastComment)
+        pluginHandleOwnLineComment(
+          comment,
+          text,
+          options,
+          ast,
+          isLastComment,
+          comments
+        )
       ) {
         // We're good
       } else if (followingNode) {
@@ -431,9 +439,25 @@ function printTrailingComment(commentPath, options) {
       locStart
     );
 
-    return lineSuffix(
+    const printed = lineSuffix(
       concat([hardline, isLineBeforeEmpty ? hardline : "", contents])
     );
+
+    if (
+      options.printer.shouldIndentComment &&
+      options.printer.shouldIndentComment(commentPath)
+    ) {
+      return indent(printed);
+    }
+
+    if (
+      options.printer.shouldDedentComment &&
+      options.printer.shouldDedentComment(commentPath)
+    ) {
+      return dedent(printed);
+    }
+
+    return printed;
   }
 
   let printed = concat([" ", contents]);

tests/js/conditional/__snapshots__/jsfmt.spec.js.snap

@@ -44,6 +44,63 @@ const { configureStore } = process.env.NODE_ENV === "production"
   ? require("./configureProdStore") // a
   : require("./configureDevStore"); // b
 
+test
+  // ?
+  // comment
+  ? foo
+  : bar;
+
+test ?
+  // comment
+  foo
+  : bar;
+
+test ?
+  // comment
+  first
+  : test
+  // comment
+  ? first
+  : second;
+
+test
+  /* comment
+     comment
+     comment
+     comment */
+  ? foo
+  : bar;
+
+test ?
+  /* comment
+     comment
+     comment
+     comment */
+  foo
+  : bar;
+
+test
+  // ?
+  // comment
+  ? foo
+  : test2
+  // comment
+  // comment
+  ? foo
+  : bar;
+
+test
+  /* comment
+     comment
+     comment
+     comment */
+  ? foo
+  : test
+  /* comment
+     comment
+     comment */
+  ? foo
+  : bar;
 test /* comment
   comment
       comment
@@ -135,8 +192,8 @@ var inspect =
       };
 
 const extractTextPluginOptions = shouldUseRelativeAssetPaths
-  ? // Making sure that the publicPath goes back to to build folder.
-    { publicPath: Array(cssFilename.split("/").length).join("../") }
+  // Making sure that the publicPath goes back to to build folder.
+  ? { publicPath: Array(cssFilename.split("/").length).join("../") }
   : {};
 
 const extractTextPluginOptions2 = shouldUseRelativeAssetPaths
@@ -153,6 +210,63 @@ const { configureStore } =
     ? require("./configureProdStore") // a
     : require("./configureDevStore"); // b
 
+test
+  // ?
+  // comment
+  ? foo
+  : bar;
+
+test
+  ? // comment
+    foo
+  : bar;
+
+test
+  ? // comment
+    first
+  : test
+  // comment
+  ? first
+  : second;
+
+test
+  /* comment
+     comment
+     comment
+     comment */
+  ? foo
+  : bar;
+
+test
+  ? /* comment
+     comment
+     comment
+     comment */
+    foo
+  : bar;
+
+test
+  // ?
+  // comment
+  ? foo
+  : test2
+  // comment
+  // comment
+  ? foo
+  : bar;
+
+test
+  /* comment
+     comment
+     comment
+     comment */
+  ? foo
+  : test
+  /* comment
+     comment
+     comment */
+  ? foo
+  : bar;
 test /* comment
   comment
       comment