Skip to content

Commit

Permalink
fix(analyze): check if a fragment has only one child element using to…
Browse files Browse the repository at this point in the history
…kens. (#3582)
  • Loading branch information
satojin219 authored Aug 13, 2024
1 parent 73e4805 commit ecbc627
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 10 deletions.
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 │
```

0 comments on commit ecbc627

Please sign in to comment.