Skip to content

Commit b051302

Browse files
fiskerfisker
authored
Add no-array-reverse rule (#2677)
1 parent 2dc27d9 commit b051302

File tree

10 files changed

+395
-3
lines changed

10 files changed

+395
-3
lines changed

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Prefer `Array#toReversed()` over `Array#reverse()`
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#toReversed()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reverse) over [`Array#reverse()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reverse).
11+
12+
## Examples
13+
14+
```js
15+
// ❌
16+
const reversed = [...array].reverse();
17+
18+
// ✅
19+
const reversed = [...array].toReversed();
20+
```
21+
22+
## Options
23+
24+
Type: `object`
25+
26+
### allowExpressionStatement
27+
28+
Type: `boolean`\
29+
Default: `true`
30+
31+
This rule allow `array.reverse()` as an expression statement by default.
32+
Pass `allowExpressionStatement: false` to forbid `Array#reverse()` even it's an expression statement.
33+
34+
#### Fail
35+
36+
```js
37+
// eslint unicorn/no-array-reverse: ["error", {"allowExpressionStatement": true}]
38+
array.reverse();
39+
```

‎readme.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export default [
8080
| [no-array-for-each](docs/rules/no-array-for-each.md) | Prefer `for…of` over the `forEach` method. | ✅ | 🔧 | 💡 |
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()`. | ✅ | | |
83+
| [no-array-reverse](docs/rules/no-array-reverse.md) | Prefer `Array#toReversed()` over `Array#reverse()`. | ✅ | | 💡 |
8384
| [no-await-expression-member](docs/rules/no-await-expression-member.md) | Disallow member access from await expression. | ✅ | 🔧 | |
8485
| [no-await-in-promise-methods](docs/rules/no-await-in-promise-methods.md) | Disallow using `await` in `Promise` method parameters. | ✅ | | 💡 |
8586
| [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
@@ -24,6 +24,7 @@ export {default as 'no-array-callback-reference'} from './no-array-callback-refe
2424
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';
27+
export {default as 'no-array-reverse'} from './no-array-reverse.js';
2728
export {default as 'no-await-expression-member'} from './no-await-expression-member.js';
2829
export {default as 'no-await-in-promise-methods'} from './no-await-in-promise-methods.js';
2930
export {default as 'no-console-spaces'} from './no-console-spaces.js';

‎rules/no-array-reverse.js

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
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+
];
92+
93+
/** @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+
};
108+
109+
export default config;

‎rules/no-nested-ternary.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const create = context => ({
1919
}
2020

2121
const {sourceCode} = context;
22-
const ancestors = sourceCode.getAncestors(node).reverse();
22+
const ancestors = sourceCode.getAncestors(node).toReversed();
2323
const nestLevel = ancestors.findIndex(node => node.type !== 'ConditionalExpression');
2424

2525
if (nestLevel === 1 && !isParenthesized(node, sourceCode)) {

‎rules/template-indent.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ const create = context => {
129129
}
130130

131131
if (options.selectors.length > 0) {
132-
const ancestors = sourceCode.getAncestors(node).reverse();
132+
const ancestors = sourceCode.getAncestors(node).toReversed();
133133
if (options.selectors.some(selector => esquery.matches(node, parseEsquerySelector(selector), ancestors))) {
134134
return true;
135135
}

‎rules/utils/global-reference-tracker.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {ReferenceTracker} from '@eslint-community/eslint-utils';
33
const createTraceMap = (object, type) => {
44
let map = {[type]: true};
55

6-
const path = object.split('.').reverse();
6+
const path = object.split('.').toReversed();
77
for (const name of path) {
88
map = {[name]: map};
99
}

‎test/no-array-reverse.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import {getTester} from './utils/test.js';
2+
3+
const {test} = getTester(import.meta);
4+
5+
const FORBID_EXPRESSION_OPTIONS = [{allowExpressionStatement: false}];
6+
7+
test.snapshot({
8+
valid: [
9+
'reversed =[...array].toReversed()',
10+
'reversed =array.toReversed()',
11+
'reversed =[...array].reverse',
12+
'reversed =[...array].reverse?.()',
13+
'array.reverse()',
14+
'array.reverse?.()',
15+
'array?.reverse()',
16+
'if (true) array.reverse()',
17+
'reversed = array.reverse(extraArgument)',
18+
],
19+
invalid: [
20+
'reversed = [...array].reverse()',
21+
'reversed = [...array]?.reverse()',
22+
'reversed = array.reverse()',
23+
'reversed = array?.reverse()',
24+
{
25+
code: 'array.reverse()',
26+
options: FORBID_EXPRESSION_OPTIONS,
27+
},
28+
{
29+
code: 'array?.reverse()',
30+
options: FORBID_EXPRESSION_OPTIONS,
31+
},
32+
// Don't care about `allowExpression`
33+
{
34+
code: '[...array].reverse()',
35+
options: FORBID_EXPRESSION_OPTIONS,
36+
},
37+
'reversed = [...(0, array)].reverse()',
38+
],
39+
});

0 commit comments

Comments
 (0)