fix PathObserver to avoid try+catch

pkg/observe/lib/src/path_observer.dart

 // Copyright (c) 2013, 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.
 
-part of observe;
+library observe.src.path_observer;
+
+import 'dart:async';
+@MirrorsUsed(metaTargets: const [Reflectable, ObservableProperty],
+    override: 'observe.src.path_observer')
+import 'dart:mirrors';
+import 'package:logging/logging.dart' show Logger, Level;
+import 'package:observe/observe.dart';
+import 'package:observe/src/observable.dart' show objectType;
 
 // This code is inspired by ChangeSummary:
 // https://github.com/rafaelw/ChangeSummary/blob/master/change_summary.js
 // ...which underlies MDV. Since we don't need the functionality of
 // ChangeSummary, we just implement what we need for data bindings.
 // This allows our implementation to be much simpler.
 
 /**
  * A data-bound path starting from a view-model or model object, for example
  * `foo.bar.baz`.
@@ skipping 52 matching lines @@
     return _values.last;
   }
 
   /** Sets the value at this path. */
   @reflectable void set value(Object value) {
     int len = _segments.length;
 
     // TODO(jmesserly): throw if property cannot be set?
     // MDV seems tolerant of these errors.
     if (len == 0) return;
-    if (!hasObservers) _updateValues();
+    if (!hasObservers) _updateValues(end: len - 1);
 
     if (_setObjectProperty(_values[len - 1], _segments[len - 1], value)) {
       // Technically, this would get updated asynchronously via a change record.
       // However, it is nice if calling the getter will yield the same value
       // that was just set. So we use this opportunity to update our cache.
       _values[len] = value;
     }
   }
 
   /**
    * Invokes the [callback] immediately with the current [value], and every time
    * the value changes. This is useful for bindings, which want to be up-to-date
    * immediately and stay bound to the value of the path.
    */
   StreamSubscription bindSync(void callback(value)) {
     var result = changes.listen((records) { callback(value); });
     callback(value);
     return result;
   }
 
-  void _observed() {
-    super._observed();
+  void observed() {
+    super.observed();
     _updateValues();
     _observePath();
   }
 
-  void _unobserved() {
+  void unobserved() {
     for (int i = 0; i < _subs.length; i++) {
       if (_subs[i] != null) {
         _subs[i].cancel();
         _subs[i] = null;
       }
     }
+    super.unobserved();
   }
 
   // TODO(jmesserly): should we be caching these values if not observing?
-  void _updateValues() {
-    for (int i = 0; i < _segments.length; i++) {
+  void _updateValues({int end}) {
+    if (end == null) end = _segments.length;
+    for (int i = 0; i < end; i++) {
       _values[i + 1] = _getObjectProperty(_values[i], _segments[i]);
     }
   }
 
-  void _updateObservedValues([int start = 0]) {
+  void _updateObservedValues({int start: 0}) {
     var oldValue, newValue;
     for (int i = start; i < _segments.length; i++) {
       oldValue = _values[i + 1];
       newValue = _getObjectProperty(_values[i], _segments[i]);
       if (identical(oldValue, newValue)) {
         _observePath(start, i);
         return;
       }
       _values[i + 1] = newValue;
     }
@@ skipping 20 matching lines @@
       // PropertyChangeRecord is modified to includes the sender object), so
       // we would need to re-evaluate the entire path. Need to evaluate perf.
       _subs[i] = object.changes.listen((List<ChangeRecord> records) {
         if (!identical(_values[i], object)) {
           // Ignore this object if we're now tracking something else.
           return;
         }
 
         for (var record in records) {
           if (_changeRecordMatches(record, _segments[i])) {
-            _updateObservedValues(i);
+            _updateObservedValues(start: i);
             return;
           }
         }
       });
     }
   }
 }
 
 bool _changeRecordMatches(record, key) {
   if (record is ListChangeRecord) {
     return key is int && (record as ListChangeRecord).indexChanged(key);
   }
   if (record is PropertyChangeRecord) {
     return (record as PropertyChangeRecord).name == key;
   }
   if (record is MapChangeRecord) {
     if (key is Symbol) key = MirrorSystem.getName(key);
     return (record as MapChangeRecord).key == key;
   }
   return false;
 }
 
 _getObjectProperty(object, property) {
-  if (object == null) {
-    return null;
-  }
+  if (object == null) return null;
 
-  if (object is List && property is int) {
-    if (property >= 0 && property < object.length) {
+  if (property is int) {
+    if (object is List && property >= 0 && property < object.length) {
       return object[property];
-    } else {
-      return null;
+    }
+  } else if (property is Symbol) {
+    var mirror = reflect(object);
+    final type = mirror.type;
+    try {
+      if (_canGetProperty(type, property)) {
+        return mirror.getField(property).reflectee;
+      }
+      // Support indexer if available, e.g. Maps or polymer_expressions Scope.
+      if (_hasMethod(type, const Symbol('[]'))) {

[Siggi Cherem (dart-lang), 2013/10/29 21:48:38]

does the # grammar support `#[]`?

[Jennifer Messerly, 2013/10/29 22:03:30]

crazy. apparently yes. changed.

+        return object[MirrorSystem.getName(property)];

[Siggi Cherem (dart-lang), 2013/10/29 21:48:38]

interesting, this will let people use:
  $.id.foo

instead of 
  $['id'].foo

?

[Jennifer Messerly, 2013/10/29 22:03:30]

yes. This has always been the case with PathObserver and the built-in
TemplateBinding syntax.

[Jennifer Messerly, 2013/10/29 22:10:33]

added a comment

+      }
+    } on NoSuchMethodError catch (e) {
+      // Rethrow, unless the type implements noSuchMethod, in which case we
+      // interpret the exception as a signal that the method was not found.
+      if (!_hasMethod(type, #noSuchMethod)) rethrow;

[Siggi Cherem (dart-lang), 2013/10/29 21:48:38]

given your earlier check with _canGetProperty, what is a scenario where this
condition is false? (i.e. aren't we always rethrowing now?)

[Jennifer Messerly, 2013/10/29 22:03:30]

consider the following:
* the object has noSuchMethod
* _canGetProperty with true -- yes, it might have the property!
* getField calls noSuchMethod, which throws
* we do not want to rethrow this error. And we don't.

[Jennifer Messerly, 2013/10/29 22:10:33]

renamed to _maybeHasGetter/_maybeHasSetter

     }
   }
 
-  if (property is Symbol) {
-    var mirror = reflect(object);
-    var result = _tryGetField(mirror, property);
-    if (result != null) return result.reflectee;
+  if (_logger.isLoggable(Level.FINER)) {
+    _logger.log("can't get $property in $object");
   }
-
-  if (object is Map) {
-    if (property is Symbol) property = MirrorSystem.getName(property);
-    return object[property];
-  }
-
   return null;
 }
 
 bool _setObjectProperty(object, property, value) {
-  if (object is List && property is int) {
-    if (property >= 0 && property < object.length) {
+  if (object == null) return false;
+
+  if (property is int) {
+    if (object is List && property >= 0 && property < object.length) {
       object[property] = value;
       return true;
-    } else {
-      return false;
+    }
+  } else if (property is Symbol) {
+    var mirror = reflect(object);
+    final type = mirror.type;
+    try {
+      if (_canSetProperty(type, property)) {
+        mirror.setField(property, value);
+        return true;
+      }
+      // Support indexer if available, e.g. Maps or polymer_expressions Scope.
+      if (_hasMethod(type, const Symbol('[]='))) {
+        object[MirrorSystem.getName(property)] = value;
+        return true;
+      }
+    } on NoSuchMethodError catch (e) {
+      if (!_hasMethod(type, #noSuchMethod)) rethrow;
     }
   }
 
-  if (property is Symbol) {
-    var mirror = reflect(object);
-    if (_trySetField(mirror, property, value)) return true;
+  if (_logger.isLoggable(Level.FINER)) {
+    _logger.log("can't set $property in $object");
   }
-
-  if (object is Map) {
-    if (property is Symbol) property = MirrorSystem.getName(property);
-    object[property] = value;
-    return true;
-  }
-
   return false;
 }
 
-InstanceMirror _tryGetField(InstanceMirror mirror, Symbol name) {
-  try {
-    return mirror.getField(name);
-  } on NoSuchMethodError catch (e) {
-    if (_hasMember(mirror, name, (m) =>
-        m is VariableMirror || m is MethodMirror && m.isGetter)) {
-      // The field/getter is there but threw a NoSuchMethod exception.
-      // This is a legitimate error in the code so rethrow.
-      rethrow;
-    }
-    // The field isn't there. PathObserver does not treat this as an error.
-    return null;
+bool _canGetProperty(ClassMirror type, Symbol name) {
+  while (type != objectType) {
+    final members = type.members;
+    if (members.containsKey(name)) return true;
+    if (members.containsKey(#noSuchMethod)) return true;
+    type = _safeSuperclass(type);
   }
-}
-
-bool _trySetField(InstanceMirror mirror, Symbol name, Object value) {
-  try {
-    mirror.setField(name, value);
-    return true;
-  } on NoSuchMethodError catch (e) {
-    if (_hasMember(mirror, name, (m) => m is VariableMirror) ||
-        _hasMember(mirror, _setterName(name))) {
-      // The field/setter is there but threw a NoSuchMethod exception.
-      // This is a legitimate error in the code so rethrow.
-      rethrow;
-    }
-    // The field isn't there. PathObserver does not treat this as an error.
-    return false;
-  }
+  return false;
 }
 
 // TODO(jmesserly): workaround for:
 // https://code.google.com/p/dart/issues/detail?id=10029
 Symbol _setterName(Symbol getter) =>
     new Symbol('${MirrorSystem.getName(getter)}=');
 
-bool _hasMember(InstanceMirror mirror, Symbol name, [bool test(member)]) {
-  var type = mirror.type;
-  while (type != null) {
-    final member = type.members[name];
-    if (member != null && (test == null || test(member))) return true;
-
-    try {
-      type = type.superclass;
-    } on UnsupportedError catch (e) {
-      // TODO(jmesserly): dart2js throws this error when the type is not
-      // reflectable.
-      return false;
-    }
+bool _canSetProperty(ClassMirror type, Symbol name) {
+  var setterName = _setterName(name);
+  while (type != objectType) {
+    final members = type.members;
+    if (members[name] is VariableMirror) return true;
+    if (members.containsKey(setterName)) return true;
+    if (members.containsKey(#noSuchMethod)) return true;
+    type = _safeSuperclass(type);
   }
   return false;
 }
 
+/**
+ * True if the type has a method, other than on Object.
+ * Doesn't consider noSuchMethod, unless [name] is `#noSuchMethod`.
+ */
+bool _hasMethod(ClassMirror type, Symbol name) {
+  while (type != objectType) {
+    final member = type.members[name];
+    if (member is MethodMirror && member.isRegularMethod) return true;
+    type = _safeSuperclass(type);
+  }
+  return false;
+}
+
+ClassMirror _safeSuperclass(ClassMirror type) {
+  try {
+    return type.superclass;
+  } on UnsupportedError catch (e) {
+    // TODO(jmesserly): dart2js throws this error when the type is not
+    // reflectable.
+    return objectType;
+  }
+}
+
 // From: https://github.com/rafaelw/ChangeSummary/blob/master/change_summary.js
 
 final _pathRegExp = () {
   const identStart = '[\$_a-zA-Z]';
   const identPart = '[\$_a-zA-Z0-9]';
   const ident = '$identStart+$identPart*';
   const elementIndex = '(?:[0-9]|[1-9]+[0-9]+)';
   const identOrElementIndex = '(?:$ident|$elementIndex)';
   const path = '(?:$identOrElementIndex)(?:\\.$identOrElementIndex)*';
   return new RegExp('^$path\$');
 }();
 
 final _spacesRegExp = new RegExp(r'\s');
 
 bool _isPathValid(String s) {
   s = s.replaceAll(_spacesRegExp, '');
 
   if (s == '') return true;
   if (s[0] == '.') return false;
   return _pathRegExp.hasMatch(s);
 }
+
+final _logger = new Logger('observe.PathObserver');