From 9ab50f8d7e360a0ba4a6f641439d56fd7313e88f Mon Sep 17 00:00:00 2001 From: Erwan Le Forestier Date: Mon, 17 Aug 2026 11:41:26 +0200 Subject: [PATCH] SONARPY-4541 Classify likely Python test files --- .../sonar/plugins/python/PythonScanner.java | 14 +- .../plugins/python/TestFileClassifier.java | 152 ------------- .../IpynbNotebookParserScannerTest.java | 14 ++ .../plugins/python/PythonSensorTest.java | 16 ++ .../python/TestFileClassifierTest.java | 200 ++++++------------ .../plugins/python/sensor/framework_helper.py | 6 + .../plugins/python/TestFileClassifier.java | 177 ++++++++++++++++ .../python/api/PythonVisitorContext.java | 44 +++- .../python/api/SubscriptionContext.java | 6 + .../org/sonar/python/SubscriptionVisitor.java | 5 + .../python/api/PythonVisitorContextTest.java | 20 ++ .../sonar/python/SubscriptionVisitorTest.java | 3 +- 12 files changed, 363 insertions(+), 294 deletions(-) delete mode 100644 python-commons/src/main/java/org/sonar/plugins/python/TestFileClassifier.java create mode 100644 python-commons/src/test/resources/org/sonar/plugins/python/sensor/framework_helper.py create mode 100644 python-frontend/src/main/java/org/sonar/plugins/python/TestFileClassifier.java diff --git a/python-commons/src/main/java/org/sonar/plugins/python/PythonScanner.java b/python-commons/src/main/java/org/sonar/plugins/python/PythonScanner.java index 8a98bb882..dc4ed1580 100644 --- a/python-commons/src/main/java/org/sonar/plugins/python/PythonScanner.java +++ b/python-commons/src/main/java/org/sonar/plugins/python/PythonScanner.java @@ -151,9 +151,9 @@ protected void logStart(int numThreads) { protected void scanFile(PythonInputFile inputFile) throws IOException { var pythonFile = SonarQubePythonFile.create(inputFile); InputFile.Type fileType = inputFile.wrappedFile().type(); - PythonVisitorContext visitorContext = createVisitorContext(inputFile, pythonFile); + PythonVisitorContext visitorContext = createVisitorContext(inputFile, pythonFile, projectRelativePath(inputFile)); InputFile.Type effectiveTypeForRules = resolveEffectiveTypeForRules( - fileType, projectRelativePath(inputFile), visitorContext.rootTree()); + fileType, visitorContext.isLikelyTestFile()); if (!testSourcesConfigured && fileType == InputFile.Type.MAIN) { indexer.writeEffectiveFileType(inputFile.wrappedFile().key(), effectiveTypeForRules); } @@ -189,7 +189,7 @@ protected void scanFile(PythonInputFile inputFile) throws IOException { searchForDataBricks(visitorContext); } - private PythonVisitorContext createVisitorContext(PythonInputFile inputFile, PythonFile pythonFile) throws IOException { + private PythonVisitorContext createVisitorContext(PythonInputFile inputFile, PythonFile pythonFile, String testFilePath) throws IOException { PythonVisitorContext visitorContext; try { AstNode astNode = parserSupplier.get().parse(inputFile.contents()); @@ -203,10 +203,11 @@ private PythonVisitorContext createVisitorContext(PythonInputFile inputFile, Pyt .typeTable(indexer.projectLevelTypeTable()) .cacheContext(indexer.cacheContext()) .sonarProduct(context.runtime().getProduct()) + .testFilePath(testFilePath) .build(); } catch (RecognitionException e) { - visitorContext = new PythonVisitorContext(pythonFile, e, context.runtime().getProduct()); + visitorContext = new PythonVisitorContext(pythonFile, e, context.runtime().getProduct(), testFilePath); var line = (inputFile.kind() == PythonInputFile.Kind.IPYTHON) ? ((GeneratedIPythonFile) inputFile).locationMap().get(e.getLine()).line() : e.getLine(); @@ -373,12 +374,11 @@ private boolean isBypassed(InputFile.Type platformType) { return testSourcesConfigured || platformType == InputFile.Type.TEST; } - private InputFile.Type resolveEffectiveTypeForRules(InputFile.Type platformType, String filePath, @Nullable FileInput tree) { + private InputFile.Type resolveEffectiveTypeForRules(InputFile.Type platformType, boolean likelyTestFile) { if (isBypassed(platformType)) { return platformType; } - boolean isTest = TestFileClassifier.looksLikeTestFile(filePath, tree); - if (isTest) { + if (likelyTestFile) { maybeEmitHeuristicWarning(); return InputFile.Type.TEST; } diff --git a/python-commons/src/main/java/org/sonar/plugins/python/TestFileClassifier.java b/python-commons/src/main/java/org/sonar/plugins/python/TestFileClassifier.java deleted file mode 100644 index 80f3f175c..000000000 --- a/python-commons/src/main/java/org/sonar/plugins/python/TestFileClassifier.java +++ /dev/null @@ -1,152 +0,0 @@ -/* - * SonarQube Python Plugin - * Copyright (C) SonarSource Sàrl - * mailto:info AT sonarsource DOT com - * - * You can redistribute and/or modify this program under the terms of - * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the Sonar Source-Available License for more details. - * - * You should have received a copy of the Sonar Source-Available License - * along with this program; if not, see https://sonarsource.com/license/ssal/ - */ -package org.sonar.plugins.python; - -import java.util.Arrays; -import java.util.Locale; -import java.util.Set; -import javax.annotation.Nullable; -import org.sonar.api.config.Configuration; -import org.sonar.plugins.python.api.tree.AssertStatement; -import org.sonar.plugins.python.api.tree.BaseTreeVisitor; -import org.sonar.plugins.python.api.tree.FileInput; -import org.sonar.plugins.python.api.tree.FunctionDef; -import org.sonar.plugins.python.api.tree.ImportFrom; -import org.sonar.plugins.python.api.tree.ImportName; -import org.sonar.plugins.python.api.tree.Statement; -import static org.sonarsource.analyzer.commons.appsec.TestFileClassifier.HEURISTIC_DISABLED_KEY; -import org.sonar.plugins.python.api.tree.StatementList; - -/** - * Heuristic classifier used when {@code sonar.tests} is not configured. - * Determines whether a file should be treated as a test file for rule-execution - * purposes, without affecting metric computation (which always uses the platform - * {@code InputFile#type()}). - */ -public class TestFileClassifier { - - private static final Set TEST_FRAMEWORK_MODULES = Set.of("unittest", "pytest"); - - private TestFileClassifier() { - } - - /** - * Returns {@code true} when the project configuration explicitly controls which files are test - * files, making the path-based heuristic unnecessary. Both {@link PythonScanner} and - * {@link org.sonar.plugins.python.indexer.SonarQubePythonIndexer} use this to decide whether - * to activate (and cache) the heuristic. - */ - public static boolean isTestSourceConfigured(Configuration config) { - return isPropertyConfigured(config, "sonar.tests") - || config.getBoolean("sonar.python.testFileHeuristic.disabled").orElse(false) - || config.getBoolean(HEURISTIC_DISABLED_KEY).orElse(false); - } - - private static boolean isPropertyConfigured(Configuration config, String key) { - return config.get(key).filter(v -> !v.isBlank()).isPresent(); - } - - /** - * Path-only check — safe to call when no parsed tree is available (e.g. cache hits). - * Matches files whose directory contains {@code test} or {@code tests}, or whose - * filename follows the standard pytest discovery patterns ({@code test_*.py} / {@code *_test.py}). - */ - static boolean looksLikeTestFileByPath(String filePath) { - if (filePath.isEmpty()) { - return false; - } - String normalizedPath = filePath.replace('\\', '/'); - String[] components = normalizedPath.split("/"); - - // Check directory components (all but the last) - boolean dirMatch = Arrays.stream(components, 0, components.length - 1) - .map(c -> c.toLowerCase(Locale.ROOT)) - .anyMatch(dir -> "test".equals(dir) || "tests".equals(dir)); - if (dirMatch) { - return true; - } - - // Check filename - String filename = components[components.length - 1].toLowerCase(Locale.ROOT); - return filename.startsWith("test_") || filename.endsWith("_test.py"); - } - - /** - * Full check — uses the parsed tree in addition to path heuristics. - * Applies when a {@link FileInput} is available (i.e. during a full parse). - */ - static boolean looksLikeTestFile(String filePath, @Nullable FileInput tree) { - if (looksLikeTestFileByPath(filePath)) { - return true; - } - if (tree == null) { - return false; - } - return isImportBasedTestFile(tree) || isPytestPatternFile(tree); - } - - private static boolean isImportBasedTestFile(FileInput tree) { - StatementList statements = tree.statements(); - if (statements == null) { - return false; - } - return statements.statements().stream().anyMatch(TestFileClassifier::isTestFrameworkImport); - } - - private static boolean isTestFrameworkImport(Statement statement) { - if (statement instanceof ImportName importName) { - return importName.modules().stream() - .map(aliasedName -> aliasedName.dottedName().names()) - .filter(names -> !names.isEmpty()) - .anyMatch(names -> TEST_FRAMEWORK_MODULES.contains(names.get(0).name())); - } - if (statement instanceof ImportFrom importFrom) { - var module = importFrom.module(); - if (module != null) { - var names = module.names(); - return !names.isEmpty() && TEST_FRAMEWORK_MODULES.contains(names.get(0).name()); - } - } - return false; - } - - private static boolean isPytestPatternFile(FileInput tree) { - StatementList statements = tree.statements(); - if (statements == null) { - return false; - } - return statements.statements().stream() - .filter(FunctionDef.class::isInstance) - .map(FunctionDef.class::cast) - .anyMatch(f -> f.name().name().startsWith("test_") && containsAssert(f)); - } - - private static boolean containsAssert(FunctionDef functionDef) { - var visitor = new AssertVisitor(); - functionDef.body().accept(visitor); - return visitor.hasAssert; - } - - private static class AssertVisitor extends BaseTreeVisitor { - boolean hasAssert = false; - - @Override - public void visitAssertStatement(AssertStatement assertStatement) { - hasAssert = true; - } - } -} diff --git a/python-commons/src/test/java/org/sonar/plugins/python/IpynbNotebookParserScannerTest.java b/python-commons/src/test/java/org/sonar/plugins/python/IpynbNotebookParserScannerTest.java index 89bb47ff1..2fa08aae6 100644 --- a/python-commons/src/test/java/org/sonar/plugins/python/IpynbNotebookParserScannerTest.java +++ b/python-commons/src/test/java/org/sonar/plugins/python/IpynbNotebookParserScannerTest.java @@ -18,6 +18,7 @@ import java.io.File; import java.io.IOException; +import java.util.Map; import org.junit.jupiter.api.Test; import org.sonar.api.batch.fs.InputFile; import org.sonar.plugins.python.api.IssueLocation; @@ -32,6 +33,19 @@ class IpynbNotebookParserScannerTest { private final File baseDir = new File("src/test/resources/org/sonar/plugins/python"); + @Test + void generated_notebook_python_uses_the_test_file_classifier() throws IOException { + var inputFile = createInputFile(baseDir, "notebook_trailing_whitespace.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN); + var notebook = IpynbNotebookParser.parseNotebook(inputFile).get(); + var generatedFile = new GeneratedIPythonFile(notebook.wrappedFile(), "import pytest\n", Map.of()); + var context = new PythonVisitorContext.Builder( + TestPythonVisitorRunner.parseNotebookFile(Map.of(), "import pytest\n"), + SonarQubePythonFile.create(generatedFile)) + .build(); + + assertThat(context.isLikelyTestFile()).isTrue(); + } + @Test void trailing_whitespace() throws IOException { var inputFile = createInputFile(baseDir, "notebook_trailing_whitespace.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN); diff --git a/python-commons/src/test/java/org/sonar/plugins/python/PythonSensorTest.java b/python-commons/src/test/java/org/sonar/plugins/python/PythonSensorTest.java index afa2a969c..4a7fda376 100644 --- a/python-commons/src/test/java/org/sonar/plugins/python/PythonSensorTest.java +++ b/python-commons/src/test/java/org/sonar/plugins/python/PythonSensorTest.java @@ -145,6 +145,7 @@ class PythonSensorTest { private static final String FILE_USING_TYPESHED = "uses_typeshed.py"; private static final String FILE_QUICKFIX = "file_quickfix.py"; private static final String FILE_TEST_FILE = "test_file.py"; + private static final String FILE_FRAMEWORK_HELPER = "framework_helper.py"; private static final String FILE_INVALID_SYNTAX = "invalid_syntax.py"; private static final String FILE_NO_SONAR_PY = "no_sonar.py"; private static final String ONE_STATEMENT_PER_LINE_RULE_KEY = "OneStatementPerLine"; @@ -650,6 +651,21 @@ void test_auto_reclassify_filename_heuristic_suppresses_main_rules() { verify(analysisWarning).addUnique(PythonScanner.UNSET_SONAR_TESTS_WARNING); } + @Test + void test_auto_reclassify_framework_import_suppresses_main_rules() { + activeRules = new ActiveRulesBuilder() + .addRule(new NewActiveRule.Builder() + .setRuleKey(RuleKey.of(PythonRuleRepository.REPOSITORY_KEY, "S1226")) + .build()) + .build(); + + inputFile(FILE_FRAMEWORK_HELPER, Type.MAIN); + sensor().execute(context); + + assertThat(context.allIssues()).isEmpty(); + verify(analysisWarning).addUnique(PythonScanner.UNSET_SONAR_TESTS_WARNING); + } + @ParameterizedTest @CsvSource({ "sonar.tests, tests", diff --git a/python-commons/src/test/java/org/sonar/plugins/python/TestFileClassifierTest.java b/python-commons/src/test/java/org/sonar/plugins/python/TestFileClassifierTest.java index 375fed964..542c50786 100644 --- a/python-commons/src/test/java/org/sonar/plugins/python/TestFileClassifierTest.java +++ b/python-commons/src/test/java/org/sonar/plugins/python/TestFileClassifierTest.java @@ -33,173 +33,109 @@ class TestFileClassifierTest { - // --- isTestSourceConfigured --- - @Test - void isTestSourceConfigured_returns_false_for_empty_config() { + void test_source_configuration_disables_the_heuristic() { var config = mock(Configuration.class); when(config.get(anyString())).thenReturn(Optional.empty()); when(config.getBoolean(anyString())).thenReturn(Optional.empty()); + assertThat(TestFileClassifier.isTestSourceConfigured(config)).isFalse(); - } - @Test - void isTestSourceConfigured_returns_true_for_python_specific_key() { - var config = mock(Configuration.class); - when(config.get(anyString())).thenReturn(Optional.empty()); - when(config.getBoolean(anyString())).thenReturn(Optional.empty()); when(config.getBoolean("sonar.python.testFileHeuristic.disabled")).thenReturn(Optional.of(true)); assertThat(TestFileClassifier.isTestSourceConfigured(config)).isTrue(); - } - @Test - void isTestSourceConfigured_returns_true_for_generic_heuristic_disabled_key() { - var config = mock(Configuration.class); - when(config.get(anyString())).thenReturn(Optional.empty()); - when(config.getBoolean(anyString())).thenReturn(Optional.empty()); when(config.getBoolean(HEURISTIC_DISABLED_KEY)).thenReturn(Optional.of(true)); assertThat(TestFileClassifier.isTestSourceConfigured(config)).isTrue(); } - // --- looksLikeTestFileByPath --- - - @Test - void empty_path_returns_false() { - assertThat(TestFileClassifier.looksLikeTestFileByPath("")).isFalse(); - } - - @Test - void file_in_tests_directory_returns_true() { - assertThat(TestFileClassifier.looksLikeTestFileByPath("tests/foo.py")).isTrue(); - } - - @Test - void file_in_test_singular_directory_returns_true() { - assertThat(TestFileClassifier.looksLikeTestFileByPath("test/foo.py")).isTrue(); - } - - @Test - void file_in_nested_tests_directory_returns_true() { - assertThat(TestFileClassifier.looksLikeTestFileByPath("src/mypackage/tests/foo.py")).isTrue(); - } - - @Test - void test_prefix_filename_returns_true() { - assertThat(TestFileClassifier.looksLikeTestFileByPath("test_foo.py")).isTrue(); - assertThat(TestFileClassifier.looksLikeTestFileByPath("src/test_bar.py")).isTrue(); - } - - @Test - void test_suffix_filename_returns_true() { - assertThat(TestFileClassifier.looksLikeTestFileByPath("foo_test.py")).isTrue(); - assertThat(TestFileClassifier.looksLikeTestFileByPath("src/bar_test.py")).isTrue(); - } - - @Test - void regular_file_in_src_returns_false() { - assertThat(TestFileClassifier.looksLikeTestFileByPath("src/foo.py")).isFalse(); - assertThat(TestFileClassifier.looksLikeTestFileByPath("module.py")).isFalse(); - } - - @Test - void directory_named_testing_not_matched() { - // only "test" and "tests" trigger — "testing" must not - assertThat(TestFileClassifier.looksLikeTestFileByPath("testing/foo.py")).isFalse(); - } - - @Test - void windows_style_path_normalized() { - assertThat(TestFileClassifier.looksLikeTestFileByPath("src\\tests\\foo.py")).isTrue(); - } - - // --- looksLikeTestFile with null tree --- - - @Test - void null_tree_non_test_path_returns_false() { - assertThat(TestFileClassifier.looksLikeTestFile("src/regular.py", null)).isFalse(); - } - - @Test - void null_tree_test_path_returns_true() { - assertThat(TestFileClassifier.looksLikeTestFile("tests/foo.py", null)).isTrue(); - } - - // --- import-based detection (visitImportName / visitImportFrom) --- - @ParameterizedTest @ValueSource(strings = { - "import unittest\n", - "import pytest\n", - "from unittest import TestCase\n", - "from pytest import mark\n" + "tests/helpers.py", + "src/TESTING/factories.py", + "src\\test\\conftest.py", + "src/TestCase.py", + "src/latest.py" }) - void test_framework_import_detected(String code) { - FileInput tree = parse(code); - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", tree)).isTrue(); + void path_signals_include_test_support_files_and_aggressive_filename_matches(String path) { + assertThat(TestFileClassifier.looksLikeTestFileByPath(path)).isTrue(); } - @Test - void multiple_top_level_imports_both_checked() { - FileInput tree = parse("import unittest\nimport unittest\n"); - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", tree)).isTrue(); + @ParameterizedTest + @ValueSource(strings = {"", "src/production.py", "src/module.py"}) + void non_test_paths_are_not_classified_without_an_ast_signal(String path) { + assertThat(TestFileClassifier.looksLikeTestFileByPath(path)).isFalse(); } @ParameterizedTest @ValueSource(strings = { - "import os\nimport sys\n", // non-test imports - "from . import foo\n", // relative import — null module, must not throw - "def test_something():\n pass\n", // test_ prefix but no assert - "def do_something():\n assert True\n" // assert but no test_ prefix + "import pytest as pt\n", + "from unittest.mock import Mock\n", + "import doctest\n", + "from django.test import TestCase\n", + "import hypothesis.strategies as st\n", + "import robot\n", + "import behave\n", + "import pytest_bdd\n", + "import nose\n", + "import nose2\n", + "from twisted.trial import unittest\n", + "import testtools\n", + "import mock\n", + "import pytest_mock\n", + "import fixtures\n", + "import factory\n", + "import factory_boy\n", + "import freezegun\n", + "import responses\n", + "import requests_mock\n", + "import respx\n", + "import httpretty\n", + "import moto\n", + "import vcr\n", + "import testcontainers\n" }) - void ast_heuristic_not_detected(String code) { - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", parse(code))).isFalse(); - } - - @Test - void nested_import_not_detected() { - // import inside a function body must not classify the file as a test file - FileInput tree = parse("def helper():\n from unittest.mock import Mock\n return Mock()\n"); - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", tree)).isFalse(); + void test_framework_and_utility_imports_are_detected(String code) { + assertThat(TestFileClassifier.looksLikeTestFile("src/production.py", parse(code))).isTrue(); } @Test - void nested_test_function_not_detected() { - // a test_* function nested inside another function must not classify the file as a test file - FileInput tree = parse("def outer():\n def test_inner():\n assert True\n"); - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", tree)).isFalse(); - } - - @Test - void multiple_from_imports_all_checked() { - FileInput tree = parse("from unittest import TestCase\nfrom unittest import mock\n"); - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", tree)).isTrue(); - } - - // --- pytest pattern detection (top-level functions only) --- - - @Test - void test_function_with_assert_detected() { - FileInput tree = parse("def test_something():\n assert True\n"); - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", tree)).isTrue(); + void test_shaped_top_level_declarations_are_detected() { + assertThat(TestFileClassifier.looksLikeTestFile("src/production.py", parse("class TestService:\n pass\n"))).isTrue(); + assertThat(TestFileClassifier.looksLikeTestFile("src/production.py", parse(""" + async def TEST_first(): + pass + def test_second(): + pass + def helper(): + pass + """))).isTrue(); } @Test - void multiple_top_level_test_functions_checked() { - FileInput tree = parse("def test_one():\n assert True\ndef test_two():\n assert True\n"); - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", tree)).isTrue(); + void function_ratio_requires_a_strict_majority() { + assertThat(TestFileClassifier.looksLikeTestFile("src/production.py", parse(""" + def test_first(): + pass + def helper(): + pass + """))).isFalse(); + assertThat(TestFileClassifier.looksLikeTestFile("src/production.py", parse("pass\n"))).isFalse(); } @Test - void path_match_short_circuits_ast_check() { - // Path already matches — tree is never examined - FileInput tree = parse("import os\n"); - assertThat(TestFileClassifier.looksLikeTestFile("tests/foo.py", tree)).isTrue(); + void nested_test_signals_do_not_affect_top_level_classification() { + assertThat(TestFileClassifier.looksLikeTestFile("src/production.py", parse(""" + def helper(): + import pytest + def test_inner(): + pass + """))).isFalse(); } @Test - void import_match_takes_precedence_over_pytest_pattern() { - FileInput tree = parse("import unittest\ndef regular_function():\n pass\n"); - assertThat(TestFileClassifier.looksLikeTestFile("regular.py", tree)).isTrue(); + void path_signal_is_available_when_parsing_fails() { + assertThat(TestFileClassifier.looksLikeTestFile("tests/helpers.py", null)).isTrue(); + assertThat(TestFileClassifier.looksLikeTestFile("src/production.py", null)).isFalse(); } private static FileInput parse(String code) { diff --git a/python-commons/src/test/resources/org/sonar/plugins/python/sensor/framework_helper.py b/python-commons/src/test/resources/org/sonar/plugins/python/sensor/framework_helper.py new file mode 100644 index 000000000..d48475eaa --- /dev/null +++ b/python-commons/src/test/resources/org/sonar/plugins/python/sensor/framework_helper.py @@ -0,0 +1,6 @@ +import doctest + + +def regular_func(x): + x = 42 + assert (a, b) diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/TestFileClassifier.java b/python-frontend/src/main/java/org/sonar/plugins/python/TestFileClassifier.java new file mode 100644 index 000000000..0d4d45b0c --- /dev/null +++ b/python-frontend/src/main/java/org/sonar/plugins/python/TestFileClassifier.java @@ -0,0 +1,177 @@ +/* + * SonarQube Python Plugin + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.plugins.python; + +import java.util.Locale; +import java.util.Set; +import javax.annotation.Nullable; +import org.sonar.api.config.Configuration; +import org.sonar.plugins.python.api.PythonFile; +import org.sonar.plugins.python.api.tree.ClassDef; +import org.sonar.plugins.python.api.tree.DottedName; +import org.sonar.plugins.python.api.tree.FileInput; +import org.sonar.plugins.python.api.tree.FunctionDef; +import org.sonar.plugins.python.api.tree.ImportFrom; +import org.sonar.plugins.python.api.tree.ImportName; +import org.sonar.plugins.python.api.tree.Statement; +import org.sonar.plugins.python.api.tree.StatementList; + +import static org.sonarsource.analyzer.commons.appsec.TestFileClassifier.HEURISTIC_DISABLED_KEY; + +/** + * Heuristically identifies files that are likely to contain test code. + */ +public final class TestFileClassifier { + + private static final Set TEST_MODULE_ROOTS = Set.of( + "pytest", "unittest", "doctest", "hypothesis", "robot", "behave", "pytest_bdd", "nose", "nose2", + "testtools", "mock", "pytest_mock", "fixtures", "factory", "factory_boy", "freezegun", "responses", + "requests_mock", "respx", "httpretty", "moto", "vcr", "testcontainers"); + private static final Set TEST_MODULE_PREFIXES = Set.of("django.test", "twisted.trial"); + private static final Set TEST_DIRECTORY_NAMES = Set.of("test", "tests", "testing"); + + private TestFileClassifier() { + } + + /** + * Checks whether test source classification is explicitly configured. + * @param config analysis configuration + * @return whether the heuristic must be disabled + */ + public static boolean isTestSourceConfigured(Configuration config) { + return isPropertyConfigured(config, "sonar.tests") + || config.getBoolean("sonar.python.testFileHeuristic.disabled").orElse(false) + || config.getBoolean(HEURISTIC_DISABLED_KEY).orElse(false); + } + + /** + * Determines whether a parsed Python file likely contains test code. + * @param pythonFile analyzed Python file + * @param tree parsed file tree + * @return whether the file is likely test content + */ + public static boolean looksLikeTestFile(PythonFile pythonFile, @Nullable FileInput tree) { + String path = pythonFile.uri() == null ? null : pythonFile.uri().getPath(); + return looksLikeTestFile(path == null ? pythonFile.fileName() : path, tree); + } + + /** + * Determines whether a path and optional parsed tree identify test content. + * @param filePath file path to inspect + * @param tree parsed file tree + * @return whether the file is likely test content + */ + public static boolean looksLikeTestFile(String filePath, @Nullable FileInput tree) { + if (looksLikeTestFileByPath(filePath)) { + return true; + } + if (tree == null || tree.statements() == null) { + return false; + } + return looksLikeTestFileByStatements(tree.statements()); + } + + /** + * Determines whether a file path identifies test content. + * @param filePath file path to inspect + * @return whether the path indicates test content + */ + static boolean looksLikeTestFileByPath(String filePath) { + if (filePath.isEmpty()) { + return false; + } + String[] components = filePath.replace('\\', '/').split("/"); + for (int i = 0; i < components.length - 1; i++) { + if (TEST_DIRECTORY_NAMES.contains(components[i].toLowerCase(Locale.ROOT))) { + return true; + } + } + return components[components.length - 1].toLowerCase(Locale.ROOT).contains("test"); + } + + /** + * Determines whether top-level statements identify test content. + * @param statements top-level statements + * @return whether the statements indicate test content + */ + private static boolean looksLikeTestFileByStatements(StatementList statements) { + int functionCount = 0; + int testFunctionCount = 0; + for (Statement statement : statements.statements()) { + if (isTestFrameworkImport(statement) || isTestClass(statement)) { + return true; + } + if (statement instanceof FunctionDef functionDef) { + functionCount++; + if (functionDef.name().name().toLowerCase(Locale.ROOT).startsWith("test")) { + testFunctionCount++; + } + } + } + return testFunctionCount * 2 > functionCount; + } + + /** + * Determines whether a statement imports a testing framework or utility. + * @param statement top-level statement to inspect + * @return whether the statement imports test-specific code + */ + private static boolean isTestFrameworkImport(Statement statement) { + if (statement instanceof ImportName importName) { + return importName.modules().stream().anyMatch(aliasedName -> isTestModule(aliasedName.dottedName())); + } + if (statement instanceof ImportFrom importFrom && importFrom.module() != null) { + return isTestModule(importFrom.module()); + } + return false; + } + + /** + * Determines whether a module belongs to a testing ecosystem. + * @param module imported module name + * @return whether the module is test-specific + */ + private static boolean isTestModule(DottedName module) { + var names = module.names(); + if (names.isEmpty()) { + return false; + } + if (TEST_MODULE_ROOTS.contains(names.get(0).name())) { + return true; + } + return names.size() > 1 && TEST_MODULE_PREFIXES.contains(names.get(0).name() + "." + names.get(1).name()); + } + + /** + * Determines whether a statement declares a top-level test class. + * @param statement top-level statement to inspect + * @return whether the statement is a test class declaration + */ + private static boolean isTestClass(Statement statement) { + return statement instanceof ClassDef classDef && classDef.name().name().startsWith("Test"); + } + + /** + * Determines whether a configuration property has a non-blank value. + * @param config analysis configuration + * @param key property key + * @return whether the property is configured + */ + private static boolean isPropertyConfigured(Configuration config, String key) { + return config.get(key).filter(value -> !value.isBlank()).isPresent(); + } +} diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/PythonVisitorContext.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/PythonVisitorContext.java index b36a70bbf..9fb851a04 100644 --- a/python-frontend/src/main/java/org/sonar/plugins/python/api/PythonVisitorContext.java +++ b/python-frontend/src/main/java/org/sonar/plugins/python/api/PythonVisitorContext.java @@ -28,6 +28,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.SonarProduct; +import org.sonar.plugins.python.TestFileClassifier; import org.sonar.plugins.python.api.PythonCheck.PreciseIssue; import org.sonar.plugins.python.api.caching.CacheContext; import org.sonar.plugins.python.api.cfg.ControlFlowGraph; @@ -63,6 +64,7 @@ public class PythonVisitorContext extends PythonInputFileContext { private final Map lvaMap; private final ReachingDefinitionsAnalysis reachingDefinitionsAnalysis; private final TypeTable typeTable; + private final boolean likelyTestFile; @@ -76,7 +78,8 @@ private PythonVisitorContext(FileInput rootTree, ModuleType moduleType, CallGraph callGraph, TypeTable typeTable, - Map cfgMap + Map cfgMap, + @Nullable String testFilePath ) { super(pythonFile, workingDirectory, cacheContext, sonarProduct, projectLevelSymbolTable); this.moduleType = moduleType; @@ -90,9 +93,23 @@ private PythonVisitorContext(FileInput rootTree, this.typeTable = typeTable; this.typeChecker = new TypeChecker(typeTable); this.issues = new ArrayList<>(); + this.likelyTestFile = testFilePath == null ? + TestFileClassifier.looksLikeTestFile(pythonFile, rootTree) : + TestFileClassifier.looksLikeTestFile(testFilePath, rootTree); } public PythonVisitorContext(PythonFile pythonFile, RecognitionException parsingException, SonarProduct sonarProduct) { + this(pythonFile, parsingException, sonarProduct, null); + } + + /** + * Creates a context for a Python file that could not be parsed. + * @param pythonFile analyzed Python file + * @param parsingException parsing failure + * @param sonarProduct analysis product + * @param testFilePath project-relative path used for test classification + */ + public PythonVisitorContext(PythonFile pythonFile, RecognitionException parsingException, SonarProduct sonarProduct, @Nullable String testFilePath) { super(pythonFile, null, CacheContextImpl.dummyCache(), sonarProduct, ProjectLevelSymbolTable.empty()); this.rootTree = null; this.parsingException = parsingException; @@ -105,12 +122,23 @@ public PythonVisitorContext(PythonFile pythonFile, RecognitionException parsingE this.lvaMap = new HashMap<>(); this.issues = new ArrayList<>(); this.moduleType = null; + this.likelyTestFile = testFilePath == null ? + TestFileClassifier.looksLikeTestFile(pythonFile, null) : + TestFileClassifier.looksLikeTestFile(testFilePath, null); } public FileInput rootTree() { return rootTree; } + /** + * Reports whether this file is likely to contain test code. + * @return whether the file is likely test content + */ + public boolean isLikelyTestFile() { + return likelyTestFile; + } + public TypeChecker typeChecker() { return typeChecker; } @@ -181,6 +209,7 @@ public static class Builder { private Optional packageName = Optional.empty(); private Optional moduleType = Optional.empty(); private Optional> cfgMap = Optional.empty(); + private Optional testFilePath = Optional.empty(); public Builder(FileInput rootTree, PythonFile pythonFile) { this.rootTree = rootTree; @@ -238,6 +267,16 @@ public Builder cfgMap(Map cfgMap) { return this; } + /** + * Sets the project-relative path used for test classification. + * @param testFilePath project-relative file path + * @return this builder + */ + public Builder testFilePath(String testFilePath) { + this.testFilePath = Optional.of(testFilePath); + return this; + } + public PythonVisitorContext build() { var symbolTable = projectLevelSymbolTable.orElseGet(ProjectLevelSymbolTable::empty); var pkgName = packageName.orElse(""); @@ -270,7 +309,8 @@ public PythonVisitorContext build() { mt, finalCallGraph, finalTypeTable, - finalCfgMap + finalCfgMap, + testFilePath.orElse(null) ); } diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/SubscriptionContext.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/SubscriptionContext.java index aec793cdb..f77078f38 100644 --- a/python-frontend/src/main/java/org/sonar/plugins/python/api/SubscriptionContext.java +++ b/python-frontend/src/main/java/org/sonar/plugins/python/api/SubscriptionContext.java @@ -53,6 +53,12 @@ public interface SubscriptionContext { PythonFile pythonFile(); + /** + * Reports whether the current file is likely to contain test code. + * @return whether the file is likely test content + */ + boolean isLikelyTestFile(); + /** * List of Python versions this project is compatible with. */ diff --git a/python-frontend/src/main/java/org/sonar/python/SubscriptionVisitor.java b/python-frontend/src/main/java/org/sonar/python/SubscriptionVisitor.java index df4b66dd6..c1aa7de58 100644 --- a/python-frontend/src/main/java/org/sonar/python/SubscriptionVisitor.java +++ b/python-frontend/src/main/java/org/sonar/python/SubscriptionVisitor.java @@ -172,6 +172,11 @@ public PythonFile pythonFile() { return pythonVisitorContext.pythonFile(); } + @Override + public boolean isLikelyTestFile() { + return pythonVisitorContext.isLikelyTestFile(); + } + @Override public Collection stubFilesSymbols() { return pythonVisitorContext.stubFilesSymbols(); diff --git a/python-frontend/src/test/java/org/sonar/plugins/python/api/PythonVisitorContextTest.java b/python-frontend/src/test/java/org/sonar/plugins/python/api/PythonVisitorContextTest.java index 99c54c93a..6e491b682 100644 --- a/python-frontend/src/test/java/org/sonar/plugins/python/api/PythonVisitorContextTest.java +++ b/python-frontend/src/test/java/org/sonar/plugins/python/api/PythonVisitorContextTest.java @@ -17,6 +17,7 @@ package org.sonar.plugins.python.api; import com.sonar.sslr.api.RecognitionException; +import java.net.URI; import java.util.Collections; import java.util.Map; import java.util.Set; @@ -45,6 +46,25 @@ import static org.sonar.python.PythonTestUtils.pythonFile; class PythonVisitorContextTest { + @Test + void likely_test_file_is_computed_from_the_parsed_file() { + var fileInput = PythonTestUtils.parse("import pytest"); + var context = new PythonVisitorContext.Builder(fileInput, pythonFile("production.py")).build(); + + assertThat(context.isLikelyTestFile()).isTrue(); + } + + @Test + void likely_test_file_uses_the_project_relative_path_when_available() { + var file = pythonFile("production.py"); + when(file.uri()).thenReturn(URI.create("file:///workspace/testing/production.py")); + var context = new PythonVisitorContext.Builder(PythonTestUtils.parse("pass"), file) + .testFilePath("src/production.py") + .build(); + + assertThat(context.isLikelyTestFile()).isFalse(); + } + @Test void fullyQualifiedModuleName() { FileInput fileInput = PythonTestUtils.parse("def foo(): pass"); diff --git a/python-frontend/src/test/java/org/sonar/python/SubscriptionVisitorTest.java b/python-frontend/src/test/java/org/sonar/python/SubscriptionVisitorTest.java index c1ff9918f..7052b2124 100644 --- a/python-frontend/src/test/java/org/sonar/python/SubscriptionVisitorTest.java +++ b/python-frontend/src/test/java/org/sonar/python/SubscriptionVisitorTest.java @@ -77,7 +77,7 @@ public void initialize(Context context) { @Test void exposed_visitor_data() { - FileInput fileInput = PythonTestUtils.parse("def foo(): ..."); + FileInput fileInput = PythonTestUtils.parse("import pytest\ndef foo(): ..."); var cache = mock(CacheContext.class); PythonFile pythonFile = PythonTestUtils.pythonFile("file"); @@ -92,6 +92,7 @@ public void initialize(Context context) { context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> { assertThat(ctx.cacheContext()).isSameAs(cache); assertThat(ctx.pythonFile()).isEqualTo(pythonFile); + assertThat(ctx.isLikelyTestFile()).isTrue(); assertThat(ctx.sourcePythonVersions()).isEqualTo(ProjectPythonVersion.currentVersions()); }); }