From 6dee7840a3af1dfe4c38a128d1c4655bdac625df Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Sun, 7 Jun 2020 04:46:41 +0900 Subject: [PATCH] feat(eslint-plugin): [require-array-sort-compare] add `ignoreStringArrays` option (#1972) --- .../docs/rules/require-array-sort-compare.md | 35 +++++++- .../src/rules/require-array-sort-compare.ts | 49 ++++++++++- .../rules/require-array-sort-compare.test.ts | 82 +++++++++++++++++++ 3 files changed, 161 insertions(+), 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 index 3a939f7d149..78de19863f0 100644 --- 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 index 2f388d364dd..d18f2bec14a 100644 --- 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({ 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 index 6362089d9c1..e0da4276d09 100644 --- 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 }], + }, ], });