SONARJAVA-5328 S7190 Before and After transaction methods should resp…

…ect the contract (#5018)

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

@@ -196,7 +196,7 @@ public void javaCheckTestSources() throws Exception {
     softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values());
     softly.assertThat(newTotal).isEqualTo(knownTotal);
     softly.assertThat(rulesCausingFPs).hasSize(10);
-    softly.assertThat(rulesNotReporting).hasSize(10);
+    softly.assertThat(rulesNotReporting).hasSize(11);
 
     /**
      * 4. Check total number of differences (FPs + FNs)

its/autoscan/src/test/resources/autoscan/diffs/diff_S7190.json

@@ -0,0 +1,6 @@
+{
+  "ruleKey": "S7190",
+  "hasTruePositives": false,
+  "falseNegatives": 0,
+  "falsePositives": 0
+}

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

@@ -0,0 +1,56 @@
+package checks.spring;
+
+import org.junit.jupiter.api.TestInfo;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.test.context.transaction.AfterTransaction;
+import org.springframework.test.context.transaction.BeforeTransaction;
+
+public class BeforeAndAfterTransactionContractCheckSample {
+
+  @BeforeTransaction
+//^^^^^^^^^^^^^^^^^^> {{Annotation}}
+  private String beforeTransaction() { // Noncompliant {{@BeforeTransaction method should return void.}}
+//        ^^^^^^
+    return "before";
+  }
+
+  @AfterTransaction
+  private String afterTransaction() { // Noncompliant {{@AfterTransaction method should return void.}}
+//        ^^^^^^
+    return "after";
+  }
+
+  @BeforeTransaction
+  public void beforeTransaction2(String name) { // Noncompliant {{@BeforeTransaction method should not have parameters.}}
+    assert name != null;
+  }
+
+  @AfterTransaction
+  public void afterTransaction2(String name, String surname) { // Noncompliant {{@AfterTransaction method should not have parameters.}}
+    //                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    assert name != null;
+    assert surname != null;
+  }
+
+  @BeforeTransaction
+  public void beforeTransaction5(@Autowired Object notAComponent) { // FN, not really a component, requires spring context
+    // ...
+  }
+
+  @BeforeTransaction
+  public void beforeTransaction3(TestInfo info) { // Compliant, jupiter TestInfo is allowed
+    // ...
+  }
+
+  @BeforeTransaction
+  public void beforeTransaction4(@Autowired MyService myService) { // Compliant, autowired components are allowed
+    // ...
+  }
+
+  @Service
+  class MyService {
+    // ...
+  }
+
+}

java-checks/src/main/java/org/sonar/java/checks/helpers/SpringUtils.java

@@ -17,11 +17,13 @@
 package org.sonar.java.checks.helpers;
 
 import java.util.List;
+import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.semantic.SymbolMetadata;
 
 public final class SpringUtils {
 
   public static final String SPRING_SCOPE_ANNOTATION = "org.springframework.context.annotation.Scope";
+  public static final String AUTOWIRED_ANNOTATION = "org.springframework.beans.factory.annotation.Autowired";
 
   private SpringUtils() {
     // Utils class
@@ -44,4 +46,8 @@ public static boolean isScopeSingleton(SymbolMetadata clazzMeta) {
     return true;
   }
 
+  public static boolean isAutowired(Symbol symbol) {
+    return symbol.metadata().isAnnotatedWith(AUTOWIRED_ANNOTATION);
+  }
+
 }

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

@@ -0,0 +1,90 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2025 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 Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks.spring;
+
+import java.util.List;
+import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.SpringUtils;
+import org.sonar.java.model.declaration.MethodTreeImpl;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.JavaFileScannerContext;
+import org.sonar.plugins.java.api.semantic.Symbol;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.VariableTree;
+
+@Rule(key = "S7190")
+public class BeforeAndAfterTransactionContractCheck extends IssuableSubscriptionVisitor {
+
+  private static final String BEFORE_TRANSACTION_FQN = "org.springframework.test.context.transaction.BeforeTransaction";
+  private static final String AFTER_TRANSACTION_FQN = "org.springframework.test.context.transaction.AfterTransaction";
+  private static final List<String> TRANSACTION_ANNOTATIONS = List.of(BEFORE_TRANSACTION_FQN, AFTER_TRANSACTION_FQN);
+
+  private static final String TEST_INFO_FQN = "org.junit.jupiter.api.TestInfo";
+
+  private static final String RETURN_VOID_MESSAGE = "%s method should return void.";
+  private static final String NO_PARAMETERS_MESSAGE = "%s method should not have parameters.";
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.METHOD);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    MethodTreeImpl methodTree = (MethodTreeImpl) tree;
+    if (methodTree.symbol().metadata().isAnnotatedWith(BEFORE_TRANSACTION_FQN)) {
+      checkReturnType(methodTree, "@BeforeTransaction");
+      checkParameters(methodTree, "@BeforeTransaction");
+    } else if (methodTree.symbol().metadata().isAnnotatedWith(AFTER_TRANSACTION_FQN)) {
+      checkReturnType(methodTree, "@AfterTransaction");
+      checkParameters(methodTree, "@AfterTransaction");
+    }
+  }
+
+  private void checkReturnType(MethodTreeImpl methodTree, String annotationName) {
+    if (!methodTree.returnType().symbolType().isVoid()) {
+      String message = String.format(RETURN_VOID_MESSAGE, annotationName);
+      reportIssue(methodTree.returnType(), message, getSecondaryLocations(methodTree), null);
+    }
+  }
+
+  private void checkParameters(MethodTreeImpl methodTree, String annotationName) {
+    List<VariableTree> parameters = methodTree.parameters();
+    if (!parameters.isEmpty() && parameters.stream().anyMatch(parameter -> !isParameterAllowed(parameter))) {
+      String message = String.format(NO_PARAMETERS_MESSAGE, annotationName);
+      var first = methodTree.parameters().get(0);
+      var last = methodTree.parameters().get(methodTree.parameters().size() - 1);
+      reportIssue(first, last, message, getSecondaryLocations(methodTree), null);
+    }
+  }
+
+  private static boolean isParameterAllowed(VariableTree parameter) {
+    Symbol parameterSymbol = parameter.symbol();
+    if (parameterSymbol.type().is(TEST_INFO_FQN)) {
+      return true;
+    }
+    return SpringUtils.isAutowired(parameterSymbol);
+  }
+
+  private static List<JavaFileScannerContext.Location> getSecondaryLocations(MethodTreeImpl methodTree) {
+    return methodTree.modifiers().annotations().stream()
+      .filter(annotation -> TRANSACTION_ANNOTATIONS.contains(annotation.symbolType().fullyQualifiedName()))
+      .map(annotation -> new JavaFileScannerContext.Location("Annotation", annotation))
+      .toList();
+  }
+
+}

java-checks/src/test/java/org/sonar/java/checks/helpers/SpringUtilsTest.java

@@ -0,0 +1,50 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2025 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 Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks.helpers;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.java.model.declaration.ClassTreeImpl;
+import org.sonar.java.model.declaration.VariableTreeImpl;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class SpringUtilsTest {
+
+  @Test
+  void is_autowired() {
+    var cu = JParserTestUtils.parse("""
+      class A {
+        @org.springframework.beans.factory.annotation.Autowired
+        Object autowiredObject;
+        
+        @Autowired
+        Object noSemaAnnotation;
+
+        @javax.annotation.Nullable
+        Object nullableObject;
+      }
+      """);
+    var clazz = (ClassTreeImpl) cu.types().get(0);
+    var obj = (VariableTreeImpl) clazz.members().get(0);
+    assertThat(SpringUtils.isAutowired(obj.symbol())).isTrue();
+    var goo = (VariableTreeImpl) clazz.members().get(1);
+    assertThat(SpringUtils.isAutowired(goo.symbol())).isFalse();
+    var hoo = (VariableTreeImpl) clazz.members().get(2);
+    assertThat(SpringUtils.isAutowired(hoo.symbol())).isFalse();
+  }
+
+}

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

@@ -0,0 +1,46 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2025 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 Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+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 BeforeAndAfterTransactionContractCheckTest {
+
+  private static final String SAMPLE_FILE = mainCodeSourcesPath("checks/spring/BeforeAndAfterTransactionContractCheckSample.java");
+  private static final BeforeAndAfterTransactionContractCheck CHECK = new BeforeAndAfterTransactionContractCheck();
+
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(SAMPLE_FILE)
+      .withCheck(CHECK)
+      .verifyIssues();
+  }
+
+  @Test
+  void test_no_semantics() {
+    CheckVerifier.newVerifier()
+      .onFile(SAMPLE_FILE)
+      .withCheck(CHECK)
+      .withoutSemantic()
+      .verifyNoIssues();
+  }
+
+}

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S7190.html

@@ -0,0 +1,47 @@
+<h2>Why is this an issue?</h2>
+<p>In tests configured with Spring’s <code>@Transactional</code> annotation, methods annotated with <code>@BeforeTransaction</code> or
+<code>@AfterTransaction</code> must be void and have no arguments. These methods are executed before or after a transaction, respectively. Deviating
+from this contract by having a non-void return type or accepting arguments will cause Spring to throw a runtime error.</p>
+<h2>How to fix it</h2>
+<p>Ensure that methods annotated with <code>@BeforeTransaction</code> or <code>@AfterTransaction</code> have a void return type and do not accept any
+arguments.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+public class TransactionalTest {
+
+    @BeforeTransaction
+    public String setupTransaction(int x) { // non-compliant, method should be void and have no argument
+        // Setup logic
+    }
+
+    @AfterTransaction
+    public int cleanupTransaction(int x) { // non-compliant, method should be void and have no argument
+        // Cleanup logic
+    }
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+public class TransactionalTest {
+
+    @BeforeTransaction
+    public void setupTransaction() {
+        // Setup logic
+    }
+
+    @AfterTransaction
+    public void cleanupTransaction() {
+        // Cleanup logic
+    }
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Spring - <a
+  href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/test/context/transaction/BeforeTransaction.html">BeforeTransaction</a> </li>
+  <li> Spring - <a
+  href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/test/context/transaction/AfterTransaction.html">AfterTransaction</a> </li>
+</ul>
+

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S7190.json

@@ -0,0 +1,25 @@
+{
+  "title": "Methods annotated with \"@BeforeTransaction\" or \"@AfterTransaction\" must respect the contract",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "spring", "tests"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-7190",
+  "sqKey": "S7190",
+  "scope": "Tests",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "LOW",
+      "RELIABILITY": "HIGH",
+      "SECURITY": "LOW"
+    },
+    "attribute": "LOGICAL"
+  }
+}

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

@@ -500,6 +500,7 @@
     "S7183",
     "S7184",
     "S7185",
-    "S7186"
+    "S7186",
+    "S7190"
   ]
 }