chore(ASTParser): Clean up and refactor the ASTParser

-> Make it injectable
-> Remove try/catch statements are extra state
-> Rename AstParser -> ASTParser
-> Rename ExpressionVisitor -> _ExpressionVisitor

For #1086

Author: jbdeboer

lib/change_detection/ast_parser.dart

@@ -5,6 +5,7 @@ import 'dart:collection';
 import 'package:angular/core/parser/syntax.dart' as syntax;
 import 'package:angular/core/parser/parser.dart';
 import 'package:angular/core/formatter.dart';
+import 'package:angular/core/annotation_src.dart';
 import 'package:angular/change_detection/watch_group.dart';
 import 'package:angular/change_detection/change_detection.dart';
 import 'package:angular/core/parser/utils.dart';
@@ -18,48 +19,29 @@ class _FunctionChain {
   }
 }
 
-class AstParser {
+@Injectable()
+class ASTParser {
   final Parser _parser;
-  int _id = 0;
-  final ExpressionVisitor _visitor;
+  final ClosureMap _closureMap;
 
-  AstParser(this._parser, ClosureMap closureMap)
-      : _visitor = new ExpressionVisitor(closureMap);
+  ASTParser(this._parser, this._closureMap);
 
   AST call(String input, {FormatterMap formatters,
                           bool collection: false }) {
-    _visitor.formatters = formatters;
-    AST contextRef = _visitor.contextRef;
-    try {
-      var exp = _parser(input);
-      return collection ? _visitor.visitCollection(exp) : _visitor.visit(exp);
-    } finally {
-      _visitor.contextRef = contextRef;
-      _visitor.formatters = null;
-    }
+    var visitor = new _ExpressionVisitor(_closureMap, formatters);
+    var exp = _parser(input);
+    return collection ? visitor.visitCollection(exp) : visitor.visit(exp);
   }
 }
 
-class ExpressionVisitor implements syntax.Visitor {
-  static final ContextReferenceAST scopeContextRef = new ContextReferenceAST();
+class _ExpressionVisitor implements syntax.Visitor {
+  static final ContextReferenceAST contextRef = new ContextReferenceAST();
   final ClosureMap _closureMap;
-  AST contextRef = scopeContextRef;
-
+  final FormatterMap _formatters;
 
-  ExpressionVisitor(this._closureMap);
+  _ExpressionVisitor(this._closureMap, this._formatters);
 
-  AST ast;
-  FormatterMap formatters;
-
-  AST visit(syntax.Expression exp) {
-    exp.accept(this);
-    assert(ast != null);
-    try {
-      return ast;
-    } finally {
-      ast = null;
-    }
-  }
+  AST visit(syntax.Expression exp) => exp.accept(this);
 
   AST visitCollection(syntax.Expression exp) => new CollectionAST(visit(exp));
   AST _mapToAst(syntax.Expression expression) => visit(expression);
@@ -76,71 +58,71 @@ class ExpressionVisitor implements syntax.Visitor {
     return result;
   }
 
-  void visitCallScope(syntax.CallScope exp) {
+  AST visitCallScope(syntax.CallScope exp) {
     List<AST> positionals = _toAst(exp.arguments.positionals);
     Map<Symbol, AST> named = _toAstMap(exp.arguments.named);
-    ast = new MethodAST(contextRef, exp.name, positionals, named);
+    return new MethodAST(contextRef, exp.name, positionals, named);
   }
-  void visitCallMember(syntax.CallMember exp) {
+  AST visitCallMember(syntax.CallMember exp) {
     List<AST> positionals = _toAst(exp.arguments.positionals);
     Map<Symbol, AST> named = _toAstMap(exp.arguments.named);
-    ast = new MethodAST(visit(exp.object), exp.name, positionals, named);
-  }
-  void visitAccessScope(syntax.AccessScope exp) {
-    ast = new FieldReadAST(contextRef, exp.name);
-  }
-  void visitAccessMember(syntax.AccessMember exp) {
-    ast = new FieldReadAST(visit(exp.object), exp.name);
-  }
-  void visitBinary(syntax.Binary exp) {
-    ast = new PureFunctionAST(exp.operation,
-                              _operationToFunction(exp.operation),
-                              [visit(exp.left), visit(exp.right)]);
-  }
-  void visitPrefix(syntax.Prefix exp) {
-    ast = new PureFunctionAST(exp.operation,
-                              _operationToFunction(exp.operation),
-                              [visit(exp.expression)]);
-  }
-  void visitConditional(syntax.Conditional exp) {
-    ast = new PureFunctionAST('?:', _operation_ternary,
-                              [visit(exp.condition), visit(exp.yes),
-                              visit(exp.no)]);
+    return new MethodAST(visit(exp.object), exp.name, positionals, named);
   }
-  void visitAccessKeyed(syntax.AccessKeyed exp) {
-    ast = new ClosureAST('[]', _operation_bracket,
-                             [visit(exp.object), visit(exp.key)]);
-  }
-  void visitLiteralPrimitive(syntax.LiteralPrimitive exp) {
-    ast = new ConstantAST(exp.value);
-  }
-  void visitLiteralString(syntax.LiteralString exp) {
-    ast = new ConstantAST(exp.value);
-  }
-  void visitLiteralArray(syntax.LiteralArray exp) {
+  AST visitAccessScope(syntax.AccessScope exp) =>
+      new FieldReadAST(contextRef, exp.name);
+
+  AST visitAccessMember(syntax.AccessMember exp) =>
+      new FieldReadAST(visit(exp.object), exp.name);
+
+  AST visitBinary(syntax.Binary exp) =>
+      new PureFunctionAST(exp.operation,
+                          _operationToFunction(exp.operation),
+                          [visit(exp.left), visit(exp.right)]);
+
+  AST visitPrefix(syntax.Prefix exp) =>
+      new PureFunctionAST(exp.operation,
+                          _operationToFunction(exp.operation),
+                          [visit(exp.expression)]);
+
+  AST visitConditional(syntax.Conditional exp) =>
+      new PureFunctionAST('?:', _operation_ternary,
+                          [visit(exp.condition), visit(exp.yes),
+                          visit(exp.no)]);
+
+  AST visitAccessKeyed(syntax.AccessKeyed exp) =>
+      new ClosureAST('[]', _operation_bracket,
+                     [visit(exp.object), visit(exp.key)]);
+
+  AST visitLiteralPrimitive(syntax.LiteralPrimitive exp) =>
+      new ConstantAST(exp.value);
+
+  AST visitLiteralString(syntax.LiteralString exp) =>
+      new ConstantAST(exp.value);
+
+  AST visitLiteralArray(syntax.LiteralArray exp) {
     List<AST> items = _toAst(exp.elements);
-    ast = new PureFunctionAST('[${items.join(', ')}]', new ArrayFn(), items);
+    return new PureFunctionAST('[${items.join(', ')}]', new ArrayFn(), items);
   }
 
-  void visitLiteralObject(syntax.LiteralObject exp) {
+  AST visitLiteralObject(syntax.LiteralObject exp) {
     List<String> keys = exp.keys;
     List<AST> values = _toAst(exp.values);
     assert(keys.length == values.length);
     var kv = <String>[];
     for (var i = 0; i < keys.length; i++) {
       kv.add('${keys[i]}: ${values[i]}');
     }
-    ast = new PureFunctionAST('{${kv.join(', ')}}', new MapFn(keys), values);
+    return new PureFunctionAST('{${kv.join(', ')}}', new MapFn(keys), values);
   }
 
-  void visitFormatter(syntax.Formatter exp) {
-    if (formatters == null) {
+  AST visitFormatter(syntax.Formatter exp) {
+    if (_formatters == null) {
       throw new Exception("No formatters have been registered");
     }
-    Function formatterFunction = formatters(exp.name);
+    Function formatterFunction = _formatters(exp.name);
     List<AST> args = [visitCollection(exp.expression)];
     args.addAll(_toAst(exp.arguments).map((ast) => new CollectionAST(ast)));
-    ast = new PureFunctionAST('|${exp.name}',
+    return new PureFunctionAST('|${exp.name}',
         new _FormatterWrapper(formatterFunction, args.length), args);
   }
 

lib/core/module_internal.dart

@@ -54,5 +54,6 @@ class CoreModule extends Module {
     bind(DynamicParser);
     bind(DynamicParserBackend);
     bind(Lexer);
+    bind(ASTParser);
   }
 }

lib/core/scope.dart

@@ -555,7 +555,7 @@ class RootScope extends Scope {
   static final STATE_FLUSH_ASSERT = 'assert';
 
   final ExceptionHandler _exceptionHandler;
-  final AstParser _astParser;
+  final ASTParser _astParser;
   final Parser _parser;
   final ScopeDigestTTL _ttl;
   final VmTurnZone _zone;
@@ -617,12 +617,12 @@ class RootScope extends Scope {
    */
   String get state => _state;
 
-  RootScope(Object context, Parser parser, FieldGetterFactory fieldGetterFactory,
+  RootScope(Object context, Parser parser, ASTParser astParser, FieldGetterFactory fieldGetterFactory,
             FormatterMap formatters, this._exceptionHandler, this._ttl, this._zone,
-            ScopeStats _scopeStats, ClosureMap closureMap)
+            ScopeStats _scopeStats)
       : _scopeStats = _scopeStats,
         _parser = parser,
-        _astParser = new AstParser(parser, closureMap),
+        _astParser = astParser,
         super(context, null, null,
             new RootWatchGroup(fieldGetterFactory,
                 new DirtyCheckingChangeDetector(fieldGetterFactory), context),

test/change_detection/watch_group_spec.dart

@@ -21,19 +21,19 @@ void main() {
     DirtyCheckingChangeDetector changeDetector;
     Logger logger;
     Parser parser;
-    ExpressionVisitor visitor;
+    ASTParser astParser;
 
-    beforeEach(inject((Logger _logger, Parser _parser) {
+    beforeEach(inject((Logger _logger, Parser _parser, ASTParser _astParser) {
       context = {};
       var getterFactory = new DynamicFieldGetterFactory();
       changeDetector = new DirtyCheckingChangeDetector(getterFactory);
       watchGrp = new RootWatchGroup(getterFactory, changeDetector, context);
-      visitor = new ExpressionVisitor(new DynamicClosureMap());
       logger = _logger;
       parser = _parser;
+      astParser = _astParser;
     }));
 
-    AST parse(String expression) => visitor.visit(parser(expression));
+    AST parse(String expression) => astParser(expression);
 
     eval(String expression, [evalContext]) {
       AST ast = parse(expression);