Skip to content

Commit

Permalink
Avoid triggering property methods when inspecting plugin attribute si…
Browse files Browse the repository at this point in the history
…gnatures
  • Loading branch information
pirate committed Sep 27, 2024
1 parent b2cf1ff commit 24895d0
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
14 changes: 14 additions & 0 deletions src/pluggy/_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,20 @@ 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`.
"""

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
# 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
plugin_is_pydantic_obj = hasattr(plugin, "__pydantic_core_schema__")
if plugin_is_pydantic_obj and name in getattr(plugin, "model_fields", {}):
# pydantic models mess with the class and attr __signature__
# so inspect.isroutine(...) throws exceptions and cant be used

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

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_manager.py#L195

Added line #L195 was not covered by tests
return None

method: object = getattr(plugin, name)
if not inspect.isroutine(method):
return None
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 @@ class A:
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

0 comments on commit 24895d0

Please sign in to comment.