manager: get rid of _plugin2hookcallers

We can do everything necessary with just the `_name2plugin` dict; while
it makes some operations slower (registration and unregistration), they
are not in any hot path (calling) so not a problem.

Author: bluetech

src/pluggy/_manager.py

@@ -115,7 +115,6 @@ class PluginManager:
     def __init__(self, project_name: str) -> None:
         self.project_name: "Final" = project_name
         self._name2plugin: "Final[Dict[str, _Plugin]]" = {}
-        self._plugin2hookcallers: "Final[Dict[_Plugin, List[_HookCaller]]]" = {}
         self._plugin_distinfo: "Final[List[Tuple[_Plugin, DistFacade]]]" = []
         self.trace: "Final" = _tracing.TagTracer().get("pluginmanage")
         self.hook: "Final" = _HookRelay()
@@ -138,11 +137,17 @@ def register(self, plugin: _Plugin, name: Optional[str] = None) -> Optional[str]
         is already registered."""
         plugin_name = name or self.get_canonical_name(plugin)
 
-        if plugin_name in self._name2plugin or plugin in self._plugin2hookcallers:
+        if plugin_name in self._name2plugin:
             if self._name2plugin.get(plugin_name, -1) is None:
                 return None  # blocked plugin, return None to indicate no registration
             raise ValueError(
-                "Plugin already registered: %s=%s\n%s"
+                "Plugin name already registered: %s=%s\n%s"
+                % (plugin_name, plugin, self._name2plugin)
+            )
+
+        if plugin in self._name2plugin.values():
+            raise ValueError(
+                "Plugin already registered under a different name: %s=%s\n%s"
                 % (plugin_name, plugin, self._name2plugin)
             )
 
@@ -151,8 +156,6 @@ def register(self, plugin: _Plugin, name: Optional[str] = None) -> Optional[str]
         self._name2plugin[plugin_name] = plugin
 
         # register matching hook implementations of the plugin
-        hookcallers: List[_HookCaller] = []
-        self._plugin2hookcallers[plugin] = hookcallers
         for name in dir(plugin):
             hookimpl_opts = self.parse_hookimpl_opts(plugin, name)
             if hookimpl_opts is not None:
@@ -168,7 +171,6 @@ def register(self, plugin: _Plugin, name: Optional[str] = None) -> Optional[str]
                     self._verify_hook(hook, hookimpl)
                     hook._maybe_apply_history(hookimpl)
                 hook._add_hookimpl(hookimpl)
-                hookcallers.append(hook)
         return plugin_name
 
     def parse_hookimpl_opts(
@@ -201,14 +203,16 @@ def unregister(
         if plugin is None:
             plugin = self.get_plugin(name)
 
+        hookcallers = self.get_hookcallers(plugin)
+        if hookcallers:
+            for hookcaller in hookcallers:
+                hookcaller._remove_plugin(plugin)
+
         # if self._name2plugin[name] == None registration was blocked: ignore
         if self._name2plugin.get(name):
             assert name is not None
             del self._name2plugin[name]
 
-        for hookcaller in self._plugin2hookcallers.pop(plugin, []):
-            hookcaller._remove_plugin(plugin)
-
         return plugin
 
     def set_blocked(self, name: str) -> None:
@@ -254,11 +258,11 @@ def parse_hookspec_opts(
 
     def get_plugins(self) -> Set[Any]:
         """return the set of registered plugins."""
-        return set(self._plugin2hookcallers)
+        return set(self._name2plugin.values())
 
     def is_registered(self, plugin: _Plugin) -> bool:
         """Return ``True`` if the plugin is already registered."""
-        return plugin in self._plugin2hookcallers
+        return any(plugin == val for val in self._name2plugin.values())
 
     def get_canonical_name(self, plugin: _Plugin) -> str:
         """Return canonical name for a plugin object. Note that a plugin
@@ -373,7 +377,14 @@ def list_name_plugin(self) -> List[Tuple[str, _Plugin]]:
 
     def get_hookcallers(self, plugin: _Plugin) -> Optional[List[_HookCaller]]:
         """get all hook callers for the specified plugin."""
-        return self._plugin2hookcallers.get(plugin)
+        if self.get_name(plugin) is 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)
+        return hookcallers
 
     def add_hookcall_monitoring(
         self, before: _BeforeTrace, after: _AfterTrace

testing/test_pluginmanager.py

@@ -153,20 +153,25 @@ def he_method1(self):
 
 def test_register(pm: PluginManager) -> None:
     class MyPlugin:
-        pass
+        @hookimpl
+        def he_method1(self):
+            ...
 
     my = MyPlugin()
     pm.register(my)
-    assert my in pm.get_plugins()
+    assert pm.get_plugins() == {my}
     my2 = MyPlugin()
     pm.register(my2)
-    assert {my, my2}.issubset(pm.get_plugins())
+    assert pm.get_plugins() == {my, my2}
 
     assert pm.is_registered(my)
     assert pm.is_registered(my2)
     pm.unregister(my)
     assert not pm.is_registered(my)
-    assert my not in pm.get_plugins()
+    assert pm.get_plugins() == {my2}
+
+    with pytest.raises(AssertionError, match=r"not registered"):
+        pm.unregister(my)
 
 
 def test_register_unknown_hooks(pm: PluginManager) -> None:
@@ -468,6 +473,58 @@ class PluginNo:
     assert hook_plugins == [plugin1, plugin2]
 
 
+def test_get_hookcallers(pm: PluginManager) -> None:
+    class Hooks:
+        @hookspec
+        def he_method1(self):
+            ...
+
+        @hookspec
+        def he_method2(self):
+            ...
+
+    pm.add_hookspecs(Hooks)
+
+    class Plugin1:
+        @hookimpl
+        def he_method1(self):
+            ...
+
+        @hookimpl
+        def he_method2(self):
+            ...
+
+    class Plugin2:
+        @hookimpl
+        def he_method1(self):
+            ...
+
+    class Plugin3:
+        @hookimpl
+        def he_method2(self):
+            ...
+
+    plugin1 = Plugin1()
+    pm.register(plugin1)
+    plugin2 = Plugin2()
+    pm.register(plugin2)
+    plugin3 = Plugin3()
+    pm.register(plugin3)
+
+    hookcallers1 = pm.get_hookcallers(plugin1)
+    assert hookcallers1 is not None
+    assert len(hookcallers1) == 2
+    hookcallers2 = pm.get_hookcallers(plugin2)
+    assert hookcallers2 is not None
+    assert len(hookcallers2) == 1
+    hookcallers3 = pm.get_hookcallers(plugin3)
+    assert hookcallers3 is not None
+    assert len(hookcallers3) == 1
+    assert hookcallers1 == hookcallers2 + hookcallers3
+
+    assert pm.get_hookcallers(object()) is None
+
+
 def test_add_hookspecs_nohooks(pm: PluginManager) -> None:
     class NoHooks:
         pass