Skip to content

SONARJAVA-3259 Implement new rule S5673#5780

Merged
romainbrenguier merged 7 commits into
masterfrom
new-rule/SONARJAVA-3259-S5673
Jul 14, 2026
Merged

SONARJAVA-3259 Implement new rule S5673#5780
romainbrenguier merged 7 commits into
masterfrom
new-rule/SONARJAVA-3259-S5673

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

This rule detects classes and interfaces annotated with @Component whose names suggest they should use specialized Spring stereotype annotations (@Service, @Repository, @Controller, or @RestController) instead.

The rule also covers interfaces, which is relevant for Spring Data repository interfaces — where Spring generates the implementation at runtime, but @Repository is still the appropriate stereotype.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-3259

This rule detects classes annotated with @component whose names suggest
they should use specialized Spring stereotype annotations (@service,
@repository, @controller, or @RestController) instead.
@romainbrenguier romainbrenguier force-pushed the new-rule/SONARJAVA-3259-S5673 branch from 053660b to 700dbe2 Compare July 13, 2026 12:40
romainbrenguier and others added 2 commits July 13, 2026 15:01
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fixed S1449 violations by adding Locale.ROOT to two toLowerCase() calls in SpringComponentSpecializationCheck.containsIgnoreCase()
- Added missing autoscan diff files for new rule S5673 (hasTruePositives=false, falseNegatives=17, falsePositives=0) by creating diff_S5673.json and adding entry to autoscan-diff-by-rules.json
- Fixed 2 S1449 code smells in SpringComponentSpecializationCheck.java: added Locale.ROOT to toLowerCase() calls in containsIgnoreCase method (already applied in commit df023db)
@romainbrenguier romainbrenguier requested a review from rombirli July 13, 2026 14:07
Replace containsIgnoreCase with endsWithIgnoreCase for the ServiceFacade
suffix check, consistent with all other name-matching patterns in the rule.
This avoids false positives on classes like ServiceFacadeBuilder or
ServiceFacadeConfiguration. Remove the now-unused containsIgnoreCase helper.
@romainbrenguier romainbrenguier marked this pull request as ready for review July 13, 2026 14:10
@gitar-bot

gitar-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Implements rule S5673 to identify @Component classes that should use specialized Spring stereotypes, while addressing the inconsistent suffix matching for ServiceFacade.

✅ 1 resolved
Edge Case: ServiceFacade matched with contains, inconsistent with other suffixes

📄 java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java:68-72 📄 java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java:90-92
Every other classification uses endsWithIgnoreCase, but the Service case additionally uses containsIgnoreCase(className, "ServiceFacade"). Because contains matches anywhere in the name, a class like ServiceFacadeBuilder or ServiceFacadeConfiguration (which is not itself a service) would be flagged and told to use @Service. This is broader than the suffix-based intent of the rule and can produce false positives.

Consider using a suffix check for consistency, e.g. endsWithIgnoreCase(className, "ServiceFacade"), so only classes ending in Facade (following the ...ServiceFacade naming) are flagged. If matching anywhere in the name is intentional, it should be applied consistently across all stereotypes and documented in the rule.

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

@rombirli rombirli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very clear implementation

String suggestedAnnotation = getSuggestedAnnotation(className);

if (suggestedAnnotation != null) {
reportIssue(componentAnnotation.get(), String.format("Use @%s instead of @Component", suggestedAnnotation));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: The issue message could also suggest renaming the class as an alternative way to fix


@Override
public List<Tree.Kind> nodesToVisit() {
return List.of(Tree.Kind.CLASS);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about records, enums, interfaces, annotation_types? Just to double-check if it is intentional to not include them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this would make sense for records enum and annotations. But for interfaces I think it does, in particular repositories can be just interfaces. Let me check.

Extend the rule to cover interface declarations in addition to classes.
This is relevant for Spring Data repository interfaces annotated with
@component, where @repository would be the appropriate stereotype.
@sonarqube-next

Copy link
Copy Markdown

@romainbrenguier romainbrenguier merged commit 015d825 into master Jul 14, 2026
15 checks passed
@romainbrenguier romainbrenguier deleted the new-rule/SONARJAVA-3259-S5673 branch July 14, 2026 16:32
@gitar-bot

gitar-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Implements rule S5673 to detect misplaced Spring stereotype annotations on classes and interfaces. The suffix matching logic for ServiceFacade was updated to be case-insensitive, resolving the identified inconsistency.

✅ 1 resolved
Edge Case: ServiceFacade matched with contains, inconsistent with other suffixes

📄 java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java:68-72 📄 java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java:90-92
Every other classification uses endsWithIgnoreCase, but the Service case additionally uses containsIgnoreCase(className, "ServiceFacade"). Because contains matches anywhere in the name, a class like ServiceFacadeBuilder or ServiceFacadeConfiguration (which is not itself a service) would be flagged and told to use @Service. This is broader than the suffix-based intent of the rule and can produce false positives.

Consider using a suffix check for consistency, e.g. endsWithIgnoreCase(className, "ServiceFacade"), so only classes ending in Facade (following the ...ServiceFacade naming) are flagged. If matching anywhere in the name is intentional, it should be applied consistently across all stereotypes and documented in the rule.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants