no-array-for-each: Ignore React.Children.forEach (#1088)

Author: fisker

rules/no-array-callback-reference.js

@@ -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';
@@ -71,7 +72,7 @@ const iteratorMethods = [
 const ignoredCallee = [
 	// http://bluebirdjs.com/docs/api/promise.map.html
 	'Promise',
-	'React.children',
+	'React.Children',
 	'Children',
 	'lodash',
 	'underscore',
@@ -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;
 
@@ -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);
 		};

rules/no-array-for-each.js

@@ -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 = {
@@ -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 = [];
@@ -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()

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;

test/no-array-for-each.js

@@ -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
@@ -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)'
 	]
 });
 

test/snapshots/no-array-for-each.js.md

@@ -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(…)`.␊
+    `