Prefer to split in || patterns in a switch expression instead of the value
#1602
Comments
munificent
added a commit
that referenced
this issue
Dec 10, 2024
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, the formatter formats the following switch expression case like so:
We already have special formatting for
||patterns in a switch expression to make them look sort of like a series of fallthrough cases in a switch statement. Given that, I think it makes sense to also tweak the splitting costs so that it prefers to split on the||instead of in the case expression. That way, you get:The text was updated successfully, but these errors were encountered: