Type annotations instead of closure comments.

lib/src/js/nodes.dart

Index: lib/src/js/nodes.dart
diff --git a/lib/src/js/nodes.dart b/lib/src/js/nodes.dart
index 0ba181426578b19f97656189834b4847ce9d161d..5b26cde6167430d6a559823e3f10c55958de7fea 100644
--- a/lib/src/js/nodes.dart
+++ b/lib/src/js/nodes.dart
@@ -4,7 +4,7 @@
 
 part of js_ast;
 
-abstract class NodeVisitor<T> {
+abstract class NodeVisitor<T> implements TypeRefVisitor<T> {
   T visitProgram(Program node);
 
   T visitBlock(Block node);
@@ -94,6 +94,18 @@ abstract class NodeVisitor<T> {
   T visitDestructuredVariable(DestructuredVariable node);
 }
 
+abstract class TypeRefVisitor<T> {
+  T visitQualifiedTypeRef(QualifiedTypeRef node);
+  T visitGenericTypeRef(GenericTypeRef node);
+  T visitUnionTypeRef(UnionTypeRef node);
+  T visitRecordTypeRef(RecordTypeRef node);
+  T visitOptionalTypeRef(OptionalTypeRef node);
+  T visitFunctionTypeRef(FunctionTypeRef node);
+  T visitAnyTypeRef(AnyTypeRef node);
+  T visitUnknownTypeRef(UnknownTypeRef node);
+  T visitArrayTypeRef(ArrayTypeRef node);
+}
+
 class BaseVisitor<T> implements NodeVisitor<T> {
   T visitNode(Node node) {
     node.visitChildren(this);
@@ -221,6 +233,17 @@ class BaseVisitor<T> implements NodeVisitor<T> {
   T visitObjectBindingPattern(ObjectBindingPattern node)
       => visitBindingPattern(node);
   T visitDestructuredVariable(DestructuredVariable node) => visitNode(node);
+
+  T visitTypeRef(TypeRef node) => visitNode(node);
+  T visitQualifiedTypeRef(QualifiedTypeRef node) => visitTypeRef(node);
+  T visitGenericTypeRef(GenericTypeRef node) => visitTypeRef(node);
+  T visitOptionalTypeRef(OptionalTypeRef node) => visitTypeRef(node);
+  T visitRecordTypeRef(RecordTypeRef node) => visitTypeRef(node);
+  T visitUnionTypeRef(UnionTypeRef node) => visitTypeRef(node);
+  T visitFunctionTypeRef(FunctionTypeRef node) => visitTypeRef(node);
+  T visitAnyTypeRef(AnyTypeRef node) => visitTypeRef(node);
+  T visitUnknownTypeRef(UnknownTypeRef node) => visitTypeRef(node);
+  T visitArrayTypeRef(ArrayTypeRef node) => visitTypeRef(node);
 }
 
 abstract class Node {
@@ -780,7 +803,8 @@ class DestructuredVariable extends Expression implements Parameter {
   final Identifier name;
   final BindingPattern structure;
   final Expression defaultValue;
-  DestructuredVariable({this.name, this.structure, this.defaultValue}) {
+  final TypeRef type;
+  DestructuredVariable({this.name, this.structure, this.defaultValue, this.type}) {
     assert(name != null || structure != null);
   }
 
@@ -1022,14 +1046,19 @@ class Postfix extends Expression {
   int get precedenceLevel => UNARY;
 }
 
-abstract class Parameter implements Expression, VariableBinding {}
+abstract class Parameter implements Expression, VariableBinding {
+  TypeRef get type;
+}
 
 class Identifier extends Expression implements Parameter, VariableBinding {
   final String name;
   final bool allowRename;
+  final TypeRef type;
 
-  Identifier(this.name, {this.allowRename: true}) {
-    assert(_identifierRE.hasMatch(name));
+  Identifier(this.name, {this.allowRename: true, this.type}) {
+    if (!_identifierRE.hasMatch(name)) {
+      throw new ArgumentError.value(name, "name", "not a valid identifier");
+    }
   }
   static RegExp _identifierRE = new RegExp(r'^[A-Za-z_$][A-Za-z_$0-9]*$');
 
@@ -1043,6 +1072,7 @@ class Identifier extends Expression implements Parameter, VariableBinding {
 // This is an expression for convenience in the AST.
 class RestParameter extends Expression implements Parameter {
   final Identifier parameter;
+  TypeRef get type => null;
 
   RestParameter(this.parameter);
 
@@ -1107,19 +1137,27 @@ class NamedFunction extends Expression {
 
 abstract class FunctionExpression extends Expression {
   List<Parameter> get params;
+
   get body; // Expression or block
+  /// For TypeScript / Closure's ES6_TYPED.

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

Maybe:

/// Type parameters passed to this generic function, if any.

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

Done.

+  List<Identifier> get typeArgs;

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

These are type parameters, right? I'd suggest naming it `typeParams`.

(aside: at some point we should probably bring JS_AST up to the style guide, and
not use abbreviations. Not now though. ;) )


I think JS_AST might use FunctionExpression for both arrow functions:

    <T>(x) => x

and for declarations:

    function foo<T>(x : T) : T {
    }

Do they all support type parameters? If not, you may want to move this to a
subclass or to NamedFunction.

That said it might be fine here, depending on what TypeScript/Closure supports.

BTW, does TypeScript/Closure support constraints on the generic, like `T extends
Foo`?
it looks like TypeScript does:
http://www.typescriptlang.org/Handbook#generics-generic-constraints

so it probably shouldn't be an identifier.

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

Both kinds of generic functions are valid TS / ES6_TYPED (isn't it neat?)
Ouch, indeed... will leave a TODO if that's okay for now... (btw, would it be
worth decoupling Identifier from Parameter, if we introduce a TypeParameter?
Could simplify some of the declaration visitors I think)

+  /// For TypeScript / Closure's ES6_TYPED.

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

/// The return type of the function, if any.

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

Done.

+  TypeRef get returnType;
 }
 
 class Fun extends FunctionExpression {
   final List<Parameter> params;
   final Block body;
+  final List<Identifier> typeArgs;

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

Not for now, but, what do you think about using @override?

I like it as a signal for "this already has documentation above".

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

I love @override, so since you mentioned it... done. (we'd need a cleanup pass
in the visitors... and maybe a hint from the analyzer when it's missing, as in
Java?)

+  final TypeRef returnType;
   /** Whether this is a JS generator (`function*`) that may contain `yield`. */
   final bool isGenerator;
 
   final AsyncModifier asyncModifier;
 
   Fun(this.params, this.body, {this.isGenerator: false,
-      this.asyncModifier: const AsyncModifier.sync()});
+      this.asyncModifier: const AsyncModifier.sync(),
+      this.typeArgs, this.returnType});
 
   accept(NodeVisitor visitor) => visitor.visitFun(this);
 
@@ -1137,8 +1175,10 @@ class Fun extends FunctionExpression {
 class ArrowFun extends FunctionExpression {
   final List<Parameter> params;
   final body; // Expression or Block
+  final List<Identifier> typeArgs;
+  final TypeRef returnType;
 
-  ArrowFun(this.params, this.body);
+  ArrowFun(this.params, this.body, {this.typeArgs, this.returnType});
 
   accept(NodeVisitor visitor) => visitor.visitArrowFun(this);
 
@@ -1425,8 +1465,13 @@ class ClassExpression extends Expression {
   final Identifier name;
   final Expression heritage; // Can be null.
   final List<Method> methods;
+  /// For TypeScript / Closure's ES6_TYPED.

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

(Same comment -- I think we should give this a proper doc comment.)

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

Done.

+  final List<Identifier> typeArgs;
+  /// For TypeScript / Closure's ES6_TYPED.
+  final List<VariableDeclarationList> fields;
 
-  ClassExpression(this.name, this.heritage, this.methods);
+  ClassExpression(this.name, this.heritage, this.methods,
+      [this.typeArgs = const [], this.fields = const []]);

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

I think it'd be fine to use a normal list. In general, we don't really use
immutable lists in js_ast, instead they're immutable by convention.

(At some point switching to BuiltList is probably a good idea.)

BTW: would this be worth named args? They're easier to version. Positional
optional args are only good if you know you won't ever want to add a named one
:)

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

Done.

 
   accept(NodeVisitor visitor) => visitor.visitClassExpression(this);
 
@@ -1434,6 +1479,8 @@ class ClassExpression extends Expression {
     name.accept(visitor);
     if (heritage != null) heritage.accept(visitor);
     for (Method element in methods) element.accept(visitor);
+    for (var field in fields) field.accept(visitor);
+    for (var typeParam in typeArgs) typeParam.accept(visitor);
   }
 
   ClassExpression _clone() => new ClassExpression(name, heritage, methods);
@@ -1501,6 +1548,7 @@ class InterpolatedLiteral extends Literal with InterpolatedNode {
 class InterpolatedParameter extends Expression with InterpolatedNode
     implements Identifier {
   final nameOrPosition;
+  TypeRef get type => null;
 
   String get name { throw "InterpolatedParameter.name must not be invoked"; }
   bool get allowRename => false;
@@ -1560,6 +1608,7 @@ class InterpolatedMethod extends Expression with InterpolatedNode
 class InterpolatedIdentifier extends Expression with InterpolatedNode
     implements Identifier {
   final nameOrPosition;
+  TypeRef get type => null;
 
   InterpolatedIdentifier(this.nameOrPosition);