fix list filtering if the filter is a method

pkg/polymer_expressions/lib/eval.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.
 
 library polymer_expressions.eval;
 
 import 'dart:async';
 import 'dart:collection';
 import 'dart:mirrors';
 
@@ skipping 130 matching lines @@
 }
 
 /**
  * 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.
  */
-class Scope extends Object {
+class Scope {

[Jennifer Messerly, 2013/10/04 02:18:19]

trivial style fix: remove "extends Object" which is implied.

   final Scope parent;
   final Object model;
   // TODO(justinfagnani): disallow adding/removing names
   final ObservableMap<String, Object> _variables;
   InstanceMirror __modelMirror;
 
   Scope({this.model, Map<String, Object> variables: const {}, this.parent})
       : _variables = new ObservableMap.from(variables);
 
   InstanceMirror get _modelMirror {
     if (__modelMirror != null) return __modelMirror;
     __modelMirror = reflect(model);
     return __modelMirror;
   }
 
   Object operator[](String name) {
     if (name == 'this') {
       return model;
     } else if (_variables.containsKey(name)) {
       return _convert(_variables[name]);
     } else if (model != null) {
       var symbol = new Symbol(name);
       var classMirror = _modelMirror.type;
       var memberMirror = getMemberMirror(classMirror, symbol);

[justinfagnani, 2013/10/04 20:46:08]

We should be able to reduce the follow if/else to:

if (memberMirror != null) {
  return _convert(_modelMirror.getField(symbol).reflectee);
}

       if (memberMirror is VariableMirror ||
           (memberMirror is MethodMirror && memberMirror.isGetter)) {
         return _convert(_modelMirror.getField(symbol).reflectee);
       } else if (memberMirror is MethodMirror) {
         return new Method(_modelMirror, symbol);
       }
     }
     if (parent != null) {
       return _convert(parent[name]);
     } else {
@@ skipping 407 matching lines @@
 
     // TODO: make Comprehension observable and update it
     _value = new Comprehension(identifier.value, iterable);
   }
 
   accept(Visitor v) => v.visitInExpression(this);
 }
 
 _toBool(v) => (v == null) ? false : v;
 
-call(dynamic receiver, List args) {
+/** Call a [Function] or a [Method]. */
+Object call(Object receiver, List args) {

[Jennifer Messerly, 2013/10/04 02:18:19]

trivial style fix: avoid "dynamic" except as type parameter.
factor out the _convert

[justinfagnani, 2013/10/04 20:46:08]

if InstanceMirror.getField() properly closurizes, we can replace call() with
Function.apply()

+  var result;
   if (receiver is Method) {
-    return
-        _convert(receiver.mirror.invoke(receiver.symbol, args, null).reflectee);
+    result = receiver.mirror.invoke(receiver.symbol, args, null).reflectee;
   } else {
-    return _convert(Function.apply(receiver, args, null));
+    result = Function.apply(receiver, args, null);
   }
+  return _convert(result);
 }
 
 /**
  * A comprehension declaration ("a in b"). [identifier] is the loop variable
  * that's added to the scope during iteration. [iterable] is the set of
  * objects to iterate over.
  */
 class Comprehension {
   final String identifier;
   final Iterable iterable;
 
   Comprehension(this.identifier, Iterable iterable)
       : iterable = (iterable != null) ? iterable : const [];
 }
 
-/**
- * A method on a model object in a [Scope].
- */
-class Method { //implements _FunctionWrapper {
+/** A method on a model object in a [Scope]. */
+class Method {
   final InstanceMirror mirror;
   final Symbol symbol;
 
   Method(this.mirror, this.symbol);
 
-  dynamic call(List args) => mirror.invoke(symbol, args, null).reflectee;
+  /**
+   * Support for calling single argument methods like [Filter]s.
+   * This does not work for calls that need to pass more than one argument.
+   */
+  call(arg0) => mirror.invoke(symbol, [arg0], null).reflectee;

[Jennifer Messerly, 2013/10/04 02:18:19]

the actual bug fix. "call" is different from Function.apply; it's expected to
match the method signature.

[justinfagnani, 2013/10/04 20:41:47]

I think this fix could be different, because InstanceMirror.getField() is
supposed to correctly closurize methods now.

Background on Method: I think that this call() originally wasn't intended to be
the Function-emulating call(), but was used to invoke the method from within
polymer expression code consistently where there used to be a FunctionWrapper
class. Unfortunate naming, that's why the signature took a list.

Right now the invoke ast node takes care of invoking most methods, and only
those that are method directly on the model object get wrapped in Method. Other
methods reached via expressions won't be wrapped, but should be closurized by
mirrors. I think we might just want to remove call() altogether.

[Jennifer Messerly, 2013/10/04 20:44:28]

Alright, I'll take a shot at removing "Method" instead. Fingers crossed it
actually works in all impls now.

 }
 
 class EvalException implements Exception {
   final String message;
   EvalException(this.message);
   String toString() => "EvalException: $message";
 }