Skip to content

Commit

Permalink
no-array-for-each: Ignore React.Children.forEach (#1088)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker committed Feb 6, 2021
1 parent b74f56b commit 5a931dd
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 16 deletions.
23 changes: 9 additions & 14 deletions rules/no-array-callback-reference.js
Expand Up @@ -3,6 +3,7 @@ const {isParenthesized} = require('eslint-utils');
const getDocumentationUrl = require('./utils/get-documentation-url');
const methodSelector = require('./utils/method-selector');
const {notFunctionSelector} = require('./utils/not-function');
const isNodeMatches = require('./utils/is-node-matches');

const ERROR_WITH_NAME_MESSAGE_ID = 'error-with-name';
const ERROR_WITHOUT_NAME_MESSAGE_ID = 'error-without-name';
Expand Down Expand Up @@ -71,7 +72,7 @@ const iteratorMethods = [
const ignoredCallee = [
// http://bluebirdjs.com/docs/api/promise.map.html
'Promise',
'React.children',
'React.Children',
'Children',
'lodash',
'underscore',
Expand All @@ -80,18 +81,6 @@ const ignoredCallee = [
'async'
];

const toSelector = name => {
const splitted = name.split('.');
return `[callee.${'object.'.repeat(splitted.length)}name!="${splitted.shift()}"]`;
};

// Select all the call expressions except the ones present in the ignore list.
const ignoredCalleeSelector = [
// `this.{map, filter, …}()`
'[callee.object.type!="ThisExpression"]',
...ignoredCallee.map(name => toSelector(name))
].join('');

function check(context, node, method, options) {
const {type} = node;

Expand Down Expand Up @@ -162,11 +151,17 @@ const create = context => {
max: 2
}),
options.extraSelector,
ignoredCalleeSelector,
ignoredFirstArgumentSelector
].join('');

rules[selector] = node => {
if (
isNodeMatches(node.callee.object, ignoredCallee) ||
node.callee.object.type === 'ThisExpression'
) {
return;
}

const [iterator] = node.arguments;
check(context, iterator, method, options, sourceCode);
};
Expand Down
10 changes: 10 additions & 0 deletions rules/no-array-for-each.js
Expand Up @@ -14,6 +14,7 @@ const shouldAddParenthesesToExpressionStatementExpression = require('./utils/sho
const getParenthesizedTimes = require('./utils/get-parenthesized-times');
const extendFixRange = require('./utils/extend-fix-range');
const isFunctionSelfUsedInside = require('./utils/is-function-self-used-inside');
const isNodeMatches = require('./utils/is-node-matches');

const MESSAGE_ID = 'no-array-for-each';
const messages = {
Expand Down Expand Up @@ -315,6 +316,11 @@ function isFixable(callExpression, sourceCode, {scope, functionInfo, allIdentifi
return true;
}

const ignoredObjects = [
'React.Children',
'Children'
];

const create = context => {
const functionStack = [];
const callExpressions = [];
Expand Down Expand Up @@ -349,6 +355,10 @@ const create = context => {
returnStatements.push(node);
},
[arrayForEachCallSelector](node) {
if (isNodeMatches(node.callee.object, ignoredObjects)) {
return;
}

callExpressions.push({
node,
scope: context.getScope()
Expand Down
36 changes: 36 additions & 0 deletions rules/utils/is-node-matches.js
@@ -0,0 +1,36 @@
'use strict';

function isNodeMatchesNameOrPath(node, nameOrPath) {
const names = nameOrPath.split('.');
for (let index = names.length - 1; index >= 0; index--) {
const name = names[index];

if (index === 0) {
return node.type === 'Identifier' && node.name === name;
}

if (
node.type !== 'MemberExpression' ||
node.optional ||
node.property.type !== 'Identifier' ||
node.property.name !== name
) {
return false;
}

node = node.object;
}
}

/**
Check if node matches any object name or key path.
@param {Node} node - The AST node to check.
@param {string[]} nameOrPaths - The object name or key paths.
@returns {boolean}
*/
function isNodeMatches(node, nameOrPaths) {
return nameOrPaths.some(nameOrPath => isNodeMatchesNameOrPath(node, nameOrPath));
}

module.exports = isNodeMatches;
12 changes: 10 additions & 2 deletions test/no-array-for-each.js
Expand Up @@ -5,7 +5,10 @@ test.snapshot({
valid: [
'new foo.forEach(element => bar())',
'forEach(element => bar())',
'foo.notForEach(element => bar())'
'foo.notForEach(element => bar())',
// #1087
'React.Children.forEach(children, (child) => {});',
'Children.forEach(children, (child) => {});'
],
invalid: [
// Not fixable
Expand Down Expand Up @@ -334,7 +337,12 @@ test.snapshot({
bar() {}
}
].forEach((Foo, bar) => process(Foo, bar))
`
`,
'foo.React.Children.forEach(bar)',
'NotReact.Children.forEach(bar)',
'React.NotChildren.forEach(bar)',
'React?.Children.forEach(bar)',
'NotChildren.forEach(bar)'
]
});

Expand Down
50 changes: 50 additions & 0 deletions test/snapshots/no-array-for-each.js.md
Expand Up @@ -1404,3 +1404,53 @@ Generated by [AVA](https://avajs.dev).
> 5 | ].forEach((Foo, bar) => process(Foo, bar))␊
| ^^^^^^^ Do not use `Array#forEach(…)`.␊
`

## Invalid #77
1 | foo.React.Children.forEach(bar)

> Error 1/1
`␊
> 1 | foo.React.Children.forEach(bar)␊
| ^^^^^^^ Do not use `Array#forEach(…)`.␊
`

## Invalid #78
1 | NotReact.Children.forEach(bar)

> Error 1/1
`␊
> 1 | NotReact.Children.forEach(bar)␊
| ^^^^^^^ Do not use `Array#forEach(…)`.␊
`

## Invalid #79
1 | React.NotChildren.forEach(bar)

> Error 1/1
`␊
> 1 | React.NotChildren.forEach(bar)␊
| ^^^^^^^ Do not use `Array#forEach(…)`.␊
`

## Invalid #80
1 | React?.Children.forEach(bar)

> Error 1/1
`␊
> 1 | React?.Children.forEach(bar)␊
| ^^^^^^^ Do not use `Array#forEach(…)`.␊
`

## Invalid #81
1 | NotChildren.forEach(bar)

> Error 1/1
`␊
> 1 | NotChildren.forEach(bar)␊
| ^^^^^^^ Do not use `Array#forEach(…)`.␊
`
Binary file modified test/snapshots/no-array-for-each.js.snap
Binary file not shown.

0 comments on commit 5a931dd

Please sign in to comment.