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

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 class holding JUnit/TestNG test methods, via + * {@link UnitTestUtils#isTestClass(ClassTree)} and the methods a type declares itself
    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. 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 - * 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 +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 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. + * + *

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,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}). @@ -171,13 +198,60 @@ static boolean isPlatformTestFile(JavaFileScannerContext context) { * @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. + * + *

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

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 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(JavaFileTypeClassifier::hasTestAnnotation); + .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/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/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/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/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/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/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/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index 071e912b4d4..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 @@ -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,67 @@ 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_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"); + + 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 +477,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/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 844897e0bb3..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 @@ -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; @@ -86,22 +87,78 @@ 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_detects_annotationOnADeeplyNestedType() { + assertAnnotationSignal(true, "src/test/files/utils/SampleWithDeeplyNestedTestAnnotation.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_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"); + } + + 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); } // ------------------------------------------------------------------------- @@ -202,6 +259,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 // ------------------------------------------------------------------------- @@ -239,7 +318,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