diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py index c20105c8..e6361d20 100644 --- a/src/pluggy/_manager.py +++ b/src/pluggy/_manager.py @@ -49,6 +49,17 @@ def _warn_for_function(warning: Warning, function: Callable[..., object]) -> Non ) +def _attr_is_property(obj: object, name: str) -> bool: + """Check if a given attr is a @property on a module, class, or object""" + if inspect.ismodule(obj): + return False # modules can never have @property methods + + base_class = obj if inspect.isclass(obj) else type(obj) + if isinstance(getattr(base_class, name, None), property): + return True + return False + + class PluginValidationError(Exception): """Plugin failed validation. @@ -184,7 +195,18 @@ def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None customize how hook implementation are picked up. By default, returns the options for items decorated with :class:`HookimplMarker`. """ - method: object = getattr(plugin, name) + + if _attr_is_property(plugin, name): + # @property methods can have side effects, and are never hookimpls + return None + + method: object + try: + method = getattr(plugin, name) + except AttributeError: + # May be raised when trying to access some descriptor/proxied fields. + return None + if not inspect.isroutine(method): return None try: @@ -289,7 +311,16 @@ def parse_hookspec_opts( customize how hook specifications are picked up. By default, returns the options for items decorated with :class:`HookspecMarker`. """ - method = getattr(module_or_class, name) + if _attr_is_property(module_or_class, name): + # @property methods can have side effects, and are never hookspecs + return None + + method: object + try: + method = getattr(module_or_class, name) + except AttributeError: + # May be raised when trying to access some descriptor/proxied fields. + return None opts: HookspecOpts | None = getattr(method, self.project_name + "_spec", None) return opts diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py index 219b17d5..b38a87c8 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -124,6 +124,46 @@ 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_raising_descriptors(he_pm: PluginManager) -> None: + """Names in dir() whose getattr raises AttributeError are skipped.""" + + 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_mismatch_method(he_pm: PluginManager) -> None: class hello: @hookimpl