feat(eslint-plugin): add require-array-sort-compare rule (#261)

mysticatea · bradzacher · commit 2a4aaaa4e315 · 2019-02-13T17:17:32.000-08:00
Fixes #247
diff --git a/packages/eslint-plugin/docs/rules/require-array-sort-compare.md b/packages/eslint-plugin/docs/rules/require-array-sort-compare.md
@@ -0,0 +1,50 @@
+# Enforce giving `compare` argument to `Array#sort` (require-array-sort-compare)
+
+This rule prevents to invoke `Array#sort()` method without `compare` argument.
+
+`Array#sort()` method sorts that element by the alphabet order.
+
+```ts
+[1, 2, 3, 10, 20, 30].sort(); //→ [1, 10, 2, 20, 3, 30]
+```
+
+The language specification also noted this trap.
+
+> NOTE 2: Method calls performed by the ToString abstract operations in steps 5 and 7 have the potential to cause SortCompare to not behave as a consistent comparison function.<br> > https://www.ecma-international.org/ecma-262/9.0/#sec-sortcompare
+
+## Rule Details
+
+This rule is aimed at preventing the calls of `Array#sort` method.
+This rule ignores the `sort` methods of user-defined types.
+
+Examples of **incorrect** code for this rule:
+
+```ts
+const array: any[];
+const stringArray: string[];
+
+array.sort();
+
+// Even if a string array, warns it in favor of `String#localeCompare` method.
+stringArray.sort();
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+const array: any[];
+const userDefinedType: { sort(): void };
+
+array.sort((a, b) => a - b);
+array.sort((a, b) => a.localeCompare(b));
+
+userDefinedType.sort();
+```
+
+### Options
+
+There is no option.
+
+## When Not To Use It
+
+If you understand the language specification enough, you can turn this rule off safely.
diff --git a/packages/eslint-plugin/src/rules/require-array-sort-compare.ts b/packages/eslint-plugin/src/rules/require-array-sort-compare.ts
@@ -0,0 +1,57 @@
+/**
+ * @fileoverview Enforce giving `compare` argument to `Array#sort`
+ * @author Toru Nagashima <https://github.com/mysticatea>
+ */
+
+import * as ts from 'typescript';
+import { TSESTree } from '@typescript-eslint/typescript-estree';
+import * as util from '../util';
+
+export default util.createRule({
+  name: 'require-array-sort-compare',
+  defaultOptions: [],
+
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'Enforce giving `compare` argument to `Array#sort`',
+      category: 'Best Practices',
+      recommended: false
+    },
+    messages: {
+      requireCompare: "Require 'compare' argument."
+    },
+    schema: []
+  },
+
+  create(context) {
+    const service = util.getParserServices(context);
+    const checker = service.program.getTypeChecker();
+
+    return {
+      "CallExpression[arguments.length=0] > MemberExpression[property.name='sort'][computed=false]"(
+        node: TSESTree.MemberExpression
+      ) {
+        // Get the symbol of the `sort` method.
+        const tsNode = service.esTreeNodeToTSNodeMap.get(node);
+        const sortSymbol = checker.getSymbolAtLocation(tsNode);
+        if (sortSymbol == null) {
+          return;
+        }
+
+        // Check the owner type of the `sort` method.
+        for (const methodDecl of sortSymbol.declarations) {
+          const typeDecl = methodDecl.parent;
+          if (
+            ts.isInterfaceDeclaration(typeDecl) &&
+            ts.isSourceFile(typeDecl.parent) &&
+            typeDecl.name.escapedText === 'Array'
+          ) {
+            context.report({ node: node.parent!, messageId: 'requireCompare' });
+            return;
+          }
+        }
+      }
+    };
+  }
+});
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
@@ -0,0 +1,127 @@
+import path from 'path';
+import rule from '../../src/rules/require-array-sort-compare';
+import { RuleTester } from '../RuleTester';
+
+const rootPath = path.join(process.cwd(), 'tests/fixtures/');
+
+const ruleTester = new RuleTester({
+  parser: '@typescript-eslint/parser',
+  parserOptions: {
+    tsconfigRootDir: rootPath,
+    project: './tsconfig.json'
+  }
+});
+
+ruleTester.run('require-array-sort-compare', rule, {
+  valid: [
+    `
+      function f(a: any[]) {
+        a.sort(undefined)
+      }
+    `,
+    `
+      function f(a: any[]) {
+        a.sort((a, b) => a - b)
+      }
+    `,
+    `
+      function f(a: Array<string>) {
+        a.sort(undefined)
+      }
+    `,
+    `
+      function f(a: Array<number>) {
+        a.sort((a, b) => a - b)
+      }
+    `,
+    `
+      function f(a: { sort(): void }) {
+        a.sort()
+      }
+    `,
+    `
+      class A { sort(): void {} }
+      function f(a: A) {
+        a.sort()
+      }
+    `,
+    `
+      interface A { sort(): void }
+      function f(a: A) {
+        a.sort()
+      }
+    `,
+    `
+      interface A { sort(): void }
+      function f<T extends A>(a: T) {
+        a.sort()
+      }
+    `,
+    `
+      function f(a: any) {
+        a.sort()
+      }
+    `,
+    `
+      namespace UserDefined {
+        interface Array {
+          sort(): void
+        }
+        function f(a: Array) {
+          a.sort()
+        }
+      }
+    `
+  ],
+  invalid: [
+    {
+      code: `
+        function f(a: Array<any>) {
+          a.sort()
+        }
+      `,
+      errors: [{ messageId: 'requireCompare' }]
+    },
+    {
+      code: `
+        function f(a: string[]) {
+          a.sort()
+        }
+      `,
+      errors: [{ messageId: 'requireCompare' }]
+    },
+    {
+      code: `
+        function f(a: string | string[]) {
+          if (Array.isArray(a))
+            a.sort()
+        }
+      `,
+      errors: [{ messageId: 'requireCompare' }]
+    },
+    {
+      code: `
+        function f(a: number[] | string[]) {
+          a.sort()
+        }
+      `,
+      errors: [{ messageId: 'requireCompare' }]
+    },
+    {
+      code: `
+        function f<T extends number[]>(a: T) {
+          a.sort()
+        }
+      `,
+      errors: [{ messageId: 'requireCompare' }]
+    },
+    {
+      code: `
+        function f<T, U extends T[]>(a: U) {
+          a.sort()
+        }
+      `,
+      errors: [{ messageId: 'requireCompare' }]
+    }
+  ]
+});
