diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py index 05407cfb..843fef8a 100644 --- a/src/pluggy/_manager.py +++ b/src/pluggy/_manager.py @@ -45,6 +45,16 @@ def _warn_for_function(warning: Warning, function: Callable[..., object]) -> Non filename=func.__code__.co_filename, ) +def _attr_is_property(obj: Any, 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. @@ -182,23 +192,16 @@ def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None options for items decorated with :class:`HookimplMarker`. """ - # IMPORTANT: @property methods can have side effects, and are never hookimpl - # if attr is a property, skip it in advance - plugin_class = plugin if inspect.isclass(plugin) else type(plugin) - if isinstance(getattr(plugin_class, name, None), property): - return None - - # pydantic model fields are like attrs and also can never be hookimpls - plugin_is_pydantic_obj = hasattr(plugin, "__pydantic_core_schema__") - if plugin_is_pydantic_obj and name in getattr(plugin, "model_fields", {}): + 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: - # AttributeError: '__signature__' attribute of 'Plugin' is class-only - # can happen for some special objects (e.g. proxies, pydantic, etc.) + # AttributeError: '__signature__' attribute of 'plugin' is class-only + # can happen if plugin is a proxy object wrapping a class/module method = getattr(type(plugin), name) # use class sig instead if not inspect.isroutine(method): @@ -305,7 +308,17 @@ 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: + # AttributeError: '__signature__' attribute of is class-only + # can happen if module_or_class is a proxy obj wrapping a class/module + method = getattr(type(module_or_class), name) # use class sig instead 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 67ea6789..40dd56fe 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -124,36 +124,20 @@ class A: assert pm.register(A(), "somename") -def test_register_skips_properties(he_pm: PluginManager) -> None: +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 - return None + self.property_was_executed = True # pragma: no cover + return None # pragma: no cover test_plugin = ClassWithProperties() he_pm.register(test_plugin) assert not test_plugin.property_was_executed -def test_register_skips_pydantic_fields(he_pm: PluginManager) -> None: - class PydanticModelClass: - # stub to make object look like a pydantic model - model_fields: Dict[str, bool] = {"some_attr": True} - - def __pydantic_core_schema__(self): ... - - @hookimpl - def some_attr(self): ... - - test_plugin = PydanticModelClass() - he_pm.register(test_plugin) - with pytest.raises(AttributeError): - he_pm.hook.some_attr.get_hookimpls() - - def test_register_mismatch_method(he_pm: PluginManager) -> None: class hello: @hookimpl