Fix #4706: Flow generics (#4736)

* Fix #4706: Comments before a PARAM_START token stay before that token

* Simplify nodes

* Add function-in-function test

* Fix #4706: Comments after class name should go after the identifier that's after `class`, not the variable assigned to

* Fix #4706: Top-level identifiers with trailing comments get wrapped in parentheses (around the comment too) so that Flow doesn't interpret it as a JavaScript label

* Cleanup

* If the source has parentheses wrapping an identifier followed by a block comment, output those parentheses rather than optimizing them away; this is a requirement of Flow, to distinguish from JavaScript labels

* More tests for Flow comments

Author: GeoffreyBooth

lib/coffeescript/grammar.js

@@ -262,7 +262,7 @@ grammar =
   # The **Code** node is the function literal. It's defined by an indented block
   # of **Block** preceded by a function arrow, with an optional parameter list.
   Code: [
-    o 'PARAM_START ParamList PARAM_END FuncGlyph Block', -> new Code $2, $5, $4
+    o 'PARAM_START ParamList PARAM_END FuncGlyph Block', -> new Code $2, $5, $4, LOC(1)(new Literal $1)
     o 'FuncGlyph Block',                                 -> new Code [], $2, $1
   ]
 

lib/coffeescript/nodes.js

@@ -502,8 +502,6 @@ exports.Block = class Block extends Base
         # We want to compile this and ignore the result.
         node.compileToFragments o
         continue
-
-      node = node.unwrapAll()
       node = (node.unfoldSoak(o) or node)
       if node instanceof Block
         # This is a nested block. We don’t do anything special here like
@@ -1676,7 +1674,9 @@ exports.Class = class Class extends Base
 
     result = []
     result.push @makeCode "class "
-    result.push @makeCode "#{@name} " if @name
+    result.push @makeCode @name if @name
+    @compileCommentFragments o, @variable, result if @variable?.comments?
+    result.push @makeCode ' ' if @name
     result.push @makeCode('extends '), @parent.compileToFragments(o)..., @makeCode ' ' if @parent
 
     result.push @makeCode '{'
@@ -2486,7 +2486,7 @@ exports.FuncGlyph = class FuncGlyph extends Base
 # When for the purposes of walking the contents of a function body, the Code
 # has no *children* -- they're within the inner scope.
 exports.Code = class Code extends Base
-  constructor: (params, body, @funcGlyph) ->
+  constructor: (params, body, @funcGlyph, @paramStart) ->
     super()
 
     @params      = params or []
@@ -2686,6 +2686,10 @@ exports.Code = class Code extends Base
       modifiers.push '*'
 
     signature = [@makeCode '(']
+    # Block comments between a function name and `(` get output between
+    # `function` and `(`.
+    if @paramStart?.comments?
+      @compileCommentFragments o, @paramStart, signature
     for param, i in params
       signature.push @makeCode ', ' if i isnt 0
       signature.push @makeCode '...' if haveSplatParam and i is params.length - 1
@@ -3339,13 +3343,21 @@ exports.Parens = class Parens extends Base
 
   compileNode: (o) ->
     expr = @body.unwrap()
-    if expr instanceof Value and expr.isAtomic() and not @csxAttribute
+    # If these parentheses are wrapping an `IdentifierLiteral` followed by a
+    # block comment, output the parentheses (or put another way, don’t optimize
+    # away these redundant parentheses). This is because Flow requires
+    # parentheses in certain circumstances to distinguish identifiers followed
+    # by comment-based type annotations from JavaScript labels.
+    shouldWrapComment = expr.comments?.some(
+      (comment) -> comment.here and not comment.unshift and not comment.newLine)
+    if expr instanceof Value and expr.isAtomic() and not @csxAttribute and not shouldWrapComment
       expr.front = @front
       return expr.compileToFragments o
     fragments = expr.compileToFragments o, LEVEL_PAREN
-    bare = o.level < LEVEL_OP and (expr instanceof Op or expr.unwrap() instanceof Call or
-      (expr instanceof For and expr.returns)) and (o.level < LEVEL_COND or
-        fragments.length <= 3)
+    bare = o.level < LEVEL_OP and not shouldWrapComment and (
+        expr instanceof Op or expr.unwrap() instanceof Call or
+        (expr instanceof For and expr.returns)
+      ) and (o.level < LEVEL_COND or fragments.length <= 3)
     return @wrapInBraces fragments if @csxAttribute
     if bare then fragments else @wrapInParentheses fragments
 

lib/coffeescript/parser.js

@@ -693,6 +693,6 @@ CONTROL_IN_IMPLICIT = ['IF', 'TRY', 'FINALLY', 'CATCH', 'CLASS', 'SWITCH']
 DISCARDED = ['(', ')', '[', ']', '{', '}', '.', '..', '...', ',', '=', '++', '--', '?',
   'AS', 'AWAIT', 'CALL_START', 'CALL_END', 'DEFAULT', 'ELSE', 'EXTENDS', 'EXPORT',
   'FORIN', 'FOROF', 'FORFROM', 'IMPORT', 'INDENT', 'INDEX_SOAK', 'LEADING_WHEN',
-  'OUTDENT', 'PARAM_START', 'PARAM_END', 'REGEX_START', 'REGEX_END', 'RETURN',
-  'STRING_END', 'THROW', 'UNARY', 'YIELD'
+  'OUTDENT', 'PARAM_END', 'REGEX_START', 'REGEX_END', 'RETURN', 'STRING_END', 'THROW',
+  'UNARY', 'YIELD'
 ].concat IMPLICIT_UNSPACED_CALL.concat IMPLICIT_END.concat CALL_CLOSERS.concat CONTROL_IN_IMPLICIT

lib/coffeescript/rewriter.js

@@ -973,3 +973,103 @@ test "Flow comment-based syntax support", ->
   fn = function(str/*: string */, num/*: number */)/*: string */ {
     return str + num;
   };'''
+
+test "#4706: Flow comments around function parameters", ->
+  eqJS '''
+  identity = ###::<T>### (value ###: T ###) ###: T ### ->
+    value
+  ''', '''
+  var identity;
+
+  identity = function/*::<T>*/(value/*: T */)/*: T */ {
+    return value;
+  };'''
+
+test "#4706: Flow comments around function parameters", ->
+  eqJS '''
+  copy = arr.map(###:: <T> ###(item ###: T ###) ###: T ### => item)
+  ''', '''
+  var copy;
+
+  copy = arr.map(/*:: <T> */(item/*: T */)/*: T */ => {
+    return item;
+  });'''
+
+test "#4706: Flow comments after class name", ->
+  eqJS '''
+  class Container ###::<T> ###
+    method: ###::<U> ### () -> true
+  ''', '''
+  var Container;
+
+  Container = class Container/*::<T> */ {
+    method() {
+      return true;
+    }
+
+  };'''
+
+test "#4706: Identifiers with comments wrapped in parentheses remain wrapped", ->
+  eqJS '(arr ###: Array<number> ###)', '(arr/*: Array<number> */);'
+  eqJS 'other = (arr ###: any ###)', '''
+  var other;
+
+  other = (arr/*: any */);'''
+
+test "#4706: Flow comments before class methods", ->
+  eqJS '''
+  class Container
+    ###::
+    method: (number) => string;
+    method: (string) => number;
+    ###
+    method: -> true
+  ''', '''
+  var Container;
+
+  Container = class Container {
+    /*::
+    method: (number) => string;
+    method: (string) => number;
+    */
+    method() {
+      return true;
+    }
+
+  };'''
+
+test "#4706: Flow comments for class method params", ->
+  eqJS '''
+  class Container
+    method: (param ###: string ###) -> true
+  ''', '''
+  var Container;
+
+  Container = class Container {
+    method(param/*: string */) {
+      return true;
+    }
+
+  };'''
+
+test "#4706: Flow comments for class method returns", ->
+  eqJS '''
+  class Container
+    method: () ###: string ### -> true
+  ''', '''
+  var Container;
+
+  Container = class Container {
+    method()/*: string */ {
+      return true;
+    }
+
+  };'''
+
+test "#4706: Flow comments for function spread", ->
+  eqJS '''
+  method = (...rest ###: Array<string> ###) =>
+  ''', '''
+  var method;
+
+  method = (...rest/*: Array<string> */) => {};'''

lib/coffeescript/sourcemap.js

@@ -171,7 +171,6 @@ test "multiline simple import", ->
       bar as baz
     } from 'lib';"""
 
-
 test "multiline complex import", ->
   eqJS """
     import foo, {
@@ -474,7 +473,6 @@ test "export default named member, within an object", ->
       bar
     };"""
 
-
 # Import and export in the same statement
 
 test "export an entire module's contents", ->