feat(useIterableCallbackReturn): add checkForEach option#8656
Closed
costajohnt wants to merge 2 commits intobiomejs:mainfrom
Closed
feat(useIterableCallbackReturn): add checkForEach option#8656costajohnt wants to merge 2 commits intobiomejs:mainfrom
costajohnt wants to merge 2 commits intobiomejs:mainfrom
Conversation
Add the `checkForEach` option to the `useIterableCallbackReturn` rule, matching ESLint's `array-callback-return` rule behavior. When `checkForEach` is `true`, the rule will report when forEach callbacks return a value. When `false` (the default), forEach callbacks are not checked. This is a breaking change from the previous behavior where forEach was always checked. The new default aligns with ESLint's behavior. Closes #8024 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: b2c56a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Linter
Contributor
WalkthroughThis PR introduces a Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (9)📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-11-24T18:05:20.371ZApplied to files:
📚 Learning: 2025-08-05T14:43:29.581ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-12-04T13:29:49.287ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-11-21T01:10:53.059ZApplied to files:
📚 Learning: 2025-09-25T12:32:59.003ZApplied to files:
🪛 LanguageTool.changeset/add-check-for-each-option.md[uncategorized] ~9-~9: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA) 🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.js (1)
1-19: Consider following the test file naming convention.Test files typically use
invalidorvalidprefixes to clearly indicate expected behaviour. Since these cases should all trigger diagnostics (they return values fromforEachwhencheckForEachis enabled), consider renaming toinvalidCheckForEachEnabled.js.Based on learnings: test files should use
invalidandvalidprefixes to represent code that should and should not trigger the rule.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/invalid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (5)
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rscrates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.jscrates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.jsoncrates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/invalid.jscrates/biome_rule_options/src/use_iterable_callback_return.rs
💤 Files with no reviewable changes (1)
- crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/invalid.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_rule_options/src/use_iterable_callback_return.rscrates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
🧠 Learnings (23)
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.jsonc : Use `.jsonc` format for test files containing multiple code snippets, where each snippet is a string in an array
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.jscrates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.json
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Use `Option<_>` wrapper for rule option fields to enable proper merging of configurations
Applied to files:
crates/biome_rule_options/src/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Apply `#[serde(deny_unknown_fields)]` to rule option structs to enforce strict configuration validation
Applied to files:
crates/biome_rule_options/src/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Rule option types must derive `Deserializable`, `Serialize`, `Deserialize`, and optionally `JsonSchema` traits
Applied to files:
crates/biome_rule_options/src/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Apply `#[serde(rename_all = "camelCase")]` to rule option structs to match JSON configuration naming convention
Applied to files:
crates/biome_rule_options/src/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Use `Box<[Box<str>]>` instead of `Vec<String>` for collections of strings in rule options to save memory
Applied to files:
crates/biome_rule_options/src/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Rule options must be defined in the `biome_rule_options` crate with a file named after the rule
Applied to files:
crates/biome_rule_options/src/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Implement `biome_deserialize::Merge` for rule option types to define how shared and user configurations are merged
Applied to files:
crates/biome_rule_options/src/use_iterable_callback_return.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/specs/**/options.json : Create an `options.json` file (formatted as `biome.json`) in test specification folders to apply non-default formatting options to all test files in that folder
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.json
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/* : Create test files with `invalid` and `valid` prefixes to represent code that should and should not trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.json
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.json
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Biome linter rules must be designed to work across languages, requiring careful naming to indicate intended scope
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.json
📚 Learning: 2025-11-21T01:10:53.059Z
Learnt from: dyc3
Repo: biomejs/biome PR: 8171
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:125-137
Timestamp: 2025-11-21T01:10:53.059Z
Learning: In the Biome codebase, each lint rule has its own options type declaration (e.g., `type Options = RuleNameOptions`) as part of the codegen process, even if the options struct is empty or unused. This is standard practice and should not be flagged as an issue.
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.json
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.json
📚 Learning: 2025-12-22T09:26:56.943Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8537
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:167-210
Timestamp: 2025-12-22T09:26:56.943Z
Learning: When defining lint rules (declare_lint_rule!), only specify fix_kind if the rule implements an action(...) function. Rules that only emit diagnostics without a code fix should omit fix_kind. This applies to all Rust lint rule definitions under crates/.../src/lint (e.g., crates/biome_js_analyze/src/lint/...).
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use helper functions like `map`, `filter`, and `and_then` to avoid excessive nested `if let` statements
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Invalid code snippets in rule documentation must emit exactly one diagnostic
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : The first paragraph of rule documentation must be written in a single line to ensure proper rendering in the rules overview table
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Deprecated rules must include a `deprecated` field in the `declare_lint_rule!` macro with an explanation of what rule to use instead
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `.ok()?` to transform `Result` types into `Option` or `let else` pattern to handle errors when the function returns `Vec`
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Implement custom `Queryable` and `Visitor` types for rules that require deep inspection of child nodes to avoid inefficient traversals
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Code blocks in rule documentation must specify a language identifier and be tagged with `expect_diagnostic` for invalid examples or remain untagged for valid examples
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Rule documentation must include an `## Examples` section followed by `### Invalid` and `### Valid` subsections, with Invalid appearing first
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs
🔇 Additional comments (8)
crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.options.json (1)
1-15: LGTM!The configuration correctly enables the
checkForEachoption for testing. The structure follows the expected format for Biome test specifications.crates/biome_rule_options/src/use_iterable_callback_return.rs (2)
8-10: LGTM!The
check_for_eachfield correctly usesOption<bool>for proper configuration merging and includesskip_serializing_ifto omitNonevalues during serialisation.As per coding guidelines: rule option fields use
Option<_>wrapper for proper config merging.
13-22: LGTM!The implementation provides a clean accessor pattern with a sensible default value (
false), aligning with the PR objective to makeforEachchecking opt-in rather than mandatory.crates/biome_js_analyze/src/lint/suspicious/use_iterable_callback_return.rs (4)
48-56: LGTM!The documentation clearly explains the new
checkForEachoption, its default value, and its behaviour. Well structured and easy to understand.
77-80: LGTM!The updated example appropriately demonstrates the new default behaviour where
forEachcallbacks may return values without triggering diagnostics.
132-135: LGTM!The early return is well-placed—it skips
forEachevaluation when the option is disabled, before performing more expensive checks like argument position validation. Efficient and correct.
406-413: LGTM!The
forEachmetadata entry withreturn_value_required: falsecorrectly supports the opt-in behaviour. WhencheckForEachis enabled, the rule will flag callbacks that return values fromforEach.crates/biome_js_analyze/tests/specs/suspicious/useIterableCallbackReturn/checkForEachEnabled.js (1)
1-19: Test cases are correctly verified through the snapshot file.All five test cases produce the expected diagnostics with
checkForEachenabled. The.snapfile confirms each forEach callback that returns a value correctly triggers the rule violation. No action needed.
- Rename checkForEachEnabled.js to invalidCheckForEachEnabled.js to follow the test naming convention (invalid/valid prefix) - Add changeset for version bump
Member
|
Please, next time, comment on the issue. There's already a PR #8289 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add the
checkForEachoption to theuseIterableCallbackReturnrule, matching ESLint'sarray-callback-returnrule behavior.checkForEachistrue, the rule reports when forEach callbacks return a valuefalse(the default), forEach callbacks are not checkedThis aligns with ESLint's default behavior where forEach is not checked unless explicitly enabled.
Breaking Change
This is a breaking change from the previous behavior where forEach was always checked. Users who want the previous behavior should set
checkForEach: truein their configuration:{ "linter": { "rules": { "suspicious": { "useIterableCallbackReturn": { "options": { "checkForEach": true } } } } } }Test Plan
checkForEachEnabled.jswith corresponding options to verify forEach checking when enabledinvalid.jsto remove forEach cases (they now pass by default)Closes #8024
Note
This PR was developed with AI assistance (Claude).