deprecate hook configuration via marks/attributes

fixes #4562

Author: RonnyPfannschmidt

changelog/4562.deprecation.rst

@@ -0,0 +1,2 @@
+Deprecate configureing hook specs/impls using attribute/marks.
+Instead ``use pytest.hookimpl`` and ``pytest.hookspec``.

doc/en/deprecations.rst

@@ -18,6 +18,36 @@ Deprecated Features
 Below is a complete list of all pytest features which are considered deprecated. Using those features will issue
 :class:`PytestWarning` or subclasses, which can be filtered using :ref:`standard warning filters <warnings>`.
 
+configuring hook specs/impls using markers
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Before pluggy was a own package and had a clear api,
+pytest just used ``pytest.mark`` to configure hooks.
+
+The :ref:`pytest.hookimpl` and :ref:`pytest.hookspec` decorators
+have been available since years and should be used.
+
+.. code-block:: python
+
+    @pytest.mark.tryfirst
+    def pytest_runtest_call():
+        ...
+
+
+    # or
+    def pytest_runtest_call():
+        ...
+
+
+    pytest_runtest_call.tryfirst = True
+
+    # becomes
+
+
+    @pytest.hookimpl(tryfirst=True)
+    def pytest_runtest_call():
+        ...
+
 
 ``py.path.local`` arguments for hooks replaced with ``pathlib.Path``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/_pytest/config/__init__.py

@@ -392,40 +392,57 @@ def parse_hookimpl_opts(self, plugin: _PluggyPlugin, name: str):
         if name == "pytest_plugins":
             return
 
-        method = getattr(plugin, name)
         opts = super().parse_hookimpl_opts(plugin, name)
+        if opts is not None:
+            return opts
 
+        method = getattr(plugin, name)
         # Consider only actual functions for hooks (#3775).
         if not inspect.isroutine(method):
             return
-
         # Collect unmarked hooks as long as they have the `pytest_' prefix.
-        if opts is None and name.startswith("pytest_"):
-            opts = {}
-        if opts is not None:
-            # TODO: DeprecationWarning, people should use hookimpl
-            # https://github.com/pytest-dev/pytest/issues/4562
-            known_marks = {m.name for m in getattr(method, "pytestmark", [])}
 
-            for name in ("tryfirst", "trylast", "optionalhook", "hookwrapper"):
-                opts.setdefault(name, hasattr(method, name) or name in known_marks)
+        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__
+            )
+            warnings.warn_explicit(
+                message,
+                type(message),
+                filename=inspect.getfile(method),
+                lineno=method.__code__.co_firstlineno,
+            )
         return opts
 
     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_"):
-                # todo: deprecate hookspec hacks
-                # https://github.com/pytest-dev/pytest/issues/4562
+
                 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,
                 }
+                if any(opts.values()):
+
+                    message = _pytest.deprecated.HOOK_LEGACY_MARKING.format(
+                        type="spec", fullname=method.__qualname__
+                    )
+                    warnings.warn_explicit(
+                        message,
+                        type(message),
+                        filename=inspect.getfile(method),
+                        lineno=method.__code__.co_firstlineno,
+                    )
         return opts
 
     def register(

src/_pytest/deprecated.py

@@ -106,6 +106,15 @@
     " Replace pytest.warns(None) by simply pytest.warns()."
 )
 
+
+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"
+    " to correctly mark hooks as pytest hooks.\n"
+    " see URL",
+)
+
 # You want to make some `__init__` or function "private".
 #
 #   def my_private_function(some, args):

testing/deprecated_test.py

@@ -51,6 +51,52 @@ def test_fillfixtures_is_deprecated() -> None:
         _pytest.fixtures.fillfixtures(mock.Mock())
 
 
+def test_hooksec_marks_are_deprecated():
+    from _pytest.config import PytestPluginManager
+
+    pm = PytestPluginManager()
+
+    class DeprecatedHookMarkerSpec:
+        def pytest_bad_hook(self):
+            pass
+
+        pytest_bad_hook.historic = True  # type: ignore[attr-defined]
+
+    with pytest.warns(
+        PytestDeprecationWarning, match="instead of pytest.mark"
+    ) as recorder:
+        pm.add_hookspecs(DeprecatedHookMarkerSpec)
+    (record,) = recorder
+    assert (
+        record.lineno
+        == DeprecatedHookMarkerSpec.pytest_bad_hook.__code__.co_firstlineno
+    )
+    assert record.filename == __file__
+
+
+def test_hookimpl_marks_are_deprecated():
+    from _pytest.config import PytestPluginManager
+
+    pm = PytestPluginManager()
+
+    class DeprecatedMarkImplPlugin:
+        def pytest_runtest_call(self):
+            pass
+
+        pytest_runtest_call.tryfirst = True  # type: ignore[attr-defined]
+
+    with pytest.warns(
+        PytestDeprecationWarning, match="instead of pytest.mark"
+    ) as recorder:
+        pm.register(DeprecatedMarkImplPlugin())
+    (record,) = recorder
+    assert (
+        record.lineno
+        == DeprecatedMarkImplPlugin.pytest_runtest_call.__code__.co_firstlineno
+    )
+    assert record.filename == __file__
+
+
 def test_minus_k_dash_is_deprecated(pytester: Pytester) -> None:
     threepass = pytester.makepyfile(
         test_threepass="""