Format function declarations and expressions. (#1303)

Format function declarations and expressions.

This covers:

- Top-level function declarations
- Local function declaration statements
- Anonymous function expressions

It handles all three kinds of bodies: `;`, `{ ... }`, and `=> ... ;`.

Most of the implementation work for functions is in:

- Their parameter lists, which is most already handled by the existing
  support (and tests) for function type annotations.
- Their `{ ... }` bodies, which are just block statements and are
  already implemented and tested.
- Their `=> ...` bodies, which are formatted like assignments so need
  little additional implementation.

This PR mostly just stitches that together, adds tests, and covers some
of the bits that can't appear in those existing constructs:

- The `external` modifier on function declarations.
- Function names.
- Default values.
- The `var` and `final` modifiers on parameters.

This PR does not do getters and setters. That will come later.

Author: munificent

lib/src/ast_extensions.dart

@@ -110,9 +110,10 @@ extension ExpressionExtensions on Expression {
   /// ];
   /// ```
   bool get isDelimited => switch (this) {
-        ParenthesizedExpression(:var expression) => expression.isDelimited,
+        FunctionExpression() => true,
         ListLiteral() => true,
         MethodInvocation() => true,
+        ParenthesizedExpression(:var expression) => expression.isDelimited,
         RecordLiteral() => true,
         SetOrMapLiteral() => true,
         SwitchExpression() => true,

lib/src/front_end/ast_node_visitor.dart

@@ -157,7 +157,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitBlockFunctionBody(BlockFunctionBody node) {
-    throw UnimplementedError();
+    functionBodyModifiers(node);
+    visit(node.block);
   }
 
   @override
@@ -300,9 +301,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
   void visitDefaultFormalParameter(DefaultFormalParameter node) {
     visit(node.parameter);
 
-    // TODO(tall): Implement default values when function declarations are
-    // implemented.
-    if (node.separator != null) throw UnimplementedError();
+    if (node.separator case var separator?) {
+      finishAssignment(separator, node.defaultValue!);
+    }
   }
 
   @override
@@ -335,7 +336,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitEmptyFunctionBody(EmptyFunctionBody node) {
-    throw UnimplementedError();
+    token(node.semicolon);
   }
 
   @override
@@ -360,7 +361,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitExpressionFunctionBody(ExpressionFunctionBody node) {
-    throw UnimplementedError();
+    functionBodyModifiers(node);
+    finishAssignment(node.functionDefinition, node.expression);
+    token(node.semicolon);
   }
 
   @override
@@ -522,17 +525,29 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitFunctionDeclaration(FunctionDeclaration node) {
-    throw UnimplementedError();
+    modifier(node.externalKeyword);
+
+    Piece? returnType;
+    if (node.returnType case var returnTypeNode?) {
+      visit(returnTypeNode);
+      returnType = pieces.split();
+    }
+
+    // TODO(tall): Get or set keywords for getters and setters.
+    if (node.propertyKeyword != null) throw UnimplementedError();
+    token(node.name);
+
+    finishFunction(returnType, node.functionExpression);
   }
 
   @override
   void visitFunctionDeclarationStatement(FunctionDeclarationStatement node) {
-    throw UnimplementedError();
+    visit(node.functionDeclaration);
   }
 
   @override
   void visitFunctionExpression(FunctionExpression node) {
-    throw UnimplementedError();
+    finishFunction(null, node);
   }
 
   @override
@@ -958,14 +973,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
   void visitSimpleFormalParameter(SimpleFormalParameter node) {
     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.
+      modifier(node.keyword);
       visit(type);
       var typePiece = pieces.split();
 
@@ -975,6 +985,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
       pieces.give(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);
     }

lib/src/front_end/piece_factory.dart

@@ -153,12 +153,9 @@ mixin PieceFactory implements CommentWriter {
     visit(typeParameters);
     visit(parameters);
     token(question);
+    var parametersPiece = pieces.take();
 
-    // Allow splitting after the return type.
-    if (returnTypePiece != null) {
-      var parametersPiece = pieces.take();
-      pieces.give(FunctionTypePiece(returnTypePiece, parametersPiece));
-    }
+    pieces.give(FunctionPiece(returnTypePiece, parametersPiece));
   }
 
   // TODO(tall): Generalize this to work with if elements too.
@@ -404,6 +401,14 @@ mixin PieceFactory implements CommentWriter {
     if (parameter.covariantKeyword != null) throw UnimplementedError();
   }
 
+  /// Handles the `async`, `sync*`, or `async*` modifiers on a function body.
+  void functionBodyModifiers(FunctionBody body) {
+    // The `async` or `sync` keyword.
+    token(body.keyword);
+    token(body.star);
+    if (body.keyword != null) space();
+  }
+
   /// Creates a [Piece] for some code followed by an `=` and an expression in
   /// any place where an `=` appears:
   ///
@@ -454,6 +459,29 @@ mixin PieceFactory implements CommentWriter {
     }
   }
 
+  /// 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, FunctionExpression function) {
+    visit(function.typeParameters);
+    visit(function.parameters);
+
+    Piece parameters;
+    Piece? body;
+    if (function.body case EmptyFunctionBody body) {
+      // 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);
+      parameters = pieces.split();
+    } else {
+      parameters = pieces.split();
+      visit(function.body);
+      body = pieces.take();
+    }
+
+    pieces.give(FunctionPiece(returnType, parameters, body));
+  }
+
   /// Writes an optional modifier that precedes other code.
   void modifier(Token? keyword) {
     token(keyword, after: space);

lib/src/piece/assign.dart

@@ -74,14 +74,33 @@ class AssignPiece extends Piece {
   //
   // ```
   // var value = operand +
-  //    operand +
-  //    operand;
+  //     operand +
+  //     operand;
   // ```
   //
-  // For now, we not implement this special case behavior. Once more of the
+  // For now, we do not implement this special case behavior. Once more of the
   // language is implemented in the new back end and we can run the formatter
   // on a large corpus of code, we can try it out and see if the special case
   // behavior is worth it.
+  //
+  // If we don't do that, consider at least not adding another level of
+  // indentation for subsequent operands in an infix operator chain. So prefer:
+  //
+  // ```
+  // var value =
+  //     operand +
+  //     operand +
+  //     operand;
+  // ```
+  //
+  // Over:
+  //
+  // ```
+  // var value =
+  //     operand +
+  //         operand +
+  //         operand;
+  // ```
 
   @override
   List<State> get additionalStates =>

lib/src/piece/function.dart

@@ -4,42 +4,54 @@
 import '../back_end/code_writer.dart';
 import 'piece.dart';
 
-/// Piece for a function type or function-typed parameter.
+/// Piece for a function declaration, function type, or function-typed
+/// parameter.
 ///
-/// Handles splitting between the return type and the rest of the function type.
-class FunctionTypePiece extends Piece {
-  /// The return type annotation.
-  final Piece _returnType;
+/// Handles splitting between the return type and the rest of the function.
+class FunctionPiece extends Piece {
+  /// The return type annotation, if any.
+  final Piece? _returnType;
 
   /// The rest of the function type signature: name, type parameters,
   /// parameters, etc.
   final Piece _signature;
 
-  FunctionTypePiece(this._returnType, this._signature);
+  /// If this is a function declaration with a (non-empty `;`) body, the body.
+  final Piece? _body;
+
+  FunctionPiece(this._returnType, this._signature, [this._body]);
 
   @override
-  List<State> get additionalStates => const [State.split];
+  List<State> get additionalStates => [if (_returnType != null) State.split];
 
   @override
   void format(CodeWriter writer, State state) {
-    // A split inside the return type forces splitting after the return type.
-    writer.setAllowNewlines(state == State.split);
-    writer.format(_returnType);
+    if (_returnType case var returnType?) {
+      // A split inside the return type forces splitting after the return type.
+      writer.setAllowNewlines(state == State.split);
+
+      writer.format(returnType);
 
-    // A split in the type parameters or parameters does not force splitting
-    // after the return type.
-    writer.setAllowNewlines(true);
-    writer.splitIf(state == State.split);
+      // A split in the type parameters or parameters does not force splitting
+      // after the return type.
+      writer.setAllowNewlines(true);
+      writer.splitIf(state == State.split);
+    }
 
     writer.format(_signature);
+    if (_body case var body?) {
+      writer.space();
+      writer.format(body);
+    }
   }
 
   @override
   void forEachChild(void Function(Piece piece) callback) {
-    callback(_returnType);
+    if (_returnType case var returnType?) callback(returnType);
     callback(_signature);
+    if (_body case var body?) callback(body);
   }
 
   @override
-  String toString() => 'FnType';
+  String toString() => 'Fn';
 }

test/function/block_body.unit

@@ -0,0 +1,35 @@
+40 columns                              |
+>>> Empty block body.
+f  (  )  {  }
+<<<
+f() {}
+>>> Collapse empty body.
+void main() {    }
+<<<
+void main() {}
+>>>
+void main() {
+
+
+
+}
+<<<
+void main() {}
+>>> Non-empty block body.
+f  (  )  {  body  ;  }
+<<<
+f() {
+  body;
+}
+>>> Async.
+f  (  )  async  {  }
+<<<
+f() async {}
+>>> Sync*.
+f  (  )  sync  *  {  }
+<<<
+f() sync* {}
+>>> Async*.
+f  (  )  async  *  {  }
+<<<
+f() async* {}

test/function/default_value.unit

@@ -0,0 +1,48 @@
+40 columns                              |
+>>> Named parameter.
+f(  {  optional  =  null  }  ) {}
+<<<
+f({optional = null}) {}
+>>> Named parameter with old separator.
+### This syntax is no longer supported by new versions of Dart, but we want to
+### support formatting older versions if possible.
+f(  {  optional  :  1  +  2  }  ) {}
+<<<
+f({optional: 1 + 2}) {}
+>>> Optional positional parameter.
+f(  [  optional  =  1  +  2  ]  ) {}
+<<<
+f([optional = 1 + 2]) {}
+>>> Split on positional default.
+doStuff([parameter = veryLongDefaultValueThatSplits, another =
+veryLongDefaultValue, third = alsoQuiteLongDefaultValue]) {}
+<<<
+doStuff([
+  parameter =
+      veryLongDefaultValueThatSplits,
+  another = veryLongDefaultValue,
+  third = alsoQuiteLongDefaultValue,
+]) {}
+>>> Split on named default.
+doStuff({parameter = veryLongDefaultValueThatSplits, another =
+veryLongDefaultValue, third = alsoAQuiteLongDefaultValue}) {}
+<<<
+doStuff({
+  parameter =
+      veryLongDefaultValueThatSplits,
+  another = veryLongDefaultValue,
+  third = alsoAQuiteLongDefaultValue,
+}) {}
+>>> Prefer block-like splitting for collection default values.
+function([param = [element, element, element, element]]) { body; }
+<<<
+function([
+  param = [
+    element,
+    element,
+    element,
+    element,
+  ],
+]) {
+  body;
+}