From b3bb906358392ff3bb207526ff7900423f73cd2f Mon Sep 17 00:00:00 2001 From: Dan Bucholtz Date: Mon, 16 Jan 2017 18:02:35 -0600 Subject: [PATCH] feat(lint): new option to have stand alone lint bail new option to have stand alone lint bail --- README.md | 18 +++++++++-- src/build.ts | 2 +- src/lint.spec.ts | 57 +++++++++++++++++++++++++++++++++ src/lint.ts | 73 +++++++++++++++++++------------------------ src/util/config.ts | 3 ++ src/util/constants.ts | 1 + src/util/helpers.ts | 5 +++ 7 files changed, 115 insertions(+), 44 deletions(-) create mode 100644 src/lint.spec.ts diff --git a/README.md b/README.md index 728c5721..862e79f8 100644 --- a/README.md +++ b/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 diff --git a/src/build.ts b/src/build.ts index 91de7fdd..570644ee 100644 --- a/src/build.ts +++ b/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); diff --git a/src/lint.spec.ts b/src/lint.spec.ts new file mode 100644 index 00000000..8a9ca1f2 --- /dev/null +++ b/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(); + }); + }); + }); +}); diff --git a/src/lint.ts b/src/lint.ts index 1a1572db..d35de4a7 100644 --- a/src/lint.ts +++ b/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 { - 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, 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, lintResult.failures); + printDiagnostics(context, DiagnosticsType.TsLint, diagnostics, true, false); + throw new BuildError(`${filePath} did not pass TSLint`); + } + return lintResult; }); } diff --git a/src/util/config.ts b/src/util/config.ts index aabe021c..12a93010 100644 --- a/src/util/config.ts +++ b/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); } diff --git a/src/util/constants.ts b/src/util/constants.ts index 8e81f3ad..0f5ac678 100644 --- a/src/util/constants.ts +++ b/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'; diff --git a/src/util/helpers.ts b/src/util/helpers.ts index 60dbb452..0fd6fafd 100644 --- a/src/util/helpers.ts +++ b/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'; +} \ No newline at end of file