refactor(core): polish failure messages for template-var-assignment schematic (#29708)

Improves the failure messages for the `template-var-assignment` schematic. After manual
testing of the schematic it's not quite clear for developers what the failure message means
without any context. The schematic now also references a short markdown file mentioning
what needs to be changed, but eventually this document needs to be expanded with more
information and context of the reasoning behind this change within Ivy.

PR Close #29708

packages/core/schematics/migrations/template-var-assignment/README.md

@@ -0,0 +1,27 @@
+## Assignments to template variables
+
+With Ivy, assignments to template variables are no longer supported
+as template variables are effectively constants.
+
+This means that assignments to template variables will break your
+application once Ivy is enabled by default. For example:
+
+```html
+<button *ngFor="let option of options"
+       (click)="option = 'newButtonText'">
+  {{ option }}
+</button>
+```
+
+In the example from above, a value is assigned to the `option`
+template variable on `click`. This will ultimately break your
+application and therefore the logic needs to be adjusted to not
+update the `option` variable, but rather the given element in
+the `options` array:
+
+```html
+<button *ngFor="let option of options; let idx = index"
+       (click)="options[idx] = 'newButtonText'">
+  {{ option }}
+</button>
+```

packages/core/schematics/migrations/template-var-assignment/index.ts

@@ -19,6 +19,10 @@ import {NgComponentTemplateVisitor} from './angular/ng_component_template';
 
 type Logger = logging.LoggerApi;
 
+const README_URL =
+    'https://github.com/angular/angular/tree/master/packages/core/schematics/migrations/template-var-assignment/README.md';
+const FAILURE_MESSAGE = `Found assignment to template variable.`;
+
 /** Entry point for the V8 template variable assignment schematic. */
 export default function(): Rule {
   return (tree: Tree, context: SchematicContext) => {
@@ -62,6 +66,7 @@ function runTemplateVariableAssignmentCheck(
   rootSourceFiles.forEach(sourceFile => templateVisitor.visitNode(sourceFile));
 
   const {resolvedTemplates} = templateVisitor;
+  const collectedFailures: string[] = [];
 
   // Analyze each resolved template and print a warning for property writes to
   // template variables.
@@ -76,9 +81,18 @@ function runTemplateVariableAssignmentCheck(
 
     nodes.forEach(n => {
       const {line, character} = template.getCharacterAndLineOfPosition(n.start);
-      logger.warn(
-          `${displayFilePath}@${line + 1}:${character + 1}: Found assignment to template ` +
-          `variable. This does not work with Ivy and needs to be updated.`);
+      collectedFailures.push(`${displayFilePath}@${line + 1}:${character + 1}: ${FAILURE_MESSAGE}`);
     });
   });
+
+  if (collectedFailures.length) {
+    logger.info('---- Template Variable Assignment schematic ----');
+    logger.info('Assignments to template variables will no longer work with Ivy as');
+    logger.info('template variables are effectively constants in Ivy. Read more about');
+    logger.info(`this change here: ${README_URL}`);
+    logger.info('');
+    logger.info('The following template assignments were found:');
+    collectedFailures.forEach(failure => logger.warn(`⮑   ${failure}`));
+    logger.info('------------------------------------------------');
+  }
 }

packages/core/schematics/test/template_var_assignment_migration_spec.ts

@@ -18,7 +18,7 @@ describe('template variable assignment migration', () => {
   let tree: UnitTestTree;
   let tmpDirPath: string;
   let previousWorkingDir: string;
-  let consoleOutput: string[];
+  let warnOutput: string[];
 
   beforeEach(() => {
     runner = new SchematicTestRunner('test', require.resolve('../migrations.json'));
@@ -31,8 +31,12 @@ describe('template variable assignment migration', () => {
       }
     }));
 
-    consoleOutput = [];
-    runner.logger.subscribe(logEntry => consoleOutput.push(logEntry.message));
+    warnOutput = [];
+    runner.logger.subscribe(logEntry => {
+      if (logEntry.level === 'warn') {
+        warnOutput.push(logEntry.message);
+      }
+    });
 
     previousWorkingDir = shx.pwd();
     tmpDirPath = getSystemPath(host.root);
@@ -67,8 +71,8 @@ describe('template variable assignment migration', () => {
 
     runMigration();
 
-    expect(consoleOutput.length).toBe(1);
-    expect(consoleOutput[0]).toMatch(/^index.ts@5:69: Found assignment/);
+    expect(warnOutput.length).toBe(1);
+    expect(warnOutput[0]).toMatch(/^⮑ {3}index.ts@5:69: Found assignment/);
   });
 
   it('should warn for two-way data binding assigning to "as" variable', () => {
@@ -89,8 +93,8 @@ describe('template variable assignment migration', () => {
 
     runMigration();
 
-    expect(consoleOutput.length).toBe(1);
-    expect(consoleOutput).toMatch(/^tmpl.html@3:31: Found assignment/);
+    expect(warnOutput.length).toBe(1);
+    expect(warnOutput).toMatch(/^⮑ {3}tmpl.html@3:31: Found assignment/);
   });
 
   it('should warn for bound event assignments to "as" variable', () => {
@@ -112,9 +116,9 @@ describe('template variable assignment migration', () => {
 
     runMigration();
 
-    expect(consoleOutput.length).toBe(2);
-    expect(consoleOutput[0]).toMatch(/^sub_dir\/tmpl.html@3:25: Found assignment/);
-    expect(consoleOutput[1]).toMatch(/^sub_dir\/tmpl.html@4:25: Found assignment/);
+    expect(warnOutput.length).toBe(2);
+    expect(warnOutput[0]).toMatch(/^⮑ {3}sub_dir\/tmpl.html@3:25: Found assignment/);
+    expect(warnOutput[1]).toMatch(/^⮑ {3}sub_dir\/tmpl.html@4:25: Found assignment/);
   });
 
   it('should warn for bound event assignments to template "let" variables', () => {
@@ -136,9 +140,9 @@ describe('template variable assignment migration', () => {
 
     runMigration();
 
-    expect(consoleOutput.length).toBe(2);
-    expect(consoleOutput[0]).toMatch(/^sub_dir\/tmpl.html@3:25: Found assignment/);
-    expect(consoleOutput[1]).toMatch(/^sub_dir\/tmpl.html@4:25: Found assignment/);
+    expect(warnOutput.length).toBe(2);
+    expect(warnOutput[0]).toMatch(/^⮑ {3}sub_dir\/tmpl.html@3:25: Found assignment/);
+    expect(warnOutput[1]).toMatch(/^⮑ {3}sub_dir\/tmpl.html@4:25: Found assignment/);
   });
 
   it('should not warn for bound event assignments to component property', () => {
@@ -155,7 +159,8 @@ describe('template variable assignment migration', () => {
 
     runMigration();
 
-    expect(consoleOutput.length).toBe(0);
+    expect(warnOutput.length).toBe(0);
+  });
   });
 
   it('should not throw an error if a detected template fails parsing', () => {
@@ -172,6 +177,6 @@ describe('template variable assignment migration', () => {
 
     runMigration();
 
-    expect(consoleOutput.length).toBe(0);
+    expect(warnOutput.length).toBe(0);
   });
 });