From fcc02615b996d71fc4725487306c4ffb054cd206 Mon Sep 17 00:00:00 2001
From: Varun Chawla <varun_6april@hotmail.com>
Date: Tue, 10 Feb 2026 00:13:49 -0800
Subject: [PATCH] Fix _remove_plugin to remove all hookimpls and
 get_hookcallers to deduplicate

When a plugin registers multiple hook implementations on the same hook
(e.g. via specname), `_remove_plugin` only removed the first matching
hookimpl. This worked because `get_hookcallers` returned duplicate
HookCaller entries, causing `_remove_plugin` to be called multiple
times. This was fragile and made `get_hookcallers` return unexpected
duplicates to callers of the public API.

Fix `_remove_plugin` to remove all hookimpls for a plugin in a single
call, and fix `get_hookcallers` to return each HookCaller at most once.

Closes #431

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 changelog/431.bugfix.rst      |  1 +
 src/pluggy/_hooks.py          | 10 ++---
 src/pluggy/_manager.py        |  5 +--
 testing/test_pluginmanager.py | 73 +++++++++++++++++++++++++++++++++++
 4 files changed, 81 insertions(+), 8 deletions(-)
 create mode 100644 changelog/431.bugfix.rst

diff --git a/changelog/431.bugfix.rst b/changelog/431.bugfix.rst
new file mode 100644
index 00000000..5c813c2a
--- /dev/null
+++ b/changelog/431.bugfix.rst
@@ -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``).
diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py
index 7fde78c9..cdd3085c 100644
--- a/src/pluggy/_hooks.py
+++ b/src/pluggy/_hooks.py
@@ -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."""
diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py
index 1b994f25..c20105c8 100644
--- a/src/pluggy/_manager.py
+++ b/src/pluggy/_manager.py
@@ -442,9 +442,8 @@ def get_hookcallers(self, plugin: _Plugin) -> list[HookCaller] | None:
             return None
         hookcallers = []
         for hookcaller in self.hook.__dict__.values():
-            for hookimpl in hookcaller.get_hookimpls():
-                if hookimpl.plugin is plugin:
-                    hookcallers.append(hookcaller)
+            if any(impl.plugin is plugin for impl in hookcaller.get_hookimpls()):
+                hookcallers.append(hookcaller)
         return hookcallers
 
     def add_hookcall_monitoring(
diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py
index f80b1b55..0c70c947 100644
--- a/testing/test_pluginmanager.py
+++ b/testing/test_pluginmanager.py
@@ -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"}