useIterableCallbackReturn should be split into two separate rules

[theshadow27]

Problem

Per the docs on useIterableCallbackReturn

The following methods require a return in their callback:

• every
• filter
• find
• findIndex
• findLast
• findLastIndex
• flatMap
• map
• reduce
• reduceRight
• some
• sort
• toSorted — from (when called on Array)
• A return value is disallowed in the method forEach. 🚨

The useIterableCallbackReturn rule conflates two completely different concerns:

1. Missing returns in value-producing methods (map, filter, etc.) ✅ catches real bugs, should be enabled by default
2. Having returns in forEach - flags idiomatic, correct code ❌ highly problematic, warning at most by default

This issue affects many codebases:

• Set/Map APIs naturally return values for chaining
• Console APIs return undefined
• Builder patterns return this
• Functional programming uses forEach for side effects
• Simple closure capture (adding an arg to a function)

This forces developers to choose between:

• Disabling the entire rule (losing valuable bug detection)
• Adding noise suppressions to every legitimate forEach usage

This was addressed in #6473, where there seems to be a fundamental dispute between:

@ematipico: While your example is acceptable, the rule is actually meant to catch these use cases. Your example can be fixed by wrapping the body of the function in curly braces.... While experienced developers such yourself know the difference, junior developers might not, and that's why I think the rule should be strict as it is now.

vs

@gius: While I understand your point, there are two additional arguments to consider for at least being able to use the rest of the useIterableCallbackReturn rules without the forEach one (if not possible to configure its behavior) ... even though a junior developer might not see the difference between => foo() and => { foo() }, in the case of forEach, there is no difference and no way it could cause unexpected behavior (even for the junior dev)

I personally agree with the 2nd, but respect the reasoning behind the first. There's no reason to force everyone into this choice, when it could easily be separated.

Real-world false positives

From a production codebase upgraded from 1.x -> 2.x, here are patterns the rule incorrectly flags:

1. Console logging (debugging/reporting)

  // Flagged as error - but this is idiomatic code
  failures.forEach((failure) => console.error(`   - ${failure}`));

2. Set/Map mutations

  // Flagged as error - Set.add() naturally returns the set for chaining
  // but we're using forEach for side effects, not chaining
  oldIds.filter(id => id && !newSet.has(id))
        .forEach(id => diffSet.add(id));

3. Builder patterns

  // Flagged as error - builder methods return 'this' for chaining
  // but forEach is being used for its side effects
  names.forEach(name => this._addDefineSymbol(name, typeDef));

4. Callback execution

  // Flagged as error - who cares what the callbacks return?
  hooks.forEach(hook => hook());

5. Closure Binding (even to void functions)

function addKey(x: string, y: string): void { 
   myKeys.push(`${x}-${y}`)
}

...
// this is detected even though addKey is void!
["a", "b", "c"].forEach( (x) => addKey(x, "y") );  

Why this matters

None of these are bugs. They're all correct, idiomatic TypeScript/JavaScript. The return values are incidental to methods designed for side effects.

Meanwhile, the actually useful checks (missing returns in map, filter, etc.) are bundled with this noise, forcing teams to disable the entire rule.

Proposed solution

Option A: Split into two rules:

1. noMissingIterableReturn (or keep current name)
   • Enforces returns in: .map, .filter, .every, .some, .find, .findIndex, .flatMap, .reduce, .sort, etc.
   • This catches real bugs!
2. noForEachReturn (new rule, off by default)
   • Warns about returns in .forEach
   • Pedagogical value for beginners who confuse forEach with map
   • Advanced users can ignore/disable

Option B: Add Configuration Option

If splitting is too disruptive, add a config option:

  {
    "useIterableCallbackReturn": {
      "checkForEach": false  // default: true
    }
  }

Have you considered the alternatives?

1. Disable the rule completely

  {
    "linter": {
      "rules": {
        "suspicious": {
          "useIterableCallbackReturn": "off"
        }
      }
    }
  }

Rejected: Disabling the rule entirely loses the valuable checks for map, filter, etc:

2. wrap everything in brackets or void

arr.forEach( x => { 
   builder.add(x);  // intentionally ignored return? how can tell?
} );

// just a cast, does nothing. if the return value did matter, there's no way to know
arr.forEach( x => void builder.add(x);  );

note that the return value is still ignored!
So assuming that was really a bug, what does this gain?

3. Change to for:of loops?

for(x of arr){
   builder.add(x); // still ignoring... but on a new line? how better?
}

that's a +2 for what? I picked biome because it has concise defaults. This is worse.

Related

• 💅 lint/nursery/useIterableCallbackReturn incorrectly flags functions that return void as returning a value #6617 claims it cannot be addressed without type inference, but does not consider separating it into two rules.
• 🐛useIterableCallbackReturn complains incorrectly about return values #7247 duplicated useIterableCallbackReturn should allow forEach arrow callback return a value #6473 , which recommends curly braces
  -The recent addition of void support (💅 lint/nursery/useIterableCallbackReturn incorrectly flags functions that return void as returning a value #6617, fix(lint/suspicious/useIterableCallbackReturn): single-expression void arrow function #7564, ci: release #7478) helps but doesn't solve the fundamental problem: developers must choose between valuable bug detection and hundreds of false positives.

But the alternative here ("separate them into 2 rules") was not considered.

Expected behavior

I should be able to:

• ✅ Get warnings for missing returns in map, filter, etc, indicating a serious bug
• ✅ Use forEach for side effects without warnings
• ✅ Optionally enable stricter checking for forEach if desired

[siketyan]

One common mistake happens by misuse of .forEach() is that a Promise is unintentionally returned. For example:

foo.forEach(async () => {
  await doSomething();
});

This will cause a floating promise because .forEach() completely ignores the returned promise.

[dyc3]

Since this rule is a direct port of https://eslint.org/docs/latest/rules/array-callback-return, I think adding an option for this makes the most sense. It's what the source rule does.

[dyc3]

I've opened #8024 to track this.