[DONTMERGE] refactor: clean up node constructors — move logic to from_parent

src/_pytest/doctest.py

@@ -31,6 +31,7 @@
 from _pytest.config import Config
 from _pytest.config.argparsing import Parser
 from _pytest.fixtures import fixture
+from _pytest.fixtures import FuncFixtureInfo
 from _pytest.fixtures import TopRequest
 from _pytest.nodes import Collector
 from _pytest.nodes import Item
@@ -249,6 +250,9 @@ def _get_runner(
 
 
 class DoctestItem(Item):
+    _fixtureinfo: FuncFixtureInfo
+    fixturenames: list[str]
+
     def __init__(
         self,
         name: str,
@@ -259,14 +263,7 @@ def __init__(
         super().__init__(name, parent)
         self.runner = runner
         self.dtest = dtest
-
-        # Stuff needed for fixture support.
         self.obj = None
-        fm = self.session._fixturemanager
-        fixtureinfo = fm.getfixtureinfo(node=self, func=None, cls=None)
-        self._fixtureinfo = fixtureinfo
-        self.fixturenames = fixtureinfo.names_closure
-        self._initrequest()
 
     @classmethod
     def from_parent(  # type: ignore[override]
@@ -279,7 +276,13 @@ def from_parent(  # type: ignore[override]
     ) -> Self:
         # incompatible signature due to imposed limits on subclass
         """The public named constructor."""
-        return super().from_parent(name=name, parent=parent, runner=runner, dtest=dtest)
+        item = super().from_parent(name=name, parent=parent, runner=runner, dtest=dtest)
+        fm = item.session._fixturemanager
+        fixtureinfo = fm.getfixtureinfo(node=item, func=None, cls=None)
+        item._fixtureinfo = fixtureinfo
+        item.fixturenames = fixtureinfo.names_closure
+        item._initrequest()
+        return item
 
     def _initrequest(self) -> None:
         self.funcargs: dict[str, object] = {}

src/_pytest/nodes.py

@@ -585,6 +585,7 @@ def __init__(
         session: Session | None = None,
         nodeid: str | None = None,
     ) -> None:
+        # Legacy path_or_parent handling — kept for non-cooperative constructors.
         if path_or_parent:
             if isinstance(path_or_parent, Node):
                 assert parent is None
@@ -593,31 +594,23 @@ def __init__(
                 assert path is None
                 path = path_or_parent
 
-        path = _imply_path(type(self), path, fspath=fspath)
+        if path is not None:
+            # Path was already resolved (typically by from_parent); skip
+            # _imply_path to avoid a duplicate deprecation warning for fspath.
+            if fspath is not None:
+                _check_path(path, fspath)
+        else:
+            path = _imply_path(type(self), path, fspath=fspath)
         if name is None:
-            name = path.name
-            if parent is not None and parent.path != path:
-                try:
-                    rel = path.relative_to(parent.path)
-                except ValueError:
-                    pass
-                else:
-                    name = str(rel)
-                name = norm_sep(name)
+            name = self._derive_name(path, parent)
         self.path = path
 
         if session is None:
             assert parent is not None
             session = parent.session
 
         if nodeid is None:
-            try:
-                nodeid = str(self.path.relative_to(session.config.rootpath))
-            except ValueError:
-                nodeid = _check_initialpaths_for_relpath(session._initialpaths, path)
-
-            if nodeid:
-                nodeid = norm_sep(nodeid)
+            nodeid = self._derive_nodeid(path, parent, session)
 
         super().__init__(
             name=name,
@@ -628,6 +621,35 @@ def __init__(
             path=path,
         )
 
+    @classmethod
+    def _derive_name(cls, path: Path, parent: Node | None) -> str:
+        """Derive a collector name from its path and parent."""
+        name = path.name
+        if parent is not None and parent.path != path:
+            try:
+                rel = path.relative_to(parent.path)
+            except ValueError:
+                pass
+            else:
+                name = str(rel)
+            name = norm_sep(name)
+        return name
+
+    @classmethod
+    def _derive_nodeid(
+        cls, path: Path, parent: Node | None, session: Session
+    ) -> str | None:
+        """Derive a node ID from its path and the session root."""
+        nodeid: str | None
+        try:
+            nodeid = str(path.relative_to(session.config.rootpath))
+        except ValueError:
+            nodeid = _check_initialpaths_for_relpath(session._initialpaths, path)
+
+        if nodeid:
+            nodeid = norm_sep(nodeid)
+        return nodeid
+
     @classmethod
     def from_parent(
         cls,
@@ -637,7 +659,18 @@ def from_parent(
         path: Path | None = None,
         **kw,
     ) -> Self:
-        """The public constructor."""
+        """The public constructor.
+
+        Pre-computes name and nodeid from the path so that ``__init__``
+        receives fully resolved values.
+        """
+        path = _imply_path(cls, path, fspath=fspath)
+
+        if "name" not in kw:
+            kw["name"] = cls._derive_name(path, parent)
+        if "nodeid" not in kw:
+            kw["nodeid"] = cls._derive_nodeid(path, parent, parent.session)
+
         return super().from_parent(parent=parent, fspath=fspath, path=path, **kw)
 
 
@@ -701,15 +734,22 @@ def __init__(
         #: for this test.
         self.user_properties: list[tuple[str, object]] = []
 
-        self._check_item_and_collector_diamond_inheritance()
+    @classmethod
+    def from_parent(cls, parent: Node, **kw) -> Self:
+        """Public constructor for Items.
 
-    def _check_item_and_collector_diamond_inheritance(self) -> None:
+        This calls the diamond inheritance check before delegating to
+        :meth:`Node.from_parent`.
+        """
+        cls._check_item_and_collector_diamond_inheritance()
+        return super().from_parent(parent=parent, **kw)
+
+    @classmethod
+    def _check_item_and_collector_diamond_inheritance(cls) -> None:
         """
         Check if the current type inherits from both File and Collector
         at the same time, emitting a warning accordingly (#8447).
         """
-        cls = type(self)
-
         # We inject an attribute in the type to avoid issuing this warning
         # for the same class more than once, which is not helpful.
         # It is a hack, but was deemed acceptable in order to avoid

src/_pytest/python.py

@@ -45,7 +45,6 @@
 from _pytest.compat import get_real_func
 from _pytest.compat import getimfunc
 from _pytest.compat import is_async_function
-from _pytest.compat import LEGACY_PATH
 from _pytest.compat import NOTSET
 from _pytest.compat import safe_getattr
 from _pytest.compat import safe_isclass
@@ -652,27 +651,20 @@ class Package(nodes.Directory):
         Now inherits from :class:`~pytest.Directory`.
     """
 
-    def __init__(
-        self,
-        fspath: LEGACY_PATH | None,
+    @classmethod
+    def from_parent(  # type: ignore[override]
+        cls,
         parent: nodes.Collector,
-        # NOTE: following args are unused:
-        config=None,
-        session=None,
-        nodeid=None,
-        path: Path | None = None,
-    ) -> None:
-        # NOTE: Could be just the following, but kept as-is for compat.
-        # super().__init__(self, fspath, parent=parent)
-        session = parent.session
-        super().__init__(
-            fspath=fspath,
-            path=path,
-            parent=parent,
-            config=config,
-            session=session,
-            nodeid=nodeid,
-        )
+        *,
+        path: Path,
+    ) -> Self:
+        """The public constructor.
+
+        :param parent: The parent collector of this Package.
+        :param path: The package directory's path.
+        :type path: pathlib.Path
+        """
+        return super().from_parent(parent=parent, path=path)
 
     def setup(self) -> None:
         init_mod = importtestmodule(self.path / "__init__.py", self.config)
@@ -1588,12 +1580,11 @@ def __init__(
         config: Config | None = None,
         callspec: CallSpec2 | None = None,
         callobj=NOTSET,
-        keywords: Mapping[str, Any] | None = None,
         session: Session | None = None,
-        fixtureinfo: FuncFixtureInfo | None = None,
         originalname: str | None = None,
+        **kw,
     ) -> None:
-        super().__init__(name, parent, config=config, session=session)
+        super().__init__(name, parent, config=config, session=session, **kw)
 
         if callobj is not NOTSET:
             self._obj = callobj
@@ -1607,16 +1598,22 @@ def __init__(
         #: .. versionadded:: 3.0
         self.originalname = originalname or name
 
-        # Note: when FunctionDefinition is introduced, we should change ``originalname``
-        # to a readonly property that returns FunctionDefinition.name.
-
-        self.own_markers.extend(get_unpacked_marks(self.obj))
         if callspec:
             self.callspec = callspec
-            self.own_markers.extend(callspec.marks)
 
-        # todo: this is a hell of a hack
-        # https://github.com/pytest-dev/pytest/issues/4569
+    def _setup_markers_and_fixtures(
+        self,
+        keywords: Mapping[str, Any] | None = None,
+        fixtureinfo: FuncFixtureInfo | None = None,
+    ) -> None:
+        """Set up markers, keywords, fixture info, and the fixture request.
+
+        Called from :meth:`from_parent` after the node is fully constructed.
+        """
+        self.own_markers.extend(get_unpacked_marks(self.obj))
+        if hasattr(self, "callspec"):
+            self.own_markers.extend(self.callspec.marks)
+
         # Note: the order of the updates is important here; indicates what
         # takes priority (ctor argument over function attributes over markers).
         # Take own_markers only; NodeKeywords handles parent traversal on its own.
@@ -1632,11 +1629,17 @@ def __init__(
         self.fixturenames = fixtureinfo.names_closure
         self._initrequest()
 
-    # todo: determine sound type limitations
     @classmethod
     def from_parent(cls, parent, **kw) -> Self:
         """The public constructor."""
-        return super().from_parent(parent=parent, **kw)
+        keywords = kw.pop("keywords", None)
+        fixtureinfo = kw.pop("fixtureinfo", None)
+        item = super().from_parent(parent=parent, **kw)
+        item._setup_markers_and_fixtures(
+            keywords=keywords,
+            fixtureinfo=fixtureinfo,
+        )
+        return item
 
     def _initrequest(self) -> None:
         self.funcargs: dict[str, object] = {}