Format constructor declarations. (#1338)

Format constructor declarations.

Includes everything that can go in a constructor:

- Factory and redirecting constructors.
- Initializing formals ("this." parameters).
- Super parameters.
- Constructor initializers.
- Assert initializers.

Constructors are tricky because there are some constraints between how
the parameter list and initializer list are allowed to split. Up until
now, aside from setAllowNewlines(), there aren't really any ways that
pieces interact.

To support this, I added a pretty simple API where when a piece calls
CodeWriter.format() on a child piece, it can pass it what state the
piece is required to be in. If the solution doesn't have the child in
that state, it invalidates the solution.

It seems to work pretty well. There is probably a more optimal way to
implement that in the solver so that we test these constraints even
before formatting and discard the solution more eagerly. I left a TODO
comment about that and we can look into it later when we're
benchmarking.

Author: munificent

lib/src/back_end/code_writer.dart

@@ -53,10 +53,16 @@ class CodeWriter {
   /// The number of characters in the line currently being written.
   int _column = 0;
 
-  /// Whether this solution has encountered a newline where none is allowed.
+  /// Whether this solution has encountered a conflict that makes it not a valid
+  /// solution.
+  ///
+  /// This can occur if:
   ///
-  /// If true, it means the solution is invalid.
-  bool _containsInvalidNewline = false;
+  /// - A mandatory newline occurs like from a line comment or statement where
+  ///   none is permitted.
+  /// - An outer piece requires a child piece to have a certain state and it
+  ///   doesn't.
+  bool _isInvalid = false;
 
   /// The stack of state for each [Piece] being formatted.
   ///
@@ -126,7 +132,7 @@ class CodeWriter {
 
     return Solution(_pieceStates, _buffer.toString(), _selectionStart,
         _selectionEnd, _nextPieceToExpand,
-        isValid: !_containsInvalidNewline, overflow: _overflow, cost: _cost);
+        isValid: !_isInvalid, overflow: _overflow, cost: _cost);
   }
 
   /// Notes that a newline has been written.
@@ -138,7 +144,7 @@ class CodeWriter {
   /// the raw text contains a newline, which can happen in multi-line block
   /// comments and multi-line string literals.
   void handleNewline() {
-    if (!_options.allowNewlines) _containsInvalidNewline = true;
+    if (!_options.allowNewlines) _isInvalid = true;
 
     // Note that this piece contains a newline so that we can propagate that
     // up to containing pieces too.
@@ -228,19 +234,33 @@ class CodeWriter {
 
   /// Format [piece] and insert the result into the code being written and
   /// returned by [finish()].
-  void format(Piece piece) {
-    // Don't bother recursing into the piece tree if we know the solution will
-    // be discarded.
-    if (_containsInvalidNewline) return;
-
+  ///
+  /// If [requireState] is given, then [piece] must be in that state or the
+  /// solution is considered invalid. This is used to create constraints
+  /// between pieces. For example, a constructor piece forces the initializer
+  /// list child piece to split if the parameter list splits.
+  void format(Piece piece, {State? requireState}) {
     _pieceOptions.add(_PieceOptions(_options.indent, _options.allowNewlines));
 
     var isUnsolved = !_pieceStates.isBound(piece) && piece.states.length > 1;
     if (isUnsolved) _currentUnsolvedPieces.add(piece);
 
     var state = _pieceStates.pieceState(piece);
 
-    _cost += state.cost;
+    // If the parent piece constrains this child to a certain state, then
+    // invalidate the solution if it doesn't match.
+    if (requireState != null && state != requireState) {
+      _isInvalid = true;
+      // TODO(perf): The solver doesn't discard invalid states that are caused
+      // by newlines because sometimes an invalid state like that can lead to
+      // a valid state by selecting values for other pieces.
+      //
+      // But in this case, once a partial solution is invalidated by one piece's
+      // state conflicting with another's, any solution derived from that is
+      // also going to be invalid and we can prune that entire solution tree.
+    }
+
+    _cost += piece.stateCost(state);
 
     // TODO(perf): Memoize this. Might want to create a nested PieceWriter
     // instead of passing in `this` so we can better control what state needs
@@ -316,7 +336,7 @@ class CodeWriter {
     // expand it next.
     if (!_foundExpandLine &&
         _nextPieceToExpand != null &&
-        (_column > _pageWidth || _containsInvalidNewline)) {
+        (_column > _pageWidth || _isInvalid)) {
       // We found a problematic line, so remember it and the piece on it.
       _foundExpandLine = true;
     } else if (!_foundExpandLine) {

lib/src/front_end/ast_node_visitor.dart

@@ -13,6 +13,7 @@ import '../piece/adjacent.dart';
 import '../piece/assign.dart';
 import '../piece/block.dart';
 import '../piece/chain.dart';
+import '../piece/constructor.dart';
 import '../piece/for.dart';
 import '../piece/if.dart';
 import '../piece/infix.dart';
@@ -130,7 +131,17 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitAssertInitializer(AssertInitializer node) {
-    throw UnimplementedError();
+    return buildPiece((b) {
+      b.token(node.assertKeyword);
+      b.add(createList(
+        leftBracket: node.leftParenthesis,
+        [
+          node.condition,
+          if (node.message case var message?) message,
+        ],
+        rightBracket: node.rightParenthesis,
+      ));
+    });
   }
 
   @override
@@ -343,18 +354,85 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitConstructorDeclaration(ConstructorDeclaration node) {
-    throw UnimplementedError();
+    var header = buildPiece((b) {
+      b.modifier(node.externalKeyword);
+      b.modifier(node.constKeyword);
+      b.modifier(node.factoryKeyword);
+      b.visit(node.returnType);
+      b.token(node.period);
+      b.token(node.name);
+    });
+
+    var parameters = nodePiece(node.parameters);
+
+    Piece? redirect;
+    Piece? initializerSeparator;
+    Piece? initializers;
+    if (node.redirectedConstructor case var constructor?) {
+      redirect = AssignPiece(
+          tokenPiece(node.separator!), nodePiece(constructor),
+          isValueDelimited: false);
+    } else if (node.initializers.isNotEmpty) {
+      initializerSeparator = tokenPiece(node.separator!);
+      initializers = createList(node.initializers,
+          style: const ListStyle(commas: Commas.nonTrailing));
+    }
+
+    var body = createFunctionBody(node.body);
+
+    return ConstructorPiece(header, parameters, body,
+        canSplitParameters: node.parameters.parameters
+            .canSplit(node.parameters.rightParenthesis),
+        hasOptionalParameter: node.parameters.rightDelimiter != null,
+        redirect: redirect,
+        initializerSeparator: initializerSeparator,
+        initializers: initializers);
   }
 
   @override
   Piece visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
-    throw UnimplementedError();
+    return buildPiece((b) {
+      b.token(node.thisKeyword);
+      b.token(node.period);
+      b.add(createAssignment(node.fieldName, node.equals, node.expression));
+    });
   }
 
   @override
   Piece visitConstructorName(ConstructorName node) {
-    throw UnsupportedError(
-        'This node is handled by visitInstanceCreationExpression().');
+    // If there is an import prefix and/or constructor name, then allow
+    // splitting before the `.`. This doesn't look good, but is consistent with
+    // constructor calls that don't have `new` or `const`. We allow splitting
+    // in the latter because there is no way to distinguish syntactically
+    // between a named constructor call and any other kind of method call or
+    // property access.
+    var operations = <Piece>[];
+
+    var builder = AdjacentBuilder(this);
+    if (node.type.importPrefix case var importPrefix?) {
+      builder.token(importPrefix.name);
+      operations.add(builder.build());
+      builder.token(importPrefix.period);
+    }
+
+    // The name of the type being constructed.
+    var type = node.type;
+    builder.token(type.name2);
+    builder.visit(type.typeArguments);
+    builder.token(type.question);
+
+    // If this is a named constructor, the name.
+    if (node.name != null) {
+      operations.add(builder.build());
+      builder.token(node.period);
+      builder.visit(node.name);
+    }
+
+    // If there was a prefix or constructor name, then make a splittable piece.
+    // Otherwise, the current piece is a simple identifier for the name.
+    operations.add(builder.build());
+    if (operations.length == 1) return operations.first;
+    return ChainPiece(operations);
   }
 
   @override
@@ -550,7 +628,30 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitFieldFormalParameter(FieldFormalParameter node) {
-    throw UnimplementedError();
+    if (node.parameters case var parameters?) {
+      // A function-typed field formal like:
+      //
+      // ```
+      // C(this.fn(parameter));
+      // ```
+      return createFunctionType(
+          node.type,
+          fieldKeyword: node.thisKeyword,
+          period: node.period,
+          node.name,
+          node.typeParameters,
+          parameters,
+          node.question,
+          parameter: node);
+    } else {
+      return createFormalParameter(
+          node,
+          mutableKeyword: node.keyword,
+          fieldKeyword: node.thisKeyword,
+          period: node.period,
+          node.type,
+          node.name);
+    }
   }
 
   @override
@@ -736,7 +837,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
   @override
   Piece visitFunctionDeclaration(FunctionDeclaration node) {
     return createFunction(
-        externalKeyword: node.externalKeyword,
+        modifiers: [node.externalKeyword],
         returnType: node.returnType,
         propertyKeyword: node.propertyKeyword,
         name: node.name,
@@ -974,11 +1075,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
   @override
   Piece visitMethodDeclaration(MethodDeclaration node) {
     return createFunction(
-        externalKeyword: node.externalKeyword,
-        modifierKeyword: node.modifierKeyword,
+        modifiers: [node.externalKeyword, node.modifierKeyword],
         returnType: node.returnType,
-        operatorKeyword: node.operatorKeyword,
-        propertyKeyword: node.propertyKeyword,
+        propertyKeyword: node.operatorKeyword ?? node.propertyKeyword,
         name: node.name,
         typeParameters: node.typeParameters,
         parameters: node.parameters,
@@ -1166,7 +1265,12 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
   @override
   Piece visitRedirectingConstructorInvocation(
       RedirectingConstructorInvocation node) {
-    throw UnimplementedError();
+    return buildPiece((b) {
+      b.token(node.thisKeyword);
+      b.token(node.period);
+      b.visit(node.constructorName);
+      b.visit(node.argumentList);
+    });
   }
 
   @override
@@ -1260,22 +1364,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitSimpleFormalParameter(SimpleFormalParameter node) {
-    var builder = AdjacentBuilder(this);
-    startFormalParameter(node, builder);
-    builder.modifier(node.keyword);
-
-    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 {
-      // 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();
-    }
+    return createFormalParameter(node, node.type, node.name,
+        mutableKeyword: node.keyword);
   }
 
   @override
@@ -1300,7 +1390,12 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitSuperConstructorInvocation(SuperConstructorInvocation node) {
-    throw UnimplementedError();
+    return buildPiece((b) {
+      b.token(node.superKeyword);
+      b.token(node.period);
+      b.visit(node.constructorName);
+      b.visit(node.argumentList);
+    });
   }
 
   @override
@@ -1310,7 +1405,30 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitSuperFormalParameter(SuperFormalParameter node) {
-    throw UnimplementedError();
+    if (node.parameters case var parameters?) {
+      // A function-typed super parameter like:
+      //
+      // ```
+      // C(super.fn(parameter));
+      // ```
+      return createFunctionType(
+          node.type,
+          fieldKeyword: node.superKeyword,
+          period: node.period,
+          node.name,
+          node.typeParameters,
+          parameters,
+          node.question,
+          parameter: node);
+    } else {
+      return createFormalParameter(
+          node,
+          mutableKeyword: node.keyword,
+          fieldKeyword: node.superKeyword,
+          period: node.period,
+          node.type,
+          node.name);
+    }
   }
 
   @override

lib/src/front_end/delimited_list_builder.dart

@@ -55,9 +55,10 @@ class DelimitedListBuilder {
     var blockElement = -1;
     if (_style.allowBlockElement) blockElement = _findBlockElement();
 
-    return ListPiece(_leftBracket, _elements, _blanksAfter, _rightBracket,
-        _style, blockElement,
-        mustSplit: _mustSplit);
+    var piece = ListPiece(_leftBracket, _elements, _blanksAfter, _rightBracket,
+        _style, blockElement);
+    if (_mustSplit) piece.pin(State.split);
+    return piece;
   }
 
   /// Adds the opening [bracket] to the built list.