partial application of review suggestions + externalize the helper

Author: RonnyPfannschmidt

changelog/4562.deprecation.rst

@@ -1,2 +1,4 @@
-Deprecate configureing hook specs/impls using attribute/marks.
-Instead ``use pytest.hookimpl`` and ``pytest.hookspec``.
+Deprecate configuring hook specs/impls using attribute/marks.
+
+Instead use :ref:`pytest.hookimpl` and :ref:`pytest.hookspec`.
+For more details, see the :ref:`docs <deprecate-hook-marks>`.

doc/en/deprecations.rst

@@ -41,8 +41,9 @@ have been available since years and should be used.
 
     pytest_runtest_call.tryfirst = True
 
-    # becomes
+This now is declared as:
 
+.. code-block:: python
 
     @pytest.hookimpl(tryfirst=True)
     def pytest_runtest_call():

src/_pytest/config/__init__.py

@@ -13,6 +13,7 @@
 import warnings
 from functools import lru_cache
 from pathlib import Path
+from types import FunctionType
 from types import TracebackType
 from typing import Any
 from typing import Callable
@@ -35,6 +36,7 @@
 from pluggy import HookimplMarker
 from pluggy import HookspecMarker
 from pluggy import PluginManager
+from typing_extensions import Literal
 
 import _pytest._code
 import _pytest.deprecated
@@ -330,6 +332,32 @@ def _prepareconfig(
         raise
 
 
+def _get_legacy_hook_marks(
+    method: FunctionType,
+    hook_type: Literal["spec", "impl"],
+    opt_names: Tuple[str, ...],
+) -> Dict[str, bool]:
+    known_marks = {m.name for m in getattr(method, "pytestmark", [])}
+    must_warn = False
+    opts = {}
+    for opt_name in opt_names:
+        if hasattr(method, opt_name) or opt_name in known_marks:
+            opts[opt_name] = True
+            must_warn = True
+        else:
+            opts[opt_name] = False
+    if must_warn:
+
+        hook_opts = ", ".join(f"{name}=True" for name, val in opts.items() if val)
+        message = _pytest.deprecated.HOOK_LEGACY_MARKING.format(
+            type=hook_type,
+            fullname=method.__qualname__,
+            hook_opts=hook_opts,
+        )
+        warn_explicit_for(method, message)
+    return opts
+
+
 @final
 class PytestPluginManager(PluginManager):
     """A :py:class:`pluggy.PluginManager <pluggy.PluginManager>` with
@@ -402,39 +430,20 @@ def parse_hookimpl_opts(self, plugin: _PluggyPlugin, name: str):
         if not inspect.isroutine(method):
             return
         # Collect unmarked hooks as long as they have the `pytest_' prefix.
-
-        known_marks = {m.name for m in getattr(method, "pytestmark", [])}
-        opts = {
-            name: hasattr(method, name) or name in known_marks
-            for name in ("tryfirst", "trylast", "optionalhook", "hookwrapper")
-        }
-        if any(opts.values()):
-            message = _pytest.deprecated.HOOK_LEGACY_MARKING.format(
-                type="spec", fullname=method.__qualname__
-            )
-            warn_explicit_for(method, message)
-
-        return opts
+        return _get_legacy_hook_marks(
+            method, "impl", ("tryfirst", "trylast", "optionalhook", "hookwrapper")
+        )
 
     def parse_hookspec_opts(self, module_or_class, name: str):
         opts = super().parse_hookspec_opts(module_or_class, name)
         if opts is None:
             method = getattr(module_or_class, name)
             if name.startswith("pytest_"):
-
-                known_marks = {m.name for m in getattr(method, "pytestmark", [])}
-                opts = {
-                    "firstresult": hasattr(method, "firstresult")
-                    or "firstresult" in known_marks,
-                    "historic": hasattr(method, "historic")
-                    or "historic" in known_marks,
-                }
-                # hook from xdist, fixing in  ...
-                if any(opts.values()) and module_or_class.__name__ != "xdist.newhooks":
-                    message = _pytest.deprecated.HOOK_LEGACY_MARKING.format(
-                        type="spec", fullname=method.__qualname__
-                    )
-                    warn_explicit_for(method, message)
+                opts = _get_legacy_hook_marks(
+                    method,
+                    "spec",
+                    ("firstresult", "historic"),
+                )
         return opts
 
     def register(

src/_pytest/deprecated.py

@@ -110,7 +110,7 @@
 HOOK_LEGACY_MARKING = UnformattedWarning(
     PytestDeprecationWarning,
     "The hook {type} {fullname} is not marked using pytest.hook{type}.\n"
-    " please use the pytest.hook{type}(...) decorator instead of pytest.mark \n"
+    " please use the pytest.hook{type}({hook_opts}) decorator instead of pytest.mark \n"
     " to correctly mark hooks as pytest hooks.\n"
     " see URL",
 )

testing/deprecated_test.py

@@ -51,7 +51,7 @@ def test_fillfixtures_is_deprecated() -> None:
         _pytest.fixtures.fillfixtures(mock.Mock())
 
 
-def test_hooksec_marks_are_deprecated():
+def test_hookspec_via_function_attributes_are_deprecated():
     from _pytest.config import PytestPluginManager
 
     pm = PytestPluginManager()
@@ -74,7 +74,7 @@ def pytest_bad_hook(self):
     assert record.filename == __file__
 
 
-def test_hookimpl_marks_are_deprecated():
+def test_hookimpl_via_function_attributes_are_deprecated():
     from _pytest.config import PytestPluginManager
 
     pm = PytestPluginManager()