Skip to content

Commit b70d1e8

Browse files
fiskerfisker
authored
Add no-array-sort rule (#2713)
1 parent 9461ca6 commit b70d1e8

14 files changed

+550
-116
lines changed

‎docs/rules/no-array-reverse.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ Pass `allowExpressionStatement: false` to forbid `Array#reverse()` even if it's
3636
#### Fail
3737

3838
```js
39-
// eslint unicorn/no-array-reverse: ["error", {"allowExpressionStatement": true}]
39+
// eslint unicorn/no-array-reverse: ["error", {"allowExpressionStatement": false}]
4040
array.reverse();
4141
```
42+
43+
## Related rules
44+
45+
- [unicorn/no-array-sort](./no-array-sort.md)

‎docs/rules/no-array-sort.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Prefer `Array#toSorted()` over `Array#sort()`
2+
3+
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config).
4+
5+
💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions).
6+
7+
<!-- end auto-generated rule header -->
8+
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
9+
10+
Prefer using [`Array#toSorted()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toSorted) over [`Array#sort()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort).
11+
12+
`Array#sort()` modifies the original array, while `Array#toSorted()` returns a new reversed array.
13+
14+
## Examples
15+
16+
```js
17+
// ❌
18+
const sorted = [...array].sort();
19+
20+
// ✅
21+
const sorted = [...array].toSorted();
22+
```
23+
24+
```js
25+
// ❌
26+
const sorted = [...array].sort((a, b) => a - b);
27+
28+
// ✅
29+
const sorted = [...array].toSorted((a, b) => a - b);
30+
```
31+
32+
## Options
33+
34+
Type: `object`
35+
36+
### allowExpressionStatement
37+
38+
Type: `boolean`\
39+
Default: `true`
40+
41+
This rule allows `array.sort()` to be used as an expression statement by default.\
42+
Pass `allowExpressionStatement: false` to forbid `Array#sort()` even if it's an expression statement.
43+
44+
#### Fail
45+
46+
```js
47+
// eslint unicorn/no-array-sort: ["error", {"allowExpressionStatement": false}]
48+
array.sort();
49+
```
50+
51+
## Related rules
52+
53+
- [unicorn/no-array-reverse](./no-array-reverse.md)

‎readme.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export default [
8181
| [no-array-method-this-argument](docs/rules/no-array-method-this-argument.md) | Disallow using the `this` argument in array methods. | ✅ | 🔧 | 💡 |
8282
| [no-array-reduce](docs/rules/no-array-reduce.md) | Disallow `Array#reduce()` and `Array#reduceRight()`. | ✅ | | |
8383
| [no-array-reverse](docs/rules/no-array-reverse.md) | Prefer `Array#toReversed()` over `Array#reverse()`. | ✅ | | 💡 |
84+
| [no-array-sort](docs/rules/no-array-sort.md) | Prefer `Array#toSorted()` over `Array#sort()`. | ✅ | | 💡 |
8485
| [no-await-expression-member](docs/rules/no-await-expression-member.md) | Disallow member access from await expression. | ✅ | 🔧 | |
8586
| [no-await-in-promise-methods](docs/rules/no-await-in-promise-methods.md) | Disallow using `await` in `Promise` method parameters. | ✅ | | 💡 |
8687
| [no-console-spaces](docs/rules/no-console-spaces.md) | Do not use leading/trailing space between `console.log` parameters. | ✅ | 🔧 | |

‎rules/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export {default as 'no-array-for-each'} from './no-array-for-each.js';
2525
export {default as 'no-array-method-this-argument'} from './no-array-method-this-argument.js';
2626
export {default as 'no-array-reduce'} from './no-array-reduce.js';
2727
export {default as 'no-array-reverse'} from './no-array-reverse.js';
28+
export {default as 'no-array-sort'} from './no-array-sort.js';
2829
export {default as 'no-await-expression-member'} from './no-await-expression-member.js';
2930
export {default as 'no-await-in-promise-methods'} from './no-await-in-promise-methods.js';
3031
export {default as 'no-console-spaces'} from './no-console-spaces.js';

‎rules/no-array-reverse.js

Lines changed: 2 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -1,109 +1,6 @@
1-
import {isMethodCall} from './ast/index.js';
2-
import {getParenthesizedText} from './utils/index.js';
3-
4-
const MESSAGE_ID_ERROR = 'no-array-reverse/error';
5-
const MESSAGE_ID_SUGGESTION_ONLY_FIX_METHOD = 'no-array-reverse/suggestion-only-fix-method';
6-
const MESSAGE_ID_SUGGESTION_SPREADING_ARRAY = 'no-array-reverse/suggestion-spreading-array';
7-
const MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY = 'no-array-reverse/suggestion-not-spreading-array';
8-
const messages = {
9-
[MESSAGE_ID_ERROR]: 'Use `Array#toReversed()` instead of `Array#reverse()`.',
10-
[MESSAGE_ID_SUGGESTION_ONLY_FIX_METHOD]: 'Switch to `.toReversed()`.',
11-
[MESSAGE_ID_SUGGESTION_SPREADING_ARRAY]: 'The spreading object is an array',
12-
[MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY]: 'The spreading object is NOT an array',
13-
};
14-
15-
/** @param {import('eslint').Rule.RuleContext} context */
16-
const create = context => {
17-
const {sourceCode} = context;
18-
const {allowExpressionStatement} = context.options[0];
19-
20-
return {
21-
CallExpression(callExpression) {
22-
if (!isMethodCall(callExpression, {
23-
method: 'reverse',
24-
argumentsLength: 0,
25-
optionalCall: false,
26-
})) {
27-
return;
28-
}
29-
30-
const array = callExpression.callee.object;
31-
32-
// `[...array].reverse()`
33-
const isSpreadAndReverse = array.type === 'ArrayExpression'
34-
&& array.elements.length === 1
35-
&& array.elements[0].type === 'SpreadElement';
36-
37-
if (allowExpressionStatement && !isSpreadAndReverse) {
38-
const maybeExpressionStatement = callExpression.parent.type === 'ChainExpression'
39-
? callExpression.parent.parent
40-
: callExpression.parent;
41-
if (maybeExpressionStatement.type === 'ExpressionStatement') {
42-
return;
43-
}
44-
}
45-
46-
const reverseProperty = callExpression.callee.property;
47-
const suggestions = [];
48-
const fixMethodName = fixer => fixer.replaceText(reverseProperty, 'toReversed');
49-
50-
/*
51-
For `[...array].reverse()`, provide two suggestion, let user choose if the object can be unwrapped,
52-
otherwise only change `.reverse()` to `.toReversed()`
53-
*/
54-
if (isSpreadAndReverse) {
55-
suggestions.push({
56-
messageId: MESSAGE_ID_SUGGESTION_SPREADING_ARRAY,
57-
* fix(fixer) {
58-
const text = getParenthesizedText(array.elements[0].argument, sourceCode);
59-
yield fixer.replaceText(array, text);
60-
yield fixMethodName(fixer);
61-
},
62-
});
63-
}
64-
65-
suggestions.push({
66-
messageId: isSpreadAndReverse
67-
? MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY
68-
: MESSAGE_ID_SUGGESTION_ONLY_FIX_METHOD,
69-
fix: fixMethodName,
70-
});
71-
72-
return {
73-
node: reverseProperty,
74-
messageId: MESSAGE_ID_ERROR,
75-
suggest: suggestions,
76-
};
77-
},
78-
};
79-
};
80-
81-
const schema = [
82-
{
83-
type: 'object',
84-
additionalProperties: false,
85-
properties: {
86-
allowExpressionStatement: {
87-
type: 'boolean',
88-
},
89-
},
90-
},
91-
];
1+
import noArrayMutateRule from './shared/no-array-mutate-rule.js';
922

933
/** @type {import('eslint').Rule.RuleModule} */
94-
const config = {
95-
create,
96-
meta: {
97-
type: 'suggestion',
98-
docs: {
99-
description: 'Prefer `Array#toReversed()` over `Array#reverse()`.',
100-
recommended: true,
101-
},
102-
hasSuggestions: true,
103-
schema,
104-
defaultOptions: [{allowExpressionStatement: true}],
105-
messages,
106-
},
107-
};
4+
const config = noArrayMutateRule('reverse');
1085

1096
export default config;

‎rules/no-array-sort.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import noArrayMutateRule from './shared/no-array-mutate-rule.js';
2+
3+
/** @type {import('eslint').Rule.RuleModule} */
4+
const config = noArrayMutateRule('sort');
5+
6+
export default config;

‎rules/prevent-abbreviations.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ const getWordReplacements = (word, {replacements, allowList}) => {
100100
.map(name => transform(name));
101101
}
102102

103-
return wordReplacement.length > 0 ? wordReplacement.sort() : [];
103+
return wordReplacement.length > 0 ? wordReplacement.toSorted() : [];
104104
};
105105

106106
const getNameReplacements = (name, options, limit = 3) => {

‎rules/shared/no-array-mutate-rule.js

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import {isMethodCall} from '../ast/index.js';
2+
import {getParenthesizedText} from '../utils/index.js';
3+
4+
const MESSAGE_ID_ERROR = 'error';
5+
const MESSAGE_ID_SUGGESTION_APPLY_REPLACEMENT = 'suggestion-apply-replacement';
6+
const MESSAGE_ID_SUGGESTION_SPREADING_ARRAY = 'suggestion-spreading-array';
7+
const MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY = 'suggestion-not-spreading-array';
8+
9+
const methods = new Map([
10+
[
11+
'reverse',
12+
{
13+
replacement: 'toReversed',
14+
predicate: callExpression => isMethodCall(callExpression, {
15+
method: 'reverse',
16+
argumentsLength: 0,
17+
optionalCall: false,
18+
}),
19+
},
20+
],
21+
[
22+
'sort',
23+
{
24+
replacement: 'toSorted',
25+
predicate: callExpression => isMethodCall(callExpression, {
26+
method: 'sort',
27+
maximumArguments: 1,
28+
optionalCall: false,
29+
}),
30+
},
31+
],
32+
]);
33+
34+
const schema = [
35+
{
36+
type: 'object',
37+
additionalProperties: false,
38+
properties: {
39+
allowExpressionStatement: {
40+
type: 'boolean',
41+
},
42+
},
43+
},
44+
];
45+
46+
function noArrayMutateRule(methodName) {
47+
const {
48+
replacement,
49+
predicate,
50+
} = methods.get(methodName);
51+
52+
const messages = {
53+
[MESSAGE_ID_ERROR]: `Use \`Array#${replacement}()\` instead of \`Array#${methodName}()\`.`,
54+
[MESSAGE_ID_SUGGESTION_APPLY_REPLACEMENT]: `Switch to \`.${replacement}()\`.`,
55+
[MESSAGE_ID_SUGGESTION_SPREADING_ARRAY]: 'The spreading object is an array.',
56+
[MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY]: 'The spreading object is NOT an array.',
57+
};
58+
59+
/** @param {import('eslint').Rule.RuleContext} context */
60+
const create = context => {
61+
const {sourceCode} = context;
62+
const {allowExpressionStatement} = context.options[0];
63+
64+
return {
65+
CallExpression(callExpression) {
66+
if (!predicate(callExpression)) {
67+
return;
68+
}
69+
70+
const array = callExpression.callee.object;
71+
72+
// `[...array].reverse()`
73+
const isSpreadAndMutate = array.type === 'ArrayExpression'
74+
&& array.elements.length === 1
75+
&& array.elements[0].type === 'SpreadElement';
76+
77+
if (allowExpressionStatement && !isSpreadAndMutate) {
78+
const maybeExpressionStatement = callExpression.parent.type === 'ChainExpression'
79+
? callExpression.parent.parent
80+
: callExpression.parent;
81+
if (maybeExpressionStatement.type === 'ExpressionStatement') {
82+
return;
83+
}
84+
}
85+
86+
const methodProperty = callExpression.callee.property;
87+
const suggestions = [];
88+
const fixMethodName = fixer => fixer.replaceText(methodProperty, replacement);
89+
90+
/*
91+
For `[...array].reverse()`, provide two suggestions, let user choose if the object can be unwrapped,
92+
otherwise only change `.reverse()` to `.toReversed()`
93+
*/
94+
if (isSpreadAndMutate) {
95+
suggestions.push({
96+
messageId: MESSAGE_ID_SUGGESTION_SPREADING_ARRAY,
97+
* fix(fixer) {
98+
const text = getParenthesizedText(array.elements[0].argument, sourceCode);
99+
yield fixer.replaceText(array, text);
100+
yield fixMethodName(fixer);
101+
},
102+
});
103+
}
104+
105+
suggestions.push({
106+
messageId: isSpreadAndMutate
107+
? MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY
108+
: MESSAGE_ID_SUGGESTION_APPLY_REPLACEMENT,
109+
fix: fixMethodName,
110+
});
111+
112+
return {
113+
node: methodProperty,
114+
messageId: MESSAGE_ID_ERROR,
115+
suggest: suggestions,
116+
};
117+
},
118+
};
119+
};
120+
121+
/** @type {import('eslint').Rule.RuleModule} */
122+
const config = {
123+
create,
124+
meta: {
125+
type: 'suggestion',
126+
docs: {
127+
description: `Prefer \`Array#${replacement}()\` over \`Array#${methodName}()\`.`,
128+
recommended: true,
129+
},
130+
hasSuggestions: true,
131+
schema,
132+
defaultOptions: [{allowExpressionStatement: true}],
133+
messages,
134+
},
135+
};
136+
137+
return config;
138+
}
139+
140+
export default noArrayMutateRule;

‎scripts/create-rules-index-file.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const DIRECTORY = new URL('../rules/', import.meta.url);
66
const files = fs.readdirSync(DIRECTORY, {withFileTypes: true})
77
.filter(file => file.isFile() && file.name.endsWith('.js') && file.name !== 'index.js')
88
.map(file => file.name)
9-
.sort();
9+
.toSorted();
1010

1111
const content = files
1212
.map(file => `export {default as '${path.basename(file, '.js')}'} from './${file}';`)

0 commit comments

Comments
 (0)