Skip to content

Commit

Permalink
Format switch statements and expressions. (#1294)
Browse files Browse the repository at this point in the history
Format switch statements and expressions.

Switch statements build on the existing SequencePiece but need some
additional flexibility in there to handle the fact that switch case
lines are indented +2 and then case body lines are indented +4.

Switch expressions build on the existing ListPiece since the body is
sort of like a collection literal. There are a few tweaks needed here
too since an unsplit switch expression body gets spaces inside the
curly brackets.

Also, switch values (the part in the leading "( ... )") are formatted
using ListPiece as well, since they are formatted essentially like a
single-element argument list, except that they don't get a trailing
comma.

This does not implement all of the kinds of patterns that can appear in
cases. It just implements basic constants needed for the tests. It also
doesn't support guard clauses yet. Those will come later.

Co-authored-by: Nate Bosch <nbosch@google.com>
  • Loading branch information
munificent and natebosch authored Oct 25, 2023
1 parent 874d511 commit 2cee560
Show file tree
Hide file tree
Showing 12 changed files with 976 additions and 121 deletions.
2 changes: 1 addition & 1 deletion lib/src/ast_extensions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ extension ExpressionExtensions on Expression {
ListLiteral() => true,
MethodInvocation() => true,
SetOrMapLiteral() => true,
SwitchExpression() => true,
// TODO(tall): Record literals.
// TODO(tall): Instance creation expressions (`new` and `const`).
// TODO(tall): Switch expressions.
_ => false,
};

Expand Down
104 changes: 89 additions & 15 deletions lib/src/front_end/ast_node_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/source/line_info.dart';

import '../constants.dart';
import '../dart_formatter.dart';
import '../piece/block.dart';
import '../piece/do_while.dart';
import '../piece/if.dart';
import '../piece/infix.dart';
Expand Down Expand Up @@ -54,28 +56,28 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>

if (node is CompilationUnit) {
if (node.scriptTag case var scriptTag?) {
sequence.add(scriptTag);
sequence.visit(scriptTag);
sequence.addBlank();
}

// Put a blank line between the library tag and the other directives.
Iterable<Directive> directives = node.directives;
if (directives.isNotEmpty && directives.first is LibraryDirective) {
sequence.add(directives.first);
sequence.visit(directives.first);
sequence.addBlank();
directives = directives.skip(1);
}

for (var directive in directives) {
sequence.add(directive);
sequence.visit(directive);
}

for (var declaration in node.declarations) {
sequence.add(declaration);
sequence.visit(declaration);
}
} else {
// Just formatting a single statement.
sequence.add(node);
sequence.visit(node);
}

// Write any comments at the end of the code.
Expand Down Expand Up @@ -260,7 +262,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>

@override
void visitConstantPattern(ConstantPattern node) {
throw UnimplementedError();
if (node.constKeyword != null) throw UnimplementedError();
visit(node.expression);
}

@override
Expand Down Expand Up @@ -574,10 +577,10 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
void visitLabeledStatement(LabeledStatement node) {
var sequence = SequenceBuilder(this);
for (var label in node.labels) {
sequence.add(label);
sequence.visit(label);
}

sequence.add(node.statement);
sequence.visit(node.statement);
writer.push(sequence.build());
}

Expand Down Expand Up @@ -643,8 +646,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
visit(node.methodName);
visit(node.typeArguments);

createDelimited(node.argumentList.leftParenthesis,
node.argumentList.arguments, node.argumentList.rightParenthesis);
createList(node.argumentList.leftParenthesis, node.argumentList.arguments,
node.argumentList.rightParenthesis);
}

@override
Expand Down Expand Up @@ -927,17 +930,88 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>

@override
void visitSwitchExpression(SwitchExpression node) {
throw UnimplementedError();
var list = DelimitedListBuilder.switchBody(this);

createSwitchValue(node.switchKeyword, node.leftParenthesis, node.expression,
node.rightParenthesis);
writer.space();
list.leftBracket(node.leftBracket);

for (var member in node.cases) {
list.add(member);
}

list.rightBracket(node.rightBracket);
writer.push(list.build());
}

@override
void visitSwitchExpressionCase(SwitchExpressionCase node) {
throw UnimplementedError();
if (node.guardedPattern.whenClause != null) throw UnimplementedError();

visit(node.guardedPattern.pattern);
writer.space();
finishAssignment(node.arrow, node.expression);
}

@override
void visitSwitchStatement(SwitchStatement node) {
throw UnimplementedError();
createSwitchValue(node.switchKeyword, node.leftParenthesis, node.expression,
node.rightParenthesis);

// Attach the ` {` after the `)` in the [ListPiece] created by
// [createSwitchValue()].
writer.space();
token(node.leftBracket);
writer.split();
var switchPiece = writer.pop();

var sequence = SequenceBuilder(this);
for (var member in node.members) {
for (var label in member.labels) {
sequence.visit(label);
}

sequence.addCommentsBefore(member.keyword);
token(member.keyword);

if (member is SwitchCase) {
writer.space();
visit(member.expression);
} else if (member is SwitchPatternCase) {
writer.space();

if (member.guardedPattern.whenClause != null) {
throw UnimplementedError();
}

visit(member.guardedPattern.pattern);
} else {
assert(member is SwitchDefault);
// Nothing to do.
}

token(member.colon);
var casePiece = writer.pop();
writer.split();

// Don't allow any blank lines between the `case` line and the first
// statement in the case (or the next case if this case has no body).
sequence.add(casePiece, indent: Indent.none, allowBlankAfter: false);

for (var statement in member.statements) {
sequence.visit(statement, indent: Indent.block);
}
}

// Place any comments before the "}" inside the sequence.
sequence.addCommentsBefore(node.rightBracket);

token(node.rightBracket);
var rightBracketPiece = writer.pop();

writer.push(BlockPiece(switchPiece, sequence.build(), rightBracketPiece,
alwaysSplit: node.members.isNotEmpty));
}

@override
Expand Down Expand Up @@ -974,7 +1048,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>

@override
void visitTypeArgumentList(TypeArgumentList node) {
createDelimited(node.leftBracket, node.arguments, node.rightBracket);
createTypeList(node.leftBracket, node.arguments, node.rightBracket);
}

@override
Expand All @@ -989,7 +1063,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>

@override
void visitTypeParameterList(TypeParameterList node) {
createDelimited(node.leftBracket, node.typeParameters, node.rightBracket);
createTypeList(node.leftBracket, node.typeParameters, node.rightBracket);
}

@override
Expand Down
78 changes: 66 additions & 12 deletions lib/src/front_end/delimited_list_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,81 @@ class DelimitedListBuilder {

late final Piece _rightBracket;

/// Whether this list is a list of type arguments or type parameters, versus
/// any other kind of list.
/// Whether this list should have a trailing comma if it splits.
///
/// Type arguments/parameters are different because:
/// This is true for most lists but false for type parameters, type arguments,
/// and switch values.
final bool _trailingComma;

/// The cost of splitting this list. Normally 1, but higher for some lists
/// that look worse when split.
final int _splitCost;

/// Whether this list should have spaces inside the bracket when it doesn't
/// split.
///
/// This is false for most lists, but true for switch expression bodies:
///
/// * The language doesn't allow a trailing comma in them.
/// * Splitting in them looks aesthetically worse, so we increase the cost
/// of doing so.
bool _isTypeList = false;
/// ```
/// v = switch (e) { 1 => 'one', 2 => 'two' };
/// // ^ ^
/// ```
final bool _spaceWhenUnsplit;

DelimitedListBuilder(this._visitor);
/// Whether a split in the [_before] piece should force the list to split too.
///
/// See [ListPiece._splitListIfBeforeSplits] for more details.
final bool _splitListIfBeforeSplits;

/// The list of comments following the most recently written element before
/// any comma following the element.
CommentSequence _commentsBeforeComma = CommentSequence.empty;

/// Creates a new [DelimitedListBuilder] for an argument list, collection
/// literal, etc.
DelimitedListBuilder(this._visitor)
: _trailingComma = true,
_splitCost = 1,
_spaceWhenUnsplit = false,
_splitListIfBeforeSplits = false;

/// Creates a new [DelimitedListBuilder] for a switch expression body.
DelimitedListBuilder.switchBody(this._visitor)
: _trailingComma = true,
_splitCost = 1,
_spaceWhenUnsplit = true,
_splitListIfBeforeSplits = true;

/// Creates a new [DelimitedListBuilder] for the value part of a switch
/// statement or expression:
///
/// ```
/// switch (value) { ... }
/// // ^^^^^^^
/// ```
DelimitedListBuilder.switchValue(this._visitor)
: _trailingComma = false,
_splitCost = 2,
_spaceWhenUnsplit = false,
_splitListIfBeforeSplits = false;

/// Creates a new [DelimitedListBuilder] for a type argument or type parameter
/// list.
DelimitedListBuilder.type(this._visitor)
: _trailingComma = false,
_splitCost = 2,
_spaceWhenUnsplit = false,
_splitListIfBeforeSplits = false;

ListPiece build() => ListPiece(
_leftBracket, _elements, _blanksAfter, _rightBracket, _isTypeList);
_leftBracket,
_elements,
_blanksAfter,
_rightBracket,
_splitCost,
_trailingComma,
_spaceWhenUnsplit,
_splitListIfBeforeSplits);

/// Adds the opening [bracket] to the built list.
///
Expand All @@ -66,9 +123,6 @@ class DelimitedListBuilder {
_visitor.token(delimiter);
_leftBracket = _visitor.writer.pop();
_visitor.writer.split();

// No trailing commas in type argument and type parameter lists.
if (bracket.type == TokenType.LT) _isTypeList = true;
}

/// Adds the closing [bracket] to the built list along with any comments that
Expand Down
60 changes: 40 additions & 20 deletions lib/src/front_end/piece_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,13 @@ mixin PieceFactory implements CommentWriter {
/// } else {}
/// ```
void createBlock(Block block, {bool forceSplit = false}) {
// Edge case: If the block is completely empty, output it as simple
// unsplittable text unless it's forced to split.
if (block.statements.isEmptyBody(block.rightBracket) && !forceSplit) {
token(block.leftBracket);
token(block.rightBracket);
return;
}

token(block.leftBracket);
var leftBracketPiece = writer.pop();
writer.split();

var sequence = SequenceBuilder(this);
for (var node in block.statements) {
sequence.add(node);
sequence.visit(node);
}

// Place any comments before the "}" inside the block.
Expand Down Expand Up @@ -115,17 +107,7 @@ mixin PieceFactory implements CommentWriter {
// The formatter will preserve the newline after element 3 and the lack of
// them after the other elements.

createDelimited(leftBracket, elements, rightBracket);
}

/// Creates a [ListPiece] for the given bracket-delimited set of elements.
void createDelimited(
Token leftBracket, Iterable<AstNode> elements, Token rightBracket) {
var builder = DelimitedListBuilder(this);
builder.leftBracket(leftBracket);
elements.forEach(builder.add);
builder.rightBracket(rightBracket);
writer.push(builder.build());
createList(leftBracket, elements, rightBracket);
}

/// Creates metadata annotations for a directive.
Expand Down Expand Up @@ -373,6 +355,44 @@ mixin PieceFactory implements CommentWriter {
writer.push(InfixPiece(operands));
}

/// Creates a [ListPiece] for the given bracket-delimited set of elements.
void createList(
Token leftBracket, Iterable<AstNode> elements, Token rightBracket) {
var builder = DelimitedListBuilder(this);
builder.leftBracket(leftBracket);
elements.forEach(builder.add);
builder.rightBracket(rightBracket);
writer.push(builder.build());
}

/// Visits the `switch (expr)` part of a switch statement or expression.
void createSwitchValue(Token switchKeyword, Token leftParenthesis,
Expression value, Token rightParenthesis) {
// Format like an argument list since it is an expression surrounded by
// parentheses.
var builder = DelimitedListBuilder.switchValue(this);

// Attach the `switch ` as part of the `(`.
token(switchKeyword);
writer.space();

builder.leftBracket(leftParenthesis);
builder.add(value);
builder.rightBracket(rightParenthesis);

writer.push(builder.build());
}

/// Creates a [ListPiece] for a type argument or type parameter list.
void createTypeList(
Token leftBracket, Iterable<AstNode> elements, Token rightBracket) {
var builder = DelimitedListBuilder.type(this);
builder.leftBracket(leftBracket);
elements.forEach(builder.add);
builder.rightBracket(rightBracket);
writer.push(builder.build());
}

/// Writes the parts of a formal parameter shared by all formal parameter
/// types: metadata, `covariant`, etc.
void startFormalParameter(FormalParameter parameter) {
Expand Down
Loading

0 comments on commit 2cee560

Please sign in to comment.