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

@@ -67,7 +67,14 @@ function handleOwnLineComment(comment, text, options, ast, isLastComment) {
       comment,
       options
     ) ||
-    handleLabeledStatementComments(enclosingNode, comment)
+    handleLabeledStatementComments(enclosingNode, comment) ||
+    handleTernaryTrailingComments(
+      enclosingNode,
+      precedingNode,
+      comment,
+      text,
+      options
+    )
   );
 }
 
@@ -406,9 +413,7 @@ function handleConditionalExpressionComments(
 
   if (
     (!precedingNode || !isSameLineAsPrecedingNode) &&
-    enclosingNode &&
-    (enclosingNode.type === "ConditionalExpression" ||
-      enclosingNode.type === "TSConditionalType") &&
+    isTernaryExpression(enclosingNode) &&
     followingNode
   ) {
     addLeadingComment(followingNode, comment);
@@ -942,6 +947,46 @@ function handleTSMappedTypeComments(
   return false;
 }
 
+function hasQuestionBetweenTestAndComment(testNode, comment, text, options) {
+  const testNodeLocEnd = options.locEnd(testNode);
+  const commentLocStart = options.locStart(comment);
+  for (let i = testNodeLocEnd; i < commentLocStart; i++) {
+    i = privateUtil.getNextNonSpaceNonCommentCharacterIndexWithStartIndex(
+      text,
+      i
+    );
+    if (text[i] === "?") {
+      return i < commentLocStart;
+    }
+  }
+  return false;
+}
+
+function handleTernaryTrailingComments(
+  enclosingNode,
+  precedingNode,
+  comment,
+  text,
+  options
+) {
+  // test
+  //   // comment
+  //   ? first
+  //   : second
+  if (
+    isTernaryTest(precedingNode, enclosingNode) &&
+    // test ?
+    //   // comment
+    //   first
+    //   : second
+    !hasQuestionBetweenTestAndComment(precedingNode, comment, text, options)
+  ) {
+    addTrailingComment(precedingNode, comment);
+    return true;
+  }
+  return false;
+}
+
 function isBlockComment(comment) {
   return comment.type === "Block" || comment.type === "CommentBlock";
 }
@@ -1008,6 +1053,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 +1078,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 +1112,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 {
@@ -431,9 +432,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
@@ -111,6 +168,26 @@ test
 test ? test /* c
 c */? foo : bar : bar;
 
+(async () => {
+  (await test)
+  // comment
+  ?
+  foo :  bar;
+});
+
+(test)
+// comment
+?
+foo : bar;
+
+(test) ?
+// comment
+foo :  bar;
+
+test ?
+// comment
+foo :  bar;
+
 =====================================output=====================================
 var inspect =
   4 === util.inspect.length
@@ -135,8 +212,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 +230,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
@@ -218,6 +352,28 @@ c */
     : bar
   : bar;
 
+async () => {
+  (await test)
+    // comment
+    ? foo
+    : bar;
+};
+
+test
+  // comment
+  ? foo
+  : bar;
+
+test
+  ? // comment
+    foo
+  : bar;
+
+test
+  ? // comment
+    foo
+  : bar;
+
 ================================================================================
 `;