Improve precision of JS printer callbacks (2nd try)

pkg/js_ast/lib/src/printer.dart

 // Copyright (c) 2012, 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.
 
 part of js_ast;
 
 
 class JavaScriptPrintingOptions {
   final bool shouldCompressOutput;
   final bool minifyLocalVariables;
   final bool preferSemicolonToNewlineInMinifiedOutput;
 
   JavaScriptPrintingOptions(
       {this.shouldCompressOutput: false,
        this.minifyLocalVariables: false,
        this.preferSemicolonToNewlineInMinifiedOutput: false});
 }
 
 
 /// An environment in which JavaScript printing is done.  Provides emitting of
 /// text and pre- and post-visit callbacks.
 abstract class JavaScriptPrintingContext {
   /// Signals an error.  This should happen only for serious internal errors.
   void error(String message) { throw message; }
 
   /// Adds [string] to the output.
   void emit(String string);
 
-  /// Callback immediately before printing [node].  Whitespace may be printed
-  /// after this callback before the first non-whitespace character for [node].
-  void enterNode(Node node) {}
-  /// Callback after printing the last character representing [node].
-  void exitNode(Node node) {}
+  /// Callback for the start of printing of [node]. [startPosition] is the
+  /// position of the first non-whitespace character of [node].
+  ///
+  /// [enterNode] is called in pre-traversal order.
+  void enterNode(Node node, int startPosition) {}
+
+  /// Callback for the end of printing of [node]. [startPosition] is the
+  /// position of the first non-whitespace character of [node] (also provided
+  /// in the [enterNode] callback), [endPosition] is the position immediately
+  /// following the last character of [node]. [delimiterPosition] is the
+  /// position of the ending delimiter of [node]. This is only provided for
+  /// [Fun] nodes and is `null` otherwise.
+  ///
+  /// [enterNode] is called in post-traversal order.
+  void exitNode(Node node,
+                int startPosition,
+                int endPosition,
+                int delimiterPosition) {}

[sra1, 2015/04/14 21:54:44]

Perhaps call this 'closingPosition' (throughout)
'delimiter' marks the boundary between separate things, i.e. is outside the
things delimited.

[Johnni Winther, 2015/04/15 11:15:28]

Done.

 }
 
 /// A simple implementation of [JavaScriptPrintingContext] suitable for tests.
 class SimpleJavaScriptPrintingContext extends JavaScriptPrintingContext {
   final StringBuffer buffer = new StringBuffer();
 
   void emit(String string) {
     buffer.write(string);
   }
 
   String getText() => buffer.toString();
 }
 
 
 class Printer implements NodeVisitor {
   final JavaScriptPrintingOptions options;
   final JavaScriptPrintingContext context;
   final bool shouldCompressOutput;
   final DanglingElseVisitor danglingElseVisitor;
   final LocalNamer localNamer;
 
+  int _charCount = 0;
   bool inForInit = false;
   bool atStatementBegin = false;
   bool pendingSemicolon = false;
   bool pendingSpace = false;
 
   // The current indentation level.
   int _indentLevel = 0;
   // A cache of all indentation strings used so far.
   List<String> _indentList = <String>[""];
 
@@ skipping 26 matching lines @@
   }
 
   void indentMore() {
     _indentLevel++;
   }
 
   void indentLess() {
     _indentLevel--;
   }
 
-
   /// Always emit a newline, even under `enableMinification`.
   void forceLine() {
-    out("\n");
+    out("\n", isWhitespace: true);
   }
+
   /// Emits a newline for readability.
   void lineOut() {
     if (!shouldCompressOutput) forceLine();
   }
+
   void spaceOut() {
-    if (!shouldCompressOutput) out(" ");
+    if (!shouldCompressOutput) out(" ", isWhitespace: true);
   }
 
   String lastAddedString = null;
+
   int get lastCharCode {
     if (lastAddedString == null) return 0;
     assert(lastAddedString.length != "");
     return lastAddedString.codeUnitAt(lastAddedString.length - 1);
   }
 
-  void out(String str) {
+  void out(String str, {bool isWhitespace: false}) {
     if (str != "") {
       if (pendingSemicolon) {
         if (!shouldCompressOutput) {
-          context.emit(";");
+          _emit(";");
         } else if (str != "}") {
           // We want to output newline instead of semicolon because it makes
           // the raw stack traces much easier to read and it also makes line-
           // based tools like diff work much better.  JavaScript will
           // automatically insert the semicolon at the newline if it means a
           // parsing error is avoided, so we can only do this trick if the
           // next line is not something that can be glued onto a valid
           // expression to make a new valid expression.
 
           // If we're using the new emitter where most pretty printed code
           // is escaped in strings, it is a lot easier to deal with semicolons
           // than newlines because the former doesn't need escaping.
           if (options.preferSemicolonToNewlineInMinifiedOutput ||
               expressionContinuationRegExp.hasMatch(str)) {
-            context.emit(";");
+            _emit(";");
           } else {
-            context.emit("\n");
+            _emit("\n");
           }
         }
       }
       if (pendingSpace &&
           (!shouldCompressOutput || identifierCharacterRegExp.hasMatch(str))) {
-        context.emit(" ");
+        _emit(" ");
       }
       pendingSpace = false;
       pendingSemicolon = false;
-      context.emit(str);
+      if (!isWhitespace) {
+        enterNode();
+      }
+      _emit(str);
       lastAddedString = str;
     }
   }
 
   void outLn(String str) {
     out(str);
     lineOut();
   }
 
   void outSemicolonLn() {
     if (shouldCompressOutput) {
       pendingSemicolon = true;
     } else {
       out(";");
       forceLine();
     }
   }
 
-  void outIndent(String str) { indent(); out(str); }
-  void outIndentLn(String str) { indent(); outLn(str); }
+  void outIndent(String str) {
+    indent(); out(str);

[sra1, 2015/04/14 19:14:00]

two lines

[Johnni Winther, 2015/04/15 11:15:28]

Done.

+  }
+
+  void outIndentLn(String str) {
+    indent(); outLn(str);
+  }
+
   void indent() {
     if (!shouldCompressOutput) {
-      out(indentation);
+      out(indentation, isWhitespace: true);
     }
   }
 
-  visit(Node node) {
-    context.enterNode(node);
-    node.accept(this);
-    context.exitNode(node);
+  EnterExitNode currentNode;
+
+  void _emit(String text) {
+    context.emit(text);
+    _charCount += text.length;
   }
 
-  visitCommaSeparated(List<Node> nodes, int hasRequiredType,
-                      {bool newInForInit, bool newAtStatementBegin}) {
+  void startNode(Node node) {
+    currentNode = new EnterExitNode(currentNode, node);
+  }
+
+  void enterNode() {
+    currentNode.enterNode(context, _charCount);
+  }
+
+  void endNode(Node node) {
+    //print('currentNode.node=${currentNode.node} node=$node');

[floitsch, 2015/04/14 15:09:31]

debugprint.

[Johnni Winther, 2015/04/15 11:15:28]

Removed.

+    assert(currentNode.node == node);
+    currentNode = currentNode.exitNode(context, _charCount);
+  }
+
+  void visit(Node node) {
+    startNode(node);
+    node.accept(this);
+    endNode(node);
+  }
+
+  void visitCommaSeparated(List<Node> nodes, int hasRequiredType,
+                           {bool newInForInit, bool newAtStatementBegin}) {
     for (int i = 0; i < nodes.length; i++) {
       if (i != 0) {
         atStatementBegin = false;
         out(",");
         spaceOut();
       }
       visitNestedExpression(nodes[i], hasRequiredType,
                             newInForInit: newInForInit,
                             newAtStatementBegin: newAtStatementBegin);
     }
   }
 
-  visitAll(List<Node> nodes) {
+  void visitAll(List<Node> nodes) {
     nodes.forEach(visit);
   }
 
-  visitProgram(Program program) {
-    visitAll(program.body);
+  @override
+  void visitProgram(Program program) {
+    if (program.body.isNotEmpty) {
+      visitAll(program.body);
+    }
   }
 
   Statement unwrapBlockIfSingleStatement(Statement body) {
     Statement result = body;
     while (result is Block) {
       Block block = result;
       if (block.statements.length != 1) break;
       result = block.statements.single;
     }
     return result;
   }
 
   bool blockBody(Statement body, {bool needsSeparation, bool needsNewline}) {
     if (body is Block) {
       spaceOut();
       blockOut(body, shouldIndent: false, needsNewline: needsNewline);
       return true;
     }
     if (shouldCompressOutput && needsSeparation) {
       // If [shouldCompressOutput] is false, then the 'lineOut' will insert
       // the separation.
-      out(" ");
+      out(" ", isWhitespace: true);
     } else {
       lineOut();
     }
     indentMore();
     visit(body);
     indentLess();
     return false;
   }
 
   void blockOutWithoutBraces(Node node) {
     if (node is Block) {
-      context.enterNode(node);
+      startNode(node);
       Block block = node;
       block.statements.forEach(blockOutWithoutBraces);
-      context.exitNode(node);
+      endNode(node);
     } else {
       visit(node);
     }
   }
 
-  void blockOut(Block node, {bool shouldIndent, bool needsNewline}) {
+  int blockOut(Block node, {bool shouldIndent, bool needsNewline}) {
     if (shouldIndent) indent();
-    context.enterNode(node);
+    startNode(node);
     out("{");
     lineOut();
     indentMore();
     node.statements.forEach(blockOutWithoutBraces);
     indentLess();
     indent();
     out("}");
-    context.exitNode(node);
+    int delimiterPosition = _charCount - 1;
+    endNode(node);
     if (needsNewline) lineOut();
+    return delimiterPosition;
   }
 
-  visitBlock(Block block) {
+  @override
+  void visitBlock(Block block) {
     blockOut(block, shouldIndent: true, needsNewline: true);
   }
 
-  visitExpressionStatement(ExpressionStatement expressionStatement) {
+  @override
+  void visitExpressionStatement(ExpressionStatement node) {
     indent();
-    visitNestedExpression(expressionStatement.expression, EXPRESSION,
+    visitNestedExpression(node.expression, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: true);
     outSemicolonLn();
   }
 
-  visitEmptyStatement(EmptyStatement nop) {
+  @override
+  void visitEmptyStatement(EmptyStatement node) {
     outIndentLn(";");
   }
 
   void ifOut(If node, bool shouldIndent) {
     Statement then = unwrapBlockIfSingleStatement(node.then);
     Statement elsePart = node.otherwise;
     bool hasElse = node.hasElse;
 
     // Handle dangling elses and a work-around for Android 4.0 stock browser.
     // Android 4.0 requires braces for a single do-while in the `then` branch.
@@ skipping 23 matching lines @@
       if (elsePart is If) {
         pendingSpace = true;
         ifOut(elsePart, false);
       } else {
         blockBody(unwrapBlockIfSingleStatement(elsePart),
                   needsSeparation: true, needsNewline: true);
       }
     }
   }
 
-  visitIf(If node) {
+  @override
+  void visitIf(If node) {
     ifOut(node, true);
   }
 
-  visitFor(For loop) {
+  @override
+  void visitFor(For loop) {
     outIndent("for");
     spaceOut();
     out("(");
     if (loop.init != null) {
       visitNestedExpression(loop.init, EXPRESSION,
                             newInForInit: true, newAtStatementBegin: false);
     }
     out(";");
     if (loop.condition != null) {
       spaceOut();
       visitNestedExpression(loop.condition, EXPRESSION,
                             newInForInit: false, newAtStatementBegin: false);
     }
     out(";");
     if (loop.update != null) {
       spaceOut();
       visitNestedExpression(loop.update, EXPRESSION,
                             newInForInit: false, newAtStatementBegin: false);
     }
     out(")");
     blockBody(unwrapBlockIfSingleStatement(loop.body),
               needsSeparation: false, needsNewline: true);
   }
 
-  visitForIn(ForIn loop) {
+  @override
+  void visitForIn(ForIn loop) {
     outIndent("for");
     spaceOut();
     out("(");
     visitNestedExpression(loop.leftHandSide, EXPRESSION,
                           newInForInit: true, newAtStatementBegin: false);
     out(" in");
     pendingSpace = true;
     visitNestedExpression(loop.object, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     out(")");
     blockBody(unwrapBlockIfSingleStatement(loop.body),
               needsSeparation: false, needsNewline: true);
   }
 
-  visitWhile(While loop) {
+  @override
+  void visitWhile(While loop) {
     outIndent("while");
     spaceOut();
     out("(");
     visitNestedExpression(loop.condition, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     out(")");
     blockBody(unwrapBlockIfSingleStatement(loop.body),
               needsSeparation: false, needsNewline: true);
   }
 
-  visitDo(Do loop) {
+  @override
+  void visitDo(Do loop) {
     outIndent("do");
     if (blockBody(unwrapBlockIfSingleStatement(loop.body),
                   needsSeparation: true, needsNewline: false)) {
       spaceOut();
     } else {
       indent();
     }
     out("while");
     spaceOut();
     out("(");
     visitNestedExpression(loop.condition, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     out(")");
     outSemicolonLn();
   }
 
-  visitContinue(Continue node) {
+  @override
+  void visitContinue(Continue node) {
     if (node.targetLabel == null) {
       outIndent("continue");
     } else {
       outIndent("continue ${node.targetLabel}");
     }
     outSemicolonLn();
   }
 
-  visitBreak(Break node) {
+  @override
+  void visitBreak(Break node) {
     if (node.targetLabel == null) {
       outIndent("break");
     } else {
       outIndent("break ${node.targetLabel}");
     }
     outSemicolonLn();
   }
 
-  visitReturn(Return node) {
+  @override
+  void visitReturn(Return node) {
     if (node.value == null) {
       outIndent("return");
     } else {
       outIndent("return");
       pendingSpace = true;
       visitNestedExpression(node.value, EXPRESSION,
                             newInForInit: false, newAtStatementBegin: false);
     }
     outSemicolonLn();
   }
 
-  visitDartYield(DartYield node) {
+  @override
+  void visitDartYield(DartYield node) {
     if (node.hasStar) {
       outIndent("yield*");
     } else {
       outIndent("yield");
     }
     pendingSpace = true;
     visitNestedExpression(node.expression, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     outSemicolonLn();
   }
 
-
-  visitThrow(Throw node) {
+  @override
+  void visitThrow(Throw node) {
     outIndent("throw");
     pendingSpace = true;
     visitNestedExpression(node.expression, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     outSemicolonLn();
   }
 
-  visitTry(Try node) {
+  @override
+  void visitTry(Try node) {
     outIndent("try");
     blockBody(node.body, needsSeparation: true, needsNewline: false);
     if (node.catchPart != null) {
       visit(node.catchPart);
     }
     if (node.finallyPart != null) {
       spaceOut();
       out("finally");
       blockBody(node.finallyPart, needsSeparation: true, needsNewline: true);
     } else {
       lineOut();
     }
   }
 
-  visitCatch(Catch node) {
+  @override
+  void visitCatch(Catch node) {
     spaceOut();
     out("catch");
     spaceOut();
     out("(");
     visitNestedExpression(node.declaration, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     out(")");
     blockBody(node.body, needsSeparation: false, needsNewline: false);
   }
 
-  visitSwitch(Switch node) {
+  @override
+  void visitSwitch(Switch node) {
     outIndent("switch");
     spaceOut();
     out("(");
     visitNestedExpression(node.key, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     out(")");
     spaceOut();
     outLn("{");
     indentMore();
     visitAll(node.cases);
     indentLess();
     outIndentLn("}");
   }
 
-  visitCase(Case node) {
+  @override
+  void visitCase(Case node) {
     outIndent("case");
     pendingSpace = true;
     visitNestedExpression(node.expression, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     outLn(":");
     if (!node.body.statements.isEmpty) {
       indentMore();
       blockOutWithoutBraces(node.body);
       indentLess();
     }
   }
 
-  visitDefault(Default node) {
+  @override
+  void visitDefault(Default node) {
     outIndentLn("default:");
     if (!node.body.statements.isEmpty) {
       indentMore();
       blockOutWithoutBraces(node.body);
       indentLess();
     }
   }
 
-  visitLabeledStatement(LabeledStatement node) {
+  @override
+  void visitLabeledStatement(LabeledStatement node) {
     Statement body = unwrapBlockIfSingleStatement(node.body);
     // `label: break label;`
     // Does not work on IE. The statement is a nop, so replace it by an empty
     // statement.
     // See:
     // https://connect.microsoft.com/IE/feedback/details/891889/parser-bugs
     if (body is Break && body.targetLabel == node.label) {
       visit(new EmptyStatement());
       return;
     }
     outIndent("${node.label}:");
     blockBody(body, needsSeparation: false, needsNewline: true);
   }
 
-  void functionOut(Fun fun, Node name, VarCollector vars) {
+  int functionOut(Fun fun, Node name, VarCollector vars) {
     out("function");
     if (name != null) {
       out(" ");
       // Name must be a [Decl]. Therefore only test for primary expressions.
       visitNestedExpression(name, PRIMARY,
                             newInForInit: false, newAtStatementBegin: false);
     }
     localNamer.enterScope(vars);
     out("(");
     if (fun.params != null) {
       visitCommaSeparated(fun.params, PRIMARY,
                           newInForInit: false, newAtStatementBegin: false);
     }
     out(")");
     switch (fun.asyncModifier) {
       case const AsyncModifier.sync():
         break;
       case const AsyncModifier.async():
-        out(' async');
+        out(' ', isWhitespace: true); out('async');

[sra1, 2015/04/14 19:14:00]

multiple lines

[Johnni Winther, 2015/04/15 11:15:28]

Done.

         break;
       case const AsyncModifier.syncStar():
-        out(' sync*');
+        out(' ', isWhitespace: true); out('sync*');
         break;
       case const AsyncModifier.asyncStar():
-        out(' async*');
+        out(' ', isWhitespace: true); out('async*');
         break;
     }
-    blockBody(fun.body, needsSeparation: false, needsNewline: false);
+    spaceOut();
+    int delimiterPosition =
+        blockOut(fun.body, shouldIndent: false, needsNewline: false);
     localNamer.leaveScope();
+    return delimiterPosition;
+
   }
 
+  @override
   visitFunctionDeclaration(FunctionDeclaration declaration) {
     VarCollector vars = new VarCollector();
     vars.visitFunctionDeclaration(declaration);
     indent();
     functionOut(declaration.function, declaration.name, vars);
     lineOut();
   }
 
   visitNestedExpression(Expression node, int requiredPrecedence,
                         {bool newInForInit, bool newAtStatementBegin}) {
@@ skipping 14 matching lines @@
       out("(");
       visit(node);
       out(")");
     } else {
       inForInit = newInForInit;
       atStatementBegin = newAtStatementBegin;
       visit(node);
     }
   }
 
+  @override
   visitVariableDeclarationList(VariableDeclarationList list) {
     out("var ");
     visitCommaSeparated(list.declarations, ASSIGNMENT,
                         newInForInit: inForInit, newAtStatementBegin: false);
   }
 
+  @override
   visitAssignment(Assignment assignment) {
     visitNestedExpression(assignment.leftHandSide, LEFT_HAND_SIDE,
                           newInForInit: inForInit,
                           newAtStatementBegin: atStatementBegin);
     if (assignment.value != null) {
       spaceOut();
       String op = assignment.op;
       if (op != null) out(op);
       out("=");
       spaceOut();
       visitNestedExpression(assignment.value, ASSIGNMENT,
                             newInForInit: inForInit,
                             newAtStatementBegin: false);
     }
   }
 
+  @override
   visitVariableInitialization(VariableInitialization initialization) {
     visitAssignment(initialization);
   }
 
+  @override
   visitConditional(Conditional cond) {
     visitNestedExpression(cond.condition, LOGICAL_OR,
                           newInForInit: inForInit,
                           newAtStatementBegin: atStatementBegin);
     spaceOut();
     out("?");
     spaceOut();
     // The then part is allowed to have an 'in'.
     visitNestedExpression(cond.then, ASSIGNMENT,
                           newInForInit: false, newAtStatementBegin: false);
     spaceOut();
     out(":");
     spaceOut();
     visitNestedExpression(cond.otherwise, ASSIGNMENT,
                           newInForInit: inForInit, newAtStatementBegin: false);
   }
 
+  @override
   visitNew(New node) {
     out("new ");
     visitNestedExpression(node.target, CALL,
                           newInForInit: inForInit, newAtStatementBegin: false);
     out("(");
     visitCommaSeparated(node.arguments, ASSIGNMENT,
                         newInForInit: false, newAtStatementBegin: false);
     out(")");
   }
 
+  @override
   visitCall(Call call) {
     visitNestedExpression(call.target, LEFT_HAND_SIDE,
                           newInForInit: inForInit,
                           newAtStatementBegin: atStatementBegin);
     out("(");
     visitCommaSeparated(call.arguments, ASSIGNMENT,
                         newInForInit: false, newAtStatementBegin: false);
     out(")");
   }
 
-  visitBinary(Binary binary) {
+  @override
+  void visitBinary(Binary binary) {
     Expression left = binary.left;
     Expression right = binary.right;
     String op = binary.op;
     int leftPrecedenceRequirement;
     int rightPrecedenceRequirement;
     bool leftSpace = true;   // left<HERE>op right
     switch (op) {
       case ',':
         //  x, (y, z) <=> (x, y), z.
         leftPrecedenceRequirement = EXPRESSION;
@@ skipping 68 matching lines @@
         context.error("Forgot operator: $op");
     }
 
     visitNestedExpression(left, leftPrecedenceRequirement,
                           newInForInit: inForInit,
                           newAtStatementBegin: atStatementBegin);
 
     if (op == "in" || op == "instanceof") {
       // There are cases where the space is not required but without further
       // analysis we cannot know.
-      out(" ");
+      out(" ", isWhitespace: true);
       out(op);
-      out(" ");
+      out(" ", isWhitespace: true);
     } else {
       if (leftSpace) spaceOut();
       out(op);
       spaceOut();
     }
     visitNestedExpression(right, rightPrecedenceRequirement,
                           newInForInit: inForInit,
                           newAtStatementBegin: false);
   }
 
-  visitPrefix(Prefix unary) {
+  @override
+  void visitPrefix(Prefix unary) {
     String op = unary.op;
     switch (op) {
       case "delete":
       case "void":
       case "typeof":
         // There are cases where the space is not required but without further
         // analysis we cannot know.
         out(op);
-        out(" ");
+        out(" ", isWhitespace: true);
         break;
       case "+":
       case "++":
-        if (lastCharCode == charCodes.$PLUS) out(" ");
+        if (lastCharCode == charCodes.$PLUS) out(" ", isWhitespace: true);
         out(op);
         break;
       case "-":
       case "--":
-        if (lastCharCode == charCodes.$MINUS) out(" ");
+        if (lastCharCode == charCodes.$MINUS) out(" ", isWhitespace: true);
         out(op);
         break;
       default:
         out(op);
     }
     visitNestedExpression(unary.argument, UNARY,
                           newInForInit: inForInit, newAtStatementBegin: false);
   }
 
-  visitPostfix(Postfix postfix) {
+  @override
+  void visitPostfix(Postfix postfix) {
     visitNestedExpression(postfix.argument, LEFT_HAND_SIDE,
                           newInForInit: inForInit,
                           newAtStatementBegin: atStatementBegin);
     out(postfix.op);
   }
 
-  visitVariableUse(VariableUse ref) {
+  @override
+  void visitVariableUse(VariableUse ref) {
     out(localNamer.getName(ref.name));
   }
 
-  visitThis(This node) {
+  @override
+  void visitThis(This node) {
     out("this");
   }
 
-  visitVariableDeclaration(VariableDeclaration decl) {
+  @override
+  void visitVariableDeclaration(VariableDeclaration decl) {
     out(localNamer.getName(decl.name));
   }
 
-  visitParameter(Parameter param) {
+  @override
+  void visitParameter(Parameter param) {
     out(localNamer.getName(param.name));
   }
 
   bool isDigit(int charCode) {
     return charCodes.$0 <= charCode && charCode <= charCodes.$9;
   }
 
   bool isValidJavaScriptId(String field) {
     if (field.length < 3) return false;
     // Ignore the leading and trailing string-delimiter.
     for (int i = 1; i < field.length - 1; i++) {
       // TODO(floitsch): allow more characters.
       int charCode = field.codeUnitAt(i);
       if (!(charCodes.$a <= charCode && charCode <= charCodes.$z ||
             charCodes.$A <= charCode && charCode <= charCodes.$Z ||
             charCode == charCodes.$$ ||
             charCode == charCodes.$_ ||
             i != 1 && isDigit(charCode))) {
         return false;
       }
     }
     // TODO(floitsch): normally we should also check that the field is not a
     // reserved word.  We don't generate fields with reserved word names except
     // for 'super'.
     if (field == '"super"') return false;
     return true;
   }
 
-  visitAccess(PropertyAccess access) {
+  @override
+  void visitAccess(PropertyAccess access) {
     visitNestedExpression(access.receiver, CALL,
                           newInForInit: inForInit,
                           newAtStatementBegin: atStatementBegin);
     Node selector = access.selector;
     if (selector is LiteralString) {
       LiteralString selectorString = selector;
       String fieldWithQuotes = selectorString.value;
       if (isValidJavaScriptId(fieldWithQuotes)) {
-        if (access.receiver is LiteralNumber) out(" ");
+        if (access.receiver is LiteralNumber) out(" ", isWhitespace: true);
         out(".");
         out(fieldWithQuotes.substring(1, fieldWithQuotes.length - 1));
         return;
       }
     }
     out("[");
     visitNestedExpression(selector, EXPRESSION,
                           newInForInit: false, newAtStatementBegin: false);
     out("]");
   }
 
-  visitNamedFunction(NamedFunction namedFunction) {
+  @override
+  void visitNamedFunction(NamedFunction namedFunction) {
     VarCollector vars = new VarCollector();
     vars.visitNamedFunction(namedFunction);
-    functionOut(namedFunction.function, namedFunction.name, vars);
+    startNode(namedFunction.function);
+    currentNode.delimiterPosition =
+        functionOut(namedFunction.function, namedFunction.name, vars);
+    endNode(namedFunction.function);
   }
 
-  visitFun(Fun fun) {
+  @override
+  void visitFun(Fun fun) {
     VarCollector vars = new VarCollector();
     vars.visitFun(fun);
-    functionOut(fun, null, vars);
+    currentNode.delimiterPosition = functionOut(fun, null, vars);
   }
 
-  visitLiteralBool(LiteralBool node) {
+  @override
+  void visitLiteralBool(LiteralBool node) {
     out(node.value ? "true" : "false");
   }
 
-  visitLiteralString(LiteralString node) {
+  @override
+  void visitLiteralString(LiteralString node) {
     out(node.value);
   }
 
-  visitLiteralNumber(LiteralNumber node) {
+  @override
+  void visitLiteralNumber(LiteralNumber node) {
     int charCode = node.value.codeUnitAt(0);
     if (charCode == charCodes.$MINUS && lastCharCode == charCodes.$MINUS) {
-      out(" ");
+      out(" ", isWhitespace: true);
     }
     out(node.value);
   }
 
-  visitLiteralNull(LiteralNull node) {
+  @override
+  void visitLiteralNull(LiteralNull node) {
     out("null");
   }
 
-  visitArrayInitializer(ArrayInitializer node) {
+  @override
+  void visitArrayInitializer(ArrayInitializer node) {
     out("[");
     List<Expression> elements = node.elements;
     for (int i = 0; i < elements.length; i++) {
       Expression element = elements[i];
       if (element is ArrayHole) {
         // Note that array holes must have a trailing "," even if they are
         // in last position. Otherwise `[,]` (having length 1) would become
         // equal to `[]` (the empty array)
         // and [1,,] (array with 1 and a hole) would become [1,] = [1].
         out(",");
         continue;
       }
       if (i != 0) spaceOut();
       visitNestedExpression(element, ASSIGNMENT,
                             newInForInit: false, newAtStatementBegin: false);
       // We can skip the trailing "," for the last element (since it's not
       // an array hole).
       if (i != elements.length - 1) out(",");
     }
     out("]");
   }
 
-  visitArrayHole(ArrayHole node) {
+  @override
+  void visitArrayHole(ArrayHole node) {
     throw "Unreachable";
   }
 
-  visitObjectInitializer(ObjectInitializer node) {
+  @override
+  void visitObjectInitializer(ObjectInitializer node) {
     // Print all the properties on one line until we see a function-valued
     // property.  Ideally, we would use a proper pretty-printer to make the
     // decision based on layout.
     List<Property> properties = node.properties;
     out("{");
     indentMore();
     for (int i = 0; i < properties.length; i++) {
       Expression value = properties[i].value;
       if (i != 0) {
         out(",");
         if (node.isOneLiner) spaceOut();
       }
       if (!node.isOneLiner) {
         forceLine();
         indent();
       }
       visit(properties[i]);
     }
     indentLess();
     if (!node.isOneLiner && !properties.isEmpty) {
       lineOut();
       indent();
     }
     out("}");
   }
 
-  visitProperty(Property node) {
+  @override
+  void visitProperty(Property node) {
     if (node.name is LiteralString) {
       LiteralString nameString = node.name;
       String name = nameString.value;
       if (isValidJavaScriptId(name)) {
         out(name.substring(1, name.length - 1));
       } else {
         out(name);
       }
     } else {
       assert(node.name is LiteralNumber);
       LiteralNumber nameNumber = node.name;
       out(nameNumber.value);
     }
     out(":");
     spaceOut();
     visitNestedExpression(node.value, ASSIGNMENT,
                           newInForInit: false, newAtStatementBegin: false);
   }
 
-  visitRegExpLiteral(RegExpLiteral node) {
+  @override
+  void visitRegExpLiteral(RegExpLiteral node) {
     out(node.pattern);
   }
 
-  visitLiteralExpression(LiteralExpression node) {
+  @override
+  void visitLiteralExpression(LiteralExpression node) {
     String template = node.template;
     List<Expression> inputs = node.inputs;
 
     List<String> parts = template.split('#');
     int inputsLength = inputs == null ? 0 : inputs.length;
     if (parts.length != inputsLength + 1) {
       context.error('Wrong number of arguments for JS: $template');
     }
     // Code that uses JS must take care of operator precedences, and
     // put parenthesis if needed.
     out(parts[0]);
     for (int i = 0; i < inputsLength; i++) {
       visit(inputs[i]);
       out(parts[i + 1]);
     }
   }
 
-  visitLiteralStatement(LiteralStatement node) {
+  @override
+  void visitLiteralStatement(LiteralStatement node) {
     outLn(node.code);
   }
 
-  visitInterpolatedNode(InterpolatedNode node) {
+  void visitInterpolatedNode(InterpolatedNode node) {
     out('#${node.nameOrPosition}');
   }
 
-  visitInterpolatedExpression(InterpolatedExpression node) =>
+  @override
+  void visitInterpolatedExpression(InterpolatedExpression node) =>
       visitInterpolatedNode(node);
 
-  visitInterpolatedLiteral(InterpolatedLiteral node) =>
+  @override
+  void visitInterpolatedLiteral(InterpolatedLiteral node) =>
       visitInterpolatedNode(node);
 
-  visitInterpolatedParameter(InterpolatedParameter node) =>
+  @override
+  void visitInterpolatedParameter(InterpolatedParameter node) =>
       visitInterpolatedNode(node);
 
-  visitInterpolatedSelector(InterpolatedSelector node) =>
+  @override
+  void visitInterpolatedSelector(InterpolatedSelector node) =>
       visitInterpolatedNode(node);
 
-  visitInterpolatedStatement(InterpolatedStatement node) {
+  @override
+  void visitInterpolatedStatement(InterpolatedStatement node) {
     outLn('#${node.nameOrPosition}');
   }
 
-  visitInterpolatedDeclaration(InterpolatedDeclaration node) {
+  @override
+  void visitInterpolatedDeclaration(InterpolatedDeclaration node) {
     visitInterpolatedNode(node);
   }
 
+  @override
   void visitComment(Comment node) {
     if (shouldCompressOutput) return;
     String comment = node.comment.trim();
     if (comment.isEmpty) return;
     for (var line in comment.split('\n')) {
       if (comment.startsWith('//')) {
         outIndentLn(line.trim());
       } else {
         outIndentLn('// ${line.trim()}');
       }
     }
   }
 
+  @override
   void visitAwait(Await node) {
     out("await ");
     visit(node.expression);
   }
 }
 
 
 class OrderedSet<T> {
   final Set<T> set;
   final List<T> list;
 
   OrderedSet() : set = new Set<T>(), list = <T>[];
 
   void add(T x) {
-    if (!set.contains(x)) {
-      set.add(x);
+    if (set.add(x)) {
+      // [Set.add] returns `true` if 'x' was added.
       list.add(x);
     }
   }
 
   void forEach(void fun(T x)) {
     list.forEach(fun);
   }
 }
 
 // Collects all the var declarations in the function.  We need to do this in a
@@ skipping 227 matching lines @@
         codes.add(nthLetter((n ~/ nameSpaceSize) % LETTERS));
       }
       codes.add(charCodes.$0 + digit);
       newName = new String.fromCharCodes(codes);
     }
     assert(new RegExp(r'[a-zA-Z][a-zA-Z0-9]*').hasMatch(newName));
     maps.last[oldName] = newName;
     return newName;
   }
 }
+
+/// Information pertaining the enter and exit callbacks for [node].
+class EnterExitNode {
+  final EnterExitNode parent;
+  final Node node;
+
+  int startPosition;
+  int delimiterPosition;
+
+  EnterExitNode(this.parent, this.node);
+
+  void enterNode(JavaScriptPrintingContext context, int position) {

[sra1, 2015/04/14 19:14:00]

Perhaps 'addToNode' is a better name.
On an If node it is called for 'if' '(' ')' directly, and indirectly via the
parent chain of the condition expression for those fragments.

[Johnni Winther, 2015/04/15 11:15:28]

Done.

+    if (parent != null) {
+      parent.enterNode(context, position);

[sra1, 2015/04/14 19:14:00]

This multiplies the cost of printing by the tree height.
Can you see the cost in the emitter time?
Even if it does not show up, I think we need to fix this.

1. Can this test be moved under 'startPosition == null'? Adding a subsequent
fragment to a child node is also adding a non-first fragment to the parent.

2. Can we create nodes that know they are already within the parent to avoid the
parent call?

[sra1, 2015/04/14 21:54:44]

I ran this command 5 times in a row.  Emitting is slightly (~20ms) slower now.
for i in 1 2 3 4 5; do sdk/bin/dart2js -v samples-dev/swarm/swarm.dart  
--out=v1.js | grep emitter; done

Before:
Info: Code emitter took 693msec
Info: Code emitter took 699msec
Info: Code emitter took 680msec
Info: Code emitter took 695msec
Info: Code emitter took 687msec

After:
Info: Code emitter took 716msec
Info: Code emitter took 733msec
Info: Code emitter took 703msec
Info: Code emitter took 742msec
Info: Code emitter took 720msec

[Johnni Winther, 2015/04/15 11:15:28]

Moved the parent call under the `startPosition == null` condition. Emitter time
is immeasurable on Windows (fluctuate between 800-2500ms regardless of
strategy).

+    }
+    if (startPosition == null) {
+      startPosition = position;
+      context.enterNode(node, position);
+    }
+  }
+
+  EnterExitNode exitNode(JavaScriptPrintingContext context, int position) {
+    // Enter must happen before exit.
+    enterNode(context, position);
+    context.exitNode(node, startPosition, position, delimiterPosition);
+    return parent;
+  }
+}