Skip to content

Conversation

munificent
Copy link
Member

@munificent munificent commented Apr 1, 2025

Consider:

function(name: (param) => another(argument1, argument2, argument3));

Here we have a ListPiece for the another(...) call wrapped in an AssignPiece for the => wrapped in an AssignPiece for the name:. When the inner ListPiece splits, it will be block-shaped. That in turn allows the surrounding AssignPiece to block format, giving:

(param) => another(
  argument1,
  argument2,
  argument3,
)

(If the ListPiece wasn't block-shaped, then the formatter would split after =>.)

An interesting question is then "what is the shape of that AssignPiece?" Prior to this PR, the answer was always "other". Regardless of the shape of the AssignPiece's RHS, a split AssignPiece always had shape other. That meant that the entire original example would end up like:

function(
  name:
      (param) => another(
        argument1,
        argument2,
        argument3,
      ),
);

Because the (param) => ... piece had shape "other", the AssignPiece for the named argument couldn't block format it, so it would split after the :.

That's not ideal. In practice with = and espectially with named arguments, users really want code on the same line as the argument name (and less indented) if at all possible.

This PR propagates the shape of the AssignPiece's contents out to its surrounding context. This means that a headline-shaped RHS causes the assignment to be headline-shaped, and likewise for block-shaped. That lets the above example be formatted as:

function(
  name: (param) => another(
    argument1,
    argument2,
    argument3,
  ),
);

Since =, :, and => all use AssignPiece, this theoretically means that arbitrarily complex chains of those propagate the shape out. For example, chained assignments like this now work:

outer = inner = [
  element,
  element,
];

In practice, 99% of the time this comes into play, it's a named argument whose expression is a => function. But those are very common for callbacks in Flutter code, so this makes a big difference.

Fix #1536.
Fix #1545.
Fix #1668.
Fix #1679.

Consider:

```dart
function(name: (param) => another(argument1, argument2, argument3));
```

Here we have a ListPiece for the `another(...)` call wrapped in an AssignPiece for the `=>` wrapped in an AssignPiece for the `name:`. When the inner ListPiece splits, it will be block-shaped. That in turn allows the surrounding AssignPiece to block format, giving:

```dart
(param) => another(
  argument1,
  argument2,
  argument3,
)
```

(If the ListPiece wasn't block-shaped, then the formatter would split after `=>`.)

An interesting question is then "what is the shape of that AssignPiece?" Prior to this PR, the answer was always "other". Regardless of the shape of the AssignPiece's RHS, a split AssignPiece always had shape other. That meant that the entire original example would end up like:

```dart
function(
  name:
      (param) => another(
        argument1,
        argument2,
        argument3,
      ),
);
```

Because the `(param) => ...` piece had shape "other", the AssignPiece for the named argument couldn't block format it, so it would split after the `:`.

That's not ideal. In practice with `=` and espectially with named arguments, users really want code on the same line as the argument name (and less indented) if at all possible.

This PR propagates the shape of the AssignPiece's contents out to its surrounding context. This means that a headline-shaped RHS causes the assignment to be headline-shaped, and likewise for block-shaped. That lets the above example be formatted as:

```dart
function(
  name: (param) => another(
    argument1,
    argument2,
    argument3,
  ),
);
```

Since `=`, `:`, and `=>` all use AssignPiece, this theoretically means that arbitrarily complex chains of those propagate the shape out. For example, chained assignments like this now work:

```dart
outer = inner = [
  element,
  element,
];
```

In practice, 99% of the time this comes into play, it's a named argument whose expression is a `=>` function. But those are very common for callbacks in Flutter code, so this makes a big difference.

Fix #1536.
Fix #1545.
Fix #1668.
Fix #1679.
@munificent munificent requested review from natebosch and kallentu April 1, 2025 22:54
Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good. All the sample changes look good except for one edge case with assignment to a field.

I think on the net this is still an improvement if we have to accept the split before the field so we can land it without a fix for that.

@munificent munificent merged commit 71421b3 into main Apr 2, 2025
5 checks passed
@munificent munificent deleted the propagate-assign-shape branch April 2, 2025 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants