Skip to content
Open
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
1 change: 1 addition & 0 deletions changelog/431.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix ``HookCaller._remove_plugin()`` to remove all hook implementations for a plugin in a single call, instead of only the first one. Also fix ``PluginManager.get_hookcallers()`` to not return duplicate ``HookCaller`` entries when a plugin has multiple implementations on the same hook (e.g. via ``specname``).
10 changes: 5 additions & 5 deletions src/pluggy/_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,11 +440,11 @@ def is_historic(self) -> bool:
return self._call_history is not None

def _remove_plugin(self, plugin: _Plugin) -> None:
for i, method in enumerate(self._hookimpls):
if method.plugin == plugin:
del self._hookimpls[i]
return
raise ValueError(f"plugin {plugin!r} not found")
"""Remove all hook implementations registered by the given plugin."""
remaining = [impl for impl in self._hookimpls if impl.plugin != plugin]
if len(remaining) == len(self._hookimpls):
raise ValueError(f"plugin {plugin!r} not found")
self._hookimpls[:] = remaining

def get_hookimpls(self) -> list[HookImpl]:
"""Get all registered hook implementations for this hook."""
Expand Down
1 change: 1 addition & 0 deletions src/pluggy/_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ def get_hookcallers(self, plugin: _Plugin) -> list[HookCaller] | None:
for hookimpl in hookcaller.get_hookimpls():
if hookimpl.plugin is plugin:
hookcallers.append(hookcaller)
break
return hookcallers

def add_hookcall_monitoring(
Expand Down
73 changes: 73 additions & 0 deletions testing/test_pluginmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -814,3 +814,76 @@ def a_hook(self, param):
PluginValidationError, match="unknown hook 'a_hook' in plugin .*"
):
pm.check_pending()


def test_unregister_plugin_with_multi_hookimpls(pm: PluginManager) -> None:
"""Verify that unregistering a plugin with multiple hookimpls on the
same hook (via specname) removes all of them (#431)."""

class Api:
@hookspec
def hello(self, arg: object) -> object: ...

pm.add_hookspecs(Api)

class Plugin:
@hookimpl
def hello(self, arg: object) -> int:
return arg + 1 # type: ignore[operator]

@hookimpl(specname="hello")
def hello_again(self, arg: object) -> int:
return arg + 100 # type: ignore[operator]

plugin = Plugin()
pm.register(plugin)

# Both implementations should be registered.
impls = pm.hook.hello.get_hookimpls()
assert len(impls) == 2

# Both implementations should run.
out = pm.hook.hello(arg=3)
assert sorted(out) == [4, 103]

# After unregister, no implementations should remain.
pm.unregister(plugin)
assert pm.hook.hello(arg=3) == []
assert pm.hook.hello.get_hookimpls() == []


def test_get_hookcallers_no_duplicates(pm: PluginManager) -> None:
"""Verify that get_hookcallers does not return duplicate HookCaller
entries when a plugin has multiple hookimpls on the same hook (#431)."""

class Api:
@hookspec
def hello(self, arg: object) -> object: ...

@hookspec
def goodbye(self, arg: object) -> object: ...

pm.add_hookspecs(Api)

class Plugin:
@hookimpl
def hello(self, arg: object) -> int:
return arg + 1 # type: ignore[operator]

@hookimpl(specname="hello")
def hello_again(self, arg: object) -> int:
return arg + 100 # type: ignore[operator]

@hookimpl
def goodbye(self, arg: object) -> int:
return arg + 200 # type: ignore[operator]

plugin = Plugin()
pm.register(plugin)

hookcallers = pm.get_hookcallers(plugin)
assert hookcallers is not None
# Should return 2 unique callers (hello + goodbye), not 3.
assert len(hookcallers) == 2
caller_names = {hc.name for hc in hookcallers}
assert caller_names == {"hello", "goodbye"}