From 76eef19a54102bf4cdc83900f112c10c36aa0728 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Mon, 21 Jul 2014 13:44:04 -0700 Subject: [PATCH] reflactor(parser): rename DynamicParser into Parser Since there is only one implementation of the Parser interface, which is used for both dynamic and static deployments, calling it DynamicParser is misleading. - Remove the Parser interface - Rename rename DynamicParser into Parser - Rename DynamicParserBackend into RuntimeParserBackend --- bin/parser_generator_for_spec.dart | 2 +- lib/application_factory.dart | 2 +- lib/application_factory_static.dart | 1 - lib/core/module.dart | 5 +- lib/core/module_internal.dart | 7 +- lib/core/parser/dynamic_parser.dart | 143 ----------------- ...parser_impl.dart => parse_expression.dart} | 9 +- lib/core/parser/parser.dart | 144 +++++++++++++++++- lib/core/parser/static_closure_map.dart | 1 - lib/tools/expression_extractor.dart | 5 +- .../transformer/expression_generator.dart | 2 +- test/angular_spec.dart | 4 +- test/cache/js_cache_register_spec.dart | 12 +- .../parser/generated_getter_setter_spec.dart | 2 +- test/core/parser/parser_spec.dart | 17 --- 15 files changed, 162 insertions(+), 194 deletions(-) delete mode 100644 lib/core/parser/dynamic_parser.dart rename lib/core/parser/{dynamic_parser_impl.dart => parse_expression.dart} (97%) diff --git a/bin/parser_generator_for_spec.dart b/bin/parser_generator_for_spec.dart index 9e8da6896..524ea48b7 100644 --- a/bin/parser_generator_for_spec.dart +++ b/bin/parser_generator_for_spec.dart @@ -10,7 +10,7 @@ main(arguments) { Module module = new Module() ..bind(Lexer) ..bind(ParserGetterSetter) - ..bind(Parser, toImplementation: DynamicParser) + ..bind(Parser) ..install(new CacheModule()); module.bind(ParserBackend, toImplementation: DartGetterSetterGen); Injector injector = new ModuleInjector([module]); diff --git a/lib/application_factory.dart b/lib/application_factory.dart index 3d1100e9b..37d7b5761 100644 --- a/lib/application_factory.dart +++ b/lib/application_factory.dart @@ -40,7 +40,7 @@ import 'dart:html'; 'angular.routing', 'angular.core.annotation_src', 'angular.core.parser.Parser', - 'angular.core.parser.dynamic_parser', + 'angular.core.parser', 'angular.core.parser.lexer', 'angular.core_dynamic.DynamicMetadataExtractor', 'perf_api', diff --git a/lib/application_factory_static.dart b/lib/application_factory_static.dart index 6dcbe83d0..47e1dc915 100644 --- a/lib/application_factory_static.dart +++ b/lib/application_factory_static.dart @@ -26,7 +26,6 @@ library angular.app.factory.static; import 'package:angular/application.dart'; import 'package:angular/core/registry.dart'; -import 'package:angular/core/parser/dynamic_parser.dart'; import 'package:angular/core/parser/parser.dart'; import 'package:angular/core/parser/static_closure_map.dart'; import 'package:angular/core/registry_static.dart'; diff --git a/lib/core/module.dart b/lib/core/module.dart index eac0a5069..329696d28 100644 --- a/lib/core/module.dart +++ b/lib/core/module.dart @@ -13,10 +13,7 @@ export "package:angular/change_detection/watch_group.dart" show ReactionFn; export "package:angular/core/parser/parser.dart" show - Parser; - -export "package:angular/core/parser/dynamic_parser.dart" show - ClosureMap; + Parser, ClosureMap; export "package:angular/change_detection/change_detection.dart" show AvgStopwatch, diff --git a/lib/core/module_internal.dart b/lib/core/module_internal.dart index a024f1480..d8127af87 100644 --- a/lib/core/module_internal.dart +++ b/lib/core/module_internal.dart @@ -48,10 +48,9 @@ class CoreModule extends Module { bind(ScopeStatsConfig); bind(Object, toValue: {}); // RootScope context - bind(Parser, toInstanceOf: DynamicParser); - bind(ParserBackend, toInstanceOf: DynamicParserBackend); - bind(DynamicParser); - bind(DynamicParserBackend); + bind(Parser); + bind(RuntimeParserBackend); + bind(ParserBackend, toInstanceOf: RuntimeParserBackend); bind(Lexer); bind(ASTParser); } diff --git a/lib/core/parser/dynamic_parser.dart b/lib/core/parser/dynamic_parser.dart deleted file mode 100644 index a46254ee9..000000000 --- a/lib/core/parser/dynamic_parser.dart +++ /dev/null @@ -1,143 +0,0 @@ -library angular.core.parser.dynamic_parser; - -import 'package:di/annotations.dart'; -import 'package:angular/cache/module.dart'; -import 'package:angular/core/annotation_src.dart' hide Formatter; -import 'package:angular/core/formatter.dart' show FormatterMap; - -import 'package:angular/core/parser/parser.dart'; -import 'package:angular/core/parser/lexer.dart'; -import 'package:angular/core/parser/dynamic_parser_impl.dart'; -import 'package:angular/core/parser/syntax.dart' show defaultFormatterMap; - -import 'package:angular/core/parser/eval.dart'; -import 'package:angular/core/parser/utils.dart' show EvalError; -import 'package:angular/utils.dart'; - -abstract class ClosureMap { - Getter lookupGetter(String name); - Setter lookupSetter(String name); - Symbol lookupSymbol(String name); - MethodClosure lookupFunction(String name, CallArguments arguments); -} - -@Injectable() -class DynamicParser implements Parser { - final Lexer _lexer; - final ParserBackend _backend; - final Map _cache = {}; - DynamicParser(this._lexer, this._backend, CacheRegister cacheRegister) { - cacheRegister.registerCache("DynamicParser", _cache); - } - - Expression call(String input) { - if (input == null) input = ''; - return _cache.putIfAbsent(input, () => _parse(input)); - } - - Expression _parse(String input) { - DynamicParserImpl parser = new DynamicParserImpl(_lexer, _backend, input); - Expression expression = parser.parseChain(); - return new DynamicExpression(expression); - } -} - -class DynamicExpression extends Expression { - final Expression _expression; - DynamicExpression(this._expression); - - bool get isAssignable => _expression.isAssignable; - bool get isChain => _expression.isChain; - - accept(Visitor visitor) => _expression.accept(visitor); - toString() => _expression.toString(); - - eval(scope, [FormatterMap formatters = defaultFormatterMap]) { - try { - return _expression.eval(scope, formatters); - } on EvalError catch (e, s) { - throw e.unwrap("$this", s); - } - } - - assign(scope, value) { - try { - return _expression.assign(scope, value); - } on EvalError catch (e, s) { - throw e.unwrap("$this", s); - } - } -} - -@Injectable() -class DynamicParserBackend extends ParserBackend { - final ClosureMap _closures; - DynamicParserBackend(this._closures); - - bool isAssignable(Expression expression) => expression.isAssignable; - - Expression newFormatter(expression, name, arguments) { - List allArguments = new List(arguments.length + 1); - allArguments[0] = expression; - allArguments.setAll(1, arguments); - return new Formatter(expression, name, arguments, allArguments); - } - - Expression newChain(expressions) => new Chain(expressions); - Expression newAssign(target, value) => new Assign(target, value); - Expression newConditional(condition, yes, no) => - new Conditional(condition, yes, no); - - Expression newAccessKeyed(object, key) => new AccessKeyed(object, key); - Expression newCallFunction(function, arguments) => - new CallFunction(function, _closures, arguments); - - Expression newPrefixNot(expression) => new PrefixNot(expression); - - Expression newBinary(operation, left, right) => - new Binary(operation, left, right); - - Expression newLiteralPrimitive(value) => new LiteralPrimitive(value); - Expression newLiteralArray(elements) => new LiteralArray(elements); - Expression newLiteralObject(keys, values) => new LiteralObject(keys, values); - Expression newLiteralString(value) => new LiteralString(value); - - Expression newAccessScope(name) { - Getter getter; - Setter setter; - if (name == 'this') { - getter = (o) => o; - } else { - _assertNotReserved(name); - getter = _closures.lookupGetter(name); - setter = _closures.lookupSetter(name); - } - return new AccessScopeFast(name, getter, setter); - } - - Expression newAccessMember(object, name) { - _assertNotReserved(name); - Getter getter = _closures.lookupGetter(name); - Setter setter = _closures.lookupSetter(name); - return new AccessMemberFast(object, name, getter, setter); - } - - Expression newCallScope(name, arguments) { - _assertNotReserved(name); - MethodClosure function = _closures.lookupFunction(name, arguments); - return new CallScope(name, function, arguments); - } - - Expression newCallMember(object, name, arguments) { - _assertNotReserved(name); - MethodClosure function = _closures.lookupFunction(name, arguments); - return new CallMember(object, function, name, arguments); - } - - _assertNotReserved(name) { - if (isReservedWord(name)) { - throw "Identifier '$name' is a reserved word."; - } - } -} - diff --git a/lib/core/parser/dynamic_parser_impl.dart b/lib/core/parser/parse_expression.dart similarity index 97% rename from lib/core/parser/dynamic_parser_impl.dart rename to lib/core/parser/parse_expression.dart index 9d8712413..4594b9af6 100644 --- a/lib/core/parser/dynamic_parser_impl.dart +++ b/lib/core/parser/parse_expression.dart @@ -1,4 +1,4 @@ -library angular.core.parser.dynamic_parser_impl; +library angular.core.parser.parse_expression; import 'package:angular/core/parser/parser.dart' show ParserBackend; import 'package:angular/core/parser/lexer.dart'; @@ -6,13 +6,16 @@ import 'package:angular/core/parser/syntax.dart'; import 'package:angular/core/parser/characters.dart'; import 'package:angular/utils.dart' show isReservedWord; -class DynamicParserImpl { +Expression parseExpression(Lexer lexer, ParserBackend backend, String input) => + new _ParseExpression(lexer, backend, input).parseChain(); + +class _ParseExpression { final ParserBackend backend; final String input; final List tokens; int index = 0; - DynamicParserImpl(Lexer lexer, this.backend, String input) + _ParseExpression(Lexer lexer, this.backend, String input) : this.input = input, tokens = lexer.call(input); Token get next => peek(0); diff --git a/lib/core/parser/parser.dart b/lib/core/parser/parser.dart index 0f9839cd0..3b32b3dbe 100644 --- a/lib/core/parser/parser.dart +++ b/lib/core/parser/parser.dart @@ -1,11 +1,19 @@ library angular.core.parser; +import 'package:di/annotations.dart'; import 'package:angular/core/parser/syntax.dart' - show CallArguments; + show defaultFormatterMap, Expression, Visitor, CallArguments; +import 'package:angular/core/parser/eval.dart'; +import 'package:angular/core/parser/utils.dart' show EvalError; +import 'package:angular/cache/module.dart'; +import 'package:angular/core/annotation_src.dart' hide Formatter; +import 'package:angular/core/module_internal.dart' show FormatterMap; +import 'package:angular/core/parser/lexer.dart'; +import 'package:angular/core/parser/parse_expression.dart'; +import 'package:angular/utils.dart'; + export 'package:angular/core/parser/syntax.dart' - show Visitor, Expression, BoundExpression, CallArguments; -export 'package:angular/core/parser/dynamic_parser.dart' - show DynamicParser, DynamicParserBackend, ClosureMap; + show Visitor, Expression, BoundExpression, CallArguments; typedef LocalsWrapper(context, locals); typedef Getter(self); @@ -14,9 +22,11 @@ typedef BoundGetter([locals]); typedef BoundSetter(value, [locals]); typedef MethodClosure(obj, List posArgs, Map namedArgs); -/// Placeholder for DI. The parser you are looking for is [DynamicParser]. -abstract class Parser { - T call(String input); +abstract class ClosureMap { + Getter lookupGetter(String name); + Setter lookupSetter(String name); + Symbol lookupSymbol(String name); + MethodClosure lookupFunction(String name, CallArguments arguments); } abstract class ParserBackend { @@ -67,3 +77,123 @@ abstract class ParserBackend { T newLiteralNumber(num value) => newLiteralPrimitive(value); T newLiteralString(String value) => null; } + +@Injectable() +class Parser { + final Lexer _lexer; + final ParserBackend _backend; + final Map _cache = {}; + Parser(this._lexer, this._backend, CacheRegister cacheRegister) { + cacheRegister.registerCache("Parser", _cache); + } + + Expression call(String input) { + if (input == null) input = ''; + return _cache.putIfAbsent(input, () => _parse(input)); + } + + Expression _parse(String input) { + Expression expression = parseExpression(_lexer, _backend, input); + return new _UnwrapExceptionDecorator(expression); + } +} + +class _UnwrapExceptionDecorator extends Expression { + final Expression _expression; + _UnwrapExceptionDecorator (this._expression); + + bool get isAssignable => _expression.isAssignable; + bool get isChain => _expression.isChain; + + accept(Visitor visitor) => _expression.accept(visitor); + toString() => _expression.toString(); + + eval(scope, [FormatterMap formatters = defaultFormatterMap]) { + try { + return _expression.eval(scope, formatters); + } on EvalError catch (e, s) { + throw e.unwrap("$this", s); + } + } + + assign(scope, value) { + try { + return _expression.assign(scope, value); + } on EvalError catch (e, s) { + throw e.unwrap("$this", s); + } + } +} + +@Injectable() +class RuntimeParserBackend extends ParserBackend { + final ClosureMap _closures; + RuntimeParserBackend(this._closures); + + bool isAssignable(Expression expression) => expression.isAssignable; + + Expression newFormatter(expression, name, arguments) { + List allArguments = new List(arguments.length + 1); + allArguments[0] = expression; + allArguments.setAll(1, arguments); + return new Formatter(expression, name, arguments, allArguments); + } + + Expression newChain(expressions) => new Chain(expressions); + Expression newAssign(target, value) => new Assign(target, value); + Expression newConditional(condition, yes, no) => + new Conditional(condition, yes, no); + + Expression newAccessKeyed(object, key) => new AccessKeyed(object, key); + Expression newCallFunction(function, arguments) => + new CallFunction(function, _closures, arguments); + + Expression newPrefixNot(expression) => new PrefixNot(expression); + + Expression newBinary(operation, left, right) => + new Binary(operation, left, right); + + Expression newLiteralPrimitive(value) => new LiteralPrimitive(value); + Expression newLiteralArray(elements) => new LiteralArray(elements); + Expression newLiteralObject(keys, values) => new LiteralObject(keys, values); + Expression newLiteralString(value) => new LiteralString(value); + + Expression newAccessScope(name) { + Getter getter; + Setter setter; + if (name == 'this') { + getter = (o) => o; + } else { + _assertNotReserved(name); + getter = _closures.lookupGetter(name); + setter = _closures.lookupSetter(name); + } + return new AccessScopeFast(name, getter, setter); + } + + Expression newAccessMember(object, name) { + _assertNotReserved(name); + Getter getter = _closures.lookupGetter(name); + Setter setter = _closures.lookupSetter(name); + return new AccessMemberFast(object, name, getter, setter); + } + + Expression newCallScope(name, arguments) { + _assertNotReserved(name); + MethodClosure function = _closures.lookupFunction(name, arguments); + return new CallScope(name, function, arguments); + } + + Expression newCallMember(object, name, arguments) { + _assertNotReserved(name); + MethodClosure function = _closures.lookupFunction(name, arguments); + return new CallMember(object, function, name, arguments); + } + + _assertNotReserved(name) { + if (isReservedWord(name)) { + throw "Identifier '$name' is a reserved word."; + } + } +} + diff --git a/lib/core/parser/static_closure_map.dart b/lib/core/parser/static_closure_map.dart index fd0d6c186..04d6ce486 100644 --- a/lib/core/parser/static_closure_map.dart +++ b/lib/core/parser/static_closure_map.dart @@ -2,7 +2,6 @@ library angular.core.static_closure_map; import 'package:angular/core/parser/parser.dart'; - class StaticClosureMap extends ClosureMap { final Map getters; final Map setters; diff --git a/lib/tools/expression_extractor.dart b/lib/tools/expression_extractor.dart index acc231aa5..e745873f4 100644 --- a/lib/tools/expression_extractor.dart +++ b/lib/tools/expression_extractor.dart @@ -55,11 +55,10 @@ main(args) { Module module = new Module() ..bind(ParserGetterSetter) ..bind(Lexer) - ..bind(DynamicParser) ..bind(DartGetterSetterGen) ..bind(CacheRegister) - ..bind(Parser, toInstanceOf: DynamicParser) - ..bind(ParserBackend, toInstanceOf: DartGetterSetterGen); + ..bind(Parser) + ..bind(ParserBackend, inject: [DartGetterSetterGen]); Injector injector = new ModuleInjector([module]); runZoned(() { diff --git a/lib/tools/transformer/expression_generator.dart b/lib/tools/transformer/expression_generator.dart index 4bd1fdb74..1133e6d7e 100644 --- a/lib/tools/transformer/expression_generator.dart +++ b/lib/tools/transformer/expression_generator.dart @@ -49,7 +49,7 @@ class ExpressionGenerator extends Transformer with ResolverTransformer { .then((_) { var module = new Module.withReflector(getReflector()) ..install(new CacheModule.withReflector(getReflector())) - ..bind(Parser, toImplementation: DynamicParser) + ..bind(Parser) ..bind(ParserBackend, toImplementation: DartGetterSetterGen) ..bind(Lexer) ..bind(_ParserGetterSetter); diff --git a/test/angular_spec.dart b/test/angular_spec.dart index 463a0f17f..577f69d12 100644 --- a/test/angular_spec.dart +++ b/test/angular_spec.dart @@ -147,7 +147,7 @@ main() { "angular.core.dom_internal.ViewFactory", "angular.core.dom_internal.ViewPort", "angular.core.parser.Parser", - "angular.core.parser.dynamic_parser.ClosureMap", + "angular.core.parser.ClosureMap", "angular.core_internal.ExceptionHandler", "angular.core_internal.Interpolate", "angular.core_internal.RootScope", @@ -159,6 +159,8 @@ main() { "angular.core_internal.ScopeStatsConfig", "angular.core_internal.ScopeStatsEmitter", "angular.core_internal.VmTurnZone", + "angular.core.parser.ClosureMap", + "angular.core.parser.Parser", "angular.directive.AHref", "angular.directive.ContentEditable", "angular.directive.DirectiveModule", diff --git a/test/cache/js_cache_register_spec.dart b/test/cache/js_cache_register_spec.dart index 667456eec..a530ed2aa 100644 --- a/test/cache/js_cache_register_spec.dart +++ b/test/cache/js_cache_register_spec.dart @@ -8,7 +8,7 @@ main() => describe('JsCacheRegister', () { s() => js.context['ngCaches']['sizes'].apply([]); // Create some caches in the system - beforeEach((JsCacheRegister js, DynamicParser dp, ViewCache vc) { }); + beforeEach((JsCacheRegister js, Parser dp, ViewCache vc) { }); it('should publish a JS interface', () { expect(js.context['ngCaches']).toBeDefined(); @@ -18,16 +18,16 @@ main() => describe('JsCacheRegister', () { expect(js.context['Object']['keys'].apply([s()]).length > 0).toBeTruthy(); }); - it('should clear one cache', (DynamicParser p) { + it('should clear one cache', (Parser p) { p('1'); - expect(s()['DynamicParser'] > 0).toBeTruthy(); + expect(s()['Parser'] > 0).toBeTruthy(); - js.context['ngCaches']['clear'].apply(['DynamicParser']); - expect(s()['DynamicParser']).toEqual(0); + js.context['ngCaches']['clear'].apply(['Parser']); + expect(s()['Parser']).toEqual(0); }); - it('should clear all caches', (DynamicParser p) { + it('should clear all caches', (Parser p) { p('1'); var stats = s(); diff --git a/test/core/parser/generated_getter_setter_spec.dart b/test/core/parser/generated_getter_setter_spec.dart index 74a4d93e4..d4847d075 100644 --- a/test/core/parser/generated_getter_setter_spec.dart +++ b/test/core/parser/generated_getter_setter_spec.dart @@ -7,7 +7,7 @@ import 'generated_getter_setter.dart' as gen; main() { describe('hybrid getter-setter', () { beforeEachModule((Module module) { - module..bind(Parser, toInstanceOf: DynamicParser) + module..bind(Parser) ..bind(ClosureMap, toValue: gen.closureMap); }); parser_spec.main(); diff --git a/test/core/parser/parser_spec.dart b/test/core/parser/parser_spec.dart index 6ef4d5036..9b3a7ed69 100644 --- a/test/core/parser/parser_spec.dart +++ b/test/core/parser/parser_spec.dart @@ -57,17 +57,6 @@ main() { module.bind(SubstringFormatter); }); - describe('DynamicParser', () { - // This is important because the DynamicParser expects to be a singleton - // to share its cache. It therefore registers with the CacheRegister and - // having more than one instance will result in a duplicate registration. - it('should be identical to Parser in dynamic mode', (Parser p, DynamicParser dp) { - if (p is DynamicParser) { - expect(identical(p, dp)).toBeTruthy();; - } - }); - }); - beforeEach((Parser injectedParser, FormatterMap injectedFormatters) { parser = injectedParser; formatters = injectedFormatters; @@ -174,12 +163,6 @@ main() { parser = p; }); - // We only care about the error strings in the DynamicParser. - var errStr = (x) { - if (parser is DynamicParser) { return x; } - return null; - }; - // PARSER ERRORS it('should throw a reasonable error for unconsumed tokens', () { expectEval(")").toThrow('Parser Error: Unconsumed token ) at column 1 in [)]');