From a1b4b60d12a9dafb6e2b7ee9121f30c31cebca5a Mon Sep 17 00:00:00 2001
From: matanl <matanl@google.com>
Date: Wed, 8 Aug 2018 22:45:51 -0700
Subject: [PATCH] fix(Compiler): Issue an error on improper `@deferred` usage.

Closes https://github.com/dart-lang/angular/issues/1538.

PiperOrigin-RevId: 207997994
---
 .../1538_deferred_template_test.dart          | 54 +++++++++++++++++++
 angular/CHANGELOG.md                          | 28 ++++++++++
 angular_ast/CHANGELOG.md                      |  3 ++
 .../lib/src/exception_handler/exceptions.dart | 11 ++++
 .../lib/src/visitors/desugar_visitor.dart     | 30 +++++++++--
 5 files changed, 122 insertions(+), 4 deletions(-)
 create mode 100644 _tests/test/regression/1538_deferred_template_test.dart

diff --git a/_tests/test/regression/1538_deferred_template_test.dart b/_tests/test/regression/1538_deferred_template_test.dart
new file mode 100644
index 0000000000..7920a00909
--- /dev/null
+++ b/_tests/test/regression/1538_deferred_template_test.dart
@@ -0,0 +1,54 @@
+@TestOn('vm')
+import 'package:_tests/compiler.dart';
+import 'package:test/test.dart';
+
+// See https://github.com/dart-lang/angular/issues/1538.
+void main() {
+  test('should disallow @deferred on a `<template>` element', () async {
+    await compilesExpecting("""
+      import '$ngImport';
+
+      @Component(
+        selector: 'example',
+        directives: [DeferMe],
+        template: r'''
+          <template @deferred>
+            <defer-me></defer-me>
+          </template>
+        ''',
+      )
+      class Example {}
+
+      @Component(
+        selector: 'defer-me',
+        template: '',
+      )
+      class DeferMe {}
+    """, errors: [
+      contains('Invalid @deferred annotation'),
+    ]);
+  });
+
+  test('should disallow @deferred with a structural directive', () async {
+    await compilesExpecting("""
+      import '$ngImport';
+
+      @Component(
+        selector: 'example',
+        directives: [DeferMe, NgIf],
+        template: r'''
+          <defer-me *ngIf="true" @deferred></defer-me>
+        ''',
+      )
+      class Example {}
+
+      @Component(
+        selector: 'defer-me',
+        template: '',
+      )
+      class DeferMe {}
+    """, errors: [
+      contains('Invalid @deferred annotation'),
+    ]);
+  });
+}
diff --git a/angular/CHANGELOG.md b/angular/CHANGELOG.md
index 17e8520204..9da6e5899a 100644
--- a/angular/CHANGELOG.md
+++ b/angular/CHANGELOG.md
@@ -1,3 +1,31 @@
+### Bug fixes
+
+*   [#1538]() A compile-time error is reported if the `@deferred` template
+    annotation is present on a `<template>` element or is a sibling to a
+    structural directive (such as `*ngIf`). Before we would silently drop/ignore
+    the annotation, so this might be considered a breaking change of an
+    incorrect program. The fix is just to move the annotation, such as:
+
+    ```html
+    <!-- Before (Both are identical) -->
+    <template @deferred [ngIf]="showArea>
+      <expensive-comp></expensive-comp>
+    </template>
+
+    <expensive-comp *ngIf="showArea" @deferred></expensive-comp>
+
+    <!-- After (Both are identical) -->
+    <template [ngIf]="showArea">
+      <expensive-comp @deferred></expensive-comp>
+    </template>
+
+    <ng-container *ngIf="showArea">
+      <expensive-comp @deferred></expensive-comp>
+    </ng-container>
+    ```
+
+[#1538]: https://github.com/dart-lang/angular/issues/153
+
 ## 5.0.0
 
 Welcome to AngularDart v5.0.0, with full support for Dart 2. Please note that
diff --git a/angular_ast/CHANGELOG.md b/angular_ast/CHANGELOG.md
index f3fd89c3d8..1461c93db4 100644
--- a/angular_ast/CHANGELOG.md
+++ b/angular_ast/CHANGELOG.md
@@ -1,5 +1,8 @@
 *   Annotations may now have compound names (for example `@foo.bar`).
 
+*   It is now an error to use `@deferred` on a `<template>` tag or combined with
+    a structural (i.e. `*ngIf`) directive.
+
 ## 0.5.6
 
 *   Maintenance release to support Dart 2.0 stable.
diff --git a/angular_ast/lib/src/exception_handler/exceptions.dart b/angular_ast/lib/src/exception_handler/exceptions.dart
index 2c41f8d657..d6b744de75 100644
--- a/angular_ast/lib/src/exception_handler/exceptions.dart
+++ b/angular_ast/lib/src/exception_handler/exceptions.dart
@@ -177,6 +177,17 @@ class NgParserWarningCode extends ErrorCode {
       NgParserWarningCode('INVALID_DECORATOR_IN_TEMPLATE',
           "Invalid decorator in 'template' element");
 
+  static const NgParserWarningCode INVALID_DEFERRED_ON_TEMPLATE =
+      NgParserWarningCode(
+          'INVALID_DEFERRED_ON_TEMPLATE',
+          "Invalid @deferred annotation on 'template' element",
+          "The @deferred annotation cannot be placed on an element with "
+          "a structural directive (such as *ngIf) or on a <template> tag. "
+          "Consider moving the structural directive as the parent, such as:\n\n"
+          "  <ng-container *ngIf=\"someCondition\">\n"
+          "    <expensive-comp @defered></expensive-comp>\n"
+          "  </ng-container>");
+
   static const NgParserWarningCode INVALID_LET_BINDING_IN_NONTEMPLATE =
       NgParserWarningCode('INVALID_LET_BINDING_IN_NONTEMPLATE',
           "'let-' binding can only be used in 'template' element");
diff --git a/angular_ast/lib/src/visitors/desugar_visitor.dart b/angular_ast/lib/src/visitors/desugar_visitor.dart
index 1456388d5e..efa440e3c9 100644
--- a/angular_ast/lib/src/visitors/desugar_visitor.dart
+++ b/angular_ast/lib/src/visitors/desugar_visitor.dart
@@ -84,14 +84,19 @@ class DesugarVisitor implements TemplateAstVisitor<TemplateAst, String> {
       astNode.bananas.clear();
     }
 
-    if (astNode.stars.isNotEmpty) {
-      return _desugarStar(astNode, astNode.stars);
-    }
-
     if (astNode.annotations.isNotEmpty) {
       final i = astNode.annotations.indexWhere((ast) => ast.name == 'deferred');
       if (i != -1) {
         final deferredAst = astNode.annotations.removeAt(i);
+        // Fail on invalid use (i.e. on a <template> tag):
+        // (https://github.com/dart-lang/angular/issues/1538)
+        if (astNode.stars.isNotEmpty) {
+          exceptionHandler.handle(AngularParserException(
+            NgParserWarningCode.INVALID_DEFERRED_ON_TEMPLATE,
+            deferredAst.sourceSpan.start.offset,
+            deferredAst.sourceSpan.length,
+          ));
+        }
         final origin = _toolFriendlyAstOrigin ? deferredAst : null;
         return EmbeddedTemplateAst.from(
           origin,
@@ -101,6 +106,10 @@ class DesugarVisitor implements TemplateAstVisitor<TemplateAst, String> {
       }
     }
 
+    if (astNode.stars.isNotEmpty) {
+      return _desugarStar(astNode, astNode.stars);
+    }
+
     return astNode;
   }
 
@@ -119,6 +128,19 @@ class DesugarVisitor implements TemplateAstVisitor<TemplateAst, String> {
 
   @override
   TemplateAst visitEmbeddedTemplate(EmbeddedTemplateAst astNode, [_]) {
+    if (astNode.annotations.isNotEmpty) {
+      final i = astNode.annotations.indexWhere((ast) => ast.name == 'deferred');
+      if (i != -1) {
+        final deferredAst = astNode.annotations.removeAt(i);
+        // Fail on invalid use (i.e. on a <template> tag):
+        // (https://github.com/dart-lang/angular/issues/1538)
+        exceptionHandler.handle(AngularParserException(
+          NgParserWarningCode.INVALID_DEFERRED_ON_TEMPLATE,
+          deferredAst.sourceSpan.start.offset,
+          deferredAst.sourceSpan.length,
+        ));
+      }
+    }
     _visitChildren(astNode);
     return astNode;
   }