Discover hooks via inspect.getattr_static

changelog/701.bugfix.rst

@@ -0,0 +1 @@
+Discover hookimpls/hookspecs via :func:`inspect.getattr_static` so properties and other descriptors are not executed during plugin registration.

src/pluggy/_manager.py

@@ -15,7 +15,6 @@
 
 from . import _tracing
 from ._callers import _multicall
-from ._hooks import _HookImplFunction
 from ._hooks import _Namespace
 from ._hooks import _Plugin
 from ._hooks import _SubsetHookCaller
@@ -49,6 +48,60 @@ def _warn_for_function(warning: Warning, function: Callable[..., object]) -> Non
     )
 
 
+def _get_hookable(obj: object, name: str) -> Callable[..., object] | None:
+    """Return a hookable callable for ``name`` without triggering descriptors.
+
+    Only plain functions, :class:`classmethod`, :class:`staticmethod`, and
+    already-bound :class:`~types.MethodType` objects are supported. Properties,
+    ``cached_property``, and other descriptors are intentionally skipped --
+    hooks provided via such descriptors (or only via ``__getattr__``) are not
+    supported.
+    """
+    static: object = inspect.getattr_static(obj, name, None)
+    if isinstance(static, staticmethod):
+        return static.__func__
+    if isinstance(static, classmethod):
+        owner = obj if inspect.isclass(obj) else type(obj)
+        return cast(Callable[..., object], static.__get__(owner, owner))
+    if isinstance(static, types.MethodType):
+        return static
+    if inspect.isfunction(static):
+        if inspect.isclass(obj) or inspect.ismodule(obj):
+            return static
+        return static.__get__(obj, type(obj))
+    return None
+
+
+def _hook_marker_holder(obj: object, name: str) -> object | None:
+    """Return the object that may carry hookimpl/hookspec marker options.
+
+    Marker attributes may live on a ``classmethod``/``staticmethod`` wrapper
+    (``@hookimpl`` applied above them) or on the underlying function
+    (``@hookimpl`` applied below them).
+    """
+    static: object = inspect.getattr_static(obj, name, None)
+    if isinstance(static, (classmethod, staticmethod)):
+        return static
+    if isinstance(static, types.MethodType):
+        return static.__func__
+    if inspect.isfunction(static):
+        return static
+    return None
+
+
+def _get_marker_opts(holder: object, attrname: str) -> dict[str, Any] | None:
+    """Read marker options from ``holder``, falling back to ``__func__``."""
+    opts = getattr(holder, attrname, None)
+    if opts is not None:
+        return opts if isinstance(opts, dict) else None
+    func = getattr(holder, "__func__", None)
+    if func is not None:
+        opts = getattr(func, attrname, None)
+        if opts is not None:
+            return opts if isinstance(opts, dict) else None
+    return None
+
+
 class PluginValidationError(Exception):
     """Plugin failed validation.
 
@@ -160,7 +213,8 @@ def register(self, plugin: _Plugin, name: str | None = None) -> str | None:
             hookimpl_opts = self.parse_hookimpl_opts(plugin, name)
             if hookimpl_opts is not None:
                 normalize_hookimpl_opts(hookimpl_opts)
-                method: _HookImplFunction[object] = getattr(plugin, name)
+                method = _get_hookable(plugin, name)
+                assert method is not None
                 hookimpl = HookImpl(plugin, plugin_name, method, hookimpl_opts)
                 name = hookimpl_opts.get("specname") or name
                 hook: HookCaller | None = getattr(self.hook, name, None)
@@ -183,20 +237,18 @@ def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None
         This method can be overridden by ``PluginManager`` subclasses to
         customize how hook implementation are picked up. By default, returns the
         options for items decorated with :class:`HookimplMarker`.
+
+        Discovery uses :func:`inspect.getattr_static` so properties and other
+        descriptors are not executed. Only functions, classmethods,
+        staticmethods, and bound methods are considered.
         """
-        method: object = getattr(plugin, name)
-        if not inspect.isroutine(method):
+        holder = _hook_marker_holder(plugin, name)
+        if holder is None:
             return None
-        try:
-            res: HookimplOpts | None = getattr(
-                method, self.project_name + "_impl", None
-            )
-        except Exception:  # pragma: no cover
-            res = {}  # type: ignore[assignment] #pragma: no cover
-        if res is not None and not isinstance(res, dict):
-            # false positive
-            res = None  # type:ignore[unreachable] #pragma: no cover
-        return res
+        return cast(
+            HookimplOpts | None,
+            _get_marker_opts(holder, self.project_name + "_impl"),
+        )
 
     def unregister(
         self, plugin: _Plugin | None = None, name: str | None = None
@@ -288,10 +340,18 @@ def parse_hookspec_opts(
         This method can be overridden by ``PluginManager`` subclasses to
         customize how hook specifications are picked up. By default, returns the
         options for items decorated with :class:`HookspecMarker`.
+
+        Discovery uses :func:`inspect.getattr_static` so properties and other
+        descriptors are not executed. Only functions, classmethods,
+        staticmethods, and bound methods are considered.
         """
-        method = getattr(module_or_class, name)
-        opts: HookspecOpts | None = getattr(method, self.project_name + "_spec", None)
-        return opts
+        holder = _hook_marker_holder(module_or_class, name)
+        if holder is None:
+            return None
+        return cast(
+            HookspecOpts | None,
+            _get_marker_opts(holder, self.project_name + "_spec"),
+        )
 
     def get_plugins(self) -> set[Any]:
         """Return a set of all registered plugin objects."""

testing/test_pluginmanager.py

@@ -124,6 +124,89 @@ class A:
     assert pm.register(A(), "somename")
 
 
+def test_register_ignores_properties(he_pm: PluginManager) -> None:
+    class ClassWithProperties:
+        property_was_executed: bool = False
+
+        @property
+        def some_func(self):
+            self.property_was_executed = True  # pragma: no cover
+            return None  # pragma: no cover
+
+    # Registering the class is harmless (getattr returns the property object).
+    he_pm.register(ClassWithProperties)
+    # Registering an instance must not evaluate the property getter.
+    test_plugin = ClassWithProperties()
+    he_pm.register(test_plugin)
+    assert not test_plugin.property_was_executed
+
+
+def test_register_ignores_cached_property(he_pm: PluginManager) -> None:
+    from functools import cached_property
+
+    class ClassWithCachedProperty:
+        cached_was_executed: bool = False
+
+        @cached_property
+        def some_func(self) -> None:
+            self.cached_was_executed = True  # pragma: no cover
+            return None  # pragma: no cover
+
+    test_plugin = ClassWithCachedProperty()
+    he_pm.register(test_plugin)
+    assert not test_plugin.cached_was_executed
+    assert "some_func" not in test_plugin.__dict__
+
+
+def test_register_ignores_raising_descriptors(he_pm: PluginManager) -> None:
+    """Descriptor attrs are skipped without accessing them via getattr."""
+
+    class RaisingDescriptor:
+        def __get__(self, obj: object, owner: type | None = None) -> object:
+            raise AttributeError("descriptor access failed")
+
+    class PluginWithRaisingDescriptor:
+        weird_attr = RaisingDescriptor()
+
+        @hookimpl
+        def he_method1(self, arg: object) -> list[object]:
+            return [arg]
+
+    plugin = PluginWithRaisingDescriptor()
+    assert "weird_attr" in dir(plugin)
+    with pytest.raises(AttributeError):
+        getattr(plugin, "weird_attr")
+
+    he_pm.register(plugin)
+    assert he_pm.hook.he_method1(arg=1) == [[1]]
+
+
+def test_register_hookimpl_above_classmethod(he_pm: PluginManager) -> None:
+    """@hookimpl applied above @classmethod is discoverable via static lookup."""
+
+    class Plugin:
+        @hookimpl
+        @classmethod
+        def he_method1(cls, arg: object) -> list[object]:
+            return [arg]
+
+    he_pm.register(Plugin())
+    assert he_pm.hook.he_method1(arg=1) == [[1]]
+
+
+def test_register_hookimpl_above_staticmethod(he_pm: PluginManager) -> None:
+    """@hookimpl applied above @staticmethod is discoverable via static lookup."""
+
+    class Plugin:
+        @hookimpl
+        @staticmethod
+        def he_method1(arg: object) -> list[object]:
+            return [arg]
+
+    he_pm.register(Plugin())
+    assert he_pm.hook.he_method1(arg=1) == [[1]]
+
+
 def test_register_mismatch_method(he_pm: PluginManager) -> None:
     class hello:
         @hookimpl