feat: allowVoid option in array-callback-return

[Tanujkanti4441]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Added a new option called allowVoid to the array-callback-return rule which allow void operator to be used in forEach callback return. This option will work only with checkForEach: true option.

/*eslint array-callback-return: ["error", { checkForEach: true, allowVoid: true }]*/

myArray.forEach(item => void handleItem(item));  // no-error

And added related tests

Is there anything you'd like reviewers to focus on?

Refs #17285

[eslint-github-bot]

Hi @Tanujkanti4441!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

• The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[netlify]

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	2d155d6
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/650708f6afe91a00083b473a

[eslint-github-bot]

Hi @Tanujkanti4441!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

• The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[eslint-github-bot]

Hi @Tanujkanti4441!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

• The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[nopeless]

Thanks for doing this work 😁

[Tanujkanti4441]

Thanks for doing this work 😁

Any reviews or anything that needs to be added

[nzakas]

This looks like a good start! I have a suggestion for refactoring to make things a bit clearer.

Also, please update the documentation to describe the new option and give examples.

[nzakas]

I think we can make this a bit easier to follow by pulling the common checks up a level.

Suggested change

	if (options.checkForEach && node.type === "ArrowFunctionExpression" && node.expression) {
	messageId = "expectedNoReturnValue";
	}
	if (options.checkForEach &&
	options.allowVoid &&
	node.type === "ArrowFunctionExpression" &&
	node.body.type === "UnaryExpression" &&
	node.body.operator === "void") {
	return;
	}
	if (options.checkForEach && node.type === "ArrowFunctionExpression") {
	if (node.expression) {
	messageId = "expectedNoReturnValue";
	}
	if (options.allowVoid &&
	node.body.type === "UnaryExpression" &&
	node.body.operator === "void") {
	return;
	}
	}

[Tanujkanti4441]

Done!
I think node.expression is true for both the conditions

[mdjermanovic]

I think this option should apply to return void, too:

/*eslint array-callback-return: ["error", { checkForEach: true, allowVoid: true }]*/

myArray.forEach(item => {
    if (item < 0) {
        return void doSomething(item); // should be okay
    }
    doSomethingElse(item);
});

Also, the original issue #17285 mentions implementing suggestions like in #17282.

[nzakas]

Thanks, this LGTM. Just one suggestion for docs formatting.

Would like another review from the team before merging.

[nzakas]

Suggested change

	**Note-** `{ "allowVoid": true }` works only if `checkForEach` option is set to `true`.
	**Note:** `{ "allowVoid": true }` works only if `checkForEach` option is set to `true`.

[nopeless]

I'm not sure how much of a priority this is, but the allowVoid for callback functions have an IDE suggestion as well.

[mdjermanovic]

I'd be fine with implementing suggestions in a follow-up, but I think the option should apply to return void (#17564 (review)) from the start.

[mdjermanovic]

One suggestion about the tests, then LGTM.

[mdjermanovic]

Can you add a few tests to check if allowVoid: true allows only void expressions? For example:

Suggested change

	{ code: "foo.forEach((x) => { if (a === b) { return void a; } bar(x) })", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] },
	{ code: "foo.forEach((x) => { if (a === b) { return void a; } bar(x) })", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] },
	{ code: "foo.forEach((x) => x)", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] },
	{ code: "foo.forEach((x) => !x)", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] },
	{ code: "foo.forEach((x) => { return x; })", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] },
	{ code: "foo.forEach((x) => { return !x; })", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] },

[Tanujkanti4441]

Done!

[mdjermanovic]

LGTM, thanks!