From 8e9c7e0e29f53c6ce1ed1ddf137c12f5fe2d93a0 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 8 Aug 2018 20:58:15 +0200 Subject: [PATCH] refactor(schematics): use parse5 for finding inputs and outputs * No longer constructs a complex and unreadable RegExp for finding Angular inputs and outputs. Since we declared `parse5` as a dependency for the schematics, we could use `parse5`. * Adds types for parse5 as dev dependency since parse5 is often used in the schematics. --- package-lock.json | 6 ++ package.json | 1 + src/lib/schematics/update/html/angular.ts | 41 +++++++++++ src/lib/schematics/update/html/elements.ts | 65 +++++++++++++++++ .../update/rules/checkTemplateMiscRule.ts | 9 +-- .../rules/switchTemplateInputNamesRule.ts | 7 +- .../rules/switchTemplateOutputNamesRule.ts | 11 ++- .../schematics/update/typescript/literal.ts | 71 ------------------- src/lib/schematics/utils/html.ts | 22 +++--- 9 files changed, 136 insertions(+), 97 deletions(-) create mode 100644 src/lib/schematics/update/html/angular.ts create mode 100644 src/lib/schematics/update/html/elements.ts diff --git a/package-lock.json b/package-lock.json index 4525aacca717..e9daaed8cf89 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1165,6 +1165,12 @@ "@types/q": "*" } }, + "@types/parse5": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/@types/parse5/-/parse5-5.0.0.tgz", + "integrity": "sha512-J5D3z703XTDIGQFYXsnU9uRCW9e9mMEFO0Kpe6kykyiboqziru/RlZ0hM2P+PKTG4NHG1SjLrqae/NrV2iJApQ==", + "dev": true + }, "@types/q": { "version": "1.5.0", "resolved": "https://registry.npmjs.org/@types/q/-/q-1.5.0.tgz", diff --git a/package.json b/package.json index e4198252e0a5..ce4b1dfceaca 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "@types/merge2": "^0.3.30", "@types/minimist": "^1.2.0", "@types/node": "^7.0.21", + "@types/parse5": "^5.0.0", "@types/run-sequence": "^0.0.29", "autoprefixer": "^6.7.6", "axe-webdriverjs": "^1.1.1", diff --git a/src/lib/schematics/update/html/angular.ts b/src/lib/schematics/update/html/angular.ts new file mode 100644 index 000000000000..c7ed1f5f801e --- /dev/null +++ b/src/lib/schematics/update/html/angular.ts @@ -0,0 +1,41 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {findAttributeOnElementWithAttrs, findAttributeOnElementWithTag} from './elements'; + +/** Finds the specified Angular @Input in the given elements with tag name. */ +export function findInputsOnElementWithTag(html: string, inputName: string, tagNames: string[]) { + // Add one column to the mapped offset because the first bracket for the @Input + // is part of the attribute and therefore also part of the offset. We only want to return + // the offset for the inner name of the input. + return findAttributeOnElementWithTag(html, `[${inputName}]`, tagNames).map(offset => offset + 1); +} + +/** Finds the specified Angular @Output in the given elements with tag name. */ +export function findOutputsOnElementWithTag(html: string, outputName: string, tagNames: string[]) { + // Add one column to the mapped offset because the first parenthesis for the @Output + // is part of the attribute and therefore also part of the offset. We only want to return + // the offset for the inner name of the output. + return findAttributeOnElementWithTag(html, `(${outputName})`, tagNames).map(offset => offset + 1); +} + +/** Finds the specified Angular @Input in elements that have one of the specified attributes. */ +export function findInputsOnElementWithAttr(html: string, inputName: string, attrs: string[]) { + // Add one column to the mapped offset because the first bracket for the @Input + // is part of the attribute and therefore also part of the offset. We only want to return + // the offset for the inner name of the input. + return findAttributeOnElementWithAttrs(html, `[${inputName}]`, attrs).map(offset => offset + 1); +} + +/** Finds the specified Angular @Output in elements that have one of the specified attributes. */ +export function findOutputsOnElementWithAttr(html: string, outputName: string, attrs: string[]) { + // Add one column to the mapped offset because the first bracket for the @Output + // is part of the attribute and therefore also part of the offset. We only want to return + // the offset for the inner name of the output. + return findAttributeOnElementWithAttrs(html, `(${outputName})`, attrs).map(offset => offset + 1); +} diff --git a/src/lib/schematics/update/html/elements.ts b/src/lib/schematics/update/html/elements.ts new file mode 100644 index 000000000000..6d741936e4e0 --- /dev/null +++ b/src/lib/schematics/update/html/elements.ts @@ -0,0 +1,65 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {DefaultTreeDocument, DefaultTreeElement, parseFragment} from 'parse5'; + +/** + * Parses a HTML fragment and traverses all AST nodes in order find elements that + * include the specified attribute. + */ +export function findElementsWithAttribute(html: string, attributeName: string) { + const document = parseFragment(html, {sourceCodeLocationInfo: true}) as DefaultTreeDocument; + const elements: DefaultTreeElement[] = []; + + const visitNodes = nodes => { + nodes.forEach(node => { + if (node.childNodes) { + visitNodes(node.childNodes); + } + + if (node.attrs && node.attrs.some(attr => attr.name === attributeName.toLowerCase())) { + elements.push(node); + } + }); + }; + + visitNodes(document.childNodes); + + return elements; +} + +/** + * Finds elements with explicit tag names that also contain the specified attribute. Returns the + * attribute start offset based on the specified HTML. + */ +export function findAttributeOnElementWithTag(html: string, name: string, tagNames: string[]) { + return findElementsWithAttribute(html, name) + .filter(element => tagNames.includes(element.tagName)) + .map(element => getStartOffsetOfAttribute(element, name)); +} + +/** + * Finds elements that contain the given attribute and contain at least one of the other + * specified attributes. Returns the primary attribute's start offset based on the specified HTML. + */ +export function findAttributeOnElementWithAttrs(html: string, name: string, attrs: string[]) { + return findElementsWithAttribute(html, name) + .filter(element => attrs.some(attr => hasElementAttribute(element, attr))) + .map(element => getStartOffsetOfAttribute(element, name)); +} + +/** Shorthand function that checks if the specified element contains the given attribute. */ +function hasElementAttribute(element: DefaultTreeElement, attributeName: string): boolean { + return element.attrs && element.attrs.some(attr => attr.name === attributeName.toLowerCase()); +} + + +/** Gets the start offset of the given attribute from a Parse5 element. */ +export function getStartOffsetOfAttribute(element: any, attributeName: string): number { + return element.sourceCodeLocation.attrs[attributeName.toLowerCase()].startOffset; +} diff --git a/src/lib/schematics/update/rules/checkTemplateMiscRule.ts b/src/lib/schematics/update/rules/checkTemplateMiscRule.ts index 75b0441fa8ad..7cdbc7e5fd52 100644 --- a/src/lib/schematics/update/rules/checkTemplateMiscRule.ts +++ b/src/lib/schematics/update/rules/checkTemplateMiscRule.ts @@ -9,9 +9,10 @@ import {bold, green, red} from 'chalk'; import {RuleFailure, Rules} from 'tslint'; import * as ts from 'typescript'; +import {findInputsOnElementWithTag, findOutputsOnElementWithTag} from '../html/angular'; import {ExternalResource} from '../tslint/component-file'; import {ComponentWalker} from '../tslint/component-walker'; -import {findAll, findAllInputsInElWithTag, findAllOutputsInElWithTag} from '../typescript/literal'; +import {findAll} from '../typescript/literal'; /** * Rule that walks through every component decorator and updates their inline or external @@ -55,7 +56,7 @@ export class CheckTemplateMiscWalker extends ComponentWalker { }))); failures = failures.concat( - findAllOutputsInElWithTag(templateContent, 'selectionChange', ['mat-list-option']) + findOutputsOnElementWithTag(templateContent, 'selectionChange', ['mat-list-option']) .map(offset => ({ start: offset, end: offset + 'selectionChange'.length, @@ -65,7 +66,7 @@ export class CheckTemplateMiscWalker extends ComponentWalker { }))); failures = failures.concat( - findAllOutputsInElWithTag(templateContent, 'selectedChanged', ['mat-datepicker']) + findOutputsOnElementWithTag(templateContent, 'selectedChanged', ['mat-datepicker']) .map(offset => ({ start: offset, end: offset + 'selectionChange'.length, @@ -75,7 +76,7 @@ export class CheckTemplateMiscWalker extends ComponentWalker { }))); failures = failures.concat( - findAllInputsInElWithTag(templateContent, 'selected', ['mat-button-toggle-group']) + findInputsOnElementWithTag(templateContent, 'selected', ['mat-button-toggle-group']) .map(offset => ({ start: offset, end: offset + 'selected'.length, diff --git a/src/lib/schematics/update/rules/switchTemplateInputNamesRule.ts b/src/lib/schematics/update/rules/switchTemplateInputNamesRule.ts index 651f1d80b6df..a530327c27b1 100644 --- a/src/lib/schematics/update/rules/switchTemplateInputNamesRule.ts +++ b/src/lib/schematics/update/rules/switchTemplateInputNamesRule.ts @@ -9,10 +9,11 @@ import {green, red} from 'chalk'; import {Replacement, RuleFailure, Rules} from 'tslint'; import * as ts from 'typescript'; +import {findInputsOnElementWithAttr, findInputsOnElementWithTag} from '../html/angular'; import {inputNames} from '../material/data/input-names'; import {ExternalResource} from '../tslint/component-file'; import {ComponentWalker} from '../tslint/component-walker'; -import {findAll, findAllInputsInElWithAttr, findAllInputsInElWithTag} from '../typescript/literal'; +import {findAll} from '../typescript/literal'; /** * Rule that walks through every component decorator and updates their inline or external @@ -53,11 +54,11 @@ export class SwitchTemplateInputNamesWalker extends ComponentWalker { inputNames.forEach(name => { let offsets: number[] = []; if (name.whitelist && name.whitelist.attributes && name.whitelist.attributes.length) { - offsets = offsets.concat(findAllInputsInElWithAttr( + offsets = offsets.concat(findInputsOnElementWithAttr( templateContent, name.replace, name.whitelist.attributes)); } if (name.whitelist && name.whitelist.elements && name.whitelist.elements.length) { - offsets = offsets.concat(findAllInputsInElWithTag( + offsets = offsets.concat(findInputsOnElementWithTag( templateContent, name.replace, name.whitelist.elements)); } if (!name.whitelist) { diff --git a/src/lib/schematics/update/rules/switchTemplateOutputNamesRule.ts b/src/lib/schematics/update/rules/switchTemplateOutputNamesRule.ts index 0d9d268ff79a..bc453bb18367 100644 --- a/src/lib/schematics/update/rules/switchTemplateOutputNamesRule.ts +++ b/src/lib/schematics/update/rules/switchTemplateOutputNamesRule.ts @@ -9,14 +9,11 @@ import {green, red} from 'chalk'; import {Replacement, RuleFailure, Rules} from 'tslint'; import * as ts from 'typescript'; +import {findOutputsOnElementWithAttr, findOutputsOnElementWithTag} from '../html/angular'; import {outputNames} from '../material/data/output-names'; import {ExternalResource} from '../tslint/component-file'; import {ComponentWalker} from '../tslint/component-walker'; -import { - findAll, - findAllOutputsInElWithAttr, - findAllOutputsInElWithTag -} from '../typescript/literal'; +import {findAll} from '../typescript/literal'; /** * Rule that walks through every component decorator and updates their inline or external @@ -57,11 +54,11 @@ export class SwitchTemplateOutputNamesWalker extends ComponentWalker { outputNames.forEach(name => { let offsets: number[] = []; if (name.whitelist && name.whitelist.attributes && name.whitelist.attributes.length) { - offsets = offsets.concat(findAllOutputsInElWithAttr( + offsets = offsets.concat(findOutputsOnElementWithAttr( templateContent, name.replace, name.whitelist.attributes)); } if (name.whitelist && name.whitelist.elements && name.whitelist.elements.length) { - offsets = offsets.concat(findAllOutputsInElWithTag( + offsets = offsets.concat(findOutputsOnElementWithTag( templateContent, name.replace, name.whitelist.elements)); } if (!name.whitelist) { diff --git a/src/lib/schematics/update/typescript/literal.ts b/src/lib/schematics/update/typescript/literal.ts index ee322a620782..fe8c3d49debc 100644 --- a/src/lib/schematics/update/typescript/literal.ts +++ b/src/lib/schematics/update/typescript/literal.ts @@ -22,74 +22,3 @@ export function findAll(str: string, search: string): number[] { } return result; } - -export function findAllInputsInElWithTag(html: string, name: string, tagNames: string[]): number[] { - return findAllIoInElWithTag(html, name, tagNames, String.raw`\[?`, String.raw`\]?`); -} - -export function findAllOutputsInElWithTag(html: string, name: string, tagNames: string[]): - number[] { - return findAllIoInElWithTag(html, name, tagNames, String.raw`\(`, String.raw`\)`); -} - -/** - * Method that can be used to rename all occurrences of an `@Input()` in a HTML string that occur - * inside an element with any of the given attributes. This is useful for replacing an `@Input()` on - * a `@Directive()` with an attribute selector. - */ -export function findAllInputsInElWithAttr(html: string, name: string, attrs: string[]): number[] { - return findAllIoInElWithAttr(html, name, attrs, String.raw`\[?`, String.raw`\]?`); -} - -/** - * Method that can be used to rename all occurrences of an `@Output()` in a HTML string that occur - * inside an element with any of the given attributes. This is useful for replacing an `@Output()` - * on a `@Directive()` with an attribute selector. - */ -export function findAllOutputsInElWithAttr(html: string, name: string, attrs: string[]): number[] { - return findAllIoInElWithAttr(html, name, attrs, String.raw`\(`, String.raw`\)`); -} - -function findAllIoInElWithTag(html: string, name: string, tagNames: string[], - startIoPattern: string, endIoPattern: string): number[] { - - const skipPattern = String.raw`[^>]*\s`; - const openTagPattern = String.raw`<\s*`; - const tagNamesPattern = String.raw`(?:${tagNames.join('|')})`; - const replaceIoPattern = String.raw` - (${openTagPattern}${tagNamesPattern}\s(?:${skipPattern})?${startIoPattern}) - ${name} - ${endIoPattern}[=\s>]`; - const replaceIoRegex = new RegExp(replaceIoPattern.replace(/\s/g, ''), 'g'); - const result: number[] = []; - - let match; - while (match = replaceIoRegex.exec(html)) { - result.push(match.index + match[1].length); - } - - return result; -} - -function findAllIoInElWithAttr(html: string, name: string, attrs: string[], startIoPattern: string, - endIoPattern: string): number[] { - const skipPattern = String.raw`[^>]*\s`; - const openTagPattern = String.raw`<\s*\S`; - const attrsPattern = String.raw`(?:${attrs.join('|')})`; - const inputAfterAttrPattern = String.raw` - (${openTagPattern}${skipPattern}${attrsPattern}[=\s](?:${skipPattern})?${startIoPattern}) - ${name} - ${endIoPattern}[=\s>]`; - const inputBeforeAttrPattern = String.raw` - (${openTagPattern}${skipPattern}${startIoPattern}) - ${name} - ${endIoPattern}[=\s](?:${skipPattern})?${attrsPattern}[=\s>]`; - const replaceIoPattern = String.raw`${inputAfterAttrPattern}|${inputBeforeAttrPattern}`; - const replaceIoRegex = new RegExp(replaceIoPattern.replace(/\s/g, ''), 'g'); - const result = []; - let match; - while (match = replaceIoRegex.exec(html)) { - result.push(match.index + (match[1] || match[2]).length); - } - return result; -} diff --git a/src/lib/schematics/utils/html.ts b/src/lib/schematics/utils/html.ts index b033eb5ac386..ac7fcfb5a979 100644 --- a/src/lib/schematics/utils/html.ts +++ b/src/lib/schematics/utils/html.ts @@ -7,7 +7,7 @@ */ import {Tree, SchematicsException} from '@angular-devkit/schematics'; -import * as parse5 from 'parse5'; +import {parse as parseHtml, DefaultTreeDocument, DefaultTreeElement} from 'parse5'; import {getIndexHtmlPath} from './ast'; import {InsertChange} from '@schematics/angular/utility/change'; import {WorkspaceProject} from '@schematics/angular/utility/config'; @@ -17,27 +17,25 @@ import {WorkspaceProject} from '@schematics/angular/utility/config'; * @param src the src path of the html file to parse */ export function getHeadTag(src: string) { - const document = parse5.parse(src, - {sourceCodeLocationInfo: true}) as parse5.AST.Default.Document; + const document = parseHtml(src, {sourceCodeLocationInfo: true}) as DefaultTreeDocument; - let head; - const visit = (nodes: parse5.AST.Default.Node[]) => { + let head: DefaultTreeElement; + const visitNodes = nodes => { nodes.forEach(node => { - const element = node; - if (element.tagName === 'head') { - head = element; + if (node.tagName === 'head') { + head = node; } else { - if (element.childNodes) { - visit(element.childNodes); + if (node.childNodes) { + visitNodes(node.childNodes); } } }); }; - visit(document.childNodes); + visitNodes(document.childNodes); if (!head) { - throw new SchematicsException('Head element not found!'); + throw new SchematicsException('Head element could not be found!'); } return {