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/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..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 @@ -157,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(