fix(StaticMetadataExtractor): Map members annotations to all annotations

Closes #904
dart-archive · Apr 18, 2014 · 9622318 · 9622318
1 parent 9fdd208
commit 9622318
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 55 deletions.
diff --git a/lib/tools/html_extractor.dart b/lib/tools/html_extractor.dart
@@ -70,7 +70,7 @@ class HtmlExpressionExtractor {
   visitNodes(List<Node> nodes, NodeVisitor visitor) {
     for (Node node in nodes) {
       visitor(node);
-      if (node.nodes.length > 0) {
+      if (node.nodes.isNotEmpty) {
         visitNodes(node.nodes, visitor);
       }
     }

diff --git a/lib/tools/io.dart b/lib/tools/io.dart
@@ -3,7 +3,7 @@ library angular.io;
 typedef FsVisitor(String file);
 
 /**
- * A simple mockabe wrapper around dart:io that can be used without introducing
+ * A simple mockable wrapper around dart:io that can be used without introducing
  * direct dependencies on dart:io.
  */
 abstract class IoService {

diff --git a/lib/tools/selector.dart b/lib/tools/selector.dart
@@ -22,17 +22,17 @@ class _SelectorPart {
   final String attrName;
   final String attrValue;
 
-  const _SelectorPart.fromElement(String this.element)
+  const _SelectorPart.fromElement(this.element)
       : className = null, attrName = null, attrValue = null;
 
-  const _SelectorPart.fromClass(String this.className)
+  const _SelectorPart.fromClass(this.className)
       : element = null, attrName = null, attrValue = null;
 
 
-  const _SelectorPart.fromAttribute(String this.attrName, String this.attrValue)
+  const _SelectorPart.fromAttribute(this.attrName, this.attrValue)
       : element = null, className = null;
 
-  toString() =>
+  String toString() =>
     element == null
       ? (className == null
          ? (attrValue == '' ? '[$attrName]' : '[$attrName=$attrValue]')

diff --git a/lib/tools/source_metadata_extractor.dart b/lib/tools/source_metadata_extractor.dart
@@ -99,7 +99,7 @@ class SourceMetadataExtractor {
       dirInfo.expressionAttrs.forEach((String attr) {
         if (attr == '.') {
           var matches = _ATTR_SELECTOR_REGEXP.allMatches(dirInfo.selector);
-          if (matches.length > 0) {
+          if (matches.isNotEmpty) {
             reprocessedAttrs.add(matches.last.group(1));
           }
         } else {

diff --git a/lib/tools/transformer/metadata_extractor.dart b/lib/tools/transformer/metadata_extractor.dart
@@ -342,53 +342,52 @@ class AnnotationExtractor {
       return;
     }
 
-    // Default to using the first acceptable annotation- not sure if
-    // more than one should ever occur.
-    var sourceAnnotation = acceptableAnnotations.first;
-
-    // Clone the annotation so we don't modify the one in the persistent AST.
-    var index = type.annotations.indexOf(sourceAnnotation);
-    var annotation = new AstCloner().visitAnnotation(sourceAnnotation);
-    ResolutionCopier.copyResolutionData(sourceAnnotation, annotation);
-    type.annotations[index] = annotation;
-
-    var mapArg = annotation.arguments.arguments.firstWhere((arg) =>
-        (arg is NamedExpression) && (arg.name.label.name == 'map'),
-        orElse: () => null);
-
-    // If we don't have a 'map' parameter yet, add one.
-    if (mapArg == null) {
-      var map = new MapLiteral(null, null, null, [], null);
-      var label = new Label(new SimpleIdentifier(
-          new _GeneratedToken(TokenType.STRING, 'map')),
-          new _GeneratedToken(TokenType.COLON, ':'));
-      mapArg = new NamedExpression(label, map);
-      annotation.arguments.arguments.add(mapArg);
-    }
+    // Merge attribute annotations in all of the class annotations
+    acceptableAnnotations.forEach((srcAnnotation) {
+      // Clone the annotation so we don't modify the one in the persistent AST.
+      var index = type.annotations.indexOf(srcAnnotation);
+      var annotation = new AstCloner().visitAnnotation(srcAnnotation);
+      ResolutionCopier.copyResolutionData(srcAnnotation, annotation);
+      type.annotations[index] = annotation;
+
+      var mapArg = annotation.arguments.arguments.firstWhere(
+          (arg) => (arg is NamedExpression) && (arg.name.label.name == 'map'),
+          orElse: () => null);
+
+      // If we don't have a 'map' parameter yet, add one.
+      if (mapArg == null) {
+        var map = new MapLiteral(null, null, null, [], null);
+        var label = new Label(new SimpleIdentifier(
+            new _GeneratedToken(TokenType.STRING, 'map')),
+        new _GeneratedToken(TokenType.COLON, ':'));
+        mapArg = new NamedExpression(label, map);
+        annotation.arguments.arguments.add(mapArg);
+      }
 
-    var map = mapArg.expression;
-    if (map is! MapLiteral) {
-      warn('Expected \'map\' argument of $annotation to be a map literal',
-          type.type);
-      return;
-    }
-    memberAnnotations.forEach((memberName, annotation) {
-      var key = annotation.arguments.arguments.first;
-      // If the key already exists then it means we have two annotations for
-      // same member.
-      if (map.entries.any((entry) => entry.key.toString() == key.toString())) {
-        warn('Directive $annotation already contains an entry for $key',
-            type.type);
+      var map = mapArg.expression;
+      if (map is! MapLiteral) {
+        warn('Expected \'map\' argument of $annotation to be a map literal',
+             type.type);
         return;
       }
+      memberAnnotations.forEach((memberName, annotation) {
+        var key = annotation.arguments.arguments.first;
+        // If the key already exists then it means we have two annotations for
+        // same member.
+        if (map.entries.any((entry) => entry.key.toString() == key.toString())) {
+          warn('Directive $annotation already contains an entry for $key',
+               type.type);
+          return;
+        }
 
-      var typeName = annotation.element.enclosingElement.name;
-      var value = '${_annotationToMapping[typeName]}$memberName';
-      var entry = new MapLiteralEntry(
-          key,
-          new _GeneratedToken(TokenType.COLON, ':'),
-          new SimpleStringLiteral(stringToken(value), value));
-      map.entries.add(entry);
+        var typeName = annotation.element.enclosingElement.name;
+        var value = '${_annotationToMapping[typeName]}$memberName';
+        var entry = new MapLiteralEntry(
+            key,
+            new _GeneratedToken(TokenType.COLON, ':'),
+            new SimpleStringLiteral(stringToken(value), value));
+        map.entries.add(entry);
+      });
     });
   }
 
@@ -439,5 +438,5 @@ class _AnnotationVisitor extends GeneralizingAstVisitor {
   }
 
   bool get hasAnnotations =>
-      !classAnnotations.isEmpty || !memberAnnotations.isEmpty;
+      classAnnotations.isNotEmpty || memberAnnotations.isNotEmpty;
 }
diff --git a/test/tools/source_metadata_extractor_spec.dart b/test/tools/source_metadata_extractor_spec.dart
@@ -116,7 +116,7 @@ void main() {
 }
 
 flattenList(list, map) => list.map(map).fold([], (prev, exprs) =>
-new List.from(prev)..addAll(exprs));
+    new List.from(prev)..addAll(exprs));
 
 List<DirectiveInfo> extractDirectiveInfo(List<DirectiveMetadata> metadata) {
   var sourceCrawler = new MockSourceCrawler();

diff --git a/test/tools/transformer/metadata_generator_spec.dart b/test/tools/transformer/metadata_generator_spec.dart
@@ -159,7 +159,12 @@ main() {
             'a|web/main.dart': '''
                 import 'package:angular/angular.dart';
 
-                @Decorator(map: {'another-expression': '=>anotherExpression'})
+                @Directive(
+                    selector: 'first',
+                    map: {'first-expression': '=>anotherExpression'})
+                @Directive(
+                    selector: 'second',
+                    map: {'second-expression': '=>anotherExpression'})
                 class Engine {
                   set anotherExpression(Function) {}
 
@@ -176,8 +181,13 @@ main() {
           ],
           classes: {
             'import_0.Engine': [
-              'const import_1.Decorator(map: const {'
-                '\'another-expression\': \'=>anotherExpression\', '
+              'const import_1.Directive(selector: \'first\', '
+                'map: const {'
+                '\'first-expression\': \'=>anotherExpression\', '
+                '\'two-way-stuff\': \'<=>twoWayStuff\'})',
+              'const import_1.Directive(selector: \'second\', '
+                'map: const {'
+                '\'second-expression\': \'=>anotherExpression\', '
                 '\'two-way-stuff\': \'<=>twoWayStuff\'})',
             ]
           });