diff --git a/crates/oxc_linter/src/rules/react/jsx_no_useless_fragment.rs b/crates/oxc_linter/src/rules/react/jsx_no_useless_fragment.rs index 1f05e2032fd0a..d9a546299c518 100644 --- a/crates/oxc_linter/src/rules/react/jsx_no_useless_fragment.rs +++ b/crates/oxc_linter/src/rules/react/jsx_no_useless_fragment.rs @@ -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::{ @@ -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) @@ -54,7 +55,8 @@ declare_oxc_lint!( /// ``` JsxNoUselessFragment, react, - pedantic + pedantic, + suggestion ); impl Rule for JsxNoUselessFragment { @@ -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); + } } } @@ -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>, 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 `<>foo` 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 { @@ -318,6 +471,88 @@ fn test() { (r"<>{moo}", None), ]; + let fix = vec![ + (r"<>", r"<>", None), + (r"<>{}", r"<>{}", None), + (r"

moo<>foo

", r"

moofoo

", None), + (r"<>{meow}", r"<>{meow}", None), + (r"

<>{meow}

", r"

{meow}

", None), + (r"<>
", r"
", None), + ( + r"<> +
+ ", + r"
", + None, + ), + (r"", r"", None), + ( + r" + + ", + r"", + None, + ), + (r"<>foo", r"<>foo", None), + (r"
<>foo
", r"
foo
", None), + (r#"
<>{"a"}{"b"}
"#, r#"
{"a"}{"b"}
"#, None), + ( + r#" +
+ + + <>{"a"}{"b"} +
+ "#, + r#" +
+ + + {"a"}{"b"} +
+ "#, + None, + ), + (r#"
{"a"}{"b"}
"#, r#"
{"a"}{"b"}
"#, None), + ( + r" +
+ git<> + hub. + + + git<> hub +
", + r" +
+ github. + + git hub +
", + None, + ), + (r#"
a <>{""}{""} a
"#, r#"
a {""}{""} a
"#, None), + ( + r" + const Comp = () => ( + + + + ); + ", + r" + const Comp = () => ( + + + + ); + ", + None, + ), + (r"<>{moo}", r"{moo}", Some(json!([{"allowExpressions": true}]))), + ]; + Tester::new(JsxNoUselessFragment::NAME, JsxNoUselessFragment::PLUGIN, pass, fail) + .expect_fix(fix) .test_and_snapshot(); } diff --git a/crates/oxc_linter/src/snapshots/react_jsx_no_useless_fragment.snap b/crates/oxc_linter/src/snapshots/react_jsx_no_useless_fragment.snap index e3c2166139d0f..1d2b100277c74 100644 --- a/crates/oxc_linter/src/snapshots/react_jsx_no_useless_fragment.snap +++ b/crates/oxc_linter/src/snapshots/react_jsx_no_useless_fragment.snap @@ -18,12 +18,14 @@ source: crates/oxc_linter/src/tester.rs 1 │

moo<>foo

· ── ╰──── + help: Replace `<>foo` with `foo`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:1:7] 1 │

moo<>foo

· ── ╰──── + help: Replace `<>foo` with `foo`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Fragments should contain more than one child. ╭─[jsx_no_useless_fragment.tsx:1:1] @@ -36,18 +38,21 @@ source: crates/oxc_linter/src/tester.rs 1 │

<>{meow}

· ── ╰──── + help: Replace `<>{meow}` with `{meow}`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:1:4] 1 │

<>{meow}

· ── ╰──── + help: Replace `<>{meow}` with `{meow}`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Fragments should contain more than one child. ╭─[jsx_no_useless_fragment.tsx:1:1] 1 │ <>
· ── ╰──── + help: Replace `<>
` with `
`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Fragments should contain more than one child. ╭─[jsx_no_useless_fragment.tsx:2:13] @@ -56,6 +61,9 @@ source: crates/oxc_linter/src/tester.rs · ── 3 │
╰──── + help: Replace `<> +
+ ` with `
`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Fragments should contain more than one child. ╭─[jsx_no_useless_fragment.tsx:1:1] @@ -70,6 +78,9 @@ source: crates/oxc_linter/src/tester.rs · ──────────────── 3 │ ╰──── + help: Replace ` + + ` with ``. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Fragments should contain more than one child. ╭─[jsx_no_useless_fragment.tsx:1:7] @@ -82,24 +93,28 @@ source: crates/oxc_linter/src/tester.rs 1 │
<>foo
· ── ╰──── + help: Replace `<>foo` with `foo`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:1:6] 1 │
<>foo
· ── ╰──── + help: Replace `<>foo` with `foo`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:1:6] 1 │
<>{"a"}{"b"}
· ── ╰──── + help: Replace `<>{"a"}{"b"}` with `{"a"}{"b"}`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:1:6] 1 │
<>{"a"}{"b"}
· ── ╰──── + help: Replace `<>{"a"}{"b"}` with `{"a"}{"b"}`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:5:15] @@ -108,12 +123,14 @@ source: crates/oxc_linter/src/tester.rs · ── 6 │ ╰──── + help: Replace `<>{"a"}{"b"}` with `{"a"}{"b"}`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:1:6] 1 │
{"a"}{"b"}
· ────────── ╰──── + help: Replace `{"a"}{"b"}` with `{"a"}{"b"}`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:3:18] @@ -122,6 +139,9 @@ source: crates/oxc_linter/src/tester.rs · ── 4 │ hub. ╰──── + help: Replace `<> + hub. + ` with `hub.`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:7:18] @@ -130,12 +150,14 @@ source: crates/oxc_linter/src/tester.rs · ── 8 │ ╰──── + help: Replace `<> hub` with ` hub`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:1:8] 1 │
a <>{""}{""} a
· ── ╰──── + help: Replace `<>{""}{""}` with `{""}{""}`. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Fragments should contain more than one child. ╭─[jsx_no_useless_fragment.tsx:4:17] @@ -144,6 +166,7 @@ source: crates/oxc_linter/src/tester.rs · ────────────────── 5 │ ╰──── + help: Replace `` with ``. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Passing a fragment to a HTML element is useless. ╭─[jsx_no_useless_fragment.tsx:4:17] @@ -152,9 +175,11 @@ source: crates/oxc_linter/src/tester.rs · ────────────────── 5 │ ╰──── + help: Replace `` with ``. ⚠ eslint-plugin-react(jsx-no-useless-fragment): Fragments should contain more than one child. ╭─[jsx_no_useless_fragment.tsx:1:1] 1 │ <>{moo} · ── ╰──── + help: Replace `<>{moo}` with `{moo}`.