Skip to content

Commit 660031b

Browse files
JeetuSuthardyc3ematipico
authored
fix(noExtraBooleanCast): preserve parentheses to maintain operator precedence (#7244)
Co-authored-by: Emanuele Stoppa <[email protected]> Co-authored-by: dyc3 <[email protected]> Co-authored-by: ematipico <[email protected]>
1 parent 1511d0c commit 660031b

File tree

5 files changed

+95
-4
lines changed

5 files changed

+95
-4
lines changed

.changeset/add-boolean-cast-test-case.md

Whitespace-only changes.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Fixed [#7225](https://github.com/biomejs/biome/issues/7225): The `noExtraBooleanCast` rule now preserves parentheses when removing `Boolean` calls inside negations.
6+
7+
```js
8+
// Before
9+
!Boolean(b0 && b1)
10+
// After
11+
!(b0 && b1) // instead of !b0 && b1
12+
```

crates/biome_js_analyze/src/lint/complexity/no_extra_boolean_cast.rs

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ use biome_analyze::{
33
};
44
use biome_console::markup;
55
use biome_diagnostics::Severity;
6+
use biome_js_factory::make;
7+
#[expect(unused_imports)]
68
use biome_js_syntax::{
7-
AnyJsExpression, JsCallArgumentList, JsCallArguments, JsCallExpression, JsNewExpression,
8-
JsSyntaxNode, JsUnaryOperator, is_in_boolean_context, is_negation,
9+
AnyJsExpression, JsAssignmentExpression, JsBinaryExpression, JsCallArgumentList,
10+
JsCallArguments, JsCallExpression, JsConditionalExpression, JsLogicalExpression,
11+
JsNewExpression, JsParenthesizedExpression, JsSequenceExpression, JsSyntaxNode,
12+
JsUnaryExpression, JsUnaryOperator, T, is_in_boolean_context, is_negation,
913
};
1014
use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt};
1115
use biome_rule_options::no_extra_boolean_cast::NoExtraBooleanCastOptions;
@@ -188,7 +192,30 @@ impl Rule for NoExtraBooleanCast {
188192
ExtraBooleanCastType::DoubleNegation => "Remove redundant double-negation",
189193
ExtraBooleanCastType::BooleanCall => "Remove redundant `Boolean` call",
190194
};
191-
mutation.replace_node(node.clone(), node_to_replace.clone());
195+
196+
// Check if the Boolean call is inside a unary negation and the argument needs parentheses
197+
let mut replacement = node_to_replace.clone();
198+
199+
// Only wrap in parentheses if this is a Boolean call inside a logical NOT with complex expression
200+
if matches!(extra_boolean_cast_type, ExtraBooleanCastType::BooleanCall) {
201+
let is_negated_boolean_call = node
202+
.syntax()
203+
.parent()
204+
.and_then(JsUnaryExpression::cast)
205+
.and_then(|expr| expr.operator().ok())
206+
.is_some_and(|op| op == JsUnaryOperator::LogicalNot);
207+
208+
if is_negated_boolean_call && needs_parentheses_when_negated(node_to_replace) {
209+
replacement =
210+
AnyJsExpression::JsParenthesizedExpression(make::js_parenthesized_expression(
211+
make::token(T!['(']),
212+
replacement,
213+
make::token(T![')']),
214+
));
215+
}
216+
}
217+
218+
mutation.replace_node(node.clone(), replacement);
192219

193220
Some(JsRuleAction::new(
194221
ctx.metadata().action_category(ctx.category(), ctx.group()),
@@ -199,6 +226,27 @@ impl Rule for NoExtraBooleanCast {
199226
}
200227
}
201228

229+
/// Determines if an expression needs parentheses when it becomes the operand of a unary negation.
230+
/// This is needed to preserve operator precedence for expressions like binary expressions.
231+
fn needs_parentheses_when_negated(expr: &AnyJsExpression) -> bool {
232+
match expr {
233+
// Binary expressions like `a + b` need parentheses in `!(a + b)` to maintain precedence
234+
AnyJsExpression::JsBinaryExpression(_) => true,
235+
// Logical expressions like `a && b` need parentheses in `!(a && b)` to maintain precedence
236+
AnyJsExpression::JsLogicalExpression(_) => true,
237+
// Conditional expressions like `a ? b : c` need parentheses
238+
AnyJsExpression::JsConditionalExpression(_) => true,
239+
// Assignment expressions need parentheses
240+
AnyJsExpression::JsAssignmentExpression(_) => true,
241+
// Sequence expressions (comma operator) need parentheses
242+
AnyJsExpression::JsSequenceExpression(_) => true,
243+
// Logical expressions that are already parenthesized don't need additional ones
244+
AnyJsExpression::JsParenthesizedExpression(_) => false,
245+
// Simple expressions like identifiers, literals, calls don't need parentheses
246+
_ => false,
247+
}
248+
}
249+
202250
/// Check if the SyntaxNode is a Double Negation. Including the edge case
203251
/// ```js
204252
/// !(!x)

crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,9 @@ new Boolean(!!x);
2020

2121
!!!x;
2222

23-
!Boolean(x);
23+
!Boolean(x);
24+
25+
// Test case for issue #7225 - should preserve parentheses
26+
const b0 = false;
27+
const b1 = false;
28+
const boolean = !Boolean(b0 && b1);

crates/biome_js_analyze/tests/specs/complexity/noExtraBooleanCast/invalid.js.snap

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ new Boolean(!!x);
2727
!!!x;
2828
2929
!Boolean(x);
30+
31+
// Test case for issue #7225 - should preserve parentheses
32+
const b0 = false;
33+
const b1 = false;
34+
const boolean = !Boolean(b0 && b1);
3035
```
3136

3237
# Diagnostics
@@ -226,6 +231,8 @@ invalid.js:23:2 lint/complexity/noExtraBooleanCast FIXABLE ━━━━━━
226231
22 │
227232
> 23 │ !Boolean(x);
228233
│ ^^^^^^^^^^
234+
24 │
235+
25 │ // Test case for issue #7225 - should preserve parentheses
229236
230237
i It is not necessary to use `Boolean` call when a value will already be coerced to a boolean.
231238
@@ -235,3 +242,22 @@ invalid.js:23:2 lint/complexity/noExtraBooleanCast FIXABLE ━━━━━━
235242
│ -------- -
236243
237244
```
245+
246+
```
247+
invalid.js:28:18 lint/complexity/noExtraBooleanCast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
248+
249+
i Avoid redundant `Boolean` call
250+
251+
26 │ const b0 = false;
252+
27 │ const b1 = false;
253+
> 28 │ const boolean = !Boolean(b0 && b1);
254+
│ ^^^^^^^^^^^^^^^^^
255+
256+
i It is not necessary to use `Boolean` call when a value will already be coerced to a boolean.
257+
258+
i Safe fix: Remove redundant `Boolean` call
259+
260+
28 │ const·boolean·=·!Boolean(b0·&&·b1);
261+
│ -------
262+
263+
```

0 commit comments

Comments
 (0)