Skip to content

Commit edf8bb6

Browse files
feat(lint): add ||= to ??= detection in useNullishCoalescing (#9257)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 412a08d commit edf8bb6

File tree

10 files changed

+615
-54
lines changed

10 files changed

+615
-54
lines changed

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

Lines changed: 154 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,31 @@ use biome_console::markup;
66
use biome_diagnostics::Severity;
77
use biome_js_factory::make;
88
use biome_js_syntax::{
9-
AnyJsExpression, JsConditionalExpression, JsDoWhileStatement, JsForStatement, JsIfStatement,
9+
AnyJsAssignmentPattern, AnyJsExpression, JsAssignmentExpression, JsAssignmentOperator,
10+
JsConditionalExpression, JsDoWhileStatement, JsForStatement, JsIfStatement,
1011
JsLogicalExpression, JsLogicalOperator, JsParenthesizedExpression, JsSyntaxKind,
1112
JsWhileStatement, T,
1213
};
1314
use biome_js_type_info::ConditionalType;
14-
use biome_rowan::{AstNode, BatchMutationExt, TextRange};
15+
use biome_rowan::{
16+
AstNode, BatchMutationExt, TextRange, declare_node_union,
17+
};
1518
use biome_rule_options::use_nullish_coalescing::UseNullishCoalescingOptions;
1619

1720
declare_lint_rule! {
18-
/// Enforce using nullish coalescing operator (`??`) instead of logical or (`||`).
21+
/// Enforce using the nullish coalescing operator (`??`) instead of logical or (`||`).
1922
///
2023
/// The `??` operator only checks for `null` and `undefined`, while `||` checks
2124
/// for any falsy value including `0`, `''`, and `false`. This can prevent bugs
2225
/// where legitimate falsy values are incorrectly treated as missing.
2326
///
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 `''`).
27+
/// For `||` expressions, this rule triggers when the left operand is possibly
28+
/// nullish (contains `null` or `undefined` in its type). A safe fix is only
29+
/// offered when type analysis confirms the left operand can only be truthy or
30+
/// nullish (not other falsy values like `0` or `''`).
31+
///
32+
/// For `||=` assignment expressions, the same logic applies: `a ||= b` is
33+
/// flagged when `a` is possibly nullish and can be rewritten as `a ??= b`.
2834
///
2935
/// By default, `||` expressions in conditional test positions (if/while/for/ternary)
3036
/// are ignored, as the falsy-checking behavior is often intentional there. This can
@@ -44,6 +50,11 @@ declare_lint_rule! {
4450
/// const value = maybeNumber || 0; // should use ??
4551
/// ```
4652
///
53+
/// ```ts
54+
/// declare let x: string | null;
55+
/// x ||= 'default'; // should use ??=
56+
/// ```
57+
///
4758
/// ### Valid
4859
///
4960
/// ```ts
@@ -66,6 +77,12 @@ declare_lint_rule! {
6677
/// }
6778
/// ```
6879
///
80+
/// ```ts
81+
/// // Already using ??=
82+
/// declare let y: string | null;
83+
/// y ??= 'default';
84+
/// ```
85+
///
6986
pub UseNullishCoalescing {
7087
version: "next",
7188
name: "useNullishCoalescing",
@@ -79,69 +96,93 @@ declare_lint_rule! {
7996
}
8097
}
8198

82-
pub struct UseNullishCoalescingState {
83-
operator_range: TextRange,
84-
can_fix: bool,
99+
declare_node_union! {
100+
pub UseNullishCoalescingQuery = JsLogicalExpression | JsAssignmentExpression
101+
}
102+
103+
pub enum UseNullishCoalescingState {
104+
LogicalOr {
105+
operator_range: TextRange,
106+
can_fix: bool,
107+
},
108+
LogicalOrAssignment {
109+
operator_range: TextRange,
110+
can_fix: bool,
111+
},
85112
}
86113

87114
impl Rule for UseNullishCoalescing {
88-
type Query = Typed<JsLogicalExpression>;
115+
type Query = Typed<UseNullishCoalescingQuery>;
89116
type State = UseNullishCoalescingState;
90117
type Signals = Option<Self::State>;
91118
type Options = UseNullishCoalescingOptions;
92119

93120
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;
121+
match ctx.query() {
122+
UseNullishCoalescingQuery::JsLogicalExpression(logical) => {
123+
run_logical_or(ctx, logical)
124+
}
125+
UseNullishCoalescingQuery::JsAssignmentExpression(assignment) => {
126+
run_logical_or_assignment(ctx, assignment)
127+
}
110128
}
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-
})
119129
}
120130

121131
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-
)
132+
match state {
133+
UseNullishCoalescingState::LogicalOr { operator_range, .. } => Some(
134+
RuleDiagnostic::new(
135+
rule_category!(),
136+
*operator_range,
137+
markup! {
138+
"Use "<Emphasis>"??"</Emphasis>" instead of "<Emphasis>"||"</Emphasis>"."
139+
},
140+
)
141+
.note(markup! {
142+
"The "<Emphasis>"||"</Emphasis>" operator checks for all falsy values (including 0, '', and false), while "<Emphasis>"??"</Emphasis>" only checks for null and undefined."
143+
}),
144+
),
145+
UseNullishCoalescingState::LogicalOrAssignment { operator_range, .. } => Some(
146+
RuleDiagnostic::new(
147+
rule_category!(),
148+
*operator_range,
149+
markup! {
150+
"Use "<Emphasis>"??="</Emphasis>" instead of "<Emphasis>"||="</Emphasis>"."
151+
},
152+
)
153+
.note(markup! {
154+
"The "<Emphasis>"||="</Emphasis>" operator assigns when the left side is falsy, while "<Emphasis>"??="</Emphasis>" only assigns when it is null or undefined."
155+
}),
156+
),
157+
}
134158
}
135159

136160
fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
137-
if !state.can_fix {
161+
let (can_fix, replacement_token, message) = match state {
162+
UseNullishCoalescingState::LogicalOr { can_fix, .. } => (
163+
*can_fix,
164+
T![??],
165+
markup! { "Use "<Emphasis>"??"</Emphasis>" instead." }.to_owned(),
166+
),
167+
UseNullishCoalescingState::LogicalOrAssignment { can_fix, .. } => (
168+
*can_fix,
169+
T![??=],
170+
markup! { "Use "<Emphasis>"??="</Emphasis>" instead." }.to_owned(),
171+
),
172+
};
173+
174+
if !can_fix {
138175
return None;
139176
}
140177

141-
let node = ctx.query();
142-
let old_token = node.operator_token().ok()?;
178+
let old_token = match ctx.query() {
179+
UseNullishCoalescingQuery::JsLogicalExpression(node) => node.operator_token().ok()?,
180+
UseNullishCoalescingQuery::JsAssignmentExpression(node) => {
181+
node.operator_token().ok()?
182+
}
183+
};
143184

144-
let new_token = make::token(T![??])
185+
let new_token = make::token(replacement_token)
145186
.with_leading_trivia_pieces(old_token.leading_trivia().pieces())
146187
.with_trailing_trivia_pieces(old_token.trailing_trivia().pieces());
147188

@@ -151,12 +192,73 @@ impl Rule for UseNullishCoalescing {
151192
Some(JsRuleAction::new(
152193
ctx.metadata().action_category(ctx.category(), ctx.group()),
153194
ctx.metadata().applicability(),
154-
markup! { "Use "<Emphasis>"??"</Emphasis>" instead." }.to_owned(),
195+
message,
155196
mutation,
156197
))
157198
}
158199
}
159200

201+
fn run_logical_or(
202+
ctx: &RuleContext<UseNullishCoalescing>,
203+
logical: &JsLogicalExpression,
204+
) -> Option<UseNullishCoalescingState> {
205+
let operator = logical.operator().ok()?;
206+
if operator != JsLogicalOperator::LogicalOr {
207+
return None;
208+
}
209+
210+
let options = ctx.options();
211+
if options.ignore_conditional_tests() && is_in_test_position(logical) {
212+
return None;
213+
}
214+
215+
let left = logical.left().ok()?;
216+
let left_ty = ctx.type_of_expression(&left);
217+
218+
if !is_possibly_nullish(&left_ty) {
219+
return None;
220+
}
221+
222+
let can_fix =
223+
is_safe_type_for_replacement(&left_ty) && is_safe_syntax_context_for_replacement(logical);
224+
225+
Some(UseNullishCoalescingState::LogicalOr {
226+
operator_range: logical.operator_token().ok()?.text_trimmed_range(),
227+
can_fix,
228+
})
229+
}
230+
231+
fn run_logical_or_assignment(
232+
ctx: &RuleContext<UseNullishCoalescing>,
233+
assignment: &JsAssignmentExpression,
234+
) -> Option<UseNullishCoalescingState> {
235+
let operator = assignment.operator().ok()?;
236+
if operator != JsAssignmentOperator::LogicalOrAssign {
237+
return None;
238+
}
239+
240+
let left = assignment.left().ok()?;
241+
let left_ty = match &left {
242+
AnyJsAssignmentPattern::AnyJsAssignment(assign) => {
243+
let id = assign.as_js_identifier_assignment()?;
244+
let name = id.name_token().ok()?;
245+
ctx.type_of_named_value(assignment.range(), name.text_trimmed())
246+
}
247+
_ => return None,
248+
};
249+
250+
if !is_possibly_nullish(&left_ty) {
251+
return None;
252+
}
253+
254+
let can_fix = is_safe_type_for_replacement(&left_ty);
255+
256+
Some(UseNullishCoalescingState::LogicalOrAssignment {
257+
operator_range: assignment.operator_token().ok()?.text_trimmed_range(),
258+
can_fix,
259+
})
260+
}
261+
160262
fn is_safe_type_for_replacement(ty: &biome_js_type_info::Type) -> bool {
161263
if ty.is_union() {
162264
ty.flattened_union_variants().all(|variant| {

crates/biome_js_analyze/src/services/typed.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ impl TypedService {
2828
.unwrap_or_default()
2929
}
3030

31+
/// Returns the [`Type`] of the value with the given `name`, as defined
32+
/// in the scope that contains `range`.
33+
pub fn type_of_named_value(&self, range: TextRange, name: &str) -> Type {
34+
self.resolver
35+
.as_ref()
36+
.map(|resolver| resolver.resolved_type_of_named_value(range, name))
37+
.unwrap_or_default()
38+
}
39+
3140
/// Returns the [`Type`] for the given `function`.
3241
pub fn type_of_function(&self, function: &AnyJsFunction) -> Type {
3342
match function {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Comment trivia preservation for ||= to ??= replacement
2+
// Using types that are safe for replacement (only truthy or nullish)
3+
4+
declare let a: object | null;
5+
6+
// Inline comment before operator
7+
a /* before */ ||= {};
8+
9+
// Inline comment after operator
10+
a ||= /* after */ {};
11+
12+
// Both sides
13+
a /* before */ ||= /* after */ {};

0 commit comments

Comments
 (0)