[Formatting issue] Missing trailing commas for positional parameters after a multiline collection

[rrousselGit]

What input code did you provide to the formatter?

              builder.writeImportedName(
                [Uri.parse('package:riverpod_annotation/riverpod_annotation.dart')],
                'riverpod',
              );

What output did the formatter produce?

              builder.writeImportedName([
                Uri.parse(
                  'package:riverpod_annotation/riverpod_annotation.dart',
                ),
              ], 'riverpod');

What output did you expect or want the formatter to produce?

Describe how you expected the formatter to format the code, ideally by showing what you had in mind as code.

              builder.writeImportedName(
                [
                  Uri.parse(
                    'package:riverpod_annotation/riverpod_annotation.dart',
                  ),
                ],
                'riverpod',
              );

Anything else we should know?

Thanks!

[munificent]

The formatter has an edge case in the argument list handling rules that if an argument list contains one block-formattable thing (collection literal, function literal, etc.), then it doesn't force the argument list to split:

function([
  stuff,
  more,
]);

That edge case rule has a corner case where it will allow that formatting even if there is one trailing argument:

function([
  stuff,
  more,
], arg);

The corner case is there mainly for calls to test() where it's pretty common to add a trailing argument and we really don't want to force the entire test body to be indented:

test('description', () {
  // Many lines...
}, skip: true);

I agree it doesn't look great in the example you have. I wonder if it's worth restricting the corner case rule to only apply when the block argument is a function literal, and not a collection literal.

[rrousselGit]

The corner case is there mainly for calls to test() where it's pretty common to add a trailing argument and we really don't want to force the entire test body to be indented:

test('description', () {
// Many lines...
}, skip: true);

I don't think that's a desirable target when considering modern Dart.

IMO the following is strictly superior:

test('description', skip: true, () {

});

And based on the description length can also format as:

test(
  'long description',
  skip: true,
  () {

]);

In all variants, skip: true is way more visible.

[munificent]

I don't think that's a desirable target when considering modern Dart.

Alas, the formatter is also still sometimes run on Dart code that hasn't changed in quite some time. I don't want to drastically regress the style and cause a huge diff from an indentation change just because an author still has skip: true after the test function expression.