Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add review comment sorter #402

Merged
merged 1 commit into from Nov 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions index.ts
Expand Up @@ -148,6 +148,7 @@ export {
DefaultReviewComment,
ProjectReview,
ReviewComment,
reviewCommentSorter,
ReviewResult,
Severity,
} from "./lib/operations/review/ReviewResult";
Expand Down
43 changes: 43 additions & 0 deletions lib/operations/review/ReviewResult.ts
Expand Up @@ -94,3 +94,46 @@ export function clean(repoId: RepoRef): ProjectReview {
comments: [],
};
}

/**
* Function suitable for use by Array.prototype.sort() to sort review
* comments by severity, category, subcategory, and sourceLocation
* path and offset. Items with the same severity, category, and
* subcategory without a location are sorted before those having a
* location.
*
* @param a First element to compare.
* @param b Second element to compare.
* @return -1 if a sorts first, 1 if b sorts first, and 0 if they are equivalent.
*/
export function reviewCommentSorter(a: ReviewComment, b: ReviewComment): number {
if (a.severity !== b.severity) {
const severities = ["error", "warn", "info"];
for (const severity of severities) {
if (a.severity === severity) {
return -1;
} else if (b.severity === severity) {
return 1;
}
}
}
if (a.category !== b.category) {
return a.category.localeCompare(b.category);
}
if (a.subcategory !== b.subcategory) {
return a.subcategory.localeCompare(b.subcategory);
}

if (!a.sourceLocation && b.sourceLocation) {
return -1;
} else if (a.sourceLocation && !b.sourceLocation) {
return 1;
} else if (a.sourceLocation && b.sourceLocation) {
if (a.sourceLocation.path !== b.sourceLocation.path) {
return a.sourceLocation.path.localeCompare(b.sourceLocation.path);
} else {
return a.sourceLocation.offset - b.sourceLocation.offset;
}
}
return 0;
}
285 changes: 285 additions & 0 deletions test/operations/review/ReviewResult.test.ts
@@ -0,0 +1,285 @@
import * as assert from "power-assert";
import {
ReviewComment,
reviewCommentSorter,
} from "../../../lib/operations/review/ReviewResult";

describe("ReviewResult", () => {

describe("reviewCommentSorter", () => {

const d: ReviewComment = {
category: "dummy",
detail: "dummy detail",
severity: "error",
subcategory: "subdummy",
sourceLocation: {
path: "/a/b/c.ts",
offset: 0,
},
};

it("should sort nothing", () => {
const r: ReviewComment[] = [];
r.sort(reviewCommentSorter);
const e: ReviewComment[] = [];
assert.deepStrictEqual(r, e);
});

it("should sort by severity", () => {
const r: ReviewComment[] = [
{ ...d, severity: "info" },
{ ...d, severity: "error" },
{ ...d, severity: "warn" },
];
r.sort(reviewCommentSorter);
const e: ReviewComment[] = [
{ ...d, severity: "error" },
{ ...d, severity: "warn" },
{ ...d, severity: "info" },
];
assert.deepStrictEqual(r, e);
});

it("should sort by category", () => {
const r: ReviewComment[] = [
{ ...d, category: "third" },
{ ...d, category: "second" },
{ ...d, category: "first" },
];
r.sort(reviewCommentSorter);
const e: ReviewComment[] = [
{ ...d, category: "first" },
{ ...d, category: "second" },
{ ...d, category: "third" },
];
assert.deepStrictEqual(r, e);
});

it("should sort by subcategory", () => {
const r: ReviewComment[] = [
{ ...d, subcategory: "third" },
{ ...d, subcategory: "second" },
{ ...d, subcategory: "first" },
];
r.sort(reviewCommentSorter);
const e: ReviewComment[] = [
{ ...d, subcategory: "first" },
{ ...d, subcategory: "second" },
{ ...d, subcategory: "third" },
];
assert.deepStrictEqual(r, e);
});

it("should sort without source locations", () => {
const r: ReviewComment[] = [
{ ...d, sourceLocation: undefined },
{ ...d, sourceLocation: undefined },
{ ...d, sourceLocation: undefined },
];
r.sort(reviewCommentSorter);
const e: ReviewComment[] = [
{ ...d, sourceLocation: undefined },
{ ...d, sourceLocation: undefined },
{ ...d, sourceLocation: undefined },
];
assert.deepStrictEqual(r, e);
});

it("should sort by path and location", () => {
const r: ReviewComment[] = [
{ ...d, sourceLocation: { path: "/d/e/f.ts", offset: 7 } },
{ ...d, sourceLocation: undefined },
{ ...d, sourceLocation: { path: "/g/h/i.ts", offset: 0 } },
{ ...d, sourceLocation: { path: "/d/e/f.ts", offset: 1 } },
];
r.sort(reviewCommentSorter);
const e: ReviewComment[] = [
{ ...d, sourceLocation: undefined },
{ ...d, sourceLocation: { path: "/d/e/f.ts", offset: 1 } },
{ ...d, sourceLocation: { path: "/d/e/f.ts", offset: 7 } },
{ ...d, sourceLocation: { path: "/g/h/i.ts", offset: 0 } },
];
assert.deepStrictEqual(r, e);
});

it("should sort comments", () => {
const r: ReviewComment[] = [
{
category: "tslint",
detail: "Exceeds maximum line length of 150",
subcategory: "max-line-length",
severity: "warn",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/test/inspection/tslint.test.ts",
columnFrom1: 1,
lineFrom1: 5,
offset: 24,
},
},
{
category: "nottslint",
detail: "Missing semicolon",
subcategory: "semicolon",
severity: "error",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/lib/inspection/tslint.ts",
columnFrom1: 14,
lineFrom1: 1,
offset: 13,
},
},
{
category: "tslint",
detail: "Calls to 'console.log' are not allowed.",
subcategory: "no-console",
severity: "warn",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/test/inspection/tslint.test.ts",
columnFrom1: 1,
lineFrom1: 2,
offset: 14,
},
},
{
category: "tslint",
detail: "Calls to 'console.log' are not allowed.",
subcategory: "no-console",
severity: "warn",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/lib/inspection/tslint.ts",
columnFrom1: 1,
lineFrom1: 2,
offset: 14,
},
},
{
category: "tslint",
detail: "Missing semicolon",
subcategory: "semicolon",
severity: "error",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/lib/inspection/tslint.ts",
columnFrom1: 14,
lineFrom1: 1,
offset: 13,
},
},
{
category: "nottslint",
detail: "Missing semicolon",
subcategory: "semicolon",
severity: "error",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/lib/inspection/reviewComment.ts",
columnFrom1: 14,
lineFrom1: 1,
offset: 13,
},
},
{
category: "tslint",
detail: "Exceeds maximum line length of 150",
subcategory: "max-line-length",
severity: "warn",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/test/inspection/tslint.test.ts",
columnFrom1: 1,
lineFrom1: 2,
offset: 14,
},
},
];
r.sort(reviewCommentSorter);
const e: ReviewComment[] = [
{
category: "nottslint",
detail: "Missing semicolon",
subcategory: "semicolon",
severity: "error",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/lib/inspection/reviewComment.ts",
columnFrom1: 14,
lineFrom1: 1,
offset: 13,
},
},
{
category: "nottslint",
detail: "Missing semicolon",
subcategory: "semicolon",
severity: "error",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/lib/inspection/tslint.ts",
columnFrom1: 14,
lineFrom1: 1,
offset: 13,
},
},
{
category: "tslint",
detail: "Missing semicolon",
subcategory: "semicolon",
severity: "error",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/lib/inspection/tslint.ts",
columnFrom1: 14,
lineFrom1: 1,
offset: 13,
},
},
{
category: "tslint",
detail: "Exceeds maximum line length of 150",
subcategory: "max-line-length",
severity: "warn",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/test/inspection/tslint.test.ts",
columnFrom1: 1,
lineFrom1: 2,
offset: 14,
},
},
{
category: "tslint",
detail: "Exceeds maximum line length of 150",
subcategory: "max-line-length",
severity: "warn",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/test/inspection/tslint.test.ts",
columnFrom1: 1,
lineFrom1: 5,
offset: 24,
},
},
{
category: "tslint",
detail: "Calls to 'console.log' are not allowed.",
subcategory: "no-console",
severity: "warn",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/lib/inspection/tslint.ts",
columnFrom1: 1,
lineFrom1: 2,
offset: 14,
},
},
{
category: "tslint",
detail: "Calls to 'console.log' are not allowed.",
subcategory: "no-console",
severity: "warn",
sourceLocation: {
path: "/home/tom/dev/waits-sdm/test/inspection/tslint.test.ts",
columnFrom1: 1,
lineFrom1: 2,
offset: 14,
},
},
];
assert.deepStrictEqual(r, e);
});

});

});