Skip to content

Commit f458028

Browse files
pkallosematipico
pkallos
and
ematipico
authored
feat(lint): add ignoreBooleanCoercion option to useNullishCoalescing (#10595)
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
1 parent 8f9dec8 commit f458028

11 files changed

Lines changed: 394 additions & 3 deletions

File tree

‎.changeset/add-ignore-boolean-coercion.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Added the option `ignoreBooleanCoercion` to [useNullishCoalescing](https://biomejs.dev/linter/rules/use-nullish-coalescing/). When enabled, Biome ignores `||` and `||=` used inside a `Boolean()` call, where coalescing on falsy values is intentional.

‎crates/biome_js_analyze/src/lint/nursery/use_nullish_coalescing.rs‎

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ use biome_diagnostics::Severity;
77
use biome_js_factory::make;
88
use biome_js_syntax::{
99
AnyJsAssignmentPattern, AnyJsExpression, JsAssignmentExpression, JsAssignmentOperator,
10-
JsBinaryOperator, JsConditionalExpression, JsDoWhileStatement, JsForStatement,
11-
JsIfStatement, JsLogicalExpression, JsLogicalOperator, JsParenthesizedExpression,
12-
JsSyntaxKind, JsWhileStatement, OperatorPrecedence, T,
10+
JsBinaryOperator, JsCallArgumentList, JsCallArguments, JsCallExpression,
11+
JsConditionalExpression, JsDoWhileStatement, JsForStatement, JsIfStatement,
12+
JsLogicalExpression, JsLogicalOperator, JsParenthesizedExpression, JsSyntaxKind,
13+
JsWhileStatement, OperatorPrecedence, T,
1314
};
1415
use biome_js_type_info::{ConditionalType, TypeData};
1516
use biome_rowan::{
@@ -157,6 +158,38 @@ declare_lint_rule! {
157158
/// assigned ||= b && 'fallback';
158159
/// ```
159160
///
161+
/// ### ignoreBooleanCoercion
162+
///
163+
/// Ignore `||` and `||=` used inside a `Boolean()` call, where coalescing on
164+
/// falsy values is intentional. Default: `false`.
165+
///
166+
/// ```json,options
167+
/// {
168+
/// "options": {
169+
/// "ignoreBooleanCoercion": true
170+
/// }
171+
/// }
172+
/// ```
173+
///
174+
/// #### Invalid
175+
///
176+
/// `||` and `||=` outside a `Boolean()` call are still reported.
177+
///
178+
/// ```ts,expect_diagnostic,use_options,file=invalid-boolean-coercion.ts
179+
/// declare const maybeString: string | null;
180+
/// const value = maybeString || 'default';
181+
/// ```
182+
///
183+
/// #### Valid
184+
///
185+
/// `||` and `||=` inside a `Boolean()` call are not reported.
186+
///
187+
/// ```ts,use_options,file=valid-boolean-coercion.ts
188+
/// declare const a: string | null;
189+
/// declare const b: string;
190+
/// const r = Boolean(a || b);
191+
/// ```
192+
///
160193
pub UseNullishCoalescing {
161194
version: "2.4.5",
162195
name: "useNullishCoalescing",
@@ -399,6 +432,12 @@ fn run_logical_or(
399432
return None;
400433
}
401434

435+
if options.ignore_boolean_coercion()
436+
&& is_in_boolean_call_context(&AnyJsLogicalOrLikeExpression::from(logical.clone()))
437+
{
438+
return None;
439+
}
440+
402441
let left = logical.left().ok()?;
403442
let left_ty = ctx.type_of_expression(&left);
404443

@@ -432,6 +471,12 @@ fn run_logical_or_assignment(
432471
return None;
433472
}
434473

474+
if options.ignore_boolean_coercion()
475+
&& is_in_boolean_call_context(&AnyJsLogicalOrLikeExpression::from(assignment.clone()))
476+
{
477+
return None;
478+
}
479+
435480
let left = assignment.left().ok()?;
436481
let left_ty = match &left {
437482
AnyJsAssignmentPattern::AnyJsAssignment(assign) => {
@@ -606,6 +651,30 @@ fn operand_reaches_logical_and(operand: SyntaxResult<AnyJsExpression>) -> bool {
606651
})
607652
}
608653

654+
/// Returns `true` when the `||`/`||=` expression is boolean-coerced by an
655+
/// enclosing `Boolean(...)` call, where coalescing on falsy values is the
656+
/// intended behavior.
657+
///
658+
/// It skips parenthesized and logical-expression ancestors (both `||` and `&&`,
659+
/// since the coerced value still flows into the call) so that
660+
/// `Boolean(a || b || c)` and `Boolean(x && (a || b))` are both treated as
661+
/// coerced, then inspects the first ancestor that is neither. That ancestor must
662+
/// be the argument list of a `Boolean(...)` call for the expression to be
663+
/// considered coerced.
664+
fn is_in_boolean_call_context(node: &AnyJsLogicalOrLikeExpression) -> bool {
665+
node.syntax()
666+
.ancestors()
667+
.skip(1)
668+
.find(|ancestor| {
669+
!(JsParenthesizedExpression::can_cast(ancestor.kind())
670+
|| JsLogicalExpression::can_cast(ancestor.kind()))
671+
})
672+
.and_then(JsCallArgumentList::cast)
673+
.and_then(|list| list.parent::<JsCallArguments>())
674+
.and_then(|args| args.parent::<JsCallExpression>())
675+
.is_some_and(|call| call.has_callee("Boolean"))
676+
}
677+
609678
fn is_in_test_position(logical: &JsLogicalExpression) -> bool {
610679
let logical_range = logical.syntax().text_trimmed_range();
611680

‎crates/biome_js_analyze/tests/specs/nursery/useNullishCoalescing/ignoreBooleanCoercionDisabled.options.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
3+
"linter": {
4+
"rules": {
5+
"nursery": {
6+
"useNullishCoalescing": {
7+
"level": "error",
8+
"options": {
9+
"ignoreBooleanCoercion": false
10+
}
11+
}
12+
}
13+
}
14+
}
15+
}

‎crates/biome_js_analyze/tests/specs/nursery/useNullishCoalescing/ignoreBooleanCoercionDisabled.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Tests for ignoreBooleanCoercion: false
2+
// `||` and `||=` inside a Boolean() call SHOULD still generate diagnostics.
3+
4+
declare const a: string | null;
5+
declare const b: string;
6+
declare const c: string;
7+
8+
// || directly inside Boolean()
9+
const r1 = Boolean(a || b);
10+
11+
// chained || inside Boolean()
12+
const r2 = Boolean(a || b || c);
13+
14+
// ||= inside Boolean()
15+
declare let assigned: string | null;
16+
const r3 = Boolean(assigned ||= b);

‎crates/biome_js_analyze/tests/specs/nursery/useNullishCoalescing/ignoreBooleanCoercionDisabled.ts.snap‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: ignoreBooleanCoercionDisabled.ts
4+
---
5+
# Input
6+
```ts
7+
// Tests for ignoreBooleanCoercion: false
8+
// `||` and `||=` inside a Boolean() call SHOULD still generate diagnostics.
9+
10+
declare const a: string | null;
11+
declare const b: string;
12+
declare const c: string;
13+
14+
// || directly inside Boolean()
15+
const r1 = Boolean(a || b);
16+
17+
// chained || inside Boolean()
18+
const r2 = Boolean(a || b || c);
19+
20+
// ||= inside Boolean()
21+
declare let assigned: string | null;
22+
const r3 = Boolean(assigned ||= b);
23+
24+
```
25+
26+
# Diagnostics
27+
```
28+
ignoreBooleanCoercionDisabled.ts:9:22 lint/nursery/useNullishCoalescing ━━━━━━━━━━━━━━━━━━━━━━━━━━━━
29+
30+
i Use ?? instead of ||.
31+
32+
8 │ // || directly inside Boolean()
33+
> 9 │ const r1 = Boolean(a || b);
34+
│ ^^
35+
10 │
36+
11 │ // chained || inside Boolean()
37+
38+
i The || operator checks for all falsy values (including 0, '', and false), while ?? only checks for null and undefined.
39+
40+
i This rule is still being actively worked on, so it may be missing features or have rough edges. Visit https://github.com/biomejs/biome/issues/8043 for more information or to report possible bugs.
41+
42+
i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information.
43+
44+
45+
```
46+
47+
```
48+
ignoreBooleanCoercionDisabled.ts:12:22 lint/nursery/useNullishCoalescing ━━━━━━━━━━━━━━━━━━━━━━━━━━━
49+
50+
i Use ?? instead of ||.
51+
52+
11 │ // chained || inside Boolean()
53+
> 12 │ const r2 = Boolean(a || b || c);
54+
│ ^^
55+
13 │
56+
14 │ // ||= inside Boolean()
57+
58+
i The || operator checks for all falsy values (including 0, '', and false), while ?? only checks for null and undefined.
59+
60+
i This rule is still being actively worked on, so it may be missing features or have rough edges. Visit https://github.com/biomejs/biome/issues/8043 for more information or to report possible bugs.
61+
62+
i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information.
63+
64+
65+
```
66+
67+
```
68+
ignoreBooleanCoercionDisabled.ts:16:29 lint/nursery/useNullishCoalescing ━━━━━━━━━━━━━━━━━━━━━━━━━━━
69+
70+
i Use ??= instead of ||=.
71+
72+
14 │ // ||= inside Boolean()
73+
15 │ declare let assigned: string | null;
74+
> 16 │ const r3 = Boolean(assigned ||= b);
75+
│ ^^^
76+
17 │
77+
78+
i The ||= operator assigns when the left side is falsy, while ??= only assigns when it is null or undefined.
79+
80+
i This rule is still being actively worked on, so it may be missing features or have rough edges. Visit https://github.com/biomejs/biome/issues/8043 for more information or to report possible bugs.
81+
82+
i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information.
83+
84+
85+
```

‎crates/biome_js_analyze/tests/specs/nursery/useNullishCoalescing/ignoreBooleanCoercionEnabled.options.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
3+
"linter": {
4+
"rules": {
5+
"nursery": {
6+
"useNullishCoalescing": {
7+
"level": "error",
8+
"options": {
9+
"ignoreBooleanCoercion": true
10+
}
11+
}
12+
}
13+
}
14+
}
15+
}

‎crates/biome_js_analyze/tests/specs/nursery/useNullishCoalescing/ignoreBooleanCoercionEnabled.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Tests for ignoreBooleanCoercion: true
2+
// `||` and `||=` inside a Boolean() call should NOT generate diagnostics.
3+
4+
declare const a: string | null;
5+
declare const b: string;
6+
declare const c: string;
7+
8+
// || directly inside Boolean()
9+
const r1 = Boolean(a || b);
10+
11+
// chained || inside Boolean()
12+
const r2 = Boolean(a || b || c);
13+
14+
// || inside parenthesized argument
15+
const r3 = Boolean((a || b));
16+
17+
// ||= inside Boolean()
18+
declare let assigned: string | null;
19+
const r4 = Boolean(assigned ||= b);
20+
21+
// || nested inside Boolean() within another call: still suppressed because the
22+
// value still flows into the Boolean() coercion.
23+
declare function identity<T>(value: T): T;
24+
const r4b = identity(Boolean(a || 'default'));
25+
26+
// || mixed with && inside Boolean(): the whole argument is boolean-coerced,
27+
// so the walk steps through the && parent and suppresses the ||.
28+
declare const d: string;
29+
const r4c = Boolean(d && (a || 'default'));
30+
31+
// These SHOULD still generate diagnostics (not inside Boolean()).
32+
const r5 = a || 'default';
33+
34+
declare const n: number | undefined;
35+
const r6 = n || 0;
36+
37+
declare let assignPlain: string | null;
38+
assignPlain ||= 'default';
39+
40+
// Inside a non-Boolean call: SHOULD generate a diagnostic.
41+
const r7 = identity(a || 'default');

0 commit comments

Comments
 (0)