From abc887854766f272123e00975774c398a3135c39 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Fri, 8 May 2015 14:02:47 -0700 Subject: [PATCH] feat(dart/transform): Reuse readDirectiveMetadata in plugin Share code for parsing `DirectiveMetadata` values between the transformer and the analyzer plugin. --- .../reflection/reflection_capabilities.dart | 3 +- .../common/directive_metadata_reader.dart | 146 ++++++++++++------ .../src/transform/common/registered_type.dart | 19 +++ .../extractor.dart | 6 +- .../all_tests.dart | 12 +- .../analyzer_plugin/lib/src/tasks.dart | 68 +------- 6 files changed, 128 insertions(+), 126 deletions(-) diff --git a/modules/angular2/src/reflection/reflection_capabilities.dart b/modules/angular2/src/reflection/reflection_capabilities.dart index 89542d5e32..85cba43491 100644 --- a/modules/angular2/src/reflection/reflection_capabilities.dart +++ b/modules/angular2/src/reflection/reflection_capabilities.dart @@ -5,8 +5,7 @@ import 'types.dart'; import 'dart:mirrors'; class ReflectionCapabilities { - ReflectionCapabilities([metadataReader]) { - } + ReflectionCapabilities([metadataReader]) {} Function factory(Type type) { ClassMirror classMirror = reflectType(type); diff --git a/modules/angular2/src/transform/common/directive_metadata_reader.dart b/modules/angular2/src/transform/common/directive_metadata_reader.dart index d398508969..3234664663 100644 --- a/modules/angular2/src/transform/common/directive_metadata_reader.dart +++ b/modules/angular2/src/transform/common/directive_metadata_reader.dart @@ -1,30 +1,47 @@ library angular2.transform.common.directive_metadata_reader; import 'package:analyzer/analyzer.dart'; +import 'package:analyzer/src/generated/element.dart'; import 'package:angular2/src/render/api.dart'; -import 'logging.dart'; -import 'parser.dart'; -/// Reads [DirectiveMetadata] from the `attributes` of `t`. -DirectiveMetadata readDirectiveMetadata(RegisteredType t) { +/// Reads [DirectiveMetadata] from the `node`. `node` is expected to be an +/// instance of [Annotation], [NodeList], ListLiteral, or +/// [InstanceCreationExpression]. +DirectiveMetadata readDirectiveMetadata(dynamic node) { + assert(node is Annotation || + node is NodeList || + node is InstanceCreationExpression || + node is ListLiteral); var visitor = new _DirectiveMetadataVisitor(); - t.annotations.accept(visitor); - if (visitor.meta != null) { - visitor.meta.id = '${t.typeName}'; - } + node.accept(visitor); return visitor.meta; } -num _getDirectiveType(String annotationName) { +num _getDirectiveType(String annotationName, Element element) { + var byNameMatch = -1; // TODO(kegluneq): Detect subtypes & implementations of `Directive`s. switch (annotationName) { case 'Directive': - return DirectiveMetadata.DIRECTIVE_TYPE; + byNameMatch = DirectiveMetadata.DIRECTIVE_TYPE; + break; case 'Component': - return DirectiveMetadata.COMPONENT_TYPE; + byNameMatch = DirectiveMetadata.COMPONENT_TYPE; + break; default: return -1; } + if (element != null) { + var byResolvedAst = -1; + var libName = element.library.name; + // If we have resolved, ensure the library is correct. + if (libName == 'angular2.src.core.annotations.annotations' || + libName == 'angular2.src.core.annotations_impl.annotations') { + byResolvedAst = byNameMatch; + } + // TODO(kegluneq): @keertip, can we expose this as a warning? + assert(byNameMatch == byResolvedAst); + } + return byNameMatch; } /// Visitor responsible for processing the `annotations` property of a @@ -33,22 +50,45 @@ class _DirectiveMetadataVisitor extends Object with RecursiveAstVisitor { DirectiveMetadata meta; + void _createEmptyMetadata(num type) { + assert(type >= 0); + meta = new DirectiveMetadata( + type: type, + compileChildren: true, + properties: {}, + hostListeners: {}, + hostProperties: {}, + hostAttributes: {}, + readAttributes: []); + } + @override - Object visitInstanceCreationExpression(InstanceCreationExpression node) { - var directiveType = _getDirectiveType('${node.constructorName.type.name}'); + Object visitAnnotation(Annotation node) { + var directiveType = _getDirectiveType('${node.name}', node.element); if (directiveType >= 0) { if (meta != null) { - logger.error('Only one Directive is allowed per class. ' - 'Found "$node" but already processed "$meta".'); + throw new FormatException('Only one Directive is allowed per class. ' + 'Found "$node" but already processed "$meta".', + '$node' /* source */); } - meta = new DirectiveMetadata( - type: directiveType, - compileChildren: true, - properties: {}, - hostListeners: {}, - hostProperties: {}, - hostAttributes: {}, - readAttributes: []); + _createEmptyMetadata(directiveType); + super.visitAnnotation(node); + } + // Annotation we do not recognize - no need to visit. + return null; + } + + @override + Object visitInstanceCreationExpression(InstanceCreationExpression node) { + var directiveType = _getDirectiveType( + '${node.constructorName.type.name}', node.staticElement); + if (directiveType >= 0) { + if (meta != null) { + throw new FormatException('Only one Directive is allowed per class. ' + 'Found "$node" but already processed "$meta".', + '$node' /* source */); + } + _createEmptyMetadata(directiveType); super.visitInstanceCreationExpression(node); } // Annotation we do not recognize - no need to visit. @@ -59,10 +99,9 @@ class _DirectiveMetadataVisitor extends Object Object visitNamedExpression(NamedExpression node) { // TODO(kegluneq): Remove this limitation. if (node.name is! Label || node.name.label is! SimpleIdentifier) { - logger.error( - 'Angular 2 currently only supports simple identifiers in directives.' - ' Source: ${node}'); - return null; + throw new FormatException( + 'Angular 2 currently only supports simple identifiers in directives.', + '$node' /* source */); } var keyString = '${node.name.label}'; // TODO(kegluneq): Populate the other values in [DirectiveMetadata] once @@ -93,9 +132,9 @@ class _DirectiveMetadataVisitor extends Object String _expressionToString(Expression node, String nodeDescription) { // TODO(kegluneq): Accept more options. if (node is! SimpleStringLiteral) { - logger.error('Angular 2 currently only supports string literals ' - 'in $nodeDescription. Source: ${node}'); - return null; + throw new FormatException( + 'Angular 2 currently only supports string literals ' + 'in $nodeDescription.', '$node' /* source */); } return stringLiteralToString(node); } @@ -104,23 +143,30 @@ class _DirectiveMetadataVisitor extends Object meta.selector = _expressionToString(selectorValue, 'Directive#selector'); } + void _checkMeta() { + if (meta == null) { + throw new ArgumentError( + 'Incorrect value passed to readDirectiveMetadata. ' + 'Expected types are Annotation and InstanceCreationExpression'); + } + } + void _populateCompileChildren(Expression compileChildrenValue) { + _checkMeta(); if (compileChildrenValue is! BooleanLiteral) { - logger.error( + throw new FormatException( 'Angular 2 currently only supports boolean literal values for ' - 'Directive#compileChildren.' - ' Source: ${compileChildrenValue}'); - return; + 'Directive#compileChildren.', '$compileChildrenValue' /* source */); } meta.compileChildren = (compileChildrenValue as BooleanLiteral).value; } void _populateProperties(Expression propertiesValue) { + _checkMeta(); if (propertiesValue is! MapLiteral) { - logger.error('Angular 2 currently only supports map literal values for ' - 'Directive#properties.' - ' Source: ${propertiesValue}'); - return; + throw new FormatException( + 'Angular 2 currently only supports map literal values for ' + 'Directive#properties.', '$propertiesValue' /* source */); } for (MapLiteralEntry entry in (propertiesValue as MapLiteral).entries) { var sKey = _expressionToString(entry.key, 'Directive#properties keys'); @@ -130,11 +176,11 @@ class _DirectiveMetadataVisitor extends Object } void _populateHostListeners(Expression hostListenersValue) { + _checkMeta(); if (hostListenersValue is! MapLiteral) { - logger.error('Angular 2 currently only supports map literal values for ' - 'Directive#hostListeners.' - ' Source: ${hostListenersValue}'); - return; + throw new FormatException( + 'Angular 2 currently only supports map literal values for ' + 'Directive#hostListeners.', '$hostListenersValue' /* source */); } for (MapLiteralEntry entry in (hostListenersValue as MapLiteral).entries) { var sKey = _expressionToString(entry.key, 'Directive#hostListeners keys'); @@ -145,11 +191,11 @@ class _DirectiveMetadataVisitor extends Object } void _populateHostProperties(Expression hostPropertyValue) { + _checkMeta(); if (hostPropertyValue is! MapLiteral) { - logger.error('Angular 2 currently only supports map literal values for ' - 'Directive#hostProperties.' - ' Source: ${hostPropertyValue}'); - return; + throw new FormatException( + 'Angular 2 currently only supports map literal values for ' + 'Directive#hostProperties.', '$hostPropertyValue' /* source */); } for (MapLiteralEntry entry in (hostPropertyValue as MapLiteral).entries) { var sKey = @@ -161,11 +207,11 @@ class _DirectiveMetadataVisitor extends Object } void _populateHostAttributes(Expression hostAttributeValue) { + _checkMeta(); if (hostAttributeValue is! MapLiteral) { - logger.error('Angular 2 currently only supports map literal values for ' - 'Directive#hostAttributes.' - ' Source: ${hostAttributeValue}'); - return; + throw new FormatException( + 'Angular 2 currently only supports map literal values for ' + 'Directive#hostAttributes.', '$hostAttributeValue' /* source */); } for (MapLiteralEntry entry in (hostAttributeValue as MapLiteral).entries) { var sKey = diff --git a/modules/angular2/src/transform/common/registered_type.dart b/modules/angular2/src/transform/common/registered_type.dart index 03d7935cd1..f401c21495 100644 --- a/modules/angular2/src/transform/common/registered_type.dart +++ b/modules/angular2/src/transform/common/registered_type.dart @@ -1,6 +1,9 @@ library angular2.transform.common.registered_type; import 'package:analyzer/analyzer.dart'; +import 'package:angular2/src/render/api.dart'; +import 'package:angular2/src/transform/common/directive_metadata_reader.dart'; +import 'package:angular2/src/transform/common/logging.dart'; import 'package:angular2/src/transform/common/names.dart'; /// A call to `Reflector#registerType` generated by `DirectiveProcessor`. @@ -16,6 +19,8 @@ class RegisteredType { /// The annotations registered. final Expression annotations; + DirectiveMetadata _directiveMetadata = null; + RegisteredType._(this.typeName, this.registerMethod, this.factoryFn, this.parameters, this.annotations); @@ -27,6 +32,20 @@ class RegisteredType { return new RegisteredType._(visitor.typeName, registerMethod, visitor.factoryFn, visitor.parameters, visitor.annotations); } + + DirectiveMetadata get directiveMetadata { + if (_directiveMetadata == null) { + try { + _directiveMetadata = readDirectiveMetadata(annotations); + if (_directiveMetadata != null) { + _directiveMetadata.id = '$typeName'; + } + } on FormatException catch (ex) { + logger.error(ex.message); + } + } + return _directiveMetadata; + } } class _ParseRegisterTypeVisitor extends Object diff --git a/modules/angular2/src/transform/directive_metadata_extractor/extractor.dart b/modules/angular2/src/transform/directive_metadata_extractor/extractor.dart index 94cb56297d..c7993b2a08 100644 --- a/modules/angular2/src/transform/directive_metadata_extractor/extractor.dart +++ b/modules/angular2/src/transform/directive_metadata_extractor/extractor.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'package:analyzer/analyzer.dart'; import 'package:angular2/src/render/api.dart'; import 'package:angular2/src/transform/common/asset_reader.dart'; -import 'package:angular2/src/transform/common/directive_metadata_reader.dart'; import 'package:angular2/src/transform/common/logging.dart'; import 'package:angular2/src/transform/common/names.dart'; import 'package:angular2/src/transform/common/parser.dart'; @@ -58,9 +57,8 @@ Map _metadataMapFromNgDeps(NgDeps ngDeps) { if (ngDeps == null || ngDeps.registeredTypes.isEmpty) return null; var retVal = {}; ngDeps.registeredTypes.forEach((rType) { - var meta = readDirectiveMetadata(rType); - if (meta != null) { - retVal['${rType.typeName}'] = meta; + if (rType.directiveMetadata != null) { + retVal['${rType.typeName}'] = rType.directiveMetadata; } }); return retVal; diff --git a/modules/angular2/test/transform/directive_metadata_extractor/all_tests.dart b/modules/angular2/test/transform/directive_metadata_extractor/all_tests.dart index 8922dd4afe..5ade4298cb 100644 --- a/modules/angular2/test/transform/directive_metadata_extractor/all_tests.dart +++ b/modules/angular2/test/transform/directive_metadata_extractor/all_tests.dart @@ -24,7 +24,7 @@ void allTests() { Future readMetadata(inputPath) async { var ngDeps = await parser.parse(new AssetId('a', inputPath)); - return readDirectiveMetadata(ngDeps.registeredTypes.first); + return ngDeps.registeredTypes.first.directiveMetadata; } describe('readMetadata', () { @@ -44,17 +44,17 @@ void allTests() { // Unset value defaults to `true`. it.moveNext(); expect('${it.current.typeName}').toEqual('UnsetComp'); - var unsetComp = readDirectiveMetadata(it.current); + var unsetComp = it.current.directiveMetadata; expect(unsetComp.compileChildren).toBeTrue(); it.moveNext(); expect('${it.current.typeName}').toEqual('FalseComp'); - var falseComp = readDirectiveMetadata(it.current); + var falseComp = it.current.directiveMetadata; expect(falseComp.compileChildren).toBeFalse(); it.moveNext(); expect('${it.current.typeName}').toEqual('TrueComp'); - var trueComp = readDirectiveMetadata(it.current); + var trueComp = it.current.directiveMetadata; expect(trueComp.compileChildren).toBeTrue(); }); @@ -85,8 +85,8 @@ void allTests() { var ngDeps = await parser.parse(new AssetId('a', 'directive_metadata_extractor/' 'directive_metadata_files/too_many_directives.ng_deps.dart')); - expect(() => readDirectiveMetadata(ngDeps.registeredTypes.first)) - .toThrowWith(anInstanceOf: PrintLoggerError); + expect(() => ngDeps.registeredTypes.first.directiveMetadata).toThrowWith( + anInstanceOf: PrintLoggerError); }); }); diff --git a/modules_dart/analyzer_plugin/lib/src/tasks.dart b/modules_dart/analyzer_plugin/lib/src/tasks.dart index 08f00d1938..63a29be631 100644 --- a/modules_dart/analyzer_plugin/lib/src/tasks.dart +++ b/modules_dart/analyzer_plugin/lib/src/tasks.dart @@ -1,13 +1,12 @@ library angular2.src.analysis.analyzer_plugin.src.tasks; import 'package:analyzer/src/generated/ast.dart' hide Directive; -import 'package:analyzer/src/generated/element.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/task/general.dart'; import 'package:analyzer/task/dart.dart'; import 'package:analyzer/task/model.dart'; -import 'package:angular2/src/core/annotations/annotations.dart'; import 'package:angular2/src/render/api.dart'; +import 'package:angular2/src/transform/common/directive_metadata_reader.dart'; /// The [DirectiveMetadata]s of a [LibrarySpecificUnit]. final ListResultDescriptor DIRECTIVES = @@ -33,74 +32,15 @@ class BuildUnitDirectivesTask extends SourceBasedAnalysisTask { List metaList = []; for (CompilationUnitMember unitMember in unit.declarations) { if (unitMember is ClassDeclaration) { - for (Annotation annotationNode in unitMember.metadata) { - Directive directive = _createDirective(annotationNode); - if (directive != null) { - DirectiveMetadata meta = new DirectiveMetadata( - type: _getDirectiveType(directive), - selector: directive.selector); - metaList.add(meta); - } + DirectiveMetadata meta = readDirectiveMetadata(unitMember.metadata); + if (meta != null) { + metaList.add(meta); } } } outputs[DIRECTIVES] = metaList; } - /// Returns an Angular [Directive] that corresponds to the given [node]. - /// Returns `null` if not an Angular annotation. - Directive _createDirective(Annotation node) { - // TODO(scheglov) add support for all arguments - if (_isAngularAnnotation(node, 'Component')) { - String selector = _getNamedArgument(node, 'selector'); - return new Component(selector: selector); - } - if (_isAngularAnnotation(node, 'Directive')) { - String selector = _getNamedArgument(node, 'selector'); - return new Directive(selector: selector); - } - return null; - } - - int _getDirectiveType(Directive directive) { - if (directive is Component) { - return DirectiveMetadata.COMPONENT_TYPE; - } - return DirectiveMetadata.DIRECTIVE_TYPE; - } - - /// Returns the value of an argument with the given [name]. - /// Returns `null` if not found or cannot be evaluated statically. - Object _getNamedArgument(Annotation node, String name) { - if (node.arguments != null) { - List arguments = node.arguments.arguments; - for (Expression argument in arguments) { - if (argument is NamedExpression && - argument.name != null && - argument.name.label != null && - argument.name.label.name == name) { - Expression expression = argument.expression; - if (expression is SimpleStringLiteral) { - return expression.value; - } - } - } - } - return null; - } - - /// Returns `true` is the given [node] is resolved to a creation of an Angular - /// annotation class with the given [name]. - bool _isAngularAnnotation(Annotation node, String name) { - if (node.element is ConstructorElement) { - ClassElement clazz = node.element.enclosingElement; - return clazz.library.name == - 'angular2.src.core.annotations.annotations' && - clazz.name == name; - } - return null; - } - static Map buildInputs(LibrarySpecificUnit target) { return {UNIT_INPUT: RESOLVED_UNIT.of(target)}; }