Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unused-function-argument (ARG001) - false positive on pytest hookspec functions #9803

Open
DetachHead opened this issue Feb 2, 2024 · 4 comments
Labels
rule Implementing or modifying a lint rule

Comments

@DetachHead
Copy link

in pytest, a new hook function is defined using the hookspec decorator, and the function has no implementation as it's basically like an abstract method:

from pytest import hookspec

@hookspec
def pytest_foo(value: int):  # error Unused function argument: `value`
    ...

playground

ruff could detect the use of the hookspec decorator and not report the error here.

related:

@charliermarsh
Copy link
Member

We should be able to support this. Can you link to any relevant docs on hookspec?

@DetachHead
Copy link
Author

DetachHead commented Feb 3, 2024

https://docs.pytest.org/en/7.1.x/how-to/writing_hook_functions.html#declaring-new-hooks

a couple things worth noting:

  • the hookspec decorator isn't actually required to declare a hook. instead you have to hook pytest_addhooks and register the whole module:

    # foo/hooks.py
    from pytest import hookspec
    
    def pytest_foo(value: int):
        ...
    # conftest.py
    def pytest_addhooks(pluginmanager):
        from foo import hooks
    
        pluginmanager.add_hookspecs(hooks)

    as far as i can tell the hookspec decorator is mainly used to specify additional options to individual hooks (so it's not needed if you are just using the default options). however using the decorator without any options like in my original example is perfectly valid, and a good way to let ruff know it's a hookspec without needing multi-file analysis (Multi-file analysis #7447)

  • pytest' plugin functionality comes from pluggy, so ideally ruff would also recognize hookspecs in any project that uses pluggy. however since decorators are defined like this, it would either need multi-file analysis to work, or just assume any decorator called hookspec is a pluggy hook:

    hookspec = pluggy.HookspecMarker("myproject")
    hookimpl = pluggy.HookimplMarker("myproject")

@charliermarsh
Copy link
Member

Assuming it's defined within the same file (which seems common), we could detect that @hookspec resolves to hookspec = HookspecMarker("...").

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Feb 5, 2024
@DetachHead
Copy link
Author

would still require special casing for pytest, since its hookspec decorators are defined in a different file to where they're used in plugins

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

2 participants