From ee3763451bd93878996596be65ffd53434490b80 Mon Sep 17 00:00:00 2001 From: Johann Beleites Date: Tue, 31 Oct 2023 18:06:40 +0100 Subject: [PATCH] =?UTF-8?q?SONARJAVA-4683=20Implement=20S6837:=20Superfluo?= =?UTF-8?q?us=20@ResponseBody=20annotations=E2=80=A6=20(#4512)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * SONARJAVA-4683 Implement S6837: Superfluous @ResponseBody annotations should be removed --- .../java/org/sonar/java/it/AutoScanTest.java | 4 +- .../autoscan/autoscan-diff-by-rules.json | 8 ++- ...uousResponseBodyAnnotationCheckSample.java | 61 +++++++++++++++++++ .../java/org/sonar/java/checks/CheckList.java | 2 + ...uperfluousResponseBodyAnnotationCheck.java | 51 ++++++++++++++++ .../org/sonar/l10n/java/rules/java/S6837.html | 58 ++++++++++++++++++ .../org/sonar/l10n/java/rules/java/S6837.json | 23 +++++++ .../java/rules/java/Sonar_way_profile.json | 3 +- ...fluousResponseBodyAnnotationCheckTest.java | 44 +++++++++++++ 9 files changed, 250 insertions(+), 4 deletions(-) create mode 100644 java-checks-test-sources/src/main/java/checks/spring/SuperfluousResponseBodyAnnotationCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/spring/SuperfluousResponseBodyAnnotationCheck.java create mode 100644 java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.html create mode 100644 java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.json create mode 100644 java-checks/src/test/java/org/sonar/java/checks/spring/SuperfluousResponseBodyAnnotationCheckTest.java diff --git a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java index fd4dbbb7f2c..8d692ed46a5 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java @@ -180,7 +180,7 @@ public void javaCheckTestSources() throws Exception { softly.assertThat(newTotal).isEqualTo(knownTotal); softly.assertThat(rulesCausingFPs).hasSize(6); softly.assertThat(rulesNotReporting).hasSize(7); - softly.assertThat(rulesSilenced).hasSize(75); + softly.assertThat(rulesSilenced).hasSize(76); /** * 4. Check total number of differences (FPs + FNs) @@ -188,7 +188,7 @@ public void javaCheckTestSources() throws Exception { * No differences would mean that we find the same issues with and without the bytecode and libraries */ String differences = Files.readString(pathFor(TARGET_ACTUAL + PROJECT_KEY + "-no-binaries_differences")); - softly.assertThat(differences).isEqualTo("Issues differences: 3478"); + softly.assertThat(differences).isEqualTo("Issues differences: 3481"); softly.assertAll(); } diff --git a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json index c521820f991..9ff2b366610 100644 --- a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -1718,7 +1718,7 @@ { "ruleKey": "S3751", "hasTruePositives": false, - "falseNegatives": 12, + "falseNegatives": 13, "falsePositives": 0 }, { @@ -2920,5 +2920,11 @@ "hasTruePositives": false, "falseNegatives": 5, "falsePositives": 0 + }, + { + "ruleKey": "S6837", + "hasTruePositives": false, + "falseNegatives": 2, + "falsePositives": 0 } ] diff --git a/java-checks-test-sources/src/main/java/checks/spring/SuperfluousResponseBodyAnnotationCheckSample.java b/java-checks-test-sources/src/main/java/checks/spring/SuperfluousResponseBodyAnnotationCheckSample.java new file mode 100644 index 00000000000..9086616dd12 --- /dev/null +++ b/java-checks-test-sources/src/main/java/checks/spring/SuperfluousResponseBodyAnnotationCheckSample.java @@ -0,0 +1,61 @@ +package checks.spring; + +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class SuperfluousResponseBodyAnnotationCheckSample { + @ResponseBody // Noncompliant [[sc=3;ec=16]] {{Remove this superfluous "@ResponseBody" annotation.}} + @GetMapping("foo") + public String get() { + return "Hello world!"; + } + + @ResponseBody // Noncompliant + @GetMapping("bar") + private String get2() { + return "Hello world!"; + } + + @GetMapping("baz") + public String get3() { + return "Hello world!"; + } + + class InnerClass { + @ResponseBody // Compliant + @GetMapping("foo") + public String get() { + return "Hello world!"; + } + } +} + +@Controller +class RegularController { + @ResponseBody // Compliant + @GetMapping("foo") + public String get() { + return "Hello world!"; + } + + @GetMapping("baz") + public String get3() { + return "Hello world!"; + } +} + +class NotAController { + @ResponseBody // Compliant + @GetMapping("foo") + public String get() { + return "Hello world!"; + } + + @GetMapping("baz") + public String get3() { + return "Hello world!"; + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/CheckList.java b/java-checks/src/main/java/org/sonar/java/checks/CheckList.java index 961a6451c82..7f3f2dd14a7 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CheckList.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CheckList.java @@ -158,6 +158,7 @@ import org.sonar.java.checks.spring.SpringScanDefaultPackageCheck; import org.sonar.java.checks.spring.SpringSecurityDisableCSRFCheck; import org.sonar.java.checks.spring.SpringSessionFixationCheck; +import org.sonar.java.checks.spring.SuperfluousResponseBodyAnnotationCheck; import org.sonar.java.checks.spring.TransactionalMethodVisibilityCheck; import org.sonar.java.checks.spring.ValueAnnotationShouldInjectPropertyOrSpELCheck; import org.sonar.java.checks.synchronization.DoubleCheckedLockingCheck; @@ -644,6 +645,7 @@ public final class CheckList { StrongCipherAlgorithmCheck.class, SubClassStaticReferenceCheck.class, SuperfluousCurlyBraceCheck.class, + SuperfluousResponseBodyAnnotationCheck.class, SuppressWarningsCheck.class, SuspiciousListRemoveCheck.class, SwitchCaseTooBigCheck.class, diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SuperfluousResponseBodyAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SuperfluousResponseBodyAnnotationCheck.java new file mode 100644 index 00000000000..9ace6330b13 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SuperfluousResponseBodyAnnotationCheck.java @@ -0,0 +1,51 @@ +/* + * SonarQube Java + * Copyright (C) 2012-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.java.checks.spring; + +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S6837") +public class SuperfluousResponseBodyAnnotationCheck extends IssuableSubscriptionVisitor { + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + var ct = (ClassTree) tree; + if (!ct.symbol().metadata().isAnnotatedWith("org.springframework.web.bind.annotation.RestController")) { + return; + } + + ct.members().stream().filter(member -> member.is(Tree.Kind.METHOD)).forEach(member -> { + var mt = (MethodTree) member; + mt.modifiers().annotations().stream() + .filter(annotationTree -> annotationTree.symbolType().is("org.springframework.web.bind.annotation.ResponseBody")) + .findFirst() + .ifPresent(annotationTree -> reportIssue(annotationTree, "Remove this superfluous \"@ResponseBody\" annotation.")); + }); + } +} diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.html b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.html new file mode 100644 index 00000000000..a7cc135e47d --- /dev/null +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.html @@ -0,0 +1,58 @@ +

Why is this an issue?

+

The Spring framework’s @RestController annotation is equivalent to using the @Controller and @ResponseBody +annotations together. As such, it is redundant to add a @ResponseBody annotation when the class is already annotated with +@RestController.

+

How to fix it

+

Remove the @ResponseBody annotation from the class or method.

+

Code examples

+

Noncompliant code example

+
+@RestController
+public class MyController {
+  @ResponseBody // Noncompliant, the @RestController annotation already implies @ResponseBody
+  @RequestMapping("/hello")
+  public String hello() {
+    return "Hello World!";
+  }
+}
+
+

Compliant solution

+
+@RestController
+public class MyController {
+  @RequestMapping("/hello")
+  public String hello() {
+    return "Hello World!";
+  }
+}
+
+

Noncompliant code example

+
+@RestController
+@ResponseBody // Noncompliant, the @RestController annotation already implies @ResponseBody
+public class MyController {
+  @RequestMapping("/hello")
+  public String hello() {
+    return "Hello World!";
+  }
+}
+
+

Compliant solution

+
+@RestController
+public class MyController {
+  @RequestMapping("/hello")
+  public String hello() {
+    return "Hello World!";
+  }
+}
+
+

Resources

+

Articles & blog posts

+ + diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.json new file mode 100644 index 00000000000..aa910fc4598 --- /dev/null +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.json @@ -0,0 +1,23 @@ +{ + "title": "Superfluous \"@ResponseBody\" annotations should be removed", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "spring" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-6837", + "sqKey": "S6837", + "scope": "All", + "quickfix": "targeted", + "code": { + "impacts": { + "MAINTAINABILITY": "LOW" + }, + "attribute": "CLEAR" + } +} diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index 87e03b7dd2a..36077d9b942 100644 --- a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -487,6 +487,7 @@ "S6813", "S6814", "S6818", - "S6829" + "S6829", + "S6837" ] } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/SuperfluousResponseBodyAnnotationCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/SuperfluousResponseBodyAnnotationCheckTest.java new file mode 100644 index 00000000000..7096b77dcb4 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/SuperfluousResponseBodyAnnotationCheckTest.java @@ -0,0 +1,44 @@ +/* + * SonarQube Java + * Copyright (C) 2012-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.java.checks.spring; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class SuperfluousResponseBodyAnnotationCheckTest { + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/SuperfluousResponseBodyAnnotationCheckSample.java")) + .withCheck(new SuperfluousResponseBodyAnnotationCheck()) + .verifyIssues(); + } + + @Test + void test_no_semantics() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/SuperfluousResponseBodyAnnotationCheckSample.java")) + .withCheck(new SuperfluousResponseBodyAnnotationCheck()) + .withoutSemantic() + .verifyNoIssues(); + } +}