Changes in polymer-expressions to prepare for codegen in polymer:

pkg/polymer_expressions/lib/eval.dart

Index: pkg/polymer_expressions/lib/eval.dart
diff --git a/pkg/polymer_expressions/lib/eval.dart b/pkg/polymer_expressions/lib/eval.dart
index 7b925a724cac8f21b9c04427bfc9def3fa3466cd..1cd12b1156d29e878060b8413cdc0ebcd05981d5 100644
--- a/pkg/polymer_expressions/lib/eval.dart
+++ b/pkg/polymer_expressions/lib/eval.dart
@@ -7,11 +7,6 @@ library polymer_expressions.eval;
 import 'dart:async';
 import 'dart:collection';
 
-@MirrorsUsed(
-    metaTargets: const [Reflectable, ObservableProperty],
-    override: 'smoke.mirrors')
-import 'dart:mirrors' show MirrorsUsed;
-
 import 'package:observe/observe.dart';
 import 'package:smoke/smoke.dart' as smoke;
 
@@ -147,74 +142,107 @@ void assign(Expression expr, Object value, Scope scope) {
   }
 }
 
+
 /**
- * A mapping of names to objects. Scopes contain a set of named [variables] and
- * a single [model] object (which can be thought of as the "this" reference).
- * Names are currently looked up in [variables] first, then the [model].
- *
- * Scopes can be nested by giving them a [parent]. If a name in not found in a
- * Scope, it will look for it in it's parent.
+ * A scope in polymer expressions that can map names to objects. Scopes contain
+ * a set of named variables and a unique model object. The scope structure
+ * is then used to lookup names using the `[]` operator. The lookup first
+ * searches for the name in the variable set (even if these are global
+ * variables), otherwise it looks up the name as a property in the model.
  */
-class Scope {
-  final Scope parent;
+abstract class Scope {
+  Scope._();
+
+  /** Create a scope containing a [model] and all of [variables]. */
+  factory Scope({Object model, Map<String, Object> variables}) {
+    var scope = new _ModelScope(model);
+    if (variables == null) return scope;
+    variables.forEach((name, value) {
+      scope = new _VariableScope(name, value, scope);

[Jennifer Messerly, 2014/03/21 20:08:54]

hmmm. Is the Map used for globals? We might want something better. Otherwise we
get O(N) lookup.

[Siggi Cherem (dart-lang), 2014/03/21 21:09:47]

good point. Added _GlobalsScope for this.

+    });
+    return scope;
+  }
+
+  /** Return the unique model in this scope. */
+  Object get model;
+
+  /**
+   * Lookup the value of [name] in the current scope. If [name] is 'this', then
+   * we return the [model]. For any other name, this finds the first variable
+   * matching [name] or, if none exists, the property [name] in the [model].
+   */
+  Object operator[](String name);
+
+  /**
+   * Return the object that defines the value of [name]. The result may be a
+   * [_VariableScope] if it is a variable on this scope, [model] if it is a
+   * member of the model object, a [_ModelScope] if name is 'this', or null if
+   * the name can't be found and [model] is null.

[Siggi Cherem (dart-lang), 2014/03/21 01:43:46]

Justin - a general question here:

I was wondering about the comment you had in line 189 (re not returning the
Scope because template destroys and recreates instances). I that still
applicable?

The only place where ownerOf is read is below in line 460 (now 487 after the
change). The immediate next line would say that the result is! Observable, so we
don't need to do anything else.

Right now I'm returning the actual owner, but I could simplify this and return
null also when looking up variables. Any preference about this?

[Siggi Cherem (dart-lang), 2014/03/21 21:09:47]

BWT - after adding the GlobalsScope and with the discussion below about
@observable, I was inclined to change this a bit more. I updated the code to
simply ask if the #name is defined in the model, in which case, we check if the
model is observable and listen for changes.

Later on, if we make the _LocalVariableScope observable, and the _GlobalsScope
map observable, we can revive the logic for _onwerOf like we had here.

+   */
+  Object _ownerOf(String name);
+
+  /** Create a new scope extending this scope with an additional variable. */
+  Scope childScope(String name, Object value) =>
+      new _VariableScope(name, value, this);
+}
+
+/**
+ * A scope that looks up names in a model object. This kind of scope has no
+ * parent scope because all our lookup operations stop when we reach the model
+ * object. Any variables added in scope or global variables are added as child
+ * scopes.
+ */
+class _ModelScope extends Scope {
   final Object model;
-  // TODO(justinfagnani): disallow adding/removing names
-  final ObservableMap<String, Object> _variables;
 
-  Scope({this.model, Map<String, Object> variables, this.parent})
-      : _variables = new ObservableMap.from(variables == null ? {} : variables);
+  _ModelScope(this.model) : super._();
 
   Object operator[](String name) {
-    if (name == 'this') {
-      return model;
-    } else if (_variables.containsKey(name)) {
-      return _convert(_variables[name]);
-    } else {
-      var symbol = smoke.nameToSymbol(name);
-      if (model != null && smoke.hasGetter(model.runtimeType, symbol)) {
-        return _convert(smoke.read(model, symbol));
-      }
-    }
-    if (parent != null) {
-      return _convert(parent[name]);
-    } else {
+    if (name == 'this') return model;
+    var symbol = smoke.nameToSymbol(name);
+    if (model == null || symbol == null) {
       throw new EvalException("variable '$name' not found");
     }
+    return _convert(smoke.read(model, symbol));
   }
 
-  Object ownerOf(String name) {
-    if (name == 'this') {
-      // we could return the Scope if it were Observable, but since assigning
-      // a model to a template destroys and recreates the instance, it doesn't
-      // seem neccessary

[Siggi Cherem (dart-lang), 2014/03/21 01:43:46]

(see comment re _ownerOf)

-      return null;
-    } else if (_variables.containsKey(name)) {
-      return _variables;
-    } else if (smoke.hasGetter(model.runtimeType, smoke.nameToSymbol(name))) {
-      return model;
-    }
-    if (parent != null) {
-      return parent.ownerOf(name);
+  Object _ownerOf(String name) => name == 'this' ? this : model;
+}
+
+/**
+ * A scope that holds a reference to a single variable. Polymer expressions
+ * introduce variables to the scope one at a time. Each time a variable is
+ * added, a new [_VariableScope] is created.
+ */
+class _VariableScope extends Scope {
+  final Scope parent;
+  final String varName;
+  // TODO(sigmund,justinfagnani): make this @observable?
+  final Object value;

[Siggi Cherem (dart-lang), 2014/03/21 01:43:46]

I had this with @observable first (matching what you used to have with
ObservableMap), but then I realized this was never updated, so I removed it. We
could add it back in the future, but I presume we need some additional changes
to make that happen.

[Jennifer Messerly, 2014/03/21 20:08:54]

Not sure I understand the TODO. Shouldn't values always be final? Neither
<template bind="{{x as y}}"> or <template repeat="{{i in list}}"> should be
assignable IMHO

[Siggi Cherem (dart-lang), 2014/03/21 21:09:47]

I was thinking if we were to make those constructs update the model and reuse
the existing template nodes instead of removing and adding nodes. So, for
example, you can update list[2] in place, or in the following example, you could
reuse the span node:

class B { @observable int x; }
class A { B b; }

# same node is reused if a.b changes
<span>{{a.b.x}}</span>

# new node is created if a.b changes (today)
<template bind={{a.b as b}}><span>{{b.x}}</span></template> 

Is this something template-binding can support?

[Jennifer Messerly, 2014/03/22 00:37:56]

It doesn't currently reuse nodes if the model changes. For better or worse the
current TemplateBinding policy is to not recycle DOM nodes. I think you could do
it at that level though.

Working around that at the polymer_expressions layer is an intriguing idea
though.

I think I lean towards trying to fix it inside <template repeat> itself though,
since that would have bigger impact. But, if for standards reasons reusing DOM
nodes won't work in template_binding, then it would be nice to do it here.

+
+  _VariableScope(this.varName, this.value, this.parent) : super._() {
+    if (varName == 'this') {
+      throw new EvalException("'this' cannot be used as a variable name.");
     }
   }
 
-  bool contains(String name) {
-    if (_variables.containsKey(name) ||
-        smoke.hasGetter(model.runtimeType, smoke.nameToSymbol(name))) {
-      return true;
-    }
-    if (parent != null) {
-      return parent.contains(name);
-    }
-    return false;
+  Object get model => parent != null ? parent.model : null;
+
+  Object operator[](String name) {
+    if (varName == name) return _convert(value);
+    if (parent != null) return parent[name];
+    throw new EvalException("variable '$name' not found");
   }
-}
 
-Object _convert(v) {
-  if (v is Stream) return new StreamBinding(v);
-  return v;
+  Object _ownerOf(String name) {
+    if (varName == name) return this;
+    if (parent != null) return parent._ownerOf(name);
+    return null;
+  }
 }
 
+Object _convert(v) => v is Stream ? new StreamBinding(v) : v;
+
 abstract class ExpressionObserver<E extends Expression> implements Expression {
   final E _expr;
   ExpressionObserver _parent;
@@ -456,12 +484,11 @@ class IdentifierObserver extends ExpressionObserver<Identifier>
   _updateSelf(Scope scope) {
     _value = scope[value];
 
-    var owner = scope.ownerOf(value);
+    var owner = scope._ownerOf(value);
     if (owner is Observable) {
       var symbol = smoke.nameToSymbol(value);
       _subscription = (owner as Observable).changes.listen((changes) {
-        if (changes.any(
-            (c) => c is PropertyChangeRecord && c.name == symbol)) {
+        if (changes.any((c) => c is PropertyChangeRecord && c.name == symbol)) {
           _invalidate(scope);
         }
       });