From 7856688f6688829b6fc22c477925e603fa417bc6 Mon Sep 17 00:00:00 2001 From: satojin219 Date: Sun, 4 Aug 2024 17:49:33 +0900 Subject: [PATCH 1/7] fix: check if a fragment has only one child element --- .../src/lint/complexity/no_useless_fragments.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 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 0bec9b298651..d4dfeb859b52 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 @@ -161,9 +161,20 @@ impl Rule for NoUselessFragments { let child_list = fragment.children(); + let mut self_closing_or_element_count = 0; + + for child in child_list.iter() { + match child.syntax().kind() { + JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT | JsSyntaxKind::JSX_ELEMENT => { + self_closing_or_element_count += 1; + } + _ => {} + } + } + if !parents_where_fragments_must_be_preserved { match child_list.first() { - Some(first) if child_list.len() == 1 => { + Some(first) if self_closing_or_element_count == 1 => { if JsxText::can_cast(first.syntax().kind()) && in_jsx_attr_expr { return None; } From 5e436dbe42abe628862fd9d1e2d56c99218dba5d Mon Sep 17 00:00:00 2001 From: satojin219 Date: Sun, 4 Aug 2024 17:51:10 +0900 Subject: [PATCH 2/7] test: fragment with a single child element on a new line --- .../noUselessFragments/issue_3545.jsx | 4 ++ .../noUselessFragments/issue_3545.jsx.snap | 41 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx.snap diff --git a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx new file mode 100644 index 000000000000..65d1c4926e86 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx @@ -0,0 +1,4 @@ +// https://github.com/biomejs/biome/issues/3545 +<> + +; diff --git a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx.snap b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx.snap new file mode 100644 index 000000000000..6e9b51a35d83 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx.snap @@ -0,0 +1,41 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 84 +expression: issue_3545.jsx +--- +# Input +```jsx +// https://github.com/biomejs/biome/issues/3545 +<> + +; + +``` + +# Diagnostics +``` +issue_3545.jsx:2:1 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid using unnecessary Fragment. + + 1 │ // https://github.com/biomejs/biome/issues/3545 + > 2 │ <> + │ ^^ + > 3 │ + > 4 │ ; + │ ^^^ + 5 │ + + i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment. + + i Unsafe fix: Remove the Fragment + + 1 1 │ // https://github.com/biomejs/biome/issues/3545 + 2 │ - <> + 3 │ - → + 4 │ - ; + 2 │ + ""; + 5 3 │ + + +``` From 05d946f594e6dd571ed567373795329e53b7ade5 Mon Sep 17 00:00:00 2001 From: satojin219 Date: Sun, 4 Aug 2024 22:30:21 +0900 Subject: [PATCH 3/7] fix: expand child kind --- .../src/lint/complexity/no_useless_fragments.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 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 d4dfeb859b52..9a858486ae99 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 @@ -165,7 +165,15 @@ impl Rule for NoUselessFragments { for child in child_list.iter() { match child.syntax().kind() { - JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT | JsSyntaxKind::JSX_ELEMENT => { + JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT + | JsSyntaxKind::JSX_ELEMENT + | JsSyntaxKind::JSX_EXPRESSION_CHILD + | JsSyntaxKind::JSX_TEXT => { + if child.syntax().kind() == JsSyntaxKind::JSX_TEXT + && child_list.len() > 1 + { + continue; + } self_closing_or_element_count += 1; } _ => {} From 65fd215a5e0f53fe8e11e9722eb03151453f237a Mon Sep 17 00:00:00 2001 From: satojin219 Date: Sat, 10 Aug 2024 21:10:22 +0900 Subject: [PATCH 4/7] fix: get first element excluding line breaks --- .../lint/complexity/no_useless_fragments.rs | 67 +++++++++++++------ 1 file changed, 45 insertions(+), 22 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 9a858486ae99..3ce5899b2cde 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 @@ -161,34 +161,46 @@ impl Rule for NoUselessFragments { let child_list = fragment.children(); - let mut self_closing_or_element_count = 0; + if !parents_where_fragments_must_be_preserved { + let mut significant_children = 0; + let mut first_significant_child = None; - for child in child_list.iter() { - match child.syntax().kind() { - JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT - | JsSyntaxKind::JSX_ELEMENT - | JsSyntaxKind::JSX_EXPRESSION_CHILD - | JsSyntaxKind::JSX_TEXT => { - if child.syntax().kind() == JsSyntaxKind::JSX_TEXT - && child_list.len() > 1 - { - continue; + for child in child_list.iter() { + match child.syntax().kind() { + JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT + | JsSyntaxKind::JSX_ELEMENT + | JsSyntaxKind::JSX_EXPRESSION_CHILD + | JsSyntaxKind::JSX_FRAGMENT => { + significant_children += 1; + if first_significant_child.is_none() { + first_significant_child = Some(child); + } } - self_closing_or_element_count += 1; + JsSyntaxKind::JSX_TEXT => { + if !child.syntax().text().to_string().trim().is_empty() { + significant_children += 1; + if first_significant_child.is_none() { + first_significant_child = Some(child); + } + } + } + _ => {} } - _ => {} } - } - if !parents_where_fragments_must_be_preserved { - match child_list.first() { - Some(first) if self_closing_or_element_count == 1 => { - if JsxText::can_cast(first.syntax().kind()) && in_jsx_attr_expr { - return None; + match significant_children { + 0 => Some(NoUselessFragmentsState::Empty), + 1 => { + if let Some(first) = first_significant_child { + if JsxText::can_cast(first.syntax().kind()) && in_jsx_attr_expr { + None + } else { + Some(NoUselessFragmentsState::Child(first)) + } + } else { + None } - Some(NoUselessFragmentsState::Child(first)) } - None => Some(NoUselessFragmentsState::Empty), _ => None, } } else { @@ -274,7 +286,18 @@ impl Rule for NoUselessFragments { None => parent.into_syntax(), }; - let child = node.children().first(); + let child = node + .children() + .iter() + .find(|child| match child.syntax().kind() { + JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT + | JsSyntaxKind::JSX_ELEMENT + | JsSyntaxKind::JSX_EXPRESSION_CHILD + | JsSyntaxKind::JSX_FRAGMENT => true, + JsSyntaxKind::JSX_TEXT => !child.syntax().text().to_string().trim().is_empty(), + _ => false, + }); + if let Some(child) = child { let new_node = match child { AnyJsxChild::JsxElement(node) => { From 7951a9c8aac6a13e68c2aa37c661fedc4d9af828 Mon Sep 17 00:00:00 2001 From: satojin219 Date: Sat, 10 Aug 2024 21:10:42 +0900 Subject: [PATCH 5/7] test: update snap shot --- .../fromImportNamespaceInvalid.jsx.snap | 32 +++++++++++++++++-- .../noUselessFragments/issue_3545.jsx.snap | 2 +- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/fromImportNamespaceInvalid.jsx.snap b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/fromImportNamespaceInvalid.jsx.snap index 7a8bf65a1871..5099cb77e2ca 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/fromImportNamespaceInvalid.jsx.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/fromImportNamespaceInvalid.jsx.snap @@ -1,5 +1,6 @@ --- source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 84 expression: fromImportNamespaceInvalid.jsx --- # Input @@ -13,6 +14,35 @@ import * as AwesomeReact from "react"; ``` # Diagnostics +``` +fromImportNamespaceInvalid.jsx:3:1 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━ + + ! Avoid using unnecessary Fragment. + + 1 │ import * as AwesomeReact from "react"; + 2 │ + > 3 │ <> + │ ^^ + > 4 │ foo + > 5 │ + │ ^^^ + 6 │ + + i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment. + + i Unsafe fix: Remove the Fragment + + 1 1 │ import * as AwesomeReact from "react"; + 2 2 │ + 3 │ - <> + 4 │ - ····foo + 5 │ - + 3 │ + foo + 6 4 │ + + +``` + ``` fromImportNamespaceInvalid.jsx:4:5 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━ @@ -32,5 +62,3 @@ fromImportNamespaceInvalid.jsx:4:5 lint/complexity/noUselessFragments FIXABLE │ ----------------------- ------------------------ ``` - - diff --git a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx.snap b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx.snap index 6e9b51a35d83..f3ea84b16d70 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3545.jsx.snap @@ -34,7 +34,7 @@ issue_3545.jsx:2:1 lint/complexity/noUselessFragments FIXABLE ━━━━━ 2 │ - <> 3 │ - → 4 │ - ; - 2 │ + ""; + 2 │ + ; 5 3 │ From d8cea556e6f77ac84bf7aadb3ed3bf20d0d8fed8 Mon Sep 17 00:00:00 2001 From: satojin219 Date: Sat, 10 Aug 2024 21:59:24 +0900 Subject: [PATCH 6/7] refactor: early return --- .../src/lint/complexity/no_useless_fragments.rs | 3 +++ 1 file changed, 3 insertions(+) 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 3ce5899b2cde..6484715d8542 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 @@ -186,6 +186,9 @@ impl Rule for NoUselessFragments { } _ => {} } + if significant_children > 1 { + break; + } } match significant_children { From fcd861c8f2271eab66e5ebad1bde086a08a3d1b5 Mon Sep 17 00:00:00 2001 From: satojin219 Date: Tue, 13 Aug 2024 22:35:13 +0900 Subject: [PATCH 7/7] chore: updaste CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5bf7f0629cb..b7bf32ebcece 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - `--log-prefix-name`: a prefix that's added to the file name of the logs. It defaults to `server.log`. The commands also accepts the environment variable `BIOME_LOG_PREFIX_NAME`. @Contributed by @ematipico - + #### Enhancements @@ -87,6 +87,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - `biome lint --write` now takes `--only` and `--skip` into account ([#3470](https://github.com/biomejs/biome/issues/3470)). Contributed by @Conaclos - Fix [#3368](https://github.com/biomejs/biome/issues/3368), now the reporter `github` tracks the diagnostics that belong to formatting and organize imports. Contributed by @ematipico +- Fix [#3545](https://github.com/biomejs/biome/issues/3545), display a warning, 'Avoid using unnecessary Fragment,' when a Fragment contains only one child element that is placed on a new line. Contributed by @satojin219 ### Configuration