nhoad / flake8-unused-arguments

Flake8 plugin to warn against unused arguments in functions
MIT License
31 stars 8 forks source link

False positive with @overload annotations #10

Closed dargueta closed 2 years ago

dargueta commented 2 years ago

If you use the @overload decorator for specific type annotations, they get counted as regular functions.

from typing import Any
from typing import Literal
from typing import overload
from typing import Union

@overload
def _create_boto3_s3_resource_or_client(
    constructor_name: Literal["resource"], **parameters: Any
) -> "S3ServiceResource":
    ...

@overload
def _create_boto3_s3_resource_or_client(
    constructor_name: Literal["client"], **parameters: Any
) -> "S3Client":
    ...

def _create_boto3_s3_resource_or_client(
    constructor_name: str, **parameters: Any
) -> Union["S3ServiceResource", "S3Client"]:
    # stuff

Result:

./aircraft/bototools.py:87:5: U100 Unused argument 'constructor_name'
./aircraft/bototools.py:87:46: U100 Unused argument 'parameters'
./aircraft/bototools.py:94:5: U100 Unused argument 'constructor_name'
./aircraft/bototools.py:94:44: U100 Unused argument 'parameters'

Theoretically this would also be solved by fixing #9 and using unused-arguments-ignore-stub-functions = true, but considering it's technically a type annotation, I think it should be completely ignored.

Environment and setup is the same as #9

nhoad commented 2 years ago

How to solve this is actually quite complex. Some options:

dargueta commented 2 years ago

Pedantically, overloads are neither stubs nor abstract functions, as they're purely a form of type annotation designed to get around an ambiguity that would result without it. By that token, in my opinion, they should be treated like ordinary annotations and always be ignored.

I've never written a flake8 plugin so have no idea how easy it is to check if a function is marked with the overload decorator. The documentation is less than fantastic too. I guess the easiest way to implement this would be option 4, and rely on flake8 or other plugins to catch the edge case where someone accidentally copy-pasted the same name twice on two non-overload functions.

nhoad commented 2 years ago

I decided to bite the bullet and just implement this as a third option - you can now use --unused-arguments-ignore-overload-functions to ignore anything decorated with @overload. I've released 0.0.11 with this.

dargueta commented 2 years ago

Thank you!