From 122c79998383549df4731e67927a310f74c7583c Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 14 Oct 2023 20:09:50 +0200 Subject: [PATCH 1/4] [mypy] Fix Item "None" of "Optional[Module]" has no attribute "__file__" --- pylint_pytest/utils.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pylint_pytest/utils.py b/pylint_pytest/utils.py index 41f8817..6c66e6e 100644 --- a/pylint_pytest/utils.py +++ b/pylint_pytest/utils.py @@ -108,12 +108,11 @@ def _is_same_module(fixtures, import_node, fixture_name): try: for fixture in fixtures[fixture_name]: for import_from in import_node.root().globals[fixture_name]: - if ( - inspect.getmodule(fixture.func).__file__ - == import_from.parent.import_module( - import_from.modname, False, import_from.level - ).file - ): + module = inspect.getmodule(fixture.func) + parent_import = import_from.parent.import_module( + import_from.modname, False, import_from.level + ) + if module is not None and module.__file__ == parent_import.file: return True except Exception: # pylint: disable=broad-except pass From 33d1313d368c7551ba6c42efcb903b57fced4341 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 14 Oct 2023 20:10:01 +0200 Subject: [PATCH 2/4] Activate mypy in pre-commit --- .pre-commit-config.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 399a480..50be1c3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -41,10 +41,11 @@ repos: - id: rst-directive-colons - id: rst-inline-touching-normal - id: text-unicode-replacement-char - # - repo: https://github.com/pre-commit/mirrors-mypy - # rev: v1.6.0 - # hooks: - # - id: mypy + - repo: https://github.com/pre-commit/mirrors-mypy + rev: v1.6.0 + hooks: + - id: mypy + exclude: "tests/input/" - repo: local hooks: - id: pylint From b65e9796f6d86db31e36f16edd682d3560ed8fbd Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Wed, 18 Oct 2023 01:17:26 +0300 Subject: [PATCH 3/4] 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> --- pylint_pytest/checkers/class_attr_loader.py | 18 +++++++---- pylint_pytest/checkers/fixture.py | 34 ++++++++++--------- pylint_pytest/checkers/types.py | 15 +++++++++ pyproject.toml | 5 +++ tests/base_tester.py | 36 +++++++++++++-------- tests/base_tester_test.py | 28 ++++++++++++++++ tests/test_pytest_yield_fixture.py | 1 - 7 files changed, 100 insertions(+), 37 deletions(-) create mode 100644 pylint_pytest/checkers/types.py create mode 100644 tests/base_tester_test.py diff --git a/pylint_pytest/checkers/class_attr_loader.py b/pylint_pytest/checkers/class_attr_loader.py index 34ce714..f9b5557 100644 --- a/pylint_pytest/checkers/class_attr_loader.py +++ b/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 diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 3ff7400..b41201f 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/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: diff --git a/pylint_pytest/checkers/types.py b/pylint_pytest/checkers/types.py new file mode 100644 index 0000000..7fd5708 --- /dev/null +++ b/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) diff --git a/pyproject.toml b/pyproject.toml index 68ad0a8..9f4abc8 100644 --- a/pyproject.toml +++ b/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= [ diff --git a/tests/base_tester.py b/tests/base_tester.py index 5653cfd..4d6498c 100644 --- a/tests/base_tester.py +++ b/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 diff --git a/tests/base_tester_test.py b/tests/base_tester_test.py new file mode 100644 index 0000000..dc1a48a --- /dev/null +++ b/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 diff --git a/tests/test_pytest_yield_fixture.py b/tests/test_pytest_yield_fixture.py index 4408fca..359dfb8 100644 --- a/tests/test_pytest_yield_fixture.py +++ b/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): From b8ce4adaade3031a11368a7ec2e5da067531b5f3 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Wed, 18 Oct 2023 01:00:45 +0300 Subject: [PATCH 4/4] Roll back `from __future__ import annotations` for Python 3.6 ... and `f{msg_id=}` Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- pylint_pytest/checkers/class_attr_loader.py | 6 +++--- pylint_pytest/checkers/fixture.py | 11 +++++------ pylint_pytest/checkers/types.py | 2 -- tests/base_tester.py | 10 ++++------ tests/base_tester_test.py | 2 +- 5 files changed, 13 insertions(+), 18 deletions(-) diff --git a/pylint_pytest/checkers/class_attr_loader.py b/pylint_pytest/checkers/class_attr_loader.py index f9b5557..6c9b4e9 100644 --- a/pylint_pytest/checkers/class_attr_loader.py +++ b/pylint_pytest/checkers/class_attr_loader.py @@ -1,4 +1,4 @@ -from __future__ import annotations +from typing import Optional, Set from astroid import Assign, Attribute, ClassDef, Name from pylint.interfaces import IAstroidChecker @@ -12,8 +12,8 @@ class ClassAttrLoader(BasePytestChecker): msgs = {"E6400": ("", "pytest-class-attr-loader", "")} in_setup = False - request_cls: set[str] = set() - class_node: ClassDef | None = None + request_cls: Set[str] = set() + class_node: Optional[ClassDef] = None def visit_functiondef(self, node): """determine if a method is a class setup method""" diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index b41201f..32ec894 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -1,9 +1,8 @@ -from __future__ import annotations - import fnmatch import os import sys from pathlib import Path +from typing import Set, Tuple import astroid import pylint @@ -22,7 +21,7 @@ from .types import FixtureDict, replacement_add_message # TODO: support pytest python_files configuration -FILE_NAME_PATTERNS: tuple[str, ...] = ("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" ) @@ -31,7 +30,7 @@ class FixtureCollector: # Same as ``_pytest.fixtures.FixtureManager._arg2fixturedefs``. fixtures: FixtureDict = {} - errors: set[pytest.CollectReport] = set() + errors: Set[pytest.CollectReport] = set() def pytest_sessionfinish(self, session): # pylint: disable=protected-access @@ -80,9 +79,9 @@ class FixtureChecker(BasePytestChecker): # Store all fixtures discovered by pytest session _pytest_fixtures: FixtureDict = {} # Stores all used function arguments - _invoked_with_func_args: set[str] = set() + _invoked_with_func_args: Set[str] = set() # Stores all invoked fixtures through @pytest.mark.usefixture(...) - _invoked_with_usefixtures: set[str] = set() + _invoked_with_usefixtures: Set[str] = set() _original_add_message = replacement_add_message def open(self): diff --git a/pylint_pytest/checkers/types.py b/pylint_pytest/checkers/types.py index 7fd5708..c4c43e8 100644 --- a/pylint_pytest/checkers/types.py +++ b/pylint_pytest/checkers/types.py @@ -1,5 +1,3 @@ -from __future__ import annotations - import sys from pprint import pprint from typing import Any, Dict, List diff --git a/tests/base_tester.py b/tests/base_tester.py index 4d6498c..12d2e63 100644 --- a/tests/base_tester.py +++ b/tests/base_tester.py @@ -1,10 +1,8 @@ -from __future__ import annotations - import os import sys from abc import ABC from pprint import pprint -from typing import Any +from typing import Any, Dict, List import astroid from pylint.testutils import MessageTest, UnittestLinter @@ -25,10 +23,10 @@ class BasePytestTester(ABC): CHECKER_CLASS = BaseChecker - IMPACTED_CHECKER_CLASSES: list[BaseChecker] = [] + IMPACTED_CHECKER_CLASSES: List[BaseChecker] = [] MSG_ID: str - msgs: list[MessageTest] = [] - CONFIG: dict[str, Any] = {} + msgs: List[MessageTest] = [] + CONFIG: Dict[str, Any] = {} def __init_subclass__(cls, **kwargs): super().__init_subclass__(**kwargs) diff --git a/tests/base_tester_test.py b/tests/base_tester_test.py index dc1a48a..4cf43ff 100644 --- a/tests/base_tester_test.py +++ b/tests/base_tester_test.py @@ -20,7 +20,7 @@ class NoMsgIDSubclass(BasePytestTester): pass -@pytest.mark.parametrize("msg_id", [123, None, ""], ids=lambda msg_id: f"{msg_id=}") +@pytest.mark.parametrize("msg_id", [123, None, ""], ids=lambda x: f"msg_id={x}") def test_init_subclass_invalid_msg_id_type(msg_id): with pytest.raises(TypeError):