Skip to content

Commit 1d2ca15

Browse files
authored
feat(lint): add useNullishCoalescing nursery rule (#8952)
1 parent 97c92a1 commit 1d2ca15

File tree

19 files changed

+1403
-0
lines changed

19 files changed

+1403
-0
lines changed

.changeset/chubby-trees-refuse.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Added the nursery rule [`useNullishCoalescing`](https://biomejs.dev/linter/rules/use-nullish-coalescing/). This rule suggests using the nullish coalescing operator (`??`) instead of logical OR (`||`) when the left operand may be nullish. This prevents bugs where falsy values like `0`, `''`, or `false` are incorrectly treated as missing. Addresses [#8043](https://github.com/biomejs/biome/issues/8043)
6+
7+
```ts
8+
// Invalid
9+
declare const x: string | null;
10+
const value = x || "default";
11+
12+
// Valid
13+
const value = x ?? "default";
14+
```

crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_configuration/src/analyzer/linter/rules.rs

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_configuration/src/generated/domain_selector.rs

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_diagnostics_categories/src/categories.rs

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
use crate::{JsRuleAction, services::typed::Typed};
2+
use biome_analyze::{
3+
FixKind, Rule, RuleDiagnostic, RuleDomain, RuleSource, context::RuleContext, declare_lint_rule,
4+
};
5+
use biome_console::markup;
6+
use biome_diagnostics::Severity;
7+
use biome_js_factory::make;
8+
use biome_js_syntax::{
9+
AnyJsExpression, JsConditionalExpression, JsDoWhileStatement, JsForStatement, JsIfStatement,
10+
JsLogicalExpression, JsLogicalOperator, JsParenthesizedExpression, JsSyntaxKind,
11+
JsWhileStatement, T,
12+
};
13+
use biome_js_type_info::ConditionalType;
14+
use biome_rowan::{AstNode, BatchMutationExt, TextRange};
15+
use biome_rule_options::use_nullish_coalescing::UseNullishCoalescingOptions;
16+
17+
declare_lint_rule! {
18+
/// Enforce using nullish coalescing operator (`??`) instead of logical or (`||`).
19+
///
20+
/// The `??` operator only checks for `null` and `undefined`, while `||` checks
21+
/// for any falsy value including `0`, `''`, and `false`. This can prevent bugs
22+
/// where legitimate falsy values are incorrectly treated as missing.
23+
///
24+
/// This rule triggers when the left operand of `||` is possibly nullish (contains
25+
/// `null` or `undefined` in its type). A safe fix is only offered when the type
26+
/// analysis confirms the left operand can only be truthy or nullish (not other
27+
/// falsy values like `0` or `''`).
28+
///
29+
/// By default, `||` expressions in conditional test positions (if/while/for/ternary)
30+
/// are ignored, as the falsy-checking behavior is often intentional there. This can
31+
/// be disabled with the `ignoreConditionalTests` option.
32+
///
33+
/// ## Examples
34+
///
35+
/// ### Invalid
36+
///
37+
/// ```ts
38+
/// declare const maybeString: string | null;
39+
/// const value = maybeString || 'default'; // should use ??
40+
/// ```
41+
///
42+
/// ```ts
43+
/// declare const maybeNumber: number | undefined;
44+
/// const value = maybeNumber || 0; // should use ??
45+
/// ```
46+
///
47+
/// ### Valid
48+
///
49+
/// ```ts
50+
/// // Already using ??
51+
/// declare const maybeString: string | null;
52+
/// const value = maybeString ?? 'default';
53+
/// ```
54+
///
55+
/// ```ts
56+
/// // Type is not nullish - no null or undefined in union
57+
/// declare const definiteString: string;
58+
/// const value = definiteString || 'fallback';
59+
/// ```
60+
///
61+
/// ```ts
62+
/// // In conditional test position (ignored by default)
63+
/// declare const cond: string | null;
64+
/// if (cond || 'fallback') {
65+
/// console.log('in if');
66+
/// }
67+
/// ```
68+
///
69+
pub UseNullishCoalescing {
70+
version: "next",
71+
name: "useNullishCoalescing",
72+
language: "js",
73+
sources: &[RuleSource::EslintTypeScript("prefer-nullish-coalescing").inspired()],
74+
recommended: false,
75+
severity: Severity::Information,
76+
fix_kind: FixKind::Safe,
77+
domains: &[RuleDomain::Types],
78+
issue_number: Some("8043"),
79+
}
80+
}
81+
82+
pub struct UseNullishCoalescingState {
83+
operator_range: TextRange,
84+
can_fix: bool,
85+
}
86+
87+
impl Rule for UseNullishCoalescing {
88+
type Query = Typed<JsLogicalExpression>;
89+
type State = UseNullishCoalescingState;
90+
type Signals = Option<Self::State>;
91+
type Options = UseNullishCoalescingOptions;
92+
93+
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
94+
let logical = ctx.query();
95+
let operator = logical.operator().ok()?;
96+
if operator != JsLogicalOperator::LogicalOr {
97+
return None;
98+
}
99+
100+
let options = ctx.options();
101+
if options.ignore_conditional_tests() && is_in_test_position(logical) {
102+
return None;
103+
}
104+
105+
let left = logical.left().ok()?;
106+
let left_ty = ctx.type_of_expression(&left);
107+
108+
if !is_possibly_nullish(&left_ty) {
109+
return None;
110+
}
111+
112+
let can_fix = is_safe_type_for_replacement(&left_ty)
113+
&& is_safe_syntax_context_for_replacement(logical);
114+
115+
Some(UseNullishCoalescingState {
116+
operator_range: logical.operator_token().ok()?.text_trimmed_range(),
117+
can_fix,
118+
})
119+
}
120+
121+
fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
122+
Some(
123+
RuleDiagnostic::new(
124+
rule_category!(),
125+
state.operator_range,
126+
markup! {
127+
"Use "<Emphasis>"??"</Emphasis>" instead of "<Emphasis>"||"</Emphasis>"."
128+
},
129+
)
130+
.note(markup! {
131+
"The "<Emphasis>"||"</Emphasis>" operator checks for all falsy values (including 0, '', and false), while "<Emphasis>"??"</Emphasis>" only checks for null and undefined."
132+
}),
133+
)
134+
}
135+
136+
fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
137+
if !state.can_fix {
138+
return None;
139+
}
140+
141+
let node = ctx.query();
142+
let old_token = node.operator_token().ok()?;
143+
144+
let new_token = make::token(T![??])
145+
.with_leading_trivia_pieces(old_token.leading_trivia().pieces())
146+
.with_trailing_trivia_pieces(old_token.trailing_trivia().pieces());
147+
148+
let mut mutation = ctx.root().begin();
149+
mutation.replace_token(old_token, new_token);
150+
151+
Some(JsRuleAction::new(
152+
ctx.metadata().action_category(ctx.category(), ctx.group()),
153+
ctx.metadata().applicability(),
154+
markup! { "Use "<Emphasis>"??"</Emphasis>" instead." }.to_owned(),
155+
mutation,
156+
))
157+
}
158+
}
159+
160+
fn is_safe_type_for_replacement(ty: &biome_js_type_info::Type) -> bool {
161+
if ty.is_union() {
162+
ty.flattened_union_variants().all(|variant| {
163+
matches!(
164+
variant.conditional_semantics(),
165+
ConditionalType::Truthy | ConditionalType::Nullish
166+
)
167+
})
168+
} else {
169+
matches!(
170+
ty.conditional_semantics(),
171+
ConditionalType::Truthy | ConditionalType::Nullish
172+
)
173+
}
174+
}
175+
176+
fn is_possibly_nullish(ty: &biome_js_type_info::Type) -> bool {
177+
if ty.is_union() {
178+
ty.flattened_union_variants()
179+
.any(|variant| variant.conditional_semantics().is_nullish())
180+
} else {
181+
ty.conditional_semantics().is_nullish()
182+
}
183+
}
184+
185+
fn is_safe_syntax_context_for_replacement(logical: &JsLogicalExpression) -> bool {
186+
let is_parenthesized = logical
187+
.syntax()
188+
.parent()
189+
.is_some_and(|parent| JsParenthesizedExpression::can_cast(parent.kind()));
190+
191+
if !is_parenthesized
192+
&& logical
193+
.syntax()
194+
.parent()
195+
.is_some_and(|parent| JsLogicalExpression::can_cast(parent.kind()))
196+
{
197+
return false;
198+
}
199+
200+
let left = logical.left().ok();
201+
let right = logical.right().ok();
202+
203+
if left
204+
.as_ref()
205+
.is_some_and(is_unparenthesized_and_or_expression)
206+
|| right
207+
.as_ref()
208+
.is_some_and(is_unparenthesized_and_or_expression)
209+
{
210+
return false;
211+
}
212+
213+
true
214+
}
215+
216+
fn is_unparenthesized_and_or_expression(expr: &AnyJsExpression) -> bool {
217+
match expr {
218+
AnyJsExpression::JsParenthesizedExpression(_) => false,
219+
AnyJsExpression::JsLogicalExpression(logical) => {
220+
logical.operator().ok().is_some_and(|op| {
221+
matches!(
222+
op,
223+
JsLogicalOperator::LogicalAnd | JsLogicalOperator::LogicalOr
224+
)
225+
})
226+
}
227+
_ => false,
228+
}
229+
}
230+
231+
fn is_in_test_position(logical: &JsLogicalExpression) -> bool {
232+
let logical_range = logical.syntax().text_trimmed_range();
233+
234+
let test_contains_logical = |test: Option<AnyJsExpression>| -> bool {
235+
test.is_some_and(|t| t.syntax().text_trimmed_range().contains_range(logical_range))
236+
};
237+
238+
for ancestor in logical.syntax().ancestors() {
239+
let is_in_test = match ancestor.kind() {
240+
JsSyntaxKind::JS_IF_STATEMENT => {
241+
test_contains_logical(JsIfStatement::cast_ref(&ancestor).and_then(|s| s.test().ok()))
242+
}
243+
JsSyntaxKind::JS_WHILE_STATEMENT => {
244+
test_contains_logical(JsWhileStatement::cast_ref(&ancestor).and_then(|s| s.test().ok()))
245+
}
246+
JsSyntaxKind::JS_FOR_STATEMENT => {
247+
test_contains_logical(JsForStatement::cast_ref(&ancestor).and_then(|s| s.test()))
248+
}
249+
JsSyntaxKind::JS_DO_WHILE_STATEMENT => {
250+
test_contains_logical(JsDoWhileStatement::cast_ref(&ancestor).and_then(|s| s.test().ok()))
251+
}
252+
JsSyntaxKind::JS_CONDITIONAL_EXPRESSION => {
253+
test_contains_logical(JsConditionalExpression::cast_ref(&ancestor).and_then(|s| s.test().ok()))
254+
}
255+
_ => false,
256+
};
257+
if is_in_test {
258+
return true;
259+
}
260+
}
261+
false
262+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Test cases for comment trivia preservation around ||
2+
// Using types that are safe for replacement (only truthy or nullish)
3+
4+
declare const a: object | null;
5+
declare const b: object | undefined;
6+
7+
// Comments should be preserved when replacing || with ??
8+
const x = a /* inline comment */ || {};
9+
const y = a || /* inline comment */ {};
10+
const z = a /* before */ || /* after */ {};
11+
12+
// Leading and trailing comments on the operator
13+
const w = b
14+
// line comment before
15+
|| {};
16+
17+
const v = b ||
18+
// line comment after
19+
{};

0 commit comments

Comments
 (0)