fixes #40, extension methods for primitive types

lib/runtime/dart_runtime.js

Index: lib/runtime/dart_runtime.js
diff --git a/lib/runtime/dart_runtime.js b/lib/runtime/dart_runtime.js
index 5761a8e897ef533aa8424e37297e0b2fbffbfb5e..875a1472ce30e2f50cc4fc4c2122487daca74244 100644
--- a/lib/runtime/dart_runtime.js
+++ b/lib/runtime/dart_runtime.js
@@ -2,7 +2,7 @@
 // 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.
 
-var dart, _js_helper, _js_primitives;
+var dart, _js_helper, _js_primitives, dartx;
 (function (dart) {
   'use strict';
 
@@ -11,38 +11,19 @@ var dart, _js_helper, _js_primitives;
   // See: https://github.com/dart-lang/dev_compiler/issues/164
   dart.globalState = null;
 
-  let defineProperty = Object.defineProperty;
-  let getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
-  let getOwnPropertyNames = Object.getOwnPropertyNames;
-  let getOwnPropertySymbols = Object.getOwnPropertySymbols;
+  const defineProperty = Object.defineProperty;
+  const getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
+  const getOwnPropertyNames = Object.getOwnPropertyNames;
+  const getOwnPropertySymbols = Object.getOwnPropertySymbols;
+  const hasOwnProperty = Object.prototype.hasOwnProperty;
+  const slice = [].slice;
 
   function getOwnNamesAndSymbols(obj) {
     return getOwnPropertyNames(obj).concat(getOwnPropertySymbols(obj));
   }
 
-  // Adapted from Angular.js
-  let FN_ARGS = /^function\s*[^\(]*\(\s*([^\)]*)\)/m;
-  let FN_ARG_SPLIT = /,/;
-  let FN_ARG = /^\s*(_?)(\S+?)\1\s*$/;
-  let STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
-
-  function formalParameterList(fn) {
-    let fnText,argDecl;
-    let args=[];
-    fnText = fn.toString().replace(STRIP_COMMENTS, '');
-    argDecl = fnText.match(FN_ARGS);
-
-    let r = argDecl[1].split(FN_ARG_SPLIT);
-    for (let arg of r) {
-      arg.replace(FN_ARG, function(all, underscore, name){
-        args.push(name);
-      });
-    }
-    return args;
-  }
-
   function dload(obj, field) {
-    field = _canonicalFieldName(obj, field);
+    field = _canonicalFieldName(obj, field, []);
     if (_getMethodType(obj, field) !== void 0) {
       return dart.bind(obj, field);
     }
@@ -52,10 +33,8 @@ var dart, _js_helper, _js_primitives;
     // See: https://github.com/dart-lang/dev_compiler/issues/169
     var result = obj[field];
 
-    // TODO(leafp): Decide whether to keep this for javascript
-    // objects, or just use the javascript semantics.
-    if (typeof result == "function" &&
-        !Object.prototype.hasOwnProperty.call(obj, field)) {
+    // TODO(vsm): Check this more robustly.
+    if (typeof result == "function" && !hasOwnProperty.call(obj, field)) {
       // This appears to be a method tearoff.  Bind this.
       return result.bind(obj);
     }
@@ -64,7 +43,7 @@ var dart, _js_helper, _js_primitives;
   dart.dload = dload;
 
   function dput(obj, field, value) {
-    field = _canonicalFieldName(obj, field);
+    field = _canonicalFieldName(obj, field, [value]);
     // TODO(vsm): Implement NSM and type checks.
     // See: https://github.com/dart-lang/dev_compiler/issues/170
     obj[field] = value;
@@ -112,51 +91,46 @@ var dart, _js_helper, _js_primitives;
   }
 
   function dcall(f/*, ...args*/) {
-    let args = Array.prototype.slice.call(arguments, 1);
+    let args = slice.call(arguments, 1);
     let ftype = _getFunctionType(f);
     return checkAndCall(f, ftype, void 0, args, 'call');
   }
   dart.dcall = dcall;
 
-  // TODO(vsm): Automatically build this.
-  // All dynamic methods should check for these.
-  // See: https://github.com/dart-lang/dev_compiler/issues/142
-  var _extensionMethods = {
-    // Lazy - as these symbols may not be loaded yet.
-    // TODO(vsm): This should record / check the receiver type
-    // as well.  E.g., only look for core.$map if the receiver
-    // is an Iterable.
-    'map': () => core.$map,
-  };
-
-  // TODO(leafp): Integrate this with the eventual proper extension
-  // method system.
-  function _canonicalFieldName(obj, name) {
-    if (obj[name] === void 0) return _extensionMethods[name]();
+  let _extensionType = Symbol('extensionType');
+  function _canonicalFieldName(obj, name, args, displayName) {
+    if (obj[_extensionType]) {
+      var extension = dartx[name];
+      if (extension) return extension;
+      // TODO(jmesserly): in the future we might have types that "overlay" Dart
+      // methods while also exposing the full native API, e.g. dart:html vs
+      // dart:dom. To support that we'd need to fall back to the normal name
+      // if an extension method wasn't found.
+      throwNoSuchMethod(obj, displayName, args);

[vsm, 2015/06/02 17:59:10]

displayName isn't passed in by dload / dput above.  Use name if that's
undefined?

[Jennifer Messerly, 2015/06/04 21:31:18]

Done.

+    }
     return name;
   }
 
-  function dsend(obj, method/*, ...args*/) {
-    let args = Array.prototype.slice.call(arguments, 2);
-    let symbol = _canonicalFieldName(obj, method);
+  /** Shared code for dsend, dindex, and dsetindex. */
+  function callMethod(obj, name, displayName, args) {

[Leaf, 2015/06/02 21:35:04]

Slightly confusing that callMethod and _canonicalFieldName take identical
argument lists but with last two args swapped.  If it's all the same, might be
worth swapping one or the other.

[Jennifer Messerly, 2015/06/04 21:31:18]

Done.

+    let symbol = _canonicalFieldName(obj, name, args, displayName);
     let f = obj[symbol];
-    let ftype = _getMethodType(obj, symbol);
-    return checkAndCall(f, ftype, obj, args, method);
+    let ftype = _getMethodType(obj, name);
+    return checkAndCall(f, ftype, obj, args, displayName);
+  }
+
+  function dsend(obj, method/*, ...args*/) {
+    return callMethod(obj, method, method, slice.call(arguments, 2));
   }
   dart.dsend = dsend;
 
   function dindex(obj, index) {
-    // TODO(jmesserly): remove this special case once Array extensions are
-    // hooked up.
-    if (obj instanceof Array && realRuntimeType(index) == core.int) {
-      return obj[index];
-    }
-    return checkAndCall(obj.get, void 0, obj, [index], '[]');
+    return callMethod(obj, 'get', '[]', [index]);
   }
   dart.dindex = dindex;
 
   function dsetindex(obj, index, value) {
-    return checkAndCall(obj.set, void 0, obj, [index, value], '[]=');
+    return callMethod(obj, 'set', '[]=', [index, value]);
   }
   dart.dsetindex = dsetindex;
 
@@ -199,7 +173,7 @@ var dart, _js_helper, _js_primitives;
 
 
   // TODO(vsm): How should we encode the runtime type?
-  let _runtimeType = Symbol('_runtimeType');
+  const _runtimeType = Symbol('_runtimeType');
 
   function checkPrimitiveType(obj) {
     switch (typeof obj) {
@@ -268,7 +242,7 @@ var dart, _js_helper, _js_primitives;
     return t;
   }
 
-  let subtypeMap = new Map();
+  const subtypeMap = new Map();
   function isSubtype(t1, t2) {
     // See if we already know the answer
     // TODO(jmesserly): general purpose memoize function?
@@ -572,17 +546,17 @@ var dart, _js_helper, _js_primitives;
     if (arguments.length == 1) {
       // No type arguments, it's all dynamic
       let len = closure.length;
-      function build() {
+      let build = () => {
         let args = Array.apply(null, new Array(len)).map(() => core.Object);
         return functionType(core.Object, args);
-      }
+      };
       // We could be called before Object is defined.
       if (core.Object === void 0) return fn(closure, build);
       t = build();
     } else {
       // We're passed the piecewise components of the function type,
       // construct it.
-      let args = Array.prototype.slice.call(arguments, 1);
+      let args = slice.call(arguments, 1);
       t = functionType.apply(null, args);
     }
     setRuntimeType(closure, t);
@@ -799,17 +773,53 @@ var dart, _js_helper, _js_primitives;
   }
   dart.copyProperties = copyProperties;
 
+  function getExtensionSymbol(name) {
+    let sym = dartx[name];
+    if (!sym) dartx[name] = sym = Symbol('dartx.' + name);
+    return sym;
+  }
+
   /**
    * Copy symbols from the prototype of the source to destination.
    * These are the only properties safe to copy onto an existing public
    * JavaScript class.
    */
-  function registerExtension(to, from) {
-    return copyPropertiesHelper(to.prototype, from.prototype,
-        getOwnPropertySymbols(from.prototype));
+  function registerExtension(jsType, dartExtType) {
+    let extProto = dartExtType.prototype;
+    let jsProto = jsType.prototype;
+
+    // Mark the JS type's instances so we can easily check for extensions.
+    jsProto[_extensionType] = extProto;

[vsm, 2015/06/02 17:59:10]

assert that this isn't already defined?

[Jennifer Messerly, 2015/06/04 21:31:18]

Done.

+    for (let name of getOwnPropertyNames(extProto)) {
+      let symbol = getExtensionSymbol(name);
+      defineProperty(jsProto, symbol, getOwnPropertyDescriptor(extProto, name));
+    }
   }
   dart.registerExtension = registerExtension;
 
+  /**
+   * Mark a concrete type as implementing extension methods.
+   * For example: `class MyIter implements Iterable`.
+   *
+   * Because it's implementing `Iterable` for the first time, it needs to
+   * define the extension method symbols and have them call the normal Dart
+   * method with the corresponding name.
+   *
+   * Unfortunately we have no way of knowing what interface members are, so
+   * we just forward everything. These calls are only generated to static types
+   * like `List list;` so it won't confuse dynamic dispatch.
+   */
+  function implementExtension(type) {

[vsm, 2015/06/02 17:59:10]

Hmm, you're calling this everywhere with a second arg, but ignoring it here. 
Could you use the second arg (the implemented type) to filter what to copy
instead of moving everything?

[Jennifer Messerly, 2015/06/02 18:16:04]

Sigh, I wish we could. The doc comment above explains the unfortunate situation.

I actually had implementExtension take just one arg. It seemed hard to
understand, because you can't easily read what is being implemented in the code.
But we don't need it here.

I wanted to emit a comment, but there isn't a good way to emit trailing
comments, e.g.

    dart.implementExtension(ListBase); // core.Iterable, core.List

Also harder to goto def, etc.

Eventually if we preserve interface members, maybe for dart:mirrors, my plan was
to update this code to copy only the methods.

But I'm not sure we should assume mirrors are always compiled in.

So it's ... tricky. Appreciate thoughts. Maybe there's a better local minima I'm
not seeing.

[vsm, 2015/06/03 15:48:39]

General thoughts: 
(1) it's worth declaring the second arg here since you're passing it in from
generated code even if you're currently ignoring.
(2) beyond this CL - it'd be nice to use the metadata if present to copy only
what's needed
  - we'd need to extend the metadata to record abstract methods (needed for
mirrors)
  - your approach might still make sense as a fallback if no / insufficient
metadata
(3) also beyond this CL - We'd also need a way to deal with laziness / eagerness
(of the second arg - currently a closure) if we want to use the metadata.

[Jennifer Messerly, 2015/06/04 21:31:18]

Update: redesigned this. We now emit extension method names explicitly

+    let proto = type.prototype;
+    for (let name of getOwnPropertyNames(proto)) {
+      // TODO(jmesserly): skip named ctors as well
+      if (name == type.name || name == 'constructor') continue;
+      let symbol = getExtensionSymbol(name);
+      defineProperty(proto, symbol, getOwnPropertyDescriptor(proto, name));

[Jennifer Messerly, 2015/06/02 23:43:42]

Oops. Just noticed, this needs to generate a stub that virtually dispatches. Or,
we need to generate it whenever a concrete method is implemented.

[vsm, 2015/06/03 15:48:40]

Good catch.  :-)

[Jennifer Messerly, 2015/06/04 21:31:18]

fixed now.

+    }
+  }
+  dart.implementExtension = implementExtension;
+
   function setBaseClass(derived, base) {
     // Link the extension to the type it's extending as a base class.
     derived.prototype.__proto__ = base.prototype;
@@ -854,7 +864,7 @@ var dart, _js_helper, _js_primitives;
   function mixin(base/*, ...mixins*/) {
     // Create an initializer for the mixin, so when derived constructor calls
     // super, we can correctly initialize base and mixins.
-    let mixins = Array.prototype.slice.call(arguments, 1);
+    let mixins = slice.call(arguments, 1);
 
     // Create a class that will hold all of the mixin methods.
     class Mixin extends base {
@@ -942,7 +952,7 @@ var dart, _js_helper, _js_primitives;
   function defineNamedConstructor(clazz, name) {
     let proto = clazz.prototype;
     let initMethod = proto[name];
-    let ctor = function() { return initMethod.apply(this, arguments); }
+    let ctor = function() { return initMethod.apply(this, arguments); };
     ctor.prototype = proto;
     // Use defineProperty so we don't hit a property defined on Function,
     // like `caller` and `arguments`.
@@ -969,7 +979,7 @@ var dart, _js_helper, _js_primitives;
       if (arguments.length != length && arguments.length != 0) {
         throw Error('requires ' + length + ' or 0 type arguments');
       }
-      let args = Array.prototype.slice.call(arguments);
+      let args = slice.call(arguments);
       // TODO(leafp): This should really be core.Object for
       // consistency, but Object is not attached to core
       // until the entire core library has been processed,
@@ -1021,8 +1031,7 @@ var dart, _js_helper, _js_primitives;
     if (sigObj === void 0) return void 0;
     let parts = sigObj[name];
     if (parts === void 0) return void 0;
-    let sig = functionType.apply(null, parts);
-    return sig;
+    return functionType.apply(null, parts);
   }
 
   /// Get the type of a constructor from a class using the stored signature
@@ -1036,8 +1045,7 @@ var dart, _js_helper, _js_primitives;
     if (sigCtor === void 0) return void 0;
     let parts = sigCtor[name];
     if (parts === void 0) return void 0;
-    let sig = functionType.apply(null, parts);
-    return sig;
+    return functionType.apply(null, parts);
   }
   dart.classGetConstructorType = _getConstructorType;
 
@@ -1047,7 +1055,7 @@ var dart, _js_helper, _js_primitives;
   /// TODO(leafp): Consider caching the tearoff on the object?
   function bind(obj, name) {
     let f = obj[name].bind(obj);
-    let sig = _getMethodType(obj, name)
+    let sig = _getMethodType(obj, name);
     assert(sig);
     setRuntimeType(f, sig);
     return f;
@@ -1076,11 +1084,10 @@ var dart, _js_helper, _js_primitives;
   // Set the lazily computed runtime type field on static methods
   function _setStaticTypes(f, names) {
     for (let name of names) {
-      function getT() {
+      defineProperty(f[name], _runtimeType, { get: function() {
         let parts = f[_staticSig][name];
         return functionType.apply(null, parts);
-      };
-      defineProperty(f[name], _runtimeType, {get : getT});
+      }});
     }
   }
 
@@ -1107,7 +1114,7 @@ var dart, _js_helper, _js_primitives;
     _setMethodSignature(f, methods);
     _setStaticSignature(f, statics);
     _setStaticTypes(f, names);
-  };
+  }
   dart.setSignature = setSignature;
 
   let _value = Symbol('_value');
@@ -1140,7 +1147,7 @@ var dart, _js_helper, _js_primitives;
   }
 
   /** The global constant table. */
-  let constants = new Map();
+  const constants = new Map();
 
   /**
    * Canonicalize a constant object.
@@ -1151,11 +1158,17 @@ var dart, _js_helper, _js_primitives;
    */
   function constant(obj) {
     let objectKey = [realRuntimeType(obj)];
-    // There's no guarantee in JS that names/symbols are returned in the same
-    // order. We could probably get the same order if we're judicious about
-    // initializing them, but easier to not depend on that.
+    // TODO(jmesserly): there's no guarantee in JS that names/symbols are
+    // returned in the same order.
+    //
+    // We could probably get the same order if we're judicious about
+    // initializing fields in a consistent order across all const constructors.
+    // Alternatively we need a way to sort them to make consistent.
+    //
+    // Right now we use the (name,value) pairs in sequence, which prevents
+    // an object with incorrect field values being returned, but won't
+    // canonicalize correctly if key order is different.
     for (let name of getOwnNamesAndSymbols(obj)) {
-      // TODO(jmesserly): we can make this faster if needed.
       objectKey.push(name);
       objectKey.push(obj[name]);
     }
@@ -1176,6 +1189,12 @@ var dart, _js_helper, _js_primitives;
   }
   dart.setType = setType;
 
+  /** Sets the element type of a list literal. */
+  function list(obj, elementType) {
+    return setType(obj, _interceptors.JSArray$(elementType));
+  }
+  dart.list = list;
+
   /** Sets the internal runtime type of `obj` to be `type` */
   function setRuntimeType(obj, type) {
     obj[_runtimeType] = type;
@@ -1282,44 +1301,13 @@ var dart, _js_helper, _js_primitives;
   _js_primitives = _js_primitives || {};
   _js_primitives.printString = (s) => console.log(s);
 
-  // TODO(vsm): Plumb this correctly.
-  // See: https://github.com/dart-lang/dev_compiler/issues/40
-  String.prototype.contains = function(sub) { return this.indexOf(sub) >= 0; }
-  let _split = String.prototype.split;
-  String.prototype.split = function() {
-    let result = _split.apply(this, arguments);
-    dart.setType(result, core.List$(core.String));
-    return result;
-  }
-  String.prototype.get = function(i) {
-    return this[i];
-  }
-  String.prototype.codeUnitAt = function(i) {
-    return this.charCodeAt(i);
-  }
-  String.prototype.replaceAllMapped =  function(from, cb) {
-    return this.replace(from.multiple, function() {
-      // Remove offset & string from the result array
-      var matches = arguments;
-      matches.splice(-2, 2);
-      // The callback receives match, p1, ..., pn
-      return cb(matches);
-    });
-  }
-  String.prototype['+'] = function(arg) { return this.valueOf() + arg; };
-
-  Boolean.prototype['!'] = function() { return !this.valueOf(); };
-  Boolean.prototype['&&'] = function(arg) { return this.valueOf() && arg; };
-  Boolean.prototype['||'] = function(arg) { return this.valueOf() || arg; };
-  Number.prototype['<'] = function(arg) { return this.valueOf() < arg; };
-  Number.prototype['<='] = function(arg) { return this.valueOf() <= arg; };
-  Number.prototype['>'] = function(arg) { return this.valueOf() > arg; };
-  Number.prototype['+'] = function(arg) { return this.valueOf() + arg; };
-
   // TODO(vsm): DOM facades?
   // See: https://github.com/dart-lang/dev_compiler/issues/173
   NodeList.prototype.get = function(i) { return this[i]; };
   NamedNodeMap.prototype.get = function(i) { return this[i]; };
   DOMTokenList.prototype.get = function(i) { return this[i]; };
 
+  /** Dart extension members. */
+  dartx = dartx || {};
+
 })(dart || (dart = {}));