From 7ae652ec8fa727fbc30c9a2debe8ac3d0f95ab08 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 5 Dec 2023 16:12:05 -0800 Subject: [PATCH] Revamp the way tokens and comments are built into pieces. (#1336) Revamp the way tokens and comments are built into pieces. I recently ran into bugs where a line comment after some AST node will cause the node to split incorrectly. A simple example is: ``` var x = 1 + 2; // comment ``` Currently, the formatter splits that to: ``` var x = 1 + 2; // comment ``` It does that because the Piece tree it creates doesn't line up with the AST node boundaries. In particular, the current design appends tokens and comments to a preceding piece. So in this example, the piece tree looks like: ``` Var( `var` Assign( `x =` Infix( `1 +` `2; // comment` ) ) ) ``` Note how the `;` and line comment are attached as part of the RHS of the `+`. That's why the formatter thinks the line comment's newline is inside the + expression and forces it to split. We could fix this specific bug by making ExpressionStatement treat the `;` as a separate piece, but I suspect that we will be playing whack-a-mole if we keep the current design. Instead, this unfortunately giant PR revamps the piece API. It has a couple of intermingled changes: ### Split pieces at all AST boundaries Whenever a `visit___()` returns, an implicit split is inserted so that no single `TextPiece` contains tokens from a parent and child AST node. This directly fixes the above bug and all similar bugs in that category. Note that while we split the tokens into separate pieces, that doesn't mean they may split in the "line splitting" sense. The TextPieces go into AdjacentPiece objects that don't insert actual splits between the pieces. This means this change shouldn't significantly impact the performance of line splitting. It's just about ensuring that the nesting structure of the piece tree mirrors the nesting structure of the AST. That way, when a newline in a child piece node invalidates an outer piece, that invalidation respecst the original syntax. ### Revamp the API for creating pieces The previous API had a DSL-like "push" API where the pieces created by PieceWriter were stored internally and exposed by a fairly confusing `give()`/`take()`/`split()` API. That was necessary because any given `visit___()` method might not be *able* to return a Piece for its node if that node just concatenated its tokens into some surrounding piece. With the previous change where every AST node corresponds to a piece, we have that option. So this PR also makes that change. Every `visit___()` method is now required to return a piece. Likewise, all of the `create___()` methods in PieceFactory return the pieces they create. This avoids the need for a weird `take()` API. ### Add an AdjacentBuilder and buildPiece() API Getting rid of the implicit storage and dataflow for pieces is good for being able to easily reason about how the piece tree gets created out of child pieces. But it can come at the cost of making code that creates pieces very verbose with lots of local variables and `List` objects to store the intermediate pieces being built. To make that nicer, I wrote an AdjacentBuilder class with an imperative API for building an AdjacentPiece out of a series of tokens, nodes, and spaces. This API closely mirrors the original DSL-like API. Except now you know exactly what object the nodes and tokens are pushing their pieces into. To make that even nicer, I added a `buildPiece()` method that takes a callback, invokes it with a new AdjacentBuilder, and return the built result. This gets most code for building pieces fairly close to the original push-based API but with hopefully clearer more explicit dataflow. I'm really sorry for the giant size of this PR. If you want, I can try to break it into a series of smaller commits (but likely still one PR), but doing so is pretty challenging given how intertwined these changes are. It's hard to change the return type of the visit methods without also getting rid of the implicit dataflow and at that point, almost all the changes are there. Also, I added more tests to cover the cases around comments that were broken. --- lib/src/back_end/code_writer.dart | 57 +- lib/src/front_end/adjacent_builder.dart | 109 ++ lib/src/front_end/ast_node_visitor.dart | 1153 +++++++++-------- lib/src/front_end/comment_writer.dart | 87 +- lib/src/front_end/delimited_list_builder.dart | 78 +- lib/src/front_end/piece_factory.dart | 615 +++++---- lib/src/front_end/piece_writer.dart | 338 ++--- lib/src/front_end/sequence_builder.dart | 26 +- lib/src/piece/adjacent.dart | 13 +- lib/src/piece/do_while.dart | 28 - lib/src/piece/function.dart | 19 +- lib/src/piece/list.dart | 112 +- lib/src/piece/piece.dart | 71 +- lib/src/piece/sequence.dart | 11 + test/declaration/enum_member_comment.unit | 26 + test/expression/assignment_comment.stmt | 13 + test/expression/binary_comment.stmt | 16 +- test/statement/if_comment.stmt | 68 +- test/statement/return.stmt | 2 +- test/statement/return_comment.stmt | 28 + test/statement/while_comment.stmt | 43 + test/variable/local_comment.stmt | 14 +- 22 files changed, 1609 insertions(+), 1318 deletions(-) create mode 100644 lib/src/front_end/adjacent_builder.dart delete mode 100644 lib/src/piece/do_while.dart create mode 100644 test/expression/assignment_comment.stmt create mode 100644 test/statement/return_comment.stmt create mode 100644 test/statement/while_comment.stmt diff --git a/lib/src/back_end/code_writer.dart b/lib/src/back_end/code_writer.dart index 9659746a..96367229 100644 --- a/lib/src/back_end/code_writer.dart +++ b/lib/src/back_end/code_writer.dart @@ -1,6 +1,8 @@ // Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:math'; + import '../piece/piece.dart'; import 'solution.dart'; @@ -34,11 +36,11 @@ class CodeWriter { /// it as pending. This ensures that we don't write trailing whitespace, /// avoids writing spaces at the beginning of lines, and allows collapsing /// multiple redundant newlines. - _Whitespace _pendingWhitespace = _Whitespace.none; + Whitespace _pendingWhitespace = Whitespace.none; /// The number of spaces of indentation that should be begin the next line - /// when [_pendingWhitespace] is [_Whitespace.newline] or - /// [_Whitespace.blankLine]. + /// when [_pendingWhitespace] is [Whitespace.newline] or + /// [Whitespace.blankLine]. int _pendingIndent = 0; /// The cost of the currently chosen line splits. @@ -194,10 +196,7 @@ class CodeWriter { /// Writes a single space to the output. void space() { - // If a newline is already pending, then ignore the space. - if (_pendingWhitespace == _Whitespace.none) { - _pendingWhitespace = _Whitespace.space; - } + whitespace(Whitespace.space); } /// Inserts a line split in the output. @@ -209,16 +208,16 @@ class CodeWriter { void newline({bool blank = false, int? indent}) { if (indent != null) setIndent(indent); - handleNewline(); + whitespace(blank ? Whitespace.blankLine : Whitespace.newline); + } - // Collapse redundant newlines. - if (blank) { - _pendingWhitespace = _Whitespace.blankLine; - } else if (_pendingWhitespace != _Whitespace.blankLine) { - _pendingWhitespace = _Whitespace.newline; + void whitespace(Whitespace whitespace) { + if (whitespace case Whitespace.newline || Whitespace.blankLine) { + handleNewline(); + _pendingIndent = _options.indent; } - _pendingIndent = _options.indent; + _pendingWhitespace = _pendingWhitespace.collapse(whitespace); } /// Sets whether newlines are allowed to occur from this point on for the @@ -286,24 +285,24 @@ class CodeWriter { /// count of the written text, including whitespace. void _flushWhitespace() { switch (_pendingWhitespace) { - case _Whitespace.none: + case Whitespace.none: break; // Nothing to do. - case _Whitespace.newline: - case _Whitespace.blankLine: + case Whitespace.newline: + case Whitespace.blankLine: _finishLine(); _buffer.writeln(); - if (_pendingWhitespace == _Whitespace.blankLine) _buffer.writeln(); + if (_pendingWhitespace == Whitespace.blankLine) _buffer.writeln(); _column = _pendingIndent; _buffer.write(' ' * _column); - case _Whitespace.space: + case Whitespace.space: _buffer.write(' '); _column++; } - _pendingWhitespace = _Whitespace.none; + _pendingWhitespace = Whitespace.none; } void _finishLine() { @@ -328,18 +327,28 @@ class CodeWriter { } /// Different kinds of pending whitespace that have been requested. -enum _Whitespace { +/// +/// Note that the order of values in the enum is significant: later ones have +/// more whitespace than previous ones. +enum Whitespace { /// No pending whitespace. none, + /// A single space. + space, + /// A single newline. newline, /// Two newlines. - blankLine, + blankLine; - /// A single space. - space + /// Combines two pending whitespaces and returns the result. + /// + /// When two whitespaces overlap, they aren't both written: we don't want + /// two spaces or a newline followed by a space. Instead, the two whitespaces + /// are collapsed such that the largest one wins. + Whitespace collapse(Whitespace other) => values[max(index, other.index)]; } /// The mutable state local to a single piece being formatted. diff --git a/lib/src/front_end/adjacent_builder.dart b/lib/src/front_end/adjacent_builder.dart new file mode 100644 index 00000000..5a9dd4bc --- /dev/null +++ b/lib/src/front_end/adjacent_builder.dart @@ -0,0 +1,109 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; + +import '../piece/adjacent.dart'; +import '../piece/piece.dart'; +import 'piece_factory.dart'; + +/// Incrementally builds an [AdjacentPiece]. +class AdjacentBuilder { + final PieceFactory _visitor; + + /// The series of adjacent pieces. + final List _pieces = []; + + AdjacentBuilder(this._visitor); + + /// Yields a new piece containing all of the pieces added to or created by + /// this builder. The caller must ensure it doesn't build an empty piece. + /// + /// Also clears the builder's list of pieces so that this builder can be + /// reused to build more pieces. + Piece build() { + assert(_pieces.isNotEmpty); + + var result = _flattenPieces(); + _pieces.clear(); + + return result; + } + + /// Adds [piece] to this builder. + void add(Piece piece) { + _pieces.add(piece); + } + + /// Emit [token], along with any comments and formatted whitespace that comes + /// before it. + /// + /// If [lexeme] is given, uses that for the token's lexeme instead of its own. + /// + /// Does nothing if [token] is `null`. If [spaceBefore] is `true`, writes a + /// space before the token, likewise with [spaceAfter]. + void token(Token? token, + {bool spaceBefore = false, bool spaceAfter = false, String? lexeme}) { + if (token == null) return; + + if (spaceBefore) space(); + add(_visitor.pieces.tokenPiece(token, lexeme: lexeme)); + if (spaceAfter) space(); + } + + /// Writes any comments that appear before [token], which will be discarded. + /// + /// Used to ensure comments before a discarded token are preserved. + void commentsBefore(Token? token) { + if (token == null) return; + + var piece = _visitor.pieces.writeCommentsBefore(token); + if (piece != null) add(piece); + } + + /// Writes an optional modifier that precedes other code. + void modifier(Token? keyword) { + token(keyword, spaceAfter: true); + } + + /// Visits [node] if not `null` and adds the resulting [Piece] to this + /// builder. + void visit(AstNode? node, + {bool spaceBefore = false, + bool commaAfter = false, + bool spaceAfter = false}) { + if (node == null) return; + + if (spaceBefore) space(); + add(_visitor.nodePiece(node, commaAfter: commaAfter)); + if (spaceAfter) space(); + } + + /// Appends a space before the previous piece and the next one. + void space() { + _pieces.add(SpacePiece()); + } + + /// Removes redundant [AdjacentPiece] wrappers from [_pieces]. + Piece _flattenPieces() { + List flattened = []; + + void traverse(List pieces) { + for (var piece in pieces) { + if (piece is AdjacentPiece) { + traverse(piece.pieces); + } else { + flattened.add(piece); + } + } + } + + traverse(_pieces); + + // If there's only one piece, don't wrap it in a pointless AdjacentPiece. + if (flattened.length == 1) return flattened[0]; + + return AdjacentPiece(flattened); + } +} diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 720d7005..3414cd42 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -9,9 +9,10 @@ import 'package:analyzer/source/line_info.dart'; import '../ast_extensions.dart'; import '../constants.dart'; import '../dart_formatter.dart'; +import '../piece/adjacent.dart'; +import '../piece/assign.dart'; import '../piece/block.dart'; import '../piece/chain.dart'; -import '../piece/do_while.dart'; import '../piece/for.dart'; import '../piece/if.dart'; import '../piece/infix.dart'; @@ -19,6 +20,7 @@ import '../piece/list.dart'; import '../piece/piece.dart'; import '../piece/variable.dart'; import '../source_code.dart'; +import 'adjacent_builder.dart'; import 'comment_writer.dart'; import 'delimited_list_builder.dart'; import 'piece_factory.dart'; @@ -32,18 +34,22 @@ import 'sequence_builder.dart'; /// To avoid this class becoming a monolith, functionality is divided into a /// couple of mixins, one for each area of functionality. This class then /// contains only shared state and the visitor methods for the AST. -class AstNodeVisitor extends ThrowingAstVisitor - with CommentWriter, PieceFactory { - /// Cached line info for calculating blank lines. +class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override - final LineInfo lineInfo; + final PieceWriter pieces; @override - final PieceWriter pieces; + final CommentWriter comments; /// Create a new visitor that will be called to visit the code in [source]. - AstNodeVisitor(DartFormatter formatter, this.lineInfo, SourceCode source) - : pieces = PieceWriter(formatter, source); + factory AstNodeVisitor( + DartFormatter formatter, LineInfo lineInfo, SourceCode source) { + var comments = CommentWriter(lineInfo); + var pieces = PieceWriter(formatter, source, comments); + return AstNodeVisitor._(pieces, comments); + } + + AstNodeVisitor._(this.pieces, this.comments); /// Visits [node] and returns the formatted result. /// @@ -97,73 +103,74 @@ class AstNodeVisitor extends ThrowingAstVisitor // Write any comments at the end of the code. sequence.addCommentsBefore(node.endToken.next!); - pieces.give(sequence.build()); - // Finish writing and return the complete result. - return pieces.finish(); + return pieces.finish(sequence.build()); } @override - void visitAdjacentStrings(AdjacentStrings node) { + Piece visitAdjacentStrings(AdjacentStrings node) { throw UnimplementedError(); } @override - void visitAnnotation(Annotation node) { + Piece visitAnnotation(Annotation node) { throw UnimplementedError(); } @override - void visitArgumentList(ArgumentList node) { - createArgumentList( + Piece visitArgumentList(ArgumentList node) { + return createArgumentList( node.leftParenthesis, node.arguments, node.rightParenthesis); } @override - void visitAsExpression(AsExpression node) { - createInfix(node.expression, node.asOperator, node.type); + Piece visitAsExpression(AsExpression node) { + return createInfix(node.expression, node.asOperator, node.type); } @override - void visitAssertInitializer(AssertInitializer node) { + Piece visitAssertInitializer(AssertInitializer node) { throw UnimplementedError(); } @override - void visitAssertStatement(AssertStatement node) { - token(node.assertKeyword); - createArgumentList( - node.leftParenthesis, - [ - node.condition, - if (node.message case var message?) message, - ], - node.rightParenthesis); - token(node.semicolon); + Piece visitAssertStatement(AssertStatement node) { + return buildPiece((b) { + b.token(node.assertKeyword); + b.add(createArgumentList( + node.leftParenthesis, + [ + node.condition, + if (node.message case var message?) message, + ], + node.rightParenthesis)); + b.token(node.semicolon); + }); } @override - void visitAssignedVariablePattern(AssignedVariablePattern node) { + Piece visitAssignedVariablePattern(AssignedVariablePattern node) { throw UnimplementedError(); } @override - void visitAssignmentExpression(AssignmentExpression node) { - visit(node.leftHandSide); - space(); - finishAssignment(node.operator, node.rightHandSide); + Piece visitAssignmentExpression(AssignmentExpression node) { + return createAssignment( + node.leftHandSide, node.operator, node.rightHandSide); } @override - void visitAwaitExpression(AwaitExpression node) { - token(node.awaitKeyword); - space(); - visit(node.expression); + Piece visitAwaitExpression(AwaitExpression node) { + return buildPiece((b) { + b.token(node.awaitKeyword); + b.space(); + b.visit(node.expression); + }); } @override - void visitBinaryExpression(BinaryExpression node) { - createInfixChain( + Piece visitBinaryExpression(BinaryExpression node) { + return createInfixChain( node, precedence: node.operator.type.precedence, (expression) => ( @@ -174,49 +181,51 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitBlock(Block node) { - createBlock(node); + Piece visitBlock(Block node) { + return createBlock(node); } @override - void visitBlockFunctionBody(BlockFunctionBody node) { - functionBodyModifiers(node); - visit(node.block); + Piece visitBlockFunctionBody(BlockFunctionBody node) { + return buildPiece((b) { + functionBodyModifiers(node, b); + b.visit(node.block); + }); } @override - void visitBooleanLiteral(BooleanLiteral node) { - token(node.literal); + Piece visitBooleanLiteral(BooleanLiteral node) { + return tokenPiece(node.literal); } @override - void visitBreakStatement(BreakStatement node) { - createBreak(node.breakKeyword, node.label, node.semicolon); + Piece visitBreakStatement(BreakStatement node) { + return createBreak(node.breakKeyword, node.label, node.semicolon); } @override - void visitCascadeExpression(CascadeExpression node) { + Piece visitCascadeExpression(CascadeExpression node) { throw UnimplementedError(); } @override - void visitCastPattern(CastPattern node) { + Piece visitCastPattern(CastPattern node) { throw UnimplementedError(); } @override - void visitCatchClause(CatchClause node) { - assert(false, 'This node is handled by visitTryStatement().'); + Piece visitCatchClause(CatchClause node) { + throw UnsupportedError('This node is handled by visitTryStatement().'); } @override - void visitCatchClauseParameter(CatchClauseParameter node) { - token(node.name); + Piece visitCatchClauseParameter(CatchClauseParameter node) { + return tokenPiece(node.name); } @override - void visitClassDeclaration(ClassDeclaration node) { - createType( + Piece visitClassDeclaration(ClassDeclaration node) { + return createType( node.metadata, [ node.abstractKeyword, @@ -241,8 +250,8 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitClassTypeAlias(ClassTypeAlias node) { - createType( + Piece visitClassTypeAlias(ClassTypeAlias node) { + return createType( node.metadata, [ node.abstractKeyword, @@ -263,36 +272,38 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitComment(Comment node) { - assert(false, 'Comments should be handled elsewhere.'); + Piece visitComment(Comment node) { + throw UnsupportedError('Comments should be handled elsewhere.'); } @override - void visitCommentReference(CommentReference node) { - assert(false, 'Comments should be handled elsewhere.'); + Piece visitCommentReference(CommentReference node) { + throw UnsupportedError('Comments should be handled elsewhere.'); } @override - void visitCompilationUnit(CompilationUnit node) { - assert(false, 'CompilationUnit should be handled directly by format().'); + Piece visitCompilationUnit(CompilationUnit node) { + throw UnsupportedError( + 'CompilationUnit should be handled directly by format().'); } @override - void visitConditionalExpression(ConditionalExpression node) { - visit(node.condition); - var condition = pieces.split(); + Piece visitConditionalExpression(ConditionalExpression node) { + var condition = nodePiece(node.condition); - token(node.question); - space(); - visit(node.thenExpression); - var thenBranch = pieces.split(); + var thenPiece = buildPiece((b) { + b.token(node.question); + b.space(); + b.visit(node.thenExpression); + }); - token(node.colon); - space(); - visit(node.elseExpression); - var elseBranch = pieces.take(); + var elsePiece = buildPiece((b) { + b.token(node.colon); + b.space(); + b.visit(node.elseExpression); + }); - var piece = InfixPiece([condition, thenBranch, elseBranch]); + var piece = InfixPiece([condition, thenPiece, elsePiece]); // If conditional expressions are directly nested, force them all to split, // both parents and children. @@ -302,129 +313,130 @@ class AstNodeVisitor extends ThrowingAstVisitor piece.pin(State.split); } - pieces.give(piece); + return piece; } @override - void visitConfiguration(Configuration node) { - token(node.ifKeyword); - space(); - token(node.leftParenthesis); + Piece visitConfiguration(Configuration node) { + return buildPiece((b) { + b.token(node.ifKeyword); + b.space(); + b.token(node.leftParenthesis); - if (node.equalToken case var equals?) { - createInfix(node.name, equals, node.value!, hanging: true); - } else { - visit(node.name); - } + if (node.equalToken case var equals?) { + b.add(createInfix(node.name, equals, node.value!, hanging: true)); + } else { + b.visit(node.name); + } - token(node.rightParenthesis); - space(); - visit(node.uri); + b.token(node.rightParenthesis); + b.space(); + b.visit(node.uri); + }); } @override - void visitConstantPattern(ConstantPattern node) { + Piece visitConstantPattern(ConstantPattern node) { if (node.constKeyword != null) throw UnimplementedError(); - visit(node.expression); + return nodePiece(node.expression); } @override - void visitConstructorDeclaration(ConstructorDeclaration node) { + Piece visitConstructorDeclaration(ConstructorDeclaration node) { throw UnimplementedError(); } @override - void visitConstructorFieldInitializer(ConstructorFieldInitializer node) { + Piece visitConstructorFieldInitializer(ConstructorFieldInitializer node) { throw UnimplementedError(); } @override - void visitConstructorName(ConstructorName node) { - assert(false, 'This node is handled by visitInstanceCreationExpression().'); + Piece visitConstructorName(ConstructorName node) { + throw UnsupportedError( + 'This node is handled by visitInstanceCreationExpression().'); } @override - void visitContinueStatement(ContinueStatement node) { - createBreak(node.continueKeyword, node.label, node.semicolon); + Piece visitContinueStatement(ContinueStatement node) { + return createBreak(node.continueKeyword, node.label, node.semicolon); } @override - void visitDeclaredIdentifier(DeclaredIdentifier node) { - modifier(node.keyword); - visit(node.type, after: space); - token(node.name); + Piece visitDeclaredIdentifier(DeclaredIdentifier node) { + return buildPiece((b) { + b.modifier(node.keyword); + b.visit(node.type, spaceAfter: true); + b.token(node.name); + }); } @override - void visitDeclaredVariablePattern(DeclaredVariablePattern node) { + Piece visitDeclaredVariablePattern(DeclaredVariablePattern node) { throw UnimplementedError(); } @override - void visitDefaultFormalParameter(DefaultFormalParameter node) { - visit(node.parameter); - + Piece visitDefaultFormalParameter(DefaultFormalParameter node) { if (node.separator case var separator?) { - finishAssignment(separator, node.defaultValue!); + return createAssignment(node.parameter, separator, node.defaultValue!, + spaceBeforeOperator: separator.type == TokenType.EQ); + } else { + return nodePiece(node.parameter); } } @override - void visitDoStatement(DoStatement node) { - token(node.doKeyword); - space(); - visit(node.body); - space(); - token(node.whileKeyword); - var body = pieces.split(); - - token(node.leftParenthesis); - visit(node.condition); - token(node.rightParenthesis); - token(node.semicolon); - var condition = pieces.take(); - - pieces.give(DoWhilePiece(body, condition)); + Piece visitDoStatement(DoStatement node) { + return buildPiece((b) { + b.token(node.doKeyword); + b.space(); + b.visit(node.body); + b.space(); + b.token(node.whileKeyword); + b.space(); + b.token(node.leftParenthesis); + b.visit(node.condition); + b.token(node.rightParenthesis); + b.token(node.semicolon); + }); } @override - void visitDottedName(DottedName node) { - createDotted(node.components); + Piece visitDottedName(DottedName node) { + return createDotted(node.components); } @override - void visitDoubleLiteral(DoubleLiteral node) { - token(node.literal); + Piece visitDoubleLiteral(DoubleLiteral node) { + return tokenPiece(node.literal); } @override - void visitEmptyFunctionBody(EmptyFunctionBody node) { - token(node.semicolon); + Piece visitEmptyFunctionBody(EmptyFunctionBody node) { + return tokenPiece(node.semicolon); } @override - void visitEmptyStatement(EmptyStatement node) { - token(node.semicolon); + Piece visitEmptyStatement(EmptyStatement node) { + return tokenPiece(node.semicolon); } @override - void visitEnumConstantDeclaration(EnumConstantDeclaration node) { - token(node.name); - if (node.arguments case var arguments?) { - visit(arguments.typeArguments); - visit(arguments.argumentList); - } + Piece visitEnumConstantDeclaration(EnumConstantDeclaration node) { + return createEnumConstant(node); } @override - void visitEnumDeclaration(EnumDeclaration node) { + Piece visitEnumDeclaration(EnumDeclaration node) { if (node.metadata.isNotEmpty) throw UnimplementedError(); - token(node.enumKeyword); - space(); - token(node.name); - visit(node.typeParameters); - space(); + var header = buildPiece((b) { + b.token(node.enumKeyword); + b.space(); + b.token(node.name); + b.visit(node.typeParameters); + }); if (node.members.isEmpty) { // If there are no members, format the constants like a delimited list. @@ -433,34 +445,26 @@ class AstNodeVisitor extends ThrowingAstVisitor this, const ListStyle( spaceWhenUnsplit: true, splitListIfBeforeSplits: true)); - builder.leftBracket(node.leftBracket); + builder.leftBracket(node.leftBracket, preceding: header); node.constants.forEach(builder.visit); builder.rightBracket(semicolon: node.semicolon, node.rightBracket); - pieces.give(builder.build()); + return builder.build(); } else { + var builder = AdjacentBuilder(this); + builder.add(header); + builder.space(); + // If there are members, format it like a block where each constant and // member is on its own line. - token(node.leftBracket); - var leftBracketPiece = pieces.split(); + var leftBracketPiece = tokenPiece(node.leftBracket); var sequence = SequenceBuilder(this); for (var constant in node.constants) { sequence.addCommentsBefore(constant.firstNonCommentToken); - visit(constant); - - if (constant != node.constants.last) { - commaAfter(constant); - } else { - // Discard the trailing comma if there is one since there is a - // semicolon to use as the separator, but preserve any comments before - // the discarded comma. - var trailingComma = constant.commaAfter; - if (trailingComma != null) writeCommentsBefore(trailingComma); - - token(node.semicolon); - } - - sequence.add(pieces.split()); + sequence.add(createEnumConstant(constant, + hasMembers: true, + isLastConstant: constant == node.constants.last, + semicolon: node.semicolon)); } // Insert a blank line between the constants and members. @@ -477,40 +481,52 @@ class AstNodeVisitor extends ThrowingAstVisitor // Place any comments before the "}" inside the block. sequence.addCommentsBefore(node.rightBracket); - token(node.rightBracket); - var rightBracketPiece = pieces.take(); + var rightBracketPiece = tokenPiece(node.rightBracket); - pieces.give( + builder.add( BlockPiece(leftBracketPiece, sequence.build(), rightBracketPiece)); + return builder.build(); } } @override - void visitExportDirective(ExportDirective node) { - createImport(node, node.exportKeyword); + Piece visitExportDirective(ExportDirective node) { + return createImport(node, node.exportKeyword); } @override - void visitExpressionFunctionBody(ExpressionFunctionBody node) { - functionBodyModifiers(node); - finishAssignment(node.functionDefinition, node.expression); - token(node.semicolon); + Piece visitExpressionFunctionBody(ExpressionFunctionBody node) { + return buildPiece((b) { + var operatorPiece = buildPiece((b) { + functionBodyModifiers(node, b); + b.token(node.functionDefinition); + }); + + var expression = nodePiece(node.expression); + + b.add(AssignPiece(operatorPiece, expression, + isValueDelimited: node.expression.canBlockSplit)); + b.token(node.semicolon); + }); } @override - void visitExpressionStatement(ExpressionStatement node) { - visit(node.expression); - token(node.semicolon); + Piece visitExpressionStatement(ExpressionStatement node) { + return buildPiece((b) { + b.visit(node.expression); + b.token(node.semicolon); + }); } @override - void visitExtendsClause(ExtendsClause node) { - assert(false, 'This node is handled by PieceFactory.createType().'); + Piece visitExtendsClause(ExtendsClause node) { + throw UnsupportedError( + 'This node is handled by PieceFactory.createType().'); } @override - void visitExtensionDeclaration(ExtensionDeclaration node) { - createType(node.metadata, const [], node.extensionKeyword, node.name, + Piece visitExtensionDeclaration(ExtensionDeclaration node) { + return createType(node.metadata, const [], node.extensionKeyword, node.name, typeParameters: node.typeParameters, onType: (node.onKeyword, node.extendedType), body: ( @@ -521,22 +537,24 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitFieldDeclaration(FieldDeclaration node) { - modifier(node.externalKeyword); - modifier(node.staticKeyword); - modifier(node.abstractKeyword); - modifier(node.covariantKeyword); - visit(node.fields); - token(node.semicolon); + Piece visitFieldDeclaration(FieldDeclaration node) { + return buildPiece((b) { + b.modifier(node.externalKeyword); + b.modifier(node.staticKeyword); + b.modifier(node.abstractKeyword); + b.modifier(node.covariantKeyword); + b.visit(node.fields); + b.token(node.semicolon); + }); } @override - void visitFieldFormalParameter(FieldFormalParameter node) { + Piece visitFieldFormalParameter(FieldFormalParameter node) { throw UnimplementedError(); } @override - void visitFormalParameterList(FormalParameterList node) { + Piece visitFormalParameterList(FormalParameterList node) { // Find the first non-mandatory parameter (if there are any). var firstOptional = node.parameters.indexWhere((p) => p is DefaultFormalParameter); @@ -559,19 +577,20 @@ class AstNodeVisitor extends ThrowingAstVisitor } builder.rightBracket(node.rightParenthesis, delimiter: node.rightDelimiter); - pieces.give(builder.build()); + return builder.build(); } @override - void visitForElement(ForElement node) { + Piece visitForElement(ForElement node) { throw UnimplementedError(); } @override - void visitForStatement(ForStatement node) { - modifier(node.awaitKeyword); - token(node.forKeyword); - var forKeyword = pieces.split(); + Piece visitForStatement(ForStatement node) { + var forKeyword = buildPiece((b) { + b.modifier(node.awaitKeyword); + b.token(node.forKeyword); + }); Piece forPartsPiece; switch (node.forLoopParts) { @@ -586,11 +605,12 @@ class AstNodeVisitor extends ThrowingAstVisitor ) && var forParts when node.rightParenthesis.precedingComments == null: - token(node.leftParenthesis); - token(forParts.leftSeparator); - token(forParts.rightSeparator); - token(node.rightParenthesis); - forPartsPiece = pieces.split(); + forPartsPiece = buildPiece((b) { + b.token(node.leftParenthesis); + b.token(forParts.leftSeparator); + b.token(forParts.rightSeparator); + b.token(node.rightParenthesis); + }); case ForParts forParts && ForPartsWithDeclarations(variables: AstNode? initializer): @@ -616,36 +636,36 @@ class AstNodeVisitor extends ThrowingAstVisitor // The initializer clause. if (initializer != null) { partsList.addCommentsBefore(initializer.beginToken); - visit(initializer); + partsList.add(buildPiece((b) { + b.visit(initializer); + b.token(forParts.leftSeparator); + })); } else { // No initializer, so look at the comments before `;`. partsList.addCommentsBefore(forParts.leftSeparator); + partsList.add(tokenPiece(forParts.leftSeparator)); } - token(forParts.leftSeparator); - partsList.add(pieces.split()); - // The condition clause. if (forParts.condition case var conditionExpression?) { partsList.addCommentsBefore(conditionExpression.beginToken); - visit(conditionExpression); + partsList.add(buildPiece((b) { + b.visit(conditionExpression); + b.token(forParts.rightSeparator); + })); } else { partsList.addCommentsBefore(forParts.rightSeparator); + partsList.add(tokenPiece(forParts.rightSeparator)); } - token(forParts.rightSeparator); - partsList.add(pieces.split()); - // The update clauses. if (forParts.updaters.isNotEmpty) { partsList.addCommentsBefore(forParts.updaters.first.beginToken); - createList(forParts.updaters, - style: const ListStyle(commas: Commas.nonTrailing)); - partsList.add(pieces.split()); + partsList.add(createList(forParts.updaters, + style: const ListStyle(commas: Commas.nonTrailing))); } partsList.rightBracket(node.rightParenthesis); - pieces.split(); forPartsPiece = partsList.build(); case ForPartsWithPattern(): @@ -665,58 +685,57 @@ class AstNodeVisitor extends ThrowingAstVisitor // body; // } // ``` - token(node.leftParenthesis); - visit(variable); - - finishAssignment(forEachParts.inKeyword, forEachParts.iterable, - splitBeforeOperator: true); - token(node.rightParenthesis); - forPartsPiece = pieces.split(); + forPartsPiece = buildPiece((b) { + b.token(node.leftParenthesis); + b.add(createAssignment( + variable, forEachParts.inKeyword, forEachParts.iterable, + splitBeforeOperator: true)); + b.token(node.rightParenthesis); + }); case ForEachPartsWithPattern(): throw UnimplementedError(); } - visit(node.body); - var body = pieces.take(); + var body = nodePiece(node.body); - pieces.give(ForPiece(forKeyword, forPartsPiece, body, - hasBlockBody: node.body is Block)); + return ForPiece(forKeyword, forPartsPiece, body, + hasBlockBody: node.body is Block); } @override - void visitForEachPartsWithDeclaration(ForEachPartsWithDeclaration node) { - assert(false, 'This node is handled by visitForStatement().'); + Piece visitForEachPartsWithDeclaration(ForEachPartsWithDeclaration node) { + throw UnsupportedError('This node is handled by visitForStatement().'); } @override - void visitForEachPartsWithIdentifier(ForEachPartsWithIdentifier node) { - assert(false, 'This node is handled by visitForStatement().'); + Piece visitForEachPartsWithIdentifier(ForEachPartsWithIdentifier node) { + throw UnsupportedError('This node is handled by visitForStatement().'); } @override - void visitForEachPartsWithPattern(ForEachPartsWithPattern node) { - assert(false, 'This node is handled by visitForStatement().'); + Piece visitForEachPartsWithPattern(ForEachPartsWithPattern node) { + throw UnsupportedError('This node is handled by visitForStatement().'); } @override - void visitForPartsWithDeclarations(ForPartsWithDeclarations node) { - assert(false, 'This node is handled by visitForStatement().'); + Piece visitForPartsWithDeclarations(ForPartsWithDeclarations node) { + throw UnsupportedError('This node is handled by visitForStatement().'); } @override - void visitForPartsWithExpression(ForPartsWithExpression node) { - assert(false, 'This node is handled by visitForStatement().'); + Piece visitForPartsWithExpression(ForPartsWithExpression node) { + throw UnsupportedError('This node is handled by visitForStatement().'); } @override - void visitForPartsWithPattern(ForPartsWithPattern node) { - assert(false, 'This node is handled by visitForStatement().'); + Piece visitForPartsWithPattern(ForPartsWithPattern node) { + throw UnsupportedError('This node is handled by visitForStatement().'); } @override - void visitFunctionDeclaration(FunctionDeclaration node) { - createFunction( + Piece visitFunctionDeclaration(FunctionDeclaration node) { + return createFunction( externalKeyword: node.externalKeyword, returnType: node.returnType, propertyKeyword: node.propertyKeyword, @@ -727,84 +746,93 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitFunctionDeclarationStatement(FunctionDeclarationStatement node) { - visit(node.functionDeclaration); + Piece visitFunctionDeclarationStatement(FunctionDeclarationStatement node) { + return nodePiece(node.functionDeclaration); } @override - void visitFunctionExpression(FunctionExpression node) { - finishFunction(null, node.typeParameters, node.parameters, node.body); + Piece visitFunctionExpression(FunctionExpression node) { + return createFunction( + typeParameters: node.typeParameters, + parameters: node.parameters, + body: node.body); } @override - void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { + Piece visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { throw UnimplementedError(); } @override - void visitFunctionReference(FunctionReference node) { + Piece visitFunctionReference(FunctionReference node) { throw UnimplementedError(); } @override - void visitFunctionTypeAlias(FunctionTypeAlias node) { + Piece visitFunctionTypeAlias(FunctionTypeAlias node) { throw UnimplementedError(); } @override - void visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) { - startFormalParameter(node); - createFunctionType(node.returnType, node.name, node.typeParameters, - node.parameters, node.question); + Piece visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) { + return createFunctionType( + parameter: node, + node.returnType, + node.name, + node.typeParameters, + node.parameters, + node.question); } @override - void visitGenericFunctionType(GenericFunctionType node) { - createFunctionType(node.returnType, node.functionKeyword, + Piece visitGenericFunctionType(GenericFunctionType node) { + return createFunctionType(node.returnType, node.functionKeyword, node.typeParameters, node.parameters, node.question); } @override - void visitGenericTypeAlias(GenericTypeAlias node) { + Piece visitGenericTypeAlias(GenericTypeAlias node) { throw UnimplementedError(); } @override - void visitHideCombinator(HideCombinator node) { - assert(false, 'Combinators are handled by createImport().'); + Piece visitHideCombinator(HideCombinator node) { + throw UnsupportedError('Combinators are handled by createImport().'); } @override - void visitIfElement(IfElement node) { + Piece visitIfElement(IfElement node) { throw UnimplementedError(); } @override - void visitIfStatement(IfStatement node) { - createIf(node); + Piece visitIfStatement(IfStatement node) { + return createIf(node); } @override - void visitImplementsClause(ImplementsClause node) { - assert(false, 'This node is handled by PieceFactory.createType().'); + Piece visitImplementsClause(ImplementsClause node) { + throw UnsupportedError( + 'This node is handled by PieceFactory.createType().'); } @override - void visitImportDirective(ImportDirective node) { - createImport(node, node.importKeyword, + Piece visitImportDirective(ImportDirective node) { + return createImport(node, node.importKeyword, deferredKeyword: node.deferredKeyword, asKeyword: node.asKeyword, prefix: node.prefix); } @override - void visitIndexExpression(IndexExpression node) { + Piece visitIndexExpression(IndexExpression node) { throw UnimplementedError(); } @override - void visitInstanceCreationExpression(InstanceCreationExpression node) { - token(node.keyword, after: space); + Piece visitInstanceCreationExpression(InstanceCreationExpression node) { + var builder = AdjacentBuilder(this); + builder.token(node.keyword, spaceAfter: true); // If there is an import prefix and/or constructor name, then allow // splitting before the `.`. This doesn't look good, but is consistent with @@ -816,51 +844,51 @@ class AstNodeVisitor extends ThrowingAstVisitor var constructor = node.constructorName; if (constructor.type.importPrefix case var importPrefix?) { - token(importPrefix.name); - operations.add(pieces.split()); - token(importPrefix.period); + builder.token(importPrefix.name); + operations.add(builder.build()); + builder.token(importPrefix.period); } - // The name of the type being constructed. + // The type being constructed. var type = constructor.type; - token(type.name2); - visit(type.typeArguments); - token(type.question); + builder.token(type.name2); + builder.visit(type.typeArguments); // If this is a named constructor call, the name. - if (constructor.name != null) { - operations.add(pieces.split()); - token(constructor.period); - visit(constructor.name); + if (constructor.name case var name?) { + operations.add(builder.build()); + builder.token(constructor.period); + builder.visit(name); } - visit(node.argumentList); + builder.visit(node.argumentList); + operations.add(builder.build()); - // If there was a prefix or constructor name, then make a splittable piece. - if (operations.isNotEmpty) { - operations.add(pieces.take()); - pieces.give(ChainPiece(operations)); + if (operations.length > 1) { + return ChainPiece(operations); + } else { + return operations.first; } } @override - void visitIntegerLiteral(IntegerLiteral node) { - token(node.literal); + Piece visitIntegerLiteral(IntegerLiteral node) { + return tokenPiece(node.literal); } @override - void visitInterpolationExpression(InterpolationExpression node) { + Piece visitInterpolationExpression(InterpolationExpression node) { throw UnimplementedError(); } @override - void visitInterpolationString(InterpolationString node) { + Piece visitInterpolationString(InterpolationString node) { throw UnimplementedError(); } @override - void visitIsExpression(IsExpression node) { - createInfix( + Piece visitIsExpression(IsExpression node) { + return createInfix( node.expression, node.isOperator, operator2: node.notOperator, @@ -868,38 +896,42 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitLabel(Label node) { - visit(node.label); - token(node.colon); + Piece visitLabel(Label node) { + return buildPiece((b) { + b.visit(node.label); + b.token(node.colon); + }); } @override - void visitLabeledStatement(LabeledStatement node) { + Piece visitLabeledStatement(LabeledStatement node) { var sequence = SequenceBuilder(this); for (var label in node.labels) { sequence.visit(label); } sequence.visit(node.statement); - pieces.give(sequence.build()); + return sequence.build(); } @override - void visitLibraryDirective(LibraryDirective node) { - createDirectiveMetadata(node); - token(node.libraryKeyword); - visit(node.name2, before: space); - token(node.semicolon); + Piece visitLibraryDirective(LibraryDirective node) { + return buildPiece((b) { + createDirectiveMetadata(node); + b.token(node.libraryKeyword); + b.visit(node.name2, spaceBefore: true); + b.token(node.semicolon); + }); } @override - void visitLibraryIdentifier(LibraryIdentifier node) { - createDotted(node.components); + Piece visitLibraryIdentifier(LibraryIdentifier node) { + return createDotted(node.components); } @override - void visitListLiteral(ListLiteral node) { - createCollection( + Piece visitListLiteral(ListLiteral node) { + return createCollection( node.constKeyword, typeArguments: node.typeArguments, node.leftBracket, @@ -909,39 +941,39 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitListPattern(ListPattern node) { + Piece visitListPattern(ListPattern node) { throw UnimplementedError(); } @override - void visitLogicalAndPattern(LogicalAndPattern node) { + Piece visitLogicalAndPattern(LogicalAndPattern node) { throw UnimplementedError(); } @override - void visitLogicalOrPattern(LogicalOrPattern node) { + Piece visitLogicalOrPattern(LogicalOrPattern node) { throw UnimplementedError(); } @override - void visitMapLiteralEntry(MapLiteralEntry node) { - visit(node.key); - finishAssignment(node.separator, node.value); + Piece visitMapLiteralEntry(MapLiteralEntry node) { + return createAssignment(node.key, node.separator, node.value, + spaceBeforeOperator: false); } @override - void visitMapPattern(MapPattern node) { + Piece visitMapPattern(MapPattern node) { throw UnimplementedError(); } @override - void visitMapPatternEntry(MapPatternEntry node) { + Piece visitMapPatternEntry(MapPatternEntry node) { throw UnimplementedError(); } @override - void visitMethodDeclaration(MethodDeclaration node) { - createFunction( + Piece visitMethodDeclaration(MethodDeclaration node) { + return createFunction( externalKeyword: node.externalKeyword, modifierKeyword: node.modifierKeyword, returnType: node.returnType, @@ -954,20 +986,22 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitMethodInvocation(MethodInvocation node) { - // TODO(tall): Support splitting at `.` or `?.`. Right now we just format - // it inline so that we can use method calls in other tests. - visit(node.target); - token(node.operator); - - visit(node.methodName); - visit(node.typeArguments); - visit(node.argumentList); + Piece visitMethodInvocation(MethodInvocation node) { + return buildPiece((b) { + // TODO(tall): Support splitting at `.` or `?.`. Right now we just format + // it inline so that we can use method calls in other tests. + b.visit(node.target); + b.token(node.operator); + b.visit(node.methodName); + b.visit(node.typeArguments); + b.visit(node.argumentList); + }); } @override - void visitMixinDeclaration(MixinDeclaration node) { - createType(node.metadata, [node.baseKeyword], node.mixinKeyword, node.name, + Piece visitMixinDeclaration(MixinDeclaration node) { + return createType( + node.metadata, [node.baseKeyword], node.mixinKeyword, node.name, typeParameters: node.typeParameters, onClause: node.onClause, implementsClause: node.implementsClause, @@ -979,155 +1013,164 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitNamedExpression(NamedExpression node) { - visit(node.name.label); - finishAssignment(node.name.colon, node.expression); + Piece visitNamedExpression(NamedExpression node) { + return createAssignment(node.name.label, node.name.colon, node.expression, + spaceBeforeOperator: false); } @override - void visitNamedType(NamedType node) { - if (node.importPrefix case var importPrefix?) { - token(importPrefix.name); - token(importPrefix.period); - } - - token(node.name2); - visit(node.typeArguments); - token(node.question); + Piece visitNamedType(NamedType node) { + return buildPiece((b) { + b.token(node.importPrefix?.name); + b.token(node.importPrefix?.period); + b.token(node.name2); + b.visit(node.typeArguments); + b.token(node.question); + }); } @override - void visitNativeClause(NativeClause node) { - space(); - token(node.nativeKeyword); - space(); - visit(node.name); + Piece visitNativeClause(NativeClause node) { + return buildPiece((b) { + b.token(node.nativeKeyword); + b.visit(node.name, spaceBefore: true); + }); } @override - void visitNativeFunctionBody(NativeFunctionBody node) { + Piece visitNativeFunctionBody(NativeFunctionBody node) { throw UnimplementedError(); } @override - void visitNullAssertPattern(NullAssertPattern node) { + Piece visitNullAssertPattern(NullAssertPattern node) { throw UnimplementedError(); } @override - void visitNullCheckPattern(NullCheckPattern node) { + Piece visitNullCheckPattern(NullCheckPattern node) { throw UnimplementedError(); } @override - void visitNullLiteral(NullLiteral node) { - token(node.literal); + Piece visitNullLiteral(NullLiteral node) { + return tokenPiece(node.literal); } @override - void visitObjectPattern(ObjectPattern node) { + Piece visitObjectPattern(ObjectPattern node) { throw UnimplementedError(); } @override - void visitOnClause(OnClause node) { - assert(false, 'This node is handled by PieceFactory.createType().'); + Piece visitOnClause(OnClause node) { + throw UnsupportedError( + 'This node is handled by PieceFactory.createType().'); } @override - void visitParenthesizedExpression(ParenthesizedExpression node) { - token(node.leftParenthesis); - visit(node.expression); - token(node.rightParenthesis); + Piece visitParenthesizedExpression(ParenthesizedExpression node) { + return buildPiece((b) { + b.token(node.leftParenthesis); + b.visit(node.expression); + b.token(node.rightParenthesis); + }); } @override - void visitParenthesizedPattern(ParenthesizedPattern node) { + Piece visitParenthesizedPattern(ParenthesizedPattern node) { throw UnimplementedError(); } @override - void visitPartDirective(PartDirective node) { - createDirectiveMetadata(node); - token(node.partKeyword); - space(); - visit(node.uri); - token(node.semicolon); + Piece visitPartDirective(PartDirective node) { + return buildPiece((b) { + createDirectiveMetadata(node); + b.token(node.partKeyword); + b.space(); + b.visit(node.uri); + b.token(node.semicolon); + }); } @override - void visitPartOfDirective(PartOfDirective node) { - createDirectiveMetadata(node); - token(node.partKeyword); - space(); - token(node.ofKeyword); - space(); + Piece visitPartOfDirective(PartOfDirective node) { + return buildPiece((b) { + createDirectiveMetadata(node); - // Part-of may have either a name or a URI. Only one of these will be - // non-null. We visit both since visit() ignores null. - visit(node.libraryName); - visit(node.uri); - token(node.semicolon); + b.token(node.partKeyword); + b.space(); + b.token(node.ofKeyword); + b.space(); + + // Part-of may have either a name or a URI. Only one of these will be + // non-null. We visit both since visit() ignores null. + b.visit(node.libraryName); + b.visit(node.uri); + b.token(node.semicolon); + }); } @override - void visitPatternAssignment(PatternAssignment node) { + Piece visitPatternAssignment(PatternAssignment node) { throw UnimplementedError(); } @override - void visitPatternField(PatternField node) { + Piece visitPatternField(PatternField node) { throw UnimplementedError(); } @override - void visitPatternVariableDeclaration(PatternVariableDeclaration node) { + Piece visitPatternVariableDeclaration(PatternVariableDeclaration node) { throw UnimplementedError(); } @override - void visitPatternVariableDeclarationStatement( + Piece visitPatternVariableDeclarationStatement( PatternVariableDeclarationStatement node) { throw UnimplementedError(); } @override - void visitPostfixExpression(PostfixExpression node) { + Piece visitPostfixExpression(PostfixExpression node) { throw UnimplementedError(); } @override - void visitPrefixedIdentifier(PrefixedIdentifier node) { + Piece visitPrefixedIdentifier(PrefixedIdentifier node) { throw UnimplementedError(); } @override - void visitPrefixExpression(PrefixExpression node) { - token(node.operator); + Piece visitPrefixExpression(PrefixExpression node) { + return buildPiece((b) { + b.token(node.operator); - // Edge case: put a space after "-" if the operand is "-" or "--" so that - // we don't merge the operator tokens. - if (node.operand - case PrefixExpression(operator: Token(lexeme: '-' || '--'))) { - space(); - } + // Edge case: put a space after "-" if the operand is "-" or "--" so that + // we don't merge the operator tokens. + if (node.operand + case PrefixExpression(operator: Token(lexeme: '-' || '--'))) { + b.space(); + } - visit(node.operand); + b.visit(node.operand); + }); } @override - void visitPropertyAccess(PropertyAccess node) { + Piece visitPropertyAccess(PropertyAccess node) { throw UnimplementedError(); } @override - void visitRedirectingConstructorInvocation( + Piece visitRedirectingConstructorInvocation( RedirectingConstructorInvocation node) { throw UnimplementedError(); } @override - void visitRecordLiteral(RecordLiteral node) { + Piece visitRecordLiteral(RecordLiteral node) { ListStyle style; if (node.fields.length == 1 && node.fields[0] is! NamedExpression) { // Single-element records always have a trailing comma, unless the single @@ -1136,7 +1179,8 @@ class AstNodeVisitor extends ThrowingAstVisitor } else { style = const ListStyle(commas: Commas.trailing); } - createCollection( + + return createCollection( node.constKeyword, node.leftParenthesis, node.fields, @@ -1146,65 +1190,61 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitRecordPattern(RecordPattern node) { + Piece visitRecordPattern(RecordPattern node) { throw UnimplementedError(); } @override - void visitRecordTypeAnnotation(RecordTypeAnnotation node) { + Piece visitRecordTypeAnnotation(RecordTypeAnnotation node) { throw UnimplementedError(); } @override - void visitRecordTypeAnnotationNamedField( + Piece visitRecordTypeAnnotationNamedField( RecordTypeAnnotationNamedField node) { throw UnimplementedError(); } @override - void visitRecordTypeAnnotationPositionalField( + Piece visitRecordTypeAnnotationPositionalField( RecordTypeAnnotationPositionalField node) { throw UnimplementedError(); } @override - void visitRelationalPattern(RelationalPattern node) { + Piece visitRelationalPattern(RelationalPattern node) { throw UnimplementedError(); } @override - void visitRethrowExpression(RethrowExpression node) { + Piece visitRethrowExpression(RethrowExpression node) { throw UnimplementedError(); } @override - void visitRestPatternElement(RestPatternElement node) { + Piece visitRestPatternElement(RestPatternElement node) { throw UnimplementedError(); } @override - void visitReturnStatement(ReturnStatement node) { - token(node.returnKeyword); - - if (node.expression case var expression) { - space(); - visit(expression); - } - - token(node.semicolon); + Piece visitReturnStatement(ReturnStatement node) { + return buildPiece((b) { + b.token(node.returnKeyword); + b.visit(node.expression, spaceBefore: true); + b.token(node.semicolon); + }); } @override - void visitScriptTag(ScriptTag node) { + Piece visitScriptTag(ScriptTag node) { // The lexeme includes the trailing newline. Strip it off since the // formatter ensures it gets a newline after it. - pieces.writeText(node.scriptTag.lexeme.trim(), - offset: node.scriptTag.offset); + return tokenPiece(node.scriptTag, lexeme: node.scriptTag.lexeme.trim()); } @override - void visitSetOrMapLiteral(SetOrMapLiteral node) { - createCollection( + Piece visitSetOrMapLiteral(SetOrMapLiteral node) { + return createCollection( node.constKeyword, typeArguments: node.typeArguments, node.leftBracket, @@ -1214,104 +1254,98 @@ class AstNodeVisitor extends ThrowingAstVisitor } @override - void visitShowCombinator(ShowCombinator node) { - assert(false, 'Combinators are handled by createImport().'); + Piece visitShowCombinator(ShowCombinator node) { + throw UnsupportedError('Combinators are handled by createImport().'); } @override - void visitSimpleFormalParameter(SimpleFormalParameter node) { - startFormalParameter(node); - - if ((node.type, node.name) case (var type?, var name?)) { - // Have both a type and name, so allow splitting between them. - modifier(node.keyword); - visit(type); - var typePiece = pieces.split(); + Piece visitSimpleFormalParameter(SimpleFormalParameter node) { + var builder = AdjacentBuilder(this); + startFormalParameter(node, builder); + builder.modifier(node.keyword); - token(name); - var namePiece = pieces.take(); - - pieces.give(VariablePiece(typePiece, [namePiece], hasType: true)); + if ((node.type, node.name) case (var _?, var name?)) { + // Have both a type and name, so allow splitting after the type. + builder.visit(node.type); + var typePiece = builder.build(); + var namePiece = tokenPiece(name); + return VariablePiece(typePiece, [namePiece], hasType: true); } else { - // Only one of name or type so just write whichever there is. - modifier(node.keyword); - visit(node.type); - token(node.name); + // Don't have both a type and name, so just write whichever one we have. + builder.visit(node.type); + builder.token(node.name); + return builder.build(); } } @override - void visitSimpleIdentifier(SimpleIdentifier node) { - token(node.token); + Piece visitSimpleIdentifier(SimpleIdentifier node) { + return tokenPiece(node.token); } @override - void visitSimpleStringLiteral(SimpleStringLiteral node) { - token(node.literal); + Piece visitSimpleStringLiteral(SimpleStringLiteral node) { + return tokenPiece(node.literal); } @override - void visitSpreadElement(SpreadElement node) { + Piece visitSpreadElement(SpreadElement node) { throw UnimplementedError(); } @override - void visitStringInterpolation(StringInterpolation node) { + Piece visitStringInterpolation(StringInterpolation node) { throw UnimplementedError(); } @override - void visitSuperConstructorInvocation(SuperConstructorInvocation node) { + Piece visitSuperConstructorInvocation(SuperConstructorInvocation node) { throw UnimplementedError(); } @override - void visitSuperExpression(SuperExpression node) { + Piece visitSuperExpression(SuperExpression node) { throw UnimplementedError(); } @override - void visitSuperFormalParameter(SuperFormalParameter node) { + Piece visitSuperFormalParameter(SuperFormalParameter node) { throw UnimplementedError(); } @override - void visitSwitchExpression(SwitchExpression node) { + Piece visitSwitchExpression(SwitchExpression node) { + var value = startControlFlow(node.switchKeyword, node.leftParenthesis, + node.expression, node.rightParenthesis); + var list = DelimitedListBuilder(this, const ListStyle(spaceWhenUnsplit: true, splitListIfBeforeSplits: true)); - - startControlFlow(node.switchKeyword, node.leftParenthesis, node.expression, - node.rightParenthesis); - space(); - list.leftBracket(node.leftBracket); + list.leftBracket(node.leftBracket, preceding: value); for (var member in node.cases) { list.visit(member); } list.rightBracket(node.rightBracket); - pieces.give(list.build()); + return list.build(); } @override - void visitSwitchExpressionCase(SwitchExpressionCase node) { + Piece visitSwitchExpressionCase(SwitchExpressionCase node) { if (node.guardedPattern.whenClause != null) throw UnimplementedError(); - visit(node.guardedPattern.pattern); - space(); - finishAssignment(node.arrow, node.expression); + return createAssignment( + node.guardedPattern.pattern, node.arrow, node.expression); } @override - void visitSwitchStatement(SwitchStatement node) { - startControlFlow(node.switchKeyword, node.leftParenthesis, node.expression, - node.rightParenthesis); - - // Attach the ` {` after the `)` in the [ListPiece] created by - // [createSwitchValue()]. - space(); - token(node.leftBracket); - var switchPiece = pieces.split(); + Piece visitSwitchStatement(SwitchStatement node) { + var leftBracket = buildPiece((b) { + b.add(startControlFlow(node.switchKeyword, node.leftParenthesis, + node.expression, node.rightParenthesis)); + b.space(); + b.token(node.leftBracket); + }); var sequence = SequenceBuilder(this); for (var member in node.members) { @@ -1320,26 +1354,27 @@ class AstNodeVisitor extends ThrowingAstVisitor } sequence.addCommentsBefore(member.keyword); - token(member.keyword); - if (member is SwitchCase) { - space(); - visit(member.expression); - } else if (member is SwitchPatternCase) { - space(); + var casePiece = buildPiece((b) { + b.token(member.keyword); - if (member.guardedPattern.whenClause != null) { - throw UnimplementedError(); - } + if (member is SwitchCase) { + b.space(); + b.visit(member.expression); + } else if (member is SwitchPatternCase) { + if (member.guardedPattern.whenClause != null) { + throw UnimplementedError(); + } - visit(member.guardedPattern.pattern); - } else { - assert(member is SwitchDefault); - // Nothing to do. - } + b.space(); + b.visit(member.guardedPattern.pattern); + } else { + assert(member is SwitchDefault); + // Nothing to do. + } - token(member.colon); - var casePiece = pieces.split(); + b.token(member.colon); + }); // 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). @@ -1352,152 +1387,176 @@ class AstNodeVisitor extends ThrowingAstVisitor // Place any comments before the "}" inside the sequence. sequence.addCommentsBefore(node.rightBracket); + var rightBracketPiece = tokenPiece(node.rightBracket); - token(node.rightBracket); - var rightBracketPiece = pieces.take(); - - pieces.give(BlockPiece(switchPiece, sequence.build(), rightBracketPiece, - alwaysSplit: node.members.isNotEmpty)); + return BlockPiece(leftBracket, sequence.build(), rightBracketPiece, + alwaysSplit: node.members.isNotEmpty || sequence.mustSplit); } @override - void visitSymbolLiteral(SymbolLiteral node) { - token(node.poundSign); - var components = node.components; - for (var component in components) { - // The '.' separator. - if (component != components.first) token(component.previous); - token(component); - } + Piece visitSymbolLiteral(SymbolLiteral node) { + return buildPiece((b) { + b.token(node.poundSign); + var components = node.components; + for (var component in components) { + // The '.' separator. + if (component != components.first) { + b.token(component.previous!); + } + + b.token(component); + } + }); } @override - void visitThisExpression(ThisExpression node) { - token(node.thisKeyword); + Piece visitThisExpression(ThisExpression node) { + return tokenPiece(node.thisKeyword); } @override - void visitThrowExpression(ThrowExpression node) { + Piece visitThrowExpression(ThrowExpression node) { throw UnimplementedError(); } @override - void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) { - modifier(node.externalKeyword); - visit(node.variables); - token(node.semicolon); + Piece visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) { + return buildPiece((b) { + b.modifier(node.externalKeyword); + b.visit(node.variables); + b.token(node.semicolon); + }); } @override - void visitTryStatement(TryStatement node) { - createTry(node); + Piece visitTryStatement(TryStatement node) { + return createTry(node); } @override - void visitTypeArgumentList(TypeArgumentList node) { - createTypeList(node.leftBracket, node.arguments, node.rightBracket); + Piece visitTypeArgumentList(TypeArgumentList node) { + return createTypeList(node.leftBracket, node.arguments, node.rightBracket); } @override - void visitTypeParameter(TypeParameter node) { - token(node.name); - if (node.bound case var bound?) { - space(); - modifier(node.extendsKeyword); - visit(bound); - } + Piece visitTypeParameter(TypeParameter node) { + return buildPiece((b) { + b.token(node.name); + if (node.bound case var bound?) { + b.space(); + b.token(node.extendsKeyword); + b.space(); + b.visit(bound); + } + }); } @override - void visitTypeParameterList(TypeParameterList node) { - createTypeList(node.leftBracket, node.typeParameters, node.rightBracket); + Piece visitTypeParameterList(TypeParameterList node) { + return createTypeList( + node.leftBracket, node.typeParameters, node.rightBracket); } @override - void visitVariableDeclaration(VariableDeclaration node) { - token(node.name); - if ((node.equals, node.initializer) case (var equals?, var initializer?)) { - finishAssignment(equals, initializer); - } + Piece visitVariableDeclaration(VariableDeclaration node) { + throw UnsupportedError('This is handled by visitVariableDeclarationList()'); } @override - void visitVariableDeclarationList(VariableDeclarationList node) { + Piece visitVariableDeclarationList(VariableDeclarationList node) { // TODO(tall): Format metadata. if (node.metadata.isNotEmpty) throw UnimplementedError(); - modifier(node.lateKeyword); - modifier(node.keyword); + var header = buildPiece((b) { + b.modifier(node.lateKeyword); + b.modifier(node.keyword); - // TODO(tall): Test how splits inside the type annotation (like in a type - // argument list or a function type's parameter list) affect the indentation - // and splitting of the surrounding variable declaration. - visit(node.type); - var header = pieces.take(); + // TODO(tall): Test how splits inside the type annotation (like in a type + // argument list or a function type's parameter list) affect the indentation + // and splitting of the surrounding variable declaration. + b.visit(node.type); + }); var variables = []; for (var variable in node.variables) { - pieces.split(); - visit(variable); - commaAfter(variable); - variables.add(pieces.take()); + if ((variable.equals, variable.initializer) + case (var equals?, var initializer?)) { + var variablePiece = buildPiece((b) { + b.token(variable.name); + b.space(); + b.token(equals); + }); + + var initializerPiece = nodePiece(initializer, commaAfter: true); + + variables.add(AssignPiece(variablePiece, initializerPiece, + isValueDelimited: initializer.canBlockSplit)); + } else { + variables.add(tokenPiece(variable.name, commaAfter: true)); + } } - pieces.give(VariablePiece(header, variables, hasType: node.type != null)); + return VariablePiece(header, variables, hasType: node.type != null); } @override - void visitVariableDeclarationStatement(VariableDeclarationStatement node) { - visit(node.variables); - token(node.semicolon); + Piece visitVariableDeclarationStatement(VariableDeclarationStatement node) { + return buildPiece((b) { + b.visit(node.variables); + b.token(node.semicolon); + }); } @override - void visitWhileStatement(WhileStatement node) { - token(node.whileKeyword); - space(); - token(node.leftParenthesis); - visit(node.condition); - token(node.rightParenthesis); - var condition = pieces.split(); + Piece visitWhileStatement(WhileStatement node) { + var condition = startControlFlow(node.whileKeyword, node.leftParenthesis, + node.condition, node.rightParenthesis); - visit(node.body); - var body = pieces.take(); + var body = nodePiece(node.body); var piece = IfPiece(); piece.add(condition, body, isBlock: node.body is Block); - pieces.give(piece); + return piece; } @override - void visitWildcardPattern(WildcardPattern node) { + Piece visitWildcardPattern(WildcardPattern node) { throw UnimplementedError(); } @override - void visitWithClause(WithClause node) { - assert(false, 'This node is handled by PieceFactory.createType().'); + Piece visitWithClause(WithClause node) { + throw UnsupportedError( + 'This node is handled by PieceFactory.createType().'); } @override - void visitYieldStatement(YieldStatement node) { - token(node.yieldKeyword); - token(node.star); - space(); - visit(node.expression); - token(node.semicolon); + Piece visitYieldStatement(YieldStatement node) { + return buildPiece((b) { + b.token(node.yieldKeyword); + b.token(node.star); + b.space(); + b.visit(node.expression); + b.token(node.semicolon); + }); } - /// If [node] is not `null`, then visit it. + /// Visits [node] and creates a piece from it. /// - /// Invokes [before] before visiting [node], and [after] afterwards, but only - /// if [node] is present. + /// If [commaAfter] is `true`, looks for a comma token after [node] and + /// writes it to the piece as well. @override - void visit(AstNode? node, {void Function()? before, void Function()? after}) { - if (node == null) return; + Piece nodePiece(AstNode node, {bool commaAfter = false}) { + var result = node.accept(this)!; + + if (commaAfter) { + var nextToken = node.endToken.next!; + if (nextToken.lexeme == ',') { + var comma = tokenPiece(nextToken); + result = AdjacentPiece([result, comma]); + } + } - if (before != null) before(); - node.accept(this); - if (after != null) after(); + return result; } } diff --git a/lib/src/front_end/comment_writer.dart b/lib/src/front_end/comment_writer.dart index 05caa978..b7f70a77 100644 --- a/lib/src/front_end/comment_writer.dart +++ b/lib/src/front_end/comment_writer.dart @@ -7,11 +7,10 @@ import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/source/line_info.dart'; import '../comment_type.dart'; -import 'piece_writer.dart'; -/// Functionality used by [AstNodeVisitor] and [SequenceBuilder] to build text -/// and pieces from the comment tokens between meaningful tokens used by AST -/// nodes. +/// Functionality used by [AstNodeVisitor], [DelimitedListBuilder], and +/// [SequenceBuilder] to build pieces from the comment tokens between meaningful +/// tokens used by AST nodes. /// /// Also handles tracking newlines between tokens and comments so that /// information can be used to preserve discretionary blank lines in places @@ -42,61 +41,43 @@ import 'piece_writer.dart'; /// construct. These get directly embedded in the [TextPiece] of the code being /// written. When that [TextPiece] is output later, it will include the comments /// as well. -mixin CommentWriter { - PieceWriter get pieces; - - LineInfo get lineInfo; +class CommentWriter { + final LineInfo _lineInfo; /// The tokens whose preceding comments have already been taken by calls to /// [takeCommentsBefore()]. final Set _takenTokens = {}; + CommentWriter(this._lineInfo); + /// Returns the comments that appear before [token]. /// - /// The caller is required to write them because a later call to [token()] - /// for this token will not write the preceding comments. + /// The caller is required to write them because a later call to write [token] + /// for this token will not write the preceding comments. Used by + /// [SequenceBuilder] and [DelimitedListBuilder] which handle comment + /// formatting themselves. CommentSequence takeCommentsBefore(Token token) { if (_takenTokens.contains(token)) return CommentSequence.empty; _takenTokens.add(token); - return _collectComments(token); + return _commentsBefore(token); } - /// Writes comments that appear before [token]. - void writeCommentsBefore(Token token) { + /// Returns the comments that appear before [token]. + CommentSequence commentsBefore(Token token) { // In the common case where there are no comments before the token, early // out. This avoids calculating the number of newlines between every pair // of tokens which is slow and unnecessary. - if (token.precedingComments == null) return; - - // Don't write the comments if some other construct has already handled - // them. - if (_takenTokens.contains(token)) return; - - var comments = _collectComments(token); - for (var i = 0; i < comments.length; i++) { - var comment = comments[i]; - - if (comments.isHanging(i)) { - // Attach the comment to the previous token. - pieces.writeComment(comment, hanging: true); - } else { - pieces.writeNewline(); - pieces.writeComment(comment); - } + if (token.precedingComments == null) return CommentSequence.empty; - if (comment.type == CommentType.line || comment.type == CommentType.doc) { - pieces.writeNewline(); - } - } + // Don't yield the comments if some other construct already handled them. + if (_takenTokens.contains(token)) return CommentSequence.empty; - if (comments.isNotEmpty && _needsSpaceAfterComment(token.lexeme)) { - pieces.writeSpace(); - } + return _commentsBefore(token); } /// Takes all of the comment tokens preceding [token] and builds a /// [CommentSequence] that tracks them and the whitespace between them. - CommentSequence _collectComments(Token token) { + CommentSequence _commentsBefore(Token token) { var previousLine = _endLine(token.previous!); var tokenLine = _startLine(token); @@ -152,28 +133,15 @@ mixin CommentWriter { return comments; } - /// Returns `true` if a space should be output after the last comment which - /// was just written and the [token] that will be written. - bool _needsSpaceAfterComment(String token) { - // It gets a space if the following token is not a delimiter or the empty - // string (for EOF). - return token != ')' && - token != ']' && - token != '}' && - token != ',' && - token != ';' && - token != ''; - } - /// Gets the 1-based line number that the beginning of [token] lies on. - int _startLine(Token token) => lineInfo.getLocation(token.offset).lineNumber; + int _startLine(Token token) => _lineInfo.getLocation(token.offset).lineNumber; /// Gets the 1-based line number that the end of [token] lies on. - int _endLine(Token token) => lineInfo.getLocation(token.end).lineNumber; + int _endLine(Token token) => _lineInfo.getLocation(token.end).lineNumber; /// Gets the 1-based column number that the beginning of [token] lies on. int _startColumn(Token token) => - lineInfo.getLocation(token.offset).columnNumber; + _lineInfo.getLocation(token.offset).columnNumber; } /// A comment in the source, with a bit of information about the surrounding @@ -200,10 +168,9 @@ class SourceComment { SourceComment(this.text, this.type, {required this.flushLeft, required this.offset}); - /// Whether this comment contains a mandatory newline, either because it's a - /// comment that should be on its own line or is a multi-line block comment. - bool get containsNewline => - type != CommentType.inlineBlock || text.contains('\n'); + /// Whether this comment ends with a mandatory newline, because it's a line + /// comment or a block comment that should be on its own line. + bool get requiresNewline => type != CommentType.inlineBlock; @override String toString() => @@ -257,6 +224,10 @@ class CommentSequence extends ListBase { const CommentSequence._(this._linesBetween, this._comments); + /// Whether this sequence contains any comments that require a newline. + bool get requiresNewline => + _comments.any((comment) => comment.requiresNewline); + /// The number of newlines between the comment at [commentIndex] and the /// preceding comment or token. int linesBefore(int commentIndex) => _linesBetween[commentIndex]; diff --git a/lib/src/front_end/delimited_list_builder.dart b/lib/src/front_end/delimited_list_builder.dart index ca3a6fc7..68f917d9 100644 --- a/lib/src/front_end/delimited_list_builder.dart +++ b/lib/src/front_end/delimited_list_builder.dart @@ -3,8 +3,8 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; -import 'package:dart_style/src/ast_extensions.dart'; +import '../ast_extensions.dart'; import '../comment_type.dart'; import '../piece/list.dart'; import '../piece/piece.dart'; @@ -34,8 +34,13 @@ class DelimitedListBuilder { /// The closing bracket after the elements, if any. Piece? _rightBracket; + bool _mustSplit = false; + final ListStyle _style; + /// The comments that should appear before the next element. + final List _leadingComments = []; + /// The list of comments following the most recently written element before /// any comma following the element. CommentSequence _commentsBeforeComma = CommentSequence.empty; @@ -51,7 +56,8 @@ class DelimitedListBuilder { if (_style.allowBlockElement) blockElement = _findBlockElement(); return ListPiece(_leftBracket, _elements, _blanksAfter, _rightBracket, - _style, blockElement); + _style, blockElement, + mustSplit: _mustSplit); } /// Adds the opening [bracket] to the built list. @@ -65,10 +71,15 @@ class DelimitedListBuilder { /// ``` /// /// Here, [bracket] will be `(` and [delimiter] will be `[`. - void leftBracket(Token bracket, {Token? delimiter}) { - _visitor.token(bracket); - _visitor.token(delimiter); - _leftBracket = _visitor.pieces.split(); + void leftBracket(Token bracket, {Piece? preceding, Token? delimiter}) { + _leftBracket = _visitor.buildPiece((b) { + if (preceding != null) { + b.add(preceding); + b.space(); + } + b.token(bracket); + b.token(delimiter); + }); } /// Adds the closing [bracket] to the built list along with any comments that @@ -89,7 +100,7 @@ class DelimitedListBuilder { /// before the `;` are kept, but the `;` itself is discarded. void rightBracket(Token bracket, {Token? delimiter, Token? semicolon}) { // Handle comments after the last element. - var commentsBefore = _visitor.takeCommentsBefore(bracket); + var commentsBefore = _visitor.comments.takeCommentsBefore(bracket); // Merge the comments before the delimiter (if there is one) and the // bracket. If there is a delimiter, this will move comments between it and @@ -103,20 +114,23 @@ class DelimitedListBuilder { // f([parameter /* comment */]) {} // ``` if (delimiter != null) { - commentsBefore = - _visitor.takeCommentsBefore(delimiter).concatenate(commentsBefore); + commentsBefore = _visitor.comments + .takeCommentsBefore(delimiter) + .concatenate(commentsBefore); } if (semicolon != null) { - commentsBefore = - _visitor.takeCommentsBefore(semicolon).concatenate(commentsBefore); + commentsBefore = _visitor.comments + .takeCommentsBefore(semicolon) + .concatenate(commentsBefore); } _addComments(commentsBefore, hasElementAfter: false); - _visitor.token(delimiter); - _visitor.token(bracket); - _rightBracket = _visitor.pieces.take(); + _rightBracket = _visitor.buildPiece((b) { + b.token(delimiter); + b.token(bracket); + }); } /// Adds [piece] to the built list. @@ -127,14 +141,15 @@ class DelimitedListBuilder { /// /// Assumes there is no comma after this piece. void add(Piece piece, [BlockFormat format = BlockFormat.none]) { - _elements.add(ListElement(piece, format)); + _elements.add(ListElement(_leadingComments, piece, format)); + _leadingComments.clear(); _commentsBeforeComma = CommentSequence.empty; } /// Writes any comments appearing before [token] to the list. void addCommentsBefore(Token token) { // Handle comments between the preceding element and this one. - var commentsBeforeElement = _visitor.takeCommentsBefore(token); + var commentsBeforeElement = _visitor.comments.takeCommentsBefore(token); _addComments(commentsBeforeElement, hasElementAfter: true); } @@ -151,12 +166,11 @@ class DelimitedListBuilder { }; // Traverse the element itself. - _visitor.visit(element); - add(_visitor.pieces.split(), format); + add(_visitor.nodePiece(element), format); var nextToken = element.endToken.next!; if (nextToken.lexeme == ',') { - _commentsBeforeComma = _visitor.takeCommentsBefore(nextToken); + _commentsBeforeComma = _visitor.comments.takeCommentsBefore(nextToken); } } @@ -187,10 +201,10 @@ class DelimitedListBuilder { // matter that much where it goes and this seems to be simple and // reasonable looking.) _commentsBeforeComma = _commentsBeforeComma - .concatenate(_visitor.takeCommentsBefore(delimiter)); + .concatenate(_visitor.comments.takeCommentsBefore(delimiter)); // Attach the delimiter to the previous element. - _elements.last = _elements.last.withDelimiter(delimiter.lexeme); + _elements.last.setDelimiter(delimiter.lexeme); } /// Adds [comments] to the list. @@ -202,6 +216,10 @@ class DelimitedListBuilder { // Early out if there's nothing to do. if (_commentsBeforeComma.isEmpty && comments.isEmpty) return; + if (_commentsBeforeComma.requiresNewline || comments.requiresNewline) { + _mustSplit = true; + } + // Figure out which comments are anchored to the preceding element, which // are freestanding, and which are attached to the next element. var ( @@ -214,19 +232,17 @@ class DelimitedListBuilder { // Add any hanging inline block comments to the previous element before the // subsequent ",". for (var comment in inlineComments) { - _visitor.space(); - _visitor.pieces.writeComment(comment, hanging: true); + var commentPiece = _visitor.pieces.writeComment(comment); + _elements.last.addComment(commentPiece, beforeDelimiter: true); } // Add any remaining hanging line comments to the previous element after // the ",". if (hangingComments.isNotEmpty) { for (var comment in hangingComments) { - _visitor.space(); - _visitor.pieces.writeComment(comment); + var commentPiece = _visitor.pieces.writeComment(comment); + _elements.last.addComment(commentPiece); } - - _elements.last = _elements.last.withComment(_visitor.pieces.split()); } // Comments that are neither hanging nor leading are treated like their own @@ -237,14 +253,14 @@ class DelimitedListBuilder { _blanksAfter.add(_elements.last); } - _visitor.pieces.writeComment(comment); - _elements.add(ListElement.comment(_visitor.pieces.split())); + var commentPiece = _visitor.pieces.writeComment(comment); + _elements.add(ListElement.comment(commentPiece)); } // Leading comments are written before the next element. for (var comment in leadingComments) { - _visitor.pieces.writeComment(comment); - _visitor.space(); + var commentPiece = _visitor.pieces.writeComment(comment); + _leadingComments.add(commentPiece); } } diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart index 7929b4a3..81229037 100644 --- a/lib/src/front_end/piece_factory.dart +++ b/lib/src/front_end/piece_factory.dart @@ -5,7 +5,6 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; import '../ast_extensions.dart'; -import '../piece/adjacent.dart'; import '../piece/assign.dart'; import '../piece/block.dart'; import '../piece/clause.dart'; @@ -17,6 +16,7 @@ import '../piece/piece.dart'; import '../piece/postfix.dart'; import '../piece/try.dart'; import '../piece/type.dart'; +import 'adjacent_builder.dart'; import 'ast_node_visitor.dart'; import 'comment_writer.dart'; import 'delimited_list_builder.dart'; @@ -47,11 +47,15 @@ typedef BinaryOperation = (AstNode left, Token operator, AstNode right); /// word for "import or export directive" or "named thing with argument list". /// To avoid that, we pick one concrete construct formatted by the function, /// usually the most common, and name it after that, as in [createImport()]. -mixin PieceFactory implements CommentWriter { - void visit(AstNode? node, {void Function()? before, void Function()? after}); +mixin PieceFactory { + PieceWriter get pieces; + + CommentWriter get comments; + + Piece nodePiece(AstNode node, {bool commaAfter = false}); /// Creates a [ListPiece] for an argument list. - void createArgumentList( + Piece createArgumentList( Token leftBracket, Iterable elements, Token rightBracket) { return createList( leftBracket: leftBracket, @@ -71,10 +75,10 @@ mixin PieceFactory implements CommentWriter { /// if (condition) { /// } else {} /// ``` - void createBody(Token leftBracket, List contents, Token rightBracket, + Piece createBody( + Token leftBracket, List contents, Token rightBracket, {bool forceSplit = false}) { - token(leftBracket); - var leftBracketPiece = pieces.split(); + var leftBracketPiece = tokenPiece(leftBracket); var sequence = SequenceBuilder(this); for (var node in contents) { @@ -88,12 +92,10 @@ mixin PieceFactory implements CommentWriter { // Place any comments before the "}" inside the block. sequence.addCommentsBefore(rightBracket); - token(rightBracket); - var rightBracketPiece = pieces.take(); + var rightBracketPiece = tokenPiece(rightBracket); - pieces.give(BlockPiece( - leftBracketPiece, sequence.build(), rightBracketPiece, - alwaysSplit: forceSplit || contents.isNotEmpty)); + return BlockPiece(leftBracketPiece, sequence.build(), rightBracketPiece, + alwaysSplit: forceSplit || contents.isNotEmpty || sequence.mustSplit); } /// Creates a [BlockPiece] for a given [Block]. @@ -106,59 +108,62 @@ mixin PieceFactory implements CommentWriter { /// if (condition) { /// } else {} /// ``` - void createBlock(Block block, {bool forceSplit = false}) { - createBody(block.leftBracket, block.statements, block.rightBracket, + Piece createBlock(Block block, {bool forceSplit = false}) { + return createBody(block.leftBracket, block.statements, block.rightBracket, forceSplit: forceSplit); } /// Creates a piece for a `break` or `continue` statement. - void createBreak(Token keyword, SimpleIdentifier? label, Token semicolon) { - token(keyword); - if (label != null) { - space(); - visit(label); - } - token(semicolon); + Piece createBreak(Token keyword, SimpleIdentifier? label, Token semicolon) { + return buildPiece((b) { + b.token(keyword); + b.visit(label, spaceBefore: true); + b.token(semicolon); + }); } /// Creates a [ListPiece] for a collection literal. - void createCollection(Token? constKeyword, Token leftBracket, + Piece createCollection(Token? constKeyword, Token leftBracket, List elements, Token rightBracket, {TypeArgumentList? typeArguments, ListStyle style = const ListStyle()}) { - modifier(constKeyword); - visit(typeArguments); - - // TODO(tall): Support a line comment inside a collection literal as a - // signal to preserve internal newlines. So if you have: - // - // ``` - // var list = [ - // 1, 2, 3, // comment - // 4, 5, 6, - // ]; - // ``` - // - // The formatter will preserve the newline after element 3 and the lack of - // them after the other elements. - - createList( - leftBracket: leftBracket, - elements, - rightBracket: rightBracket, - style: style, - ); + return buildPiece((b) { + b.modifier(constKeyword); + b.visit(typeArguments); + + // TODO(tall): Support a line comment inside a collection literal as a + // signal to preserve internal newlines. So if you have: + // + // ``` + // var list = [ + // 1, 2, 3, // comment + // 4, 5, 6, + // ]; + // ``` + // + // The formatter will preserve the newline after element 3 and the lack of + // them after the other elements. + + b.add(createList( + leftBracket: leftBracket, + elements, + rightBracket: rightBracket, + style: style, + )); + }); } /// Visits the leading keyword and parenthesized expression at the beginning /// of an `if`, `while`, or `switch` expression or statement. - void startControlFlow(Token keyword, Token leftParenthesis, Expression value, + Piece startControlFlow(Token keyword, Token leftParenthesis, Expression value, Token rightParenthesis) { // Attach the keyword to the `(`. - token(keyword); - space(); - token(leftParenthesis); - visit(value); - token(rightParenthesis); + return buildPiece((b) { + b.token(keyword); + b.space(); + b.token(leftParenthesis); + b.visit(value); + b.token(rightParenthesis); + }); } /// Creates metadata annotations for a directive. @@ -170,15 +175,48 @@ mixin PieceFactory implements CommentWriter { } /// Creates a dotted or qualified identifier. - void createDotted(NodeList components) { - for (var component in components) { - // Write the preceding ".". - if (component != components.first) { - token(component.beginToken.previous); + Piece createDotted(NodeList components) { + return buildPiece((b) { + for (var component in components) { + // Write the preceding ".". + if (component != components.first) { + b.token(component.beginToken.previous!); + } + + b.visit(component); } + }); + } - visit(component); - } + /// Creates a [Piece] for an enum constant. + /// + /// If the constant is in an enum declaration that also declares members, then + /// [hasMembers] should be `true`, [semicolon] is the `;` token before the + /// members (if any), and [isLastConstant] is `true` if [node] is the last + /// constant before the members. + Piece createEnumConstant(EnumConstantDeclaration node, + {bool hasMembers = false, + bool isLastConstant = false, + Token? semicolon}) { + return buildPiece((b) { + b.token(node.name); + if (node.arguments case var arguments?) { + b.visit(arguments.typeArguments); + b.visit(arguments.argumentList); + } + + if (hasMembers) { + if (!isLastConstant) { + b.token(node.commaAfter); + } else { + // Discard the trailing comma if there is one since there is a + // semicolon to use as the separator, but preserve any comments before + // the discarded comma. + b.commentsBefore(node.commaAfter); + b.token(semicolon); + } + } + }); } /// Creates a function, method, getter, or setter declaration. @@ -188,106 +226,105 @@ mixin PieceFactory implements CommentWriter { /// should be the `operator` keyword on an operator declaration. If /// [propertyKeyword] is given, it should be the `get` or `set` keyword on a /// getter or setter declaration. - void createFunction( + Piece createFunction( {Token? externalKeyword, Token? modifierKeyword, AstNode? returnType, Token? operatorKeyword, Token? propertyKeyword, - required Token name, + Token? name, TypeParameterList? typeParameters, FormalParameterList? parameters, required FunctionBody body}) { - modifier(externalKeyword); - modifier(modifierKeyword); + var builder = AdjacentBuilder(this); + builder.modifier(externalKeyword); + builder.modifier(modifierKeyword); Piece? returnTypePiece; if (returnType != null) { - visit(returnType); - returnTypePiece = pieces.split(); + builder.visit(returnType); + returnTypePiece = builder.build(); } - modifier(operatorKeyword); - modifier(propertyKeyword); - token(name); + builder.modifier(operatorKeyword); + builder.modifier(propertyKeyword); + builder.token(name); + builder.visit(typeParameters); + builder.visit(parameters); + var signature = builder.build(); + + var bodyPiece = nodePiece(body); - finishFunction(returnTypePiece, typeParameters, parameters, body); + return FunctionPiece(returnTypePiece, signature, + body: bodyPiece, spaceBeforeBody: body is! EmptyFunctionBody); } /// Creates a function type or function-typed formal. - void createFunctionType( + /// + /// If creating a piece for a function-typed formal, then [parameter] is the + /// formal parameter. + Piece createFunctionType( TypeAnnotation? returnType, - Token? functionKeywordOrName, + Token functionKeywordOrName, TypeParameterList? typeParameters, FormalParameterList parameters, - Token? question) { + Token? question, + {FunctionTypedFormalParameter? parameter}) { + var builder = AdjacentBuilder(this); + + if (parameter != null) startFormalParameter(parameter, builder); + Piece? returnTypePiece; if (returnType != null) { - visit(returnType); - returnTypePiece = pieces.split(); + builder.visit(returnType); + returnTypePiece = builder.build(); } - token(functionKeywordOrName); - visit(typeParameters); - visit(parameters); - token(question); - var parametersPiece = pieces.take(); + builder.token(functionKeywordOrName); + builder.visit(typeParameters); + builder.visit(parameters); + builder.token(question); - pieces.give(FunctionPiece(returnTypePiece, parametersPiece)); + return FunctionPiece(returnTypePiece, builder.build()); } /// Creates a [TryPiece] for try statement. - void createTry(TryStatement tryStatement) { + Piece createTry(TryStatement tryStatement) { var piece = TryPiece(); - token(tryStatement.tryKeyword); - var tryHeader = pieces.split(); - createBlock(tryStatement.body); - var tryBlock = pieces.split(); + var tryHeader = tokenPiece(tryStatement.tryKeyword); + var tryBlock = createBlock(tryStatement.body); piece.add(tryHeader, tryBlock); for (var i = 0; i < tryStatement.catchClauses.length; i++) { var catchClause = tryStatement.catchClauses[i]; - Piece? onPiece; - if (catchClause.onKeyword case var onKeyword?) { - token(onKeyword, after: space); - visit(catchClause.exceptionType); - onPiece = pieces.split(); - } - - Piece? catchPiece; - if (catchClause.catchKeyword case var catchKeyword?) { - token(catchKeyword); - var catchKeywordPiece = pieces.split(); - - var builder = DelimitedListBuilder(this); - builder.leftBracket(catchClause.leftParenthesis!); - if (catchClause.exceptionParameter case var exceptionParameter?) { - builder.visit(exceptionParameter); + var catchClauseHeader = buildPiece((b) { + if (catchClause.onKeyword case var onKeyword?) { + b.token(onKeyword, spaceAfter: true); + b.visit(catchClause.exceptionType); } - if (catchClause.stackTraceParameter case var stackTraceParameter?) { - builder.visit(stackTraceParameter); - } - builder.rightBracket(catchClause.rightParenthesis!); - catchPiece = AdjacentPiece( - [catchKeywordPiece, builder.build()], - spaceAfter: [catchKeywordPiece], - ); - } + if (catchClause.onKeyword != null && catchClause.catchKeyword != null) { + b.space(); + } - if (onPiece != null && catchPiece != null) { - pieces.give(AdjacentPiece( - [onPiece, catchPiece], - spaceAfter: [onPiece], - )); - } else if (onPiece != null) { - pieces.give(onPiece); - } else if (catchPiece != null) { - pieces.give(catchPiece); - } - var catchClauseHeader = pieces.split(); + if (catchClause.catchKeyword case var catchKeyword?) { + b.token(catchKeyword); + b.space(); + + var parameters = DelimitedListBuilder(this); + parameters.leftBracket(catchClause.leftParenthesis!); + if (catchClause.exceptionParameter case var exceptionParameter?) { + parameters.visit(exceptionParameter); + } + if (catchClause.stackTraceParameter case var stackTraceParameter?) { + parameters.visit(stackTraceParameter); + } + parameters.rightBracket(catchClause.rightParenthesis!); + b.add(parameters.build()); + } + }); // Edge case: When there's another catch/on/finally after this one, we // want to force the block to split even if it's empty. @@ -302,37 +339,33 @@ mixin PieceFactory implements CommentWriter { // ``` var forceSplit = i < tryStatement.catchClauses.length - 1 || tryStatement.finallyBlock != null; - createBlock( - catchClause.body, - forceSplit: forceSplit, - ); - var catchClauseBody = pieces.split(); - + var catchClauseBody = + createBlock(catchClause.body, forceSplit: forceSplit); piece.add(catchClauseHeader, catchClauseBody); } if (tryStatement.finallyBlock case var finallyBlock?) { - token(tryStatement.finallyKeyword); - var finallyHeader = pieces.split(); - createBlock(finallyBlock); - var finallyBody = pieces.split(); + var finallyHeader = tokenPiece(tryStatement.finallyKeyword!); + var finallyBody = createBlock(finallyBlock); piece.add(finallyHeader, finallyBody); } - pieces.give(piece); + return piece; } // TODO(tall): Generalize this to work with if elements too. /// Creates a piece for a chain of if-else-if... statements. - void createIf(IfStatement ifStatement) { + Piece createIf(IfStatement ifStatement) { var piece = IfPiece(); // Recurses through the else branches to flatten them into a linear if-else // chain handled by a single [IfPiece]. - void traverse(IfStatement node) { - startControlFlow(node.ifKeyword, node.leftParenthesis, node.expression, - node.rightParenthesis); - var condition = pieces.split(); + void traverse(Piece? previousElse, IfStatement node) { + var condition = buildPiece((b) { + if (previousElse != null) b.add(previousElse); + b.add(startControlFlow(node.ifKeyword, node.leftParenthesis, + node.expression, node.rightParenthesis)); + }); // Edge case: When the then branch is a block and there is an else clause // after it, we want to force the block to split even if empty, like: @@ -343,86 +376,76 @@ mixin PieceFactory implements CommentWriter { // body; // } // ``` - if (node.thenStatement case Block thenBlock - when node.elseStatement != null) { - createBlock(thenBlock, forceSplit: true); - } else { - visit(node.thenStatement); - } + var thenStatement = switch (node.thenStatement) { + Block thenBlock when node.elseStatement != null => + createBlock(thenBlock, forceSplit: true), + _ => nodePiece(node.thenStatement) + }; - var thenStatement = pieces.split(); piece.add(condition, thenStatement, isBlock: node.thenStatement is Block); switch (node.elseStatement) { case IfStatement elseIf: // Hit an else-if, so flatten it into the chain with the `else` // becoming part of the next section's header. - token(node.elseKeyword); - space(); - traverse(elseIf); + traverse(buildPiece((b) { + b.token(node.elseKeyword); + b.space(); + }), elseIf); case var elseStatement?: // Any other kind of else body ends the chain, with the header for // the last section just being the `else` keyword. - token(node.elseKeyword); - var header = pieces.split(); - - visit(elseStatement); - var statement = pieces.take(); + var header = tokenPiece(node.elseKeyword!); + var statement = nodePiece(elseStatement); piece.add(header, statement, isBlock: elseStatement is Block); } } - traverse(ifStatement); - - pieces.give(piece); + traverse(null, ifStatement); + return piece; } /// Creates an [ImportPiece] for an import or export directive. - void createImport(NamespaceDirective directive, Token keyword, + Piece createImport(NamespaceDirective directive, Token keyword, {Token? deferredKeyword, Token? asKeyword, SimpleIdentifier? prefix}) { + var builder = AdjacentBuilder(this); createDirectiveMetadata(directive); - token(keyword); - space(); - visit(directive.uri); - var importPieces = [pieces.take()]; + builder.token(keyword); + builder.space(); + builder.visit(directive.uri); if (directive.configurations.isNotEmpty) { var configurations = []; for (var configuration in directive.configurations) { - pieces.split(); - visit(configuration); - configurations.add(pieces.take()); + configurations.add(nodePiece(configuration)); } - importPieces.add(PostfixPiece(configurations)); + builder.add(PostfixPiece(configurations)); } if (asKeyword != null) { - pieces.split(); - token(deferredKeyword, after: space); - token(asKeyword); - space(); - visit(prefix); - importPieces.add(PostfixPiece([pieces.take()])); + builder.add(PostfixPiece([ + buildPiece((b) { + b.token(deferredKeyword, spaceAfter: true); + b.token(asKeyword); + b.space(); + b.visit(prefix!); + }) + ])); } if (directive.combinators.isNotEmpty) { var combinators = []; for (var combinatorNode in directive.combinators) { - pieces.split(); - token(combinatorNode.keyword); - var combinatorKeyword = pieces.split(); + var combinatorKeyword = tokenPiece(combinatorNode.keyword); switch (combinatorNode) { case HideCombinator(hiddenNames: var names): case ShowCombinator(shownNames: var names): var parts = []; for (var name in names) { - pieces.split(); - token(name.token); - commaAfter(name); - parts.add(pieces.take()); + parts.add(tokenPiece(name.token, commaAfter: true)); } var combinator = ClausePiece(combinatorKeyword, parts); @@ -432,12 +455,11 @@ mixin PieceFactory implements CommentWriter { } } - importPieces.add(ClausesPiece(combinators)); + builder.add(ClausesPiece(combinators)); } - token(directive.semicolon); - - pieces.give(AdjacentPiece(importPieces)); + builder.token(directive.semicolon); + return builder.build(); } /// Creates a single infix operation. @@ -448,27 +470,28 @@ mixin PieceFactory implements CommentWriter { /// /// The [operator2] parameter may be passed if the "operator" is actually two /// separate tokens, as in `foo is! Bar`. - void createInfix(AstNode left, Token operator, AstNode right, + Piece createInfix(AstNode left, Token operator, AstNode right, {bool hanging = false, Token? operator2}) { - var operands = []; + var leftPiece = buildPiece((b) { + b.visit(left); + if (hanging) { + b.space(); + b.token(operator); + b.token(operator2); + } + }); - visit(left); + var rightPiece = buildPiece((b) { + if (!hanging) { + b.token(operator); + b.token(operator2); + b.space(); + } - if (hanging) { - space(); - token(operator); - token(operator2); - operands.add(pieces.split()); - } else { - operands.add(pieces.split()); - token(operator); - token(operator2); - space(); - } + b.visit(right); + }); - visit(right); - operands.add(pieces.take()); - pieces.give(InfixPiece(operands)); + return InfixPiece([leftPiece, rightPiece]); } /// Creates a chained infix operation: a binary operator expression, or @@ -486,9 +509,10 @@ mixin PieceFactory implements CommentWriter { /// /// If [precedence] is given, then this only flattens binary nodes with that /// same precedence. - void createInfixChain( + Piece createInfixChain( T node, BinaryOperation Function(T node) destructure, {int? precedence}) { + var builder = AdjacentBuilder(this); var operands = []; void traverse(AstNode e) { @@ -498,26 +522,26 @@ mixin PieceFactory implements CommentWriter { var (left, operator, right) = destructure(e); if (precedence == null || operator.type.precedence == precedence) { traverse(left); - space(); - token(operator); - pieces.split(); + builder.space(); + builder.token(operator); + operands.add(builder.build()); traverse(right); return; } } // Otherwise, just write the node itself. - visit(e); - operands.add(pieces.take()); + builder.visit(e); } traverse(node); + operands.add(builder.build()); - pieces.give(InfixPiece(operands)); + return InfixPiece(operands); } /// Creates a [ListPiece] for the given bracket-delimited set of elements. - void createList(Iterable elements, + Piece createList(Iterable elements, {Token? leftBracket, Token? rightBracket, ListStyle style = const ListStyle()}) { @@ -525,7 +549,7 @@ mixin PieceFactory implements CommentWriter { if (leftBracket != null) builder.leftBracket(leftBracket); elements.forEach(builder.visit); if (rightBracket != null) builder.rightBracket(rightBracket); - pieces.give(builder.build()); + return builder.build(); } /// Creates a class, enum, extension, mixin, or mixin application class @@ -540,7 +564,7 @@ mixin PieceFactory implements CommentWriter { /// /// If the type is an extension, then [onType] is a record containing the /// `on` keyword and the on type. - void createType(NodeList metadata, List modifiers, + Piece createType(NodeList metadata, List modifiers, Token keyword, Token? name, {TypeParameterList? typeParameters, Token? equals, @@ -555,33 +579,33 @@ mixin PieceFactory implements CommentWriter { Token? semicolon}) { if (metadata.isNotEmpty) throw UnimplementedError('Type metadata.'); - modifiers.forEach(modifier); - token(keyword); - token(name, before: space); - visit(typeParameters); - - // Mixin application classes have ` = Superclass` after the declaration - // name. - if (equals != null) { - space(); - token(equals); - space(); - visit(superclass); - } + var header = buildPiece((b) { + modifiers.forEach(b.modifier); + b.token(keyword); + b.token(name, spaceBefore: true); - var header = pieces.split(); + if (typeParameters != null) { + b.visit(typeParameters); + } + + // Mixin application classes have ` = Superclass` after the declaration + // name. + if (equals != null) { + b.space(); + b.token(equals); + b.space(); + b.visit(superclass!); + } + }); var clauses = []; void typeClause(Token keyword, List types) { - token(keyword); - var keywordPiece = pieces.split(); + var keywordPiece = tokenPiece(keyword); var typePieces = []; for (var type in types) { - visit(type); - commaAfter(type); - typePieces.add(pieces.split()); + typePieces.add(nodePiece(type, commaAfter: true)); } clauses.add(ClausePiece(keywordPiece, typePieces)); @@ -608,28 +632,29 @@ mixin PieceFactory implements CommentWriter { typeClause(onKeyword, [onType]); } + if (nativeClause != null) { + typeClause(nativeClause.nativeKeyword, + [if (nativeClause.name case var name?) name]); + } + ClausesPiece? clausesPiece; if (clauses.isNotEmpty) { clausesPiece = ClausesPiece(clauses, allowLeadingClause: extendsClause != null || onClause != null); } - visit(nativeClause); - space(); - + Piece bodyPiece; if (body != null) { - createBody(body.leftBracket, body.members, body.rightBracket); + bodyPiece = createBody(body.leftBracket, body.members, body.rightBracket); } else { - token(semicolon); + bodyPiece = tokenPiece(semicolon!); } - var bodyPiece = pieces.take(); - pieces.give( - TypePiece(header, clausesPiece, bodyPiece, hasBody: body != null)); + return TypePiece(header, clausesPiece, bodyPiece, hasBody: body != null); } /// Creates a [ListPiece] for a type argument or type parameter list. - void createTypeList( + Piece createTypeList( Token leftBracket, Iterable elements, Token rightBracket) { return createList( leftBracket: leftBracket, @@ -640,19 +665,20 @@ mixin PieceFactory implements CommentWriter { /// Writes the parts of a formal parameter shared by all formal parameter /// types: metadata, `covariant`, etc. - void startFormalParameter(FormalParameter parameter) { + void startFormalParameter( + FormalParameter parameter, AdjacentBuilder builder) { if (parameter.metadata.isNotEmpty) throw UnimplementedError(); - modifier(parameter.requiredKeyword); - modifier(parameter.covariantKeyword); + builder.modifier(parameter.requiredKeyword); + builder.modifier(parameter.covariantKeyword); } /// Handles the `async`, `sync*`, or `async*` modifiers on a function body. - void functionBodyModifiers(FunctionBody body) { + void functionBodyModifiers(FunctionBody body, AdjacentBuilder builder) { // The `async` or `sync` keyword. - token(body.keyword); - token(body.star); - if (body.keyword != null) space(); + builder.token(body.keyword); + builder.token(body.star); + if (body.keyword != null) builder.space(); } /// Creates a [Piece] with "assignment-like" splitting. @@ -669,92 +695,53 @@ mixin PieceFactory implements CommentWriter { /// * Map entry (`:`) /// * For-in loop iterator (`in`) /// - /// This method assumes the code to the left of the operator has already - /// been visited. - /// /// If [splitBeforeOperator] is `true`, then puts [operator] at the beginning /// of the next line when it splits. Otherwise, puts the operator at the end /// of the preceding line. - void finishAssignment(Token operator, Expression rightHandSide, - {bool splitBeforeOperator = false}) { - Piece target; + Piece createAssignment( + AstNode target, Token operator, Expression rightHandSide, + {bool splitBeforeOperator = false, + bool includeComma = false, + spaceBeforeOperator = true}) { if (splitBeforeOperator) { - target = pieces.split(); - token(operator); - space(); - } else { - if (operator.type == TokenType.EQ) space(); - token(operator); - target = pieces.split(); - } - - visit(rightHandSide); + var targetPiece = nodePiece(target); - var initializer = pieces.take(); - pieces.give(AssignPiece(target, initializer, - isValueDelimited: rightHandSide.canBlockSplit)); - } + var initializer = buildPiece((b) { + b.token(operator); + b.space(); + b.visit(rightHandSide, commaAfter: includeComma); + }); - /// Finishes writing a named function declaration or anonymous function - /// expression after the return type (if any) and name (if any) has been - /// written. - void finishFunction(Piece? returnType, TypeParameterList? typeParameters, - FormalParameterList? parameters, FunctionBody body) { - visit(typeParameters); - visit(parameters); - - Piece parametersPiece; - Piece? bodyPiece; - if (body is EmptyFunctionBody) { - // If the body is just `;`, then don't allow a space or split before the - // semicolon by making it part of the parameters piece. - token(body.semicolon); - parametersPiece = pieces.split(); + return AssignPiece(targetPiece, initializer, + isValueDelimited: rightHandSide.canBlockSplit); } else { - parametersPiece = pieces.split(); - visit(body); - bodyPiece = pieces.take(); - } + var targetPiece = buildPiece((b) { + b.visit(target); + b.token(operator, spaceBefore: spaceBeforeOperator); + }); - pieces.give(FunctionPiece(returnType, parametersPiece, bodyPiece)); - } + var initializer = nodePiece(rightHandSide, commaAfter: includeComma); - /// Writes an optional modifier that precedes other code. - void modifier(Token? keyword) { - token(keyword, after: space); + return AssignPiece(targetPiece, initializer, + isValueDelimited: rightHandSide.canBlockSplit); + } } - /// Write a single space. - void space() { - pieces.writeSpace(); + /// Invokes [buildCallback] with a new [AdjacentBuilder] and returns the + /// built result. + Piece buildPiece(Function(AdjacentBuilder) buildCallback) { + var builder = AdjacentBuilder(this); + buildCallback(builder); + return builder.build(); } - /// Emit [token], along with any comments and formatted whitespace that comes - /// before it. + /// Creates a piece for only [token]. /// - /// Does nothing if [token] is `null`. If [before] is given, it will be - /// executed before the token is outout. Likewise, [after] will be called - /// after the token is output. - void token(Token? token, {void Function()? before, void Function()? after}) { - if (token == null) return; - - writeCommentsBefore(token); - - if (before != null) before(); - pieces.writeToken(token); - if (after != null) after(); - } - - /// Writes a comma after [node], if there is one. - void commaAfter(AstNode node, {bool trailing = false}) { - var nextToken = node.endToken.next!; - if (nextToken.lexeme == ',') { - token(nextToken); - } else if (trailing) { - // If there isn't a comma there, it must be a place where a trailing - // comma can appear, so synthesize it. During formatting, we will decide - // whether to include it. - pieces.writeText(','); - } + /// If [lexeme] is given, uses that for the token's lexeme instead of its own. + /// + /// If [commaAfter] is `true`, will look for and write a comma following the + /// token if there is one. + Piece tokenPiece(Token token, {String? lexeme, bool commaAfter = false}) { + return pieces.tokenPiece(token, lexeme: lexeme, commaAfter: commaAfter); } } diff --git a/lib/src/front_end/piece_writer.dart b/lib/src/front_end/piece_writer.dart index 21ff6e9e..93e0f3d7 100644 --- a/lib/src/front_end/piece_writer.dart +++ b/lib/src/front_end/piece_writer.dart @@ -6,126 +6,24 @@ import 'package:analyzer/dart/ast/token.dart'; import '../back_end/solver.dart'; import '../dart_formatter.dart'; import '../debug.dart' as debug; +import '../piece/adjacent.dart'; import '../piece/piece.dart'; import '../source_code.dart'; import 'comment_writer.dart'; -/// Incrementally builds [Piece]s while visiting AST nodes. +/// Builds [TextPiece]s for [Token]s and comments. /// -/// The nodes in the piece tree don't always map precisely to AST nodes. For -/// example, in: -/// -/// ``` -/// a + b; -/// ``` -/// -/// The AST structure is like: -/// -/// ``` -/// ExpressionStatement -/// BinaryExpression -/// SimpleIdentifier("a") -/// Token("+") -/// SimpleIdentifier("b") -/// ``` -/// -/// But the resulting piece tree looks like: -/// -/// ``` -/// Infix -/// TextPiece("a +") -/// TextPiece("b;") -/// ``` -/// -/// Note how the infix operator is attached to the preceding piece (which -/// happens to just be an identifier but could be a more complex piece if the -/// left operand was a nested expression). Notice also that there is no piece -/// for the expression statement and, instead, the `;` is just appended to the -/// trailing TextPiece which may be deeply nested inside the binary expression. -/// -/// This class implements that "slippage" between the two representations. It -/// has mutable state to allow incrementally building up pieces while traversing -/// the source AST nodes. -/// -/// To visit an AST node and translate it to pieces, call [token()] and -/// [visit()] to process the individual tokens and subnodes of the current -/// node. Those will ultimately bottom out on calls to [write()], which appends -/// literal text to the current [TextPiece] being written. -/// -/// Those [TextPiece]s are aggregated into a tree of composite pieces which -/// break the code into separate sections for line splitting. The main API for -/// composing those pieces is [split()], [give()], and [take()]. -/// -/// Here is a simplified example of how they work: -/// -/// ``` -/// visitIfStatement(IfStatement node) { -/// // No split() here. The caller may have code they want to prepend to the -/// // first piece in this one. -/// visit(node.condition); -/// -/// // Call split() because we may want to split between the condition and -/// // then branches and we know there will be a then branch. -/// var conditionPiece = pieces.split(); -/// -/// visit(node.thenBranch); -/// // Call take() instead of split() because there may not be an else branch. -/// // If there isn't, then the thenBranch will be the trailing piece created -/// // by this function and we want to allow the caller to append to its -/// // innermost TextPiece. -/// var thenPiece = pieces.take(); -/// -/// Piece? elsePiece; -/// if (node.elseBranch case var elseBranch?) { -/// // Call split() here because it turns out we do have something after -/// // the thenPiece and we want to be able to split between the then and -/// // else parts. -/// pieces.split(); -/// visit(elseBranch); -/// -/// // Use take() to capture the else branch while allowing the caller to -/// // append more code to it. -/// elsePiece = pieces.take(); -/// } -/// -/// // Create a new aggregate piece out of the subpieces and allow the caller -/// // to get it. -/// pieces.give(IfPiece(conditionPiece, thenPiece, elsePiece)); -/// } -/// ``` -/// -/// The basic rules are: -/// -/// - Use [split()] to insert a point where a line break can occur and -/// capture the piece for the code you've just written. You'll usually call -/// this when you have already traversed some part of an AST node and have -/// more to traverse after it. -/// -/// - Use [take()] to capture the current piece while allowing further code to -/// be appended to it. You'll usually call this to grab the last part of an -/// AST node where there is no more subsequent code. -/// -/// - Use [give()] to return the newly created aggregate piece so that the -/// caller can capture it with a later call to [split()] or [take()]. +/// Handles updating selection markers and attaching comments to the tokens +/// before and after the comments. class PieceWriter { final DartFormatter _formatter; final SourceCode _source; - /// The current [TextPiece] being written to or `null` if no text piece has - /// been started yet. - TextPiece? get currentText => _currentText; - TextPiece? _currentText; - - /// The most recently given piece, waiting to be taken by some surrounding - /// piece. - Piece? _given; + final CommentWriter _comments; - /// Whether we should write a space before the next text that is written. - bool _pendingSpace = false; - - /// Whether we should create a new [TextPiece] the next time text is written. - bool _pendingSplit = false; + /// The current [TextPiece] being written to. + TextPiece _currentText = TextPiece(); /// Whether we have reached a token or comment that lies at or beyond the /// selection start offset in the original code. @@ -147,136 +45,176 @@ class PieceWriter { /// This can only be accessed if there is a selection. late final int _selectionEnd = _findSelectionEnd(); - PieceWriter(this._formatter, this._source); - - /// Gives the builder a newly completed [piece], to be taken by a later call - /// to [take()] or [split()] from some surrounding piece. - void give(Piece piece) { - // Any previously given piece should already be taken (and used as a child - // of [piece]). - assert(_given == null); - _given = piece; - } + PieceWriter(this._formatter, this._source, this._comments); - /// Yields the most recent piece. + /// Creates a piece for [token], including any comments that should be + /// attached to that token. /// - /// If a completed piece was added through a call to [give()], then returns - /// that piece. A specific given piece will only be returned once from either - /// a call to [take()] or [split()]. + /// If [lexeme] is given, uses that for the token's lexeme instead of its own. /// - /// If there is no given piece to return, returns the most recently created - /// [TextPiece]. In this case, it still allows more text to be written to - /// that piece. For example, in: - /// - /// ``` - /// a + b; - /// ``` - /// - /// The code for the infix expression will call [take()] to capture the second - /// `b` operand. Then the surrounding code for the expression statement will - /// call [token()] for the `;`, which will correctly append it to the - /// [TextPiece] for `b`. - Piece take() { - if (_given case var piece?) { - _given = null; - return piece; + /// If [commaAfter] is `true`, will look for and write a comma following the + /// token if there is one. + Piece tokenPiece(Token token, {String? lexeme, bool commaAfter = false}) { + _writeToken(token, lexeme: lexeme); + var tokenPiece = _currentText; + + if (commaAfter) { + var nextToken = token.next!; + if (nextToken.lexeme == ',') { + _writeToken(nextToken); + return AdjacentPiece([tokenPiece, _currentText]); + } } - return _currentText!; + return tokenPiece; } - /// Takes the most recent piece and begins a new one. - /// - /// Any text written after this will go into a new [TextPiece] instead of - /// being appended to the end of the taken one. Call this wherever a line - /// break may be inserted by a piece during line splitting. - Piece split() { - _pendingSplit = true; - return take(); - } + // TODO(tall): Much of the comment handling code in CommentWriter got moved + // into here, so there isn't great separation of concerns anymore. Can we + // organize this code better? Or just combine CommentWriter with this class + // completely? - /// Writes raw [text] to the current innermost [TextPiece]. Starts a new - /// one if needed. + /// Writes any comments before [token]. + /// + /// Used to ensure comments before a token which will be discarded aren't + /// lost. /// - /// If [offset] is given, it should be the number of code points preceding - /// this [text] in the original source code. - void writeText(String text, {int? offset}) { - _write(text, offset: offset); + /// If there are any comments before [token] that should end up in their own + /// piece, returns a piece for them. + Piece? writeCommentsBefore(Token token) { + // If we created a new piece while writing the comments, make sure it + // doesn't get lost. + if (_writeCommentsBefore(token)) return _currentText; + + // Otherwise, there are no comments, or all comments are hanging off the + // previous TextPiece. + return null; } - /// Writes the text of [token] to the current innermost [TextPiece], tracking - /// any selection markers that may appear in it. - void writeToken(Token token) { - _write(token.lexeme, offset: token.offset); - } + /// Writes [comment] to a new [Piece] and returns it. + Piece writeComment(SourceComment comment) { + _currentText = TextPiece(); - /// Writes a space to the current [TextPiece]. - void writeSpace() { - _pendingSpace = true; + _write(comment.text, + offset: comment.offset, containsNewline: comment.text.contains('\n')); + return _currentText; } - /// Writes a mandatory newline from a comment to the current [TextPiece]. - void writeNewline() { - _currentText!.newline(); + /// Writes all of the comments that appear between [token] and the previous + /// one. + /// + /// Any hanging comments will be written to the current [TextPiece] for the + /// previous token. Remaining comments are written to a new [TextPiece]. + /// Returns `true` if it created a new [TextPiece]. + bool _writeCommentsBefore(Token token) { + var comments = _comments.commentsBefore(token); + if (comments.isEmpty) return false; + + var createdPiece = false; + + for (var i = 0; i < comments.length; i++) { + var comment = comments[i]; + + // The whitespace between the previous code or comment and this one. + if (comments.isHanging(i)) { + // Write a space before hanging comments. + _currentText.space(); + } else if (!createdPiece) { + // The previous piece must end in a newline before this comment. + _currentText.newline(); + + // Only split once between the last hanging comment and the remaining + // non-hanging ones. Otherwise, we would end up dropping comment pieces + // on the floor. So given: + // + // ``` + // before + // one + // // two + // // three + // // four + // after; + // ``` + // + // The pieces are: + // + // - `before + // one` + // - `// two¬// three¬// four¬after` + // - `;` + _currentText = TextPiece(); + createdPiece = true; + } else { + // There are multiple comments before the token that each need to be on + // their own lines, so split between the previous one and this one. + _currentText.newline(); + } + + _write(comment.text, + offset: comment.offset, containsNewline: comment.text.contains('\n')); + } + + // Output a trailing newline after the last comment if it needs one. + if (comments.last.requiresNewline) { + _currentText.newline(); + } else if (_needsSpaceAfterComment(token.lexeme)) { + _currentText.space(); + } + + return createdPiece; } - /// Write the contents of [comment] to the current innermost [TextPiece], - /// handling any newlines that may appear in it. - /// - /// If [hanging] is `true`, then the comment is appended to the current line - /// even if a call to [split()] has happened. This is used for writing a - /// comment that should be on the end of a line. - void writeComment(SourceComment comment, {bool hanging = false}) { - _write(comment.text, - offset: comment.offset, - containsNewline: comment.containsNewline, - hanging: hanging); + /// Returns `true` if a space should be output after an inline comment + /// which is followed by [lexeme]. + bool _needsSpaceAfterComment(String lexeme) { + // It gets a space unless the next token is a delimiting punctuation. + return lexeme != ')' && + lexeme != ']' && + lexeme != '}' && + lexeme != ',' && + lexeme != ';'; } - void _write(String text, - {bool containsNewline = false, bool hanging = false, int? offset}) { - var textPiece = _currentText; - - // Create a new text piece if we don't have one or we are after a split. - // Ignore the split if the text is deliberately intended to follow the - // current text. - if (textPiece == null || _pendingSplit && !hanging) { - textPiece = _currentText = TextPiece(); - } else if (_pendingSpace || hanging) { - // Always write a space before hanging comments. - textPiece.appendSpace(); + /// Writes [token] and any comments that precede it to the current [TextPiece] + /// and updates any selection markers that appear in it. + void _writeToken(Token token, {String? lexeme}) { + if (!_writeCommentsBefore(token)) { + // We want this token to be in its own TextPiece, so if the comments + // didn't already lead to ending the previous TextPiece than do so now. + _currentText = TextPiece(); } + _write(lexeme ?? token.lexeme, offset: token.offset); + } + + /// Writes [text] to the current [TextPiece]. + /// + /// If [offset] is given and it contains any selection markers, then attaches + /// those markers to the [TextPiece]. + void _write(String text, {bool containsNewline = false, int? offset}) { if (offset != null) { // If this text contains any of the selection endpoints, note their // relative locations in the text piece. if (_findSelectionStartWithin(offset, text.length) case var start?) { - textPiece.startSelection(start); + _currentText.startSelection(start); } if (_findSelectionEndWithin(offset, text.length) case var end?) { - textPiece.endSelection(end); + _currentText.endSelection(end); } } - textPiece.append(text, containsNewline: containsNewline); - - _pendingSpace = false; - if (!hanging) _pendingSplit = false; + _currentText.append(text, containsNewline: containsNewline); } /// Finishes writing and returns a [SourceCode] containing the final output /// and updated selection, if any. - SourceCode finish() { + SourceCode finish(Piece rootPiece) { var formatter = Solver(_formatter.pageWidth); - var piece = take(); - if (debug.tracePieceBuilder) { - print(debug.pieceTree(piece)); + print(debug.pieceTree(rootPiece)); } - var result = formatter.format(piece); + var result = formatter.format(rootPiece); var outputCode = result.text; // Be a good citizen, end with a newline. diff --git a/lib/src/front_end/sequence_builder.dart b/lib/src/front_end/sequence_builder.dart index e2e2ca97..7529032e 100644 --- a/lib/src/front_end/sequence_builder.dart +++ b/lib/src/front_end/sequence_builder.dart @@ -3,8 +3,8 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; -import 'package:dart_style/src/ast_extensions.dart'; +import '../ast_extensions.dart'; import '../constants.dart'; import '../piece/piece.dart'; import '../piece/sequence.dart'; @@ -32,6 +32,9 @@ class SequenceBuilder { SequenceBuilder(this._visitor); + bool _mustSplit = false; + bool get mustSplit => _mustSplit; + SequencePiece build() => SequencePiece(_elements); /// Adds [piece] to this sequence. @@ -48,8 +51,7 @@ class SequenceBuilder { /// any comments or blank lines that appear before it. void visit(AstNode node, {int? indent}) { addCommentsBefore(node.firstNonCommentToken); - _visitor.visit(node); - add(_visitor.pieces.split(), indent: indent); + add(_visitor.nodePiece(node), indent: indent); } /// Appends a blank line before the next piece in the sequence. @@ -64,7 +66,7 @@ class SequenceBuilder { /// Comments between sequence elements get special handling where comments /// on their own line become standalone sequence elements. void addCommentsBefore(Token token) { - var comments = _visitor.takeCommentsBefore(token); + var comments = _visitor.comments.takeCommentsBefore(token); // Edge case: if we require a blank line, but there exists one between // some of the comments, or after the last one, then we don't need to @@ -83,24 +85,26 @@ class SequenceBuilder { } for (var i = 0; i < comments.length; i++) { - var comment = comments[i]; + var comment = _visitor.pieces.writeComment(comments[i]); + if (_elements.isNotEmpty && comments.isHanging(i)) { - // Attach the comment to the previous token. - _visitor.space(); - _visitor.pieces.writeComment(comment, hanging: true); + // Attach the comment to the previous element. + _elements.last.hangingComments.add(comment); } else { - // Write the comment as its own sequence piece. - _visitor.pieces.writeComment(comment); if (comments.linesBefore(i) > 1) { // Always preserve a blank line above sequence-level comments. _allowBlank = true; addBlank(); } - add(_visitor.pieces.split()); + // Write the comment as its own sequence piece. + add(comment); } } + // If the sequence contains any line comments, make sure it splits. + if (comments.requiresNewline) _mustSplit = true; + // Write a blank before the token if there should be one. if (comments.linesBeforeNextToken > 1) addBlank(); } diff --git a/lib/src/piece/adjacent.dart b/lib/src/piece/adjacent.dart index c504f2a9..9d6c97ba 100644 --- a/lib/src/piece/adjacent.dart +++ b/lib/src/piece/adjacent.dart @@ -6,24 +6,19 @@ import 'piece.dart'; /// A simple piece that just writes its child pieces one after the other. class AdjacentPiece extends Piece { - final List _pieces; + final List pieces; - /// The pieces that should have a space after them. - final Set _spaceAfter; - - AdjacentPiece(this._pieces, {List spaceAfter = const []}) - : _spaceAfter = spaceAfter.toSet(); + AdjacentPiece(this.pieces); @override void format(CodeWriter writer, State state) { - for (var piece in _pieces) { + for (var piece in pieces) { writer.format(piece); - if (_spaceAfter.contains(piece)) writer.space(); } } @override void forEachChild(void Function(Piece piece) callback) { - _pieces.forEach(callback); + pieces.forEach(callback); } } diff --git a/lib/src/piece/do_while.dart b/lib/src/piece/do_while.dart deleted file mode 100644 index 892ccd78..00000000 --- a/lib/src/piece/do_while.dart +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. -import '../back_end/code_writer.dart'; -import '../constants.dart'; -import 'piece.dart'; - -/// A piece for a do-while statement. -class DoWhilePiece extends Piece { - final Piece _body; - final Piece _condition; - - DoWhilePiece(this._body, this._condition); - - @override - void format(CodeWriter writer, State state) { - writer.setIndent(Indent.none); - writer.format(_body); - writer.space(); - writer.format(_condition); - } - - @override - void forEachChild(void Function(Piece piece) callback) { - callback(_body); - callback(_condition); - } -} diff --git a/lib/src/piece/function.dart b/lib/src/piece/function.dart index a42aa373..88aae1b1 100644 --- a/lib/src/piece/function.dart +++ b/lib/src/piece/function.dart @@ -21,7 +21,22 @@ class FunctionPiece extends Piece { /// If this is a function declaration with a (non-empty `;`) body, the body. final Piece? _body; - FunctionPiece(this._returnType, this._signature, [this._body]); + /// Whether we should write a space between the function signature and body. + /// + /// This is `true` for most bodies except for empty function bodies, like: + /// + /// ``` + /// class C { + /// C(); + /// // ^ No space before `;`. + /// } + /// ``` + final bool _spaceBeforeBody; + + FunctionPiece(this._returnType, this._signature, + {Piece? body, bool spaceBeforeBody = false}) + : _body = body, + _spaceBeforeBody = spaceBeforeBody; @override List get additionalStates => @@ -43,7 +58,7 @@ class FunctionPiece extends Piece { writer.format(_signature); if (_body case var body?) { - writer.space(); + if (_spaceBeforeBody) writer.space(); writer.format(body); } } diff --git a/lib/src/piece/list.dart b/lib/src/piece/list.dart index 962d8c6d..5c3f3984 100644 --- a/lib/src/piece/list.dart +++ b/lib/src/piece/list.dart @@ -79,8 +79,11 @@ class ListPiece extends Piece { final int _blockElement; ListPiece(this._before, this._elements, this._blanksAfter, this._after, - this._style, this._blockElement) - : _splitState = State(2, cost: _style.splitCost); + this._style, this._blockElement, + {required bool mustSplit}) + : _splitState = State(2, cost: _style.splitCost) { + if (mustSplit) pin(_splitState); + } @override List get additionalStates => [if (_elements.isNotEmpty) _splitState]; @@ -121,12 +124,10 @@ class ListPiece extends Piece { writer.setAllowNewlines(i == _blockElement || state == _splitState); var element = _elements[i]; - element.format(writer, appendComma: appendComma); - - // Only allow newlines in comments if we're fully split. - writer.setAllowNewlines(state == _splitState); - - element.formatComment(writer); + element.format(writer, + appendComma: appendComma, + // Only allow newlines in comments if we're fully split. + allowNewlinesInComments: state == _splitState); // Write a space or newline between elements. if (!isLast) { @@ -182,7 +183,10 @@ class ListPiece extends Piece { /// [ListElement] with both where `second` is the element and `// Hanging` is /// the comment. final class ListElement { - final Piece? _element; + /// The leading inline block comments before the content. + final List _leadingComments; + + final Piece? _content; /// What kind of block formatting can be applied to this element. final BlockFormat blockFormat; @@ -199,54 +203,82 @@ final class ListElement { /// int parameter2, /// ]); /// ``` - final String _delimiter; + String _delimiter = ''; - final Piece? _comment; + /// The hanging inline block and line comments that appear after the content. + final List _hangingComments = []; + + /// The number of hanging comments that should appear before the delimiter. + /// + /// A list item may have hanging comments before and after the delimiter, as + /// in: + /// + /// ``` + /// function( + /// argument /* 1 */ /* 2 */, /* 3 */ /* 4 */ // 5 + /// ); + /// ``` + /// + /// This field counts the number of comments that should be before the + /// delimiter (here `,` and 2). + int _commentsBeforeDelimiter = 0; - ListElement(Piece element, BlockFormat format, [Piece? comment]) - : this._(element, format, '', comment); + ListElement(List leadingComments, Piece element, BlockFormat format) + : _leadingComments = [...leadingComments], + _content = element, + blockFormat = format; ListElement.comment(Piece comment) - : this._(null, BlockFormat.none, '', comment); + : _leadingComments = const [], + _content = null, + blockFormat = BlockFormat.none { + _hangingComments.add(comment); + } - ListElement._(this._element, this.blockFormat, this._delimiter, - [this._comment]); + void addComment(Piece comment, {bool beforeDelimiter = false}) { + _hangingComments.add(comment); + if (beforeDelimiter) _commentsBeforeDelimiter++; + } + + void setDelimiter(String delimiter) { + _delimiter = delimiter; + } + + void format(CodeWriter writer, + {required bool appendComma, required bool allowNewlinesInComments}) { + for (var comment in _leadingComments) { + writer.format(comment); + writer.space(); + } + + if (_content case var content?) { + writer.format(content); + + for (var i = 0; i < _commentsBeforeDelimiter; i++) { + writer.space(); + writer.format(_hangingComments[i]); + } - /// Writes this element to [writer]. - /// - /// If [appendComma] is `true`, writes a comma after the element, unless the - /// element shouldn't have one because it's a comment. - void format(CodeWriter writer, {required bool appendComma}) { - if (_element case var element?) { - writer.format(element); if (appendComma) writer.write(','); + if (_delimiter.isNotEmpty) { writer.space(); writer.write(_delimiter); } } - } - void formatComment(CodeWriter writer) { - if (_comment case var comment?) { - if (_element != null) writer.space(); - writer.format(comment); + writer.setAllowNewlines(allowNewlinesInComments); + + for (var i = _commentsBeforeDelimiter; i < _hangingComments.length; i++) { + if (i > 0 || _content != null) writer.space(); + writer.format(_hangingComments[i]); } } void forEachChild(void Function(Piece piece) callback) { - if (_element case var expression?) callback(expression); - if (_comment case var comment?) callback(comment); - } - - /// Returns a new [ListElement] containing this one's element and [comment]. - ListElement withComment(Piece comment) { - assert(_comment == null); // Shouldn't already have one. - return ListElement._(_element, blockFormat, _delimiter, comment); - } - - ListElement withDelimiter(String delimiter) { - return ListElement._(_element, blockFormat, delimiter, _comment); + _leadingComments.forEach(callback); + if (_content case var content?) callback(content); + _hangingComments.forEach(callback); } } diff --git a/lib/src/piece/piece.dart b/lib/src/piece/piece.dart index 0d9f7387..116763a3 100644 --- a/lib/src/piece/piece.dart +++ b/lib/src/piece/piece.dart @@ -88,12 +88,15 @@ class TextPiece extends Piece { /// multiline strings, etc. bool _containsNewline = false; - /// Whether this piece should have a newline written at the end of it. + /// Whitespace at the end of this [TextPiece]. /// - /// This is true during piece construction while lines are still being - /// written. It may also be true once a piece is fully complete if it ends in - /// a line comment. - bool _trailingNewline = false; + /// This will be turned into actual text if non-whitespace is written after + /// the pending whitespace is set. Otherwise, it will be written to the output + /// when the [TextPiece] is formatted. + /// + /// Initially [Whitespace.newline] so that we insert a new string into the + /// empty [_lines] list on the first write. + Whitespace _trailingWhitespace = Whitespace.newline; /// The offset from the beginning of [text] where the selection starts, or /// `null` if the selection does not start within this chunk. @@ -111,25 +114,33 @@ class TextPiece extends Piece { /// If [text] internally contains a newline, then [containsNewline] should /// be `true`. void append(String text, {bool containsNewline = false}) { - if (_lines.isEmpty || _trailingNewline) _lines.add(''); + // Write any pending whitespace into the text. + switch (_trailingWhitespace) { + case Whitespace.none: + break; // Nothing to do. + case Whitespace.space: + // TODO(perf): Consider a faster way of accumulating text. + _lines.last += ' '; + case Whitespace.newline: + _lines.add(''); + case Whitespace.blankLine: + throw UnsupportedError('No blank lines in TextPieces.'); + } + + _trailingWhitespace = Whitespace.none; // TODO(perf): Consider a faster way of accumulating text. _lines.last = _lines.last + text; if (containsNewline) _containsNewline = true; - - _trailingNewline = false; } - void appendSpace() { - // Don't write an unnecessary space at the beginning of a line. - if (_trailingNewline) return; - - append(' '); + void space() { + _trailingWhitespace = _trailingWhitespace.collapse(Whitespace.space); } void newline() { - _trailingNewline = true; + _trailingWhitespace = _trailingWhitespace.collapse(Whitespace.newline); } @override @@ -151,7 +162,7 @@ class TextPiece extends Piece { writer.write(_lines[i]); } - if (_trailingNewline) writer.newline(); + writer.whitespace(_trailingWhitespace); } @override @@ -160,29 +171,41 @@ class TextPiece extends Piece { /// Sets [selectionStart] to be [start] code units after the end of the /// current text in this piece. void startSelection(int start) { - // Convert it to relative to the end of this piece. - for (var line in _lines) { - start += line.length; - } - - _selectionStart = start; + _selectionStart = _adjustSelection(start); } /// Sets [selectionEnd] to be [end] code units after the end of the /// current text in this piece. void endSelection(int end) { - // Convert it to relative to the end of this piece. + _selectionEnd = _adjustSelection(end); + } + + /// Adjust [offset] by the current length of this [TextPiece]. + int _adjustSelection(int offset) { for (var line in _lines) { - end += line.length; + offset += line.length; } - _selectionEnd = end; + if (_trailingWhitespace == Whitespace.space) offset++; + + return offset; } @override String toString() => '`${_lines.join('¬')}`${_containsNewline ? '!' : ''}'; } +/// A piece that writes a single space. +class SpacePiece extends Piece { + @override + void forEachChild(void Function(Piece piece) callback) {} + + @override + void format(CodeWriter writer, State state) { + writer.space(); + } +} + /// A state that a piece can be in. /// /// Each state identifies one way that a piece can be split into multiple lines. diff --git a/lib/src/piece/sequence.dart b/lib/src/piece/sequence.dart index c428d45c..039a2a68 100644 --- a/lib/src/piece/sequence.dart +++ b/lib/src/piece/sequence.dart @@ -24,6 +24,11 @@ class SequencePiece extends Piece { var element = _elements[i]; writer.format(element.piece); + for (var comment in element.hangingComments) { + writer.space(); + writer.format(comment); + } + if (i < _elements.length - 1) { writer.newline( blank: element.blankAfter, indent: _elements[i + 1].indent); @@ -35,6 +40,9 @@ class SequencePiece extends Piece { void forEachChild(void Function(Piece piece) callback) { for (var element in _elements) { callback(element.piece); + for (var comment in element.hangingComments) { + callback(comment); + } } } @@ -53,6 +61,9 @@ class SequenceElement { /// The [Piece] for the element. final Piece piece; + /// The comments that should appear at the end of this element's line. + final List hangingComments = []; + /// Whether there should be a blank line after this element. bool blankAfter = false; diff --git a/test/declaration/enum_member_comment.unit b/test/declaration/enum_member_comment.unit index 159141a1..b0564eae 100644 --- a/test/declaration/enum_member_comment.unit +++ b/test/declaration/enum_member_comment.unit @@ -252,5 +252,31 @@ enum E { // 2 ; // 3 + f() {} +} +>>> Multiple comments around trailing comma and semicolon. +enum E { a // 1-1 +// 1-2 +// 1-3 +,// 2-1 +// 2-2 +// 2-3 +;// 3-1 +// 3-2 +// 3-3 +f() {} +} +<<< +enum E { + a // 1-1 + // 1-2 + // 1-3 + // 2-1 + // 2-2 + // 2-3 + ; // 3-1 + + // 3-2 + // 3-3 f() {} } \ No newline at end of file diff --git a/test/expression/assignment_comment.stmt b/test/expression/assignment_comment.stmt new file mode 100644 index 00000000..1298c5ee --- /dev/null +++ b/test/expression/assignment_comment.stmt @@ -0,0 +1,13 @@ +40 columns | +>>> Line comment after value. +a = 1 // comment +; +<<< +### Weird, but users rarely write this. +a = + 1 // comment + ; +>>> Line comment after assignment and semicolon. +a = 1; // comment +<<< +a = 1; // comment \ No newline at end of file diff --git a/test/expression/binary_comment.stmt b/test/expression/binary_comment.stmt index 47b11e89..80e467ee 100644 --- a/test/expression/binary_comment.stmt +++ b/test/expression/binary_comment.stmt @@ -72,4 +72,18 @@ foo && { // comment 1 + 2; -} \ No newline at end of file +} +>>> Multiple line comments in expression. +a + // one +// two +// three +// four +// five +b; +<<< +a + // one + // two + // three + // four + // five + b; \ No newline at end of file diff --git a/test/statement/if_comment.stmt b/test/statement/if_comment.stmt index 8fb13b9c..585caa28 100644 --- a/test/statement/if_comment.stmt +++ b/test/statement/if_comment.stmt @@ -31,50 +31,64 @@ if (c) // comment { body; } ->>> Line comment after body. -if (c) -{ body; } // comment +>>> Line comment after non-block then body. +if (true) body; // comment <<< -if (c) { +if (true) body; // comment +>>> Line comment after block then body. +if (true) {body;} // comment +<<< +if (true) { body; } // comment ->>> Line comment before `else`. -if (c) { body; } // comment -else { other; } +>>> Line comment after non-block then body with else. +if (true) body; // comment +else other; +<<< +if (true) + body; // comment +else + other; +>>> Line comment after block then body with else. +if (true) {body;} // comment +else {other;} <<< -if (c) { +if (true) { body; } // comment else { other; } ->>> Line comment after `else`. -if (c) { body; } else// comment -{ other; } +>>> Line comment after `else` with block body. +if (true) {body;} else // comment +{other;} <<< -if (c) { +if (true) { body; } else // comment { other; } ->>> Line comment after `else` body. -if (c) { body; } else { other; }// comment +>>> Line comment after `else` with non-block body. +if (true) body; else // comment +other; <<< -if (c) { +if (true) body; -} else { +else // comment other; -} // comment ->>> Line comments in logic condition. -if (// Do stuff. - condition1 || - // More stuff. - condition2) { body; } +>>> Line comment after non-block else body. +if (true) body; else other; // comment <<< -if ( // Do stuff. - condition1 || - // More stuff. - condition2) { +if (true) body; -} \ No newline at end of file +else + other; // comment +>>> Line comment after block else body. +if (true) {body;} else {other;} // comment +<<< +if (true) { + body; +} else { + other; +} // comment diff --git a/test/statement/return.stmt b/test/statement/return.stmt index 9045c022..0dc63eb9 100644 --- a/test/statement/return.stmt +++ b/test/statement/return.stmt @@ -2,7 +2,7 @@ >>> Without value. return ; <<< -return ; +return; >>> With value. return value ; <<< diff --git a/test/statement/return_comment.stmt b/test/statement/return_comment.stmt new file mode 100644 index 00000000..6d9684ef --- /dev/null +++ b/test/statement/return_comment.stmt @@ -0,0 +1,28 @@ +40 columns | +>>> Line comment after return without value. +return // comment +; +<<< +return // comment +; +>>> Line comment after semicolon without value. +return; // comment +<<< +return; // comment +>>> Line comment after return with value. +return // comment +1 + 2; +<<< +return // comment +1 + 2; +>>> Line comment after return value. +return 1 + 2 // comment +; +<<< +return 1 + + 2 // comment + ; +>>> Line comment after semicolon with value. +return 1 + 2; // comment +<<< +return 1 + 2; // comment \ No newline at end of file diff --git a/test/statement/while_comment.stmt b/test/statement/while_comment.stmt new file mode 100644 index 00000000..3b448e80 --- /dev/null +++ b/test/statement/while_comment.stmt @@ -0,0 +1,43 @@ +40 columns | +>>> Line comment after `while`. +while // comment +(true) {body;} +<<< +while // comment +(true) { + body; +} +>>> Line comment after `(`. +while (// comment +true) {body;} +<<< +while ( // comment +true) { + body; +} +>>> Line comment after condition. +while (true// comment +) {body;} +<<< +while (true // comment +) { + body; +} +>>> Line comment after `)`. +while (true) // comment +{body;} +<<< +while (true) // comment +{ + body; +} +>>> Line comment after non-block body. +while (true) body; // comment +<<< +while (true) body; // comment +>>> Line comment after block body. +while (true) {body;} // comment +<<< +while (true) { + body; +} // comment \ No newline at end of file diff --git a/test/variable/local_comment.stmt b/test/variable/local_comment.stmt index 734485db..4dbf0670 100644 --- a/test/variable/local_comment.stmt +++ b/test/variable/local_comment.stmt @@ -37,4 +37,16 @@ var variable = // comment value; <<< var variable = // comment - value; \ No newline at end of file + value; +>>> Line comment after value. +var variable = value // comment +; +<<< +### Weird, but users rarely write this. +var variable = + value // comment + ; +>>> Line comment after semicolon. +var variable = value; // comment +<<< +var variable = value; // comment \ No newline at end of file