From ef88e0f8045703c3d3ed835ea9c591353b3a4d82 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Wed, 2 Sep 2015 10:06:10 -0700 Subject: [PATCH] refactor(transformers): extract lifecycle hooks from the interfaces --- .../analyzer_plugin/lib/src/tasks.dart | 2 +- .../common/directive_metadata_reader.dart | 155 ++++++++++++------ .../src/transform/common/registered_type.dart | 22 ++- .../directive_processor/rewriter.dart | 30 +--- .../changeDetection.ng_deps.dart | 9 +- .../events.ng_deps.dart | 2 +- .../lifecycle.ng_deps.dart | 42 +++-- 7 files changed, 160 insertions(+), 102 deletions(-) diff --git a/modules_dart/analyzer_plugin/lib/src/tasks.dart b/modules_dart/analyzer_plugin/lib/src/tasks.dart index 58bb1a6a54..1945ba4930 100644 --- a/modules_dart/analyzer_plugin/lib/src/tasks.dart +++ b/modules_dart/analyzer_plugin/lib/src/tasks.dart @@ -34,7 +34,7 @@ class BuildUnitDirectivesTask extends SourceBasedAnalysisTask { List metaList = []; for (CompilationUnitMember unitMember in unit.declarations) { if (unitMember is ClassDeclaration) { - RenderDirectiveMetadata meta = readDirectiveMetadata(unitMember.metadata); + RenderDirectiveMetadata meta = readDirectiveMetadata(unitMember); if (meta != null) { metaList.add(meta); } diff --git a/modules_dart/transform/lib/src/transform/common/directive_metadata_reader.dart b/modules_dart/transform/lib/src/transform/common/directive_metadata_reader.dart index 0258ef80d2..bfa5233183 100644 --- a/modules_dart/transform/lib/src/transform/common/directive_metadata_reader.dart +++ b/modules_dart/transform/lib/src/transform/common/directive_metadata_reader.dart @@ -6,14 +6,20 @@ import 'package:angular2/src/core/render/api.dart'; import 'package:angular2/src/core/change_detection/change_detection.dart'; /// Reads [RenderDirectiveMetadata] from the `node`. `node` is expected to be an -/// instance of [Annotation], [NodeList], ListLiteral, or -/// [InstanceCreationExpression]. -RenderDirectiveMetadata readDirectiveMetadata(dynamic node) { - assert(node is Annotation || - node is NodeList || - node is InstanceCreationExpression || - node is ListLiteral); - var visitor = new _DirectiveMetadataVisitor(); +/// instance of [ClassDeclaration] (a class which may have a [Directive] or +/// [Component] annotation) or an [InstanceCreationExpression] (an instantiation +/// of [ReflectionInfo]). +RenderDirectiveMetadata readDirectiveMetadata(AstNode node) { + var visitor; + if (node is ClassDeclaration) { + visitor = new _DeclarationVisitor(); + } else if (node is InstanceCreationExpression) { + visitor = new _ReflectionInfoVisitor(); + } else { + throw new ArgumentError('Incorrect value passed to readDirectiveMetadata. ' + 'Expected types are ClassDeclaration and InstanceCreationExpression ' + 'Provided type was ${node.runtimeType}'); + } node.accept(visitor); return visitor.meta; } @@ -45,8 +51,79 @@ num _getDirectiveType(String annotationName, Element element) { return byNameMatch; } -/// Visitor responsible for processing the `annotations` property of a -/// [RegisterType] object and pulling out [RenderDirectiveMetadata]. +class _ReflectionInfoVisitor extends _DirectiveMetadataVisitor { + // TODO(kegluneq): Create a more robust check that this is a real + // `ReflectionInfo` instantiation. + static bool _isReflectionInfo(InstanceCreationExpression node) { + var name = node.constructorName.type.name; + name = name is PrefixedIdentifier ? name.identifier : name; + return '$name' == 'ReflectionInfo'; + } + + @override + Object visitInstanceCreationExpression(InstanceCreationExpression node) { + if (_isReflectionInfo(node)) { + // NOTE(kegluneq): This is very tightly coupled with the `Reflector` + // implementation. Clean this up with a better intermediate representation + // for .ng_deps.dart files. + var reflectionInfoArgs = node.argumentList.arguments; + if (reflectionInfoArgs.length > 0) { + // Process annotations to determine information specified via + // `Component` and `Directive` parameters. + reflectionInfoArgs[0].accept(this); + if (_hasMeta && reflectionInfoArgs.length > 3) { + // Process interfaces to determine which lifecycle events we need to + // react to for this `Directive`. + _processInterfaces(reflectionInfoArgs[3]); + } + } + return null; + } + var directiveType = _getDirectiveType( + '${node.constructorName.type.name}', node.staticElement); + if (directiveType >= 0) { + if (_hasMeta) { + throw new FormatException( + 'Only one Directive is allowed per class. ' + 'Found "$node" but already processed "$meta".', + '$node' /* source */); + } + _initializeMetadata(directiveType); + super.visitInstanceCreationExpression(node); + } + // Annotation we do not recognize - no need to visit. + return null; + } + + void _processInterfaces(Expression lifecycleValue) { + _checkMeta(); + if (lifecycleValue is! ListLiteral) { + throw new FormatException( + 'Angular 2 expects a List but could not understand the value for interfaces. ' + '$lifecycleValue'); + } + ListLiteral l = lifecycleValue; + _populateLifecycle(l.elements.map((s) => s.toSource().split('.').last)); + } +} + +class _DeclarationVisitor extends _DirectiveMetadataVisitor { + @override + Object visitClassDeclaration(ClassDeclaration node) { + node.metadata.accept(this); + if (this._hasMeta) { + if (node.implementsClause != null && + node.implementsClause.interfaces != null) { + _populateLifecycle(node.implementsClause.interfaces + .map((s) => s.toSource().split('.').last)); + } + } + return null; + } +} + +/// Visitor responsible for processing [Directive] code into a +/// [RenderDirectiveMetadata] object. class _DirectiveMetadataVisitor extends Object with RecursiveAstVisitor { bool get _hasMeta => _type != null; @@ -130,24 +207,6 @@ class _DirectiveMetadataVisitor extends Object return null; } - @override - Object visitInstanceCreationExpression(InstanceCreationExpression node) { - var directiveType = _getDirectiveType( - '${node.constructorName.type.name}', node.staticElement); - if (directiveType >= 0) { - if (_hasMeta) { - throw new FormatException( - 'Only one Directive is allowed per class. ' - 'Found "$node" but already processed "$meta".', - '$node' /* source */); - } - _initializeMetadata(directiveType); - super.visitInstanceCreationExpression(node); - } - // Annotation we do not recognize - no need to visit. - return null; - } - @override Object visitNamedExpression(NamedExpression node) { // TODO(kegluneq): Remove this limitation. @@ -170,10 +229,6 @@ class _DirectiveMetadataVisitor extends Object case 'host': _populateHost(node.expression); break; - // TODO(vicb) should come from the interfaces on the class - case 'lifecycle': - _populateLifecycle(node.expression); - break; case 'exportAs': _populateExportAs(node.expression); break; @@ -207,8 +262,7 @@ class _DirectiveMetadataVisitor extends Object if (!_hasMeta) { throw new ArgumentError( 'Incorrect value passed to readDirectiveMetadata. ' - 'Expected types are Annotation, InstanceCreationExpression, ' - 'NodeList or ListLiteral'); + 'Expected types are ClassDeclaration and InstanceCreationExpression'); } } @@ -271,23 +325,19 @@ class _DirectiveMetadataVisitor extends Object _exportAs = _expressionToString(exportAsValue, 'Directive#exportAs'); } - void _populateLifecycle(Expression lifecycleValue) { + void _populateLifecycle(Iterable lifecycleInterfaceNames) { _checkMeta(); - if (lifecycleValue is! ListLiteral) { - throw new FormatException( - 'Angular 2 expects a List but could not understand the value for lifecycle. ' - '$lifecycleValue'); - } - ListLiteral l = lifecycleValue; - var lifecycleEvents = l.elements.map((s) => s.toSource().split('.').last); - _callOnDestroy = lifecycleEvents.contains("OnDestroy"); - _callOnChange = lifecycleEvents.contains("OnChanges"); - _callDoCheck = lifecycleEvents.contains("DoCheck"); - _callOnInit = lifecycleEvents.contains("OnInit"); - _callAfterContentInit = lifecycleEvents.contains("AfterContentInit"); - _callAfterContentChecked = lifecycleEvents.contains("AfterContentChecked"); - _callAfterViewInit = lifecycleEvents.contains("AfterViewInit"); - _callAfterViewChecked = lifecycleEvents.contains("AfterViewChecked"); + _callOnDestroy = lifecycleInterfaceNames.contains("OnDestroy"); + _callOnChange = lifecycleInterfaceNames.contains("OnChanges"); + _callDoCheck = lifecycleInterfaceNames.contains("DoCheck"); + _callOnInit = lifecycleInterfaceNames.contains("OnInit"); + _callAfterContentInit = + lifecycleInterfaceNames.contains("AfterContentInit"); + _callAfterContentChecked = + lifecycleInterfaceNames.contains("AfterContentChecked"); + _callAfterViewInit = lifecycleInterfaceNames.contains("AfterViewInit"); + _callAfterViewChecked = + lifecycleInterfaceNames.contains("AfterViewChecked"); } void _populateEvents(Expression eventsValue) { @@ -301,5 +351,6 @@ class _DirectiveMetadataVisitor extends Object } } -final Map changeDetectionStrategies - = new Map.fromIterable(ChangeDetectionStrategy.values, key: (v) => v.toString()); +final Map changeDetectionStrategies = + new Map.fromIterable(ChangeDetectionStrategy.values, + key: (v) => v.toString()); diff --git a/modules_dart/transform/lib/src/transform/common/registered_type.dart b/modules_dart/transform/lib/src/transform/common/registered_type.dart index 2250194f70..e8d680a026 100644 --- a/modules_dart/transform/lib/src/transform/common/registered_type.dart +++ b/modules_dart/transform/lib/src/transform/common/registered_type.dart @@ -14,6 +14,9 @@ class RegisteredType { /// The actual call to `Reflector#registerType`. final MethodInvocation registerMethod; + /// The `ReflectionInfo` [InstanceCreationExpression] + final InstanceCreationExpression reflectionInfoCreate; + /// The factory method registered. final Expression factoryFn; @@ -25,22 +28,28 @@ class RegisteredType { RenderDirectiveMetadata _directiveMetadata = null; - RegisteredType._(this.typeName, this.registerMethod, this.factoryFn, - this.parameters, this.annotations); + RegisteredType._( + this.typeName, + this.registerMethod, + this.reflectionInfoCreate, + this.factoryFn, + this.parameters, + this.annotations); /// Creates a {@link RegisteredType} given a {@link MethodInvocation} node representing /// a call to `registerType`. factory RegisteredType.fromMethodInvocation(MethodInvocation registerMethod) { var visitor = new _ParseRegisterTypeVisitor(); registerMethod.accept(visitor); - return new RegisteredType._(visitor.typeName, registerMethod, + return new RegisteredType._(visitor.typeName, registerMethod, visitor.info, visitor.factoryFn, visitor.parameters, visitor.annotations); } RenderDirectiveMetadata get directiveMetadata { if (_directiveMetadata == null) { try { - _directiveMetadata = readDirectiveMetadata(annotations); + /// TODO(kegluneq): Allow passing a lifecycle interface matcher. + _directiveMetadata = readDirectiveMetadata(reflectionInfoCreate); if (_directiveMetadata != null) { _directiveMetadata.id = '$typeName'; } @@ -55,6 +64,7 @@ class RegisteredType { class _ParseRegisterTypeVisitor extends Object with RecursiveAstVisitor { Identifier typeName; + InstanceCreationExpression info; Expression factoryFn; Expression parameters; Expression annotations; @@ -68,9 +78,9 @@ class _ParseRegisterTypeVisitor extends Object ? node.argumentList.arguments[0] : null; - // The second argument to a `registerType` call is the RegistrationInfo + // The second argument to a `registerType` call is the `ReflectionInfo` // object creation. - var info = node.argumentList.arguments[1] as InstanceCreationExpression; + info = node.argumentList.arguments[1] as InstanceCreationExpression; var args = info.argumentList.arguments; for (int i = 0; i < args.length; i++) { var arg = args[i]; diff --git a/modules_dart/transform/lib/src/transform/directive_processor/rewriter.dart b/modules_dart/transform/lib/src/transform/directive_processor/rewriter.dart index 6e0307b617..876c81c02e 100644 --- a/modules_dart/transform/lib/src/transform/directive_processor/rewriter.dart +++ b/modules_dart/transform/lib/src/transform/directive_processor/rewriter.dart @@ -8,7 +8,6 @@ import 'package:angular2/src/core/render/xhr.dart' show XHR; import 'package:angular2/src/transform/common/annotation_matcher.dart'; import 'package:angular2/src/transform/common/asset_reader.dart'; import 'package:angular2/src/transform/common/async_string_writer.dart'; -import 'package:angular2/src/transform/common/interface_matcher.dart'; import 'package:angular2/src/transform/common/logging.dart'; import 'package:angular2/src/transform/common/names.dart'; import 'package:angular2/src/transform/common/xhr_impl.dart'; @@ -54,12 +53,7 @@ Future createNgDeps(AssetReader reader, AssetId assetId, var declarationsCode = await _getAllDeclarations(reader, assetId, code, directivesVisitor); var declarationsVisitor = new _NgDepsDeclarationsVisitor( - assetId, - writer, - new XhrImpl(reader, assetId), - annotationMatcher, - _interfaceMatcher, - ngMeta, + assetId, writer, new XhrImpl(reader, assetId), annotationMatcher, ngMeta, inlineViews: inlineViews); parseCompilationUnit(declarationsCode, name: '${assetId.path} and parts') .declarations @@ -75,8 +69,6 @@ Future createNgDeps(AssetReader reader, AssetId assetId, return writer.asyncToString(); } -InterfaceMatcher _interfaceMatcher = new InterfaceMatcher(); - /// Processes `visitor.parts`, reading and appending their contents to the /// original `code`. /// Order of `part`s is preserved. That is, if in the main library we have @@ -278,29 +270,20 @@ class _NgDepsDeclarationsVisitor extends Object with SimpleAstVisitor { /// Angular 2, for example `@Component`. final AnnotationMatcher _annotationMatcher; - /// Responsible for testing whether interfaces are recognized by Angular2, - /// for example `OnChanges`. - final InterfaceMatcher _interfaceMatcher; - /// Used to fetch linked files. final XHR _xhr; - _NgDepsDeclarationsVisitor( - AssetId assetId, - AsyncStringWriter writer, - XHR xhr, - AnnotationMatcher annotationMatcher, - InterfaceMatcher interfaceMatcher, - this.ngMeta, + _NgDepsDeclarationsVisitor(AssetId assetId, AsyncStringWriter writer, XHR xhr, + AnnotationMatcher annotationMatcher, this.ngMeta, {bool inlineViews}) : writer = writer, _copyVisitor = new ToSourceVisitor(writer), _factoryVisitor = new FactoryTransformVisitor(writer), _paramsVisitor = new ParameterTransformVisitor(writer), _metaVisitor = new AnnotationsTransformVisitor( - writer, xhr, annotationMatcher, assetId, inlineViews: inlineViews), + writer, xhr, annotationMatcher, assetId, + inlineViews: inlineViews), _annotationMatcher = annotationMatcher, - _interfaceMatcher = interfaceMatcher, this.assetId = assetId, _xhr = xhr; @@ -445,4 +428,5 @@ class _NgDepsDeclarationsVisitor extends Object with SimpleAstVisitor { } const _REF_PREFIX = '_ngRef'; -const _REFLECTOR_IMPORT = 'package:angular2/src/core/reflection/reflection.dart'; +const _REFLECTOR_IMPORT = + 'package:angular2/src/core/reflection/reflection.dart'; diff --git a/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/changeDetection.ng_deps.dart b/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/changeDetection.ng_deps.dart index 299d9d9506..1cc03c2dbb 100644 --- a/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/changeDetection.ng_deps.dart +++ b/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/changeDetection.ng_deps.dart @@ -11,8 +11,9 @@ void initReflector(reflector) { reflector ..registerType( HelloCmp, - new ReflectionInfo( - const [const Component(changeDetection: ChangeDetectionStrategy.CheckOnce)], - const [const []], - () => new HelloCmp())); + new ReflectionInfo(const [ + const Component(changeDetection: ChangeDetectionStrategy.CheckOnce) + ], const [ + const [] + ], () => new HelloCmp())); } diff --git a/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/events.ng_deps.dart b/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/events.ng_deps.dart index d11b76a913..0538a0234a 100644 --- a/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/events.ng_deps.dart +++ b/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/events.ng_deps.dart @@ -12,7 +12,7 @@ void initReflector(reflector) { ..registerType( HelloCmp, new ReflectionInfo(const [ - const Component(events: ['onFoo', 'onBar']) + const Component(events: const ['onFoo', 'onBar']) ], const [ const [] ], () => new HelloCmp())); diff --git a/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/lifecycle.ng_deps.dart b/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/lifecycle.ng_deps.dart index ea6f680c0c..20b0483fe9 100644 --- a/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/lifecycle.ng_deps.dart +++ b/modules_dart/transform/test/transform/directive_metadata_extractor/directive_metadata_files/lifecycle.ng_deps.dart @@ -2,7 +2,19 @@ library examples.hello_world.index_common_dart.ng_deps.dart; import 'hello.dart'; import 'package:angular2/angular2.dart' - show Component, Directive, View, NgElement, LifecycleEvent; + show + Component, + Directive, + View, + NgElement, + OnChanges, + OnDestroy, + OnInit, + DoCheck, + AfterContentInit, + AfterContentChecked, + AfterViewInit, + AfterViewChecked; var _visited = false; void initReflector(reflector) { @@ -11,18 +23,18 @@ void initReflector(reflector) { reflector ..registerType( HelloCmp, - new ReflectionInfo(const [ - const Component(lifecycle: [ - LifecycleEvent.OnChanges, - LifecycleEvent.OnDestroy, - LifecycleEvent.OnInit, - LifecycleEvent.DoCheck, - LifecycleEvent.AfterContentInit, - LifecycleEvent.AfterContentChecked, - LifecycleEvent.AfterViewInit, - LifecycleEvent.AfterViewChecked - ]) - ], const [ - const [] - ], () => new HelloCmp())); + new ReflectionInfo( + const [const Component()], + const [const []], + () => new HelloCmp(), + const [ + OnChanges, + OnDestroy, + OnInit, + DoCheck, + AfterContentInit, + AfterContentChecked, + AfterViewInit, + AfterViewChecked + ])); }