Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions changelog/282.feature.rst
Original file line number Diff line number Diff line change
@@ -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)
10 changes: 10 additions & 0 deletions src/pluggy/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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`."""
Expand Down
2 changes: 1 addition & 1 deletion testing/test_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def x1meth(self):

@hookimpl(hookwrapper=True, tryfirst=True)
def x1meth2(self):
pass
yield # pragma: no cover

class Spec:
@hookspec
Expand Down
13 changes: 13 additions & 0 deletions testing/test_pluginmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down