Fix Flow generic comment positioning (#5290)

* add test for ensuring that calls with zero arguments look right

src/language-js/printer-estree.js

@@ -34,7 +34,10 @@ const insertPragma = require("./pragma").insertPragma;
 const handleComments = require("./comments");
 const pathNeedsParens = require("./needs-parens");
 const preprocess = require("./preprocess");
-const { hasFlowShorthandAnnotationComment } = require("./utils");
+const {
+  hasFlowAnnotationComment,
+  hasFlowShorthandAnnotationComment
+} = require("./utils");
 
 const {
   builders: {
@@ -1115,6 +1118,19 @@ function printPathNoParens(path, options, print, args) {
         ]);
       }
 
+      // Inline Flow annotation comments following Identifiers in Call nodes need to
+      // stay with the Identifier. For example:
+      //
+      // foo /*:: <SomeGeneric> */(bar);
+      //
+      // Here, we ensure that such comments stay between the Identifier and the Callee.
+      const isIdentifierWithFlowAnnotation =
+        n.callee.type === "Identifier" &&
+        hasFlowAnnotationComment(n.callee.trailingComments);
+      if (isIdentifierWithFlowAnnotation) {
+        n.callee.trailingComments[0].printed = true;
+      }
+
       // We detect calls on member lookups and possibly print them in a
       // special chain format. See `printMemberChain` for more info.
       if (!isNew && isMemberish(n.callee)) {
@@ -1125,6 +1141,9 @@ function printPathNoParens(path, options, print, args) {
         isNew ? "new " : "",
         path.call(print, "callee"),
         optional,
+        isIdentifierWithFlowAnnotation
+          ? `/*:: ${n.callee.trailingComments[0].value.substring(2).trim()} */`
+          : "",
         printFunctionTypeParameters(path, options, print),
         printArgumentsList(path, options, print)
       ]);
@@ -2876,7 +2895,27 @@ function printPathNoParens(path, options, print, args) {
     }
 
     case "TypeParameterDeclaration":
-    case "TypeParameterInstantiation":
+    case "TypeParameterInstantiation": {
+      const value = path.getValue();
+      const commentStart = value.range
+        ? options.originalText.substring(0, value.range[0]).lastIndexOf("/*")
+        : -1;
+      // As noted in the TypeCastExpression comments above, we're able to use a normal whitespace regex here
+      // because we know for sure that this is a type definition.
+      const commentSyntax =
+        commentStart >= 0 &&
+        options.originalText.substring(commentStart).match(/^\/\*\s*::/);
+      if (commentSyntax) {
+        return concat([
+          "/*:: ",
+          printTypeParameters(path, options, print, "params"),
+          " */"
+        ]);
+      }
+
+      return printTypeParameters(path, options, print, "params");
+    }
+
     case "TSTypeParameterDeclaration":
     case "TSTypeParameterInstantiation":
       return printTypeParameters(path, options, print, "params");
@@ -6025,7 +6064,13 @@ function willPrintOwnComments(path) {
   const parent = path.getParentNode();
 
   return (
-    ((node && (isJSXNode(node) || hasFlowShorthandAnnotationComment(node))) ||
+    ((node &&
+      (isJSXNode(node) ||
+        hasFlowShorthandAnnotationComment(node) ||
+        (parent &&
+          parent.type === "CallExpression" &&
+          (hasFlowAnnotationComment(node.leadingComments) ||
+            hasFlowAnnotationComment(node.trailingComments))))) ||
       (parent &&
         (parent.type === "JSXSpreadAttribute" ||
           parent.type === "JSXSpreadChild" ||

src/language-js/utils.js

@@ -1,5 +1,20 @@
 "use strict";
 
+// We match any whitespace except line terminators because
+// Flow annotation comments cannot be split across lines. For example:
+//
+// (this /*
+// : any */).foo = 5;
+//
+// is not picked up by Flow (see https://github.com/facebook/flow/issues/7050), so
+// removing the newline would create a type annotation that the user did not intend
+// to create.
+const NON_LINE_TERMINATING_WHITE_SPACE = "(?:(?=.)\\s)";
+const FLOW_SHORTHAND_ANNOTATION = new RegExp(
+  `^${NON_LINE_TERMINATING_WHITE_SPACE}*:`
+);
+const FLOW_ANNOTATION = new RegExp(`^${NON_LINE_TERMINATING_WHITE_SPACE}*::`);
+
 function hasFlowShorthandAnnotationComment(node) {
   // https://flow.org/en/docs/types/comments/
   // Syntax example: const r = new (window.Request /*: Class<Request> */)("");
@@ -8,19 +23,15 @@ function hasFlowShorthandAnnotationComment(node) {
     node.extra &&
     node.extra.parenthesized &&
     node.trailingComments &&
-    // We match any whitespace except line terminators because
-    // Flow annotation comments cannot be split across lines. For example:
-    //
-    // (this /*
-    // : any */).foo = 5;
-    //
-    // is not picked up by Flow (see https://github.com/facebook/flow/issues/7050), so
-    // removing the newline would create a type annotation that the user did not intend
-    // to create.
-    node.trailingComments[0].value.match(/^(?:(?=.)\s)*:/)
+    node.trailingComments[0].value.match(FLOW_SHORTHAND_ANNOTATION)
   );
 }
 
+function hasFlowAnnotationComment(comments) {
+  return comments && comments[0].value.match(FLOW_ANNOTATION);
+}
+
 module.exports = {
-  hasFlowShorthandAnnotationComment
+  hasFlowShorthandAnnotationComment,
+  hasFlowAnnotationComment
 };

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

@@ -45,6 +45,33 @@ function foo<T>(bar /*: T[] */, baz /*: T */) /*: S */ {}
 
 `;
 
+exports[`generics.js - flow-verify 1`] = `
+const Component = branch/*::     <Props, ExternalProps> */(
+  ({ src }) => !src,
+  // $FlowFixMe
+  renderNothing,
+)(BaseComponent);
+
+const C = b/*:: <A> */(foo) + c/*:: <B> */(bar);
+
+foo/*::<bar>*/(baz);
+
+foo/*::<bar>*/();
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+const Component = branch/*:: <Props, ExternalProps> */(
+  ({ src }) => !src,
+  // $FlowFixMe
+  renderNothing
+)(BaseComponent);
+
+const C = b/*:: <A> */(foo) + c/*:: <B> */(bar);
+
+foo/*:: <bar> */(baz);
+
+foo/*:: <bar> */();
+
+`;
+
 exports[`let.js - flow-verify 1`] = `
 let foo /*: Groups<T> */;
 let foo /*: string */ = 'a';

tests/flow_comments/babylon_only/__snapshots__/jsfmt.spec.js.snap

@@ -0,0 +1,28 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`class_with_generics.js - babylon-verify 1`] = `
+import React from 'react';
+
+/*:: type Props = {
+  foo?: ?string,
+  bar: number,
+}; */
+
+/*:: type State = { baz: number }; */
+
+class Component extends React.Component/*:: <Props, State> */ {
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+import React from "react";
+
+/*:: type Props = {
+  foo?: ?string,
+  bar: number,
+}; */
+
+/*:: type State = { baz: number }; */
+
+class Component extends React.Component /*:: <Props, State> */ {
+}
+
+`;

tests/flow_comments/babylon_only/class_with_generics.js

@@ -0,0 +1,11 @@
+import React from 'react';
+
+/*:: type Props = {
+  foo?: ?string,
+  bar: number,
+}; */
+
+/*:: type State = { baz: number }; */
+
+class Component extends React.Component/*:: <Props, State> */ {
+}

tests/flow_comments/babylon_only/jsfmt.spec.js

@@ -0,0 +1 @@
+run_spec(__dirname, ["babylon"]);

tests/flow_comments/generics.js

@@ -0,0 +1,11 @@
+const Component = branch/*::     <Props, ExternalProps> */(
+  ({ src }) => !src,
+  // $FlowFixMe
+  renderNothing,
+)(BaseComponent);
+
+const C = b/*:: <A> */(foo) + c/*:: <B> */(bar);
+
+foo/*::<bar>*/(baz);
+
+foo/*::<bar>*/();