Skip to content

fix fixture checker #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 16 additions & 25 deletions pylint_pytest/__init__.py
Original file line number Diff line number Diff line change
@@ -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)
95 changes: 1 addition & 94 deletions pylint_pytest/checkers/fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
8 changes: 0 additions & 8 deletions pylint_pytest/checkers/types.py
Original file line number Diff line number Diff line change
@@ -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)
107 changes: 107 additions & 0 deletions pylint_pytest/checkers/variables.py
Original file line number Diff line number Diff line change
@@ -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)
8 changes: 8 additions & 0 deletions tests/input/unused-import/mark_usesfixtures.py
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions tests/input/unused-import/module.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import pytest


def test_no_using_module():
assert True
6 changes: 6 additions & 0 deletions tests/input/unused-import/other_fixture.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import pytest


@pytest.fixture
def other_fixture_not_in_conftest():
return True
29 changes: 29 additions & 0 deletions tests/test_pylint_integration.py
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions tests/test_redefined_outer_name.py
Original file line number Diff line number Diff line change
@@ -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])
Expand Down
4 changes: 2 additions & 2 deletions tests/test_regression.py
Original file line number Diff line number Diff line change
@@ -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])
Expand Down
4 changes: 2 additions & 2 deletions tests/test_unused_argument.py
Original file line number Diff line number Diff line change
@@ -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])
Expand Down
Loading