Repository navigation
SONARJAVA-7166 S9352: Don't register beans declared in test scope - #6320
Conversation
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) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
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) <noreply@anthropic.com>
e50cdc1 to
229385a
Compare
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) <noreply@anthropic.com>
Code Review ✅ Approved 3 closed / 3 findings🟡 Medium risk · New test-scope detection suppresses Spring bean and scan gathering, which could hide dependency issues if production files are misclassified. Excludes test-scope classes, ✅ 3 closedClosed: Quality: Null guard misses the no-SonarComponents case its Javadoc describes
Closed: Bug: Test-scope gate also drops main beans that only match path/name heuristic
Closed: Bug: Deep nested-test detection excludes the whole file, not just the subtree
Verification Repos 4 inspectedFunctional validation No results Rules No rules evaluated Cross-repo 4 repositories inspected 0 open impacts
Auto-approval Not enabledFor more verification details, comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
aurelien-coet-sonarsource
left a comment
aurelien-coet-sonarsource
left a comment
There was a problem hiding this comment.
LGTM 👍
2 participants
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.