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.

Author: devversion

package-lock.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",

package.json

@@ -29,7 +29,7 @@ export const methodCallChecks: VersionChanges<MaterialMethodCallData> = {
           invalidArgCounts: [
             {
               count: 3,
-              message: 'The "r{{renderer}}" argument has been removed'
+              message: 'The "renderer" argument has been removed'
             }
           ]
         }

src/lib/schematics/update/material/color.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}`);
   }
 }

src/lib/schematics/update/material/data/method-call-checks.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 = '<span>External template</span>';
+
+    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: '<span>My component</span>'
+  }
+  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 {}
+`;

src/lib/schematics/update/rules/signature-check/methodCallsCheckRule.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<string>;
 
@@ -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.

src/lib/schematics/update/tslint/component-walker.spec.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[] = [];