Skip to content

Commit 26288ff

Browse files
aduh95aduh95
committed
tools: add ESLint rule no-array-destructuring
Iterating over arrays should be avoided because it relies on user-mutable global methods (`Array.prototype[Symbol.iterator]` and `%ArrayIteratorPrototype%.next`), we should instead use other alternatives. This commit adds a rule that disallow array destructuring syntax in favor of object destructuring syntax. Note that you can ignore this rule if you are using the array destructuring syntax over a safe iterable, or actually want to iterate over a user-provided object. PR-URL: #36818 Reviewed-By: Rich Trott <[email protected]>
1 parent f34d8de commit 26288ff

File tree

3 files changed

+225
-0
lines changed

3 files changed

+225
-0
lines changed

‎lib/.eslintrc.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ rules:
2828
# Custom rules in tools/eslint-rules
2929
node-core/lowercase-name-for-primitive: error
3030
node-core/non-ascii-character: error
31+
node-core/no-array-destructuring: error
3132
node-core/prefer-primordials:
3233
- error
3334
- name: Array

‎test/parallel/test-eslint-no-array-destructuring.js

+141
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
common.skipIfEslintMissing();
8+
9+
const { RuleTester } = require('../../tools/node_modules/eslint');
10+
const rule = require('../../tools/eslint-rules/no-array-destructuring');
11+
12+
const USE_OBJ_DESTRUCTURING =
13+
'Use object destructuring instead of array destructuring.';
14+
const USE_ARRAY_METHODS =
15+
'Use primordials.ArrayPrototypeSlice to avoid unsafe array iteration.';
16+
17+
new RuleTester({
18+
parserOptions: { ecmaVersion: 2021 },
19+
env: { es6: true }
20+
})
21+
.run('no-array-destructuring', rule, {
22+
valid: [
23+
'const first = [1, 2, 3][0];',
24+
'const {0:first} = [1, 2, 3];',
25+
'({1:elem} = array);',
26+
'function name(param, { 0: key, 1: value },) {}',
27+
],
28+
invalid: [
29+
{
30+
code: 'const [Array] = args;',
31+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
32+
output: 'const {0:Array} = args;'
33+
},
34+
{
35+
code: 'const [ , res] = args;',
36+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
37+
output: 'const { 1:res} = args;',
38+
},
39+
{
40+
code: '[, elem] = options;',
41+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
42+
output: '({ 1:elem} = options);',
43+
},
44+
{
45+
code: 'const {values:[elem]} = options;',
46+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
47+
output: 'const {values:{0:elem}} = options;',
48+
},
49+
{
50+
code: '[[[elem]]] = options;',
51+
errors: [
52+
{ message: USE_OBJ_DESTRUCTURING },
53+
{ message: USE_OBJ_DESTRUCTURING },
54+
{ message: USE_OBJ_DESTRUCTURING },
55+
],
56+
output: '({0:[[elem]]} = options);',
57+
},
58+
{
59+
code: '[, ...rest] = options;',
60+
errors: [{ message: USE_ARRAY_METHODS }],
61+
},
62+
{
63+
code: 'for(const [key, value] of new Map);',
64+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
65+
output: 'for(const {0:key, 1:value} of new Map);',
66+
},
67+
{
68+
code: 'let [first,,,fourth] = array;',
69+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
70+
output: 'let {0:first,3:fourth} = array;',
71+
},
72+
{
73+
code: 'let [,second,,fourth] = array;',
74+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
75+
output: 'let {1:second,3:fourth} = array;',
76+
},
77+
{
78+
code: 'let [ ,,,fourth ] = array;',
79+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
80+
output: 'let { 3:fourth } = array;',
81+
},
82+
{
83+
code: 'let [,,,fourth, fifth,, minorFall, majorLift,...music] = arr;',
84+
errors: [{ message: USE_ARRAY_METHODS }],
85+
},
86+
{
87+
code: 'function map([key, value]) {}',
88+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
89+
output: 'function map({0:key, 1:value}) {}',
90+
},
91+
{
92+
code: 'function map([key, value],) {}',
93+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
94+
output: 'function map({0:key, 1:value},) {}',
95+
},
96+
{
97+
code: '(function([key, value]) {})',
98+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
99+
output: '(function({0:key, 1:value}) {})',
100+
},
101+
{
102+
code: '(function([key, value] = [null, 0]) {})',
103+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
104+
output: '(function({0:key, 1:value} = [null, 0]) {})',
105+
},
106+
{
107+
code: 'function map([key, ...values]) {}',
108+
errors: [{ message: USE_ARRAY_METHODS }],
109+
},
110+
{
111+
code: 'function map([key, value], ...args) {}',
112+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
113+
output: 'function map({0:key, 1:value}, ...args) {}',
114+
},
115+
{
116+
code: 'async function map([key, value], ...args) {}',
117+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
118+
output: 'async function map({0:key, 1:value}, ...args) {}',
119+
},
120+
{
121+
code: 'async function* generator([key, value], ...args) {}',
122+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
123+
output: 'async function* generator({0:key, 1:value}, ...args) {}',
124+
},
125+
{
126+
code: 'function* generator([key, value], ...args) {}',
127+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
128+
output: 'function* generator({0:key, 1:value}, ...args) {}',
129+
},
130+
{
131+
code: 'const cb = ([key, value], ...args) => {}',
132+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
133+
output: 'const cb = ({0:key, 1:value}, ...args) => {}',
134+
},
135+
{
136+
code: 'class name{ method([key], ...args){} }',
137+
errors: [{ message: USE_OBJ_DESTRUCTURING }],
138+
output: 'class name{ method({0:key}, ...args){} }',
139+
},
140+
]
141+
});

‎tools/eslint-rules/no-array-destructuring.js

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/**
2+
* @fileoverview Iterating over arrays should be avoided because it relies on
3+
* user-mutable global methods (`Array.prototype[Symbol.iterator]`
4+
* and `%ArrayIteratorPrototype%.next`), we should instead use
5+
* other alternatives. This file defines a rule that disallow
6+
* array destructuring syntax in favor of object destructuring
7+
* syntax. Note that you can ignore this rule if you are using
8+
* the array destructuring syntax over a safe iterable, or
9+
* actually want to iterate over a user-provided object.
10+
* @author aduh95 <[email protected]>
11+
*/
12+
'use strict';
13+
14+
//------------------------------------------------------------------------------
15+
// Rule Definition
16+
//------------------------------------------------------------------------------
17+
18+
const USE_OBJ_DESTRUCTURING =
19+
'Use object destructuring instead of array destructuring.';
20+
const USE_ARRAY_METHODS =
21+
'Use primordials.ArrayPrototypeSlice to avoid unsafe array iteration.';
22+
23+
const findComma = (sourceCode, elements, i, start) => {
24+
if (i === 0)
25+
return sourceCode.getTokenAfter(sourceCode.getTokenByRangeStart(start));
26+
27+
let element;
28+
const originalIndex = i;
29+
while (i && !element) {
30+
element = elements[--i];
31+
}
32+
let token = sourceCode.getTokenAfter(
33+
element ?? sourceCode.getTokenByRangeStart(start)
34+
);
35+
for (; i < originalIndex; i++) {
36+
token = sourceCode.getTokenAfter(token);
37+
}
38+
return token;
39+
};
40+
const createFix = (fixer, sourceCode, { range: [start, end], elements }) => [
41+
fixer.replaceTextRange([start, start + 1], '{'),
42+
fixer.replaceTextRange([end - 1, end], '}'),
43+
...elements.map((node, i) =>
44+
(node === null ?
45+
fixer.remove(findComma(sourceCode, elements, i, start)) :
46+
fixer.insertTextBefore(node, i + ':'))
47+
),
48+
];
49+
const arrayPatternContainsRestOperator = ({ elements }) =>
50+
elements?.find((node) => node?.type === 'RestElement');
51+
52+
module.exports = {
53+
meta: {
54+
type: 'suggestion',
55+
fixable: 'code',
56+
schema: [],
57+
},
58+
create(context) {
59+
const sourceCode = context.getSourceCode();
60+
61+
return {
62+
ArrayPattern(node) {
63+
const hasRest = arrayPatternContainsRestOperator(node);
64+
context.report({
65+
message: hasRest ? USE_ARRAY_METHODS : USE_OBJ_DESTRUCTURING,
66+
node: hasRest || node,
67+
fix: hasRest ?
68+
undefined :
69+
(fixer) => [
70+
...(node.parent.type === 'AssignmentExpression' ?
71+
[
72+
fixer.insertTextBefore(node.parent, '('),
73+
fixer.insertTextAfter(node.parent, ')'),
74+
] :
75+
[]),
76+
...createFix(fixer, sourceCode, node),
77+
],
78+
});
79+
80+
},
81+
};
82+
},
83+
};

0 commit comments

Comments
 (0)