Merge pull request #389 from bluetech/un-result

Hook wrapping without `_Result`

.coveragerc

@@ -19,6 +19,7 @@ exclude_lines =
     pragma: no cover
 
     if TYPE_CHECKING:
+    if False:
 
     if __name__ == .__main__.:
 

changelog/260.feature.rst

@@ -0,0 +1,10 @@
+Add "new-style" hook wrappers, a simpler but equally powerful alternative to the existing ``hookwrapper=True`` wrappers.
+
+New-style wrappers are generator functions, similarly to ``hookwrapper``, but do away with the :class:`result <pluggy._callers._Result>` object.
+Instead, the return value is sent directly to the ``yield`` statement, or, if inner calls raised an exception, it is raised from the ``yield``.
+The wrapper is excepted to return a value or raise an exception, which will become the result of the hook call.
+
+New-style wrappers are fully interoperable with old-style wrappers.
+We encourage users to use the new style, however we do not intend to deprecate the old style any time soon.
+
+See :ref:`hookwrappers` for the full documentation.

docs/index.rst

@@ -362,13 +362,73 @@ For another example see the :ref:`pytest:plugin-hookorder` section of the
 
 Wrappers
 ^^^^^^^^
-A *hookimpl* can be marked with a ``"hookwrapper"`` option which indicates that
-the function will be called to *wrap* (or surround) all other normal *hookimpl*
-calls. A *hookwrapper* can thus execute some code ahead and after the execution
-of all corresponding non-wrappper *hookimpls*.
 
-Much in the same way as a :py:func:`@contextlib.contextmanager <python:contextlib.contextmanager>`, *hookwrappers* must
-be implemented as generator function with a single ``yield`` in its body:
+.. note::
+    This section describes "new-style hook wrappers", which were added in Pluggy
+    1.1. For earlier versions, see the "old-style hook wrappers" section below.
+    The two styles are fully interoperable.
+
+A *hookimpl* can be a generator function, which indicates that the function will
+be called to *wrap* (or surround) all other normal *hookimpl* calls. A *hook
+wrapper* can thus execute some code ahead and after the execution of all
+corresponding non-wrappper *hookimpls*.
+
+Much in the same way as a :py:func:`@contextlib.contextmanager <python:contextlib.contextmanager>`,
+*hook wrappers* must be implemented as generator function with a single ``yield`` in its body:
+
+.. code-block:: python
+
+    @hookimpl
+    def setup_project(config, args):
+        """Wrap calls to ``setup_project()`` implementations which
+        should return json encoded config options.
+        """
+        # get initial default config
+        defaults = config.tojson()
+
+        if config.debug:
+            print("Pre-hook config is {}".format(config.tojson()))
+
+        # all corresponding hookimpls are invoked here
+        result = yield
+
+        for item in result:
+            print("JSON config override is {}".format(item))
+
+        if config.debug:
+            print("Post-hook config is {}".format(config.tojson()))
+
+        if config.use_defaults:
+            return defaults
+        else:
+            return result
+
+The generator is :py:meth:`sent <python:generator.send>` the return value
+of the hook thus far, or, if the previous calls raised an exception, it is
+:py:meth:`thrown <python:generator.throw>` the exception.
+
+The function should do one of two things:
+- Return a value, which can be the same value as received from the ``yield``, or
+something else entirely.
+- Raise an exception.
+The return value or exception propagate to further hook wrappers, and finally
+to the hook caller.
+
+Also see the :ref:`pytest:hookwrapper` section in the ``pytest`` docs.
+
+Old-style wrappers
+^^^^^^^^^^^^^^^^^^
+
+.. note::
+    Prefer to use new-style hook wrappers, unless you need to support Pluggy
+    versions before 1.1.
+
+A *hookimpl* can be marked with the ``"hookwrapper"`` option, which indicates
+that the function will be called to *wrap* (or surround) all other normal
+*hookimpl* calls. A *hookwrapper* can thus execute some code ahead and after the
+execution of all corresponding non-wrappper *hookimpls*.
+
+*hookwrappers* must be implemented as generator function with a single ``yield`` in its body:
 
 
 .. code-block:: python
@@ -411,16 +471,14 @@ the exception using the :py:meth:`~pluggy._callers._Result.force_exception`
 method.
 
 .. note::
-    Hookwrappers can **not** return results; they can only modify them using
-    the :py:meth:`~pluggy._callers._Result.force_result` API.
+    Old-style hook wrappers can **not** return results; they can only modify
+    them using the :py:meth:`~pluggy._callers._Result.force_result` API.
 
-    Hookwrappers should **not** raise exceptions; this will cause further
-    hookwrappers to be skipped. They should use
+    Old-style Hook wrappers should **not** raise exceptions; this will cause
+    further hookwrappers to be skipped. They should use
     :py:meth:`~pluggy._callers._Result.force_exception` to adjust the
     exception.
 
-Also see the :ref:`pytest:hookwrapper` section in the ``pytest`` docs.
-
 .. _specs:
 
 Specifications
@@ -538,7 +596,7 @@ than ``None``.
 This can be useful for optimizing a call loop for which you are only
 interested in a single core *hookimpl*. An example is the
 :func:`~_pytest.hookspec.pytest_cmdline_main` central routine of ``pytest``.
-Note that all ``hookwrappers`` are still invoked with the first result.
+Note that all hook wrappers are still invoked with the first result.
 
 Also see the :ref:`pytest:firstresult` section in the ``pytest`` docs.
 
@@ -750,10 +808,9 @@ single value (which is not ``None``) will be returned.
 
 Exception handling
 ------------------
-If any *hookimpl* errors with an exception no further callbacks
-are invoked and the exception is packaged up and delivered to
-any :ref:`wrappers <hookwrappers>` before being re-raised at the
-hook invocation point:
+If any *hookimpl* errors with an exception no further callbacks are invoked and
+the exception is delivered to any :ref:`wrappers <hookwrappers>` before being
+re-raised at the hook invocation point:
 
 .. code-block:: python
 
@@ -780,15 +837,14 @@ hook invocation point:
             return 3
 
 
-    @hookimpl(hookwrapper=True)
+    @hookimpl
     def myhook(self, args):
-        outcome = yield
-
         try:
-            outcome.get_result()
-        except RuntimeError:
-            # log the error details
-            print(outcome.exception)
+            return (yield)
+        except RuntimeError as exc:
+            # log runtime error details
+            print(exc)
+            raise
 
 
     pm = PluginManager("myproject")

src/pluggy/_callers.py

@@ -7,7 +7,9 @@
 from typing import Generator
 from typing import Mapping
 from typing import Sequence
+from typing import Tuple
 from typing import TYPE_CHECKING
+from typing import Union
 
 from ._result import _raise_wrapfail
 from ._result import _Result
@@ -17,6 +19,14 @@
     from ._hooks import HookImpl
 
 
+# Need to distinguish between old- and new-style hook wrappers.
+# Wrapping one a singleton tuple is the fastest type-safe way I found to do it.
+Teardown = Union[
+    Tuple[Generator[None, _Result[object], None]],
+    Generator[None, object, object],
+]
+
+
 def _multicall(
     hook_name: str,
     hook_impls: Sequence[HookImpl],
@@ -32,7 +42,7 @@ def _multicall(
     results: list[object] = []
     exception = None
     try:  # run impl and wrapper setup functions in a loop
-        teardowns = []
+        teardowns: list[Teardown] = []
         try:
             for hook_impl in reversed(hook_impls):
                 try:
@@ -49,11 +59,21 @@ def _multicall(
                         # If this cast is not valid, a type error is raised below,
                         # which is the desired response.
                         res = hook_impl.function(*args)
-                        gen = cast(Generator[None, _Result[object], None], res)
-                        next(gen)  # first yield
-                        teardowns.append(gen)
+                        wrapper_gen = cast(Generator[None, _Result[object], None], res)
+                        next(wrapper_gen)  # first yield
+                        teardowns.append((wrapper_gen,))
                     except StopIteration:
-                        _raise_wrapfail(gen, "did not yield")
+                        _raise_wrapfail(wrapper_gen, "did not yield")
+                elif hook_impl.isgeneratorfunction:
+                    try:
+                        # If this cast is not valid, a type error is raised below,
+                        # which is the desired response.
+                        res = hook_impl.function(*args)
+                        function_gen = cast(Generator[None, object, object], res)
+                        next(function_gen)  # first yield
+                        teardowns.append(function_gen)
+                    except StopIteration:
+                        _raise_wrapfail(function_gen, "did not yield")
                 else:
                     res = hook_impl.function(*args)
                     if res is not None:
@@ -71,11 +91,29 @@ def _multicall(
             outcome = _Result(results, exception)
 
         # run all wrapper post-yield blocks
-        for gen in reversed(teardowns):
-            try:
-                gen.send(outcome)
-                _raise_wrapfail(gen, "has second yield")
-            except StopIteration:
-                pass
+        for teardown in reversed(teardowns):
+            if isinstance(teardown, tuple):
+                try:
+                    teardown[0].send(outcome)
+                    _raise_wrapfail(teardown[0], "has second yield")
+                except StopIteration:
+                    pass
+            else:
+                try:
+                    if outcome._exception is not None:
+                        teardown.throw(outcome._exception)
+                    else:
+                        teardown.send(outcome._result)
+                        # Following is unreachable for a well behaved hook wrapper.
+                        # Try to force finalizers otherwise postponed till GC action.
+                        # Note: close() may raise if generator handles GeneratorExit.
+                        teardown.close()
+                except StopIteration as si:
+                    outcome.force_result(si.value)
+                    continue
+                except BaseException as e:
+                    outcome.force_exception(e)
+                    continue
+                _raise_wrapfail(teardown, "has second yield")
 
         return outcome.get_result()

src/pluggy/_hooks.py

@@ -185,17 +185,30 @@ def __call__(  # noqa: F811
         If ``trylast`` is ``True``, this hook implementation will run as late as
         possible in the chain of N hook implementations.
 
-        If ``hookwrapper`` is ``True``, the hook implementations needs to
-        execute exactly one ``yield``. The code before the ``yield`` is run
-        early before any non-hookwrapper function is run. The code after the
-        ``yield`` is run after all non-hookwrapper function have run  The
-        ``yield`` receives a :class:`_Result` object representing the exception
-        or result outcome of the inner calls (including other hookwrapper
-        calls).
+        If the hook implementation is a generator function, and ``hookwrapper``
+        is ``False`` or not set ("new-style hook wrapper"), the hook
+        implementation needs to execute exactly one ``yield``. The code before
+        the ``yield`` is run early before any non-hook-wrapper function is run.
+        The code after the ``yield`` is run after all non-hook-wrapper functions
+        have run. The ``yield`` receives the result value of the inner calls, or
+        raises the exception of inner calls (including earlier hook wrapper
+        calls). The return value of the function becomes the return value of the
+        hook, and a raised exception becomes the exception of the hook.
+
+        If ``hookwrapper`` is ``True`` ("old-style hook wrapper"), the hook
+        implementation needs to execute exactly one ``yield``. The code before
+        the ``yield`` is run early before any non-hook-wrapper function is run.
+        The code after the ``yield`` is run after all non-hook-wrapper function
+        have run  The ``yield`` receives a :class:`_Result` object representing
+        the exception or result outcome of the inner calls (including earlier
+        hook wrapper calls).
 
         If ``specname`` is provided, it will be used instead of the function
         name when matching this hook implementation to a hook specification
         during registration.
+
+        .. versionadded:: 1.1
+            New-style hook wrappers.
         """
 
         def setattr_hookimpl_opts(func: _F) -> _F:
@@ -360,12 +373,12 @@ def get_hookimpls(self) -> list[HookImpl]:
     def _add_hookimpl(self, hookimpl: HookImpl) -> None:
         """Add an implementation to the callback chain."""
         for i, method in enumerate(self._hookimpls):
-            if method.hookwrapper:
+            if method.hookwrapper or method.isgeneratorfunction:
                 splitpoint = i
                 break
         else:
             splitpoint = len(self._hookimpls)
-        if hookimpl.hookwrapper:
+        if hookimpl.hookwrapper or hookimpl.isgeneratorfunction:
             start, end = splitpoint, len(self._hookimpls)
         else:
             start, end = 0, splitpoint
@@ -455,7 +468,11 @@ def call_extra(
             hookimpl = HookImpl(None, "<temp>", method, opts)
             # Find last non-tryfirst nonwrapper method.
             i = len(hookimpls) - 1
-            while i >= 0 and hookimpls[i].tryfirst and not hookimpls[i].hookwrapper:
+            while (
+                i >= 0
+                and hookimpls[i].tryfirst
+                and not (hookimpls[i].hookwrapper or hookimpls[i].isgeneratorfunction)
+            ):
                 i -= 1
             hookimpls.insert(i + 1, hookimpl)
         firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
@@ -528,6 +545,7 @@ class HookImpl:
         "plugin",
         "opts",
         "plugin_name",
+        "isgeneratorfunction",
         "hookwrapper",
         "optionalhook",
         "tryfirst",
@@ -546,6 +564,7 @@ def __init__(
         self.plugin = plugin
         self.opts = hook_impl_opts
         self.plugin_name = plugin_name
+        self.isgeneratorfunction = inspect.isgeneratorfunction(self.function)
         self.hookwrapper = hook_impl_opts["hookwrapper"]
         self.optionalhook = hook_impl_opts["optionalhook"]
         self.tryfirst = hook_impl_opts["tryfirst"]

src/pluggy/_manager.py

@@ -290,10 +290,12 @@ def get_name(self, plugin: _Plugin) -> str | None:
         return None
 
     def _verify_hook(self, hook: _HookCaller, hookimpl: HookImpl) -> None:
-        if hook.is_historic() and hookimpl.hookwrapper:
+        if hook.is_historic() and (
+            hookimpl.hookwrapper or hookimpl.isgeneratorfunction
+        ):
             raise PluginValidationError(
                 hookimpl.plugin,
-                "Plugin %r\nhook %r\nhistoric incompatible to hookwrapper"
+                "Plugin %r\nhook %r\nhistoric incompatible with yield/hookwrapper"
                 % (hookimpl.plugin_name, hook.name),
             )
 

src/pluggy/_result.py

@@ -23,7 +23,10 @@
 
 
 def _raise_wrapfail(
-    wrap_controller: Generator[None, _Result[_T], None], msg: str
+    wrap_controller: (
+        Generator[None, _Result[_T], None] | Generator[None, object, object]
+    ),
+    msg: str,
 ) -> NoReturn:
     co = wrap_controller.gi_code
     raise RuntimeError(