Fix mypy issues

Most of the issues are "almost clear-cut", except the
astroid stuff - `pylint_pytest/checkers/class_attr_loader.py`.

Those were mostly placated (but hopefully done right).

Additionally, add some sanity features to `BasePytestTester`:
* Marked class as `ABC`
* Enforce subclasses to initialize `MSG_ID`

Mostly both of those changes are "aesthetical",
and do not contribute to strict guarantees of behavior, or type assertions.

But at least `pytest` cries loudly when used wrong - so that's something.

Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>

Author: stdedos

pylint_pytest/checkers/class_attr_loader.py

@@ -1,4 +1,6 @@
-import astroid
+from __future__ import annotations
+
+from astroid import Assign, Attribute, ClassDef, Name
 from pylint.interfaces import IAstroidChecker
 
 from ..utils import _can_use_fixture, _is_class_autouse_fixture
@@ -10,8 +12,8 @@ class ClassAttrLoader(BasePytestChecker):
     msgs = {"E6400": ("", "pytest-class-attr-loader", "")}
 
     in_setup = False
-    request_cls = set()
-    class_node = None
+    request_cls: set[str] = set()
+    class_node: ClassDef | None = None
 
     def visit_functiondef(self, node):
         """determine if a method is a class setup method"""
@@ -23,12 +25,13 @@ def visit_functiondef(self, node):
             self.in_setup = True
             self.class_node = node.parent
 
-    def visit_assign(self, node):
+    def visit_assign(self, node: Assign):
         """store the aliases for `cls`"""
         if (
             self.in_setup
-            and isinstance(node.value, astroid.Attribute)
+            and isinstance(node.value, Attribute)
             and node.value.attrname == "cls"
+            and isinstance(node.value.expr, Name)
             and node.value.expr.name == "request"
         ):
             # storing the aliases for cls from request.cls
@@ -37,14 +40,15 @@ def visit_assign(self, node):
     def visit_assignattr(self, node):
         if (
             self.in_setup
-            and isinstance(node.expr, astroid.Name)
+            and isinstance(node.expr, Name)
             and node.expr.name in self.request_cls
+            and self.class_node is not None
             and node.attrname not in self.class_node.locals
         ):
             try:
                 # find Assign node which contains the source "value"
                 assign_node = node
-                while not isinstance(assign_node, astroid.Assign):
+                while not isinstance(assign_node, Assign):
                     assign_node = assign_node.parent
 
                 # hack class locals

pylint_pytest/checkers/fixture.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import fnmatch
 import os
 import sys
@@ -17,17 +19,19 @@
     _is_same_module,
 )
 from . import BasePytestChecker
+from .types import FixtureDict, replacement_add_message
 
 # TODO: support pytest python_files configuration
-FILE_NAME_PATTERNS = ("test_*.py", "*_test.py")
+FILE_NAME_PATTERNS: tuple[str, ...] = ("test_*.py", "*_test.py")
 ARGUMENT_ARE_KEYWORD_ONLY = (
     "https://docs.pytest.org/en/stable/deprecations.html#pytest-fixture-arguments-are-keyword-only"
 )
 
 
 class FixtureCollector:
-    fixtures = {}
-    errors = set()
+    # Same as ``_pytest.fixtures.FixtureManager._arg2fixturedefs``.
+    fixtures: FixtureDict = {}
+    errors: set[pytest.CollectReport] = set()
 
     def pytest_sessionfinish(self, session):
         # pylint: disable=protected-access
@@ -73,10 +77,13 @@ class FixtureChecker(BasePytestChecker):
         ),
     }
 
-    _pytest_fixtures = {}
-    _invoked_with_func_args = set()
-    _invoked_with_usefixtures = set()
-    _original_add_message = callable
+    # Store all fixtures discovered by pytest session
+    _pytest_fixtures: FixtureDict = {}
+    # Stores all used function arguments
+    _invoked_with_func_args: set[str] = set()
+    # Stores all invoked fixtures through @pytest.mark.usefixture(...)
+    _invoked_with_usefixtures: set[str] = set()
+    _original_add_message = replacement_add_message
 
     def open(self):
         # patch VariablesChecker.add_message
@@ -87,7 +94,7 @@ def close(self):
         """restore & reset class attr for testing"""
         # restore add_message
         VariablesChecker.add_message = FixtureChecker._original_add_message
-        FixtureChecker._original_add_message = callable
+        FixtureChecker._original_add_message = replacement_add_message
 
         # reset fixture info storage
         FixtureChecker._pytest_fixtures = {}
@@ -100,14 +107,9 @@ def visit_module(self, node):
         - invoke pytest session to collect available fixtures
         - create containers for the module to store args and fixtures
         """
-        # storing all fixtures discovered by pytest session
-        FixtureChecker._pytest_fixtures = {}  # Dict[List[_pytest.fixtures.FixtureDef]]
-
-        # storing all used function arguments
-        FixtureChecker._invoked_with_func_args = set()  # Set[str]
-
-        # storing all invoked fixtures through @pytest.mark.usefixture(...)
-        FixtureChecker._invoked_with_usefixtures = set()  # Set[str]
+        FixtureChecker._pytest_fixtures = {}
+        FixtureChecker._invoked_with_func_args = set()
+        FixtureChecker._invoked_with_usefixtures = set()
 
         is_test_module = False
         for pattern in FILE_NAME_PATTERNS:

pylint_pytest/checkers/types.py

@@ -0,0 +1,15 @@
+from __future__ import annotations
+
+import sys
+from pprint import pprint
+from typing import Any, Dict, List
+
+from _pytest.fixtures import FixtureDef
+
+FixtureDict = Dict[str, List[FixtureDef[Any]]]
+
+
+def replacement_add_message(*args, **kwargs):
+    print("Called un-initialized _original_add_message with:", file=sys.stderr)
+    pprint(args, sys.stderr)
+    pprint(kwargs, sys.stderr)

pyproject.toml

@@ -89,6 +89,9 @@ ignore = [
     "RUF012", # Mutable class attributes should be annotated with `typing.ClassVar`
 ]
 
+# py36, but ruff does not support it :/
+target-version = "py37"
+
 [tool.ruff.pydocstyle]
 convention = "google"
 
@@ -110,6 +113,8 @@ convention = "google"
 
 [tool.pylint]
 
+py-version = "3.6"
+
 ignore-paths="tests/input" # Ignore test inputs
 
 load-plugins= [

tests/base_tester.py

@@ -1,9 +1,13 @@
+from __future__ import annotations
+
 import os
 import sys
+from abc import ABC
 from pprint import pprint
+from typing import Any
 
 import astroid
-from pylint.testutils import UnittestLinter
+from pylint.testutils import MessageTest, UnittestLinter
 
 try:
     from pylint.utils import ASTWalker
@@ -15,30 +19,36 @@
 
 import pylint_pytest.checkers.fixture
 
-# XXX: allow all file name
+# XXX: allow all file names
 pylint_pytest.checkers.fixture.FILE_NAME_PATTERNS = ("*",)
 
 
-class BasePytestTester:
+class BasePytestTester(ABC):
     CHECKER_CLASS = BaseChecker
-    IMPACTED_CHECKER_CLASSES = []
-    MSG_ID = None
-    msgs = None
-    CONFIG = {}
+    IMPACTED_CHECKER_CLASSES: list[BaseChecker] = []
+    MSG_ID: str
+    msgs: list[MessageTest] = []
+    CONFIG: dict[str, Any] = {}
+
+    def __init_subclass__(cls, **kwargs):
+        super().__init_subclass__(**kwargs)
+        if not hasattr(cls, "MSG_ID") or not isinstance(cls.MSG_ID, str) or not cls.MSG_ID:
+            raise TypeError("Subclasses must define a non-empty MSG_ID of type str")
 
     enable_plugin = True
 
-    def run_linter(self, enable_plugin, file_path=None):
+    def run_linter(self, enable_plugin):
         self.enable_plugin = enable_plugin
 
-        # pylint: disable=protected-access
-        if file_path is None:
-            module = sys._getframe(1).f_code.co_name.replace("test_", "", 1)
-            file_path = os.path.join(os.getcwd(), "tests", "input", self.MSG_ID, module + ".py")
+        # pylint: disable-next=protected-access
+        target_test_file = sys._getframe(1).f_code.co_name.replace("test_", "", 1)
+        file_path = os.path.join(
+            os.getcwd(), "tests", "input", self.MSG_ID, target_test_file + ".py"
+        )
 
         with open(file_path) as fin:
             content = fin.read()
-            module = astroid.parse(content, module_name=module)
+            module = astroid.parse(content, module_name=target_test_file)
             module.file = fin.name
 
         self.walk(module)  # run all checkers

tests/base_tester_test.py

@@ -0,0 +1,28 @@
+import pytest
+from base_tester import BasePytestTester
+
+# pylint: disable=unused-variable
+
+
+def test_init_subclass_valid_msg_id():
+    some_string = "some_string"
+
+    class ValidSubclass(BasePytestTester):
+        MSG_ID = some_string
+
+    assert ValidSubclass.MSG_ID == some_string
+
+
+def test_init_subclass_no_msg_id():
+    with pytest.raises(TypeError):
+
+        class NoMsgIDSubclass(BasePytestTester):
+            pass
+
+
+@pytest.mark.parametrize("msg_id", [123, None, ""], ids=lambda msg_id: f"{msg_id=}")
+def test_init_subclass_invalid_msg_id_type(msg_id):
+    with pytest.raises(TypeError):
+
+        class Subclass(BasePytestTester):
+            MSG_ID = msg_id

tests/test_pytest_yield_fixture.py

@@ -5,7 +5,6 @@
 
 class TestDeprecatedPytestYieldFixture(BasePytestTester):
     CHECKER_CLASS = FixtureChecker
-    IMPACTED_CHECKER_CLASSES = []
     MSG_ID = "deprecated-pytest-yield-fixture"
 
     def test_smoke(self):