Wrapperless dart:html and friends

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 js_codegen;
 
 import 'dart:collection' show HashSet, HashMap, SplayTreeSet;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/src/generated/ast.dart' hide ConstantEvaluator;
@@ skipping 651 matching lines @@
     for (var m in node.members) {
       if (m is ConstructorDeclaration) {
         jsMethods.add(_emitConstructor(m, type, fields, isObject));
       } else if (m is MethodDeclaration) {
         jsMethods.add(_emitMethodDeclaration(type, m));
 
         if (!hasJsPeer && m.isGetter && m.name.name == 'iterator') {
           hasIterator = true;
           jsMethods.add(_emitIterable(type));
         }
+      } else if (m is FieldDeclaration) {
+        // TODO(vsm): Refactor common logic out.

[Jennifer Messerly, 2016/02/16 17:46:38]

yeah if this was in a method like _emitRenamedNativeField, with a doc comment,
it'd be good. The logic makes sense.

BTW, in the past we've tended to care about code size & readability, so these
type of metaprogramming things are often generated via a helper:

    dart.renameField(HTMLElement, 'elementList', 'elements');

Whether that's the long term direction, I'm not sure. All of the existing
transpilers do use helpers to make the code more readable, though. Even though
it does make Closure advanced optimizations unhappy, I'd rather have
closure-advanced specific lowering.

Only other thing I'd check is if this can share any logic with existing
vitrualField code path.

[vsm, 2016/02/18 01:13:09]

Refactored and added a TODO to look into metaprogramming.  Another advantage of
your metaprogramming suggestion is code size.  These added methods are fed later
into dart.registerExtensions - perhaps it makes sense to just install there
directly?

+        if (!m.isStatic && _extensionTypes.contains(element)) {
+          for (var decl in m.fields.variables) {
+            FieldElement field = decl.element;

[Jacob, 2016/02/16 17:47:35]

nit: change these to variables to var?

[vsm, 2016/02/18 01:13:09]

Done.

+            String name = decl.name.name;
+            var annotation = findAnnotation(field, isJsName);
+            if (annotation != null) {
+              name = getConstantField(annotation, 'name', types.stringType)
+                  ?.toStringValue();
+            }
+            // Generate getter
+            var fn = new JS.Fun([], js.statement('{ return this.#; }', [name]));
+            var method = new JS.Method(_elementMemberName(field.getter), fn,
+                isGetter: true);
+            jsMethods.add(method);
+
+            // Generate setter
+            if (!decl.isFinal) {
+              var value = new JS.TemporaryId('value');
+              fn = new JS.Fun(
+                  [value], js.statement('{ this.# = #; }', [name, value]));
+              method = new JS.Method(_elementMemberName(field.setter), fn,
+                  isSetter: true);
+              jsMethods.add(method);
+            }
+          }
+        }
       }
     }
 
     // If the type doesn't have an `iterator`, but claims to implement Iterable,
     // we inject the adaptor method here, as it's less code size to put the
     // helper on a parent class. This pattern is common in the core libraries
     // (e.g. IterableMixin<E> and IterableBase<E>).
     //
     // (We could do this same optimization for any interface with an `iterator`
     // method, but that's more expensive to check for, so it doesn't seem worth
@@ skipping 80 matching lines @@
     var name = classElem.name;
     var body = <JS.Statement>[];
 
     if (_extensionTypes.contains(classElem)) {
       var dartxNames = <JS.Expression>[];
       for (var m in methods) {
         if (!m.isAbstract && !m.isStatic && m.element.isPublic) {
           dartxNames.add(_elementMemberName(m.element, allowExtensions: false));
         }
       }
+      for (var f in fields) {
+        if (!f.isStatic) {
+          for (var d in f.fields.variables) {
+            if (d.element.isPublic) {
+              dartxNames.add(
+                  _elementMemberName(d.element.getter, allowExtensions: false));
+            }
+          }
+        }
+      }
       if (dartxNames.isNotEmpty) {
         body.add(js.statement('dart.defineExtensionNames(#)',
             [new JS.ArrayInitializer(dartxNames, multiline: true)]));
       }
     }
 
     body.add(_emitClassHeritageWorkaround(cls));
 
     // TODO(jmesserly): we should really just extend native Array.
     if (jsPeerName != null && classElem.typeParameters.isNotEmpty) {
@@ skipping 176 matching lines @@
       _collectExtensions(m, types);
     }
     for (var i in type.interfaces) {
       _collectExtensions(i, types);
     }
     _collectExtensions(type.superclass, types);
   }
 
   JS.Statement _overrideField(FieldElement e) {
     var cls = e.enclosingElement;
-    return js.statement('dart.virtualField(#, #)',
-        [cls.name, _emitMemberName(e.name, type: cls.type)]);
+    var name;
+    if (_extensionTypes.contains(cls)) {

[Jennifer Messerly, 2016/02/16 17:46:38]

eeep. This is a little bit sketchy :). We try and make sure all extension names
are already defined by the library we're importing.

if the extension symbol isn't defined previously at the top-level, what
guarantee do we have that it's getting defined for the other code that uses it,
like method bodies?

my intuition here is that this might be papering over a problem you were seeing,
but there's still a bug that can be hit in other ways...

[vsm, 2016/02/18 01:13:09]

Hmm, seems to work now if I revert.  I had probably temporarily messed this up
somehow.  Thanks for the catch!

+      name = js.call('dart.getExtensionSymbol(#)', js.string(e.name));
+    } else {
+      name = _emitMemberName(e.name, type: cls.type);
+    }

[Jacob, 2016/02/16 17:47:35]

nit: consider

 var name = _extensionTypes.contains(cls) ?
    js.call('dart.getExtensionSymbol(#)', js.string(e.name)) :
    _emitMemberName(e.name, type: cls.type);

[vsm, 2016/02/18 01:13:09]

Removed change altogether.

+    return js.statement('dart.virtualField(#, #)', [cls.name, name]);
   }
 
   /// Generates the implicit default constructor for class C of the form
   /// `C() : super() {}`.
   JS.Method _emitImplicitConstructor(
       ClassDeclaration node, List<FieldDeclaration> fields) {
     assert(_hasUnnamedConstructor(node.element) == fields.isNotEmpty);
 
     // If we don't have a method body, skip this.
     var superCall = _superConstructorCall(node.element);
@@ skipping 341 matching lines @@
       t is ParameterizedType && t.typeArguments.any(_hasUnsoundTypeParameter);
 
   JS.Expression _defaultParamValue(FormalParameter param) {
     if (param is DefaultFormalParameter && param.defaultValue != null) {
       return _visit(param.defaultValue);
     } else {
       return new JS.LiteralNull();
     }
   }
 
-  JS.Fun _emitNativeFunctionBody(
-      List<JS.Parameter> params, List<JS.Expression> paramRefs,
-      MethodDeclaration node) {
+  JS.Fun _emitNativeFunctionBody(List<JS.Parameter> params,
+      List<JS.Expression> paramRefs, MethodDeclaration node) {
     if (node.isStatic) {
       // TODO(vsm): Do we need to handle this case?
       return null;
     }
 
     String name = node.name.name;
     var annotation = findAnnotation(node.element, isJsName);
     if (annotation != null) {
       name = getConstantField(annotation, 'name', types.stringType)
           ?.toStringValue();
@@ skipping 214 matching lines @@
       // arrow functions yet (e.g. `({a} = {}) => 1`) but happily accepts them
       // with regular functions (e.g. `function({a} = {}) { return 1 }`).
       // Note that Firefox accepts both syntaxes just fine.
       // TODO(ochafik): Simplify this code when Chrome Canary catches up.
       var canUseArrowFun = !node.parameters.parameters.any(_isNamedParam);
 
       JS.Node jsBody;
       var body = node.body;
       if (body.isGenerator || body.isAsynchronous) {
         var paramRefs = _emitParameterReferences(node.parameters);
-        jsBody = _emitGeneratorFunctionBody(
-            params, paramRefs, body, returnType);
+        jsBody =
+            _emitGeneratorFunctionBody(params, paramRefs, body, returnType);
       } else if (body is ExpressionFunctionBody) {
         jsBody = _visit(body.expression);
       } else {
         jsBody = _visit(body);
       }
       if (jsBody is JS.Expression && !canUseArrowFun) {
         jsBody = js.statement("{ return #; }", [jsBody]);
       }
       var clos = canUseArrowFun
           ? new JS.ArrowFun(params, jsBody,
               typeParams: typeParams, returnType: returnType)
           : new JS.Fun(params, jsBody,
               typeParams: typeParams, returnType: returnType);
       if (!inStmt) {
         var type = getStaticType(node);
         return _emitFunctionTagged(clos, type,
             topLevel: _executesAtTopLevel(node));
       }
       return clos;
     }
   }
 
-  JS.Fun _emitFunctionBody(List<JS.Parameter> params,
-      List<JS.Expression> paramRefs, FunctionBody body,
-      List<JS.Identifier> typeParams, JS.TypeRef returnType) {
+  JS.Fun _emitFunctionBody(
+      List<JS.Parameter> params,
+      List<JS.Expression> paramRefs,
+      FunctionBody body,
+      List<JS.Identifier> typeParams,
+      JS.TypeRef returnType) {
     // sync*, async, async*
     if (body.isAsynchronous || body.isGenerator) {
       // TODO(ochafik): Refine params: we don't need default values in the
       // nested function, so we'd need to generate a custom, simpler params
       // list here.
       return new JS.Fun(
           params,
           js.statement('{ return #; }', [
             _emitGeneratorFunctionBody(params, paramRefs, body, returnType)
           ]),
           returnType: returnType);
     }
     // normal function (sync)
     return new JS.Fun(params, _visit(body),
         typeParams: typeParams, returnType: returnType);
   }
 
-  JS.Expression _emitGeneratorFunctionBody(
-      List<JS.Parameter> params, List<JS.Expression> paramRefs,
-      FunctionBody body, JS.TypeRef returnType) {
+  JS.Expression _emitGeneratorFunctionBody(List<JS.Parameter> params,
+      List<JS.Expression> paramRefs, FunctionBody body, JS.TypeRef returnType) {
     var kind = body.isSynchronous ? 'sync' : 'async';
     if (body.isGenerator) kind += 'Star';
 
     // Transforms `sync*` `async` and `async*` function bodies
     // using ES6 generators.
     //
     // `sync*` wraps a generator in a Dart Iterable<T>:
     //
     // function name(<args>) {
     //   return dart.syncStar(function*(<args>) {
@@ skipping 548 matching lines @@
   bool _isNamedParam(FormalParameter param) =>
       param.kind == ParameterKind.NAMED;
 
   @override
   List<JS.Parameter> visitFormalParameterList(FormalParameterList node) =>
       _emitFormalParameterList(node);
 
   // TODO(ochafik): Decouple Parameter from Identifier.
   List<JS.Expression> _emitParameterReferences(FormalParameterList node) =>
       node == null
-      ? <JS.Expression>[]
-      : _emitFormalParameterList(node, allowDestructuring: false)
-          .map((JS.Parameter p) {
-            if (p is JS.RestParameter) return new JS.Spread(p.parameter);
-            return p as JS.Identifier;
-          })
-          .toList();
+          ? <JS.Expression>[]
+          : _emitFormalParameterList(node, allowDestructuring: false)
+              .map((JS.Parameter p) {
+              if (p is JS.RestParameter) return new JS.Spread(p.parameter);
+              return p as JS.Identifier;
+            }).toList();
 
   List<JS.Parameter> _emitFormalParameterList(FormalParameterList node,
       {bool allowDestructuring: true}) {
     var result = <JS.Parameter>[];
 
     var namedVars = <JS.DestructuredVariable>[];
     var destructure = allowDestructuring && options.destructureNamedParams;
     var hasNamedArgsConflictingWithObjectProperties = false;
     var needsOpts = false;
 
@@ skipping 14 matching lines @@
           }
           namedVars.add(new JS.DestructuredVariable(
               name: name,
               structure: structure,
               defaultValue: _defaultParamValue(param)));
         } else {
           needsOpts = true;
         }
       } else {
         var jsParam = _visit(param);
-        result.add(
-            param is DefaultFormalParameter && destructure
-                ? new JS.DestructuredVariable(
-                    name: jsParam, defaultValue: _defaultParamValue(param))
-                : jsParam);
+        result.add(param is DefaultFormalParameter && destructure
+            ? new JS.DestructuredVariable(
+                name: jsParam, defaultValue: _defaultParamValue(param))
+            : jsParam);
       }
     }
 
     if (needsOpts) {
       result.add(_namedArgTemp);
     } else if (namedVars.isNotEmpty) {
       // Note: `var {valueOf} = {}` extracts `Object.prototype.valueOf`, so
       // in case there are conflicting names we create an object without
       // any prototype.
       var defaultOpts = hasNamedArgsConflictingWithObjectProperties
@@ skipping 744 matching lines @@
       return AstBuilder.propertyAccess(newTarget, node.propertyName);
     } else {
       var invoke = node as MethodInvocation;
       return AstBuilder.methodInvoke(
           newTarget, invoke.methodName, invoke.argumentList.arguments);
     }
   }
 
   bool _requiresStaticDispatch(Expression target, String memberName) {
     var type = getStaticType(target);
-    if (!_isObjectProperty(memberName)) {
+    if (!_isObjectProperty(memberName) || target is SuperExpression) {

[Jennifer Messerly, 2016/02/16 17:46:38]

nice fix.

[Jacob, 2016/02/16 17:47:35]

Why does target is SuperExpression require static dispatch?
Naively would have thought 
super.dispatch could work dynamically by just following the JS prototype chain?

[Jennifer Messerly, 2016/02/16 18:22:00]

Yup, that's what the CL is fixing, if I understand correctly. The bug was:

// Dart
class Foo extends Object {
  toString() => super.toString();
}

we were generating this incorrectly:

// JS
class Foo extends core.Object {
  toString() { return dart.toString(super); }
}

Vijay's change makes sure that "super." never uses static dispatch. So it ends
up as:

// JS
class Foo extends core.Object {
  toString() { return super.toString(); }
}

... which is correct. BTW, the reason "Object" methods generate like this is so
we can handle null correctly. But for "super." null is not possible.

BTW, Vijay -- I'm kind of surprised !_isNullable(target) below doesn't handle
this. We should know that "super" cannot be null (same with "this") ... so maybe
there is a bug here, although not necessarily one you should try to fix unless
you're motivated. Discussed more below.

thanks Jacob for making me take another look

       return false;
     }
     if (!type.isObject &&

[Jennifer Messerly, 2016/02/16 18:22:01]

wow, this code is confusing. Can we rewrite it like?

// In these cases, the target could be `null` so we need static dispatch.
return type.isObject ||
     _isJSBuiltinType(type) ||
     _extensionTypes.contains(type.element) ||
     _isNullable(target);


... now it's clearer, but it seems totally wrong :)

1. Why check for _isJSBuiltinType and _extensionTypes? That seems redundant.
2. Why do extension types always use static dispatch for object members?
3. Why is type.isObject a special case anyway?

Anyway, for 2, I think it's related to other brokenness in
https://github.com/dart-lang/dev_compiler/issues/451...

But yeah, what a mess. Here's what the code should be:

    return _isNullable(target);

Anyway I'm fine with the "super" workaround in the meantime. I guess because
extension methods fall into the wrong path, the existing _isNullable check
doesn't help.

[vsm, 2016/02/18 01:13:09]

Excellent point - cleaned up code!  The explicit super check was just hacking
around the problem you noted.  :-)

[vsm, 2016/02/18 15:23:55]

Ugh, had to back off this a little bit.  We don't symbolize hashCode.  E.g., we
compile "hello".hashCode to "hello".hashCode with the intermediate change.  This
puts it back to dart.hashCode("hello").

         !_isJSBuiltinType(type) &&
         !_extensionTypes.contains(type.element) &&
         !_isNullable(target)) {
       return false;
     }
     return true;
   }
 
   /// Shared code for [PrefixedIdentifier] and [PropertyAccess].
   JS.Expression _emitGet(Expression target, SimpleIdentifier memberId) {
@@ skipping 572 matching lines @@
       // conflict. We could use practically any character for this.
       name = '+$name';
     }
 
     // Dart "extension" methods. Used for JS Array, Boolean, Number, String.
     var baseType = type;
     while (baseType is TypeParameterType) {
       baseType = baseType.element.bound;
     }
     if (allowExtensions &&
+        baseType != null &&
         _extensionTypes.contains(baseType.element) &&
         !_isObjectProperty(name)) {
       return js.call('dartx.#', _propertyName(name));
     }
 
     return _propertyName(name);
   }
 
   bool _externalOrNative(node) =>
       node.externalKeyword != null || _functionBody(node) is NativeFunctionBody;
@@ skipping 126 matching lines @@
 
   JSGenerator(AbstractCompiler compiler)
       : _types = compiler.context.typeProvider,
         super(compiler) {
     // TODO(vsm): Eventually, we want to make this extensible - i.e., find
     // annotations in user code as well.  It would need to be summarized in
     // the element model - not searched this way on every compile.
     var finder = new _ExtensionFinder(context, _extensionTypes, _types);
     finder._addExtensionTypes('dart:_interceptors');
     finder._addExtensionTypes('dart:_native_typed_data');
+    finder._addExtensionTypes('dart:html');
+    finder._addExtensionTypes('dart:indexed_db');
+    finder._addExtensionTypes('dart:svg');
+    finder._addExtensionTypes('dart:web_audio');
+    finder._addExtensionTypes('dart:web_gl');
+    finder._addExtensionTypes('dart:web_sql');
 
     // TODO(vsm): If we're analyzing against the main SDK, those
     // types are not explicitly annotated.
     finder._addExtensionType(_types.intType);
     finder._addExtensionType(_types.doubleType);
     finder._addExtensionType(_types.boolType);
     finder._addExtensionType(_types.stringType);
   }
 
   String generateLibrary(LibraryUnit unit) {
@@ skipping 34 matching lines @@
 
 /// A special kind of element created by the compiler, signifying a temporary
 /// variable. These objects use instance equality, and should be shared
 /// everywhere in the tree where they are treated as the same variable.
 class TemporaryVariableElement extends LocalVariableElementImpl {
   TemporaryVariableElement.forNode(Identifier name) : super.forNode(name);
 
   int get hashCode => identityHashCode(this);
   bool operator ==(Object other) => identical(this, other);
 }