Make PyCollector an implementation detail - don't use in hook type annotation

The `pytest_pycollector_makeitem` argument `collector` is currently
annotated with type `PyCollector`. As part of #7469, that would have
required us to expose it in the public API. But really it's an
implementation detail, not something we want to expose. So replace the
annotation with the concrete python collector types that are passed.

Strictly speaking, `pytest_pycollector_makeitem` is called from
`PyCollector.collect()`, so the new type annotation is incorrect if
another type subclasses `PyCollector`. But the set of python collectors
is closed (mapping to language constructs), and the type is private, so
there shouldn't be any other deriving classes, and we can consider it
effectively sealed (unfortunately Python does not provide a way to
express this - yet?).

Author: bluetech

changelog/7469.feature.rst

@@ -6,11 +6,11 @@ The newly-exported types are:
 - ``pytest.Mark`` for :class:`marks <pytest.Mark>`.
 - ``pytest.MarkDecorator`` for :class:`mark decorators <pytest.MarkDecorator>`.
 - ``pytest.MarkGenerator`` for the :class:`pytest.mark <pytest.MarkGenerator>` singleton.
-- ``pytest.Metafunc`` for the :class:`metafunc <pytest.MarkGenerator>` argument to the :func:`pytest_generate_tests <pytest.hookspec.pytest_generate_tests>` hook.
+- ``pytest.Metafunc`` for the :class:`metafunc <pytest.MarkGenerator>` argument to the :func:`pytest_generate_tests <_pytest.hookspec.pytest_generate_tests>` hook.
 - ``pytest.CallInfo`` for the :class:`CallInfo <pytest.CallInfo>` type passed to various hooks.
 - ``pytest.PytestPluginManager`` for :class:`PytestPluginManager <pytest.PytestPluginManager>`.
 - ``pytest.ExceptionInfo`` for the :class:`ExceptionInfo <pytest.ExceptionInfo>` type returned from :func:`pytest.raises` and passed to various hooks.
-- ``pytest.Parser`` for the :class:`Parser <pytest.Parser>` type passed to the :func:`pytest_addoption <pytest.hookspec.pytest_addoption>` hook.
+- ``pytest.Parser`` for the :class:`Parser <pytest.Parser>` type passed to the :func:`pytest_addoption <_pytest.hookspec.pytest_addoption>` hook.
 - ``pytest.OptionGroup`` for the :class:`OptionGroup <pytest.OptionGroup>` type returned from the :func:`parser.addgroup <pytest.Parser.getgroup>` method.
 - ``pytest.HookRecorder`` for the :class:`HookRecorder <pytest.HookRecorder>` type returned from :class:`~pytest.Pytester`.
 - ``pytest.RecordedHookCall`` for the :class:`RecordedHookCall <pytest.HookRecorder>` type returned from :class:`~pytest.HookRecorder`.

src/_pytest/hookspec.py

@@ -34,10 +34,11 @@
     from _pytest.nodes import Collector
     from _pytest.nodes import Item
     from _pytest.outcomes import Exit
+    from _pytest.python import Class
     from _pytest.python import Function
+    from _pytest.python import Instance
     from _pytest.python import Metafunc
     from _pytest.python import Module
-    from _pytest.python import PyCollector
     from _pytest.reports import CollectReport
     from _pytest.reports import TestReport
     from _pytest.runner import CallInfo
@@ -360,7 +361,7 @@ def pytest_pycollect_makemodule(
 
 @hookspec(firstresult=True)
 def pytest_pycollect_makeitem(
-    collector: "PyCollector", name: str, obj: object
+    collector: Union["Module", "Class", "Instance"], name: str, obj: object
 ) -> Union[None, "Item", "Collector", List[Union["Item", "Collector"]]]:
     """Return a custom item/collector for a Python object in a module, or None.
 

src/_pytest/python.py

@@ -218,11 +218,15 @@ def pytest_pycollect_makemodule(fspath: Path, parent) -> "Module":
 
 
 @hookimpl(trylast=True)
-def pytest_pycollect_makeitem(collector: "PyCollector", name: str, obj: object):
+def pytest_pycollect_makeitem(
+    collector: Union["Module", "Class", "Instance"], name: str, obj: object
+) -> Union[None, nodes.Item, nodes.Collector, List[Union[nodes.Item, nodes.Collector]]]:
+    assert isinstance(collector, (Class, Module, Instance)), type(collector)
     # Nothing was collected elsewhere, let's do it here.
     if safe_isclass(obj):
         if collector.istestclass(obj, name):
-            return Class.from_parent(collector, name=name, obj=obj)
+            klass: Class = Class.from_parent(collector, name=name, obj=obj)
+            return klass
     elif collector.istestfunction(obj, name):
         # mock seems to store unbound methods (issue473), normalize it.
         obj = getattr(obj, "__func__", obj)
@@ -241,15 +245,16 @@ def pytest_pycollect_makeitem(collector: "PyCollector", name: str, obj: object):
             )
         elif getattr(obj, "__test__", True):
             if is_generator(obj):
-                res = Function.from_parent(collector, name=name)
+                res: Function = Function.from_parent(collector, name=name)
                 reason = "yield tests were removed in pytest 4.0 - {name} will be ignored".format(
                     name=name
                 )
                 res.add_marker(MARK_GEN.xfail(run=False, reason=reason))
                 res.warn(PytestCollectionWarning(reason))
+                return res
             else:
-                res = list(collector._genfunctions(name, obj))
-            return res
+                return list(collector._genfunctions(name, obj))
+    return None
 
 
 class PyobjMixin(nodes.Node):

src/_pytest/unittest.py

@@ -27,7 +27,8 @@
 from _pytest.outcomes import xfail
 from _pytest.python import Class
 from _pytest.python import Function
-from _pytest.python import PyCollector
+from _pytest.python import Instance
+from _pytest.python import Module
 from _pytest.runner import CallInfo
 from _pytest.scope import Scope
 
@@ -42,7 +43,7 @@
 
 
 def pytest_pycollect_makeitem(
-    collector: PyCollector, name: str, obj: object
+    collector: Union[Module, Class, Instance], name: str, obj: object
 ) -> Optional["UnitTestCase"]:
     # Has unittest been imported and is obj a subclass of its TestCase?
     try: