Skip to content

Commit

Permalink
Don't indent outermost "||" pattern operands in switch expression cas…
Browse files Browse the repository at this point in the history
…es. (#1203)

* Don't indent outermost "||" pattern operands in switch expression cases.

This makes them all line up as if they were empty cases in a switch
statement, which is essentially how they behave. Instead of:

```dart
e = switch (obj) {
  constant1 ||
     constant2 ||
     constant3 =>
    body
};
```

Gives:

```dart
e = switch (obj) {
  constant1 ||
  constant2 ||
  constant3 =>
    body
};
```

This addresses some of the bad formatting in #1197. The rest is #1202.

* Add comment.
  • Loading branch information
munificent authored Apr 4, 2023
1 parent b9b484c commit 33efb25
Show file tree
Hide file tree
Showing 6 changed files with 330 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Hide `--fix` and related options in `--help`. The options are still there and
supported, but are no longer shown by default. Eventually, we would like all
users to move to using `dart fix` instead of `dart format --fix`.
* Don't indent `||` pattern operands in switch expression cases.
* Don't format `sealed`, `interface`, and `final` keywords on mixin
declarations. The proposal was updated to no longer support them.

Expand Down
61 changes: 55 additions & 6 deletions lib/src/source_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2755,25 +2755,72 @@ class SourceVisitor extends ThrowingAstVisitor {

@override
void visitSwitchExpressionCase(SwitchExpressionCase node) {
// If the pattern is a series of `||` patterns, then flatten them out and
// format them like empty cases with fallthrough in a switch statement
// instead of like a single indented binary pattern. Prefer:
//
// e = switch (obj) {
// constant1 ||
// constant2 ||
// constant3 =>
// body
// };
//
// Instead of:
//
// e = switch (obj) {
// constant1 ||
// constant2 ||
// constant3 =>
// body
// };
var orBranches = <DartPattern>[];
var orTokens = <Token>[];

void flattenOr(DartPattern e) {
if (e is! LogicalOrPattern) {
orBranches.add(e);
} else {
flattenOr(e.leftOperand);
orTokens.add(e.operator);
flattenOr(e.rightOperand);
}
}

flattenOr(node.guardedPattern.pattern);

// Wrap the rule for splitting after "=>" around the pattern so that a
// split in the pattern forces the expression to move to the next line too.
builder.startLazyRule();

// Wrap the expression's nesting around the pattern too so that a split in
// Write the "||" operands up to the last one.
for (var i = 0; i < orBranches.length - 1; i++) {
// Note that orBranches will always have one more element than orTokens.
visit(orBranches[i]);
space();
token(orTokens[i]);
split();
}

// Wrap the expression's nesting around the final pattern so that a split in
// the pattern is indented farther then the body expression. Used +2 indent
// because switch expressions are block-like, similar to how we split the
// bodies of if and for elements in collections.
builder.nestExpression(indent: Indent.block);

var whenClause = node.guardedPattern.whenClause;
if (whenClause == null) {
visit(node.guardedPattern.pattern);
} else {
if (whenClause != null) {
// Wrap the when clause rule around the pattern so that if the pattern
// splits then we split before "when" too.
builder.startRule();
builder.nestExpression(indent: Indent.block);
visit(node.guardedPattern.pattern);
}

// Write the last pattern in the "||" chain. If the case pattern isn't an
// "||" pattern at all, this writes the one pattern.
visit(orBranches.last);

if (whenClause != null) {
split();
builder.startBlockArgumentNesting();
_visitWhenClause(whenClause);
Expand All @@ -2785,10 +2832,12 @@ class SourceVisitor extends ThrowingAstVisitor {
space();
token(node.arrow);
split();

builder.endRule();

builder.startBlockArgumentNesting();
visit(node.expression);
builder.endBlockArgumentNesting();

builder.unnest();
}

Expand Down
104 changes: 104 additions & 0 deletions test/regression/1100/1197.unit
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
>>>
main() {
{
return TextFieldTapRegion(
onLongPressMoveUpdate: (longPressMoveUpdateDetails) {
(switch (Theme.of(this.context).platform) {
TargetPlatform.iOS || TargetPlatform.macOS =>
_renderEditable.selectPositionAt(
from: longPressMoveUpdateDetails.globalPosition,
cause: SelectionChangedCause.longPress,
),
TargetPlatform.android ||
TargetPlatform.fuchsia ||
TargetPlatform.linux ||
TargetPlatform.windows =>
_renderEditable.selectWordsInRange(
from: longPressMoveUpdateDetails.globalPosition -
longPressMoveUpdateDetails.offsetFromOrigin,
to: longPressMoveUpdateDetails.globalPosition,
cause: SelectionChangedCause.longPress,
)
});
},
);
}
}
<<<
main() {
{
return TextFieldTapRegion(
onLongPressMoveUpdate: (longPressMoveUpdateDetails) {
(switch (Theme.of(this.context).platform) {
TargetPlatform.iOS ||
TargetPlatform.macOS =>
_renderEditable.selectPositionAt(
from: longPressMoveUpdateDetails.globalPosition,
cause: SelectionChangedCause.longPress,
),
TargetPlatform.android ||
TargetPlatform.fuchsia ||
TargetPlatform.linux ||
TargetPlatform.windows =>
_renderEditable.selectWordsInRange(
from: longPressMoveUpdateDetails.globalPosition -
longPressMoveUpdateDetails.offsetFromOrigin,
to: longPressMoveUpdateDetails.globalPosition,
cause: SelectionChangedCause.longPress,
)
});
},
);
}
}
>>>
main() {
{
return TextFieldTapRegion(
onLongPressMoveUpdate: (longPressMoveUpdateDetails) =>
switch (Theme.of(this.context).platform) {
TargetPlatform.iOS || TargetPlatform.macOS =>
_renderEditable.selectPositionAt(
from: longPressMoveUpdateDetails.globalPosition,
cause: SelectionChangedCause.longPress,
),
TargetPlatform.android ||
TargetPlatform.fuchsia ||
TargetPlatform.linux ||
TargetPlatform.windows =>
_renderEditable.selectWordsInRange(
from: longPressMoveUpdateDetails.globalPosition -
longPressMoveUpdateDetails.offsetFromOrigin,
to: longPressMoveUpdateDetails.globalPosition,
cause: SelectionChangedCause.longPress,
)
},
);
}
}
<<<
main() {
{
return TextFieldTapRegion(
onLongPressMoveUpdate: (longPressMoveUpdateDetails) =>
switch (Theme.of(this.context).platform) {
TargetPlatform.iOS ||
TargetPlatform.macOS =>
_renderEditable.selectPositionAt(
from: longPressMoveUpdateDetails.globalPosition,
cause: SelectionChangedCause.longPress,
),
TargetPlatform.android ||
TargetPlatform.fuchsia ||
TargetPlatform.linux ||
TargetPlatform.windows =>
_renderEditable.selectWordsInRange(
from: longPressMoveUpdateDetails.globalPosition -
longPressMoveUpdateDetails.offsetFromOrigin,
to: longPressMoveUpdateDetails.globalPosition,
cause: SelectionChangedCause.longPress,
)
},
);
}
}
30 changes: 29 additions & 1 deletion test/splitting/arrows.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,32 @@ doSomethingWithFn((argument) =>
{
() => one;
() => two;
}
}
>>> indent entire block body
SomeLongFunctionName(
(longParameterName______) =>
<LongTypeArgument>[
longListElement,
],
);
<<<
SomeLongFunctionName(
(longParameterName______) =>
<LongTypeArgument>[
longListElement,
],
);
>>>
SomeLongFunctionName(
(longParameterName______) =>
switch (value) {
constant => body,
},
);
<<<
SomeLongFunctionName(
(longParameterName______) =>
switch (value) {
constant => body,
},
);
11 changes: 11 additions & 0 deletions test/splitting/switch.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,15 @@ switch (obj) {
element,
]:
body;
}
>>> indent || patterns when outermost in switch statement (as opposed to expr)
switch (obj) {
case oneConstant || twoConstant || threeConstant: body;
}
<<<
switch (obj) {
case oneConstant ||
twoConstant ||
threeConstant:
body;
}
Loading

0 comments on commit 33efb25

Please sign in to comment.