Skip to content

Commit f8c285f

Browse files
authored
Remove some unnecessary Piece classes. (#1491)
Remove some unnecessary Piece classes. In exploring various optimizations, it's clear that I'll probably need to make some changes to how all of the Piece classes work. The more of those classes there are, the harder that work is. Fortunately, there are a few Piece classes that don't really need to exist. Most were created before we had AdjacentPiece or the ability to insert spaces between pieces. This change: * Removes ClausePiece which it turns out behaves exactly like InfixPiece (and then renames ClausesPiece to ClausePiece). * Removes TryPiece. All it did was write its children with some spaces between them. * Removes PostfixPiece. It was just like InfixPiece but without a first operand. But it's only used by imports, where we can use the import header as the first operand and just use InfixPiece. * Remove AdjacentStringsPiece. It's just an InfixPiece that always splits. * Get rid of CaseStatementPiece. Once you hoist out the optionality of the guard, it's just an InfixPiece. * Reorganize ForPiece. Make that Piece just for the header part of a for statement or element. (We can almost get rid of this entirely and use AdjacentPiece, but it does some indentation that isn't otherwise supported.) Then to handle the body, we either write it inline if it's a block (the most common), or we use IfPiece, which has the same logic. Rename IfPiece to ControlFlowPiece to emphasize its more general use. It was already used for while statements, so this makes sense.
1 parent 0160142 commit f8c285f

File tree

10 files changed

+128
-374
lines changed

10 files changed

+128
-374
lines changed

lib/src/front_end/ast_node_visitor.dart

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@ import 'package:analyzer/source/line_info.dart';
99
import '../ast_extensions.dart';
1010
import '../constants.dart';
1111
import '../dart_formatter.dart';
12-
import '../piece/adjacent_strings.dart';
1312
import '../piece/assign.dart';
1413
import '../piece/case.dart';
1514
import '../piece/constructor.dart';
16-
import '../piece/if.dart';
15+
import '../piece/control_flow.dart';
1716
import '../piece/infix.dart';
1817
import '../piece/list.dart';
1918
import '../piece/piece.dart';
@@ -133,8 +132,13 @@ class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
133132

134133
@override
135134
void visitAdjacentStrings(AdjacentStrings node) {
136-
pieces.add(AdjacentStringsPiece(node.strings.map(nodePiece).toList(),
137-
indent: node.indentStrings));
135+
var piece = InfixPiece(const [], node.strings.map(nodePiece).toList(),
136+
indent: node.indentStrings);
137+
138+
// Adjacent strings always split.
139+
piece.pin(State.split);
140+
141+
pieces.add(piece);
138142
}
139143

140144
@override
@@ -873,7 +877,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
873877

874878
@override
875879
void visitIfElement(IfElement node) {
876-
var piece = IfPiece(isStatement: false);
880+
var piece = ControlFlowPiece(isStatement: false);
877881

878882
// Recurses through the else branches to flatten them into a linear if-else
879883
// chain handled by a single [IfPiece].
@@ -960,7 +964,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
960964

961965
@override
962966
void visitIfStatement(IfStatement node) {
963-
var piece = IfPiece(isStatement: true);
967+
var piece = ControlFlowPiece();
964968

965969
// Recurses through the else branches to flatten them into a linear if-else
966970
// chain handled by a single [IfPiece].
@@ -1759,10 +1763,13 @@ class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
17591763
pieces.space();
17601764

17611765
var patternPiece = nodePiece(member.guardedPattern.pattern);
1762-
var guardPiece =
1763-
optionalNodePiece(member.guardedPattern.whenClause);
17641766

1765-
pieces.add(CaseStatementPiece(patternPiece, guardPiece));
1767+
if (member.guardedPattern.whenClause case var whenClause?) {
1768+
pieces.add(
1769+
InfixPiece(const [], [patternPiece, nodePiece(whenClause)]));
1770+
} else {
1771+
pieces.add(patternPiece);
1772+
}
17661773

17671774
case SwitchDefault():
17681775
break; // Nothing to do.
@@ -1912,7 +1919,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
19121919

19131920
var body = nodePiece(node.body);
19141921

1915-
var piece = IfPiece(isStatement: true);
1922+
var piece = ControlFlowPiece();
19161923
piece.add(condition, body, isBlock: node.body is Block);
19171924
pieces.add(piece);
19181925
}

lib/src/front_end/piece_factory.dart

Lines changed: 83 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@ import '../ast_extensions.dart';
88
import '../piece/adjacent.dart';
99
import '../piece/assign.dart';
1010
import '../piece/clause.dart';
11+
import '../piece/control_flow.dart';
1112
import '../piece/for.dart';
1213
import '../piece/function.dart';
1314
import '../piece/if_case.dart';
1415
import '../piece/infix.dart';
1516
import '../piece/list.dart';
1617
import '../piece/piece.dart';
17-
import '../piece/postfix.dart';
1818
import '../piece/sequence.dart';
19-
import '../piece/try.dart';
2019
import '../piece/type.dart';
2120
import '../piece/variable.dart';
2221
import 'ast_node_visitor.dart';
@@ -506,12 +505,21 @@ mixin PieceFactory {
506505
_ => false,
507506
};
508507

509-
var forPiece = ForPiece(forKeywordPiece, forPartsPiece, bodyPiece,
510-
indentForParts: indentHeader, hasBlockBody: hasBlockBody);
511-
512-
if (forceSplitBody) forPiece.pin(State.split);
513-
514-
pieces.add(forPiece);
508+
if (hasBlockBody) {
509+
pieces
510+
.add(ForPiece(forKeywordPiece, forPartsPiece, indent: indentHeader));
511+
pieces.space();
512+
pieces.add(bodyPiece);
513+
} else {
514+
var forPiece = ControlFlowPiece();
515+
forPiece.add(
516+
ForPiece(forKeywordPiece, forPartsPiece, indent: indentHeader),
517+
bodyPiece,
518+
isBlock: false);
519+
520+
if (forceSplitBody) forPiece.pin(State.split);
521+
pieces.add(forPiece);
522+
}
515523
}
516524

517525
/// Writes a normal (not function-typed) formal parameter with a name and/or
@@ -738,43 +746,40 @@ mixin PieceFactory {
738746

739747
/// Writes a [TryPiece] for try statement.
740748
void writeTry(TryStatement tryStatement) {
741-
var piece = TryPiece();
742-
743-
var tryHeader = tokenPiece(tryStatement.tryKeyword);
744-
var tryBlock = pieces.build(() {
745-
writeBlock(tryStatement.body);
746-
});
747-
piece.add(tryHeader, tryBlock);
749+
pieces.token(tryStatement.tryKeyword);
750+
pieces.space();
751+
writeBlock(tryStatement.body);
748752

749753
for (var i = 0; i < tryStatement.catchClauses.length; i++) {
750754
var catchClause = tryStatement.catchClauses[i];
751755

752-
var catchClauseHeader = pieces.build(() {
753-
if (catchClause.onKeyword case var onKeyword?) {
754-
pieces.token(onKeyword, spaceAfter: true);
755-
pieces.visit(catchClause.exceptionType);
756-
}
756+
pieces.space();
757+
if (catchClause.onKeyword case var onKeyword?) {
758+
pieces.token(onKeyword, spaceAfter: true);
759+
pieces.visit(catchClause.exceptionType);
760+
}
757761

758-
if (catchClause.onKeyword != null && catchClause.catchKeyword != null) {
759-
pieces.space();
760-
}
762+
if (catchClause.onKeyword != null && catchClause.catchKeyword != null) {
763+
pieces.space();
764+
}
761765

762-
if (catchClause.catchKeyword case var catchKeyword?) {
763-
pieces.token(catchKeyword);
764-
pieces.space();
766+
if (catchClause.catchKeyword case var catchKeyword?) {
767+
pieces.token(catchKeyword);
768+
pieces.space();
765769

766-
var parameters = DelimitedListBuilder(this);
767-
parameters.leftBracket(catchClause.leftParenthesis!);
768-
if (catchClause.exceptionParameter case var exceptionParameter?) {
769-
parameters.visit(exceptionParameter);
770-
}
771-
if (catchClause.stackTraceParameter case var stackTraceParameter?) {
772-
parameters.visit(stackTraceParameter);
773-
}
774-
parameters.rightBracket(catchClause.rightParenthesis!);
775-
pieces.add(parameters.build());
770+
var parameters = DelimitedListBuilder(this);
771+
parameters.leftBracket(catchClause.leftParenthesis!);
772+
if (catchClause.exceptionParameter case var exceptionParameter?) {
773+
parameters.visit(exceptionParameter);
776774
}
777-
});
775+
if (catchClause.stackTraceParameter case var stackTraceParameter?) {
776+
parameters.visit(stackTraceParameter);
777+
}
778+
parameters.rightBracket(catchClause.rightParenthesis!);
779+
pieces.add(parameters.build());
780+
}
781+
782+
pieces.space();
778783

779784
// Edge case: When there's another catch/on/finally after this one, we
780785
// want to force the block to split even if it's empty.
@@ -787,72 +792,70 @@ mixin PieceFactory {
787792
// }
788793
var forceSplit = i < tryStatement.catchClauses.length - 1 ||
789794
tryStatement.finallyBlock != null;
790-
var catchClauseBody = pieces.build(() {
791-
writeBlock(catchClause.body, forceSplit: forceSplit);
792-
});
793-
piece.add(catchClauseHeader, catchClauseBody);
795+
writeBlock(catchClause.body, forceSplit: forceSplit);
794796
}
795797

796798
if (tryStatement.finallyBlock case var finallyBlock?) {
797-
var finallyHeader = tokenPiece(tryStatement.finallyKeyword!);
798-
var finallyBody = pieces.build(() {
799-
writeBlock(finallyBlock);
800-
});
801-
piece.add(finallyHeader, finallyBody);
799+
pieces.space();
800+
pieces.token(tryStatement.finallyKeyword!);
801+
pieces.space();
802+
writeBlock(finallyBlock);
802803
}
803-
804-
pieces.add(piece);
805804
}
806805

807806
/// Writes an [ImportPiece] for an import or export directive.
808807
void writeImport(NamespaceDirective directive, Token keyword,
809808
{Token? deferredKeyword, Token? asKeyword, SimpleIdentifier? prefix}) {
810809
pieces.withMetadata(directive.metadata, () {
811-
pieces.token(keyword);
812-
pieces.space();
813-
pieces.visit(directive.uri);
810+
if (directive.configurations.isEmpty && asKeyword == null) {
811+
// If there are no configurations or prefix (the common case), just
812+
// write the import directly inline.
813+
pieces.token(keyword);
814+
pieces.space();
815+
pieces.visit(directive.uri);
816+
} else {
817+
// Otherwise, allow splitting between the configurations and prefix.
818+
var sections = [
819+
pieces.build(() {
820+
pieces.token(keyword);
821+
pieces.space();
822+
pieces.visit(directive.uri);
823+
})
824+
];
814825

815-
if (directive.configurations.isNotEmpty) {
816-
var configurations = <Piece>[];
817826
for (var configuration in directive.configurations) {
818-
configurations.add(nodePiece(configuration));
827+
sections.add(nodePiece(configuration));
819828
}
820829

821-
pieces.add(PostfixPiece(configurations));
822-
}
823-
824-
if (asKeyword != null) {
825-
pieces.add(PostfixPiece([
826-
pieces.build(() {
830+
if (asKeyword != null) {
831+
sections.add(pieces.build(() {
827832
pieces.token(deferredKeyword, spaceAfter: true);
828833
pieces.token(asKeyword);
829834
pieces.space();
830835
pieces.visit(prefix!);
831-
})
832-
]));
836+
}));
837+
}
838+
839+
pieces.add(InfixPiece(const [], sections));
833840
}
834841

835842
if (directive.combinators.isNotEmpty) {
836-
var combinators = <ClausePiece>[];
843+
var combinators = <Piece>[];
837844
for (var combinatorNode in directive.combinators) {
838-
var combinatorKeyword = tokenPiece(combinatorNode.keyword);
839-
840845
switch (combinatorNode) {
841846
case HideCombinator(hiddenNames: var names):
842847
case ShowCombinator(shownNames: var names):
843-
var parts = <Piece>[];
844-
for (var name in names) {
845-
parts.add(tokenPiece(name.token, commaAfter: true));
846-
}
847-
848-
var combinator = ClausePiece(combinatorKeyword, parts);
849-
combinators.add(combinator);
848+
combinators.add(InfixPiece(const [], [
849+
tokenPiece(combinatorNode.keyword),
850+
for (var name in names)
851+
tokenPiece(name.token, commaAfter: true),
852+
]));
850853
default:
851854
throw StateError('Unknown combinator type $combinatorNode.');
852855
}
853856
}
854857

855-
pieces.add(ClausesPiece(combinators));
858+
pieces.add(ClausePiece(combinators));
856859
}
857860

858861
pieces.token(directive.semicolon);
@@ -1209,17 +1212,13 @@ mixin PieceFactory {
12091212
}
12101213
});
12111214

1212-
var clauses = <ClausePiece>[];
1215+
var clauses = <Piece>[];
12131216

12141217
void typeClause(Token keyword, List<AstNode> types) {
1215-
var keywordPiece = tokenPiece(keyword);
1216-
1217-
var typePieces = <Piece>[];
1218-
for (var type in types) {
1219-
typePieces.add(nodePiece(type, commaAfter: true));
1220-
}
1221-
1222-
clauses.add(ClausePiece(keywordPiece, typePieces));
1218+
clauses.add(InfixPiece(const [], [
1219+
tokenPiece(keyword),
1220+
for (var type in types) nodePiece(type, commaAfter: true),
1221+
]));
12231222
}
12241223

12251224
if (extendsClause != null) {
@@ -1248,9 +1247,9 @@ mixin PieceFactory {
12481247
[if (nativeClause.name case var name?) name]);
12491248
}
12501249

1251-
ClausesPiece? clausesPiece;
1250+
ClausePiece? clausesPiece;
12521251
if (clauses.isNotEmpty) {
1253-
clausesPiece = ClausesPiece(clauses,
1252+
clausesPiece = ClausePiece(clauses,
12541253
allowLeadingClause: extendsClause != null || onClause != null);
12551254
}
12561255

lib/src/piece/adjacent_strings.dart

Lines changed: 0 additions & 37 deletions
This file was deleted.

0 commit comments

Comments
 (0)