Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support deprecating hook parameters #495

Merged
merged 1 commit into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.
27 changes: 25 additions & 2 deletions docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

bluetech marked this conversation as resolved.
Show resolved Hide resolved
.. 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
Expand Down
18 changes: 18 additions & 0 deletions src/pluggy/_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@
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>`.

Check warning on line 52 in src/pluggy/_hooks.py

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_hooks.py#L52

Added line #L52 was not covered by tests
#:
#: .. versionadded:: 1.5
warn_on_impl_args: Mapping[str, Warning] | None


class HookimplOpts(TypedDict):
Expand Down Expand Up @@ -92,6 +97,7 @@
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 +107,7 @@
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 +116,7 @@
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 +136,13 @@
: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`.

Check warning on line 143 in src/pluggy/_hooks.py

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_hooks.py#L142-L143

Added lines #L142 - L143 were not covered by tests

.. versionadded:: 1.5
"""

def setattr_hookspec_opts(func: _F) -> _F:
Expand All @@ -137,6 +152,7 @@
"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 +702,7 @@
"kwargnames",
"opts",
"warn_on_impl",
"warn_on_impl_args",
)

def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None:
Expand All @@ -695,3 +712,4 @@
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):
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).
Expand Down