diff --git a/pylint_pytest/__init__.py b/pylint_pytest/__init__.py index 93e4690..93a2da3 100644 --- a/pylint_pytest/__init__.py +++ b/pylint_pytest/__init__.py @@ -1,30 +1,21 @@ -import glob -import importlib -import inspect -import os +from pylint.checkers.variables import VariablesChecker +from pylint.lint import PyLinter -from .checkers import BasePytestChecker +from .checkers.class_attr_loader import ClassAttrLoader +from .checkers.fixture import FixtureChecker +from .checkers.variables import CustomVariablesChecker -def register(linter): - """auto discover pylint checker classes""" - dirname = os.path.dirname(__file__) - for module in glob.glob(os.path.join(dirname, "checkers", "*.py")): - # trim file extension - module = os.path.splitext(module)[0] +def register(linter: PyLinter) -> None: + """Register the checker classes""" + remove_original_variables_checker(linter) + linter.register_checker(CustomVariablesChecker(linter)) + linter.register_checker(FixtureChecker(linter)) + linter.register_checker(ClassAttrLoader(linter)) - # use relative path only - module = module.replace(dirname, "", 1) - # translate file path into module import path - module = module.replace(os.sep, ".") - - checker = importlib.import_module(module, package=os.path.basename(dirname)) - for attr_name in dir(checker): - attr_val = getattr(checker, attr_name) - if ( - attr_val != BasePytestChecker - and inspect.isclass(attr_val) - and issubclass(attr_val, BasePytestChecker) - ): - linter.register_checker(attr_val(linter)) +def remove_original_variables_checker(linter: PyLinter) -> None: + """We need to remove VariablesChecker before registering CustomVariablesChecker""" + variable_checkers = linter._checkers[VariablesChecker.name] # pylint: disable=protected-access + for checker in [x for x in variable_checkers if isinstance(x, VariablesChecker)]: + variable_checkers.remove(checker) diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 5b13360..791caca 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -7,17 +7,15 @@ import astroid import pytest -from pylint.checkers.variables import VariablesChecker from ..utils import ( _can_use_fixture, _is_pytest_fixture, _is_pytest_mark, _is_pytest_mark_usefixtures, - _is_same_module, ) from . import BasePytestChecker -from .types import FixtureDict, replacement_add_message +from .types import FixtureDict # TODO: support pytest python_files configuration FILE_NAME_PATTERNS: tuple[str, ...] = ("test_*.py", "*_test.py") @@ -80,19 +78,9 @@ class FixtureChecker(BasePytestChecker): _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 - FixtureChecker._original_add_message = VariablesChecker.add_message - VariablesChecker.add_message = FixtureChecker.patch_add_message def close(self): """restore & reset class attr for testing""" - # restore add_message - VariablesChecker.add_message = FixtureChecker._original_add_message - FixtureChecker._original_add_message = replacement_add_message - # reset fixture info storage FixtureChecker._pytest_fixtures = {} FixtureChecker._invoked_with_func_args = set() @@ -232,84 +220,3 @@ def visit_functiondef(self, node): self.add_message("deprecated-pytest-yield-fixture", node=node) for arg in node.args.args: self._invoked_with_func_args.add(arg.name) - - # pylint: disable=bad-staticmethod-argument # The function itself is an if-return logic. - @staticmethod - def patch_add_message( - self, msgid, line=None, node=None, args=None, confidence=None, col_offset=None - ): - """ - - intercept and discard unwanted warning messages - """ - # check W0611 unused-import - if msgid == "unused-import": - # actual attribute name is not passed as arg so...dirty hack - # message is usually in the form of '%s imported from %s (as %)' - message_tokens = args.split() - fixture_name = message_tokens[0] - - # ignoring 'import %s' message - if message_tokens[0] == "import" and len(message_tokens) == 2: - pass - - # fixture is defined in other modules and being imported to - # conftest for pytest magic - elif ( - isinstance(node.parent, astroid.Module) - and node.parent.name.split(".")[-1] == "conftest" - and fixture_name in FixtureChecker._pytest_fixtures - ): - return - - # imported fixture is referenced in test/fixture func - elif ( - fixture_name in FixtureChecker._invoked_with_func_args - and fixture_name in FixtureChecker._pytest_fixtures - ): - if _is_same_module( - fixtures=FixtureChecker._pytest_fixtures, - import_node=node, - fixture_name=fixture_name, - ): - return - - # fixture is referenced in @pytest.mark.usefixtures - elif ( - fixture_name in FixtureChecker._invoked_with_usefixtures - and fixture_name in FixtureChecker._pytest_fixtures - ): - if _is_same_module( - fixtures=FixtureChecker._pytest_fixtures, - import_node=node, - fixture_name=fixture_name, - ): - return - - # check W0613 unused-argument - if ( - msgid == "unused-argument" - and _can_use_fixture(node.parent.parent) - and isinstance(node.parent, astroid.Arguments) - ): - if node.name in FixtureChecker._pytest_fixtures: - # argument is used as a fixture - return - - fixnames = ( - arg.name for arg in node.parent.args if arg.name in FixtureChecker._pytest_fixtures - ) - for fixname in fixnames: - if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames: - # argument is used by a fixture - return - - # check W0621 redefined-outer-name - if ( - msgid == "redefined-outer-name" - and _can_use_fixture(node.parent.parent) - and isinstance(node.parent, astroid.Arguments) - and node.name in FixtureChecker._pytest_fixtures - ): - return - - FixtureChecker._original_add_message(self, msgid, line, node, args, confidence, col_offset) diff --git a/pylint_pytest/checkers/types.py b/pylint_pytest/checkers/types.py index 7fd5708..2f2b5bc 100644 --- a/pylint_pytest/checkers/types.py +++ b/pylint_pytest/checkers/types.py @@ -1,15 +1,7 @@ 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/pylint_pytest/checkers/variables.py b/pylint_pytest/checkers/variables.py new file mode 100644 index 0000000..7dde882 --- /dev/null +++ b/pylint_pytest/checkers/variables.py @@ -0,0 +1,107 @@ +from typing import Any, Optional + +from astroid import Arguments, Module +from astroid.nodes.node_ng import NodeNG +from pylint.checkers.variables import VariablesChecker +from pylint.interfaces import Confidence + +from pylint_pytest.utils import _can_use_fixture, _is_same_module + +from .fixture import FixtureChecker + + +class CustomVariablesChecker(VariablesChecker): + """Overrides the default VariablesChecker of pylint to discard unwanted warning messages""" + + # pylint: disable=protected-access + # this class needs to access the fixture checker registries + + def add_message( + self, + msgid: str, + line: Optional[int] = None, + node: Optional[NodeNG] = None, + args: Any = None, + confidence: Confidence = None, + col_offset: Optional[int] = None, + end_lineno: Optional[int] = None, + end_col_offset: Optional[int] = None, + ) -> None: + """ + - intercept and discard unwanted warning messages + """ + # check W0611 unused-import + if msgid == "unused-import": + # actual attribute name is not passed as arg so...dirty hack + # message is usually in the form of '%s imported from %s (as %)' + message_tokens = args.split() + fixture_name = message_tokens[0] + + # ignoring 'import %s' message + if message_tokens[0] == "import" and len(message_tokens) == 2: + pass + + # fixture is defined in other modules and being imported to + # conftest for pytest magic + elif ( + node + and isinstance(node.parent, Module) + and node.parent.name.split(".")[-1] == "conftest" + and fixture_name in FixtureChecker._pytest_fixtures + ): + return + + # imported fixture is referenced in test/fixture func + elif ( + fixture_name in FixtureChecker._invoked_with_func_args + and fixture_name in FixtureChecker._pytest_fixtures + ): + if _is_same_module( + fixtures=FixtureChecker._pytest_fixtures, + import_node=node, + fixture_name=fixture_name, + ): + return + + # fixture is referenced in @pytest.mark.usefixtures + elif ( + fixture_name in FixtureChecker._invoked_with_usefixtures + and fixture_name in FixtureChecker._pytest_fixtures + ): + if _is_same_module( + fixtures=FixtureChecker._pytest_fixtures, + import_node=node, + fixture_name=fixture_name, + ): + return + + # check W0613 unused-argument + if ( + msgid == "unused-argument" + and node + and _can_use_fixture(node.parent.parent) + and isinstance(node.parent, Arguments) + ): + if node.name in FixtureChecker._pytest_fixtures: + # argument is used as a fixture + return + + fixnames = ( + arg.name for arg in node.parent.args if arg.name in FixtureChecker._pytest_fixtures + ) + for fixname in fixnames: + if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames: + # argument is used by a fixture + return + + # check W0621 redefined-outer-name + if ( + msgid == "redefined-outer-name" + and node + and _can_use_fixture(node.parent.parent) + and isinstance(node.parent, Arguments) + and node.name in FixtureChecker._pytest_fixtures + ): + return + + super().add_message(msgid, line, node, args, confidence, col_offset) diff --git a/tests/input/unused-import/mark_usesfixtures.py b/tests/input/unused-import/mark_usesfixtures.py new file mode 100644 index 0000000..82aaea4 --- /dev/null +++ b/tests/input/unused-import/mark_usesfixtures.py @@ -0,0 +1,8 @@ +import pytest + +from other_fixture import other_fixture_not_in_conftest + + +@pytest.mark.usefixtures("other_fixture_not_in_conftest") +def uses_imported_fixture_with_decorator(): + assert True diff --git a/tests/input/unused-import/module.py b/tests/input/unused-import/module.py new file mode 100644 index 0000000..4bb8645 --- /dev/null +++ b/tests/input/unused-import/module.py @@ -0,0 +1,5 @@ +import pytest + + +def test_no_using_module(): + assert True diff --git a/tests/input/unused-import/other_fixture.py b/tests/input/unused-import/other_fixture.py new file mode 100644 index 0000000..652d908 --- /dev/null +++ b/tests/input/unused-import/other_fixture.py @@ -0,0 +1,6 @@ +import pytest + + +@pytest.fixture +def other_fixture_not_in_conftest(): + return True diff --git a/tests/test_pylint_integration.py b/tests/test_pylint_integration.py new file mode 100644 index 0000000..b8b2a67 --- /dev/null +++ b/tests/test_pylint_integration.py @@ -0,0 +1,29 @@ +""" +The tests in this file shall detect any error related to actual execution of pylint, while the +other test are more unit tests that focuses on the checkers behaviour. + +Notes: + Tests here are voluntarily minimalistic, the goal is not to test pylint, it is only checking + that pylint_pytest integrates just fine +""" +import subprocess + + +def test_simple_process(): + result = subprocess.run( + ["pylint", "--load-plugins", "pylint_pytest", "tests"], + capture_output=True, + check=False, + ) + # then no error + assert not result.stderr + + +def test_multi_process(): + result = subprocess.run( + ["pylint", "--load-plugins", "pylint_pytest", "-j", "2", "tests"], + capture_output=True, + check=False, + ) + # then no error + assert not result.stderr diff --git a/tests/test_redefined_outer_name.py b/tests/test_redefined_outer_name.py index ec0beef..5ff3dff 100644 --- a/tests/test_redefined_outer_name.py +++ b/tests/test_redefined_outer_name.py @@ -1,13 +1,13 @@ import pytest from base_tester import BasePytestTester -from pylint.checkers.variables import VariablesChecker from pylint_pytest.checkers.fixture import FixtureChecker +from pylint_pytest.checkers.variables import CustomVariablesChecker class TestRedefinedOuterName(BasePytestTester): CHECKER_CLASS = FixtureChecker - IMPACTED_CHECKER_CLASSES = [VariablesChecker] + IMPACTED_CHECKER_CLASSES = [CustomVariablesChecker] MSG_ID = "redefined-outer-name" @pytest.mark.parametrize("enable_plugin", [True, False]) diff --git a/tests/test_regression.py b/tests/test_regression.py index a4e7113..b061549 100644 --- a/tests/test_regression.py +++ b/tests/test_regression.py @@ -1,15 +1,15 @@ import pytest from base_tester import BasePytestTester -from pylint.checkers.variables import VariablesChecker from pylint_pytest.checkers.fixture import FixtureChecker +from pylint_pytest.checkers.variables import CustomVariablesChecker class TestRegression(BasePytestTester): """Covering some behaviors that shouldn't get impacted by the plugin""" CHECKER_CLASS = FixtureChecker - IMPACTED_CHECKER_CLASSES = [VariablesChecker] + IMPACTED_CHECKER_CLASSES = [CustomVariablesChecker] MSG_ID = "regression" @pytest.mark.parametrize("enable_plugin", [True, False]) diff --git a/tests/test_unused_argument.py b/tests/test_unused_argument.py index a164672..6c79a7c 100644 --- a/tests/test_unused_argument.py +++ b/tests/test_unused_argument.py @@ -1,13 +1,13 @@ import pytest from base_tester import BasePytestTester -from pylint.checkers.variables import VariablesChecker from pylint_pytest.checkers.fixture import FixtureChecker +from pylint_pytest.checkers.variables import CustomVariablesChecker class TestUnusedArgument(BasePytestTester): CHECKER_CLASS = FixtureChecker - IMPACTED_CHECKER_CLASSES = [VariablesChecker] + IMPACTED_CHECKER_CLASSES = [CustomVariablesChecker] MSG_ID = "unused-argument" @pytest.mark.parametrize("enable_plugin", [True, False]) diff --git a/tests/test_unused_import.py b/tests/test_unused_import.py index 1f2bd4b..b522856 100644 --- a/tests/test_unused_import.py +++ b/tests/test_unused_import.py @@ -1,13 +1,13 @@ import pytest from base_tester import BasePytestTester -from pylint.checkers.variables import VariablesChecker from pylint_pytest.checkers.fixture import FixtureChecker +from pylint_pytest.checkers.variables import CustomVariablesChecker class TestUnusedImport(BasePytestTester): CHECKER_CLASS = FixtureChecker - IMPACTED_CHECKER_CLASSES = [VariablesChecker] + IMPACTED_CHECKER_CLASSES = [CustomVariablesChecker] MSG_ID = "unused-import" @pytest.mark.parametrize("enable_plugin", [True, False]) @@ -40,3 +40,15 @@ def test_conftest(self, enable_plugin): for pytest to do its magic""" self.run_linter(enable_plugin) self.verify_messages(0 if enable_plugin else 1) + + @pytest.mark.parametrize("enable_plugin", [True, False]) + def test_module(self, enable_plugin): + """an unused module import shall still be an error""" + self.run_linter(enable_plugin) + self.verify_messages(1) + + @pytest.mark.parametrize("enable_plugin", [True, False]) + def test_mark_usesfixtures(self, enable_plugin): + """an unused module import shall still be an error""" + self.run_linter(enable_plugin) + self.verify_messages(0 if enable_plugin else 1)