From e70fd0fd2ad9b134ef5a4af85b9a5ffeb8d1b96b Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Fri, 14 Aug 2026 08:26:03 +0200 Subject: [PATCH 1/3] SONARJAVA-6758: Fix FPs in S5673 for controllers without mappings and redundant annotations - Require request mapping annotations before suggesting @Controller/@RestController - Skip raising when a specialized stereotype annotation is already present alongside @Component - Exclude classes implementing non-web framework interfaces (ApplicationRunner, CommandLineRunner, HealthIndicator) - Exclude classes annotated with actuator endpoint annotations Co-Authored-By: Claude Opus 4.6 --- .../resources/autoscan/diffs/diff_S5673.json | 2 +- ...ingComponentSpecializationCheckSample.java | 77 ++++++++++++++++++- .../SpringComponentSpecializationCheck.java | 77 ++++++++++++++++++- 3 files changed, 150 insertions(+), 6 deletions(-) diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5673.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5673.json index 559fd7787c9..07775157976 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5673.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5673.json @@ -1,6 +1,6 @@ { "ruleKey": "S5673", "hasTruePositives": false, - "falseNegatives": 20, + "falseNegatives": 17, "falsePositives": 0 } \ No newline at end of file diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java index 8672679f351..8f3ccead08e 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java +++ b/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 diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java index a91e398f42b..8e3a39f2f6a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java +++ b/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 SPECIALIZED_STEREOTYPE_ANNOTATIONS = Set.of( + SpringUtils.CONTROLLER_ANNOTATION, + SpringUtils.REST_CONTROLLER_ANNOTATION, + SpringUtils.SERVICE_ANNOTATION, + SpringUtils.REPOSITORY_ANNOTATION); + + private static final List 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 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 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 nodesToVisit() { return List.of(Tree.Kind.CLASS, Tree.Kind.INTERFACE); @@ -46,14 +74,61 @@ public void visitNode(Tree tree) { 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 From c7196a7996e48577a1ea13504ed0007bec892340 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Fri, 14 Aug 2026 10:10:05 +0200 Subject: [PATCH 2/3] SONARJAVA-6758: Fix Quality Gate issues in S5673 Extract duplicated "Controller" and "RestController" string literals into constants to fix S1192 issues. Add test cases for HealthIndicator, ReactiveHealthIndicator, and @Endpoint to improve coverage on new code above 90% threshold. Co-Authored-By: Claude Opus 4.6 --- java-checks-test-sources/default/pom.xml | 6 ++++ ...ingComponentSpecializationCheckSample.java | 31 +++++++++++++++++++ .../SpringComponentSpecializationCheck.java | 13 +++++--- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/java-checks-test-sources/default/pom.xml b/java-checks-test-sources/default/pom.xml index 4a13f6cb24b..ae5a0c951ce 100644 --- a/java-checks-test-sources/default/pom.xml +++ b/java-checks-test-sources/default/pom.xml @@ -356,6 +356,12 @@ 2.5.15 provided + + org.springframework.boot + spring-boot-actuator + 2.0.2.RELEASE + provided + org.springframework.security spring-security-crypto diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java index 8f3ccead08e..bd981231a65 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java @@ -2,6 +2,9 @@ import org.springframework.boot.ApplicationRunner; import org.springframework.boot.CommandLineRunner; +import org.springframework.boot.actuate.endpoint.annotation.Endpoint; +import org.springframework.boot.actuate.health.HealthIndicator; +import org.springframework.boot.actuate.health.ReactiveHealthIndicator; import org.springframework.stereotype.Component; import org.springframework.stereotype.Service; import org.springframework.stereotype.Repository; @@ -109,6 +112,34 @@ public class InitController implements CommandLineRunner { public void run(String... args) { } } + // Compliant - Controllers with request mappings but implementing HealthIndicator + @Component + public class HealthCheckController implements HealthIndicator { + @GetMapping("/health") + public String healthStatus() { return "UP"; } + + @Override + public org.springframework.boot.actuate.health.Health health() { return null; } + } + + // Compliant - Controllers with request mappings but implementing ReactiveHealthIndicator + @Component + public class ReactiveHealthCheckController implements ReactiveHealthIndicator { + @GetMapping("/health/reactive") + public String reactiveHealthStatus() { return "UP"; } + + @Override + public reactor.core.publisher.Mono health() { return null; } + } + + // Compliant - Controllers with request mappings but annotated with @Endpoint + @Component + @Endpoint(id = "custom") + public class CustomEndpointController { + @GetMapping("/custom") + public String custom() { return "custom"; } + } + // Compliant - Redundant annotation: @Component alongside a specialized stereotype @Component diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java index 8e3a39f2f6a..dea306b2fbc 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java @@ -52,6 +52,9 @@ public class SpringComponentSpecializationCheck extends IssuableSubscriptionVisi "org.springframework.boot.actuate.health.HealthIndicator", "org.springframework.boot.actuate.health.ReactiveHealthIndicator"); + private static final String CONTROLLER = "Controller"; + private static final String REST_CONTROLLER = "RestController"; + private static final List NON_WEB_FRAMEWORK_ANNOTATIONS = List.of( "org.springframework.boot.actuate.endpoint.annotation.Endpoint", "org.springframework.boot.actuate.endpoint.web.annotation.RestControllerEndpoint", @@ -92,7 +95,7 @@ private static boolean hasSpecializedStereotypeAnnotation(ClassTree classTree) { } private static boolean shouldRaise(String suggestedAnnotation, ClassTree classTree) { - if ("Controller".equals(suggestedAnnotation) || "RestController".equals(suggestedAnnotation)) { + if (CONTROLLER.equals(suggestedAnnotation) || REST_CONTROLLER.equals(suggestedAnnotation)) { return hasRequestMappingMethod(classTree) && !implementsNonWebFrameworkInterface(classTree) && !hasNonWebFrameworkAnnotation(classTree); } return true; @@ -132,12 +135,12 @@ private static boolean hasNonWebFrameworkAnnotation(ClassTree classTree) { @CheckForNull private static String getSuggestedAnnotation(String className) { // Check RestController first to avoid false matches with Controller - if (endsWithIgnoreCase(className, "RestController") || endsWithIgnoreCase(className, "RestControllerImpl")) { - return "RestController"; + if (endsWithIgnoreCase(className, REST_CONTROLLER) || endsWithIgnoreCase(className, REST_CONTROLLER + "Impl")) { + return REST_CONTROLLER; } - if (endsWithIgnoreCase(className, "Controller") || endsWithIgnoreCase(className, "ControllerImpl")) { - return "Controller"; + if (endsWithIgnoreCase(className, CONTROLLER) || endsWithIgnoreCase(className, CONTROLLER + "Impl")) { + return CONTROLLER; } if (endsWithIgnoreCase(className, "Service") || From ee3c24d74294312b16b2595b5f9e577adb5ea237 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 17 Aug 2026 09:48:19 +0200 Subject: [PATCH 3/3] SONARJAVA-6758: Walk superclass hierarchy for request mappings and add actuator endpoint tests Walk the superclass hierarchy in hasRequestMappingMethod() to detect inherited mapping annotations, fixing false negatives for @Component subclasses of base controllers with @GetMapping/@PostMapping methods. Add test cases for @RestControllerEndpoint and @ControllerEndpoint annotations to verify they are correctly excluded from the rule. Co-Authored-By: Claude Opus 4.6 --- ...ingComponentSpecializationCheckSample.java | 48 +++++++++++++++++++ .../SpringComponentSpecializationCheck.java | 12 +++++ 2 files changed, 60 insertions(+) diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java index bd981231a65..c2958acbd84 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java @@ -3,6 +3,8 @@ import org.springframework.boot.ApplicationRunner; import org.springframework.boot.CommandLineRunner; import org.springframework.boot.actuate.endpoint.annotation.Endpoint; +import org.springframework.boot.actuate.endpoint.web.annotation.ControllerEndpoint; +import org.springframework.boot.actuate.endpoint.web.annotation.RestControllerEndpoint; import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.boot.actuate.health.ReactiveHealthIndicator; import org.springframework.stereotype.Component; @@ -140,6 +142,52 @@ public class CustomEndpointController { public String custom() { return "custom"; } } + // Compliant - Controllers with request mappings but annotated with @RestControllerEndpoint + @Component + @RestControllerEndpoint(id = "restEndpoint") + public class ActuatorRestController { + @GetMapping("/actuator/rest") + public String restEndpoint() { return "rest"; } + } + + // Compliant - Controllers with request mappings but annotated with @ControllerEndpoint + @Component + @ControllerEndpoint(id = "controllerEndpoint") + public class ActuatorController { + @GetMapping("/actuator/controller") + public String controllerEndpoint() { return "controller"; } + } + + // Controllers with inherited request mapping methods + + public abstract class BaseRestController { + @GetMapping("/status") + public String status() { return "ok"; } + } + + @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}} + public class StatusRestController extends BaseRestController { + } + + public abstract class BaseController { + @PostMapping("/submit") + public String submit() { return "submitted"; } + } + + @Component // Noncompliant {{Use @Controller instead of @Component, or rename this type if the @Component annotation is intentional}} + public class FormController extends BaseController { + } + + // Compliant - Controller subclass without inherited mapping methods + + public abstract class BaseProcessController { + public void process() { } + } + + @Component + public class TaskController extends BaseProcessController { + } + // Compliant - Redundant annotation: @Component alongside a specialized stereotype @Component diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java index dea306b2fbc..48a46743c31 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java @@ -23,6 +23,7 @@ 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.Symbol; import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.AnnotationTree; import org.sonar.plugins.java.api.tree.ClassTree; @@ -111,9 +112,20 @@ private static boolean hasRequestMappingMethod(ClassTree classTree) { } } } + for (Type superType : classTree.symbol().superTypes()) { + if (hasRequestMappingMethodInSymbol(superType.symbol())) { + return true; + } + } return false; } + private static boolean hasRequestMappingMethodInSymbol(Symbol.TypeSymbol typeSymbol) { + return typeSymbol.memberSymbols().stream() + .filter(Symbol::isMethodSymbol) + .anyMatch(method -> REQUEST_MAPPING_ANNOTATIONS.stream().anyMatch(method.metadata()::isAnnotatedWith)); + } + private static boolean implementsNonWebFrameworkInterface(ClassTree classTree) { Type classType = classTree.symbol().type(); if (classType == null) {