Type annotations instead of closure comments.

lib/src/js/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;
+  final bool printTypes;

[Jennifer Messerly, 2016/02/09 01:21:33]

this sounds like a method name. Maybe "shouldPrintTypes" so it will read like
`if (options.shouldPrintTypes)`? Other ideas are "shouldOutputTypes" or
"shouldEmitTypes" in place of "print", since the class name contains print. Even
"includeTypes" might work `if (options.includeTypes) { ... }`


Up to you tho

[ochafik, 2016/02/10 18:12:45]

Thanks, went for shouldEmitTypes

   final bool allowSingleLineIfStatements;
 
   /// True to allow keywords in properties, such as `obj.var` or `obj.function`
   /// Modern JS engines support this.
   final bool allowKeywordsInProperties;
 
   JavaScriptPrintingOptions(
       {this.shouldCompressOutput: false,
        this.minifyLocalVariables: false,
        this.preferSemicolonToNewlineInMinifiedOutput: false,
+       this.printTypes: false,
        this.allowKeywordsInProperties: false,
        this.allowSingleLineIfStatements: 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; }
@@ skipping 13 matching lines @@
   final StringBuffer buffer = new StringBuffer();
 
   void emit(String string) {
     buffer.write(string);
   }
 
   String getText() => buffer.toString();
 }
 
 
-class Printer implements NodeVisitor {
+class Printer extends TypeScriptTypePrinter implements NodeVisitor {

[Jennifer Messerly, 2016/02/09 01:21:34]

Hmmm. I haven't seen the other class yet, but this inheritance direction feels
backwards :)
This is begging for a comment at least :)

[ochafik, 2016/02/10 18:12:45]

Good point, left a TODO for when/if we drop ClosureTypePrinter.

   final JavaScriptPrintingOptions options;
   final JavaScriptPrintingContext context;
   final bool shouldCompressOutput;
   final DanglingElseVisitor danglingElseVisitor;
   final LocalNamer localNamer;
 
   bool inForInit = false;
   bool atStatementBegin = false;
   bool inNewTarget = false;
   bool pendingSemicolon = false;
@@ skipping 468 matching lines @@
   void functionOut(Fun fun, Node name) {
     out("function");
     if (fun.isGenerator) out("*");
     if (name != null) {
       out(" ");
       // Name must be a [Decl]. Therefore only test for primary expressions.
       visitNestedExpression(name, PRIMARY,
                             newInForInit: false, newAtStatementBegin: false);
     }
     localNamer.enterScope(fun);
+    outTypeArgs(fun.typeArgs);
     out("(");
     if (fun.params != null) {
       visitCommaSeparated(fun.params, PRIMARY,
                           newInForInit: false, newAtStatementBegin: false);
     }
     out(")");
+    outTypeAnnotation(fun.returnType);
     switch (fun.asyncModifier) {
       case const AsyncModifier.sync():
         break;
       case const AsyncModifier.async():
         out(' async');
         break;
       case const AsyncModifier.syncStar():
         out(' sync*');
         break;
       case const AsyncModifier.asyncStar():
@@ skipping 34 matching lines @@
       out(")");
     } else {
       inForInit = newInForInit;
       atStatementBegin = newAtStatementBegin;
       visit(node);
     }
   }
 
   visitVariableDeclarationList(VariableDeclarationList list) {
     outClosureAnnotation(list);
-    out(list.keyword);
-    out(" ");
+    // Note: keyword can be null for non-static field declarations.
+    if (list.keyword != null) {
+      out(list.keyword);
+      out(" ");
+    }
     visitCommaSeparated(list.declarations, ASSIGNMENT,
                         newInForInit: inForInit, newAtStatementBegin: false);
   }
 
   visitArrayBindingPattern(ArrayBindingPattern node) {
     out("[");
     visitCommaSeparated(node.variables, EXPRESSION,
                         newInForInit: false, newAtStatementBegin: false);
     out("]");
   }
   visitObjectBindingPattern(ObjectBindingPattern node) {
     out("{");
     visitCommaSeparated(node.variables, EXPRESSION,
                         newInForInit: false, newAtStatementBegin: false);
     out("}");
   }
 
   visitDestructuredVariable(DestructuredVariable node) {
     var hasName = node.name != null;
     if (hasName) visit(node.name);
     if (node.structure != null) {
       if (hasName) {
         out(":");
         spaceOut();
       }
       visit(node.structure);
     }
+    outTypeAnnotation(node.type);
     if (node.defaultValue != null) {
       spaceOut();
       out("=");
       spaceOut();
       visitNestedExpression(node.defaultValue, EXPRESSION,
                             newInForInit: false, newAtStatementBegin: false);
     }
   }
 
   visitAssignment(Assignment assignment) {
@@ skipping 206 matching lines @@
   visitThis(This node) {
     out("this");
   }
 
   visitSuper(Super node) {
     out("super");
   }
 
   visitIdentifier(Identifier node) {
     out(localNamer.getName(node));
+    outTypeAnnotation(node.type);
   }
 
   visitRestParameter(RestParameter node) {
     out('...');
     visitIdentifier(node.parameter);
   }
 
   bool isDigit(int charCode) {
     return charCodes.$0 <= charCode && charCode <= charCodes.$9;
   }
@@ skipping 45 matching lines @@
   visitNamedFunction(NamedFunction namedFunction) {
     functionOut(namedFunction.function, namedFunction.name);
   }
 
   visitFun(Fun fun) {
     functionOut(fun, null);
   }
 
   visitArrowFun(ArrowFun fun) {
     localNamer.enterScope(fun);
-    if (fun.params.length == 1) {
+    if (fun.params.length == 1 &&
+        (fun.params.single.type == null || !options.printTypes)) {
       visitNestedExpression(fun.params.single, SPREAD,
           newInForInit: false, newAtStatementBegin: false);
     } else {
       out("(");
       visitCommaSeparated(fun.params, SPREAD,
           newInForInit: false, newAtStatementBegin: false);
       out(")");
     }
+    outTypeAnnotation(fun.returnType);
     spaceOut();
     out("=>");
     if (fun.body is Expression) {
       spaceOut();
       // Object initializers require parenthesis to disambiguate
       // AssignmentExpression from FunctionBody. See:
       // https://people.mozilla.org/~jorendorff/es6-draft.html#sec-arrow-function-definitions
       var needsParen = fun.body is ObjectInitializer;
       if (needsParen) out("(");
       visitNestedExpression(fun.body, ASSIGNMENT,
@@ skipping 118 matching lines @@
     visit(node.tag);
     visit(node.template);
   }
 
   visitClassDeclaration(ClassDeclaration node) {
     indent();
     visit(node.classExpr);
     lineOut();
   }
 
+  outTypeArgs(List<Identifier> typeArgs) {
+    if (options.printTypes && typeArgs != null && typeArgs.isNotEmpty) {

[Jennifer Messerly, 2016/02/09 01:21:33]

null check on variable/param is probably fastest, so if you wanted to put it
first, that'd probably be best

[ochafik, 2016/02/10 18:12:45]

Done.

+      out("<");

[Jennifer Messerly, 2016/02/09 01:21:33]

does typescript have any cases where precedence could matter?

I know in Dart we had an odd corner case:

    f(a < b, c >( d ));

That could be interpreted as:

// generic function call "a" with type args <b,c> and argument d
f( (a<b,c>) (  d  ) );

or as:

// call "f" with two arguments, a<b and c>d
f( (a < b),  (c > (d)) );

[ochafik, 2016/02/10 18:12:45]

It *looks* fine in TS (since the : would announce a switch to type space: `f(a:
< b, c >( d ) => number)`), but left a TODO in outTypeParams to reconsider when
we start emitting type params expressions (generic method call, etc).

+      var first = true;
+      for (var typeParam in typeArgs) {
+        if (!first) out(", ");
+        first = false;
+        visit(typeParam);
+      }
+      out(">");
+    }
+  }
+
   visitClassExpression(ClassExpression node) {
     out('class ');
     visit(node.name);
+    outTypeArgs(node.typeArgs);
     if (node.heritage != null) {
       out(' extends ');
       visit(node.heritage);
     }
     spaceOut();
     if (node.methods.isNotEmpty) {
       out('{');
       lineOut();
       indentMore();
+      if (options.printTypes) {
+        for (var field in node.fields) {
+          indent();
+          visit(field);
+          out(";");
+          lineOut();
+        }
+      }
       for (var method in node.methods) {
         indent();
         visit(method);
         lineOut();
       }
       indentLess();
       indent();
       out('}');
     } else {
       out('{}');
@@ skipping 211 matching lines @@
     } else {
       out('/* $comment */');
     }
     visit(node.expression);
   }
 
   void visitAwait(Await node) {
     out("await ");
     visit(node.expression);
   }
+
+  void outTypeAnnotation(TypeRef node) {
+    if (!options.printTypes || node == null || node.isUnknown) return;

[Jennifer Messerly, 2016/02/09 01:21:33]

silly micro opt: check null first

[ochafik, 2016/02/10 18:12:45]

Done.

+
+    if (node is OptionalTypeRef) {
+      out("?: ");
+      visit(node.type);
+    } else {
+      out(": ");
+      visit(node);
+    }
+  }
 }
 
 // Collects all the var declarations in the function.  We need to do this in a
 // separate pass because JS vars are lifted to the top of the function.
 class VarCollector extends BaseVisitor {
   bool nested;
   final Set<String> vars;
   final Set<String> params;
 
   VarCollector() : nested = false,
@@ skipping 289 matching lines @@
     declare(node.name);
     node.function.accept(this);
   }
 
   visitClassExpression(ClassExpression node) {
     declare(node.name);
     if (node.heritage != null) node.heritage.accept(this);
     for (Method element in node.methods) element.accept(this);
   }
 }