Skip to content

Commit 1c79d02

Browse files
PeterCardenasPeterCardenas
authored
feat(linter): add react/jsx-fragments rule (#12988)
Adds the [`react/jsx-fragments`](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-fragments.md) rule. Replaced the eslint auto-generated tests with much simpler ones since the ones there seem outdated and/or esoteric. It might be worth checking if the import renames `Fragment` to something else, but we don't do this for `react/jsx-no-useless-fragment` (see https://github.com/oxc-project/oxc/blob/234abd465396c799f7cea1f6e41d44fcda6f54eb/crates/oxc_linter/src/rules/react/jsx_no_useless_fragment.rs#L324-L338). Thus, not gonna do it here. Might be worth adding a utility function file for `is_jsx_fragment` since I copy-pasted it, but seems pedantic imo. works towards #1022
1 parent 618ee87 commit 1c79d02

File tree

3 files changed

+254
-0
lines changed

3 files changed

+254
-0
lines changed

‎crates/oxc_linter/src/rules.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,7 @@ mod react {
337337
pub mod jsx_boolean_value;
338338
pub mod jsx_curly_brace_presence;
339339
pub mod jsx_filename_extension;
340+
pub mod jsx_fragments;
340341
pub mod jsx_key;
341342
pub mod jsx_no_comment_textnodes;
342343
pub mod jsx_no_duplicate_props;
@@ -958,6 +959,7 @@ oxc_macros::declare_all_lint_rules! {
958959
react::forbid_elements,
959960
react::forward_ref_uses_ref,
960961
react::iframe_missing_sandbox,
962+
react::jsx_fragments,
961963
react::jsx_filename_extension,
962964
react::jsx_boolean_value,
963965
react::jsx_curly_brace_presence,

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

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
use oxc_ast::{
2+
AstKind,
3+
ast::{JSXElementName, JSXMemberExpressionObject, JSXOpeningElement},
4+
};
5+
use oxc_diagnostics::OxcDiagnostic;
6+
use oxc_macros::declare_oxc_lint;
7+
use oxc_span::{GetSpan, Span};
8+
use serde_json::Value;
9+
10+
use crate::{AstNode, context::LintContext, rule::Rule};
11+
12+
fn jsx_fragments_diagnostic(span: Span, mode: FragmentMode) -> OxcDiagnostic {
13+
let msg = if mode == FragmentMode::Element {
14+
"Standard form for React fragments is preferred"
15+
} else {
16+
"Shorthand form for React fragments is preferred"
17+
};
18+
let help = if mode == FragmentMode::Element {
19+
"Use <React.Fragment></React.Fragment> instead of <></>"
20+
} else {
21+
"Use <></> instead of <React.Fragment></React.Fragment>"
22+
};
23+
OxcDiagnostic::warn(msg).with_help(help).with_label(span)
24+
}
25+
26+
#[derive(Debug, Default, Clone)]
27+
pub struct JsxFragments {
28+
mode: FragmentMode,
29+
}
30+
31+
#[derive(Debug, Default, Clone, PartialEq, Eq, Copy)]
32+
pub enum FragmentMode {
33+
#[default]
34+
Syntax,
35+
Element,
36+
}
37+
38+
impl From<&str> for FragmentMode {
39+
fn from(value: &str) -> Self {
40+
if value == "element" { Self::Element } else { Self::Syntax }
41+
}
42+
}
43+
44+
// See <https://github.com/oxc-project/oxc/issues/6050> for documentation details.
45+
declare_oxc_lint!(
46+
/// ### What it does
47+
///
48+
/// Enforces the shorthand or standard form for React Fragments.
49+
///
50+
/// ### Why is this bad?
51+
///
52+
/// Makes code using fragments more consistent one way or the other.
53+
///
54+
/// ### Options
55+
///
56+
/// `{ "mode": "syntax" | "element" }`
57+
///
58+
/// #### `syntax` mode
59+
/// This is the default mode. It will enforce the shorthand syntax for React fragments, with one exception.
60+
/// Keys or attributes are not supported by the shorthand syntax, so the rule will not warn on standard-form fragments that use those.
61+
///
62+
/// Examples of **incorrect** code for this rule:
63+
/// ```jsx
64+
/// <React.Fragment><Foo /></React.Fragment>
65+
/// ```
66+
///
67+
/// Examples of **correct** code for this rule:
68+
/// ```jsx
69+
/// <><Foo /></>
70+
/// ```
71+
///
72+
/// ```jsx
73+
/// <React.Fragment key="key"><Foo /></React.Fragment>
74+
/// ```
75+
///
76+
/// #### `element` mode
77+
/// This mode enforces the standard form for React fragments.
78+
///
79+
/// Examples of **incorrect** code for this rule:
80+
/// ```jsx
81+
/// <><Foo /></>
82+
/// ```
83+
///
84+
/// Examples of **correct** code for this rule:
85+
/// ```jsx
86+
/// <React.Fragment><Foo /></React.Fragment>
87+
/// ```
88+
///
89+
/// ```jsx
90+
/// <React.Fragment key="key"><Foo /></React.Fragment>
91+
/// ```
92+
JsxFragments,
93+
react,
94+
style,
95+
fix
96+
);
97+
98+
impl Rule for JsxFragments {
99+
fn from_configuration(value: Value) -> Self {
100+
let obj = value.get(0);
101+
Self {
102+
mode: obj
103+
.and_then(|v| v.get("mode"))
104+
.and_then(Value::as_str)
105+
.map(FragmentMode::from)
106+
.unwrap_or_default(),
107+
}
108+
}
109+
110+
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
111+
match node.kind() {
112+
AstKind::JSXElement(jsx_elem) if self.mode == FragmentMode::Syntax => {
113+
let Some(closing_element) = &jsx_elem.closing_element else {
114+
return;
115+
};
116+
if !is_jsx_fragment(&jsx_elem.opening_element)
117+
|| !jsx_elem.opening_element.attributes.is_empty()
118+
{
119+
return;
120+
}
121+
ctx.diagnostic_with_fix(
122+
jsx_fragments_diagnostic(jsx_elem.opening_element.name.span(), self.mode),
123+
|fixer| {
124+
let before_opening_tag = ctx.source_range(Span::new(
125+
jsx_elem.span().start,
126+
jsx_elem.opening_element.span().start,
127+
));
128+
let between_opening_tag_and_closing_tag = ctx.source_range(Span::new(
129+
jsx_elem.opening_element.span().end,
130+
closing_element.span().start,
131+
));
132+
let after_closing_tag = ctx.source_range(Span::new(
133+
closing_element.span().end,
134+
jsx_elem.span().end,
135+
));
136+
let mut replacement = String::new();
137+
replacement.push_str(before_opening_tag);
138+
replacement.push_str("<>");
139+
replacement.push_str(between_opening_tag_and_closing_tag);
140+
replacement.push_str("</>");
141+
replacement.push_str(after_closing_tag);
142+
fixer.replace(jsx_elem.span(), replacement)
143+
},
144+
);
145+
}
146+
AstKind::JSXFragment(jsx_frag) if self.mode == FragmentMode::Element => {
147+
ctx.diagnostic_with_fix(
148+
jsx_fragments_diagnostic(jsx_frag.opening_fragment.span(), self.mode),
149+
|fixer| {
150+
let before_opening_tag = ctx.source_range(Span::new(
151+
jsx_frag.span().start,
152+
jsx_frag.opening_fragment.span().start,
153+
));
154+
let between_opening_tag_and_closing_tag = ctx.source_range(Span::new(
155+
jsx_frag.opening_fragment.span().end,
156+
jsx_frag.closing_fragment.span().start,
157+
));
158+
let after_closing_tag = ctx.source_range(Span::new(
159+
jsx_frag.closing_fragment.span().end,
160+
jsx_frag.span().end,
161+
));
162+
let mut replacement = String::new();
163+
replacement.push_str(before_opening_tag);
164+
replacement.push_str("<React.Fragment>");
165+
replacement.push_str(between_opening_tag_and_closing_tag);
166+
replacement.push_str("</React.Fragment>");
167+
replacement.push_str(after_closing_tag);
168+
fixer.replace(jsx_frag.span(), replacement)
169+
},
170+
);
171+
}
172+
_ => {}
173+
}
174+
}
175+
176+
fn should_run(&self, ctx: &crate::context::ContextHost) -> bool {
177+
ctx.source_type().is_jsx()
178+
}
179+
}
180+
181+
fn is_jsx_fragment(elem: &JSXOpeningElement) -> bool {
182+
match &elem.name {
183+
JSXElementName::IdentifierReference(ident) => ident.name == "Fragment",
184+
JSXElementName::MemberExpression(mem_expr) => {
185+
if let JSXMemberExpressionObject::IdentifierReference(ident) = &mem_expr.object {
186+
ident.name == "React" && mem_expr.property.name == "Fragment"
187+
} else {
188+
false
189+
}
190+
}
191+
JSXElementName::NamespacedName(_)
192+
| JSXElementName::Identifier(_)
193+
| JSXElementName::ThisExpression(_) => false,
194+
}
195+
}
196+
197+
#[test]
198+
fn test() {
199+
use crate::tester::Tester;
200+
use serde_json::json;
201+
202+
let pass = vec![
203+
("<><Foo /></>", None),
204+
(r#"<Fragment key="key"><Foo /></Fragment>"#, None),
205+
(r#"<React.Fragment key="key"><Foo /></React.Fragment>"#, None),
206+
("<Fragment />", None),
207+
("<React.Fragment />", None),
208+
("<React.Fragment><Foo /></React.Fragment>", Some(json!([{"mode": "element"}]))),
209+
];
210+
211+
let fail = vec![
212+
("<Fragment><Foo /></Fragment>", None),
213+
("<React.Fragment><Foo /></React.Fragment>", None),
214+
("<><Foo /></>", Some(json!([{"mode": "element"}]))),
215+
];
216+
217+
let fix = vec![
218+
("<Fragment><Foo /></Fragment>", "<><Foo /></>", None),
219+
("<React.Fragment><Foo /></React.Fragment>", "<><Foo /></>", None),
220+
(
221+
"<><Foo /></>",
222+
"<React.Fragment><Foo /></React.Fragment>",
223+
Some(json!([{"mode": "element"}])),
224+
),
225+
];
226+
Tester::new(JsxFragments::NAME, JsxFragments::PLUGIN, pass, fail)
227+
.expect_fix(fix)
228+
.test_and_snapshot();
229+
}

‎crates/oxc_linter/src/snapshots/react_jsx_fragments.snap‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: crates/oxc_linter/src/tester.rs
3+
---
4+
⚠ eslint-plugin-react(jsx-fragments): Shorthand form for React fragments is preferred
5+
╭─[jsx_fragments.tsx:1:2]
6+
1 │ <Fragment><Foo /></Fragment>
7+
· ────────
8+
╰────
9+
help: Use <></> instead of <React.Fragment></React.Fragment>
10+
11+
⚠ eslint-plugin-react(jsx-fragments): Shorthand form for React fragments is preferred
12+
╭─[jsx_fragments.tsx:1:2]
13+
1 │ <React.Fragment><Foo /></React.Fragment>
14+
· ──────────────
15+
╰────
16+
help: Use <></> instead of <React.Fragment></React.Fragment>
17+
18+
⚠ eslint-plugin-react(jsx-fragments): Standard form for React fragments is preferred
19+
╭─[jsx_fragments.tsx:1:1]
20+
1 │ <><Foo /></>
21+
· ──
22+
╰────
23+
help: Use <React.Fragment></React.Fragment> instead of <></>

0 commit comments

Comments
 (0)