feat(AstParser): Made the AST parser private to the scope

relates to dart-archive#648

Author: vicb

lib/core/interpolate.dart

@@ -1,34 +1,12 @@
 part of angular.core;
 
-class Interpolation implements Function {
-  final String template;
-  final List<String> separators;
-  final List<String> expressions;
-  Function setter = (_) => _;
-
-  Interpolation(this.template, this.separators, this.expressions);
-
-  String call(List parts, [_]) {
-    if (parts == null) return separators.join('');
-    var sb = new StringBuffer();
-    for (var i = 0; i < parts.length; i++) {
-      sb.write(separators[i]);
-      var value = parts[i];
-      sb.write(value == null ? '' : '$value');
-    }
-    sb.write(separators.last);
-    return setter(sb.toString());
-  }
-}
-
 /**
- * Compiles a string with markup into an interpolation function. This service
- * is used by the HTML [Compiler] service for data binding.
- *
+ * Compiles a string with markup into an expression. This service is used by the
+ * HTML [Compiler] service for data binding.
  *
  *     var $interpolate = ...; // injected
  *     var exp = $interpolate('Hello {{name}}!');
- *     expect(exp({name:'Angular'}).toEqual('Hello Angular!');
+ *     expect(exp).toEqual('"Hello "+(name)+"!"');
  */
 @NgInjectableService()
 class Interpolate implements Function {
@@ -37,49 +15,54 @@ class Interpolate implements Function {
   Interpolate(this._parse);
 
   /**
-   * Compiles markup text into interpolation function.
+   * Compiles markup text into expression.
    *
-   * - `template`: The markup text to interpolate in form `foo {{expr}} bar`.
-   * - `mustHaveExpression`: if set to true then the interpolation string must
-   *      have embedded expression in order to return an interpolation function.
-   *      Strings with no embedded expression will return null for the
-   *      interpolation function.
-   * - `startSymbol`: The symbol to start interpolation. '{{' by default.
-   * - `endSymbol`: The symbol to end interpolation. '}}' by default.
+   * - [template]: The markup text to interpolate in form `foo {{expr}} bar`.
+   * - [mustHaveExpression]: if set to true then the interpolation string must
+   *   have embedded expression in order to return an expression. Strings with
+   *   no embedded expression will return null.
+   * - [startSymbol]: The symbol to start interpolation. '{{' by default.
+   * - [endSymbol]: The symbol to end interpolation. '}}' by default.
    */
-  Interpolation call(String template, [bool mustHaveExpression = false,
-      String startSymbol = '{{', String endSymbol = '}}']) {
-    int startSymbolLength = startSymbol.length;
-    int endSymbolLength = endSymbol.length;
-    int startIndex;
-    int endIndex;
+
+  String call(String template, [bool mustHaveExpression = false,
+  String startSymbol = '{{', String endSymbol = '}}']) {
+    if (template == null || template.isEmpty) return "";
+
+    final startLen = startSymbol.length;
+    final endLen = endSymbol.length;
+    final length = template.length;
+
+    int startIdx;
+    int endIdx;
     int index = 0;
-    int length = template.length;
+
     bool hasInterpolation = false;
-    bool shouldAddSeparator = true;
+
     String exp;
-    final separators = <String>[];
-    final expressions = <String>[];
+    final expParts = <String>[];
 
     while (index < length) {
-      if (((startIndex = template.indexOf(startSymbol, index)) != -1) &&
-          ((endIndex = template.indexOf(endSymbol, startIndex + startSymbolLength)) != -1) ) {
-        separators.add(template.substring(index, startIndex));
-        exp = template.substring(startIndex + startSymbolLength, endIndex);
-        expressions.add(exp);
-        index = endIndex + endSymbolLength;
+      startIdx = template.indexOf(startSymbol, index);
+      endIdx = template.indexOf(endSymbol, startIdx + startLen);
+      if (startIdx != -1 && endIdx != -1) {
+        if (index < startIdx) {
+          // Empty strings could be stripped thanks to the stringify
+          // filter
+          expParts.add('"${template.substring(index, startIdx)}"');
+        }
+        expParts.add('(' + template.substring(startIdx + startLen, endIdx) +
+        '|stringify)');
+
+        index = endIdx + endLen;
         hasInterpolation = true;
       } else {
-        // we did not find anything, so we have to add the remainder to the
-        // chunks array
-        separators.add(template.substring(index));
-        shouldAddSeparator = false;
+        // we did not find any interpolation, so add the remainder
+        expParts.add('"${template.substring(index)}"');
         break;
       }
     }
-    if (shouldAddSeparator) separators.add('');
-    return (!mustHaveExpression || hasInterpolation)
-        ? new Interpolation(template, separators, expressions)
-        : null;
+
+    return !mustHaveExpression || hasInterpolation ? expParts.join('+') : null;
   }
-}
+}

lib/core/scope.dart

@@ -199,32 +199,27 @@ class Scope {
    * On the opposite, [readOnly] should be set to [:false:] if the [reactionFn]
    * could change the model so that the watch is observed in the [digest] cycle.
    */
-  Watch watch(expression, ReactionFn reactionFn,
-              {context, FilterMap filters, bool readOnly: false}) {
+  Watch watch(String expression, ReactionFn reactionFn, {context,
+      FilterMap filters, bool readOnly: false, bool collection: false}) {
     assert(isAttached);
-    assert(expression != null);
-    AST ast;
+    assert(expression is String);
     Watch watch;
     ReactionFn fn = reactionFn;
-    if (expression is AST) {
-      ast = expression;
-    } else if (expression is String) {
-      if (expression.startsWith('::')) {
-        expression = expression.substring(2);
-        fn = (value, last) {
-          if (value != null) {
-            watch.remove();
-            return reactionFn(value, last);
-          }
-        };
-      } else if (expression.startsWith(':')) {
-        expression = expression.substring(1);
-        fn = (value, last) => value == null ? null : reactionFn(value, last);
-      }
-      ast = rootScope._astParser(expression, context: context, filters: filters);
-    } else {
-      throw 'expressions must be String or AST got $expression.';
+    if (expression.startsWith('::')) {
+      expression = expression.substring(2);
+      fn = (value, last) {
+        if (value != null) {
+          watch.remove();
+          return reactionFn(value, last);
+        }
+      };
+    } else if (expression.startsWith(':')) {
+      expression = expression.substring(1);
+      fn = (value, last) => value == null ? null : reactionFn(value, last);
     }
+
+    AST ast = rootScope._astParser(expression, context: context,
+        filters: filters, collection: collection);
     WatchGroup group = readOnly ? _readOnlyGroup : _readWriteGroup;
     return watch = group.watch(ast, fn);
   }
@@ -256,10 +251,9 @@ class Scope {
     } catch (e, s) {
       rootScope._exceptionHandler(e, s);
     } finally {
-      rootScope
-          .._transitionState(RootScope.STATE_APPLY, null)
-          ..digest()
-          ..flush();
+      rootScope.._transitionState(RootScope.STATE_APPLY, null)
+               ..digest()
+               ..flush();
     }
   }
 
@@ -436,11 +430,12 @@ class RootScope extends Scope {
 
   String _state;
 
-  RootScope(Object context, this._astParser, this._parser,
-            GetterCache cacheGetter, FilterMap filterMap,
-            this._exceptionHandler, this._ttl, this._zone,
+  RootScope(Object context, Parser parser, GetterCache cacheGetter,
+            FilterMap filterMap, this._exceptionHandler, this._ttl, this._zone,
             ScopeStats _scopeStats)
       : _scopeStats = _scopeStats,
+        _parser = parser,
+        _astParser = new AstParser(parser),
         super(context, null, null,
             new RootWatchGroup(new DirtyCheckingChangeDetector(cacheGetter), context),
             new RootWatchGroup(new DirtyCheckingChangeDetector(cacheGetter), context),
@@ -963,6 +958,9 @@ class ExpressionVisitor implements Visitor {
   }
 
   void visitFilter(Filter exp) {
+    if (filters == null) {
+      throw new Exception("No filters have been registered");
+    }
     Function filterFunction = filters(exp.name);
     List<AST> args = [visitCollection(exp.expression)];
     args.addAll(_toAst(exp.arguments).map((ast) => new CollectionAST(ast)));

lib/core_dom/element_binder.dart

@@ -136,7 +136,7 @@ class ElementBinder {
           nodeModule.factory(NgTextMustacheDirective, (Injector injector) {
             return new NgTextMustacheDirective(
                 node, ref.value, injector.get(Interpolate), injector.get(Scope),
-                injector.get(AstParser), injector.get(FilterMap));
+                injector.get(FilterMap));
           });
         } else if (ref.type == NgAttrMustacheDirective) {
           if (nodesAttrsDirectives == null) {
@@ -146,7 +146,7 @@ class ElementBinder {
               var interpolate = injector.get(Interpolate);
               for (var ref in nodesAttrsDirectives) {
                 new NgAttrMustacheDirective(nodeAttrs, ref.value, interpolate,
-                scope, injector.get(AstParser), injector.get(FilterMap));
+                    scope, injector.get(FilterMap));
               }
             });
           }

lib/core_dom/ng_mustache.dart

@@ -3,20 +3,20 @@ part of angular.core.dom;
 // This Directive is special and does not go through injection.
 @NgDirective(selector: r':contains(/{{.*}}/)')
 class NgTextMustacheDirective {
-  NgTextMustacheDirective(dom.Node element,
-                          String markup,
+  final dom.Node _element;
+
+  NgTextMustacheDirective(this._element,
+                          String template,
                           Interpolate interpolate,
                           Scope scope,
-                          AstParser parser,
                           FilterMap filters) {
-    Interpolation interpolation = interpolate(markup);
-    interpolation.setter = (text) => element.text = text;
+    String expression = interpolate(template);
+
+    scope.watch(expression, _updateMarkup, readOnly: true, filters: filters);
+  }
 
-    List items = interpolation.expressions
-        .map((exp) => parser(exp, filters: filters))
-        .toList();
-    AST ast = new PureFunctionAST('[[$markup]]', new ArrayFn(), items);
-    scope.watch(ast, interpolation.call, readOnly: true);
+  void _updateMarkup(text, previousText) {
+    _element.text = text;
   }
 }
 
@@ -25,40 +25,35 @@ class NgTextMustacheDirective {
 class NgAttrMustacheDirective {
   bool _hasObservers;
   Watch _watch;
+  NodeAttrs _attrs;
+  String _attrName;
 
   // This Directive is special and does not go through injection.
-  NgAttrMustacheDirective(NodeAttrs attrs,
-                          String markup,
+  NgAttrMustacheDirective(this._attrs,
+                          String template,
                           Interpolate interpolate,
                           Scope scope,
-                          AstParser parser,
                           FilterMap filters) {
-
-    var eqPos = markup.indexOf('=');
-    var attrName = markup.substring(0, eqPos);
-    var attrValue = markup.substring(eqPos + 1);
-    var lastValue = markup;
-    Interpolation interpolation = interpolate(attrValue)..setter = (text) {
-      if (lastValue != text) lastValue = attrs[attrName] = text;
-    };
-
-    // TODO(misko): figure out how to remove call to setter. It slows down
-    // View instantiation
-    interpolation.setter('');
-
-    List items = interpolation.expressions
-        .map((exp) => parser(exp, filters: filters))
-        .toList();
-
-    AST ast = new PureFunctionAST('[[$markup]]', new ArrayFn(), items);
-
-    attrs.listenObserverChanges(attrName, (hasObservers) {
-      if (_hasObservers != hasObservers) {
-        _hasObservers = hasObservers;
-        if (_watch != null) _watch.remove();
-        _watch = scope.watch(ast, interpolation.call, readOnly: !hasObservers);
+    var eqPos = template.indexOf('=');
+    _attrName = template.substring(0, eqPos);
+    String expression = interpolate(template.substring(eqPos + 1));
+
+    _updateMarkup('', template);
+
+    _attrs.listenObserverChanges(_attrName, (hasObservers) {
+    if (_hasObservers != hasObservers) {
+      _hasObservers = hasObservers;
+      if (_watch != null) _watch.remove();
+        _watch = scope.watch(expression, _updateMarkup, filters: filters,
+            readOnly: !_hasObservers);
       }
     });
   }
+
+  void _updateMarkup(text, previousText) {
+    if (text != previousText && !(previousText == null && text == '')) {
+        _attrs[_attrName] = text;
+    }
+  }
 }
 

lib/directive/ng_class.dart

@@ -68,8 +68,8 @@ part of angular.directive;
     exportExpressionAttrs: const ['ng-class'])
 class NgClassDirective extends _NgClassBase {
   NgClassDirective(dom.Element element, Scope scope, NodeAttrs attrs,
-                   AstParser parser, NgAnimate animate)
-      : super(element, scope, null, attrs, parser, animate);
+                   NgAnimate animate)
+      : super(element, scope, null, attrs, animate);
 }
 
 /**
@@ -104,8 +104,8 @@ class NgClassDirective extends _NgClassBase {
     exportExpressionAttrs: const ['ng-class-odd'])
 class NgClassOddDirective extends _NgClassBase {
   NgClassOddDirective(dom.Element element, Scope scope, NodeAttrs attrs,
-                      AstParser parser, NgAnimate animate)
-      : super(element, scope, 0, attrs, parser, animate);
+                      NgAnimate animate)
+      : super(element, scope, 0, attrs, animate);
 }
 
 /**
@@ -140,23 +140,22 @@ class NgClassOddDirective extends _NgClassBase {
     exportExpressionAttrs: const ['ng-class-even'])
 class NgClassEvenDirective extends _NgClassBase {
   NgClassEvenDirective(dom.Element element, Scope scope, NodeAttrs attrs,
-                       AstParser parser, NgAnimate animate)
-      : super(element, scope, 1, attrs, parser, animate);
+                       NgAnimate animate)
+      : super(element, scope, 1, attrs, animate);
 }
 
 abstract class _NgClassBase {
   final dom.Element element;
   final Scope scope;
   final int mode;
   final NodeAttrs nodeAttrs;
-  final AstParser _parser;
   final NgAnimate _animate;
   var previousSet = [];
   var currentSet = [];
   Watch _watch;
 
   _NgClassBase(this.element, this.scope, this.mode, this.nodeAttrs,
-               this._parser, this._animate)
+               this._animate)
   {
     var prevClass;
 
@@ -168,14 +167,17 @@ abstract class _NgClassBase {
     });
   }
 
-  set valueExpression(currentExpression) {
+  set valueExpression(expression) {
     if (_watch != null) _watch.remove();
-    _watch = scope.watch(_parser(currentExpression, collection: true), (current, _) {
+    _watch = scope.watch(expression, (current, _) {
       currentSet = _flatten(current);
       _handleChange(scope.context[r'$index']);
-    }, readOnly: true);
+    },
+    readOnly: true,
+    collection: true);
+
     if (mode != null) {
-      scope.watch(_parser(r'$index'), (index, oldIndex) {
+      scope.watch(r'$index', (index, oldIndex) {
         var mod = index % 2;
         if (oldIndex == null || mod != oldIndex % 2) {
           if (mod == mode) {