Merge pull request #282 from bluetech/hookwraper-gen-validate

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

Author: bluetech

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)

src/pluggy/manager.py

@@ -221,8 +221,10 @@ def _verify_hook(self, hook, hookimpl):
                 "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 @@ def _verify_hook(self, hook, hookimpl):
                 ),
             )
 
+        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`."""

testing/test_details.py

@@ -21,7 +21,7 @@ def x1meth(self):
 
         @hookimpl(hookwrapper=True, tryfirst=True)
         def x1meth2(self):
-            pass
+            yield  # pragma: no cover
 
     class Spec:
         @hookspec

testing/test_pluginmanager.py

@@ -135,6 +135,19 @@ def he_method1(self, qlwkje):
     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