fix(migrations): handle comma-separated syntax in ngFor (#52525)

This is something that came up when running the script against the Components repo. The `ngFor` syntax can be delimited either by semicolons or by commas, but the migration only accounted for commas.

PR Close #52525

packages/core/schematics/ng-generate/control-flow-migration/types.ts

@@ -18,6 +18,11 @@ export const switchcase = '*ngSwitchCase';
 export const nakedcase = 'ngSwitchCase';
 export const switchdefault = '*ngSwitchDefault';
 export const nakeddefault = 'ngSwitchDefault';
+export const commaSeparatedSyntax = new Map([
+  ['(', ')'],
+  ['{', '}'],
+  ['[', ']'],
+]);
 
 const attributesToMigrate = [
   ngif,

packages/core/schematics/ng-generate/control-flow-migration/util.ts

@@ -6,11 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {HtmlParser, Node, ParseTreeResult, visitAll} from '@angular/compiler';
+import {HtmlParser, ParseTreeResult, visitAll} from '@angular/compiler';
 import {dirname, join} from 'path';
 import ts from 'typescript';
 
-import {AnalyzedFile, boundcase, boundngif, ElementCollector, ElementToMigrate, MigrateError, nakedcase, nakeddefault, nakedngif, ngfor, ngif, ngswitch, Result, switchcase, switchdefault, Template} from './types';
+import {AnalyzedFile, boundcase, boundngif, ElementCollector, ElementToMigrate, MigrateError, nakedcase, nakeddefault, nakedngif, ngfor, ngif, ngswitch, Result, switchcase, switchdefault, Template, commaSeparatedSyntax} from './types';
 
 /**
  * Analyzes a source file to find file that need to be migrated and the text ranges within them.
@@ -320,7 +320,7 @@ function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Resu
   const aliases = [];
   const lbString = etm.hasLineBreaks ? lb : '';
   const lbSpaces = etm.hasLineBreaks ? `${lb}  ` : '';
-  const parts = etm.attr.value.split(';');
+  const parts = getNgForParts(etm.attr.value);
 
   const originals = getOriginals(etm, tmpl, offset);
 
@@ -384,6 +384,37 @@ function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Resu
   return {tmpl: updatedTmpl, offsets: {pre, post}};
 }
 
+function getNgForParts(expression: string): string[] {
+  const parts: string[] = [];
+  const syntaxStack: string[] = [];
+  let current = '';
+
+  for (let i = 0; i < expression.length; i++) {
+    const char = expression[i];
+
+    // Any semicolon is a delimiter, as well as any comma outside of comma-separated syntax.
+    if (current.length > 0 && (char === ';' || (char === ',' && syntaxStack.length === 0))) {
+      parts.push(current);
+      current = '';
+      continue;
+    }
+
+    if (commaSeparatedSyntax.has(char)) {
+      syntaxStack.push(commaSeparatedSyntax.get(char)!);
+    } else if (syntaxStack.length > 0 && syntaxStack[syntaxStack.length - 1] === char) {
+      syntaxStack.pop();
+    }
+
+    current += char;
+  }
+
+  if (current.length > 0) {
+    parts.push(current);
+  }
+
+  return parts;
+}
+
 function getOriginals(
     etm: ElementToMigrate, tmpl: string, offset: number): {start: string, end: string} {
   // original opening block

packages/core/schematics/test/control_flow_migration_spec.ts

@@ -963,6 +963,50 @@ describe('control flow migration', () => {
           'template: `<ul>@for (itm of items; track itm; let index = $index) {<li>{{itm.text}}</li>}</ul>`');
     });
 
+    it('should migrate with a comma-separated index alias', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgFor} from '@angular/common';
+        interface Item {
+          id: number;
+          text: string;
+        }
+
+        @Component({
+          imports: [NgFor],
+          template: \`<ul><li *ngFor="let itm of items, let index = index">{{itm.text}}</li></ul>\`
+        })
+        class Comp {
+          items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
+        }
+      `);
+
+      await runMigration();
+      const content = tree.readContent('/comp.ts');
+
+      expect(content).toContain(
+          'template: `<ul>@for (itm of items; track itm; let index = $index) {<li>{{itm.text}}</li>}</ul>`');
+    });
+
+    it('should migrate an index alias after an expression containing commas', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgFor} from '@angular/common';
+
+        @Component({
+          imports: [NgFor],
+          template: \`<ul><li *ngFor="let itm of foo({a: 1, b: 2}, [1, 2]), let index = index">{{itm.text}}</li></ul>\`
+        })
+        class Comp {}
+      `);
+
+      await runMigration();
+      const content = tree.readContent('/comp.ts');
+
+      expect(content).toContain(
+          'template: `<ul>@for (itm of foo({a: 1, b: 2}, [1, 2]); track itm; let index = $index) {<li>{{itm.text}}</li>}</ul>`');
+    });
+
     it('should migrate with an index alias with no spaces', async () => {
       writeFile('/comp.ts', `
         import {Component} from '@angular/core';
@@ -1013,6 +1057,31 @@ describe('control flow migration', () => {
           'template: `<ul>@for (itm of items; track itm; let myIndex = $index) {<li>{{itm.text}}</li>}</ul>`');
     });
 
+    it('should migrate with alias declared with a comma-separated `as` expression', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgFor} from '@angular/common';
+        interface Item {
+          id: number;
+          text: string;
+        }
+
+        @Component({
+          imports: [NgFor],
+          template: \`<ul><li *ngFor="let itm of items, index as myIndex">{{itm.text}}</li></ul>\`
+        })
+        class Comp {
+          items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
+        }
+      `);
+
+      await runMigration();
+      const content = tree.readContent('/comp.ts');
+
+      expect(content).toContain(
+          'template: `<ul>@for (itm of items; track itm; let myIndex = $index) {<li>{{itm.text}}</li>}</ul>`');
+    });
+
     it('should migrate with a trackBy function and an aliased index', async () => {
       writeFile('/comp.ts', `
         import {Component} from '@angular/core';