diff --git a/changelog/774.removal.rst b/changelog/774.removal.rst new file mode 100644 index 00000000..7382395b --- /dev/null +++ b/changelog/774.removal.rst @@ -0,0 +1,3 @@ +Deprecate positional-only hook implementation parameters, without changing +their current call behavior. Use positional-or-keyword parameters instead. +Positional-only implementation parameters will become an error in pluggy 2.0. diff --git a/src/pluggy/_decorators.py b/src/pluggy/_decorators.py index c5574a67..dd2630c9 100644 --- a/src/pluggy/_decorators.py +++ b/src/pluggy/_decorators.py @@ -246,7 +246,7 @@ def setattr_hookimpl_opts(func: _F) -> _F: def varnames( - func: object, *, legacy_noself: bool = False + func: object, *, legacy_noself: bool = False, warn_posonly: bool = False ) -> tuple[tuple[str, ...], tuple[str, ...]]: """Return tuple of positional and keyword parameter names for a callable. @@ -260,6 +260,9 @@ def varnames( If ``True``, support hookspec classes whose methods omit ``self``. When the function looks like a class method but has no implicit first parameter, a :class:`DeprecationWarning` is emitted. + :param warn_posonly: + If ``True``, warn about positional-only hook implementation parameters, + excluding the already-bound instance/class parameter. """ is_bound = False if inspect.isclass(func): @@ -322,6 +325,23 @@ def varnames( stacklevel=2, ) + if warn_posonly: + positional_only = tuple( + name + for name in code.co_varnames[: code.co_posonlyargcount] + if name in args or name in kwargs + ) + if positional_only: + warnings.warn_explicit( + f"Positional-only parameters {positional_only!r} in {qualname} " + "are deprecated. Use positional-or-keyword parameters instead. " + "Positional-only hook implementation parameters will be an error " + "in pluggy 2.0.", + DeprecationWarning, + filename=code.co_filename, + lineno=code.co_firstlineno, + ) + return args, kwargs diff --git a/src/pluggy/_impl.py b/src/pluggy/_impl.py index 1b7182f1..c4e36f37 100644 --- a/src/pluggy/_impl.py +++ b/src/pluggy/_impl.py @@ -50,7 +50,7 @@ def __init__( """:meta private:""" #: The hook implementation function. self.function: Final = function - argnames, kwargnames = varnames(self.function) + argnames, kwargnames = varnames(self.function, warn_posonly=True) #: The positional parameter names of ``function``. self.argnames: Final = argnames #: The keyword parameter names of ``function`` which declare defaults. diff --git a/testing/test_posonly_deprecation.py b/testing/test_posonly_deprecation.py new file mode 100644 index 00000000..8d5099b2 --- /dev/null +++ b/testing/test_posonly_deprecation.py @@ -0,0 +1,167 @@ +from collections.abc import Callable +from functools import wraps +import inspect +import warnings + +import pytest + +from pluggy import HookimplMarker +from pluggy import HookspecMarker +from pluggy import PluginManager + + +hookspec = HookspecMarker("posonly") +hookimpl = HookimplMarker("posonly") + + +@pytest.mark.parametrize("style", ["plain", "wrapper", "hookwrapper"]) +@pytest.mark.parametrize("defaulted", [False, True]) +def test_positional_only_deprecation_preserves_calls(style, defaulted): + class Spec: + @hookspec + def hello(self, value): + pass + + def plain_required(self, value, /): + return value + + def plain_defaulted(self, value="fallback", /): + return value + + def wrapper_required(self, value, /): + yield + return value + + def wrapper_defaulted(self, value="fallback", /): + yield + return value + + def hookwrapper_required(self, value, /): + outcome = yield + outcome.force_result([value]) + + def hookwrapper_defaulted(self, value="fallback", /): + outcome = yield + outcome.force_result([value]) + + variants: dict[tuple[str, bool], Callable[..., object]] = { + ("plain", False): plain_required, + ("plain", True): plain_defaulted, + ("wrapper", False): wrapper_required, + ("wrapper", True): wrapper_defaulted, + ("hookwrapper", False): hookwrapper_required, + ("hookwrapper", True): hookwrapper_defaulted, + } + hello = variants[style, defaulted] + marked = hookimpl(wrapper=style == "wrapper", hookwrapper=style == "hookwrapper")( + hello + ) + plugin = type("Plugin", (), {"hello": marked})() + manager = PluginManager("posonly") + manager.add_hookspecs(Spec) + with pytest.warns( + DeprecationWarning, match="Positional-only.*pluggy 2.0" + ) as recorded: + manager.register(plugin) + assert len(recorded) == 1 + assert recorded[0].filename == plain_required.__code__.co_filename + assert recorded[0].lineno == inspect.getsourcelines(hello)[1] + if defaulted: + # Preserve the pre-existing failure rather than adding new binding semantics. + with pytest.raises(TypeError, match="positional-only"): + manager.hook.hello(value="caller") + with pytest.warns(UserWarning, match="cannot be found"): + result = manager.hook.hello() + assert result == (["fallback"] if style != "wrapper" else "fallback") + else: + result = manager.hook.hello(value="caller") + assert result == (["caller"] if style != "wrapper" else "caller") + + +@pytest.mark.parametrize( + "kind", ["method", "classmethod", "staticmethod", "decorated", "callable", "class"] +) +def test_positional_only_callable_shapes(kind): + class Spec: + @hookspec + def hello(self, value): + pass + + class Plugin: + @hookimpl + def hello(self, value, /): + return value + + class ClassPlugin: + @classmethod + @hookimpl + def hello(cls, value, /) -> object: + return value + + class StaticPlugin: + @staticmethod + @hookimpl + def hello(value, /) -> object: + return value + + class CallablePlugin: + def __call__(self, value, /): + return value + + class ExtraClass: + def __init__(self, value, /): + self.value = value + + def decorate(function: Callable[..., object]) -> Callable[..., object]: + @wraps(function) + def wrapped(*args, **kwargs): + return function(*args, **kwargs) + + return wrapped + + manager = PluginManager("posonly") + manager.add_hookspecs(Spec) + if kind in ("method", "classmethod", "staticmethod"): + plugin = { + "method": Plugin, + "classmethod": ClassPlugin, + "staticmethod": StaticPlugin, + }[kind]() + with pytest.warns(DeprecationWarning, match="Positional-only") as recorded: + manager.register(plugin) + result = manager.hook.hello(value="caller") + else: + extras: dict[str, Callable[..., object]] = { + "decorated": decorate(Plugin().hello), + "callable": CallablePlugin(), + "class": ExtraClass, + } + with pytest.warns(DeprecationWarning, match="Positional-only") as recorded: + result = manager.hook.hello.call_extra([extras[kind]], {"value": "caller"}) + assert len(recorded) == 1 + assert "'self'" not in str(recorded[0].message) + assert "'cls'" not in str(recorded[0].message) + if kind == "class": + assert result[0].value == "caller" + else: + assert result == ["caller"] + + +def test_implicit_positional_only_receiver_does_not_warn(): + class Spec: + @hookspec + def hello(self, value, /): + pass + + class Plugin: + @hookimpl + def hello(self, /, value="fallback"): + return value + + manager = PluginManager("posonly") + with warnings.catch_warnings(record=True) as recorded: + warnings.simplefilter("always") + manager.add_hookspecs(Spec) + manager.register(Plugin()) + assert manager.hook.hello(value="caller") == ["caller"] + assert not recorded