Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format || patterns like fallthrough cases in switch expressions. #1620

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

munificent
Copy link
Member

@munificent munificent commented Dec 10, 2024

Switch statements allow multiple cases to share a body like:

switch (obj) {
  case pattern1:
  case pattern2:
    body;
}

Switch expressions don't support that, but || patterns are the idiomatic way to accomplish the same thing. Because of that, the formatter has some special formatting when the outermost pattern in a switch expression case is ||:

x = switch (obj) {
  pattern1 ||
  pattern2 =>
    body,
};

Note how the pattern2 operand isn't indented.

This PR extends that special handling to allow the => on the same line as the => even if the pattern is a split || pattern, like:

x = switch (obj) {
  pattern1 ||
  pattern2 => body,
};

And it prefers to split the || over the body when the body is block formatted:

// Prefer:
x = switch (obj) {
  pattern1 ||
  pattern2 => function(argument),
};

// Over:
x = switch (obj) {
  pattern1 || pattern2 => function(
    argument,
  ),
};

This is one of those rules that's mostly a matter of taste, but I ran this on a large corpus and most of the diffs look better to me. Here are a few examples:

// Before:
      typeName = switch (targetType) {
        DriftSqlType.int || DriftSqlType.bigInt || DriftSqlType.bool =>
          'INTEGER',
        DriftSqlType.string => 'CHAR',
        DriftSqlType.double => 'DOUBLE',
        DriftSqlType.blob => 'BINARY',
        DriftSqlType.dateTime => 'DATETIME',
        DriftSqlType.any => '',
        CustomSqlType() || DialectAwareSqlType() => targetType.sqlTypeName(
          context,
        ),
      };

// After:
      typeName = switch (targetType) {
        DriftSqlType.int ||
        DriftSqlType.bigInt ||
        DriftSqlType.bool => 'INTEGER',
        DriftSqlType.string => 'CHAR',
        DriftSqlType.double => 'DOUBLE',
        DriftSqlType.blob => 'BINARY',
        DriftSqlType.dateTime => 'DATETIME',
        DriftSqlType.any => '',
        CustomSqlType() ||
        DialectAwareSqlType() => targetType.sqlTypeName(context),
      };

// Before:
    return switch (side) {
      AxisSide.right || AxisSide.left =>
        titlesPadding.vertical + borderPadding.vertical,
      AxisSide.top || AxisSide.bottom =>
        titlesPadding.horizontal + borderPadding.horizontal,
    };

// After:
    return switch (side) {
      AxisSide.right ||
      AxisSide.left => titlesPadding.vertical + borderPadding.vertical,
      AxisSide.top ||
      AxisSide.bottom => titlesPadding.horizontal + borderPadding.horizontal,
    };

// Before:
    final defaultConstraints = switch (side) {
      ShadSheetSide.top || ShadSheetSide.bottom => BoxConstraints(
        minWidth: mSize.width,
      ),
      ShadSheetSide.left || ShadSheetSide.right => BoxConstraints(
        minHeight: mSize.height,
      ),
    };

    final defaultConstraints = switch (side) {
      ShadSheetSide.top ||
      ShadSheetSide.bottom => BoxConstraints(minWidth: mSize.width),
      ShadSheetSide.left ||
      ShadSheetSide.right => BoxConstraints(minHeight: mSize.height),
    };

Fix #1602.

Switch statements allow multiple cases to share a body like:

```dart
switch (obj) {
  case pattern1:
  case pattern2:
    body;
}
```

Switch expressions don't support that, but `||` patterns are the idiomatic way to accomplish the same thing. Because of that, the formatter has some special formatting when the outermost pattern in a switch expression case is `||`:

```dart
x = switch (obj) {
  pattern1 ||
  pattern2 =>
    body,
};
```

Note how the `pattern2` operand isn't indented.

This PR extends that special handling to allow the `=>` on the same line as the `=>` even if the pattern is a split `||` pattern, like:

```dart
x = switch (obj) {
  pattern1 ||
  pattern2 => body,
};
```

And it prefers to split the `||` over the body when the body is block formatted:

```dart
// Prefer:
x = switch (obj) {
  pattern1 ||
  pattern2 => function(argument),
};

// Over:
x = switch (obj) {
  pattern1 || pattern2 => function(
    argument,
  ),
};
```

This is one of those rules that's mostly a matter of taste, but I ran this on a large corpus and most of the diffs look better to me. Here are a few examples:

```dart
// Before:
      typeName = switch (targetType) {
        DriftSqlType.int || DriftSqlType.bigInt || DriftSqlType.bool =>
          'INTEGER',
        DriftSqlType.string => 'CHAR',
        DriftSqlType.double => 'DOUBLE',
        DriftSqlType.blob => 'BINARY',
        DriftSqlType.dateTime => 'DATETIME',
        DriftSqlType.any => '',
        CustomSqlType() || DialectAwareSqlType() => targetType.sqlTypeName(
          context,
        ),
      };

// After:
      typeName = switch (targetType) {
        DriftSqlType.int ||
        DriftSqlType.bigInt ||
        DriftSqlType.bool => 'INTEGER',
        DriftSqlType.string => 'CHAR',
        DriftSqlType.double => 'DOUBLE',
        DriftSqlType.blob => 'BINARY',
        DriftSqlType.dateTime => 'DATETIME',
        DriftSqlType.any => '',
        CustomSqlType() ||
        DialectAwareSqlType() => targetType.sqlTypeName(context),
      };

// Before:
    return switch (side) {
      AxisSide.right || AxisSide.left =>
        titlesPadding.vertical + borderPadding.vertical,
      AxisSide.top || AxisSide.bottom =>
        titlesPadding.horizontal + borderPadding.horizontal,
    };

// After:
    return switch (side) {
      AxisSide.right ||
      AxisSide.left => titlesPadding.vertical + borderPadding.vertical,
      AxisSide.top ||
      AxisSide.bottom => titlesPadding.horizontal + borderPadding.horizontal,
    };

// Before:
    final defaultConstraints = switch (side) {
      ShadSheetSide.top || ShadSheetSide.bottom => BoxConstraints(
        minWidth: mSize.width,
      ),
      ShadSheetSide.left || ShadSheetSide.right => BoxConstraints(
        minHeight: mSize.height,
      ),
    };

    final defaultConstraints = switch (side) {
      ShadSheetSide.top ||
      ShadSheetSide.bottom => BoxConstraints(minWidth: mSize.width),
      ShadSheetSide.left ||
      ShadSheetSide.right => BoxConstraints(minHeight: mSize.height),
    };
```

Fix #1602.
@munificent munificent merged commit 7c4a960 into main Dec 10, 2024
7 checks passed
@munificent munificent deleted the 1602-or-pattern-switch-expr branch December 10, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prefer to split in || patterns in a switch expression instead of the value
3 participants