Adapting observe to use smoke (this is built on top of the previous change that

pkg/observe/lib/src/observable.dart

Index: pkg/observe/lib/src/observable.dart
diff --git a/pkg/observe/lib/src/observable.dart b/pkg/observe/lib/src/observable.dart
index 35e91c8912e989ececca48a1e89f0f74c384c686..a5bb209fce4a3a416737b5a27a69e685cd9db22d 100644
--- a/pkg/observe/lib/src/observable.dart
+++ b/pkg/observe/lib/src/observable.dart
@@ -7,14 +7,16 @@ library observe.src.observable;
 import 'dart:async';
 import 'dart:collection';
 
+// TODO(sigmund): figure out how to remove this annotation entirely
 // Note: ObservableProperty is in this list only for the unusual use case of
 // dart2js without deploy tool. The deploy tool (see "transformer.dart") will
 // add the @reflectable annotation, which makes it work with Polymer's
 // @published.
-@MirrorsUsed(metaTargets: const [Reflectable, ObservableProperty],
-    override: 'observe.src.observable')
-import 'dart:mirrors';
-
+@unused.MirrorsUsed(
+    metaTargets: const [Reflectable, ObservableProperty],
+    override: 'smoke.mirrors')
+import 'dart:mirrors' as unused;
+import 'package:smoke/smoke.dart' as smoke;
 import 'package:observe/observe.dart';
 
 // Note: this is an internal library so we can import it from tests.
@@ -46,7 +48,6 @@ abstract class Observable {
   static void dirtyCheck() => dirtyCheckObservables();
 
   StreamController _changes;
-  InstanceMirror _mirror;
 
   Map<Symbol, Object> _values;
   List<ChangeRecord> _records;
@@ -75,32 +76,23 @@ abstract class Observable {
     // Register this object for dirty checking purposes.
     registerObservable(this);
 
-    var mirror = reflect(this);
     var values = new Map<Symbol, Object>();
 
     // Note: we scan for @observable regardless of whether the base type
     // actually includes this mixin. While perhaps too inclusive, it lets us
     // avoid complex logic that walks "with" and "implements" clauses.
-    for (var type = mirror.type; type != objectType; type = type.superclass) {
-      for (var field in type.declarations.values) {
-        if (field is! VariableMirror ||
-            field.isFinal ||
-            field.isStatic ||
-            field.isPrivate) continue;
-
-        for (var meta in field.metadata) {
-          if (meta.reflectee is ObservableProperty) {
-            var name = field.simpleName;
-            // Note: since this is a field, getting the value shouldn't execute
-            // user code, so we don't need to worry about errors.
-            values[name] = mirror.getField(name).reflectee;
-            break;
-          }
-        }
-      }
+    // TODO(sigmund): should we exclude getters here? users should only use
+    // @observable on fields, but 'smoke' includes getters with fields (might
+    // need to extend smoke for this). See also note below on `smoke.read`.
+    var queryOptions = new smoke.QueryOptions(includeInherited: true,
+        withAnnotations: const [ ObservableProperty ]);
+    for (var decl in smoke.query(this.runtimeType, queryOptions)) {
+      var name = decl.name;
+      // Note: since this is a field, getting the value shouldn't execute
+      // user code, so we don't need to worry about errors.
+      values[name] = smoke.read(this, name);
     }
 
-    _mirror = mirror;
     _values = values;
   }
 
@@ -109,7 +101,6 @@ abstract class Observable {
     // Note: we don't need to explicitly unregister from the dirty check list.
     // This will happen automatically at the next call to dirtyCheck.
     if (_values != null) {
-      _mirror = null;
       _values = null;
     }
   }
@@ -145,7 +136,7 @@ abstract class Observable {
     _records = null;
 
     _values.forEach((name, oldValue) {
-      var newValue = _mirror.getField(name).reflectee;
+      var newValue = smoke.read(this, name);
       if (oldValue != newValue) {
         if (records == null) records = [];
         records.add(new PropertyChangeRecord(this, name, oldValue, newValue));
@@ -203,5 +194,4 @@ notifyPropertyChangeHelper(Observable obj, Symbol field, Object oldValue,
   return newValue;
 }
 
-// NOTE: this is not exported publically.
-final objectType = reflectClass(Object);
+_unused() => unused.MirrorSystem; // to prevent analyzer warnings

[kasperl, 2014/02/20 10:16:18]

File analyzer bug for this? The import is clearly used because we get
MirrorsUsed from it.

[Siggi Cherem (dart-lang), 2014/02/20 16:47:48]

Thanks for pointing this one out. It seems that this is now handled correctly by
the analyzer. I was probably confused because in another place I had to include
dart:mirrors without any @MirrorsUsed annotation (to preserve everything), so I
had to add this there as well.