Skip to content

SIP-27 Trailing Commas #533

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

Merged
merged 9 commits into from
Sep 23, 2016
Merged
146 changes: 146 additions & 0 deletions sips/pending/_posts/2016-06-25-trailing-commas.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
---
layout: sip
disqus: true
title: SIP-27 - Trailing Commas
---

# SIP-27: Trailing Commas

**By: Dale Wijnand**

## History

| Date | Version |
| ---------------|----------------------------------------------|
| Jun 25th 2016 | Initial Draft |
| Jun 27th 2016 | Added drawback of changing existing tools |
| Jun 27th 2016 | Added motivation that it simplifies codegen |
| Jun 28th 2016 | Fixed a typo |
| Aug 10th 2016 | Renamed from SIP-NN to SIP-27 |
| Aug 10th 2016 | Changed scala-commas URL (repo was moved) |
| Aug 10th 2016 | Dialed back some of the language from review |
| Sep 04th 2016 | Split the motivation into sections |
| Sep 04th 2016 | Add VCS authorship attribution to motivation |
| Sep 04th 2016 | Add Cross building hinderance to drawbacks |

## Motivation

### Easy to modify lists

When using a comma-separated sequence of elements on multiple lines, such as:

{% highlight scala %}
Seq(
foo,
bar,
baz
)
{% endhighlight %}

It is quite inconvenient to remove or comment out any element because one has to think about the fact that the last element mustn't have a trailing comma:

{% highlight scala %}
Map(
foo,
bar //,
// baz
)
{% endhighlight %}

Secondly, it is quite inconvenient to re-order the sequence, for instance if you wanted `baz` before `bar` you need to micromanage which is followed by a comma and which isn't:

{% highlight scala %}
val xs = Seq(
foo,
baz // This isn't going to work
bar,
)
{% endhighlight %}

### Reduce diff noise

Allowing trailing commas also reduces a lot of noise in diffs, such as:

{% highlight diff %}
@@ -4,7 +4,8 @@
Map(
foo,
bar,
- baz
+ baz,
+ quux
)
{% endhighlight %}

### VCS authorship attribution

Using the example above, the authorship of the `baz` line would be preserved, instead of becoming that of the author of the `quux` line.

### Simplify code generation

Such a feature would also simplify generating Scala source code.

### Long standing ticket

There is an open ticket ([SI-4986][]) where this feature was requested, referencing the fact that it facilitates code generation by tools and allows for easier sorting of the values, initially in the context of import selectors but later also for other constructs in the syntax.

### Real-world use-cases

Some real-world use-cases where elements of a sequence are typically added, removed or moved are:

* invoking constructors or methods (such as `apply` or `copy`) which present a lot of options defined with default values
* `settings(...)` arguments or elements of `libraryDependencies`, `scalacOptions` or `javaOptions` sequences in sbt

## Design Decisions

There are a number of different elements of the Scala syntax that are comma separated, but instead of changing them all a subset of the more useful ones was chosen:

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

From the spec these are:

* SimpleExpr1, ArgumentExprs via Exprs
* ParamClause, ParamClauses via Params
* ClassParamClause, ClassParamClauses via ClassParams
* ImportSelector

The elements that have not changed are:

* ValDcl, VarDcl, VarDef via ids
* Type via FunctionArgTypes
* SimpleType, TypeArgs via Types
* Expr, ResultExpr via Bindings
* SimplePattern via Patterns
* TypeParamClause, FunTypeParamClause
* ImportExp
* PatDef

## Implementation

The implementation is a simple change to the parser, allowing for a trailing comma, for the groups detailed above, and has been proposed in [scala/scala#5245][].

## Drawbacks/Trade-offs

The drawback, or trade-off, to this change is that it adds another way in which it is possible to do something in Scala. But it is the opinion of this SIP that the pragmatic advantage of being able to have trailing commas is worth this drawback.

Given that this is a change in syntax, another drawback is that it requires changing the existing tools, such as those that parse Scala: intellij-scala, scalariform, scala.meta and scalaparse.

Another drawback is that for projects that cross build to previous versions of Scala they would have to take into account that this feature wouldn't be available for those versions (assuming this feature isn't backported).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this any worse than for any other change to Scala? Even simple additions to the standard library are subject to this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed

I thought it's fair to mention it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is noise. I was trying to figure out what made this SIP special about this. IMO this paragraph should be removed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It leads to source incompatibility at the language level. It implies a major version bump, not only a minor one (as far as I understand Scala policy regarding that, for ex. https://twitter.com/viktorklang/status/735772498780819457)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not forward source compatible (most things aren't), but it is backwards source compatible. So an existing codebase will always compile with a new compiler. This only requires a minor version bump.

FTR, adding a method to the standard library is less compatible than that. It can always break backward source compatibility of some codebases, if they pimped a method of the same name via implicits: the new method would suddenly take over instead of the implicit pimp. If the semantics are different, this breaks code.

This change is much more compatible than adding a method, as it can never break existing codebases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjrd I specifically mentioned Cross Build Hinderance (#533 (comment)) as a drawback.

It's a drawback worth considering for this SIP in particular because the added benefits are not as feature-based, and closer to the pro/con 50:50 point than, let's say SIP-14 Futures and Promises. Sure adding Future to the standard library would prevent cross building, but it does more things.

Another reason to think about this specifically for this SIP is that the surface area that might affect this SIP is potentially all Scala sources, which I don't think can be said of just adding a method to the standard library. Closer analogy might be SIP-18 Modularizing Language Features which might have added import language.implicitConversions etc many existing source. The potential surface area becomes relevant especially given the rise of auto formatting of Scala source using Scalariform or Scalafmt.


## Alternatives

As an alternative, trailing commas support could be added universally to all the comma-separated elements of the syntax. This would mean changing more (but still only in the parser), but it would make it consistent.

As an alternative to changing the language, there already exists today a compiler plugin called [scala-commas][] that provides this feature. It also provides some evidence that people would even use unsupported compiler apis and reflection to add this functionality, even when such a plugin won't compose with other plugins well, though arguably only weak evidence as it's a young and obscure plugin.

## References

1. [SI-4986][]
2. [scala/scala#5245][]
3. [scala-commas][]

[SI-4986]: https://issues.scala-lang.org/browse/SI-4986
[scala/scala#5245]: https://github.com/scala/scala/pull/524://github.com/scala/scala/pull/5245
[scala-commas]: https://github.com/47deg/scala-commas