Skip to content

Commit 00a008f

Browse files
guillaume-dequennesonartech
authored andcommitted
SONARPY-4018 Silence main-scope issues on test files when sonar.tests is not set
Introduce TestFileClassifier to heuristically identify test files based on naming conventions when sonar.tests is not configured. PythonScanner now resolves an effective file type for rules so that issues raised by test-specific rules (e.g. S2699) are silenced on main-scope files that look like tests. Update IT orchestrator and ruling tests to cover the new behaviour. GitOrigin-RevId: 750d41a38353850d9b10e5b369eadbda66c8482e
1 parent c5b9982 commit 00a008f

16 files changed

Lines changed: 845 additions & 22 deletions

File tree

its/plugin/it-python-plugin-test/projects/test_code/main_src/.gitkeep

Whitespace-only changes.

its/plugin/it-python-plugin-test/src/test/java/com/sonar/python/it/plugin/TestRulesTest.java

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
import com.sonar.python.it.ConcurrentOrchestratorExtension;
2121
import com.sonar.python.it.TestsUtils;
2222
import java.io.File;
23+
import java.util.Comparator;
2324
import java.util.List;
25+
import java.util.stream.Collectors;
2426
import org.junit.jupiter.api.BeforeAll;
2527
import org.junit.jupiter.api.Test;
2628
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -38,13 +40,14 @@ public class TestRulesTest {
3840
private static final String PROJECT_KEY = "test-rules";
3941
private static final String PROJECT_NAME = "Test Rules";
4042

41-
private static SonarScanner BUILD;
42-
4343
@BeforeAll
4444
static void prepare() {
4545
orchestrator.getServer().provisionProject(PROJECT_KEY, PROJECT_NAME);
4646
orchestrator.getServer().associateProjectToQualityProfile(PROJECT_KEY, "py", "python-test-rules-profile");
47-
BUILD = orchestrator.createSonarScanner()
47+
}
48+
49+
private SonarScanner newBuild() {
50+
return orchestrator.createSonarScanner()
4851
.setProjectDir(new File("projects/test_code"))
4952
.setProjectKey(PROJECT_KEY)
5053
.setProjectName(PROJECT_NAME)
@@ -53,9 +56,9 @@ static void prepare() {
5356

5457
@Test
5558
void test_rules_run_on_test_files() {
56-
BUILD.setSourceDirs("src")
57-
.setTestDirs("tests");
58-
orchestrator.executeBuild(BUILD);
59+
orchestrator.executeBuild(newBuild()
60+
.setSourceDirs("src")
61+
.setTestDirs("tests"));
5962

6063
List<Issues.Issue> assertOnTupleIssues = issues("python:S5905");
6164
List<Issues.Issue> dedicatedAssertionIssues = issues("python:S5906");
@@ -70,9 +73,12 @@ void test_rules_run_on_test_files() {
7073
}
7174

7275
@Test
73-
void declare_all_files_as_test_files() {
74-
BUILD.setTestDirs(".");
75-
orchestrator.executeBuild(BUILD);
76+
void main_rules_suppressed_when_all_files_declared_as_test() {
77+
// main_src/ is an empty placeholder so sonar.sources points to a valid path with no Python files.
78+
// All actual Python files are under sonar.tests=src,tests → classified as TEST → S3923 (MAIN-scoped) fires nowhere.
79+
orchestrator.executeBuild(newBuild()
80+
.setSourceDirs("main_src")
81+
.setTestDirs("src,tests"));
7682

7783
List<Issues.Issue> testRuleIssues = issues("python:S5905");
7884
List<Issues.Issue> mainRuleIssues = issues("python:S3923");
@@ -81,13 +87,79 @@ void declare_all_files_as_test_files() {
8187
assertThat(mainRuleIssues).isEmpty();
8288
}
8389

90+
@Test
91+
void auto_detect_test_files_when_sonar_tests_not_configured() {
92+
// When sonar.tests is not set, all files get InputFile.Type.MAIN from the platform.
93+
// The plugin's path-based heuristic detects files in the `tests/` directory and
94+
// suppresses MAIN-scoped rules on them, while CheckScope.ALL and TEST-scoped rules still fire.
95+
orchestrator.executeBuild(newBuild()
96+
.setProperty("sonar.sources", "src,tests")); // all files are MAIN — no sonar.tests configured
97+
98+
List<Issues.Issue> mainRuleIssues = issues("python:S3923");
99+
List<Issues.Issue> allFilesRuleIssues = issues("python:S5905");
100+
List<Issues.Issue> testScopedRuleIssues = issues("python:S5906");
101+
102+
// S3923 (MAIN-scoped): fires only on src/some_code.py; heuristic silences it on tests/
103+
assertThat(mainRuleIssues).hasSize(1);
104+
assertIssue(mainRuleIssues.get(0), 3,
105+
"Remove this if statement or edit its code blocks so that they're not all the same.",
106+
"test-rules:src/some_code.py");
107+
108+
// S5905 (CheckScope.ALL): fires on both files regardless of effective type
109+
assertThat(allFilesRuleIssues).hasSize(2);
110+
assertIssue(allFilesRuleIssues.get(0), 2,
111+
"Fix this assertion on a tuple literal.", "test-rules:src/some_code.py");
112+
assertIssue(allFilesRuleIssues.get(1), 3,
113+
"Fix this assertion on a tuple literal.", "test-rules:tests/test_my_code.py");
114+
115+
// S5906 (TEST-scoped): fires on heuristic-detected test file
116+
assertThat(testScopedRuleIssues).hasSize(1);
117+
assertIssue(testScopedRuleIssues.get(0), 14,
118+
"Consider using \"assertEqual\" instead.", "test-rules:tests/test_my_code.py");
119+
}
120+
121+
@Test
122+
void sonar_tests_configured_bypasses_heuristic() {
123+
// When sonar.tests is explicitly configured, the path-based heuristic is inactive.
124+
// Platform typing governs: files under tests/ are TEST because sonar.tests=tests.
125+
// Setting sonar.tests is the recommended approach — it gives explicit, predictable
126+
// control and produces the same isolation as the heuristic.
127+
orchestrator.executeBuild(newBuild()
128+
.setProperty("sonar.sources", "src")
129+
.setProperty("sonar.tests", "tests"));
130+
131+
List<Issues.Issue> mainRuleIssues = issues("python:S3923");
132+
List<Issues.Issue> allFilesRuleIssues = issues("python:S5905");
133+
List<Issues.Issue> testScopedRuleIssues = issues("python:S5906");
134+
135+
// S3923 (MAIN-scoped): tests/ files are TEST by platform typing — same suppression as heuristic
136+
assertThat(mainRuleIssues).hasSize(1);
137+
assertIssue(mainRuleIssues.get(0), 3,
138+
"Remove this if statement or edit its code blocks so that they're not all the same.",
139+
"test-rules:src/some_code.py");
140+
141+
assertThat(allFilesRuleIssues).hasSize(2);
142+
assertIssue(allFilesRuleIssues.get(0), 2,
143+
"Fix this assertion on a tuple literal.", "test-rules:src/some_code.py");
144+
assertIssue(allFilesRuleIssues.get(1), 3,
145+
"Fix this assertion on a tuple literal.", "test-rules:tests/test_my_code.py");
146+
147+
// S5906 (TEST-scoped): fires on the explicit test file
148+
assertThat(testScopedRuleIssues).hasSize(1);
149+
assertIssue(testScopedRuleIssues.get(0), 14,
150+
"Consider using \"assertEqual\" instead.", "test-rules:tests/test_my_code.py");
151+
}
152+
84153
private void assertIssue(Issues.Issue issue, int expectedLine, String expectedMessage, String expectedComponent) {
85154
assertThat(issue.getLine()).isEqualTo(expectedLine);
86155
assertThat(issue.getMessage()).isEqualTo(expectedMessage);
87156
assertThat(issue.getComponent()).isEqualTo(expectedComponent);
88157
}
89158

90159
private static List<Issues.Issue> issues(String rulekey) {
91-
return newWsClient().issues().search(new SearchRequest().setRules(singletonList(rulekey))).getIssuesList();
160+
return newWsClient().issues().search(new SearchRequest().setRules(singletonList(rulekey))).getIssuesList()
161+
.stream()
162+
.sorted(Comparator.comparing(Issues.Issue::getComponent).thenComparingInt(Issues.Issue::getLine))
163+
.collect(Collectors.toList());
92164
}
93165
}

python-commons/src/main/java/org/sonar/plugins/python/IPynbSensor.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.sonar.plugins.python.indexer.SonarQubePythonIndexer;
4343
import org.sonar.plugins.python.nosonar.NoSonarLineInfoCollector;
4444
import org.sonar.plugins.python.telemetry.SensorTelemetryStorage;
45+
import org.sonar.plugins.python.warnings.AnalysisWarningsWrapper;
4546
import org.sonar.plugins.python.telemetry.TelemetryMetricKey;
4647
import org.sonar.python.caching.CacheContextImpl;
4748
import org.sonar.python.parser.PythonParser;
@@ -119,7 +120,7 @@ public void execute(SensorContext context) {
119120
}
120121
if (isInSonarLintRuntime(context)) {
121122
PythonScanner scanner = new PythonScanner(context, checks, fileLinesContextFactory, noSonarFilter, PythonParser::createIPythonParser,
122-
indexer, new DummyArchitectureCallback(), noSonarLineInfoCollector);
123+
indexer, new DummyArchitectureCallback(), noSonarLineInfoCollector, new AnalysisWarningsWrapper());
123124
scanner.execute(pythonFiles, context);
124125
} else {
125126
processNotebooksFiles(pythonFiles, context);
@@ -139,7 +140,7 @@ private void processNotebooksFiles(List<PythonInputFile> pythonFiles, SensorCont
139140
CacheContext cacheContext = CacheContextImpl.dummyCache();
140141
PythonIndexer pythonIndexer = new SonarQubePythonIndexer(pythonFiles, cacheContext, context, projectConfigurationBuilder);
141142
PythonScanner scanner = new PythonScanner(context, checks, fileLinesContextFactory, noSonarFilter, PythonParser::createIPythonParser,
142-
pythonIndexer, new DummyArchitectureCallback(), noSonarLineInfoCollector);
143+
pythonIndexer, new DummyArchitectureCallback(), noSonarLineInfoCollector, new AnalysisWarningsWrapper());
143144
scanner.execute(pythonFiles, context);
144145
sensorTelemetryStorage.updateMetric(TelemetryMetricKey.NOTEBOOK_RECOGNITION_ERROR_KEY, scanner.getRecognitionErrorCount());
145146
updateDatabricksTelemetry(scanner);

python-commons/src/main/java/org/sonar/plugins/python/PythonScanner.java

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.sonar.sslr.api.AstNode;
2020
import com.sonar.sslr.api.RecognitionException;
2121
import java.io.File;
22+
import javax.annotation.Nullable;
2223
import java.io.IOException;
2324
import java.util.ArrayList;
2425
import java.util.Collection;
@@ -56,6 +57,7 @@
5657
import org.sonar.plugins.python.telemetry.collectors.TestFileTelemetryCollector;
5758
import org.sonar.plugins.python.telemetry.collectors.TypeInferenceTelemetry;
5859
import org.sonar.plugins.python.telemetry.collectors.TypeInferenceTelemetryCollector;
60+
import org.sonar.plugins.python.warnings.AnalysisWarningsWrapper;
5961
import org.sonar.python.IPythonLocation;
6062
import org.sonar.python.SubscriptionVisitor;
6163
import org.sonar.python.parser.PythonParser;
@@ -85,15 +87,24 @@ public class PythonScanner extends Scanner {
8587
private final Lock lock;
8688
private final TypeInferenceTelemetryCollector typeInferenceTelemetryCollector;
8789
private final TestFileTelemetryCollector testFileTelemetryCollector;
90+
private final boolean testSourcesConfigured;
91+
private final AnalysisWarningsWrapper analysisWarnings;
92+
private final AtomicBoolean heuristicWarningEmitted = new AtomicBoolean(false);
93+
static final String UNSET_SONAR_TESTS_WARNING = "SonarPython detected files that look like test code " +
94+
"but 'sonar.tests' is not configured. Rules targeting production code were not executed on these files. " +
95+
"Configure 'sonar.tests' in your project properties for a more accurate analysis.";
8896

8997
public PythonScanner(
9098
SensorContext context, PythonChecks checks, FileLinesContextFactory fileLinesContextFactory, NoSonarFilter noSonarFilter,
91-
Supplier<PythonParser> parserSupplier, PythonIndexer indexer, PythonFileConsumer architectureCallback, NoSonarLineInfoCollector noSonarLineInfoCollector) {
99+
Supplier<PythonParser> parserSupplier, PythonIndexer indexer, PythonFileConsumer architectureCallback,
100+
NoSonarLineInfoCollector noSonarLineInfoCollector, AnalysisWarningsWrapper analysisWarnings) {
92101
super(context);
93102
this.checks = checks;
94103
this.parserSupplier = parserSupplier;
95104
this.indexer = indexer;
96105
this.noSonarLineInfoCollector = noSonarLineInfoCollector;
106+
this.analysisWarnings = analysisWarnings;
107+
this.testSourcesConfigured = TestFileClassifier.isTestSourceConfigured(context.config());
97108
this.indexer.buildOnce(context);
98109
this.architectureCallback = architectureCallback;
99110
this.checksExecutedWithoutParsingByFiles = new ConcurrentHashMap<>();
@@ -125,9 +136,14 @@ protected void scanFile(PythonInputFile inputFile) throws IOException {
125136
var pythonFile = SonarQubePythonFile.create(inputFile);
126137
InputFile.Type fileType = inputFile.wrappedFile().type();
127138
PythonVisitorContext visitorContext = createVisitorContext(inputFile, pythonFile);
139+
InputFile.Type effectiveTypeForRules = resolveEffectiveTypeForRules(
140+
fileType, projectRelativePath(inputFile), visitorContext.rootTree());
141+
if (!testSourcesConfigured && fileType == InputFile.Type.MAIN) {
142+
indexer.writeEffectiveFileType(inputFile.wrappedFile().key(), effectiveTypeForRules);
143+
}
128144

129-
executeChecks(visitorContext, checks.sonarPythonChecks(), fileType, inputFile);
130-
executeOtherChecks(inputFile, visitorContext, fileType);
145+
executeChecks(visitorContext, checks.sonarPythonChecks(), effectiveTypeForRules, inputFile);
146+
executeOtherChecks(inputFile, visitorContext, effectiveTypeForRules);
131147

132148

133149
runLockedByRepository(ARCHITECTURE_CALLBACK_LOCK_KEY, () -> architectureCallback.scanFile(visitorContext));
@@ -241,6 +257,11 @@ private static Map<Integer, IPythonLocation> getOffsetLocations(PythonInputFile
241257
@Override
242258
public boolean scanFileWithoutParsing(PythonInputFile inputFile) {
243259
InputFile.Type fileType = inputFile.wrappedFile().type();
260+
InputFile.Type effectiveTypeForRules = resolveEffectiveTypeForRulesFromCache(
261+
fileType, projectRelativePath(inputFile), inputFile.wrappedFile().key());
262+
if (effectiveTypeForRules == null) {
263+
return false;
264+
}
244265
boolean result = true;
245266
PythonFile pythonFile = SonarQubePythonFile.create(inputFile.wrappedFile());
246267
PythonInputFileContext inputFileContext = new PythonInputFileContext(
@@ -251,13 +272,13 @@ public boolean scanFileWithoutParsing(PythonInputFile inputFile) {
251272
indexer.projectLevelSymbolTable()
252273
);
253274

254-
result = scanFileWithoutParsingSonarPython(inputFile, fileType, inputFileContext, result);
275+
result = scanFileWithoutParsingSonarPython(inputFile, effectiveTypeForRules, inputFileContext, result);
255276

256277
var atomicResult = new AtomicBoolean(result);
257278
var otherChecks = checks.noSonarPythonChecks();
258279
otherChecks.forEach((repositoryKey, repositoryChecks) -> runLockedByRepository(repositoryKey, () -> {
259280
for (var check : repositoryChecks) {
260-
var scanResult = scanFileWithoutParsingNotSonarPython(inputFile, check, fileType, atomicResult.get(), inputFileContext);
281+
var scanResult = scanFileWithoutParsingNotSonarPython(inputFile, check, effectiveTypeForRules, atomicResult.get(), inputFileContext);
261282
atomicResult.set(scanResult);
262283
}
263284
}));
@@ -322,6 +343,49 @@ private void endOfAnalysisForRepository(String repositoryKey, List<EndOfAnalysis
322343
runLockedByRepository(repositoryKey, () -> endOfAnalyses.forEach(c -> c.endOfAnalysis(indexer.cacheContext())));
323344
}
324345

346+
private String projectRelativePath(PythonInputFile inputFile) {
347+
return context.fileSystem().baseDir().toURI()
348+
.relativize(inputFile.wrappedFile().uri())
349+
.toString();
350+
}
351+
352+
private boolean isBypassed(InputFile.Type platformType) {
353+
return testSourcesConfigured || platformType == InputFile.Type.TEST;
354+
}
355+
356+
private InputFile.Type resolveEffectiveTypeForRules(InputFile.Type platformType, String filePath, @Nullable FileInput tree) {
357+
if (isBypassed(platformType)) {
358+
return platformType;
359+
}
360+
boolean isTest = TestFileClassifier.looksLikeTestFile(filePath, tree);
361+
if (isTest) {
362+
maybeEmitHeuristicWarning();
363+
return InputFile.Type.TEST;
364+
}
365+
return InputFile.Type.MAIN;
366+
}
367+
368+
@Nullable
369+
private InputFile.Type resolveEffectiveTypeForRulesFromCache(InputFile.Type platformType, String filePath, String fileKey) {
370+
if (isBypassed(platformType)) {
371+
return platformType;
372+
}
373+
InputFile.Type cached = indexer.readEffectiveFileType(fileKey);
374+
if (cached == null) {
375+
// Cache entry is missing while the file is unchanged — the cache is in an inconsistent state
376+
// This should never happen in normal operation. Fall back to a full parse rather than guessing the type.
377+
LOG.debug("No cached effective file type for '{}': triggering full parse", filePath);
378+
}
379+
return cached;
380+
}
381+
382+
private void maybeEmitHeuristicWarning() {
383+
if (heuristicWarningEmitted.compareAndSet(false, true)) {
384+
LOG.warn(UNSET_SONAR_TESTS_WARNING);
385+
analysisWarnings.addUnique(UNSET_SONAR_TESTS_WARNING);
386+
}
387+
}
388+
325389
boolean isCheckNotApplicable(PythonCheck pythonCheck, InputFile.Type fileType) {
326390
return fileType != InputFile.Type.MAIN && pythonCheck.scope() != PythonCheck.CheckScope.ALL;
327391
}

python-commons/src/main/java/org/sonar/plugins/python/PythonSensor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public void execute(SensorContext context) {
153153
pythonIndexer.setSonarLintCache(sonarLintCache);
154154
TypeShed.setProjectLevelSymbolTable(pythonIndexer.projectLevelSymbolTable());
155155
PythonScanner scanner = new PythonScanner(context, checks, fileLinesContextFactory, noSonarFilter, PythonParser::create,
156-
pythonIndexer, architectureCallback, noSonarLineInfoCollector);
156+
pythonIndexer, architectureCallback, noSonarLineInfoCollector, analysisWarnings);
157157
scanner.execute(pythonFiles, context);
158158
Duration sensorTime = Duration.between(sensorStartTime, Instant.now());
159159

0 commit comments

Comments
 (0)