Merge pull request pytest-dev#14567 from pytest-dev/more-visibility-deprecate

fixtures: deprecate `FixtureDef.has_location` and `None` global fixture visibility

Author: bluetech

changelog/14004.deprecation.rst

@@ -2,5 +2,6 @@ Passing ``baseid`` to :class:`~pytest.FixtureDef` or ``nodeid`` strings to fixtu
 
 Use the ``node`` parameter instead for fixture scoping. This enables more robust node-based
 matching instead of string prefix matching.
+If you've used ``nodeid=None``, pass ``node=session`` instead.
 
 This will be removed in pytest 10.

changelog/14513.deprecation.rst

@@ -0,0 +1,2 @@
+The private ``FixtureDef.has_location`` attribute is now deprecated and will be removed in pytest 10.
+See :ref:`fixturedef-has-location-deprecated` for details.

doc/en/deprecations.rst

@@ -20,7 +20,7 @@ Below is a complete list of all pytest features which are considered deprecated.
 Passing ``baseid``/``nodeid`` strings to fixture registration APIs
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-.. deprecated:: 9.2
+.. deprecated:: 9.1
 
 Passing ``baseid`` to :class:`~pytest.FixtureDef` or ``nodeid`` strings to
 ``FixtureManager._register_fixture`` and ``FixtureManager.parsefactories``
@@ -39,9 +39,25 @@ node-based matching instead of fragile string prefix matching.
     fixture_manager.parsefactories(holder=plugin_obj, node=directory_node)
     fixture_manager._register_fixture(name="fix", func=func, node=directory_node)
 
+The equivalent of passing ``nodeid=None`` (global visibility) is ``node=session``.
+
 In pytest 10, the ``baseid`` and ``nodeid`` string parameters will be removed.
 
 
+.. _fixturedef-has-location-deprecated:
+
+``FixtureDef.has_location``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. deprecated:: 9.1
+
+The private ``FixtureDef.has_location`` attribute is deprecated and will be removed in pytest 10.
+
+It indicated whether a fixture was found from a node or a conftest in the collection tree (as opposed to a non-conftest plugin).
+It was used to determine the override order of fixtures, pushing fixtures with "no location" to the front of the override chain (such that they are chosen last).
+The override order is now determined by the visibility of the fixtures in the collection tree, making this distinction obsolete.
+
+
 .. _console-main:
 
 ``pytest.console_main()``

src/_pytest/deprecated.py

@@ -123,6 +123,11 @@
     "Pass node instead for fixture scoping."
 )
 
+FIXTUREDEF_HAS_LOCATION_DEPRECATED = PytestRemovedIn10Warning(
+    "FixtureDef.has_location is deprecated and will be removed in pytest 10. "
+    "See https://docs.pytest.org/en/stable/deprecations.html#fixturedef-has-location-deprecated"
+)
+
 PARSEFACTORIES_NODEID_DEPRECATED = PytestRemovedIn10Warning(
     "Passing nodeid string to parsefactories is deprecated. "
     "Use parsefactories(holder=obj, node=node) instead."

src/_pytest/fixtures.py

@@ -32,7 +32,6 @@
 from typing import TypeVar
 import warnings
 
-from .compat import deprecated
 import _pytest
 from _pytest import nodes
 from _pytest._code import getfslineno
@@ -41,6 +40,7 @@
 from _pytest._code.code import TerminalRepr
 from _pytest._io import TerminalWriter
 from _pytest.compat import assert_never
+from _pytest.compat import deprecated
 from _pytest.compat import get_real_func
 from _pytest.compat import getfuncargnames
 from _pytest.compat import getimfunc
@@ -60,6 +60,7 @@
 from _pytest.deprecated import FIXTURE_BASEID_DEPRECATED
 from _pytest.deprecated import FIXTURE_GETFIXTUREVALUE_DURING_TEARDOWN
 from _pytest.deprecated import FIXTURE_NODEID_DEPRECATED
+from _pytest.deprecated import FIXTUREDEF_HAS_LOCATION_DEPRECATED
 from _pytest.deprecated import PARSEFACTORIES_NODEID_DEPRECATED
 from _pytest.deprecated import YIELD_FIXTURE
 from _pytest.main import Session
@@ -140,9 +141,8 @@ def is_visibility_more_specific(
     than that of ``other``, i.e. ``candidate`` is defined on a strict descendant
     in the collection tree of where ``other`` is defined."""
     if candidate.node is None or other.node is None:
-        # Fallback for fixtures registered with a string nodeid (deprecated) or
-        # with global visibility (no node). In this case compare baseids, which
-        # are nodeid prefixes.
+        # Fallback for fixtures registered with a string nodeid (deprecated).
+        # In this case compare baseids, which are nodeid prefixes.
         # This branch can be removed once baseid deprecation is done (pytest 10).
         if candidate.baseid == other.baseid:
             return False
@@ -1056,26 +1056,27 @@ class FixtureDef(Generic[FixtureValue]):
     def __init__(
         self,
         config: Config,
-        baseid: str | None,
+        baseid: str | None | NotSetType,
         argname: str,
         func: _FixtureFunc[FixtureValue],
         scope: Scope | ScopeName | Callable[[str, Config], ScopeName] | None,
         params: Sequence[object] | None,
         ids: tuple[object | None, ...] | Callable[[Any], object | None] | None = None,
         *,
-        _ispytest: bool = False,
+        node: nodes.Node | NotSetType = NOTSET,
         # only used in a deprecationwarning msg, can be removed in pytest9
         _autouse: bool = False,
-        node: nodes.Node | None = None,
+        _ispytest: bool = False,
     ) -> None:
         check_ispytest(_ispytest)
-        # Emit deprecation warning if baseid string is used when node could be provided.
-        # baseid=None (global plugins) and baseid="" (synthetic fixtures) are fine.
-        if baseid and node is None:
+        # Emit deprecation warning if deprecated baseid string is used.
+        if node is NOTSET:
             warnings.warn(FIXTURE_BASEID_DEPRECATED, stacklevel=2)
+        if baseid is NOTSET:
+            baseid = None
         # The node where this fixture was defined, if available.
         # Used for node-based matching which is more robust than string matching.
-        self.node: Final = node
+        self.node: Final = node if node is not NOTSET else None
         # The "base" node ID for the fixture.
         #
         # This is a node ID prefix. A fixture is only available to a node (e.g.
@@ -1090,11 +1091,15 @@ def __init__(
         #
         # For other plugins, the baseid is the empty string (always matches).
         # When node is available, baseid is derived from node.nodeid.
-        self.baseid: Final = node.nodeid if node is not None else (baseid or "")
+        #
+        # Deprecated: replaced by ``node``.
+        self.baseid: Final = node.nodeid if node is not NOTSET else (baseid or "")
         # Whether the fixture was found from a node or a conftest in the
         # collection tree. Will be false for fixtures defined in non-conftest
         # plugins.
-        self.has_location: Final = node is not None or baseid is not None
+        #
+        # Deprecated: kept only to back the deprecated ``has_location`` property.
+        self._has_location: Final = node is not NOTSET or baseid is not None
         # The fixture factory function.
         self.func: Final = func
         # The name by which the fixture may be requested.
@@ -1129,6 +1134,11 @@ def scope(self) -> ScopeName:
         """Scope string, one of "function", "class", "module", "package", "session"."""
         return self._scope.value
 
+    @property
+    def has_location(self) -> bool:
+        warnings.warn(FIXTUREDEF_HAS_LOCATION_DEPRECATED, stacklevel=2)
+        return self._has_location
+
     def addfinalizer(self, finalizer: Callable[[], object]) -> None:
         self._finalizers.append(finalizer)
 
@@ -1243,11 +1253,12 @@ class RequestFixtureDef(FixtureDef[FixtureRequest]):
     def __init__(self, request: FixtureRequest) -> None:
         super().__init__(
             config=request.config,
-            baseid=None,
+            baseid=NOTSET,
             argname="request",
             func=lambda: request,
             scope=Scope.Function,
             params=None,
+            node=request.node,
             _ispytest=True,
         )
         self.cached_result = (request, [0], None)
@@ -1776,8 +1787,8 @@ def pytest_plugin_registered(self, plugin: _PluggyPlugin, plugin_name: str) -> N
             # Store conftest for deferred parsing when its Directory is collected.
             self._pending_conftests[conftest_dir] = plugin
         else:
-            # Non-conftest plugins have global visibility (nodeid=None).
-            self.parsefactories(plugin, None)
+            # Non-conftest plugins have global visibility.
+            self.parsefactories(holder=plugin, node=self.session)
 
     @hookimpl(wrapper=True)
     def pytest_make_collect_report(
@@ -1926,12 +1937,12 @@ def _register_fixture(
         *,
         name: str,
         func: _FixtureFunc[object],
-        nodeid: str | None = None,
+        nodeid: str | None | NotSetType = NOTSET,
         scope: Scope | ScopeName | Callable[[str, Config], ScopeName] = "function",
         params: Sequence[object] | None = None,
         ids: tuple[object | None, ...] | Callable[[Any], object | None] | None = None,
         autouse: bool = False,
-        node: nodes.Node | None = None,
+        node: nodes.Node | NotSetType = NOTSET,
     ) -> None:
         """Register a fixture
 
@@ -1955,13 +1966,12 @@ def _register_fixture(
         :param autouse:
             Whether this is an autouse fixture.
         """
-        # Emit deprecation warning if nodeid string is used when node could be provided.
-        # nodeid=None (global plugins) is fine.
-        if nodeid and node is None:
+        # Emit deprecation warning if nodeid string.
+        if nodeid is not NOTSET or node is NOTSET:
             warnings.warn(FIXTURE_NODEID_DEPRECATED, stacklevel=2)
         fixture_def = FixtureDef(
             config=self.config,
-            baseid=nodeid if node is None else None,
+            baseid=nodeid,
             argname=name,
             func=func,
             scope=scope,
@@ -1991,9 +2001,9 @@ def _register_fixture(
         else:
             faclist.append(fixture_def)
         if autouse:
-            if node is not None:
+            if node is not NOTSET:
                 self._node_autousenames.setdefault(node, []).append(name)
-            elif nodeid:
+            elif nodeid is not NOTSET and nodeid is not None:
                 # Legacy: plugin passed nodeid string without node reference.
                 self._nodeid_autousenames.setdefault(nodeid, []).append(name)
             else:
@@ -2008,6 +2018,9 @@ def parsefactories(
         raise NotImplementedError()
 
     @overload
+    @deprecated(
+        "parsefactories(obj, nodeid) is deprecated, use parsefactories(holder=obj, node=node) instead"
+    )
     def parsefactories(
         self,
         node_or_obj: object,
@@ -2018,8 +2031,8 @@ def parsefactories(
     @overload
     def parsefactories(
         self,
-        node_or_obj: None = ...,
-        nodeid: None = ...,
+        node_or_obj: NotSetType = ...,
+        nodeid: NotSetType = ...,
         *,
         holder: object,
         node: nodes.Node,
@@ -2028,44 +2041,42 @@ def parsefactories(
 
     def parsefactories(
         self,
-        node_or_obj: nodes.Node | object | None = None,
-        nodeid: str | NotSetType | None = NOTSET,
+        node_or_obj: nodes.Node | object | NotSetType = NOTSET,
+        nodeid: str | None | NotSetType = NOTSET,
         *,
-        holder: object | None = None,
-        node: nodes.Node | None = None,
+        holder: object | NotSetType = NOTSET,
+        node: nodes.Node | NotSetType = NOTSET,
     ) -> None:
         """Collect fixtures from a collection node or object.
 
         Found fixtures are parsed into `FixtureDef`s and saved.
 
         The preferred API uses keyword-only arguments:
         - ``holder``: The object to scan for fixtures.
-        - ``node``: The node determining fixture visibility scope.
+        - ``node``: The node determining fixture visibility.
 
         Legacy positional API (translated internally):
         - ``parsefactories(node)``: Uses node.obj as holder, node for scope.
         - ``parsefactories(obj, nodeid)``: Uses obj as holder, nodeid string for scope.
         """
         # Translate legacy API to holder/node sources of truth
         # Either effective_node or effective_nodeid will be set, not both
-        effective_node: nodes.Node | None = None
-        effective_nodeid: str | None = None
+        effective_node: nodes.Node | NotSetType = NOTSET
+        effective_nodeid: str | None | NotSetType = NOTSET
 
-        if holder is not None:
+        if holder is not NOTSET:
             # New API: holder and node explicitly provided
             holderobj = holder
             effective_node = node
-        elif node_or_obj is None:
+        elif node_or_obj is NOTSET:
             raise TypeError("parsefactories() requires holder or node_or_obj")
         elif nodeid is not NOTSET:
-            # Legacy: parsefactories(obj, nodeid) - string-based scoping only
-            # Only warn if a non-None nodeid string is passed (None means global plugin)
-            if nodeid is not None:
-                warnings.warn(PARSEFACTORIES_NODEID_DEPRECATED, stacklevel=2)
+            # Legacy: parsefactories(obj, nodeid) - string-based scoping only.
+            warnings.warn(PARSEFACTORIES_NODEID_DEPRECATED, stacklevel=2)
             holderobj = node_or_obj
             effective_nodeid = nodeid
         else:
-            # Legacy: parsefactories(node) - node has .obj attribute
+            # parsefactories(node) - node has .obj attribute
             assert isinstance(node_or_obj, nodes.Node)
             holderobj = cast(object, node_or_obj.obj)  # type: ignore[attr-defined]
             effective_node = node_or_obj

src/_pytest/python.py

@@ -1164,15 +1164,16 @@ class DirectParamFixtureDef(FixtureDef[FixtureValue]):
     usually behaves like any other FixtureDef.
     """
 
-    def __init__(self, *, config: Config, argname: str, scope: Scope) -> None:
+    def __init__(self, *, node: nodes.Node, argname: str, scope: Scope) -> None:
         super().__init__(
-            config=config,
-            baseid="",
+            config=node.config,
+            baseid=NOTSET,
             argname=argname,
             func=get_direct_param_fixture_func,
             scope=scope,
             params=None,
             ids=None,
+            node=node,
             _ispytest=True,
         )
 
@@ -1395,7 +1396,7 @@ def parametrize(
                 fixturedef = name2directparamfixturedef[argname]
             else:
                 fixturedef = DirectParamFixtureDef(
-                    config=self.config,
+                    node=self.definition.session,
                     argname=argname,
                     scope=scope_,
                 )

testing/deprecated_test.py

@@ -174,7 +174,7 @@ def fix_b():
                     fm.parsefactories(mod_none, None)
 
                 nodeid_warns = [x for x in w if "parsefactories" in str(x.message)]
-                assert len(nodeid_warns) == 1, f"Expected 1 warning, got: {w}"
+                assert len(nodeid_warns) == 2, f"Expected 2 warning, got: {w}"
             """
         )
         pytester.makepyfile(
@@ -287,3 +287,26 @@ def test_scoped_invisible(request):
         )
         result = pytester.runpytest("-W", "ignore::pytest.PytestRemovedIn10Warning")
         result.assert_outcomes(passed=2)
+
+    def test_fixturedef_has_location_deprecated(self, pytester: Pytester) -> None:
+        """Accessing FixtureDef.has_location warns."""
+        pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture
+            def fix():
+                return 1
+
+            def test_it(request):
+                fixturedef = request._fixturemanager.getfixturedefs(
+                    "fix", request._pyfuncitem
+                )[0]
+                with pytest.warns(
+                    pytest.PytestRemovedIn10Warning, match="has_location"
+                ):
+                    assert fixturedef.has_location is True
+            """
+        )
+        result = pytester.runpytest()
+        result.assert_outcomes(passed=1)

testing/python/metafunc.py

@@ -10,6 +10,7 @@
 import textwrap
 from typing import Any
 from typing import cast
+from typing import ClassVar
 
 import hypothesis
 from hypothesis import strategies
@@ -44,7 +45,9 @@ class FixtureManagerMock:
 
         @dataclasses.dataclass
         class SessionMock:
+            config: Any
             _fixturemanager: FixtureManagerMock
+            nodeid: ClassVar = ""
 
         @dataclasses.dataclass
         class DefinitionMock(python.FunctionDefinition):
@@ -55,7 +58,7 @@ class DefinitionMock(python.FunctionDefinition):
         fixtureinfo: Any = FuncFixtureInfoMock(names)
         definition: Any = DefinitionMock._create(obj=func, _nodeid="mock::nodeid")
         definition._fixtureinfo = fixtureinfo
-        definition.session = SessionMock(FixtureManagerMock({}))
+        definition.session = SessionMock(config, FixtureManagerMock({}))
         return python.Metafunc(definition, fixtureinfo, config, _ispytest=True)
 
     def test_no_funcargs(self) -> None: