Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/add-checkforeach-option.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@biomejs/biome": minor
---

Add `checkForEach` option to [`useIterableCallbackReturn`](https://biomejs.dev/linter/rules/use-iterable-callback-return/) rule. This option allows checking `forEach` callbacks for unexpected return values. When `true`, the rule reports `forEach` callbacks that return a value since `forEach` ignores return values. Default: `false`, matching ESLint's `array-callback-return` rule behavior.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,15 @@ declare_lint_rule! {
/// - `toSorted`
/// — `from` (when called on `Array`)
///
/// A return value is disallowed in the method `forEach`.
/// The `forEach` method is only checked when the `checkForEach` option is enabled.
/// When enabled, return values in `forEach` callbacks are reported since they have no effect.
///
/// ## Options
///
/// ### checkForEach
///
/// When `true`, the rule also checks `forEach` callbacks for unexpected return values.
/// Default: `false`
///
/// ## Examples
///
Expand All @@ -57,12 +65,6 @@ declare_lint_rule! {
/// });
/// ```
///
/// ```js,expect_diagnostic
/// [].forEach(() => {
/// return 1; // Should not return a value
/// });
/// ```
///
/// ### Valid
///
/// ```js
Expand All @@ -72,14 +74,11 @@ declare_lint_rule! {
/// ```
///
/// ```js
/// // forEach is not checked by default
/// [].forEach(() => {
/// // No return value, which is correct
/// return 1;
/// });
/// ```
///
/// ```js
/// [].forEach(() => void null); // Void return value, which doesn't trigger the rule
/// ```
pub UseIterableCallbackReturn {
version: "2.0.0",
name: "useIterableCallbackReturn",
Expand Down Expand Up @@ -130,6 +129,11 @@ impl Rule for UseIterableCallbackReturn {

let method_config = ITERABLE_METHOD_INFOS.get(member_name.text_trimmed())?;

// Skip forEach if checkForEach option is false (default)
if method_config.method_name == "forEach" && !ctx.options().check_for_each {
return None;
}

let arg_position = argument_list
.elements()
.position(|arg| arg.node.is_ok() && arg.node.unwrap().syntax().eq(&cfg.node))?;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// These should be invalid when checkForEach is true

// Block body with return
[].forEach((a) => {
return a.fn();
});
[].forEach(function(a) {
return a.fn();
});

// Concise body (expression body) - returns implicitly
[].forEach(a => a.fn());

// Conditional returns
[].forEach((a) => {
if (a) {
return a.fn();
}
});
[].forEach((a) => {
if (a) {
return;
}
return a.fn();
});
[].forEach((a) => {
if (a) {
throw new Error();
}
return a.fn();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: checkForEach.js
---
# Input
```js
// These should be invalid when checkForEach is true

// Block body with return
[].forEach((a) => {
return a.fn();
});
[].forEach(function(a) {
return a.fn();
});

// Concise body (expression body) - returns implicitly
[].forEach(a => a.fn());

// Conditional returns
[].forEach((a) => {
if (a) {
return a.fn();
}
});
[].forEach((a) => {
if (a) {
return;
}
return a.fn();
});
[].forEach((a) => {
if (a) {
throw new Error();
}
return a.fn();
});

```

# Diagnostics
```
checkForEach.js:4:4 lint/suspicious/useIterableCallbackReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This callback passed to forEach() iterable method should not return a value.

3 │ // Block body with return
> 4 │ [].forEach((a) => {
│ ^^^^^^^
5 │ return a.fn();
6 │ });

i Either remove this return or remove the returned value.

3 │ // Block body with return
> 4 │ [].forEach((a) => {
> 5 │ return a.fn();
│ ^^^^^^^
6 │ });
7 │ [].forEach(function(a) {


```

```
checkForEach.js:7:4 lint/suspicious/useIterableCallbackReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This callback passed to forEach() iterable method should not return a value.

5 │ return a.fn();
6 │ });
> 7 │ [].forEach(function(a) {
│ ^^^^^^^
8 │ return a.fn();
9 │ });

i Either remove this return or remove the returned value.

5 │ return a.fn();
6 │ });
> 7 │ [].forEach(function(a) {
> 8 │ return a.fn();
│ ^^^^^^^
9 │ });
10 │


```

```
checkForEach.js:12:4 lint/suspicious/useIterableCallbackReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This callback passed to forEach() iterable method should not return a value.

11 │ // Concise body (expression body) - returns implicitly
> 12 │ [].forEach(a => a.fn());
│ ^^^^^^^
13 │
14 │ // Conditional returns

i Either remove this return or remove the returned value.

11 │ // Concise body (expression body) - returns implicitly
> 12 │ [].forEach(a => a.fn());
│ ^^^^^^
13 │
14 │ // Conditional returns


```

```
checkForEach.js:15:4 lint/suspicious/useIterableCallbackReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This callback passed to forEach() iterable method should not return a value.

14 │ // Conditional returns
> 15 │ [].forEach((a) => {
│ ^^^^^^^
16 │ if (a) {
17 │ return a.fn();

i Either remove this return or remove the returned value.

14 │ // Conditional returns
15 │ [].forEach((a) => {
> 16 │ if (a) {
> 17 │ return a.fn();
│ ^^^^^^^
18 │ }
19 │ });


```

```
checkForEach.js:20:4 lint/suspicious/useIterableCallbackReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This callback passed to forEach() iterable method should not return a value.

18 │ }
19 │ });
> 20 │ [].forEach((a) => {
│ ^^^^^^^
21 │ if (a) {
22 │ return;

i Either remove this return or remove the returned value.

21 │ if (a) {
22 │ return;
> 23 │ }
> 24 │ return a.fn();
│ ^^^^^^^
25 │ });
26 │ [].forEach((a) => {


```

```
checkForEach.js:26:4 lint/suspicious/useIterableCallbackReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This callback passed to forEach() iterable method should not return a value.

24 │ return a.fn();
25 │ });
> 26 │ [].forEach((a) => {
│ ^^^^^^^
27 │ if (a) {
28 │ throw new Error();

i Either remove this return or remove the returned value.

27 │ if (a) {
28 │ throw new Error();
> 29 │ }
> 30 │ return a.fn();
│ ^^^^^^^
31 │ });
32 │


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"suspicious": {
"useIterableCallbackReturn": {
"level": "error",
"options": {
"checkForEach": true
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/* should not generate diagnostics */

// void expressions should not trigger diagnostics even with checkForEach: true
// because void returns undefined, which is "no return value"
[].forEach(a => void a.fn());
[].forEach(() => void null);
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: checkForEachValid.js
---
# Input
```js
/* should not generate diagnostics */

// void expressions should not trigger diagnostics even with checkForEach: true
// because void returns undefined, which is "no return value"
[].forEach(a => void a.fn());
[].forEach(() => void null);

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"suspicious": {
"useIterableCallbackReturn": {
"level": "error",
"options": {
"checkForEach": true
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,32 +1,3 @@
[].forEach((a) => {
return a.fn();
});
[].forEach(function(a) {
return a.fn();
});
[].forEach((a) => {
if (a) {
return a.fn();
}
});
[].forEach((a) => {
if (a) {
return;
}
return a.fn();
});
[].forEach((a) => {
if (a) {
return;
}
return a.fn();
});
[].forEach((a) => {
if (a) {
throw new Error();
}
return a.fn();
});
Array.from([], () => {});
Array.from([], function() {});
Array.from([], () => {
Expand Down
Loading