Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): [require-array-sort-compare] add ignoreStringArrays option #1972

Merged
merged 3 commits into from Jun 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 34 additions & 1 deletion packages/eslint-plugin/docs/rules/require-array-sort-compare.md
Expand Up @@ -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

Expand Down
49 changes: 45 additions & 4 deletions 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',
Expand All @@ -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,
Expand All @@ -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' });
}
Expand Down
Expand Up @@ -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: [
{
Expand Down Expand Up @@ -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 }],
},
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
],
});