diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index a1fc65a5..c85fc322 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -383,7 +383,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { if (node.redirectedConstructor case var constructor?) { redirect = AssignPiece( tokenPiece(node.separator!), nodePiece(constructor), - isValueDelimited: false); + allowInnerSplit: false); } else if (node.initializers.isNotEmpty) { initializerSeparator = tokenPiece(node.separator!); initializers = createList(node.initializers, @@ -595,7 +595,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { var expression = nodePiece(node.expression); b.add(AssignPiece(operatorPiece, expression, - isValueDelimited: node.expression.canBlockSplit)); + allowInnerSplit: node.expression.canBlockSplit)); b.token(node.semicolon); }); } @@ -693,117 +693,36 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override Piece visitForElement(ForElement node) { - throw UnimplementedError(); - } - - @override - Piece visitForStatement(ForStatement node) { var forKeyword = buildPiece((b) { b.modifier(node.awaitKeyword); b.token(node.forKeyword); }); - Piece forPartsPiece; - switch (node.forLoopParts) { - // Edge case: A totally empty for loop is formatted just as `(;;)` with - // no splits or spaces anywhere. - case ForPartsWithExpression( - initialization: null, - leftSeparator: Token(precedingComments: null), - condition: null, - rightSeparator: Token(precedingComments: null), - updaters: NodeList(isEmpty: true), - ) && - var forParts - when node.rightParenthesis.precedingComments == null: - 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): - case ForParts forParts && - ForPartsWithExpression(initialization: AstNode? initializer): - // In a C-style for loop, treat the for loop parts like an argument list - // where each clause is a separate argument. This means that when they - // split, they split like: - // - // for ( - // initializerClause; - // conditionClause; - // incrementClause - // ) { - // body; - // } - var partsList = - DelimitedListBuilder(this, const ListStyle(commas: Commas.none)); - partsList.leftBracket(node.leftParenthesis); - - // The initializer clause. - if (initializer != null) { - partsList.addCommentsBefore(initializer.beginToken); - 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)); - } - - // The condition clause. - if (forParts.condition case var conditionExpression?) { - partsList.addCommentsBefore(conditionExpression.beginToken); - partsList.add(buildPiece((b) { - b.visit(conditionExpression); - b.token(forParts.rightSeparator); - })); - } else { - partsList.addCommentsBefore(forParts.rightSeparator); - partsList.add(tokenPiece(forParts.rightSeparator)); - } - - // The update clauses. - if (forParts.updaters.isNotEmpty) { - partsList.addCommentsBefore(forParts.updaters.first.beginToken); - partsList.add(createList(forParts.updaters, - style: const ListStyle(commas: Commas.nonTrailing))); - } + var forPartsPiece = createForLoopParts( + node.leftParenthesis, node.forLoopParts, node.rightParenthesis); + var body = nodePiece(node.body); - partsList.rightBracket(node.rightParenthesis); - forPartsPiece = partsList.build(); + var forPiece = ForPiece(forKeyword, forPartsPiece, body, + hasBlockBody: node.body.isSpreadCollection); - case ForPartsWithPattern(): - throw UnimplementedError(); + // It looks weird to have multiple nested control flow elements on the same + // line, so force the outer one to split if there is an inner one. + if (node.body.isControlFlowElement) { + forPiece.pin(State.split); + } - case ForEachParts forEachParts && - ForEachPartsWithDeclaration(loopVariable: AstNode variable): - case ForEachParts forEachParts && - ForEachPartsWithIdentifier(identifier: AstNode variable): - // If a for-in loop, treat the for parts like an assignment, so they - // split like: - // - // for (var variable in [ - // initializer, - // ]) { - // body; - // } - forPartsPiece = buildPiece((b) { - b.token(node.leftParenthesis); - b.add(createAssignment( - variable, forEachParts.inKeyword, forEachParts.iterable, - splitBeforeOperator: true)); - b.token(node.rightParenthesis); - }); + return forPiece; + } - case ForEachPartsWithPattern(): - throw UnimplementedError(); - } + @override + Piece visitForStatement(ForStatement node) { + var forKeyword = buildPiece((b) { + b.modifier(node.awaitKeyword); + b.token(node.forKeyword); + }); + var forPartsPiece = createForLoopParts( + node.leftParenthesis, node.forLoopParts, node.rightParenthesis); var body = nodePiece(node.body); return ForPiece(forKeyword, forPartsPiece, body, @@ -1824,7 +1743,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { var initializerPiece = nodePiece(initializer, commaAfter: true); variables.add(AssignPiece(variablePiece, initializerPiece, - isValueDelimited: initializer.canBlockSplit)); + allowInnerSplit: initializer.canBlockSplit)); } else { variables.add(tokenPiece(variable.name, commaAfter: true)); } diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart index 687f7844..f8c952c4 100644 --- a/lib/src/front_end/piece_factory.dart +++ b/lib/src/front_end/piece_factory.dart @@ -213,6 +213,137 @@ mixin PieceFactory { }); } + /// Creates a piece for the parentheses and inner parts of a for statement or + /// for element. + Piece createForLoopParts(Token leftParenthesis, ForLoopParts forLoopParts, + Token rightParenthesis) { + switch (forLoopParts) { + // Edge case: A totally empty for loop is formatted just as `(;;)` with + // no splits or spaces anywhere. + case ForPartsWithExpression( + initialization: null, + leftSeparator: Token(precedingComments: null), + condition: null, + rightSeparator: Token(precedingComments: null), + updaters: NodeList(isEmpty: true), + ) + when rightParenthesis.precedingComments == null: + return buildPiece((b) { + b.token(leftParenthesis); + b.token(forLoopParts.leftSeparator); + b.token(forLoopParts.rightSeparator); + b.token(rightParenthesis); + }); + + case ForParts forParts && + ForPartsWithDeclarations(variables: AstNode? initializer): + case ForParts forParts && + ForPartsWithExpression(initialization: AstNode? initializer): + // In a C-style for loop, treat the for loop parts like an argument list + // where each clause is a separate argument. This means that when they + // split, they split like: + // + // for ( + // initializerClause; + // conditionClause; + // incrementClause + // ) { + // body; + // } + var partsList = + DelimitedListBuilder(this, const ListStyle(commas: Commas.none)); + partsList.leftBracket(leftParenthesis); + + // The initializer clause. + if (initializer != null) { + partsList.addCommentsBefore(initializer.beginToken); + 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)); + } + + // The condition clause. + if (forParts.condition case var conditionExpression?) { + partsList.addCommentsBefore(conditionExpression.beginToken); + partsList.add(buildPiece((b) { + b.visit(conditionExpression); + b.token(forParts.rightSeparator); + })); + } else { + partsList.addCommentsBefore(forParts.rightSeparator); + partsList.add(tokenPiece(forParts.rightSeparator)); + } + + // The update clauses. + if (forParts.updaters.isNotEmpty) { + partsList.addCommentsBefore(forParts.updaters.first.beginToken); + partsList.add(createList(forParts.updaters, + style: const ListStyle(commas: Commas.nonTrailing))); + } + + partsList.rightBracket(rightParenthesis); + return partsList.build(); + + case ForPartsWithPattern(): + throw UnimplementedError(); + + case ForEachParts forEachParts && + ForEachPartsWithDeclaration(loopVariable: AstNode variable): + case ForEachParts forEachParts && + ForEachPartsWithIdentifier(identifier: AstNode variable): + // If a for-in loop, treat the for parts like an assignment, so they + // split like: + // + // for (var variable in [ + // initializer, + // ]) { + // body; + // } + // TODO(tall): Passing `allowInnerSplit: true` allows output like: + // + // // 1 + // for (variable in longExpression + + // thatWraps) { + // ... + // } + // + // Versus the split in the initializer forcing a split before `in` too: + // + // // 2 + // for (variable + // in longExpression + + // thatWraps) { + // ... + // } + // + // This is also allowed: + // + // // 3 + // for (variable + // in longExpression + thatWraps) { + // ... + // } + // + // Currently, the formatter prefers 1 over 3. We may want to revisit + // that and prefer 3 instead. + return buildPiece((b) { + b.token(leftParenthesis); + b.add(createAssignment( + variable, forEachParts.inKeyword, forEachParts.iterable, + splitBeforeOperator: true, allowInnerSplit: true)); + b.token(rightParenthesis); + }); + + case ForEachPartsWithPattern(): + throw UnimplementedError(); + } + } + /// Creates a normal (not function-typed) formal parameter with a name and/or /// type annotation. /// @@ -688,11 +819,23 @@ mixin PieceFactory { /// 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. + /// + /// If [allowInnerSplit] is `true`, then a newline inside the target or + /// right-hand side doesn't force splitting at the operator itself. Piece createAssignment( AstNode target, Token operator, Expression rightHandSide, {bool splitBeforeOperator = false, bool includeComma = false, - bool spaceBeforeOperator = true}) { + bool spaceBeforeOperator = true, + bool allowInnerSplit = false}) { + // If the right-hand side can have block formatting, then a newline in + // it doesn't force the operator to split, as in: + // + // var list = [ + // element, + // ]; + allowInnerSplit |= rightHandSide.canBlockSplit; + if (splitBeforeOperator) { var targetPiece = nodePiece(target); @@ -703,7 +846,7 @@ mixin PieceFactory { }); return AssignPiece(targetPiece, initializer, - isValueDelimited: rightHandSide.canBlockSplit); + allowInnerSplit: allowInnerSplit); } else { var targetPiece = buildPiece((b) { b.visit(target); @@ -713,7 +856,7 @@ mixin PieceFactory { var initializer = nodePiece(rightHandSide, commaAfter: includeComma); return AssignPiece(targetPiece, initializer, - isValueDelimited: rightHandSide.canBlockSplit); + allowInnerSplit: allowInnerSplit); } } diff --git a/lib/src/piece/assign.dart b/lib/src/piece/assign.dart index efb9bc74..fbd41ad0 100644 --- a/lib/src/piece/assign.dart +++ b/lib/src/piece/assign.dart @@ -44,12 +44,12 @@ class AssignPiece extends Piece { /// The right-hand side of the operation. final Piece value; - /// Whether the right-hand side is a delimited expression that should receive - /// block-like formatting. - final bool _isValueDelimited; + /// Whether a newline is allowed in the right-hand side without forcing a + /// split at the assignment operator. + final bool _allowInnerSplit; - AssignPiece(this.target, this.value, {bool isValueDelimited = false}) - : _isValueDelimited = isValueDelimited; + AssignPiece(this.target, this.value, {bool allowInnerSplit = false}) + : _allowInnerSplit = allowInnerSplit; // TODO(tall): The old formatter allows the first operand of a split // conditional expression to be on the same line as the `=`, as in: @@ -91,9 +91,9 @@ class AssignPiece extends Piece { @override void format(CodeWriter writer, State state) { - // A split in either child piece forces splitting after the "=" unless it's - // a delimited expression. - if (state == State.unsplit && !_isValueDelimited) { + // A split in either child piece forces splitting at assignment operator + // unless specifically allowed. + if (!_allowInnerSplit && state == State.unsplit) { writer.setAllowNewlines(false); } diff --git a/test/expression/collection_for.stmt b/test/expression/collection_for.stmt new file mode 100644 index 00000000..2f5d753b --- /dev/null +++ b/test/expression/collection_for.stmt @@ -0,0 +1,141 @@ +40 columns | +>>> C-style for. +var l = [for ( var i = 0 ; i < 1 ; i++ ) i]; +<<< +var l = [for (var i = 0; i < 1; i++) i]; +>>> Empty clauses. +var l = [for( ; ; ) 1]; +<<< +var l = [for (;;) 1]; +>>> Empty initializer clause. +var l = [for ( ; f; bar) 1]; +<<< +var l = [for (; f; bar) 1]; +>>> Split in initializer. +var list = [for (a = initializerExpression + thatDoesNotFit; a < 1; a++) body]; +<<< +var list = [ + for ( + a = + initializerExpression + + thatDoesNotFit; + a < 1; + a++ + ) + body, +]; +>>> Split in condition. +var list = [for (a = b; conditionExpression + thatDoesNotFit; a++) body]; +<<< +var list = [ + for ( + a = b; + conditionExpression + + thatDoesNotFit; + a++ + ) + body, +]; +>>> Split in increment. +var list = [for (a = b; a < 1; anIncrementExpression + thatDoesNotFit) body]; +<<< +var list = [ + for ( + a = b; + a < 1; + anIncrementExpression + + thatDoesNotFit + ) + body, +]; +>>> Split inside for variable type. +var list = [for (LongGenericTypeName a = 0; a < 1; a++) body]; +<<< +var list = [ + for ( + LongGenericTypeName< + TypeArg, + AnotherTypeArgument + > a = 0; + a < 1; + a++ + ) + body, +]; +>>> Split inside variable type with empty clauses. +var list = [for (LongGenericTypeName a;;) body]; +<<< +var list = [ + for ( + LongGenericTypeName< + TypeArg, + AnotherTypeArgument + > a; + ; + ) + body, +]; +>>> Prefer splitting collection instead of body. +var list = [for (;;) longThingThatIsLong]; +<<< +var list = [ + for (;;) longThingThatIsLong, +]; +>>> Split outer for but not inner. +var list = [for (;;) for (c in d) longThingThatIsLong]; +<<< +var list = [ + for (;;) + for (c in d) longThingThatIsLong, +]; +>>> Unsplit in split collection. +var list = [veryLongThingThatForcesASplit, for (;;) 2, 3]; +<<< +var list = [ + veryLongThingThatForcesASplit, + for (;;) 2, + 3, +]; +>>> Long loop body forces split. +var list = [1, for (;;) veryLongThingThatForcesASplit, 3]; +<<< +var list = [ + 1, + for (;;) + veryLongThingThatForcesASplit, + 3, +]; +>>> Split inside loop body. +var list = [1, for (;;) veryLongThingThatForcesASplit + anotherLongThing, 3]; +<<< +var list = [ + 1, + for (;;) + veryLongThingThatForcesASplit + + anotherLongThing, + 3, +]; +>>> Force split if loop body is for element. +var l = [for (;;) for (c in d) t]; +<<< +var l = [ + for (;;) + for (c in d) t, +]; +>>> Force split if loop body is if element. +var map = { + for (;;) if (c) d +}; +<<< +var map = { + for (;;) + if (c) d, +}; +>>> A control flow element in an inner list doesn't force the outer to split. +var l = [for (;;) [if (c) d]]; +<<< +var l = [for (;;) [if (c) d]]; +>>> +var l = [for (;;) [for (c in d) e]]; +<<< +var l = [for (;;) [for (c in d) e]]; \ No newline at end of file diff --git a/test/expression/collection_for_comment.stmt b/test/expression/collection_for_comment.stmt new file mode 100644 index 00000000..a365d4eb --- /dev/null +++ b/test/expression/collection_for_comment.stmt @@ -0,0 +1,162 @@ +40 columns | +>>> Line comment after `for`. +var list = [ +for // comment +(a; b; c) e]; +<<< +var list = [ + for // comment + (a; b; c) + e, +]; +>>> Line comment before initializer +var list = [ + for (// comment +a; b; c) e]; +<<< +var list = [ + for ( + // comment + a; + b; + c + ) + e, +]; +>>> Line comment after initializer. +var list = [ +for (a // comment +; b; c) e]; +<<< +var list = [ + for ( + a // comment + ; + b; + c + ) + e, +]; +>>> Line comment before condition. +var list = [ +for (a; // comment +b; c) e]; +<<< +var list = [ + for ( + a; // comment + b; + c + ) + e, +]; +>>> Line comment before condition. +var list = [ +for (a; +// comment +b; c) e]; +<<< +var list = [ + for ( + a; + // comment + b; + c + ) + e, +]; +>>> Line comment after condition. +var list = [ +for (a; b// comment +; c) e]; +<<< +var list = [ + for ( + a; + b // comment + ; + c + ) + e, +]; +>>> Line comment before increment. +var list = [ +for (a; b; // comment +c) e]; +<<< +var list = [ + for ( + a; + b; // comment + c + ) + e, +]; +>>> +var list = [ +for (a; b; +// comment +c) e]; +<<< +var list = [ + for ( + a; + b; + // comment + c + ) + e, +]; +>>> Line comment after increment. +var list = [ +for (a; b; c // comment +) e]; +<<< +var list = [ + for ( + a; + b; + c // comment + ) + e, +]; +>>> +var list = [ +for (a; b; c +// comment +) e]; +<<< +var list = [ + for ( + a; + b; + c + // comment + ) + e, +]; +>>> Line comment after `)`. +var list = [ + for (a; b; c) // comment +e]; +<<< +var list = [ + for (a; b; c) // comment + e, +]; +>>> Line comment after non-spread body. +var list = [ + for (;;) e // comment +]; +<<< +var list = [ + for (;;) e, // comment +]; +>>> Line comment after spread body. +var list = [ + for (;;) ...[e] // comment +]; +<<< +var list = [ + for (;;) ...[e], // comment +]; \ No newline at end of file diff --git a/test/expression/collection_for_in.stmt b/test/expression/collection_for_in.stmt new file mode 100644 index 00000000..e495fc66 --- /dev/null +++ b/test/expression/collection_for_in.stmt @@ -0,0 +1,33 @@ +40 columns | +>>> +var l = [for (var i in i ) i]; +<<< +var l = [for (var i in i) i]; +>>> With type annotation. +var l = [for (Foo f in foos) f]; +<<< +var l = [for (Foo f in foos) f]; +>>> With `final` and type annotation. +var l = [for (final Foo f in foos) f]; +<<< +var l = [for (final Foo f in foos) f]; +>>> With just `final`. +var l = [for (final f in foos) f]; +<<< +var l = [for (final f in foos) f]; +>>> Await for. +f() async { + var l = [await for(x in y) x]; +} +<<< +f() async { + var l = [await for (x in y) x]; +} +>>> Split inside initializer. +var list = [for (a in sequenceExpression + thatDoesNotFit) body]; +<<< +var list = [ + for (a in sequenceExpression + + thatDoesNotFit) + body, +]; \ No newline at end of file diff --git a/test/expression/collection_for_spread_list.stmt b/test/expression/collection_for_spread_list.stmt new file mode 100644 index 00000000..94e3c7f9 --- /dev/null +++ b/test/expression/collection_for_spread_list.stmt @@ -0,0 +1,51 @@ +40 columns | +### Tests for spread (and unspread) lists inside for elements. +>>> Spread list inside for stays on one line if it fits. +var list = [for (;;) ...[1, 2]]; +<<< +var list = [for (;;) ...[1, 2]]; +>>> Spread list inside for formats like block if it splits. +var list = [for (;;) ...[element1, element2, element3]]; +<<< +var list = [ + for (;;) ...[ + element1, + element2, + element3, + ], +]; +>>> A split collection that isn't spread wraps and indents. +var list = [for (;;) [element1, element2, element3]]; +<<< +var list = [ + for (;;) + [element1, element2, element3], +]; +>>> A split collection that isn't spread wraps and indents. +var list = [for (;;) [element1, element2, element3, element4]]; +<<< +var list = [ + for (;;) + [ + element1, + element2, + element3, + element4, + ], +]; +>>> Force split if loop body is for element. +var l = [for (;;) for (c in d) t]; +<<< +var l = [ + for (;;) + for (c in d) t, +]; +>>> Force split if loop body is if element. +var list = [ + for (;;) if (c) d +]; +<<< +var list = [ + for (;;) + if (c) d, +]; \ No newline at end of file diff --git a/test/expression/collection_for_spread_map.stmt b/test/expression/collection_for_spread_map.stmt new file mode 100644 index 00000000..5a6f5870 --- /dev/null +++ b/test/expression/collection_for_spread_map.stmt @@ -0,0 +1,34 @@ +40 columns | +### Tests for spread (and unspread) maps inside for elements. +>>> Spread list inside for stays on one line if it fits. +var map = {for (;;) ...{1: 1, 2: 2}}; +<<< +var map = {for (;;) ...{1: 1, 2: 2}}; +>>> Spread list inside for formats like block if it splits. +var map = {for (;;) ...{element1: 1, element2: 2, element3: 3}}; +<<< +var map = { + for (;;) ...{ + element1: 1, + element2: 2, + element3: 3, + }, +}; +>>> A split collection that isn't spread wraps and indents. +var map = {for (;;) {element1: one, element2: two}}; +<<< +var map = { + for (;;) + {element1: one, element2: two}, +}; +>>> A split collection that isn't spread wraps and indents. +var map = {for (;;) {element1: 1, element2: 2, element3: 3}}; +<<< +var map = { + for (;;) + { + element1: 1, + element2: 2, + element3: 3, + }, +}; \ No newline at end of file diff --git a/test/expression/collection_for_spread_set.stmt b/test/expression/collection_for_spread_set.stmt new file mode 100644 index 00000000..a5ae425f --- /dev/null +++ b/test/expression/collection_for_spread_set.stmt @@ -0,0 +1,51 @@ +40 columns | +### Tests for spread (and unspread) sets inside for elements. +>>> Spread list inside for stays on one line if it fits. +var set = {for (;;) ...{1, 2}}; +<<< +var set = {for (;;) ...{1, 2}}; +>>> Spread list inside for formats like block if it splits. +var set = {for (;;) ...{element1, element2, element3}}; +<<< +var set = { + for (;;) ...{ + element1, + element2, + element3, + }, +}; +>>> A split collection that isn't spread wraps and indents. +var set = {for (;;) {element1, element2, element3}}; +<<< +var set = { + for (;;) + {element1, element2, element3}, +}; +>>> A split collection that isn't spread wraps and indents. +var set = {for (;;) {element1, element2, element3, element4}}; +<<< +var set = { + for (;;) + { + element1, + element2, + element3, + element4, + }, +}; +>>> Force split if loop body is for element. +var l = {for (;;) for (c in d) t}; +<<< +var l = { + for (;;) + for (c in d) t, +}; +>>> Force split if loop body is if element. +var set = { + for (;;) if (c) d +}; +<<< +var set = { + for (;;) + if (c) d, +}; \ No newline at end of file diff --git a/test/statement/for_in.stmt b/test/statement/for_in.stmt index 9fc1b33a..8db13302 100644 --- a/test/statement/for_in.stmt +++ b/test/statement/for_in.stmt @@ -58,6 +58,13 @@ for (var identifier in iteratableExpression) { body; } +>>> Split inside initializer. +for (var identifier in someVeryLong + iterableExpression) { body; } +<<< +for (var identifier in someVeryLong + + iterableExpression) { + body; +} >>> Prefer block-like splitting after `in`. for (var identifier in [element, element, element]) { body; } <<<