Skip to content

Commit

Permalink
tools: add ESLint rule no-array-destructuring
Browse files Browse the repository at this point in the history
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 <rtrott@gmail.com>
  • Loading branch information
aduh95 committed Mar 3, 2021
1 parent f34d8de commit 26288ff
Show file tree
Hide file tree
Showing 3 changed files with 225 additions and 0 deletions.
1 change: 1 addition & 0 deletions lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rules:
# Custom rules in tools/eslint-rules
node-core/lowercase-name-for-primitive: error
node-core/non-ascii-character: error
node-core/no-array-destructuring: error
node-core/prefer-primordials:
- error
- name: Array
Expand Down
141 changes: 141 additions & 0 deletions test/parallel/test-eslint-no-array-destructuring.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

common.skipIfEslintMissing();

const { RuleTester } = require('../../tools/node_modules/eslint');
const rule = require('../../tools/eslint-rules/no-array-destructuring');

const USE_OBJ_DESTRUCTURING =
'Use object destructuring instead of array destructuring.';
const USE_ARRAY_METHODS =
'Use primordials.ArrayPrototypeSlice to avoid unsafe array iteration.';

new RuleTester({
parserOptions: { ecmaVersion: 2021 },
env: { es6: true }
})
.run('no-array-destructuring', rule, {
valid: [
'const first = [1, 2, 3][0];',
'const {0:first} = [1, 2, 3];',
'({1:elem} = array);',
'function name(param, { 0: key, 1: value },) {}',
],
invalid: [
{
code: 'const [Array] = args;',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'const {0:Array} = args;'
},
{
code: 'const [ , res] = args;',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'const { 1:res} = args;',
},
{
code: '[, elem] = options;',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: '({ 1:elem} = options);',
},
{
code: 'const {values:[elem]} = options;',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'const {values:{0:elem}} = options;',
},
{
code: '[[[elem]]] = options;',
errors: [
{ message: USE_OBJ_DESTRUCTURING },
{ message: USE_OBJ_DESTRUCTURING },
{ message: USE_OBJ_DESTRUCTURING },
],
output: '({0:[[elem]]} = options);',
},
{
code: '[, ...rest] = options;',
errors: [{ message: USE_ARRAY_METHODS }],
},
{
code: 'for(const [key, value] of new Map);',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'for(const {0:key, 1:value} of new Map);',
},
{
code: 'let [first,,,fourth] = array;',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'let {0:first,3:fourth} = array;',
},
{
code: 'let [,second,,fourth] = array;',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'let {1:second,3:fourth} = array;',
},
{
code: 'let [ ,,,fourth ] = array;',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'let { 3:fourth } = array;',
},
{
code: 'let [,,,fourth, fifth,, minorFall, majorLift,...music] = arr;',
errors: [{ message: USE_ARRAY_METHODS }],
},
{
code: 'function map([key, value]) {}',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'function map({0:key, 1:value}) {}',
},
{
code: 'function map([key, value],) {}',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'function map({0:key, 1:value},) {}',
},
{
code: '(function([key, value]) {})',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: '(function({0:key, 1:value}) {})',
},
{
code: '(function([key, value] = [null, 0]) {})',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: '(function({0:key, 1:value} = [null, 0]) {})',
},
{
code: 'function map([key, ...values]) {}',
errors: [{ message: USE_ARRAY_METHODS }],
},
{
code: 'function map([key, value], ...args) {}',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'function map({0:key, 1:value}, ...args) {}',
},
{
code: 'async function map([key, value], ...args) {}',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'async function map({0:key, 1:value}, ...args) {}',
},
{
code: 'async function* generator([key, value], ...args) {}',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'async function* generator({0:key, 1:value}, ...args) {}',
},
{
code: 'function* generator([key, value], ...args) {}',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'function* generator({0:key, 1:value}, ...args) {}',
},
{
code: 'const cb = ([key, value], ...args) => {}',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'const cb = ({0:key, 1:value}, ...args) => {}',
},
{
code: 'class name{ method([key], ...args){} }',
errors: [{ message: USE_OBJ_DESTRUCTURING }],
output: 'class name{ method({0:key}, ...args){} }',
},
]
});
83 changes: 83 additions & 0 deletions tools/eslint-rules/no-array-destructuring.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* @fileoverview 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 file defines 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.
* @author aduh95 <duhamelantoine1995@gmail.com>
*/
'use strict';

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const USE_OBJ_DESTRUCTURING =
'Use object destructuring instead of array destructuring.';
const USE_ARRAY_METHODS =
'Use primordials.ArrayPrototypeSlice to avoid unsafe array iteration.';

const findComma = (sourceCode, elements, i, start) => {
if (i === 0)
return sourceCode.getTokenAfter(sourceCode.getTokenByRangeStart(start));

let element;
const originalIndex = i;
while (i && !element) {
element = elements[--i];
}
let token = sourceCode.getTokenAfter(
element ?? sourceCode.getTokenByRangeStart(start)
);
for (; i < originalIndex; i++) {
token = sourceCode.getTokenAfter(token);
}
return token;
};
const createFix = (fixer, sourceCode, { range: [start, end], elements }) => [
fixer.replaceTextRange([start, start + 1], '{'),
fixer.replaceTextRange([end - 1, end], '}'),
...elements.map((node, i) =>
(node === null ?
fixer.remove(findComma(sourceCode, elements, i, start)) :
fixer.insertTextBefore(node, i + ':'))
),
];
const arrayPatternContainsRestOperator = ({ elements }) =>
elements?.find((node) => node?.type === 'RestElement');

module.exports = {
meta: {
type: 'suggestion',
fixable: 'code',
schema: [],
},
create(context) {
const sourceCode = context.getSourceCode();

return {
ArrayPattern(node) {
const hasRest = arrayPatternContainsRestOperator(node);
context.report({
message: hasRest ? USE_ARRAY_METHODS : USE_OBJ_DESTRUCTURING,
node: hasRest || node,
fix: hasRest ?
undefined :
(fixer) => [
...(node.parent.type === 'AssignmentExpression' ?
[
fixer.insertTextBefore(node.parent, '('),
fixer.insertTextAfter(node.parent, ')'),
] :
[]),
...createFix(fixer, sourceCode, node),
],
});

},
};
},
};

0 comments on commit 26288ff

Please sign in to comment.