feat(lint): added new rule no-leaked-render from eslint-react
#8171
+1,146
−85
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| --- | ||
| '@biomejs/biome': patch | ||
| --- | ||
|
|
||
| Added the new rule [`noLeakedRender`](https://biomejs.dev/linter/rules/no-leaked-render). This rule helps prevent potential leaks when rendering components that use binary expressions or ternaries. | ||
|
|
||
| For example, the following code triggers the rule because the component would render `0`: | ||
|
|
||
| ```jsx | ||
| const Component = () => { | ||
| const count = 0; | ||
| return <div>{count && <span>Count: {count}</span>}</div>; | ||
| } | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,276 @@ | ||||||
| use biome_analyze::{ | ||||||
| Rule, RuleDiagnostic, RuleDomain, RuleSource, context::RuleContext, declare_lint_rule, | ||||||
| }; | ||||||
| use biome_console::markup; | ||||||
| use biome_js_syntax::{ | ||||||
| AnyJsExpression, JsConditionalExpression, JsLogicalExpression, JsLogicalOperator, JsSyntaxNode, | ||||||
| JsxExpressionAttributeValue, JsxExpressionChild, JsxTagExpression, | ||||||
| binding_ext::AnyJsBindingDeclaration, | ||||||
| }; | ||||||
| use biome_rowan::{AstNode, declare_node_union}; | ||||||
| use biome_rule_options::no_leaked_render::NoLeakedRenderOptions; | ||||||
|
|
||||||
| use crate::services::semantic::Semantic; | ||||||
|
|
||||||
| declare_lint_rule! { | ||||||
| /// Prevent problematic leaked values from being rendered. | ||||||
| /// | ||||||
| /// This rule prevents values that might cause unintentionally rendered values | ||||||
| /// or rendering crashes in React JSX. When using conditional rendering with the | ||||||
| /// logical AND operator (`&&`), if the left-hand side evaluates to a falsy value like | ||||||
| /// `0`, `NaN`, or any empty string, these values will be rendered instead of rendering nothing. | ||||||
| /// | ||||||
| /// | ||||||
| /// ## Examples | ||||||
| /// | ||||||
| /// ### Invalid | ||||||
| /// | ||||||
| /// ```jsx,expect_diagnostic | ||||||
| /// const Component = () => { | ||||||
| /// const count = 0; | ||||||
| /// return <div>{count && <span>Count: {count}</span>}</div>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// ```jsx,expect_diagnostic | ||||||
| /// const Component = () => { | ||||||
| /// const items = []; | ||||||
| /// return <div>{items.length && <List items={items} />}</div>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// ```jsx,expect_diagnostic | ||||||
| /// const Component = () => { | ||||||
| /// const user = null; | ||||||
| /// return <div>{user && <Profile user={user} />}</div>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// | ||||||
| /// ### Valid | ||||||
| /// | ||||||
| /// ```jsx | ||||||
| /// const Component = () => { | ||||||
| /// const count = 0; | ||||||
| /// return <div>{count > 0 && <span>Count: {count}</span>}</div>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// ```jsx | ||||||
| /// const Component = () => { | ||||||
| /// const items = []; | ||||||
| /// return <div>{!!items.length && <List items={items} />}</div>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// ```jsx | ||||||
| /// const Component = () => { | ||||||
| /// const user = null; | ||||||
| /// return <div>{user ? <Profile user={user} /> : null}</div>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// ```jsx | ||||||
| /// const Component = () => { | ||||||
| /// const condition = false; | ||||||
| /// return <div>{condition ? <Content /> : <Fallback />}</div>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// ```jsx | ||||||
| /// const Component = () => { | ||||||
| /// const isReady = true; | ||||||
| /// return <div>{isReady && <Content />}</div>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| pub NoLeakedRender{ | ||||||
| version: "next", | ||||||
| name: "noLeakedRender", | ||||||
| language: "jsx", | ||||||
| domains: &[RuleDomain::React], | ||||||
| sources: &[ | ||||||
| RuleSource::EslintReact("no-leaked-render").inspired(), | ||||||
| ], | ||||||
| recommended: false, | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| impl Rule for NoLeakedRender { | ||||||
| type Query = Semantic<NoLeakedRenderQuery>; | ||||||
| type State = bool; | ||||||
| type Signals = Option<Self::State>; | ||||||
| type Options = NoLeakedRenderOptions; | ||||||
|
|
||||||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||||||
| let query = ctx.query(); | ||||||
| let model = ctx.model(); | ||||||
|
|
||||||
| if !is_inside_jsx_expression(query.syntax()) { | ||||||
| return None; | ||||||
| } | ||||||
|
|
||||||
| match query { | ||||||
| NoLeakedRenderQuery::JsLogicalExpression(exp) => { | ||||||
| let op = exp.operator().ok()?; | ||||||
|
|
||||||
| if op != JsLogicalOperator::LogicalAnd { | ||||||
| return None; | ||||||
| } | ||||||
| let left = exp.left().ok()?; | ||||||
|
|
||||||
| let is_left_hand_side_safe = matches!( | ||||||
| left, | ||||||
| AnyJsExpression::JsUnaryExpression(_) | ||||||
| | AnyJsExpression::JsCallExpression(_) | ||||||
| | AnyJsExpression::JsBinaryExpression(_) | ||||||
| ); | ||||||
|
|
||||||
| if is_left_hand_side_safe { | ||||||
| return None; | ||||||
| } | ||||||
|
|
||||||
| let mut is_nested_left_hand_side_safe = false; | ||||||
|
|
||||||
| let mut stack = vec![left.clone()]; | ||||||
|
|
||||||
| // Traverse the expression tree iteratively using a stack | ||||||
| // This allows us to check nested expressions without recursion | ||||||
| while let Some(current) = stack.pop() { | ||||||
| match current { | ||||||
| AnyJsExpression::JsLogicalExpression(expr) => { | ||||||
| let left = expr.left().ok()?; | ||||||
| let right = expr.right().ok()?; | ||||||
| stack.push(left); | ||||||
| stack.push(right); | ||||||
| } | ||||||
| AnyJsExpression::JsParenthesizedExpression(expr) => { | ||||||
| stack.push(expr.expression().ok()?); | ||||||
| } | ||||||
| // If we find expressions that coerce to boolean (unary, call, binary), | ||||||
| // then the entire expression is considered safe | ||||||
| AnyJsExpression::JsUnaryExpression(_) | ||||||
| | AnyJsExpression::JsCallExpression(_) | ||||||
| | AnyJsExpression::JsBinaryExpression(_) => { | ||||||
| is_nested_left_hand_side_safe = true; | ||||||
| break; | ||||||
| } | ||||||
| _ => {} | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if is_nested_left_hand_side_safe { | ||||||
| return None; | ||||||
| } | ||||||
|
|
||||||
| if let AnyJsExpression::JsIdentifierExpression(ident) = &left { | ||||||
| let name = ident.name().ok()?; | ||||||
|
|
||||||
| // Use the semantic model to resolve the variable binding and check | ||||||
| // if it's initialized with a boolean literal. This allows us to | ||||||
| // handle cases like: | ||||||
| // let isOpen = false; // This is safe | ||||||
| // return <div>{isOpen && <Content />}</div>; // This should pass | ||||||
| if let Some(binding) = model.binding(&name) | ||||||
| && binding | ||||||
| .tree() | ||||||
| .declaration() | ||||||
| .and_then(|declaration| { | ||||||
| if let AnyJsBindingDeclaration::JsVariableDeclarator(declarator) = | ||||||
| declaration | ||||||
| { | ||||||
| Some(declarator) | ||||||
| } else { | ||||||
| None | ||||||
| } | ||||||
| }) | ||||||
| .and_then(|declarator| declarator.initializer()) | ||||||
| .and_then(|initializer| initializer.expression().ok()) | ||||||
| .and_then(|expr| { | ||||||
| if let AnyJsExpression::AnyJsLiteralExpression(literal) = expr { | ||||||
| Some(literal) | ||||||
| } else { | ||||||
| None | ||||||
| } | ||||||
| }) | ||||||
| .and_then(|literal| literal.value_token().ok()) | ||||||
| .is_some_and(|token| matches!(token.text_trimmed(), "true" | "false")) | ||||||
| { | ||||||
| return None; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| let is_literal = matches!(left, AnyJsExpression::AnyJsLiteralExpression(_)); | ||||||
| if is_literal && left.to_trimmed_text().is_empty() { | ||||||
| return None; | ||||||
| } | ||||||
|
|
||||||
| Some(true) | ||||||
| } | ||||||
| NoLeakedRenderQuery::JsConditionalExpression(expr) => { | ||||||
| let alternate = expr.alternate().ok()?; | ||||||
| let is_alternate_identifier = | ||||||
| matches!(alternate, AnyJsExpression::JsIdentifierExpression(_)); | ||||||
| let is_jsx_element_alt = matches!(alternate, AnyJsExpression::JsxTagExpression(_)); | ||||||
| if !is_alternate_identifier || is_jsx_element_alt { | ||||||
| return None; | ||||||
| } | ||||||
|
|
||||||
| Some(true) | ||||||
| } | ||||||
dyc3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> { | ||||||
| let node = ctx.query(); | ||||||
|
|
||||||
| match node { | ||||||
| NoLeakedRenderQuery::JsLogicalExpression(_) => { | ||||||
| Some( | ||||||
| RuleDiagnostic::new( | ||||||
| rule_category!(), | ||||||
| node.range(), | ||||||
| markup! { | ||||||
| "Potential leaked value that might cause unintended rendering." | ||||||
| }, | ||||||
| ) | ||||||
| .note(markup! { | ||||||
| "JavaScript's && operator returns the left value when it's falsy (e.g., 0, NaN, ''). React will render that value, causing unexpected UI output." | ||||||
| }) | ||||||
| .note(markup! { | ||||||
| "Make sure the condition is explicitly boolean.Use !!value, value > 0, or a ternary expression." | ||||||
| }) | ||||||
| ) | ||||||
| } | ||||||
| NoLeakedRenderQuery::JsConditionalExpression(_) => { | ||||||
| Some( | ||||||
| RuleDiagnostic::new( | ||||||
| rule_category!(), | ||||||
| node.range(), | ||||||
| markup! { | ||||||
| "Potential leaked value that might cause unintended rendering." | ||||||
| }, | ||||||
| ) | ||||||
| .note(markup! { | ||||||
| "This happens when you use ternary operators in JSX with alternate values that could be variables" | ||||||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
ematipico marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| }) | ||||||
| .note(markup! { | ||||||
| "Replace with a safe alternate value like an empty string , null or another JSX element" | ||||||
|
||||||
| "Replace with a safe alternate value like an empty string , null or another JSX element" | |
| "Replace with a safe alternate value like an empty string, null, or another JSX element." |
🤖 Prompt for AI Agents
In crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs around line 262,
fix the diagnostic note string by removing the extra space before the comma and
adding terminal punctuation: change `"Replace with a safe alternate value like
an empty string , null or another JSX element"` to `"Replace with a safe
alternate value like an empty string, null, or another JSX element."` so spacing
and punctuation are correct.
ematipico marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
ematipico marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last thing. If
left,rightandexpression()areAnyJsExpression, I suggest you go callomit_parentheses(), so we remove the parentheses.In fact, we have a few tests with binary expressions that contain parentheses. Can we add more of them? Let's get creative and add absurd cases, e.g.:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried adding the code you suggested, but parentheses were automatically omitted by the formatter, haha