Skip to content

Commit

Permalink
Add support deprecating hook parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
bluetech committed Apr 18, 2024
1 parent 29f104d commit 709c784
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 2 deletions.
3 changes: 3 additions & 0 deletions changelog/178.feature.rst
Original file line number Diff line number Diff line change
@@ -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.
22 changes: 20 additions & 2 deletions docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -659,11 +659,29 @@ 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 the entire hook, but just specific parameters of it,
you can specify ``warn_on_impl_args``, a dict mapping parameter names to warnings.

.. 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
.. _manage:

The Plugin registry
Expand Down
14 changes: 14 additions & 0 deletions src/pluggy/_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class HookspecOpts(TypedDict):
historic: bool
#: Whether the hook :ref:`warns when implemented <warn_on_impl>`.
warn_on_impl: Warning | None
#: Whether the hook warns when :ref:`certain arguments are requested
#: <warn_on_impl>`.
warn_on_impl_args: Mapping[str, Warning] | None


class HookimplOpts(TypedDict):
Expand Down Expand Up @@ -92,6 +95,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
Expand All @@ -101,6 +105,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
Expand All @@ -109,6 +114,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`.
Expand All @@ -128,6 +134,11 @@ 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`.
"""

def setattr_hookspec_opts(func: _F) -> _F:
Expand All @@ -137,6 +148,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
Expand Down Expand Up @@ -686,6 +698,7 @@ class HookSpec:
"kwargnames",
"opts",
"warn_on_impl",
"warn_on_impl_args",
)

def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None:
Expand All @@ -695,3 +708,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")
6 changes: 6 additions & 0 deletions src/pluggy/_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
33 changes: 33 additions & 0 deletions testing/test_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
pass

class Plugin:
@hookimpl
def foo(self, old2, old1, new):
pass

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

0 comments on commit 709c784

Please sign in to comment.