Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
259 changes: 247 additions & 12 deletions crates/oxc_linter/src/rules/react/jsx_no_useless_fragment.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
use crate::{
AstNode,
context::{ContextHost, LintContext},
fixer::{RuleFix, RuleFixer},
rule::Rule,
};
use oxc_allocator::Vec as ArenaVec;
use oxc_ast::{
AstKind,
ast::{
Expand All @@ -8,13 +15,7 @@ use oxc_ast::{
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_semantic::NodeId;
use oxc_span::Span;

use crate::{
AstNode,
context::{ContextHost, LintContext},
rule::Rule,
};
use oxc_span::{GetSpan, Span};

fn needs_more_children(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Fragments should contain more than one child.").with_label(span)
Expand Down Expand Up @@ -54,7 +55,8 @@ declare_oxc_lint!(
/// ```
JsxNoUselessFragment,
react,
pedantic
pedantic,
suggestion
);

impl Rule for JsxNoUselessFragment {
Expand Down Expand Up @@ -99,12 +101,26 @@ impl JsxNoUselessFragment {
&& !(self.allow_expressions && is_fragment_with_single_expression(&elem.children))
{
let span = elem.opening_element.span;
ctx.diagnostic(needs_more_children(span));
let diagnostic = needs_more_children(span);
if can_fix(node, &elem.children, ctx) {
ctx.diagnostic_with_suggestion(diagnostic, |fixer| {
fix_fragment_element(elem, ctx, fixer)
});
} else {
ctx.diagnostic(diagnostic);
}
}

if is_child_of_html_element(node, ctx) {
let span = elem.opening_element.span;
ctx.diagnostic(child_of_html_element(span));
let diagnostic = child_of_html_element(span);
if can_fix(node, &elem.children, ctx) {
ctx.diagnostic_with_suggestion(diagnostic, |fixer| {
fix_fragment_element(elem, ctx, fixer)
});
} else {
ctx.diagnostic(diagnostic);
}
}
}

Expand All @@ -114,14 +130,151 @@ impl JsxNoUselessFragment {
&& !(self.allow_expressions && is_fragment_with_single_expression(&elem.children))
{
let span = elem.opening_fragment.span;
ctx.diagnostic(needs_more_children(span));
let diagnostic = needs_more_children(span);
if can_fix(node, &elem.children, ctx) {
ctx.diagnostic_with_suggestion(diagnostic, |fixer| {
fix_jsx_fragment(elem, ctx, fixer)
});
} else {
ctx.diagnostic(diagnostic);
}
}

if is_child_of_html_element(node, ctx) {
let span = elem.opening_fragment.span;
ctx.diagnostic(child_of_html_element(span));
let diagnostic = child_of_html_element(span);
if can_fix(node, &elem.children, ctx) {
ctx.diagnostic_with_suggestion(diagnostic, |fixer| {
fix_jsx_fragment(elem, ctx, fixer)
});
} else {
ctx.diagnostic(diagnostic);
}
}
}
}

fn fix_fragment_element<'a>(
elem: &JSXElement,
ctx: &LintContext<'a>,
fixer: RuleFixer<'_, 'a>,
) -> RuleFix<'a> {
let replacement = if let Some(closing_elem) = &elem.closing_element {
trim_like_react(
Span::new(elem.opening_element.span.end, closing_elem.span.start)
.source_text(ctx.source_text()),
)
} else {
""
};

fixer.replace(elem.span(), trim_like_react(replacement))
}

fn fix_jsx_fragment<'a>(
elem: &JSXFragment,
ctx: &LintContext<'a>,
fixer: RuleFixer<'_, 'a>,
) -> RuleFix<'a> {
fixer.replace(
elem.span(),
trim_like_react(
Span::new(elem.opening_fragment.span.end, elem.closing_fragment.span.start)
.source_text(ctx.source_text()),
),
)
}

fn trim_like_react(text: &str) -> &str {
let bytes = text.as_bytes();
let len = bytes.len();

if len == 0 {
return text;
}

// Find leading whitespace
let mut leading_end = 0;
let mut has_leading_newline = false;

for &byte in bytes {
if byte.is_ascii_whitespace() {
if byte == b'\n' {
has_leading_newline = true;
}
leading_end += 1;
} else {
break;
}
}

// Find trailing whitespace
let mut trailing_start = len;
let mut has_trailing_newline = false;

for &byte in bytes.iter().rev() {
if byte.is_ascii_whitespace() {
if byte == b'\n' {
has_trailing_newline = true;
}
trailing_start -= 1;
} else {
break;
}
}

// Apply React-like trimming rules
let start = if has_leading_newline { leading_end } else { 0 };
let end = if has_trailing_newline { trailing_start } else { len };

// Handle edge cases
if start >= end {
return "";
}

&text[start..end]
}

fn can_fix(node: &AstNode, children: &ArenaVec<JSXChild<'_>>, ctx: &LintContext) -> bool {
let Some(parent) = ctx.nodes().parent_kind(node.id()) else {
return false;
};

if !matches!(parent, AstKind::JSXElement(_) | AstKind::JSXFragment(_)) {
// const a = <></>
if children.is_empty() {
return false;
}

// const a = <>cat {meow}</>
if children.iter().all(|child| {
is_whitespace_only_text(child) || matches!(child, JSXChild::ExpressionContainer(_))
}) {
return false;
}
}

// Not safe to fix `<Eeee><>foo</></Eeee>` because `Eeee` might require its children be a ReactElement.
if let AstKind::JSXElement(el) = parent {
if !el
.opening_element
.name
.get_identifier_name()
.is_some_and(|ident| ident.chars().all(char::is_lowercase))
&& !is_jsx_fragment(&el.opening_element)
{
return false;
}
}

true
}

fn is_whitespace_only_text(child: &JSXChild) -> bool {
match child {
JSXChild::Text(text) => text.value.trim().is_empty(),
_ => false,
}
}

fn jsx_elem_has_key_attr(elem: &JSXElement) -> bool {
Expand Down Expand Up @@ -318,6 +471,88 @@ fn test() {
(r"<><Foo>{moo}</Foo></>", None),
];

let fix = vec![
(r"<></>", r"<></>", None),
(r"<>{}</>", r"<>{}</>", None),
(r"<p>moo<>foo</></p>", r"<p>moofoo</p>", None),
(r"<>{meow}</>", r"<>{meow}</>", None),
(r"<p><>{meow}</></p>", r"<p>{meow}</p>", None),
(r"<><div/></>", r"<div/>", None),
(
r"<>
<div/>
</>",
r"<div/>",
None,
),
(r"<Fragment />", r"<Fragment />", None),
(
r"<React.Fragment>
<Foo />
</React.Fragment>",
r"<Foo />",
None,
),
(r"<Eeee><>foo</></Eeee>", r"<Eeee><>foo</></Eeee>", None),
(r"<div><>foo</></div>", r"<div>foo</div>", None),
(r#"<div><>{"a"}{"b"}</></div>"#, r#"<div>{"a"}{"b"}</div>"#, None),
(
r#"
<section>
<Eeee />
<Eeee />
<>{"a"}{"b"}</>
</section>
"#,
r#"
<section>
<Eeee />
<Eeee />
{"a"}{"b"}
</section>
"#,
None,
),
(r#"<div><Fragment>{"a"}{"b"}</Fragment></div>"#, r#"<div>{"a"}{"b"}</div>"#, None),
(
r"
<section>
git<>
<b>hub</b>.
</>

git<> <b>hub</b></>
</section>",
r"
<section>
git<b>hub</b>.

git <b>hub</b>
</section>",
None,
),
(r#"<div>a <>{""}{""}</> a</div>"#, r#"<div>a {""}{""} a</div>"#, None),
(
r"
const Comp = () => (
<html>
<React.Fragment />
</html>
);
",
r"
const Comp = () => (
<html>

</html>
);
",
None,
),
(r"<><Foo>{moo}</Foo></>", r"<Foo>{moo}</Foo>", Some(json!([{"allowExpressions": true}]))),
];

Tester::new(JsxNoUselessFragment::NAME, JsxNoUselessFragment::PLUGIN, pass, fail)
.expect_fix(fix)
.test_and_snapshot();
}
Loading
Loading