fixtures: deprecate FixtureDef.has_location and None global fixture visibility

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.

changelog/14513.improvement.rst

@@ -0,0 +1,5 @@
+The order in which fixture definitions overriding each other are resolved is now determined first by their *visibility* in the collection tree rather than by the order in which they happened to be registered.
+
+A fixture defined for a more specific node (e.g. a module or an item) now always takes precedence over one with the same name defined for a more general node (e.g. the session), even when the more general one was registered later.
+Fixtures with non-comparable visibility keep the existing behavior of "last registered wins".
+This change is supposed to only affect plugins which register multiple fixtures programmatically with the same name.

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
@@ -133,6 +134,31 @@ def get_scope_package(
     return node.session
 
 
+def is_visibility_more_specific(
+    candidate: FixtureDef[Any], other: FixtureDef[Any]
+) -> bool:
+    """Return whether the visibility of ``candidate`` is strictly 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).
+        # 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
+        if other.baseid == "":
+            return True
+        # `candidate.baseid` must continue with a node separator for it to be a
+        # true descendant.
+        return candidate.baseid.startswith(other.baseid) and candidate.baseid[
+            len(other.baseid)
+        ] in ("/", ":")
+
+    return (
+        candidate.node is not other.node and other.node in candidate.node.iter_parents()
+    )
+
+
 def get_scope_node(node: nodes.Node, scope: Scope) -> nodes.Node | None:
     """Get the closest parent node (including self) which matches the given
     scope.
@@ -1030,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.
@@ -1064,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.
@@ -1103,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)
 
@@ -1217,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)
@@ -1751,8 +1788,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(
@@ -1901,12 +1938,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
 
@@ -1930,13 +1967,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,
@@ -1948,19 +1984,27 @@ def _register_fixture(
         )
 
         faclist = self._arg2fixturedefs.setdefault(name, [])
-        if fixture_def.has_location:
-            faclist.append(fixture_def)
+        # Insert the fixturedef into the list while maintaining a partial order
+        # based on visibility: a fixturedef whose visibility is more specific
+        # sorts after a more general one, so that it takes precedence in the
+        # override chain (the last applicable fixturedef in the list is used
+        # first, see getfixturedefs).
+        # fixturedefs with the same visibility keep registration order, i.e. the
+        # last registered wins.
+        # The order between non-comparable fixturedefs doesn't matter since they
+        # cannot be visible together.
+        # The idea is that a fixture that is defined closer to the item should
+        # take precedence.
+        for i, existing in enumerate(faclist):
+            if is_visibility_more_specific(existing, fixture_def):
+                faclist.insert(i, fixture_def)
+                break
         else:
-            # fixturedefs with no location are at the front
-            # so this inserts the current fixturedef after the
-            # existing fixturedefs from external plugins but
-            # before the fixturedefs provided in conftests.
-            i = len([f for f in faclist if not f.has_location])
-            faclist.insert(i, fixture_def)
+            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:
@@ -1975,6 +2019,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,
@@ -1985,8 +2032,8 @@ def parsefactories(
     @overload
     def parsefactories(
         self,
-        node_or_obj: None = ...,
-        nodeid: None = ...,
+        node_or_obj: NotSetType = ...,
+        nodeid: NotSetType = ...,
         *,
         holder: object,
         node: nodes.Node,
@@ -1995,44 +2042,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_,
                 )