pytest-dev / pluggy

A minimalist production ready plugin system
https://pluggy.readthedocs.io/en/latest/
MIT License
1.23k stars 121 forks source link

Add type checking for hook specifications #191

Open rbdixon opened 5 years ago

rbdixon commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

youtux commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Actually, it seems that the codebase it's already annotated since this PR: https://github.com/pytest-dev/pluggy/pull/340.

Was a version ever released since then?

nicoddemus commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

kpfleming commented 1 year ago

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.