fix(lint): improve linting performance

* chore: ignore WebStorm config files

* chore: upgrade internal tslint dependencies
- update tslint-eslint-rules
- run tslint with --type-check flag

* fix(interfaces): remove unnecessary semicolons

* fix: remove unnecessary semicolons

* chore: use stylish formatter for tslint

* fix: fix tslint errors

* chore: update mock-fs (allows using Node v7+)

* feat(lint): upgrade to TSLint v5

* feat(lint): add ENV_TYPE_CHECK_ON_LINT and default to true

* test(lint): cleanup a little

* refactor(lint): improve linting logic

* fix(lint): make CLI argument the same as the env var

* feat(lint): add support for enabling type check during lint

* test(lint): more cleanup

* fix(lint): set project dir to context {rootDir}

* chore: update tslint

* chore(lint): add typeCheck() fn

* chore: fix tslint errors

* feat: implement and run type check if enabled

* refactor(lint): rename generateFormattedErrorMsg() to generateErrorMessageForFiles()

* fix(lint): filter duplicate file names

* docs(lint): update note

* chore(lint): add some TODOs

* feat(lint): set default value for --typeCheck to false

* docs(lint): document --typeCheck flag

* fix(lint): don't create a new instance of BuildError when throwing after linting failed

* fix(lint): set --typeCheckOnLint to null as default instead of false

* test(lint): fix typeCheckOnLint flag tests

* test(lint): add tests for generateErrorMessageForFiles(), getFileNames() and removeDuplicateFileNames()

* chore(lint): remove some TODOs

* fix(lint): properly pass CLI arguments to linter

* feat(lint): properly disable type check during builds

* fix(lint): allow null TSLint config file

* chore: revert string change

* chore: fix npm warning

* chore: upgrade tslint-eslint-rules

* perf(lint): only create the linter and TS program once per linting instance instead of once per file

* chore: fix tslint errors

* chore(lint): only fetch lint result after all files have been linted
- rename processLintResults() to processLintResult()
- update jsdocs

* test(lint): test all lint fns

* chore: fix tslint errors

* refactor(lint): use Set to filter unique file names

Author: rolandjitsu

package.json

@@ -63,6 +63,7 @@
     "xml2js": "0.4.17"
   },
   "devDependencies": {
+    "@angular/animations": "4.1.3",
     "@angular/common": "4.1.3",
     "@angular/compiler": "4.1.3",
     "@angular/compiler-cli": "4.1.3",

src/generators/util.spec.ts

@@ -410,7 +410,7 @@ $TAB_CONTENT
     const suppliedName = 'settings view';
 
     it('should return a succesful promise', () => {
-			// set up spies
+      // set up spies
       spyOn(helpers, helpers.readFileAsync.name).and.returnValue(Promise.resolve('file content'));
       spyOn(fs, 'readdirSync').and.returnValue([
         join(process.cwd(), 'path', 'to', 'nowhere'),
@@ -420,7 +420,7 @@ $TAB_CONTENT
       spyOn(TypeScriptUtils, TypeScriptUtils.insertNamedImportIfNeeded.name).and.returnValue('file content');
       spyOn(TypeScriptUtils, TypeScriptUtils.appendNgModuleDeclaration.name).and.returnValue('sliced string');
 
-			// what we want to test
+      // what we want to test
       const promise = util.tabsModuleManipulation([['/src/pages/cool-tab-one/cool-tab-one.module.ts']], { name: suppliedName, className: className, fileName: fileName }, [{ name: suppliedName, className: className, fileName: fileName }]);
 
       // test

src/lint.ts

@@ -2,7 +2,7 @@ import { access } from 'fs';
 import { join } from 'path';
 
 import { lintFiles } from './lint/lint-utils';
-import { getFileNames } from './lint/lint-factory';
+import { createProgram, getFileNames } from './lint/lint-factory';
 import { Logger } from './logger/logger';
 import { getUserConfigFile } from './util/config';
 import { ENV_BAIL_ON_LINT_ERROR, ENV_TYPE_CHECK_ON_LINT } from './util/constants';
@@ -64,16 +64,18 @@ export function lintUpdate(changedFiles: ChangedFile[], context: BuildContext, t
 }
 
 export function lintUpdateWorker(context: BuildContext, {tsConfig, tsLintConfig, filePaths, typeCheck}: LintWorkerConfig) {
+  const program = createProgram(context, tsConfig);
   return getLintConfig(context, tsLintConfig)
-    .then(tsLintConfig => lintFiles(context, tsConfig, tsLintConfig, filePaths, {typeCheck}))
+    .then(tsLintConfig => lintFiles(context, program, tsLintConfig, filePaths, {typeCheck}))
     // Don't throw if linting failed
     .catch(() => {});
 }
 
 
 function lintApp(context: BuildContext, {tsConfig, tsLintConfig, typeCheck}: LintWorkerConfig) {
-  const files = getFileNames(context, tsConfig);
-  return lintFiles(context, tsConfig, tsLintConfig, files, {typeCheck});
+  const program = createProgram(context, tsConfig);
+  const files = getFileNames(context, program);
+  return lintFiles(context, program, tsLintConfig, files, {typeCheck});
 }
 
 

src/lint/lint-factory.spec.ts

@@ -0,0 +1,159 @@
+import { Configuration, Linter } from 'tslint';
+import { DiagnosticCategory } from 'typescript';
+import * as ts from 'typescript';
+import { isObject } from 'util';
+import {
+  createLinter,
+  createProgram,
+  getFileNames,
+  getLintResult,
+  getTsLintConfig,
+  lint,
+  typeCheck
+} from './lint-factory';
+
+
+describe('lint factory', () => {
+  describe('createProgram()', () => {
+    it('should create a TS Program', () => {
+      const context: any = {rootDir: ''};
+      const program: any = createProgram(context, '');
+      const fns = [
+        'getSourceFiles',
+        'getTypeChecker'
+      ];
+
+      expect(isObject(program)).toBeTruthy();
+      for (const fn of fns) {
+        expect(typeof program[fn]).toEqual('function');
+      }
+    });
+  });
+
+  describe('getTsLintConfig()', () => {
+    it('should fetch the TSLint configuration from file path', () => {
+      const tsConfigFilePath = 'tsconfig.json';
+      const mockConfig = {rulesDirectory: ['node_modules/@ionic']};
+      spyOn(Configuration, Configuration.loadConfigurationFromPath.name).and.returnValue(mockConfig);
+
+      const config = getTsLintConfig(tsConfigFilePath);
+
+      expect(isObject(config)).toBeTruthy();
+      expect(Configuration.loadConfigurationFromPath).toHaveBeenLastCalledWith(tsConfigFilePath);
+      expect(config).toEqual(mockConfig);
+    });
+
+    it('should extend configuration with {linterOptions} if provided', () => {
+      const tsConfigFilePath = 'tsconfig.json';
+      const mockConfig = {rulesDirectory: ['node_modules/@ionic']};
+      spyOn(Configuration, Configuration.loadConfigurationFromPath.name).and.returnValue(mockConfig);
+      const config = getTsLintConfig(tsConfigFilePath, {
+        typeCheck: true
+      });
+
+      expect(config.linterOptions).toEqual({
+        typeCheck: true
+      });
+    });
+  });
+
+  describe('createLinter()', () => {
+    it('should create a Linter', () => {
+      const context: any = {rootDir: ''};
+      const program = createProgram(context, '');
+      const linter = createLinter(context, program);
+
+      expect(linter instanceof Linter).toBeTruthy();
+    });
+  });
+
+  describe('getFileNames()', () => {
+    it('should get the file names referenced in the tsconfig.json', () => {
+      const context: any = {rootDir: ''};
+      const program = createProgram(context, '');
+      const mockFiles = ['test.ts'];
+      spyOn(Linter, 'getFileNames').and.returnValue(mockFiles);
+      const files = getFileNames(context, program);
+
+      expect(Array.isArray(files)).toBeTruthy();
+      expect(files).toEqual(mockFiles);
+    });
+  });
+
+  describe('typeCheck()', () => {
+    it('should not be called if {typeCheck} is false', done => {
+      const context: any = {rootDir: ''};
+      const program = createProgram(context, '');
+
+      spyOn(ts, ts.getPreEmitDiagnostics.name).and.returnValue([]);
+
+      typeCheck(context, program, {typeCheck: false})
+        .then((result) => {
+          expect(ts.getPreEmitDiagnostics).toHaveBeenCalledTimes(0);
+          expect(result).toEqual([]);
+          done();
+        });
+    });
+
+    it('should type check if {typeCheck} is true', done => {
+      const context: any = {rootDir: ''};
+      const program = createProgram(context, '');
+
+      const diagnostics: any = [{
+        file: {},
+        start: 2,
+        length: 10,
+        messageText: 'Oops',
+        category: DiagnosticCategory.Warning,
+        code: 120
+      }];
+
+      spyOn(ts, ts.getPreEmitDiagnostics.name).and.returnValue(diagnostics);
+
+      typeCheck(context, program, {typeCheck: true})
+        .then((result) => {
+          expect(ts.getPreEmitDiagnostics).toHaveBeenCalledWith(program);
+          expect(result).toEqual(diagnostics);
+          done();
+        });
+    });
+  });
+
+  describe('lint()', () => {
+    it('should lint a file', () => {
+      const context: any = {rootDir: ''};
+      const program = createProgram(context, '');
+      const linter = createLinter(context, program);
+      spyOn(linter, 'lint').and.returnValue(undefined);
+      const config = {};
+      const filePath = 'test.ts';
+      const fileContents = 'const test = true;';
+
+      lint(linter, config, filePath, fileContents);
+
+      expect(linter.lint).toHaveBeenCalledWith(filePath, fileContents, config);
+    });
+  });
+  describe('getLintResult()', () => {
+    it('should get the lint results after linting a file', () => {
+      const context: any = {rootDir: ''};
+      const program = createProgram(context, '');
+      const linter = createLinter(context, program);
+      spyOn(linter, 'lint').and.returnValue(undefined);
+      const mockResult: any = {};
+      spyOn(linter, 'getResult').and.returnValue(mockResult);
+      const config = {
+        jsRules: new Map(),
+        rules: new Map()
+      };
+      const filePath = 'test.ts';
+      const fileContents = 'const test = true;';
+
+      lint(linter, config, filePath, fileContents);
+
+      const result = getLintResult(linter);
+      expect(isObject(result)).toBeTruthy();
+      expect(result).toEqual(mockResult);
+    });
+  });
+});

src/lint/lint-factory.ts

@@ -8,43 +8,49 @@ export interface LinterOptions {
   typeCheck?: boolean;
 }
 
+export interface LinterConfig {
+  [key: string]: any;
+}
+
 
 /**
- * Run linter on a file
- * @param {BuildContext} context
- * @param {string} tsLintConfig
- * @param {string} tsConfig
+ * Lint a file according to config
+ * @param {Linter} linter
+ * @param {LinterConfig} config
  * @param {string} filePath
  * @param {string} fileContents
- * @param {LinterOptions} linterOptions
+ */
+export function lint(linter: Linter, config: LinterConfig, filePath: string, fileContents: string): void {
+  linter.lint(filePath, fileContents, config as any);
+}
+
+/**
+ * Get the linter result
+ * @param {Linter} linter
  * @return {LintResult}
  */
-export function lint(context: BuildContext, tsConfig: string, tsLintConfig: string | null, filePath: string, fileContents: string, linterOptions?: LinterOptions): LintResult {
-  const linter = getLinter(context, tsConfig);
-  const configuration = Configuration.findConfiguration(tsLintConfig, filePath);
-  linter.lint(filePath, fileContents, Object.assign(configuration.results, isObject(linterOptions) ? {linterOptions} : {}));
+export function getLintResult(linter: Linter): LintResult {
   return linter.getResult();
 }
 
 
 /**
  * Type check a TS program
  * @param {BuildContext} context
- * @param {string} tsConfig
+ * @param {Program} program
  * @param {LinterOptions} linterOptions
  * @return {Promise<Diagnostic[]>}
  */
-export function typeCheck(context: BuildContext, tsConfig: string, linterOptions?: LinterOptions): Promise<Diagnostic[]> {
+export function typeCheck(context: BuildContext, program: Program, linterOptions?: LinterOptions): Promise<Diagnostic[]> {
   if (isObject(linterOptions) && linterOptions.typeCheck) {
-    const program = createProgram(context, tsConfig);
     return Promise.resolve(getPreEmitDiagnostics(program));
   }
   return Promise.resolve([]);
 }
 
 
 /**
- * Create a TS program based on the BuildContext {srcDir} or TS config file path (if provided)
+ * Create a TS program based on the BuildContext {rootDir} or TS config file path (if provided)
  * @param {BuildContext} context
  * @param {string} tsConfig
  * @return {Program}
@@ -57,24 +63,33 @@ export function createProgram(context: BuildContext, tsConfig: string): Program
 /**
  * Get all files that are sourced in TS config
  * @param {BuildContext} context
- * @param {string} tsConfig
+ * @param {Program} program
  * @return {Array<string>}
  */
-export function getFileNames(context: BuildContext, tsConfig: string): string[] {
-  const program = createProgram(context, tsConfig);
+export function getFileNames(context: BuildContext, program: Program): string[] {
   return Linter.getFileNames(program);
 }
 
 
 /**
- * Get linter
- * @param {BuildContext} context
- * @param {string} tsConfig
+ * Get lint configuration
+ * @param {string} tsLintConfig
+ * @param {LinterOptions} linterOptions
  * @return {Linter}
  */
-export function getLinter(context: BuildContext, tsConfig: string): Linter {
-  const program = createProgram(context, tsConfig);
+export function getTsLintConfig(tsLintConfig: string, linterOptions?: LinterOptions): LinterConfig {
+  const config = Configuration.loadConfigurationFromPath(tsLintConfig);
+  Object.assign(config, isObject(linterOptions) ? {linterOptions} : {});
+  return config;
+}
 
+/**
+ * Create a TS linter
+ * @param {BuildContext} context
+ * @param {Program} program
+ * @return {Linter}
+ */
+export function createLinter(context: BuildContext, program: Program): Linter {
   return new Linter({
     fix: false
   }, program);