From fec745762e369ab4d67d1b719138153c9fd733de Mon Sep 17 00:00:00 2001 From: zoomdong <1344492820@qq.com> Date: Fri, 27 Sep 2024 18:06:37 +0800 Subject: [PATCH 1/8] fix(linter): noUselessFragments deal html escapes under fragements wrong --- CHANGELOG.md | 11 +++++++++ .../lint/complexity/no_useless_fragments.rs | 22 +++++++++++++++-- .../noUselessFragments/issue_4059.jsx | 17 +++++++++++++ .../noUselessFragments/issue_4059.jsx.snap | 24 +++++++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4059.jsx create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4059.jsx.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e8ccc473fc9..8a81af133c18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -252,6 +252,17 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) don't create invaild JSX code when Fragments children contains JSX Expression and in a LogicalExpression. Contributed by @fireairforce +- [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) Fragments containing HTML escapes (e.g.  ) inside expression escapes `{ ... }` should not be considered useless. +The following code is no longer reported: + +```jsx +function Component() { + return ( +
{line || <> }
+ ) +} +``` + ### Parser #### Bug fixes diff --git a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs index a846f71944b5..e280e244de73 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs @@ -11,7 +11,7 @@ use biome_js_factory::make::{ use biome_js_syntax::{ AnyJsxChild, AnyJsxElementName, AnyJsxTag, JsLanguage, JsLogicalExpression, JsParenthesizedExpression, JsSyntaxKind, JsxChildList, JsxElement, JsxExpressionAttributeValue, - JsxFragment, JsxTagExpression, JsxText, T, + JsxExpressionChild, JsxFragment, JsxTagExpression, JsxText, T, }; use biome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutation, BatchMutationExt}; @@ -124,6 +124,7 @@ impl Rule for NoUselessFragments { let model = ctx.model(); let mut in_jsx_attr_expr = false; let mut in_js_logical_expr = false; + let mut in_jsx_expr = false; match node { NoUselessFragmentsQuery::JsxFragment(fragment) => { let parents_where_fragments_must_be_preserved = node.syntax().parent().map_or( @@ -139,6 +140,9 @@ impl Rule for NoUselessFragments { if JsLogicalExpression::can_cast(parent.kind()) { in_js_logical_expr = true; } + if JsxExpressionChild::can_cast(parent.kind()) { + in_jsx_expr = true; + } match JsParenthesizedExpression::try_cast(parent) { Ok(parenthesized_expression) => { parenthesized_expression.syntax().parent() @@ -190,7 +194,17 @@ impl Rule for NoUselessFragments { } } JsSyntaxKind::JSX_TEXT => { - if !child.syntax().text().to_string().trim().is_empty() { + let child_text = + child.syntax().text().to_string().trim().to_string(); + + if (in_jsx_expr || in_js_logical_expr) + && contains_html_entity(&child_text) + { + children_where_fragments_must_preserved = true; + break; + } + + if !child_text.is_empty() { significant_children += 1; if first_significant_child.is_none() { first_significant_child = Some(child); @@ -401,3 +415,7 @@ impl Rule for NoUselessFragments { })) } } + +fn contains_html_entity(s: &str) -> bool { + s.contains("&") && s.contains(";") +} diff --git a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4059.jsx b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4059.jsx new file mode 100644 index 000000000000..2bd7ec6335c5 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4059.jsx @@ -0,0 +1,17 @@ +function MyComponent() { + return ( +
{line || <> }
+ ) +} + +function MyComponent2() { + return ( +
{<> }
+ ) +} + +function MyComponent3() { + return ( +
{value ?? <> }
+ ) +} \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4059.jsx.snap b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4059.jsx.snap new file mode 100644 index 000000000000..4dc837242e07 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4059.jsx.snap @@ -0,0 +1,24 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: issue_4059.jsx +--- +# Input +```jsx +function MyComponent() { + return ( +
{line || <> }
+ ) +} + +function MyComponent2() { + return ( +
{<> }
+ ) +} + +function MyComponent3() { + return ( +
{value ?? <> }
+ ) +} +``` From 60c2f62596a9ac711664e4533c1addfa85d19a8c Mon Sep 17 00:00:00 2001 From: zoomdong <1344492820@qq.com> Date: Fri, 27 Sep 2024 18:14:46 +0800 Subject: [PATCH 2/8] fix: clippy --- .../src/lint/complexity/no_useless_fragments.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs index e280e244de73..2eae3dc5bad4 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs @@ -417,5 +417,5 @@ impl Rule for NoUselessFragments { } fn contains_html_entity(s: &str) -> bool { - s.contains("&") && s.contains(";") + s.contains('&') && s.contains(';') } From 1832a4f880a78f11d2fd97107da82d9ca9569a86 Mon Sep 17 00:00:00 2001 From: zoomdong <1344492820@qq.com> Date: Fri, 27 Sep 2024 21:04:59 +0800 Subject: [PATCH 3/8] chore: update html entity --- CHANGELOG.md | 2 ++ .../src/lint/complexity/no_useless_fragments.rs | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a81af133c18..eb54355f7b2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -263,6 +263,8 @@ function Component() { } ``` +Contributed by @fireairforce + ### Parser #### Bug fixes diff --git a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs index 2eae3dc5bad4..003ec6caaa92 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs @@ -417,5 +417,7 @@ impl Rule for NoUselessFragments { } fn contains_html_entity(s: &str) -> bool { - s.contains('&') && s.contains(';') + let and = s.find('&'); + let semi = s.find(';'); + matches!((and, semi), (Some(and), Some(semi)) if and < semi) } From faf402cffabf04b64cbe21174eb51e671722bc3c Mon Sep 17 00:00:00 2001 From: zoomdong <1344492820@qq.com> Date: Fri, 27 Sep 2024 21:48:01 +0800 Subject: [PATCH 4/8] chore: update name --- .../src/lint/complexity/no_useless_fragments.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs index 003ec6caaa92..2b3f190d3e1d 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs @@ -198,7 +198,7 @@ impl Rule for NoUselessFragments { child.syntax().text().to_string().trim().to_string(); if (in_jsx_expr || in_js_logical_expr) - && contains_html_entity(&child_text) + && contains_html_character_references(&child_text) { children_where_fragments_must_preserved = true; break; @@ -416,7 +416,7 @@ impl Rule for NoUselessFragments { } } -fn contains_html_entity(s: &str) -> bool { +fn contains_html_character_references(s: &str) -> bool { let and = s.find('&'); let semi = s.find(';'); matches!((and, semi), (Some(and), Some(semi)) if and < semi) From 522f5f9f7de90b47e2bcf2d8c5e9461eb3c90b68 Mon Sep 17 00:00:00 2001 From: zoomdong <1344492820@qq.com> Date: Tue, 8 Oct 2024 14:04:35 +0800 Subject: [PATCH 5/8] fix: string update --- .../src/lint/complexity/no_useless_fragments.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs index 2b3f190d3e1d..5ef9847da719 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs @@ -194,11 +194,11 @@ impl Rule for NoUselessFragments { } } JsSyntaxKind::JSX_TEXT => { - let child_text = - child.syntax().text().to_string().trim().to_string(); + let original_text = child.syntax().text().to_string(); + let child_text = original_text.trim(); if (in_jsx_expr || in_js_logical_expr) - && contains_html_character_references(&child_text) + && contains_html_character_references(child_text) { children_where_fragments_must_preserved = true; break; From 0ad6b6c50fe673ebd6771b578ba1587c03946124 Mon Sep 17 00:00:00 2001 From: zoomdong <1344492820@qq.com> Date: Tue, 8 Oct 2024 16:07:35 +0800 Subject: [PATCH 6/8] chore: update changelog --- CHANGELOG.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb54355f7b2b..2a1bce37eac1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,19 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b Contributed by @Conaclos +- [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) Fragments containing HTML escapes (e.g.  ) inside expression escapes `{ ... }` should not be considered useless. +The following code is no longer reported: + +```jsx +function Component() { + return ( +
{line || <> }
+ ) +} +``` + +Contributed by @fireairforce + ### Parser #### Bug Fixes @@ -252,19 +265,6 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) don't create invaild JSX code when Fragments children contains JSX Expression and in a LogicalExpression. Contributed by @fireairforce -- [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) Fragments containing HTML escapes (e.g.  ) inside expression escapes `{ ... }` should not be considered useless. -The following code is no longer reported: - -```jsx -function Component() { - return ( -
{line || <> }
- ) -} -``` - -Contributed by @fireairforce - ### Parser #### Bug fixes From ecd4cd1e7c0c46453560d5f018b8a44e50bff474 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 8 Oct 2024 11:31:06 +0100 Subject: [PATCH 7/8] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a1bce37eac1..36d0023ea4b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,7 +74,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b Contributed by @Conaclos -- [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) Fragments containing HTML escapes (e.g.  ) inside expression escapes `{ ... }` should not be considered useless. +- Fixes [#4059](https://github.com/biomejs/biome/issues/4059), the rule [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) now correctly handles fragments containing HTML escapes (e.g. ` `) inside expression escapes `{ ... }`. The following code is no longer reported: ```jsx From 02824b18b7e439f2394093ea367b0c9ae528773c Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 8 Oct 2024 11:53:09 +0100 Subject: [PATCH 8/8] add comment to explain the allocation --- .../src/lint/complexity/no_useless_fragments.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs index 5ef9847da719..0043a119fe5f 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs @@ -13,7 +13,9 @@ use biome_js_syntax::{ JsParenthesizedExpression, JsSyntaxKind, JsxChildList, JsxElement, JsxExpressionAttributeValue, JsxExpressionChild, JsxFragment, JsxTagExpression, JsxText, T, }; -use biome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutation, BatchMutationExt}; +use biome_rowan::{ + declare_node_union, AstNode, AstNodeList, BatchMutation, BatchMutationExt, SyntaxNodeText, +}; declare_lint_rule! { /// Disallow unnecessary fragments @@ -194,7 +196,9 @@ impl Rule for NoUselessFragments { } } JsSyntaxKind::JSX_TEXT => { - let original_text = child.syntax().text().to_string(); + // We need to whitespaces and newlines from the original string. + // Since in the JSX newlines aren't trivia, we require to allocate a string to trim from those characters. + let original_text = child.text(); let child_text = original_text.trim(); if (in_jsx_expr || in_js_logical_expr)