From 9000479b69c1f4808acf1dee284b47fbbbfbef13 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Sat, 23 Feb 2019 13:57:06 -0700 Subject: [PATCH] Pass all linted file names to formatter (#3838) --- src/formatters/checkstyleFormatter.ts | 61 ++++++++++----------- src/language/formatter/abstractFormatter.ts | 6 +- src/language/formatter/formatter.ts | 3 +- src/linter.ts | 4 +- test/formatters/checkstyleFormatterTests.ts | 11 +++- 5 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/formatters/checkstyleFormatter.ts b/src/formatters/checkstyleFormatter.ts index 36c224fc970..ce5746f8948 100644 --- a/src/formatters/checkstyleFormatter.ts +++ b/src/formatters/checkstyleFormatter.ts @@ -26,7 +26,7 @@ export class Formatter extends AbstractFormatter { formatterName: "checkstyle", description: "Formats errors as though they were Checkstyle output.", descriptionDetails: Utils.dedent` - Imitates the XMLLogger from Checkstyle 4.3. All failures have the 'warning' severity.`, + Imitates the XMLLogger from Checkstyle 4.3. All failures have the 'warning' severity. Files without errors are still included.`, sample: Utils.dedent` @@ -38,39 +38,38 @@ export class Formatter extends AbstractFormatter { }; /* tslint:enable:object-literal-sort-keys */ - public format(failures: RuleFailure[]): string { - let output = ''; - - if (failures.length !== 0) { - const failuresSorted = failures.sort((a, b) => - a.getFileName().localeCompare(b.getFileName()), - ); - let previousFilename: string | null = null; - for (const failure of failuresSorted) { - const severity = failure.getRuleSeverity(); - if (failure.getFileName() !== previousFilename) { - if (previousFilename !== null) { - output += ""; - } - previousFilename = failure.getFileName(); - output += ``; - } - output += `dotdot - output += `source="failure.tslint.${this.escapeXml(failure.getRuleName())}" />`; - } - if (previousFilename !== null) { - output += ""; + public format(failures: RuleFailure[], _fixes: RuleFailure[], fileNames: string[]): string { + const groupedFailures: { [k: string]: RuleFailure[] } = {}; + for (const failure of failures) { + const fileName = failure.getFileName(); + if (groupedFailures[fileName] !== undefined) { + groupedFailures[fileName].push(failure); + } else { + groupedFailures[fileName] = [failure]; } } - output += ""; - return output; + const formattedFiles = fileNames.map(fileName => { + const formattedFailures = + groupedFailures[fileName] !== undefined + ? groupedFailures[fileName].map(f => this.formatFailure(f)) + : []; + const joinedFailures = formattedFailures.join(""); // may be empty + return `${joinedFailures}`; + }); + const joinedFiles = formattedFiles.join(""); + return `${joinedFiles}`; + } + + private formatFailure(failure: RuleFailure): string { + const line = failure.getStartPosition().getLineAndCharacter().line + 1; + const column = failure.getStartPosition().getLineAndCharacter().character + 1; + const severity = failure.getRuleSeverity(); + const message = this.escapeXml(failure.getFailure()); + // checkstyle parser wants "source" to have structure like dotdot + const source = `failure.tslint.${this.escapeXml(failure.getRuleName())}`; + + return ``; } private escapeXml(str: string): string { diff --git a/src/language/formatter/abstractFormatter.ts b/src/language/formatter/abstractFormatter.ts index 28b262714fb..69529774d27 100644 --- a/src/language/formatter/abstractFormatter.ts +++ b/src/language/formatter/abstractFormatter.ts @@ -21,7 +21,11 @@ import { IFormatter, IFormatterMetadata } from "./formatter"; export abstract class AbstractFormatter implements IFormatter { public static metadata: IFormatterMetadata; - public abstract format(failures: RuleFailure[]): string; + public abstract format( + failures: RuleFailure[], + fixes: RuleFailure[], + fileNames: string[], + ): string; protected sortFailures(failures: RuleFailure[]): RuleFailure[] { return failures.slice().sort(RuleFailure.compare); diff --git a/src/language/formatter/formatter.ts b/src/language/formatter/formatter.ts index a13a92cd2fa..1314ce66811 100644 --- a/src/language/formatter/formatter.ts +++ b/src/language/formatter/formatter.ts @@ -60,6 +60,7 @@ export interface IFormatter { * Formats linter results * @param failures Linter failures that were not fixed * @param fixes Fixed linter failures. Available when the `--fix` argument is used on the command line + * @param fileNames All of the file paths that were linted */ - format(failures: RuleFailure[], fixes?: RuleFailure[]): string; + format(failures: RuleFailure[], fixes?: RuleFailure[], fileNames?: string[]): string; } diff --git a/src/linter.ts b/src/linter.ts index 42d5b8e00f6..ceced932a88 100644 --- a/src/linter.ts +++ b/src/linter.ts @@ -118,6 +118,7 @@ export class Linter { private failures: RuleFailure[] = []; private fixes: RuleFailure[] = []; + private readonly fileNames: string[] = []; constructor(private readonly options: ILinterOptions, private program?: ts.Program) { if (typeof options !== "object") { @@ -139,6 +140,7 @@ export class Linter { if (isFileExcluded(fileName, configuration)) { return; } + this.fileNames.push(fileName); const sourceFile = this.getSourceFile(fileName, source); const isJs = /\.jsx?$/i.test(fileName); const enabledRules = this.getEnabledRules(configuration, isJs); @@ -191,7 +193,7 @@ export class Linter { } const formatter = new Formatter(); - const output = formatter.format(failures, this.fixes); + const output = formatter.format(failures, this.fixes, this.fileNames); const errorCount = errors.length; return { diff --git a/test/formatters/checkstyleFormatterTests.ts b/test/formatters/checkstyleFormatterTests.ts index 257c7710997..4374e2649f5 100644 --- a/test/formatters/checkstyleFormatterTests.ts +++ b/test/formatters/checkstyleFormatterTests.ts @@ -25,6 +25,7 @@ import { createFailure } from "./utils"; describe("Checkstyle Formatter", () => { const TEST_FILE_1 = "formatters/jsonFormatter.test.ts"; // reuse existing sample file const TEST_FILE_2 = "formatters/pmdFormatter.test.ts"; // reuse existing sample file + const TEST_FILE_3 = "formatters/proseFormatter.test.ts"; // reuse existing sample file let sourceFile1: ts.SourceFile; let sourceFile2: ts.SourceFile; let formatter: IFormatter; @@ -79,6 +80,7 @@ describe("Checkstyle Formatter", () => { undefined, "warning", ), + // no failures for file 3 ]; // tslint:disable max-line-length const expectedResult = ` @@ -93,13 +95,18 @@ describe("Checkstyle Formatter", () => { + + `.replace(/>\s+/g, ">"); // Remove whitespace between tags - assert.equal(formatter.format(failures), expectedResult); + assert.equal( + formatter.format(failures, [], [TEST_FILE_1, TEST_FILE_2, TEST_FILE_3]), + expectedResult, + ); }); it("handles no failures", () => { - const result = formatter.format([]); + const result = formatter.format([], [], []); assert.deepEqual( result, '',