Lexer: Block comment in array literal unexpectedly throws ‘unexpected newline’

[cueedee]

The following code produces Error on line 2: unexpected newline

[
    ###  ###
    'foobar'
]

Whereas this will compile just fine:

{
    ###  ###
    'foobar'
}

[rdeforest]

Since arrays are defined by

  Array: [
    o '[ ]',                                    -> new Arr []
    o '[ ArgList OptComma ]',                   -> new Arr $2
  ]

The bug also exists for block comments in function arg declarations.

f = (a...) -> console.log "f(#{a.join ", "})"

f('foo'
# this compiles
'bar'
)

f('foo' ### this doesn't ###, 'bar')

And unsurprisingly the same goes for function expression parameter lists:

thisCompiles = ( # despite this comment
) ->

thisFailsToCompile = ( ### just like array literals and function calls ###
) ->

This led me to wonder why. The reason is that the lexer consumes single-line comments without generating a token, but generates a HERECOMMENT token for here-comments which is then aliased to Comment in grammar.coffee and included in the definitions of the non-terminal tokens Statement and AssignObj.

It seems that the reason HERECOMMENT tokens are generated is so that they can be converted into multi-line JavaScript comments. Why wouldn't we want to do the same with single-line comments?

I have test cases for the array and function arg bugs and I'm going to make them pass and send a PR, but now that I understand the two kinds of comments I'm wondering what the correct fix is. Should I also preserve single-line comments, open a separate issue for that or just leave the inconsistency there?

I went to js2.coffee to see what they do with comments and the results surprised me a little:

• Multi-line comments generally get converted into here-comments
  • but not inside arrays or function arguments, which makes sense given this bug,
  • and not inside object literals either, which could be considered a js2.coffee bug I guess?
• Single-line comments usually get converted into single-line CS comments
  • but not in arrays, objects or function arguments, so at least that's consistent?

I'm going to come up with a fix for just this bug but would like some guidance on the above.

Thanks!

P.S. It amused me to realize syntax highlighting on GitHub and in github.com/kchmck/vim-coffee-script were not confused by my examples. :)

[GeoffreyBooth]

This is something that https://github.com/decaffeinate/decaffeinate has also addressed, as that project aims to preserve CoffeeScript comments in their JavaScript output.

[jashkenas]

We could really use a better overall strategy for preserving herecomments that doesn't depend on them being actual nodes in the AST. There must be some wise techniques for comment parsing out there.

[GeoffreyBooth]

Fixed via #4572.

[cueedee]

@GeoffreyBooth, am I right in assuming that this fix has landed in CoffeeScript-2 only and not 1?

[GeoffreyBooth]

That is correct. We’re not backporting fixes into 1. There are very few breaking changes in 2, it should be fairly easy for most projects to upgrade.

[cueedee]

Thanks for the quick response... he said, puzzling over how on earth he can avoid using this before invoking super from his constructors...

[connec]

@cueedee might not help in your case, but bear in mind that @-params and bound methods still work in CS2 (and so can emulate some 'usage' of this before invoking super):

class Child extends Parent
  constructor: (@name) ->
    super arguments...
    setTimeout @sayHi, 1000

  sayHi: =>
    console.log @name

[cueedee]

@connec, thank you for caring. At the risk of pushing this thread further off-topic, It don't think it will indeed help in my case ( which has to to do with transforming a @routes property in an extended Backbone.Router's constructor before invoking super ).

[connec]

Looks like your best bet would be to rework it to use the options argument (though I appreciate that would be a non-trivial refactor, compared to it Just Works™ing).

class MyRouter extends Backbone.Router
  constructor: ({ routes }) ->
    do_something_with routes
    super { routes }