feat(dart/transform): Parse `directives` dependencies from the Dart ast

Previously, we parsed dependencies out of a the stringified value of
`directives`, which is brittle and error-prone.

Move this parsing into `DirectiveProcessor` where we have the full Dart
ast to help.

modules_dart/transform/lib/src/transform/common/code/reflection_info_code.dart

@@ -83,6 +83,10 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
 
     if (node.metadata != null) {
       node.metadata.forEach((node) {
+        final extractedDirectives = _extractDirectives(node);
+        if (extractedDirectives != null) {
+          model.directives.addAll(extractedDirectives);
+        }
         model.annotations.add(_annotationVisitor.visitAnnotation(node));
       });
     }
@@ -137,6 +141,36 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
     }
   }
 
+  Iterable<PrefixedDirective> _extractDirectives(Annotation node) {
+    var shouldProcess = _annotationMatcher.isComponent(node, assetId);
+    shouldProcess = shouldProcess || _annotationMatcher.isView(node, assetId);
+
+    if (node.arguments == null && node.arguments.arguments == null) return null;
+    final directivesNode = node.arguments.arguments.firstWhere((arg) {
+      return arg is NamedExpression && '${arg.name.label}' == 'directives';
+    }, orElse: () => null);
+    if (directivesNode == null) return null;
+
+    if (directivesNode.expression is! ListLiteral) {
+      logger.warning('Angular 2 expects a list literal for `directives` '
+          'but found a ${directivesNode.expression.runtimeType}');
+      return null;
+    }
+    final directives = <PrefixedDirective>[];
+    for (var dep in (directivesNode.expression as ListLiteral).elements) {
+      if (dep is PrefixedIdentifier) {
+        directives.add(new PrefixedDirective()
+          ..prefix = '${dep.prefix}'
+          ..name = '${dep.identifier}');
+      } else if (dep is Identifier) {
+        directives.add(new PrefixedDirective()..name = '${dep}');
+      } else {
+        logger.warning('Found unexpected value $dep in `directives`.');
+      }
+    }
+    return directives;
+  }
+
   @override
   ReflectionInfoModel visitFunctionDeclaration(FunctionDeclaration node) {
     if (!node.metadata

modules_dart/transform/lib/src/transform/common/model/reflection_info_model.pb.dart

@@ -52,6 +52,55 @@ class PropertyMetadataModel extends GeneratedMessage {
 class _ReadonlyPropertyMetadataModel extends PropertyMetadataModel
     with ReadonlyMessageMixin {}
 
+class PrefixedDirective extends GeneratedMessage {
+  static final BuilderInfo _i = new BuilderInfo('PrefixedDirective')
+    ..a(1, 'prefix', PbFieldType.OS)
+    ..a(2, 'name', PbFieldType.OS)
+    ..hasRequiredFields = false;
+
+  PrefixedDirective() : super();
+  PrefixedDirective.fromBuffer(List<int> i,
+      [ExtensionRegistry r = ExtensionRegistry.EMPTY])
+      : super.fromBuffer(i, r);
+  PrefixedDirective.fromJson(String i,
+      [ExtensionRegistry r = ExtensionRegistry.EMPTY])
+      : super.fromJson(i, r);
+  PrefixedDirective clone() => new PrefixedDirective()..mergeFromMessage(this);
+  BuilderInfo get info_ => _i;
+  static PrefixedDirective create() => new PrefixedDirective();
+  static PbList<PrefixedDirective> createRepeated() =>
+      new PbList<PrefixedDirective>();
+  static PrefixedDirective getDefault() {
+    if (_defaultInstance == null) _defaultInstance =
+        new _ReadonlyPrefixedDirective();
+    return _defaultInstance;
+  }
+
+  static PrefixedDirective _defaultInstance;
+  static void $checkItem(PrefixedDirective v) {
+    if (v is! PrefixedDirective) checkItemFailed(v, 'PrefixedDirective');
+  }
+
+  String get prefix => $_get(0, 1, '');
+  void set prefix(String v) {
+    $_setString(0, 1, v);
+  }
+
+  bool hasPrefix() => $_has(0, 1);
+  void clearPrefix() => clearField(1);
+
+  String get name => $_get(1, 2, '');
+  void set name(String v) {
+    $_setString(1, 2, v);
+  }
+
+  bool hasName() => $_has(1, 2);
+  void clearName() => clearField(2);
+}
+
+class _ReadonlyPrefixedDirective extends PrefixedDirective
+    with ReadonlyMessageMixin {}
+
 class ReflectionInfoModel extends GeneratedMessage {
   static final BuilderInfo _i = new BuilderInfo('ReflectionInfoModel')
     ..a(1, 'name', PbFieldType.QS)
@@ -63,7 +112,9 @@ class ReflectionInfoModel extends GeneratedMessage {
         ParameterModel.create)
     ..p(6, 'interfaces', PbFieldType.PS)
     ..pp(7, 'propertyMetadata', PbFieldType.PM,
-        PropertyMetadataModel.$checkItem, PropertyMetadataModel.create);
+        PropertyMetadataModel.$checkItem, PropertyMetadataModel.create)
+    ..pp(8, 'directives', PbFieldType.PM, PrefixedDirective.$checkItem,
+        PrefixedDirective.create);
 
   ReflectionInfoModel() : super();
   ReflectionInfoModel.fromBuffer(List<int> i,
@@ -120,6 +171,8 @@ class ReflectionInfoModel extends GeneratedMessage {
   List<String> get interfaces => $_get(5, 6, null);
 
   List<PropertyMetadataModel> get propertyMetadata => $_get(6, 7, null);
+
+  List<PrefixedDirective> get directives => $_get(7, 8, null);
 }
 
 class _ReadonlyReflectionInfoModel extends ReflectionInfoModel
@@ -139,6 +192,14 @@ const PropertyMetadataModel$json = const {
   ],
 };
 
+const PrefixedDirective$json = const {
+  '1': 'PrefixedDirective',
+  '2': const [
+    const {'1': 'prefix', '3': 1, '4': 1, '5': 9},
+    const {'1': 'name', '3': 2, '4': 1, '5': 9},
+  ],
+};
+
 const ReflectionInfoModel$json = const {
   '1': 'ReflectionInfoModel',
   '2': const [
@@ -167,12 +228,19 @@ const ReflectionInfoModel$json = const {
       '5': 11,
       '6': '.angular2.src.transform.common.model.proto.PropertyMetadataModel'
     },
+    const {
+      '1': 'directives',
+      '3': 8,
+      '4': 3,
+      '5': 11,
+      '6': '.angular2.src.transform.common.model.proto.PrefixedDirective'
+    },
   ],
 };
 
 /**
  * Generated with:
- * reflection_info_model.proto (71d723738054f1276f792a2672a956ef9be94a4c)
+ * reflection_info_model.proto (e81bf93b6872b2bd5fabc6625be2560bacc3d186)
  * libprotoc 2.6.1
  * dart-protoc-plugin (af5fc2bf1de367a434c3b1847ab260510878ffc0)
  */

modules_dart/transform/lib/src/transform/common/model/reflection_info_model.proto

@@ -13,6 +13,15 @@ message PropertyMetadataModel {
   repeated AnnotationModel annotations = 2;
 }
 
+message PrefixedDirective {
+  // The prefix used to reference this Directive, if any.
+  optional string prefix = 1;
+
+  // The name of the Directive or directive alias.
+  // See https://goo.gl/d8XPt0 for info on directive aliases.
+  optional string name = 2;
+}
+
 message ReflectionInfoModel {
   // The (potentially prefixed) name of this Injectable.
   // This can be a `Type` or a function name.
@@ -32,4 +41,8 @@ message ReflectionInfoModel {
 
   // Entries for all properties with associated metadata.
   repeated PropertyMetadataModel propertyMetadata = 7;
+
+  // Directive dependencies parsed from the @View or @Component `directives`
+  // parameter.
+  repeated PrefixedDirective directives = 8;
 }

modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart

@@ -38,58 +38,6 @@ class CompileDataResults {
       this.ngMeta, this.viewDefinitions, this.directiveMetadatas);
 }
 
-class _PrefixedDirectiveName {
-  final String prefix;
-  final String directiveName;
-
-  _PrefixedDirectiveName(String prefix, this.directiveName)
-      : this.prefix = prefix == null ? '' : prefix;
-
-  @override
-  String toString() {
-    if (prefix.isEmpty) {
-      return directiveName;
-    } else {
-      return '${prefix}.${directiveName}';
-    }
-  }
-}
-
-String _directivesValue(ReflectionInfoModel model, bool test(AnnotationModel)) {
-  final viewAnnotation =  model.annotations.firstWhere(test, orElse: () => null);
-  if (viewAnnotation == null) return null;
-  final directivesParam = viewAnnotation.namedParameters
-        .firstWhere((p) => p.name == 'directives', orElse: () => null);
-  return directivesParam != null ? directivesParam.value : null;
-}
-
-// TODO(kegluneq): Parse this value when creating [NgDepsModel]?
-/// Find the `directives` parameter on the @View annotation and make sure that
-/// all necessary [CompileDirectiveMetadata] objects are available.
-Iterable<_PrefixedDirectiveName> _getDirectiveDeps(ReflectionInfoModel model) {
-  var directives = _directivesValue(model, (m) => m.isView);
-  if (directives == null) {
-    directives = _directivesValue(model, (m) => m.isComponent);
-  }
-  if (directives == null) return const [];
-  directives = directives.trim();
-  for (var toTrim in ['const [', '[']) {
-    if (directives.startsWith(toTrim) && directives.endsWith(']')) {
-      directives =
-          directives.substring(toTrim.length, directives.length - 1).trim();
-    }
-  }
-  if (directives.length == 0) return const [];
-  return directives.split(',').map((p) {
-    var parts = p.trim().split('.');
-    if (parts.length == 1) {
-      return new _PrefixedDirectiveName(null, parts[0]);
-    } else {
-      return new _PrefixedDirectiveName(parts[0], parts[1]);
-    }
-  });
-}
-
 /// Creates [ViewDefinition] objects for all `View` `Directive`s defined in
 /// `entryPoint`.
 class _CompileDataCreator {
@@ -127,7 +75,7 @@ class _CompileDataCreator {
         if (compileDirectiveMetadata.template != null) {
           final compileDatum = new NormalizedComponentWithViewDirectives(
               compileDirectiveMetadata, <CompileDirectiveMetadata>[]);
-          for (var dep in _getDirectiveDeps(reflectable)) {
+          for (var dep in reflectable.directives) {
             if (!ngMetaMap.containsKey(dep.prefix)) {
               logger.warning(
                   'Missing prefix "${dep.prefix}" '
@@ -137,11 +85,10 @@ class _CompileDataCreator {
             }
             final depNgMeta = ngMetaMap[dep.prefix];
 
-            if (depNgMeta.types.containsKey(dep.directiveName)) {
+            if (depNgMeta.types.containsKey(dep.name)) {
-              compileDatum.directives.add(depNgMeta.types[dep.directiveName]);
+              compileDatum.directives.add(depNgMeta.types[dep.name]);
-            } else if (depNgMeta.aliases.containsKey(dep.directiveName)) {
+            } else if (depNgMeta.aliases.containsKey(dep.name)) {
-              compileDatum.directives
+              compileDatum.directives.addAll(depNgMeta.flatten(dep.name));
-                  .addAll(depNgMeta.flatten(dep.directiveName));
             } else {
               logger.warning('Could not find Directive entry for $dep. '
                   'Please be aware that Dart transformers have limited support for '

modules_dart/transform/test/transform/directive_processor/all_tests.dart

@@ -10,6 +10,7 @@ import 'package:angular2/src/transform/common/annotation_matcher.dart';
 import 'package:angular2/src/transform/common/code/ng_deps_code.dart';
 import 'package:angular2/src/transform/common/asset_reader.dart';
 import 'package:angular2/src/transform/common/logging.dart' as log;
+import 'package:angular2/src/transform/common/model/ng_deps_model.pb.dart';
 import 'package:angular2/src/transform/common/model/reflection_info_model.pb.dart';
 import 'package:angular2/src/transform/common/ng_meta.dart';
 import 'package:barback/barback.dart';
@@ -441,6 +442,58 @@ void allTests() {
           .toContain('[soup]');
     });
   });
+
+  describe('directives', () {
+    final reflectableNamed = (NgDepsModel model, String name) {
+      return model.reflectables
+          .firstWhere((r) => r.name == name, orElse: () => null);
+    };
+
+    it('should populate `directives` from @View value specified second.',
+        () async {
+      var model =
+          (await _testCreateModel('directives_files/components.dart')).ngDeps;
+      final componentFirst = reflectableNamed(model, 'ComponentFirst');
+      expect(componentFirst).toBeNotNull();
+      expect(componentFirst.directives).toBeNotNull();
+      expect(componentFirst.directives.length).toEqual(2);
+      expect(componentFirst.directives.first)
+          .toEqual(new PrefixedDirective()..name = 'Dep');
+      expect(componentFirst.directives[1]).toEqual(new PrefixedDirective()
+        ..name = 'Dep'
+        ..prefix = 'dep2');
+    });
+
+    it('should populate `directives` from @View value specified first.',
+        () async {
+      var model =
+          (await _testCreateModel('directives_files/components.dart')).ngDeps;
+      final viewFirst = reflectableNamed(model, 'ViewFirst');
+      expect(viewFirst).toBeNotNull();
+      expect(viewFirst.directives).toBeNotNull();
+      expect(viewFirst.directives.length).toEqual(2);
+      expect(viewFirst.directives.first).toEqual(new PrefixedDirective()
+        ..name = 'Dep'
+        ..prefix = 'dep2');
+      expect(viewFirst.directives[1])
+          .toEqual(new PrefixedDirective()..name = 'Dep');
+    });
+
+    it('should populate `directives` from @Component value with no @View.',
+        () async {
+      var model =
+          (await _testCreateModel('directives_files/components.dart')).ngDeps;
+      final componentOnly = reflectableNamed(model, 'ComponentOnly');
+      expect(componentOnly).toBeNotNull();
+      expect(componentOnly.directives).toBeNotNull();
+      expect(componentOnly.directives.length).toEqual(2);
+      expect(componentOnly.directives.first)
+          .toEqual(new PrefixedDirective()..name = 'Dep');
+      expect(componentOnly.directives[1]).toEqual(new PrefixedDirective()
+        ..name = 'Dep'
+        ..prefix = 'dep2');
+    });
+  });
 }
 
 Future<NgMeta> _testCreateModel(String inputPath,

modules_dart/transform/test/transform/directive_processor/directives_files/components.dart

@@ -0,0 +1,20 @@
+library angular2.test.transform.directive_processor.directive_files.components;
+
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+import 'dep1.dart';
+import 'dep2.dart' as dep2;
+
+@Component(selector: 'component-first')
+@View(template: '<dep1></dep1><dep2></dep2>', directives: [Dep, dep2.Dep])
+class ComponentFirst {}
+
+@View(template: '<dep1></dep1><dep2></dep2>', directives: [dep2.Dep, Dep])
+@Component(selector: 'view-first')
+class ViewFirst {}
+
+@Component(
+    selector: 'component-only',
+    template: '<dep1></dep1><dep2></dep2>',
+    directives: [Dep, dep2.Dep])
+class ComponentOnly {}

modules_dart/transform/test/transform/directive_processor/directives_files/dep1.dart

@@ -0,0 +1,8 @@
+library angular2.test.transform.directive_processor.directive_files.dep1;
+
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+
+@Component(selector: 'dep1')
+@View(template: 'Dep1')
+class Dep {}

modules_dart/transform/test/transform/directive_processor/directives_files/dep2.dart

@@ -0,0 +1,8 @@
+library angular2.test.transform.directive_processor.directive_files.dep2;
+
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+
+@Component(selector: 'dep2')
+@View(template: 'Dep2')
+class Dep {}

modules_dart/transform/test/transform/template_compiler/all_tests.dart

@@ -76,8 +76,7 @@ void allTests() {
 
   Future<String> process(AssetId assetId) {
     logger = new RecordingLogger();
-    return log.setZoned(logger,
+    return log.setZoned(logger, () => processTemplates(reader, assetId));
-        () => processTemplates(reader, assetId));
   }
 
   // TODO(tbosch): This is just a temporary test that makes sure that the dart
@@ -88,10 +87,9 @@ void allTests() {
     final viewAnnotation = new AnnotationModel()
       ..name = 'View'
       ..isView = true;
-    viewAnnotation.namedParameters.add(new NamedParameter()
-      ..name = 'directives'
-      ..value = 'const [NgFor]');
     fooNgMeta.ngDeps.reflectables.first.annotations.add(viewAnnotation);
+    fooNgMeta.ngDeps.reflectables.first.directives
+        .add(new PrefixedDirective()..name = 'NgFor');
     fooNgMeta.ngDeps.imports.add(
         new ImportModel()..uri = 'package:angular2/src/directives/ng_for.dart');
 
@@ -155,6 +153,8 @@ void allTests() {
       ..name = 'directives'
       ..value = 'const [${barComponentMeta.type.name}]');
     fooNgMeta.ngDeps.reflectables.first.annotations.add(viewAnnotation);
+    fooNgMeta.ngDeps.reflectables.first.directives
+        .add(new PrefixedDirective()..name = barComponentMeta.type.name);
     fooNgMeta.ngDeps.imports.add(new ImportModel()..uri = 'bar.dart');
     barComponentMeta.template =
         new CompileTemplateMetadata(template: 'BarTemplate');
@@ -176,69 +176,6 @@ void allTests() {
       ..toContain("import 'bar.template.dart'");
   });
 
-  it('should handle `directives` regardless of annotation ordering', () async {
-    fooComponentMeta.template =
-    new CompileTemplateMetadata(template: '<${barComponentMeta.selector}>');
-    final viewAnnotation = new AnnotationModel()
-      ..name = 'View'
-      ..isView = true;
-    final directivesParameter = new NamedParameter()
-      ..name = 'directives'
-      ..value = 'const [${barComponentMeta.type.name}]';
-    viewAnnotation.namedParameters.add(directivesParameter);
-    final componentAnnotation = new AnnotationModel()
-      ..name = 'Component'
-      ..isComponent = true;
-    fooNgMeta.ngDeps.reflectables.first.annotations
-        .addAll([viewAnnotation, componentAnnotation]);
-    fooNgMeta.ngDeps.imports.add(new ImportModel()..uri = 'bar.dart');
-    barComponentMeta.template =
-    new CompileTemplateMetadata(template: 'BarTemplate');
-    updateReader();
-
-    final viewFirstOutputs = await process(fooAssetId);
-
-    fooNgMeta.ngDeps.reflectables.first.annotations.clear();
-    fooNgMeta.ngDeps.reflectables.first.annotations
-        .addAll([componentAnnotation, viewAnnotation]);
-    updateReader();
-
-    final componentFirstOutputs = await process(fooAssetId);
-
-    expect(viewFirstOutputs.templatesCode).toEqual(componentFirstOutputs.templatesCode);
-  });
-
-  it('should handle `directives` on @Component or @View', () async {
-    fooComponentMeta.template =
-    new CompileTemplateMetadata(template: '<${barComponentMeta.selector}>');
-    final viewAnnotation = new AnnotationModel()
-      ..name = 'View'
-      ..isView = true;
-    final directivesParameter = new NamedParameter()
-      ..name = 'directives'
-      ..value = 'const [${barComponentMeta.type.name}]';
-    viewAnnotation.namedParameters.add(directivesParameter);
-    final componentAnnotation = new AnnotationModel()
-      ..name = 'Component'
-      ..isComponent = true;
-    fooNgMeta.ngDeps.reflectables.first.annotations
-        .addAll([viewAnnotation, componentAnnotation]);
-    fooNgMeta.ngDeps.imports.add(new ImportModel()..uri = 'bar.dart');
-    barComponentMeta.template =
-    new CompileTemplateMetadata(template: 'BarTemplate');
-    updateReader();
-
-    final onViewOutputs = await process(fooAssetId);
-
-    viewAnnotation.namedParameters.clear();
-    componentAnnotation.namedParameters.add(directivesParameter);
-    updateReader();
-
-    final onComponentOutputs = await process(fooAssetId);
-
-    expect(onComponentOutputs.templatesCode).toEqual(onViewOutputs.templatesCode);
-  });
-
   it('should parse `View` directives with a single prefixed dependency.',
       () async {
     fooComponentMeta.template =
@@ -246,10 +183,10 @@ void allTests() {
     final componentAnnotation = new AnnotationModel()
       ..name = 'View'
       ..isView = true;
-    componentAnnotation.namedParameters.add(new NamedParameter()
-      ..name = 'directives'
-      ..value = 'const [prefix.${barComponentMeta.type.name}]');
     fooNgMeta.ngDeps.reflectables.first.annotations.add(componentAnnotation);
+    fooNgMeta.ngDeps.reflectables.first.directives.add(new PrefixedDirective()
+      ..name = barComponentMeta.type.name
+      ..prefix = 'prefix');
     fooNgMeta.ngDeps.imports.add(new ImportModel()
       ..uri = 'bar.dart'
       ..prefix = 'prefix');
@@ -279,11 +216,9 @@ void allTests() {
     final componentAnnotation = new AnnotationModel()
       ..name = 'View'
       ..isView = true;
-    final directivesParam = new NamedParameter()
-      ..name = 'directives'
-      ..value = 'const [directiveAlias]';
-    componentAnnotation.namedParameters.add(directivesParam);
     fooNgMeta.ngDeps.reflectables.first.annotations.add(componentAnnotation);
+    fooNgMeta.ngDeps.reflectables.first.directives
+        .add(new PrefixedDirective()..name = 'directiveAlias');
     fooNgMeta.ngDeps.imports.add(new ImportModel()..uri = 'bar.dart');
 
     fooNgMeta.aliases['directiveAlias'] = [barComponentMeta.type.name];