Enforce a blank line after local function declarations.

lib/src/source_visitor.dart

 // Copyright (c) 2014, 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.
 
 library dart_style.src.source_visitor;
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/src/generated/source.dart';
 
@@ skipping 235 matching lines @@
 
   visitBlock(Block node) {
     // Don't allow splitting in an empty block.
     if (node.statements.isEmpty &&
         node.rightBracket.precedingComments == null) {
       token(node.leftBracket);
       token(node.rightBracket);
       return;
     }
 
-    // For a block that is not a function body, just bump the indentation and
-    // keep it in the current block.
-    if (node.parent is! BlockFunctionBody) {
-      _writeBody(node.leftBracket, node.rightBracket, body: () {
-        visitNodes(node.statements, between: oneOrTwoNewlines, after: newline);
-      });
-      return;
+    // If the block is a function body, it may get expression-level indentation,
+    // so handle it specially. Otherwise, just bump the indentation and keep it
+    // in the current block.
+    if (node.parent is BlockFunctionBody) {
+      _startLiteralBody(node.leftBracket);
+    } else {
+      _beginBody(node.leftBracket);
     }
 
-    _startLiteralBody(node.leftBracket);
-    visitNodes(node.statements, between: oneOrTwoNewlines, after: newline);
-    _endLiteralBody(node.rightBracket, forceSplit: node.statements.isNotEmpty);
+    var needsDouble = true;
+    for (var statement in node.statements) {
+      if (needsDouble) {
+        twoNewlines();
+      } else {
+        oneOrTwoNewlines();
+      }
+
+      visit(statement);
+
+      needsDouble = false;
+      if (statement is FunctionDeclarationStatement) {
+        // Add a blank line after non-empty block functions.
+        if (statement.functionDeclaration.functionExpression.body is BlockFunctionBody) {
+          var body = statement.functionDeclaration.functionExpression.body as BlockFunctionBody;

[nweiz, 2016/07/28 21:00:45]

Long lines.

If you assign `body` outside the inner if statement, you can avoid the cast here
since the `is` check will propagate.

[Bob Nystrom, 2016/07/28 21:27:51]

Done.
Done.

+          needsDouble = body.block.statements.isNotEmpty;
+        }
+      }
+    }
+
+    if (node.statements.isNotEmpty) newline();
+
+    if (node.parent is BlockFunctionBody) {
+      _endLiteralBody(node.rightBracket, forceSplit: node.statements.isNotEmpty);
+    } else {
+      _endBody(node.rightBracket);
+    }
   }
 
   visitBlockFunctionBody(BlockFunctionBody node) {
     // Space after the parameter list.
     space();
 
     // The "async" or "sync" keyword.
     token(node.keyword);
 
     // The "*" in "async*" or "sync*".
@@ skipping 121 matching lines @@
 
     builder.startRule(new CombinatorRule());
     visit(node.withClause);
     visit(node.implementsClause);
     builder.endRule();
 
     visit(node.nativeClause, before: space);
     space();
 
     builder.unnest();
-    _writeBody(node.leftBracket, node.rightBracket, body: () {
-      if (node.members.isNotEmpty) {
-        for (var member in node.members) {
-          visit(member);
+    _beginBody(node.leftBracket);
 
-          if (member == node.members.last) {
-            newline();
-            break;
-          }
+    if (node.members.isNotEmpty) {
+      for (var member in node.members) {
+        visit(member);
 
-          var needsDouble = false;
-          if (member is ClassDeclaration) {
-            // Add a blank line after classes.
-            twoNewlines();
-          } else if (member is MethodDeclaration) {
-            // Add a blank line after non-empty block methods.
-            if (member.body is BlockFunctionBody) {
-              var body = member.body as BlockFunctionBody;
-              needsDouble = body.block.statements.isNotEmpty;
-            }
-          }
+        if (member == node.members.last) {
+          newline();
+          break;
+        }
 
-          if (needsDouble) {
-            twoNewlines();
-          } else {
-            // Variables and arrow-bodied members can be more tightly packed if
-            // the user wants to group things together.
-            oneOrTwoNewlines();
+        var needsDouble = false;
+        if (member is ClassDeclaration) {
+          // Add a blank line after classes.
+          twoNewlines();
+        } else if (member is MethodDeclaration) {
+          // Add a blank line after non-empty block methods.
+          if (member.body is BlockFunctionBody) {
+            var body = member.body as BlockFunctionBody;
+            needsDouble = body.block.statements.isNotEmpty;
           }
         }
+
+        if (needsDouble) {
+          twoNewlines();
+        } else {
+          // Variables and arrow-bodied members can be more tightly packed if
+          // the user wants to group things together.
+          oneOrTwoNewlines();
+        }
       }
-    });
+    }
+
+    _endBody(node.rightBracket);
   }
 
   visitClassTypeAlias(ClassTypeAlias node) {
     visitMetadata(node.metadata);
 
     _simpleStatement(node, () {
       modifier(node.abstractKeyword);
       token(node.typedefKeyword);
       space();
       visit(node.name);
@@ skipping 22 matching lines @@
     var directives = node.directives;
     if (directives.isNotEmpty && directives.first is LibraryDirective) {
       visit(directives.first);
       twoNewlines();
 
       directives = directives.skip(1);
     }
 
     visitNodes(directives, between: oneOrTwoNewlines);
 
-    if (node.declarations.isNotEmpty) {
-      var needsDouble = true;
+    var needsDouble = true;
+    for (var declaration in node.declarations) {
+      // Add a blank line before classes.
+      if (declaration is ClassDeclaration) needsDouble = true;
 
-      for (var declaration in node.declarations) {
-        // Add a blank line before classes.
-        if (declaration is ClassDeclaration) needsDouble = true;
+      if (needsDouble) {
+        twoNewlines();
+      } else {
+        // Variables and arrow-bodied members can be more tightly packed if
+        // the user wants to group things together.
+        oneOrTwoNewlines();
+      }
 
-        if (needsDouble) {
-          twoNewlines();
-        } else {
-          // Variables and arrow-bodied members can be more tightly packed if
-          // the user wants to group things together.
-          oneOrTwoNewlines();
-        }
+      visit(declaration);
 
-        visit(declaration);
-
-        needsDouble = false;
-        if (declaration is ClassDeclaration) {
-          // Add a blank line after classes.
-          needsDouble = true;
-        } else if (declaration is FunctionDeclaration) {
-          // Add a blank line after non-empty block functions.
-          if (declaration.functionExpression.body is BlockFunctionBody) {
-            var body = declaration.functionExpression.body as BlockFunctionBody;
-            needsDouble = body.block.statements.isNotEmpty;
-          }
+      needsDouble = false;
+      if (declaration is ClassDeclaration) {
+        // Add a blank line after classes.
+        needsDouble = true;
+      } else if (declaration is FunctionDeclaration) {
+        // Add a blank line after non-empty block functions.
+        if (declaration.functionExpression.body is BlockFunctionBody) {
+          var body = declaration.functionExpression.body as BlockFunctionBody;
+          needsDouble = body.block.statements.isNotEmpty;
         }
       }
     }
   }
 
   visitConditionalExpression(ConditionalExpression node) {
     builder.nestExpression();
 
     // Start lazily so we don't force the operator to split if a line comment
     // appears before the first operand. If we split after one clause in a
@@ skipping 207 matching lines @@
   }
 
   visitEnumDeclaration(EnumDeclaration node) {
     visitMetadata(node.metadata);
 
     token(node.enumKeyword);
     space();
     visit(node.name);
     space();
 
-    _writeBody(node.leftBracket, node.rightBracket, space: true, body: () {
-      visitCommaSeparatedNodes(node.constants, between: split);
-    });
+    _beginBody(node.leftBracket, space: true);
+    visitCommaSeparatedNodes(node.constants, between: split);
+    _endBody(node.rightBracket, space: true);
   }
 
   visitExportDirective(ExportDirective node) {
     visitMetadata(node.metadata);
 
     _simpleStatement(node, () {
       token(node.keyword);
       space();
       visit(node.uri);
 
@@ skipping 1399 matching lines @@
 
   /// Marks the collection literal that starts with [leftBracket] as being
   /// controlled by [argumentList].
   ///
   /// When the collection is visited, [argumentList] will determine the
   /// indentation and splitting rule for the collection.
   void beforeCollection(Token leftBracket, ArgumentSublist argumentList) {
     _collectionArgumentLists[leftBracket] = argumentList;
   }
 
-  /// Writes an bracket-delimited body and handles indenting and starting the
-  /// rule used to split the contents.
-  ///
-  /// If [space] is `true`, then the contents and delimiters will have a space
-  /// between then when unsplit.
-  void _writeBody(Token leftBracket, Token rightBracket,

[nweiz, 2016/07/28 21:00:45]

Why not keep `_writeBody` and just define it in terms of [_beginBody] and
[_endBody]?

[Bob Nystrom, 2016/07/28 21:27:51]

I could, but it's only used in like two other places so it doesn't really
justify having a helper for it. Also, in one of the places, I found the big
nested function literal actually harder to read.

-      {bool space: false, body()}) {
+  /// Writes the beginning of a brace-delimited body and handles indenting and
+  /// starting the rule used to split the contents.
+  void _beginBody(Token leftBracket, {bool space: false}) {
     token(leftBracket);
 
     // Indent the body.
     builder.indent();
 
     // Split after the bracket.
     builder.startRule();
     builder.split(isDouble: false, nest: false, space: space);
+  }
 
-    body();
-
+  /// Finishes the body started by a call to [_beginBody].
+  void _endBody(Token rightBracket, {bool space: false}) {
     token(rightBracket, before: () {
       // Split before the closing bracket character.
       builder.unindent();
       builder.split(nest: false, space: space);
     });
 
     builder.endRule();
   }
 
   /// Returns `true` if [node] is immediately contained within an anonymous
@@ skipping 283 matching lines @@
   /// Gets the 1-based line number that the beginning of [token] lies on.
   int _startLine(Token token) => _lineInfo.getLocation(token.offset).lineNumber;
 
   /// Gets the 1-based line number that the end of [token] lies on.
   int _endLine(Token token) => _lineInfo.getLocation(token.end).lineNumber;
 
   /// Gets the 1-based column number that the beginning of [token] lies on.
   int _startColumn(Token token) =>
       _lineInfo.getLocation(token.offset).columnNumber;
 }