SONARJAVA-6758: Fix FPs in S5673 for controllers without mappings and redundant annotations

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

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

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

@@ -1,9 +1,14 @@
 package checks.spring;
 
+import org.springframework.boot.ApplicationRunner;
+import org.springframework.boot.CommandLineRunner;
 import org.springframework.stereotype.Component;
 import org.springframework.stereotype.Service;
 import org.springframework.stereotype.Repository;
 import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 
 public class SpringComponentSpecializationCheckSample {
@@ -40,28 +45,90 @@ public class OrderDao {
   public class CustomerDao {
   }
 
-  // RestController patterns
+  // RestController patterns - with request mapping methods
 
   @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}}
   public class FooBarRestController {
+    @GetMapping("/foo")
+    public String foo() { return "foo"; }
   }
 
   @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}}
   public class ApiRestController {
+    @RequestMapping("/api")
+    public String api() { return "api"; }
   }
 
   @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}}
   public class UserRestControllerImpl {
+    @PostMapping("/users")
+    public void createUser() { }
   }
 
-  // Controller patterns
+  // Controller patterns - with request mapping methods
 
   @Component // Noncompliant {{Use @Controller instead of @Component, or rename this type if the @Component annotation is intentional}}
   public class HomeController {
+    @GetMapping("/home")
+    public String home() { return "home"; }
   }
 
   @Component // Noncompliant {{Use @Controller instead of @Component, or rename this type if the @Component annotation is intentional}}
   public class LoginControllerImpl {
+    @PostMapping("/login")
+    public String login() { return "login"; }
+  }
+
+  // Compliant - Controllers without request mapping methods (FP fix)
+
+  @Component
+  public class BatchController {
+  }
+
+  @Component
+  public class DataProcessingController {
+    public void process() { }
+  }
+
+  @Component
+  public class SchedulerRestController {
+    public void runTask() { }
+  }
+
+  // Compliant - Controllers implementing non-web framework interfaces
+
+  @Component
+  public class StartupController implements ApplicationRunner {
+    @Override
+    public void run(org.springframework.boot.ApplicationArguments args) { }
+  }
+
+  @Component
+  public class InitController implements CommandLineRunner {
+    @Override
+    public void run(String... args) { }
+  }
+
+  // Compliant - Redundant annotation: @Component alongside a specialized stereotype
+
+  @Component
+  @Service
+  public class RedundantServiceAnnotation {
+  }
+
+  @Component
+  @Controller
+  public class RedundantControllerAnnotation {
+  }
+
+  @Component
+  @RestController
+  public class RedundantRestControllerAnnotation {
+  }
+
+  @Component
+  @Repository
+  public class RedundantRepositoryAnnotation {
   }
 
   // Compliant - Correct annotations used
@@ -115,12 +182,14 @@ public class userservice {
   public class USERREPOSITORY {
   }
 
-  @Component // Noncompliant {{Use @Controller instead of @Component, or rename this type if the @Component annotation is intentional}}
+  @Component
   public class maincontroller {
+    // Compliant - no request mapping methods
   }
 
-  @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}}
+  @Component
   public class apirestcontroller {
+    // Compliant - no request mapping methods
   }
 
   // Interface patterns

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

@@ -18,17 +18,45 @@
 
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import javax.annotation.CheckForNull;
 import org.sonar.check.Rule;
 import org.sonar.java.checks.helpers.SpringUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.semantic.Type;
 import org.sonar.plugins.java.api.tree.AnnotationTree;
 import org.sonar.plugins.java.api.tree.ClassTree;
+import org.sonar.plugins.java.api.tree.MethodTree;
 import org.sonar.plugins.java.api.tree.Tree;
 
 @Rule(key = "S5673")
 public class SpringComponentSpecializationCheck extends IssuableSubscriptionVisitor {
 
+  private static final Set<String> SPECIALIZED_STEREOTYPE_ANNOTATIONS = Set.of(
+    SpringUtils.CONTROLLER_ANNOTATION,
+    SpringUtils.REST_CONTROLLER_ANNOTATION,
+    SpringUtils.SERVICE_ANNOTATION,
+    SpringUtils.REPOSITORY_ANNOTATION);
+
+  private static final List<String> REQUEST_MAPPING_ANNOTATIONS = List.of(
+    "org.springframework.web.bind.annotation.RequestMapping",
+    "org.springframework.web.bind.annotation.GetMapping",
+    "org.springframework.web.bind.annotation.PostMapping",
+    "org.springframework.web.bind.annotation.PutMapping",
+    "org.springframework.web.bind.annotation.DeleteMapping",
+    "org.springframework.web.bind.annotation.PatchMapping");
+
+  private static final List<String> NON_WEB_FRAMEWORK_INTERFACES = List.of(
+    "org.springframework.boot.ApplicationRunner",
+    "org.springframework.boot.CommandLineRunner",
+    "org.springframework.boot.actuate.health.HealthIndicator",
+    "org.springframework.boot.actuate.health.ReactiveHealthIndicator");
+
+  private static final List<String> NON_WEB_FRAMEWORK_ANNOTATIONS = List.of(
+    "org.springframework.boot.actuate.endpoint.annotation.Endpoint",
+    "org.springframework.boot.actuate.endpoint.web.annotation.RestControllerEndpoint",
+    "org.springframework.boot.actuate.endpoint.web.annotation.ControllerEndpoint");
+
   @Override
   public List<Tree.Kind> nodesToVisit() {
     return List.of(Tree.Kind.CLASS, Tree.Kind.INTERFACE);
@@ -46,14 +74,61 @@
       return;
     }
 
+    if (hasSpecializedStereotypeAnnotation(classTree)) {
+      return;
+    }
+
     String className = classTree.simpleName().name();
     String suggestedAnnotation = getSuggestedAnnotation(className);
 
-    if (suggestedAnnotation != null) {
+    if (suggestedAnnotation != null && shouldRaise(suggestedAnnotation, classTree)) {
       reportIssue(componentAnnotation.get(), String.format("Use @%s instead of @Component, or rename this type if the @Component annotation is intentional", suggestedAnnotation));
     }
   }
 
+  private static boolean hasSpecializedStereotypeAnnotation(ClassTree classTree) {
+    return classTree.modifiers().annotations().stream()
+      .anyMatch(a -> SPECIALIZED_STEREOTYPE_ANNOTATIONS.contains(a.annotationType().symbolType().fullyQualifiedName()));
+  }
+
+  private static boolean shouldRaise(String suggestedAnnotation, ClassTree classTree) {
+    if ("Controller".equals(suggestedAnnotation) || "RestController".equals(suggestedAnnotation)) {
+      return hasRequestMappingMethod(classTree) && !implementsNonWebFrameworkInterface(classTree) && !hasNonWebFrameworkAnnotation(classTree);
+    }
+    return true;
+  }
+
+  private static boolean hasRequestMappingMethod(ClassTree classTree) {
+    for (Tree member : classTree.members()) {
+      if (member instanceof MethodTree method) {
+        for (AnnotationTree annotation : method.modifiers().annotations()) {
+          if (REQUEST_MAPPING_ANNOTATIONS.contains(annotation.annotationType().symbolType().fullyQualifiedName())) {
+            return true;
+          }
+        }
+      }
+    }
+    return false;
+  }
+
+  private static boolean implementsNonWebFrameworkInterface(ClassTree classTree) {
+    Type classType = classTree.symbol().type();
+    if (classType == null) {
+      return false;
+    }
+    for (String interfaceFqn : NON_WEB_FRAMEWORK_INTERFACES) {
+      if (classType.isSubtypeOf(interfaceFqn)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private static boolean hasNonWebFrameworkAnnotation(ClassTree classTree) {
+    return classTree.modifiers().annotations().stream()
+      .anyMatch(a -> NON_WEB_FRAMEWORK_ANNOTATIONS.contains(a.annotationType().symbolType().fullyQualifiedName()));
+  }
+
   @CheckForNull
   private static String getSuggestedAnnotation(String className) {
     // Check RestController first to avoid false matches with Controller