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..b601a338195 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/RedundantSpringAnnotationCheckSample.java @@ -0,0 +1,292 @@ +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.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; +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 { +} + +// === 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 { +} + +// === Compliant: @Component with explicit attributes (bean name) === + +@Component("orders") +@Service +class ComponentWithBeanNameAndService { +} + +// === Compliant: @Controller with explicit attributes (bean name) + @RestController === + +@Controller("myCustomBeanName") +@RestController +class ControllerWithBeanNameAndRestController { +} + +// === 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: 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 { +} + +// === @ExtendWith with single-element array syntax === + +@ExtendWith({SpringExtension.class}) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@SpringBootTest".}} +@SpringBootTest +class ExtendWithArraySingleSpringExtension { +} + +@ExtendWith({SpringExtension.class}) // Noncompliant {{Remove this "@ExtendWith" annotation, already implied by "@WebMvcTest".}} +@WebMvcTest +class ExtendWithArraySingleSpringExtensionWebMvc { +} + +// === Compliant: @ExtendWith with explicit value= attribute (named parameter) === + +@ExtendWith(value = SpringExtension.class) +@SpringBootTest +class ExtendWithNamedValueSpringExtension { +} + +// === Compliant: @ExtendWith with single-element array containing non-SpringExtension === + +@ExtendWith({org.mockito.junit.jupiter.MockitoExtension.class}) +@SpringBootTest +class ExtendWithArraySingleMockitoExtension { +} + +// === 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 new file mode 100644 index 00000000000..13821758b88 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java @@ -0,0 +1,162 @@ +/* + * 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.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; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaFileScannerContext; +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") +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), + RedundantSpringAnnotationCheck::hasNoExplicitAttributes), + new RedundancyRule(SpringUtils.CONTROLLER_ANNOTATION, + List.of(SpringUtils.REST_CONTROLLER_ANNOTATION), RedundantSpringAnnotationCheck::hasNoExplicitAttributes), + // 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), RedundantSpringAnnotationCheck::hasNoExplicitAttributes), + new RedundancyRule(ENABLE_AUTO_CONFIGURATION, + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), RedundantSpringAnnotationCheck::hasNoExplicitAttributes), + new RedundancyRule(COMPONENT_SCAN, + List.of(SpringUtils.SPRING_BOOT_APP_ANNOTATION), RedundantSpringAnnotationCheck::hasNoExplicitAttributes), + new RedundancyRule(SPRING_BOOT_CONFIGURATION, + 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::isExtendWithSpringExtensionOnly), + new RedundancyRule(SpringUtils.TRANSACTIONAL_ANNOTATION, + List.of(DATA_JPA_TEST), RedundantSpringAnnotationCheck::hasNoExplicitAttributes) + ); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS, Tree.Kind.RECORD); + } + + @Override + public void visitNode(Tree tree) { + var classTree = (ClassTree) tree; + Map> annotationsByFqn = collectAnnotations(classTree); + + for (RedundancyRule rule : REDUNDANCY_RULES) { + List redundantAnnotations = annotationsByFqn.get(rule.redundantFqn); + if (redundantAnnotations == null) { + continue; + } + 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<>(); + for (AnnotationTree annotation : classTree.modifiers().annotations()) { + String fqn = annotation.annotationType().symbolType().fullyQualifiedName(); + map.computeIfAbsent(fqn, k -> new ArrayList<>()).add(annotation); + } + return map; + } + + private static boolean passesSpecialCondition(RedundancyRule rule, AnnotationTree redundantAnnotation) { + if (rule.specialCondition == null) { + return true; + } + return rule.specialCondition.test(redundantAnnotation); + } + + 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 hasNoExplicitAttributes(AnnotationTree annotation) { + return annotation.arguments().isEmpty(); + } + + private static boolean isExtendWithSpringExtensionOnly(AnnotationTree annotation) { + var arguments = annotation.arguments(); + if (arguments.size() != 1) { + return false; + } + 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 isSpringExtensionClassRef(MemberSelectExpressionTree memberSelect) { + return memberSelect.expression().symbolType().is(SPRING_EXTENSION); + } + + private record RedundancyRule(String redundantFqn, List impliedByFqns, + Predicate 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