Add type checking for hook specifications

[rbdixon]

I think it would be helpful to support type annotations in hook specifications.

It isn't hard to add the necessary annotations to a hook specification but I couldn't work out how to integrate this with pluggy. I spent some time on this and worked out the specifics:

1. pluggy.HookspecMarker must be modified with a type hint so that the decorator does not obscure the type hints added to the specification.
2. When a hook is registered the .hook attribute of the pluggy.manager.PluginManager instance myst be cast so that mypy can connect the specification to the registered hooks.

Here is a full example:

import pluggy  # type: ignore
from typing import TypeVar, Callable, Any, cast

# Improvement suggested by @oremanj on python/typing gitter
F = TypeVar("F", bound=Callable[..., Any])
hookspec = cast(Callable[[F], F], pluggy.HookspecMarker("myproject"))
hookimpl = pluggy.HookimplMarker("myproject")


class MySpec(object):
    """A hook specification namespace."""

    @hookspec
    def myhook(self, arg1: int, arg2: int) -> int:
        """My special little hook that you can customize."""


class Plugin_1(object):
    """A hook implementation namespace."""

    @hookimpl
    def myhook(self, arg1: int, arg2: int) -> int:
        print("inside Plugin_1.myhook()")
        return arg1 + arg2 + 'a'


# create a manager and add the spec
pm = pluggy.PluginManager("myproject")
pm.add_hookspecs(MySpec)

# register plugins
pm.register(Plugin_1())

# Add cast so that mypy knows that pm.hook
# is actually a MySpec instance. Without this
# hint there really is no way for mypy to know
# this.
pm.hook = cast(MySpec, pm.hook)

# Uncomment these when running through mypy to see
# how mypy regards the type
# reveal_type(pm.hook)
# reveal_type(pm.hook.myhook)
# reveal_type(MySpec.myhook)

# this will now be caught by mypy
results = pm.hook.myhook(arg1=1, arg2="1")
print(results)

Output when checking with mypy:

$ mypy plug.py
plug.py:24: error: Unsupported operand types for + ("int" and "str")
plug.py:47: error: Argument "arg2" to "myhook" of "MySpec" has incompatible type "str"; expec
ted "int"

My original StackOverflow question and answer: https://stackoverflow.com/questions/54674679/how-can-i-annotate-types-for-a-pluggy-hook-specification

[goodboy]

@rbdixon woo I actually really like this.

Would you mind making a PR and some tests.
We might need to think about how to handle py2 as well.

[nicoddemus]

We also can consider just waiting a bit regarding py2; pytest plans to drop support in 5.0 mid-year, and we are almost May. We have to see the timeline for the other projects though (tox and devpi).

cc @gaborbernat @fschulze

[fschulze]

The policy for devpi is pinning dependencies when necessary and dropping Python 2.x and 3.4 support when the workarounds become too cumbersome. We can't force others to hold back too much.

[gaborbernat]

tox plan is also mid-year but might go into autumn... not in a hurry yet 👍

[youtux]

any update on this? Mypy returns an error because of pytest.hookimpl() not being annotated:

❯ tox -e mypy
mypy inst-nodeps: /Users/youtux/Developer/pytest-factoryboy/.tox/.tmp/package/1/pytest_factoryboy-2.5.0.tar.gz
mypy installed: attrs==22.1.0,factory-boy==3.2.1,Faker==15.3.4,inflection==0.5.1,iniconfig==1.1.1,mypy==0.991,mypy-extensions==0.4.3,packaging==21.3,pluggy==1.0.0,pyparsing==3.0.9,pytest==7.2.0,pytest-factoryboy @ file:///Users/youtux/Developer/pytest-factoryboy/.tox/.tmp/package/1/pytest_factoryboy-2.5.0.tar.gz,python-dateutil==2.8.2,six==1.16.0,typing_extensions==4.4.0
mypy run-test-pre: PYTHONHASHSEED='2215478772'
mypy run-test: commands[0] | mypy .
pytest_factoryboy/plugin.py:113: error: Untyped decorator makes function
"pytest_runtest_call" untyped  [misc]
    @pytest.hookimpl(tryfirst=True)
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Found 1 error in 1 file (checked 17 source files)
ERROR: InvocationError for command /Users/youtux/Developer/pytest-factoryboy/.tox/mypy/bin/mypy . (exited with code 1)
___________________________________ summary ____________________________________
ERROR:   mypy: commands failed

[nicoddemus]

Thanks @youtux for the ping.

So far no updates, but now that we dropped Python 2, should be simpler. If anybody wants to work on that, we would be glad to review and merge a PR! 👍

[youtux]

Actually, it seems that the codebase it's already annotated since this PR: #340.

Was a version ever released since then?

[nicoddemus]

The codebase is indeed type annotated, but the type checking added by #340 is for internal use only: mypy will not check pluggy for users of the library because it does not publish a py.typed file yet.

This issue however is about this code in particular being type checked:

# this will now be caught by mypy
results = pm.hook.myhook(arg1=1, arg2="1")

Which we don't support currently (@bluetech can correct me if I'm wrong).

Was a version ever released since then?

No, but mostly because those were internal changes so far not warranting a new release... we have #364, but the official 3.11 support was mostly adding it to CI as it was working with 3.11 already.

[youtux]

I see.

I have a different use case though, so having just a release of pluggy with the py.typed marker would fix my issue (probably other users are experiencing the same issue, since it's about pytest.hookimpl not being annotated).

[youtux]

(probably I should have opened a different issue rather than posting in this one)

[kpfleming]

I just found the OP's StackOverflow post and used it as a guide to implement type annotations for my hookspecs; the content in the first post of this issue is not actually quite right, as the hook attribute of the PluginManager returns a Sequence of results, not a single result.

I would be happy to write up a section for the docs giving users some guidance on how to implement the proper types in their usage of pluggy; part of what I've done will be obsoleted by the type hints that were just added to the typeshed (and then later when py.typed is present in this project directly), but the rest is specific to the hooks themselves and can't be done in a generic way as far as I can tell.

[pirate]

Ok I think I implemented basically ideal static type hinting & runtime signature copying for Pluggy HookCallers, grafting the kwargs and return types from the underlying hookspecs. It took a few tries to get right, particularly around getting the @override order right.

Overview

• Type hints for pm.hook.call(abc1: int, abc2: str) args/kwargs (enforces kwarg-only at callsite)
• Type hints for pm.hook.call() return types (including firstresult handling)
• Does not require manually generating static types or a Protocol for the HookCaller and saving in .pyi files
• does not require "lying" about the return type on the hookspecs (marking them as returning List/Iterable manually when the body of the function clearly returns a single item)

Methodology for compile-time hints

I defined some extra @overload signatures on HookspecMarker that preserve arg types using a ParamSpec + lock down the return type based on the firstresult: Literal[True] | Literal[False] value.

Methodology for runtime signatures

I copy over the hookspec method args types and return types to HookCaller.__signature__ when pm.add_hookspecs(...) is called.

from pluggy import HookspecMarker, HookimplMarker, PluginManager, HookimplOpts, HookCaller, HookspecOpts


ParamsT = ParamSpec('ParamsT')
ReturnT = TypeVar('ReturnT')

class HookSpecDecoratorThatReturnsFirstResult(Protocol):    
    def __call__(self, func: Callable[ParamsT, ReturnT]) -> Callable[ParamsT, ReturnT]: ...

class HookSpecDecoratorThatReturnsListResults(Protocol):
    def __call__(self, func: Callable[ParamsT, ReturnT]) -> Callable[ParamsT, List[ReturnT]]: ...


class TypedHookspecMarker(HookspecMarker):
    """Improved version of pluggy.HookspecMarker that supports type inference of hookspecs with firstresult=True|False correctly"""

    # handle @hookspec(firstresult=False) -> List[ReturnT] (test_firstresult_False_hookspec)
    @overload
    def __call__(
        self,
        function: None = ...,
        firstresult: Literal[False] = ...,
        historic: bool = ...,
        warn_on_impl: Warning | None = ...,
        warn_on_impl_args: Mapping[str, Warning] | None = ...,
    ) -> HookSpecDecoratorThatReturnsListResults: ...

    # handle @hookspec(firstresult=True) -> ReturnT (test_firstresult_True_hookspec)
    @overload
    def __call__(
        self,
        function: None = ...,
        firstresult: Literal[True] = ...,
        historic: bool = ...,
        warn_on_impl: Warning | None = ...,
        warn_on_impl_args: Mapping[str, Warning] | None = ...,
    ) -> HookSpecDecoratorThatReturnsFirstResult: ...
    
    # handle @hookspec -> List[ReturnT] (test_normal_hookspec)
    # @overload order matters!!! this one must come last
    @overload         
    def __call__(
        self,
        function: Callable[ParamsT, ReturnT] = ...,
        firstresult: Literal[False] = ...,
        historic: bool = ...,
        warn_on_impl: Warning | None = ...,
        warn_on_impl_args: Mapping[str, Warning] | None = ...,
    ) -> Callable[ParamsT, List[ReturnT]]: ...

    def __call__(
        self,
        function: Callable[ParamsT, ReturnT] | None = None,
        firstresult: bool = False,
        historic: bool = False,
        warn_on_impl: Warning | None = None,
        warn_on_impl_args: Mapping[str, Warning] | None = None,
    ) -> Callable[ParamsT, List[ReturnT]] | HookSpecDecoratorThatReturnsFirstResult | HookSpecDecoratorThatReturnsListResults:
        
        return super().__call__(function=function, firstresult=firstresult, historic=historic, warn_on_impl=warn_on_impl, warn_on_impl_args=warn_on_impl_args)



PluginSpec = TypeVar("PluginSpec")

class TypedPluginManager(PluginManager, Generic[PluginSpec]):
    """
    Improved version of pluggy.PluginManager that allows static type inference of HookCaller calls based on underlying hookspec.
    """
    
    # enable static type checking of pm.hook.call() calls
    # https://stackoverflow.com/a/62871889/2156113
    # https://github.com/pytest-dev/pluggy/issues/191
    hook: PluginSpec
    
    def create_typed_hookcaller(self, name: str, module_or_class: Type[PluginSpec], spec_opts: Dict[str, Any]) -> HookCaller:
        """
        create a new HookCaller subclass with a modified __signature__
        so that the return type is correct and args are converted to kwargs
        """
        TypedHookCaller = type('TypedHookCaller', (HookCaller,), {})
        
        hookspec_signature = inspect.signature(getattr(module_or_class, name))
        hookspec_return_type = hookspec_signature.return_annotation
        
        # replace return type with list if firstresult=False
        hookcall_return_type = hookspec_return_type if spec_opts['firstresult'] else List[hookspec_return_type]
        
        # replace each arg with kwarg equivalent (pm.hook.call() only accepts kwargs)
        args_as_kwargs = [
            param.replace(kind=inspect.Parameter.KEYWORD_ONLY) if param.name != 'self' else param
            for param in hookspec_signature.parameters.values()
        ]
        TypedHookCaller.__signature__ = hookspec_signature.replace(parameters=args_as_kwargs, return_annotation=hookcall_return_type)
        TypedHookCaller.__name__ = f'{name}_HookCaller'
        
        return TypedHookCaller(name, self._hookexec, module_or_class, spec_opts)
    
    def add_hookspecs(self, module_or_class: Type[PluginSpec]) -> None:
        """Add HookSpecs from the given class, (generic type allows us to enforce types of pm.hook.call() statically)"""
        names = []
        for name in dir(module_or_class):
            spec_opts = self.parse_hookspec_opts(module_or_class, name)
            if spec_opts is not None:
                hc: HookCaller | None = getattr(self.hook, name, None)
                if hc is None:
                    hc = self.create_typed_hookcaller(name, module_or_class, spec_opts)
                    setattr(self.hook, name, hc)
                else:
                    # Plugins registered this hook without knowing the spec.
                    hc.set_specification(module_or_class, spec_opts)
                    for hookfunction in hc.get_hookimpls():
                        self._verify_hook(hc, hookfunction)
                names.append(name)
                
        if not names:
            raise ValueError(
                f"did not find any {self.project_name!r} hooks in {module_or_class!r}"
            )

Usage

hookspec = TypedHookspecMarker("test")

class TestSpec:
    @hookspec
    def test_normal_hookspec(self, abc1: int) -> int:
        ...
    
    @hookspec(firstresult=False)
    def test_firstresult_False_hookspec(self, abc1: int) -> int:
        ...
    
    @hookspec(firstresult=True)
    def test_firstresult_True_hookspec(self, abc1: int) -> int:
        ...


TestPluginManager = TypedPluginManager[TestSpec]
pm = TestPluginManager("test")
pm.add_hookspecs(TestSpec)

# note this does not limit to a single PluginSpec, you can use multiple like so:
#
# class CombinedPluginSpec(Spec1, Spec2, Spec3):
#     pass
#     
# PluginManager = TypedPluginManager[CombinedPluginSpec]
# pm = PluginManager("test")
# pm.add_hookspecs(Spec1)
# pm.add_hookspecs(Spec2)
# pm.add_hookspecs(Spec3)

Results

• @hookspec (plain)

  • Static hint (mypy/pyright):
    
  • Runtime signature:

    inspect.signature(pm.hook.test_normal_hookspec)
    # <Signature (*, abc1: int) -> List[int]>

• @hookspec(firstresult=False)

  • Static hint (mypy/pyright):
    
  • Runtime signature:

    inspect.signature(pm.hook.test_firstresult_False_hookspec)
    # <Signature (*, abc1: int) -> List[int]>

• @hookspec(firstresult=True)

  • Static hint (mypy/pyright):
    
  • Runtime signature:

    inspect.signature(pm.hook.test_firstresult_True_hookspec)
    # <Signature (*, abc1: int) -> int>

Can I submit a PR to pluggy with my changes?

[RonnyPfannschmidt]

I think it's a helpful addition

We ought to figure if there's a reasonable way to provide a mypy plugin that handles modules as spec and or unions of specs like pytest plugins that add new hooks

[kpfleming]

For what it's worth, my solution was much simpler but probably not as comprehensive.

[pirate]

@RonnyPfannschmidt yeah I had the same concern. It's not impossible to add module support to my implementation, but it is hard to provide a straighforward, consistent syntax for making a spec union of a mix of modules and classes together.

I think it wouldn't be unreasonable to ask people to do this in order to get pluggy static type hinting, it's not too hard to make everything into a class manually, and that makes it more explicit / is less "magic" than if we try to invent a new union syntax:

class FirstHookspec:
    @hookspec
    def some_hookspec(self) -> int: ...
    
import second_hookspec

SecondHookspecAsClass = type('SecondHookspecAsClass', (), second_hookspec.__dict__)

# Combining Two hookspecs
class CombinedHookspec(FirstHookspec, SecondHookspecAsClass):
    pass
    
PluginManager = TypedPluginManager[CombinedHookspec]
pm = PluginManager("test")
pm.add_hookspecs(CombinedHookspec)
# OR you can register them the normal way, either way works:
pm.add_hookspecs(FirstHookspec)
pm.add_hookspecs(second_hookspec)

I don't think something like this is possible with the python type system anyway:

PluginManager = TypedPluginManager[FirstHookspec | second_hookspec]

Even with the new TypeVarTuple you cant make dynamic unions of arbitrary namespaces at compile time without hardcoding.

[RonnyPfannschmidt]

That's why I mentioned mypy plugins,, it's simply impossible natively

[RonnyPfannschmidt]

However having a starting point is key

[pirate]

I'm not sure extending mypy & pyright is worth it, imo it's too "magic" to merge modules and classes at the type level without forcing the user to do a little bit of work to understanad what's happening.

I think you'd eventually run into weird issues where tooling trying to introspect pm.hook would end up with surprising inconsistencies around method.__module__/method.__package__/method.__file__/.__annotations__/.__signature__/etc. too many things are different between class methods and module functions.

[RonnyPfannschmidt]

One certainly wouldn't want to merge the types themselves

But being able to do valiated casts and/or enumerations of the hooks might be enough

It's certainly not going to be easy

[pirate]

Modules are accepted in place of a Protocol in some places, which could help with this:

• https://peps.python.org/pep-0544/#modules-as-implementations-of-protocols
• Allow using modules as subtypes of protocols python/mypy#5018
• modules as protocols doesn't type-check against the protocol python/mypy#13771
• Modules as implementations of protocols microsoft/pyright#1341

It seems like Union[*TypeVarTuple] support was removed from pyright/mypy and the current status is that it will require a Python language PEP to be added back:

• Support for Union[*TypeVarTuple] python/typing#1523 (comment)
• https://discuss.python.org/t/proposal-allow-typevartuple-unpacking-in-unions/42932

Interestingly I am able to get combined type hinting for a simple Union[ModuleType, Type] (though it does show lots of warnings):

spec_from_module.py:

def test_func_from_module(abc1: int) -> int:
    return 123

spec_from_class.py:

class SpecFromClass:
    def test_func_from_class(self, abc1: int) -> int:
        return 456

test.py:

import inspect

from typing import TypeVar, Union, TypeVar, Type, Protocol, cast, List, Tuple, reveal_type
from types import ModuleType

import spec_from_module
import spec_from_class

ModuleT = TypeVar('ModuleT', bound=ModuleType)
ClassT = TypeVar('ClassT', bound=Type)


def combined_spec(*namespaces: ModuleT | ClassT]) -> ModuleT | ClassT:
    return type('CombinedSpec', tuple(
        namespace if inspect.isclass(namespace) else type(namespace.__name__, (), namespace.__dict__)
        for namespace in namespaces
    ), {})

CombinedSpec = combined_spec(spec_from_module, spec_from_class.SpecFromClass)

print(inspect.signature(CombinedSpec.test_func_from_class))
# (self, abc1: int) -> int

print(inspect.signature(CombinedSpec.test_func_from_module))
# (abc1: int) -> int

reveal_type(CombinedSpec)
# Module("spec_from_module") | SpecFromClass

print(CombinedSpec.test_func_from_module(abc1=123))
# 123

print(CombinedSpec().test_func_from_class(abc1=123))
# 456