Update: array-callback-return checks Array.forEach (fixes eslint#12551)

array-callback-return rule now checks if Array.forEach returns a value, and, if it does, reports an error.
This feature is being added disabled by default, and can be enabled by setting the option "checkForEach" to true.

Signed-off-by: Gabriel R Sezefredo <g@briel.dev>

docs/rules/array-callback-return.md

@@ -74,7 +74,7 @@ var bar = foo.map(node => node.getAttribute("id"));
 
 ## Options
 
-This rule has an object option:
+This rule has two object options:
 
 * `"allowImplicit": false` (default) When set to true, allows implicitly returning `undefined` with a `return` statement containing no expression.
 
@@ -87,6 +87,33 @@ var undefAllTheThings = myArray.map(function(item) {
 });
 ```
 
+* `"checkForEach": false` (default) When set to true, rule will also report forEach callbacks that returns a value
+
+Examples of **incorrect** code for the `{ "checkForEach": true }` option:
+
+```js
+/*eslint array-callback-return: ["error", { checkForEach: true }]*/
+myArray.forEach(function(item) {
+    return handleItem(item)
+});
+```
+
+Examples of **correct** code for the `{ "checkForEach": true }` option:
+
+```js
+/*eslint array-callback-return: ["error", { checkForEach: true }]*/
+myArray.forEach(function(item) {
+    handleItem(item)
+});
+
+myArray.forEach(function(item) {
+    if (item < 0) {
+        return undefined;
+    }
+    handleItem(item);
+});
+```
+
 ## Known Limitations
 
 This rule checks callback functions of methods with the given names, *even if* the object which has the method is *not* an array.

lib/rules/array-callback-return.js

@@ -18,7 +18,7 @@ const astUtils = require("./utils/ast-utils");
 //------------------------------------------------------------------------------
 
 const TARGET_NODE_TYPE = /^(?:Arrow)?FunctionExpression$/u;
-const TARGET_METHODS = /^(?:every|filter|find(?:Index)?|map|reduce(?:Right)?|some|sort)$/u;
+const TARGET_METHODS = /^(?:every|filter|find(?:Index)?|map|reduce(?:Right)?|some|sort|forEach)$/u;
 
 /**
  * Checks a given code path segment is reachable.
@@ -153,6 +153,10 @@ module.exports = {
                     allowImplicit: {
                         type: "boolean",
                         default: false
+                    },
+                    checkForEach: {
+                        type: "boolean",
+                        default: false
                     }
                 },
                 additionalProperties: false
@@ -162,13 +166,14 @@ module.exports = {
         messages: {
             expectedAtEnd: "Expected to return a value at the end of {{name}}.",
             expectedInside: "Expected to return a value in {{name}}.",
-            expectedReturnValue: "{{name}} expected a return value."
+            expectedReturnValue: "{{name}} expected a return value.",
+            noReturnExpected: "{{name}} did not expect a return value."
         }
     },
 
     create(context) {
 
-        const options = context.options[0] || { allowImplicit: false };
+        const options = context.options[0] || { allowImplicit: false, checkForEach: false };
 
         let funcInfo = {
             upper: null,
@@ -188,7 +193,20 @@ module.exports = {
          * @returns {void}
          */
         function checkLastSegment(node) {
-            if (funcInfo.shouldCheck &&
+            const method = astUtils.getStaticPropertyName(funcInfo.node.parent.callee);
+
+            if (funcInfo.shouldCheck && method === "forEach" && options.checkForEach) {
+                if (!funcInfo.codePath.currentSegments.some(isReachable)) {
+                    context.report({
+                        node,
+                        loc: getLocation(node, context.getSourceCode()).loc.start,
+                        messageId: "noReturnExpected",
+                        data: {
+                            name: lodash.upperFirst(astUtils.getFunctionNameWithKind(funcInfo.node))
+                        }
+                    });
+                }
+            } else if (funcInfo.shouldCheck &&
                 funcInfo.codePath.currentSegments.some(isReachable)
             ) {
                 context.report({