SONARJAVA-4648 Implement S6810: Async methods should return void or F…

…uture (#4482)

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

@@ -180,15 +180,15 @@ public void javaCheckTestSources() throws Exception {
     assertThat(newTotal).isEqualTo(knownTotal);
     assertThat(rulesCausingFPs).hasSize(6);
     assertThat(rulesNotReporting).hasSize(7);
-    assertThat(rulesSilenced).hasSize(68);
+    assertThat(rulesSilenced).hasSize(69);
 
     /**
      * 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"));
-    assertThat(differences).isEqualTo("Issues differences: 3299");
+    assertThat(differences).isEqualTo("Issues differences: 3304");
   }
 
   private static Path pathFor(String path) {

its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json

@@ -2872,5 +2872,11 @@
     "hasTruePositives": true,
     "falseNegatives": 0,
     "falsePositives": 0
+  },
+  {
+    "ruleKey": "S6810",
+    "hasTruePositives": false,
+    "falseNegatives": 5,
+    "falsePositives": 0
   }
 ]

java-checks-test-sources/src/main/java/checks/spring/AsyncMethodsReturnTypeCheckSample.java

@@ -0,0 +1,77 @@
+package checks.spring;
+
+import com.google.common.util.concurrent.ListenableFuture;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Future;
+import org.springframework.scheduling.annotation.Async;
+
+public class AsyncMethodsReturnTypeCheckSample {
+  @Async
+  String asyncString() { // Noncompliant [[sc=3;ec=9]] {{Async methods should return 'void' or a 'Future' type.}}
+    return null;
+  }
+
+  @Async
+  int asyncInt() { // Noncompliant
+    return 0;
+  }
+
+  @Async
+  void asyncVoid() { // Compliant
+  }
+
+  @Async
+  Future<String> asyncFutureString() { // Compliant
+    return null;
+  }
+
+  @Async
+  ListenableFuture<String> asyncListenableFutureString() { // Compliant
+    return null;
+  }
+
+  @Async
+  CompletableFuture<String> asyncCompletableFutureString() { // Compliant
+    return null;
+  }
+
+  String synchronousMethod() { // Compliant
+    return null;
+  }
+
+  @Async
+  <T> T generic() { // Noncompliant
+    return null;
+  }
+
+  @Async
+  <T extends URL> T genericExtNotFuture() { // Noncompliant
+    return null;
+  }
+
+  @Async
+  <T extends Future<?>> T genericExtFuture() { // Compliant
+    return null;
+  }
+
+  @Async
+  <T extends CompletableFuture<?>> T genericExtCompletableFuture() { // Compliant
+    return null;
+  }
+}
+
+class MyTypeOfList<T> extends ArrayList<T> {
+  @Async
+  T doSomething(int unused) { // Noncompliant
+    return null;
+  }
+}
+
+class MyTypeOfListExtFuture<T extends Future<?>> extends ArrayList<T> {
+  @Async
+  T doSomething(int unused) { // Compliant, as T extends Future
+    return null;
+  }
+}

java-checks/src/main/java/org/sonar/java/checks/CheckList.java

@@ -135,6 +135,7 @@
 import org.sonar.java.checks.serialization.SerializableFieldInSerializableClassCheck;
 import org.sonar.java.checks.serialization.SerializableObjectInSessionCheck;
 import org.sonar.java.checks.serialization.SerializableSuperConstructorCheck;
+import org.sonar.java.checks.spring.AsyncMethodsReturnTypeCheck;
 import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck;
 import org.sonar.java.checks.spring.PersistentEntityUsedAsRequestParameterCheck;
 import org.sonar.java.checks.spring.RequestMappingMethodPublicCheck;
@@ -273,6 +274,7 @@ public final class CheckList {
     AssertOnBooleanVariableCheck.class,
     AssertionsInProductionCodeCheck.class,
     AssertsOnParametersOfPublicMethodCheck.class,
+    AsyncMethodsReturnTypeCheck.class,
     AtLeastOneConstructorCheck.class,
     AuthorizationsStrongDecisionsCheck.class,
     AwsConsumerBuilderUsageCheck.class,

java-checks/src/main/java/org/sonar/java/checks/spring/AsyncMethodsReturnTypeCheck.java

@@ -0,0 +1,48 @@
+/*
+ * 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.MethodTree;
+import org.sonar.plugins.java.api.tree.Tree;
+
+@Rule(key = "S6810")
+public class AsyncMethodsReturnTypeCheck extends IssuableSubscriptionVisitor {
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.METHOD);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    var mt = (MethodTree) tree;
+    if (mt.symbol().metadata().isAnnotatedWith("org.springframework.scheduling.annotation.Async")) {
+      var returnType = mt.returnType();
+      // returnType can only be null if the method is a constructor. Since the @Async annotation is not allowed on constructors, and since
+      // we hence only visit methods, not constructors, we assume that returnType is not null.
+      if (!returnType.symbolType().isVoid() && !returnType.symbolType().isSubtypeOf("java.util.concurrent.Future")) {
+        reportIssue(returnType, "Async methods should return 'void' or a 'Future' type.");
+      }
+    }
+  }
+}

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

@@ -0,0 +1,39 @@
+<h2>Why is this an issue?</h2>
+<p>The Spring framework provides the annotation <code>Async</code> to mark a method (or all methods of a type) as a candidate for asynchronous
+execution.</p>
+<p>Asynchronous methods do not necessarily, by their nature, return the result of their calculation immediately. Hence, it is unexpected and in clear
+breach of the <code>Async</code> contract for such methods to have a return type that is neither <code>void</code> nor a <code>Future</code> type.</p>
+<h2>How to fix it</h2>
+<p>Use <code>void</code> as the return type if the method is not expected to return a result. Otherwise, a <code>Future</code> should be returned,
+allowing the caller to retrieve the result once it is ready. It is permitted to return more specific subtypes that inherit from
+<code>Future</code>.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+@Async
+public String asyncMethod() {
+  ...
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+@Async
+public Future&lt;String&gt; asyncMethod() {
+  ...
+}
+</pre>
+<p>Alternatively, if the method does not need to return a result:</p>
+<pre>
+@Async
+public void asyncMethod() {
+  ...
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Spring Framework Documentation - <a
+  href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/scheduling/annotation/Async.html">Annotation Interface
+  Async</a> </li>
+</ul>
+

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6810.json

@@ -0,0 +1,21 @@
+{
+  "title": "Async methods should return void or Future",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6810",
+  "sqKey": "S6810",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "RELIABILITY": "MEDIUM"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json

@@ -479,6 +479,7 @@
     "S6485",
     "S6539",
     "S6541",
-    "S6548"
+    "S6548",
+    "S6810"
   ]
 }

java-checks/src/test/java/org/sonar/java/checks/spring/AsyncMethodsReturnTypeCheckTest.java

@@ -0,0 +1,35 @@
+/*
+ * 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 AsyncMethodsReturnTypeCheckTest {
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/spring/AsyncMethodsReturnTypeCheckSample.java"))
+      .withCheck(new AsyncMethodsReturnTypeCheck())
+      .verifyIssues();
+  }
+}