From cffc150c74e62e0c7feb0934181f8e6046f3efa0 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Wed, 4 Mar 2026 14:19:13 +0100 Subject: [PATCH 1/2] SONARJAVA-6184 S4605: FP when having SpringBootApplication followed by ComponentScan annotation --- .../app/SpringBootApp1.java | 3 ++ .../app/notOk/NotOkImpl.java | 10 +++++ .../SpringBeansShouldBeAccessibleCheck.java | 40 ++++++++++++------- ...pringBeansShouldBeAccessibleCheckTest.java | 1 + 4 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/notOk/NotOkImpl.java diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/SpringBootApp1.java b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/SpringBootApp1.java index fd24507224b..8fb938c1da6 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/SpringBootApp1.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/SpringBootApp1.java @@ -2,9 +2,12 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.FilterType; import org.springframework.stereotype.Component; @SpringBootApplication +@ComponentScan(excludeFilters = {@ComponentScan.Filter(type = FilterType.REGEX, pattern = "checks.spring.s4605.springBootApplication.app.smth")}) public class SpringBootApp1 { public static void main(String[] args) { diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/notOk/NotOkImpl.java b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/notOk/NotOkImpl.java new file mode 100644 index 00000000000..5926c024c0a --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/notOk/NotOkImpl.java @@ -0,0 +1,10 @@ +package checks.spring.s4605.springBootApplication.app.notOk; + +import org.springframework.stereotype.Component; + +interface NotOk { +} + +@Component +public class NotOkImpl implements NotOk { // Compliant +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheck.java index 5bfbf07be19..15845ec482d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheck.java @@ -65,7 +65,7 @@ public class SpringBeansShouldBeAccessibleCheck extends IssuableSubscriptionVisi }; private static final String COMPONENT_SCAN_ANNOTATION = "org.springframework.context.annotation.ComponentScan"; - private static final Set COMPONENT_SCAN_ARGUMENTS = SetUtils.immutableSetOf("basePackages", "basePackageClasses", "value"); + private static final Set COMPONENT_SCAN_BASE_ARGUMENTS = SetUtils.immutableSetOf("basePackages", "basePackageClasses", "value"); private static final String CACHE_KEY_PREFIX = "java:S4605:targeted:"; @@ -117,16 +117,17 @@ public void visitNode(Tree tree) { String classPackageName = packageNameOf(classTree.symbol()); SymbolMetadata classSymbolMetadata = classTree.symbol().metadata(); - - List componentScanValues = classSymbolMetadata.valuesForAnnotation(COMPONENT_SCAN_ANNOTATION); - if (componentScanValues != null) { - componentScanValues.forEach(this::addToScannedPackages); - } else if (hasAnnotation(classSymbolMetadata, SpringUtils.SPRING_BOOT_APP_ANNOTATION)) { - var targetedPackages = targetedPackages(classPackageName, classSymbolMetadata); - packagesScannedBySpringAtProjectLevel.addAll(targetedPackages); - packagesScannedBySpringAtFileLevel.addAll(targetedPackages); - } else if (hasAnnotation(classSymbolMetadata, SPRING_BEAN_ANNOTATIONS)) { - addMessageToMap(classPackageName, classTree.simpleName()); + // try to apply "direct" annotation first + if (!handledByComponentScan(classSymbolMetadata)) { + if (hasAnnotation(classSymbolMetadata, SpringUtils.SPRING_BOOT_APP_ANNOTATION)) { + // apply scan setting from @SpringBootApplication annotation + var targetedPackages = targetedPackages(classPackageName, classSymbolMetadata); + packagesScannedBySpringAtProjectLevel.addAll(targetedPackages); + packagesScannedBySpringAtFileLevel.addAll(targetedPackages); + } else if (hasAnnotation(classSymbolMetadata, SPRING_BEAN_ANNOTATIONS)) { + // include this class as a candidate for issue reporting + addMessageToMap(classPackageName, classTree.simpleName()); + } } } @@ -145,6 +146,20 @@ public void leaveFile(JavaFileScannerContext context) { packagesScannedBySpringAtFileLevel.clear(); } + private boolean handledByComponentScan(SymbolMetadata classSymbolMetadata) { + boolean handledByComponentScan = false; + List componentScanAttributes = classSymbolMetadata.valuesForAnnotation(COMPONENT_SCAN_ANNOTATION); + if (componentScanAttributes != null) { + List componentScanBaseAttributes = componentScanAttributes.stream().filter(v -> COMPONENT_SCAN_BASE_ARGUMENTS.contains(v.name())).toList(); + if (!componentScanBaseAttributes.isEmpty()) { + handledByComponentScan = true; + componentScanBaseAttributes.forEach(this::addToScannedPackages); + } + } + + return handledByComponentScan; + } + private static String cacheKey(InputFile inputFile) { return CACHE_KEY_PREFIX + inputFile.key(); } @@ -200,9 +215,6 @@ private void addMessageToMap(String classPackageName, IdentifierTree classNameTr } private void addToScannedPackages(SymbolMetadata.AnnotationValue annotationValue) { - if (!COMPONENT_SCAN_ARGUMENTS.contains(annotationValue.name())) { - return; - } if (annotationValue.value() instanceof Object[] objects) { for (Object o : objects) { if (o instanceof String oString) { diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java index 9ff1576e716..6c97b4a69e4 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java @@ -115,6 +115,7 @@ void testSpringBootApplication() { List files = Arrays.asList( mainCodeSourcesPath(testFolder + "Ko/Ko.java"), mainCodeSourcesPath(testFolder + "app/Ok/Ok.java"), + mainCodeSourcesPath(testFolder + "app/notOk/NotOkImpl.java"), mainCodeSourcesPath(testFolder + "app/SpringBootApp1.java"), mainCodeSourcesPath(testFolder + "secondApp/AnotherOk.java"), mainCodeSourcesPath(testFolder + "secondApp/SpringBootApp2.java")); From 19350912f3f38b4ea9eb2e284bf30194167444a5 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Thu, 5 Mar 2026 11:32:24 +0100 Subject: [PATCH 2/2] SONARJAVA-6184 S4605: FP when having SpringBootApplication followed by ComponentScan annotation --- .../java/checks/spring/s4605/mixed/app1/App1.java | 14 ++++++++++++++ .../s4605/mixed/app1/visible/VisibleService.java | 10 ++++++++++ .../springBootApplication/app/SpringBootApp1.java | 3 --- .../springBootApplication/app/notOk/NotOkImpl.java | 10 ---------- .../SpringBeansShouldBeAccessibleCheckTest.java | 14 +++++++++++++- 5 files changed, 37 insertions(+), 14 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s4605/mixed/app1/App1.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s4605/mixed/app1/visible/VisibleService.java delete mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/notOk/NotOkImpl.java diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/mixed/app1/App1.java b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/mixed/app1/App1.java new file mode 100644 index 00000000000..8ca05ee2996 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/mixed/app1/App1.java @@ -0,0 +1,14 @@ +package checks.spring.s4605.mixed.app1; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.FilterType; + +@SpringBootApplication +@ComponentScan(excludeFilters = {@ComponentScan.Filter(type = FilterType.REGEX, pattern = "checks.spring.s4605.mixed.app1.smth")}) +public class App1 { + static void main(String[] args) { + SpringApplication.run(checks.spring.s4605.mixed.app1.App1.class, args); + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/mixed/app1/visible/VisibleService.java b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/mixed/app1/visible/VisibleService.java new file mode 100644 index 00000000000..016763fb22a --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/mixed/app1/visible/VisibleService.java @@ -0,0 +1,10 @@ +package checks.spring.s4605.mixed.app1.visible; + +import org.springframework.stereotype.Component; + +interface VisibleServiceI { +} + +@Component +public class VisibleService implements VisibleServiceI { // Compliant +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/SpringBootApp1.java b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/SpringBootApp1.java index 8fb938c1da6..fd24507224b 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/SpringBootApp1.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/SpringBootApp1.java @@ -2,12 +2,9 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; -import org.springframework.context.annotation.ComponentScan; -import org.springframework.context.annotation.FilterType; import org.springframework.stereotype.Component; @SpringBootApplication -@ComponentScan(excludeFilters = {@ComponentScan.Filter(type = FilterType.REGEX, pattern = "checks.spring.s4605.springBootApplication.app.smth")}) public class SpringBootApp1 { public static void main(String[] args) { diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/notOk/NotOkImpl.java b/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/notOk/NotOkImpl.java deleted file mode 100644 index 5926c024c0a..00000000000 --- a/java-checks-test-sources/default/src/main/java/checks/spring/s4605/springBootApplication/app/notOk/NotOkImpl.java +++ /dev/null @@ -1,10 +0,0 @@ -package checks.spring.s4605.springBootApplication.app.notOk; - -import org.springframework.stereotype.Component; - -interface NotOk { -} - -@Component -public class NotOkImpl implements NotOk { // Compliant -} diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java index 6c97b4a69e4..dce585e41c2 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java @@ -115,7 +115,6 @@ void testSpringBootApplication() { List files = Arrays.asList( mainCodeSourcesPath(testFolder + "Ko/Ko.java"), mainCodeSourcesPath(testFolder + "app/Ok/Ok.java"), - mainCodeSourcesPath(testFolder + "app/notOk/NotOkImpl.java"), mainCodeSourcesPath(testFolder + "app/SpringBootApp1.java"), mainCodeSourcesPath(testFolder + "secondApp/AnotherOk.java"), mainCodeSourcesPath(testFolder + "secondApp/SpringBootApp2.java")); @@ -158,6 +157,19 @@ void testSpringBootApplicationWithAnnotation() { .verifyIssues(); } + @Test + void testBothAnnotationsTogether() { + final String folderApp = BASE_PATH + "mixed/app1/"; + List testFiles = Arrays.asList( + mainCodeSourcesPath(folderApp + "App1.java"), + mainCodeSourcesPath(folderApp + "visible/VisibleService.java")); + + CheckVerifier.newVerifier() + .onFiles(testFiles) + .withCheck(new SpringBeansShouldBeAccessibleCheck()) + .verifyNoIssues(); + } + @Test void caching() throws NoSuchAlgorithmException, IOException { var unchangedFiles = Stream.of(