SONARJAVA-6767 Implement new rule S9341: Redundant Spring annotations should be removed

[romainbrenguier]

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.

Part of

[hashicorp-vault-sonar-prod]

SONARJAVA-6767

[nathsou]

Thanks for the implementation. I found four issues that need addressing before this can be merged.

[nathsou]

[P1] The removal is unsafe when the parent annotation has explicit attributes. For example, @Component("orders") next to @Service supplies the bean name; removing it changes that name. Similarly, @Configuration(proxyBeanMethods = false) alongside @SpringBootApplication changes configuration semantics, and @EnableAutoConfiguration(exclude = Foo.class) loses the exclusion. Only @ComponentScan and @Transactional are guarded today. Report these pairs only when the parent annotation has no explicit attributes, and add compliant regression cases.

[nathsou]

[P1] Collapsing annotations by FQN breaks repeatable annotations. With @ExtendWith(SpringExtension.class), then @ExtendWith(MockitoExtension.class), then @SpringBootTest, this map retains Mockito while valuesForAnnotation examines the first matching semantic annotation. The check can therefore report Mockito as redundant. @ComponentScan has the same risk. Preserve/evaluate each annotation instance rather than one annotation per FQN, and cover repeated-annotation cases.

[nathsou]

[P2] Spring stereotype annotations can target records, but the visitor subscribes only to CLASS; @Component @Service record Foo() {} is ignored. Subscribe to Tree.Kind.RECORD as well and add a record test case.

[nathsou]

[P2] The linked RSPEC says method-level @ResponseBody in a @RestController should be reported, but this check visits only classes. Either implement that behavior (while resolving the overlap with S6837) or update RSPEC to avoid promising it.

[gitar-bot]

💡 Edge Case: @ExtendWith(value = SpringExtension.class) not detected

isExtendWithSpringExtensionOnly only handles a single argument that is a MEMBER_SELECT (SpringExtension.class) or a NEW_ARRAY. When the argument is written in the explicit named form @ExtendWith(value = SpringExtension.class), the argument tree is an ASSIGNMENT, so the method returns false and the redundant annotation is not reported (false negative). Consider unwrapping an ASSIGNMENT whose name is value to its expression before checking, so the named form is treated the same as the shorthand.

_{Was this helpful? React with 👍 / 👎}

[nathsou]

Found one issue that should be addressed before merging.

[nathsou]

The @Controller → @RestController redundancy rule has no hasNoExplicitAttributes guard, unlike the sibling @Component/@Configuration rules, so it fires even when @Controller carries a custom bean name.

For example:

@Controller("myCustomBeanName")
@RestController
class MyController {}

This reports "Remove this @controller annotation, already implied by @RestController", even though @RestController is meta-annotated with a plain, argument-less @Controller. Removing the annotated one would silently drop the custom bean name and fall back to Spring's default auto-generated name — a behavior-changing false positive that the analogous COMPONENT_ANNOTATION/CONFIGURATION_ANNOTATION rules explicitly avoid via RedundantSpringAnnotationCheck::hasNoExplicitAttributes.

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
95.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[gitar-bot]

Code Review 👍 Approved with suggestions 2 resolved / 3 findings

Implements rule S9341 to detect redundant Spring annotations while properly handling custom attributes and record types. Consider updating isExtendWithSpringExtensionOnly to correctly detect @ExtendWith without member selection wrappers.

💡 Edge Case: @ExtendWith(value = SpringExtension.class) not detected

📄 java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java:136-150

isExtendWithSpringExtensionOnly only handles a single argument that is a MEMBER_SELECT (SpringExtension.class) or a NEW_ARRAY. When the argument is written in the explicit named form @ExtendWith(value = SpringExtension.class), the argument tree is an ASSIGNMENT, so the method returns false and the redundant annotation is not reported (false negative). Consider unwrapping an ASSIGNMENT whose name is value to its expression before checking, so the named form is treated the same as the shorthand.

✅ 2 resolved

✅ Edge Case: @transactional flagged redundant even when it carries custom attributes

📄 java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java:65-66
The rule for @transactional implied by @DataJpaTest (RedundantSpringAnnotationCheck.java:65-66) has no special condition, so ANY @transactional is reported as removable when @DataJpaTest is present. But @DataJpaTest only supplies a default @transactional; a user commonly overrides it, e.g. @transactional(propagation = Propagation.NOT_SUPPORTED), readOnly = true, a custom isolation/timeout, or a specific transactionManager. Removing such an annotation silently changes runtime behavior, so this is a false positive. Add a special condition (like isComponentScanWithoutCustomAttributes) that only flags @transactional when it has no explicitly-set attributes, and add test cases covering the attribute-carrying variant.

✅ Bug: ComponentScan redundancy check misses filter/other custom attributes

📄 java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java:126-139 📄 java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java:58-59
isComponentScanWithoutCustomAttributes (RedundantSpringAnnotationCheck.java:126-139) only treats @componentscan as non-redundant when value/basePackages/basePackageClasses are set. Any other explicit attribute — excludeFilters, includeFilters, nameGenerator, lazyInit, useDefaultFilters, etc. — causes the method to return true and the annotation to be reported as removable. @SpringBootApplication does not expose includeFilters/excludeFilters, so removing a @componentscan(excludeFilters = ...) drops scanning configuration and changes behavior — a false positive. Broaden the condition to return false whenever ANY attribute is explicitly set (i.e. treat a non-empty attribute list as custom), and add corresponding test cases.

🤖 Prompt for agents

Code Review: Implements rule S9341 to detect redundant Spring annotations while properly handling custom attributes and record types. Consider updating isExtendWithSpringExtensionOnly to correctly detect @ExtendWith without member selection wrappers.

1. 💡 Edge Case: @ExtendWith(value = SpringExtension.class) not detected
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java:136-150

   isExtendWithSpringExtensionOnly only handles a single argument that is a MEMBER_SELECT (SpringExtension.class) or a NEW_ARRAY. When the argument is written in the explicit named form `@ExtendWith(value = SpringExtension.class)`, the argument tree is an ASSIGNMENT, so the method returns false and the redundant annotation is not reported (false negative). Consider unwrapping an ASSIGNMENT whose name is `value` to its expression before checking, so the named form is treated the same as the shorthand.

Implementation Status ✅ 1 / 1 issues implemented

✅ SONARJAVA-6767 — 1 / 1 objectives

The PR implements rule S9341 for removing redundant Spring annotations.

✅ 1 complete

• ✅ Implement new rule S9341: Redundant Spring annotations should be removed

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}