From 239c781f59bf91c0bc7340130d8fa25637f10d6b Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 13 Sep 2018 19:43:49 +0200 Subject: [PATCH] refactor(schematics): add tests for component walker (#13106) * The `ComponentWalker` is a major part of the update schematics and we should verify that external resource files are resolved properly. All test-cases currently just use inline stylesheets or templates. * Removes the `color` helper function. * Removes the superfluous utility function for retrieving a `StringLiteral`'s text content. --- package-lock.json | 15 ++ package.json | 2 + src/lib/schematics/update/material/color.ts | 23 ---- .../material/data/method-call-checks.ts | 2 +- .../signature-check/methodCallsCheckRule.ts | 3 +- .../update/tslint/component-walker.spec.ts | 130 ++++++++++++++++++ .../update/tslint/component-walker.ts | 27 ++-- .../schematics/update/typescript/literal.ts | 7 - 8 files changed, 161 insertions(+), 48 deletions(-) delete mode 100644 src/lib/schematics/update/material/color.ts create mode 100644 src/lib/schematics/update/tslint/component-walker.spec.ts diff --git a/package-lock.json b/package-lock.json index 3fd65bbc197..746df218e41 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1160,6 +1160,15 @@ "integrity": "sha1-aaI6OtKcrwCX8G7aWbNh7i8GOfY=", "dev": true }, + "@types/mock-fs": { + "version": "3.6.30", + "resolved": "https://registry.npmjs.org/@types/mock-fs/-/mock-fs-3.6.30.tgz", + "integrity": "sha1-TYElQeh7I1dyYaWqlfcE3T0B5BA=", + "dev": true, + "requires": { + "@types/node": "*" + } + }, "@types/node": { "version": "7.0.67", "resolved": "https://registry.npmjs.org/@types/node/-/node-7.0.67.tgz", @@ -11845,6 +11854,12 @@ "mkdirp": "*" } }, + "mock-fs": { + "version": "4.7.0", + "resolved": "https://registry.npmjs.org/mock-fs/-/mock-fs-4.7.0.tgz", + "integrity": "sha512-WlQNtUlzMRpvLHf8dqeUmNqfdPjGY29KrJF50Ldb4AcL+vQeR8QH3wQcFMgrhTwb1gHjZn9xggho+84tBskLgA==", + "dev": true + }, "modelo": { "version": "4.2.3", "resolved": "https://registry.npmjs.org/modelo/-/modelo-4.2.3.tgz", diff --git a/package.json b/package.json index 1c42e6199bf..0cdf98709d0 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,7 @@ "@types/jasmine": "^2.8.8", "@types/merge2": "^0.3.30", "@types/minimist": "^1.2.0", + "@types/mock-fs": "^3.6.30", "@types/node": "^7.0.21", "@types/parse5": "^5.0.0", "@types/run-sequence": "^0.0.29", @@ -112,6 +113,7 @@ "magic-string": "^0.22.4", "minimatch": "^3.0.4", "minimist": "^1.2.0", + "mock-fs": "^4.7.0", "moment": "^2.18.1", "node-sass": "^4.9.3", "parse5": "^5.0.0", diff --git a/src/lib/schematics/update/material/color.ts b/src/lib/schematics/update/material/color.ts deleted file mode 100644 index c8be943a043..00000000000 --- a/src/lib/schematics/update/material/color.ts +++ /dev/null @@ -1,23 +0,0 @@ -/** - * @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 {bold, green, red} from 'chalk'; - -const colorFns = { - 'b': bold, - 'g': green, - 'r': red, -}; - -export function color(message: string): string { - // 'r{{text}}' with red 'text', 'g{{text}}' with green 'text', and 'b{{text}}' with bold 'text'. - return message.replace(/(.){{(.*?)}}/g, (_m, fnName, text) => { - const fn = colorFns[fnName]; - return fn ? fn(text) : text; - }); -} diff --git a/src/lib/schematics/update/material/data/method-call-checks.ts b/src/lib/schematics/update/material/data/method-call-checks.ts index 1c1b1e70d18..5322ae05f48 100644 --- a/src/lib/schematics/update/material/data/method-call-checks.ts +++ b/src/lib/schematics/update/material/data/method-call-checks.ts @@ -29,7 +29,7 @@ export const methodCallChecks: VersionChanges = { invalidArgCounts: [ { count: 3, - message: 'The "r{{renderer}}" argument has been removed' + message: 'The "renderer" argument has been removed' } ] } diff --git a/src/lib/schematics/update/rules/signature-check/methodCallsCheckRule.ts b/src/lib/schematics/update/rules/signature-check/methodCallsCheckRule.ts index 75dc92a9dd5..7e6a31ec59c 100644 --- a/src/lib/schematics/update/rules/signature-check/methodCallsCheckRule.ts +++ b/src/lib/schematics/update/rules/signature-check/methodCallsCheckRule.ts @@ -9,7 +9,6 @@ import {bold} from 'chalk'; import {ProgramAwareRuleWalker, RuleFailure, Rules} from 'tslint'; import * as ts from 'typescript'; -import {color} from '../../material/color'; import {methodCallChecks} from '../../material/data/method-call-checks'; import {getChangesForTarget} from '../../material/transform-change-data'; @@ -60,6 +59,6 @@ export class Walker extends ProgramAwareRuleWalker { } this.addFailureAtNode(node, `Found call to "${bold(hostTypeName + '.' + methodName)}" ` + - `with ${bold(`${failure.count}`)} arguments. Message: ${color(failure.message)}`); + `with ${bold(`${failure.count}`)} arguments. Message: ${failure.message}`); } } diff --git a/src/lib/schematics/update/tslint/component-walker.spec.ts b/src/lib/schematics/update/tslint/component-walker.spec.ts new file mode 100644 index 00000000000..0ec822b314a --- /dev/null +++ b/src/lib/schematics/update/tslint/component-walker.spec.ts @@ -0,0 +1,130 @@ +import * as mockFs from 'mock-fs'; +import {dirname, join} from 'path'; +import {IOptions} from 'tslint'; +import * as ts from 'typescript'; +import {ComponentWalker} from './component-walker'; + +describe('ComponentWalker', () => { + + const defaultRuleOptions: IOptions = { + ruleArguments: [], + ruleSeverity: 'error', + disabledIntervals: [], + ruleName: 'component-walker-test' + }; + + afterEach(() => mockFs.restore()); + + function createSourceFile(content: string) { + return ts.createSourceFile(join(__dirname, 'test-source-file.ts'), content, + ts.ScriptTarget.Latest, true); + } + + it('should report inline stylesheets', () => { + const sourceFile = createSourceFile(inlineStylesheetSource); + const walker = new ComponentWalker(sourceFile, defaultRuleOptions); + + spyOn(walker, 'visitInlineStylesheet'); + + walker.walk(sourceFile); + + expect(walker.visitInlineStylesheet).toHaveBeenCalledTimes(1); + }); + + it('should report external stylesheets', () => { + const sourceFile = createSourceFile(externalStylesheetSource); + const walker = new ComponentWalker(sourceFile, defaultRuleOptions); + const stylePath = join(dirname(sourceFile.fileName), 'my-component.css'); + const styleContent = ':host { color: red; }'; + + spyOn(walker, 'visitExternalStylesheet').and.callFake(node => { + expect(node.getFullText()).toBe(styleContent); + }); + + mockFs({[stylePath]: styleContent}); + + walker.walk(sourceFile); + + expect(walker.visitExternalStylesheet).toHaveBeenCalledTimes(1); + }); + + it('should not throw if an external stylesheet could not be resolved', () => { + const sourceFile = createSourceFile(externalStylesheetSource); + const walker = new ComponentWalker(sourceFile, defaultRuleOptions); + + spyOn(console, 'error'); + + expect(() => walker.walk(sourceFile)).not.toThrow(); + expect(console.error).toHaveBeenCalledTimes(1); + }); + + it('should report inline templates', () => { + const sourceFile = createSourceFile(inlineTemplateSource); + const walker = new ComponentWalker(sourceFile, defaultRuleOptions); + + spyOn(walker, 'visitInlineTemplate'); + + walker.walk(sourceFile); + + expect(walker.visitInlineTemplate).toHaveBeenCalledTimes(1); + }); + + it('should report external templates', () => { + const sourceFile = createSourceFile(externalTemplateSource); + const walker = new ComponentWalker(sourceFile, defaultRuleOptions); + const templatePath = join(dirname(sourceFile.fileName), 'my-component.html'); + const templateContent = 'External template'; + + spyOn(walker, 'visitExternalTemplate').and.callFake(node => { + expect(node.getFullText()).toBe(templateContent); + }); + + mockFs({[templatePath]: templateContent}); + + walker.walk(sourceFile); + + expect(walker.visitExternalTemplate).toHaveBeenCalledTimes(1); + }); + + it('should not throw if an external template could not be resolved', () => { + const sourceFile = createSourceFile(externalTemplateSource); + const walker = new ComponentWalker(sourceFile, defaultRuleOptions); + + spyOn(console, 'error'); + + expect(() => walker.walk(sourceFile)).not.toThrow(); + expect(console.error).toHaveBeenCalledTimes(1); + }); +}); + +/** TypeScript source file content that includes a component with inline styles. */ +const inlineStylesheetSource = ` + @Component({ + styles: [':host { color: red; }'] + } + export class MyComponent {} +`; + +/** TypeScript source file content that includes an inline component template. */ +const inlineTemplateSource = ` + @Component({ + template: 'My component' + } + export class MyComponent {} +`; + +/** TypeScript source file that includes a component with external styles. */ +const externalStylesheetSource = ` + @Component({ + styleUrls: ['./my-component.css'] + }) + export class MyComponent {} +`; + +/** TypeScript source file that includes a component with an external template. */ +const externalTemplateSource = ` + @Component({ + templateUrl: './my-component.html', + }) + export class MyComponent {} +`; diff --git a/src/lib/schematics/update/tslint/component-walker.ts b/src/lib/schematics/update/tslint/component-walker.ts index 0bcd33faeb7..a2d823087a3 100644 --- a/src/lib/schematics/update/tslint/component-walker.ts +++ b/src/lib/schematics/update/tslint/component-walker.ts @@ -13,7 +13,6 @@ import {existsSync, readFileSync} from 'fs'; import {dirname, join, resolve} from 'path'; import {IOptions, RuleWalker} from 'tslint'; import * as ts from 'typescript'; -import {getLiteralTextWithoutQuotes} from '../typescript/literal'; import {createComponentFile, ExternalResource} from './component-file'; /** @@ -22,11 +21,11 @@ import {createComponentFile, ExternalResource} from './component-file'; */ export class ComponentWalker extends RuleWalker { - protected visitInlineTemplate(_template: ts.StringLiteral) {} - protected visitInlineStylesheet(_stylesheet: ts.StringLiteral) {} + visitInlineTemplate(_template: ts.StringLiteral) {} + visitInlineStylesheet(_stylesheet: ts.StringLiteral) {} - protected visitExternalTemplate(_template: ExternalResource) {} - protected visitExternalStylesheet(_stylesheet: ExternalResource) {} + visitExternalTemplate(_template: ExternalResource) {} + visitExternalStylesheet(_stylesheet: ExternalResource) {} private extraFiles: Set; @@ -92,9 +91,8 @@ export class ComponentWalker extends RuleWalker { } private _visitExternalStylesArrayLiteral(styleUrls: ts.ArrayLiteralExpression) { - styleUrls.elements.forEach(styleUrlLiteral => { - const styleUrl = getLiteralTextWithoutQuotes(styleUrlLiteral as ts.StringLiteral); - const stylePath = resolve(join(dirname(this.getSourceFile().fileName), styleUrl)); + styleUrls.elements.forEach((node: ts.StringLiteral) => { + const stylePath = resolve(join(dirname(this.getSourceFile().fileName), node.text)); // Do not report the specified additional files multiple times. if (!this.extraFiles.has(stylePath)) { @@ -103,9 +101,8 @@ export class ComponentWalker extends RuleWalker { }); } - private _reportExternalTemplate(templateUrlLiteral: ts.StringLiteral) { - const templateUrl = getLiteralTextWithoutQuotes(templateUrlLiteral); - const templatePath = resolve(join(dirname(this.getSourceFile().fileName), templateUrl)); + private _reportExternalTemplate(node: ts.StringLiteral) { + const templatePath = resolve(join(dirname(this.getSourceFile().fileName), node.text)); // Do not report the specified additional files multiple times. if (this.extraFiles.has(templatePath)) { @@ -116,7 +113,7 @@ export class ComponentWalker extends RuleWalker { if (!existsSync(templatePath)) { console.error(`PARSE ERROR: ${this.getSourceFile().fileName}:` + ` Could not find template: "${templatePath}".`); - process.exit(1); + return; } // Create a fake TypeScript source file that includes the template content. @@ -128,9 +125,9 @@ export class ComponentWalker extends RuleWalker { _reportExternalStyle(stylePath: string) { // Check if the external stylesheet file exists before proceeding. if (!existsSync(stylePath)) { - console.error(`PARSE ERROR: ${this.getSourceFile().fileName}:` + - ` Could not find stylesheet: "${stylePath}".`); - process.exit(1); + console.error(`PARSE ERROR: ${this.getSourceFile().fileName}: ` + + `Could not find stylesheet: "${stylePath}".`); + return; } // Create a fake TypeScript source file that includes the stylesheet content. diff --git a/src/lib/schematics/update/typescript/literal.ts b/src/lib/schematics/update/typescript/literal.ts index c9ce90a0523..646c76a4d14 100644 --- a/src/lib/schematics/update/typescript/literal.ts +++ b/src/lib/schematics/update/typescript/literal.ts @@ -6,13 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import * as ts from 'typescript'; - -/** Returns the text of a string literal without the quotes. */ -export function getLiteralTextWithoutQuotes(literal: ts.StringLiteral) { - return literal.getText().substring(1, literal.getText().length - 1); -} - /** Finds all start indices of the given search string in the input string. */ export function findAllSubstringIndices(input: string, search: string): number[] { const result: number[] = [];