diff --git a/changelog/178.feature.rst b/changelog/178.feature.rst new file mode 100644 index 00000000..5bcf64da --- /dev/null +++ b/changelog/178.feature.rst @@ -0,0 +1,3 @@ +Add support for deprecating specific hook parameters, or more generally, for issuing a warning whenever a hook implementation requests certain parameters. + +See :ref:`warn_on_impl` for details. diff --git a/docs/index.rst b/docs/index.rst index d5b18ca6..b98c4956 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -659,11 +659,34 @@ If a hookspec specifies a ``warn_on_impl``, pluggy will trigger it for any plugi .. code-block:: python @hookspec( - warn_on_impl=DeprecationWarning("oldhook is deprecated and will be removed soon") + warn_on_impl=DeprecationWarning("old_hook is deprecated and will be removed soon") ) - def oldhook(): + def old_hook(): pass + +If you don't want to deprecate implementing the entire hook, but just specific +parameters of it, you can specify ``warn_on_impl_args``, a dict mapping +parameter names to warnings. The warnings will trigger whenever any plugin +implements the hook requesting one of the specified parameters. + +.. code-block:: python + + @hookspec( + warn_on_impl_args={ + "lousy_arg": DeprecationWarning( + "The lousy_arg parameter of refreshed_hook is deprecated and will be removed soon; " + "use awesome_arg instead" + ), + }, + ) + def refreshed_hook(lousy_arg, awesome_arg): + pass + +.. versionadded:: 1.5 + The ``warn_on_impl_args`` parameter. + + .. _manage: The Plugin registry diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index 72c57dbc..362d7918 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -48,6 +48,11 @@ class HookspecOpts(TypedDict): 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): @@ -92,6 +97,7 @@ def __call__( firstresult: bool = False, historic: bool = False, warn_on_impl: Warning | None = None, + warn_on_impl_args: Mapping[str, Warning] | None = None, ) -> _F: ... @overload # noqa: F811 @@ -101,6 +107,7 @@ def __call__( # noqa: F811 firstresult: bool = ..., historic: bool = ..., warn_on_impl: Warning | None = ..., + warn_on_impl_args: Mapping[str, Warning] | None = ..., ) -> Callable[[_F], _F]: ... def __call__( # noqa: F811 @@ -109,6 +116,7 @@ def __call__( # noqa: F811 firstresult: bool = False, historic: bool = False, warn_on_impl: Warning | None = None, + warn_on_impl_args: Mapping[str, Warning] | None = None, ) -> _F | Callable[[_F], _F]: """If passed a function, directly sets attributes on the function which will make it discoverable to :meth:`PluginManager.add_hookspecs`. @@ -128,6 +136,13 @@ def __call__( # noqa: F811 :param warn_on_impl: If given, every implementation of this hook will trigger the given warning. See :ref:`warn_on_impl`. + + :param warn_on_impl_args: + If given, every implementation of this hook which requests one of + the arguments in the dict will trigger the corresponding warning. + See :ref:`warn_on_impl`. + + .. versionadded:: 1.5 """ def setattr_hookspec_opts(func: _F) -> _F: @@ -137,6 +152,7 @@ def setattr_hookspec_opts(func: _F) -> _F: "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) return func @@ -686,6 +702,7 @@ class HookSpec: "kwargnames", "opts", "warn_on_impl", + "warn_on_impl_args", ) def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None: @@ -695,3 +712,4 @@ def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None self.argnames, self.kwargnames = varnames(self.function) self.opts = opts self.warn_on_impl = opts.get("warn_on_impl") + self.warn_on_impl_args = opts.get("warn_on_impl_args") diff --git a/src/pluggy/_manager.py b/src/pluggy/_manager.py index 13a57da1..9998dd81 100644 --- a/src/pluggy/_manager.py +++ b/src/pluggy/_manager.py @@ -353,6 +353,12 @@ def _verify_hook(self, hook: HookCaller, hookimpl: HookImpl) -> None: ), ) + if hook.spec.warn_on_impl_args: + for hookimpl_argname in hookimpl.argnames: + argname_warning = hook.spec.warn_on_impl_args.get(hookimpl_argname) + if argname_warning is not None: + _warn_for_function(argname_warning, hookimpl.function) + if ( hookimpl.wrapper or hookimpl.hookwrapper ) and not inspect.isgeneratorfunction(hookimpl.function): diff --git a/testing/test_details.py b/testing/test_details.py index 71c77471..9b68a081 100644 --- a/testing/test_details.py +++ b/testing/test_details.py @@ -93,6 +93,39 @@ def foo(self): assert record.lineno == Plugin.foo.__code__.co_firstlineno +def test_warn_when_deprecated_args_specified(recwarn) -> None: + warning1 = DeprecationWarning("old1 is deprecated") + warning2 = DeprecationWarning("old2 is deprecated") + + class Spec: + @hookspec( + warn_on_impl_args={ + "old1": warning1, + "old2": warning2, + }, + ) + def foo(self, old1, new, old2): + raise NotImplementedError() + + class Plugin: + @hookimpl + def foo(self, old2, old1, new): + raise NotImplementedError() + + pm = PluginManager(hookspec.project_name) + pm.add_hookspecs(Spec) + + with pytest.warns(DeprecationWarning) as records: + pm.register(Plugin()) + (record1, record2) = records + assert record1.message is warning2 + assert record1.filename == Plugin.foo.__code__.co_filename + assert record1.lineno == Plugin.foo.__code__.co_firstlineno + assert record2.message is warning1 + assert record2.filename == Plugin.foo.__code__.co_filename + assert record2.lineno == Plugin.foo.__code__.co_firstlineno + + def test_plugin_getattr_raises_errors() -> None: """Pluggy must be able to handle plugins which raise weird exceptions when getattr() gets called (#11).