Skip to content

Commit 2b41e82

Browse files
matanshavitdyc3
andauthored
fix(noNestedTernary): detect nested ternaries wrapped in parentheses (#8086)
Co-authored-by: Carson McManus <[email protected]>
1 parent 7983940 commit 2b41e82

File tree

6 files changed

+232
-15
lines changed

6 files changed

+232
-15
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Fixed [#8045](https://github.com/biomejs/biome/issues/8045): The [`noNestedTernary`](https://biomejs.dev/linter/rules/no-nested-ternary/) rule now correctly detects nested ternary expressions even when they are wrapped in parentheses (e.g. `foo ? (bar ? 1 : 2) : 3`).
6+
7+
Previously, the rule would not flag nested ternaries like `foo ? (bar ? 1 : 2) : 3` because the parentheses prevented detection. The rule now looks through parentheses to identify nested conditionals.
8+
9+
**Previously not detected (now flagged):**
10+
11+
```js
12+
const result = foo ? (bar ? 1 : 2) : 3;
13+
```
14+
15+
**Still valid (non-nested with parentheses):**
16+
17+
```js
18+
const result = (foo ? bar : baz);
19+
```

crates/biome_js_analyze/src/lint/style/no_nested_ternary.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,11 @@ impl Rule for NoNestedTernary {
6363
let alternate = node.alternate().ok()?;
6464
let consequent = node.consequent().ok()?;
6565

66-
if let AnyJsExpression::JsConditionalExpression(expr) = consequent {
66+
if let AnyJsExpression::JsConditionalExpression(expr) = consequent.omit_parentheses() {
6767
return Some(expr.range());
6868
}
6969

70-
if let AnyJsExpression::JsConditionalExpression(expr) = alternate {
70+
if let AnyJsExpression::JsConditionalExpression(expr) = alternate.omit_parentheses() {
7171
return Some(expr.range());
7272
}
7373

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
11
var thing = foo ? bar : baz === qux ? quxx : foobar;
22

3-
foo ? baz === qux ? quxx() : foobar() : bar();
3+
foo ? baz === qux ? quxx() : foobar() : bar();
4+
5+
var thing1 = foo ? (bar ? 1 : 2) : 3;
6+
7+
var thing2 = foo ? 1 : (bar ? 2 : 3);
8+
9+
var thing3 = foo ? ((bar ? 1 : 2)) : 3;
10+
11+
var thing4 = foo ? 1 : ((bar ? 2 : 3));
12+
13+
var thing5 = foo ? (baz === qux ? quxx : foobar) : bar;
14+
15+
const case1 = val ? (Math.random() ? 1 : 2) : 3;
16+
17+
const case2 = val ? 1 : Math.random() ? 2 : 3;
18+
19+
const case3 = (val ? (Math.random() ? 1 : 2) : 3);

crates/biome_js_analyze/tests/specs/style/noNestedTernary/invalid.js.snap

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,23 @@ expression: invalid.js
77
var thing = foo ? bar : baz === qux ? quxx : foobar;
88
99
foo ? baz === qux ? quxx() : foobar() : bar();
10+
11+
var thing1 = foo ? (bar ? 1 : 2) : 3;
12+
13+
var thing2 = foo ? 1 : (bar ? 2 : 3);
14+
15+
var thing3 = foo ? ((bar ? 1 : 2)) : 3;
16+
17+
var thing4 = foo ? 1 : ((bar ? 2 : 3));
18+
19+
var thing5 = foo ? (baz === qux ? quxx : foobar) : bar;
20+
21+
const case1 = val ? (Math.random() ? 1 : 2) : 3;
22+
23+
const case2 = val ? 1 : Math.random() ? 2 : 3;
24+
25+
const case3 = (val ? (Math.random() ? 1 : 2) : 3);
26+
1027
```
1128

1229
# Diagnostics
@@ -36,6 +53,159 @@ invalid.js:3:7 lint/style/noNestedTernary ━━━━━━━━━━━━
3653
2 │
3754
> 3 │ foo ? baz === qux ? quxx() : foobar() : bar();
3855
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
56+
4 │
57+
5 │ var thing1 = foo ? (bar ? 1 : 2) : 3;
58+
59+
i Nesting ternary expressions can make code more difficult to understand.
60+
61+
i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.
62+
63+
64+
```
65+
66+
```
67+
invalid.js:5:21 lint/style/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
68+
69+
i Do not nest ternary expressions.
70+
71+
3 │ foo ? baz === qux ? quxx() : foobar() : bar();
72+
4 │
73+
> 5 │ var thing1 = foo ? (bar ? 1 : 2) : 3;
74+
│ ^^^^^^^^^^^
75+
6 │
76+
7 │ var thing2 = foo ? 1 : (bar ? 2 : 3);
77+
78+
i Nesting ternary expressions can make code more difficult to understand.
79+
80+
i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.
81+
82+
83+
```
84+
85+
```
86+
invalid.js:7:25 lint/style/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
87+
88+
i Do not nest ternary expressions.
89+
90+
5 │ var thing1 = foo ? (bar ? 1 : 2) : 3;
91+
6 │
92+
> 7 │ var thing2 = foo ? 1 : (bar ? 2 : 3);
93+
│ ^^^^^^^^^^^
94+
8 │
95+
9 │ var thing3 = foo ? ((bar ? 1 : 2)) : 3;
96+
97+
i Nesting ternary expressions can make code more difficult to understand.
98+
99+
i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.
100+
101+
102+
```
103+
104+
```
105+
invalid.js:9:22 lint/style/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
106+
107+
i Do not nest ternary expressions.
108+
109+
7 │ var thing2 = foo ? 1 : (bar ? 2 : 3);
110+
8 │
111+
> 9 │ var thing3 = foo ? ((bar ? 1 : 2)) : 3;
112+
│ ^^^^^^^^^^^
113+
10 │
114+
11 │ var thing4 = foo ? 1 : ((bar ? 2 : 3));
115+
116+
i Nesting ternary expressions can make code more difficult to understand.
117+
118+
i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.
119+
120+
121+
```
122+
123+
```
124+
invalid.js:11:26 lint/style/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
125+
126+
i Do not nest ternary expressions.
127+
128+
9 │ var thing3 = foo ? ((bar ? 1 : 2)) : 3;
129+
10 │
130+
> 11 │ var thing4 = foo ? 1 : ((bar ? 2 : 3));
131+
│ ^^^^^^^^^^^
132+
12 │
133+
13 │ var thing5 = foo ? (baz === qux ? quxx : foobar) : bar;
134+
135+
i Nesting ternary expressions can make code more difficult to understand.
136+
137+
i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.
138+
139+
140+
```
141+
142+
```
143+
invalid.js:13:21 lint/style/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
144+
145+
i Do not nest ternary expressions.
146+
147+
11 │ var thing4 = foo ? 1 : ((bar ? 2 : 3));
148+
12 │
149+
> 13 │ var thing5 = foo ? (baz === qux ? quxx : foobar) : bar;
150+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
151+
14 │
152+
15 │ const case1 = val ? (Math.random() ? 1 : 2) : 3;
153+
154+
i Nesting ternary expressions can make code more difficult to understand.
155+
156+
i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.
157+
158+
159+
```
160+
161+
```
162+
invalid.js:15:22 lint/style/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
163+
164+
i Do not nest ternary expressions.
165+
166+
13 │ var thing5 = foo ? (baz === qux ? quxx : foobar) : bar;
167+
14 │
168+
> 15 │ const case1 = val ? (Math.random() ? 1 : 2) : 3;
169+
│ ^^^^^^^^^^^^^^^^^^^^^
170+
16 │
171+
17 │ const case2 = val ? 1 : Math.random() ? 2 : 3;
172+
173+
i Nesting ternary expressions can make code more difficult to understand.
174+
175+
i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.
176+
177+
178+
```
179+
180+
```
181+
invalid.js:17:25 lint/style/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
182+
183+
i Do not nest ternary expressions.
184+
185+
15 │ const case1 = val ? (Math.random() ? 1 : 2) : 3;
186+
16 │
187+
> 17 │ const case2 = val ? 1 : Math.random() ? 2 : 3;
188+
│ ^^^^^^^^^^^^^^^^^^^^^
189+
18 │
190+
19 │ const case3 = (val ? (Math.random() ? 1 : 2) : 3);
191+
192+
i Nesting ternary expressions can make code more difficult to understand.
193+
194+
i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.
195+
196+
197+
```
198+
199+
```
200+
invalid.js:19:23 lint/style/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
201+
202+
i Do not nest ternary expressions.
203+
204+
17 │ const case2 = val ? 1 : Math.random() ? 2 : 3;
205+
18 │
206+
> 19 │ const case3 = (val ? (Math.random() ? 1 : 2) : 3);
207+
│ ^^^^^^^^^^^^^^^^^^^^^
208+
20 │
39209
40210
i Nesting ternary expressions can make code more difficult to understand.
41211
Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
/* should not generate diagnostics */
2-
const thing = foo ? bar : foobar;
2+
const thing1 = foo ? bar : foobar;
33

4-
let thing;
4+
let thing2;
55

66
if (foo) {
7-
thing = bar;
7+
thing2 = bar;
88
} else if (baz === qux) {
9-
thing = quxx;
9+
thing2 = quxx;
1010
} else {
11-
thing = foobar;
12-
}
11+
thing2 = foobar;
12+
}
13+
14+
const thing3 = (foo ? bar : baz);
15+
16+
const thing4 = foo ? (bar) : (baz);
17+
18+
const thing5 = ((foo ? bar : baz));
Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,26 @@
11
---
22
source: crates/biome_js_analyze/tests/spec_tests.rs
33
expression: valid.js
4-
snapshot_kind: text
54
---
65
# Input
76
```js
87
/* should not generate diagnostics */
9-
const thing = foo ? bar : foobar;
8+
const thing1 = foo ? bar : foobar;
109
11-
let thing;
10+
let thing2;
1211
1312
if (foo) {
14-
thing = bar;
13+
thing2 = bar;
1514
} else if (baz === qux) {
16-
thing = quxx;
15+
thing2 = quxx;
1716
} else {
18-
thing = foobar;
17+
thing2 = foobar;
1918
}
19+
20+
const thing3 = (foo ? bar : baz);
21+
22+
const thing4 = foo ? (bar) : (baz);
23+
24+
const thing5 = ((foo ? bar : baz));
25+
2026
```

0 commit comments

Comments
 (0)