From ffaa56a1313ae90c2f8ccba7b2c20a1404911dfe Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Mon, 12 Oct 2026 12:18:16 +0200 Subject: [PATCH 1/3] SONARJAVA-7166 S9352: Don't register beans declared in test scope Spring Boot's TestTypeExcludeFilter keeps test classes, @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 Spring context model describes. Collecting them let test beans compete with main ones and raised false positives such as the nested test @Configuration in Spring Initializr's InitializrAutoConfigurationTests. JavaFileTypeClassifier gains the two signals that filter keys on: @TestConfiguration and @Testable among the class-level annotations, and declaresTestMethods, which defers to UnitTestUtils.isTestClass for the method-level markers the annotation set deliberately omits. The gatherers are gated in SpringContextModelGatherer rather than in BeanDefinitionGatherer alone, so a @ComponentScan declared on a test class no longer widens the production scan scope either. leaveSpringFile still runs for a test file, which publishes an empty result and keeps a valid cache entry so unchanged files are not re-parsed. The scanner integration test harness now builds with test-compile and declares sonar.tests along with the test binaries and libraries when a module has test sources, which is what lets the new fixture in AmbiguousDependencyCrossModuleTest cover the real scanner path: platform test-type assignment and the MAIN/TEST gatherer registration in JavaSensor. Co-Authored-By: Claude Opus 5 (1M context) --- .../org/sonar/java/it/MavenBuildHelper.java | 12 +++- .../it/ScannerIntegrationAbstractTest.java | 9 ++- .../AmbiguousDependencyCrossModuleTest.java | 14 +++++ .../app/pom.xml | 5 ++ .../com/example/app/PaymentConsumerTest.java | 32 +++++++++++ .../SpringContextModelGatherer.java | 17 ++++++ .../java/utils/JavaFileTypeClassifier.java | 43 +++++++++++++-- .../TestClassWithNestedConfiguration.java | 30 ++++++++++ ...tConfigurationWithNestedConfiguration.java | 19 +++++++ .../utils/SampleWithNestedTestClass.java | 13 +++++ .../utils/SampleWithTestConfiguration.java | 5 ++ .../files/utils/SampleWithTestMethods.java | 8 +++ .../BeanDefinitionGathererTest.java | 55 ++++++++++++++++++- .../ComponentScanPackageGathererTest.java | 9 ++- .../SpringContextGathererTest.java | 28 +++++++++- .../SpringContextModelGathererTest.java | 2 +- .../utils/JavaFileTypeClassifierTest.java | 55 +++++++++++++++++-- 17 files changed, 337 insertions(+), 19 deletions(-) create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/test/java/com/example/app/PaymentConsumerTest.java create mode 100644 java-frontend/src/test/files/springcontext/TestClassWithNestedConfiguration.java create mode 100644 java-frontend/src/test/files/springcontext/TestConfigurationWithNestedConfiguration.java create mode 100644 java-frontend/src/test/files/utils/SampleWithNestedTestClass.java create mode 100644 java-frontend/src/test/files/utils/SampleWithTestConfiguration.java create mode 100644 java-frontend/src/test/files/utils/SampleWithTestMethods.java diff --git a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/MavenBuildHelper.java b/its/scanner-integration-tests/src/test/java/org/sonar/java/it/MavenBuildHelper.java index 84dd7d56051..f182ab36550 100644 --- a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/MavenBuildHelper.java +++ b/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 allModuleNames) { diff --git a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java b/its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java index 7c98fc93ef1..81cedfa61b5 100644 --- a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java +++ b/its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java @@ -189,8 +189,13 @@ private Map buildScannerProperties(Map> 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"); diff --git a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/spring/AmbiguousDependencyCrossModuleTest.java b/its/scanner-integration-tests/src/test/java/org/sonar/java/it/spring/AmbiguousDependencyCrossModuleTest.java index c6719dfcdb9..b45da7f059b 100644 --- a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/spring/AmbiguousDependencyCrossModuleTest.java +++ b/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")); + } } diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml index 18a40f2557f..21d9e07891c 100644 --- a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml @@ -33,5 +33,10 @@ org.springframework.boot spring-boot-autoconfigure + + org.junit.jupiter + junit-jupiter-api + test + diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/test/java/com/example/app/PaymentConsumerTest.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/test/java/com/example/app/PaymentConsumerTest.java new file mode 100644 index 00000000000..460ec9c7e9c --- /dev/null +++ b/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. + *

+ * 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 + }; + } + } +} diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextModelGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextModelGatherer.java index 48c9b8011fc..0eb2891efeb 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextModelGatherer.java +++ b/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 @@ *

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. * + *

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. + * *

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> .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); diff --git a/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java b/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java index 608c397b0c3..8d1eb0dbee2 100644 --- a/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java +++ b/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java @@ -20,6 +20,7 @@ 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; @@ -35,18 +36,23 @@ * {@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). * - *

This classifier combines three signals: + *

This classifier combines four signals: *

    *
  1. Platform truth: {@link InputFile#type()} from the Sonar scanner
    2. *
  2. Path and naming heuristics: delegated to {@link TestFileClassifier} from * sonar-analyzer-commons, extended with Java-specific path conventions
    3. *
  3. AST annotations: class-level test framework annotations ({@code @RunWith}, * {@code @SpringBootTest}, etc.)
    4. + *
  4. Test methods: a top-level class holding JUnit/TestNG test methods, via + * {@link UnitTestUtils#isTestClass(ClassTree)}
    5. *
* + *

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. + * *

A file is considered a test file if any 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. * *

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 +69,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 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" ); @@ -148,7 +157,8 @@ private JavaFileTypeClassifier() { public static boolean isTestFile(JavaFileScannerContext context) { return isPlatformTestFile(context) || getClassifier(context.getConfiguration()).looksLikeTestFile(context.getInputFile()) - || hasTestFrameworkAnnotation(context); + || hasTestFrameworkAnnotation(context) + || declaresTestMethods(context); } /** @@ -171,13 +181,36 @@ static boolean isPlatformTestFile(JavaFileScannerContext context) { * @param context the current file scanner context */ static boolean hasTestFrameworkAnnotation(JavaFileScannerContext context) { + return anyTopLevelClassMatches(context, JavaFileTypeClassifier::hasTestAnnotation); + } + + /** + * Returns {@code true} if any top-level class in the compilation unit declares test methods, + * as determined by {@link UnitTestUtils#isTestClass(ClassTree)}. + * + *

This covers the method-level markers that {@link #TEST_CLASS_ANNOTATIONS} deliberately omits, + * such as a plain class holding JUnit {@code @Test} methods. + * + * @param context the current file scanner context + */ + static boolean declaresTestMethods(JavaFileScannerContext context) { + return anyTopLevelClassMatches(context, UnitTestUtils::isTestClass); + } + + /** + * Applies {@code predicate} to every top-level class of the compilation unit. + * + *

Gracefully returns {@code false} if the file was not successfully parsed + * ({@link JavaFileScannerContext#fileParsed()} is {@code false}). + */ + private static boolean anyTopLevelClassMatches(JavaFileScannerContext context, Predicate predicate) { if (!context.fileParsed()) { return false; } return context.getTree().types().stream() .filter(tree -> tree.is(Tree.Kind.CLASS)) .map(ClassTree.class::cast) - .anyMatch(JavaFileTypeClassifier::hasTestAnnotation); + .anyMatch(predicate); } private static boolean hasTestAnnotation(ClassTree classTree) { diff --git a/java-frontend/src/test/files/springcontext/TestClassWithNestedConfiguration.java b/java-frontend/src/test/files/springcontext/TestClassWithNestedConfiguration.java new file mode 100644 index 00000000000..aca7df9a9bb --- /dev/null +++ b/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 { + } + } +} diff --git a/java-frontend/src/test/files/springcontext/TestConfigurationWithNestedConfiguration.java b/java-frontend/src/test/files/springcontext/TestConfigurationWithNestedConfiguration.java new file mode 100644 index 00000000000..b5c797bafdc --- /dev/null +++ b/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; + } + } +} diff --git a/java-frontend/src/test/files/utils/SampleWithNestedTestClass.java b/java-frontend/src/test/files/utils/SampleWithNestedTestClass.java new file mode 100644 index 00000000000..29eb323d1f7 --- /dev/null +++ b/java-frontend/src/test/files/utils/SampleWithNestedTestClass.java @@ -0,0 +1,13 @@ +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +class SampleWithNestedTestClass { + + @Nested + class Inner { + + @Test + void does_something() { + } + } +} diff --git a/java-frontend/src/test/files/utils/SampleWithTestConfiguration.java b/java-frontend/src/test/files/utils/SampleWithTestConfiguration.java new file mode 100644 index 00000000000..f2a3bafc160 --- /dev/null +++ b/java-frontend/src/test/files/utils/SampleWithTestConfiguration.java @@ -0,0 +1,5 @@ +import org.springframework.boot.test.context.TestConfiguration; + +@TestConfiguration +class SampleWithTestConfiguration { +} diff --git a/java-frontend/src/test/files/utils/SampleWithTestMethods.java b/java-frontend/src/test/files/utils/SampleWithTestMethods.java new file mode 100644 index 00000000000..85543071860 --- /dev/null +++ b/java-frontend/src/test/files/utils/SampleWithTestMethods.java @@ -0,0 +1,8 @@ +import org.junit.jupiter.api.Test; + +class SampleWithTestMethods { + + @Test + void does_something() { + } +} diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index 071e912b4d4..26fc1d3c85c 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -34,6 +34,7 @@ import org.sonar.api.batch.sensor.cache.WriteCache; import org.sonar.java.TestUtils; import org.sonar.java.telemetry.NoOpTelemetry; +import org.sonar.java.test.classpath.TestClasspathUtils; import org.sonar.plugins.java.api.InputFileScannerContext; import org.sonar.plugins.java.api.caching.CacheContext; import org.sonar.plugins.java.api.caching.JavaReadCache; @@ -279,6 +280,58 @@ void bean_package_is_captured() { assertThat(beans.get(0).getBeanPackage()).isEqualTo("checks.spring.context"); } + // ---- Test scope is excluded, mirroring Spring Boot's TestTypeExcludeFilter ---- + + @Test + void beans_in_a_file_the_scanner_classified_as_test_are_not_collected() { + scanAsTestScope("src/test/files/springcontext/SimpleComponent.java"); + + assertThat(model.getTypeToBeansIndex().getKeys()).isEmpty(); + assertThat(model.getBeanDefinitionRegistry().getByName("simpleComponent")).isEmpty(); + } + + @Test + void beans_nested_inside_a_class_holding_test_methods_are_not_collected() { + scan("src/test/files/springcontext/TestClassWithNestedConfiguration.java"); + + assertThat(model.getTypeToBeansIndex().getKeys()).isEmpty(); + assertThat(model.getBeanDefinitionRegistry().getByName("nestedConfiguration")).isEmpty(); + assertThat(model.getBeanDefinitionRegistry().getByName("nestedApplicationContext")).isEmpty(); + assertThat(model.getBeanDefinitionRegistry().getByName("deeplyNestedComponent")).isEmpty(); + } + + @Test + void beans_nested_inside_a_TestConfiguration_class_are_not_collected() { + scan("src/test/files/springcontext/TestConfigurationWithNestedConfiguration.java"); + + assertThat(model.getTypeToBeansIndex().getKeys()).isEmpty(); + assertThat(model.getBeanDefinitionRegistry().getByName("nestedConfiguration")).isEmpty(); + assertThat(model.getBeanDefinitionRegistry().getByName("nestedApplicationContext")).isEmpty(); + } + + @Test + void injection_points_in_test_scope_are_not_collected() { + scanAsTestScope("src/test/files/springcontext/QualifiedFieldDependencies.java"); + + assertThat(model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.context.ApplicationContext")).isEmpty(); + } + + @Test + void a_test_file_still_writes_an_empty_cache_entry() { + WriteCache writeCache = mock(WriteCache.class); + SensorContextTester ctx = mainScopeSensorContext(); + ctx.setCacheEnabled(true); + ctx.setNextCache(writeCache); + + scan(TestClasspathUtils.DEFAULT_MODULE.getClassPath(), ctx, InputFile.Type.TEST, + "src/test/files/springcontext/SimpleComponent.java"); + + var dataCaptor = ArgumentCaptor.forClass(byte[].class); + verify(writeCache).write(anyString(), dataCaptor.capture()); + assertThat(JsonParser.parseString(new String(dataCaptor.getValue(), StandardCharsets.UTF_8)) + .getAsJsonObject().getAsJsonArray("beans")).isEmpty(); + } + // ---- Caching: what the gatherer collects reaches the cache and comes back ---- @Test @@ -415,7 +468,7 @@ private record CacheEntry(String key, byte[] data) { */ private CacheEntry scanAndCaptureCacheEntry(String filePath) { WriteCache writeCache = mock(WriteCache.class); - SensorContextTester ctx = SensorContextTester.create(new File("")); + SensorContextTester ctx = mainScopeSensorContext(); ctx.setCacheEnabled(true); ctx.setNextCache(writeCache); diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/ComponentScanPackageGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/ComponentScanPackageGathererTest.java index 7ca883b766f..91121305613 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/ComponentScanPackageGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/ComponentScanPackageGathererTest.java @@ -66,6 +66,13 @@ void componentScan_value_and_basePackages_attributes_are_collected() { ); } + @Test + void componentScan_declared_in_test_scope_does_not_widen_the_scan_scope() { + scanAsTestScope("src/test/files/springcontext/ComponentScanWithBasePackages.java"); + + assertThat(model.getProjectPackageScan().getPackagesForModule(MODULE_KEY)).isEmpty(); + } + // ---- @SpringBootApplication ----------------------------------------------- @Test @@ -159,7 +166,7 @@ void packages_from_multiple_files_are_merged() { @Test void leaveFile_writes_collected_packages_to_cache() { WriteCache writeCache = mock(WriteCache.class); - SensorContextTester ctx = SensorContextTester.create(new File("")); + SensorContextTester ctx = mainScopeSensorContext(); ctx.setCacheEnabled(true); ctx.setNextCache(writeCache); diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextGathererTest.java index 3cc66e11a14..8c8e4258c4e 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextGathererTest.java @@ -30,6 +30,7 @@ import org.sonar.plugins.java.api.caching.CacheContext; import org.sonar.plugins.java.api.caching.JavaReadCache; import org.sonar.plugins.java.api.caching.JavaWriteCache; +import org.sonar.scanner.plugin.api.impl.config.MapSettings; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -40,26 +41,47 @@ abstract class SpringContextGathererTest { protected SpringContextModelGatherer gatherer; protected SpringContextModel model; + /** + * The sample files of these tests live under {@code src/test/files/}, a path that + * {@link org.sonar.java.utils.JavaFileTypeClassifier}'s heuristic would read as test scope, which would stop the + * gatherers from collecting anything. Declaring {@code sonar.tests} disables that heuristic, as it does in a real + * analysis where the scanner already assigns {@link InputFile.Type#TEST} itself. + */ + protected static SensorContextTester mainScopeSensorContext() { + return SensorContextTester.create(new File("")).setSettings(new MapSettings().setProperty("sonar.tests", "src/test/java")); + } + protected void scan(String... filePaths) { - scan(TestClasspathUtils.DEFAULT_MODULE.getClassPath(), SensorContextTester.create(new File("")), filePaths); + scan(TestClasspathUtils.DEFAULT_MODULE.getClassPath(), mainScopeSensorContext(), filePaths); } protected void scan(List classpath, String... filePaths) { - scan(classpath, SensorContextTester.create(new File("")), filePaths); + scan(classpath, mainScopeSensorContext(), filePaths); } protected void scan(SensorContextTester ctx, String... filePaths) { scan(TestClasspathUtils.DEFAULT_MODULE.getClassPath(), ctx, filePaths); } + /** + * Scans the given files as the scanner would when the build tool places them in the test source set. + */ + protected void scanAsTestScope(String... filePaths) { + scan(TestClasspathUtils.DEFAULT_MODULE.getClassPath(), mainScopeSensorContext(), InputFile.Type.TEST, filePaths); + } + protected void scan(List classpath, SensorContextTester ctx, String... filePaths) { + scan(classpath, ctx, InputFile.Type.MAIN, filePaths); + } + + protected void scan(List classpath, SensorContextTester ctx, InputFile.Type type, String... filePaths) { var sonarComponents = new SonarComponents(null, null, null, null, null, null); sonarComponents.setSensorContext(ctx); VisitorsBridge visitorsBridge = new VisitorsBridge(List.of((JavaCheck) gatherer), classpath, sonarComponents); for (String filePath : filePaths) { File file = new File(filePath); var compilationUnit = JParserTestUtils.parse(file, classpath); - visitorsBridge.setCurrentFile(TestUtils.inputFile(file)); + visitorsBridge.setCurrentFile(TestUtils.inputFile("", file, type)); visitorsBridge.visitFile(compilationUnit, false); } visitorsBridge.endOfAnalysis(); diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextModelGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextModelGathererTest.java index 31b8eee5f44..5c0edb93347 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextModelGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextModelGathererTest.java @@ -118,7 +118,7 @@ private void scanFile(String filePath, JavaCheck check, List classpath) { } private VisitorsBridge bridgeFor(JavaCheck check, List classpath) { - SensorContextTester sensorContextTester = SensorContextTester.create(new File("")); + SensorContextTester sensorContextTester = SpringContextGathererTest.mainScopeSensorContext(); var sonarComponents = new SonarComponents(null, null, null, null, null, null); sonarComponents.setSensorContext(sensorContextTester); return new VisitorsBridge(List.of(check), classpath, sonarComponents); diff --git a/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java b/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java index 844897e0bb3..cbbaf855b80 100644 --- a/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java +++ b/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java @@ -86,22 +86,67 @@ void hasTestFrameworkAnnotation_detects_SpringAutoconfigure_WebMvcTest() { assertAnnotationSignal(true, "src/test/files/utils/SampleWithWebMvcTest.java"); } + @Test + void hasTestFrameworkAnnotation_detects_TestConfiguration() { + assertAnnotationSignal(true, "src/test/files/utils/SampleWithTestConfiguration.java"); + } + @Test void hasTestFrameworkAnnotation_returnsFalse_forPlainClass() { assertAnnotationSignal(false, "src/test/files/utils/PlainSample.java"); } + @Test + void hasTestFrameworkAnnotation_returnsFalse_forClassWithOnlyTestMethods() { + // Method-level markers are deliberately out of scope here; declaresTestMethods covers them. + assertAnnotationSignal(false, "src/test/files/utils/SampleWithTestMethods.java"); + } + private void assertAnnotationSignal(boolean expected, String filePath) { + assertThat(JavaFileTypeClassifier.hasTestFrameworkAnnotation(contextFor(filePath))) + .as("Expected hasTestFrameworkAnnotation=%s for '%s'", expected, filePath) + .isEqualTo(expected); + } + + // ------------------------------------------------------------------------- + // declaresTestMethods + // ------------------------------------------------------------------------- + + @Test + void declaresTestMethods_returnsFalse_whenNotParsed() { + JavaFileScannerContext context = mock(JavaFileScannerContext.class); + when(context.fileParsed()).thenReturn(false); + assertThat(JavaFileTypeClassifier.declaresTestMethods(context)).isFalse(); + } + + @Test + void declaresTestMethods_detects_plainClassHoldingTestMethods() { + assertTestMethodSignal(true, "src/test/files/utils/SampleWithTestMethods.java"); + } + + @Test + void declaresTestMethods_detects_plainClassHoldingNestedTestClass() { + assertTestMethodSignal(true, "src/test/files/utils/SampleWithNestedTestClass.java"); + } + + @Test + void declaresTestMethods_returnsFalse_forPlainClass() { + assertTestMethodSignal(false, "src/test/files/utils/PlainSample.java"); + } + + private void assertTestMethodSignal(boolean expected, String filePath) { + assertThat(JavaFileTypeClassifier.declaresTestMethods(contextFor(filePath))) + .as("Expected declaresTestMethods=%s for '%s'", expected, filePath) + .isEqualTo(expected); + } + + private static JavaFileScannerContext contextFor(String filePath) { var classpath = TestClasspathUtils.DEFAULT_MODULE.getClassPath(); var bridge = new VisitorsBridgeForTests.Builder(List.of()) .enableSemanticWithProjectClasspath(classpath) .build(); JavaAstScanner.scanSingleFileForTests(TestUtils.inputFile(filePath), bridge); - - JavaFileScannerContext context = bridge.testContexts().get(0); - assertThat(JavaFileTypeClassifier.hasTestFrameworkAnnotation(context)) - .as("Expected hasTestFrameworkAnnotation=%s for '%s'", expected, filePath) - .isEqualTo(expected); + return bridge.testContexts().get(0); } // ------------------------------------------------------------------------- From 229385acab013b7789c64f9b11cc7ed6d69322a3 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Mon, 12 Oct 2026 14:40:06 +0200 Subject: [PATCH 2/3] SONARJAVA-7166 Fix the tests broken by the test-scope exclusion JavaFrontendTest and SpringContextModelTest both asserted that a bean declared in a test file is registered, which is exactly what the exclusion changes. Their expectations now state the new behaviour, and the main-scope assertions they also carry are left untouched. Change SpringContextModelTest to have a Configuration: its main fixture lives under src/test/files/, which the path heuristic reads as test scope, so it declares sonar.tests the way a real analysis does. JavaSensor handed the gatherers to RuleScope.TEST as well, which is now a guaranteed no-op: getTestFiles collects InputFile.Type.TEST files, those feed testChecks and astScannerForTests, and the test-scope exclusion short-circuits every one of them before anything is gathered. Dropping the registration also spares each test file a setContext and an empty cache write, and stops the gatherers from reporting a scanWithoutParsing miss for test files on a first run, which would have forced unchanged ones to be parsed. The exclusion itself stays: RuleScope.TEST is a strict subset of what JavaFileTypeClassifier covers, so a main-scope file that is test code by its path, its annotations or its test methods still reaches the gatherers and is still excluded by them. JavaFrontendTest keeps registering both scopes on purpose, so that the exclusion is pinned on its own rather than only through this wiring. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/utils/JavaFileTypeClassifier.java | 15 ++++++++++- .../java/org/sonar/java/JavaFrontendTest.java | 8 ++++-- .../springcontext/SpringContextModelTest.java | 12 ++++++--- .../utils/JavaFileTypeClassifierTest.java | 25 ++++++++++++++++++- .../org/sonar/plugins/java/JavaSensor.java | 5 ++-- 5 files changed, 55 insertions(+), 10 deletions(-) diff --git a/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java b/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java index 8d1eb0dbee2..d3b091cfd39 100644 --- a/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java +++ b/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java @@ -156,11 +156,24 @@ private JavaFileTypeClassifier() { */ public static boolean isTestFile(JavaFileScannerContext context) { return isPlatformTestFile(context) - || getClassifier(context.getConfiguration()).looksLikeTestFile(context.getInputFile()) + || looksLikeTestPath(context) || hasTestFrameworkAnnotation(context) || declaresTestMethods(context); } + /** + * Returns {@code true} if the path/naming heuristic recognizes the file as test code. + * + *

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()); + } + /** * Returns {@code true} if the platform scanner classified this file as {@link InputFile.Type#TEST}. * This is the authoritative signal — it cannot be overridden by the other signals. diff --git a/java-frontend/src/test/java/org/sonar/java/JavaFrontendTest.java b/java-frontend/src/test/java/org/sonar/java/JavaFrontendTest.java index fdfd31bf296..368f9ed81aa 100644 --- a/java-frontend/src/test/java/org/sonar/java/JavaFrontendTest.java +++ b/java-frontend/src/test/java/org/sonar/java/JavaFrontendTest.java @@ -241,7 +241,7 @@ void scanning_empty_project_should_be_logged_in_autoscan() { } @Test - void spring_bean_is_registered_once_when_main_and_test_scanners_finish() throws IOException { + void spring_bean_is_registered_once_and_only_from_main_scope_when_both_scanners_finish() throws IOException { sensorContext = SensorContextTester.create(temp.toFile().getAbsoluteFile()); sensorContext.setRuntime(SONARQUBE_RUNTIME); InputFile mainFile = addFile(temp, "@org.springframework.stereotype.Component class MainBean {}", sensorContext); @@ -259,6 +259,8 @@ void spring_bean_is_registered_once_when_main_and_test_scanners_finish() throws sonarComponents.setSensorContext(sensorContext); var gatheringModel = new SpringContextGatheringModel(); var telemetry = new NoOpTelemetry(); + // JavaSensor registers the gatherers for RuleScope.MAIN only. Registering them for both scopes here is + // deliberate: it checks that the test-scope exclusion holds on its own, not merely because of that wiring. SpringContextModelGatherers.getAllGatherers(gatheringModel, telemetry) .forEach(gatherer -> sonarComponents.registerCustomFileScanner(RuleScope.MAIN, gatherer)); SpringContextModelGatherers.getAllGatherers(gatheringModel, telemetry) @@ -270,7 +272,9 @@ void spring_bean_is_registered_once_when_main_and_test_scanners_finish() throws var springContextModel = SpringContextModel.of(gatheringModel); assertThat(springContextModel.getBeanDefinitionRegistry().getByName("mainBean")).hasSize(1); - assertThat(springContextModel.getBeanDefinitionRegistry().getByName("helperTest")).hasSize(1); + // HelperTest is a test file, so its bean is not registered: Spring Boot's TestTypeExcludeFilter keeps test + // classes out of component scanning. + assertThat(springContextModel.getBeanDefinitionRegistry().getByName("helperTest")).isEmpty(); } @Test diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextModelTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextModelTest.java index 76ba96eeace..9c10638900c 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextModelTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/SpringContextModelTest.java @@ -29,6 +29,7 @@ import org.sonar.java.test.classpath.TestClasspathUtils; import org.sonar.plugins.java.api.JavaCheck; import org.sonar.plugins.java.api.JavaResourceLocator; +import org.sonar.scanner.plugin.api.impl.config.MapSettings; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -53,6 +54,10 @@ void scan_fills_project_package_scan_in_spring_context_model() { when(sonarComponents.getJavaClasspath()).thenReturn(TestClasspathUtils.DEFAULT_MODULE.getClassPath()); when(sonarComponents.getJavaTestClasspath()).thenReturn(TestClasspathUtils.DEFAULT_MODULE.getClassPath()); when(sonarComponents.getModuleKey()).thenReturn("a"); + // The main fixture below lives under src/test/files/, which the path heuristic would read as test scope; + // declaring sonar.tests disables it, as it does in a real analysis. + when(sonarComponents.getConfiguration()) + .thenReturn(new MapSettings().setProperty("sonar.tests", "src/test/java").asConfig()); var gatheringModel = new SpringContextGatheringModel(); var telemetry = new NoOpTelemetry(); @@ -71,10 +76,9 @@ void scan_fills_project_package_scan_in_spring_context_model() { assertThat(springContextModel.getProjectPackageScan().getModules()).isNotEmpty(); assertThat(springContextModel.getProjectPackageScan().getPackagesForModule("a")) .containsExactly("springcontext"); - assertThat(springContextModel.getBeanDefinitionRegistry().getByName("springContextComponent")) - .hasSize(1) - .first() - .satisfies(bean -> assertThat(bean.getType()).isEqualTo("springcontext.SpringContextComponent")); + // SpringContextComponent is scanned as a test file, so its bean is not registered: Spring Boot's + // TestTypeExcludeFilter keeps test classes out of component scanning. + assertThat(springContextModel.getBeanDefinitionRegistry().getByName("springContextComponent")).isEmpty(); } } diff --git a/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java b/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java index cbbaf855b80..a79237f9ba6 100644 --- a/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java +++ b/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java @@ -17,6 +17,7 @@ package org.sonar.java.utils; import java.util.List; +import javax.annotation.Nullable; import org.junit.jupiter.api.Test; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.config.Configuration; @@ -247,6 +248,28 @@ void isTestFile_withSonarTestsConfigured_annotationSignalStillApplies() { assertAnnotationSignal(true, "src/test/files/utils/SampleWithRunWith.java"); } + // ------------------------------------------------------------------------- + // isTestFile — no Configuration available + // ------------------------------------------------------------------------- + + @Test + void isTestFile_withoutConfiguration_skipsPathHeuristicInsteadOfThrowing() { + // A context built without SonarComponents has no Configuration. Throwing here would be swallowed by the + // scanner and would silently classify every file as main code. + JavaFileScannerContext context = contextWithInputFileAndConfig( + TestUtils.emptyInputFile("FooTest.java", InputFile.Type.MAIN), null); + + assertThat(JavaFileTypeClassifier.isTestFile(context)).isFalse(); + } + + @Test + void isTestFile_withoutConfiguration_stillHonoursThePlatformSignal() { + JavaFileScannerContext context = contextWithInputFileAndConfig( + TestUtils.emptyInputFile("Foo.java", InputFile.Type.TEST), null); + + assertThat(JavaFileTypeClassifier.isTestFile(context)).isTrue(); + } + // ------------------------------------------------------------------------- // isTestFile — no signal // ------------------------------------------------------------------------- @@ -284,7 +307,7 @@ private static JavaFileScannerContext contextWithInputFile(InputFile inputFile) return contextWithInputFileAndConfig(inputFile, NO_SONAR_TESTS_CONFIG); } - private static JavaFileScannerContext contextWithInputFileAndConfig(InputFile inputFile, Configuration config) { + private static JavaFileScannerContext contextWithInputFileAndConfig(InputFile inputFile, @Nullable Configuration config) { JavaFileScannerContext context = mock(JavaFileScannerContext.class); when(context.getInputFile()).thenReturn(inputFile); when(context.getConfiguration()).thenReturn(config); diff --git a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/JavaSensor.java b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/JavaSensor.java index c683e74dc7c..603019ee9dd 100644 --- a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/JavaSensor.java +++ b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/JavaSensor.java @@ -91,10 +91,11 @@ public JavaSensor(SonarComponents sonarComponents, JavaResourceLocator javaResou this.springContextGatheringModel = springContextGatheringModel; this.sonarComponents.registerMainChecks(GeneratedCheckList.REPOSITORY_KEY, GeneratedCheckList.getJavaChecks()); this.sonarComponents.registerTestChecks(GeneratedCheckList.REPOSITORY_KEY, GeneratedCheckList.getJavaTestChecks()); + // Main scope only: the Spring context model describes the application context, which never contains the beans + // declared by test code. Gatherers registered for RuleScope.TEST would be handed exactly the files + // JavaFileTypeClassifier already excludes, and collect nothing. SpringContextModelGatherers.getAllGatherers(springContextGatheringModel, telemetry) .forEach(gatherer -> this.sonarComponents.registerCustomFileScanner(RuleScope.MAIN, gatherer)); - SpringContextModelGatherers.getAllGatherers(springContextGatheringModel, telemetry) - .forEach(gatherer -> this.sonarComponents.registerCustomFileScanner(RuleScope.TEST, gatherer)); } @Override From c980888db090329290827821dd3a4e8b08d3e1dc Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Mon, 12 Oct 2026 16:50:16 +0200 Subject: [PATCH 3/3] SONARJAVA-7166 Detect test classes nested at any depth The AST signals inspected only the top-level types of a compilation unit, so a test class that is itself nested went unnoticed whenever the outer class carried no test marker of its own: class Outer { // nothing marks this as test code static class InnerTest { @Test void probe() {} @Configuration static class Cfg { @Bean ApplicationContext bean() { return null; } } } } Spring excludes Cfg: TestTypeExcludeFilter resolves a candidate's enclosing class and re-applies itself recursively, reaching InnerTest and its test methods. We collected the bean instead. Walking down from the compilation unit into nested types reaches the same set as walking up from each candidate. UnitTestUtils.isTestClass only answers for top-level classes, and spots a nested test class solely through an @Nested member, so the method signal now also asks whether a type declares test methods itself. That additionally covers an abstract base test class, which isTestClass rejects outright while Spring excludes it along with anything nested inside it. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/utils/JavaFileTypeClassifier.java | 56 ++++++++++++++----- .../NestedTestClassWithConfiguration.java | 25 +++++++++ .../SampleWithDeeplyNestedTestAnnotation.java | 12 ++++ ...estedTestClassWithoutNestedAnnotation.java | 11 ++++ .../BeanDefinitionGathererTest.java | 9 +++ .../utils/JavaFileTypeClassifierTest.java | 11 ++++ 6 files changed, 110 insertions(+), 14 deletions(-) create mode 100644 java-frontend/src/test/files/springcontext/NestedTestClassWithConfiguration.java create mode 100644 java-frontend/src/test/files/utils/SampleWithDeeplyNestedTestAnnotation.java create mode 100644 java-frontend/src/test/files/utils/SampleWithNestedTestClassWithoutNestedAnnotation.java diff --git a/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java b/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java index d3b091cfd39..07efca308e6 100644 --- a/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java +++ b/java-frontend/src/main/java/org/sonar/java/utils/JavaFileTypeClassifier.java @@ -26,6 +26,7 @@ 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; @@ -43,12 +44,15 @@ * sonar-analyzer-commons, extended with Java-specific path conventions *

  • AST annotations: class-level test framework annotations ({@code @RunWith}, * {@code @SpringBootTest}, etc.)
    • - *
  • Test methods: a top-level class holding JUnit/TestNG test methods, via - * {@link UnitTestUtils#isTestClass(ClassTree)}
    • + *
  • Test methods: a class holding JUnit/TestNG test methods, via + * {@link UnitTestUtils#isTestClass(ClassTree)} and the methods a type declares itself
    • * * *

    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. + * 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. * *

    A file is considered a test file if any signal indicates it. The platform type * takes priority when {@code TEST}, but a {@code MAIN}-typed file can be upgraded to test scope @@ -185,8 +189,8 @@ 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. * *

    Gracefully returns {@code false} if the file was not successfully parsed * ({@link JavaFileScannerContext#fileParsed()} is {@code false}). @@ -194,36 +198,60 @@ static boolean isPlatformTestFile(JavaFileScannerContext context) { * @param context the current file scanner context */ static boolean hasTestFrameworkAnnotation(JavaFileScannerContext context) { - return anyTopLevelClassMatches(context, JavaFileTypeClassifier::hasTestAnnotation); + return anyTypeMatches(context, JavaFileTypeClassifier::hasTestAnnotation); } /** - * Returns {@code true} if any top-level class in the compilation unit declares test methods, - * as determined by {@link UnitTestUtils#isTestClass(ClassTree)}. + * Returns {@code true} if any type in the compilation unit declares test methods. * *

    This covers the method-level markers that {@link #TEST_CLASS_ANNOTATIONS} deliberately omits, - * such as a plain class holding JUnit {@code @Test} methods. + * 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 anyTopLevelClassMatches(context, UnitTestUtils::isTestClass); + 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 top-level class of the compilation unit. + * Applies {@code predicate} to every type declared in the compilation unit, nested ones included, at any depth. + * + *

    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. * *

    Gracefully returns {@code false} if the file was not successfully parsed * ({@link JavaFileScannerContext#fileParsed()} is {@code false}). */ - private static boolean anyTopLevelClassMatches(JavaFileScannerContext context, Predicate predicate) { + private static boolean anyTypeMatches(JavaFileScannerContext context, Predicate 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 predicate) { + if (predicate.test(classTree)) { + return true; + } + return classTree.members().stream() + .filter(ClassTree.class::isInstance) .map(ClassTree.class::cast) - .anyMatch(predicate); + .anyMatch(nested -> matchesTypeOrNested(nested, predicate)); } private static boolean hasTestAnnotation(ClassTree classTree) { diff --git a/java-frontend/src/test/files/springcontext/NestedTestClassWithConfiguration.java b/java-frontend/src/test/files/springcontext/NestedTestClassWithConfiguration.java new file mode 100644 index 00000000000..a7a3136b990 --- /dev/null +++ b/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; + } + } + } +} diff --git a/java-frontend/src/test/files/utils/SampleWithDeeplyNestedTestAnnotation.java b/java-frontend/src/test/files/utils/SampleWithDeeplyNestedTestAnnotation.java new file mode 100644 index 00000000000..43bf8156b34 --- /dev/null +++ b/java-frontend/src/test/files/utils/SampleWithDeeplyNestedTestAnnotation.java @@ -0,0 +1,12 @@ +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +class SampleWithDeeplyNestedTestAnnotation { + + static class Holder { + + @ExtendWith(MockitoExtension.class) + static class InnerTest { + } + } +} diff --git a/java-frontend/src/test/files/utils/SampleWithNestedTestClassWithoutNestedAnnotation.java b/java-frontend/src/test/files/utils/SampleWithNestedTestClassWithoutNestedAnnotation.java new file mode 100644 index 00000000000..844bb07fbc0 --- /dev/null +++ b/java-frontend/src/test/files/utils/SampleWithNestedTestClassWithoutNestedAnnotation.java @@ -0,0 +1,11 @@ +import org.junit.jupiter.api.Test; + +class SampleWithNestedTestClassWithoutNestedAnnotation { + + static class InnerTest { + + @Test + void does_something() { + } + } +} diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index 26fc1d3c85c..46dce50cfe7 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -300,6 +300,15 @@ void beans_nested_inside_a_class_holding_test_methods_are_not_collected() { assertThat(model.getBeanDefinitionRegistry().getByName("deeplyNestedComponent")).isEmpty(); } + @Test + void beans_nested_inside_a_test_class_that_is_itself_nested_are_not_collected() { + scan("src/test/files/springcontext/NestedTestClassWithConfiguration.java"); + + assertThat(model.getTypeToBeansIndex().getKeys()).isEmpty(); + assertThat(model.getBeanDefinitionRegistry().getByName("nestedConfiguration")).isEmpty(); + assertThat(model.getBeanDefinitionRegistry().getByName("nestedApplicationContext")).isEmpty(); + } + @Test void beans_nested_inside_a_TestConfiguration_class_are_not_collected() { scan("src/test/files/springcontext/TestConfigurationWithNestedConfiguration.java"); diff --git a/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java b/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java index a79237f9ba6..d067197ffe6 100644 --- a/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java +++ b/java-frontend/src/test/java/org/sonar/java/utils/JavaFileTypeClassifierTest.java @@ -92,6 +92,11 @@ void hasTestFrameworkAnnotation_detects_TestConfiguration() { assertAnnotationSignal(true, "src/test/files/utils/SampleWithTestConfiguration.java"); } + @Test + void hasTestFrameworkAnnotation_detects_annotationOnADeeplyNestedType() { + assertAnnotationSignal(true, "src/test/files/utils/SampleWithDeeplyNestedTestAnnotation.java"); + } + @Test void hasTestFrameworkAnnotation_returnsFalse_forPlainClass() { assertAnnotationSignal(false, "src/test/files/utils/PlainSample.java"); @@ -130,6 +135,12 @@ void declaresTestMethods_detects_plainClassHoldingNestedTestClass() { assertTestMethodSignal(true, "src/test/files/utils/SampleWithNestedTestClass.java"); } + @Test + void declaresTestMethods_detects_nestedTestClassWithoutNestedAnnotation() { + // The outer class carries no test marker at all: only the nested class declares test methods. + assertTestMethodSignal(true, "src/test/files/utils/SampleWithNestedTestClassWithoutNestedAnnotation.java"); + } + @Test void declaresTestMethods_returnsFalse_forPlainClass() { assertTestMethodSignal(false, "src/test/files/utils/PlainSample.java");