Skip to content
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.

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

Add rule S6749 (jsx-no-useless-fragment): Redundant React fragments should be removed #4147

Merged
merged 3 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions its/ruling/src/test/expected/js/fireact/javascript-S6749.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"fireact:src/components/Loader/index.js": [
24
],
"fireact:src/components/Logo/index.js": [
6
],
"fireact:src/components/menus/UserMenu/index.js": [
22
],
"fireact:src/pages/auth/accounts/AddUser/index.js": [
60
],
"fireact:src/pages/auth/accounts/DeleteAccount/index.js": [
57
],
"fireact:src/pages/auth/accounts/PaymentList/index.js": [
72
],
"fireact:src/pages/auth/accounts/PaymentMethod/index.js": [
131
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"ant-design:components/button/__tests__/index.test.tsx": [
345
]
}
27 changes: 27 additions & 0 deletions its/ruling/src/test/expected/ts/courselit/typescript-S6749.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"courselit:apps/web/components/admin/media/index.tsx": [
173
],
"courselit:apps/web/components/public/base-layout/scaffold/drawer-content.tsx": [
57
],
"courselit:apps/web/components/public/base-layout/template/section.tsx": [
40
],
"courselit:apps/web/components/public/checkout/index.tsx": [
32,
33
],
"courselit:apps/web/components/public/code-injector.tsx": [
45
],
"courselit:apps/web/components/public/items.tsx": [
90
],
"courselit:apps/web/components/public/purchase-status.tsx": [
105
],
"courselit:packages/common-widgets/src/tagged-content/widget.tsx": [
77
]
}
10 changes: 10 additions & 0 deletions its/ruling/src/test/expected/ts/desktop/typescript-S6749.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"desktop:app/src/ui/diff/diff-options.tsx": [
123
],
"desktop:app/src/ui/drag-elements/commit-drag-element.tsx": [
93,
101,
111
]
}
77 changes: 77 additions & 0 deletions its/ruling/src/test/expected/ts/eigen/typescript-S6749.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
{
"eigen:src/app/Components/AuctionResultsList.tsx": [
90
],
"eigen:src/app/Components/States/ZeroState.tsx": [
17,
27
],
"eigen:src/app/Components/StickyTabPage/StickyTabPage.tsx": [
51
],
"eigen:src/app/Scenes/Artist/SearchCriteria.tests.tsx": [
18,
33,
52,
57,
66
],
"eigen:src/app/Scenes/Artwork/Components/CommercialButtons/InquiryModal.tsx": [
77
],
"eigen:src/app/Scenes/Artwork/Components/CommercialEditionSetInformation.tsx": [
57
],
"eigen:src/app/Scenes/City/Components/SavedEventSection/index.tsx": [
46
],
"eigen:src/app/Scenes/Home/Components/AuctionResultsRail.tsx": [
51
],
"eigen:src/app/Scenes/Home/Components/FairsRail.tsx": [
57
],
"eigen:src/app/Scenes/Home/Home.tsx": [
206
],
"eigen:src/app/Scenes/MyBids/MyBids.tsx": [
114
],
"eigen:src/app/Scenes/MyCollection/Screens/Artwork/Components/ArtworkInsights/MyCollectionArtworkArtistAuctionResults.tsx": [
50
],
"eigen:src/app/Scenes/MyCollection/Screens/Artwork/Components/MyCollectionArtworkHeader.tsx": [
112
],
"eigen:src/app/Scenes/SavedAddresses/SavedAddresses.tsx": [
165
],
"eigen:src/app/Scenes/SavedSearchAlertsList/Components/SavedSearchAlertsListPlaceholder.tsx": [
8
],
"eigen:src/app/Scenes/Search/components/placeholders/AlgoliaSearchPlaceholder.tsx": [
15
],
"eigen:src/app/Scenes/SellWithArtsy/SubmitArtwork/SubmitArtwork.tsx": [
217
],
"eigen:src/app/Scenes/Tag/Tag.tsx": [
65
],
"eigen:src/palette/elements/CollapsibleMenuItem/CollapsibleMenuItem.stories.tsx": [
154
],
"eigen:src/palette/organisms/screenStructure/Screen.tsx": [
152
],
"eigen:src/palette/svgs/CreditCardIcon.tsx": [
23,
35,
46,
56,
71
],
"eigen:src/shared/utils/flattenChildren.tests.tsx": [
13
]
}
5 changes: 5 additions & 0 deletions its/ruling/src/test/expected/ts/moose/typescript-S6749.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"moose:renderer/components/Cast/Cast.tsx": [
42
]
}
5 changes: 5 additions & 0 deletions its/ruling/src/test/expected/ts/vuetify/typescript-S6749.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"vuetify:packages/vuetify/src/components/VTabs/__tests__/VTabs.spec.cy.tsx": [
10
]
}
29 changes: 15 additions & 14 deletions packages/jsts/src/linter/quickfixes/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,35 +27,36 @@
const quickFixMessages = new Map<string, string>([
['comma-dangle', 'Remove this trailing comma'],
['eol-last', 'Add a new line at the end of file'],
['jsx-no-useless-fragment', 'Remove redundant fragment'],
['no-empty-interface', 'Replace with type alias'],
['no-extra-bind', 'Remove .bind() call'],
['no-extra-boolean-cast', 'Remove extra cast'],
['no-extra-semi', 'Remove extra semicolon'],
['no-inferrable-types', 'Remove type declaration'],
['no-lonely-if', "Replace with 'else if'"],
['no-non-null-assertion', "Replace with optional chaining '.?'"],
['no-trailing-spaces', 'Remove trailing space'],
['no-undef-init', 'Remove initialization'],
['no-unnecessary-type-arguments', 'Remove type argument'],
['no-unnecessary-type-assertion', 'Remove type assertion'],
['no-unneeded-ternary', 'Replace with a simpler expression'],
['no-useless-rename', 'Remove alias'],
['no-var', "Replace 'var' with 'let'"],
['object-shorthand', 'Use shorthand property'],
['prefer-as-const', "Replace with 'as const'"],
['prefer-const', "Replace with 'const'"],
['prefer-function-type', 'Replace with a function type'],
['prefer-immediate-return', 'Return value immediately'],
['prefer-namespace-keyword', "Replace with 'namespace' keyword"],
['prefer-object-has-own', 'Replace with Object.hasOwn()'],
['prefer-object-spread', 'Replace with object spread syntax'],
['prefer-readonly', "Add 'readonly'"],
['prefer-return-this-type', "Replace return type with 'this'"],
['prefer-template', 'Replace with template string literal'],
['prefer-while', "Replace with 'while' loop"],
['quotes', 'Fix quotes'],
['radix', 'Add 10 as radix'],
['semi', 'Add semicolon'],
['prefer-immediate-return', 'Return value immediately'],
['prefer-while', "Replace with 'while' loop"],
['no-empty-interface', 'Replace with type alias'],
['no-inferrable-types', 'Remove type declaration'],
['no-lonely-if', "Replace with 'else if'"],
['no-undef-init', 'Remove initialization'],
['no-unnecessary-type-arguments', 'Remove type argument'],
['no-unnecessary-type-assertion', 'Remove type assertion'],
['prefer-function-type', 'Replace with a function type'],
['prefer-namespace-keyword', "Replace with 'namespace' keyword"],
['prefer-readonly', "Add 'readonly'"],
['no-non-null-assertion', "Replace with optional chaining '.?'"],
['no-unneeded-ternary', 'Replace with a simpler expression'],
['no-useless-rename', 'Remove alias'],
]);

/**
Expand Down
1 change: 1 addition & 0 deletions packages/jsts/src/linter/quickfixes/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export const quickFixRules = new Set([
'prefer-while',

// eslint-plugin-react
'jsx-no-useless-fragment',
'no-unknown-property',

// @typescript-eslint plugin
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<><Foo /></>
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ public static List<Class<? extends JavaScriptCheck>> getAllChecks() {
JsxNoCommentTextnodesCheck.class,
JsxNoConstructedContextValuesCheck.class,
JsxNoLeakedRenderCheck.class,
JsxNoUselessFragmentCheck.class,
JumpStatementInFinallyCheck.class,
LabelPlacementCheck.class,
LabelledStatementCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.javascript.checks;

import java.util.List;
import org.sonar.check.Rule;
import org.sonar.plugins.javascript.api.EslintBasedCheck;
import org.sonar.plugins.javascript.api.JavaScriptRule;
import org.sonar.plugins.javascript.api.TypeScriptRule;

@JavaScriptRule
@TypeScriptRule
@Rule(key = "S6749")
public class JsxNoUselessFragmentCheck implements EslintBasedCheck {

@Override
public String eslintKey() {
return "jsx-no-useless-fragment";
}

@Override
public List<Object> configurations() {
return List.of(new Config());
}

private static class Config {

boolean allowExpressions = true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<h2>Why is this an issue?</h2>
<p>React fragments are a feature in React that allows you to group multiple elements together without adding an extra DOM element. They are a way to
return multiple elements from a component’s render method without requiring a wrapping parent element.</p>
<p>However, a fragment is redundant if it contains only one child, or if it is the child of an HTML element.</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
&lt;&gt;&lt;Foo /&gt;&lt;/&gt;; // Noncompliant: The fragment has only one child
&lt;p&gt;&lt;&gt;foo&lt;/&gt;&lt;/p&gt;; // Noncompliant: The fragment is the child of the HTML element 'p'
</pre>
<p>You can safely remove the redundant fragment while preserving the original behaviour.</p>
<pre data-diff-id="1" data-diff-type="compliant">
&lt;Foo /&gt;;
&lt;p&gt;foo&lt;/p&gt;;
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://react.dev/reference/react/Fragment">React - Fragments</a> </li>
</ul>

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"title": "Redundant React fragments should be removed",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"react"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-6749",
"sqKey": "S6749",
"scope": "All",
"quickfix": "covered",
"code": {
"impacts": {
"MAINTAINABILITY": "HIGH",
"RELIABILITY": "MEDIUM",
"SECURITY": "LOW"
},
"attribute": "CONVENTIONAL"
},
"compatibleLanguages": [
"JAVASCRIPT",
"TYPESCRIPT"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@
"S6676",
"S6679",
"S6746",
"S6747"
"S6747",
"S6749"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.javascript.checks;

import static org.assertj.core.api.Assertions.assertThat;

import com.google.gson.Gson;
import org.junit.jupiter.api.Test;

class JsxNoUselessFragmentCheckTest {

@Test
void test() {
var configAsString = new Gson().toJson(new JsxNoUselessFragmentCheck().configurations());
assertThat(configAsString).isEqualTo("[{\"allowExpressions\":true}]");
}
}