SONARJAVA-4651 extend rule S2230 with @Asynch annotations (#4484)

Author: erwan-serandour

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

@@ -52,8 +52,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.assertj.core.api.Assertions.assertThat;
-
 public class AutoScanTest {
 
   private static final Gson GSON = new GsonBuilder().setPrettyPrinting().create();
@@ -190,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: 3430");
+    softly.assertThat(differences).isEqualTo("Issues differences: 3446");
 
     softly.assertAll();
   }

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

@@ -1112,7 +1112,7 @@
   {
     "ruleKey": "S2230",
     "hasTruePositives": true,
-    "falseNegatives": 0,
+    "falseNegatives": 15,
     "falsePositives": 0
   },
   {
@@ -2882,7 +2882,7 @@
   {
     "ruleKey": "S6810",
     "hasTruePositives": false,
-    "falseNegatives": 5,
+    "falseNegatives": 6,
     "falsePositives": 0
   },
   {

java-checks-test-sources/src/main/files/non-compiling/checks/spring/TransactionalMethodVisibilityCheck.java

@@ -5,11 +5,11 @@
 class TransactionalMethodVisibilityCheck {
   // Cannot compile because a Transactional method should be overridable
   @org.springframework.transaction.annotation.Transactional
-  private void privateTransactionalMethod() {} // Noncompliant [[sc=16;ec=42]] {{Make this method "public" or remove the "@Transactional" annotation}}
+  private void privateTransactionalMethod() {} // Noncompliant [[sc=16;ec=42]] {{Make this method "public" or remove the "@Transactional" annotation.}}
 
   // Cannot compile because a Transactional method should be overridable
   @Transactional
-  private void privateTransactionalMethodWithImportBasedAnnotation() {} // Noncompliant {{Make this method "public" or remove the "@Transactional" annotation}}
+  private void privateTransactionalMethodWithImportBasedAnnotation() {} // Noncompliant {{Make this method "public" or remove the "@Transactional" annotation.}}
 
   @org.xxx.Transactional
   private void privateMethodWithNonSpringAnnotation() {} // Compliant

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

@@ -1,8 +1,10 @@
 package checks.spring;
 
+import java.util.concurrent.Future;
+import org.springframework.scheduling.annotation.Async;
 import org.springframework.transaction.annotation.Transactional;
 
-class TransactionalMethodVisibilityCheck {
+abstract class TransactionalMethodVisibilityCheck {
 
   public interface C {
     @Transactional
@@ -13,13 +15,32 @@ private interface B {
     @Transactional
     int bar(); // Compliant
   }
-  
+
+  private interface A {
+    @Async
+    int bar(); // Compliant
+  }
+
+  @Async
+  protected abstract Future<String> aMethod(); // Noncompliant
+
+
+  @Async
+  public Future<String> asyncMethod(){ // compliant
+    return  null;
+  }
+
+  @Async
+  private  Future<String> asyncMethodPrivate(){ // Noncompliant {{Make this method "public" or remove the "@Async" annotation.}}
+    return  null;
+  }
+
   @org.springframework.transaction.annotation.Transactional
   public void publicTransactionalMethod() {} // Compliant
 
   @org.springframework.transaction.annotation.Transactional
-  protected void protectedTransactionalMethod() {} // Noncompliant {{Make this method "public" or remove the "@Transactional" annotation}}
+  protected void protectedTransactionalMethod() {} // Noncompliant [[sc=18;ec=46]] {{Make this method "public" or remove the "@Transactional" annotation.}}
 
   @org.springframework.transaction.annotation.Transactional
-  void defaultVisibilityTransactionalMethod() {} // Noncompliant {{Make this method "public" or remove the "@Transactional" annotation}}
+  void defaultVisibilityTransactionalMethod() {} // Noncompliant  {{Make this method "public" or remove the "@Transactional" annotation.}}
 }

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

@@ -21,6 +21,7 @@
 
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import org.sonar.check.Rule;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.tree.AnnotationTree;
@@ -30,6 +31,14 @@
 @Rule(key = "S2230")
 public class TransactionalMethodVisibilityCheck extends IssuableSubscriptionVisitor {
 
+  private static final List<String> proxyAnnotations = List.of(
+    "org.springframework.transaction.annotation.Transactional",
+    "org.springframework.scheduling.annotation.Async");
+
+  private static final Map<String, String> annShortName = Map.of(
+    "org.springframework.transaction.annotation.Transactional", "@Transactional",
+    "org.springframework.scheduling.annotation.Async", "@Async");
+
   @Override
   public List<Tree.Kind> nodesToVisit() {
     return Collections.singletonList(Tree.Kind.METHOD);
@@ -38,14 +47,18 @@ public List<Tree.Kind> nodesToVisit() {
   @Override
   public void visitNode(Tree tree) {
     MethodTree method = (MethodTree) tree;
-    if (!method.symbol().isPublic() && hasTransactionalAnnotation(method)) {
-      reportIssue(method.simpleName(), "Make this method \"public\" or remove the \"@Transactional\" annotation");
+    if (!method.symbol().isPublic()) {
+      proxyAnnotations.stream()
+        .filter(annSymbol -> hasAnnotation(method, annSymbol))
+        .forEach(annSymbol -> reportIssue(
+          method.simpleName(),
+          "Make this method \"public\" or remove the \"" + annShortName.get(annSymbol) + "\" annotation."));
     }
   }
 
-  private static boolean hasTransactionalAnnotation(MethodTree method) {
+  private static boolean hasAnnotation(MethodTree method, String annotationSymbol) {
     for (AnnotationTree annotation : method.modifiers().annotations()) {
-      if (annotation.symbolType().is("org.springframework.transaction.annotation.Transactional")) {
+      if (annotation.symbolType().is(annotationSymbol)) {
         return true;
       }
     }