fix(noExtraBooleanCast): preserve parentheses to maintain operator precedence #7244
Conversation
Fixes biomejs#7225 Added test case to verify that noExtraBooleanCast rule correctly preserves parentheses when removing Boolean calls inside negations. The expression !Boolean(b0 && b1) should become !(b0 && b1) not !b0 && b1 to maintain correct operator precedence. Test Plan Added reproduction test case for the reported issue. The test will verify whether the existing parentheses preservation logic correctly handles binary expressions inside negated Boolean calls.
🦋 Changeset detectedLatest commit: 1617b75 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 |
WalkthroughThe PR updates the no_extra_boolean_cast lint to preserve parentheses when removing Boolean() or new Boolean() inside a logical negation. It adds needs_parentheses_when_negated to detect expressions requiring wrapping, emits a JsParenthesizedExpression when required, updates tests to include a case for !Boolean(b0 && b1), and adds a changeset documenting the behaviour change. No public APIs or signatures were modified. Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (25)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js (1)
25-28: Great reproduction for #7225; consider adding an OR variant too.This captures the precedence pitfall for
&&. For broader coverage, add an||case as well.Suggested addition (after these lines):
+// Also verify OR precedence is preserved +const booleanOr = !Boolean(b0 || b1);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/add-boolean-cast-test-case.md(1 hunks)crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
.changeset/*.md
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
.changeset/*.md: Create changesets withjust new-changeset; store them in.changeset/with correct frontmatter (package keys and change type).
In changeset descriptions, follow content conventions: user-facing changes only; past tense for what you did; present tense for current behavior; link issues for fixes; link rules/assists; include representative code blocks; end every sentence with a period.
When adding headers in a changeset, only use #### or ##### levels.
Files:
.changeset/add-boolean-cast-test-case.md
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js
**/tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js
⏰ 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). (8)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test Node.js API
- GitHub Check: autofix
🔇 Additional comments (2)
.changeset/add-boolean-cast-test-case.md (1)
2-2: Consider using a “none” release type for a test-only change.If there’s no user-facing behaviour change, a release bump may be unnecessary. Switch to
noneunless you intentionally want a patch release.Proposed change:
-"@biomejs/biome": patch +"@biomejs/biome": noneCan you confirm the repo’s policy on releasing for test-only PRs?
crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js (1)
23-24: LGTM: additional bare expression case is useful.Adding
!Boolean(x);broadens contexts covered by the rule’s invalid samples. No concerns.
…olean calls - Add parentheses preservation logic for Boolean calls inside negations - Add needs_parentheses_when_negated helper function - Ensure !Boolean(b0 && b1) becomes !(b0 && b1) not !b0 && b1 - Preserve operator precedence for binary/conditional/assignment expressions Fixes biomejs#7225
|
@dyc3 I've implemented the actual fix that preserves parentheses when removing Boolean calls inside negations - the rule now correctly transforms |
CodSpeed Performance ReportMerging #7244 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/fix-boolean-cast-parentheses.md(1 hunks)crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/fix-boolean-cast-parentheses.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
🧠 Learnings (7)
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : When a token is mandatory and present in the AST, use the AST-provided token (e.g., node.l_paren_token().format()) instead of hardcoded tokens
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to fix code: if a mandatory token/node is missing, return None instead
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use dbg_write! to debug and inspect the written IR elements during formatter development
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use the generic Format trait and FormatNode for AST nodes when implementing the formatter
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the FormatNode trait and implement it for your Node
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/src/cst.rs : Define FormatHtmlSyntaxNode in cst.rs and implement FormatRule<HtmlSyntaxNode>, plus AsFormat and IntoFormat for HtmlSyntaxNode using the provided mapping code
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Build an intermediate IR via the provided helper APIs when formatting
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
⏰ 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). (25)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: autofix
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs (1)
193-218: Smart precedence-preserving fix under negation — nice touchConditionally wrapping the Boolean argument when under a logical-not is the right approach and aligns with the regression you’re testing for.
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
Outdated
Show resolved
Hide resolved
…preservation - Add JsLogicalExpression and JsSequenceExpression to imports - Update needs_parentheses_when_negated to handle && || ?? operators - Update needs_parentheses_when_negated to handle comma operator - Improve comments to clarify binary vs logical expressions Addresses CodeRabbit review feedback
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs (2)
194-222: Precedence preserved under negation; add a small safety check (optional).The fix correctly wraps complex expressions in parentheses only when inside a logical-not. For extra robustness/readability, prefer typed parent lookup and ensure the current node is indeed the unary argument.
Apply this diff to tighten the check:
- let replacement = if matches!(extra_boolean_cast_type, ExtraBooleanCastType::BooleanCall) { - // Check if this Boolean call is inside a unary negation - if let Some(unary_expr) = node.syntax().parent().and_then(JsUnaryExpression::cast) { - if matches!(unary_expr.operator(), Ok(JsUnaryOperator::LogicalNot)) { + let replacement = if matches!(extra_boolean_cast_type, ExtraBooleanCastType::BooleanCall) { + // Check if this Boolean call is inside a unary negation and we're the direct argument + if let Some(unary_expr) = node.parent::<JsUnaryExpression>() { + if matches!(unary_expr.operator(), Ok(JsUnaryOperator::LogicalNot)) + && unary_expr.argument().ok().map(|arg| arg.syntax() == node.syntax()).unwrap_or(false) + { // Check if the argument is a complex expression that needs parentheses if needs_parentheses_when_negated(node_to_replace) { // Wrap in parentheses to preserve operator precedence AnyJsExpression::JsParenthesizedExpression( make::js_parenthesized_expression( make::token(T!['(']), node_to_replace.clone(), make::token(T![')']), ), ) } else { node_to_replace.clone() } } else { node_to_replace.clone() } } else { node_to_replace.clone() } } else { node_to_replace.clone() };
234-254: Good coverage of cases needing parentheses; tiny comment nit.The helper sensibly flags binary/logical/conditional/assignment/sequence expressions. The comment on Line 249 mentions “Logical expressions that are already parenthesized”; it’s really “Expressions that are already parenthesized”.
Apply this tiny wording tweak:
- // Logical expressions that are already parenthesized don't need additional ones + // Expressions that are already parenthesized don't need additional onesOptional follow-up (no code change required): consider adding tests for
||and??to mirror the new logical handling.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
🧠 Learnings (6)
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : When a token is mandatory and present in the AST, use the AST-provided token (e.g., node.l_paren_token().format()) instead of hardcoded tokens
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use the generic Format trait and FormatNode for AST nodes when implementing the formatter
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the FormatNode trait and implement it for your Node
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to fix code: if a mandatory token/node is missing, return None instead
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Build an intermediate IR via the provided helper APIs when formatting
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
🧬 Code Graph Analysis (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs (4)
crates/biome_js_syntax/src/expr_ext.rs (1)
is_in_boolean_context(2206-2231)crates/biome_js_analyze/src/lint/style/use_array_literals.rs (3)
node(264-265)mutation(247-247)mutation(251-251)crates/biome_js_analyze/src/lint/style/use_exponentiation_operator.rs (2)
node(182-182)node(186-186)crates/biome_js_analyze/src/lint/suspicious/use_number_to_fixed_digits_argument.rs (1)
mutation(115-115)
⏰ 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). (22)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Documentation
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: autofix
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs (1)
6-12: Imports updated to support correct parenthesising under negation — nice.Adding the factory and AST variants (Logical, Sequence) is exactly what’s needed to preserve precedence.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs (1)
6-12: Imports extended appropriately to support precedence-aware fixesGood call adding the factory and the logical/sequence expr variants. This closes the gap for
&&,||, and??under negation and enables constructing the parenthesised replacement.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs (1)
234-253: Update parenthesized expression commentIn
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs:- // Logical expressions that are already parenthesized don't need additional ones + // Expressions that are already parenthesized don't need additional onesOptional: extend your invalid tests (e.g. in
crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js) to cover other operators under!Boolean(...):
!Boolean(a || b)!Boolean(a ?? b)!Boolean(a ? b : c)!Boolean(a = b)!Boolean(a, b)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
🧠 Learnings (7)
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : When a token is mandatory and present in the AST, use the AST-provided token (e.g., node.l_paren_token().format()) instead of hardcoded tokens
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use the generic Format trait and FormatNode for AST nodes when implementing the formatter
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Build an intermediate IR via the provided helper APIs when formatting
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the FormatNode trait and implement it for your Node
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to fix code: if a mandatory token/node is missing, return None instead
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use dbg_write! to debug and inspect the written IR elements during formatter development
Applied to files:
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs
🧬 Code Graph Analysis (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs (5)
crates/biome_js_syntax/src/expr_ext.rs (1)
is_in_boolean_context(2206-2231)crates/biome_js_analyze/src/lint/style/use_array_literals.rs (3)
node(264-265)mutation(247-247)mutation(251-251)crates/biome_js_analyze/src/lint/style/use_exponentiation_operator.rs (2)
node(182-182)node(186-186)crates/biome_js_analyze/src/lint/complexity/no_this_in_static.rs (1)
token(189-194)crates/biome_js_analyze/src/lint/suspicious/use_number_to_fixed_digits_argument.rs (1)
mutation(115-115)
⏰ 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). (23)
- GitHub Check: Test Node.js API
- GitHub Check: End-to-end tests
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: autofix
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs (1)
195-221: Parentheses-preserving fix under negation looks spot onThe conditional wrapping when the callee is
Boolean(...)and the parent is!correctly preserves precedence for binary/logical/conditional/assignment/sequence expressions. This should transform!Boolean(b0 && b1)to!(b0 && b1), addressing the regression in #7225 without over-parenthesising simples. Usingmake::js_parenthesized_expressionis the right tool here.Also applies to: 223-223
reproduction_test.js
Outdated
test_boolean_cast_bug.js
Outdated
test_case.js
Outdated
| let replacement = if matches!(extra_boolean_cast_type, ExtraBooleanCastType::BooleanCall) { | ||
| // Check if this Boolean call is inside a unary negation | ||
| if let Some(unary_expr) = node.syntax().parent().and_then(JsUnaryExpression::cast) { | ||
| if matches!(unary_expr.operator(), Ok(JsUnaryOperator::LogicalNot)) { | ||
| // Check if the argument is a complex expression that needs parentheses |
There was a problem hiding this comment.
You can definitely reduce the indentation here a bit. Something like:
if let Some(unary_expr) = node.syntax().parent().and_then(JsUnaryExpression::cast).map(|e| e.operator()).ok().is_some_and(|op| op == JsUnaryOperator::LogicalNot)| } else { | ||
| node_to_replace.clone() | ||
| } | ||
| } else { | ||
| node_to_replace.clone() | ||
| } | ||
| } else { | ||
| node_to_replace.clone() | ||
| } | ||
| } else { | ||
| node_to_replace.clone() | ||
| }; |
There was a problem hiding this comment.
And you can definitely refactor this so that these elses aren't necessary.
Something like this probably:
let mut replacement = ...;
if cond {
replacement = AnyJsExpression::JsParenthesizedExpression(
make::js_parenthesized_expression(
make::token(T!['(']),
replacement,
make::token(T![')']),
),
)
}
mutation.replace_node(node.clone(), replacement);|
@ematipico I removed the files which are not needed...Can you please check and confirm :-)) |
|
Thankyou @ematipico |
…ecedence (#7244) Co-authored-by: Emanuele Stoppa <[email protected]> Co-authored-by: dyc3 <[email protected]> Co-authored-by: ematipico <[email protected]>
…ecedence (biomejs#7244) Co-authored-by: Emanuele Stoppa <[email protected]> Co-authored-by: dyc3 <[email protected]> Co-authored-by: ematipico <[email protected]>
…ecedence (biomejs#7244) Co-authored-by: Emanuele Stoppa <[email protected]> Co-authored-by: dyc3 <[email protected]> Co-authored-by: ematipico <[email protected]>
Fixes #7225
Added test case to verify that the
noExtraBooleanCastrule correctly preserves parentheses when removing Boolean calls inside negations. The expression!Boolean(b0 && b1)should become!(b0 && b1)not!b0 && b1to maintain correct operator precedence.Summary
Added reproduction test case for issue #7225 where the
noExtraBooleanCastrule was incorrectly transforming!Boolean(b0 && b1)to!b0 && b1instead of!(b0 && b1). This test case will verify whether the existing parentheses preservation logic correctly handles binary expressions inside negated Boolean calls.Test Plan
The test case added to
invalid.jswill reproduce the reported issue and validate the fix behavior. The existing rule implementation appears to have parentheses preservation logic that should handle this case, but this test ensures proper coverage and verification. GitHub CI will validate the build and run all tests to confirm the expected behavior.Changes
!Boolean(b0 && b1)incrates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js