diff --git a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts index 15636b62218e8..c62bc74b8a024 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts @@ -13,7 +13,7 @@ import {migrateFor} from './fors'; import {migrateIf} from './ifs'; import {migrateSwitch} from './switches'; import {AnalyzedFile, endI18nMarker, endMarker, MigrateError, startI18nMarker, startMarker} from './types'; -import {canRemoveCommonModule, formatTemplate, parseTemplate, processNgTemplates, removeImports} from './util'; +import {canRemoveCommonModule, formatTemplate, parseTemplate, processNgTemplates, removeImports, validateI18nStructure, validateMigratedTemplate} from './util'; /** * Actually migrates a given template to the new syntax @@ -42,15 +42,9 @@ export function migrateTemplate( if (changed) { // determine if migrated template is a valid structure // if it is not, fail out - const parsed = parseTemplate(migrated); - if (parsed.errors.length > 0) { - const parsingError = { - type: 'parse', - error: new Error( - `The migration resulted in invalid HTML for ${file.sourceFile.fileName}. ` + - `Please check the template for valid HTML structures and run the migration again.`) - }; - return {migrated: template, errors: [parsingError]}; + const errors = validateMigratedTemplate(migrated, file.sourceFile.fileName); + if (errors.length > 0) { + return {migrated: template, errors}; } } diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts index 6655631664d94..f21ff98a65b56 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -10,7 +10,7 @@ import {Attribute, Element, HtmlParser, Node, ParseTreeResult, visitAll} from '@ import {dirname, join} from 'path'; import ts from 'typescript'; -import {AnalyzedFile, CommonCollector, ElementCollector, ElementToMigrate, endI18nMarker, endMarker, i18nCollector, importRemovals, importWithCommonRemovals, ParseResult, startI18nMarker, startMarker, Template, TemplateCollector} from './types'; +import {AnalyzedFile, CommonCollector, ElementCollector, ElementToMigrate, endI18nMarker, endMarker, i18nCollector, importRemovals, importWithCommonRemovals, MigrateError, ParseResult, startI18nMarker, startMarker, Template, TemplateCollector} from './types'; const startMarkerRegex = new RegExp(startMarker, 'gm'); const endMarkerRegex = new RegExp(endMarker, 'gm'); @@ -233,6 +233,49 @@ export function parseTemplate(template: string): ParseResult { return {tree: parsed, errors: []}; } +export function validateMigratedTemplate(migrated: string, fileName: string): MigrateError[] { + const parsed = parseTemplate(migrated); + let errors: MigrateError[] = []; + if (parsed.errors.length > 0) { + errors.push({ + type: 'parse', + error: new Error( + `The migration resulted in invalid HTML for ${fileName}. ` + + `Please check the template for valid HTML structures and run the migration again.`) + }); + } + if (parsed.tree) { + const i18nError = validateI18nStructure(parsed.tree, fileName); + if (i18nError !== null) { + errors.push({type: 'i18n', error: i18nError}); + } + } + return errors; +} + +export function validateI18nStructure(parsed: ParseTreeResult, fileName: string): Error|null { + const visitor = new i18nCollector(); + visitAll(visitor, parsed.rootNodes); + const parents = visitor.elements.filter(el => el.children.length > 0); + for (const p of parents) { + for (const el of visitor.elements) { + if (el === p) continue; + if (isChildOf(p, el)) { + return new Error( + `i18n Nesting error: The migration would result in invalid i18n nesting for ` + + `${fileName}. Element with i18n attribute "${p.name}" would result having a child of ` + + `element with i18n attribute "${el.name}". Please fix and re-run the migration.`); + } + } + } + return null; +} + +function isChildOf(parent: Element, el: Element): boolean { + return parent.sourceSpan.start.offset < el.sourceSpan.start.offset && + parent.sourceSpan.end.offset > el.sourceSpan.end.offset; +} + /** * calculates the level of nesting of the items in the collector */ diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 2a65363dc8d3f..3bba14b9e6b5c 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -5511,5 +5511,47 @@ describe('control flow migration', () => { `Element node: "strong" would result in invalid migrated @switch block structure. ` + `@switch can only have @case or @default as children.`); }); + + it('should not migrate a template that would result in invalid i18n nesting', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './comp.html' + }) + class Comp { + testOpts = 2; + } + `); + + writeFile('/comp.html', [ + ``, + `
`, + ` If content here`, + `
`, + `
`, + ``, + `
Else content here
`, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + expect(content).toBe([ + ``, + `
`, + ` If content here`, + `
`, + `
`, + ``, + `
Else content here
`, + `
`, + ].join('\n')); + + expect(warnOutput.join(' ')) + .toContain( + `Element with i18n attribute "ng-container" would result having a child of element with i18n attribute ` + + `"ng-container". Please fix and re-run the migration.`); + }); }); });