Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid triggering @property methods on plugins when looking for hookimpls during registration #536

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
21 changes: 20 additions & 1 deletion src/pluggy/_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,26 @@
customize how hook implementation are picked up. By default, returns the
options for items decorated with :class:`HookimplMarker`.
"""
method: object = getattr(plugin, name)

Check warning on line 184 in src/pluggy/_manager.py

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_manager.py#L184

Added line #L184 was not covered by tests
# IMPORTANT: @property methods can have side effects, and are never hookimpl
pirate marked this conversation as resolved.
Show resolved Hide resolved
# if attr is a property, skip it in advance

Check warning on line 186 in src/pluggy/_manager.py

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_manager.py#L186

Added line #L186 was not covered by tests
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

Check warning on line 191 in src/pluggy/_manager.py

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_manager.py#L191

Added line #L191 was not covered by tests
pirate marked this conversation as resolved.
Show resolved Hide resolved
plugin_is_pydantic_obj = hasattr(plugin, "__pydantic_core_schema__")
if plugin_is_pydantic_obj and name in getattr(plugin, "model_fields", {}):
return None

method: object

Check warning on line 196 in src/pluggy/_manager.py

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_manager.py#L195-L196

Added lines #L195 - L196 were not covered by tests
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.)
method = getattr(type(plugin), name) # use class sig instead

if not inspect.isroutine(method):
return None
try:
Expand Down
31 changes: 31 additions & 0 deletions testing/test_pluginmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import importlib.metadata
from typing import Any
from typing import Dict
from typing import List

import pytest
Expand Down Expand Up @@ -123,6 +124,36 @@
assert pm.register(A(), "somename")


def test_register_skips_properties(he_pm: PluginManager) -> None:
class ClassWithProperties:
property_was_executed: bool = False

@property
def some_func(self):
self.property_was_executed = True
return None

Check warning on line 134 in testing/test_pluginmanager.py

View check run for this annotation

Codecov / codecov/patch

testing/test_pluginmanager.py#L133-L134

Added lines #L133 - L134 were not covered by tests

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
Expand Down