Better mirrors support for mixins and private fields

pkg/dev_compiler/tool/input_sdk/private/js_mirrors.dart

Index: pkg/dev_compiler/tool/input_sdk/private/js_mirrors.dart
diff --git a/pkg/dev_compiler/tool/input_sdk/private/js_mirrors.dart b/pkg/dev_compiler/tool/input_sdk/private/js_mirrors.dart
index 6c985d79c0e830447fad5f0e16ff962a5dc0013e..e0f63f2a6a2f55ae053525eba42c9b96f7d25fd7 100644
--- a/pkg/dev_compiler/tool/input_sdk/private/js_mirrors.dart
+++ b/pkg/dev_compiler/tool/input_sdk/private/js_mirrors.dart
@@ -71,71 +71,108 @@ dynamic _defaultConstructorType(type) {
   return JS('', '#.definiteFunctionType(#, [])', _dart, type);
 }
 
+dynamic _getMixins(type) {
+  return JS('', '#.getMixins(#, [])', _dart, type);
+}
+
 typedef T _Lazy<T>();
 
+dynamic _getESSymbol(Symbol symbol) =>
+    _internal.Symbol.getNativeSymbol(symbol as _internal.Symbol);
+
+dynamic _getMember(Symbol symbol) {
+  var privateSymbol = _getESSymbol(symbol);
+  if (privateSymbol != null) {
+    return privateSymbol;
+  }
+  return getName(symbol);
+}
+
 String _getNameForESSymbol(member) {
   // Convert private JS symbol "Symbol(_foo)" to string "_foo".
+  assert(JS('bool', 'typeof # == "symbol"', member));
   var str = member.toString();
   assert(str.startsWith('Symbol(') && str.endsWith(')'));
   return str.substring(7, str.length - 1);
 }
 
-Map _toDartMap(data) {
+Symbol _getSymbolForESSymbol(member) {
+  var name = _getNameForESSymbol(member);
+  return new _internal.Symbol.es6(name, member);
+}
+
+// The [member] must be either a string (public) or an ES6 symbol (private).
+Symbol _getSymbolForMember(member) {
+  if (member is String) {
+    return new Symbol(member);
+  } else {
+    var name = _getNameForESSymbol(member);
+    return new _internal.Symbol.es6(name, member);
+  }
+}
+
+Map<Symbol, dynamic> _toDartMap(data) {
   if (data == null) return {};
-  var map = JS('Map', '#.map(#)', _dart, data);
+  var map = new Map<Symbol, dynamic>();
   // Note: we recorded a map from fields/methods to their type and metadata.
   // The key is a string name for public members but an ES6 symbol for private
   // ones.  That's works nicely for dynamic operations, but dart:mirrors expects
-  // strings, so we convert back here.
+  // Dart symbols, so we convert here.
+  var publicMembers = JS('', 'Object.getOwnPropertyNames(#)', data);
+  for (var member in publicMembers) {
+    var symbol = new Symbol(member);
+    map[symbol] = JS('', '#[#]', data, member);
+  }
+
   var privateMembers = JS('', 'Object.getOwnPropertySymbols(#)', data);
   for (var member in privateMembers) {
-    var name = _getNameForESSymbol(member);
-    map[name] = JS('', '#[#]', data, member);
+    var symbol = _getSymbolForESSymbol(member);
+    map[symbol] = JS('', '#[#]', data, member);
   }
   return map;
 }
 
-Map _getConstructors(obj) {
+Map<Symbol, dynamic> _getConstructors(obj) {
   List sig = JS('', '#.getConstructorSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
 
-Map _getFields(obj) {
+Map<Symbol, dynamic> _getFields(obj) {
   List sig = JS('', '#.getFieldSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
 
-Map _getMethods(obj) {
+Map<Symbol, dynamic> _getMethods(obj) {
   List sig = JS('', '#.getMethodSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
 
-Map _getGetters(obj) {
+Map<Symbol, dynamic> _getGetters(obj) {
   List sig = JS('', '#.getGetterSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
 
-Map _getSetters(obj) {
+Map<Symbol, dynamic> _getSetters(obj) {
   List sig = JS('', '#.getSetterSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
 
-Map _getStaticFields(obj) {
+Map<Symbol, dynamic> _getStaticFields(obj) {
   List sig = JS('', '#.getStaticFieldSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
 
-Map _getStatics(obj) {
+Map<Symbol, dynamic> _getStatics(obj) {
   List sig = JS('', '#.getStaticSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
 
-Map _getStaticGetters(obj) {
+Map<Symbol, dynamic> _getStaticGetters(obj) {
   List sig = JS('', '#.getStaticGetterSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
 
-Map _getStaticSetters(obj) {
+Map<Symbol, dynamic> _getStaticSetters(obj) {
   List sig = JS('', '#.getStaticSetterSig(#)', _dart, obj);
   return _toDartMap(sig);
 }
@@ -211,21 +248,7 @@ class JsInstanceMirror extends JsObjectMirror implements InstanceMirror {
   // Returns a String for public members or an ES6 symbol for private members.
   _getAccessor(dynamic reflectee, Symbol symbol, [List<dynamic> args,
       Map<Symbol, dynamic> namedArgs]) {
-    var name = getName(symbol);
-    if (!name.startsWith('_')) return name;
-
-    // TODO(vsm): Ideally, we'd record ES6 symbols properly during codegen if
-    // mirrors is enabled.  Here, we're trying to recover it from the receiver
-    // instead.
-    //
-    // Get private fields and members.  Members are on proto.
-    var privateMembers = JS('', 'Object.getOwnPropertySymbols(#)', reflectee)
-      ..addAll(JS('', 'Object.getOwnPropertySymbols(#.__proto__)', reflectee));
-    for (var member in privateMembers) {
-      var privateName = _getNameForESSymbol(member);
-      if (name == privateName) return member;
-    }
-    return new NoSuchMethodError(reflectee, symbol, args, namedArgs);
+    return _getMember(symbol);
   }
 
   InstanceMirror getField(Symbol symbol) {
@@ -275,7 +298,7 @@ class JsClassMirror extends JsMirror implements ClassMirror {
   final dynamic _raw;
 
   // TODO(vsm): Do this properly
-  final ClassMirror mixin = null;
+  ClassMirror _mixin = null;
   List<TypeMirror> _typeArguments;
 
   List<InstanceMirror> _metadata;
@@ -305,67 +328,67 @@ class JsClassMirror extends JsMirror implements ClassMirror {
       _declarations = new Map<Symbol, DeclarationMirror>();
       var unwrapped = _unwrap(_cls);
       var constructors = _getConstructors(unwrapped);
-      constructors.forEach((String name, ft) {
-        var symbol = new Symbol(name);
-        _declarations[symbol] = new JsMethodMirror._constructor(this, name, ft);
+      constructors.forEach((symbol, ft) {
+        var name = getName(symbol);
+        _declarations[symbol] = new JsMethodMirror._constructor(this, symbol, ft);
       });
       if (constructors.isEmpty) {
         // Add a default
         var name = 'new';
         var ft = _defaultConstructorType(_unwrap(_cls));
         var symbol = new Symbol(name);
-        _declarations[symbol] = new JsMethodMirror._constructor(this, name, ft);
+        _declarations[symbol] = new JsMethodMirror._constructor(this, symbol, ft);
       }
       var fields = _getFields(unwrapped);
-      fields.forEach((String name, t) {
-        var symbol = new Symbol(name);
+      fields.forEach((symbol, t) {
         var metadata = [];
         if (t is List) {
           metadata = t.skip(1).toList();
           t = t[0];
         }
-        _declarations[symbol] = new JsVariableMirror._(name, _wrap(t), metadata);
+        _declarations[symbol] = new JsVariableMirror._(symbol, _wrap(t), metadata);
       });
       var methods = _getMethods(unwrapped);
-      methods.forEach((String name, ft) {
-        var symbol = new Symbol(name);
-        _declarations[symbol] = new JsMethodMirror._instanceMethod(this, name, ft);
+      methods.forEach((symbol, ft) {
+        var name = getName(symbol);
+        _declarations[symbol] = new JsMethodMirror._instanceMethod(this, symbol, ft);
       });
       var getters = _getGetters(unwrapped);
-      getters.forEach((String name, ft) {
-        var symbol = new Symbol(name);
-        _declarations[symbol] = new JsMethodMirror._instanceMethod(this, name, ft);
+      getters.forEach((symbol, ft) {
+        var name = getName(symbol);
+        _declarations[symbol] = new JsMethodMirror._instanceMethod(this, symbol, ft);
       });
       var setters = _getSetters(unwrapped);
-      setters.forEach((String name, ft) {
-        name += '=';
-        var symbol = new Symbol(name);
-        _declarations[symbol] = new JsMethodMirror._instanceMethod(this, name, ft);
+      setters.forEach((symbol, ft) {
+        var name = getName(symbol) + '=';
+        // Create a separate symbol for the setter.
+        symbol = new _internal.Symbol.es6(name, _getESSymbol(symbol));
+        _declarations[symbol] = new JsMethodMirror._instanceMethod(this, symbol, ft);
       });
       var staticFields = _getStaticFields(unwrapped);
-      staticFields.forEach((String name, t) {
-        var symbol = new Symbol(name);
+      staticFields.forEach((symbol, t) {
+        var name = getName(symbol);
         var metadata = [];
         if (t is List) {
           metadata = t.skip(1).toList();
           t = t[0];
         }
-        _declarations[symbol] = new JsVariableMirror._(name, _wrap(t), metadata);
+        _declarations[symbol] = new JsVariableMirror._(symbol, _wrap(t), metadata);
       });
       var statics = _getStatics(unwrapped);
-      statics.forEach((String name, ft) {
-        var symbol = new Symbol(name);
-        _declarations[symbol] = new JsMethodMirror._staticMethod(this, name, ft);
+      statics.forEach((symbol, ft) {
+        var name = getName(symbol);
+        _declarations[symbol] = new JsMethodMirror._staticMethod(this, symbol, ft);
       });
       var staticGetters = _getStaticGetters(unwrapped);
-      staticGetters.forEach((String name, ft) {
-        var symbol = new Symbol(name);
-        _declarations[symbol] = new JsMethodMirror._staticMethod(this, name, ft);
+      staticGetters.forEach((symbol, ft) {
+        var name = getName(symbol);
+        _declarations[symbol] = new JsMethodMirror._staticMethod(this, symbol, ft);
       });
       var staticSetters = _getStaticSetters(unwrapped);
-      staticSetters.forEach((String name, ft) {
-        var symbol = new Symbol(name);
-        _declarations[symbol] = new JsMethodMirror._staticMethod(this, name, ft);
+      staticSetters.forEach((symbol, ft) {
+        var name = getName(symbol);
+        _declarations[symbol] = new JsMethodMirror._staticMethod(this, symbol, ft);
       });
       _declarations = new Map<Symbol, DeclarationMirror>.unmodifiable(_declarations);
     }
@@ -455,23 +478,43 @@ class JsClassMirror extends JsMirror implements ClassMirror {
     }
   }
 
+  ClassMirror get mixin {
+    if (_mixin != null) {
+      return _mixin;
+    }
+    var mixins = _getMixins(_unwrap(_cls));
+    if (mixins == null || mixins.length == 0) {

[Bob Nystrom, 2016/11/29 21:28:13]

".length == 0" -> ".isEmpty"

[vsm, 2016/11/29 21:41:05]

Done.

+      _mixin = this;

[Bob Nystrom, 2016/11/29 21:28:13]

What's this about? A class is its own mixin?

[vsm, 2016/11/29 21:41:05]

Yep.

https://api.dartlang.org/stable/1.20.1/dart-mirrors/ClassMirror/mixin.html

reflectClass(Foo) == reflectClass(Foo).mixin

if Foo isn't the result of a mixin.

+      return _mixin;
+    }
+    if (mixins.length > 1) {
+      throw new UnsupportedError("ClassMirror.mixin not yet supported for "
+        "classes ($_cls) with multiple mixins");
+    }
+    _mixin = reflectType(_wrap(mixins[0]));
+    return _mixin;
+  }
+
   String toString() => "ClassMirror on '$_cls'";
 }
 
 class JsVariableMirror extends JsMirror implements VariableMirror {
+  final Symbol _symbol;
   final String _name;
   final TypeMirror type;
   final List<InstanceMirror> metadata;
 
   // TODO(vsm): Refactor this out.
-  Symbol get simpleName => new Symbol(_name);
+  Symbol get simpleName => _symbol;
 
   // TODO(vsm): Fix this
   final bool isStatic = false;
   final bool isFinal = false;
 
-  JsVariableMirror._(this._name, Type t, List annotations)
-      : type = reflectType(t),
+  JsVariableMirror._(Symbol symbol, Type t, List annotations)
+      : _symbol = symbol,
+        _name = getName(symbol),
+        type = reflectType(t),
         metadata = new List<InstanceMirror>.unmodifiable(
             annotations.map((a) => reflect(a)));
 
@@ -479,14 +522,14 @@ class JsVariableMirror extends JsMirror implements VariableMirror {
 }
 
 class JsParameterMirror extends JsVariableMirror implements ParameterMirror {
-  JsParameterMirror._(String name, Type t, List annotations)
-      : super._(name, t, annotations);
+  JsParameterMirror._(Symbol member, Type t, List annotations)
+      : super._(member, t, annotations);
 
   String toString() => "ParameterMirror on '$_name'";
 }
 
 class JsMethodMirror extends JsMirror implements MethodMirror {
-  // TODO(vsm): This could be a JS symbol for private methods
+  final Symbol _symbol;
   final String _name;
   List<ParameterMirror> _params;
   List<InstanceMirror> _metadata;
@@ -499,25 +542,25 @@ class JsMethodMirror extends JsMirror implements MethodMirror {
   bool get isPrivate => _name.startsWith('_');
 
   // TODO(vsm): Refactor this out.
-  Symbol get simpleName => new Symbol(_name);
+  Symbol get simpleName => _symbol;
 
-  JsMethodMirror._constructor(JsClassMirror cls, String name, ftype)
-    : _name = name, isConstructor = true, isStatic = false {
+  JsMethodMirror._constructor(JsClassMirror cls, Symbol symbol, ftype)
+    : _symbol = symbol, _name = getName(symbol), isConstructor = true, isStatic = false {
       _createParameterMirrorList(ftype);
   }
 
-  JsMethodMirror._instanceMethod(JsClassMirror cls, String name, ftype)
-    : _name = name, isConstructor = false, isStatic = false {
+  JsMethodMirror._instanceMethod(JsClassMirror cls, Symbol symbol, ftype)
+    : _symbol = symbol, _name = getName(symbol), isConstructor = false, isStatic = false {
       _createParameterMirrorList(ftype);
   }
 
-  JsMethodMirror._staticMethod(JsClassMirror cls, String name, ftype)
-    : _name = name, isConstructor = false, isStatic = true {
+  JsMethodMirror._staticMethod(JsClassMirror cls, Symbol symbol, ftype)
+    : _symbol = symbol, _name = getName(symbol), isConstructor = false, isStatic = true {
       _createParameterMirrorList(ftype);
   }
 
   // TODO(vsm): Support named constructors.
-  Symbol get constructorName => isConstructor ? new Symbol(_name) : null;
+  Symbol get constructorName => isConstructor ? _symbol : null;
   List<ParameterMirror> get parameters => _params;
   List<InstanceMirror> get metadata => _metadata;
 
@@ -556,7 +599,7 @@ class JsMethodMirror extends JsMirror implements MethodMirror {
       var type = args[i];
       var metadata = ftype.metadata[i];
       // TODO(vsm): Recover the param name.
-      var param = new JsParameterMirror._('', _wrap(type), metadata);
+      var param = new JsParameterMirror._(new Symbol(''), _wrap(type), metadata);
       params[i] = param;
     }
 
@@ -564,7 +607,7 @@ class JsMethodMirror extends JsMirror implements MethodMirror {
       var type = opts[i];
       var metadata = ftype.metadata[args.length + i];
       // TODO(vsm): Recover the param name.
-      var param = new JsParameterMirror._('', _wrap(type), metadata);
+      var param = new JsParameterMirror._(new Symbol(''), _wrap(type), metadata);
       params[i + args.length] = param;
     }