Merge pull request #495 from bluetech/warn-on-impl-args

Add support deprecating hook parameters

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.

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

src/pluggy/_hooks.py

@@ -48,6 +48,11 @@ 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>`.
+    #:
+    #: .. 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")

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):

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