From 746e2324fc315a074eb7e642ea318535767dfdb5 Mon Sep 17 00:00:00 2001 From: Yassin Kammoun <52890329+yassin-kammoun-sonarsource@users.noreply.github.com> Date: Tue, 12 Sep 2023 10:22:01 +0200 Subject: [PATCH] Add rule S6754 (`hook-use-state`): The return value of "useState" should be destructured and named symmetrically (#4152) --- .../js/jira-clone/javascript-S6754.json | 31 +++++++ .../react-cloud-music/javascript-S6754.json | 5 ++ .../ts/courselit/typescript-S6754.json | 8 ++ .../expected/ts/eigen/typescript-S6754.json | 86 +++++++++++++++++++ .../ts/searchkit/typescript-S6754.json | 8 ++ packages/jsts/src/linter/quickfixes/rules.ts | 1 + .../wrapper/quickfixes/hook-use-state.jsx | 5 ++ .../sonar/javascript/checks/CheckList.java | 1 + .../javascript/checks/HookUseStateCheck.java | 48 +++++++++++ .../javascript/rules/javascript/S6754.html | 36 ++++++++ .../javascript/rules/javascript/S6754.json | 27 ++++++ .../rules/javascript/Sonar_way_profile.json | 1 + .../checks/HookUseStateCheckTest.java | 34 ++++++++ 13 files changed, 291 insertions(+) create mode 100644 its/ruling/src/test/expected/js/jira-clone/javascript-S6754.json create mode 100644 its/ruling/src/test/expected/js/react-cloud-music/javascript-S6754.json create mode 100644 its/ruling/src/test/expected/ts/courselit/typescript-S6754.json create mode 100644 its/ruling/src/test/expected/ts/eigen/typescript-S6754.json create mode 100644 its/ruling/src/test/expected/ts/searchkit/typescript-S6754.json create mode 100644 packages/jsts/tests/linter/fixtures/wrapper/quickfixes/hook-use-state.jsx create mode 100644 sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/HookUseStateCheck.java create mode 100644 sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6754.html create mode 100644 sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6754.json create mode 100644 sonar-plugin/javascript-checks/src/test/java/org/sonar/javascript/checks/HookUseStateCheckTest.java diff --git a/its/ruling/src/test/expected/js/jira-clone/javascript-S6754.json b/its/ruling/src/test/expected/js/jira-clone/javascript-S6754.json new file mode 100644 index 00000000000..d55107e3117 --- /dev/null +++ b/its/ruling/src/test/expected/js/jira-clone/javascript-S6754.json @@ -0,0 +1,31 @@ +{ +"jira-clone:client/src/Project/Board/IssueDetails/Comments/Comment/index.jsx": [ +27, +28 +], +"jira-clone:client/src/Project/Board/IssueDetails/Comments/Create/index.jsx": [ +18, +19 +], +"jira-clone:client/src/Project/Board/IssueDetails/Description/index.jsx": [ +16 +], +"jira-clone:client/src/shared/components/ConfirmModal/index.jsx": [ +36 +], +"jira-clone:client/src/shared/components/CopyLinkButton.jsx": [ +7 +], +"jira-clone:client/src/shared/components/DatePicker/index.jsx": [ +26 +], +"jira-clone:client/src/shared/components/Modal/index.jsx": [ +44 +], +"jira-clone:client/src/shared/components/Select/Dropdown.jsx": [ +47 +], +"jira-clone:client/src/shared/components/Select/index.jsx": [ +71 +] +} diff --git a/its/ruling/src/test/expected/js/react-cloud-music/javascript-S6754.json b/its/ruling/src/test/expected/js/react-cloud-music/javascript-S6754.json new file mode 100644 index 00000000000..c3cbc4eb77a --- /dev/null +++ b/its/ruling/src/test/expected/js/react-cloud-music/javascript-S6754.json @@ -0,0 +1,5 @@ +{ +"react-cloud-music:src/application/Player/index.jsx": [ +25 +] +} diff --git a/its/ruling/src/test/expected/ts/courselit/typescript-S6754.json b/its/ruling/src/test/expected/ts/courselit/typescript-S6754.json new file mode 100644 index 00000000000..21933ee1555 --- /dev/null +++ b/its/ruling/src/test/expected/ts/courselit/typescript-S6754.json @@ -0,0 +1,8 @@ +{ +"courselit:apps/web/components/admin/media/index.tsx": [ +59 +], +"courselit:apps/web/components/public/lesson-viewer.tsx": [ +80 +] +} diff --git a/its/ruling/src/test/expected/ts/eigen/typescript-S6754.json b/its/ruling/src/test/expected/ts/eigen/typescript-S6754.json new file mode 100644 index 00000000000..46568d3e742 --- /dev/null +++ b/its/ruling/src/test/expected/ts/eigen/typescript-S6754.json @@ -0,0 +1,86 @@ +{ +"eigen:src/app/Components/Artist/ArtistArtworks/ArtistArtworks.tsx": [ +41 +], +"eigen:src/app/Components/Gene/GeneArtworks.tsx": [ +89 +], +"eigen:src/app/Components/HeaderArtworksFilter/HeaderArtworksFilter.tsx": [ +59 +], +"eigen:src/app/Components/PopoverMessage/PopoverMessageProvider.tsx": [ +29, +30 +], +"eigen:src/app/Components/StickyTabPage/StickyTabPageTabBar.tsx": [ +24 +], +"eigen:src/app/Components/Tag/TagArtworks.tsx": [ +91 +], +"eigen:src/app/Components/Toast/ToastComponent.tsx": [ +38 +], +"eigen:src/app/Scenes/About/About.tsx": [ +16 +], +"eigen:src/app/Scenes/ArtistSeries/ArtistSeries.tsx": [ +33 +], +"eigen:src/app/Scenes/Artwork/Components/CommercialButtons/CollapsibleArtworkDetails.tsx": [ +41 +], +"eigen:src/app/Scenes/Collection/Components/CollectionArtworksFilter.tsx": [ +18 +], +"eigen:src/app/Scenes/Fair/Fair.tsx": [ +59 +], +"eigen:src/app/Scenes/Fair/FairAllFollowedArtists.tsx": [ +33 +], +"eigen:src/app/Scenes/Home/Components/EmailConfirmationBanner.tsx": [ +17, +18 +], +"eigen:src/app/Scenes/Inbox/Components/Conversations/CTAPopUp.tsx": [ +5 +], +"eigen:src/app/Scenes/MyCollection/Screens/ArtworkForm/Components/Rarity.tsx": [ +13 +], +"eigen:src/app/Scenes/MyCollection/Screens/Insights/AverageSalePriceAtAuction.tsx": [ +18 +], +"eigen:src/app/Scenes/Sale/Sale.tsx": [ +83, +84 +], +"eigen:src/app/Scenes/Search/SearchArtworksGrid.tsx": [ +34 +], +"eigen:src/app/navigation/NavStack.tsx": [ +27 +], +"eigen:src/app/utils/LinkedAccounts/apple.ts": [ +15 +], +"eigen:src/app/utils/LinkedAccounts/facebook.ts": [ +11 +], +"eigen:src/app/utils/LinkedAccounts/google.ts": [ +16 +], +"eigen:src/app/utils/useGlobalState.ts": [ +4 +], +"eigen:src/palette/elements/Select/Select.tsx": [ +228 +], +"eigen:src/palette/elements/Select/SelectV2.tsx": [ +186 +], +"eigen:src/palette/organisms/screenStructure/Screen.tsx": [ +36 +] +} diff --git a/its/ruling/src/test/expected/ts/searchkit/typescript-S6754.json b/its/ruling/src/test/expected/ts/searchkit/typescript-S6754.json new file mode 100644 index 00000000000..6f3aeac2eb5 --- /dev/null +++ b/its/ruling/src/test/expected/ts/searchkit/typescript-S6754.json @@ -0,0 +1,8 @@ +{ +"searchkit:packages/searchkit-client/src/searchkit.tsx": [ +243 +], +"searchkit:packages/searchkit-sdk/src/react-hooks/index.ts": [ +9 +] +} diff --git a/packages/jsts/src/linter/quickfixes/rules.ts b/packages/jsts/src/linter/quickfixes/rules.ts index 98fe04848db..34a0e201ef0 100644 --- a/packages/jsts/src/linter/quickfixes/rules.ts +++ b/packages/jsts/src/linter/quickfixes/rules.ts @@ -77,6 +77,7 @@ export const quickFixRules = new Set([ 'prefer-while', // eslint-plugin-react + 'hook-use-state', 'jsx-no-useless-fragment', 'no-unknown-property', diff --git a/packages/jsts/tests/linter/fixtures/wrapper/quickfixes/hook-use-state.jsx b/packages/jsts/tests/linter/fixtures/wrapper/quickfixes/hook-use-state.jsx new file mode 100644 index 00000000000..fe6bf126c52 --- /dev/null +++ b/packages/jsts/tests/linter/fixtures/wrapper/quickfixes/hook-use-state.jsx @@ -0,0 +1,5 @@ +import { useState } from 'react'; +function useSomething() { + const [state, , setState] = useState(); + return [state, setState]; +} diff --git a/sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java b/sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java index e3df3b44343..a8fac927df1 100644 --- a/sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java +++ b/sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java @@ -181,6 +181,7 @@ public static List> getAllChecks() { HardcodedCredentialsCheck.class, HashingCheck.class, HiddenFilesCheck.class, + HookUseStateCheck.class, IdenticalExpressionOnBinaryOperatorCheck.class, IdenticalFunctionsCheck.class, IgnoredReturnCheck.class, diff --git a/sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/HookUseStateCheck.java b/sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/HookUseStateCheck.java new file mode 100644 index 00000000000..415e1dc136d --- /dev/null +++ b/sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/HookUseStateCheck.java @@ -0,0 +1,48 @@ +/** + * 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.Collections; +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 = "S6754") +public class HookUseStateCheck implements EslintBasedCheck { + + @Override + public String eslintKey() { + return "hook-use-state"; + } + + @Override + public List configurations() { + return Collections.singletonList(new Config()); + } + + static class Config { + + boolean allowDestructuredState = true; + } +} diff --git a/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6754.html b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6754.html new file mode 100644 index 00000000000..ec59eff0014 --- /dev/null +++ b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6754.html @@ -0,0 +1,36 @@ +

Why is this an issue?

+

In React, useState is a hook that allows functional components to manage and update state in a manner similar to class components. +When you use the useState hook, it returns an array with two values: the current state value and a function to update that state +value.

+

Destructuring these values and naming them symmetrically (i.e., using consistent variable names for both the current state and the update function) +is a recommended best practice:

+ +
+import { useState } from 'react';
+function MyComponent() {
+  const [count, update] = useState(0); // Noncompliant
+  return <div onClick={() => update(count + 1)}>{count}</div>
+}
+
+

You should destructure the return value of useState calls in terms of the current state and a function to update that state and name +them symmetrically.

+
+import { useState } from 'react';
+function MyComponent() {
+  const [count, setCount] = useState(0);
+  return <div onClick={() => setCount(count + 1)}>{count}</div>
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6754.json b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6754.json new file mode 100644 index 00000000000..469987731d5 --- /dev/null +++ b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6754.json @@ -0,0 +1,27 @@ +{ + "title": "The return value of \"useState\" should be destructured and named symmetrically", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-6754", + "sqKey": "S6754", + "scope": "All", + "quickfix": "covered", + "code": { + "impacts": { + "MAINTAINABILITY": "HIGH", + "RELIABILITY": "MEDIUM", + "SECURITY": "LOW" + }, + "attribute": "CONVENTIONAL" + }, + "compatibleLanguages": [ + "JAVASCRIPT", + "TYPESCRIPT" + ] +} diff --git a/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json index fd702494c2b..16d3e7fdf09 100644 --- a/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json +++ b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json @@ -279,6 +279,7 @@ "S6748", "S6749", "S6750", + "S6754", "S6756" ] } diff --git a/sonar-plugin/javascript-checks/src/test/java/org/sonar/javascript/checks/HookUseStateCheckTest.java b/sonar-plugin/javascript-checks/src/test/java/org/sonar/javascript/checks/HookUseStateCheckTest.java new file mode 100644 index 00000000000..cddb451c9ee --- /dev/null +++ b/sonar-plugin/javascript-checks/src/test/java/org/sonar/javascript/checks/HookUseStateCheckTest.java @@ -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 HookUseStateCheckTest { + + @Test + void configurations() { + String configAsString = new Gson().toJson(new HookUseStateCheck().configurations()); + assertThat(configAsString).isEqualTo("[{\"allowDestructuredState\":true}]"); + } +}