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

B027 and typing.overload #308

Closed
yakMM opened this issue Nov 7, 2022 · 5 comments
Closed

B027 and typing.overload #308

yakMM opened this issue Nov 7, 2022 · 5 comments

Comments

@yakMM
Copy link

yakMM commented Nov 7, 2022

Consider the following:

class Foo(ABC):
    @t.overload
    def bar(self, arg: t.Literal[False]) -> t.Literal[False]:
        ...

    @t.overload
    def bar(self, arg: t.Literal[True]) -> t.Literal[True]:
        ...

    def bar(self, arg: bool) -> bool:
        return arg

    @abstractmethod
    def abstract_method(self) -> None:
        ...

The overloads of bar are considered as "real" methods and B027 warning is emitted:

 B027 bar is an empty method in an abstract base class, but has no abstract decorator. Consider adding @abstractmethod.
 B027 bar is an empty method in an abstract base class, but has no abstract decorator. Consider adding @abstractmethod.

Is it worth not emitting the warning when typing.overload is used or is it a case that should be supressed manually with # noqa?

@jakkdl
Copy link
Contributor

jakkdl commented Nov 7, 2022

B027 checks for @overload and @typing.overload https://github.com/PyCQA/flake8-bugbear/blob/main/bugbear.py#L672

but since you did import typing as t it's not picked up by the linter. Searching github it seems that's relatively common, so I can add that to the logic as well.

@yakMM
Copy link
Author

yakMM commented Nov 12, 2022

Ah okay makes sense, I didn't know this kind of details mattered, I thought the imports were evaluated

@jakkdl
Copy link
Contributor

jakkdl commented Nov 14, 2022

Typecheckers evaluate imports, but AST checkers like flake8 usually don't try and just looks at identifiers hoping/assuming they're well-named. (afaik)

Anyway, fixed with #309 - forgot to mention this issue in it.

@cooperlees
Copy link
Collaborator

So, is this fine to be closed now?

@jakkdl
Copy link
Contributor

jakkdl commented Nov 14, 2022

yep!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants