Merge pull request #29 from anis-campos/fix-checkers

stdedos · web-flow · commit 1ca1d86c97bd · 2024-01-25T22:50:06.000+02:00
See more details in #29
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
