Skip to content

Commit 48f2cf8

Browse files
hyobanRel1cx
andauthored
feat(no-useless-fragment): auto fix support, closes #899 (#926)
Co-authored-by: Rel1cx <[email protected]>
1 parent 9ce72e1 commit 48f2cf8

File tree

2 files changed

+93
-5
lines changed

2 files changed

+93
-5
lines changed

packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.spec.ts

+41
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,33 @@ ruleTester.run(RULE_NAME, rule, {
88
{
99
code: /* tsx */ `<></>`,
1010
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
11+
output: null,
1112
},
1213
{
1314
code: /* tsx */ `<p><>foo</></p>`,
1415
errors: [
1516
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
1617
{ type: T.JSXFragment, messageId: "noUselessFragment" },
1718
],
19+
output: /* tsx */ `<p>foo</p>`,
1820
},
1921
{
2022
code: /* tsx */ `<p>moo<>foo</></p>`,
2123
errors: [
2224
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
2325
{ type: T.JSXFragment, messageId: "noUselessFragment" },
2426
],
27+
output: "<p>moofoo</p>",
2528
},
2629
{
2730
code: /* tsx */ `<p><>{meow}</></p>`,
2831
errors: [{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" }],
32+
output: "<p>{meow}</p>",
2933
},
3034
{
3135
code: /* tsx */ `<><div/></>`,
3236
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
37+
output: /* tsx */ `<div/>`,
3338
},
3439
{
3540
code: /* tsx */ `
@@ -38,10 +43,14 @@ ruleTester.run(RULE_NAME, rule, {
3843
</>
3944
`,
4045
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
46+
output: /* tsx */ `
47+
<div/>
48+
`,
4149
},
4250
{
4351
code: /* tsx */ `<Fragment />`,
4452
errors: [{ type: T.JSXElement, messageId: "noUselessFragment" }],
53+
output: null,
4554
},
4655
{
4756
code: /* tsx */ `
@@ -50,21 +59,27 @@ ruleTester.run(RULE_NAME, rule, {
5059
</React.Fragment>
5160
`,
5261
errors: [{ type: T.JSXElement, messageId: "noUselessFragment" }],
62+
output: /* tsx */ `
63+
<Foo />
64+
`,
5365
},
5466
{
5567
code: /* tsx */ `<Eeee><>foo</></Eeee>`,
5668
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
69+
output: null,
5770
},
5871
{
5972
code: /* tsx */ `<div><>foo</></div>`,
6073
errors: [
6174
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
6275
{ type: T.JSXFragment, messageId: "noUselessFragment" },
6376
],
77+
output: "<div>foo</div>",
6478
},
6579
{
6680
code: '<div><>{"a"}{"b"}</></div>',
6781
errors: [{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" }],
82+
output: '<div>{"a"}{"b"}</div>',
6883
},
6984
{
7085
code: /* tsx */ `
@@ -75,10 +90,18 @@ ruleTester.run(RULE_NAME, rule, {
7590
</section>
7691
`,
7792
errors: [{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" }],
93+
output: /* tsx */ `
94+
<section>
95+
<Eeee />
96+
<Eeee />
97+
{"a"}{"b"}
98+
</section>
99+
`,
78100
},
79101
{
80102
code: '<div><Fragment>{"a"}{"b"}</Fragment></div>',
81103
errors: [{ type: T.JSXElement, messageId: "noUselessFragmentInBuiltIn" }],
104+
output: '<div>{"a"}{"b"}</div>',
82105
},
83106
{
84107
// whitespace tricky case
@@ -95,10 +118,18 @@ ruleTester.run(RULE_NAME, rule, {
95118
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
96119
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
97120
],
121+
output: /* tsx */ `
122+
<section>
123+
git<b>hub</b>.
124+
125+
git <b>hub</b>
126+
</section>
127+
`,
98128
},
99129
{
100130
code: '<div>a <>{""}{""}</> a</div>',
101131
errors: [{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" }],
132+
output: '<div>a {""}{""} a</div>',
102133
},
103134
{
104135
code: /* tsx */ `
@@ -112,11 +143,20 @@ ruleTester.run(RULE_NAME, rule, {
112143
{ type: T.JSXElement, messageId: "noUselessFragmentInBuiltIn" },
113144
{ type: T.JSXElement, messageId: "noUselessFragment" },
114145
],
146+
// eslint-disable-next-line unicorn/template-indent
147+
output: /* tsx */ `
148+
const Comp = () => (
149+
<html>
150+
151+
</html>
152+
);
153+
`,
115154
},
116155
// Ensure allowExpressions still catches expected violations
117156
{
118157
code: /* tsx */ `<><Foo>{moo}</Foo></>`,
119158
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
159+
output: /* tsx */ `<Foo>{moo}</Foo>`,
120160
},
121161
{
122162
code: /* tsx */ `<>{moo}</>`,
@@ -135,6 +175,7 @@ ruleTester.run(RULE_NAME, rule, {
135175
messageId: "noUselessFragment",
136176
}],
137177
options: [{ allowExpressions: false }],
178+
output: /* tsx */ `<>{moo}</>`,
138179
},
139180
{
140181
code: /* tsx */ `<Foo bar={<>baz</>}/>`,

packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.ts

+52-5
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as JSX from "@eslint-react/jsx";
33
import type { RuleContext, RuleFeature } from "@eslint-react/shared";
44
import { AST_NODE_TYPES as T } from "@typescript-eslint/types";
55
import type { TSESTree } from "@typescript-eslint/utils";
6+
import type { RuleFixer } from "@typescript-eslint/utils/ts-eslint";
67

78
import { createRule } from "../utils";
89

@@ -27,23 +28,68 @@ const defaultOptions = [{
2728
allowExpressions: true,
2829
}] as const satisfies Options;
2930

31+
function trimLikeReact(text: string) {
32+
const leadingSpaces = /^\s*/.exec(text)?.[0] ?? "";
33+
const trailingSpaces = /\s*$/.exec(text)?.[0] ?? "";
34+
35+
const start = leadingSpaces.includes("\n") ? leadingSpaces.length : 0;
36+
const end = trailingSpaces.includes("\n") ? text.length - trailingSpaces.length : text.length;
37+
38+
return text.slice(start, end);
39+
}
40+
3041
function checkAndReport(
3142
node: TSESTree.JSXElement | TSESTree.JSXFragment,
3243
context: RuleContext,
3344
allowExpressions: boolean,
3445
) {
46+
function fix(fixer: RuleFixer) {
47+
// Not safe to fix fragments without a jsx parent.
48+
if (!(node.parent.type === T.JSXElement || node.parent.type === T.JSXFragment)) {
49+
// const a = <></>
50+
if (node.children.length === 0) {
51+
return null;
52+
}
53+
54+
// const a = <>cat {meow}</>
55+
if (
56+
node.children.some(
57+
(child) =>
58+
(JSX.isLiteral(child) && !JSX.isWhiteSpace(child))
59+
|| AST.is(T.JSXExpressionContainer)(child),
60+
)
61+
) {
62+
return null;
63+
}
64+
}
65+
66+
// Not safe to fix `<Eeee><>foo</></Eeee>` because `Eeee` might require its children be a ReactElement.
67+
if (JSX.isUserDefinedElement(node.parent)) {
68+
return null;
69+
}
70+
71+
const opener = node.type === T.JSXFragment ? node.openingFragment : node.openingElement;
72+
const closer = node.type === T.JSXFragment ? node.closingFragment : node.closingElement;
73+
74+
const childrenText = opener.type === T.JSXOpeningElement && opener.selfClosing
75+
? ""
76+
: context.sourceCode.getText().slice(opener.range[1], closer?.range[0]);
77+
78+
return fixer.replaceText(node, trimLikeReact(childrenText));
79+
}
80+
3581
const initialScope = context.sourceCode.getScope(node);
3682
// return if the fragment is keyed (e.g. <Fragment key={key}>)
3783
if (JSX.isKeyedElement(node, initialScope)) {
3884
return;
3985
}
4086
// report if the fragment is placed inside a built-in component (e.g. <div><></></div>)
4187
if (JSX.isBuiltInElement(node.parent)) {
42-
context.report({ messageId: "noUselessFragmentInBuiltIn", node });
88+
context.report({ messageId: "noUselessFragmentInBuiltIn", node, fix });
4389
}
4490
// report and return if the fragment has no children (e.g. <></>)
4591
if (node.children.length === 0) {
46-
context.report({ messageId: "noUselessFragment", node });
92+
context.report({ messageId: "noUselessFragment", node, fix });
4793
return;
4894
}
4995
const isChildElement = AST.isOneOf([T.JSXElement, T.JSXFragment])(node.parent);
@@ -58,15 +104,15 @@ function checkAndReport(
58104
// <Foo><>hello, world</></Foo>
59105
case !allowExpressions
60106
&& isChildElement: {
61-
context.report({ messageId: "noUselessFragment", node });
107+
context.report({ messageId: "noUselessFragment", node, fix });
62108
return;
63109
}
64110
case !allowExpressions
65111
&& !isChildElement
66112
&& node.children.length === 1: {
67113
// const foo = <>{children}</>;
68114
// return <>{children}</>;
69-
context.report({ messageId: "noUselessFragment", node });
115+
context.report({ messageId: "noUselessFragment", node, fix });
70116
return;
71117
}
72118
}
@@ -76,7 +122,7 @@ function checkAndReport(
76122
case nonPaddingChildren.length === 0:
77123
case nonPaddingChildren.length === 1
78124
&& firstNonPaddingChild?.type !== T.JSXExpressionContainer: {
79-
context.report({ messageId: "noUselessFragment", node });
125+
context.report({ messageId: "noUselessFragment", node, fix });
80126
return;
81127
}
82128
}
@@ -90,6 +136,7 @@ export default createRule<Options, MessageID>({
90136
docs: {
91137
description: "disallow unnecessary fragments",
92138
},
139+
fixable: "code",
93140
messages: {
94141
noUselessFragment: "A fragment contains less than two children is unnecessary.",
95142
noUselessFragmentInBuiltIn: "A fragment placed inside a built-in component is unnecessary.",

0 commit comments

Comments
 (0)