SONARJAVA-4683 Implement S6837: Superfluous @responsebody annotations… (#4512)

* SONARJAVA-4683 Implement S6837: Superfluous @responsebody annotations should be removed

Author: johann-beleites-sonarsource

its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java

@@ -180,15 +180,15 @@ 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)
      *
      * 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();
   }

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
   }
 ]

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!";
+  }
+}

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,

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<Tree.Kind> 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."));
+    });
+  }
+}

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6837.html

@@ -0,0 +1,58 @@
+<h2>Why is this an issue?</h2>
+<p>The Spring framework’s <code>@RestController</code> annotation is equivalent to using the <code>@Controller</code> and <code>@ResponseBody</code>
+annotations together. As such, it is redundant to add a <code>@ResponseBody</code> annotation when the class is already annotated with
+<code>@RestController</code>.</p>
+<h2>How to fix it</h2>
+<p>Remove the <code>@ResponseBody</code> annotation from the class or method.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+@RestController
+public class MyController {
+  @ResponseBody // Noncompliant, the @RestController annotation already implies @ResponseBody
+  @RequestMapping("/hello")
+  public String hello() {
+    return "Hello World!";
+  }
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+@RestController
+public class MyController {
+  @RequestMapping("/hello")
+  public String hello() {
+    return "Hello World!";
+  }
+}
+</pre>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+@RestController
+@ResponseBody // Noncompliant, the @RestController annotation already implies @ResponseBody
+public class MyController {
+  @RequestMapping("/hello")
+  public String hello() {
+    return "Hello World!";
+  }
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+@RestController
+public class MyController {
+  @RequestMapping("/hello")
+  public String hello() {
+    return "Hello World!";
+  }
+}
+</pre>
+<h2>Resources</h2>
+<h3>Articles &amp; blog posts</h3>
+<ul>
+  <li> Spring Guides - <a href="https://spring.io/guides/gs/rest-service/">Building a RESTful Web Service</a> </li>
+  <li> Baeldung - <a href="https://www.baeldung.com/spring-controller-vs-restcontroller">The Spring @Controller and @RestController Annotations</a>
+  </li>
+  <li> Baeldung - <a href="https://www.baeldung.com/spring-request-response-body">Spring’s RequestBody and ResponseBody Annotations</a> </li>
+</ul>
+

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"
+  }
+}

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"
   ]
 }

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();
+  }
+}