Skip to content

Commit

Permalink
Add rule S6749 (jsx-no-useless-fragment): Redundant React fragments…
Browse files Browse the repository at this point in the history
… should be removed (#4147)
  • Loading branch information
yassin-kammoun-sonarsource committed Sep 6, 2023
1 parent 1bf25e7 commit e44af61
Show file tree
Hide file tree
Showing 16 changed files with 301 additions and 15 deletions.
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}]");
}
}

0 comments on commit e44af61

Please sign in to comment.