From 8b86b10fbb522c54307218430dcac4510e1ed4a2 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 22 Jul 2026 14:40:06 +0200 Subject: [PATCH] feat(config): replace TypedDict options with Hook*Configuration Markers attach HookspecConfiguration/HookimplConfiguration objects. Registration discovers those privately; parse_hookimpl_opts and parse_hookspec_opts remain a deprecated pytest concession that returns legacy dicts and is only called when a subclass overrides them and no modern configuration attribute was found. Co-authored-by: Cursor AI Co-authored-by: Cursor Grok 4.5 --- changelog/704.removal.rst | 8 + docs/api_reference.rst | 6 +- docs/index.rst | 9 +- src/pluggy/__init__.py | 8 +- src/pluggy/_caller.py | 23 +-- src/pluggy/_config.py | 207 +++++++++++++++++++------ src/pluggy/_decorators.py | 46 +++--- src/pluggy/_hooks.py | 10 +- src/pluggy/_impl.py | 17 ++- src/pluggy/_manager.py | 168 +++++++++++++------- src/pluggy/_pytest_compat.py | 55 +++++++ testing/test_configuration.py | 278 ++++++++++++++++++++++++++++++++++ testing/test_details.py | 8 +- testing/test_hookcaller.py | 8 +- 14 files changed, 685 insertions(+), 166 deletions(-) create mode 100644 changelog/704.removal.rst create mode 100644 src/pluggy/_pytest_compat.py create mode 100644 testing/test_configuration.py diff --git a/changelog/704.removal.rst b/changelog/704.removal.rst new file mode 100644 index 00000000..4d5eef43 --- /dev/null +++ b/changelog/704.removal.rst @@ -0,0 +1,8 @@ +Hook options are now :class:`pluggy.HookspecConfiguration` / +:class:`pluggy.HookimplConfiguration` objects (markers attach these instead of +dicts). ``PluginManager.parse_hookimpl_opts`` / +``parse_hookspec_opts`` remain as a deprecated pytest/support concession that +returns legacy dicts and are only invoked during registration when a subclass +overrides them and no modern configuration attribute was found. +``HookspecOpts`` / ``HookimplOpts`` TypedDicts remain importable for +pytest/typing compatibility. diff --git a/docs/api_reference.rst b/docs/api_reference.rst index b14d725d..7d19a4a6 100644 --- a/docs/api_reference.rst +++ b/docs/api_reference.rst @@ -40,12 +40,10 @@ API Reference .. autoclass:: pluggy.HookImpl() :members: -.. autoclass:: pluggy.HookspecOpts() - :show-inheritance: +.. autoclass:: pluggy.HookspecConfiguration() :members: -.. autoclass:: pluggy.HookimplOpts() - :show-inheritance: +.. autoclass:: pluggy.HookimplConfiguration() :members: diff --git a/docs/index.rst b/docs/index.rst index 9d56f019..99e33b51 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -800,10 +800,13 @@ and particular plugins in it: Parsing mark options ^^^^^^^^^^^^^^^^^^^^ -You can retrieve the *options* applied to a particular -*hookspec* or *hookimpl* as per :ref:`marking_hooks` using the +Markers attach :class:`~pluggy.HookspecConfiguration` / +:class:`~pluggy.HookimplConfiguration` objects to functions. The :py:meth:`~pluggy.PluginManager.parse_hookspec_opts()` and -:py:meth:`~pluggy.PluginManager.parse_hookimpl_opts()` respectively. +:py:meth:`~pluggy.PluginManager.parse_hookimpl_opts()` methods remain as a +**deprecated** pytest/support concession that returns legacy dict-shaped +options; registration only calls them when a subclass overrides them and no +modern configuration attribute was found. .. _calling: diff --git a/src/pluggy/__init__.py b/src/pluggy/__init__.py index 32c7eae5..83064d90 100644 --- a/src/pluggy/__init__.py +++ b/src/pluggy/__init__.py @@ -3,8 +3,10 @@ "HookCaller", "HookImpl", "HookRelay", + "HookimplConfiguration", "HookimplMarker", "HookimplOpts", + "HookspecConfiguration", "HookspecMarker", "HookspecOpts", "PluggyTeardownRaisedWarning", @@ -14,15 +16,17 @@ "Result", "__version__", ] +from ._config import HookimplConfiguration +from ._config import HookspecConfiguration from ._hooks import HookCaller from ._hooks import HookImpl from ._hooks import HookimplMarker -from ._hooks import HookimplOpts from ._hooks import HookRelay from ._hooks import HookspecMarker -from ._hooks import HookspecOpts from ._manager import PluginManager from ._manager import PluginValidationError +from ._pytest_compat import HookimplOpts +from ._pytest_compat import HookspecOpts from ._result import HookCallError from ._result import Result from ._warnings import PluggyTeardownRaisedWarning diff --git a/src/pluggy/_caller.py b/src/pluggy/_caller.py index 52b02316..8295982c 100644 --- a/src/pluggy/_caller.py +++ b/src/pluggy/_caller.py @@ -15,8 +15,8 @@ from typing import TypeAlias import warnings -from ._config import HookimplOpts -from ._config import HookspecOpts +from ._config import HookimplConfiguration +from ._config import HookspecConfiguration from ._decorators import _Namespace from ._decorators import HookSpec from ._impl import _Plugin @@ -69,7 +69,7 @@ def __init__( name: str, hook_execute: _HookExec, specmodule_or_class: _Namespace | None = None, - spec_opts: HookspecOpts | None = None, + spec_opts: HookspecConfiguration | None = None, ) -> None: """:meta private:""" #: Name of the hook getting called. @@ -98,7 +98,7 @@ def has_spec(self) -> bool: def set_specification( self, specmodule_or_class: _Namespace, - spec_opts: HookspecOpts, + spec_opts: HookspecConfiguration, ) -> None: if self.spec is not None: raise ValueError( @@ -106,7 +106,7 @@ def set_specification( f"within namespace {self.spec.namespace}" ) self.spec = HookSpec(specmodule_or_class, self.name, spec_opts) - if spec_opts.get("historic"): + if spec_opts.historic: self._call_history = [] def is_historic(self) -> bool: @@ -192,7 +192,7 @@ def __call__(self, **kwargs: object) -> Any: ) call_kwargs = self._apply_defaults(kwargs) self._verify_all_args_are_provided(call_kwargs) - firstresult = self.spec.opts.get("firstresult", False) if self.spec else False + firstresult = self.spec.opts.firstresult if self.spec else False # Copy because plugins may register other plugins during iteration (#438). return self._hookexec( self.name, self._hookimpls.copy(), call_kwargs, firstresult @@ -237,14 +237,7 @@ def call_extra( ) kwargs = self._apply_defaults(kwargs) self._verify_all_args_are_provided(kwargs) - opts: HookimplOpts = { - "wrapper": False, - "hookwrapper": False, - "optionalhook": False, - "trylast": False, - "tryfirst": False, - "specname": None, - } + opts = HookimplConfiguration() hookimpls = self._hookimpls.copy() for method in methods: hookimpl = HookImpl(None, "", method, opts) @@ -258,7 +251,7 @@ def call_extra( ): i -= 1 hookimpls.insert(i + 1, hookimpl) - firstresult = self.spec.opts.get("firstresult", False) if self.spec else False + firstresult = self.spec.opts.firstresult if self.spec else False return self._hookexec(self.name, hookimpls, kwargs, firstresult) def _maybe_apply_history(self, method: HookImpl) -> None: diff --git a/src/pluggy/_config.py b/src/pluggy/_config.py index 576a7794..48ead414 100644 --- a/src/pluggy/_config.py +++ b/src/pluggy/_config.py @@ -5,50 +5,163 @@ from __future__ import annotations from collections.abc import Mapping -from typing import TypedDict - - -class HookspecOpts(TypedDict): - """Options for a hook specification.""" - - #: Whether the hook is :ref:`first result only `. - firstresult: bool - #: Whether the hook is :ref:`historic `. - historic: bool - #: Whether the hook :ref:`warns when implemented `. - warn_on_impl: Warning | None - #: Whether the hook warns when :ref:`certain arguments are requested - #: `. - #: - #: .. versionadded:: 1.5 - warn_on_impl_args: Mapping[str, Warning] | None - - -class HookimplOpts(TypedDict): - """Options for a hook implementation.""" - - #: Whether the hook implementation is a :ref:`wrapper `. - wrapper: bool - #: Whether the hook implementation is an :ref:`old-style wrapper - #: `. - hookwrapper: bool - #: Whether validation against a hook specification is :ref:`optional - #: `. - optionalhook: bool - #: Whether to try to order this hook implementation :ref:`first - #: `. - tryfirst: bool - #: Whether to try to order this hook implementation :ref:`last - #: `. - trylast: bool - #: The name of the hook specification to match, see :ref:`specname`. - specname: str | None - - -def normalize_hookimpl_opts(opts: HookimplOpts) -> None: - opts.setdefault("tryfirst", False) - opts.setdefault("trylast", False) - opts.setdefault("wrapper", False) - opts.setdefault("hookwrapper", False) - opts.setdefault("optionalhook", False) - opts.setdefault("specname", None) +from typing import Any +from typing import Final +from typing import final + + +@final +class HookspecConfiguration: + """Configuration for a hook specification.""" + + __slots__ = ( + "firstresult", + "historic", + "warn_on_impl", + "warn_on_impl_args", + ) + firstresult: Final[bool] + historic: Final[bool] + warn_on_impl: Final[Warning | None] + warn_on_impl_args: Final[Mapping[str, Warning] | None] + + def __init__( + self, + firstresult: bool = False, + historic: bool = False, + warn_on_impl: Warning | None = None, + warn_on_impl_args: Mapping[str, Warning] | None = None, + ) -> None: + if historic and firstresult: + raise ValueError("cannot have a historic firstresult hook") + #: Whether the hook is :ref:`first result only `. + self.firstresult = firstresult + #: Whether the hook is :ref:`historic `. + self.historic = historic + #: Whether the hook :ref:`warns when implemented `. + self.warn_on_impl = warn_on_impl + #: Whether the hook warns when :ref:`certain arguments are requested + #: `. + self.warn_on_impl_args = warn_on_impl_args + + def __repr__(self) -> str: + attrs = [ + f"{slot}={getattr(self, slot)!r}" + for slot in self.__slots__ + if getattr(self, slot) + ] + return f"HookspecConfiguration({', '.join(attrs)})" + + +@final +class HookimplConfiguration: + """Configuration for a hook implementation.""" + + __slots__ = ( + "hookwrapper", + "optionalhook", + "specname", + "tryfirst", + "trylast", + "wrapper", + ) + wrapper: Final[bool] + hookwrapper: Final[bool] + optionalhook: Final[bool] + tryfirst: Final[bool] + trylast: Final[bool] + specname: Final[str | None] + + def __init__( + self, + wrapper: bool = False, + hookwrapper: bool = False, + optionalhook: bool = False, + tryfirst: bool = False, + trylast: bool = False, + specname: str | None = None, + ) -> None: + #: Whether the hook implementation is a :ref:`wrapper `. + self.wrapper = wrapper + #: Whether the hook implementation is an :ref:`old-style wrapper + #: `. + self.hookwrapper = hookwrapper + #: Whether validation against a hook specification is :ref:`optional + #: `. + self.optionalhook = optionalhook + #: Whether to try to order this hook implementation :ref:`first + #: `. + self.tryfirst = tryfirst + #: Whether to try to order this hook implementation :ref:`last + #: `. + self.trylast = trylast + #: The name of the hook specification to match, see :ref:`specname`. + self.specname = specname + + def __repr__(self) -> str: + attrs = [ + f"{slot}={getattr(self, slot)!r}" + for slot in self.__slots__ + if getattr(self, slot) + ] + return f"HookimplConfiguration({', '.join(attrs)})" + + +def hookspec_config_from_mapping( + opts: Mapping[str, Any], +) -> HookspecConfiguration: + """Build a :class:`HookspecConfiguration` from a mapping. + + Intended for pytest/support migration only — not the public options API. + Prefer constructing :class:`HookspecConfiguration` directly. + """ + return HookspecConfiguration( + firstresult=bool(opts.get("firstresult", False)), + historic=bool(opts.get("historic", False)), + warn_on_impl=opts.get("warn_on_impl"), + warn_on_impl_args=opts.get("warn_on_impl_args"), + ) + + +def hookimpl_config_from_mapping( + opts: Mapping[str, Any], +) -> HookimplConfiguration: + """Build a :class:`HookimplConfiguration` from a mapping. + + Intended for pytest/support migration only — not the public options API. + Prefer constructing :class:`HookimplConfiguration` directly. + """ + return HookimplConfiguration( + wrapper=bool(opts.get("wrapper", False)), + hookwrapper=bool(opts.get("hookwrapper", False)), + optionalhook=bool(opts.get("optionalhook", False)), + tryfirst=bool(opts.get("tryfirst", False)), + trylast=bool(opts.get("trylast", False)), + specname=opts.get("specname"), + ) + + +def hookspec_config_to_mapping( + config: HookspecConfiguration, +) -> dict[str, Any]: + """Serialize configuration to a legacy mapping (pytest/support only).""" + return { + "firstresult": config.firstresult, + "historic": config.historic, + "warn_on_impl": config.warn_on_impl, + "warn_on_impl_args": config.warn_on_impl_args, + } + + +def hookimpl_config_to_mapping( + config: HookimplConfiguration, +) -> dict[str, Any]: + """Serialize configuration to a legacy mapping (pytest/support only).""" + return { + "wrapper": config.wrapper, + "hookwrapper": config.hookwrapper, + "optionalhook": config.optionalhook, + "tryfirst": config.tryfirst, + "trylast": config.trylast, + "specname": config.specname, + } diff --git a/src/pluggy/_decorators.py b/src/pluggy/_decorators.py index c5574a67..08342702 100644 --- a/src/pluggy/_decorators.py +++ b/src/pluggy/_decorators.py @@ -17,8 +17,8 @@ from typing import TypeVar import warnings -from ._config import HookimplOpts -from ._config import HookspecOpts +from ._config import HookimplConfiguration +from ._config import HookspecConfiguration _F = TypeVar("_F", bound=Callable[..., object]) @@ -96,15 +96,13 @@ def __call__( """ def setattr_hookspec_opts(func: _F) -> _F: - if historic and firstresult: - raise ValueError("cannot have a historic firstresult hook") - opts: HookspecOpts = { - "firstresult": firstresult, - "historic": historic, - "warn_on_impl": warn_on_impl, - "warn_on_impl_args": warn_on_impl_args, - } - setattr(func, self.project_name + "_spec", opts) + config = HookspecConfiguration( + firstresult=firstresult, + historic=historic, + warn_on_impl=warn_on_impl, + warn_on_impl_args=warn_on_impl_args, + ) + setattr(func, self.project_name + "_spec", config) return func if function is not None: @@ -213,15 +211,15 @@ def __call__( """ def setattr_hookimpl_opts(func: _F) -> _F: - opts: HookimplOpts = { - "wrapper": wrapper, - "hookwrapper": hookwrapper, - "optionalhook": optionalhook, - "tryfirst": tryfirst, - "trylast": trylast, - "specname": specname, - } - setattr(func, self.project_name + "_impl", opts) + config = HookimplConfiguration( + wrapper=wrapper, + hookwrapper=hookwrapper, + optionalhook=optionalhook, + tryfirst=tryfirst, + trylast=trylast, + specname=specname, + ) + setattr(func, self.project_name + "_impl", config) return func if function is None: @@ -339,7 +337,9 @@ class HookSpec: "warn_on_impl_args", ) - def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None: + def __init__( + self, namespace: _Namespace, name: str, opts: HookspecConfiguration + ) -> None: self.namespace = namespace self.name = name self.function: Callable[..., object] = getattr(namespace, name) @@ -352,5 +352,5 @@ def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None defaults = inspect.unwrap(self.function).__defaults__ self.kwargdefaults = dict(zip(self.kwargnames, defaults or ())) self.opts = opts - self.warn_on_impl = opts.get("warn_on_impl") - self.warn_on_impl_args = opts.get("warn_on_impl_args") + self.warn_on_impl = opts.warn_on_impl + self.warn_on_impl_args = opts.warn_on_impl_args diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index b57aae66..55197772 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -13,9 +13,8 @@ from ._caller import _SubsetHookCaller from ._caller import HookCaller from ._caller import HookRelay -from ._config import HookimplOpts -from ._config import HookspecOpts -from ._config import normalize_hookimpl_opts +from ._config import HookimplConfiguration +from ._config import HookspecConfiguration from ._decorators import _Namespace from ._decorators import HookimplMarker from ._decorators import HookSpec @@ -31,10 +30,10 @@ "HookImpl", "HookRelay", "HookSpec", + "HookimplConfiguration", "HookimplMarker", - "HookimplOpts", + "HookspecConfiguration", "HookspecMarker", - "HookspecOpts", "_HookCaller", "_HookExec", "_HookImplFunction", @@ -42,6 +41,5 @@ "_Namespace", "_Plugin", "_SubsetHookCaller", - "normalize_hookimpl_opts", "varnames", ] diff --git a/src/pluggy/_impl.py b/src/pluggy/_impl.py index 4f82ebe5..5a2043b6 100644 --- a/src/pluggy/_impl.py +++ b/src/pluggy/_impl.py @@ -11,7 +11,7 @@ from typing import TypeAlias from typing import TypeVar -from ._config import HookimplOpts +from ._config import HookimplConfiguration from ._decorators import varnames from ._result import Result @@ -45,7 +45,7 @@ def __init__( plugin: _Plugin, plugin_name: str, function: _HookImplFunction[object], - hook_impl_opts: HookimplOpts, + hook_impl_opts: HookimplConfiguration, ) -> None: """:meta private:""" #: The hook implementation function. @@ -57,24 +57,25 @@ def __init__( self.kwargnames: Final = kwargnames #: The plugin which defined this hook implementation. self.plugin: Final = plugin - #: The :class:`HookimplOpts` used to configure this hook implementation. + #: The :class:`HookimplConfiguration` used to configure this hook + #: implementation. self.opts: Final = hook_impl_opts #: The name of the plugin which defined this hook implementation. self.plugin_name: Final = plugin_name #: Whether the hook implementation is a :ref:`wrapper `. - self.wrapper: Final = hook_impl_opts["wrapper"] + self.wrapper: Final = hook_impl_opts.wrapper #: Whether the hook implementation is an :ref:`old-style wrapper #: `. - self.hookwrapper: Final = hook_impl_opts["hookwrapper"] + self.hookwrapper: Final = hook_impl_opts.hookwrapper #: Whether validation against a hook specification is :ref:`optional #: `. - self.optionalhook: Final = hook_impl_opts["optionalhook"] + self.optionalhook: Final = hook_impl_opts.optionalhook #: Whether to try to order this hook implementation :ref:`first #: `. - self.tryfirst: Final = hook_impl_opts["tryfirst"] + self.tryfirst: Final = hook_impl_opts.tryfirst #: Whether to try to order this hook implementation :ref:`last #: `. - self.trylast: Final = hook_impl_opts["trylast"] + self.trylast: Final = hook_impl_opts.trylast def __repr__(self) -> str: return f"" diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py index 080ef333..d003d24b 100644 --- a/src/pluggy/_manager.py +++ b/src/pluggy/_manager.py @@ -15,16 +15,21 @@ from . import _tracing from ._callers import _multicall +from ._config import hookimpl_config_from_mapping +from ._config import hookimpl_config_to_mapping +from ._config import HookimplConfiguration +from ._config import hookspec_config_from_mapping +from ._config import hookspec_config_to_mapping +from ._config import HookspecConfiguration from ._hooks import _HookImplFunction from ._hooks import _Namespace from ._hooks import _Plugin from ._hooks import _SubsetHookCaller from ._hooks import HookCaller from ._hooks import HookImpl -from ._hooks import HookimplOpts from ._hooks import HookRelay -from ._hooks import HookspecOpts -from ._hooks import normalize_hookimpl_opts +from ._pytest_compat import HookimplOpts +from ._pytest_compat import HookspecOpts from ._result import Result @@ -113,16 +118,19 @@ def _static_hook_attr( return None -def _get_marker_opts(holder: object, attrname: str) -> dict[str, Any] | None: - """Read marker options from ``holder``, falling back to ``__func__``.""" - opts = getattr(holder, attrname, None) - if opts is not None: - return opts if isinstance(opts, dict) else None - func = getattr(holder, "__func__", None) +def _get_marker_attr(holder: object, attrname: str) -> object | None: + """Read a marker attribute from ``holder``, falling back to ``__func__``. + + The marker sits on the ``classmethod``/``staticmethod`` wrapper when it was + applied above it, and on the wrapped function when applied below. + """ + marker: object = getattr(holder, attrname, None) + if marker is not None: + return marker + func: object = getattr(holder, "__func__", None) if func is not None: - opts = getattr(func, attrname, None) - if opts is not None: - return opts if isinstance(opts, dict) else None + wrapped: object = getattr(func, attrname, None) + return wrapped return None @@ -221,9 +229,8 @@ def register(self, plugin: _Plugin, name: str | None = None) -> str | None: # register matching hook implementations of the plugin for attr_name in dir(plugin): - hookimpl_opts = self.parse_hookimpl_opts(plugin, attr_name) - if hookimpl_opts is not None: - normalize_hookimpl_opts(hookimpl_opts) + hookimpl_config = self._discover_hookimpl_configuration(plugin, attr_name) + if hookimpl_config is not None: found = _static_hook_attr(plugin, attr_name) # Only reachable when a subclass overrode parse_hookimpl_opts # to claim an attribute pluggy cannot bind. @@ -231,8 +238,8 @@ def register(self, plugin: _Plugin, name: str | None = None) -> str | None: f"{plugin!r}.{attr_name} is not a hookable attribute" ) method: _HookImplFunction[object] = found[1] - hookimpl = HookImpl(plugin, plugin_name, method, hookimpl_opts) - hook_name = hookimpl_opts.get("specname") or attr_name + hookimpl = HookImpl(plugin, plugin_name, method, hookimpl_config) + hook_name = hookimpl_config.specname or attr_name hook: HookCaller | None = getattr(self.hook, hook_name, None) if hook is None: hook = HookCaller(hook_name, self._hookexec) @@ -243,16 +250,10 @@ def register(self, plugin: _Plugin, name: str | None = None) -> str | None: hook._add_hookimpl(hookimpl) return plugin_name - def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None: - """Try to obtain a hook implementation from an item with the given name - in the given plugin which is being searched for hook impls. - - :returns: - The parsed hookimpl options, or None to skip the given item. - - This method can be overridden by ``PluginManager`` subclasses to - customize how hook implementation are picked up. By default, returns the - options for items decorated with :class:`HookimplMarker`. + def _read_hookimpl_configuration( + self, plugin: _Plugin, name: str + ) -> HookimplConfiguration | None: + """Read a modern :class:`HookimplConfiguration` from a plugin attribute. Discovery uses :func:`inspect.getattr_static` so properties and other descriptors are not executed. Only functions, classmethods, @@ -261,10 +262,46 @@ def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None found = _static_hook_attr(plugin, name) if found is None: return None - return cast( - HookimplOpts | None, - _get_marker_opts(found[0], self.project_name + "_impl"), - ) + res = _get_marker_attr(found[0], self.project_name + "_impl") + if isinstance(res, HookimplConfiguration): + return res + if isinstance(res, Mapping): + return hookimpl_config_from_mapping(res) + return None + + def _discover_hookimpl_configuration( + self, plugin: _Plugin, name: str + ) -> HookimplConfiguration | None: + """Discover hookimpl configuration for registration. + + Prefer the modern marker attribute. Only call the deprecated + :meth:`parse_hookimpl_opts` when a subclass actually overrides it and + no modern configuration was found (pytest unmarked-hook concession). + """ + config = self._read_hookimpl_configuration(plugin, name) + if config is not None: + return config + parse_hookimpl_opts = type(self).parse_hookimpl_opts + if parse_hookimpl_opts is PluginManager.parse_hookimpl_opts: + return None + legacy = parse_hookimpl_opts(self, plugin, name) + if legacy is None: + return None + return hookimpl_config_from_mapping(legacy) + + def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None: + """Return legacy dict-shaped hookimpl options, if any. + + .. deprecated:: + Thin pytest/support concession. Registration uses private discovery + of :class:`HookimplConfiguration` and only invokes this method when + a subclass overrides it and no modern configuration attribute was + found. Prefer marker-attached configuration objects. + """ + config = self._read_hookimpl_configuration(plugin, name) + if config is None: + return None + return cast(HookimplOpts, hookimpl_config_to_mapping(config)) def unregister( self, plugin: _Plugin | None = None, name: str | None = None @@ -325,15 +362,15 @@ def add_hookspecs(self, module_or_class: _Namespace) -> None: """ names = [] for name in dir(module_or_class): - spec_opts = self.parse_hookspec_opts(module_or_class, name) - if spec_opts is not None: + spec_config = self._discover_hookspec_configuration(module_or_class, name) + if spec_config is not None: hc: HookCaller | None = getattr(self.hook, name, None) if hc is None: - hc = HookCaller(name, self._hookexec, module_or_class, spec_opts) + hc = HookCaller(name, self._hookexec, module_or_class, spec_config) setattr(self.hook, name, hc) else: # Plugins registered this hook without knowing the spec. - hc.set_specification(module_or_class, spec_opts) + hc.set_specification(module_or_class, spec_config) for hookfunction in hc.get_hookimpls(): self._verify_hook(hc, hookfunction) names.append(name) @@ -343,19 +380,10 @@ def add_hookspecs(self, module_or_class: _Namespace) -> None: f"did not find any {self.project_name!r} hooks in {module_or_class!r}" ) - def parse_hookspec_opts( + def _read_hookspec_configuration( self, module_or_class: _Namespace, name: str - ) -> HookspecOpts | None: - """Try to obtain a hook specification from an item with the given name - in the given module or class which is being searched for hook specs. - - :returns: - The parsed hookspec options for defining a hook, or None to skip the - given item. - - This method can be overridden by ``PluginManager`` subclasses to - customize how hook specifications are picked up. By default, returns the - options for items decorated with :class:`HookspecMarker`. + ) -> HookspecConfiguration | None: + """Read a modern :class:`HookspecConfiguration` from a marked function. Discovery uses :func:`inspect.getattr_static` so properties and other descriptors are not executed. Only functions, classmethods, @@ -364,10 +392,48 @@ def parse_hookspec_opts( found = _static_hook_attr(module_or_class, name) if found is None: return None - return cast( - HookspecOpts | None, - _get_marker_opts(found[0], self.project_name + "_spec"), - ) + opts = _get_marker_attr(found[0], self.project_name + "_spec") + if isinstance(opts, HookspecConfiguration): + return opts + if isinstance(opts, Mapping): + return hookspec_config_from_mapping(opts) + return None + + def _discover_hookspec_configuration( + self, module_or_class: _Namespace, name: str + ) -> HookspecConfiguration | None: + """Discover hookspec configuration for ``add_hookspecs``. + + Prefer the modern marker attribute. Only call the deprecated + :meth:`parse_hookspec_opts` when a subclass actually overrides it and + no modern configuration was found. + """ + config = self._read_hookspec_configuration(module_or_class, name) + if config is not None: + return config + parse_hookspec_opts = type(self).parse_hookspec_opts + if parse_hookspec_opts is PluginManager.parse_hookspec_opts: + return None + legacy = parse_hookspec_opts(self, module_or_class, name) + if legacy is None: + return None + return hookspec_config_from_mapping(legacy) + + def parse_hookspec_opts( + self, module_or_class: _Namespace, name: str + ) -> HookspecOpts | None: + """Return legacy dict-shaped hookspec options, if any. + + .. deprecated:: + Thin pytest/support concession. ``add_hookspecs`` uses private + discovery of :class:`HookspecConfiguration` and only invokes this + method when a subclass overrides it and no modern configuration + attribute was found. Prefer marker-attached configuration objects. + """ + config = self._read_hookspec_configuration(module_or_class, name) + if config is None: + return None + return cast(HookspecOpts, hookspec_config_to_mapping(config)) def get_plugins(self) -> set[Any]: """Return a set of all registered plugin objects.""" diff --git a/src/pluggy/_pytest_compat.py b/src/pluggy/_pytest_compat.py new file mode 100644 index 00000000..e9d3b693 --- /dev/null +++ b/src/pluggy/_pytest_compat.py @@ -0,0 +1,55 @@ +"""Pytest/support compatibility helpers for legacy option encodings. + +The live pluggy API uses :class:`~pluggy.HookspecConfiguration` and +:class:`~pluggy.HookimplConfiguration`. This module keeps TypedDict shapes and +mapping conversion for pytest and other callers that still type or attach +dict-shaped options during migration. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from typing import TypedDict + +from ._config import hookimpl_config_from_mapping +from ._config import HookimplConfiguration +from ._config import hookspec_config_from_mapping +from ._config import HookspecConfiguration + + +class HookspecOpts(TypedDict): + """Legacy TypedDict for hook specification options. + + Prefer :class:`~pluggy.HookspecConfiguration`. Kept for pytest/typing + compatibility during migration. + """ + + firstresult: bool + historic: bool + warn_on_impl: Warning | None + warn_on_impl_args: Mapping[str, Warning] | None + + +class HookimplOpts(TypedDict): + """Legacy TypedDict for hook implementation options. + + Prefer :class:`~pluggy.HookimplConfiguration`. Kept for pytest/typing + compatibility during migration. + """ + + wrapper: bool + hookwrapper: bool + optionalhook: bool + tryfirst: bool + trylast: bool + specname: str | None + + +__all__ = [ + "HookimplConfiguration", + "HookimplOpts", + "HookspecConfiguration", + "HookspecOpts", + "hookimpl_config_from_mapping", + "hookspec_config_from_mapping", +] diff --git a/testing/test_configuration.py b/testing/test_configuration.py new file mode 100644 index 00000000..c7dbef0f --- /dev/null +++ b/testing/test_configuration.py @@ -0,0 +1,278 @@ +""" +Tests for configuration classes. +""" + +from __future__ import annotations + +import pytest + +from pluggy import HookimplConfiguration +from pluggy import HookimplMarker +from pluggy import HookspecConfiguration +from pluggy import HookspecMarker +from pluggy import PluginManager +from pluggy._config import hookimpl_config_from_mapping +from pluggy._config import hookspec_config_from_mapping + + +class TestHookspecConfiguration: + def test_basic_creation(self) -> None: + config = HookspecConfiguration() + assert config.firstresult is False + assert config.historic is False + assert config.warn_on_impl is None + assert config.warn_on_impl_args is None + + def test_firstresult(self) -> None: + config = HookspecConfiguration(firstresult=True) + assert config.firstresult is True + assert config.historic is False + + def test_historic(self) -> None: + config = HookspecConfiguration(historic=True) + assert config.firstresult is False + assert config.historic is True + + def test_historic_firstresult_validation(self) -> None: + with pytest.raises(ValueError, match="cannot have a historic firstresult"): + HookspecConfiguration(historic=True, firstresult=True) + + def test_warn_on_impl(self) -> None: + warning = UserWarning("test warning") + config = HookspecConfiguration(warn_on_impl=warning) + assert config.warn_on_impl is warning + + def test_warn_on_impl_args(self) -> None: + warnings_dict = {"arg1": UserWarning("arg1 warning")} + config = HookspecConfiguration(warn_on_impl_args=warnings_dict) + assert config.warn_on_impl_args is warnings_dict + + +class TestHookimplConfiguration: + def test_basic_creation(self) -> None: + config = HookimplConfiguration() + assert config.wrapper is False + assert config.hookwrapper is False + assert config.optionalhook is False + assert config.tryfirst is False + assert config.trylast is False + assert config.specname is None + + def test_wrapper(self) -> None: + config = HookimplConfiguration(wrapper=True) + assert config.wrapper is True + assert config.hookwrapper is False + + def test_hookwrapper(self) -> None: + config = HookimplConfiguration(hookwrapper=True) + assert config.wrapper is False + assert config.hookwrapper is True + + def test_both_wrappers_allowed(self) -> None: + """Both wrapper types are allowed at config level; validation is later.""" + config = HookimplConfiguration(wrapper=True, hookwrapper=True) + assert config.wrapper is True + assert config.hookwrapper is True + + def test_tryfirst(self) -> None: + config = HookimplConfiguration(tryfirst=True) + assert config.tryfirst is True + assert config.trylast is False + + def test_trylast(self) -> None: + config = HookimplConfiguration(trylast=True) + assert config.tryfirst is False + assert config.trylast is True + + def test_optionalhook(self) -> None: + config = HookimplConfiguration(optionalhook=True) + assert config.optionalhook is True + + def test_specname(self) -> None: + config = HookimplConfiguration(specname="custom_name") + assert config.specname == "custom_name" + + +class TestMappingShim: + def test_hookspec_config_from_mapping(self) -> None: + warning = UserWarning("w") + config = hookspec_config_from_mapping( + { + "firstresult": True, + "warn_on_impl": warning, + } + ) + assert config.firstresult is True + assert config.historic is False + assert config.warn_on_impl is warning + + def test_hookimpl_config_from_mapping(self) -> None: + config = hookimpl_config_from_mapping( + { + "tryfirst": True, + "specname": "other", + } + ) + assert config.tryfirst is True + assert config.specname == "other" + assert config.wrapper is False + + def test_read_hookimpl_accepts_legacy_dict_attribute(self) -> None: + pm = PluginManager("test") + + def method() -> str: + return "ok" + + # setattr, not attribute access: the marker is dynamic and must not + # be type-checked against the function object. + setattr(method, "test_impl", {"tryfirst": True}) # noqa: B010 + + class Plugin: + pass + + plugin = Plugin() + plugin.method = method # type: ignore[attr-defined] + config = pm._read_hookimpl_configuration(plugin, "method") + assert isinstance(config, HookimplConfiguration) + assert config.tryfirst is True + + def test_parse_hookimpl_opts_returns_legacy_dict(self) -> None: + hookimpl = HookimplMarker("test") + + @hookimpl(tryfirst=True) + def method() -> str: + return "ok" + + class Plugin: + pass + + plugin = Plugin() + plugin.method = method # type: ignore[attr-defined] + opts = PluginManager("test").parse_hookimpl_opts(plugin, "method") + assert opts == { + "wrapper": False, + "hookwrapper": False, + "optionalhook": False, + "tryfirst": True, + "trylast": False, + "specname": None, + } + + def test_read_hookspec_accepts_legacy_dict_attribute(self) -> None: + pm = PluginManager("test") + + class Spec: + def myhook(self) -> None: + pass + + setattr(Spec.myhook, "test_spec", {"firstresult": True}) # noqa: B010 + config = pm._read_hookspec_configuration(Spec, "myhook") + assert isinstance(config, HookspecConfiguration) + assert config.firstresult is True + + def test_discover_skips_parse_hookimpl_opts_unless_overridden(self) -> None: + calls: list[str] = [] + + class TrackingPluginManager(PluginManager): + def parse_hookimpl_opts(self, plugin: object, name: str): + calls.append(name) + return super().parse_hookimpl_opts(plugin, name) + + class Spec: + @HookspecMarker("test") + def marked(self) -> None: + pass + + def unmarked(self) -> None: + pass + + class Plugin: + @HookimplMarker("test") + def marked(self) -> str: + return "marked" + + def unmarked(self) -> str: + return "unmarked" + + pm = TrackingPluginManager("test") + pm.add_hookspecs(Spec) + pm.register(Plugin()) + # Marked impl is discovered privately; unmarked has no config and the + # override returns None, so parse_hookimpl_opts is only tried for names + # without a modern configuration attribute. + assert "marked" not in calls + assert "unmarked" in calls + + +def test_markers_attach_configuration_objects() -> None: + hookspec = HookspecMarker("test") + hookimpl = HookimplMarker("test") + + @hookspec(firstresult=True) + def myspec(arg: object) -> None: + pass + + @hookimpl(tryfirst=True) + def myimpl(arg: object) -> str: + return "x" + + spec_config = getattr(myspec, "test_spec") # noqa: B009 + impl_config = getattr(myimpl, "test_impl") # noqa: B009 + assert isinstance(spec_config, HookspecConfiguration) + assert spec_config.firstresult is True + assert isinstance(impl_config, HookimplConfiguration) + assert impl_config.tryfirst is True + + +def test_config_integration_with_hooks() -> None: + pm = PluginManager("test") + hookspec = HookspecMarker("test") + hookimpl = HookimplMarker("test") + + class MySpec: + @hookspec(firstresult=True) + def myhook(self, arg: object) -> None: + pass + + class Plugin1: + @hookimpl(trylast=True) + def myhook(self, arg: object) -> str: + return f"plugin1: {arg}" + + class Plugin2: + @hookimpl(tryfirst=True) + def myhook(self, arg: object) -> str: + return f"plugin2: {arg}" + + pm.add_hookspecs(MySpec) + pm.register(Plugin1()) + pm.register(Plugin2()) + + result = pm.hook.myhook(arg="test") + assert result == "plugin2: test" + + +def test_historic_hook_configuration() -> None: + pm = PluginManager("test") + hookspec = HookspecMarker("test") + hookimpl = HookimplMarker("test") + + results: list[str] = [] + + class MySpec: + @hookspec(historic=True) + def myhook(self, arg: object) -> None: + pass + + pm.add_hookspecs(MySpec) + pm.hook.myhook.call_historic( + kwargs={"arg": "call1"}, result_callback=results.append + ) + + class Plugin1: + @hookimpl + def myhook(self, arg: object) -> str: + return f"plugin1: {arg}" + + pm.register(Plugin1()) + assert "plugin1: call1" in results diff --git a/testing/test_details.py b/testing/test_details.py index 8df167f4..c73e43a5 100644 --- a/testing/test_details.py +++ b/testing/test_details.py @@ -16,9 +16,11 @@ def test_parse_hookimpl_override() -> None: class MyPluginManager(PluginManager): def parse_hookimpl_opts(self, module_or_class, name): opts = PluginManager.parse_hookimpl_opts(self, module_or_class, name) - if opts is None and name.startswith("x1"): - opts = {} # type: ignore[assignment] - return opts + if opts is not None: + return opts + if name.startswith("x1"): + return {} + return None class Plugin: def x1meth(self): diff --git a/testing/test_hookcaller.py b/testing/test_hookcaller.py index cdf79ca0..4a365c45 100644 --- a/testing/test_hookcaller.py +++ b/testing/test_hookcaller.py @@ -317,11 +317,11 @@ def he_myhook3(self, arg1) -> None: pm.add_hookspecs(HookSpec) assert pm.hook.he_myhook1.spec is not None - assert not pm.hook.he_myhook1.spec.opts["firstresult"] + assert not pm.hook.he_myhook1.spec.opts.firstresult assert pm.hook.he_myhook2.spec is not None - assert pm.hook.he_myhook2.spec.opts["firstresult"] + assert pm.hook.he_myhook2.spec.opts.firstresult assert pm.hook.he_myhook3.spec is not None - assert not pm.hook.he_myhook3.spec.opts["firstresult"] + assert not pm.hook.he_myhook3.spec.opts.firstresult @pytest.mark.parametrize("name", ["hookwrapper", "optionalhook", "tryfirst", "trylast"]) @@ -332,7 +332,7 @@ def he_myhook1(arg1) -> None: pass if val: - assert he_myhook1.example_impl.get(name) + assert getattr(he_myhook1.example_impl, name) else: assert not hasattr(he_myhook1, name)