FR: --fix-trailing-commas to always add trailing commas

[srawlins]

Motivation

Large function calls (e.g. constructors), such as the large annotation calls in Angular, can be formatted 3 (three) (un, deux, trois) ways, based on the commas:

@Component(selector: 'button', templateUrl: 'button.html', directives: [
  ItemOne,
  NoTrailingComma
], providers: [
  providerOne,
  Provider(FooBuilder, useFactory: provideFooBuilder)
], exports: [
  ExportOne,
  ExportTwo
])
int a;

@Component(
  selector: 'button',
  directives: [
    ItemOne,
    TrailingComma,
  ],
  providers: [
    providerOne,
    Provider(FooBuilder, useFactory: provideFooBuilder),
  ],
  exports: [
    ExportOne,
    ExportTwo,
  ],
  templateUrl: 'button.html',
)
int b;

@Component(
    selector: 'button',
    directives: [
      ItemOne,
      TrailingComma,
    ],
    providers: [
      providerOne,
      Provider(FooBuilder, useFactory: provideFooBuilder),
    ],
    exports: [
      ExportOne,
      ExportTwo,
    ],
    templateUrl: 'button.html')
int c;

The Component constructor calls are semantically exactly the same. The CST differences:

1. In the first case, none of the inner list literals have trailing commas. The result is ] positioned at column 0.
2. In the second case, the inner lists have trailing commas, and the call to Component has a trailing comma. Also templateUrl, which is not given a List literal, has been listed out-of-alphabetical-order, as the second argument. The result is ] positioned at column 2, and each argument starting on its own line.
3. In the third case, the inner lists have trailing commas, but the call to Component has not. The result is ] positioned at column 4, with each argument starting on its own line.

OK, I get it, the trailing comma situation results in different code. I don't want to address that here.

The ask

What I want is a --fix feature, like --fix-trailing-commas, which adds trailing commas to list literals and function calls which are already being split into multiple lines. For example I would not want a trailing comma added to print('hi'), or to the single-lined Provider() above.

The goal is to unify all long method calls and list literals to the second case above, which I think today is most common anyway, and is the most visually appealing.

And actually, in my example I've severely trimmed the list of directives and such. The code I took this from had a dozen or so directives, so it was always going to be multiple lines.

(Also Map literals and Set literals I imagine...)

[natebosch]

What about method signatures at the definition point? They also allow trailing commas that impact formatting? Further complicated by optional positional or named arguments...

I agree that making the edits manually and keeping things consistent within a project is tough now and could be automated. What I worry about is whether this is opening dartfmt up for too much configurability and style arguments - there are at least 3 situations where this could be applied - signatures, collection literals, function calls - who decides which of the 3 get it?

[srawlins]

Rather than decide on which ones, I'd be happy if --fix-trailing-commas added trailing commas everywhere they are allowed (in line-wrapped contexts).

[Empty2k12]

Rather than decide on which ones, I'd be happy if --fix-trailing-commas added trailing commas everywhere they are allowed (in line-wrapped contexts).

That would be the most sensible option. The (in line-wrapped contexts) is very important.

What's the status on this issue?

[srawlins]

I think the next step is for @munificent needs to triage. WDYT, @munificent?

[munificent]

Anything related to trailing commas and the resultant formatting differences is a huge rathole that I don't have time to crawl into right now. Sorry. :(

[srawlins]

Hmm, I don't think the resultant formatting differences is related to this issue. I just thought you applied labels to issues as they came in, in a short triage process. Not a P1 from my perspective though.

[munificent]

OK, labeled. :)

[srawlins]

😛 I'll take that as a "This seems like an acceptable idea. I'm open to pull requests 😁 "

[munificent]

I'll take that as "I won't be offended if the PR languishes indefinitely." :)

[srawlins]

Well played, sir!

[ciriousjoker]

Will this be implemented anytime soon? Formatting in Flutter looks way more consistent if I add , after long lists.

[munificent]

It's unlikely to rise to the top of the priority list any time soon, unfortunately.

[Jordan-Nelson]

Has the priority of this changed at all? The flutter docs recommend using trailing commas (see below). The docs say to use them, "at the end of a parameter list in functions, methods, and constructors where you care about keeping the formatting you crafted." The last part is subjective (and manual) and this leads to pretty significant inconsistencies in formatting throughout most flutter projects.

Here is the full quote from the docs:

Flutter code often involves building fairly deep tree-shaped data structures, for example in a build method. To get good automatic formatting, we recommend you adopt the optional trailing commas. The guideline for adding a trailing comma is simple: Always add a trailing comma at the end of a parameter list in functions, methods, and constructors where you care about keeping the formatting you crafted. This helps the automatic formatter to insert an appropriate amount of line breaks for Flutter-style code.

I think it is worth noting that other languages/formatters either support this as an option, or at least have proposals to support it. Some even make it the default. Here are some examples:

• Prettier changed the default preferences to include trailing commas for JS/TS about a year ago, and had supported it as an option for long before that. See Enforce Trailing Commas by Default prettier/prettier#68 (comment).
• Kotlin supports this as an option and it can be enabled in Android Studio / IntelliJ to be the default. See https://kotlinlang.org/docs/coding-conventions.html#trailing-commas.
• Swift does not (as far as I know) have this option, but there is a proposal for it from a core contributor to add this option. It admittedly doesn't seem to have a lot of recent activity despite having a fair amount of support. See https://forums.swift.org/t/trailing-commas-in-all-expression-lists/19527.

I think over the past few years this has started to become the norm as there are many compelling reasons to enforce trailing commas (formatting consistency, developer ergonomics, more meaningful diffs).

[munificent]

Has the priority of this changed at all?

Not yet, but I wouldn't be surprised if it did. I'm stretched pretty thin right now, but I would like to put some time into dartfmt if I can find it.

[carlosfiori]

this feature would be great

[philiparpin]

Any updates here? I want this 😛

[gmcdowell]

adding to the request impetus.. please.. pretty please!!

[robert-j-webb]

I'd be willing to write a PR for this - is there any interest from the dev team about that?

[munificent]

I definitely appreciate the offer, but I think any work here should be coordinated with a more holistic revisiting of how argument lists are formatted. Right now, the formatter does not assume most users want Flutter's style of argument formatting, so it would be very inconsistent if one of the --fix flags did make that assumption.

Before dartfmt starts adding and/or removing trailing commas on your behalf, I'd like to reach consensus on what the preferred style around argument lists and trailing commas should be. That's a lot more social-level work than just a PR. The actual coding is the fun part.

[Cajuteq]

Hi, where can we find this social-level work ? it seems to be a rather consensual topic among other languages and i'm interested in this formatting for me and my team, so it would be cool to at least read want the defenders of "no-trailing comma" think on it. Thanks

[munificent]

Hi, where can we find this social-level work ?

It hasn't started. It's going to be a time-consuming process and we haven't carved out the time for it yet.

[dietpizza]

Any updates?

[munificent]

No update yet, but I'm hoping to be able to put some time into it soon.

[srawlins]

Proposal at #1253

[munificent]

I'm going to close this out now since it's superseded by #1253. :)