Fixes #178

lib/src/codegen/js_codegen.dart

 // Copyright (c) 2015, 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 dev_compiler.src.codegen.js_codegen;
 
 import 'dart:collection' show HashSet, HashMap;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/src/generated/ast.dart' hide ConstantEvaluator;
@@ skipping 50 matching lines @@
   final HashSet<FieldElement> _fieldsNeedingStorage;
 
   /// The variable for the target of the current `..` cascade expression.
   SimpleIdentifier _cascadeTarget;
 
   /// The variable for the current catch clause
   SimpleIdentifier _catchParameter;
 
   ConstantEvaluator _constEvaluator;
 
+  InterfaceType _currentClass = null;
+
   /// Imported libraries, and the temporaries used to refer to them.
   final _imports = new Map<LibraryElement, JS.TemporaryId>();
   final _exports = new Set<String>();
   final _lazyFields = <VariableDeclaration>[];
   final _properties = <FunctionDeclaration>[];
   final _privateNames = new HashMap<String, JS.TemporaryId>();
   final _extensionMethodNames = new HashSet<String>();
   final _pendingStatements = <JS.Statement>[];
   final _temps = new HashMap<Element, JS.TemporaryId>();
 
@@ skipping 244 matching lines @@
   JS.Statement visitClassDeclaration(ClassDeclaration node) {
     // If we've already emitted this class, skip it.
     var classElem = node.element;
     var type = classElem.type;
     if (_pendingClasses.remove(classElem) == null) return null;
 
     var jsName = getAnnotationValue(node, _isJsNameAnnotation);
 
     if (jsName != null) return _emitJsType(node.name.name, jsName);
 
+    // Set current class
+    assert(_currentClass == null);
+    _currentClass = type;

[Jennifer Messerly, 2015/05/11 16:35:08]

suggestion: maybe save the Element here, not the type. Will make it easier to
compare later.

[vsm, 2015/05/11 17:51:09]

Done.

+
     var ctors = <ConstructorDeclaration>[];
     var fields = <FieldDeclaration>[];
     var staticFields = <FieldDeclaration>[];
     for (var member in node.members) {
       if (member is ConstructorDeclaration) {
         ctors.add(member);
       } else if (member is FieldDeclaration) {
         (member.isStatic ? staticFields : fields).add(member);
       }
     }
 
     var classExpr = new JS.ClassExpression(new JS.Identifier(type.name),
         _classHeritage(node), _emitClassMethods(node, ctors, fields));

[Jennifer Messerly, 2015/05/11 16:35:08]

I think after this change, calls to static methods from inside the class might
generate as:

let Foo$ = dart.generic((E) => {
  class Foo {
    static foo() { ... }
    static bar() { Foo(dart.dynamic).foo(); }
  }
});

(ditto for static field references)


Perf cost, but maybe not a big deal for now?
Anyway, just something I've been wondering about. Possibly worth a test case.

[vsm, 2015/05/11 17:51:09]

Added a test case to track this.

 
     String jsPeerName;
     var jsPeer = getAnnotationValue(node, _isJsPeerInterface);
     if (jsPeer != null) {
       jsPeerName = getConstantField(jsPeer, 'name', types.stringType);
     }
 
     var body = _finishClassMembers(
         classElem, classExpr, ctors, fields, staticFields, jsPeerName);
 
+    // Unset current class
+    assert(_currentClass == type);
+    _currentClass = null;
+
     var result = _finishClassDef(type, body);
 
     if (jsPeerName != null) {
       // This class isn't allowed to be lazy, because we need to set up
       // the native JS type eagerly at this point.
       // If we wanted to support laziness, we could defer the hookup until
       // the end of the Dart library cycle load.
       assert(!_lazyClass(type));
 
       // TODO(jmesserly): this copies the dynamic members.
@@ skipping 860 matching lines @@
           ]);
         }
       }
       // TODO(jmesserly): remove when we're using coercion reifier.
       return _unimplementedCall('Unimplemented type $type');
     }
 
     var typeArgs = null;
     if (type is ParameterizedType) {
       var args = type.typeArguments;
-      if (args.any((a) => a != types.dynamicType)) {
+      // For a generic class with dynamic type parameters, try to use the
+      // raw form:  new S() instead of new S<dynamic>() if possible.
+      var isCurrentClass =

[Jennifer Messerly, 2015/05/11 16:35:08]

could this just be:

    var isCurrentClass = type.element == _currentClass?

(with suggestion above to use the ClassElement)

[vsm, 2015/05/11 17:51:09]

Done.

+          type is InterfaceType ? type.element.type == _currentClass : false;
+      if (args.isNotEmpty && isCurrentClass ||

[Jennifer Messerly, 2015/05/11 16:35:08]

maybe split these into two cases:

if (args.any((a) => a != types.dynamicType)) {
  ... original code ...
} else if (args.isNotEmpty && isCurrentClass) {
  // When creating a `new S<dynamic>` we try and use the raw form `new S()`,
  // but this does not work if we're inside the same class, because `S` refers
  // to the current S<T> we are generating.
  name = '$name\$';
  typeArgs = [];
}


The benefit is the second case will generate using the shorter form `Map$()`
instead of `Map$(dart.dynamic, dart.dynamic)`  [those two forms are equivalent]

[vsm, 2015/05/11 17:51:09]

Ahh, nice.  Done!

+          args.any((a) => a != types.dynamicType)) {
         name = '$name\$';
         typeArgs = args.map(_emitTypeName);
       }
     }
 
     JS.Expression result;
     if (_needQualifiedName(element)) {
       result = js.call('#.#', [_libraryName(element.library), name]);
     } else {
       result = new JS.Identifier(name);
@@ skipping 1345 matching lines @@
 // TODO(jmesserly): validate the library. See issue #135.
 bool _isJsNameAnnotation(DartObjectImpl value) => value.type.name == 'JsName';
 
 bool _isJsPeerInterface(DartObjectImpl value) =>
     value.type.name == 'JsPeerInterface';
 
 // TODO(jacobr): we would like to do something like the following
 // but we don't have summary support yet.
 // bool _supportJsExtensionMethod(AnnotatedNode node) =>
 //    _getAnnotation(node, "SupportJsExtensionMethod") != null;