Skip to content

Commit 8de2774

Browse files
dibashthapadibashthapa
authored
fix(lint): fixed the issue with false positive errors for noLeakedRender rule (#8306)
1 parent efa694c commit 8de2774

File tree

8 files changed

+172
-145
lines changed

8 files changed

+172
-145
lines changed

‎.changeset/red-buckets-sing.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@biomejs/biome': patch
3+
---
4+
5+
Fixed [#8288](https://github.com/biomejs/biome/issues/8288): Fixed the issue with false positive errors
6+
7+
This new change will ignore attribute and only show diagnostics for JSX Expressions
8+
9+
For example
10+
11+
Valid:
12+
13+
```jsx
14+
<Something checked={isOpen && items.length} />
15+
```
16+
17+
Invalid:
18+
```jsx
19+
const Component = () =>{
20+
return isOpen && items.length
21+
}
22+
```

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use biome_analyze::{
44
use biome_console::markup;
55
use biome_js_syntax::{
66
AnyJsExpression, JsConditionalExpression, JsLogicalExpression, JsLogicalOperator, JsSyntaxNode,
7-
JsxExpressionAttributeValue, JsxExpressionChild, JsxTagExpression,
8-
binding_ext::AnyJsBindingDeclaration,
7+
JsxExpressionChild, JsxTagExpression, binding_ext::AnyJsBindingDeclaration,
8+
jsx_ext::AnyJsxElement,
99
};
1010
use biome_rowan::{AstNode, declare_node_union};
1111
use biome_rule_options::no_leaked_render::NoLeakedRenderOptions;
@@ -106,7 +106,7 @@ impl Rule for NoLeakedRender {
106106
let query = ctx.query();
107107
let model = ctx.model();
108108

109-
if !is_inside_jsx_expression(query.syntax()) {
109+
if !is_inside_jsx_expression(query.syntax()).unwrap_or_default() {
110110
return None;
111111
}
112112

@@ -267,10 +267,12 @@ declare_node_union! {
267267
pub NoLeakedRenderQuery = JsLogicalExpression | JsConditionalExpression
268268
}
269269

270-
fn is_inside_jsx_expression(node: &JsSyntaxNode) -> bool {
271-
node.ancestors().any(|ancestor| {
272-
JsxExpressionChild::can_cast(ancestor.kind())
273-
|| JsxExpressionAttributeValue::can_cast(ancestor.kind())
274-
|| JsxTagExpression::can_cast(ancestor.kind())
275-
})
270+
fn is_inside_jsx_expression(node: &JsSyntaxNode) -> Option<bool> {
271+
let parent = node.parent()?;
272+
273+
Some(
274+
JsxExpressionChild::can_cast(parent.kind())
275+
|| JsxTagExpression::can_cast(parent.kind())
276+
|| AnyJsxElement::can_cast(parent.kind()),
277+
)
276278
}

‎crates/biome_js_analyze/tests/specs/nursery/noLeakedRender/invalid.jsx‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,28 +56,14 @@ const MyComponent3 = () => {
5656
return <div>{maybeObject && (isFoo ? <Aaa /> : <Bbb />)}</div>;
5757
};
5858

59-
const MyComponent4 = () => {
60-
return <Something checked={v ? false : isChecked} />;
61-
};
62-
63-
const MyComponent5 = () => {
64-
return <Something checked={cond && isIndeterminate ? false : isChecked} />;
65-
};
66-
67-
const isOpen1 = 0;
68-
const Component7 = () => {
69-
return <Popover open={isOpen1 && items.length > 0} />;
70-
};
71-
72-
const Component8 = ({ count, title }) => {
59+
const MyComponent4 = ({ count, title }) => {
7360
return <div>{(((((count))))) && ((title))}</div>;
7461
};
7562

76-
const Component9 = ({ data }) => {
63+
const MyComponent5 = ({ data }) => {
7764
return <div>{(((((data)))) && (((((data.value))))))}</div>;
78-
};
65+
}
7966

80-
const Component = ({ value }) => {
67+
const MyComponent6 = ({ value }) => {
8168
return <div>{(((value))) && <Item />}</div>;
8269
};
83-

‎crates/biome_js_analyze/tests/specs/nursery/noLeakedRender/invalid.jsx.snap‎

Lines changed: 13 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -62,32 +62,18 @@ const MyComponent3 = () => {
6262
return <div>{maybeObject && (isFoo ? <Aaa /> : <Bbb />)}</div>;
6363
};
6464
65-
const MyComponent4 = () => {
66-
return <Something checked={v ? false : isChecked} />;
67-
};
68-
69-
const MyComponent5 = () => {
70-
return <Something checked={cond && isIndeterminate ? false : isChecked} />;
71-
};
72-
73-
const isOpen1 = 0;
74-
const Component7 = () => {
75-
return <Popover open={isOpen1 && items.length > 0} />;
76-
};
77-
78-
const Component8 = ({ count, title }) => {
65+
const MyComponent4 = ({ count, title }) => {
7966
return <div>{(((((count))))) && ((title))}</div>;
8067
};
8168
82-
const Component9 = ({ data }) => {
69+
const MyComponent5 = ({ data }) => {
8370
return <div>{(((((data)))) && (((((data.value))))))}</div>;
84-
};
71+
}
8572
86-
const Component = ({ value }) => {
73+
const MyComponent6 = ({ value }) => {
8774
return <div>{(((value))) && <Item />}</div>;
8875
};
8976
90-
9177
```
9278

9379
# Diagnostics
@@ -318,107 +304,16 @@ invalid.jsx:56:15 lint/nursery/noLeakedRender ━━━━━━━━━━━
318304
```
319305

320306
```
321-
invalid.jsx:60:29 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
307+
invalid.jsx:60:15 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
322308
323309
i Potential leaked value that might cause unintended rendering.
324310
325-
59 │ const MyComponent4 = () => {
326-
> 60 │ return <Something checked={v ? false : isChecked} />;
327-
│ ^^^^^^^^^^^^^^^^^^^^^
311+
59 │ const MyComponent4 = ({ count, title }) => {
312+
> 60 │ return <div>{(((((count))))) && ((title))}</div>;
313+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
328314
61 │ };
329315
62 │
330316
331-
i This happens when you use ternary operators in JSX with alternate values that could be variables.
332-
333-
i Replace with a safe alternate value like an empty string , null or another JSX element.
334-
335-
336-
```
337-
338-
```
339-
invalid.jsx:64:29 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
340-
341-
i Potential leaked value that might cause unintended rendering.
342-
343-
63 │ const MyComponent5 = () => {
344-
> 64 │ return <Something checked={cond && isIndeterminate ? false : isChecked} />;
345-
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
346-
65 │ };
347-
66 │
348-
349-
i This happens when you use ternary operators in JSX with alternate values that could be variables.
350-
351-
i Replace with a safe alternate value like an empty string , null or another JSX element.
352-
353-
354-
```
355-
356-
```
357-
invalid.jsx:64:29 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
358-
359-
i Potential leaked value that might cause unintended rendering.
360-
361-
63 │ const MyComponent5 = () => {
362-
> 64 │ return <Something checked={cond && isIndeterminate ? false : isChecked} />;
363-
│ ^^^^^^^^^^^^^^^^^^^^^^^
364-
65 │ };
365-
66 │
366-
367-
i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.
368-
369-
i Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression.
370-
371-
372-
```
373-
374-
```
375-
invalid.jsx:69:24 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
376-
377-
i Potential leaked value that might cause unintended rendering.
378-
379-
67 │ const isOpen1 = 0;
380-
68 │ const Component7 = () => {
381-
> 69 │ return <Popover open={isOpen1 && items.length > 0} />;
382-
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
383-
70 │ };
384-
71 │
385-
386-
i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.
387-
388-
i Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression.
389-
390-
391-
```
392-
393-
```
394-
invalid.jsx:73:15 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
395-
396-
i Potential leaked value that might cause unintended rendering.
397-
398-
72 │ const Component8 = ({ count, title }) => {
399-
> 73 │ return <div>{(((((count))))) && ((title))}</div>;
400-
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
401-
74 │ };
402-
75 │
403-
404-
i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.
405-
406-
i Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression.
407-
408-
409-
```
410-
411-
```
412-
invalid.jsx:77:16 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
413-
414-
i Potential leaked value that might cause unintended rendering.
415-
416-
76 │ const Component9 = ({ data }) => {
417-
> 77 │ return <div>{(((((data)))) && (((((data.value))))))}</div>;
418-
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
419-
78 │ };
420-
79 │
421-
422317
i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.
423318
424319
i Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression.
@@ -427,15 +322,15 @@ invalid.jsx:77:16 lint/nursery/noLeakedRender ━━━━━━━━━━━
427322
```
428323

429324
```
430-
invalid.jsx:81:15 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
325+
invalid.jsx:68:15 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
431326
432327
i Potential leaked value that might cause unintended rendering.
433328
434-
80 │ const Component = ({ value }) => {
435-
> 81 │ return <div>{(((value))) && <Item />}</div>;
329+
67 │ const MyComponent6 = ({ value }) => {
330+
> 68 │ return <div>{(((value))) && <Item />}</div>;
436331
│ ^^^^^^^^^^^^^^^^^^^^^^^
437-
82 │ };
438-
83 │
332+
69 │ };
333+
70 │
439334
440335
i JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output.
441336

‎crates/biome_js_analyze/tests/specs/nursery/noLeakedRender/issue8288.invalid.jsx‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/* should generate diagnostics */
2+
const Component1 = () => {
3+
return (
4+
<Nested>
5+
<SecondNested>{userId ? 1 : undefined} </SecondNested>
6+
</Nested>
7+
);
8+
};

‎crates/biome_js_analyze/tests/specs/nursery/noLeakedRender/issue8288.invalid.jsx.snap‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: issue8288.invalid.jsx
4+
---
5+
# Input
6+
```jsx
7+
/* should generate diagnostics */
8+
const Component1 = () => {
9+
return (
10+
<Nested>
11+
<SecondNested>{userId ? 1 : undefined} </SecondNested>
12+
</Nested>
13+
);
14+
};
15+
16+
```
17+
18+
# Diagnostics
19+
```
20+
issue8288.invalid.jsx:5:19 lint/nursery/noLeakedRender ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
21+
22+
i Potential leaked value that might cause unintended rendering.
23+
24+
3 │ return (
25+
4 │ <Nested>
26+
> 5 │ <SecondNested>{userId ? 1 : undefined} </SecondNested>
27+
│ ^^^^^^^^^^^^^^^^^^^^^^
28+
6 │ </Nested>
29+
7 │ );
30+
31+
i This happens when you use ternary operators in JSX with alternate values that could be variables.
32+
33+
i Replace with a safe alternate value like an empty string , null or another JSX element.
34+
35+
36+
```

‎crates/biome_js_analyze/tests/specs/nursery/noLeakedRender/issue8288.valid.jsx‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/* should not generate diagnostics */
2+
function getValue() {
3+
return Math.random() > 0.5;
4+
}
5+
6+
export default function MyComponent() {
7+
return <button onClick={getValue() ? getValue : undefined} />;
8+
}
9+
10+
const Component1 = () => {
11+
return isPending ? loader : userResults;
12+
};
13+
14+
const Component2 = () => {
15+
return someRandomId && !someRandomBooleanValue ? <ContentHere /> : <NotFound />;
16+
};
17+
18+
// Ignore Attribute
19+
const Component3 = () => {
20+
return (
21+
<NoErrorWithAttribute
22+
error={firstBool && secondBool}
23+
coerceError={!!(firstBool && secondBool)}
24+
orderId={isChecked ? orderId : undefined}
25+
/>
26+
);
27+
};
28+
29+
const Component4 = () => {
30+
return (
31+
<Nested>
32+
<SecondNested>{userId ? 1 : null} </SecondNested>
33+
</Nested>
34+
);
35+
};

‎crates/biome_js_analyze/tests/specs/nursery/noLeakedRender/issue8288.valid.jsx.snap‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: issue8288.valid.jsx
4+
---
5+
# Input
6+
```jsx
7+
/* should not generate diagnostics */
8+
function getValue() {
9+
return Math.random() > 0.5;
10+
}
11+
12+
export default function MyComponent() {
13+
return <button onClick={getValue() ? getValue : undefined} />;
14+
}
15+
16+
const Component1 = () => {
17+
return isPending ? loader : userResults;
18+
};
19+
20+
const Component2 = () => {
21+
return someRandomId && !someRandomBooleanValue ? <ContentHere /> : <NotFound />;
22+
};
23+
24+
// Ignore Attribute
25+
const Component3 = () => {
26+
return (
27+
<NoErrorWithAttribute
28+
error={firstBool && secondBool}
29+
coerceError={!!(firstBool && secondBool)}
30+
orderId={isChecked ? orderId : undefined}
31+
/>
32+
);
33+
};
34+
35+
const Component4 = () => {
36+
return (
37+
<Nested>
38+
<SecondNested>{userId ? 1 : null} </SecondNested>
39+
</Nested>
40+
);
41+
};
42+
43+
```

0 commit comments

Comments
 (0)