Repository: stoplightio/spectral
Pull request #967 (2020-02-15 18:46:34)

chore: introduce comparePosition
In #939, I initially started with a range based comparison approach. That proved to be a bad idea. However, while building the now defunct rangeEquals, I refactored that piece of code.

Although quite a minor change, I decided to keep it along (as it cleans the code a little) and covered it with tests.

**Checklist**

- [ ] Tests added / updated
- [ ] Docs added / updated

**Does this PR introduce a breaking change?**

- [ ] Yes
- [x] No



Patch:
diff --git a/src/utils/sortResults.ts b/src/utils/sortResults.ts
index 29238078..c09e4fda 100644
--- a/src/utils/sortResults.ts
+++ b/src/utils/sortResults.ts
@@ -1,3 +1,4 @@
+import { IPosition } from '@stoplight/types';
 import { IRuleResult } from '../types';
 
 const compareCode = (left: string | number | undefined, right: string | number | undefined): number => {
@@ -44,6 +45,18 @@ const normalize = (value: number): -1 | 0 | 1 => {
   return 0;
 };
 
+export const comparePosition = (left: IPosition, right: IPosition): -1 | 0 | 1 => {
+  const diffLine = left.line - right.line;
+
+  if (diffLine !== 0) {
+    return normalize(diffLine);
+  }
+
+  const diffChar = left.character - right.character;
+
+  return normalize(diffChar);
+};
+
 export const compareResults = (left: IRuleResult, right: IRuleResult): -1 | 0 | 1 => {
   const diffSource = compareSource(left.source, right.source);
 
@@ -51,16 +64,10 @@ export const compareResults = (left: IRuleResult, right: IRuleResult): -1 | 0 |
     return normalize(diffSource);
   }
 
-  const diffLine = left.range.start.line - right.range.start.line;
-
-  if (diffLine !== 0) {
-    return normalize(diffLine);
-  }
-
-  const diffChar = left.range.start.character - right.range.start.character;
+  const diffStart = comparePosition(left.range.start, right.range.start);
 
-  if (diffChar !== 0) {
-    return normalize(diffChar);
+  if (diffStart !== 0) {
+    return diffStart;
   }
 
   const diffCode = compareCode(left.code, right.code);


Test patch:
diff --git a/src/utils/__tests__/sortResults.test.ts b/src/utils/__tests__/sortResults.test.ts
index 00b2441f..af4d439c 100644
--- a/src/utils/__tests__/sortResults.test.ts
+++ b/src/utils/__tests__/sortResults.test.ts
@@ -1,6 +1,6 @@
-import { DiagnosticSeverity } from '@stoplight/types';
+import { DiagnosticSeverity, IPosition } from '@stoplight/types';
 import { IRuleResult } from '../../types';
-import { compareResults, sortResults } from '../sortResults';
+import { comparePosition, compareResults, sortResults } from '../sortResults';
 
 const results: IRuleResult[] = [
   {
@@ -204,3 +204,31 @@ describe('compareResults', () => {
     });
   });
 });
+
+const buildPosition = (line: number, char: number): IPosition => {
+  return { line, character: char };
+};
+
+describe('comparePosition', () => {
+  const positionTestCases = [
+    [2, 2, 1, 1, 1],
+    [2, 1, 1, 1, 1],
+    [1, 2, 1, 1, 1],
+
+    [1, 1, 1, 1, 0],
+    [1, 2, 1, 2, 0],
+    [2, 1, 2, 1, 0],
+    [2, 2, 2, 2, 0],
+
+    [1, 1, 1, 2, -1],
+    [1, 1, 2, 1, -1],
+    [1, 1, 2, 2, -1],
+  ];
+
+  test.each(positionTestCases)(
+    'should properly order locations (%i, %i) vs (%i, %i)',
+    (leftLine, leftChar, rightLine, rightChar, expected) => {
+      expect(comparePosition(buildPosition(leftLine, leftChar), buildPosition(rightLine, rightChar))).toEqual(expected);
+    },
+  );
+});