feat: remove non-natural, case-sensitive sorting

BREAKING CHANGE: * Breaks compatibility with eslint sorting rule. In practise, there is never a case when someone would want to use the other options, though.
gajus · May 15, 2020 · 46c8b4a · 46c8b4a
1 parent fc5563d
commit 46c8b4a
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 176 deletions.
diff --git a/.README/rules/sort-keys.md b/.README/rules/sort-keys.md
@@ -2,9 +2,7 @@
 
 _The `--fix` option on the command line automatically fixes problems reported by this rule._
 
-Enforces sorting of Object annotations.
-
-This rule mirrors ESlint's [sort-keys](http://eslint.org/docs/rules/sort-keys) rule.
+Enforces natural, case-insensitive sorting of Object annotations.
 
 #### Options
 
@@ -13,20 +11,12 @@ The first option specifies sort order.
 * `"asc"` (default) - enforce ascending sort order.
 * `"desc"` - enforce descending sort order.
 
-The second option takes an object with two possible properties.
-
-* `caseSensitive` - if `true`, enforce case-sensitive sort order. Default is `true`.
-* `natural` - if `true`, enforce [natural sort order](https://en.wikipedia.org/wiki/Natural_sort_order). Default is `false`.
-
 ```js
 {
   "rules": {
     "flowtype/sort-keys": [
       2,
-      "asc", {
-        "caseSensitive": true,
-        "natural": false
-      }
+      "asc"
     ]
   }
 }

diff --git a/package.json b/package.json
@@ -5,7 +5,8 @@
     "url": "http://gajus.com"
   },
   "dependencies": {
-    "lodash": "^4.17.15"
+    "lodash": "^4.17.15",
+    "string-natural-compare": "^3.0.1"
   },
   "description": "Flowtype linting rules for ESLint.",
   "devDependencies": {

diff --git a/src/rules/sortKeys.js b/src/rules/sortKeys.js
@@ -1,67 +1,33 @@
 import _ from 'lodash';
+import naturalCompare from 'string-natural-compare';
 import {
   getParameterName,
 } from '../utilities';
 
-const defaults = {
-  caseSensitive: true,
-  natural: false,
-};
-
 const schema = [
   {
     enum: ['asc', 'desc'],
     type: 'string',
   },
   {
     additionalProperties: false,
-    properties: {
-      caseSensitive: {
-        type: 'boolean',
-      },
-      natural: {
-        type: 'boolean',
-      },
-    },
     type: 'object',
   },
 ];
 
 /**
- * Functions to compare the order of two strings
- *
- * Based on a similar function from eslint's sort-keys rule.
- * https://github.com/eslint/eslint/blob/master/lib/rules/sort-keys.js
- *
  * @private
  */
-const isValidOrders = {
-  asc (str1, str2) {
-    return str1 <= str2;
-  },
-  ascI (str1, str2) {
-    return str1.toLowerCase() <= str2.toLowerCase();
-  },
-  ascIN (str1, str2) {
-    return isValidOrders.naturalCompare(str1.toLowerCase(), str2.toLowerCase()) <= 0;
-  },
-  ascN (str1, str2) {
-    return isValidOrders.naturalCompare(str1, str2) <= 0;
-  },
-  desc (str1, str2) {
-    return isValidOrders.asc(str2, str1);
-  },
-  descI (str1, str2) {
-    return isValidOrders.ascI(str2, str1);
-  },
-  descIN (str1, str2) {
-    return isValidOrders.ascIN(str2, str1);
-  },
-  descN (str1, str2) {
-    return isValidOrders.ascN(str2, str1);
+const sorters = {
+  asc: (a, b) => {
+    return naturalCompare(a, b, {
+      caseInsensitive: true,
+    });
   },
-  naturalCompare (str1, str2) {
-    return str1.localeCompare(str2, 'en-US', {numeric: true});
+  desc: (a, b) => {
+    return naturalCompare(b, a, {
+      caseInsensitive: true,
+    });
   },
 };
 
@@ -101,6 +67,7 @@ const generateOrderedList = (context, sort, properties) => {
 
     // Preserve all code until the colon verbatim:
     const key = source.getText().slice(startIndex, colonToken.range[0]);
+
     let value;
 
     if (property.value.type === 'ObjectTypeAnnotation') {
@@ -123,7 +90,12 @@ const generateOrderedList = (context, sort, properties) => {
       value = text;
     }
 
-    return [property, name, key, value];
+    return [
+      property,
+      name,
+      key,
+      value,
+    ];
   });
 
   const itemGroups = [[]];
@@ -142,9 +114,11 @@ const generateOrderedList = (context, sort, properties) => {
   const orderedList = [];
   itemGroups.forEach((itemGroup) => {
     if (itemGroup[0] && itemGroup[0].type !== 'ObjectTypeSpreadProperty') {
+      // console.log('itemGroup', itemGroup);
+
       itemGroup
         .sort((first, second) => {
-          return sort(first[1], second[1]) ? -1 : 1;
+          return sort(first[1], second[1]);
         });
     }
     orderedList.push(...itemGroup.map((item) => {
@@ -201,8 +175,6 @@ const generateFix = (node, context, sort) => {
 
 const create = (context) => {
   const order = _.get(context, ['options', 0], 'asc');
-  const {natural, caseSensitive} = _.get(context, ['options', 1], defaults);
-  const insensitive = caseSensitive === false;
 
   let prev;
   const checkKeyOrder = (node) => {
@@ -219,24 +191,22 @@ const create = (context) => {
         return;
       }
 
-      const isValidOrder = isValidOrders[order + (insensitive ? 'I' : '') + (natural ? 'N' : '')];
+      const sort = sorters[order];
 
-      if (isValidOrder(last, current) === false) {
+      if (sort(last, current) > 0) {
         context.report({
           data: {
             current,
-            insensitive: insensitive ? 'insensitive ' : '',
             last,
-            natural: natural ? 'natural ' : '',
             order,
           },
           fix (fixer) {
-            const nodeText = generateFix(node, context, isValidOrder);
+            const nodeText = generateFix(node, context, sort);
 
             return fixer.replaceText(node, nodeText);
           },
           loc: identifierNode.loc,
-          message: 'Expected type annotations to be in {{natural}}{{insensitive}}{{order}}ending order. "{{current}}" should be before "{{last}}".',
+          message: 'Expected type annotations to be in {{order}}ending order. "{{current}}" should be before "{{last}}".',
           node: identifierNode,
         });
       }

diff --git a/tests/rules/assertions/sortKeys.js b/tests/rules/assertions/sortKeys.js
@@ -5,50 +5,34 @@ export default {
       errors: [{message: 'Expected type annotations to be in ascending order. "b" should be before "c".'}],
       output: 'type FooType = { a: number, b: string, c: number }',
     },
-    {
-      code: 'type FooType = { a: number, b: number, C: number }',
-      errors: [{message: 'Expected type annotations to be in ascending order. "C" should be before "b".'}],
-      output: 'type FooType = { C: number, a: number, b: number }',
-    },
-    {
-      code: 'type FooType = { 1: number, 2: number, 10: number }',
-      errors: [{message: 'Expected type annotations to be in ascending order. "10" should be before "2".'}],
-      output: 'type FooType = { 1: number, 10: number, 2: number }',
-    },
     {
       code: 'type FooType = { a: number, b: number }',
       errors: [{message: 'Expected type annotations to be in descending order. "b" should be before "a".'}],
       options: ['desc'],
       output: 'type FooType = { b: number, a: number }',
     },
     {
-      code: 'type FooType = { C: number, b: number, a: string }',
-      errors: [{message: 'Expected type annotations to be in descending order. "b" should be before "C".'}],
-      options: ['desc'],
-      output: 'type FooType = { b: number, a: string, C: number }',
-    },
-    {
-      code: 'type FooType = { 10: number, 2: number, 1: number }',
-      errors: [{message: 'Expected type annotations to be in descending order. "2" should be before "10".'}],
+      code: 'type FooType = { b: number, C: number, a: string }',
+      errors: [{message: 'Expected type annotations to be in descending order. "C" should be before "b".'}],
       options: ['desc'],
-      output: 'type FooType = { 2: number, 10: number, 1: number }',
+      output: 'type FooType = { C: number, b: number, a: string }',
     },
     {
       code: 'type FooType = { a: number, c: number, C: number, b: string }',
-      errors: [{message: 'Expected type annotations to be in insensitive ascending order. "b" should be before "C".'}],
-      options: ['asc', {caseSensitive: false}],
+      errors: [{message: 'Expected type annotations to be in ascending order. "b" should be before "C".'}],
+      options: ['asc'],
       output: 'type FooType = { a: number, b: string, c: number, C: number }',
     },
     {
       code: 'type FooType = { a: number, C: number, c: number, b: string }',
-      errors: [{message: 'Expected type annotations to be in insensitive ascending order. "b" should be before "c".'}],
-      options: ['asc', {caseSensitive: false}],
+      errors: [{message: 'Expected type annotations to be in ascending order. "b" should be before "c".'}],
+      options: ['asc'],
       output: 'type FooType = { a: number, b: string, C: number, c: number }',
     },
     {
       code: 'type FooType = { 1: number, 10: number, 2: boolean }',
-      errors: [{message: 'Expected type annotations to be in natural ascending order. "2" should be before "10".'}],
-      options: ['asc', {natural: true}],
+      errors: [{message: 'Expected type annotations to be in ascending order. "2" should be before "10".'}],
+      options: ['asc'],
       output: 'type FooType = { 1: number, 2: boolean, 10: number }',
     },
     {
@@ -486,74 +470,6 @@ export default {
       ],
       options: ['random'],
     },
-    {
-      errors: [
-        {
-          data: {
-            language: 'jp-JP',
-          },
-          dataPath: '[1]',
-          keyword: 'additionalProperties',
-          message: 'should NOT have additional properties',
-          params: {
-            additionalProperty: 'language',
-          },
-          parentSchema: {
-            additionalProperties: false,
-            properties: {
-              caseSensitive: {
-                type: 'boolean',
-              },
-              natural: {
-                type: 'boolean',
-              },
-            },
-            type: 'object',
-          },
-          schema: false,
-          schemaPath: '#/items/1/additionalProperties',
-        },
-      ],
-      options: ['asc', {language: 'jp-JP'}],
-    },
-    {
-      errors: [
-        {
-          data: 'no',
-          dataPath: '[1].caseSensitive',
-          keyword: 'type',
-          message: 'should be boolean',
-          params: {
-            type: 'boolean',
-          },
-          parentSchema: {
-            type: 'boolean',
-          },
-          schema: 'boolean',
-          schemaPath: '#/items/1/properties/caseSensitive/type',
-        },
-      ],
-      options: ['desc', {caseSensitive: 'no'}],
-    },
-    {
-      errors: [
-        {
-          data: 'no',
-          dataPath: '[1].natural',
-          keyword: 'type',
-          message: 'should be boolean',
-          params: {
-            type: 'boolean',
-          },
-          parentSchema: {
-            type: 'boolean',
-          },
-          schema: 'boolean',
-          schemaPath: '#/items/1/properties/natural/type',
-        },
-      ],
-      options: ['desc', {natural: 'no'}],
-    },
   ],
   valid: [
     {
@@ -563,35 +479,23 @@ export default {
       code: 'type FooType = { a: number, b: number, c: (boolean | number) }',
     },
     {
-      code: 'type FooType = { C: number, a: string, b: foo }',
+      code: 'type FooType = { a: string, b: foo, C: number }',
     },
     {
-      code: 'type FooType = { 1: number, 10: number, 2: boolean }',
+      code: 'type FooType = { 1: number, 2: boolean, 10: number }',
     },
     {
       code: 'type FooType = { c: number, b: number, a: number }',
       options: ['desc'],
     },
     {
-      code: 'type FooType = { b: string, a: {}, C: number }',
+      code: 'type FooType = { C: number, b: string, a: {} }',
       options: ['desc'],
     },
     {
-      code: 'type FooType = { 2: number, 10: number, 1: boolean }',
+      code: 'type FooType = { 10: number, 2: number, 1: boolean }',
       options: ['desc'],
     },
-    {
-      code: 'type FooType = { a: number, b: number, c: number, C: number }',
-      options: ['asc', {caseSensitive: false}],
-    },
-    {
-      code: 'type FooType = { a: number, b: number, C: number, c: number }',
-      options: ['asc', {caseSensitive: false}],
-    },
-    {
-      code: 'type FooType = { 1:number, 2: number, 10: number }',
-      options: ['asc', {natural: true}],
-    },
     {
       code: 'type FooType = { b: number, a: number }',
       settings: {