From d019111302c30e5f1baf564a90288cd8ae261a31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ro=C5=BCek?=
 <9273484+P0lip@users.noreply.github.com>
Date: Tue, 1 Oct 2019 00:15:29 +0200
Subject: [PATCH] feat(alphabetical): improve validation results (#613)

* perf(alphabetical): avoid sorting

* feat(alphabetical): improve validation results
---
 src/functions/__tests__/alphabetical.test.ts | 17 ++--
 src/functions/alphabetical.ts                | 81 ++++++++++++--------
 2 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/src/functions/__tests__/alphabetical.test.ts b/src/functions/__tests__/alphabetical.test.ts
index ce5d11046..bafe24937 100644
--- a/src/functions/__tests__/alphabetical.test.ts
+++ b/src/functions/__tests__/alphabetical.test.ts
@@ -21,7 +21,8 @@ describe('alphabetical', () => {
       }),
     ).toEqual([
       {
-        message: 'properties are not in alphabetical order',
+        message: 'at least 2 properties are not in alphabetical order: "c" should be placed after "b"',
+        path: ['$', 'c'],
       },
     ]);
   });
@@ -30,7 +31,8 @@ describe('alphabetical', () => {
     test('given an unsorted array of strings should return error', () => {
       expect(runAlphabetical(['b', 'a'])).toEqual([
         {
-          message: 'properties are not in alphabetical order',
+          message: 'at least 2 properties are not in alphabetical order: "b" should be placed after "a"',
+          path: ['$', 0],
         },
       ]);
     });
@@ -42,15 +44,8 @@ describe('alphabetical', () => {
     test('given an unsorted array of numbers should return error', () => {
       expect(runAlphabetical([10, 1])).toEqual([
         {
-          message: 'properties are not in alphabetical order',
-        },
-      ]);
-    });
-
-    test('given an unsorted array of numbers should return error', () => {
-      expect(runAlphabetical([10, 1])).toEqual([
-        {
-          message: 'properties are not in alphabetical order',
+          message: 'at least 2 properties are not in alphabetical order: "10" should be placed after "1"',
+          path: ['$', 0],
         },
       ]);
     });
diff --git a/src/functions/alphabetical.ts b/src/functions/alphabetical.ts
index 27c5e87e0..5c7977724 100644
--- a/src/functions/alphabetical.ts
+++ b/src/functions/alphabetical.ts
@@ -1,51 +1,66 @@
-import { isEqual } from 'lodash';
-
 import { IAlphaRuleOptions, IFunction, IFunctionResult } from '../types';
+import { isObject } from '../utils';
 
-export const alphabetical: IFunction<IAlphaRuleOptions> = (targetVal, opts) => {
-  const results: IFunctionResult[] = [];
+const compare = (a: unknown, b: unknown): number => {
+  if ((typeof a === 'number' || Number.isNaN(Number(a))) && (typeof b === 'number' || !Number.isNaN(Number(b)))) {
+    return Math.min(1, Math.max(-1, Number(a) - Number(b)));
+  }
 
-  if (!targetVal) {
-    return results;
+  if (typeof a !== 'string' || typeof b !== 'string') {
+    return 0;
   }
 
-  let targetArray: any[] = targetVal;
-  if (!Array.isArray(targetVal)) {
-    targetArray = Object.keys(targetVal);
+  return a.localeCompare(b);
+};
+
+const getUnsortedItems = <T>(arr: T[], compareFn: (a: T, B: T) => number): null | [number, number] => {
+  for (let i = 0; i < arr.length - 1; i += 1) {
+    if (compareFn(arr[i], arr[i + 1]) >= 1) {
+      return [i, i + 1];
+    }
   }
 
-  // don't mutate original array
-  const copiedArray = targetArray.slice();
+  return null;
+};
+
+export const alphabetical: IFunction<IAlphaRuleOptions> = (targetVal, opts, paths) => {
+  const results: IFunctionResult[] = [];
 
-  if (copiedArray.length < 2) {
+  if (!isObject(targetVal)) {
     return results;
   }
 
-  const { keyedBy } = opts;
+  const targetArray: any[] | string[] = Array.isArray(targetVal) ? targetVal : Object.keys(targetVal);
 
-  // If we aren't expecting an object keyed by a specific property, then treat the
-  // object as a simple array.
-  if (keyedBy) {
-    copiedArray.sort((a, b) => {
-      if (typeof a !== 'object') {
-        return 0;
-      }
-
-      if (a[keyedBy] < b[keyedBy]) {
-        return -1;
-      } else if (a[keyedBy] > b[keyedBy]) {
-        return 1;
-      }
-
-      return 0;
-    });
-  } else {
-    copiedArray.sort();
+  if (targetArray.length < 2) {
+    return results;
   }
 
-  if (!isEqual(targetArray, copiedArray)) {
+  const { keyedBy } = opts;
+
+  const unsortedItems = getUnsortedItems<unknown>(
+    targetArray,
+    keyedBy
+      ? (a, b) => {
+          if (!isObject(a) || !isObject(b)) return 0;
+
+          return compare(a[keyedBy], b[keyedBy]);
+        }
+      : // If we aren't expecting an object keyed by a specific property, then treat the
+        // object as a simple array.
+        compare,
+  );
+
+  if (unsortedItems != null) {
+    const path = paths.target || paths.given;
+
     results.push({
-      message: 'properties are not in alphabetical order',
+      ...(!keyedBy && { path: [...path, Array.isArray(targetVal) ? unsortedItems[0] : targetArray[unsortedItems[0]]] }),
+      message: keyedBy
+        ? 'properties are not in alphabetical order'
+        : `at least 2 properties are not in alphabetical order: "${
+            targetArray[unsortedItems[0]]
+          }" should be placed after "${targetArray[unsortedItems[1]]}"`,
     });
   }