Raise ValueError when unregister's plugin and name disagree

changelog/759.bugfix.rst

@@ -0,0 +1,2 @@
+:meth:`PluginManager.unregister() <pluggy.PluginManager.unregister>` now raises :exc:`ValueError` when both ``plugin`` and ``name`` are given but refer to different plugins, instead of removing one plugin's hook implementations and another plugin's registry entry.
+It also fully unregisters plugin objects that are falsy (e.g. define ``__len__`` returning ``0``); previously their name stayed in the registry.

src/pluggy/_manager.py

@@ -294,15 +294,27 @@ def unregister(
             plugin = self.get_plugin(name)
             if plugin is None:
                 return None
+        else:
+            # Both given: hook callers are looked up by plugin but the registry
+            # entry is removed by name, so they must refer to the same plugin.
+            registered_plugin = self._name2plugin.get(name)
+            registered_name = self.get_name(plugin)
+            if (registered_plugin is not None and registered_plugin is not plugin) or (
+                registered_name is not None and registered_name != name
+            ):
+                raise ValueError(
+                    f"Plugin name {name!r} and plugin {plugin!r} do not agree"
+                    f" (name is registered to {registered_plugin!r},"
+                    f" plugin is registered as {registered_name!r})"
+                )
 
         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
+        if self._name2plugin.get(name) is not None:
             del self._name2plugin[name]
 
         return plugin
@@ -499,7 +511,7 @@ def load_setuptools_entrypoints(self, group: str, name: str | None = None) -> in
                     ep.group != group
                     or (name is not None and ep.name != name)
                     # already registered
-                    or self.get_plugin(ep.name)
+                    or self.has_plugin(ep.name)
                     or self.is_blocked(ep.name)
                 ):
                     continue

testing/test_pluginmanager.py

@@ -1281,3 +1281,51 @@ def goodbye(self, arg: object) -> int:
     # Both hello impls are still wired up, and each caller works.
     assert pm.hook.hello(arg=1) == [101, 2]
     assert pm.hook.goodbye(arg=1) == [201]
+
+
+def test_unregister_plugin_and_name_must_agree(pm: PluginManager) -> None:
+    class Plugin:
+        @hookimpl
+        def he_method1(self, arg):
+            return arg
+
+    a, b, c = Plugin(), Plugin(), Plugin()
+    pm.register(a, "a")
+    pm.register(b, "b")
+    # Both registered, but under different names.
+    with pytest.raises(ValueError, match="do not agree"):
+        pm.unregister(a, "b")
+    # Plugin not registered, name belongs to another plugin.
+    with pytest.raises(ValueError, match="do not agree"):
+        pm.unregister(c, "b")
+    # Plugin registered, name unknown.
+    with pytest.raises(ValueError, match="do not agree"):
+        pm.unregister(a, "unknown")
+    # Rejected calls leave everything untouched.
+    assert pm.get_plugin("a") is a
+    assert pm.get_plugin("b") is b
+    assert pm.hook.he_method1(arg=1) == [1, 1]
+    # Agreeing arguments still work.
+    assert pm.unregister(a, "a") is a
+    assert not pm.is_registered(a)
+
+
+def test_unregister_falsy_plugin(pm: PluginManager) -> None:
+    class FalsyPlugin:
+        def __len__(self) -> int:
+            return 0
+
+        @hookimpl
+        def he_method1(self, arg):
+            return arg
+
+    plugin = FalsyPlugin()
+    assert not plugin
+    pm.register(plugin, "falsy")
+    assert pm.hook.he_method1(arg=1) == [1]
+    assert pm.unregister(plugin) is plugin
+    assert not pm.is_registered(plugin)
+    assert pm.get_plugin("falsy") is None
+    # The name is free again.
+    pm.register(plugin, "falsy")
+    assert pm.is_registered(plugin)