From ad2e7890717d6027ac6580f05539947fb03af8b7 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Tue, 17 Apr 2018 00:10:03 -0600 Subject: [PATCH] Pass all listed file names to formatter. For #3837, allows the Checkstyle and JUnit formatters to output the files that did not have any failures. Adding a third argument to `IFormatter.format` seems like a more reasonable option than changing its signature and having a breaking change. --- src/formatters/checkstyleFormatter.ts | 55 ++++++++++----------- src/language/formatter/abstractFormatter.ts | 2 +- src/language/formatter/formatter.ts | 3 +- src/linter.ts | 4 +- test/formatters/checkstyleFormatterTests.ts | 8 ++- 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/formatters/checkstyleFormatter.ts b/src/formatters/checkstyleFormatter.ts index 2d85ff805f0..c16d8e72e66 100644 --- a/src/formatters/checkstyleFormatter.ts +++ b/src/formatters/checkstyleFormatter.ts @@ -27,7 +27,7 @@ export class Formatter extends AbstractFormatter { formatterName: "checkstyle", description: "Formats errors as through 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` @@ -39,36 +39,35 @@ 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 (fileName in groupedFailures) { + groupedFailures[fileName].push(failure); + } else { + groupedFailures[fileName] = [failure]; } } - output += ""; - return output; + const formattedFiles = fileNames.map((fileName) => { + const formattedFailures = fileName in groupedFailures ? 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 d9de37a7c46..e6aa56ff24f 100644 --- a/src/language/formatter/abstractFormatter.ts +++ b/src/language/formatter/abstractFormatter.ts @@ -20,7 +20,7 @@ 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 f3c2b60e4bd..dd880d18835 100644 --- a/src/language/formatter/formatter.ts +++ b/src/language/formatter/formatter.ts @@ -55,6 +55,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 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 f03d10a3699..e7df0e7ca02 100644 --- a/src/linter.ts +++ b/src/linter.ts @@ -51,6 +51,7 @@ export class Linter { private failures: RuleFailure[] = []; private fixes: RuleFailure[] = []; + private readonly fileNames: string[] = []; /** * Creates a TypeScript program object from a tsconfig.json file path and optional project directory. @@ -116,6 +117,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); @@ -156,7 +158,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 2b139e0de4a..dae74a7a9db 100644 --- a/test/formatters/checkstyleFormatterTests.ts +++ b/test/formatters/checkstyleFormatterTests.ts @@ -24,6 +24,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; @@ -46,6 +47,7 @@ describe("Checkstyle Formatter", () => { createFailure(sourceFile2, 0, 1, "first failure", "first-name", undefined, "error"), createFailure(sourceFile2, 2, 3, "&<>'\" should be escaped", "escape", undefined, "warning"), createFailure(sourceFile2, maxPosition2 - 1, maxPosition2, "last failure", "last-name", undefined, "warning"), + // no failures for file 3 ]; // tslint:disable max-line-length const expectedResult = @@ -61,13 +63,15 @@ 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, ''); }); });