Format list literals (and some other things). (#1281)

Format list literals (and some other things).

I'm trying to keep these PRs mostly well-contained and not just adding a
random pile of formatting. But I'm also trying to make sure I don't drop
stuff on the floor. For this one, I filled in a few more things that
lists touch on:

- Handle type argument lists in lists and function calls.
- Support block-like formatting for right-hand sides of `=`.
- Format parenthesized expressions.
- Format named type annotations with type arguments and nullable types.
- Add support for states with different costs so we can prioritize splitting choices.

Co-authored-by: Nate Bosch <[email protected]>

Author: munificent

lib/src/ast_extensions.dart

@@ -92,6 +92,34 @@ extension AstIterableExtensions on Iterable<AstNode> {
 }
 
 extension ExpressionExtensions on Expression {
+  /// Whether this expression is a "delimited" one that allows block-like
+  /// formatting in some contexts. For example, in an assignment, a split in
+  /// the assigned value is usually indented:
+  ///
+  /// ```
+  /// var variableName =
+  ///     longValue;
+  /// ```
+  ///
+  /// But not if the initializer is a delimited expression and we don't split
+  /// at the `=`:
+  ///
+  /// ```
+  /// var variableName = [
+  ///   element,
+  /// ];
+  /// ```
+  bool get isDelimited => switch (this) {
+        ParenthesizedExpression(:var expression) => expression.isDelimited,
+        ListLiteral() => true,
+        MethodInvocation() => true,
+        // TODO(tall): Map and set literals.
+        // TODO(tall): Record literals.
+        // TODO(tall): Instance creation expressions (`new` and `const`).
+        // TODO(tall): Switch expressions.
+        _ => false,
+      };
+
   /// Whether this is an argument in an argument list with a trailing comma.
   bool get isTrailingCommaArgument {
     var parent = this.parent;

lib/src/back_end/code_writer.dart

@@ -174,9 +174,7 @@ class CodeWriter {
 
     var state = _pieceStates.pieceState(piece);
 
-    // TODO(tall): Support pieces with different split costs, and possibly
-    // different costs for each state value.
-    if (state != State.initial) _cost++;
+    _cost += state.cost;
 
     // TODO(perf): Memoize this. Might want to create a nested PieceWriter
     // instead of passing in `this` so we can better control what state needs

lib/src/front_end/ast_node_visitor.dart

@@ -526,7 +526,28 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitListLiteral(ListLiteral node) {
-    throw UnimplementedError();
+    visit(node.typeArguments);
+
+    var builder = DelimitedListBuilder(this);
+    builder.leftBracket(node.leftBracket);
+
+    // TODO(tall): Support a line comment inside a list 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.
+
+    node.elements.forEach(builder.add);
+
+    builder.rightBracket(node.rightBracket);
+    writer.push(builder.build());
   }
 
   @override
@@ -570,9 +591,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
     if (node.target != null) throw UnimplementedError();
 
     visit(node.methodName);
-
-    // TODO(tall): Support type arguments to method calls.
-    if (node.typeArguments != null) throw UnimplementedError();
+    visit(node.typeArguments);
 
     var builder = DelimitedListBuilder(this);
     builder.leftBracket(node.argumentList.leftParenthesis);
@@ -603,9 +622,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
     if (node.importPrefix != null) throw UnimplementedError();
 
     token(node.name2);
-
-    // TODO(tall): Handle type arguments.
-    if (node.typeArguments != null) throw UnimplementedError();
+    visit(node.typeArguments);
 
     // TODO(tall): Handle nullable types.
     if (node.question != null) throw UnimplementedError();
@@ -648,7 +665,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitParenthesizedExpression(ParenthesizedExpression node) {
-    throw UnimplementedError();
+    token(node.leftParenthesis);
+    visit(node.expression);
+    token(node.rightParenthesis);
   }
 
   @override
@@ -877,7 +896,15 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitTypeArgumentList(TypeArgumentList node) {
-    throw UnimplementedError();
+    var builder = DelimitedListBuilder(this);
+    builder.leftBracket(node.leftBracket);
+
+    for (var arguments in node.arguments) {
+      builder.add(arguments);
+    }
+
+    builder.rightBracket(node.rightBracket);
+    writer.push(builder.build());
   }
 
   @override

lib/src/front_end/delimited_list_builder.dart

@@ -31,20 +31,25 @@ class DelimitedListBuilder {
 
   late final Piece _rightBracket;
 
+  bool _trailingComma = true;
+
   DelimitedListBuilder(this._visitor);
 
   /// The list of comments following the most recently written element before
   /// any comma following the element.
   CommentSequence _commentsBeforeComma = CommentSequence.empty;
 
-  ListPiece build() =>
-      ListPiece(_leftBracket, _elements, _blanksAfter, _rightBracket);
+  ListPiece build() => ListPiece(
+      _leftBracket, _elements, _blanksAfter, _rightBracket, _trailingComma);
 
   /// Adds the opening [bracket] to the built list.
   void leftBracket(Token bracket) {
     _visitor.token(bracket);
     _leftBracket = _visitor.writer.pop();
     _visitor.writer.split();
+
+    // No trailing commas in type argument and type parameter lists.
+    if (bracket.type == TokenType.LT) _trailingComma = false;
   }
 
   /// Adds the closing [bracket] to the built list along with any comments that

lib/src/front_end/piece_factory.dart

@@ -5,6 +5,7 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
 
 import '../ast_extensions.dart';
+import '../piece/assign.dart';
 import '../piece/block.dart';
 import '../piece/import.dart';
 import '../piece/infix.dart';
@@ -245,13 +246,14 @@ mixin PieceFactory implements CommentWriter {
 
     writer.space();
     token(equalsOperator);
-    var equals = writer.pop();
+    var target = writer.pop();
     writer.split();
 
     visit(rightHandSide);
 
     var initializer = writer.pop();
-    writer.push(InfixPiece([equals, initializer]));
+    writer.push(AssignPiece(target, initializer,
+        isValueDelimited: rightHandSide!.isDelimited));
   }
 
   /// Writes an optional modifier that precedes other code.

lib/src/piece/assign.dart

@@ -0,0 +1,83 @@
+// 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 any construct where `=` is followed by an expression: variable
+/// initializer, assignment, constructor initializer, etc. Assignments can be
+/// formatted three ways:
+///
+/// [State.initial] No split at all:
+///
+/// ```
+/// var x = 123;
+/// ```
+///
+/// [_insideValue] If the value is a delimited "block-like" expression,
+/// then we can split inside the block but not at the `=` with no additional
+/// indentation:
+///
+/// ```
+/// var list = [
+///   element,
+/// ];
+/// ```
+///
+/// [State.split] Split after the `=`:
+///
+/// ```
+/// var name =
+///   longValueExpression;
+/// ```
+class AssignPiece extends Piece {
+  /// Split inside the value but not at the `=`.
+  ///
+  /// This is only allowed when the value is a delimited expression.
+  static const State _insideValue = State(1);
+
+  /// Split after the `=` and allow splitting inside the value.
+  ///
+  /// This is more costly because, when it's possible to split inside a
+  /// delimited value, we want to prefer that.
+  static const State _atEquals = State(2, cost: 2);
+
+  /// The left-hand side of the `=` and the `=` itself.
+  final Piece target;
+
+  /// The right-hand side of the `=`.
+  final Piece value;
+
+  final bool _isValueDelimited;
+
+  AssignPiece(this.target, this.value, {required bool isValueDelimited})
+      : _isValueDelimited = isValueDelimited;
+
+  @override
+  List<State> get states => [if (_isValueDelimited) _insideValue, _atEquals];
+
+  @override
+  void format(CodeWriter writer, State state) {
+    writer.format(target);
+
+    // A split inside the value forces splitting at the "=" unless it's a
+    // delimited expression.
+    if (state == State.initial) writer.setAllowNewlines(false);
+
+    // Don't indent a split delimited expression.
+    if (state != _insideValue) writer.setIndent(Indent.expression);
+
+    writer.splitIf(state == _atEquals);
+    writer.format(value);
+  }
+
+  @override
+  void forEachChild(void Function(Piece piece) callback) {
+    callback(target);
+    callback(value);
+  }
+
+  @override
+  String toString() => 'Assign';
+}

lib/src/piece/list.dart

@@ -32,7 +32,14 @@ class ListPiece extends Piece {
   /// The ")" after the arguments.
   final Piece _after;
 
-  ListPiece(this._before, this._arguments, this._blanksAfter, this._after);
+  /// Whether a split list should get a trailing comma.
+  ///
+  /// This is true in most constructs in Dart, but trailing commas are
+  /// disallowed by the language in type argument and type parameter lists.
+  final bool _trailingComma;
+
+  ListPiece(this._before, this._arguments, this._blanksAfter, this._after,
+      this._trailingComma);
 
   /// Don't let the list split if there is nothing in it.
   @override
@@ -67,7 +74,8 @@ class ListPiece extends Piece {
         writer.newline();
         for (var i = 0; i < _arguments.length; i++) {
           var argument = _arguments[i];
-          argument.format(writer, omitComma: false);
+          argument.format(writer,
+              omitComma: i == _arguments.length - 1 && !_trailingComma);
           if (i < _arguments.length - 1) {
             writer.newline(blank: _blanksAfter.contains(argument));
           }
@@ -76,6 +84,7 @@ class ListPiece extends Piece {
         writer.newline();
     }
 
+    writer.setAllowNewlines(true);
     writer.format(_after);
   }
 

lib/src/piece/piece.dart

@@ -96,7 +96,7 @@ class TextPiece extends Piece {
 /// Each state identifies one way that a piece can be split into multiple lines.
 /// Each piece determines how its states are interpreted.
 class State implements Comparable<State> {
-  static const initial = State(0);
+  static const initial = State(0, cost: 0);
 
   /// The maximally split state a piece can be in.
   ///
@@ -106,7 +106,10 @@ class State implements Comparable<State> {
 
   final int _value;
 
-  const State(this._value);
+  /// How much a solution is penalized when this state is chosen.
+  final int cost;
+
+  const State(this._value, {this.cost = 1});
 
   @override
   int compareTo(State other) => _value.compareTo(other._value);

lib/src/piece/variable.dart

@@ -39,7 +39,7 @@ class VariablePiece extends Piece {
   static const State _betweenVariables = State(1);
 
   /// Split after the type annotation and between each variable.
-  static const State _afterType = State(2);
+  static const State _afterType = State(2, cost: 2);
 
   /// The leading keywords (`var`, `final`, `late`) and optional type
   /// annotation.

test/README.md

@@ -68,6 +68,7 @@ invocation/   - Test formatting function and member invocations.
 member/       - Test formatting class/enum/extension/mixin member declarations.
 statement/    - Test formatting statements.
 top_level/    - Test formatting top-level declarations and directives.
+type/         - Test formatting type annotations.
 ```
 
 These tests are all run by `tall_format_test.dart`.