Skip to content

Commit 7824f01

Browse files
camchenrycamc314
camchenry
and
camc314
authored
feat(linter): implement suggestion for jsx/no-useless-fragment (#10800)
Co-authored-by: Cameron Clark <[email protected]>
1 parent e5eb45c commit 7824f01

File tree

2 files changed

+272
-12
lines changed

2 files changed

+272
-12
lines changed

‎crates/oxc_linter/src/rules/react/jsx_no_useless_fragment.rs‎

Lines changed: 247 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
use crate::{
2+
AstNode,
3+
context::{ContextHost, LintContext},
4+
fixer::{RuleFix, RuleFixer},
5+
rule::Rule,
6+
};
7+
use oxc_allocator::Vec as ArenaVec;
18
use oxc_ast::{
29
AstKind,
310
ast::{
@@ -8,13 +15,7 @@ use oxc_ast::{
815
use oxc_diagnostics::OxcDiagnostic;
916
use oxc_macros::declare_oxc_lint;
1017
use oxc_semantic::NodeId;
11-
use oxc_span::Span;
12-
13-
use crate::{
14-
AstNode,
15-
context::{ContextHost, LintContext},
16-
rule::Rule,
17-
};
18+
use oxc_span::{GetSpan, Span};
1819

1920
fn needs_more_children(span: Span) -> OxcDiagnostic {
2021
OxcDiagnostic::warn("Fragments should contain more than one child.").with_label(span)
@@ -54,7 +55,8 @@ declare_oxc_lint!(
5455
/// ```
5556
JsxNoUselessFragment,
5657
react,
57-
pedantic
58+
pedantic,
59+
suggestion
5860
);
5961

6062
impl Rule for JsxNoUselessFragment {
@@ -99,12 +101,26 @@ impl JsxNoUselessFragment {
99101
&& !(self.allow_expressions && is_fragment_with_single_expression(&elem.children))
100102
{
101103
let span = elem.opening_element.span;
102-
ctx.diagnostic(needs_more_children(span));
104+
let diagnostic = needs_more_children(span);
105+
if can_fix(node, &elem.children, ctx) {
106+
ctx.diagnostic_with_suggestion(diagnostic, |fixer| {
107+
fix_fragment_element(elem, ctx, fixer)
108+
});
109+
} else {
110+
ctx.diagnostic(diagnostic);
111+
}
103112
}
104113

105114
if is_child_of_html_element(node, ctx) {
106115
let span = elem.opening_element.span;
107-
ctx.diagnostic(child_of_html_element(span));
116+
let diagnostic = child_of_html_element(span);
117+
if can_fix(node, &elem.children, ctx) {
118+
ctx.diagnostic_with_suggestion(diagnostic, |fixer| {
119+
fix_fragment_element(elem, ctx, fixer)
120+
});
121+
} else {
122+
ctx.diagnostic(diagnostic);
123+
}
108124
}
109125
}
110126

@@ -114,14 +130,151 @@ impl JsxNoUselessFragment {
114130
&& !(self.allow_expressions && is_fragment_with_single_expression(&elem.children))
115131
{
116132
let span = elem.opening_fragment.span;
117-
ctx.diagnostic(needs_more_children(span));
133+
let diagnostic = needs_more_children(span);
134+
if can_fix(node, &elem.children, ctx) {
135+
ctx.diagnostic_with_suggestion(diagnostic, |fixer| {
136+
fix_jsx_fragment(elem, ctx, fixer)
137+
});
138+
} else {
139+
ctx.diagnostic(diagnostic);
140+
}
118141
}
119142

120143
if is_child_of_html_element(node, ctx) {
121144
let span = elem.opening_fragment.span;
122-
ctx.diagnostic(child_of_html_element(span));
145+
let diagnostic = child_of_html_element(span);
146+
if can_fix(node, &elem.children, ctx) {
147+
ctx.diagnostic_with_suggestion(diagnostic, |fixer| {
148+
fix_jsx_fragment(elem, ctx, fixer)
149+
});
150+
} else {
151+
ctx.diagnostic(diagnostic);
152+
}
153+
}
154+
}
155+
}
156+
157+
fn fix_fragment_element<'a>(
158+
elem: &JSXElement,
159+
ctx: &LintContext<'a>,
160+
fixer: RuleFixer<'_, 'a>,
161+
) -> RuleFix<'a> {
162+
let replacement = if let Some(closing_elem) = &elem.closing_element {
163+
trim_like_react(
164+
Span::new(elem.opening_element.span.end, closing_elem.span.start)
165+
.source_text(ctx.source_text()),
166+
)
167+
} else {
168+
""
169+
};
170+
171+
fixer.replace(elem.span(), trim_like_react(replacement))
172+
}
173+
174+
fn fix_jsx_fragment<'a>(
175+
elem: &JSXFragment,
176+
ctx: &LintContext<'a>,
177+
fixer: RuleFixer<'_, 'a>,
178+
) -> RuleFix<'a> {
179+
fixer.replace(
180+
elem.span(),
181+
trim_like_react(
182+
Span::new(elem.opening_fragment.span.end, elem.closing_fragment.span.start)
183+
.source_text(ctx.source_text()),
184+
),
185+
)
186+
}
187+
188+
fn trim_like_react(text: &str) -> &str {
189+
let bytes = text.as_bytes();
190+
let len = bytes.len();
191+
192+
if len == 0 {
193+
return text;
194+
}
195+
196+
// Find leading whitespace
197+
let mut leading_end = 0;
198+
let mut has_leading_newline = false;
199+
200+
for &byte in bytes {
201+
if byte.is_ascii_whitespace() {
202+
if byte == b'\n' {
203+
has_leading_newline = true;
204+
}
205+
leading_end += 1;
206+
} else {
207+
break;
208+
}
209+
}
210+
211+
// Find trailing whitespace
212+
let mut trailing_start = len;
213+
let mut has_trailing_newline = false;
214+
215+
for &byte in bytes.iter().rev() {
216+
if byte.is_ascii_whitespace() {
217+
if byte == b'\n' {
218+
has_trailing_newline = true;
219+
}
220+
trailing_start -= 1;
221+
} else {
222+
break;
223+
}
224+
}
225+
226+
// Apply React-like trimming rules
227+
let start = if has_leading_newline { leading_end } else { 0 };
228+
let end = if has_trailing_newline { trailing_start } else { len };
229+
230+
// Handle edge cases
231+
if start >= end {
232+
return "";
233+
}
234+
235+
&text[start..end]
236+
}
237+
238+
fn can_fix(node: &AstNode, children: &ArenaVec<JSXChild<'_>>, ctx: &LintContext) -> bool {
239+
let Some(parent) = ctx.nodes().parent_kind(node.id()) else {
240+
return false;
241+
};
242+
243+
if !matches!(parent, AstKind::JSXElement(_) | AstKind::JSXFragment(_)) {
244+
// const a = <></>
245+
if children.is_empty() {
246+
return false;
247+
}
248+
249+
// const a = <>cat {meow}</>
250+
if children.iter().all(|child| {
251+
is_whitespace_only_text(child) || matches!(child, JSXChild::ExpressionContainer(_))
252+
}) {
253+
return false;
254+
}
255+
}
256+
257+
// Not safe to fix `<Eeee><>foo</></Eeee>` because `Eeee` might require its children be a ReactElement.
258+
if let AstKind::JSXElement(el) = parent {
259+
if !el
260+
.opening_element
261+
.name
262+
.get_identifier_name()
263+
.is_some_and(|ident| ident.chars().all(char::is_lowercase))
264+
&& !is_jsx_fragment(&el.opening_element)
265+
{
266+
return false;
123267
}
124268
}
269+
270+
true
271+
}
272+
273+
fn is_whitespace_only_text(child: &JSXChild) -> bool {
274+
match child {
275+
JSXChild::Text(text) => text.value.trim().is_empty(),
276+
_ => false,
277+
}
125278
}
126279

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

474+
let fix = vec![
475+
(r"<></>", r"<></>", None),
476+
(r"<>{}</>", r"<>{}</>", None),
477+
(r"<p>moo<>foo</></p>", r"<p>moofoo</p>", None),
478+
(r"<>{meow}</>", r"<>{meow}</>", None),
479+
(r"<p><>{meow}</></p>", r"<p>{meow}</p>", None),
480+
(r"<><div/></>", r"<div/>", None),
481+
(
482+
r"<>
483+
<div/>
484+
</>",
485+
r"<div/>",
486+
None,
487+
),
488+
(r"<Fragment />", r"<Fragment />", None),
489+
(
490+
r"<React.Fragment>
491+
<Foo />
492+
</React.Fragment>",
493+
r"<Foo />",
494+
None,
495+
),
496+
(r"<Eeee><>foo</></Eeee>", r"<Eeee><>foo</></Eeee>", None),
497+
(r"<div><>foo</></div>", r"<div>foo</div>", None),
498+
(r#"<div><>{"a"}{"b"}</></div>"#, r#"<div>{"a"}{"b"}</div>"#, None),
499+
(
500+
r#"
501+
<section>
502+
<Eeee />
503+
<Eeee />
504+
<>{"a"}{"b"}</>
505+
</section>
506+
"#,
507+
r#"
508+
<section>
509+
<Eeee />
510+
<Eeee />
511+
{"a"}{"b"}
512+
</section>
513+
"#,
514+
None,
515+
),
516+
(r#"<div><Fragment>{"a"}{"b"}</Fragment></div>"#, r#"<div>{"a"}{"b"}</div>"#, None),
517+
(
518+
r"
519+
<section>
520+
git<>
521+
<b>hub</b>.
522+
</>
523+
524+
git<> <b>hub</b></>
525+
</section>",
526+
r"
527+
<section>
528+
git<b>hub</b>.
529+
530+
git <b>hub</b>
531+
</section>",
532+
None,
533+
),
534+
(r#"<div>a <>{""}{""}</> a</div>"#, r#"<div>a {""}{""} a</div>"#, None),
535+
(
536+
r"
537+
const Comp = () => (
538+
<html>
539+
<React.Fragment />
540+
</html>
541+
);
542+
",
543+
r"
544+
const Comp = () => (
545+
<html>
546+
547+
</html>
548+
);
549+
",
550+
None,
551+
),
552+
(r"<><Foo>{moo}</Foo></>", r"<Foo>{moo}</Foo>", Some(json!([{"allowExpressions": true}]))),
553+
];
554+
321555
Tester::new(JsxNoUselessFragment::NAME, JsxNoUselessFragment::PLUGIN, pass, fail)
556+
.expect_fix(fix)
322557
.test_and_snapshot();
323558
}

0 commit comments

Comments
 (0)