Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(analyze): check if a fragment has only one child element using tokens. #3582

Merged
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,48 @@ impl Rule for NoUselessFragments {
let child_list = fragment.children();

if !parents_where_fragments_must_be_preserved {
match child_list.first() {
Some(first) if child_list.len() == 1 => {
if JsxText::can_cast(first.syntax().kind()) && in_jsx_attr_expr {
return None;
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_FRAGMENT => {
significant_children += 1;
if first_significant_child.is_none() {
first_significant_child = Some(child);
}
}
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 significant_children > 1 {
break;
}
}

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 {
Expand Down Expand Up @@ -254,7 +288,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) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 84
expression: fromImportNamespaceInvalid.jsx
---
# Input
Expand All @@ -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 │ <AwesomeReact.Fragment>foo</AwesomeReact.Fragment>
> 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 │ - ····<AwesomeReact.Fragment>foo</AwesomeReact.Fragment>
5 │ - </>
3 │ + <AwesomeReact.Fragment>foo</AwesomeReact.Fragment>
6 4 │


```

```
fromImportNamespaceInvalid.jsx:4:5 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━

Expand All @@ -32,5 +62,3 @@ fromImportNamespaceInvalid.jsx:4:5 lint/complexity/noUselessFragments FIXABLE
│ ----------------------- ------------------------

```


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// https://github.com/biomejs/biome/issues/3545
<>
<Component />
</>;
Original file line number Diff line number Diff line change
@@ -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
<>
<Component />
</>;

```

# Diagnostics
```
issue_3545.jsx:2:1 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid using unnecessary Fragment.

1 │ // https://github.com/biomejs/biome/issues/3545
> 2 │ <>
│ ^^
> 3 │ <Component />
> 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 │ - → <Component·/>
4 │ - </>;
2 │ + <Component·/>;
5 3 │


```
Loading