Skip to content
3 changes: 2 additions & 1 deletion lib/coffeescript/grammar.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 31 additions & 12 deletions lib/coffeescript/nodes.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion lib/coffeescript/parser.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/coffeescript/rewriter.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 9 additions & 9 deletions lib/coffeescript/sourcemap.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/grammar.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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
]

Expand Down
28 changes: 20 additions & 8 deletions src/nodes.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@connec Do you think there’s any concern about me removing this line? I removed it in order to not necessarily always optimize away redundant parentheses (which is what we’re dealing with in this case).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GeoffreyBooth why would we not want to optimize away redundant parentheses? Otherwise nothing jumps out at me!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of this particular case. When parentheses wrap an IdentifierLiteral that has a comment, we want those parentheses to be output because Flow requires them. See #4706 (comment).

I’m assuming that removing this optimization doesn’t break anything else, though, right?

node = (node.unfoldSoak(o) or node)
if node instanceof Block
# This is a nested block. We don’t do anything special here like
Expand Down Expand Up @@ -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 '{'
Expand Down Expand Up @@ -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 []
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions src/rewriter.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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
100 changes: 100 additions & 0 deletions test/comments.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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> */) => {};'''
2 changes: 0 additions & 2 deletions test/modules.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ test "multiline simple import", ->
bar as baz
} from 'lib';"""


test "multiline complex import", ->
eqJS """
import foo, {
Expand Down Expand Up @@ -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", ->
Expand Down