From 1bac79a3caa08c5449a3530ea6acc6b20e5b718b Mon Sep 17 00:00:00 2001
From: Nick Sweeting <github@sweeting.me>
Date: Tue, 1 Oct 2024 13:17:55 -0700
Subject: [PATCH] remove pydantic-specific logic and support hookspecs too

---
 src/pluggy/_manager.py        | 37 +++++++++++++++++++++++------------
 testing/test_pluginmanager.py | 22 +++------------------
 2 files changed, 28 insertions(+), 31 deletions(-)

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 <m_or_c> 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