Fix last comment of an if test (#1042)

This one sucks.

The range of the `test` of `if (a /* comment */) {}` is only `a` and doesn't include the comment nor parenthesis. This means that we have no way to know if the comment is placed before or after the `)` unless you look at the actual source and strip all the whitespace/comments characters to see if it's a `)`...

This happened on the babel source code twice and many times in the fb codebase. I think we need to fix it unfortunately :(

Fixes #867

src/comments.js

@@ -19,6 +19,7 @@ var comparePos = util.comparePos;
 var childNodesCacheKey = Symbol("child-nodes");
 var locStart = util.locStart;
 var locEnd = util.locEnd;
+var getNextNonSpaceNonCommentCharacter = util.getNextNonSpaceNonCommentCharacter;
 
 // TODO Move a non-caching implementation of this function into ast-types,
 // and implement a caching wrapper function here.
@@ -154,7 +155,7 @@ function attach(comments, ast, text, options) {
           comment
         ) ||
         handleMemberExpressionComments(enclosingNode, followingNode, comment) ||
-        handleIfStatementComments(enclosingNode, followingNode, comment) ||
+        handleIfStatementComments(text, enclosingNode, followingNode, comment) ||
         handleTryStatementComments(enclosingNode, followingNode, comment) ||
         handleClassComments(enclosingNode, comment) ||
         handleImportSpecifierComments(enclosingNode, comment) ||
@@ -217,7 +218,7 @@ function attach(comments, ast, text, options) {
       }
     } else {
       if (
-        handleIfStatementComments(enclosingNode, followingNode, comment) ||
+        handleIfStatementComments(text, enclosingNode, followingNode, comment) ||
         handleObjectPropertyAssignment(enclosingNode, precedingNode, comment) ||
         handleTemplateLiteralComments(enclosingNode, comment) ||
         handleCommentInEmptyParens(enclosingNode, comment) ||
@@ -363,13 +364,25 @@ function addBlockOrNotComment(node, comment) {
 //     // comment
 //     ...
 //   }
-function handleIfStatementComments(enclosingNode, followingNode, comment) {
+function handleIfStatementComments(text, enclosingNode, followingNode, comment) {
   if (
-    !enclosingNode || enclosingNode.type !== "IfStatement" || !followingNode
+    !enclosingNode ||
+    enclosingNode.type !== "IfStatement" ||
+    !followingNode
   ) {
     return false;
   }
 
+  // We unfortunately have no way using the AST or location of nodes to know
+  // if the comment is positioned before or after the condition parenthesis:
+  //   if (a /* comment */) {}
+  //   if (a) /* comment */ {}
+  // The only workaround I found is to look at the next character to see if
+  // it is a ).
+  if (getNextNonSpaceNonCommentCharacter(text, comment) === ")") {
+    return false;
+  }
+
   if (followingNode.type === "BlockStatement") {
     addBlockStatementFirstComment(followingNode, comment);
     return true;

src/util.js

@@ -229,6 +229,19 @@ function isNextLineEmpty(text, node) {
   return hasNewline(text, idx);
 }
 
+function getNextNonSpaceNonCommentCharacter(text, node) {
+  let oldIdx = null;
+  let idx = locEnd(node);
+  while (idx !== oldIdx) {
+    oldIdx = idx;
+    idx = skipSpaces(text, idx);
+    idx = skipInlineComment(text, idx);
+    idx = skipTrailingComment(text, idx);
+    idx = skipNewline(text, idx);
+  }
+  return text.charAt(idx);
+}
+
 function hasSpaces(text, index, opts) {
   opts = opts || {};
   const idx = skipSpaces(text, opts.backwards ? index - 1 : index, opts);
@@ -306,6 +319,7 @@ module.exports = {
   getParentExportDeclaration,
   getPenultimate,
   getLast,
+  getNextNonSpaceNonCommentCharacter,
   skipWhitespace,
   skipSpaces,
   skipNewline,

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

@@ -115,3 +115,24 @@ async function f() {
 }
 "
 `;
+
+exports[`trailing_comment.js 1`] = `
+"if (code === 92 /* '\\\\' */) {}
+if (code === 92 /* '\\\\' */ /* '\\\\' */) {}
+
+if (code === 92) /* '\\\\' */ {}
+if (code === 92) { /* '\\\\' */ }
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+if (code === 92 /* '\\\\' */) {
+}
+if (code === 92 /* '\\\\' */ /* '\\\\' */) {
+}
+
+if (code === 92) {
+  /* '\\\\' */
+}
+if (code === 92) {
+  /* '\\\\' */
+}
+"
+`;

tests/if/trailing_comment.js

@@ -0,0 +1,5 @@
+if (code === 92 /* '\' */) {}
+if (code === 92 /* '\' */ /* '\' */) {}
+
+if (code === 92) /* '\' */ {}
+if (code === 92) { /* '\' */ }