dart-lang / dart_style

An opinionated formatter/linter for Dart code
https://pub.dev/packages/dart_style
BSD 3-Clause "New" or "Revised" License
646 stars 121 forks source link

Tweak the call chain splitting rules. #1428

Closed munificent closed 7 months ago

munificent commented 7 months ago

Some of the most complex formatting rules are around call chains. This is because argument lists themselves are fairly complex to format, and a call chain can contain many of them. Also, method and property accesses and argument lists make up the majority of Dart code, so the ways it can appear are varied and users expect nice formatting for all of the different ways chains and calls can be combined.

I've been migrating regression tests and looking at the results to see what rules we want to tweak. One thing I noticed is that I believe the current call chain rule tries too hard to block format a trailing call.

Block formatted calls are very often important in chains like:

thing.someStuff.forEach((element) {
  ...
});

But in some cases, it's better to just split the whole chain even if we could block format the last call if we really wanted to as in:

var overlapping = _directories.keys.where(
    (directory) =>
        path.isWithin(directory, rootDirectory) ||
        path.isWithin(rootDirectory, directory),
    ).toList();

That output isn't horrific, but I think it's better if we split the chain and emphasize that:

var overlapping = _directories.keys
    .where((directory) =>
        path.isWithin(directory, rootDirectory) ||
        path.isWithin(rootDirectory, directory))
    .toList();

(That's also much closer to how the short style formats this code.)

This commit tweaks the rule to determine whether a call in a chain can block format or not. The basic change is that we only allow the trailing (or next to last) call to be a block call if either:

In other words, if the leading calls have any argument lists that can split, and the last call has a regular (non-block) argument list, then we don't allow the last call to be a block call and the whole chain splits:

// Before this PR:
target.property.leading(argument1).trailing(
  argument3,
  argument4,
);

// With this PR:
target.property
    .leading(argument1)
    .trailing(argument3, argument4);