fix(linter): don't remove empty blocks containing comments #8696
Conversation
The noUselessLoneBlockStatements rule was removing empty blocks even when they contained comments, which could delete important notes like TODOs or commented-out code. Now empty blocks with comments are preserved. Fixes biomejs#8685
🦋 Changeset detectedLatest commit: 2e000cc 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
WalkthroughThe pull request updates the NoUselessLoneBlockStatements lint rule: empty blocks that contain comment descendants are no longer diagnosed (the rule returns None for those blocks). Behaviour for non-empty blocks is unchanged. Tests were added covering empty blocks with single-line, multi-line and commented-out statements in various labelled and nested contexts. A changelog entry documents the patch and references issue Suggested labels
Suggested reviewers
Pre-merge checks✅ Passed checks (4 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/complexity/no_useless_lone_block_statements.rs (1)
15-45: Consider documenting the comment preservation behaviour.The rule documentation doesn't mention that empty blocks with comments are preserved. Adding a valid example showing this would help users understand the behaviour.
💡 Suggested documentation addition
/// ### Valid /// /// ```js /// while (foo) { /// bar(); /// } /// ``` /// +/// ```js +/// { +/// // TODO: add implementation +/// } +/// ``` +///
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/complexity/noUselessLoneBlockStatements/valid.cjs.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_js_analyze/src/lint/complexity/no_useless_lone_block_statements.rscrates/biome_js_analyze/tests/specs/complexity/noUselessLoneBlockStatements/valid.cjs
🧰 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_js_analyze/src/lint/complexity/no_useless_lone_block_statements.rs
🧠 Learnings (12)
📓 Common learnings
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.
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: For new lint rules in changesets, show an example of invalid case in inline code or code block
📚 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/complexity/no_useless_lone_block_statements.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/complexity/no_useless_lone_block_statements.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/complexity/no_useless_lone_block_statements.rs
📚 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/complexity/no_useless_lone_block_statements.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 names should use the `no` prefix when the rule's sole intention is to forbid a single concept
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_useless_lone_block_statements.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 : Check if a variable is global using the semantic model before reporting diagnostics for rules that ban global functions or variables
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_useless_lone_block_statements.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/complexity/no_useless_lone_block_statements.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 names should use the `use` prefix when the rule's sole intention is to mandate a single concept
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_useless_lone_block_statements.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 `declare_node_union!` macro to query multiple node types together to avoid redundant traversal passes
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_useless_lone_block_statements.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/complexity/no_useless_lone_block_statements.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/**/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/complexity/noUselessLoneBlockStatements/valid.cjs
🔇 Additional comments (2)
crates/biome_js_analyze/tests/specs/complexity/noUselessLoneBlockStatements/valid.cjs (1)
87-104: LGTM! Solid test coverage.The test cases appropriately cover single-line comments, multi-line comments, and commented-out code within empty blocks.
crates/biome_js_analyze/src/lint/complexity/no_useless_lone_block_statements.rs (1)
80-83: No issues found.The
has_comments_descendants()method is properly defined and correctly used. The logic for preserving empty blocks with comments is sound and follows established patterns throughout the codebase.
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
Looks good, but it needs a changeset.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/fix-lone-block-comments.md (2)
5-5: Consider adding a code example to illustrate the fix.Whilst the description is clear, a before/after code snippet would help readers understand the exact scenario being fixed. For instance:
// Now preserved (previously removed): { // TODO: revisit this logic }This would align with best practices for changelog entries documenting lint rule changes and provide immediate context for the fix. Based on learnings, examples in changesets make the change more tangible for end users.
5-5: Minor: consider hyphenating "to-dos" for consistency with standard style guides.The phrase "TODOs or commented-out code" reads naturally, but formal style guides typically hyphenate this as "to-dos" when used as a noun. This is a nitpick, so feel free to ignore if you prefer the uppercase acronym style.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/fix-lone-block-comments.md
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: For new lint rules in changesets, show an example of invalid case in inline code or code block
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: For new lint rules in changesets, show an example of invalid case in inline code or code block
Applied to files:
.changeset/fix-lone-block-comments.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/fix-lone-block-comments.md
📚 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:
.changeset/fix-lone-block-comments.md
🪛 LanguageTool
.changeset/fix-lone-block-comments.md
[uncategorized] ~5-~5: Possible missing comma found.
Context: ... comments. The rule now preserves these blocks since comments may contain important in...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~5-~5: It appears that a hyphen is missing in the plural noun “to-dos”?
Context: ... may contain important information like TODOs or commented-out code.
(TO_DO_HYPHEN)
CodSpeed Performance ReportMerging #8696 will not alter performanceComparing Summary
Footnotes
|
|
Just a thought, but could/should this perhaps be an option? |
4 participants
Summary
Fixes #8685
The
noUselessLoneBlockStatementsrule was flagging empty blocks even when they contained comments. The safe fix would then remove these blocks, deleting potentially important comments like TODOs or commented-out code.Changes
run()function to skip empty blocks that contain commentsTest Plan