Format function types. (#1288)

* Format function types.

This involves formatting parameter lists which is a big piece of work.
They work similar to argument lists and collection literals... except
for the special syntax around `[ ... ]` and `{ ... }` for optional
parameters.

Getting that working right involved adding some more functionality to
DelimitedListBuilder and ListPiece.

Getting comments working with that syntax was particularly difficult.
I ended up deciding that the formatter can move comments before or after
the optional parameter delimiters if it needs to. Doing so produces
better looking output, as in:

```
// Before:
f(// weird comment location
[param] // another
);

// After:
f([
  // weird comment location
  param, // another
]);
```

It also, I think is cleaner to implement than trying to laboriously
keep the comment where the user authored it.

In the process of reworking how comments are handled in lists, I also
tweaked how inline block comments are handled before commas. Previously,
they would get moved after the comma, like:

```
// Before:
[element /* comment */, another];

// After:
[element, /* comment */ another];
```

Now, they stay where they were, which is I think what users want. A
common use case for block comments in argument/collection lists is
describing the meaning of a preceding null argument and this keeps that.

Line after an element but before the comma continue to be moved after
the comma:

```
// Before:
f(param // comment
,);

// After:
f(
  param, // comment
);
```

Also:

- Added tests for the loosened behavior of equalIgnoringWhitespace().
- Added a test for a corner case of variable splitting that I ran into
  with parameters, which use the same VariablePiece.

* Add TODO comment to investigate something better than
equalIgnoringWhitespace().

* Apply review feedback.

Author: munificent

lib/src/back_end/code_writer.dart

@@ -81,6 +81,10 @@ class CodeWriter {
   /// comments and multi-line string literals.
   void handleNewline() {
     if (!_options.allowNewlines) _containsInvalidNewline = true;
+
+    // Note that this piece contains a newline so that we can propagate that
+    // up to containing pieces too.
+    _options.hasNewline = true;
   }
 
   /// Appends [text] to the output.
@@ -158,7 +162,7 @@ class CodeWriter {
   }
 
   /// Sets whether newlines are allowed to occur from this point on for the
-  /// current piece or any of its children.
+  /// current piece.
   void setAllowNewlines(bool allowed) {
     _options.allowNewlines = allowed;
   }
@@ -186,7 +190,12 @@ class CodeWriter {
     // to be used as the key in the memoization table.
     piece.format(this, state);
 
-    _pieceOptions.removeLast();
+    var childOptions = _pieceOptions.removeLast();
+
+    // If the child [piece] contains a newline then this one transitively does.
+    // TODO(tall): At some point, we may want to provide an API so that pieces
+    // can block this from propagating outward.
+    if (childOptions.hasNewline) handleNewline();
   }
 
   /// Format [piece] if not null.
@@ -221,5 +230,8 @@ class _PieceOptions {
   /// considered invalid and gets discarded.
   bool allowNewlines;
 
+  /// Whether any newlines have occurred in this piece or any of its children.
+  bool hasNewline = false;
+
   _PieceOptions(this.indent, this.nesting, this.allowNewlines);
 }

lib/src/front_end/ast_node_visitor.dart

@@ -267,7 +267,11 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitDefaultFormalParameter(DefaultFormalParameter node) {
-    throw UnimplementedError();
+    visit(node.parameter);
+
+    // TODO(tall): Implement default values when function declarations are
+    // implemented.
+    if (node.separator != null) throw UnimplementedError();
   }
 
   @override
@@ -357,7 +361,29 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitFormalParameterList(FormalParameterList node) {
-    throw UnimplementedError();
+    // Find the first non-mandatory parameter (if there are any).
+    var firstOptional =
+        node.parameters.indexWhere((p) => p is DefaultFormalParameter);
+
+    // If all parameters are optional, put the `[` or `{` right after `(`.
+    var builder = DelimitedListBuilder(this);
+    if (node.parameters.isNotEmpty && firstOptional == 0) {
+      builder.leftBracket(node.leftParenthesis, delimiter: node.leftDelimiter);
+    } else {
+      builder.leftBracket(node.leftParenthesis);
+    }
+
+    for (var i = 0; i < node.parameters.length; i++) {
+      // If this is the first optional parameter, put the delimiter before it.
+      if (firstOptional > 0 && i == firstOptional) {
+        builder.leftDelimiter(node.leftDelimiter!);
+      }
+
+      builder.add(node.parameters[i]);
+    }
+
+    builder.rightBracket(node.rightParenthesis, delimiter: node.rightDelimiter);
+    writer.push(builder.build());
   }
 
   @override
@@ -432,12 +458,15 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) {
-    throw UnimplementedError();
+    startFormalParameter(node);
+    createFunctionType(node.returnType, node.name, node.typeParameters,
+        node.parameters, node.question);
   }
 
   @override
   void visitGenericFunctionType(GenericFunctionType node) {
-    throw UnimplementedError();
+    createFunctionType(node.returnType, node.functionKeyword,
+        node.typeParameters, node.parameters, node.question);
   }
 
   @override
@@ -586,15 +615,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
     visit(node.methodName);
     visit(node.typeArguments);
 
-    var builder = DelimitedListBuilder(this);
-    builder.leftBracket(node.argumentList.leftParenthesis);
-
-    for (var argument in node.argumentList.arguments) {
-      builder.add(argument);
-    }
-
-    builder.rightBracket(node.argumentList.rightParenthesis);
-    writer.push(builder.build());
+    createDelimited(node.argumentList.leftParenthesis,
+        node.argumentList.arguments, node.argumentList.rightParenthesis);
   }
 
   @override
@@ -814,7 +836,30 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitSimpleFormalParameter(SimpleFormalParameter node) {
-    throw UnimplementedError();
+    startFormalParameter(node);
+
+    if (node.keyword != null) throw UnimplementedError();
+
+    // TODO(tall): When function declarations are implemented, test that the
+    // formatter won't split after `var` or `final` in a parameter (with a
+    // type or not).
+
+    if ((node.type, node.name) case (var type?, var name?)) {
+      // Have both a type and name, so allow splitting between them.
+      visit(type);
+      var typePiece = writer.pop();
+      writer.split();
+
+      token(name);
+      var namePiece = writer.pop();
+      writer.split();
+
+      writer.push(VariablePiece(typePiece, [namePiece], hasType: true));
+    } else {
+      // Only one of name or type so just write whichever there is.
+      visit(node.type);
+      token(node.name);
+    }
   }
 
   @override
@@ -901,25 +946,22 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitTypeArgumentList(TypeArgumentList node) {
-    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());
+    createDelimited(node.leftBracket, node.arguments, node.rightBracket);
   }
 
   @override
   void visitTypeParameter(TypeParameter node) {
-    throw UnimplementedError();
+    token(node.name);
+    if (node.bound case var bound?) {
+      writer.space();
+      modifier(node.extendsKeyword);
+      visit(bound);
+    }
   }
 
   @override
   void visitTypeParameterList(TypeParameterList node) {
-    throw UnimplementedError();
+    createDelimited(node.leftBracket, node.typeParameters, node.rightBracket);
   }
 
   @override

lib/src/front_end/comment_writer.dart

@@ -85,7 +85,9 @@ mixin CommentWriter {
         writer.writeComment(comment);
       }
 
-      if (comment.type == CommentType.line) writer.writeNewline();
+      if (comment.type == CommentType.line || comment.type == CommentType.doc) {
+        writer.writeNewline();
+      }
     }
 
     if (comments.isNotEmpty && _needsSpaceAfterComment(token.lexeme)) {
@@ -130,6 +132,10 @@ mixin CommentWriter {
       } else if (comment.type == TokenType.SINGLE_LINE_COMMENT) {
         type = CommentType.line;
       } else if (commentLine == previousLine || commentLine == tokenLine) {
+        // TODO(tall): I'm not sure if it makes sense to distinguish block
+        // comments with newlines around them from other block comments in the
+        // new Piece representation. Consider merging CommentType.inlineBlock
+        // and CommentType.block into a single type.
         type = CommentType.inlineBlock;
       } else {
         type = CommentType.block;
@@ -191,8 +197,10 @@ class SourceComment {
   SourceComment(this.text, this.type, {required this.flushLeft});
 
   /// Whether this comment contains a mandatory newline, either because it's a
-  /// line comment or a multi-line block comment.
-  bool get containsNewline => type == CommentType.line || text.contains('\n');
+  /// comment that should be on its own line or a lexeme with a newline inside
+  /// it (i.e. multi-line block comment, multi-line string).
+  bool get containsNewline =>
+      type != CommentType.inlineBlock || text.contains('\n');
 
   @override
   String toString() =>
@@ -261,9 +269,10 @@ class CommentSequence extends ListBase<SourceComment> {
     if (linesBefore(commentIndex) != 0) return false;
 
     // Doc comments and non-inline `/* ... */` comments are always pushed to
-    // the next line.
+    // the next line. Only inline block comments and line comments are allowed
+    // to hang at the end of a line.
     var type = _comments[commentIndex].type;
-    return type != CommentType.doc && type != CommentType.block;
+    return type == CommentType.inlineBlock || type == CommentType.line;
   }
 
   /// Whether the comment at [commentIndex] should be attached to the following