From d4b02afe0ed37b13a62f0e10778f30cbfeb0e9e4 Mon Sep 17 00:00:00 2001 From: Xucheng Zhou Date: Thu, 15 Oct 2026 21:35:56 +0800 Subject: [PATCH 1/3] Restore class-valued hook specifications Signed-off-by: Xucheng Zhou --- changelog/794.bugfix.rst | 3 + src/pluggy/_decorators.py | 5 +- src/pluggy/_manager.py | 11 +++- testing/test_class_hookspec.py | 101 +++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 changelog/794.bugfix.rst create mode 100644 testing/test_class_hookspec.py diff --git a/changelog/794.bugfix.rst b/changelog/794.bugfix.rst new file mode 100644 index 00000000..890a165d --- /dev/null +++ b/changelog/794.bugfix.rst @@ -0,0 +1,3 @@ +Restore discovery of class-valued hook specifications, such as those used by +ScanCode's PluginCode, without evaluating descriptors. Read constructor defaults +when a class supplies the specification signature. diff --git a/src/pluggy/_decorators.py b/src/pluggy/_decorators.py index c5574a67..11fdf02d 100644 --- a/src/pluggy/_decorators.py +++ b/src/pluggy/_decorators.py @@ -349,7 +349,10 @@ def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None self.argnames, self.kwargnames = varnames( self.function, legacy_noself=legacy_noself ) - defaults = inspect.unwrap(self.function).__defaults__ + function = self.function + if inspect.isclass(function): + function = function.__init__ + defaults = getattr(inspect.unwrap(function), "__defaults__", None) self.kwargdefaults = dict(zip(self.kwargnames, defaults or ())) self.opts = opts self.warn_on_impl = opts.get("warn_on_impl") diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py index aa54088c..a7571558 100644 --- a/src/pluggy/_manager.py +++ b/src/pluggy/_manager.py @@ -383,14 +383,19 @@ def parse_hookspec_opts( Discovery uses :func:`inspect.getattr_static` so properties and other descriptors are not executed. Only functions, classmethods, - staticmethods, and bound methods are considered. + staticmethods, bound methods, and class-valued specifications are + considered. """ found = _static_hook_attr(module_or_class, name) if found is None: - return None + holder = inspect.getattr_static(module_or_class, name, None) + if not inspect.isclass(holder): + return None + else: + holder = found[0] return cast( HookspecOpts | None, - _get_marker_opts(found[0], self.project_name + "_spec"), + _get_marker_opts(holder, self.project_name + "_spec"), ) def get_plugins(self) -> set[Any]: diff --git a/testing/test_class_hookspec.py b/testing/test_class_hookspec.py new file mode 100644 index 00000000..bfc9efa4 --- /dev/null +++ b/testing/test_class_hookspec.py @@ -0,0 +1,101 @@ +from functools import wraps +import types +from typing import Any + +import pytest + +from pluggy import HookimplMarker +from pluggy import HookspecMarker +from pluggy import PluginManager + + +hookspec = HookspecMarker("example") +hookimpl = HookimplMarker("example") + + +@pytest.mark.parametrize("namespace_kind", ["module", "class", "instance"]) +@pytest.mark.parametrize( + "constructor", ["implicit", "required", "default", "inherited", "wrapped"] +) +def test_class_valued_hookspec(namespace_kind: str, constructor: str) -> None: + class Implicit: + pass + + class Required: + def __init__(self, value: object) -> None: + raise AssertionError("hookspec constructors must not execute") + + class Default: + def __init__(self, value: object = "fallback") -> None: + raise AssertionError("hookspec constructors must not execute") + + class Inherited(Default): + pass + + class Wrapped: + @wraps(Default.__init__) # type: ignore[misc] + def __init__(self, *args: object, **kwargs: object) -> None: + raise AssertionError("hookspec constructors must not execute") + + specification = hookspec( + { + "implicit": Implicit, + "required": Required, + "default": Default, + "inherited": Inherited, + "wrapped": Wrapped, + }[constructor] + ) + namespace: Any + if namespace_kind == "module": + namespace = types.ModuleType("specification") + setattr(namespace, "hook", specification) + else: + namespace_class = type("Specification", (), {"hook": specification}) + namespace = namespace_class if namespace_kind == "class" else namespace_class() + pm = PluginManager("example") + pm.add_hookspecs(namespace) + spec = pm.hook.hook.spec + assert spec is not None + assert spec.function is specification + assert spec.argnames == (("value",) if constructor == "required" else ()) + assert spec.kwargnames == ( + () if constructor in ("implicit", "required") else ("value",) + ) + assert spec.kwargdefaults == ( + {} if constructor in ("implicit", "required") else {"value": "fallback"} + ) + + plugin = types.ModuleType("implementation") + if constructor == "implicit": + @hookimpl + def empty_hook() -> str: + return "control" + + setattr(plugin, "hook", empty_hook) + pm.register(plugin) + assert pm.hook.hook() == ["control"] + else: + @hookimpl + def value_hook(value: object) -> object: + return value + + setattr(plugin, "hook", value_hook) + pm.register(plugin) + assert pm.hook.hook(value="caller") == ["caller"] + if constructor != "required": + assert pm.hook.hook() == ["fallback"] + + +def test_class_valued_hookimpl_is_still_ignored() -> None: + plugin = types.ModuleType("implementation") + + @hookimpl + class Implementation: + pass + + setattr(plugin, "hook", Implementation) + pm = PluginManager("example") + assert pm.parse_hookimpl_opts(plugin, "hook") is None + pm.register(plugin) + assert pm.get_hookcallers(plugin) == [] From 62f1b6eef176e8b2652ba6d7e2c73ff74d261d6d Mon Sep 17 00:00:00 2001 From: Xucheng Zhou Date: Thu, 15 Oct 2026 21:44:02 +0800 Subject: [PATCH 2/3] Align class hookspec tests with lint and coverage checks Signed-off-by: Xucheng Zhou --- testing/test_class_hookspec.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/testing/test_class_hookspec.py b/testing/test_class_hookspec.py index bfc9efa4..2587cda0 100644 --- a/testing/test_class_hookspec.py +++ b/testing/test_class_hookspec.py @@ -23,11 +23,11 @@ class Implicit: class Required: def __init__(self, value: object) -> None: - raise AssertionError("hookspec constructors must not execute") + raise AssertionError("hookspec constructors must not execute") # pragma: no cover class Default: def __init__(self, value: object = "fallback") -> None: - raise AssertionError("hookspec constructors must not execute") + raise AssertionError("hookspec constructors must not execute") # pragma: no cover class Inherited(Default): pass @@ -35,7 +35,7 @@ class Inherited(Default): class Wrapped: @wraps(Default.__init__) # type: ignore[misc] def __init__(self, *args: object, **kwargs: object) -> None: - raise AssertionError("hookspec constructors must not execute") + raise AssertionError("hookspec constructors must not execute") # pragma: no cover specification = hookspec( { @@ -49,7 +49,7 @@ def __init__(self, *args: object, **kwargs: object) -> None: namespace: Any if namespace_kind == "module": namespace = types.ModuleType("specification") - setattr(namespace, "hook", specification) + namespace.hook = specification else: namespace_class = type("Specification", (), {"hook": specification}) namespace = namespace_class if namespace_kind == "class" else namespace_class() @@ -66,21 +66,23 @@ def __init__(self, *args: object, **kwargs: object) -> None: {} if constructor in ("implicit", "required") else {"value": "fallback"} ) - plugin = types.ModuleType("implementation") + plugin: Any = types.ModuleType("implementation") if constructor == "implicit": + @hookimpl def empty_hook() -> str: return "control" - setattr(plugin, "hook", empty_hook) + plugin.hook = empty_hook pm.register(plugin) assert pm.hook.hook() == ["control"] else: + @hookimpl def value_hook(value: object) -> object: return value - setattr(plugin, "hook", value_hook) + plugin.hook = value_hook pm.register(plugin) assert pm.hook.hook(value="caller") == ["caller"] if constructor != "required": @@ -88,13 +90,13 @@ def value_hook(value: object) -> object: def test_class_valued_hookimpl_is_still_ignored() -> None: - plugin = types.ModuleType("implementation") + plugin: Any = types.ModuleType("implementation") @hookimpl class Implementation: pass - setattr(plugin, "hook", Implementation) + plugin.hook = Implementation pm = PluginManager("example") assert pm.parse_hookimpl_opts(plugin, "hook") is None pm.register(plugin) From 75a891465a1b466e9c5bb78642e8d4af24064089 Mon Sep 17 00:00:00 2001 From: Xucheng Zhou Date: Thu, 15 Oct 2026 21:45:59 +0800 Subject: [PATCH 3/3] Keep unreachable constructor guards within line limit Signed-off-by: Xucheng Zhou --- testing/test_class_hookspec.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/test_class_hookspec.py b/testing/test_class_hookspec.py index 2587cda0..cf73eb35 100644 --- a/testing/test_class_hookspec.py +++ b/testing/test_class_hookspec.py @@ -23,11 +23,11 @@ class Implicit: class Required: def __init__(self, value: object) -> None: - raise AssertionError("hookspec constructors must not execute") # pragma: no cover + raise AssertionError("constructor executed") # pragma: no cover class Default: def __init__(self, value: object = "fallback") -> None: - raise AssertionError("hookspec constructors must not execute") # pragma: no cover + raise AssertionError("constructor executed") # pragma: no cover class Inherited(Default): pass @@ -35,7 +35,7 @@ class Inherited(Default): class Wrapped: @wraps(Default.__init__) # type: ignore[misc] def __init__(self, *args: object, **kwargs: object) -> None: - raise AssertionError("hookspec constructors must not execute") # pragma: no cover + raise AssertionError("constructor executed") # pragma: no cover specification = hookspec( {