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

Scalafmt doesn't handle 2.12.2 syntax for commas at the end of a multi-line list #909

Closed
lespea opened this issue Apr 24, 2017 · 11 comments
Closed

Comments

@lespea
Copy link

lespea commented Apr 24, 2017

This template is a guideline, not a strict requirement.

  • Version: 0.6.8
  • Integration: IntelliJ / console tool
  • Configuration: any

Steps

Having a comma in the last item of a list (spanning multiple lines) is now valid syntax in scala 2.12.2. Scalafmt doesn't seem to support this and throws this exception: illegal start of simple expression.

# cat t.scala
object Test {
  val a = Seq(
    1,
    2,
    3,
  )
}
# scalac -version
Scala compiler version 2.12.2 -- Copyright 2002-2017, LAMP/EPFL and Lightbend, Inc.
# scalac t.scala
# scalafmt
[warn] Error in .../t.scala: <input>:6: error: illegal start of simple expression
)
    ^

Expectations

Scalafmt handles this syntax

Wishlist

It would also be cool if there was a config option to force all lists to include a comma on the last item.

@olafurpg
Copy link
Member

Thank you for reporting! This should actually be easy to accommodate, I added support for parsing trailing commas in scala.meta (scalameta/scalameta#660) as soon as they got merged into scala/scala. I am very excited to use them myself :)

I agree that it would be nice to have a flag to insert them where appropriate (e.g. in config style only). That should totally be doable!

@mushtaq
Copy link

mushtaq commented May 8, 2017

+1

@lespea
Copy link
Author

lespea commented Jun 5, 2017

Any update on this?

@olafurpg
Copy link
Member

olafurpg commented Jun 6, 2017

I am currently swamped with tasks in scalameta+scalafix, so I haven't had much bandwidth to work on scalafmt lately. All the pieces for this issue are already in place, the only thing missing is to add support to use a scala.meta.Dialect that supports trailing commas. The current supported dialects are here

implicit lazy val dialectReader: ConfDecoder[Dialect] =
ReaderUtil.oneOf[Dialect](Scala211, Sbt0137, Dotty, Paradise211)

I believe we need to add another case for Scala212 where

val Scala212 = scala.meta.dialects.Scala212.copy(allowTrailingCommas = true) // we forgot to change in last 1.7 release

@olafurpg
Copy link
Member

olafurpg commented Jun 6, 2017

I don't have the luxury to work with 2.12-only projects (only cross 2.11/2.12 even 2.10) so trailing commas haven't hit my day to day workflow yet 😄

@olafurpg
Copy link
Member

olafurpg commented Jun 6, 2017

BTW, I implemented a while ago a rewrite to insert trailing commas where they might make sense olafurpg/scalafix@21e35a3#diff-601bbf131241a9bce3e6683e6143eb83R17 Here's a diff from running it olafurpg/scala-repos#19 We could probably add that as a rewrite.rules = [UseTrailingCommas] to force usage of trailing commas in for example config style

@lespea
Copy link
Author

lespea commented Jun 6, 2017

Cool. It obviously isn't a showstopper or anything I was just curious. Thanks for the update :)

@olafurpg
Copy link
Member

olafurpg commented Jul 9, 2017

This got fixed in #979

@olafurpg olafurpg closed this as completed Jul 9, 2017
@mjaghouri
Copy link

This still fails if there a comment on the line after the trailing comma.

object Test {
  val a = Seq(
    1,
    2,
    3,  // this comment makes formatting fail
  )
}

@gabro
Copy link
Member

gabro commented May 31, 2018

@mjaghouri just stumbled upon this ticket and the error you were seeing was a Scalameta parser bug, that recently got fixed (scalameta/scalameta#1532). The upcoming Scalafmt 1.6 will handle that case too.

@olafurpg
Copy link
Member

It can already be tested in 1.6.0-RC1

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

5 participants