From efddc9069ca3d76ffe7df71b06cf5196c83feb90 Mon Sep 17 00:00:00 2001
From: Tim Blasi <tjblasi@google.com>
Date: Wed, 14 Oct 2015 09:33:00 -0700
Subject: [PATCH] fix(dart/transform): Parse directives agnostic of annotation
 order

Now that we can specify `directives` in either `@View` or `@Component`,
we will try to parse from both.

Previously, we would trash any `directives` parsed in the first
annotation upon encountering the second annotation. This ensures that we
maintain that list of `directives` regardless of annotation ordering.
---
 .../compile_data_creator.dart                 | 15 ++++++---
 .../template_compiler/all_tests.dart          | 21 +++++++++---
 .../component_first.ng_deps.dart              | 24 ++++++++++++++
 .../component_first.ng_meta.json              | 32 +++++++++++++++++++
 .../view_first.ng_deps.dart                   | 24 ++++++++++++++
 .../view_first.ng_meta.json                   | 32 +++++++++++++++++++
 6 files changed, 139 insertions(+), 9 deletions(-)
 create mode 100644 modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/component_first.ng_deps.dart
 create mode 100644 modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/component_first.ng_meta.json
 create mode 100644 modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/view_first.ng_deps.dart
 create mode 100644 modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/view_first.ng_meta.json

diff --git a/modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart b/modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart
index e46d5007f6..27498b45f2 100644
--- a/modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart
+++ b/modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart
@@ -172,6 +172,7 @@ class _CompileDataCreator {
     return retVal;
   }
 }
+
 /// Visitor responsible for processing the `annotations` property of a
 /// [RegisterType] object, extracting the `directives` dependencies, and adding
 /// their associated [CompileDirectiveMetadata] to the `directives` of a
@@ -198,8 +199,14 @@ class _DirectiveDependenciesVisitor extends Object
   @override
   Object visitInstanceCreationExpression(InstanceCreationExpression node) {
     if (_isViewAnnotation(node) || _isComponentAnnotation(node)) {
-      compileData = new NormalizedComponentWithViewDirectives(
-          null, <CompileDirectiveMetadata>[]);
+      if (compileData == null) {
+        compileData = new NormalizedComponentWithViewDirectives(
+            null, <CompileDirectiveMetadata>[]);
+      } else {
+        // This is set above, after the visitor is finished. If this value is
+        // non-null it indicates that we forgot to call `reset()`.
+        assert(compileData.component == null);
+      }
       node.visitChildren(this);
     }
     return null;
@@ -212,7 +219,7 @@ class _DirectiveDependenciesVisitor extends Object
     if (node.name is! Label || node.name.label is! SimpleIdentifier) {
       logger.error(
           'Angular 2 currently only supports simple identifiers in directives.'
-              ' Source: ${node}');
+          ' Source: ${node}');
       return null;
     }
     if ('${node.name.label}' == 'directives') {
@@ -244,7 +251,7 @@ class _DirectiveDependenciesVisitor extends Object
       } else {
         logger.error(
             'Angular 2 currently only supports simple and prefixed identifiers '
-                'as values for "directives". Source: $node');
+            'as values for "directives". Source: $node');
         return;
       }
       if (ngMeta.types.containsKey(name)) {
diff --git a/modules_dart/transform/test/transform/template_compiler/all_tests.dart b/modules_dart/transform/test/transform/template_compiler/all_tests.dart
index 8650f04197..54a5c33070 100644
--- a/modules_dart/transform/test/transform/template_compiler/all_tests.dart
+++ b/modules_dart/transform/test/transform/template_compiler/all_tests.dart
@@ -61,12 +61,23 @@ void changeDetectorTests() {
         'template_compiler/directive_aliases_files/hello2.ng_deps.dart';
     // Except for the directive argument in the View annotation, the generated
     // change detectors are identical.
+    var output1 = (await process(new AssetId('a', input1Path))).templatesCode;
+    var output2 = (await process(new AssetId('a', input2Path))).templatesCode;
+    _formatThenExpectEquals(output1, output2);
+  });
+
+  it('should handle `directives` regardless of annotation ordering', () async {
+    // Input 2 is the same as input1, but has the @View annotation listed first.
+    var input1Path = 'template_compiler/annotation_ordering_files/'
+        'component_first.ng_deps.dart';
+    var input2Path = 'template_compiler/annotation_ordering_files/'
+        'view_first.ng_deps.dart';
+    // Except for the type name, the generated change detectors are identical.
     var output1 = (await process(new AssetId('a', input1Path)))
-        .ngDepsCode
-        .replaceFirst(
-            'directives: const [alias1]', 'directives: const [GoodbyeCmp]')
-        .replaceFirst('hello1', 'hello2');
-    var output2 = (await process(new AssetId('a', input2Path))).ngDepsCode;
+        .templatesCode
+        .replaceAll('ComponentFirst', 'ViewFirst')
+        .replaceAll('component_first', 'view_first');
+    var output2 = (await process(new AssetId('a', input2Path))).templatesCode;
     _formatThenExpectEquals(output1, output2);
   });
 }
diff --git a/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/component_first.ng_deps.dart b/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/component_first.ng_deps.dart
new file mode 100644
index 0000000000..50acacfb13
--- /dev/null
+++ b/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/component_first.ng_deps.dart
@@ -0,0 +1,24 @@
+library test.src.transform.template_compiler.annotation_ordering_files.component_first.ng_deps.dart;
+
+import 'component_first.dart';
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+import 'package:angular2/src/directives/ng_for.dart';
+export 'component_first.dart';
+
+var _visited = false;
+void initReflector(reflector) {
+  if (_visited) return;
+  _visited = true;
+  reflector
+    ..registerType(
+        ComponentFirst,
+        new ReflectionInfo(const [
+          const Component(selector: 'hello-app'),
+          const View(
+              template: '<li *ng-for="#thing of things"><div>test</div></li>',
+              directives: const [NgFor])
+        ], const [
+          const []
+        ], () => new ComponentFirst()));
+}
diff --git a/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/component_first.ng_meta.json b/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/component_first.ng_meta.json
new file mode 100644
index 0000000000..17f466a482
--- /dev/null
+++ b/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/component_first.ng_meta.json
@@ -0,0 +1,32 @@
+{
+  "ComponentFirst":
+  {
+    "kind": "type",
+    "value": {
+      "isComponent": true,
+      "dynamicLoadable": true,
+      "selector":"hello-app",
+      "exportAs": null,
+      "type": {
+        "id": 1,
+        "name": "ComponentFirst",
+        "moduleUrl": "asset:angular2/test/transform/template_compiler/ng_for_files/hello.dart"
+      },
+      "changeDetection": 5,
+      "inputs": {},
+      "outputs": {},
+      "hostListeners": {},
+      "hostProperties": {},
+      "hostAttributes": {},
+      "lifecycleHooks": [],
+      "template": {
+        "encapsulation": 0,
+        "template": "<li *ng-for=\"#thing of things\"><div>test</div></li>",
+        "templateUrl": null,
+        "styles": null,
+        "styleUrls": null,
+        "ngContentSelectors": null
+      }
+    }
+  }
+}
diff --git a/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/view_first.ng_deps.dart b/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/view_first.ng_deps.dart
new file mode 100644
index 0000000000..b060746e3c
--- /dev/null
+++ b/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/view_first.ng_deps.dart
@@ -0,0 +1,24 @@
+library test.src.transform.template_compiler.annotation_ordering_files.view_first.ng_deps.dart;
+
+import 'view_first.dart';
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+import 'package:angular2/src/directives/ng_for.dart';
+export 'view_first.dart';
+
+var _visited = false;
+void initReflector(reflector) {
+  if (_visited) return;
+  _visited = true;
+  reflector
+    ..registerType(
+        ViewFirst,
+        new ReflectionInfo(const [
+          const View(
+              template: '<li *ng-for="#thing of things"><div>test</div></li>',
+              directives: const [NgFor]),
+          const Component(selector: 'hello-app')
+        ], const [
+          const []
+        ], () => new ViewFirst()));
+}
diff --git a/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/view_first.ng_meta.json b/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/view_first.ng_meta.json
new file mode 100644
index 0000000000..462d227fd2
--- /dev/null
+++ b/modules_dart/transform/test/transform/template_compiler/annotation_ordering_files/view_first.ng_meta.json
@@ -0,0 +1,32 @@
+{
+  "ViewFirst":
+  {
+    "kind": "type",
+    "value": {
+      "isComponent": true,
+      "dynamicLoadable": true,
+      "selector":"hello-app",
+      "exportAs": null,
+      "type": {
+        "id": 1,
+        "name": "ViewFirst",
+        "moduleUrl": "asset:angular2/test/transform/template_compiler/ng_for_files/hello.dart"
+      },
+      "changeDetection": 5,
+      "inputs": {},
+      "outputs": {},
+      "hostListeners": {},
+      "hostProperties": {},
+      "hostAttributes": {},
+      "lifecycleHooks": [],
+      "template": {
+        "encapsulation": 0,
+        "template": "<li *ng-for=\"#thing of things\"><div>test</div></li>",
+        "templateUrl": null,
+        "styles": null,
+        "styleUrls": null,
+        "ngContentSelectors": null
+      }
+    }
+  }
+}