diff --git a/packages/eslint-plugin/docs/rules/require-array-sort-compare.md b/packages/eslint-plugin/docs/rules/require-array-sort-compare.md new file mode 100644 index 000000000000..ead78b3c3d2e --- /dev/null +++ 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.
> 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 new file mode 100644 index 000000000000..c1aeeb50d130 --- /dev/null +++ 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 + */ + +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 new file mode 100644 index 000000000000..7614ab6c06a6 --- /dev/null +++ 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) { + a.sort(undefined) + } + `, + ` + function f(a: Array) { + 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(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) { + 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(a: T) { + a.sort() + } + `, + errors: [{ messageId: 'requireCompare' }] + }, + { + code: ` + function f(a: U) { + a.sort() + } + `, + errors: [{ messageId: 'requireCompare' }] + } + ] +});