feat(eslint-plugin): [require-array-sort-compare] add `ignoreStringAr…

…rays` option (#1972)
typescript-eslint · Jun 6, 2020 · 6dee784 · 6dee784
1 parent fe4c0bf
commit 6dee784
Show file tree

Hide file tree

Showing 3 changed files with 161 additions and 5 deletions.
diff --git a/packages/eslint-plugin/docs/rules/require-array-sort-compare.md b/packages/eslint-plugin/docs/rules/require-array-sort-compare.md
@@ -45,7 +45,40 @@ userDefinedType.sort();
 
 ## Options
 
-None.
+The rule accepts an options object with the following properties:
+
+```ts
+type Options = {
+  /**
+   * If true, an array which all elements are string is ignored.
+   */
+  ignoreStringArrays?: boolean;
+};
+
+const defaults = {
+  ignoreStringArrays: false,
+};
+```
+
+### `ignoreStringArrays`
+
+Examples of **incorrect** code for this rule with `{ ignoreStringArrays: true }`:
+
+```ts
+const one = 1;
+const two = 2;
+const three = 3;
+[one, two, three].sort();
+```
+
+Examples of **correct** code for this rule with `{ ignoreStringArrays: true }`:
+
+```ts
+const one = '1';
+const two = '2';
+const three = '3';
+[one, two, three].sort();
+```
 
 ## When Not To Use It
 

diff --git a/packages/eslint-plugin/src/rules/require-array-sort-compare.ts b/packages/eslint-plugin/src/rules/require-array-sort-compare.ts
@@ -1,9 +1,20 @@
 import { TSESTree } from '@typescript-eslint/experimental-utils';
 import * as util from '../util';
 
-export default util.createRule({
+export type Options = [
+  {
+    ignoreStringArrays?: boolean;
+  },
+];
+export type MessageIds = 'requireCompare';
+
+export default util.createRule<Options, MessageIds>({
   name: 'require-array-sort-compare',
-  defaultOptions: [],
+  defaultOptions: [
+    {
+      ignoreStringArrays: false,
+    },
+  ],
 
   meta: {
     type: 'problem',
@@ -17,13 +28,39 @@ export default util.createRule({
     messages: {
       requireCompare: "Require 'compare' argument.",
     },
-    schema: [],
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          ignoreStringArrays: {
+            type: 'boolean',
+          },
+        },
+      },
+    ],
   },
 
-  create(context) {
+  create(context, [options]) {
     const service = util.getParserServices(context);
     const checker = service.program.getTypeChecker();
 
+    /**
+     * Check if a given node is an array which all elements are string.
+     * @param node
+     */
+    function isStringArrayNode(node: TSESTree.LeftHandSideExpression): boolean {
+      const type = checker.getTypeAtLocation(
+        service.esTreeNodeToTSNodeMap.get(node),
+      );
+      if (checker.isArrayType(type) || checker.isTupleType(type)) {
+        const typeArgs = checker.getTypeArguments(type);
+        return typeArgs.every(
+          arg => util.getTypeName(checker, arg) === 'string',
+        );
+      }
+      return false;
+    }
+
     return {
       ":matches(CallExpression, OptionalCallExpression)[arguments.length=0] > :matches(MemberExpression, OptionalMemberExpression)[property.name='sort'][computed=false]"(
         callee: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
@@ -34,6 +71,10 @@ export default util.createRule({
           tsNode,
         );
 
+        if (options.ignoreStringArrays && isStringArrayNode(callee.object)) {
+          return;
+        }
+
         if (util.isTypeArrayTypeOrUnionOfArrayTypes(calleeObjType, checker)) {
           context.report({ node: callee.parent!, messageId: 'requireCompare' });
         }

diff --git a/packages/eslint-plugin/tests/rules/require-array-sort-compare.test.ts b/packages/eslint-plugin/tests/rules/require-array-sort-compare.test.ts
@@ -93,6 +93,37 @@ ruleTester.run('require-array-sort-compare', rule, {
         }
       }
     `,
+    {
+      code: `
+        ['foo', 'bar', 'baz'].sort();
+      `,
+      options: [{ ignoreStringArrays: true }],
+    },
+    {
+      code: `
+        function getString() {
+          return 'foo';
+        }
+        [getString(), getString()].sort();
+      `,
+      options: [{ ignoreStringArrays: true }],
+    },
+    {
+      code: `
+        const foo = 'foo';
+        const bar = 'bar';
+        const baz = 'baz';
+        [foo, bar, baz].sort();
+      `,
+      options: [{ ignoreStringArrays: true }],
+    },
+    {
+      code: `
+        declare const x: string[];
+        x.sort();
+      `,
+      options: [{ ignoreStringArrays: true }],
+    },
   ],
   invalid: [
     {
@@ -152,5 +183,56 @@ ruleTester.run('require-array-sort-compare', rule, {
       `,
       errors: [{ messageId: 'requireCompare' }],
     },
+    {
+      code: `
+        ['foo', 'bar', 'baz'].sort();
+      `,
+      errors: [{ messageId: 'requireCompare' }],
+    },
+    {
+      code: `
+        function getString() {
+          return 'foo';
+        }
+        [getString(), getString()].sort();
+      `,
+      errors: [{ messageId: 'requireCompare' }],
+    },
+    {
+      code: `
+        const foo = 'foo';
+        const bar = 'bar';
+        const baz = 'baz';
+        [foo, bar, baz].sort();
+      `,
+      errors: [{ messageId: 'requireCompare' }],
+    },
+    {
+      code: `
+        [2, 'bar', 'baz'].sort();
+      `,
+      errors: [{ messageId: 'requireCompare' }],
+      options: [{ ignoreStringArrays: true }],
+    },
+    {
+      code: `
+        function getNumber() {
+          return 2;
+        }
+        [2, 3].sort();
+      `,
+      errors: [{ messageId: 'requireCompare' }],
+      options: [{ ignoreStringArrays: true }],
+    },
+    {
+      code: `
+        const one = 1;
+        const two = 2;
+        const three = 3;
+        [one, two, three].sort();
+      `,
+      errors: [{ messageId: 'requireCompare' }],
+      options: [{ ignoreStringArrays: true }],
+    },
   ],
 });