SONARJAVA-7166 S9352: Don't register beans declared in test scope

its/scanner-integration-tests/src/test/java/org/sonar/java/it/MavenBuildHelper.java

@@ -31,6 +31,8 @@ class MavenBuildHelper {
 
   static final String SOURCES_DIR = "src/main/java";
   static final String BINARIES_DIR = "target/classes";
+  static final String TEST_SOURCES_DIR = "src/test/java";
+  static final String TEST_BINARIES_DIR = "target/test-classes";
 
   private static final long BUILD_TIMEOUT_SECONDS = 45;
 
@@ -41,7 +43,15 @@ class MavenBuildHelper {
   }
 
   void build() {
-    runMaven("clean", "compile", "dependency:build-classpath", "-Dmdep.outputFile=target/classpath.txt");
+    runMaven("clean", "test-compile", "dependency:build-classpath", "-Dmdep.outputFile=target/classpath.txt");
+  }
+
+  /**
+   * Returns whether the given module declares test sources, so that {@code sonar.tests} is only set for modules
+   * that have them: the scanner fails on a path that does not exist.
+   */
+  boolean hasTestSources(String moduleName) {
+    return Files.isDirectory(projectBaseDir.resolve(moduleName).resolve(TEST_SOURCES_DIR));
   }
 
   String resolveLibraries(String moduleName, Set<String> allModuleNames) {

its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java

@@ -189,8 +189,13 @@ private Map<String, String> buildScannerProperties(Map<String, List<Path>> modul
       if (mavenHelper != null) {
         properties.put(prefix + "sonar.sources", MavenBuildHelper.SOURCES_DIR);
         properties.put(prefix + "sonar.java.binaries", MavenBuildHelper.BINARIES_DIR);
-        properties.put(prefix + "sonar.java.libraries",
-          mavenHelper.resolveLibraries(moduleName, moduleFiles.keySet()));
+        String libraries = mavenHelper.resolveLibraries(moduleName, moduleFiles.keySet());
+        properties.put(prefix + "sonar.java.libraries", libraries);
+        if (mavenHelper.hasTestSources(moduleName)) {
+          properties.put(prefix + "sonar.tests", MavenBuildHelper.TEST_SOURCES_DIR);
+          properties.put(prefix + "sonar.java.test.binaries", MavenBuildHelper.TEST_BINARIES_DIR);
+          properties.put(prefix + "sonar.java.test.libraries", libraries);
+        }
       } else {
         properties.put(prefix + "sonar.sources", ".");
         Path emptyBinDir = projectBaseDir.resolve(moduleName + "-binaries");

its/scanner-integration-tests/src/test/java/org/sonar/java/it/spring/AmbiguousDependencyCrossModuleTest.java

@@ -16,6 +16,7 @@
  */
 package org.sonar.java.it.spring;
 
+import com.sonarsource.scanner.integrationtester.dsl.issue.FileIssue;
 import com.sonarsource.scanner.integrationtester.dsl.issue.TextRange;
 import com.sonarsource.scanner.integrationtester.dsl.issue.TextRangeIssue;
 import org.junit.jupiter.api.Test;
@@ -53,4 +54,17 @@ void test() {
           "Multiple beans match this dependency (defaultAuditLogger, stagingAuditLogger) in profile 'staging'; disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\".",
           new TextRange(16, 16, 24, 35)));
   }
+
+  /**
+   * {@code app/src/test/java} declares a third PaymentGateway bean inside a nested {@code @Configuration} of a JUnit
+   * test class. Spring Boot's TestTypeExcludeFilter keeps it out of component scanning, so it must not be collected:
+   * were it, PaymentConsumer's issue above would name three candidates instead of two.
+   */
+  @Test
+  void beans_declared_in_test_scope_are_not_candidates() {
+    var issues = analyze(Path.of("ambiguous-dependencies-should-be-resolved"), "S9352");
+    assertThat(issues)
+      .extracting(FileIssue::message)
+      .noneMatch(message -> message.contains("testPaymentGateway"));
+  }
 }

its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml

@@ -33,5 +33,10 @@
       <groupId>org.springframework.boot</groupId>
       <artifactId>spring-boot-autoconfigure</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-api</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>

its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/test/java/com/example/app/PaymentConsumerTest.java

@@ -0,0 +1,32 @@
+package com.example.app;
+
+import com.example.common.PaymentGateway;
+import org.junit.jupiter.api.Test;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+/**
+ * CASE: a bean declared in test scope must not compete with the main-scope ones.
+ * <p>
+ * Spring Boot's TestTypeExcludeFilter keeps every class nested inside a test class out of component scanning, so
+ * testPaymentGateway never joins the context that holds CreditCardPaymentGateway and DigitalWalletPaymentGateway.
+ * If it were collected, PaymentConsumer's issue would name three candidates instead of two.
+ */
+class PaymentConsumerTest {
+
+    @Test
+    void paymentGatewayIsInjected() {
+        // the body is irrelevant: this class exists to be scanned, not run
+    }
+
+    @Configuration
+    static class TestPaymentGatewayConfiguration {
+
+        @Bean
+        PaymentGateway testPaymentGateway() {
+            return amount -> {
+                // no-op stub
+            };
+        }
+    }
+}

java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextModelGatherer.java

@@ -23,6 +23,7 @@
 import org.sonar.java.annotations.VisibleForTesting;
 import org.sonar.java.telemetry.Telemetry;
 import org.sonar.java.telemetry.TelemetryKey;
+import org.sonar.java.utils.JavaFileTypeClassifier;
 import org.sonar.plugins.java.api.DependencyVersionAware;
 import org.sonar.plugins.java.api.InputFileScannerContext;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
@@ -45,6 +46,12 @@
  * <p>All gatherers are skipped when none of {@code spring-context}, {@code spring-beans},
  * {@code spring-boot-starter}, or {@code spring-boot-starter-web} is present in the module classpath.
  *
+ * <p>Nothing is gathered from files in test scope, as determined by {@link JavaFileTypeClassifier#isTestFile}.
+ * Spring Boot's {@code TestTypeExcludeFilter} keeps test classes, {@code @TestConfiguration} classes, and
+ * anything nested inside them out of component scanning, so the beans and injection points they declare are
+ * never part of the application context the gathered model describes. {@link #leaveSpringFile} still runs for
+ * such files, so they publish an empty result and keep a valid cache entry.
+ *
  * <p>Extends {@link IssuableSubscriptionVisitor} so that {@link #leaveFile} is invoked by
  * {@code IssuableSubscriptionVisitorsRunner} after each file. This is required for per-file cache
  * writes: without it, cache entries are never stored and {@link #scanWithoutParsing} always misses,
@@ -56,6 +63,7 @@ public abstract class SpringContextModelGatherer extends IssuableSubscriptionVis
   private final Telemetry telemetry;
   private final LongSupplier nanoTime;
   private long gatheringTimeNanos;
+  private boolean currentFileIsInTestScope;
 
   protected SpringContextModelGatherer(SpringContextGatheringModel springContextGatheringModel, Telemetry telemetry) {
     this(springContextGatheringModel, telemetry, System::nanoTime);
@@ -77,8 +85,17 @@ public boolean isCompatibleWithDependencies(Function<String, Optional<Version>>
       .isPresent();
   }
 
+  @Override
+  public void setContext(JavaFileScannerContext context) {
+    super.setContext(context);
+    currentFileIsInTestScope = JavaFileTypeClassifier.isTestFile(context);
+  }
+
   @Override
   public final void visitNode(Tree tree) {
+    if (currentFileIsInTestScope) {
+      return;
+    }
     long startTime = nanoTime.getAsLong();
     try {
       visitSpringNode(tree);

java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java

@@ -20,11 +20,13 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Predicate;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.config.Configuration;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
 import org.sonar.plugins.java.api.semantic.SymbolMetadata;
 import org.sonar.plugins.java.api.tree.ClassTree;
+import org.sonar.plugins.java.api.tree.MethodTree;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonarsource.analyzer.commons.appsec.TestFileClassifier;
 
@@ -35,18 +37,26 @@
  * {@link InputFile.Type#MAIN} based on build-tool conventions, but this classification
  * can be incomplete (e.g. test helpers in {@code src/main/java}, misconfigured projects).
  *
- * <p>This classifier combines three signals:
+ * <p>This classifier combines four signals:
  * <ol>
  *   <li>Platform truth: {@link InputFile#type()} from the Sonar scanner</li>
  *   <li>Path and naming heuristics: delegated to {@link TestFileClassifier} from
  *       sonar-analyzer-commons, extended with Java-specific path conventions</li>
  *   <li>AST annotations: class-level test framework annotations ({@code @RunWith},
  *       {@code @SpringBootTest}, etc.)</li>
+ *   <li>Test methods: a class holding JUnit/TestNG test methods, via
+ *       {@link UnitTestUtils#isTestClass(ClassTree)} and the methods a type declares itself</li>
  * </ol>
  *
+ * <p>The last two signals mirror the markers Spring Boot's {@code TestTypeExcludeFilter} uses to keep
+ * test classes, and anything nested inside them, out of component scanning. Both examine nested types at
+ * any depth, not only the top-level ones, because that filter re-applies itself to a candidate's enclosing
+ * class recursively: a test class buried inside an otherwise unremarkable outer class still takes everything
+ * nested within it out of scanning.
+ *
  * <p>A file is considered a test file if <em>any</em> signal indicates it. The platform type
  * takes priority when {@code TEST}, but a {@code MAIN}-typed file can be upgraded to test scope
- * by the path, naming, or annotation signals.
+ * by the path, naming, annotation, or test-method signals.
  *
  * <p>The path/naming heuristic is only applied when {@code sonar.tests} is not configured; if it
  * is configured the platform already classifies test files as {@link InputFile.Type#TEST}.
@@ -63,17 +73,20 @@ public final class JavaFileTypeClassifier {
   /**
    * Class-level annotations that unambiguously mark a class as part of a test framework.
    * Method-level annotations (e.g. JUnit 4/5 {@code @Test}) are intentionally excluded here;
-   * use {@code UnitTestUtils.isTestClass(ClassTree)} for method-level detection.
+   * they are covered by {@link #declaresTestMethods} instead.
    */
   private static final Set<String> TEST_CLASS_ANNOTATIONS = Set.of(
     // JUnit 4
     "org.junit.runner.RunWith",
     // JUnit 5
     "org.junit.jupiter.api.extension.ExtendWith",
+    // JUnit Platform (meta-annotation carried by @Test, @TestFactory, @TestTemplate...)
+    "org.junit.platform.commons.annotation.Testable",
     // TestNG (class-level @Test marks all methods as tests)
     "org.testng.annotations.Test",
     // Spring Boot Test
     "org.springframework.boot.test.context.SpringBootTest",
+    "org.springframework.boot.test.context.TestConfiguration",
     // Spring Test (used for integration tests without Spring Boot)
     "org.springframework.test.context.ContextConfiguration"
   );
@@ -147,8 +160,22 @@ private JavaFileTypeClassifier() {
    */
   public static boolean isTestFile(JavaFileScannerContext context) {
     return isPlatformTestFile(context)
-      || getClassifier(context.getConfiguration()).looksLikeTestFile(context.getInputFile())
-      || hasTestFrameworkAnnotation(context);
+      || looksLikeTestPath(context)
+      || hasTestFrameworkAnnotation(context)
+      || declaresTestMethods(context);
+  }
+
+  /**
+   * Returns {@code true} if the path/naming heuristic recognizes the file as test code.
+   *
+   * <p>The heuristic is the only signal that needs the module {@link Configuration}, which a context built without
+   * {@code SonarComponents} does not have. It is skipped in that case rather than failing, so that the platform,
+   * annotation and test-method signals keep working: throwing here would be swallowed by the scanner and would
+   * silently turn every file into non-test code.
+   */
+  private static boolean looksLikeTestPath(JavaFileScannerContext context) {
+    Configuration configuration = context.getConfiguration();
+    return configuration != null && getClassifier(configuration).looksLikeTestFile(context.getInputFile());
   }
 
   /**
@@ -162,22 +189,69 @@ static boolean isPlatformTestFile(JavaFileScannerContext context) {
   }
 
   /**
-   * Returns {@code true} if any top-level class in the compilation unit carries a recognized
-   * test framework annotation at the class level.
+   * Returns {@code true} if any type in the compilation unit carries a recognized test framework
+   * annotation at the class level.
    *
    * <p>Gracefully returns {@code false} if the file was not successfully parsed
    * ({@link JavaFileScannerContext#fileParsed()} is {@code false}).
    *
    * @param context the current file scanner context
    */
   static boolean hasTestFrameworkAnnotation(JavaFileScannerContext context) {
+    return anyTypeMatches(context, JavaFileTypeClassifier::hasTestAnnotation);
+  }
+
+  /**
+   * Returns {@code true} if any type in the compilation unit declares test methods.
+   *
+   * <p>This covers the method-level markers that {@link #TEST_CLASS_ANNOTATIONS} deliberately omits,
+   * such as a plain class holding JUnit {@code @Test} methods. A top-level type is matched through
+   * {@link UnitTestUtils#isTestClass(ClassTree)}, which also recognizes a class whose only test code sits in
+   * {@code @Nested} members; any type is matched by declaring test methods itself, which is what catches a
+   * nested test class whose enclosing type carries no test marker of its own.
+   *
+   * @param context the current file scanner context
+   */
+  static boolean declaresTestMethods(JavaFileScannerContext context) {
+    return anyTypeMatches(context, type -> UnitTestUtils.isTestClass(type) || declaresTestMethodsItself(type));
+  }
+
+  private static boolean declaresTestMethodsItself(ClassTree classTree) {
+    return classTree.members().stream()
+      .filter(member -> member.is(Tree.Kind.METHOD))
+      .map(MethodTree.class::cast)
+      .anyMatch(UnitTestUtils::hasTestAnnotation);
+  }
+
+  /**
+   * Applies {@code predicate} to every type declared in the compilation unit, nested ones included, at any depth.
+   *
+   * <p>Descending matters because Spring Boot's {@code TestTypeExcludeFilter} resolves the enclosing class of a
+   * candidate and re-applies itself to it, recursively: a class nested inside a test class is excluded from
+   * component scanning however deep it sits, and however unremarkable the outermost class looks. Walking down
+   * from the compilation unit reaches the same set as walking up from each candidate.
+   *
+   * <p>Gracefully returns {@code false} if the file was not successfully parsed
+   * ({@link JavaFileScannerContext#fileParsed()} is {@code false}).
+   */
+  private static boolean anyTypeMatches(JavaFileScannerContext context, Predicate<ClassTree> predicate) {
     if (!context.fileParsed()) {
       return false;
     }
     return context.getTree().types().stream()
-      .filter(tree -> tree.is(Tree.Kind.CLASS))
+      .filter(ClassTree.class::isInstance)
+      .map(ClassTree.class::cast)
+      .anyMatch(type -> matchesTypeOrNested(type, predicate));
+  }
+
+  private static boolean matchesTypeOrNested(ClassTree classTree, Predicate<ClassTree> predicate) {
+    if (predicate.test(classTree)) {
+      return true;
+    }
+    return classTree.members().stream()
+      .filter(ClassTree.class::isInstance)
       .map(ClassTree.class::cast)
-      .anyMatch(JavaFileTypeClassifier::hasTestAnnotation);
+      .anyMatch(nested -> matchesTypeOrNested(nested, predicate));
   }
 
   private static boolean hasTestAnnotation(ClassTree classTree) {

java-frontend/src/test/files/springcontext/NestedTestClassWithConfiguration.java

@@ -0,0 +1,25 @@
+package checks.spring.context;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+class NestedTestClassWithConfiguration {
+
+  static class InnerTest {
+
+    @Test
+    void application_context_is_customized() {
+    }
+
+    @Configuration
+    static class NestedConfiguration {
+
+      @Bean
+      ApplicationContext nestedApplicationContext() {
+        return null;
+      }
+    }
+  }
+}

java-frontend/src/test/files/springcontext/TestClassWithNestedConfiguration.java

@@ -0,0 +1,30 @@
+package checks.spring.context;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.stereotype.Component;
+
+class TestClassWithNestedConfiguration {
+
+  @Test
+  void application_context_is_customized() {
+  }
+
+  @Configuration
+  static class NestedConfiguration {
+
+    @Bean
+    ApplicationContext nestedApplicationContext() {
+      return null;
+    }
+  }
+
+  static class NestedHolder {
+
+    @Component
+    static class DeeplyNestedComponent {
+    }
+  }
+}

java-frontend/src/test/files/springcontext/TestConfigurationWithNestedConfiguration.java

@@ -0,0 +1,19 @@
+package checks.spring.context;
+
+import org.springframework.boot.test.context.TestConfiguration;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+@TestConfiguration
+class TestConfigurationWithNestedConfiguration {
+
+  @Configuration
+  static class NestedConfiguration {
+
+    @Bean
+    ApplicationContext nestedApplicationContext() {
+      return null;
+    }
+  }
+}