Add no-array-for-each rule

docs/rules/no-array-for-each.md

@@ -0,0 +1,17 @@
+# Prefer `for…of` over `Array#forEach(…)`.
+
+<!-- More detailed description. Remove this comment. -->
+
+This rule is partly fixable.
+
+## Fail
+
+```js
+const foo = 'unicorn';
+```
+
+## Pass
+
+```js
+const foo = '🦄';
+```

index.js

@@ -55,6 +55,7 @@ module.exports = {
 				'unicorn/new-for-builtins': 'error',
 				'unicorn/no-abusive-eslint-disable': 'error',
 				'unicorn/no-array-callback-reference': 'error',
+				'unicorn/no-array-for-each': 'error',
 				'unicorn/no-array-reduce': 'error',
 				'unicorn/no-console-spaces': 'error',
 				'unicorn/no-for-loop': 'error',

readme.md

@@ -49,6 +49,7 @@ Configure it in `package.json`.
 			"unicorn/new-for-builtins": "error",
 			"unicorn/no-abusive-eslint-disable": "error",
 			"unicorn/no-array-callback-reference": "error",
+			"unicorn/no-array-for-each": "error",
 			"unicorn/no-array-reduce": "error",
 			"unicorn/no-console-spaces": "error",
 			"unicorn/no-for-loop": "error",
@@ -125,6 +126,7 @@ Configure it in `package.json`.
 - [new-for-builtins](docs/rules/new-for-builtins.md) - Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. *(partly fixable)*
 - [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) - Enforce specifying rules to disable in `eslint-disable` comments.
 - [no-array-callback-reference](docs/rules/no-array-callback-reference.md) - Prevent passing a function reference directly to iterator methods.
+- [no-array-for-each](docs/rules/no-array-for-each.md) - Prefer `for…of` over `Array#forEach(…)`. *(partly fixable)*
 - [no-array-reduce](docs/rules/no-array-reduce.md) - Disallow `Array#reduce()` and `Array#reduceRight()`.
 - [no-console-spaces](docs/rules/no-console-spaces.md) - Do not use leading/trailing space between `console.log` parameters. *(fixable)*
 - [no-for-loop](docs/rules/no-for-loop.md) - Do not use a `for` loop that can be replaced with a `for-of` loop. *(partly fixable)*

rules/no-array-for-each.js

@@ -0,0 +1,284 @@
+'use strict';
+const {isParenthesized, isArrowToken, isCommaToken, isSemicolonToken} = require('eslint-utils');
+const getDocumentationUrl = require('./utils/get-documentation-url');
+const methodSelector = require('./utils/method-selector');
+const needsSemicolon = require('./utils/needs-semicolon');
+const shouldAddParenthesesToMemberExpressionObject = require('./utils/should-add-parentheses-to-member-expression-object');
+const shouldAddParenthesesToExpressionStatementExpression = require('./utils/should-add-parentheses-to-expression-statement-expression');
+
+const MESSAGE_ID = 'no-array-for-each';
+const messages = {
+	[MESSAGE_ID]: 'Do not use `Array#forEach(…)`.'
+};
+
+const arrayForEachCallSelector = methodSelector({
+	name: 'forEach',
+	includeOptional: true
+});
+
+const continueAbleNodeTypes = new Set([
+	'WhileStatement',
+	'DoWhileStatement',
+	'ForStatement',
+	'ForOfStatement',
+	'ForInStatement'
+]);
+
+function isReturnStatementInContinueAbleNodes(returnStatement, callbackFunction) {
+	for (let node = returnStatement; node && node !== callbackFunction; node = node.parent) {
+		if (continueAbleNodeTypes.has(node.type)) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
+function getFixFunction(callExpression, sourceCode, functionReturnStatements) {
+	const [callback] = callExpression.arguments;
+	const parameters = callback.params;
+	const array = callExpression.callee.object;
+	const returnStatements = functionReturnStatements.get(callback);
+
+	const getForOfLoopHeadText = () => {
+		const parametersText = parameters.map(parameter => sourceCode.getText(parameter));
+		const useEntries = parameters.length === 2;
+
+		let text = 'for (const ';
+		text += useEntries ? `[${parametersText.join(', ')}]` : parametersText[0];
+
+		text += ' of ';
+
+		let arrayText = sourceCode.getText(array);
+		if (
+			isParenthesized(callExpression, sourceCode) ||
+			(useEntries && shouldAddParenthesesToMemberExpressionObject(array, sourceCode))
+		) {
+			arrayText = `(${arrayText})`;
+		}
+
+		text += arrayText;
+
+		if (useEntries) {
+			text += '.entries()';
+		}
+
+		text += ') ';
+
+		return text;
+	};
+
+	const getForOfLoopHeadRange = () => {
+		const [start] = callExpression.range;
+		let end;
+		if (callback.body.type === 'BlockStatement') {
+			end = callback.body.range[0];
+		} else {
+			const arrowToken = sourceCode.getFirstToken(callback, isArrowToken);
+			end = arrowToken.range[1];
+		}
+
+		return [start, end];
+	};
+
+	function * replaceReturnStatement(returnStatement, fixer) {
+		const returnToken = sourceCode.getFirstToken(returnStatement);
+
+		/* istanbul ignore next: `ReturnStatement` firstToken should be `return` */
+		if (returnToken.value !== 'return') {
+			throw new Error(`Unexpected token ${returnToken.value}.`);
+		}
+
+		if (!returnStatement.argument) {
+			yield fixer.replaceText(returnToken, 'continue');
+			return;
+		}
+
+		// Remove `return`
+		yield fixer.remove(returnToken);
+
+		const previousToken = sourceCode.getTokenBefore(returnToken);
+		const nextToken = sourceCode.getTokenAfter(returnToken);
+		let textBefore = '';
+		let textAfter = '';
+		const shouldAddParentheses =
+			!isParenthesized(returnStatement.argument, sourceCode) &&
+			shouldAddParenthesesToExpressionStatementExpression(returnStatement.argument);
+		if (shouldAddParentheses) {
+			textBefore = '(';
+			textAfter = ')';
+		}
+
+		const shouldAddSemicolonBefore = needsSemicolon(previousToken, sourceCode, shouldAddParentheses ? '(' : nextToken.value);
+		if (shouldAddSemicolonBefore) {
+			textBefore = `;${textBefore}`;
+		}
+
+		if (textBefore) {
+			yield fixer.insertTextBefore(nextToken, textBefore);
+		}
+
+		if (textAfter) {
+			yield fixer.insertTextAfter(returnStatement.argument, textAfter);
+		}
+
+		// If `returnStatement` has no semi
+		const lastToken = sourceCode.getLastToken(returnStatement);
+		yield fixer.insertTextAfter(
+			returnStatement,
+			`${isSemicolonToken(lastToken) ? '' : ';'} continue;`
+		);
+	}
+
+	const shouldRemoveExpressionStatementLastToken = (token) => {
+		if (!isSemicolonToken(token)) {
+			return false;
+		}
+
+		if (callback.body.type === 'BlockStatement') {
+			return true;
+		}
+
+		const nextToken = sourceCode.getTokenAfter(token);
+		if (nextToken && needsSemicolon(token, sourceCode, nextToken.value)) {
+			return false;
+		}
+
+		return true;
+	};
+
+	return function * (fixer) {
+		yield fixer.replaceTextRange(getForOfLoopHeadRange(), getForOfLoopHeadText());
+
+		// Remove call expression trailing comma
+		const [penultimateToken, lastToken] = sourceCode.getLastTokens(callExpression, 2);
+		if (isCommaToken(penultimateToken)) {
+			yield fixer.remove(penultimateToken);
+		}
+
+		yield fixer.remove(lastToken);
+
+		for (const returnStatement of returnStatements) {
+			yield * replaceReturnStatement(returnStatement, fixer);
+		}
+
+		const expressionStatementLastToken = sourceCode.getLastToken(callExpression.parent);
+		if (shouldRemoveExpressionStatementLastToken(expressionStatementLastToken)) {
+			yield fixer.remove(expressionStatementLastToken, fixer);
+		}
+	};
+}
+
+function isFixable(callExpression, sourceCode, functionReturnStatements) {
+	// Check `CallExpression`
+	if (
+		callExpression.optional ||
+		isParenthesized(callExpression, sourceCode) ||
+		callExpression.arguments.length !== 1
+	) {
+		return false;
+	}
+
+	// Check `CallExpression.parent`
+	if (callExpression.parent.type !== 'ExpressionStatement') {
+		return false;
+	}
+
+	// Check `CallExpression.callee`
+	if (callExpression.callee.optional) {
+		return false;
+	}
+
+	// Check `CallExpression.arguments[0]`;
+	const [callback] = callExpression.arguments;
+	if (
+		// Leave non-function type to `no-array-callback-reference` rule
+		(callback.type !== 'FunctionExpression' && callback.type !== 'ArrowFunctionExpression') ||
+    callback.async ||
+    callback.generator
+	) {
+		return false;
+	}
+
+	// Check `callback.params`
+	const parameters = callback.params;
+	if (
+		!(parameters.length === 1 || parameters.length === 2) ||
+		parameters.some(parameter => parameter.type !== 'Identifier')
+	) {
+		return false;
+	}
+
+	// TODO: check parameters conflicts
+
+	// Check `ReturnStatement`s in `callback`
+	const returnStatements = functionReturnStatements.get(callback);
+	if (returnStatements.some(returnStatement => isReturnStatementInContinueAbleNodes(returnStatement, callback))) {
+		return false;
+	}
+
+	// Check `callback` self
+	if (callback.type === 'FunctionExpression') {
+		// TODO: check `.id` `arguments` `this` of `FunctionExpression`
+	}
+
+	return true;
+}
+
+const create = context => {
+	const functionStacks = [];
+	const functionReturnStatements = new Map();
+	const callExpressions = [];
+
+	const sourceCode = context.getSourceCode();
+
+	return {
+		':function'(node) {
+			functionStacks.push(node);
+			functionReturnStatements.set(node, []);
+		},
+		':function:exit'() {
+			functionStacks.pop();
+		},
+		ReturnStatement(node) {
+			const currentFunction = functionStacks[functionStacks.length - 1];
+			// `globalReturn `
+			/* istanbul ignore next: ESLint deprecated `ecmaFeatures`, can't test */
+			if (!currentFunction) {
+				return;
+			}
+
+			const returnStatements = functionReturnStatements.get(currentFunction);
+			returnStatements.push(node);
+		},
+		[arrayForEachCallSelector](node) {
+			callExpressions.push(node);
+		},
+		'Program:exit'() {
+			for (const callExpression of callExpressions) {
+				const problem = {
+					node: callExpression.callee.property,
+					messageId: MESSAGE_ID
+				};
+
+				if (isFixable(callExpression, sourceCode, functionReturnStatements)) {
+					problem.fix = getFixFunction(callExpression, sourceCode, functionReturnStatements);
+				}
+
+				context.report(problem);
+			}
+		}
+	};
+};
+
+module.exports = {
+	create,
+	meta: {
+		type: 'suggestion',
+		docs: {
+			url: getDocumentationUrl(__filename)
+		},
+		fixable: 'code',
+		messages
+	}
+};

rules/utils/method-selector.js

@@ -8,7 +8,8 @@ module.exports = options => {
 		object,
 		min,
 		max,
-		property = ''
+		property = '',
+		includeOptional = false
 	} = {
 		min: 0,
 		max: Number.POSITIVE_INFINITY,
@@ -20,10 +21,13 @@ module.exports = options => {
 	const selector = [
 		`[${prefix}type="CallExpression"]`,
 		`[${prefix}callee.type="MemberExpression"]`,
-		`[${prefix}callee.computed=false]`,
 		`[${prefix}callee.property.type="Identifier"]`
 	];
 
+	if (!includeOptional) {
+		selector.push(`[${prefix}callee.computed=false]`);
+	}
+
 	if (name) {
 		selector.push(`[${prefix}callee.property.name="${name}"]`);
 	}

rules/utils/should-add-parentheses-to-expression-statement-expression.js

@@ -0,0 +1,21 @@
+'use strict';
+
+/**
+Check if parentheses should to be added to a `node` when it's used as an `expression` of `ExpressionStatement`.
+
+@param {Node} node - The AST node to check.
+@param {SourceCode} sourceCode - The source code object.
+@returns {boolean}
+*/
+function shouldAddParenthesesToExpressionStatementExpression(node) {
+	switch (node.type) {
+		case 'ObjectExpression':
+			return true;
+		case 'AssignmentExpression':
+			return node.left.type === 'ObjectPattern' || node.left.type === 'ArrayPattern';
+		default:
+			return false;
+	}
+}
+
+module.exports = shouldAddParenthesesToExpressionStatementExpression;