From 0affeb76ded9ee886e7f19fcd1cf29d69c60d899 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 17 Aug 2026 14:29:16 +0200 Subject: [PATCH 1/5] Implement new rule S9341 Detect redundant Spring annotations where a more specific composed annotation already implies the parent. Covers stereotype annotations (@Component with @Service/@Repository/@Controller/@Configuration), @RestController composition, @SpringBootApplication composition, and Spring test annotation redundancies. --- .../RedundantSpringAnnotationCheckSample.java | 184 ++++++++++++++++++ .../RedundantSpringAnnotationCheck.java | 170 ++++++++++++++++ .../RedundantSpringAnnotationCheckTest.java | 42 ++++ .../org/sonar/l10n/java/rules/java/S9341.html | 82 ++++++++ .../org/sonar/l10n/java/rules/java/S9341.json | 23 +++ .../main/resources/profiles/Sonar_way/S9341 | 0 6 files changed, 501 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheckTest.java create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9341.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9341.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9341 diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java new file mode 100644 index 00000000000..eb69d73fa9e --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java @@ -0,0 +1,184 @@ +package checks.spring; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; +import org.springframework.stereotype.Component; +import org.springframework.stereotype.Controller; +import org.springframework.stereotype.Repository; +import org.springframework.stereotype.Service; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.RestController; + +// === Stereotype redundancy === + +@Component // Noncompliant {{Remove this "@Component" annotation, already implied by "@Service".}} +@Service +class ComponentWithService { +} + +@Component // Noncompliant {{Remove this "@Component" annotation, already implied by "@Repository".}} +@Repository +class ComponentWithRepository { +} + +@Component // Noncompliant {{Remove this "@Component" annotation, already implied by "@Controller".}} +@Controller +class ComponentWithController { +} + +@Component // Noncompliant {{Remove this "@Component" annotation, already implied by "@Configuration".}} +@Configuration +class ComponentWithConfiguration { +} + +// === RestController composition === + +@Controller // Noncompliant {{Remove this "@Controller" annotation, already implied by "@RestController".}} +@RestController +class ControllerWithRestController { +} + +@ResponseBody // Noncompliant {{Remove this "@ResponseBody" annotation, already implied by "@RestController".}} +@RestController +class ResponseBodyWithRestController { +} + +// === SpringBootApplication composition === + +@Configuration // Noncompliant {{Remove this "@Configuration" annotation, already implied by "@SpringBootApplication".}} +@SpringBootApplication +class ConfigurationWithSpringBootApp { +} + +@EnableAutoConfiguration // Noncompliant {{Remove this "@EnableAutoConfiguration" annotation, already implied by "@SpringBootApplication".}} +@SpringBootApplication +class EnableAutoConfigWithSpringBootApp { +} + +@ComponentScan // Noncompliant {{Remove this "@ComponentScan" annotation, already implied by "@SpringBootApplication".}} +@SpringBootApplication +class ComponentScanWithSpringBootApp { +} + +@SpringBootConfiguration // Noncompliant {{Remove this "@SpringBootConfiguration" annotation, already implied by "@SpringBootApplication".}} +@SpringBootApplication +class SpringBootConfigWithSpringBootApp { +} + +// === Multiple redundant annotations on same class === + +@Configuration // Noncompliant {{Remove this "@Configuration" annotation, already implied by "@SpringBootApplication".}} +@EnableAutoConfiguration // Noncompliant {{Remove this "@EnableAutoConfiguration" annotation, already implied by "@SpringBootApplication".}} +@ComponentScan // Noncompliant {{Remove this "@ComponentScan" annotation, already implied by "@SpringBootApplication".}} +@SpringBootApplication +class AllRedundantWithSpringBootApp { +} + +// === Spring Test redundancy === + +@ExtendWith(SpringExtension.class) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@SpringBootTest".}} +@SpringBootTest +class ExtendWithSpringExtAndSpringBootTest { + @Test + void test() { + } +} + +@ExtendWith(SpringExtension.class) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@WebMvcTest".}} +@WebMvcTest +class ExtendWithSpringExtAndWebMvcTest { +} + +@ExtendWith(SpringExtension.class) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@DataJpaTest".}} +@DataJpaTest +class ExtendWithSpringExtAndDataJpaTest { +} + +@ExtendWith(SpringExtension.class) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@WebFluxTest".}} +@WebFluxTest +class ExtendWithSpringExtAndWebFluxTest { +} + +@Transactional // Noncompliant {{Remove this "@Transactional" annotation, already implied by "@DataJpaTest".}} +@DataJpaTest +class TransactionalWithDataJpaTest { +} + +@ExtendWith(SpringExtension.class) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@DataJpaTest".}} +@Transactional // Noncompliant {{Remove this "@Transactional" annotation, already implied by "@DataJpaTest".}} +@DataJpaTest +class MultipleRedundantWithDataJpaTest { +} + +// === Compliant cases === + +@Service +class ServiceAlone { +} + +@Component +class ComponentAlone { +} + +@RestController +class RestControllerAlone { +} + +@SpringBootApplication +class SpringBootAppAlone { +} + +@Controller +@ResponseBody +class ControllerWithResponseBody { + @GetMapping("/foo") + public String get() { + return "foo"; + } +} + +@SpringBootApplication +@ComponentScan(basePackages = "com.example.custom") +class SpringBootAppWithCustomComponentScan { +} + +@SpringBootTest +class SpringBootTestAlone { + @Test + void test() { + } +} + +@ExtendWith(org.mockito.junit.jupiter.MockitoExtension.class) +@SpringBootTest +class ExtendWithMockitoAndSpringBootTest { + @Test + void test() { + } +} + +@SpringBootTest +@Transactional +class TransactionalWithSpringBootTest { +} + +@DataJpaTest +class DataJpaTestAlone { +} + +@ExtendWith({SpringExtension.class, org.mockito.junit.jupiter.MockitoExtension.class}) +@SpringBootTest +class MixedExtensionsWithSpringBootTest { +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java new file mode 100644 index 00000000000..50bdfb40e8d --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java @@ -0,0 +1,170 @@ +/* + * 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.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.BiPredicate; +import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.QuickFixHelper; +import org.sonar.java.checks.helpers.SpringUtils; +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.semantic.SymbolMetadata; +import org.sonar.plugins.java.api.tree.AnnotationTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S9341") +public class RedundantSpringAnnotationCheck extends IssuableSubscriptionVisitor { + + private static final String RESPONSE_BODY = "org.springframework.web.bind.annotation.ResponseBody"; + private static final String ENABLE_AUTO_CONFIGURATION = "org.springframework.boot.autoconfigure.EnableAutoConfiguration"; + private static final String COMPONENT_SCAN = "org.springframework.context.annotation.ComponentScan"; + private static final String SPRING_BOOT_CONFIGURATION = "org.springframework.boot.SpringBootConfiguration"; + private static final String EXTEND_WITH = "org.junit.jupiter.api.extension.ExtendWith"; + private static final String SPRING_EXTENSION = "org.springframework.test.context.junit.jupiter.SpringExtension"; + private static final String WEB_MVC_TEST = "org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest"; + private static final String DATA_JPA_TEST = "org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest"; + private static final String WEB_FLUX_TEST = "org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest"; + + private static final List REDUNDANCY_RULES = List.of( + new RedundancyRule(SpringUtils.COMPONENT_ANNOTATION, + List.of(SpringUtils.SERVICE_ANNOTATION, SpringUtils.REPOSITORY_ANNOTATION, SpringUtils.CONTROLLER_ANNOTATION, SpringUtils.CONFIGURATION_ANNOTATION), null), + new RedundancyRule(SpringUtils.CONTROLLER_ANNOTATION, + List.of(SpringUtils.REST_CONTROLLER_ANNOTATION), null), + new RedundancyRule(RESPONSE_BODY, + List.of(SpringUtils.REST_CONTROLLER_ANNOTATION), null), + new RedundancyRule(SpringUtils.CONFIGURATION_ANNOTATION, + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), null), + new RedundancyRule(ENABLE_AUTO_CONFIGURATION, + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), null), + new RedundancyRule(COMPONENT_SCAN, + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), RedundantSpringAnnotationCheck::isComponentScanWithoutCustomAttributes), + new RedundancyRule(SPRING_BOOT_CONFIGURATION, + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), null), + new RedundancyRule(EXTEND_WITH, + List.of(SpringUtils.SPRING_BOOT_TEST_ANNOTATION, WEB_MVC_TEST, DATA_JPA_TEST, WEB_FLUX_TEST), + RedundantSpringAnnotationCheck::isExtendWithSpringExtension), + new RedundancyRule(SpringUtils.TRANSACTIONAL_ANNOTATION, + List.of(DATA_JPA_TEST), null) + ); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + var classTree = (ClassTree) tree; + Map annotationsByFqn = collectAnnotations(classTree); + + for (RedundancyRule rule : REDUNDANCY_RULES) { + AnnotationTree redundantAnnotation = annotationsByFqn.get(rule.redundantFqn); + if (redundantAnnotation == null) { + continue; + } + for (String impliedByFqn : rule.impliedByFqns) { + AnnotationTree impliedByAnnotation = annotationsByFqn.get(impliedByFqn); + if (impliedByAnnotation != null && passesSpecialCondition(rule, classTree, impliedByFqn)) { + reportRedundancy(redundantAnnotation, impliedByAnnotation); + break; + } + } + } + } + + private static Map collectAnnotations(ClassTree classTree) { + Map map = new HashMap<>(); + for (AnnotationTree annotation : classTree.modifiers().annotations()) { + String fqn = annotation.annotationType().symbolType().fullyQualifiedName(); + map.put(fqn, annotation); + } + return map; + } + + private static boolean passesSpecialCondition(RedundancyRule rule, ClassTree classTree, String impliedByFqn) { + if (rule.specialCondition == null) { + return true; + } + return rule.specialCondition.test(classTree, impliedByFqn); + } + + private void reportRedundancy(AnnotationTree redundantAnnotation, AnnotationTree impliedByAnnotation) { + String redundantName = simpleName(redundantAnnotation); + String impliedByName = simpleName(impliedByAnnotation); + QuickFixHelper.newIssue(context) + .forRule(this) + .onTree(redundantAnnotation) + .withMessage("Remove this \"@%s\" annotation, already implied by \"@%s\".", redundantName, impliedByName) + .withSecondaries(List.of( + new JavaFileScannerContext.Location("Already implied by this annotation.", impliedByAnnotation))) + .report(); + } + + private static String simpleName(AnnotationTree annotation) { + return annotation.annotationType().symbolType().name(); + } + + private static boolean isComponentScanWithoutCustomAttributes(ClassTree classTree, String impliedByFqn) { + SymbolMetadata metadata = classTree.symbol().metadata(); + List values = metadata.valuesForAnnotation(COMPONENT_SCAN); + if (values == null || values.isEmpty()) { + return true; + } + for (SymbolMetadata.AnnotationValue av : values) { + String name = av.name(); + if ("value".equals(name) || "basePackages".equals(name) || "basePackageClasses".equals(name)) { + return false; + } + } + return true; + } + + private static boolean isExtendWithSpringExtension(ClassTree classTree, String impliedByFqn) { + SymbolMetadata metadata = classTree.symbol().metadata(); + List values = metadata.valuesForAnnotation(EXTEND_WITH); + if (values == null) { + return false; + } + for (SymbolMetadata.AnnotationValue av : values) { + if (isOnlySpringExtensionClass(av.value())) { + return true; + } + } + return false; + } + + private static boolean isOnlySpringExtensionClass(Object value) { + if (value instanceof Symbol symbol) { + return symbol.type().is(SPRING_EXTENSION); + } + if (value instanceof Object[] values) { + // Skip mixed arrays like @ExtendWith({SpringExtension.class, MockitoExtension.class}) + // since the annotation cannot simply be removed + return values.length == 1 && values[0] instanceof Symbol symbol && symbol.type().is(SPRING_EXTENSION); + } + return false; + } + + private record RedundancyRule(String redundantFqn, List impliedByFqns, + BiPredicate specialCondition) { + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheckTest.java new file mode 100644 index 00000000000..2d07cf3609d --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheckTest.java @@ -0,0 +1,42 @@ +/* + * 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 org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class RedundantSpringAnnotationCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/RedundantSpringAnnotationCheckSample.java")) + .withCheck(new RedundantSpringAnnotationCheck()) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/RedundantSpringAnnotationCheckSample.java")) + .withCheck(new RedundantSpringAnnotationCheck()) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9341.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9341.html new file mode 100644 index 00000000000..7758173cf8f --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9341.html @@ -0,0 +1,82 @@ +

Why is this an issue?

+

Some Spring annotations are composed from other annotations through meta-annotation. When you use an annotation that already includes another +annotation's behavior, explicitly adding the parent annotation is redundant. It creates visual noise and may indicate a misunderstanding of the +framework's annotation composition model.

+

Common examples include:

+
    +
  • @RestController is meta-annotated with @Controller and @ResponseBody
    • +
  • @Service, @Repository, @Controller, and @Configuration are meta-annotated with + @Component
    • +
  • @SpringBootApplication is meta-annotated with @Configuration, @EnableAutoConfiguration, and + @ComponentScan
    • +
  • Spring test annotations like @SpringBootTest already include @ExtendWith(SpringExtension.class)
    • +
+

How to fix it in Spring

+

Remove the redundant parent annotation. Keep only the most specific annotation that provides the functionality you need.

+

Code examples

+

Noncompliant code example

+
+@Component // Noncompliant, @Service already implies @Component
+@Service
+public class UserService {
+}
+
+

Compliant solution

+
+@Service
+public class UserService {
+}
+
+

Noncompliant code example

+
+@Controller // Noncompliant, @RestController already implies @Controller
+@RestController
+public class UserController {
+}
+
+

Compliant solution

+
+@RestController
+public class UserController {
+}
+
+

How to fix it in Spring Boot

+

Remove @Configuration, @EnableAutoConfiguration, and @ComponentScan (when used without custom +attributes) since @SpringBootApplication already includes all of these.

+

Code examples

+

Noncompliant code example

+
+@Configuration // Noncompliant
+@SpringBootApplication
+public class MyApplication {
+}
+
+

Compliant solution

+
+@SpringBootApplication
+public class MyApplication {
+}
+
+

How to fix it in Spring Test

+

Remove @ExtendWith(SpringExtension.class) when using specialized test annotations like @SpringBootTest, +@WebMvcTest, @DataJpaTest, or @WebFluxTest since they already include this extension.

+

Code examples

+

Noncompliant code example

+
+@ExtendWith(SpringExtension.class) // Noncompliant
+@SpringBootTest
+class UserServiceIntegrationTest {
+}
+
+

Compliant solution

+
+@SpringBootTest
+class UserServiceIntegrationTest {
+}
+
+

Resources

+

Documentation

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9341.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9341.json new file mode 100644 index 00000000000..9fec6709d23 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9341.json @@ -0,0 +1,23 @@ +{ + "title": "Redundant Spring annotations should be removed", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "spring" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-9341", + "sqKey": "S9341", + "scope": "All", + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CLEAR" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9341 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9341 new file mode 100644 index 00000000000..e69de29bb2d From 0de6f8aa3aef7faef355b5f23e6917bdee380521 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Tue, 18 Aug 2026 11:43:49 +0200 Subject: [PATCH 2/5] Fix FPs in S9341 for @Transactional with custom attributes and @ComponentScan with filters - Fix @Transactional + @DataJpaTest: only flag as redundant when no attributes are set, since custom attributes like readOnly or propagation change runtime behavior - Fix @ComponentScan + @SpringBootApplication: reject any attribute (not just value/basePackages/basePackageClasses), since attributes like excludeFilters, lazyInit, useDefaultFilters are not exposed by @SpringBootApplication - Add compliant test cases for both fixes Co-Authored-By: Claude Opus 4.6 --- .../RedundantSpringAnnotationCheckSample.java | 31 +++++++++++++++++++ .../RedundantSpringAnnotationCheck.java | 19 +++++------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java index eb69d73fa9e..762105f4be1 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java @@ -11,11 +11,13 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.FilterType; import org.springframework.stereotype.Component; import org.springframework.stereotype.Controller; import org.springframework.stereotype.Repository; import org.springframework.stereotype.Service; import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.ResponseBody; @@ -182,3 +184,32 @@ class DataJpaTestAlone { @SpringBootTest class MixedExtensionsWithSpringBootTest { } + +// === Compliant: @Transactional with custom attributes + @DataJpaTest === + +@Transactional(readOnly = true) +@DataJpaTest +class TransactionalReadOnlyWithDataJpaTest { +} + +@Transactional(propagation = Propagation.NOT_SUPPORTED) +@DataJpaTest +class TransactionalNotSupportedWithDataJpaTest { +} + +// === Compliant: @ComponentScan with filters/other attributes + @SpringBootApplication === + +@ComponentScan(excludeFilters = @ComponentScan.Filter(type = FilterType.REGEX, pattern = "com.example.excluded")) +@SpringBootApplication +class ComponentScanWithExcludeFiltersAndSpringBootApp { +} + +@ComponentScan(lazyInit = true) +@SpringBootApplication +class ComponentScanWithLazyInitAndSpringBootApp { +} + +@ComponentScan(useDefaultFilters = false) +@SpringBootApplication +class ComponentScanWithUseDefaultFiltersAndSpringBootApp { +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java index 50bdfb40e8d..a6e23a9c4d5 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java @@ -63,7 +63,7 @@ public class RedundantSpringAnnotationCheck extends IssuableSubscriptionVisitor List.of(SpringUtils.SPRING_BOOT_TEST_ANNOTATION, WEB_MVC_TEST, DATA_JPA_TEST, WEB_FLUX_TEST), RedundantSpringAnnotationCheck::isExtendWithSpringExtension), new RedundancyRule(SpringUtils.TRANSACTIONAL_ANNOTATION, - List.of(DATA_JPA_TEST), null) + List.of(DATA_JPA_TEST), RedundantSpringAnnotationCheck::isTransactionalWithoutCustomAttributes) ); @Override @@ -126,16 +126,13 @@ private static String simpleName(AnnotationTree annotation) { private static boolean isComponentScanWithoutCustomAttributes(ClassTree classTree, String impliedByFqn) { SymbolMetadata metadata = classTree.symbol().metadata(); List values = metadata.valuesForAnnotation(COMPONENT_SCAN); - if (values == null || values.isEmpty()) { - return true; - } - for (SymbolMetadata.AnnotationValue av : values) { - String name = av.name(); - if ("value".equals(name) || "basePackages".equals(name) || "basePackageClasses".equals(name)) { - return false; - } - } - return true; + return values == null || values.isEmpty(); + } + + private static boolean isTransactionalWithoutCustomAttributes(ClassTree classTree, String impliedByFqn) { + SymbolMetadata metadata = classTree.symbol().metadata(); + List values = metadata.valuesForAnnotation(SpringUtils.TRANSACTIONAL_ANNOTATION); + return values == null || values.isEmpty(); } private static boolean isExtendWithSpringExtension(ClassTree classTree, String impliedByFqn) { From 8f3bc405f507024b02254c932ea95798664f49ed Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Tue, 18 Aug 2026 16:14:59 +0200 Subject: [PATCH 3/5] Fix FPs in S9341 for annotations with explicit attributes, repeatable annotations, and records - Use multimap for annotation collection to properly handle repeatable annotations like multiple @ExtendWith or @ComponentScan instances - Add attribute guards to prevent unsafe removal of annotations with explicit attributes (@Component with bean name, @Configuration with proxyBeanMethods, @EnableAutoConfiguration with exclude, @SpringBootConfiguration with attributes) - Add Tree.Kind.RECORD to visited nodes so records are also checked - Evaluate @ExtendWith per annotation instance using AST arguments instead of merged metadata to avoid false positives on non-Spring extensions - Document that method-level @ResponseBody is handled by S6837 Co-Authored-By: Claude Opus 4.6 --- .../RedundantSpringAnnotationCheckSample.java | 51 +++++++++ .../RedundantSpringAnnotationCheck.java | 105 +++++++++--------- 2 files changed, 103 insertions(+), 53 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java index 762105f4be1..8042ea03be6 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java @@ -213,3 +213,54 @@ class ComponentScanWithLazyInitAndSpringBootApp { @SpringBootApplication class ComponentScanWithUseDefaultFiltersAndSpringBootApp { } + +// === Compliant: @Component with explicit attributes (bean name) === + +@Component("orders") +@Service +class ComponentWithBeanNameAndService { +} + +// === Compliant: @Configuration with explicit attributes + @SpringBootApplication === + +@Configuration(proxyBeanMethods = false) +@SpringBootApplication +class ConfigurationWithProxyBeanMethodsAndSpringBootApp { +} + +// === Compliant: @EnableAutoConfiguration with explicit attributes + @SpringBootApplication === + +@EnableAutoConfiguration(exclude = Configuration.class) +@SpringBootApplication +class EnableAutoConfigWithExcludeAndSpringBootApp { +} + +// === Compliant: @SpringBootConfiguration with explicit attributes + @SpringBootApplication === + +@SpringBootConfiguration(proxyBeanMethods = false) +@SpringBootApplication +class SpringBootConfigWithProxyBeanMethodsAndSpringBootApp { +} + +// === Compliant: Repeatable @ExtendWith — only SpringExtension instance reported, not MockitoExtension === + +@ExtendWith(SpringExtension.class) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@SpringBootTest".}} +@ExtendWith(org.mockito.junit.jupiter.MockitoExtension.class) +@SpringBootTest +class RepeatableExtendWithSpringBootTest { +} + +// === Compliant: Repeatable @ComponentScan with custom attributes — neither reported === + +@ComponentScan("com.example.pkg1") +@ComponentScan("com.example.pkg2") +@SpringBootApplication +class RepeatableComponentScanWithSpringBootApp { +} + +// === Records with redundant annotations === + +@Component // Noncompliant {{Remove this "@Component" annotation, already implied by "@Service".}} +@Service +record OrderServiceRecord(String name) { +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java index a6e23a9c4d5..a5a1737ac14 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java @@ -16,19 +16,20 @@ */ package org.sonar.java.checks.spring; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.BiPredicate; import org.sonar.check.Rule; import org.sonar.java.checks.helpers.QuickFixHelper; import org.sonar.java.checks.helpers.SpringUtils; 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.semantic.SymbolMetadata; import org.sonar.plugins.java.api.tree.AnnotationTree; import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.NewArrayTree; import org.sonar.plugins.java.api.tree.Tree; @Rule(key = "S9341") @@ -46,65 +47,70 @@ public class RedundantSpringAnnotationCheck extends IssuableSubscriptionVisitor private static final List REDUNDANCY_RULES = List.of( new RedundancyRule(SpringUtils.COMPONENT_ANNOTATION, - List.of(SpringUtils.SERVICE_ANNOTATION, SpringUtils.REPOSITORY_ANNOTATION, SpringUtils.CONTROLLER_ANNOTATION, SpringUtils.CONFIGURATION_ANNOTATION), null), + List.of(SpringUtils.SERVICE_ANNOTATION, SpringUtils.REPOSITORY_ANNOTATION, SpringUtils.CONTROLLER_ANNOTATION, SpringUtils.CONFIGURATION_ANNOTATION), + RedundantSpringAnnotationCheck::hasNoExplicitAttributes), new RedundancyRule(SpringUtils.CONTROLLER_ANNOTATION, List.of(SpringUtils.REST_CONTROLLER_ANNOTATION), null), + // Class-level @ResponseBody only; method-level @ResponseBody in @RestController is handled by S6837 new RedundancyRule(RESPONSE_BODY, List.of(SpringUtils.REST_CONTROLLER_ANNOTATION), null), new RedundancyRule(SpringUtils.CONFIGURATION_ANNOTATION, - List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), null), + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), RedundantSpringAnnotationCheck::hasNoExplicitAttributes), new RedundancyRule(ENABLE_AUTO_CONFIGURATION, - List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), null), + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), RedundantSpringAnnotationCheck::hasNoExplicitAttributes), new RedundancyRule(COMPONENT_SCAN, - List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), RedundantSpringAnnotationCheck::isComponentScanWithoutCustomAttributes), + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), RedundantSpringAnnotationCheck::hasNoExplicitAttributes), new RedundancyRule(SPRING_BOOT_CONFIGURATION, - List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), null), + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), RedundantSpringAnnotationCheck::hasNoExplicitAttributes), new RedundancyRule(EXTEND_WITH, List.of(SpringUtils.SPRING_BOOT_TEST_ANNOTATION, WEB_MVC_TEST, DATA_JPA_TEST, WEB_FLUX_TEST), - RedundantSpringAnnotationCheck::isExtendWithSpringExtension), + RedundantSpringAnnotationCheck::isExtendWithSpringExtensionOnly), new RedundancyRule(SpringUtils.TRANSACTIONAL_ANNOTATION, - List.of(DATA_JPA_TEST), RedundantSpringAnnotationCheck::isTransactionalWithoutCustomAttributes) + List.of(DATA_JPA_TEST), RedundantSpringAnnotationCheck::hasNoExplicitAttributes) ); @Override public List nodesToVisit() { - return List.of(Tree.Kind.CLASS); + return List.of(Tree.Kind.CLASS, Tree.Kind.RECORD); } @Override public void visitNode(Tree tree) { var classTree = (ClassTree) tree; - Map annotationsByFqn = collectAnnotations(classTree); + Map> annotationsByFqn = collectAnnotations(classTree); for (RedundancyRule rule : REDUNDANCY_RULES) { - AnnotationTree redundantAnnotation = annotationsByFqn.get(rule.redundantFqn); - if (redundantAnnotation == null) { + List redundantAnnotations = annotationsByFqn.get(rule.redundantFqn); + if (redundantAnnotations == null) { continue; } - for (String impliedByFqn : rule.impliedByFqns) { - AnnotationTree impliedByAnnotation = annotationsByFqn.get(impliedByFqn); - if (impliedByAnnotation != null && passesSpecialCondition(rule, classTree, impliedByFqn)) { - reportRedundancy(redundantAnnotation, impliedByAnnotation); - break; + for (AnnotationTree redundantAnnotation : redundantAnnotations) { + for (String impliedByFqn : rule.impliedByFqns) { + List impliedByAnnotations = annotationsByFqn.get(impliedByFqn); + if (impliedByAnnotations != null && !impliedByAnnotations.isEmpty() + && passesSpecialCondition(rule, redundantAnnotation)) { + reportRedundancy(redundantAnnotation, impliedByAnnotations.get(0)); + break; + } } } } } - private static Map collectAnnotations(ClassTree classTree) { - Map map = new HashMap<>(); + private static Map> collectAnnotations(ClassTree classTree) { + Map> map = new HashMap<>(); for (AnnotationTree annotation : classTree.modifiers().annotations()) { String fqn = annotation.annotationType().symbolType().fullyQualifiedName(); - map.put(fqn, annotation); + map.computeIfAbsent(fqn, k -> new ArrayList<>()).add(annotation); } return map; } - private static boolean passesSpecialCondition(RedundancyRule rule, ClassTree classTree, String impliedByFqn) { + private static boolean passesSpecialCondition(RedundancyRule rule, AnnotationTree redundantAnnotation) { if (rule.specialCondition == null) { return true; } - return rule.specialCondition.test(classTree, impliedByFqn); + return rule.specialCondition.test(redundantAnnotation); } private void reportRedundancy(AnnotationTree redundantAnnotation, AnnotationTree impliedByAnnotation) { @@ -123,45 +129,38 @@ private static String simpleName(AnnotationTree annotation) { return annotation.annotationType().symbolType().name(); } - private static boolean isComponentScanWithoutCustomAttributes(ClassTree classTree, String impliedByFqn) { - SymbolMetadata metadata = classTree.symbol().metadata(); - List values = metadata.valuesForAnnotation(COMPONENT_SCAN); - return values == null || values.isEmpty(); + private static boolean hasNoExplicitAttributes(AnnotationTree annotation) { + return annotation.arguments().isEmpty(); } - private static boolean isTransactionalWithoutCustomAttributes(ClassTree classTree, String impliedByFqn) { - SymbolMetadata metadata = classTree.symbol().metadata(); - List values = metadata.valuesForAnnotation(SpringUtils.TRANSACTIONAL_ANNOTATION); - return values == null || values.isEmpty(); - } - - private static boolean isExtendWithSpringExtension(ClassTree classTree, String impliedByFqn) { - SymbolMetadata metadata = classTree.symbol().metadata(); - List values = metadata.valuesForAnnotation(EXTEND_WITH); - if (values == null) { + private static boolean isExtendWithSpringExtensionOnly(AnnotationTree annotation) { + var arguments = annotation.arguments(); + if (arguments.size() != 1) { return false; } - for (SymbolMetadata.AnnotationValue av : values) { - if (isOnlySpringExtensionClass(av.value())) { - return true; - } + ExpressionTree arg = arguments.get(0); + if (arg.is(Tree.Kind.MEMBER_SELECT)) { + return isSpringExtensionClassRef((MemberSelectExpressionTree) arg); + } + if (arg.is(Tree.Kind.NEW_ARRAY)) { + var initializers = ((NewArrayTree) arg).initializers(); + return initializers.size() == 1 + && initializers.get(0).is(Tree.Kind.MEMBER_SELECT) + && isSpringExtensionClassRef((MemberSelectExpressionTree) initializers.get(0)); } return false; } - private static boolean isOnlySpringExtensionClass(Object value) { - if (value instanceof Symbol symbol) { - return symbol.type().is(SPRING_EXTENSION); - } - if (value instanceof Object[] values) { - // Skip mixed arrays like @ExtendWith({SpringExtension.class, MockitoExtension.class}) - // since the annotation cannot simply be removed - return values.length == 1 && values[0] instanceof Symbol symbol && symbol.type().is(SPRING_EXTENSION); - } - return false; + private static boolean isSpringExtensionClassRef(MemberSelectExpressionTree memberSelect) { + return memberSelect.expression().symbolType().is(SPRING_EXTENSION); + } + + @FunctionalInterface + private interface AnnotationPredicate { + boolean test(AnnotationTree annotation); } private record RedundancyRule(String redundantFqn, List impliedByFqns, - BiPredicate specialCondition) { + AnnotationPredicate specialCondition) { } } From 512792c2b39769d266bdcaaf9e08fe582dfe0148 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Tue, 18 Aug 2026 16:30:41 +0200 Subject: [PATCH 4/5] Remove test case using proxyBeanMethods on SpringBootConfiguration The default module uses Spring Boot 2.0.2 which does not have the proxyBeanMethods attribute on @SpringBootConfiguration (added in 2.2). This caused a compilation failure in CI. Co-Authored-By: Claude Opus 4.6 --- .../spring/RedundantSpringAnnotationCheckSample.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java index 8042ea03be6..ce357177768 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java @@ -235,13 +235,6 @@ class ConfigurationWithProxyBeanMethodsAndSpringBootApp { class EnableAutoConfigWithExcludeAndSpringBootApp { } -// === Compliant: @SpringBootConfiguration with explicit attributes + @SpringBootApplication === - -@SpringBootConfiguration(proxyBeanMethods = false) -@SpringBootApplication -class SpringBootConfigWithProxyBeanMethodsAndSpringBootApp { -} - // === Compliant: Repeatable @ExtendWith — only SpringExtension instance reported, not MockitoExtension === @ExtendWith(SpringExtension.class) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@SpringBootTest".}} From b35b922c94e4d7149111d4038e4e57cea6193e04 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Wed, 19 Aug 2026 09:04:58 +0200 Subject: [PATCH 5/5] Replace custom AnnotationPredicate interface with java.util.function.Predicate Co-Authored-By: Claude Opus 4.6 --- .../checks/spring/RedundantSpringAnnotationCheck.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java index a5a1737ac14..5000cde6df1 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java @@ -20,6 +20,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import org.sonar.check.Rule; import org.sonar.java.checks.helpers.QuickFixHelper; import org.sonar.java.checks.helpers.SpringUtils; @@ -155,12 +156,7 @@ private static boolean isSpringExtensionClassRef(MemberSelectExpressionTree memb return memberSelect.expression().symbolType().is(SPRING_EXTENSION); } - @FunctionalInterface - private interface AnnotationPredicate { - boolean test(AnnotationTree annotation); - } - private record RedundancyRule(String redundantFqn, List impliedByFqns, - AnnotationPredicate specialCondition) { + Predicate specialCondition) { } }