Validate that a hookwrapper's function is a generator function during registration

Previously this error was only reported when calling the hook. Now it is
reported during registration.

4 files changed, 52 insertions, 1 deletions

diff --git a/changelog/282.feature.rst b/changelog/282.feature.rst
new file mode 100644
index 0000000..d5f2b9b
--- /dev/null
+++ b/changelog/282.feature.rst
@@ -0,0 +1,28 @@
+When registering a hookimpl which is declared as ``hookwrapper=True`` but whose
+function is not a generator function, a ``PluggyValidationError`` exception is
+now raised.
+
+Previously this problem would cause an error only later, when calling the hook.
+
+In the unlikely case that you have a hookwrapper that *returns* a generator
+instead of yielding directly, for example:
+
+.. code-block:: python
+
+    def my_hook_real_implementation(arg):
+        print("before")
+        yield
+        print("after")
+
+
+    @hookimpl(hookwrapper=True)
+    def my_hook(arg):
+        return my_hook_implementation(arg)
+
+change it to use ``yield from`` instead:
+
+.. code-block:: python
+
+    @hookimpl(hookwrapper=True)
+    def my_hook(arg):
+        yield from my_hook_implementation(arg)
diff --git a/src/pluggy/manager.py b/src/pluggy/manager.py
index e7acd3d..62b999f 100644
--- a/src/pluggy/manager.py
+++ b/src/pluggy/manager.py
@@ -221,8 +221,10 @@ class PluginManager:
                 "Plugin %r\nhook %r\nhistoric incompatible to hookwrapper"
                 % (hookimpl.plugin_name, hook.name),
             )
+
         if hook.spec.warn_on_impl:
             _warn_for_function(hook.spec.warn_on_impl, hookimpl.function)
+
         # positional arg checking
         notinspec = set(hookimpl.argnames) - set(hook.spec.argnames)
         if notinspec:
@@ -239,6 +241,14 @@ class PluginManager:
                 ),
             )
 
+        if hookimpl.hookwrapper and not inspect.isgeneratorfunction(hookimpl.function):
+            raise PluginValidationError(
+                hookimpl.plugin,
+                "Plugin %r for hook %r\nhookimpl definition: %s\n"
+                "Declared as hookwrapper=True but function is not a generator function"
+                % (hookimpl.plugin_name, hook.name, _formatdef(hookimpl.function)),
+            )
+
     def check_pending(self):
         """ Verify that all hooks which have not been verified against
         a hook specification are optional, otherwise raise :py:class:`.PluginValidationError`."""
diff --git a/testing/test_details.py b/testing/test_details.py
index dc2cec7..08f82e1 100644
--- a/testing/test_details.py
+++ b/testing/test_details.py
@@ -21,7 +21,7 @@ def test_parse_hookimpl_override():
 
         @hookimpl(hookwrapper=True, tryfirst=True)
         def x1meth2(self):
-            pass
+            yield  # pragma: no cover
 
     class Spec:
         @hookspec
diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py
index 2ee7512..6eb5045 100644
--- a/testing/test_pluginmanager.py
+++ b/testing/test_pluginmanager.py
@@ -135,6 +135,19 @@ def test_register_mismatch_arg(he_pm):
     assert excinfo.value.plugin is plugin
 
 
+def test_register_hookwrapper_not_a_generator_function(he_pm):
+    class hello:
+        @hookimpl(hookwrapper=True)
+        def he_method1(self):
+            pass  # pragma: no cover
+
+    plugin = hello()
+
+    with pytest.raises(PluginValidationError, match="generator function") as excinfo:
+        he_pm.register(plugin)
+    assert excinfo.value.plugin is plugin
+
+
 def test_register(pm):
     class MyPlugin:
         pass