Sort failures by failure's line and character (palantir#3345)

HyphnKnight · Apr 9, 2018 · d87100d · d87100d
1 parent 892a1ce
commit d87100d
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 8 deletions.
diff --git a/src/formatters/codeFrameFormatter.ts b/src/formatters/codeFrameFormatter.ts
@@ -50,6 +50,7 @@ export class Formatter extends AbstractFormatter {
         if (typeof failures[0] === "undefined") {
             return "\n";
         }
+        failures = this.sortFailures(failures);
 
         const outputLines: string[] = [];
 

diff --git a/src/formatters/proseFormatter.ts b/src/formatters/proseFormatter.ts
@@ -33,6 +33,7 @@ export class Formatter extends AbstractFormatter {
         if (failures.length === 0 && (fixes === undefined || fixes.length === 0)) {
             return "\n";
         }
+        failures = this.sortFailures(failures);
 
         const fixLines: string[] = [];
         if (fixes !== undefined) {

diff --git a/src/formatters/stylishFormatter.ts b/src/formatters/stylishFormatter.ts
@@ -39,6 +39,7 @@ export class Formatter extends AbstractFormatter {
     /* tslint:enable:object-literal-sort-keys */
 
     public format(failures: RuleFailure[]): string {
+        failures = this.sortFailures(failures);
         const outputLines = this.mapToMessages(failures);
 
         // Removes initial blank line

diff --git a/src/formatters/verboseFormatter.ts b/src/formatters/verboseFormatter.ts
@@ -31,6 +31,7 @@ export class Formatter extends AbstractFormatter {
     /* tslint:enable:object-literal-sort-keys */
 
     public format(failures: RuleFailure[]): string {
+        failures = this.sortFailures(failures);
         return `${this.mapToMessages(failures).join("\n")}\n`;
     }
 

diff --git a/src/language/formatter/abstractFormatter.ts b/src/language/formatter/abstractFormatter.ts
@@ -21,4 +21,8 @@ import { IFormatter, IFormatterMetadata } from "./formatter";
 export abstract class AbstractFormatter implements IFormatter {
     public static metadata: IFormatterMetadata;
     public abstract format(failures: RuleFailure[]): string;
+
+    protected sortFailures(failures: RuleFailure[]): RuleFailure[] {
+        return failures.slice().sort(RuleFailure.compare);
+    }
 }
diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts
@@ -244,6 +244,13 @@ export class RuleFailure {
     private rawLines: string;
     private ruleSeverity: RuleSeverity;
 
+    public static compare(a: RuleFailure, b: RuleFailure): number {
+        if (a.fileName !== b.fileName) {
+            return a.fileName < b.fileName ? -1 : 1;
+        }
+        return a.startPosition.getPosition() - b.startPosition.getPosition();
+    }
+
     constructor(private sourceFile: ts.SourceFile,
                 start: number,
                 end: number,

diff --git a/test/formatters/codeFrameFormatterTests.ts b/test/formatters/codeFrameFormatterTests.ts
@@ -82,6 +82,12 @@ describe("CodeFrame Formatter", () => {
             \u001b[90m 3 | \u001b[39m        private name\u001b[33m:\u001b[39m string\u001b[33m;\u001b[39m
             \u001b[90m 4 | \u001b[39m\u001b[0m
 
+            \u001b[31mfull failure\u001b[39m \u001b[90m(full-name)\u001b[39m
+            \u001b[0m\u001b[31m\u001b[1m>\u001b[22m\u001b[39m\u001b[90m 1 | \u001b[39mmodule \u001b[33mCodeFrameModule\u001b[39m {
+            \u001b[90m 2 | \u001b[39m    \u001b[36mexport\u001b[39m \u001b[36mclass\u001b[39m \u001b[33mCodeFrameClass\u001b[39m {
+            \u001b[90m 3 | \u001b[39m        private name\u001b[33m:\u001b[39m string\u001b[33m;\u001b[39m
+            \u001b[90m 4 | \u001b[39m\u001b[0m
+
             \u001b[31m&<>'\" should be escaped\u001b[39m \u001b[90m(escape)\u001b[39m
             \u001b[0m\u001b[31m\u001b[1m>\u001b[22m\u001b[39m\u001b[90m 1 | \u001b[39mmodule \u001b[33mCodeFrameModule\u001b[39m {
             \u001b[90m   | \u001b[39m \u001b[31m\u001b[1m^\u001b[22m\u001b[39m
@@ -96,12 +102,6 @@ describe("CodeFrame Formatter", () => {
             \u001b[90m    | \u001b[39m\u001b[31m\u001b[1m^\u001b[22m\u001b[39m
             \u001b[90m 10 | \u001b[39m\u001b[0m
 
-            \u001b[31mfull failure\u001b[39m \u001b[90m(full-name)\u001b[39m
-            \u001b[0m\u001b[31m\u001b[1m>\u001b[22m\u001b[39m\u001b[90m 1 | \u001b[39mmodule \u001b[33mCodeFrameModule\u001b[39m {
-            \u001b[90m 2 | \u001b[39m    \u001b[36mexport\u001b[39m \u001b[36mclass\u001b[39m \u001b[33mCodeFrameClass\u001b[39m {
-            \u001b[90m 3 | \u001b[39m        private name\u001b[33m:\u001b[39m string\u001b[33m;\u001b[39m
-            \u001b[90m 4 | \u001b[39m\u001b[0m
-
         `;
 
         /** Convert output lines to an array of trimmed lines for easier comparing */

diff --git a/test/formatters/stylishFormatterTests.ts b/test/formatters/stylishFormatterTests.ts
@@ -49,9 +49,9 @@ describe("Stylish Formatter", () => {
         const expectedResult = dedent`
             formatters/stylishFormatter.test.ts
             \u001b[31mERROR: 1:1\u001b[39m  \u001b[90mfirst-name\u001b[39m  \u001b[33mfirst failure\u001b[39m
+            \u001b[31mERROR: 1:1\u001b[39m  \u001b[90mfull-name \u001b[39m  \u001b[33mfull failure\u001b[39m
             \u001b[31mERROR: 1:3\u001b[39m  \u001b[90mescape    \u001b[39m  \u001b[33m&<>'\" should be escaped\u001b[39m
-            \u001b[31mERROR: ${maxPositionTuple}\u001b[39m  \u001b[90mlast-name \u001b[39m  \u001b[33mlast failure\u001b[39m
-            \u001b[31mERROR: 1:1\u001b[39m  \u001b[90mfull-name \u001b[39m  \u001b[33mfull failure\u001b[39m\n`
+            \u001b[31mERROR: ${maxPositionTuple}\u001b[39m  \u001b[90mlast-name \u001b[39m  \u001b[33mlast failure\u001b[39m\n`
             .slice(1); // remove leading newline
 
         assert.equal(formatter.format(failures), expectedResult);