From 8ba0b7bc2ad2ead90a827ffeec5bfed1e264431c Mon Sep 17 00:00:00 2001
From: Ronny Pfannschmidt <ronny.pfannschmidt@redhat.com>
Date: Tue, 31 Dec 2019 20:20:28 +0100
Subject: [PATCH] fix #6341 - disallow session/config in Node.from_parent

---
 changelog/5975.deprecation.rst |  4 ++++
 src/_pytest/doctest.py         | 10 ++++++++--
 src/_pytest/nodes.py           | 23 ++++++++++++++++++++---
 src/_pytest/python.py          | 12 +++++++++---
 testing/python/collect.py      |  2 +-
 testing/test_nodes.py          |  7 +++++++
 6 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/changelog/5975.deprecation.rst b/changelog/5975.deprecation.rst
index 6e5dbc2ac..257249efe 100644
--- a/changelog/5975.deprecation.rst
+++ b/changelog/5975.deprecation.rst
@@ -4,3 +4,7 @@ Instead they are new constructed via ``Node.from_parent``.
 
 This transitional mechanism enables us to detangle the very intensely
 entangled ``Node`` relationships by enforcing more controlled creation/configruation patterns.
+
+As part of that session/config are already disallowed parameters and as we work on the details we might need disallow a few more as well.
+
+Subclasses are expected to use `super().from_parent` if they intend to expand the creation of `Nodes`.
diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py
index 75eac7db6..aa318dfbf 100644
--- a/src/_pytest/doctest.py
+++ b/src/_pytest/doctest.py
@@ -216,8 +216,14 @@ class DoctestItem(pytest.Item):
         self.fixture_request = None
 
     @classmethod
-    def from_parent(cls, parent, *, name, runner, dtest):
-        return cls._create(name=name, parent=parent, runner=runner, dtest=dtest)
+    def from_parent(  # type: ignore
+        cls, parent: "Union[DoctestTextfile, DoctestModule]", *, name, runner, dtest
+    ):
+        # incompatible signature due to to imposed limits on sublcass
+        """
+        the public named constructor
+        """
+        return super().from_parent(name=name, parent=parent, runner=runner, dtest=dtest)
 
     def setup(self):
         if self.dtest is not None:
diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py
index 3eaafa91d..385d8d6e4 100644
--- a/src/_pytest/nodes.py
+++ b/src/_pytest/nodes.py
@@ -144,8 +144,22 @@ class Node(metaclass=NodeMeta):
                 self._nodeid += "::" + self.name
 
     @classmethod
-    def from_parent(cls, parent, *, name):
-        return cls._create(parent=parent, name=name)
+    def from_parent(cls, parent: "Node", **kw):
+        """
+        Public Constructor for Nodes
+
+        This indirection got introduced in order to enable removing
+        the fragile logic from the node constructors.
+
+        Subclasses can use ``super().from_parent(...)`` when overriding the construction
+
+        :param parent: the parent node of this test Node
+        """
+        if "config" in kw:
+            raise TypeError("config is not a valid argument for from_parent")
+        if "session" in kw:
+            raise TypeError("session is not a valid argument for from_parent")
+        return cls._create(parent=parent, **kw)
 
     @property
     def ihook(self):
@@ -434,7 +448,10 @@ class FSCollector(Collector):
 
     @classmethod
     def from_parent(cls, parent, *, fspath):
-        return cls._create(parent=parent, fspath=fspath)
+        """
+        The public constructor
+        """
+        return super().from_parent(parent=parent, fspath=fspath)
 
 
 class File(FSCollector):
diff --git a/src/_pytest/python.py b/src/_pytest/python.py
index 1b01f4faa..a51be68c8 100644
--- a/src/_pytest/python.py
+++ b/src/_pytest/python.py
@@ -679,7 +679,10 @@ class Class(PyCollector):
 
     @classmethod
     def from_parent(cls, parent, *, name, obj=None):
-        return cls._create(name=name, parent=parent)
+        """
+        The public constructor
+        """
+        return super().from_parent(name=name, parent=parent)
 
     def collect(self):
         if not safe_getattr(self.obj, "__test__", True):
@@ -1458,8 +1461,11 @@ class Function(FunctionMixin, nodes.Item):
         self.originalname = originalname
 
     @classmethod
-    def from_parent(cls, parent, **kw):
-        return cls._create(parent=parent, **kw)
+    def from_parent(cls, parent, **kw):  # todo: determine sound type limitations
+        """
+        The public  constructor
+        """
+        return super().from_parent(parent=parent, **kw)
 
     def _initrequest(self):
         self.funcargs = {}
diff --git a/testing/python/collect.py b/testing/python/collect.py
index 9ac1c9d31..93e3de424 100644
--- a/testing/python/collect.py
+++ b/testing/python/collect.py
@@ -284,7 +284,7 @@ class TestFunction:
         session = testdir.Session.from_config(config)
         session._fixturemanager = FixtureManager(session)
 
-        return pytest.Function.from_parent(config=config, parent=session, **kwargs)
+        return pytest.Function.from_parent(parent=session, **kwargs)
 
     def test_function_equality(self, testdir, tmpdir):
         def func1():
diff --git a/testing/test_nodes.py b/testing/test_nodes.py
index b13ce1fe6..dbb3e2e8f 100644
--- a/testing/test_nodes.py
+++ b/testing/test_nodes.py
@@ -22,6 +22,13 @@ def test_ischildnode(baseid, nodeid, expected):
     assert result is expected
 
 
+def test_node_from_parent_disallowed_arguments():
+    with pytest.raises(TypeError, match="session is"):
+        nodes.Node.from_parent(None, session=None)
+    with pytest.raises(TypeError, match="config is"):
+        nodes.Node.from_parent(None, config=None)
+
+
 def test_std_warn_not_pytestwarning(testdir):
     items = testdir.getitems(
         """