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 c84acf3a80e..1bae480882f 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 { 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) @@ -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")); - assertThat(differences).isEqualTo("Issues differences: 3299"); + assertThat(differences).isEqualTo("Issues differences: 3304"); } private static Path pathFor(String path) { 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 ab90634d05a..ddb7d2732f4 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 @@ -2872,5 +2872,11 @@ "hasTruePositives": true, "falseNegatives": 0, "falsePositives": 0 + }, + { + "ruleKey": "S6810", + "hasTruePositives": false, + "falseNegatives": 5, + "falsePositives": 0 } ] diff --git a/java-checks-test-sources/src/main/java/checks/spring/AsyncMethodsReturnTypeCheckSample.java b/java-checks-test-sources/src/main/java/checks/spring/AsyncMethodsReturnTypeCheckSample.java new file mode 100644 index 00000000000..daf062f8a66 --- /dev/null +++ b/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 asyncFutureString() { // Compliant + return null; + } + + @Async + ListenableFuture asyncListenableFutureString() { // Compliant + return null; + } + + @Async + CompletableFuture asyncCompletableFutureString() { // Compliant + return null; + } + + String synchronousMethod() { // Compliant + return null; + } + + @Async + T generic() { // Noncompliant + return null; + } + + @Async + T genericExtNotFuture() { // Noncompliant + return null; + } + + @Async + > T genericExtFuture() { // Compliant + return null; + } + + @Async + > T genericExtCompletableFuture() { // Compliant + return null; + } +} + +class MyTypeOfList extends ArrayList { + @Async + T doSomething(int unused) { // Noncompliant + return null; + } +} + +class MyTypeOfListExtFuture> extends ArrayList { + @Async + T doSomething(int unused) { // Compliant, as T extends Future + return null; + } +} 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 e8b681e9a09..bf1e5617109 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 @@ -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, diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AsyncMethodsReturnTypeCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AsyncMethodsReturnTypeCheck.java new file mode 100644 index 00000000000..513dd0df26f --- /dev/null +++ b/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 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."); + } + } + } +} diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6810.html b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6810.html new file mode 100644 index 00000000000..865f773c56a --- /dev/null +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6810.html @@ -0,0 +1,39 @@ +

Why is this an issue?

+

The Spring framework provides the annotation Async to mark a method (or all methods of a type) as a candidate for asynchronous +execution.

+

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 Async contract for such methods to have a return type that is neither void nor a Future type.

+

How to fix it

+

Use void as the return type if the method is not expected to return a result. Otherwise, a Future 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 +Future.

+

Code examples

+

Noncompliant code example

+
+@Async
+public String asyncMethod() {
+  ...
+}
+
+

Compliant solution

+
+@Async
+public Future<String> asyncMethod() {
+  ...
+}
+
+

Alternatively, if the method does not need to return a result:

+
+@Async
+public void asyncMethod() {
+  ...
+}
+
+

Resources

+

Documentation

+ + diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6810.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6810.json new file mode 100644 index 00000000000..dc37a72d071 --- /dev/null +++ b/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" + } +} 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 7f16d90f134..3ace38ad440 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 @@ -479,6 +479,7 @@ "S6485", "S6539", "S6541", - "S6548" + "S6548", + "S6810" ] } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AsyncMethodsReturnTypeCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AsyncMethodsReturnTypeCheckTest.java new file mode 100644 index 00000000000..256aee7d5a9 --- /dev/null +++ b/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(); + } +}