Pass all linted file names to formatter (#3838)

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`
         <?xml version="1.0" encoding="utf-8"?>
         <checkstyle version="4.3">
@@ -38,39 +38,38 @@ export class Formatter extends AbstractFormatter {
     };
     /* tslint:enable:object-literal-sort-keys */
 
-    public format(failures: RuleFailure[]): string {
-        let output = '<?xml version="1.0" encoding="utf-8"?><checkstyle version="4.3">';
-
-        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 += "</file>";
-                    }
-                    previousFilename = failure.getFileName();
-                    output += `<file name="${this.escapeXml(failure.getFileName())}">`;
-                }
-                output += `<error line="${failure.getStartPosition().getLineAndCharacter().line +
-                    1}" `;
-                output += `column="${failure.getStartPosition().getLineAndCharacter().character +
-                    1}" `;
-                output += `severity="${severity}" `;
-                output += `message="${this.escapeXml(failure.getFailure())}" `;
-                // checkstyle parser wants "source" to have structure like <anything>dot<category>dot<type>
-                output += `source="failure.tslint.${this.escapeXml(failure.getRuleName())}" />`;
-            }
-            if (previousFilename !== null) {
-                output += "</file>";
+    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 += "</checkstyle>";
-        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 `<file name="${this.escapeXml(fileName)}">${joinedFailures}</file>`;
+        });
+        const joinedFiles = formattedFiles.join("");
+        return `<?xml version="1.0" encoding="utf-8"?><checkstyle version="4.3">${joinedFiles}</checkstyle>`;
+    }
+
+    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 <anything>dot<category>dot<type>
+        const source = `failure.tslint.${this.escapeXml(failure.getRuleName())}`;
+
+        return `<error line="${line}" column="${column}" severity="${severity}" message="${message}" source="${source}" />`;
     }
 
     private escapeXml(str: string): string {

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);

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;
 }

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 {

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 = `<?xml version="1.0" encoding="utf-8"?>
@@ -93,13 +95,18 @@ describe("Checkstyle Formatter", () => {
                 <error line="1" column="3" severity="warning" message="&amp;&lt;&gt;&#39;&quot; should be escaped" source="failure.tslint.escape" />
                 <error line="6" column="3" severity="warning" message="last failure" source="failure.tslint.last-name" />
             </file>
+            <file name="${TEST_FILE_3}">
+            </file>
             </checkstyle>`.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,
             '<?xml version="1.0" encoding="utf-8"?><checkstyle version="4.3"></checkstyle>',