From 2066591787aad775bf7aa684c789fab349006eb2 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Sun, 25 Aug 2024 22:16:45 +0300 Subject: [PATCH] `no-member`: Support tracking/ignoring `request.cls.X` / `self.X` It seems we already support the (weird) scenario that someone does `clls = request.cls`, and subsequently uses `clls.X = Y` However, it appears that we are missing the trivial `request.cls.X = Y` case Fixes https://github.com/pylint-dev/pylint-pytest/issues/74 Additionally: * Factorize condititons for semantic naming * Make some "not-docstrings" comments, and fix some grammar issues. In order to "properly" analyze fixtures that might change the class itself (which it can usually be out of order typed), we need to walk the `fixture` `FunctionDef`s in advance, in their topological order, per-scope. For that, we need to hand-roll out own ASTWalker / ASTVisitor (pylint's is a bit tight to their `checker` implementation). This enables us to (almost) correctly allow pylint to introspect `used-before-assignment` issues (see `tests/input/no-member/not_using_cls.py` changes) For project internals: * Split `pylint_pytest/utils.py` to modules Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- .pre-commit-config.yaml | 2 + pylint_pytest/checkers/__init__.py | 2 +- pylint_pytest/checkers/class_attr_loader.py | 268 +++++++++++++++--- pylint_pytest/checkers/fixture.py | 4 +- pylint_pytest/checkers/types.py | 7 - pylint_pytest/checkers/variables.py | 3 +- pylint_pytest/utils/ast_walker.py | 94 ++++++ .../{utils.py => utils/pytest_logic.py} | 132 +++++++-- pyproject.toml | 2 + requirements/dev.txt | 14 +- tests/input/no-member/issue_74.py | 25 ++ tests/input/no-member/issue_74_reverse.py | 26 ++ tests/input/no-member/not_using_cls.py | 25 +- tests/test_no_member.py | 15 +- 14 files changed, 537 insertions(+), 82 deletions(-) delete mode 100644 pylint_pytest/checkers/types.py create mode 100644 pylint_pytest/utils/ast_walker.py rename pylint_pytest/{utils.py => utils/pytest_logic.py} (52%) create mode 100644 tests/input/no-member/issue_74.py create mode 100644 tests/input/no-member/issue_74_reverse.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 59ce6c9..76ab985 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -79,7 +79,9 @@ repos: - id: mypy exclude: tests/input/ additional_dependencies: + - attrs - pylint + - types-networkx - repo: local hooks: - id: pylint diff --git a/pylint_pytest/checkers/__init__.py b/pylint_pytest/checkers/__init__.py index 19d1d2c..0bdb6cb 100644 --- a/pylint_pytest/checkers/__init__.py +++ b/pylint_pytest/checkers/__init__.py @@ -1,6 +1,6 @@ from pylint.checkers import BaseChecker -from pylint_pytest.utils import PYLINT_VERSION_MAJOR +from pylint_pytest.utils.pytest_logic import PYLINT_VERSION_MAJOR class BasePytestChecker(BaseChecker): diff --git a/pylint_pytest/checkers/class_attr_loader.py b/pylint_pytest/checkers/class_attr_loader.py index fdd8339..2de6ee3 100644 --- a/pylint_pytest/checkers/class_attr_loader.py +++ b/pylint_pytest/checkers/class_attr_loader.py @@ -1,56 +1,240 @@ from __future__ import annotations -from astroid import Assign, Attribute, ClassDef, Name +from typing import cast -from ..utils import _can_use_fixture, _is_class_autouse_fixture -from . import BasePytestChecker +from astroid import Assign, AssignAttr, Attribute, ClassDef, FunctionDef, Name, NodeNG + +from pylint_pytest.checkers import BasePytestChecker +from pylint_pytest.utils.ast_walker import ASTVisitor, ASTWalker +from pylint_pytest.utils.pytest_logic import ( + AssignPair, + FixtureProperties, + FixtureScopes, + FixturesStructure, + _get_fixture_kwarg, + _is_fixture_function, + _is_test_function, + fixtures_and_deps_in_topo_order, +) + +CLASS_CONVENTIONAL_NAME = "cls" +REQUEST_FIXTURE_NAME = "request" + + +def _is_request_cls(attr_node: NodeNG) -> bool: + """Is node exactly ``request.cls.*``?""" + + return ( + isinstance(attr_node, Attribute) + and attr_node.attrname == CLASS_CONVENTIONAL_NAME + and isinstance(attr_node.expr, Name) + and attr_node.expr.name == REQUEST_FIXTURE_NAME + ) class ClassAttrLoader(BasePytestChecker): + """ + This checker is responsible for pre-processing any class-defined fixtures, + and ensure that test classes' ``self`` see the attributes defined in the fixtures. + + The algorithm is as follows: + + #. When visiting a testing file, from top to bottom + #. When visiting a testing class + #. Collect all fixtures, their properties + #. Calculate the dependencies between fixtures + #. Evaluate auto fixtures in scope-order. + #. Use ``nx.DiGraph`` to topologically sort the fixtures. + #. Evaluate the fixtures in order + #. Use a thin ``FixtureVisitor`` to evaluate the fixtures. + #. Resume the class visitation + #. When visiting a fixture or a test function + #. Evaluate any reference-by-name fixture requested. + #. For fixtures: + #. Mark a fixture as AST-evaluated. + #. Apply any hacks, as-visited + #. When leaving, revert any hacks applied in this scope. + """ + msgs = {"E6400": ("", "pytest-class-attr-loader", "")} - in_setup = False - request_cls: set[str] = set() class_node: ClassDef | None = None + fixtures: FixturesStructure = {} + + curr_fixture: FunctionDef | None = None + request_cls_aliases: set[str] = set() + + ooo_applied_assigns: set[AssignPair] = set() + + def visit_classdef(self, node: ClassDef): + if not (node.name.lower().startswith("test") or "unittest.TestCase" in node.basenames): + return + + self.class_node = node + + for fn_node in node.nodes_of_class(FunctionDef): + if not _is_fixture_function(fn_node): + continue - def visit_functiondef(self, node): - """determine if a method is a class setup method""" - self.in_setup = False - self.request_cls = set() + self.fixtures[fn_node] = FixtureProperties( + name=repr(fn_node), + autouse=_get_fixture_kwarg(fn_node, "autouse", False), + scope=FixtureScopes.from_str( + _get_fixture_kwarg(fn_node, "scope", str(FixtureScopes.FUNCTION)) + ), + ) + + for fixture_fn, fixture_data in self.fixtures.items(): + for param in fixture_fn.args.args: + for other_fn, _ in self.fixtures.items(): + if param.name is other_fn.name: + fixture_data.deps.add(other_fn) + + self.evaluate_auto_fixtures() + + def leave_classdef(self, _node: ClassDef): + self.fixtures.clear() self.class_node = None - if _can_use_fixture(node) and _is_class_autouse_fixture(node): - self.in_setup = True - self.class_node = node.parent + def evaluate_auto_fixtures(self): + for scope in FixtureScopes: + filtered_fixtures = {f: d for f, d in self.fixtures.items() if d.scope == scope} + order = fixtures_and_deps_in_topo_order(filtered_fixtures) + + for fixture_fn in order: + if not self.fixtures[fixture_fn].autouse: + continue + + if self.fixtures[fixture_fn].pre_evaluated: + continue + + self.walk_fixture(fixture_fn) + + def walk_fixture(self, fixture_fn: FunctionDef): + walker = ASTWalker() + walker.add_visitor(FixtureVisitor(fixture_fn, self)) + walker.walk(fixture_fn) + self.fixtures[fixture_fn].pre_evaluated = True + self.curr_fixture = None + self.request_cls_aliases.clear() + + def __add_to_locals(self, dest_node: AssignAttr, value_node: Assign): + self.class_node = cast(ClassDef, self.class_node) # XXX: Cannot fight anymore + self.class_node.locals[dest_node.attrname] = [value_node.value] + + def visit_functiondef(self, fn_node: FunctionDef): + if not self.class_node or (not _is_test_function(fn_node) and fn_node not in self.fixtures): + return + + self.__ooo_apply_fixtures(fn_node) + if _is_test_function(fn_node): + return + + if not self.fixtures[fn_node].pre_evaluated: + self.walk_fixture(fn_node) + + self.curr_fixture = fn_node + self.fixtures[fn_node].ast_evaluating = True + + def __ooo_apply_fixtures(self, fn_node: FunctionDef): + """ + Apply all relevant fixtures to the current function's class' namespace. + """ + sort_fixtures: FixturesStructure = {} + + # Detect any referenced fixtures in function arguments + for arg in fn_node.args.args: + for fixture_fn, fixture_data in self.fixtures.items(): + if arg.name != fixture_fn.name: + continue + + sort_fixtures[fixture_fn] = fixture_data + + # Add all autouse fixtures + sort_fixtures.update( + {f: d for f, d in self.fixtures.items() if d.autouse and f is not fn_node} + ) + + sorted_fixtures = fixtures_and_deps_in_topo_order(sort_fixtures) + + for fixture_fn in sorted_fixtures: + fixture_data = self.fixtures[fixture_fn] + + if not fixture_data.pre_evaluated: + # Not pre-evaluated, so that means + # we have no ``fixture_data.assign_attrs``. + self.walk_fixture(fixture_fn) + + for pair in fixture_data.assign_attrs: + self.__add_to_locals(pair.attr, pair.assign) + self.ooo_applied_assigns.add(pair) + + def leave_functiondef(self, fn_node: FunctionDef): + if not self.class_node: + return + + for pair in self.ooo_applied_assigns: + del self.class_node.locals[pair.attr.attrname] + + self.ooo_applied_assigns.clear() + + if fn_node in self.fixtures: + self.curr_fixture = None + self.request_cls_aliases.clear() def visit_assign(self, node: Assign): - """store the aliases for `cls`""" - if ( - self.in_setup - 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 - self.request_cls = set(t.name for t in node.targets) - - def visit_assignattr(self, node): - if ( - self.in_setup - 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, Assign): - assign_node = assign_node.parent - - # hack class locals - self.class_node.locals[node.attrname] = [assign_node.value] - except Exception: # pylint: disable=broad-except - # cannot find valid assign expr, skipping the entire attribute - pass + if not self.curr_fixture or not _is_request_cls(node.value): + return + + self.request_cls_aliases = set(t.name for t in node.targets) + + def visit_assignattr(self, node: AssignAttr): + # "Intra"-apply any `request.cls.X` assign "also" to `self.X`. + # "External" apply is handled in ``self.__ooo_apply_fixtures``. + self.class_node = cast(ClassDef, self.class_node) # XXX: Cannot fight anymore + if not self.eligible_for_assignattr_visit(node) or node.attrname in self.class_node.locals: + return + + # We did all the heavy lifting in the FixtureVisitor; now, to hack: + for pair in self.fixtures[self.curr_fixture].assign_attrs: + if pair.attr is node: + self.__add_to_locals(pair.attr, pair.assign) + return + + def eligible_for_assignattr_visit(self, node: AssignAttr) -> bool: + return ( + self.class_node is not None + and self.curr_fixture is not None + and (self.is_node_request_cls_alias(node) or _is_request_cls(node.expr)) + ) + + def is_node_request_cls_alias(self, node: AssignAttr) -> bool: + """Is node a ``var``, where ``var = request.cls``?""" + return isinstance(node.expr, Name) and node.expr.name in self.request_cls_aliases + + +class FixtureVisitor(ASTVisitor): + def __init__(self, fixture: FunctionDef, checker: ClassAttrLoader): + self.checker = checker + self.checker.curr_fixture = fixture + + def visit_assign(self, node: Assign): + self.checker.visit_assign(node) + + def visit_assignattr(self, node: AssignAttr): + if not self.checker.eligible_for_assignattr_visit(node): + return + + try: + # Find the ``Assign`` node, which contains the source "value" + assign_node = node + while not isinstance(assign_node, Assign): + assign_node = assign_node.parent + + # When the time comes, we will hack the class locals + self.checker.fixtures[self.checker.curr_fixture].assign_attrs.add( + AssignPair(node, assign_node) + ) + except Exception: # pylint: disable=broad-except + # Cannot find a valid assign expr, skipping the entire attribute + pass diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 791caca..ff4ae4a 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -8,14 +8,14 @@ import astroid import pytest -from ..utils import ( +from ..utils.pytest_logic import ( + FixtureDict, _can_use_fixture, _is_pytest_fixture, _is_pytest_mark, _is_pytest_mark_usefixtures, ) from . import BasePytestChecker -from .types import FixtureDict # TODO: support pytest python_files configuration FILE_NAME_PATTERNS: tuple[str, ...] = ("test_*.py", "*_test.py") diff --git a/pylint_pytest/checkers/types.py b/pylint_pytest/checkers/types.py deleted file mode 100644 index 2f2b5bc..0000000 --- a/pylint_pytest/checkers/types.py +++ /dev/null @@ -1,7 +0,0 @@ -from __future__ import annotations - -from typing import Any, Dict, List - -from _pytest.fixtures import FixtureDef - -FixtureDict = Dict[str, List[FixtureDef[Any]]] diff --git a/pylint_pytest/checkers/variables.py b/pylint_pytest/checkers/variables.py index 8c3f6d6..bd3b951 100644 --- a/pylint_pytest/checkers/variables.py +++ b/pylint_pytest/checkers/variables.py @@ -6,8 +6,7 @@ from pylint.checkers.variables import VariablesChecker from pylint.interfaces import Confidence -from pylint_pytest.utils import _can_use_fixture, _is_same_module - +from ..utils.pytest_logic import _can_use_fixture, _is_same_module from .fixture import FixtureChecker diff --git a/pylint_pytest/utils/ast_walker.py b/pylint_pytest/utils/ast_walker.py new file mode 100644 index 0000000..6776556 --- /dev/null +++ b/pylint_pytest/utils/ast_walker.py @@ -0,0 +1,94 @@ +from __future__ import annotations + +import sys +import traceback +from collections import defaultdict +from collections.abc import Sequence +from typing import Literal, TypeVar + +from astroid import nodes +from pylint.utils.ast_walker import AstCallback + + +class ASTVisitor: # pylint: disable=too-few-public-methods + """ + Thin interface for AST visitors. + + Used only for type-hinting purposes, + and allow developers to navigate usages. + """ + + +ASTVisitorT = TypeVar("ASTVisitorT", bound=ASTVisitor) + + +class ASTWalker: + """Almost-blatant copy from pylint.utils.ast_walker.ASTWalker.""" + + VISIT_METH: Literal["visit_"] = "visit_" + LEAVE_METH: Literal["leave_"] = "leave_" + DEFAULT: Literal["default"] = "default" + VISIT_DEFAULT = VISIT_METH + DEFAULT + + def __init__(self) -> None: + self.visit_events: defaultdict[str, list[AstCallback]] = defaultdict(list) + self.leave_events: defaultdict[str, list[AstCallback]] = defaultdict(list) + self.exception_msg = False + + def add_visitor(self, visitor: ASTVisitorT) -> None: + """Walk to the visitor's dir and collect the visit and leave methods.""" + vcids: set[str] = set() + visits = self.visit_events + leaves = self.leave_events + for member in dir(visitor): + if (cid := member[6:]) == self.DEFAULT: + continue + + if member.startswith(self.VISIT_METH): + v_meth = getattr(visitor, member) + visits[cid].append(v_meth) + vcids.add(cid) + + elif member.startswith(self.LEAVE_METH): + l_meth = getattr(visitor, member) + leaves[cid].append(l_meth) + + if visit_default := getattr(visitor, self.VISIT_DEFAULT, None): + for cls in nodes.ALL_NODE_CLASSES: + if (cid := cls.__name__.lower()) not in vcids: + visits[cid].append(visit_default) + # For now, we have no "leave_default" method in Pylint + + def walk(self, astroid: nodes.NodeNG) -> None: + """ + Call visit events of astroid checkers for the given node, + recurse on its children, then leave events. + """ + cid = astroid.__class__.__name__.lower() + + # Detect if the node is a new name for a deprecated alias. + # In this case, favour the methods for the deprecated + # alias if any, in order to maintain backwards + # compatibility. + visit_events: Sequence[AstCallback] = self.visit_events.get(cid, ()) + leave_events: Sequence[AstCallback] = self.leave_events.get(cid, ()) + + try: + # generate events for this node on each checker + for callback in visit_events: + callback(astroid) + # recurse on children + for child in astroid.get_children(): + self.walk(child) + for callback in leave_events: + callback(astroid) + except Exception: + if self.exception_msg is False: + file = getattr(astroid.root(), "file", None) + print( + f"Exception on node {astroid!r} in file '{file}'", + file=sys.stderr, + ) + traceback.print_exc() + self.exception_msg = True + raise diff --git a/pylint_pytest/utils.py b/pylint_pytest/utils/pytest_logic.py similarity index 52% rename from pylint_pytest/utils.py rename to pylint_pytest/utils/pytest_logic.py index b044569..0d744dd 100644 --- a/pylint_pytest/utils.py +++ b/pylint_pytest/utils/pytest_logic.py @@ -1,11 +1,71 @@ +from __future__ import annotations + import inspect +from collections.abc import Iterator +from enum import IntEnum +from typing import Any, Dict, List, NamedTuple, TypeVar import astroid +import attrs +import networkx as nx import pylint +from _pytest.fixtures import FixtureDef PYLINT_VERSION_MAJOR = int(pylint.__version__.split(".")[0]) +class AssignPair(NamedTuple): + """A pair of an AssignAttr node and its Assign.""" + + attr: astroid.AssignAttr + assign: astroid.Assign + + +class FixtureScopes(IntEnum): + """pytest fixture scopes.""" + + SESSION = 1 + PACKAGE = 2 + MODULE = 3 + CLASS = 4 + FUNCTION = 5 + + @classmethod + def from_str(cls, scope: str) -> FixtureScopes: + return cls[scope.upper()] + + def __str__(self) -> str: + return self.name.lower() + + def __repr__(self) -> str: + return f"{self.__class__.__name__}.{self.name}" + + +@attrs.define +class FixtureProperties: + """ + Properties of a pytest fixture. + + Some of them are our metadata, some are the actual fixture's properties. + """ + + autouse: bool = False + scope: FixtureScopes = FixtureScopes.FUNCTION + deps: set[astroid.FunctionDef] = attrs.field(factory=set) + assign_attrs: set[AssignPair] = attrs.field(factory=set) + pre_evaluated: bool = False + """fixture has been evaluated via ``evaluate_auto_fixtures``.""" + ast_evaluating: bool = False + """fixture has started evaluation via pylint's AST walk.""" + name: str = "" + """For debugging-complete purposes only.""" + + +FixtureDict = Dict[str, List[FixtureDef[Any]]] +FixturesStructure = Dict[astroid.FunctionDef, FixtureProperties] +DefaultT = TypeVar("DefaultT") + + def _is_pytest_mark_usefixtures(decorator): # expecting @pytest.mark.usefixture(...) try: @@ -80,37 +140,34 @@ def _check_name(name_): return False -def _is_class_autouse_fixture(function): +def _is_fixture_function(node: astroid.FunctionDef) -> bool: + """Is the function a pytest fixture?""" + if node.decorators is None: + return False + + return any(_is_pytest_fixture(decorator) for decorator in node.decorators.nodes) + + +def _get_fixture_kwarg( + fn: astroid.FunctionDef, kwarg: str, default: DefaultT | None = None +) -> Any | DefaultT: try: - for decorator in function.decorators.nodes: - if isinstance(decorator, astroid.Call): - func = decorator.func - - if ( - func - and func.attrname in ("fixture", "yield_fixture") - and func.expr.name == "pytest" - ): - is_class = is_autouse = False - - for kwarg in decorator.keywords or []: - if kwarg.arg == "scope" and kwarg.value.value == "class": - is_class = True - if kwarg.arg == "autouse" and kwarg.value.value is True: - is_autouse = True - - if is_class and is_autouse: - return True + for decorator in fn.decorators.nodes: + if not isinstance(decorator, astroid.Call): + continue + + for keyword in decorator.keywords or []: + if keyword.arg == kwarg: + return keyword.value.value except AttributeError: pass - return False + return default def _can_use_fixture(function): if isinstance(function, astroid.FunctionDef): - # test_*, *_test - if function.name.startswith("test_") or function.name.endswith("_test"): + if _is_test_function(function): return True if function.decorators: @@ -126,6 +183,13 @@ def _can_use_fixture(function): return False +def _is_test_function(fn: astroid.FunctionDef): + """If function name is like ``test_*`` or ``*_test``.""" + # ToDo: Read pytest configuration? + # https://doc.pytest.org/en/latest/reference/reference.html#confval-python_functions + return fn.name.startswith("test_") or fn.name.endswith("_test") + + def _is_same_module(fixtures, import_node, fixture_name): """Comparing pytest fixture node with astroid.ImportFrom""" try: @@ -140,3 +204,25 @@ def _is_same_module(fixtures, import_node, fixture_name): except Exception: # pylint: disable=broad-except pass return False + + +def fixtures_and_deps_in_topo_order( + fixtures: FixturesStructure, +) -> Iterator[astroid.FunctionDef]: + """ + Topologically sort fixtures and their dependencies. + + The function orders only the given fixtures. + As such, if the user wants to order by scope, + multiple "disjoint" fixtures, or any other criteria, + make sure the input fixtures are already filtered accordingly. + """ + dag: nx.DiGraph[astroid.FunctionDef] = nx.DiGraph() + + for fixture_fn, fixture_data in fixtures.items(): + dag.add_node(fixture_fn) + + for dep_fn in fixture_data.deps: + dag.add_edge(dep_fn, fixture_fn) + + return nx.topological_sort(dag) diff --git a/pyproject.toml b/pyproject.toml index 401ddec..1fa4924 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,6 +43,8 @@ requires-python = ">=3.8" dependencies = [ "pylint>=2,<4", "pytest>=4.6", + "attrs>=23.2.0", + "networkx>=3.3.0", ] # [project.optional-dependencies] moved to requirements/dev.in diff --git a/requirements/dev.txt b/requirements/dev.txt index e832d31..babdfc8 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -12,6 +12,10 @@ asttokens==2.4.1 \ --hash=sha256:051ed49c3dcae8913ea7cd08e46a606dba30b79993209636c4875bc1d637bc24 \ --hash=sha256:b03869718ba9a6eb027e134bfdf69f38a236d681c83c160d510768af11254ba0 # via stack-data +attrs==24.2.0 \ + --hash=sha256:5cfb1b9148b5b086569baec03f20d7b6bf3bcacc9a42bebf87ffaaca362f6346 \ + --hash=sha256:81921eb96de3191c8258c199618104dd27ac608d9366f5e35d011eae1867ede2 + # via pylint-pytest (pyproject.toml) black==24.4.2 \ --hash=sha256:257d724c2c9b1660f353b36c802ccece186a30accc7742c176d29c146df6e474 \ --hash=sha256:37aae07b029fa0174d39daf02748b379399b909652a806e5708199bd93899da1 \ @@ -203,6 +207,10 @@ mypy-extensions==1.0.0 \ # via # black # mypy +networkx==3.3 \ + --hash=sha256:0c127d8b2f4865f59ae9cb8aafcd60b5c70f3241ebd66f7defad7c4ab90126c9 \ + --hash=sha256:28575580c6ebdaf4505b22c6256a2b9de86b316dc63ba9e93abde3d78dfdbcf2 + # via pylint-pytest (pyproject.toml) nodeenv==1.9.1 \ --hash=sha256:6ec12890a2dab7946721edbfbcd91f3319c6ccc9aec47be7c7e6b7011ee6645f \ --hash=sha256:ba11c9782d29c27c70ffbdda2d7415098754709be8a7056d79a737cd901155c9 @@ -438,9 +446,9 @@ traitlets==5.14.3 \ # via # ipython # matplotlib-inline -types-networkx==3.2.1.20240703 \ - --hash=sha256:b8c44440f6fc2f06ca844bf2f3dfbce3e07505c4e93d2658d31489d433b524e1 \ - --hash=sha256:f24d0ba3de0664f182aec66bccf361b673a7a906a5b08e6d2b3a7fafff9fe7b4 +types-networkx==3.2.1.20240820 \ + --hash=sha256:66fa71b43dc8601dcd75f6c696a78cde434b4afe18a062d6e2dfc4fa1c0f2ce0 \ + --hash=sha256:820b6cd0abd0721aa1bdf3fe98cb1a328d914b949ce72d3b61ca083a94393176 # via -r requirements/dev.in typing-extensions==4.12.2 \ --hash=sha256:04e5ca0351e0f3f85c6853954072df659d0d13fac324d0072316b67d7794700d \ diff --git a/tests/input/no-member/issue_74.py b/tests/input/no-member/issue_74.py new file mode 100644 index 0000000..7894132 --- /dev/null +++ b/tests/input/no-member/issue_74.py @@ -0,0 +1,25 @@ +"""Example https://github.com/pylint-dev/pylint-pytest/issues/74#issuecomment-2219850110""" + +import pytest + + +class TestExample: + @pytest.fixture(scope="class") + def setup_teardown_username(self, request: pytest.FixtureRequest): + print(self.username) + request.cls.username = "dylan" + yield True + print(self.username) + del request.cls.username + print(self.username) + + @pytest.fixture(scope="function") + def setup_teardown_password(self, request: pytest.FixtureRequest, setup_teardown_username): + assert self.username + request.cls.password = "*****" + yield setup_teardown_username, True + del request.cls.password + + def test_username_and_password(self, setup_teardown_password): + assert self.username == "dylan" + assert self.password == "*****" diff --git a/tests/input/no-member/issue_74_reverse.py b/tests/input/no-member/issue_74_reverse.py new file mode 100644 index 0000000..363c4b6 --- /dev/null +++ b/tests/input/no-member/issue_74_reverse.py @@ -0,0 +1,26 @@ +"""Example https://github.com/pylint-dev/pylint-pytest/issues/74#issuecomment-2219850110""" + +import pytest + + +class TestExample: + + def test_username_and_password(self, setup_teardown_password): + assert self.username == "dylan" + assert self.password == "*****" + + @pytest.fixture(scope="function") + def setup_teardown_password(self, request: pytest.FixtureRequest, setup_teardown_username): + assert self.username + request.cls.password = "*****" + yield setup_teardown_username, True + del request.cls.password + + @pytest.fixture(scope="class") + def setup_teardown_username(self, request: pytest.FixtureRequest): + print(self.username) + request.cls.username = "dylan" + print(self.username) + yield True + del request.cls.username + print(self.username) diff --git a/tests/input/no-member/not_using_cls.py b/tests/input/no-member/not_using_cls.py index 95438af..4c44fd1 100644 --- a/tests/input/no-member/not_using_cls.py +++ b/tests/input/no-member/not_using_cls.py @@ -4,9 +4,32 @@ class TestClass: @staticmethod @pytest.fixture(scope="class", autouse=True) - def setup_class(request): + def setup_class_orig(request): clls = request.cls clls.defined_in_setup_class = 123 + @pytest.fixture(scope="class", autouse=True) + def setup_class(self, request: pytest.FixtureRequest): + # vvvvvvvvvv Cannot access ``self.klass`` before it is defined! + print(self.klass) + request.cls.klass = "class" + print(self.klass) + + @pytest.fixture(scope="function", autouse=True) + def setup_function(self, request: pytest.FixtureRequest): + request.cls.function = "function" + print(self.function) + def test_foo(self): assert self.defined_in_setup_class + assert self.temp1 + assert self.klass == "class" + assert self.function == "function" + + # Ordering MATTERS! astroid traverses the file from top to bottom. + # We should be able to cover that case as well, as nothing enforces a specific order. + @pytest.fixture(autouse=True) + def setup_teardown_environment(self, request: pytest.FixtureRequest): + print(request.cls.temp1) # ToDo: Cannot access ``request.cls.temp1`` before it is defined! + request.cls.temp1 = 54321 + print(self.temp1) diff --git a/tests/test_no_member.py b/tests/test_no_member.py index 15e652e..580cedd 100644 --- a/tests/test_no_member.py +++ b/tests/test_no_member.py @@ -24,13 +24,26 @@ def test_yield_fixture(self, enable_plugin): @pytest.mark.parametrize("enable_plugin", [True, False]) def test_not_using_cls(self, enable_plugin): self.run_linter(enable_plugin) - self.verify_messages(0 if enable_plugin else 1) + # l.13: ``print(self.klass)`` before ``request.cls.klass = "class"``. + self.verify_messages(1 + (0 if enable_plugin else 7)) @pytest.mark.parametrize("enable_plugin", [True, False]) def test_inheritance(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(0 if enable_plugin else 1) + @pytest.mark.parametrize("enable_plugin", [True, False]) + def test_issue_74(self, enable_plugin): + self.run_linter(enable_plugin) + # l.9: ``print(self.username)`` before ``request.cls.username = "dylan"`` + self.verify_messages(1 + (0 if enable_plugin else 5)) + + @pytest.mark.parametrize("enable_plugin", [True, False]) + def test_issue_74_reverse(self, enable_plugin): + self.run_linter(enable_plugin) + # l.9: ``print(self.username)`` before ``request.cls.username = "dylan"`` + self.verify_messages(1 + (0 if enable_plugin else 5)) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_from_unpack(self, enable_plugin): self.run_linter(enable_plugin)