scala-ide / scalariform

Scala source code formatter
http://scala-ide.github.com/scalariform/
MIT License
527 stars 148 forks source link

trailing comma support - "Expected identifier, but got Token(RPAREN,),xxx,)) #260

Closed marekzebrowski closed 5 years ago

marekzebrowski commented 6 years ago

linked issue with reproducer https://github.com/scalastyle/scalastyle/issues/276 even simpler example of valid scala 2.12 code

case class A(i:Int,
)

is not parsed correctly

godenji commented 6 years ago

For everyone who thumbs up'd this issue the companion PR won't get merged until there's some test coverage that verifies trailing commas work as expected (and don't break existing tests).

A simple test implementation might look like this.

c-dante commented 6 years ago

I made a parser spec that the PR fails on -- working on a fix that supports http://docs.scala-lang.org/sips/completed/trailing-commas.html

TL;DR: https://github.com/c-dante/scalariform/tree/260-trailing-comma Current way the parser/lexer work seems it won't allow a nice way to accommodate trailing commas with formatting intents. Debating what to do to support this.

Edit: Seems this issue is going to be annoying to fix for only the allowed cases, since you want to know what your terminal token is -- inParens, inBrackets, and inBraces don't know if they allow trailing --- tokenSeparated can know if it allows trailing, and allow the R* cases if true, but that feels loose.

Edit2: Further complicated because this is valid:

Seq(1,
)

However this is not: Seq(1,)

Which the current lexer setup doesn't allow checking for

Edit3: Further still, the current way of representing token expressions in the parser prevents a setting to control intent-based handling. This is a similar setting to dangling parenthesis setting.

godenji commented 5 years ago

fixed in #262