Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
marekzebrowski opened this issue Nov 8, 2017 · 3 comments

Comments

@marekzebrowski
Copy link
Contributor

linked issue with reproducer scalastyle/scalastyle#276
even simpler example of valid scala 2.12 code

case class A(i:Int,
)

is not parsed correctly

This was referenced Nov 11, 2017
@godenji
Copy link
Collaborator

godenji commented Jan 4, 2018

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
Copy link

c-dante commented Feb 5, 2018

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 godenji changed the title trailing coma support - "Expected identifier, but got Token(RPAREN,),xxx,)) trailing comma support - "Expected identifier, but got Token(RPAREN,),xxx,)) Feb 8, 2019
@godenji
Copy link
Collaborator

godenji commented Feb 8, 2019

fixed in #262

@godenji godenji closed this as completed Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants