Initializers should always start in the same line as the variable declaration

[rrousselGit]

Hello!
I've been playing around with the new formatting, and encountered one formatting change I think is a downgrade:

  final character = await repository.fetchCharacter(
    id,
    cancelToken: cancelToken,
  );

Now becomes:

  final character =
      await repository.fetchCharacter(id, cancelToken: cancelToken);

What I really dislike here is how the variable initializer is in the next line as the variable declaration.
The whole statement doesn't fit in a single line. So IMO we should use trailing commas here, instead of a newline.

This is consistent with how a different scenario I encountered is formatting.
Before, we had:

final character =
    FutureProvider.autoDispose.family<Character, String>((ref, id) async {
});

But with the new formatting, we now have:

final character = FutureProvider.autoDispose.family<Character, String>((
  ref,
  id,
) async {
});

IMO this is an improvement. The initializer is in the same line as the variable declaration. But it's inconsistent with the previous snippet to me.

[rrousselGit]

Another example I faced:

    final charactersResponse = await repository.fetchCharacters(
      offset: meta.page * kCharactersPageLimit,
      limit: kCharactersPageLimit,
      nameStartsWith: meta.name,
      cancelToken: cancelToken,
    );

now becomes:

      final charactersResponse =
          await repository.fetchCharacters(
            offset: meta.page * kCharactersPageLimit,
            limit: kCharactersPageLimit,
            nameStartsWith: meta.name,
            cancelToken: cancelToken,
          );

Here, I see no purpose in that newline. It just adds one extra level of indentation.

[rrousselGit]

Redirecting factory constructors seem to be facing a similar issue:

@freezed
class CharacterOffset with _$CharacterOffset {
  factory CharacterOffset({
    required int offset,
    @Default('') String name,
  }) = _CharacterOffset;
}

Becomes:

@freezed
class CharacterOffset with _$CharacterOffset {
  factory CharacterOffset({required int offset, @Default('') String name}) =
      _CharacterOffset;
}

Overall it seems to be a theme where a "statement" tries to fit the left operand of the = operator in a single line, even if it means placing the right one on a new line.

[rrousselGit]

Some more examples that I think are downgrades:

  final content = json.decode(
    await rootBundle.loadString('assets/configurations.json'),
  ) as Map<String, Object?>;
// becomes:
  final content =
     json.decode(await rootBundle.loadString('assets/configurations.json'))
         as Map<String, Object?>;

    final response = await _get(
      'characters',
      queryParameters: <String, Object?>{
        'offset': offset,
        if (limit != null) 'limit': limit,
        if (cleanNameFilter != null && cleanNameFilter.isNotEmpty)
          'nameStartsWith': cleanNameFilter,
      },
      cancelToken: cancelToken,
    );
// becomes:
    final response =
        await _get('characters', queryParameters: <String, Object?>{
          'offset': offset,
          if (limit != null) 'limit': limit,
          if (cleanNameFilter != null && cleanNameFilter.isNotEmpty)
            'nameStartsWith': cleanNameFilter,
        }, cancelToken: cancelToken);

[rrousselGit]

Is this caused by await?
I found a similar scenario, but the formatting is different.

This:

    final results =
        annotationsOf(element, throwOnUnresolved: throwOnUnresolved);

Became:

    final results = annotationsOf(
      element,
      throwOnUnresolved: throwOnUnresolved,
    );

Or this:

// Before:
        final newListeners =
            List<_Listener<T>?>.filled(_listeners.length * 2, null);
// After
        final newListeners = List<_Listener<T>?>.filled(
          _listeners.length * 2,
          null,
        );

That makes sense to me. The "old" syntax is my mistake. I forgot to add a trailing comma, and would've added it if I spotted the issue.

But that's at odds with how some earlier examples are formatted.

[munificent]

Yes, the first two examples of yours and the last one are because of await. I filed #1531 for that.

Redirecting factory constructors are an interesting one. I think people generally prefer to split between a declaration and its body versus splitting the parameter list. Like:

// Better:
function(int argument, int another) =>
    someLongBodyExpression;

// Worse:
function(
  int argument,
  int another,
) => someLongBodyExpression;

And redirecting factory constructors pretty much follow that too. We could tweak it, but I suspect it's honestly not even worth it given how rare they are.

The other remaining example is because of the as expression. We do block formatting of assignment initializers if the RHS is one of a few recognized "block formattable" types. That's mainly collection literals and function calls with non-empty argument lists. Currently, we don't treat any binary operators as block formattable even if their left operand is. We could do that, but I suspect it would some cases better but more cases worse.

For what it's worth, this doesn't look unpalatable to me:

  final content =
     json.decode(await rootBundle.loadString('assets/configurations.json'))
         as Map<String, Object?>;

It may be a downgrade when you're looking at the diff versus the old formatter. But if you had written that code from scratch and then run the new formatter on it, I'm not sure if you would have even noticed what it did at all.

But I can look at tweaking the formatter and see how it looks across a wide corpus if we allow block formatting as expressions when the LHS is block formattable and maybe some other binary operators too (is?). I suspect it will be a net loss, but it's worth trying it out on a corpus and seeing.

[rrousselGit]

Like:

// Better:
function(int argument, int another) =>
    someLongBodyExpression;

// Worse:
function(
  int argument,
  int another,
) => someLongBodyExpression;

And redirecting factory constructors pretty much follow that too. We could tweak it, but I suspect it's honestly not even worth it given how rare they are.

I never use => when the result is in a different line.
=> expression; on the same line is the only variant I'd accept in a codebase.

If anything, I'd much rather write:

function(int arg, ...) {
  return expression;
}

To me, refactoring => to {return} is very much like adding a trailing comma, but for closures.
In fact, I'd kind of like if the formatter automatically refactored to {} here.

[rrousselGit]

For what it's worth, this doesn't look unpalatable to me:

  final content =
     json.decode(await rootBundle.loadString('assets/configurations.json'))
         as Map<String, Object?>;

It may be a downgrade when you're looking at the diff versus the old formatter. But if you had written that code from scratch and then run the new formatter on it, I'm not sure if you would have even noticed what it did at all.

This is 100% something I'd spot.

My eyes really dislike when there's a newline after the =. Any formatting that does that would immediately be refactored such that it no longer happens.

For instance, I could write:

final Object? content = json.decode(...);
content as Map<...>;

// TO-DO use content

I prefer the repetition of the content identifier over splitting the declaration and initializer.
That's how much I dislike the split.