Skip to content

Commit 1948b72

Browse files
sjh9714sjh9714
authored
fix(lint): detect optional-chain inequality guards (#10425)
1 parent 0c718da commit 1948b72

6 files changed

Lines changed: 221 additions & 5 deletions

File tree

‎.changeset/fluffy-papayas-obey.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+
Fixed [#10244](https://github.com/biomejs/biome/issues/10244): The `useOptionalChain` rule now detects negated guard inequality chains like `!foo || foo.bar !== "x"`.

‎crates/biome_js_analyze/src/lint/complexity/use_optional_chain.rs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,13 @@ impl Rule for UseOptionalChain {
133133
))
134134
}
135135
JsLogicalOperator::NullishCoalescing | JsLogicalOperator::LogicalOr => {
136+
if matches!(operator, JsLogicalOperator::LogicalOr)
137+
&& let Some(chain_nodes) =
138+
optional_chain_nodes_from_negated_or_inequality(logical, model)
139+
{
140+
return Some(UseOptionalChainState::LogicalAnd(chain_nodes));
141+
}
142+
136143
// Check for negated || chains like `!foo || !foo.bar`
137144
if matches!(operator, JsLogicalOperator::LogicalOr) && is_negated_or_chain(logical)
138145
{
@@ -534,6 +541,62 @@ fn is_negated_or_chain(logical: &JsLogicalExpression) -> bool {
534541
false
535542
}
536543

544+
/// Match `!foo || foo.bar !== "value"` and the loose `!=` variant.
545+
///
546+
/// This intentionally stays narrower than general binary comparisons:
547+
/// optional chaining preserves the guard only when the compared value is static
548+
/// and non-nullish, so comparing `undefined` to that value remains truthy.
549+
fn optional_chain_nodes_from_negated_or_inequality(
550+
logical: &JsLogicalExpression,
551+
model: &SemanticModel,
552+
) -> Option<LogicalAndChainNodes> {
553+
let guard = strip_negation(&logical.left().ok()?)?;
554+
let binary = logical.right().ok()?.as_js_binary_expression()?.clone();
555+
if !matches!(
556+
binary.operator().ok()?,
557+
JsBinaryOperator::StrictInequality | JsBinaryOperator::Inequality
558+
) {
559+
return None;
560+
}
561+
562+
let left = binary.left().ok()?;
563+
let right = binary.right().ok()?;
564+
let compared_chain = if is_static_non_nullish_value(&right) {
565+
left
566+
} else if is_static_non_nullish_value(&left) {
567+
right
568+
} else {
569+
return None;
570+
};
571+
572+
let mut chain = LogicalAndChain::from_expression(compared_chain).ok()?;
573+
let guard_chain =
574+
LogicalAndChain::from_expression(normalized_optional_chain_like(guard, model).ok()?)
575+
.ok()?;
576+
if !matches!(
577+
chain.cmp_chain(&guard_chain).ok()?,
578+
LogicalAndChainOrdering::SubChain
579+
) {
580+
return None;
581+
}
582+
583+
let mut tail = chain.buf.split_off(guard_chain.buf.len());
584+
let optional_node = tail.pop_front()?;
585+
Some(LogicalAndChainNodes {
586+
nodes: VecDeque::from([optional_node]),
587+
prefix: None,
588+
negated: false,
589+
})
590+
}
591+
592+
fn is_static_non_nullish_value(expression: &AnyJsExpression) -> bool {
593+
expression
594+
.clone()
595+
.omit_parentheses()
596+
.as_static_value()
597+
.is_some_and(|value| !value.is_null_or_undefined())
598+
}
599+
537600
/// `LogicalAndChainOrdering` is the result of a comparison between two logical
538601
/// AND chains.
539602
enum LogicalAndChainOrdering {

‎crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidNegatedOrChain.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,7 @@
1010
!a.b || !a.b();
1111
(!foo || !foo.bar) && (!baz || !baz.bar);
1212
!foo || !foo?.bar.baz;
13+
!foo || foo.bar !== "x";
14+
!foo || foo.bar != "x";
15+
!foo || "x" !== foo.bar;
16+
!foo || "x" != foo.bar;

‎crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidNegatedOrChain.js.snap‎

Lines changed: 105 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
---
22
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
assertion_line: 149
34
expression: invalidNegatedOrChain.js
45
---
56
# Input
@@ -16,6 +17,10 @@ expression: invalidNegatedOrChain.js
1617
!a.b || !a.b();
1718
(!foo || !foo.bar) && (!baz || !baz.bar);
1819
!foo || !foo?.bar.baz;
20+
!foo || foo.bar !== "x";
21+
!foo || foo.bar != "x";
22+
!foo || "x" !== foo.bar;
23+
!foo || "x" != foo.bar;
1924
2025
```
2126

@@ -221,7 +226,7 @@ invalidNegatedOrChain.js:11:2 lint/complexity/useOptionalChain FIXABLE ━━
221226
> 11 │ (!foo || !foo.bar) && (!baz || !baz.bar);
222227
│ ^^^^^^^^^^^^^^^^
223228
12 │ !foo || !foo?.bar.baz;
224-
13 │
229+
13 │ !foo || foo.bar !== "x";
225230
226231
i Unsafe fix: Change to an optional chain.
227232
@@ -230,7 +235,7 @@ invalidNegatedOrChain.js:11:2 lint/complexity/useOptionalChain FIXABLE ━━
230235
11 │ - (!foo·||·!foo.bar)·&&·(!baz·||·!baz.bar);
231236
11 │ + (!foo?.bar)·&&·(!baz·||·!baz.bar);
232237
12 12 │ !foo || !foo?.bar.baz;
233-
13 13 │
238+
13 13 │ !foo || foo.bar !== "x";
234239
235240
236241
```
@@ -245,7 +250,7 @@ invalidNegatedOrChain.js:11:24 lint/complexity/useOptionalChain FIXABLE ━━
245250
> 11 │ (!foo || !foo.bar) && (!baz || !baz.bar);
246251
│ ^^^^^^^^^^^^^^^^
247252
12 │ !foo || !foo?.bar.baz;
248-
13 │
253+
13 │ !foo || foo.bar !== "x";
249254
250255
i Unsafe fix: Change to an optional chain.
251256
@@ -254,7 +259,7 @@ invalidNegatedOrChain.js:11:24 lint/complexity/useOptionalChain FIXABLE ━━
254259
11 │ - (!foo·||·!foo.bar)·&&·(!baz·||·!baz.bar);
255260
11 │ + (!foo·||·!foo.bar)·&&·(!baz?.bar);
256261
12 12 │ !foo || !foo?.bar.baz;
257-
13 13 │
262+
13 13 │ !foo || foo.bar !== "x";
258263
259264
260265
```
@@ -268,11 +273,106 @@ invalidNegatedOrChain.js:12:1 lint/complexity/useOptionalChain FIXABLE ━━
268273
11 │ (!foo || !foo.bar) && (!baz || !baz.bar);
269274
> 12 │ !foo || !foo?.bar.baz;
270275
│ ^^^^^^^^^^^^^^^^^^^^^
271-
13 │
276+
13 │ !foo || foo.bar !== "x";
277+
14 │ !foo || foo.bar != "x";
272278
273279
i Unsafe fix: Change to an optional chain.
274280
275281
12 │ !foo·||·!foo?.bar.baz;
276282
│ --------
277283
278284
```
285+
286+
```
287+
invalidNegatedOrChain.js:13:1 lint/complexity/useOptionalChain FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━
288+
289+
! Change to an optional chain.
290+
291+
11 │ (!foo || !foo.bar) && (!baz || !baz.bar);
292+
12 │ !foo || !foo?.bar.baz;
293+
> 13 │ !foo || foo.bar !== "x";
294+
│ ^^^^^^^^^^^^^^^^^^^^^^^
295+
14 │ !foo || foo.bar != "x";
296+
15 │ !foo || "x" !== foo.bar;
297+
298+
i Unsafe fix: Change to an optional chain.
299+
300+
11 11 │ (!foo || !foo.bar) && (!baz || !baz.bar);
301+
12 12 │ !foo || !foo?.bar.baz;
302+
13 │ - !foo·||·foo.bar·!==·"x";
303+
13 │ + foo?.bar·!==·"x";
304+
14 14 │ !foo || foo.bar != "x";
305+
15 15 │ !foo || "x" !== foo.bar;
306+
307+
308+
```
309+
310+
```
311+
invalidNegatedOrChain.js:14:1 lint/complexity/useOptionalChain FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━
312+
313+
! Change to an optional chain.
314+
315+
12 │ !foo || !foo?.bar.baz;
316+
13 │ !foo || foo.bar !== "x";
317+
> 14 │ !foo || foo.bar != "x";
318+
│ ^^^^^^^^^^^^^^^^^^^^^^
319+
15 │ !foo || "x" !== foo.bar;
320+
16 │ !foo || "x" != foo.bar;
321+
322+
i Unsafe fix: Change to an optional chain.
323+
324+
12 12 │ !foo || !foo?.bar.baz;
325+
13 13 │ !foo || foo.bar !== "x";
326+
14 │ - !foo·||·foo.bar·!=·"x";
327+
14 │ + foo?.bar·!=·"x";
328+
15 15 │ !foo || "x" !== foo.bar;
329+
16 16 │ !foo || "x" != foo.bar;
330+
331+
332+
```
333+
334+
```
335+
invalidNegatedOrChain.js:15:1 lint/complexity/useOptionalChain FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━
336+
337+
! Change to an optional chain.
338+
339+
13 │ !foo || foo.bar !== "x";
340+
14 │ !foo || foo.bar != "x";
341+
> 15 │ !foo || "x" !== foo.bar;
342+
│ ^^^^^^^^^^^^^^^^^^^^^^^
343+
16 │ !foo || "x" != foo.bar;
344+
17 │
345+
346+
i Unsafe fix: Change to an optional chain.
347+
348+
13 13 │ !foo || foo.bar !== "x";
349+
14 14 │ !foo || foo.bar != "x";
350+
15 │ - !foo·||·"x"·!==·foo.bar;
351+
15 │ + "x"·!==·foo?.bar;
352+
16 16 │ !foo || "x" != foo.bar;
353+
17 17 │
354+
355+
356+
```
357+
358+
```
359+
invalidNegatedOrChain.js:16:1 lint/complexity/useOptionalChain FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━
360+
361+
! Change to an optional chain.
362+
363+
14 │ !foo || foo.bar != "x";
364+
15 │ !foo || "x" !== foo.bar;
365+
> 16 │ !foo || "x" != foo.bar;
366+
│ ^^^^^^^^^^^^^^^^^^^^^^
367+
17 │
368+
369+
i Unsafe fix: Change to an optional chain.
370+
371+
14 14 │ !foo || foo.bar != "x";
372+
15 15 │ !foo || "x" !== foo.bar;
373+
16 │ - !foo·||·"x"·!=·foo.bar;
374+
16 │ + "x"·!=·foo?.bar;
375+
17 17 │
376+
377+
378+
```

‎crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/validNegatedOrChain.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,25 @@ a || !a.b;
1313

1414
// Mixed operators
1515
!a || !a.b && !a.b.c;
16+
17+
// Equality comparisons are not equivalent when the base is nullish
18+
!foo || foo.bar === "x";
19+
!foo || foo.bar == "x";
20+
21+
// Ordering comparisons are not equivalent when the base is nullish
22+
!foo || foo.bar > 0;
23+
!foo || foo.bar <= 0;
24+
25+
// Dynamic comparison values are outside the supported static-safe shape
26+
!foo || foo.bar !== baz;
27+
28+
// Nullish comparison values are outside the supported static-safe shape
29+
!foo || foo.bar !== undefined;
30+
!foo || foo.bar !== null;
31+
!foo || foo.bar != null;
32+
33+
// Mismatched chains are not equivalent
34+
!foo || bar.baz !== "x";
35+
36+
// The guard must be a negated base
37+
foo || foo.bar !== "x";

‎crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/validNegatedOrChain.js.snap‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,26 @@ a || !a.b;
2020
// Mixed operators
2121
!a || !a.b && !a.b.c;
2222
23+
// Equality comparisons are not equivalent when the base is nullish
24+
!foo || foo.bar === "x";
25+
!foo || foo.bar == "x";
26+
27+
// Ordering comparisons are not equivalent when the base is nullish
28+
!foo || foo.bar > 0;
29+
!foo || foo.bar <= 0;
30+
31+
// Dynamic comparison values are outside the supported static-safe shape
32+
!foo || foo.bar !== baz;
33+
34+
// Nullish comparison values are outside the supported static-safe shape
35+
!foo || foo.bar !== undefined;
36+
!foo || foo.bar !== null;
37+
!foo || foo.bar != null;
38+
39+
// Mismatched chains are not equivalent
40+
!foo || bar.baz !== "x";
41+
42+
// The guard must be a negated base
43+
foo || foo.bar !== "x";
44+
2345
```

0 commit comments

Comments
 (0)