From d87100d2fdf61bc916934e29c2073555c89dfa94 Mon Sep 17 00:00:00 2001 From: yadan Date: Fri, 20 Oct 2017 02:29:32 -0500 Subject: [PATCH] Sort failures by failure's line and character (#3345) --- src/formatters/codeFrameFormatter.ts | 1 + src/formatters/proseFormatter.ts | 1 + src/formatters/stylishFormatter.ts | 1 + src/formatters/verboseFormatter.ts | 1 + src/language/formatter/abstractFormatter.ts | 4 ++++ src/language/rule/rule.ts | 7 +++++++ test/formatters/codeFrameFormatterTests.ts | 12 ++++++------ test/formatters/stylishFormatterTests.ts | 4 ++-- 8 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/formatters/codeFrameFormatter.ts b/src/formatters/codeFrameFormatter.ts index 633c9c13eb1..f48c35c8525 100644 --- 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 index 67affeef21e..6a66289919d 100644 --- 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 index e78a56e360d..78aeb8100be 100644 --- 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 index c119c7203f5..767d2a41013 100644 --- 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 index 4ace63a4f1a..d9de37a7c46 100644 --- 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 index c4d73a5b98e..b5ce66c61b9 100644 --- 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 index 234e4872b04..c9ec0dae421 100644 --- 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 index d5918ba694b..4e8381fbb30 100644 --- 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);