diff --git a/docs/rules/array-callback-return.md b/docs/rules/array-callback-return.md index 34ffeffcebd2..dc018af353a1 100644 --- a/docs/rules/array-callback-return.md +++ b/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. diff --git a/lib/rules/array-callback-return.js b/lib/rules/array-callback-return.js index d632a3f30c28..2570a682ba48 100644 --- a/lib/rules/array-callback-return.js +++ b/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({