feat(lint): new option to have stand alone lint bail

new option to have stand alone lint bail

README.md

@@ -124,8 +124,7 @@ npm run build --rollup ./config/rollup.config.js
 | output js map file | `ionic_output_js_map_file_name`  | `--outputJsMapFileName` | `main.js.map` | name of js source map file generated in `buildDir` |
 | output css file | `ionic_output_css_file_name`  | `--outputCssFileName` | `main.css` | name of css file generated in `buildDir` |
 | output css map file | `ionic_output_css_map_file_name`  | `--outputCssMapFileName` | `main.css.map` | name of css source map file generated in `buildDir` |
-
-
+| bail on lint error | `ionic_bail_on_lint_error`  | `--bailOnLintError` | `null` | Set to `true` to make stand-alone lint commands fail with non-zero status code |
 
 
 
@@ -157,7 +156,7 @@ These environment variables are automatically set to [Node's `process.env`](http
 | `IONIC_OUTPUT_CSS_MAP_FILE_NAME` | The file name of the generated css source map file             |
 | `IONIC_WEBPACK_FACTORY`    | The absolute path to Ionic's `webpack-factory` script                |
 | `IONIC_WEBPACK_LOADER`     | The absolute path to Ionic's custom webpack loader                   |
-
+| `IONIC_BAIL_ON_LINT_ERROR`     | Boolean determining whether to exit with a non-zero status code on error |
 
 
 The `process.env.IONIC_ENV` environment variable can be used to test whether it is a `prod` or `dev` build, which automatically gets set by any command. By default the `build` and `serve` tasks produce `dev` builds (a build that does not include Ahead of Time (AoT) compilation or minification). To force a `prod` build you should use the `--prod` command line flag.
@@ -190,6 +189,19 @@ Example NPM Script:
 
 ## Tips
 1. The Webpack `devtool` setting is driven by the `ionic_source_map_type` variable. It defaults to `source-map` for the best quality source map. Developers can enable significantly faster builds by setting `ionic_source_map_type` to `eval`.
+2. By default, the `lint` command does not exit with a non-zero status code on error. To enable this, pass `--bailOnLintError true` to the command.
+
+```
+"scripts" : {
+  ...
+  "lint": "ionic-app-scripts lint"
+  ...
+}
+```
+
+```
+npm run lint --bailOnLintError true
+```
 
 
 ## The Stack

src/build.ts

@@ -123,7 +123,7 @@ function buildProject(context: BuildContext) {
     .then(() => {
       // kick off the tslint after everything else
       // nothing needs to wait on its completion
-      lint(context);
+      return lint(context);
     })
     .catch(err => {
       throw new BuildError(err);

src/lint.spec.ts

@@ -0,0 +1,57 @@
+import *  as lint from './lint';
+import * as workerClient from './worker-client';
+import * as Constants from './util/constants';
+
+let originalEnv = process.env;
+
+describe('lint task', () => {
+  describe('lint', () => {
+
+    beforeEach(() => {
+      originalEnv = process.env;
+      process.env = {};
+    });
+
+    afterEach(() => {
+      process.env = originalEnv;
+    });
+
+    it('Should return resolved promise', (done: Function) => {
+      // arrange
+      spyOn(workerClient, workerClient.runWorker.name).and.returnValue(Promise.resolve());
+      // act
+      const promise = lint.lint(null);
+
+      // assert
+      promise.then(() => {
+        done();
+      });
+    });
+
+    it('Should return resolved promise when bail on error is not set', (done: Function) => {
+      // arrange
+      spyOn(workerClient, workerClient.runWorker.name).and.returnValue(Promise.reject(new Error('Simulating an error')));
+      // act
+      const promise = lint.lint(null);
+
+      // assert
+      promise.then(() => {
+        done();
+      });
+    });
+
+    it('Should return rejected promise when bail on error is set', (done: Function) => {
+
+      spyOn(workerClient, workerClient.runWorker.name).and.returnValue(Promise.reject(new Error('Simulating an error')));
+      process.env[Constants.ENV_BAIL_ON_LINT_ERROR] = 'true';
+
+      // act
+      const promise = lint.lint(null);
+
+      // assert
+      promise.catch(() => {
+        done();
+      });
+    });
+  });
+});

src/lint.ts

@@ -3,6 +3,8 @@ import { BuildContext, ChangedFile, TaskInfo } from './util/interfaces';
 import { BuildError } from './util/errors';
 import { createProgram, findConfiguration, getFileNames } from 'tslint';
 import { getUserConfigFile } from './util/config';
+import * as Constants from './util/constants';
+import { readFileAsync, getBooleanPropertyValue } from './util/helpers';
 import { join } from 'path';
 import { Logger } from './logger/logger';
 import { printDiagnostics, DiagnosticsType } from './logger/logger-diagnostics';
@@ -14,9 +16,17 @@ import * as ts from 'typescript';
 
 
 export function lint(context: BuildContext, configFile?: string) {
+  const logger = new Logger('lint');
+  console.log('process.env: ', process.env);
   return runWorker('lint', 'lintWorker', context, configFile)
+    .then(() => {
+      logger.finish();
+    })
     .catch(err => {
-      throw new BuildError(err);
+      if (getBooleanPropertyValue(Constants.ENV_BAIL_ON_LINT_ERROR)){
+        throw logger.fail(err);
+      }
+      logger.finish();
     });
 }
 
@@ -25,7 +35,7 @@ export function lintWorker(context: BuildContext, configFile: string) {
   return getLintConfig(context, configFile).then(configFile => {
     // there's a valid tslint config, let's continue
     return lintApp(context, configFile);
-  }).catch(() => {});
+  });
 }
 
 
@@ -62,7 +72,7 @@ function lintApp(context: BuildContext, configFile: string) {
     return lintFile(context, program, file);
   });
 
-  return Promise.all(promises);
+  return Promise.all(promises)
 }
 
 function lintFiles(context: BuildContext, program: ts.Program, filePaths: string[]) {
@@ -74,45 +84,28 @@ function lintFiles(context: BuildContext, program: ts.Program, filePaths: string
 }
 
 function lintFile(context: BuildContext, program: ts.Program, filePath: string): Promise<any> {
-  return new Promise((resolve) => {
-
+  return Promise.resolve().then(() => {
     if (isMpegFile(filePath)) {
-      // silly .ts files actually being video files
-      resolve();
-      return;
+      throw new Error(`${filePath} is not a valid TypeScript file`);
     }
-
-    fs.readFile(filePath, 'utf8', (err, contents) => {
-      if (err) {
-        // don't care if there was an error
-        // let's just move on with our lives
-        resolve();
-        return;
-      }
-
-      try {
-        const configuration = findConfiguration(null, filePath);
-
-        const linter = new Linter(filePath, contents, {
-          configuration: configuration,
-          formatter: null,
-          formattersDirectory: null,
-          rulesDirectory: null,
-        }, program);
-
-        const lintResult = linter.lint();
-        if (lintResult && lintResult.failures) {
-          const diagnostics = runTsLintDiagnostics(context, <any>lintResult.failures);
-          printDiagnostics(context, DiagnosticsType.TsLint, diagnostics, true, false);
-        }
-
-      } catch (e) {
-        Logger.debug(`Linter ${e}`);
-      }
-
-      resolve();
-    });
-
+    return readFileAsync(filePath);
+  }).then((fileContents: string) => {
+    const configuration = findConfiguration(null, filePath);
+
+    const linter = new Linter(filePath, fileContents, {
+      configuration: configuration,
+      formatter: null,
+      formattersDirectory: null,
+      rulesDirectory: null,
+    }, program);
+
+    const lintResult = linter.lint();
+    if (lintResult && lintResult.failures && lintResult.failures.length) {
+      const diagnostics = runTsLintDiagnostics(context, <any>lintResult.failures);
+      printDiagnostics(context, DiagnosticsType.TsLint, diagnostics, true, false);
+      throw new BuildError(`${filePath} did not pass TSLint`);
+    }
+    return lintResult;
   });
 }
 

src/util/config.ts

@@ -120,6 +120,9 @@ export function generateContext(context?: BuildContext): BuildContext {
   const aotWriteToDisk = getConfigValue(context, '--aotWriteToDisk', null, Constants.ENV_AOT_WRITE_TO_DISK, Constants.ENV_AOT_WRITE_TO_DISK.toLowerCase(), null);
   setProcessEnvVar(Constants.ENV_AOT_WRITE_TO_DISK, aotWriteToDisk);
 
+  const bailOnLintError = getConfigValue(context, '--bailOnLintError', null, Constants.ENV_BAIL_ON_LINT_ERROR, Constants.ENV_BAIL_ON_LINT_ERROR.toLowerCase(), null);
+  setProcessEnvVar(Constants.ENV_BAIL_ON_LINT_ERROR, bailOnLintError);
+
   if (!isValidBundler(context.bundler)) {
     context.bundler = bundlerStrategy(context);
   }

src/util/constants.ts

@@ -38,6 +38,7 @@ export const ENV_OUTPUT_CSS_MAP_FILE_NAME = 'IONIC_OUTPUT_CSS_MAP_FILE_NAME';
 export const ENV_WEBPACK_FACTORY = 'IONIC_WEBPACK_FACTORY';
 export const ENV_WEBPACK_LOADER = 'IONIC_WEBPACK_LOADER';
 export const ENV_AOT_WRITE_TO_DISK = 'IONIC_AOT_WRITE_TO_DISK';
+export const ENV_BAIL_ON_LINT_ERROR = 'IONIC_BAIL_ON_LINT_ERROR';
 
 export const BUNDLER_ROLLUP = 'rollup';
 export const BUNDLER_WEBPACK = 'webpack';

src/util/helpers.ts

@@ -217,3 +217,8 @@ export function stringSplice(source: string, startIndex: number, numToDelete: nu
 export function toUnixPath(filePath: string) {
   return filePath.replace(/\\/g, '/');
 }
+
+export function getBooleanPropertyValue(propertyName: string) {
+  const result = process.env[propertyName];
+  return result === 'true';
+}