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

false positive: abstract-method for typing.Protocol inheritance with abc.abstractmethod #7209

Closed
huwcbjones opened this issue Jul 20, 2022 · 3 comments · Fixed by #7839
Closed
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@huwcbjones
Copy link
Contributor

Bug description

It appears that if:

  1. you have two protocols that have abstract methods
  2. create a third protocol that inherits from both of them and does not implement the abstractmethods (because a protocol is not a concrete implementation)

pylint thinks that the abstract methods from the parent classes are not implemented.
From reading PEP 544 I believe this is acceptable and therefore shouldn't be a false positive.

"""FooBar Protocol"""
# pylint: disable=too-few-public-methods,disallowed-name
from abc import abstractmethod
from typing import Protocol, Literal


class FooProtocol(Protocol):
    """Foo Protocol"""

    @abstractmethod
    def foo(self) -> Literal["foo"]:  # pylint: disable=invalid-name
        """foo method"""


class BarProtocol(Protocol):
    """Bar Protocol"""
    @abstractmethod
    def bar(self) -> Literal["bar"]:
        """bar method"""


class FooBarProtocol(FooProtocol, BarProtocol, Protocol):
    """FooBar Protocol"""


class FooBar(FooBarProtocol):
    """FooBar object"""

    def bar(self) -> Literal["bar"]:
        return "bar"

    def foo(self) -> Literal["foo"]:
        return "foo"

Configuration

No response

Command used

pylint mwe.py

Pylint output

************* Module mwe
mwe.py:22:0: W0223: Method 'bar' is abstract in class 'BarProtocol' but is not overridden (abstract-method)
mwe.py:22:0: W0223: Method 'foo' is abstract in class 'FooProtocol' but is not overridden (abstract-method)

Expected behavior

No warnings

Pylint version

pylint 2.13.9
astroid 2.11.7
Python 3.9.1 (default, Mar 15 2021, 18:22:18)
[Clang 12.0.0 (clang-1200.0.32.27)]

pylint3 2.2.2
astroid 2.1.0
Python 3.7.3 (default, Jun  2 2021, 09:53:27)
[GCC 8.3.0]

OS / Environment

macOS 11.6.1

~Debian Buster

Additional dependencies

No response

@huwcbjones huwcbjones added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jul 20, 2022
@clavedeluna
Copy link
Contributor

Trying to triage this but reading PEP544 I'm more confused than ever. @huwcbjones could you maybe specify which part of the PEP suggests to you this is a FP? In reading it I'm getting the opposite in some examples.

@Pierre-Sassoulas Pierre-Sassoulas added the Waiting on author Indicate that maintainers are waiting for a message of the author label Nov 21, 2022
@DanielNoord
Copy link
Collaborator

DanielNoord commented Nov 21, 2022

We should simply never raise abstract-method if it comes from a Protocol. That should be the fix here 😄

@huwcbjones
Copy link
Contributor Author

@clavedeluna see the section about Merging and extending protocols

I agree with @DanielNoord

@clavedeluna clavedeluna added False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Waiting on author Indicate that maintainers are waiting for a message of the author labels Nov 21, 2022
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.15.9, 2.15.8 Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants