Formatter improvements (comments and empty bodies).

pkg/analyzer/lib/src/services/formatter_impl.dart

 // Copyright (c) 2013, 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 formatter_impl;
 
 import 'dart:math';
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/src/generated/java_core.dart' show CharSequence;
@@ skipping 375 matching lines @@
     visit(node.arguments);
   }
 
   visitArgumentDefinitionTest(ArgumentDefinitionTest node) {
     token(node.question);
     visit(node.identifier);
   }
 
   visitArgumentList(ArgumentList node) {
     token(node.leftParenthesis);
-    visitNodes(node.arguments, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.arguments);
     token(node.rightParenthesis);
   }
 
   visitAsExpression(AsExpression node) {
     visit(node.expression);
     space();
     token(node.asOperator);
     space();
     visit(node.type);
   }
@@ skipping 181 matching lines @@
     visitPrefixedBody(space, body);
   }
 
   visitConstructorInitializers(ConstructorDeclaration node) {
     newlines();
     indent(2);
     token(node.separator /* : */);
     space();
     for (var i = 0; i < node.initializers.length; i++) {
       if (i > 0) {
-        comma();
+        // preceding comma
+        token(node.initializers[i].beginToken.previous);
         newlines();
         space(2);
       }
       node.initializers[i].accept(this);
     }
     unindent(2);
   }
 
   visitConstructorRedirects(ConstructorDeclaration node) {
     token(node.separator /* = */, precededBy: space, followedBy: space);
-    visitNodes(node.initializers, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.initializers);
     visit(node.redirectedConstructor);
   }
 
   visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
     token(node.keyword);
     token(node.period);
     visit(node.fieldName);
     space();
     token(node.equals);
     space();
@@ skipping 137 matching lines @@
     token(node.rightParenthesis);
   }
 
   visitForStatement(ForStatement node) {
     token(node.forKeyword);
     space();
     token(node.leftParenthesis);
     if (node.initialization != null) {
       visit(node.initialization);
     } else {
-      visit(node.variables);
+      if (node.variables == null) {
+        space();
+      } else {
+        visit(node.variables);
+      }
     }
     token(node.leftSeparator);
     space();
     visit(node.condition);
     token(node.rightSeparator);
-    visitNodes(node.updaters, precededBy: space, separatedBy: space);
+    if (node.updaters != null) {
+      space();
+      visitCommaSeparatedNodes(node.updaters);
+    }
     token(node.rightParenthesis);
-    space();
+    if (node.body is! EmptyStatement) {
+      space();
+    }
     visit(node.body);
   }
 
   visitFunctionDeclaration(FunctionDeclaration node) {
     visitNode(node.returnType, followedBy: space);
     token(node.propertyKeyword, followedBy: space);
     visit(node.name);
     visit(node.functionExpression);
   }
 
@@ skipping 24 matching lines @@
 
   visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) {
     visitNode(node.returnType, followedBy: space);
     visit(node.identifier);
     visit(node.parameters);
   }
 
   visitHideCombinator(HideCombinator node) {
     token(node.keyword);
     space();
-    visitNodes(node.hiddenNames, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.hiddenNames);
   }
 
   visitIfStatement(IfStatement node) {
     var hasElse = node.elseStatement != null;
     token(node.ifKeyword);
     space();
     token(node.leftParenthesis);
     visit(node.condition);
     token(node.rightParenthesis);
     space();
     if (hasElse) {
       printAsBlock(node.thenStatement);
       space();
       token(node.elseKeyword);
       space();
       printAsBlock(node.elseStatement);
     } else {
       visit(node.thenStatement);
     }
   }
 
   visitImplementsClause(ImplementsClause node) {
     token(node.keyword);
     space();
-    visitNodes(node.interfaces, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.interfaces);
   }
 
   visitImportDirective(ImportDirective node) {
     token(node.keyword);
     space();
     visit(node.uri);
     token(node.asToken, precededBy: space, followedBy: space);
     visit(node.prefix);
     visitNodes(node.combinators, precededBy: space, separatedBy: space);
     token(node.semicolon);
@@ skipping 63 matching lines @@
   }
 
   visitLibraryIdentifier(LibraryIdentifier node) {
     append(node.name);
   }
 
   visitListLiteral(ListLiteral node) {
     modifier(node.constKeyword);
     visit(node.typeArguments);
     token(node.leftBracket);
-    visitNodes(node.elements, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.elements);
     optionalTrailingComma(node.rightBracket);
     token(node.rightBracket);
   }
 
   visitMapLiteral(MapLiteral node) {
     modifier(node.constKeyword);
     visitNode(node.typeArguments, followedBy: space);
     token(node.leftBracket);
-    visitNodes(node.entries, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.entries);
     optionalTrailingComma(node.rightBracket);
     token(node.rightBracket);
   }
 
   visitMapLiteralEntry(MapLiteralEntry node) {
     visit(node.key);
     token(node.separator);
     space();
     visit(node.value);
   }
@@ skipping 112 matching lines @@
     }
   }
 
   visitScriptTag(ScriptTag node) {
     token(node.scriptTag);
   }
 
   visitShowCombinator(ShowCombinator node) {
     token(node.keyword);
     space();
-    visitNodes(node.shownNames, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.shownNames);
   }
 
   visitSimpleFormalParameter(SimpleFormalParameter node) {
     modifier(node.keyword);
     visitNode(node.type, followedBy: space);
     visit(node.identifier);
   }
 
   visitSimpleIdentifier(SimpleIdentifier node) {
     token(node.token);
@@ skipping 76 matching lines @@
     token(node.tryKeyword);
     space();
     visit(node.body);
     visitNodes(node.catchClauses, precededBy: space, separatedBy: space);
     token(node.finallyKeyword, precededBy: space, followedBy: space);
     visit(node.finallyBlock);
   }
 
   visitTypeArgumentList(TypeArgumentList node) {
     token(node.leftBracket);
-    visitNodes(node.arguments, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.arguments);
     token(node.rightBracket);
   }
 
   visitTypeName(TypeName node) {
     visit(node.name);
     visit(node.typeArguments);
   }
 
   visitTypeParameter(TypeParameter node) {
     visit(node.name);
     token(node.keyword /* extends */, precededBy: space, followedBy: space);
     visit(node.bound);
   }
 
   visitTypeParameterList(TypeParameterList node) {
     token(node.leftBracket);
-    visitNodes(node.typeParameters, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.typeParameters);
     token(node.rightBracket);
   }
 
   visitVariableDeclaration(VariableDeclaration node) {
     visit(node.name);
     if (node.initializer != null) {
       space();
       token(node.equals);
       space();
       visit(node.initializer);
     }
   }
 
   visitVariableDeclarationList(VariableDeclarationList node) {
     modifier(node.keyword);
     visitNode(node.type, followedBy: space);
-    visitNodes(node.variables, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.variables);
   }
 
   visitVariableDeclarationStatement(VariableDeclarationStatement node) {
     visit(node.variables);
     token(node.semicolon);
   }
 
   visitWhileStatement(WhileStatement node) {
     token(node.keyword);
     space();
     token(node.leftParenthesis);
     visit(node.condition);
     token(node.rightParenthesis);
-    space();
+    if (node.body is! EmptyStatement) {
+      space();
+    }
     visit(node.body);
   }
 
   visitWithClause(WithClause node) {
     token(node.withKeyword);
     space();
-    visitNodes(node.mixinTypes, separatedBy: commaSeperator);
+    visitCommaSeparatedNodes(node.mixinTypes);
   }
 
   /// Safely visit the given [node].
   visit(ASTNode node) {
     if (node != null) {
       node.accept(this);
     }
   }
 
   /// Visit the given function [body], printing the [prefix] before if given
@@ skipping 21 matching lines @@
           }
           nodes[i].accept(this);
         }
         if (followedBy != null) {
           followedBy();
         }
       }
     }
   }
 
+  /// Visit a comma-separated list of [nodes] if not null.
+  visitCommaSeparatedNodes(NodeList<ASTNode> nodes) {
+    if (nodes != null) {
+      var size = nodes.length;
+      if (size > 0) {
+        var node;
+        for (var i = 0; i < size; i++) {
+          node = nodes[i];
+          if (i > 0) {
+            var comma = node.beginToken.previous;
+            token(comma);
+            space();
+          }
+          node.accept(this);
+        }
+      }
+    }
+  }
+
+
   /// Visit a [node], and if not null, optionally preceded or followed by the
   /// specified functions.
   visitNode(ASTNode node, {precededBy(): null, followedBy(): null}) {
     if (node != null) {
       if (precededBy != null) {
         precededBy();
       }
       node.accept(this);
       if (followedBy != null) {
         followedBy();
@@ skipping 10 matching lines @@
 
   /// Indicate that at least one newline should be emitted and possibly more
   /// if the source has them.
   newlines() {
     needsNewline = true;
   }
 
   /// Optionally emit a trailing comma.
   optionalTrailingComma(Token rightBracket) {
     if (rightBracket.previous.lexeme == ',') {
-      comma();
+      token(rightBracket.previous);
     }
   }
 
   token(Token token, {precededBy(), followedBy(), int minNewlines: 0}) {
     if (token != null) {
       if (needsNewline) {
         minNewlines = max(1, minNewlines);
       }
       var emitted = emitPrecedingCommentsAndNewlines(token, min: minNewlines);
       if (emitted > 0) {
@@ skipping 25 matching lines @@
       var overshot = token.offset - preSelection.offset;
       if (overshot >= 0) {
         //TODO(pquitslund): update length (may need truncating)
         selection = new Selection(
             writer.toString().length + leadingSpaces - overshot,
             preSelection.length);
       }
     }
   }
 
-  commaSeperator() {
-    comma();
-    space();
-  }
-
-  comma() {
-    writer.print(',');
-  }
-
-
   /// Emit a non-breakable space.
   space([n = 1]) {
     //TODO(pquitslund): replace with a proper space token
     leadingSpaces+=n;
   }
 
   /// Emit a breakable space
   breakableSpace() {
     //Implement
   }
@@ skipping 45 matching lines @@
     //Handle EOLs before newlines
     if (isAtEOL(comment)) {
       emitComment(comment, previousToken);
       comment = comment.next;
       currentToken = comment != null ? comment : token;
     }
 
     var lines = max(min, countNewlinesBetween(previousToken, currentToken));
     writer.newlines(lines);
 
-    previousToken = currentToken.previous;
+    previousToken =
+        currentToken.previous != null ? currentToken.previous : token.previous;

[Brian Wilkerson, 2013/12/04 19:52:50]

Unless you're creating your own tokens, "currentToken.previous" should never be
null (the beginning and end of the token stream is a special EOF token that
loops around to itself). Was this just defensive programming, or did you
actually see this return null at some point?

[pquitslund, 2013/12/04 21:09:10]

I believe I added this when I was creating token streams for testing.  I think I
didn't know about the looping around bit...  I'll do some investigating and see
if I can make this go away.

 
     while (comment != null) {
 
       emitComment(comment, previousToken);
 
       var nextToken = comment.next != null ? comment.next : token;
       var newlines = calculateNewlinesBetweenComments(comment, nextToken);
       if (newlines > 0) {
         writer.newlines(newlines);
         lines += newlines;
-      } else if (!isEOF(token)) {
-        append(' ');
+      } else {
+        var spaces = countSpacesBetween(comment, nextToken);
+        if (spaces > 0) {
+          space();
+        }
       }
 
       previousToken = comment;
       comment = comment.next;
     }
 
     previousToken = token;
     return lines;
   }
 
-
   ensureTrailingNewline() {
     if (writer.lastToken is! NewlineToken) {
       writer.newline();
     }
   }
 
 
   /// Test if this [comment] is at the end of a line.
   bool isAtEOL(Token comment) =>
       comment != null && comment.toString().trim().startsWith(twoSlashes) &&
       sameLine(comment, previousToken);
 
   /// Emit this [comment], inserting leading whitespace if appropriate.
   emitComment(Token comment, Token previousToken) {
-    if (!writer.currentLine.isWhitespace() && !isBlock(comment)) {
+    if (!writer.currentLine.isWhitespace() && previousToken != null) {
       var ws = countSpacesBetween(previousToken, comment);
       // Preserve one space but no more
-      if (ws > 0) {
-        append(' ');
+      if (ws > 0 && leadingSpaces == 0) {
+        space();
       }
     }
 
     append(comment.toString().trim());
   }
 
   /// Count spaces between these tokens.  Tokens on different lines return 0.
   int countSpacesBetween(Token last, Token current) => isEOF(last) ||
       countNewlinesBetween(last, current) > 0 ? 0 : current.offset - last.end;
 
@@ skipping 57 matching lines @@
     var lastLine =
         lineInfo.getLocation(lastOffset).lineNumber;
     var currentLine =
         lineInfo.getLocation(currentOffset).lineNumber;
     return currentLine - lastLine;
   }
 
   String toString() => writer.toString();
 
 }