Skip to content

Commit

Permalink
fix(migrations): Add support for nested structures inside a switch st…
Browse files Browse the repository at this point in the history 
…atement

This updates the code to handle switches more elegantly in line with how the other blocks are handled. This allows nesting to be handled just like other blocks.
  • Loading branch information
@thePunderWoman
thePunderWoman committed Oct 24, 2023
1 parent 739f1fc commit bbf7973
Show file tree
Hide file tree
Showing 3 changed files with 204 additions and 150 deletions.
36 changes: 11 additions & 25 deletions packages/core/schematics/ng-generate/control-flow-migration/types.ts
View file
Expand Up @@ -13,21 +13,23 @@ export const boundngif = '[ngIf]';
export const nakedngif = 'ngIf';
export const ngfor = '*ngFor';
export const ngswitch = '[ngSwitch]';
export const boundcase = '[ngSwitchCase]';
export const switchcase = '*ngSwitchCase';
export const nakedcase = 'ngSwitchCase';
export const switchdefault = '*ngSwitchDefault';
export const nakeddefault = 'ngSwitchDefault';

const attributesToMigrate = [
ngif,
nakedngif,
boundngif,
ngfor,
ngswitch,
];

const casesToMigrate = [
'[ngSwitchCase]',
'*ngSwitchCase',
'ngSwitchCase',
'*ngSwitchDefault',
'ngSwitchDefault',
boundcase,
switchcase,
nakedcase,
switchdefault,
nakeddefault,
];

/**
Expand Down Expand Up @@ -61,7 +63,7 @@ export class ElementToMigrate {
el: Element;
attr: Attribute;
nestCount = 0;
lineBreaks = false;
hasLineBreaks = false;

constructor(el: Element, attr: Attribute) {
this.el = el;
Expand Down Expand Up @@ -173,19 +175,3 @@ export class ElementCollector extends RecursiveVisitor {
super.visitElement(el, null);
}
}

export class CaseCollector extends RecursiveVisitor {
readonly elements: ElementToMigrate[] = [];

override visitElement(el: Element): void {
if (el.attrs.length > 0) {
for (const attr of el.attrs) {
if (casesToMigrate.includes(attr.name)) {
this.elements.push(new ElementToMigrate(el, attr));
}
}
}

super.visitElement(el, null);
}
}
179 changes: 96 additions & 83 deletions packages/core/schematics/ng-generate/control-flow-migration/util.ts
View file
Expand Up @@ -10,7 +10,7 @@ import {HtmlParser, Node, ParseTreeResult, visitAll} from '@angular/compiler';
import {dirname, join} from 'path';
import ts from 'typescript';

import {AnalyzedFile, boundngif, CaseCollector, ElementCollector, ElementToMigrate, MigrateError, nakedngif, ngfor, ngif, ngswitch, Result, Template} from './types';
import {AnalyzedFile, boundcase, boundngif, ElementCollector, ElementToMigrate, MigrateError, nakedcase, nakeddefault, nakedngif, ngfor, ngif, ngswitch, Result, switchcase, switchdefault, Template} from './types';

/**
* Analyzes a source file to find file that need to be migrated and the text ranges within them.
Expand Down Expand Up @@ -138,10 +138,12 @@ export function migrateTemplate(template: string): {migrated: string|null, error

// start from top of template
// loop through each element
visitor.elements[0].hasLineBreaks = hasLineBreaks;
let prevElEnd = visitor.elements[0]?.el.sourceSpan.end.offset ?? result.length - 1;
let nestedQueue: number[] = [prevElEnd];
for (let i = 1; i < visitor.elements.length; i++) {
let currEl = visitor.elements[i];
currEl.hasLineBreaks = hasLineBreaks;
currEl.nestCount = getNestedCount(currEl, nestedQueue);
if (currEl.el.sourceSpan.end.offset !== nestedQueue[nestedQueue.length - 1]) {
nestedQueue.push(currEl.el.sourceSpan.end.offset);
Expand Down Expand Up @@ -169,19 +171,32 @@ export function migrateTemplate(template: string): {migrated: string|null, error
// these are all migratable nodes
if (el.attr.name === ngif || el.attr.name === nakedngif || el.attr.name === boundngif) {
try {
migrateResult = migrateNgIf(el, visitor.templates, result, offset, hasLineBreaks);
migrateResult = migrateNgIf(el, visitor.templates, result, offset);
} catch (error: unknown) {
errors.push({type: ngif, error});
}
} else if (el.attr.name === ngfor) {
try {
migrateResult = migrateNgFor(el, result, offset, hasLineBreaks);
migrateResult = migrateNgFor(el, result, offset);
} catch (error: unknown) {
errors.push({type: ngfor, error});
}
} else if (el.attr.name === ngswitch) {
try {
migrateResult = migrateNgSwitch(el, result, offset, hasLineBreaks);
migrateResult = migrateNgSwitch(el, result, offset);
} catch (error: unknown) {
errors.push({type: ngswitch, error});
}
} else if (
el.attr.name === switchcase || el.attr.name === nakedcase || el.attr.name === boundcase) {
try {
migrateResult = migrateNgSwitchCase(el, result, offset);
} catch (error: unknown) {
errors.push({type: ngswitch, error});
}
} else if (el.attr.name === switchdefault || el.attr.name === nakeddefault) {
try {
migrateResult = migrateNgSwitchDefault(el, result, offset);
} catch (error: unknown) {
errors.push({type: ngswitch, error});
}
Expand All @@ -202,26 +217,24 @@ export function migrateTemplate(template: string): {migrated: string|null, error
}

function migrateNgIf(
etm: ElementToMigrate, ngTemplates: Map<string, Template>, tmpl: string, offset: number,
hasLineBreaks: boolean): Result {
etm: ElementToMigrate, ngTemplates: Map<string, Template>, tmpl: string,
offset: number): Result {
const matchThen = etm.attr.value.match(/;\s+then/gm);
const matchElse = etm.attr.value.match(/;\s+else/gm);

if (matchThen && matchThen.length > 0) {
return buildIfThenElseBlock(
etm, ngTemplates, tmpl, matchThen[0], matchElse![0], offset, hasLineBreaks);
return buildIfThenElseBlock(etm, ngTemplates, tmpl, matchThen[0], matchElse![0], offset);
} else if (matchElse && matchElse.length > 0) {
// just else
return buildIfElseBlock(etm, ngTemplates, tmpl, matchElse[0], offset, hasLineBreaks);
return buildIfElseBlock(etm, ngTemplates, tmpl, matchElse[0], offset);
}

return buildIfBlock(etm, tmpl, offset, hasLineBreaks);
return buildIfBlock(etm, tmpl, offset);
}

function buildIfBlock(
etm: ElementToMigrate, tmpl: string, offset: number, hasLineBreaks: boolean): Result {
function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result {
// includes the mandatory semicolon before as
const lbString = hasLineBreaks ? lb : '';
const lbString = etm.hasLineBreaks ? lb : '';
const condition = etm.attr.value.replace(' as ', '; as ');

const originals = getOriginals(etm, tmpl, offset);
Expand All @@ -243,9 +256,9 @@ function buildIfBlock(

function buildIfElseBlock(
etm: ElementToMigrate, ngTemplates: Map<string, Template>, tmpl: string, elseString: string,
offset: number, hasLineBreaks: boolean): Result {
offset: number): Result {
// includes the mandatory semicolon before as
const lbString = hasLineBreaks ? lb : '';
const lbString = etm.hasLineBreaks ? lb : '';
const condition = etm.getCondition(elseString).replace(' as ', '; as ');

const originals = getOriginals(etm, tmpl, offset);
Expand Down Expand Up @@ -274,9 +287,9 @@ function buildIfElseBlock(

function buildIfThenElseBlock(
etm: ElementToMigrate, ngTemplates: Map<string, Template>, tmpl: string, thenString: string,
elseString: string, offset: number, hasLineBreaks: boolean): Result {
elseString: string, offset: number): Result {
const condition = etm.getCondition(thenString).replace(' as ', '; as ');
const lbString = hasLineBreaks ? lb : '';
const lbString = etm.hasLineBreaks ? lb : '';

const originals = getOriginals(etm, tmpl, offset);

Expand Down Expand Up @@ -306,13 +319,12 @@ function buildIfThenElseBlock(
return {tmpl: updatedTmpl, offsets: {pre, post}};
}

function migrateNgFor(
etm: ElementToMigrate, tmpl: string, offset: number, hasLineBreaks: boolean): Result {
function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Result {
const aliasWithEqualRegexp = /=\s+(count|index|first|last|even|odd)/gm;
const aliasWithAsRegexp = /(count|index|first|last|even|odd)\s+as/gm;
const aliases = [];
const lbString = hasLineBreaks ? lb : '';
const lbSpaces = hasLineBreaks ? `${lb} ` : '';
const lbString = etm.hasLineBreaks ? lb : '';
const lbSpaces = etm.hasLineBreaks ? `${lb} ` : '';
const parts = etm.attr.value.split(';');

const originals = getOriginals(etm, tmpl, offset);
Expand Down Expand Up @@ -384,7 +396,8 @@ function getOriginals(

function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number):
{start: string, middle: string, end: string} {
if (etm.el.name === 'ng-container' && etm.el.attrs.length === 1) {
if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') &&
etm.el.attrs.length === 1) {
// this is the case where we're migrating and there's no need to keep the ng-container
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
Expand All @@ -393,7 +406,10 @@ function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number):
}

const attrStart = etm.attr.keySpan!.start.offset - 1 - offset;
const valEnd = etm.attr.valueSpan!.end.offset + 1 - offset;
const valEnd =
(etm.attr.valueSpan ? (etm.attr.valueSpan.end.offset + 1) : etm.attr.keySpan!.end.offset) -
offset;

let childStart = valEnd;
let childEnd = valEnd;

Expand All @@ -404,81 +420,78 @@ function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number):

let start = tmpl.slice(etm.start(offset), attrStart);
start += tmpl.slice(valEnd, childStart);

const middle = tmpl.slice(childStart, childEnd);
const end = tmpl.slice(childEnd, etm.end(offset));

return {start, middle, end};
}

function migrateNgSwitch(
etm: ElementToMigrate, tmpl: string, offset: number, hasLineBreaks: boolean): Result {
function migrateNgSwitch(etm: ElementToMigrate, tmpl: string, offset: number): Result {
const lbString = etm.hasLineBreaks ? lb : '';
const condition = etm.attr.value;
const startBlock = `@switch (${condition}) {`;
const lbString = hasLineBreaks ? lb : '';

const {openTag, closeTag, children} = getSwitchBlockElements(etm, tmpl, offset);
const cases = getSwitchCases(children, tmpl, offset, hasLineBreaks);
const switchBlock = openTag + startBlock + cases.join('') + `${lbString}}` + closeTag;
const originals = getOriginals(etm, tmpl, offset);

const {start, middle, end} = getMainBlock(etm, tmpl, offset);
const startBlock = `${start}${lbString}@switch (${condition}) {`;
const endBlock = `}${lbString}${end}`;

const switchBlock = startBlock + middle + endBlock;
const updatedTmpl = tmpl.slice(0, etm.start(offset)) + switchBlock + tmpl.slice(etm.end(offset));
const pre = etm.length() - switchBlock.length;

return {tmpl: updatedTmpl, offsets: {pre, post: 0}};
}
// this should be the difference between the starting element up to the start of the closing
// element and the mainblock sans }
const pre = originals.start.length - startBlock.length;
const post = originals.end.length - endBlock.length;

function getSwitchBlockElements(etm: ElementToMigrate, tmpl: string, offset: number) {
const attrStart = etm.attr.keySpan!.start.offset - 1 - offset;
const valEnd = etm.attr.valueSpan!.end.offset + 1 - offset;
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
let openTag = (etm.el.name === 'ng-container') ?
'' :
tmpl.slice(etm.start(offset), attrStart) + tmpl.slice(valEnd, childStart);
if (tmpl.slice(childStart, childStart + 1) === lb) {
openTag += lb;
}
let closeTag = (etm.el.name === 'ng-container') ? '' : tmpl.slice(childEnd, etm.end(offset));
if (tmpl.slice(childEnd - 1, childEnd) === lb) {
closeTag = lb + closeTag;
}
return {
openTag,
closeTag,
children: etm.el.children,
};
return {tmpl: updatedTmpl, offsets: {pre, post}};
}

function getSwitchCases(children: Node[], tmpl: string, offset: number, hasLineBreaks: boolean) {
const collector = new CaseCollector();
visitAll(collector, children);
return collector.elements.map(etm => getSwitchCaseBlock(etm, tmpl, offset, hasLineBreaks));
function migrateNgSwitchCase(etm: ElementToMigrate, tmpl: string, offset: number): Result {
// includes the mandatory semicolon before as
const lbString = etm.hasLineBreaks ? lb : '';
const lbSpaces = etm.hasLineBreaks ? ' ' : '';
const leadingSpace = etm.hasLineBreaks ? '' : ' ';
const condition = etm.attr.value;

const originals = getOriginals(etm, tmpl, offset);

const {start, middle, end} = getMainBlock(etm, tmpl, offset);
const startBlock =
`${leadingSpace}@case (${condition}) {${leadingSpace}${lbString}${lbSpaces}${start}`;
const endBlock = `${end}${lbString}${leadingSpace}}`;

const defaultBlock = startBlock + middle + endBlock;
const updatedTmpl = tmpl.slice(0, etm.start(offset)) + defaultBlock + tmpl.slice(etm.end(offset));

// this should be the difference between the starting element up to the start of the closing
// element and the mainblock sans }
const pre = originals.start.length - startBlock.length;
const post = originals.end.length - endBlock.length;

return {tmpl: updatedTmpl, offsets: {pre, post}};
}

function getSwitchCaseBlock(
etm: ElementToMigrate, tmpl: string, offset: number, hasLineBreaks: boolean): string {
let elStart = etm.el.sourceSpan?.start.offset - offset;
let elEnd = etm.el.sourceSpan?.end.offset - offset;
const lbString = hasLineBreaks ? '\n ' : ' ';
const lbSpaces = hasLineBreaks ? ' ' : '';
let shift = 0;
function migrateNgSwitchDefault(etm: ElementToMigrate, tmpl: string, offset: number): Result {
// includes the mandatory semicolon before as
const lbString = etm.hasLineBreaks ? lb : '';
const lbSpaces = etm.hasLineBreaks ? ' ' : '';
const leadingSpace = etm.hasLineBreaks ? '' : ' ';

if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') &&
etm.el.attrs.length === 1) {
// no need to keep the containers
elStart = etm.el.children[0].sourceSpan.start.offset - offset;
elEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
// account for the `>` that isn't needed
shift += 1;
}
const originals = getOriginals(etm, tmpl, offset);

const attrStart = etm.attr.keySpan!.start.offset - 1 - offset + shift;
// ngSwitchDefault case has no valueSpan and relies on the end of the key
if (etm.attr.name === '*ngSwitchDefault' || etm.attr.name === 'ngSwitchDefault') {
const attrEnd = etm.attr.keySpan!.end.offset - offset + shift;
return `${lbString}@default {${lbString}${lbSpaces}${
tmpl.slice(elStart, attrStart) + tmpl.slice(attrEnd, elEnd)}${lbString}}`;
}
// ngSwitchCase has a valueSpan
let valEnd = etm.attr.valueSpan!.end.offset + 1 - offset + shift;
return `${lbString}@case (${etm.attr.value}) {${lbString}${lbSpaces}${
tmpl.slice(elStart, attrStart) + tmpl.slice(valEnd, elEnd)}${lbString}}`;
const {start, middle, end} = getMainBlock(etm, tmpl, offset);
const startBlock = `${leadingSpace}@default {${leadingSpace}${lbString}${lbSpaces}${start}`;
const endBlock = `${end}${lbString}${leadingSpace}}`;

const defaultBlock = startBlock + middle + endBlock;
const updatedTmpl = tmpl.slice(0, etm.start(offset)) + defaultBlock + tmpl.slice(etm.end(offset));

// this should be the difference between the starting element up to the start of the closing
// element and the mainblock sans }
const pre = originals.start.length - startBlock.length;
const post = originals.end.length - endBlock.length;

return {tmpl: updatedTmpl, offsets: {pre, post}};
}

0 comments on commit bbf7973

Please sign in to comment.