language icon indicating copy to clipboard operation
language copied to clipboard

Allow trailing commas in constructor initializer lists and generic type lists

Open alexeyinkin opened this issue 3 years ago • 5 comments

Trailing comma is supported in many places like argument lists, list entries, etc. However there are still places where it is not supported by would be beneficial:

  1. Constructor initializer:
  PlatformInstance()
      :
        _environment = Platform.environment,
        _executable = Platform.executable,
        _executableArguments = Platform.executableArguments,
        _localeName = Platform.localeName,
        _localHostname = Platform.localHostname,
        _numberOfProcessors = Platform.numberOfProcessors,
        _operatingSystem = Platform.operatingSystem,
        _operatingSystemVersion = Platform.operatingSystemVersion,
        _packageConfig = Platform.packageConfig,
        _pathSeparator = Platform.pathSeparator,
        _resolvedExecutable = Platform.resolvedExecutable,
        _script = Platform.script,
        _version = Platform.version,                               // <========
  ;
  1. Generic type lists:
class GenericClass<
  A,
  B extends SomeClass<A>,
  C extends AnotherClass<A, B>,  // <=======
> {
}

Proposal: to allow trailing comma in these cases to minimize changes when adding or moving new lines there.

alexeyinkin avatar Jan 15 '21 09:01 alexeyinkin

I've never liked trailing commas anywhere, but I have to agree that if we're going to allow them some places, we should allow them everywhere.

munificent avatar Jun 15 '21 20:06 munificent

One more usage is a list of implemented interfaces:

class A implements
  B,
  C,
{
}

alexeyinkin avatar Aug 25 '21 02:08 alexeyinkin

We can allow it almost everywhere we have a comma separated list. It requires that what comes after is clearly not part of the list. That's generally true, but I think I'd also prefer to not need to look too far ahead.

Take import "a" show show, hide, show show, hide;. Since show and hide are valid identifiers, this is technically valid with trailing commas and needs to parse. The comma after the first hide is a trailing comma. We can't see that on the following show, nor on the next following show, we need to find the comma after that in order to recognize that the latter of those shows is an identifier, not a keyword. So, I'd probably not allow trailing commas there.

Interfaces in class or mixin declarations, they are currently always followed by a {. The on-types of mixin declarations, and with types of Mixon applications, t are followed by { or implements (which is a reserved word), or ; in mixin applications class declarations. Initializer lists are followed by ; or {. Type lists are followed by >.

All of those are safe to allow trailing commas for. It's really just show and hide clauses which won't work as easily. We could fix that by only allowing you to have one.

lrhn avatar Sep 06 '21 09:09 lrhn

@munificent, will list structured terms like <initializers> and <typeParameters> be treated in the same way as other lists under the upcoming formatting rules?

Seems reasonable to treat <initializers> in the same way as the <enumEntry> list in an <enumType> (as far as possible), and <typeParameters> and <typeArguments> in the same way as <arguments>.

eernstg avatar Sep 08 '23 09:09 eernstg

@munificent, will list structured terms like <initializers> and <typeParameters> be treated in the same way as other lists under the upcoming formatting rules?

They aren't treated entirely uniformly because the language doesn't let us. As far as line splitting goes, most comma-separated things are handled the same way. But the upcoming formatter has some special code for each construct to handle whether or not it allows a trailing comma.

So, for example, while a type argument list splits similarly to an argument list, the formatter has some code to know to omit a trailing comma on the former while adding one to the latter.

Enum values are even more special because it looks silly to have a trailing comma immediately followed by a ; if there are members. So it adds a trailing comma if the enum value list splits and there are no members, but removes the trailing comma if there are members.

Seems reasonable to treat <initializers> in the same way as the <enumEntry> list in an <enumType> (as far as possible), and <typeParameters> and <typeArguments> in the same way as <arguments>.

+1 as far as formatting goes. But I do worry somewhat that allowing trailing commas after undelimited constructs would make it harder to make semicolons optional in the future.

munificent avatar Jan 09 '24 22:01 munificent