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

@@ -39,15 +39,13 @@ class AccessKeyed extends syntax.AccessKeyed {
  * use to do the access the field.
  */
 // todo(vicb) - parser should not depend on ContextLocals
-// todo(vicb) - Map should not be a special case so that we can access the props
 abstract class AccessFast {
   String get name;
   Getter get getter;
   Setter get setter;
 
   dynamic _eval(holder) {
     if (holder == null) return null;
-    if (holder is Map) return holder[name];
     return getter(holder);
   }
 
@@ -56,7 +54,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/core/scope.dart

@@ -96,7 +96,7 @@ class ScopeDigestTTL {
  * a `scope` setter:
  *
  *     @Component(...)
- *     class MyComponent {
+ *     class MyComponent implements ScopeAware {
  *       Watch watch;
  *
  *       MyComponent(Dependency myDep) {
@@ -111,7 +111,7 @@ class ScopeDigestTTL {
  *     }
  */
 abstract class ScopeAware {
-  void set context(ctx);
+  void set scope(Scope scope);
 }
 
 /**
@@ -128,7 +128,7 @@ class Scope {
   /**
    * The default execution context for [watch]es [observe]ers, and [eval]uation.
    */
-  final Object context;
+  final context;
 
   /**
    * The [RootScope] of the application.
@@ -165,20 +165,20 @@ class Scope {
   // TODO(misko): WatchGroup should be private.
   // Instead we should expose performance stats about the watches
   // such as # of watches, checks/1ms, field checks, function checks, etc
-  WatchGroup _readWriteGroup;
-  WatchGroup _readOnlyGroup;
+  final WatchGroup _readWriteGroup;
+  final WatchGroup _readOnlyGroup;
 
   Scope _childHead, _childTail, _next, _prev;
   _Streams _streams;
 
   /// Do not use. Exposes internal state for testing.
   bool get hasOwnStreams => _streams != null  && _streams._scope == this;
 
-  Scope(this.context, this.rootScope, this._parentScope,
+  Scope(Object this.context, this.rootScope, this._parentScope,
         this._readWriteGroup, this._readOnlyGroup, this.id,
         this._stats)
   {
-    if (context is ScopeAware) context.scope = this;
+    if (context is ScopeAware) (context as ScopeAware).scope = this;
   }
 
   /**

lib/formatter/filter.dart

@@ -1,8 +1,7 @@
 part of angular.formatter_internal;
 
-// Too bad you can't stick typedef's inside a class.
-typedef bool _Predicate(e);
-typedef bool _Equals(a, b);
+typedef bool _PredicateFn(e);
+typedef bool _CompareFn(a, b);
 
 /**
  * Selects a subset of items from the provided [List] and returns it as a new
@@ -17,37 +16,37 @@ typedef bool _Equals(a, b);
  *
  * The expression can be of the following types:
  *
- * - [String], [bool] and [num]:  Only items in the list that directly
+ * - [String], [bool] and [num]: Only items in the list that directly
  *   match this expression, items that are Maps with any value matching this
  *   item, and items that are lists containing a matching items are returned.
  *
- * - [Map]:  This defines a pattern map.  Filters specific properties on objects
+ * - [Map]:  This defines a pattern map. Filters specific properties on objects
  *   contained in the input list.  For example `{name:"M", phone:"1"}` predicate
  *   will return a list of items which have property `name` containing "M" and
  *   property `phone` containing "1".  A special property name, `$`, can be used
  *   (as in `{$: "text"}`) to accept a match against any property of the object.
  *   That's equivalent to the simple substring match with a `String` as
  *   described above.
  *
- * - [Function]:  This allows you to supply a custom function to filter the
- *   List.  The function is called for each element of the List.  The returned
+ * - [Function]: This allows you to supply a custom function to filter the
+ *   List.  The function is called for each element of the List. The returned
  *   List contains exactly those elements for which this function returned
  *   `true`.
  *
  *
  * The comparator is optional and can be one of the following:
  *
- * - `bool comparator(expected, actual)`:  The function will be called with the
+ * - `bool comparator(expected, actual)`: The function will be called with the
  *   object value and the predicate value to compare and should return true if
  *   the item should be included in filtered result.
  *
  * - `true`:  Specifies that only identical objects matching the expression
- *   exactly should be considered matches.  Two strings are considered identical
- *   if they are equal.  Two numbers are considered identical if they are either
- *   equal or both are `NaN`.  All other objects are identical iff
+ *   exactly should be considered matches. Two strings are considered identical
+ *   if they are equal. Two numbers are considered identical if they are either
+ *   equal or both are `NaN`. All other objects are identical iff
  *   identical(expected, actual) is true.
  *
- * - `false|null`:  Specifies case insensitive substring matching.
+ * - `false|null`: Specifies case insensitive substring matching.
  *
  *
  * # Example ([view in plunker](http://plnkr.co/edit/6Mxz6r?p=info)):
@@ -105,27 +104,30 @@ typedef bool _Equals(a, b);
 @Formatter(name: 'filter')
 class Filter implements Function {
   Parser _parser;
-  _Equals _comparator;
-  _Equals _stringComparator;
+  _CompareFn _comparator;
+  _CompareFn _stringComparator;
 
   static _nop(e) => e;
+
   static _ensureBool(val) => (val is bool && val);
+
   static _isSubstringCaseInsensitive(String a, String b) =>
       a != null && b != null && a.toLowerCase().contains(b.toLowerCase());
+
   static _identical(a, b) => identical(a, b) ||
                              (a is String && b is String && a == b) ||
                              (a is num && b is num && a.isNaN && b.isNaN);
 
   Filter(this._parser);
 
-  void _configureComparator(var comparatorExpression) {
+  void _configureComparator(comparatorExpression) {
     if (comparatorExpression == null || comparatorExpression == false) {
       _stringComparator = _isSubstringCaseInsensitive;
       _comparator = _defaultComparator;
     } else if (comparatorExpression == true) {
       _stringComparator = _identical;
       _comparator = _defaultComparator;
-    } else if (comparatorExpression is _Equals) {
+    } else if (comparatorExpression is _CompareFn) {
       _comparator = (a, b) => _ensureBool(comparatorExpression(a, b));
     } else {
       _comparator = null;
@@ -135,67 +137,58 @@ class Filter implements Function {
   // Preconditions
   // - what: NOT a Map
   // - item: neither a Map nor a List
-  bool _defaultComparator(var item, var what) {
-    if (what == null) {
-      return false;
-    } else if (item == null) {
-      return what == '';
-    } else if (what is String && what.startsWith('!')) {
-      return !_search(item, what.substring(1));
-    } else if (item is String) {
-      return (what is String) && _stringComparator(item, what);
-    } else if (item is bool) {
+  bool _defaultComparator(item, what) {
+    if (what == null) return false;
+    if (item == null) return what == '';
+    if (what is String && what.startsWith('!')) return !_search(item, what.substring(1));
+    if (item is String) return (what is String) && _stringComparator(item, what);
+    if (item is bool) {
       if (what is bool) {
         return item == what;
       } else if (what is String) {
         what = (what as String).toLowerCase();
         return item ? (what == "true"  || what == "yes" || what == "on")
                     : (what == "false" || what == "no"  || what == "off");
-      } else {
-        return false;
       }
-    } else if (item is num) {
-      if (what is num) {
-        return item == what || (item.isNaN && what.isNaN);
-      } else {
-        return what is String && _stringComparator('$item', what);
-      }
-    } else {
-      return false; // Unsupported item type.
+      return false;
     }
+    if (item is num) {
+      return what is num ?
+          item == what || (item.isNaN && what.isNaN) :
+          what is String && _stringComparator('$item', what);
+    }
+    return false; // Unsupported item type.
   }
 
-  bool _search(var item, var what) {
+  bool _search(item, what) {
     if (what is Map) {
       return what.keys.every((key) => _search(
               (key == r'$') ? item : _parser(key).eval(item), what[key]));
-    } else if (item is Map) {
-      return item.keys.any((k) => !k.startsWith(r'$') && _search(item[k], what));
-    } else if (item is List) {
-      return item.any((i) => _search(i, what));
-    } else {
-      return _comparator(item, what);
     }
+    if (item is Map) return item.keys.any((k) => !k.startsWith(r'$') && _search(item[k], what));
+    if (item is List) return item.any((i) => _search(i, what));
+    return _comparator(item, what);
   }
 
-  _Predicate _toPredicate(var expression) {
-    if (expression is _Predicate) {
-      return (item) => _ensureBool(expression(item));
-    } else if (_comparator == null) {
-      return (item) => false; // Bad comparator → no items for you!
-    } else {
-      return (item) => _search(item, expression);
-    }
+  _PredicateFn _toPredicate(var expression) {
+    if (expression is _PredicateFn) return (item) => _ensureBool(expression(item));
+    if (_comparator == null) return (_) => false; // Bad comparator → no items for you!
+    return (item) => _search(item, expression);
   }
 
-  List call(List items, var expression, [var comparator]) {
-    if (expression == null) {
-      return items.toList(growable: false); // Missing expression → passthrough.
-    } else if (expression is! Map && expression is! Function &&
-               expression is! String && expression is! bool &&
-               expression is! num) {
-      return const []; // Bad expression → no items for you!
+  List call(List items, expression, [comparator]) {
+    // Missing expression → passthrough.
+    if (expression == null) return items.toList(growable: false);
+
+    // Bad expression → no items for you!
+    if (expression is! Map &&
+        expression is! Function &&
+        expression is! String &&
+        expression is! bool &&
+        expression is! num) {
+      return const [];
     }
+
     _configureComparator(comparator);
     List results = items.where(_toPredicate(expression)).toList(growable: false);
     _comparator = null;