Enhance no-array-for-each to handle optional chaining

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

@@ -21,6 +21,12 @@ array.forEach(element => {
 });
 ```
 
+```js
+array?.forEach(element => {
+	bar(element);
+});
+```
+
 ```js
 array.forEach((element, index) => {
 	bar(element, index);

rules/no-array-for-each.js

@@ -6,6 +6,7 @@ const {
 	isSemicolonToken,
 	isClosingParenToken,
 	findVariable,
+	hasSideEffect,
 } = require('eslint-utils');
 const {methodCallSelector, referenceIdentifierSelector} = require('./selectors/index.js');
 const {extendFixRange} = require('./fix/index.js');
@@ -73,6 +74,7 @@ function getFixFunction(callExpression, functionInfo, context) {
 	const parameters = callback.params;
 	const array = callExpression.callee.object;
 	const {returnStatements} = functionInfo.get(callback);
+	const isOptionalChaining = callExpression.callee.optional;
 
 	const getForOfLoopHeadText = () => {
 		const [elementText, indexText] = parameters.map(parameter => sourceCode.getText(parameter));
@@ -182,6 +184,11 @@ function getFixFunction(callExpression, functionInfo, context) {
 		return true;
 	};
 
+	const getIfCondition = (calleeObject) => {
+		if (calleeObject.type === 'Identifier') return calleeObject.name
+		return sourceCode.getText(calleeObject)
+	}
+
 	function * removeCallbackParentheses(fixer) {
 		// Opening parenthesis tokens already included in `getForOfLoopHeadRange`
 		const closingParenthesisTokens = getParentheses(callback, sourceCode)
@@ -228,7 +235,7 @@ function getFixFunction(callExpression, functionInfo, context) {
 			yield * replaceReturnStatement(returnStatement, fixer);
 		}
 
-		const expressionStatementLastToken = sourceCode.getLastToken(callExpression.parent);
+		const expressionStatementLastToken = sourceCode.getLastToken(isOptionalChaining ? callExpression.parent.parent : callExpression.parent);
 		// Remove semicolon if it's not needed anymore
 		// foo.forEach(bar => {});
 		//                       ^
@@ -238,6 +245,11 @@ function getFixFunction(callExpression, functionInfo, context) {
 
 		yield * fixSpaceAroundKeyword(fixer, callExpression.parent, sourceCode);
 
+		if (isOptionalChaining) {
+			const ifCondition = callExpression.callee.object.type === 'Identifier' ? callExpression.callee.object.name : sourceCode.getText(callExpression.callee.object)
+			yield fixer.insertTextBefore(callExpression, `if (${ifCondition}) `);
+		}
+
 		// Prevent possible variable conflicts
 		yield * extendFixRange(fixer, callExpression.parent.range);
 	};
@@ -302,7 +314,18 @@ function isFunctionParameterVariableReassigned(callbackFunction, context) {
 		});
 }
 
+function isSuggestable(callExpression, context) {
+	const isOptionalChaining = callExpression.callee.optional;
+
+	if (!isOptionalChaining) {
+		return false;
+	}
+
+	return callExpression.callee.object.type === 'CallExpression' && hasSideEffect(callExpression.callee.object, context.getSourceCode());
+}
+
 function isFixable(callExpression, {scope, functionInfo, allIdentifiers, context}) {
+	const isOptionalChaining = callExpression.callee.optional;
 	const sourceCode = context.getSourceCode();
 	// Check `CallExpression`
 	if (
@@ -314,14 +337,15 @@ function isFixable(callExpression, {scope, functionInfo, allIdentifiers, context
 	}
 
 	// Check `CallExpression.parent`
-	if (callExpression.parent.type !== 'ExpressionStatement') {
+	if (callExpression.parent.type !== 'ExpressionStatement' && callExpression.parent.type !== 'ChainExpression') {
 		return false;
 	}
 
-	// Check `CallExpression.callee`
-	// Because of `ChainExpression` wrapper, `foo?.forEach()` is already failed on previous check keep this just for safety
-	/* c8 ignore next 3 */
-	if (callExpression.callee.optional) {
+	if (isOptionalChaining && callExpression.parent.parent.type !== 'ExpressionStatement') {
+		return false;
+	}
+
+	if (isOptionalChaining && callExpression.callee.object.type !== 'Identifier' && callExpression.callee.object.type !== 'MemberExpression') {
 		return false;
 	}
 
@@ -414,6 +438,13 @@ const create = context => {
 					problem.fix = getFixFunction(node, functionInfo, context);
 				}
 
+				if (isSuggestable(node, context)) {
+					problem.suggest = [{
+						desc: 'Call function twice because it has no side effects',
+						fix: getFixFunction(node, functionInfo, context),
+					}];
+				}
+
 				yield problem;
 			}
 		},
@@ -429,6 +460,7 @@ module.exports = {
 			description: 'Prefer `for…of` over `Array#forEach(…)`.',
 		},
 		fixable: 'code',
+		hasSuggestions: true,
 		messages,
 	},
 };