fix(noDoneCallback): avoid false positives on non-hook identifiers #8315

hirokiokada77 · 2025-11-30T14:02:04Z

Summary

This PR fixes an issue where the noDoneCallback rule flagged false positives when a method was called on a regular variable bound to identifiers such as before, after, beforeEach, and afterEach.

Closes #5213.

Test Plan

Test cases were added to:

crates/biome_js_analyze/tests/specs/style/noDoneCallback/valid.js

changeset-bot · 2025-11-30T14:02:08Z

🦋 Changeset detected

Latest commit: cad4508

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@biomejs/biome	Patch
@biomejs/cli-win32-x64	Patch
@biomejs/cli-win32-arm64	Patch
@biomejs/cli-darwin-x64	Patch
@biomejs/cli-darwin-arm64	Patch
@biomejs/cli-linux-x64	Patch
@biomejs/cli-linux-arm64	Patch
@biomejs/cli-linux-x64-musl	Patch
@biomejs/cli-linux-arm64-musl	Patch
@biomejs/wasm-web	Patch
@biomejs/wasm-bundler	Patch
@biomejs/wasm-nodejs	Patch
@biomejs/backend-jsonrpc	Patch

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

coderabbitai · 2025-11-30T14:07:03Z

Walkthrough

Adds a changeset and fixes a false positive in the noDoneCallback linter rule: the rule no longer flags method calls on regular variables named like test hooks (before, after, beforeEach, afterEach). The implementation refactors callee-name extraction and argument-index determination to use contains-based checks for hook and test names. Test fixtures were updated to include additional statements validating the fix.

Suggested reviewers

ematipico
dyc3

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main fix: preventing false positives on non-hook identifiers in the noDoneCallback rule.
Description check	✅ Passed	The description relates directly to the changeset, explaining the bug fix and referencing the closed issue with test plan details.
Linked Issues check	✅ Passed	The PR successfully addresses issue #5213 by modifying the noDoneCallback rule to distinguish actual test hooks from ordinary variables sharing hook names, eliminating false positives.
Out of Scope Changes check	✅ Passed	All changes align with the stated objective: the linter rule fix, the changeset entry, and test cases specifically targeting the false positive issue.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8528d14 and cad4508.

📒 Files selected for processing (1)

crates/biome_js_analyze/src/lint/style/no_done_callback.rs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Use the dbg!() macro for debugging output during testing, and pass the --show-output flag to cargo to view debug output
Use cargo t or cargo test to run tests; for a single test, pass the test name after the test command
Use snapshot testing with the insta crate; run cargo insta accept, cargo insta reject, or cargo insta review to manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Use just f (alias for just format) to format Rust and TOML files before committing

Files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

🧠 Learnings (11)

📓 Common learnings

Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.077Z
Learning: Applies to .changeset/**/*.md : For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use a `diff` code block

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Check if a variable is global before banning it to avoid false positives when the variable is redeclared in local scope; use the semantic model to verify global scope

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Avoid unnecessary string allocations by comparing against `&str` or `TokenText` instead of calling `to_string()` which allocates heap memory

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnused` prefix for rules that report unused entities (e.g., `noUnusedVariables`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUndeclared` prefix for rules that report undefined entities (e.g., `noUndeclaredVariables`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noRedundant` prefix for rules that report redundant code (e.g., `noRedundantUseStrict`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Rules should use the `no` prefix naming convention when the sole intention is to forbid a single concept (e.g., `noDebugger` to disallow debugger statements)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noEmpty` prefix for rules that report empty code (e.g., `noEmptyBlockStatements`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUseless` prefix for rules that report unnecessary code that could be removed or simplified (e.g., `noUselessConstructor`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Framework-specific rules should be named using the `use` or `no` prefix followed by the framework name (e.g., `noVueReservedProps`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noExcessive` prefix for rules that report code exceeding configurable limits (e.g., `noExcessiveNestedTestSuites`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

🧬 Code graph analysis (1)

crates/biome_js_analyze/src/lint/style/no_done_callback.rs (1)

crates/biome_js_syntax/src/expr_ext.rs (1)

callee (33-38)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)

GitHub Check: Documentation
GitHub Check: Test (depot-ubuntu-24.04-arm-16)
GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
GitHub Check: Test (depot-windows-2022-16)
GitHub Check: End-to-end tests
GitHub Check: Check Dependencies
GitHub Check: Lint project (depot-windows-2022)
GitHub Check: Test Node.js API
GitHub Check: Bench (biome_js_analyze)
GitHub Check: Validate rules documentation
GitHub Check: Bench (biome_js_formatter)
GitHub Check: Bench (biome_js_parser)
GitHub Check: autofix

🔇 Additional comments (1)

crates/biome_js_analyze/src/lint/style/no_done_callback.rs (1)

83-98: Logic correctly addresses the false positive issue.

The fix properly distinguishes actual test hooks from method calls on regular variables. before.find(...) will now correctly return None since the full callee text won't match "before" exactly, whilst beforeEach(...) continues to work as expected.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

crates/biome_js_analyze/src/lint/style/no_done_callback.rs (1)

81-97: Hook matching now correctly ignores member calls on before/after variables

Switching the hook check to callee.to_trimmed_string() while keeping the it/test branch on callee_object_name neatly avoids treating before.find(...) / after.filter(...) as hooks, without changing the existing before*/after* and test / test.each behaviour. The control flow and selected argument indices still look consistent with the rule’s intent, so this reads as a focused, low‑risk fix.

.changeset/honest-lions-call.md (1)

1-5: Changeset succinctly documents the user‑visible fix

The entry has the right front matter, tense, and level of detail for a patch note and clearly states what behaviour changed. If you want to go the extra mile, you could add a one‑line code example of a now‑valid before.find(...)/after.filter(...) pattern, but it’s not strictly necessary.

As per coding guidelines, this is already compliant and concise.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8de2774 and 8528d14.

⛔ Files ignored due to path filters (1)

crates/biome_js_analyze/tests/specs/style/noDoneCallback/valid.js.snap is excluded by !**/*.snap and included by **

📒 Files selected for processing (3)

.changeset/honest-lions-call.md (1 hunks)
crates/biome_js_analyze/src/lint/style/no_done_callback.rs (1 hunks)
crates/biome_js_analyze/tests/specs/style/noDoneCallback/valid.js (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Use the dbg!() macro for debugging output during testing, and pass the --show-output flag to cargo to view debug output
Use cargo t or cargo test to run tests; for a single test, pass the test name after the test command
Use snapshot testing with the insta crate; run cargo insta accept, cargo insta reject, or cargo insta review to manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Use just f (alias for just format) to format Rust and TOML files before committing

Files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

.changeset/**/*.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

.changeset/**/*.md: Create changesets for user-facing changes using just new-changeset; use headers with #### or ##### only; keep descriptions concise (1-3 sentences) and focus on user-facing changes
Use past tense when describing what was done ('Added new feature'), present tense when describing Biome behavior ('Biome now supports'); end sentences with a full stop
For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use a diff code block

Files:

.changeset/honest-lions-call.md

🧠 Learnings (10)

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Rules should use the `no` prefix naming convention when the sole intention is to forbid a single concept (e.g., `noDebugger` to disallow debugger statements)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noRedundant` prefix for rules that report redundant code (e.g., `noRedundantUseStrict`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnused` prefix for rules that report unused entities (e.g., `noUnusedVariables`)

Applied to files:

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

📚 Learning: 2025-11-28T09:08:10.077Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.077Z
Learning: Applies to .changeset/**/*.md : Use past tense when describing what was done ('Added new feature'), present tense when describing Biome behavior ('Biome now supports'); end sentences with a full stop

Applied to files:

.changeset/honest-lions-call.md

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Commit rule changes with message format: `feat(biome_<crate>): <ruleName>` to follow Biome's conventional commit style

Applied to files:

.changeset/honest-lions-call.md

📚 Learning: 2025-11-28T09:08:10.077Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.077Z
Learning: Applies to .changeset/**/*.md : Create changesets for user-facing changes using `just new-changeset`; use headers with `####` or `#####` only; keep descriptions concise (1-3 sentences) and focus on user-facing changes

Applied to files:

.changeset/honest-lions-call.md

📚 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:

.changeset/honest-lions-call.md

📚 Learning: 2025-11-28T09:08:10.077Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.077Z
Learning: Applies to .changeset/**/*.md : For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use a `diff` code block

Applied to files:

.changeset/honest-lions-call.md

📚 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:

.changeset/honest-lions-call.md

📚 Learning: 2025-11-27T23:04:02.022Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.jsonc : Use `.jsonc` files in test specs with code snippets as array of strings to test rules in script environment (no import/export syntax)

Applied to files:

crates/biome_js_analyze/tests/specs/style/noDoneCallback/valid.js

🧬 Code graph analysis (1)

crates/biome_js_analyze/src/lint/style/no_done_callback.rs (1)

crates/biome_js_syntax/src/expr_ext.rs (1)

callee (33-38)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)

GitHub Check: Documentation
GitHub Check: Test Node.js API
GitHub Check: Test (depot-windows-2022-16)
GitHub Check: Test (depot-ubuntu-24.04-arm-16)
GitHub Check: End-to-end tests
GitHub Check: Check Dependencies
GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
GitHub Check: Lint project (depot-windows-2022)
GitHub Check: Bench (biome_js_analyze)
GitHub Check: autofix
GitHub Check: Bench (biome_js_formatter)
GitHub Check: Bench (biome_js_parser)

🔇 Additional comments (1)

crates/biome_js_analyze/tests/specs/style/noDoneCallback/valid.js (1)

25-28: Good regression coverage for the reported false positive

These extra cases precisely capture the “array named before/after with a method call” scenario, so they should guard against the rule regressing back to flagging ordinary variables. Nicely minimal and targeted.

codspeed-hq · 2025-11-30T14:16:23Z

CodSpeed Performance Report

Merging #8315 will not alter performance

_{Comparing hirokiokada77:fix/no-done-callback (cad4508) with main (efa694c)}

Summary

✅ 58 untouched
⏩ 95 skipped¹

95 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

dyc3

Looks good, just a minor comment

crates/biome_js_analyze/src/lint/style/no_done_callback.rs

siketyan

Thank you!

…iomejs#8315)

fix(noDoneCallback): avoid false positives on non-hook identifiers

8528d14

github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Nov 30, 2025

coderabbitai bot reviewed Nov 30, 2025

View reviewed changes

dyc3 approved these changes Nov 30, 2025

View reviewed changes

crates/biome_js_analyze/src/lint/style/no_done_callback.rs Outdated Show resolved Hide resolved

avoid heap allocation via to_trimmed_string

cad4508

siketyan approved these changes Dec 2, 2025

View reviewed changes

siketyan merged commit c7915c4 into biomejs:main Dec 2, 2025
18 checks passed

github-actions bot mentioned this pull request Dec 1, 2025

ci: release #8286

Merged

hirokiokada77 deleted the fix/no-done-callback branch December 2, 2025 15:03

This was referenced Dec 15, 2025

ci: release #8467

Merged

ci: release #8469

Merged

l0ngvh pushed a commit to l0ngvh/biome that referenced this pull request Dec 21, 2025

fix(noDoneCallback): avoid false positives on non-hook identifiers (b…

0d4dd64

…iomejs#8315)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(noDoneCallback): avoid false positives on non-hook identifiers #8315

fix(noDoneCallback): avoid false positives on non-hook identifiers #8315

Uh oh!

hirokiokada77 commented Nov 30, 2025

Uh oh!

changeset-bot bot commented Nov 30, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 30, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

codspeed-hq bot commented Nov 30, 2025 •

edited

Loading

Uh oh!

dyc3 left a comment

Uh oh!

Uh oh!

siketyan left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

fix(noDoneCallback): avoid false positives on non-hook identifiers #8315

fix(noDoneCallback): avoid false positives on non-hook identifiers #8315

Uh oh!

Conversation

hirokiokada77 commented Nov 30, 2025

Summary

Test Plan

Uh oh!

changeset-bot bot commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

coderabbitai bot commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #8315 will not alter performance

Summary

Footnotes

Uh oh!

dyc3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

siketyan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot bot commented Nov 30, 2025 •

edited

Loading

coderabbitai bot commented Nov 30, 2025 •

edited

Loading

codspeed-hq bot commented Nov 30, 2025 •

edited

Loading