python: Add a check for misplaced EPYTEST_* variables

Add a check for `EPYTEST_*` variables being declared after calling
`distutils_enable_tests`, so they don't affect it.  This catches only
global scope use, as it is valid to override them in function scope.
Since using `${var:=...}` assignment is also valid, hack a quick method
to handle them as well.

Closes: #739
Signed-off-by: Michał Górny <mgorny@gentoo.org>
Part-of: #747
Signed-off-by: Arthur Zamarin <arthurzam@gentoo.org>

Author: mgorny

src/pkgcheck/bash/__init__.py

@@ -13,6 +13,7 @@
 cmd_query = query("(command) @call")
 func_query = query("(function_definition) @func")
 var_assign_query = query("(variable_assignment) @assign")
+var_expansion_query = query("(expansion) @exp")
 var_query = query("(variable_name) @var")
 
 

src/pkgcheck/checks/python.py

@@ -247,6 +247,25 @@ class PythonMissingSCMDependency(results.VersionResult, results.Warning):
     )
 
 
+class MisplacedEPyTestVar(results.LineResult, results.Error):
+    """Invalid placement of ``EPYTEST_*`` variable
+
+    ``EPYTEST_*`` variables need to be set prior to ``distutils_enable_tests``
+    to enable its functionality.  The exception to that rule are local overrides
+    in ``python_test()`` -- we presume the author knows what they're doing.
+    """
+
+    def __init__(self, variable, **kwargs):
+        super().__init__(**kwargs)
+        self.variable = variable
+
+    @property
+    def desc(self):
+        return (
+            f"line {self.lineno}: variable set after calling distutils_enable_tests: {self.line!r}"
+        )
+
+
 class PythonCheck(Check):
     """Python eclass checks.
 
@@ -256,7 +275,7 @@ class PythonCheck(Check):
 
     _source = sources.EbuildParseRepoSource
     known_results = frozenset(
-        [
+        {
             MissingPythonEclass,
             PythonMissingRequiredUse,
             PythonMissingDeps,
@@ -268,7 +287,8 @@ class PythonCheck(Check):
             PythonAnyMismatchedUseHasVersionCheck,
             PythonAnyMismatchedDepHasVersionCheck,
             PythonMissingSCMDependency,
-        ]
+            MisplacedEPyTestVar,
+        }
     )
 
     has_version_known_flags = {
@@ -389,6 +409,53 @@ def check_pep517(self, pkg):
             if not self.setuptools_scm.intersection(bdepends):
                 yield PythonMissingSCMDependency(pkg=pkg)
 
+    def _get_all_global_assignments(self, pkg):
+        """Iterate over global plain and expansion assignments"""
+        for node in pkg.global_query(bash.var_assign_query):
+            name = pkg.node_str(node.child_by_field_name("name"))
+            yield (name, node)
+
+        # ${var:=value} expansions.
+        for node in pkg.global_query(bash.var_expansion_query):
+            if node.children[0].text != b"${":
+                continue
+            name = node.children[1].text
+            if node.children[2].text not in (b"=", b":="):
+                continue
+            yield (name.decode(), node)
+
+    def check_epytest_vars(self, pkg):
+        """Check for incorrect use of EPYTEST_* variables"""
+        # TODO: do we want to check for multiple det calls? Quite unlikely.
+        for node in pkg.global_query(bash.cmd_query):
+            name = pkg.node_str(node.child_by_field_name("name"))
+            if name != "distutils_enable_tests":
+                continue
+            for argument_node in node.children_by_field_name("argument"):
+                argument = pkg.node_str(argument_node)
+                # Check if the first argument is "pytest", but we need
+                # to skip line continuations explicitly.
+                if argument == "pytest":
+                    det_lineno, _ = node.start_point
+                    break
+                elif argument != "\\":
+                    return
+            else:
+                return
+
+        for var_name, var_node in self._get_all_global_assignments(pkg):
+            # While not all variables affect distutils_enable_tests, make it
+            # future proof.  Also, it makes sense to keep them together.
+            # Just opt out from the long lists.
+            if var_name.startswith("EPYTEST_") and var_name not in (
+                "EPYTEST_DESELECT",
+                "EPYTEST_IGNORE",
+            ):
+                lineno, _ = var_node.start_point
+                if lineno > det_lineno:
+                    line = pkg.node_str(var_node)
+                    yield MisplacedEPyTestVar(var_name, line=line, lineno=lineno + 1, pkg=pkg)
+
     @staticmethod
     def _prepare_deps(deps: str):
         try:
@@ -545,6 +612,7 @@ def feed(self, pkg):
 
         if "distutils-r1" in pkg.inherited:
             yield from self.check_pep517(pkg)
+            yield from self.check_epytest_vars(pkg)
 
         any_dep_func = self.eclass_any_dep_func[eclass]
         python_check_deps = self.build_python_gen_any_dep_calls(pkg, any_dep_func)

testdata/data/repos/python/PythonCheck/MisplacedEPyTestVar/expected.json

@@ -0,0 +1,4 @@
+{"__class__": "MisplacedEPyTestVar", "category": "PythonCheck", "package": "MisplacedEPyTestVar", "version": "0", "line": "EPYTEST_PLUGIN_AUTOLOAD=1", "lineno": 15, "variable": "EPYTEST_PLUGIN_AUTOLOAD"}
+{"__class__": "MisplacedEPyTestVar", "category": "PythonCheck", "package": "MisplacedEPyTestVar", "version": "0", "line": "EPYTEST_PLUGINS=( foo bar baz )", "lineno": 16, "variable": "EPYTEST_PLUGINS"}
+{"__class__": "MisplacedEPyTestVar", "category": "PythonCheck", "package": "MisplacedEPyTestVar", "version": "0", "line": "EPYTEST_XDIST=1", "lineno": 17, "variable": "EPYTEST_XDIST"}
+{"__class__": "MisplacedEPyTestVar", "category": "PythonCheck", "package": "MisplacedEPyTestVar", "version": "0", "line": "${EPYTEST_TIMEOUT:=180}", "lineno": 18, "variable": "EPYTEST_TIMEOUT"}

testdata/data/repos/python/PythonCheck/MisplacedEPyTestVar/fix.patch

@@ -0,0 +1,19 @@
+diff '--color=auto' -Naur python/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild fixed/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild
+--- python/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild	2025-07-12 17:10:51.665298954 +0200
++++ fixed/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild	2025-07-12 17:15:30.258231253 +0200
+@@ -10,13 +10,13 @@
+ LICENSE="BSD"
+ SLOT="0"
+ 
+-distutils_enable_tests pytest
+-
+ EPYTEST_PLUGIN_AUTOLOAD=1
+ EPYTEST_PLUGINS=( foo bar baz )
+ EPYTEST_XDIST=1
+ : ${EPYTEST_TIMEOUT:=180}
+ 
++distutils_enable_tests pytest
++
+ EPYTEST_DESELECT=(
+ 	tests/test_foo.py::test_foo
+ )

testdata/repos/python/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild

@@ -0,0 +1,31 @@
+EAPI=8
+
+DISTUTILS_USE_PEP517=flit
+PYTHON_COMPAT=( python3_10 )
+
+inherit distutils-r1
+
+DESCRIPTION="Ebuild with misplaced EPYTEST vars"
+HOMEPAGE="https://github.com/pkgcore/pkgcheck"
+LICENSE="BSD"
+SLOT="0"
+
+distutils_enable_tests pytest
+
+EPYTEST_PLUGIN_AUTOLOAD=1
+EPYTEST_PLUGINS=( foo bar baz )
+EPYTEST_XDIST=1
+: ${EPYTEST_TIMEOUT:=180}
+
+EPYTEST_DESELECT=(
+	tests/test_foo.py::test_foo
+)
+EPYTEST_IGNORE=(
+	tests/test_bar.py
+)
+
+python_test() {
+	: ${EPYTEST_TIMEOUT:=300}
+	local EPYTEST_PLUGINS=( "${EPYTEST_PLUGINS[@]}" more )
+	EPYTEST_XDIST= epytest
+}

testdata/repos/python/eclass/distutils-r1.eclass

@@ -58,3 +58,5 @@ _distutils_set_globals() {
 }
 _distutils_set_globals
 unset -f _distutils_set_globals
+
+distutils_enable_tests() { :; }