fix(migrations): control flow migration formatting fixes (#53076)

thePunderWoman · pkozlowski-opensource · commit 00cb3339bab7 · 2023-11-28T14:03:29.000+01:00
This fix preserves leading indents in inline templates and also adds better handling for self closing tags PR Close #53076
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
@@ -112,7 +112,7 @@ function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Resu
 function buildStandardIfElseBlock(
     etm: ElementToMigrate, tmpl: string, elseString: string, offset: number): Result {
   // includes the mandatory semicolon before as
-  const condition = etm.getCondition(elseString)
+  const condition = etm.getCondition()
                         .replace(' as ', '; as ')
                         // replace 'let' with 'as' whatever spaces are between ; and 'let'
                         .replace(/;\s*let/g, '; as');
@@ -160,7 +160,7 @@ function buildStandardIfThenElseBlock(
     etm: ElementToMigrate, tmpl: string, thenString: string, elseString: string,
     offset: number): Result {
   // includes the mandatory semicolon before as
-  const condition = etm.getCondition(thenString)
+  const condition = etm.getCondition()
                         .replace(' as ', '; as ')
                         // replace 'let' with 'as' whatever spaces are between ; and 'let'
                         .replace(/;\s*let/g, '; as');
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts
@@ -23,7 +23,7 @@ export function migrateTemplate(
     format: boolean = true): {migrated: string, errors: MigrateError[]} {
   let errors: MigrateError[] = [];
   let migrated = template;
-  if (templateType === 'template') {
+  if (templateType === 'template' || templateType === 'templateUrl') {
     const ifResult = migrateIf(template);
     const forResult = migrateFor(ifResult.migrated);
     const switchResult = migrateSwitch(forResult.migrated);
@@ -32,7 +32,7 @@ export function migrateTemplate(
     const changed =
         ifResult.changed || forResult.changed || switchResult.changed || caseResult.changed;
     if (format && changed) {
-      migrated = formatTemplate(migrated);
+      migrated = formatTemplate(migrated, templateType);
     }
     file.removeCommonModule = canRemoveCommonModule(template);
 
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/types.ts b/packages/core/schematics/ng-generate/control-flow-migration/types.ts
@@ -114,18 +114,22 @@ export class ElementToMigrate {
     this.forAttrs = forAttrs;
   }
 
-  getCondition(targetStr: string): string {
-    const targetLocation = this.attr.value.indexOf(targetStr);
-    return this.attr.value.slice(0, targetLocation);
+  getCondition(): string {
+    const chunks = this.attr.value.split(';');
+    let letVar = chunks.find(c => c.search(/\s*let\s/) > -1);
+    return chunks[0] + (letVar ? ';' + letVar : '');
   }
 
   getTemplateName(targetStr: string, secondStr?: string): string {
     const targetLocation = this.attr.value.indexOf(targetStr);
     if (secondStr) {
       const secondTargetLocation = this.attr.value.indexOf(secondStr);
-      return this.attr.value.slice(targetLocation + targetStr.length, secondTargetLocation).trim();
+      return this.attr.value.slice(targetLocation + targetStr.length, secondTargetLocation)
+          .trim()
+          .split(';')[0]
+          .trim();
     }
-    return this.attr.value.slice(targetLocation + targetStr.length).trim();
+    return this.attr.value.slice(targetLocation + targetStr.length).trim().split(';')[0].trim();
   }
 
   start(offset: number): number {
@@ -175,7 +179,7 @@ export class AnalyzedFile {
   /** Returns the ranges in the order in which they should be migrated. */
   getSortedRanges(): Range[] {
     const templateRanges = this.ranges.slice()
-                               .filter(x => x.type === 'template')
+                               .filter(x => x.type !== 'import')
                                .sort((aStart, bStart) => bStart.start - aStart.start);
     const importRanges = this.ranges.slice()
                              .filter(x => x.type === 'import')
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -160,7 +160,7 @@ function analyzeDecorators(
         // Leave the end as undefined which means that the range is until the end of the file.
         if (ts.isStringLiteralLike(prop.initializer)) {
           const path = join(dirname(sourceFile.fileName), prop.initializer.text);
-          AnalyzedFile.addRange(path, analyzedFiles, {start: 0, node: prop, type: 'template'});
+          AnalyzedFile.addRange(path, analyzedFiles, {start: 0, node: prop, type: 'templateUrl'});
         }
         break;
     }
@@ -429,11 +429,16 @@ export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number
   return {start, middle, end};
 }
 
+const selfClosingList = 'input|br|img|base|wbr|area|col|embed|hr|link|meta|param|source|track';
+
 /**
  * re-indents all the lines in the template properly post migration
  */
-export function formatTemplate(tmpl: string): string {
+export function formatTemplate(tmpl: string, templateType: string): string {
   if (tmpl.indexOf('\n') > -1) {
+    // tracks if a self closing element opened without closing yet
+    let openSelfClosingEl = false;
+
     // match any type of control flow block as start of string ignoring whitespace
     // @if | @switch | @case | @default | @for | } @else
     const openBlockRegex = /^\s*\@(if|switch|case|default|for)|^\s*\}\s\@else/;
@@ -442,45 +447,93 @@ export function formatTemplate(tmpl: string): string {
     // <div thing="stuff" [binding]="true"> || <div thing="stuff" [binding]="true"
     const openElRegex = /^\s*<([a-z0-9]+)(?![^>]*\/>)[^>]*>?/;
 
+    // regex for matching a self closing html element that has no />
+    // <div thing="stuff" [binding]="true"> || <div thing="stuff" [binding]="true"
+    const selfClosingRegex = new RegExp(`^\\s*<(${selfClosingList}).+\\/?>`);
+
+    // regex for matching a self closing html element that is on multi lines
+    // <div thing="stuff" [binding]="true"> || <div thing="stuff" [binding]="true"
+    const openSelfClosingRegex = new RegExp(`^\\s*<(${selfClosingList})(?![^>]*\\/>)[^>]*$`);
+
     // match closing block or else block
     // } | } @else
     const closeBlockRegex = /^\s*\}\s*$|^\s*\}\s\@else/;
 
     // matches closing of an html element
     // </element>
-    const closeElRegex = /\s*<\/([a-z0-9\-]+)*>/;
+    const closeElRegex = /\s*<\/([a-zA-Z0-9\-]+)*>/;
 
     // matches closing of a self closing html element when the element is on multiple lines
     // [binding]="value" />
-    const closeMultiLineElRegex = /^\s*([a-z0-9\-\[\]]+)?=?"?([^”<]+)?"?\s?\/>$/;
+    const closeMultiLineElRegex = /^\s*([a-zA-Z0-9\-\[\]]+)?=?"?([^”<]+)?"?\s?\/>$/;
+
+    // matches closing of a self closing html element when the element is on multiple lines
+    // with no / in the closing: [binding]="value">
+    const closeSelfClosingMultiLineRegex = /^\s*([a-zA-Z0-9\-\[\]]+)?=?"?([^”\/<]+)?"?\s?>$/;
 
     // matches an open and close of an html element on a single line with no breaks
     // <div>blah</div>
-    const singleLineElRegex = /^\s*<([a-z0-9]+)(?![^>]*\/>)[^>]*>.*<\/([a-z0-9\-]+)*>/;
+    const singleLineElRegex = /\s*<([a-zA-Z0-9]+)(?![^>]*\/>)[^>]*>.*<\/([a-zA-Z0-9\-]+)*>/;
     const lines = tmpl.split('\n');
     const formatted = [];
+    // the indent applied during formatting
     let indent = '';
+    // the pre-existing indent in an inline template that we'd like to preserve
+    let mindent = '';
     for (let [index, line] of lines.entries()) {
       if (line.trim() === '' && index !== 0 && index !== lines.length - 1) {
         // skip blank lines except if it's the first line or last line
+        // this preserves leading and trailing spaces if they are already present
         continue;
       }
+      // preserves the indentation of an inline template
+      if (templateType === 'template' && index <= 1) {
+        // first real line of an inline template
+        const ind = line.search(/\S/);
+        mindent = (ind > -1) ? line.slice(0, ind) : '';
+      }
+
+      // if a block closes, an element closes, and it's not an element on a single line or the end
+      // of a self closing tag
       if ((closeBlockRegex.test(line) ||
            (closeElRegex.test(line) &&
             (!singleLineElRegex.test(line) && !closeMultiLineElRegex.test(line)))) &&
           indent !== '') {
         // close block, reduce indent
         indent = indent.slice(2);
       }
-      formatted.push(indent + line.trim());
+
+      formatted.push(mindent + indent + line.trim());
+
+      // this matches any self closing element that actually has a />
       if (closeMultiLineElRegex.test(line)) {
         // multi line self closing tag
         indent = indent.slice(2);
+        if (openSelfClosingEl) {
+          openSelfClosingEl = false;
+        }
+      }
+
+      // this matches a self closing element that doesn't have a / in the >
+      if (closeSelfClosingMultiLineRegex.test(line) && openSelfClosingEl) {
+        openSelfClosingEl = false;
+        indent = indent.slice(2);
       }
-      if ((openBlockRegex.test(line) || openElRegex.test(line)) && !singleLineElRegex.test(line)) {
+
+      // this matches an open control flow block, an open HTML element, but excludes single line
+      // self closing tags
+      if ((openBlockRegex.test(line) || openElRegex.test(line)) && !singleLineElRegex.test(line) &&
+          !selfClosingRegex.test(line) && !openSelfClosingRegex.test(line)) {
         // open block, increase indent
         indent += '  ';
       }
+
+      // This is a self closing element that is definitely not fully closed and is on multiple lines
+      if (openSelfClosingRegex.test(line)) {
+        openSelfClosingEl = true;
+        // add to the indent for the properties on it to look nice
+        indent += '  ';
+      }
     }
     tmpl = formatted.join('\n');
   }
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
