SONARJAVA-4926 Implement new rule S8989

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

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

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

@@ -0,0 +1,15 @@
+package checks.spring;
+
+import org.springframework.transaction.annotation.Transactional;
+
+class TransactionalMethodCheckedExceptionCheckSampleNonCompiling {
+
+  // Test unknown/unresolved exception type
+  @Transactional
+  public void unknownException() throws UnresolvedCheckedException { // No issue - type is unknown
+  }
+
+  @Transactional // Noncompliant
+  public void knownException() throws java.io.IOException {
+  }
+}

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

@@ -0,0 +1,172 @@
+package checks.spring;
+
+import java.io.IOException;
+import java.sql.SQLException;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import org.springframework.transaction.annotation.Transactional;
+
+// Composed/meta-annotation for testing
+@Retention(RetentionPolicy.RUNTIME)
+@Transactional
+@interface MyTransactional {
+}
+
+class Order {}
+class NotificationException extends Exception {}
+class CustomCheckedException extends Exception {}
+
+public class TransactionalMethodCheckedExceptionCheckSample {
+
+  @Transactional // Noncompliant {{Specify rollback behavior for checked exceptions using "rollbackFor" or "noRollbackFor" attributes.}} [[quickfixes=qf1,qf2]]
+//^^^^^^^^^^^^^^
+  // fix@qf1 {{Add rollbackFor attribute}}
+  // edit@qf1 [[sc=3;ec=17]] {{@Transactional(rollbackFor = {java.io.IOException.class, java.sql.SQLException.class})}}
+  // fix@qf2 {{Add rollbackFor = Exception.class}}
+  // edit@qf2 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
+  public void processOrder(Order order) throws IOException, SQLException {
+  }
+
+  @Transactional // Noncompliant [[quickfixes=qf3]]
+//^^^^^^^^^^^^^^
+  // fix@qf3 {{Add rollbackFor = Exception.class}}
+  // edit@qf3 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
+  public void importData() throws Exception {
+  }
+
+  @Transactional(timeout = 30) // Noncompliant
+//^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  public void withOtherAttributes() throws SQLException {
+  }
+
+  @Transactional // Noncompliant [[quickfixes=qf7,qf8]]
+//^^^^^^^^^^^^^^
+  // fix@qf7 {{Add rollbackFor attribute}}
+  // edit@qf7 [[sc=3;ec=17]] {{@Transactional(rollbackFor = checks.spring.CustomCheckedException.class)}}
+  // fix@qf8 {{Add rollbackFor = Exception.class}}
+  // edit@qf8 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
+  public void customException() throws CustomCheckedException {
+  }
+
+  @Transactional // Noncompliant [[quickfixes=qf9,qf10]]
+//^^^^^^^^^^^^^^
+  // fix@qf9 {{Add rollbackFor attribute}}
+  // edit@qf9 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.io.IOException.class)}}
+  // fix@qf10 {{Add rollbackFor = Exception.class}}
+  // edit@qf10 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
+  public void mixedExceptions() throws IOException, RuntimeException {
+  }
+
+  @Transactional(rollbackFor = IOException.class)
+  public void withRollbackFor() throws IOException {
+  }
+
+  @Transactional(rollbackFor = {IOException.class, SQLException.class})
+  public void withMultipleRollbackFor() throws IOException, SQLException {
+  }
+
+  @Transactional(rollbackFor = Exception.class)
+  public void rollbackForAll() throws Exception {
+  }
+
+  @Transactional(noRollbackFor = NotificationException.class)
+  public void withNoRollbackFor() throws NotificationException {
+  }
+
+  @Transactional(rollbackFor = Exception.class, noRollbackFor = NotificationException.class)
+  public void withBoth() throws Exception {
+  }
+
+  @Transactional(rollbackForClassName = "java.io.IOException")
+  public void rollbackForClassName() throws IOException {
+  }
+
+  @Transactional(noRollbackForClassName = "checks.spring.NotificationException")
+  public void noRollbackForClassName() throws NotificationException {
+  }
+
+  @Transactional
+  public void noExceptions() {
+  }
+
+  @Transactional
+  public void uncheckedOnly() throws RuntimeException {
+  }
+
+  public void noAnnotation() throws IOException {
+  }
+
+  @Transactional(rollbackFor = Exception.class)
+  static class ClassLevelConfig {
+    public void inherited() throws IOException {
+    }
+  }
+
+  @Transactional // Noncompliant [[quickfixes=qf11,qf12]]
+//^^^^^^^^^^^^^^
+  // fix@qf11 {{Add rollbackFor attribute}}
+  // edit@qf11 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.io.IOException.class)}}
+  // fix@qf12 {{Add rollbackFor = Exception.class}}
+  // edit@qf12 [[sc=3;ec=17]] {{@Transactional(rollbackFor = java.lang.Exception.class)}}
+  static class ClassLevelNoConfig {
+    public void noConfig() throws IOException {
+    }
+
+    @Transactional(rollbackFor = IOException.class)
+    public void methodOverrides() throws IOException {
+    }
+  }
+
+  @Transactional
+  public void errorNotChecked() throws Error {
+  }
+
+  @org.springframework.transaction.annotation.Transactional // Noncompliant [[quickfixes=qf13,qf14]]
+//>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>^^^^^^^^^^^^^^^^^^^^^^^^
+  // fix@qf13 {{Add rollbackFor attribute}}
+  // edit@qf13 [[sc=3;ec=60]] {{@org.springframework.transaction.annotation.Transactional(rollbackFor = java.io.IOException.class)}}
+  // fix@qf14 {{Add rollbackFor = Exception.class}}
+  // edit@qf14 [[sc=3;ec=60]] {{@org.springframework.transaction.annotation.Transactional(rollbackFor = java.lang.Exception.class)}}
+  public void fullyQualified() throws IOException {
+  }
+
+  @Transactional(rollbackFor = IOException.class)
+  public void partialConfig() throws SQLException {
+  }
+
+  @Transactional(value = "txManager") // Noncompliant
+//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  public void withValueAttribute() throws IOException {
+  }
+
+  // Test nested structure to ensure parent traversal works
+  static class OuterClass {
+    @Transactional // Noncompliant
+    static class InnerClassWithAnnotation {
+      public void nestedMethod() throws IOException {
+      }
+    }
+
+    static class InnerClassNoAnnotation {
+      public void nestedMethodNoAnnotation() throws IOException {
+        // No @Transactional at any level, so no issue
+      }
+    }
+  }
+
+  @Transactional // Noncompliant
+  interface TransactionalInterface {
+    void interfaceMethod() throws IOException;
+  }
+
+  // Test meta-annotated (composed) annotation
+  @MyTransactional // Noncompliant
+  public void metaAnnotated() throws IOException {
+  }
+
+  // Test annotation with value attribute (transaction manager name)
+  @Transactional("txManager") // Noncompliant
+  public void valueShorthand() throws IOException {
+    // Has value attribute but no rollback configuration
+  }
+}

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

@@ -0,0 +1,197 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.checks.helpers.SpringUtils;
+import org.sonar.java.reporting.JavaQuickFix;
+import org.sonar.java.reporting.JavaTextEdit;
+import org.sonar.plugins.java.api.DependencyVersionAware;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.Version;
+import org.sonar.plugins.java.api.semantic.Type;
+import org.sonar.plugins.java.api.tree.AnnotationTree;
+import org.sonar.plugins.java.api.tree.Arguments;
+import org.sonar.plugins.java.api.tree.ClassTree;
+import org.sonar.plugins.java.api.tree.MethodTree;
+import org.sonar.plugins.java.api.tree.SyntaxToken;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.TypeTree;
+
+@Rule(key = "S8989")
+public class TransactionalMethodCheckedExceptionCheck extends IssuableSubscriptionVisitor implements DependencyVersionAware {
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return Collections.singletonList(Tree.Kind.METHOD);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    MethodTree method = (MethodTree) tree;
+
+    List<TypeTree> throwsClauses = method.throwsClauses();
+    if (throwsClauses.isEmpty()) {
+      return;
+    }
+
+    List<Type> checkedExceptions = throwsClauses.stream()
+      .map(TypeTree::symbolType)
+      .filter(TransactionalMethodCheckedExceptionCheck::isCheckedException)
+      .toList();
+
+    if (checkedExceptions.isEmpty()) {
+      return;
+    }
+
+    AnnotationTree transactionalAnnotation = getTransactionalAnnotation(method);
+    if (transactionalAnnotation == null) {
+      return;
+    }
+
+    // Check if the annotation tree itself has rollback configuration
+    if (hasRollbackConfiguration(transactionalAnnotation)) {
+      return;
+    }
+
+    QuickFixHelper.newIssue(context)
+      .forRule(this)
+      .onTree(transactionalAnnotation)
+      .withMessage("Specify rollback behavior for checked exceptions using \"rollbackFor\" or \"noRollbackFor\" attributes.")
+      .withQuickFixes(() -> computeQuickFixes(transactionalAnnotation, checkedExceptions))
+      .report();
+  }
+
+  private static AnnotationTree getTransactionalAnnotation(MethodTree method) {
+    // Check method-level annotation first (including meta-annotations)
+    for (AnnotationTree annotation : method.modifiers().annotations()) {
+      if (isTransactionalAnnotation(annotation)) {
+        return annotation;
+      }
+    }
+
+    // Check class-level annotation
+    Tree parent = method.parent();
+    while (parent != null && !parent.is(Tree.Kind.CLASS, Tree.Kind.INTERFACE, Tree.Kind.ENUM, Tree.Kind.RECORD)) {
+      parent = parent.parent();
+    }
+
+    if (parent instanceof ClassTree classTree) {
+      for (AnnotationTree annotation : classTree.modifiers().annotations()) {
+        if (isTransactionalAnnotation(annotation)) {
+          return annotation;
+        }
+      }
+    }
+
+    return null;
+  }
+
+  private static boolean isTransactionalAnnotation(AnnotationTree annotation) {
+    // Check if the annotation itself is @Transactional
+    if (annotation.symbolType().is(SpringUtils.TRANSACTIONAL_ANNOTATION)) {
+      return true;
+    }
+    // Check if the annotation is meta-annotated with @Transactional (composed annotation)
+    return annotation.symbolType().symbol().metadata().isAnnotatedWith(SpringUtils.TRANSACTIONAL_ANNOTATION);
+  }
+
+  private List<JavaQuickFix> computeQuickFixes(AnnotationTree annotation, List<Type> checkedExceptions) {
+    List<JavaQuickFix> quickFixes = new ArrayList<>();
+
+    // Quick fix 1: Add rollbackFor with all checked exceptions (using fully qualified names)
+    String exceptionsList = checkedExceptions.stream()
+      .map(Type::fullyQualifiedName)
+      .map(name -> name + ".class")
+      .collect(Collectors.joining(", "));
+
+    String rollbackForAttribute = (checkedExceptions.size() == 1)
+      ? ("rollbackFor = " + exceptionsList)
+      : ("rollbackFor = {" + exceptionsList + "}");
+
+    // Only add the specific exceptions quick fix if it's different from Exception.class
+    boolean isAlreadyExceptionClass = checkedExceptions.size() == 1
+      && "java.lang.Exception".equals(checkedExceptions.get(0).fullyQualifiedName());
+
+    if (!isAlreadyExceptionClass) {
+      quickFixes.add(createQuickFix(annotation, rollbackForAttribute, "Add rollbackFor attribute"));
+    }
+
+    // Quick fix 2: Add rollbackFor = Exception.class (covers all checked exceptions)
+    quickFixes.add(createQuickFix(annotation, "rollbackFor = java.lang.Exception.class", "Add rollbackFor = Exception.class"));
+
+    return quickFixes;
+  }
+
+  private JavaQuickFix createQuickFix(AnnotationTree annotation, String attribute, String description) {
+    Arguments arguments = annotation.arguments();
+
+    if (arguments.isEmpty()) {
+      // @Transactional -> @Transactional(rollbackFor = ...)
+      String annotationTypeName = QuickFixHelper.contentForTree(annotation.annotationType(), context);
+      String replacement = annotationTypeName + "(" + attribute + ")";
+      return JavaQuickFix.newQuickFix(description)
+        .addTextEdit(JavaTextEdit.replaceTree(annotation, "@" + replacement))
+        .build();
+    } else {
+      // @Transactional(timeout = 30) -> @Transactional(timeout = 30, rollbackFor = ...)
+      SyntaxToken closeParenToken = arguments.closeParenToken();
+      return JavaQuickFix.newQuickFix(description)
+        .addTextEdit(JavaTextEdit.insertBeforeTree(closeParenToken, ", " + attribute))
+        .build();
+    }
+  }
+
+  private static boolean isCheckedException(Type type) {
+    if (type.isUnknown()) {
+      return false;
+    }
+
+    return type.isSubtypeOf("java.lang.Exception")
+      && !type.isSubtypeOf("java.lang.RuntimeException")
+      && !type.isSubtypeOf("java.lang.Error");
+  }
+
+  private static boolean hasRollbackConfiguration(AnnotationTree annotation) {
+    return annotation.arguments().stream()
+      .anyMatch(arg -> {
+        if (arg.is(Tree.Kind.ASSIGNMENT)) {
+          var assignment = (org.sonar.plugins.java.api.tree.AssignmentExpressionTree) arg;
+          String name = ((org.sonar.plugins.java.api.tree.IdentifierTree) assignment.variable()).name();
+          return "rollbackFor".equals(name)
+            || "rollbackForClassName".equals(name)
+            || "noRollbackFor".equals(name)
+            || "noRollbackForClassName".equals(name);
+        }
+        return false;
+      });
+  }
+
+  @Override
+  public boolean isCompatibleWithDependencies(Function<String, Optional<Version>> dependencyFinder) {
+    Optional<Version> springContextVersion = dependencyFinder.apply("spring-context");
+    Optional<Version> springTxVersion = dependencyFinder.apply("spring-tx");
+    return springTxVersion.isPresent() || springContextVersion.isPresent();
+  }
+}