SONARJAVA-6184 S4605: FP when having SpringBootApplication followed by ComponentScan annotation (#5510)

Author: asya-vorobeva

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);
+  }
+}

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
+}

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<String> COMPONENT_SCAN_ARGUMENTS = SetUtils.immutableSetOf("basePackages", "basePackageClasses", "value");
+  private static final Set<String> 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<SymbolMetadata.AnnotationValue> 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<SymbolMetadata.AnnotationValue> componentScanAttributes = classSymbolMetadata.valuesForAnnotation(COMPONENT_SCAN_ANNOTATION);
+    if (componentScanAttributes != null) {
+      List<SymbolMetadata.AnnotationValue> 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) {

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<String> 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(