SI-4986 Allow trailing commas before newlines

[dwijnand]

.. with updated specification.

I've taken a pragmatic approach of adding trailing comma support to:

• tuples
• argument and parameter groups, including for implicits, for functions, methods and constructors
• import selectors

I'm happy to add more, but I wanted to start conservative.

If need be, I'm happy to draft a SIP (or SLIP) for this.

Review by @adriaanm.

[edit]

SIP: scala/docs.scala-lang#533

[mpociecha]

Was it discussed somewhere? Personally I don't like this change [edit: as much as others do]. I don't think benefits are significant and it can be error-prone e.g. in case of overloaded methods (I mean the typical users).

[densh]

For consistency patterns, types and type parameters should all support trailing commas if those exist for expressions and method parameters. (See SimplePattern, SimpleType and TypeParamClause in the syntax summary.)

[densh]

Here is the full list of rules that contain comma-separated parts:

• ValDcl, VarDcl, VarDef via ids
• Type via FunctionArgTypes
• SimpleType, TypeArgs via Types
• Expr, ResultExpr via Bindings
• SimpleExpr1, ArgumentExprs via Exprs
• SimplePattern via Patterns
• TypeParamClause, FunTypeParamClause
• ParamClause, ParamClauses via Params
• ClassParamClause, ClassParamClauses via ClassParams
• ImportExp
• ImportSelector
• PatDef

[sjrd]

As much as I would welcome this change, I agree with @mpociecha that this needs some kind of open discussion. It should actually be a SIP, at least insofar as having a document explaining the change and why it is important.

[dwijnand]

/rebuild

[gabrieljones]

How about make commas optional the same way semicolons are optional.

[cvogt]

I wish we had this forever. Makes it easier to move code around and comment out lines. +1

[som-snytt]

SI-8739 is about autocompletion of f(g, x) at f(x => x. ,, where the comma is currently needed because of arity check for applicability. I wonder if there may be an edge case in function args.

[dwijnand]

@mpociecha

Was it discussed somewhere? Personally I don't like this change. I don't think benefits are significant and it can be error-prone e.g. in case of overloaded methods (I mean the typical users).

and @sjrd

As much as I would welcome this change, I agree with @mpociecha that this needs some kind of open discussion. It should actually be a SIP, at least insofar as having a document explaining the change and why it is important.

For context, Scala used to support trailing commas in the past. Then they got removed without discussion. Since then, over the years, there have been a number of short conversations about it, on scala-internals, scala-language and more recently on Gitter and Reddit, but nothing extensive or complete, and typically mixed up with discussing other language or library changes in Scala. Additionally, I'm sure there have also been discussions around this language feature in other languages.

I knew that that there was a high probability that a SIP would be necessary, my approach was to provide a (hopefully) complete-looking PR for early feedback and start an open discussion here.

But I will draft up a SIP for this, such that it can be fully discussed and cleanly documented for the future.

Also, @mpociecha, please could you provide an example where this is error-prone, e.g. in case of overloaded methods?

[dwijnand]

@densh

For consistency patterns, types and type parameters should all support trailing commas if those exist for expressions and method parameters. (See SimplePattern, SimpleType and TypeParamClause in the syntax summary.)

I would be perfectly happy to add ubiquitous, consistent support to trailing commas in every case, but I chose instead this subset as it's the minimum that I feel much much more strongly about, and think would have a greater chance of success (ie. inclusion).

But I will make sure to add this as to the SIP as a potential alternative.

[dwijnand]

@gabrieljones

How about make commas optional the same way semicolons are optional.

Personally I find that a very interesting idea, but it's much more of an aggressive change then my little thing here.

[dwijnand]

@som-snytt

SI-8739 is about autocompletion of f(g, x) at f(x => x. ,, where the comma is currently needed because of arity check for applicability. I wonder if there may be an edge case in function args.

Thanks for the link, I'll circle back to that after working on the SIP.

[mpociecha]

@dwijnand Thanks for the explanation. Now "return" in the title makes sense.

Honestly I didn't mean anything more than:

scala> :paste
// Entering paste mode (ctrl-D to finish)

def foo(bar: Int) = bar
def foo(bar: Int, baz: Int) = bar + baz
foo(1,)

// Exiting paste mode, now interpreting.

foo: (bar: Int)Int <and> (bar: Int, baz: Int)Int
foo: (bar: Int)Int <and> (bar: Int, baz: Int)Int
res0: Int = 1

The question is whether someone forgot to add the second param and the compiler didn't report this due to the new behaviour, or they came up with the idea of adding commas at the end in case they would like to add another param in the future. ;) Yes, I know that I'm exaggerating, but there are people who write

def foo = {
  something
}

instead of

def foo = something

"in case they'd want to add something else to such a method in the future". Hence, nothing will surprise me. ;)

[dwijnand]

@mpociecha Yep, I see what you mean. But I'm not sure I would classify that as "error-prone" - that line of thinking would mean that all overloading should be removed: you might've written bar(1) when you meant to write bar(1, 2).

[mpociecha]

Yes, that's why I admitted that I'm exaggerating.

[som-snytt]

The completion example is not an exaggeration. But the effects might be subtle. (The proposed fix for that example says the method can't be overloaded, FWIW.)

[adriaanm]

Happy to discuss here, but we can't schedule this for 2.12.0, so I'm assigning it to the WIP milestone. Also, yes, this will need to be SIPped.

[dwijnand]

I've drafted and pull requested the SIP for this: scala/docs.scala-lang#533.

[xeno-by]

I thought that you guys should know that we've reviewed Dale's proposal during our last SIP meeting and decided that it sounds interesting. In the coming weeks/months, we'll be working with Dale to comprehensively review his SIP. Feel free to join the discussion at scala/docs.scala-lang#533.

[milessabin]

A backport of this to 2.11.x for 2.11.9 would be useful as an aid to migration.

[dwijnand]

/rebuild 0692bde

[dwijnand]

The SIP for this (SIP-27) was accepted in the November SIP committee meeting.

I've updated the implementation to reflect the final design decision.

I've also written a pos and a neg test to test at least one instance of trailing commas in each of the relevant elements of the syntax grammar.

Could anyone spare some time to review this and give my their feedback please?

Also, could this have it's on-hold label removed and moved into the 2.12.2 milestone please?

[adriaanm]

🎉

Thank you, @dwijnand for your hard work, and everyone involved for your feedback and your patience! Feel free to let your commas trail henceforth.

[adriaanm]

PS: one more thing this will need a spec update ;-)

[retronym]

/cc @Alefas @pavelfatin for IntelliJ support

[som-snytt]

Probably "organize imports" will eliminate trailing commas. Teams with a mix of IDE users and real programmers will have to tread carefully. It might be the last straw for groups who worked through tabs vs spaces.

[jvican]

/cc For scalameta/scalafix support @olafurpg @xeno-by

[dwijnand]

Thanks @adriaanm for the review and merge!

TODO:

• PR an update to the SIP document with the final implementation details
• PR an update the spec
• PR intellij-scala
• PR scalariform
• PR scala.meta
• PR scalaparse (lihaoyi/fastparse)

[adriaanm]

TODO list moved to scala/scala-dev#299

[milessabin]

FYI: this has been merged in Typelevel Scala 2.12.1.

[xeno-by]

Hi everyone! Can we have this pull request backported to 2.11.10 please?

Judging by the history of 2.10.x, I think that people are going to use 2.11.x for quite a while. Therefore, I believe that trailing comma support in 2.11.x will be quite important for cross-compilation reasons.

[gabrieljones]

How hard would it be to backport to 2.10 while we're at it?

[adriaanm]

Sorry, no, we're not going to backport this to 2.10 or 2.11. (As always, it's not about the cost of doing the backport, which is quite low compared to the ongoing cost of such changes. For example: the impact on the tooling eco-system, user experience, required support,... Also, note that this has nothing to do with our commercial side, since we'd happily generate more paying support tickets there ;-))

[milessabin]

It'll be in Typelevel Scala 2.11.10.