[NO-SQUASH-MERGE] fix varnames on py3.14 deferred annotations

changelog/629.bugfix.rst

@@ -0,0 +1 @@
+Fix hooks failing to register on Python 3.14+ when type annotations use forward references.

src/pluggy/_hooks.py

@@ -11,6 +11,7 @@
 from collections.abc import Set
 import inspect
 import sys
+import types
 from types import ModuleType
 from typing import Any
 from typing import Final
@@ -287,70 +288,86 @@ def normalize_hookimpl_opts(opts: HookimplOpts) -> None:
     opts.setdefault("specname", None)
 
 
-_PYPY = hasattr(sys, "pypy_version_info")
+_PYPY = sys.implementation.name == "pypy"
+_IMPLICIT_NAMES = ("self", "cls", "obj") if _PYPY else ("self", "cls")
 
 
-def varnames(func: object) -> tuple[tuple[str, ...], tuple[str, ...]]:
-    """Return tuple of positional and keywrord argument names for a function,
-    method, class or callable.
+def varnames(
+    func: object, *, legacy_noself: bool = False
+) -> tuple[tuple[str, ...], tuple[str, ...]]:
+    """Return tuple of positional and keyword parameter names for a callable.
 
     In case of a class, its ``__init__`` method is considered.
-    For methods the ``self`` parameter is not included.
+    For bound methods, the already-bound first parameter is not included.
+    For unbound methods with a dotted ``__qualname__``, the first parameter is
+    stripped only if its name is a known implicit name (``self``, ``cls``).
+    Keyword-only parameters are not included.
+
+    :param legacy_noself:
+        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:`FutureWarning` is emitted.
     """
+    is_bound = False
     if inspect.isclass(func):
         try:
             func = func.__init__
         except AttributeError:  # pragma: no cover - pypy special case
             return (), ()
+        is_bound = True
     elif not inspect.isroutine(func):  # callable object?
         try:
             func = getattr(func, "__call__", func)
         except Exception:  # pragma: no cover - pypy special case
             return (), ()
 
+    # Track bound methods before unwrapping, since __func__ loses that info.
+    if inspect.ismethod(func):
+        is_bound = True
+    func = inspect.unwrap(func)  # type: ignore[arg-type]
+    if inspect.ismethod(func):
+        is_bound = True
+        func = func.__func__
+
     try:
-        # func MUST be a function or method here or we won't parse any args.
-        sig = inspect.signature(
-            func.__func__ if inspect.ismethod(func) else func  # type:ignore[arg-type]
-        )
-    except TypeError:  # pragma: no cover
+        code: types.CodeType = func.__code__  # type: ignore[attr-defined]
+        defaults: tuple[object, ...] | None = func.__defaults__  # type: ignore[attr-defined]
+        qualname: str = func.__qualname__  # type: ignore[attr-defined]
+    except AttributeError:  # pragma: no cover
         return (), ()
 
-    _valid_param_kinds = (
-        inspect.Parameter.POSITIONAL_ONLY,
-        inspect.Parameter.POSITIONAL_OR_KEYWORD,
-    )
-    _valid_params = {
-        name: param
-        for name, param in sig.parameters.items()
-        if param.kind in _valid_param_kinds
-    }
-    args = tuple(_valid_params)
-    defaults = (
-        tuple(
-            param.default
-            for param in _valid_params.values()
-            if param.default is not param.empty
-        )
-        or None
-    )
+    # Get positional argument names (positional-only + positional-or-keyword)
+    args: tuple[str, ...] = code.co_varnames[: code.co_argcount]
 
+    # Determine which args have defaults
+    kwargs: tuple[str, ...]
     if defaults:
         index = -len(defaults)
-        args, kwargs = args[:index], tuple(args[index:])
+        args, kwargs = args[:index], args[index:]
     else:
         kwargs = ()
 
-    # strip any implicit instance arg
-    # pypy3 uses "obj" instead of "self" for default dunder methods
-    if not _PYPY:
-        implicit_names: tuple[str, ...] = ("self",)
-    else:
-        implicit_names = ("self", "obj")
+    # Strip implicit instance/class arg.
+    # Check if this looks like a method defined in a class by examining the
+    # qualname after the last "<locals>." segment (if any). A remaining dot
+    # means it's a class method (e.g. "MyClass.method" or
+    # "func.<locals>.MyClass.method"), not just a nested function.
+    _tail = qualname.rsplit("<locals>.", maxsplit=1)[-1]
+    _is_class_method = "." in _tail
     if args:
-        qualname: str = getattr(func, "__qualname__", "")
-        if inspect.ismethod(func) or ("." in qualname and args[0] in implicit_names):
+        if is_bound:
             args = args[1:]
+        elif _is_class_method and args[0] in _IMPLICIT_NAMES:
+            args = args[1:]
+        elif _is_class_method and legacy_noself:
+            warnings.warn(
+                f"{qualname} is a method but its first parameter"
+                f" {args[0]!r} is not 'self'."
+                f" Add 'self' as the first parameter or use @staticmethod."
+                f" This will become an error in a future version of pluggy.",
+                FutureWarning,
+                stacklevel=2,
+            )
 
     return args, kwargs
 
@@ -708,9 +725,14 @@ class HookSpec:
 
     def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None:
         self.namespace = namespace
-        self.function: Callable[..., object] = getattr(namespace, name)
         self.name = name
-        self.argnames, self.kwargnames = varnames(self.function)
+        self.function: Callable[..., object] = getattr(namespace, name)
+        legacy_noself = inspect.isclass(namespace) and not isinstance(
+            inspect.getattr_static(namespace, name), staticmethod
+        )
+        self.argnames, self.kwargnames = varnames(
+            self.function, legacy_noself=legacy_noself
+        )
         self.opts = opts
         self.warn_on_impl = opts.get("warn_on_impl")
         self.warn_on_impl_args = opts.get("warn_on_impl_args")

testing/benchmark.py

@@ -2,6 +2,8 @@
 Benchmarking and performance tests.
 """
 
+import inspect
+import sys
 from typing import Any
 
 import pytest
@@ -11,6 +13,66 @@
 from pluggy import PluginManager
 from pluggy._callers import _multicall
 from pluggy._hooks import HookImpl
+from pluggy._hooks import varnames
+
+
+def _varnames_legacy(func: object) -> tuple[tuple[str, ...], tuple[str, ...]]:
+    """Pre-structural-detection implementation using inspect.signature and
+    name-based heuristics for comparison."""
+    if inspect.isclass(func):
+        try:
+            func = func.__init__
+        except AttributeError:
+            return (), ()
+    elif not inspect.isroutine(func):
+        try:
+            func = getattr(func, "__call__", func)
+        except Exception:
+            return (), ()
+
+    try:
+        sig = inspect.signature(
+            func.__func__ if inspect.ismethod(func) else func  # type: ignore[arg-type]
+        )
+    except TypeError:
+        return (), ()
+
+    _valid_param_kinds = (
+        inspect.Parameter.POSITIONAL_ONLY,
+        inspect.Parameter.POSITIONAL_OR_KEYWORD,
+    )
+    _valid_params = {
+        name: param
+        for name, param in sig.parameters.items()
+        if param.kind in _valid_param_kinds
+    }
+    args = tuple(_valid_params)
+    defaults = (
+        tuple(
+            param.default
+            for param in _valid_params.values()
+            if param.default is not param.empty
+        )
+        or None
+    )
+
+    if defaults:
+        index = -len(defaults)
+        args, kwargs = args[:index], tuple(args[index:])
+    else:
+        kwargs = ()
+
+    _pypy = hasattr(sys, "pypy_version_info")
+    if not _pypy:
+        implicit_names: tuple[str, ...] = ("self",)
+    else:
+        implicit_names = ("self", "obj")
+    if args:
+        qualname: str = getattr(func, "__qualname__", "")
+        if inspect.ismethod(func) or ("." in qualname and args[0] in implicit_names):
+            args = args[1:]
+
+    return args, kwargs
 
 
 hookspec = HookspecMarker("example")
@@ -106,3 +168,30 @@ def fun(self):
         pm.register(PluginWrap(i), name=f"wrap_plug_{i}")
 
     benchmark(pm.hook.fun, hooks=pm.hook, nesting=nesting)
+
+
+def _plain_func(x: int, y: str, z: float = 1.0) -> None:
+    pass
+
+
+class _MethodHolder:
+    def method(self, x: int, y: str, z: float = 1.0) -> None:
+        pass
+
+
+_varnames_funcs = [
+    pytest.param(_plain_func, id="plain_function"),
+    pytest.param(_MethodHolder.method, id="unbound_method"),
+    pytest.param(_MethodHolder().method, id="bound_method"),
+    pytest.param(_MethodHolder, id="class"),
+]
+
+
+@pytest.mark.parametrize("func", _varnames_funcs)
+def test_varnames(benchmark, func: object) -> None:
+    benchmark(varnames, func)
+
+
+@pytest.mark.parametrize("func", _varnames_funcs)
+def test_varnames_legacy(benchmark, func: object) -> None:
+    benchmark(_varnames_legacy, func)

testing/test_helpers.py

@@ -112,5 +112,141 @@ def example_method(self, x, y=1) -> None:
     ex_inst = Example()
 
     assert varnames(example) == (("a",), ("b",))
+    # Unbound: self is stripped because it's in _IMPLICIT_NAMES and qualname is dotted.
     assert varnames(Example.example_method) == (("x",), ("y",))
+    # Bound: self is already consumed.
     assert varnames(ex_inst.example_method) == (("x",), ("y",))
+
+
+def test_varnames_bound_method_from_module_function() -> None:
+    """A module-level function assigned to a class attribute becomes a bound
+    method when accessed on an instance, but its __qualname__ has no dot.
+    varnames must still strip the first parameter."""
+
+    def standalone(self, x) -> None:
+        pass  # pragma: no cover
+
+    class MyClass:
+        method = standalone
+
+    assert varnames(MyClass().method) == (("x",), ())
+
+
+def test_varnames_unconventional_first_param_name() -> None:
+    """Bound methods strip unconditionally, but unbound methods with
+    non-standard first parameter names preserve all arguments."""
+
+    class MyClass:
+        def method(this, x) -> None:
+            pass  # pragma: no cover
+
+    # Bound: stripped regardless of name.
+    assert varnames(MyClass().method) == (("x",), ())
+    # Unbound with dotted qualname but non-implicit name: NOT stripped.
+    assert varnames(MyClass.method) == (("this", "x"), ())
+
+
+def test_varnames_classmethod() -> None:
+    class MyClass:
+        @classmethod
+        def cm(cls, x, y=1) -> None:
+            pass  # pragma: no cover
+
+    # Classmethods are always bound (even from the class).
+    assert varnames(MyClass.cm) == (("x",), ("y",))
+    assert varnames(MyClass().cm) == (("x",), ("y",))
+
+
+def test_varnames_staticmethod() -> None:
+    class MyClass:
+        @staticmethod
+        def sm(x, y=1) -> None:
+            pass  # pragma: no cover
+
+    # Staticmethods have no implicit first arg.
+    assert varnames(MyClass.sm) == (("x",), ("y",))
+    assert varnames(MyClass().sm) == (("x",), ("y",))
+
+
+def test_varnames_hookspec_without_self() -> None:
+    """Hookspec-style class methods without self/cls preserve all parameters.
+
+    This is the convention used by projects like pytest-timeout where hookspec
+    classes define methods without ``self`` since they serve as pure signatures.
+    By default varnames does not warn; the warning is emitted when
+    ``legacy_noself=True`` is passed (as HookSpec.__init__ does).
+    """
+
+    class MySpecs:
+        def my_hook(item, extra) -> None:
+            pass  # pragma: no cover
+
+    # Accessed as unbound: first arg is not an implicit name, keep it.
+    assert varnames(MySpecs.my_hook) == (("item", "extra"), ())
+    # Accessed as bound (via instance): first arg is stripped.
+    assert varnames(MySpecs().my_hook) == (("extra",), ())
+
+
+def test_varnames_legacy_noself_warns() -> None:
+    """With ``legacy_noself=True``, varnames warns when it encounters a
+    class method whose first parameter is not an implicit name."""
+    import warnings
+
+    class MySpecs:
+        def my_hook(item, extra) -> None:
+            pass  # pragma: no cover
+
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("always")
+        result = varnames(MySpecs.my_hook, legacy_noself=True)
+    assert result == (("item", "extra"), ())
+    assert len(w) == 1
+    assert issubclass(w[0].category, FutureWarning)
+    assert "'item' is not 'self'" in str(w[0].message)
+
+
+def test_varnames_legacy_noself_no_warn_with_self() -> None:
+    """With ``legacy_noself=True``, no warning when the method has ``self``."""
+    import warnings
+
+    class MySpecs:
+        def my_hook(self, item, extra) -> None:
+            pass  # pragma: no cover
+
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("always")
+        result = varnames(MySpecs.my_hook, legacy_noself=True)
+    assert result == (("item", "extra"), ())
+    assert len(w) == 0
+
+
+def test_varnames_no_legacy_noself_no_warn() -> None:
+    """Without ``legacy_noself``, no warning even for class methods without self."""
+    import warnings
+
+    class MySpecs:
+        def my_hook(item, extra) -> None:
+            pass  # pragma: no cover
+
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("always")
+        result = varnames(MySpecs.my_hook)
+    assert result == (("item", "extra"), ())
+    assert len(w) == 0
+
+
+def test_varnames_unresolvable_annotation() -> None:
+    """Test that varnames works with annotations that cannot be resolved.
+
+    In Python 3.14+, inspect.signature() tries to resolve string annotations
+    by default, which can fail if the annotation refers to a type that isn't
+    importable. Using __code__ directly avoids this issue.
+    """
+
+    def func_with_bad_annotation(
+        x: "NonExistentType",  # type: ignore[name-defined]  # noqa: F821
+        y,
+    ) -> None:
+        pass
+
+    assert varnames(func_with_bad_annotation) == (("x", "y"), ())