[WIP] feat: Removing implicit map dereference in expressions

Closes dart-archive#915

Before this PR a.b would have match either a.b or a['b']. The reason is
that the context was a Map (before dart-archive#914) and we don't want to write
ctrl['foo'] but ctrl.foo. It was also not possible to access Map
properties, ie map.length would have returned map['length'].

Now "a.b" return a.b and "a['b']" return a['b']

Author: vicb

lib/change_detection/dirty_checking_change_detector.dart

@@ -460,22 +460,17 @@ class DirtyCheckingRecord<H> implements WatchRecord<H> {
       return;
     }
 
-    if (object is Map) {
-      _mode =  _MODE_MAP_FIELD_;
-      _getter = null;
-    } else {
-      while (object is ContextLocals) {
-        var ctx = object as ContextLocals;
-        if (ctx.hasProperty(field)) {
-          _mode =  _MODE_MAP_FIELD_;
-          _getter = null;
-          return;
-        }
-        object = ctx.parentContext;
+    while (object is ContextLocals) {
+      var ctx = object as ContextLocals;
+      if (ctx.hasProperty(field)) {
+        _mode =  _MODE_MAP_FIELD_;
+        _getter = null;
+        return;
       }
-      _mode = _MODE_GETTER_OR_METHOD_CLOSURE_;
-      _getter = _fieldGetterFactory.getter(object, field);
+      object = ctx.parentContext;
     }
+    _mode = _MODE_GETTER_OR_METHOD_CLOSURE_;
+    _getter = _fieldGetterFactory.getter(object, field);
   }
 
   bool check() {

lib/core/parser/eval_access.dart

@@ -47,7 +47,6 @@ abstract class AccessFast {
 
   dynamic _eval(holder) {
     if (holder == null) return null;
-    if (holder is Map) return holder[name];
     return getter(holder);
   }
 
@@ -56,7 +55,6 @@ abstract class AccessFast {
       _assignToNonExisting(scope, value);
       return value;
     } else {
-      if (holder is Map) return holder[name] = value;
       return setter(holder, value);
     }
   }

lib/core/parser/parser_dynamic.dart

@@ -8,24 +8,14 @@ class DynamicClosureMap implements ClosureMap {
   final Map<String, Symbol> symbols = {};
   Getter lookupGetter(String name) {
     var symbol = new Symbol(name);
-    return (o) {
-      if (o is Map) {
-        return o[name];
-      } else {
-        return reflect(o).getField(symbol).reflectee;
-      }
-    };
+    return (o) => reflect(o).getField(symbol).reflectee;
   }
 
   Setter lookupSetter(String name) {
     var symbol = new Symbol(name);
     return (o, value) {
-      if (o is Map) {
-        return o[name] = value;
-      } else {
-        reflect(o).setField(symbol, value);
-        return value;
-      }
+      reflect(o).setField(symbol, value);
+      return value;
     };
   }
 
@@ -37,19 +27,10 @@ class DynamicClosureMap implements ClosureMap {
         var symbol = symbols.putIfAbsent(name, () => new Symbol(name));
         sNamedArgs[symbol] = value;
       });
-      if (o is Map) {
-        var fn = o[name];
-        if (fn is Function) {
-          return Function.apply(fn, posArgs, sNamedArgs);
-        } else {
-          throw "Property '$name' is not of type function.";
-        }
-      } else {
-        try {
-          return reflect(o).invoke(symbol, posArgs, sNamedArgs).reflectee;
-        } on NoSuchMethodError catch (e) {
-          throw 'Undefined function $name';
-        }
+      try {
+        return reflect(o).invoke(symbol, posArgs, sNamedArgs).reflectee;
+      } on NoSuchMethodError catch (e) {
+        throw 'Undefined function $name';
       }
     };
   }

lib/core/parser/parser_static.dart

@@ -27,16 +27,7 @@ class StaticClosureMap extends ClosureMap {
     return (o, posArgs, namedArgs) {
       var sNamedArgs = {};
       namedArgs.forEach((name, value) => sNamedArgs[symbols[name]] = value);
-      if (o is Map) {
-        var fn = o[name];
-        if (fn is Function) {
-          return Function.apply(fn, posArgs, sNamedArgs);
-        } else {
-          throw "Property '$name' is not of type function.";
-        }
-      } else {
-        return Function.apply(fn(o), posArgs, sNamedArgs);
-      }
+      return Function.apply(fn(o), posArgs, sNamedArgs);
     };
   }
 

lib/mock/module.dart

@@ -17,6 +17,7 @@ import 'dart:async' as dart_async;
 import 'dart:collection' show ListBase;
 import 'dart:html';
 import 'dart:js' as js;
+import 'dart:mirrors';
 
 import 'package:angular/angular.dart';
 import 'package:angular/core/module_internal.dart';
@@ -65,5 +66,33 @@ class AngularMockModule extends Module {
         ..onError = (e, s, LongStackTrace ls) => dump('EXCEPTION: $e\n$s\n$ls');
     });
     bind(Window, toImplementation: MockWindow);
+    bind(Object, toImplementation: TestContext);
+  }
+}
+
+class TestContext {
+  Map _locals = {};
+
+  void operator []=(k, v) {
+    _locals[k] = v;
+  }
+
+  dynamic operator[](k) => _locals[k];
+
+  noSuchMethod(Invocation invocation) {
+    var pArgs = invocation.positionalArguments;
+    var field = MirrorSystem.getName(invocation.memberName);
+    if (invocation.isGetter) {
+      return _locals[field];
+    }
+    if (invocation.isSetter) {
+      field = field.substring(0, field.length - 1);
+      _locals[field] = pArgs[0];
+      return;
+    }
+    if (invocation.isMethod) {
+      return Function.apply(_locals[field], pArgs, invocation.namedArguments);
+    }
+    throw new UnimplementedError(field);
   }
 }

test/change_detection/dirty_checking_change_detector_spec.dart

@@ -76,7 +76,8 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
       });
 
       it('should treat map field dereference as []', () {
-        var obj = {'name':'misko'};
+        var obj = new TestContext();
+        obj.name = 'misko';
         detector.watch(obj, 'name', null);
         detector.collectChanges(); // throw away first set
 
@@ -90,7 +91,7 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
 
     describe('insertions / removals', () {
       it('should insert at the end of list', () {
-        var obj = {};
+        var obj = new TestContext();
         var a = detector.watch(obj, 'a', 'a');
         var b = detector.watch(obj, 'b', 'b');
 
@@ -116,7 +117,7 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
       });
 
       it('should remove all watches in group and group\'s children', () {
-        var obj = {};
+        var obj = new TestContext();
         detector.watch(obj, 'a', '0a');
         var child1a = detector.newGroup();
         var child1b = detector.newGroup();
@@ -138,11 +139,10 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
       });
 
       it('should add watches within its own group', () {
-        var obj = {};
+        var obj = new TestContext();
         var ra = detector.watch(obj, 'a', 'a');
         var child = detector.newGroup();
         var cb = child.watch(obj,'b', 'b');
-        var iterotar;
 
         obj['a'] = obj['b'] = 1;
         expect(detector.collectChanges(), toEqualChanges(['a', 'b']));
@@ -723,6 +723,12 @@ void main() {
       "toString": (o) => o.toString,
       "isUnderAge": (o) => o.isUnderAge,
       "isUnderAgeAsVariable": (o) => o.isUnderAgeAsVariable,
+      "a": (o) => o.a,
+      "b": (o) => o.b,
+      "name": (o) => o.name,
+      "someField": (o) => o.someField,
+      "f1": (o) => o.f1,
+      "f2": (o) => o.f2,
   }));
 }
 

test/change_detection/watch_group_spec.dart

@@ -23,7 +23,7 @@ void main() {
     ExpressionVisitor visitor;
 
     beforeEach(inject((Logger _logger, Parser _parser) {
-      context = {};
+      context = new TestContext();
       var getterFactory = new DynamicFieldGetterFactory();
       changeDetector = new DirtyCheckingChangeDetector(getterFactory);
       watchGrp = new RootWatchGroup(getterFactory, changeDetector, context);
@@ -217,8 +217,8 @@ void main() {
         // should fire on initial adding
         expect(watchGrp.fieldCost).toEqual(0);
         expect(changeDetector.count).toEqual(0);
-        var watch = watchGrp.watch(parse('a.b'), (v, p) => logger(v));
-        expect(watch.expression).toEqual('a.b');
+        var watch = watchGrp.watch(parse('a["b"]'), (v, p) => logger(v));
+        expect(watch.expression).toEqual('[](a, "b")');
         expect(watchGrp.fieldCost).toEqual(2);
         expect(changeDetector.count).toEqual(2);
         watchGrp.detectChanges();
@@ -260,8 +260,8 @@ void main() {
         // should fire on initial adding
         expect(watchGrp.fieldCost).toEqual(0);
         var watch = watchGrp.watch(parse('user'), (v, p) => logger(v));
-        var watchFirst = watchGrp.watch(parse('user.first'), (v, p) => logger(v));
-        var watchLast = watchGrp.watch(parse('user.last'), (v, p) => logger(v));
+        var watchFirst = watchGrp.watch(parse('user["first"]'), (v, p) => logger(v));
+        var watchLast = watchGrp.watch(parse('user["last"]'), (v, p) => logger(v));
         expect(watchGrp.fieldCost).toEqual(3);
 
         watchGrp.detectChanges();
@@ -290,7 +290,7 @@ void main() {
 
         FunctionApply fn = new LoggingFunctionApply(logger);
         var watch = watchGrp.watch(
-            new PureFunctionAST('add', fn, [parse('a.val')]),
+            new PureFunctionAST('add', fn, [parse('a["val"]')]),
             (v, p) => logger(v)
         );
 
@@ -316,7 +316,7 @@ void main() {
         var watch = watchGrp.watch(
            new PureFunctionAST('add',
                (a, b) { logger('+'); return a+b; },
-               [parse('a.val'), parse('b.val')]
+               [parse('a["val"]'), parse('b["val"]')]
            ),
            (v, p) => logger(v)
         );
@@ -361,7 +361,7 @@ void main() {
         var watch = watchGrp.watch(
             new ClosureAST('sum',
                 (a, b) { logger('+'); return innerState+a+b; },
-                [parse('a.val'), parse('b.val')]
+                [parse('a["val"]'), parse('b["val"]')]
             ),
             (v, p) => logger(v)
         );
@@ -414,11 +414,11 @@ void main() {
 
         var a_plus_b = new PureFunctionAST('add1',
             (a, b) { logger('$a+$b'); return a + b; },
-            [parse('a.val'), parse('b.val')]);
+            [parse('a["val"]'), parse('b["val"]')]);
 
         var a_plus_b_plus_c = new PureFunctionAST('add2',
             (b, c) { logger('$b+$c'); return b + c; },
-            [a_plus_b, parse('c.val')]);
+            [a_plus_b, parse('c["val"]')]);
 
         var watch = watchGrp.watch(a_plus_b_plus_c, (v, p) => logger(v));
 
@@ -762,8 +762,8 @@ void main() {
       });
 
       it('should support custom context', () {
-        expect(eval('x', {'x': 42})).toBe(42);
-        expect(eval('x', {'x': 87})).toBe(87);
+        expect(eval('x', new Context(42))).toBe(42);
+        expect(eval('x', new Context(87))).toBe(87);
       });
 
       it('should support named arguments for scope calls', () {
@@ -857,10 +857,10 @@ void main() {
         watchGrp.watch(countMethod, (v, p) => logger('0a'));
         expectOrder(['0a']);
 
-        var child1a = watchGrp.newGroup({'my': myClass});
-        var child1b = watchGrp.newGroup({'my': myClass});
-        var child2 = child1a.newGroup({'my': myClass});
-        var child3 = child2.newGroup({'my': myClass});
+        var child1a = watchGrp.newGroup(new ContextLocals(context, {'my': myClass}));
+        var child1b = watchGrp.newGroup(new ContextLocals(context, {'my': myClass}));
+        var child2 = child1a.newGroup(new ContextLocals(context, {'my': myClass}));
+        var child3 = child2.newGroup(new ContextLocals(context, {'my': myClass}));
         child1a.watch(countMethod, (v, p) => logger('1a'));
         expectOrder(['0a', '1a']);
         child1b.watch(countMethod, (v, p) => logger('1b'));
@@ -888,7 +888,7 @@ void main() {
         context['my'] = myClass;
         AST countMethod = new MethodAST(parse('my'), 'count', []);
         var ra = watchGrp.watch(countMethod, (v, p) => logger('a'));
-        var child = watchGrp.newGroup({'my': myClass});
+        var child = watchGrp.newGroup(new ContextLocals(context, {'my': myClass}));
         var cb = child.watch(countMethod, (v, p) => logger('b'));
 
         expectOrder(['a', 'b']);
@@ -975,3 +975,8 @@ class LoggingFunctionApply extends FunctionApply {
   LoggingFunctionApply(this.logger);
   apply(List args) => logger(args);
 }
+
+class Context {
+  var x;
+  Context(this.x);
+}