diff --git a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json index 3677d0acf84..55cea45ed01 100644 --- a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -2117,6 +2117,12 @@ "falseNegatives": 1, "falsePositives": 0 }, + { + "ruleKey": "S5673", + "hasTruePositives": false, + "falseNegatives": 17, + "falsePositives": 0 + }, { "ruleKey": "S5679", "hasTruePositives": false, diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5673.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5673.json new file mode 100644 index 00000000000..559fd7787c9 --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5673.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S5673", + "hasTruePositives": false, + "falseNegatives": 20, + "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 new file mode 100644 index 00000000000..8672679f351 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/SpringComponentSpecializationCheckSample.java @@ -0,0 +1,155 @@ +package checks.spring; + +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.RestController; + +public class SpringComponentSpecializationCheckSample { + + // Service patterns + + @Component // Noncompliant {{Use @Service instead of @Component, or rename this type if the @Component annotation is intentional}} + public class CustomerServiceImpl { + } + + @Component // Noncompliant {{Use @Service instead of @Component, or rename this type if the @Component annotation is intentional}} + public class OrderService { + } + + @Component // Noncompliant {{Use @Service instead of @Component, or rename this type if the @Component annotation is intentional}} + public class PaymentServiceFacade { + } + + // Repository patterns + + @Component // Noncompliant {{Use @Repository instead of @Component, or rename this type if the @Component annotation is intentional}} + public class ProductRepository { + } + + @Component // Noncompliant {{Use @Repository instead of @Component, or rename this type if the @Component annotation is intentional}} + public class UserRepositoryImpl { + } + + @Component // Noncompliant {{Use @Repository instead of @Component, or rename this type if the @Component annotation is intentional}} + public class OrderDao { + } + + @Component // Noncompliant {{Use @Repository instead of @Component, or rename this type if the @Component annotation is intentional}} + public class CustomerDao { + } + + // RestController patterns + + @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}} + public class FooBarRestController { + } + + @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}} + public class ApiRestController { + } + + @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}} + public class UserRestControllerImpl { + } + + // Controller patterns + + @Component // Noncompliant {{Use @Controller instead of @Component, or rename this type if the @Component annotation is intentional}} + public class HomeController { + } + + @Component // Noncompliant {{Use @Controller instead of @Component, or rename this type if the @Component annotation is intentional}} + public class LoginControllerImpl { + } + + // Compliant - Correct annotations used + + @Service + public class CustomerServiceImplCorrect { + } + + @Repository + public class ProductRepositoryCorrect { + } + + @RestController + public class FooBarRestControllerCorrect { + } + + @Controller + public class HomeControllerCorrect { + } + + // Generic component names + + @Component + public class SomeOtherComponent { + } + + @Component + public class UtilityHelper { + } + + @Component + public class CacheManager { + } + + @Component + public class DataProcessor { + } + + // No annotation + + public class PlainClass { + } + + // Case variations + + @Component // Noncompliant {{Use @Service instead of @Component, or rename this type if the @Component annotation is intentional}} + public class userservice { + } + + @Component // Noncompliant {{Use @Repository instead of @Component, or rename this type if the @Component annotation is intentional}} + public class USERREPOSITORY { + } + + @Component // Noncompliant {{Use @Controller instead of @Component, or rename this type if the @Component annotation is intentional}} + public class maincontroller { + } + + @Component // Noncompliant {{Use @RestController instead of @Component, or rename this type if the @Component annotation is intentional}} + public class apirestcontroller { + } + + // Interface patterns + + @Component // Noncompliant {{Use @Repository instead of @Component, or rename this type if the @Component annotation is intentional}} + public interface UserRepository { + } + + @Component // Noncompliant {{Use @Service instead of @Component, or rename this type if the @Component annotation is intentional}} + public interface PaymentService { + } + + @Component // Noncompliant {{Use @Repository instead of @Component, or rename this type if the @Component annotation is intentional}} + public interface ProductDao { + } + + // Compliant interfaces - correct annotations used + + @Repository + public interface CategoryRepository { + } + + @Service + public interface NotificationService { + } + + // Compliant interfaces - generic names + + @Component + public interface EventListener { + } +} 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 new file mode 100644 index 00000000000..a91e398f42b --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheck.java @@ -0,0 +1,90 @@ +/* + * 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.List; +import java.util.Optional; +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.tree.AnnotationTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S5673") +public class SpringComponentSpecializationCheck extends IssuableSubscriptionVisitor { + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS, Tree.Kind.INTERFACE); + } + + @Override + public void visitNode(Tree tree) { + var classTree = (ClassTree) tree; + + Optional componentAnnotation = classTree.modifiers().annotations().stream() + .filter(a -> SpringUtils.COMPONENT_ANNOTATION.equals(a.annotationType().symbolType().fullyQualifiedName())) + .findFirst(); + + if (componentAnnotation.isEmpty()) { + return; + } + + String className = classTree.simpleName().name(); + String suggestedAnnotation = getSuggestedAnnotation(className); + + if (suggestedAnnotation != null) { + reportIssue(componentAnnotation.get(), String.format("Use @%s instead of @Component, or rename this type if the @Component annotation is intentional", suggestedAnnotation)); + } + } + + @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, "Controller") || endsWithIgnoreCase(className, "ControllerImpl")) { + return "Controller"; + } + + if (endsWithIgnoreCase(className, "Service") || + endsWithIgnoreCase(className, "ServiceImpl") || + endsWithIgnoreCase(className, "ServiceFacade")) { + return "Service"; + } + + if (endsWithIgnoreCase(className, "Repository") || + endsWithIgnoreCase(className, "RepositoryImpl") || + endsWithIgnoreCase(className, "Dao")) { + return "Repository"; + } + + return null; + } + + private static boolean endsWithIgnoreCase(String str, String suffix) { + if (str.length() < suffix.length()) { + return false; + } + return str.substring(str.length() - suffix.length()).equalsIgnoreCase(suffix); + } + +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheckTest.java new file mode 100644 index 00000000000..e6998fde07e --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/SpringComponentSpecializationCheckTest.java @@ -0,0 +1,33 @@ +/* + * 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 SpringComponentSpecializationCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/SpringComponentSpecializationCheckSample.java")) + .withCheck(new SpringComponentSpecializationCheck()) + .verifyIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5673.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5673.html new file mode 100644 index 00000000000..74c51da93b2 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5673.html @@ -0,0 +1,45 @@ +

The Spring Framework provides several specializations of the generic @Component stereotype annotation which better express the programmer's intent. Using them should be preferred.

+

Why is this an issue?

+

Spring provides specialized stereotype annotations (@Service, @Repository, @Controller, and @RestController) that make code more expressive and provide additional semantics. When a class name suggests it should use one of these specialized annotations but uses @Component instead, it reduces code clarity.

+

How to fix it

+

Replace @Component with the appropriate specialized annotation based on the class's role in the application.

+

Code examples

+

Noncompliant code example

+
+@Component // Noncompliant
+public class CustomerServiceImpl {
+  // ...
+}
+
+@Component // Noncompliant
+public class ProductRepository {
+    // ...
+}
+
+@Component // Noncompliant
+public class FooBarRestController {
+    // ...
+}
+
+

Compliant solution

+
+@Service // Compliant
+public class CustomerServiceImpl {
+  // ...
+}
+
+@Repository // Compliant
+public class ProductRepository {
+    // ...
+}
+
+@RestController // Compliant
+public class FooBarRestController {
+    // ...
+}
+
+

Resources

+

Documentation

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5673.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5673.json new file mode 100644 index 00000000000..66e17c3344f --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S5673.json @@ -0,0 +1,23 @@ +{ + "title": "Spring components should use specialized annotations", + "type": "CODE_SMELL", + "code": { + "impacts": { + "MAINTAINABILITY": "LOW" + }, + "attribute": "CLEAR" + }, + "status": "ready", + "remediation": { + "func": "Constant/Issue", + "constantCost": "2min" + }, + "tags": [ + "spring" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-5673", + "sqKey": "S5673", + "scope": "All", + "quickfix": "unknown" +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S5673 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S5673 new file mode 100644 index 00000000000..e69de29bb2d