Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit cc3d37c

Browse files
alan-agius4alexeagle
authored andcommittedDec 6, 2018
fix(@angular-devkit/build-angular): lint non human readable formatters produces invalid output
fixes #12674
·
v7.1.4v7.1.2
1 parent 64583ba commit cc3d37c

File tree

5 files changed

+47
-10
lines changed

5 files changed

+47
-10
lines changed
 

‎packages/angular/cli/models/architect-command.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import {
99
Architect,
1010
BuilderConfiguration,
11+
BuilderContext,
1112
TargetSpecifier,
1213
} from '@angular-devkit/architect';
1314
import { experimental, json, schema, tags } from '@angular-devkit/core';
@@ -147,8 +148,11 @@ export abstract class ArchitectCommand<
147148
return 1;
148149
}
149150
const realBuilderConf = this._architect.getBuilderConfiguration({ ...targetSpec, overrides });
150-
151-
const result = await this._architect.run(realBuilderConf, { logger: this.logger }).toPromise();
151+
const builderContext: Partial<BuilderContext> = {
152+
logger: this.logger,
153+
targetSpecifier: targetSpec,
154+
};
155+
const result = await this._architect.run(realBuilderConf, builderContext).toPromise();
152156

153157
return result.success ? 0 : 1;
154158
}

‎packages/angular_devkit/architect/src/architect.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export interface BuilderContext {
6464
host: virtualFs.Host<{}>;
6565
workspace: experimental.workspace.Workspace;
6666
architect: Architect;
67+
targetSpecifier?: TargetSpecifier;
6768
}
6869

6970
// TODO: use Build Event Protocol

‎packages/angular_devkit/architect/testing/run-target-spec.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import { experimental, logging, normalize } from '@angular-devkit/core';
1010
import { Observable, merge, throwError, timer } from 'rxjs';
1111
import { concatMap, concatMapTo, finalize, takeUntil } from 'rxjs/operators';
12-
import { Architect, BuildEvent, TargetSpecifier } from '../src';
12+
import { Architect, BuildEvent, BuilderContext, TargetSpecifier } from '../src';
1313
import { TestProjectHost } from './test-project-host';
1414

1515
export const DefaultTimeout = 45000;
@@ -33,9 +33,13 @@ export function runTargetSpec(
3333
});
3434

3535
// Load the workspace from the root of the host, then run a target.
36+
const builderContext: Partial<BuilderContext> = {
37+
logger,
38+
targetSpecifier: targetSpec,
39+
};
3640
const runArchitect$ = workspace.loadWorkspaceFromHost(workspaceFile).pipe(
3741
concatMap(ws => new Architect(ws).loadArchitect()),
38-
concatMap(arch => arch.run(arch.getBuilderConfiguration(targetSpec), { logger })),
42+
concatMap(arch => arch.run(arch.getBuilderConfiguration(targetSpec), builderContext)),
3943
finalize(() => finalizeCB()),
4044
);
4145

‎packages/angular_devkit/build_angular/src/tslint/index.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,17 @@ export default class TslintBuilder implements Builder<TslintBuilderOptions> {
6060
const root = this.context.workspace.root;
6161
const systemRoot = getSystemPath(root);
6262
const options = builderConfig.options;
63+
const targetSpecifier = this.context.targetSpecifier;
64+
const projectName = targetSpecifier && targetSpecifier.project || '';
65+
66+
// Print formatter output directly for non human-readable formats.
67+
if (!['prose', 'verbose', 'stylish'].includes(options.format)) {
68+
options.silent = true;
69+
}
70+
71+
if (!options.silent) {
72+
this.context.logger.info(`Linting ${JSON.stringify(projectName)}...`);
73+
}
6374

6475
if (!options.tsConfig && options.typeCheck) {
6576
throw new Error('A "project" must be specified to enable type checking.');
@@ -116,11 +127,6 @@ export default class TslintBuilder implements Builder<TslintBuilderOptions> {
116127
}
117128
}
118129

119-
// Print formatter output directly for non human-readable formats.
120-
if (['prose', 'verbose', 'stylish'].indexOf(options.format) == -1) {
121-
options.silent = true;
122-
}
123-
124130
if (result.warningCount > 0 && !options.silent) {
125131
this.context.logger.warn('Lint warnings found in the listed files.');
126132
}

‎packages/angular_devkit/build_angular/test/tslint/works_spec_large.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { tap } from 'rxjs/operators';
1212
import { TslintBuilderOptions } from '../../src';
1313
import { host, tslintTargetSpec } from '../utils';
1414

15-
15+
// tslint:disable-next-line:no-big-function
1616
describe('Tslint Target', () => {
1717
const filesWithErrors = { 'src/foo.ts': 'const foo = "";\n' };
1818

@@ -25,6 +25,28 @@ describe('Tslint Target', () => {
2525
).toPromise().then(done, done.fail);
2626
}, 30000);
2727

28+
it(`should show project name when formatter is human readable`, (done) => {
29+
const logger = new TestLogger('lint-info');
30+
31+
runTargetSpec(host, tslintTargetSpec, undefined, undefined, logger).pipe(
32+
tap((buildEvent) => expect(buildEvent.success).toBe(true)),
33+
tap(() => {
34+
expect(logger.includes('Linting "app"...')).toBe(true);
35+
}),
36+
).toPromise().then(done, done.fail);
37+
}, 30000);
38+
39+
it(`should not show project name when formatter is non human readable`, (done) => {
40+
const logger = new TestLogger('lint-info');
41+
42+
runTargetSpec(host, tslintTargetSpec, { format: 'checkstyle' }, undefined, logger).pipe(
43+
tap((buildEvent) => expect(buildEvent.success).toBe(true)),
44+
tap(() => {
45+
expect(logger.includes('Linting "app"...')).toBe(false);
46+
}),
47+
).toPromise().then(done, done.fail);
48+
}, 30000);
49+
2850
it('should report lint error once', (done) => {
2951
host.writeMultipleFiles({'src/app/app.component.ts': 'const foo = "";\n' });
3052
const logger = new TestLogger('lint-error');

0 commit comments

Comments
 (0)
Please sign in to comment.