Skip to content

feat(no-useless-fragment): auto fix support, closes #899 #926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Rel1cx merged 5 commits into from
Jan 25, 2025
Merged

feat(no-useless-fragment): auto fix support, closes #899 #926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f07308b
feat(no-useless-fragment): auto fix support
hyoban Jan 23, 2025
f3fa3ee
chore: update
hyoban Jan 23, 2025
5ebfc28
Merge branch 'main' into feat/fix-for-no-useless-fragment
Rel1cx Jan 25, 2025
ab6016e
Merge branch 'main' into feat/fix-for-no-useless-fragment
Rel1cx Jan 25, 2025
f625c28
Merge branch 'main' into feat/fix-for-no-useless-fragment
Rel1cx Jan 25, 2025
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
41 changes: 41 additions & 0 deletions packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,33 @@ ruleTester.run(RULE_NAME, rule, {
{
code: /* tsx */ `<></>`,
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
output: null,
},
{
code: /* tsx */ `<p><>foo</></p>`,
errors: [
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
{ type: T.JSXFragment, messageId: "noUselessFragment" },
],
output: /* tsx */ `<p>foo</p>`,
},
{
code: /* tsx */ `<p>moo<>foo</></p>`,
errors: [
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
{ type: T.JSXFragment, messageId: "noUselessFragment" },
],
output: "<p>moofoo</p>",
},
{
code: /* tsx */ `<p><>{meow}</></p>`,
errors: [{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" }],
output: "<p>{meow}</p>",
},
{
code: /* tsx */ `<><div/></>`,
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
output: /* tsx */ `<div/>`,
},
{
code: /* tsx */ `
Expand All @@ -38,10 +43,14 @@ ruleTester.run(RULE_NAME, rule, {
</>
`,
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
output: /* tsx */ `
<div/>
`,
},
{
code: /* tsx */ `<Fragment />`,
errors: [{ type: T.JSXElement, messageId: "noUselessFragment" }],
output: null,
},
{
code: /* tsx */ `
Expand All @@ -50,21 +59,27 @@ ruleTester.run(RULE_NAME, rule, {
</React.Fragment>
`,
errors: [{ type: T.JSXElement, messageId: "noUselessFragment" }],
output: /* tsx */ `
<Foo />
`,
},
{
code: /* tsx */ `<Eeee><>foo</></Eeee>`,
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
output: null,
},
{
code: /* tsx */ `<div><>foo</></div>`,
errors: [
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
{ type: T.JSXFragment, messageId: "noUselessFragment" },
],
output: "<div>foo</div>",
},
{
code: '<div><>{"a"}{"b"}</></div>',
errors: [{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" }],
output: '<div>{"a"}{"b"}</div>',
},
{
code: /* tsx */ `
Expand All @@ -75,10 +90,18 @@ ruleTester.run(RULE_NAME, rule, {
</section>
`,
errors: [{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" }],
output: /* tsx */ `
<section>
<Eeee />
<Eeee />
{"a"}{"b"}
</section>
`,
},
{
code: '<div><Fragment>{"a"}{"b"}</Fragment></div>',
errors: [{ type: T.JSXElement, messageId: "noUselessFragmentInBuiltIn" }],
output: '<div>{"a"}{"b"}</div>',
},
{
// whitespace tricky case
Expand All @@ -95,10 +118,18 @@ ruleTester.run(RULE_NAME, rule, {
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" },
],
output: /* tsx */ `
<section>
git<b>hub</b>.

git <b>hub</b>
</section>
`,
},
{
code: '<div>a <>{""}{""}</> a</div>',
errors: [{ type: T.JSXFragment, messageId: "noUselessFragmentInBuiltIn" }],
output: '<div>a {""}{""} a</div>',
},
{
code: /* tsx */ `
Expand All @@ -112,11 +143,20 @@ ruleTester.run(RULE_NAME, rule, {
{ type: T.JSXElement, messageId: "noUselessFragmentInBuiltIn" },
{ type: T.JSXElement, messageId: "noUselessFragment" },
],
// eslint-disable-next-line unicorn/template-indent
output: /* tsx */ `
const Comp = () => (
<html>

</html>
);
`,
},
// Ensure allowExpressions still catches expected violations
{
code: /* tsx */ `<><Foo>{moo}</Foo></>`,
errors: [{ type: T.JSXFragment, messageId: "noUselessFragment" }],
output: /* tsx */ `<Foo>{moo}</Foo>`,
},
{
code: /* tsx */ `<>{moo}</>`,
Expand All @@ -135,6 +175,7 @@ ruleTester.run(RULE_NAME, rule, {
messageId: "noUselessFragment",
}],
options: [{ allowExpressions: false }],
output: /* tsx */ `<>{moo}</>`,
},
{
code: /* tsx */ `<Foo bar={<>baz</>}/>`,
Expand Down
57 changes: 52 additions & 5 deletions packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as JSX from "@eslint-react/jsx";
import type { RuleContext, RuleFeature } from "@eslint-react/shared";
import { AST_NODE_TYPES as T } from "@typescript-eslint/types";
import type { TSESTree } from "@typescript-eslint/utils";
import type { RuleFixer } from "@typescript-eslint/utils/ts-eslint";

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

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

function trimLikeReact(text: string) {
const leadingSpaces = /^\s*/.exec(text)?.[0] ?? "";
const trailingSpaces = /\s*$/.exec(text)?.[0] ?? "";

const start = leadingSpaces.includes("\n") ? leadingSpaces.length : 0;
const end = trailingSpaces.includes("\n") ? text.length - trailingSpaces.length : text.length;

return text.slice(start, end);
}

function checkAndReport(
node: TSESTree.JSXElement | TSESTree.JSXFragment,
context: RuleContext,
allowExpressions: boolean,
) {
function fix(fixer: RuleFixer) {
// Not safe to fix fragments without a jsx parent.
if (!(node.parent.type === T.JSXElement || node.parent.type === T.JSXFragment)) {
// const a = <></>
if (node.children.length === 0) {
return null;
}

// const a = <>cat {meow}</>
if (
node.children.some(
(child) =>
(JSX.isLiteral(child) && !JSX.isWhiteSpace(child))
|| AST.is(T.JSXExpressionContainer)(child),
)
) {
return null;
}
}

// Not safe to fix `<Eeee><>foo</></Eeee>` because `Eeee` might require its children be a ReactElement.
if (JSX.isUserDefinedElement(node.parent)) {
return null;
}

const opener = node.type === T.JSXFragment ? node.openingFragment : node.openingElement;
const closer = node.type === T.JSXFragment ? node.closingFragment : node.closingElement;

const childrenText = opener.type === T.JSXOpeningElement && opener.selfClosing
? ""
: context.sourceCode.getText().slice(opener.range[1], closer?.range[0]);

return fixer.replaceText(node, trimLikeReact(childrenText));
}

const initialScope = context.sourceCode.getScope(node);
// return if the fragment is keyed (e.g. <Fragment key={key}>)
if (JSX.isKeyedElement(node, initialScope)) {
return;
}
// report if the fragment is placed inside a built-in component (e.g. <div><></></div>)
if (JSX.isBuiltInElement(node.parent)) {
context.report({ messageId: "noUselessFragmentInBuiltIn", node });
context.report({ messageId: "noUselessFragmentInBuiltIn", node, fix });
}
// report and return if the fragment has no children (e.g. <></>)
if (node.children.length === 0) {
context.report({ messageId: "noUselessFragment", node });
context.report({ messageId: "noUselessFragment", node, fix });
return;
}
const isChildElement = AST.isOneOf([T.JSXElement, T.JSXFragment])(node.parent);
Expand All @@ -58,15 +104,15 @@ function checkAndReport(
// <Foo><>hello, world</></Foo>
case !allowExpressions
&& isChildElement: {
context.report({ messageId: "noUselessFragment", node });
context.report({ messageId: "noUselessFragment", node, fix });
return;
}
case !allowExpressions
&& !isChildElement
&& node.children.length === 1: {
// const foo = <>{children}</>;
// return <>{children}</>;
context.report({ messageId: "noUselessFragment", node });
context.report({ messageId: "noUselessFragment", node, fix });
return;
}
}
Expand All @@ -76,7 +122,7 @@ function checkAndReport(
case nonPaddingChildren.length === 0:
case nonPaddingChildren.length === 1
&& firstNonPaddingChild?.type !== T.JSXExpressionContainer: {
context.report({ messageId: "noUselessFragment", node });
context.report({ messageId: "noUselessFragment", node, fix });
return;
}
}
Expand All @@ -90,6 +136,7 @@ export default createRule<Options, MessageID>({
docs: {
description: "disallow unnecessary fragments",
},
fixable: "code",
messages: {
noUselessFragment: "A fragment contains less than two children is unnecessary.",
noUselessFragmentInBuiltIn: "A fragment placed inside a built-in component is unnecessary.",
Expand Down
Loading