feat(eslint-plugin): [member-ordering] add natural sort order (#5662)

* [WIP] feat(eslint-plugin): [member-ordering] add natural sort order * Fix yarn.lock and split option on case sensitivity * Document it too * Remove last todos * Move member-ordering sub-tests into sub-dirs
typescript-eslint · Oct 24, 2022 · 1eaae09 · 1eaae09
1 parent 3bd38ca
commit 1eaae09
Show file tree

Hide file tree

Showing 8 changed files with 352 additions and 22 deletions.
diff --git a/packages/eslint-plugin/docs/rules/member-ordering.md b/packages/eslint-plugin/docs/rules/member-ordering.md
@@ -24,7 +24,12 @@ type OrderConfig = MemberType[] | SortedOrderConfig | 'never';
 
 interface SortedOrderConfig {
   memberTypes?: MemberType[] | 'never';
-  order: 'alphabetically' | 'alphabetically-case-insensitive' | 'as-written';
+  order:
+    | 'alphabetically'
+    | 'alphabetically-case-insensitive'
+    | 'as-written'
+    | 'natural'
+    | 'natural-case-insensitive';
 }
 
 // See below for the more specific MemberType strings
@@ -56,6 +61,17 @@ The supported member attributes are, in order:
 Member attributes may be joined with a `'-'` to combine into more specific groups.
 For example, `'public-field'` would come before `'private-field'`.
 
+### Orders
+
+The `order` value specifies what order members should be within a group.
+It defaults to `as-written`, meaning any order is fine.
+Other allowed values are:
+
+- `alphabetically`: Sorted in a-z alphabetical order, directly using string `<` comparison (so `B` comes before `a`)
+- `alphabetically-case-insensitive`: Sorted in a-z alphabetical order, ignoring case (so `a` comes before `B`)
+- `natural`: Same as `alphabetically`, but using [`natural-compare-lite`](https://github.com/litejs/natural-compare-lite) for more friendly sorting of numbers
+- `natural-case-insensitive`: Same as `alphabetically-case-insensitive`, but using [`natural-compare-lite`](https://github.com/litejs/natural-compare-lite) for more friendly sorting of numbers
+
 ### Default configuration
 
 The default configuration looks as follows:

diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json
@@ -49,6 +49,7 @@
     "@typescript-eslint/utils": "5.41.0",
     "debug": "^4.3.4",
     "ignore": "^5.2.0",
+    "natural-compare-lite": "^1.4.0",
     "regexpp": "^3.2.0",
     "semver": "^7.3.7",
     "tsutils": "^3.21.0"
@@ -57,6 +58,7 @@
     "@types/debug": "*",
     "@types/json-schema": "*",
     "@types/marked": "*",
+    "@types/natural-compare-lite": "^1.4.0",
     "@types/prettier": "*",
     "chalk": "^5.0.1",
     "json-schema": "*",

diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts
@@ -1,5 +1,6 @@
 import type { JSONSchema, TSESLint, TSESTree } from '@typescript-eslint/utils';
 import { AST_NODE_TYPES } from '@typescript-eslint/utils';
+import naturalCompare from 'natural-compare-lite';
 
 import * as util from '../util';
 
@@ -34,10 +35,13 @@ type BaseMemberType =
 
 type MemberType = BaseMemberType | BaseMemberType[];
 
-type Order =
+type AlphabeticalOrder =
   | 'alphabetically'
   | 'alphabetically-case-insensitive'
-  | 'as-written';
+  | 'natural'
+  | 'natural-case-insensitive';
+
+type Order = AlphabeticalOrder | 'as-written';
 
 interface SortedOrderConfig {
   memberTypes?: MemberType[] | 'never';
@@ -87,7 +91,13 @@ const objectConfig = (memberTypes: MemberType[]): JSONSchema.JSONSchema4 => ({
     },
     order: {
       type: 'string',
-      enum: ['alphabetically', 'alphabetically-case-insensitive', 'as-written'],
+      enum: [
+        'alphabetically',
+        'alphabetically-case-insensitive',
+        'as-written',
+        'natural',
+        'natural-case-insensitive',
+      ],
     },
   },
   additionalProperties: false,
@@ -629,7 +639,7 @@ export default util.createRule<Options, MessageIds>({
      */
     function checkAlphaSort(
       members: Member[],
-      caseSensitive: boolean,
+      order: AlphabeticalOrder,
     ): boolean {
       let previousName = '';
       let isCorrectlySorted = true;
@@ -640,11 +650,7 @@ export default util.createRule<Options, MessageIds>({
 
         // Note: Not all members have names
         if (name) {
-          if (
-            caseSensitive
-              ? name < previousName
-              : name.toLowerCase() < previousName.toLowerCase()
-          ) {
+          if (naturalOutOfOrder(name, previousName, order)) {
             context.report({
               node: member,
               messageId: 'incorrectOrder',
@@ -664,6 +670,25 @@ export default util.createRule<Options, MessageIds>({
       return isCorrectlySorted;
     }
 
+    function naturalOutOfOrder(
+      name: string,
+      previousName: string,
+      order: AlphabeticalOrder,
+    ): boolean {
+      switch (order) {
+        case 'alphabetically':
+          return name < previousName;
+        case 'alphabetically-case-insensitive':
+          return name.toLowerCase() < previousName.toLowerCase();
+        case 'natural':
+          return naturalCompare(name, previousName) !== 1;
+        case 'natural-case-insensitive':
+          return (
+            naturalCompare(name.toLowerCase(), previousName.toLowerCase()) !== 1
+          );
+      }
+    }
+
     /**
      * Validates if all members are correctly sorted.
      *
@@ -681,7 +706,7 @@ export default util.createRule<Options, MessageIds>({
       }
 
       // Standardize config
-      let order: Order | null = null;
+      let order: Order | undefined;
       let memberTypes;
 
       if (Array.isArray(orderConfig)) {
@@ -691,9 +716,7 @@ export default util.createRule<Options, MessageIds>({
         memberTypes = orderConfig.memberTypes;
       }
 
-      const hasAlphaSort = order?.startsWith('alphabetically');
-      const alphaSortIsCaseSensitive =
-        order !== 'alphabetically-case-insensitive';
+      const hasAlphaSort = !!(order && order !== 'as-written');
 
       // Check order
       if (Array.isArray(memberTypes)) {
@@ -706,11 +729,11 @@ export default util.createRule<Options, MessageIds>({
         if (hasAlphaSort) {
           grouped.some(
             groupMember =>
-              !checkAlphaSort(groupMember, alphaSortIsCaseSensitive),
+              !checkAlphaSort(groupMember, order as AlphabeticalOrder),
           );
         }
       } else if (hasAlphaSort) {
-        checkAlphaSort(members, alphaSortIsCaseSensitive);
+        checkAlphaSort(members, order as AlphabeticalOrder);
       }
     }
 

diff --git a/...abetically-case-insensitive-order.test.ts → ...abetically-case-insensitive-order.test.ts b/...abetically-case-insensitive-order.test.ts → ...abetically-case-insensitive-order.test.ts
@@ -1,8 +1,8 @@
 import type { TSESLint } from '@typescript-eslint/utils';
 
-import type { MessageIds, Options } from '../../src/rules/member-ordering';
-import rule, { defaultOrder } from '../../src/rules/member-ordering';
-import { RuleTester } from '../RuleTester';
+import type { MessageIds, Options } from '../../../src/rules/member-ordering';
+import rule, { defaultOrder } from '../../../src/rules/member-ordering';
+import { RuleTester } from '../../RuleTester';
 
 const ruleTester = new RuleTester({
   parser: '@typescript-eslint/parser',

diff --git a/...ber-ordering-alphabetically-order.test.ts → ...ber-ordering-alphabetically-order.test.ts b/...ber-ordering-alphabetically-order.test.ts → ...ber-ordering-alphabetically-order.test.ts
@@ -1,8 +1,8 @@
 import type { TSESLint } from '@typescript-eslint/utils';
 
-import type { MessageIds, Options } from '../../src/rules/member-ordering';
-import rule, { defaultOrder } from '../../src/rules/member-ordering';
-import { RuleTester } from '../RuleTester';
+import type { MessageIds, Options } from '../../../src/rules/member-ordering';
+import rule, { defaultOrder } from '../../../src/rules/member-ordering';
+import { RuleTester } from '../../RuleTester';
 
 const ruleTester = new RuleTester({
   parser: '@typescript-eslint/parser',

diff --git a/...plugin/tests/rules/member-ordering/member-ordering-natural-case-insensitive-order.test.ts b/...plugin/tests/rules/member-ordering/member-ordering-natural-case-insensitive-order.test.ts
@@ -0,0 +1,135 @@
+import rule from '../../../src/rules/member-ordering';
+import { RuleTester } from '../../RuleTester';
+
+const ruleTester = new RuleTester({
+  parser: '@typescript-eslint/parser',
+});
+
+ruleTester.run('member-ordering-natural-order', rule, {
+  valid: [
+    {
+      code: `
+interface Example {
+  1: number;
+  5: number;
+  10: number;
+}
+      `,
+      options: [
+        {
+          default: {
+            order: 'natural-case-insensitive',
+          },
+        },
+      ],
+    },
+    {
+      code: `
+interface Example {
+  new (): unknown;
+
+  a1(): void;
+  a5(): void;
+  a10(): void;
+  B1(): void;
+  B5(): void;
+  B10(): void;
+
+  a1: number;
+  a5: number;
+  a10: number;
+  B1: number;
+  B5: number;
+  B10: number;
+}
+      `,
+      options: [
+        {
+          default: {
+            memberTypes: ['constructor', 'method', 'field'],
+            order: 'natural-case-insensitive',
+          },
+        },
+      ],
+    },
+  ],
+  invalid: [
+    {
+      code: `
+interface Example {
+  1: number;
+  10: number;
+  5: number;
+}
+      `,
+      errors: [
+        {
+          messageId: 'incorrectOrder',
+          data: {
+            beforeMember: 10,
+            member: 5,
+          },
+          line: 5,
+          column: 3,
+        },
+      ],
+      options: [
+        {
+          default: {
+            order: 'natural-case-insensitive',
+          },
+        },
+      ],
+    },
+
+    {
+      code: `
+interface Example {
+  new (): unknown;
+
+  a1(): void;
+  a10(): void;
+  a5(): void;
+  B5(): void;
+  B10(): void;
+  B1(): void;
+
+  a5: number;
+  a10: number;
+  B1: number;
+  a1: number;
+  B5: number;
+  B10: number;
+}
+      `,
+      errors: [
+        {
+          column: 3,
+          data: {
+            beforeMember: 'a10',
+            member: 'a5',
+          },
+          line: 7,
+          messageId: 'incorrectOrder',
+        },
+        {
+          column: 3,
+          data: {
+            beforeMember: 'B10',
+            member: 'B1',
+          },
+          line: 10,
+          messageId: 'incorrectOrder',
+        },
+      ],
+      options: [
+        {
+          default: {
+            memberTypes: ['constructor', 'method', 'field'],
+            order: 'natural-case-insensitive',
+          },
+        },
+      ],
+    },
+  ],
+});