SONARJAVA-6660 Fix S2229 FP for self-invocations inside TransactionTemplate callbacks

java-checks-test-sources/default/pom.xml

@@ -371,7 +371,7 @@
     <dependency>
       <groupId>org.springframework</groupId>
       <artifactId>spring-tx</artifactId>
-      <version>5.0.6.RELEASE</version>
+      <version>5.3.21</version>
       <scope>provided</scope>
     </dependency>
     <dependency>

java-checks-test-sources/default/src/main/java/checks/spring/SpringIncompatibleTransactionalCheckSample.java

@@ -3,6 +3,9 @@
 import javax.transaction.Transactional.TxType;
 import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
+import org.springframework.transaction.support.TransactionTemplate;
+
+import java.util.UUID;
 
 import static javax.transaction.Transactional.TxType.REQUIRED;
 
@@ -342,3 +345,35 @@ public void methodB() {
   }
 
 }
+
+class SpringIncompatibleTransactionalCheckSampleSupportTransactionTemplate {
+
+  private TransactionTemplate transactionTemplate;
+
+  @Transactional
+  public void retry(UUID id) {
+  }
+
+  @Transactional(propagation = Propagation.REQUIRES_NEW)
+  public void retryNew(UUID id) {
+  }
+
+  public void retryBatch(UUID id) {
+    // Possible FNs: calls inside a TransactionTemplate callback are suppressed because a transaction is guaranteed
+    // to be active at the call site. However, if the template's propagation is customized (e.g. NEVER or NOT_SUPPORTED),
+    // self-invocations that require a transaction would still be problematic. Resolving the template's propagation
+    // statically requires data-flow / symbolic-execution techniques and is out of scope for this rule.
+    transactionTemplate.executeWithoutResult(status -> {
+      retry(id); // compliant - inside a TransactionTemplate callback: a transaction is already active
+      retryNew(id); // compliant - self-invocation reports are suppressed inside TransactionTemplate callbacks
+    });
+    transactionTemplate.execute(status -> {
+      retry(id);
+      retryNew(id);
+      return id.toString();
+    });
+
+    retry(id); // Noncompliant {{"retry's" @Transactional requirement is incompatible with the one for this method.}} [[secondary=354]]
+    retryNew(id); // Noncompliant {{"retryNew's" @Transactional requirement is incompatible with the one for this method.}} [[secondary=358]]
+  }
+}

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

@@ -31,6 +31,7 @@
 import org.sonar.java.model.ExpressionUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
+import org.sonar.plugins.java.api.semantic.MethodMatchers;
 import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.semantic.SymbolMetadata;
 import org.sonar.plugins.java.api.semantic.SymbolMetadata.AnnotationValue;
@@ -59,6 +60,12 @@ public class SpringIncompatibleTransactionalCheck extends IssuableSubscriptionVi
   // Made name to represent no annotation
   private static final String NOT_TRANSACTIONAL = "SONAR_NOT_TRANSACTIONAL";
 
+  private static final MethodMatchers TRANSACTION_TEMPLATE_EXECUTE = MethodMatchers.create()
+    .ofTypes("org.springframework.transaction.support.TransactionTemplate")
+    .names("execute", "executeWithoutResult")
+    .withAnyParameters()
+    .build();
+
   private static final Map<String, Set<String>> INCOMPATIBLE_PROPAGATION_MAP = buildIncompatiblePropagationMap();
 
   private static Map<String, Set<String>> buildIncompatiblePropagationMap() {
@@ -96,14 +103,21 @@ private void checkMethodInvocations(MethodTree method, @Nullable String callerPr
       return;
     }
     methodBody.accept(new BaseTreeVisitor() {
+      private boolean insideTransactionTemplateCallback = false;
+
       @Override
       public void visitMethodInvocation(MethodInvocationTree methodInvocation) {
+        boolean previousState = insideTransactionTemplateCallback;
+        if (TRANSACTION_TEMPLATE_EXECUTE.matches(methodInvocation)) {
+          insideTransactionTemplateCallback = true;
+        }
         super.visitMethodInvocation(methodInvocation);
+        insideTransactionTemplateCallback = previousState;
         Symbol calleeMethodSymbol = methodInvocation.methodSymbol();
         if (calleeMethodSymbol.isUnknown()) {
           return;
         }
-        if (methodsPropagationMap.containsKey(calleeMethodSymbol) && methodInvocationOnThisInstance(methodInvocation)) {
+        if (!insideTransactionTemplateCallback && methodsPropagationMap.containsKey(calleeMethodSymbol) && methodInvocationOnThisInstance(methodInvocation)) {
           String calleePropagation = methodsPropagationMap.get(calleeMethodSymbol);
           checkIncompatiblePropagation(methodInvocation, callerPropagation, calleeMethodSymbol, calleePropagation);
         }