Type annotations instead of closure comments.

lib/src/js/printer.dart

Index: lib/src/js/printer.dart
diff --git a/lib/src/js/printer.dart b/lib/src/js/printer.dart
index bd8de3b40aa49c9bae41dc8051df78f5f84e9465..23b3a15d144f8b0e35f1625ec271907d9ad85411 100644
--- a/lib/src/js/printer.dart
+++ b/lib/src/js/printer.dart
@@ -9,6 +9,7 @@ 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`
@@ -19,6 +20,7 @@ class JavaScriptPrintingOptions {
       {this.shouldCompressOutput: false,
        this.minifyLocalVariables: false,
        this.preferSemicolonToNewlineInMinifiedOutput: false,
+       this.printTypes: false,
        this.allowKeywordsInProperties: false,
        this.allowSingleLineIfStatements: false});
 }
@@ -52,7 +54,7 @@ class SimpleJavaScriptPrintingContext extends JavaScriptPrintingContext {
 }
 
 
-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;
@@ -541,12 +543,14 @@ class Printer implements NodeVisitor {
                             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;
@@ -601,8 +605,11 @@ class Printer implements NodeVisitor {
 
   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);
   }
@@ -630,6 +637,7 @@ class Printer implements NodeVisitor {
       }
       visit(node.structure);
     }
+    outTypeAnnotation(node.type);
     if (node.defaultValue != null) {
       spaceOut();
       out("=");
@@ -856,6 +864,7 @@ class Printer implements NodeVisitor {
 
   visitIdentifier(Identifier node) {
     out(localNamer.getName(node));
+    outTypeAnnotation(node.type);
   }
 
   visitRestParameter(RestParameter node) {
@@ -921,7 +930,8 @@ class Printer implements NodeVisitor {
 
   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 {
@@ -930,6 +940,7 @@ class Printer implements NodeVisitor {
           newInForInit: false, newAtStatementBegin: false);
       out(")");
     }
+    outTypeAnnotation(fun.returnType);
     spaceOut();
     out("=>");
     if (fun.body is Expression) {
@@ -1068,9 +1079,23 @@ class Printer implements NodeVisitor {
     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);
@@ -1080,6 +1105,14 @@ class Printer implements NodeVisitor {
       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);
@@ -1311,6 +1344,18 @@ class Printer implements NodeVisitor {
     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