From 45b6df511ff31e7a111d223a6239a3215e87fe53 Mon Sep 17 00:00:00 2001 From: Alan Agius <alan.agius4@gmail.com> Date: Fri, 30 Nov 2018 11:50:01 +0100 Subject: [PATCH] fix(@angular-devkit/build-angular): lint non human readable formatters produces invalid output fixes #12674 --- .../angular/cli/models/architect-command.ts | 8 +++++-- .../angular_devkit/architect/src/architect.ts | 1 + .../architect/testing/run-target-spec.ts | 8 +++++-- .../build_angular/src/tslint/index.ts | 16 +++++++++---- .../test/tslint/works_spec_large.ts | 24 ++++++++++++++++++- 5 files changed, 47 insertions(+), 10 deletions(-) diff --git a/packages/angular/cli/models/architect-command.ts b/packages/angular/cli/models/architect-command.ts index 61973decec..d847788f31 100644 --- a/packages/angular/cli/models/architect-command.ts +++ b/packages/angular/cli/models/architect-command.ts @@ -7,6 +7,7 @@ */ import { Architect, + BuilderContext, TargetSpecifier, } from '@angular-devkit/architect'; import { experimental, json, schema, tags } from '@angular-devkit/core'; @@ -185,8 +186,11 @@ export abstract class ArchitectCommand< return 1; } const realBuilderConf = this._architect.getBuilderConfiguration({ ...targetSpec, overrides }); - - const result = await this._architect.run(realBuilderConf, { logger: this.logger }).toPromise(); + const builderContext: Partial<BuilderContext> = { + logger: this.logger, + targetSpecifier: targetSpec, + }; + const result = await this._architect.run(realBuilderConf, builderContext).toPromise(); return result.success ? 0 : 1; } diff --git a/packages/angular_devkit/architect/src/architect.ts b/packages/angular_devkit/architect/src/architect.ts index fa81d51424..c83994aa33 100644 --- a/packages/angular_devkit/architect/src/architect.ts +++ b/packages/angular_devkit/architect/src/architect.ts @@ -64,6 +64,7 @@ export interface BuilderContext { host: virtualFs.Host<{}>; workspace: experimental.workspace.Workspace; architect: Architect; + targetSpecifier?: TargetSpecifier; } // TODO: use Build Event Protocol diff --git a/packages/angular_devkit/architect/testing/run-target-spec.ts b/packages/angular_devkit/architect/testing/run-target-spec.ts index 1224589e4e..20688782b5 100644 --- a/packages/angular_devkit/architect/testing/run-target-spec.ts +++ b/packages/angular_devkit/architect/testing/run-target-spec.ts @@ -9,7 +9,7 @@ import { experimental, logging, normalize } from '@angular-devkit/core'; import { Observable, merge, throwError, timer } from 'rxjs'; import { concatMap, concatMapTo, finalize, takeUntil } from 'rxjs/operators'; -import { Architect, BuildEvent, TargetSpecifier } from '../src'; +import { Architect, BuildEvent, BuilderContext, TargetSpecifier } from '../src'; import { TestProjectHost } from './test-project-host'; export const DefaultTimeout = 45000; @@ -33,9 +33,13 @@ export function runTargetSpec( }); // Load the workspace from the root of the host, then run a target. + const builderContext: Partial<BuilderContext> = { + logger, + targetSpecifier: targetSpec, + }; const runArchitect$ = workspace.loadWorkspaceFromHost(workspaceFile).pipe( concatMap(ws => new Architect(ws).loadArchitect()), - concatMap(arch => arch.run(arch.getBuilderConfiguration(targetSpec), { logger })), + concatMap(arch => arch.run(arch.getBuilderConfiguration(targetSpec), builderContext)), finalize(() => finalizeCB()), ); diff --git a/packages/angular_devkit/build_angular/src/tslint/index.ts b/packages/angular_devkit/build_angular/src/tslint/index.ts index 9fe76ac1fd..6fcf8b1185 100644 --- a/packages/angular_devkit/build_angular/src/tslint/index.ts +++ b/packages/angular_devkit/build_angular/src/tslint/index.ts @@ -60,6 +60,17 @@ export default class TslintBuilder implements Builder<TslintBuilderOptions> { const root = this.context.workspace.root; const systemRoot = getSystemPath(root); const options = builderConfig.options; + const targetSpecifier = this.context.targetSpecifier; + const projectName = targetSpecifier && targetSpecifier.project || ''; + + // Print formatter output directly for non human-readable formats. + if (!['prose', 'verbose', 'stylish'].includes(options.format)) { + options.silent = true; + } + + if (!options.silent) { + this.context.logger.info(`Linting ${JSON.stringify(projectName)}...`); + } if (!options.tsConfig && options.typeCheck) { throw new Error('A "project" must be specified to enable type checking.'); @@ -116,11 +127,6 @@ export default class TslintBuilder implements Builder<TslintBuilderOptions> { } } - // Print formatter output directly for non human-readable formats. - if (['prose', 'verbose', 'stylish'].indexOf(options.format) == -1) { - options.silent = true; - } - if (result.warningCount > 0 && !options.silent) { this.context.logger.warn('Lint warnings found in the listed files.'); } diff --git a/packages/angular_devkit/build_angular/test/tslint/works_spec_large.ts b/packages/angular_devkit/build_angular/test/tslint/works_spec_large.ts index 69192e5457..583557cdf3 100644 --- a/packages/angular_devkit/build_angular/test/tslint/works_spec_large.ts +++ b/packages/angular_devkit/build_angular/test/tslint/works_spec_large.ts @@ -12,7 +12,7 @@ import { tap } from 'rxjs/operators'; import { TslintBuilderOptions } from '../../src'; import { host, tslintTargetSpec } from '../utils'; - +// tslint:disable-next-line:no-big-function describe('Tslint Target', () => { const filesWithErrors = { 'src/foo.ts': 'const foo = "";\n' }; @@ -25,6 +25,28 @@ describe('Tslint Target', () => { ).toPromise().then(done, done.fail); }, 30000); + it(`should show project name when formatter is human readable`, (done) => { + const logger = new TestLogger('lint-info'); + + runTargetSpec(host, tslintTargetSpec, undefined, undefined, logger).pipe( + tap((buildEvent) => expect(buildEvent.success).toBe(true)), + tap(() => { + expect(logger.includes('Linting "app"...')).toBe(true); + }), + ).toPromise().then(done, done.fail); + }, 30000); + + it(`should not show project name when formatter is non human readable`, (done) => { + const logger = new TestLogger('lint-info'); + + runTargetSpec(host, tslintTargetSpec, { format: 'checkstyle' }, undefined, logger).pipe( + tap((buildEvent) => expect(buildEvent.success).toBe(true)), + tap(() => { + expect(logger.includes('Linting "app"...')).toBe(false); + }), + ).toPromise().then(done, done.fail); + }, 30000); + it('should report lint error once', (done) => { host.writeMultipleFiles({'src/app/app.component.ts': 'const foo = "";\n' }); const logger = new TestLogger('lint-error');