fix(migrations): avoid conflicts with some greek letters in control flow migration (#55113)

crisbeto · atscott · commit 949dec26b860 · 2024-03-29T13:10:55.000-07:00
The control flow migration was using a couple of Greek letters as placeholders. This ended up conflicting with templates authored in Greek. These changes use a more obscure placeholder to make conflicts less likely. It also moves the placeholder generation to a centralized function so it's easier to make changes if we decide to update the pattern again. Fixes #55085. PR Close #55113
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/fors.ts b/packages/core/schematics/ng-generate/control-flow-migration/fors.ts
@@ -9,7 +9,7 @@
 import {visitAll} from '@angular/compiler';
 
 import {ElementCollector, ElementToMigrate, endMarker, MigrateError, Result, startMarker} from './types';
-import {calculateNesting, getMainBlock, getOriginals, hasLineBreaks, parseTemplate, reduceNestingOffset} from './util';
+import {calculateNesting, getMainBlock, getOriginals, getPlaceholder, hasLineBreaks, parseTemplate, PlaceholderKind, reduceNestingOffset} from './util';
 
 export const ngfor = '*ngFor';
 export const nakedngfor = 'ngFor';
@@ -112,9 +112,8 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe
     }
     // template
     if (part.startsWith('template:')) {
-      // this generates a special template placeholder just for this use case
-      // which has a φ at the end instead of the standard δ in other placeholders
-      tmplPlaceholder = `θ${part.split(':')[1].trim()}φ`;
+      // use an alternate placeholder here to avoid conflicts
+      tmplPlaceholder = getPlaceholder(part.split(':')[1].trim(), PlaceholderKind.Alternate);
     }
     // aliases
     // declared with `let myIndex = index`
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
@@ -9,7 +9,7 @@
 import {visitAll} from '@angular/compiler';
 
 import {ElementCollector, ElementToMigrate, endMarker, MigrateError, Result, startMarker} from './types';
-import {calculateNesting, getMainBlock, getOriginals, hasLineBreaks, parseTemplate, reduceNestingOffset} from './util';
+import {calculateNesting, getMainBlock, getOriginals, getPlaceholder, hasLineBreaks, parseTemplate, PlaceholderKind, reduceNestingOffset} from './util';
 
 export const ngif = '*ngIf';
 export const boundngif = '[ngIf]';
@@ -133,7 +133,7 @@ function buildStandardIfElseBlock(
                         .replace(' as ', '; as ')
                         // replace 'let' with 'as' whatever spaces are between ; and 'let'
                         .replace(/;\s*let/g, '; as');
-  const elsePlaceholder = `θ${etm.getTemplateName(elseString)}δ`;
+  const elsePlaceholder = getPlaceholder(etm.getTemplateName(elseString));
   return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
 }
 
@@ -153,9 +153,9 @@ function buildBoundIfElseBlock(etm: ElementToMigrate, tmpl: string, offset: numb
   } else if (aliases.length === 1) {
     condition += `; as ${aliases[0]}`;
   }
-  const elsePlaceholder = `θ${etm.elseAttr!.value.trim()}δ`;
+  const elsePlaceholder = getPlaceholder(etm.elseAttr!.value.trim());
   if (etm.thenAttr !== undefined) {
-    const thenPlaceholder = `θ${etm.thenAttr!.value.trim()}δ`;
+    const thenPlaceholder = getPlaceholder(etm.thenAttr!.value.trim());
     return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
   }
   return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
@@ -194,8 +194,8 @@ function buildStandardIfThenElseBlock(
                         .replace(' as ', '; as ')
                         // replace 'let' with 'as' whatever spaces are between ; and 'let'
                         .replace(/;\s*let/g, '; as');
-  const thenPlaceholder = `θ${etm.getTemplateName(thenString, elseString)}δ`;
-  const elsePlaceholder = `θ${etm.getTemplateName(elseString)}δ`;
+  const thenPlaceholder = getPlaceholder(etm.getTemplateName(thenString, elseString));
+  const elsePlaceholder = getPlaceholder(etm.getTemplateName(elseString));
   return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
 }
 
@@ -206,7 +206,7 @@ function buildStandardIfThenBlock(
                         .replace(' as ', '; as ')
                         // replace 'let' with 'as' whatever spaces are between ; and 'let'
                         .replace(/;\s*let/g, '; as');
-  const thenPlaceholder = `θ${etm.getTemplateName(thenString)}δ`;
+  const thenPlaceholder = getPlaceholder(etm.getTemplateName(thenString));
   return buildIfThenBlock(etm, tmpl, condition, thenPlaceholder, offset);
 }
 
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -283,6 +283,21 @@ function isChildOf(parent: Element, el: Element): boolean {
       parent.sourceSpan.end.offset > el.sourceSpan.end.offset;
 }
 
+/** Possible placeholders that can be generated by `getPlaceholder`. */
+export enum PlaceholderKind {
+  Default,
+  Alternate,
+}
+
+/**
+ * Wraps a string in a placeholder that makes it easier to identify during replacement operations.
+ */
+export function getPlaceholder(
+    value: string, kind: PlaceholderKind = PlaceholderKind.Default): string {
+  const name = `<<<ɵɵngControlFlowMigration_${kind}ɵɵ>>>`;
+  return `___${name}${value}${name}___`;
+}
+
 /**
  * calculates the level of nesting of the items in the collector
  */
@@ -387,8 +402,8 @@ export function processNgTemplates(template: string): {migrated: string, err: Er
 
     // swap placeholders and remove
     for (const [name, t] of templates) {
-      const replaceRegex = new RegExp(`θ${name.slice(1)}\\δ`, 'g');
-      const forRegex = new RegExp(`θ${name.slice(1)}\\φ`, 'g');
+      const replaceRegex = new RegExp(getPlaceholder(name.slice(1)), 'g');
+      const forRegex = new RegExp(getPlaceholder(name.slice(1), PlaceholderKind.Alternate), 'g');
       const forMatches = [...template.matchAll(forRegex)];
       const matches = [...forMatches, ...template.matchAll(replaceRegex)];
       let safeToRemove = true;
@@ -429,11 +444,15 @@ export function processNgTemplates(template: string): {migrated: string, err: Er
 }
 
 function replaceRemainingPlaceholders(template: string): string {
-  const replaceRegex = new RegExp(`θ.*δ`, 'g');
+  const pattern = '.*';
+  const placeholderPattern = getPlaceholder(pattern);
+  const replaceRegex = new RegExp(placeholderPattern, 'g');
+  const [placeholderStart, placeholderEnd] = placeholderPattern.split(pattern);
   const placeholders = [...template.matchAll(replaceRegex)];
   for (let ph of placeholders) {
     const placeholder = ph[0];
-    const name = placeholder.slice(1, placeholder.length - 1);
+    const name =
+        placeholder.slice(placeholderStart.length, placeholder.length - placeholderEnd.length);
     template =
         template.replace(placeholder, `<ng-template [ngTemplateOutlet]="${name}"></ng-template>`);
   }
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -4364,6 +4364,30 @@ describe('control flow migration', () => {
         `}`,
       ].join('\n'));
     });
+
+    it('should migrate a template using the θδ characters', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgIf} from '@angular/common';
+
+        @Component({
+          selector: 'declare-comp',
+          templateUrl: './comp.html',
+          standalone: true,
+          imports: [NgIf],
+        })
+        class DeclareComp {
+          show = false;
+        }
+      `);
+
+      writeFile('/comp.html', `<div *ngIf="show">Some greek characters: θδ!</div>`);
+
+      await runMigration();
+
+      expect(tree.readContent('/comp.html'))
+          .toBe('@if (show) {<div>Some greek characters: θδ!</div>}');
+    });
   });
 
   describe('formatting', () => {
