consider extending require_trailing_commas to include record expressions

[pq]

For example, should the following get flagged by require_trailing_commas?

var r = (x: 1, y: 2, z: 3);

/fyi @lauweijie

[bwilkerson]

@munificent Do you expect that the formatter will treat trailing commas in records and record types similar to the way they're treated in other lists (that is, forcing each item to appear on a separate line)? Will that include single field records where the trailing comma is required?

[pq]

@munificent: any thoughts here on how the formatter will treat records? (Like lists?)

(Formatter tracking issue: dart-lang/dart_style#1128.)

[munificent]

Strangely, I haven't thought much about how records will be formatted. Yes, it will probably be like list literals. I think it's reasonable if this lint applies to record expressions (and record type annotations?) too.

[srawlins]

To be precise and not cross wires, require_trailing_commas currently does not flag list literals. See #3607. It flags things like argument lists. But think we all have the right understanding. require_trailing_commas should flag list literals, and it should treat record literals in the same way.

[srawlins]

It also currently flags function declarations, which are similar to record types.

[bwilkerson]

Note that the lint should not flag a record literal with a single element that has no trailing comma (even if it splits across lines) because that's already a compile time error and we don't want to double report the issue.

[srawlins]

Maybe can be done in same issue, maybe not, but just commenting: should also be added in RecordPatterns, ListPatterns, MapPatterns, and ObjectPatterns (similar to InstanceCreationExpressions).

[munificent]

Circling back...

Do you expect that the formatter will treat trailing commas in records and record types similar to the way they're treated in other lists (that is, forcing each item to appear on a separate line)?

Yes, that's how the formatter formats them now, with the exception of records with only a single positional field. Those must have a trailing comma, but the trailing comma doesn't cause a split. So the formatter produces:

var one = (1,);
var two = (
  1,
  2,
);
var oneNamed = (
  name: 1,
);

Note that the lint should not flag a record literal with a single element that has no trailing comma (even if it splits across lines) because that's already a compile time error

By "compile time error" do you mean "perfectly valid parenthesized expression"?

Maybe can be done in same issue, maybe not, but just commenting: should also be added in RecordPatterns, ListPatterns, MapPatterns, and ObjectPatterns (similar to InstanceCreationExpressions).

If you really want to be exhaustive (because I have just run through this in my experimental Flutter-style branch of the formatter), the other one is assert() statements.

It is a curious irregularity of the language that type parameter and type argument lists completely forbid trailing commas.

[bwilkerson]

Note that the lint should not flag a record literal with a single element that has no trailing comma (even if it splits across lines) because that's already a compile time error

By "compile time error" do you mean "perfectly valid parenthesized expression"?

Nope. It appears that I misspoke. I guess I should have said "warning". The analyzer currently produces a warning in this code:

void f((int,) i) {
  f((1)); // A record literal with exactly one positional field requires a trailing comma.
}

I'm not sure why it's a warning. I thought it was a replacement for the compiler error "A value of type 'int' can't be assigned to a variable of type '(int)'.", where the replacement was done because the lexical difference between the types can be hard to see. That might be a bug.

[munificent]

I'm not sure why it's a warning.

Yeah, that definitely seems wrong. There should be a compile error in that program.