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 diff --git a/pylint_pytest/checkers/class_attr_loader.py b/pylint_pytest/checkers/class_attr_loader.py index 34ce714..6c9b4e9 100644 --- a/pylint_pytest/checkers/class_attr_loader.py +++ b/pylint_pytest/checkers/class_attr_loader.py @@ -1,4 +1,6 @@ -import astroid +from typing import Optional, Set + +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: Optional[ClassDef] = 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..32ec894 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -2,6 +2,7 @@ import os import sys from pathlib import Path +from typing import Set, Tuple import astroid import pylint @@ -17,17 +18,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 +76,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 +93,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 +106,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..c4c43e8 --- /dev/null +++ b/pylint_pytest/checkers/types.py @@ -0,0 +1,13 @@ +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/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 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..12d2e63 100644 --- a/tests/base_tester.py +++ b/tests/base_tester.py @@ -1,9 +1,11 @@ import os import sys +from abc import ABC from pprint import pprint +from typing import Any, Dict, List import astroid -from pylint.testutils import UnittestLinter +from pylint.testutils import MessageTest, UnittestLinter try: from pylint.utils import ASTWalker @@ -15,30 +17,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..4cf43ff --- /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 x: f"msg_id={x}") +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):