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

Define Protocol as abstract to prevent abstract-method FP #7839

Merged
merged 4 commits into from Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/7209.false_positive
@@ -0,0 +1,3 @@
Fixes false positive ``abstract-method`` on Protocol classes.

Closes #7209
12 changes: 1 addition & 11 deletions pylint/checkers/classes/class_checker.py
Expand Up @@ -2098,19 +2098,9 @@ def _check_init(self, node: nodes.FunctionDef, klass_node: nodes.ClassDef) -> No
if node_frame_class(method) in parents_with_called_inits:
return

# Return if klass is protocol
if klass.qname() in utils.TYPING_PROTOCOLS:
if utils.is_protocol_class(klass):
return

# Return if any of the klass' first-order bases is protocol
for base in klass.bases:
try:
for inf_base in base.infer():
if inf_base.qname() in utils.TYPING_PROTOCOLS:
return
except astroid.InferenceError:
continue

if decorated_with(node, ["typing.overload"]):
continue
self.add_message(
Expand Down
21 changes: 17 additions & 4 deletions pylint/checkers/utils.py
Expand Up @@ -1167,6 +1167,10 @@ def class_is_abstract(node: nodes.ClassDef) -> bool:
"""Return true if the given class node should be considered as an abstract
class.
"""
# Protocol classes are considered "abstract"
if is_protocol_class(node):
return True

# Only check for explicit metaclass=ABCMeta on this specific class
meta = node.declared_metaclass()
if meta is not None:
Expand Down Expand Up @@ -1653,14 +1657,23 @@ def is_protocol_class(cls: nodes.NodeNG) -> bool:
"""Check if the given node represents a protocol class.

:param cls: The node to check
:returns: True if the node is a typing protocol class, false otherwise.
:returns: True if the node is or inherits from typing.Protocol directly, false otherwise.
"""
if not isinstance(cls, nodes.ClassDef):
return False

# Use .ancestors() since not all protocol classes can have
# their mro deduced.
return any(parent.qname() in TYPING_PROTOCOLS for parent in cls.ancestors())
# Return if klass is protocol
if cls.qname() in TYPING_PROTOCOLS:
return True

for base in cls.bases:
try:
for inf_base in base.infer():
if inf_base.qname() in TYPING_PROTOCOLS:
return True
except astroid.InferenceError:
continue
return False
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out is_protocol_class was incorrectly defined since it concluded that if any parent/grandparent class inherited from Protocol, this class was one too, but as we discussed the class has to inherit from Protocol itself. This definition was already in the code base so I just moved it here



def is_call_of_name(node: nodes.NodeNG, name: str) -> bool:
Expand Down
46 changes: 46 additions & 0 deletions tests/functional/p/protocol_classes_abstract.py
@@ -0,0 +1,46 @@
"""Test that classes inheriting directly from Protocol should not warn about abstract-method."""

# pylint: disable=too-few-public-methods,disallowed-name,invalid-name

from abc import abstractmethod, ABCMeta
from typing import Protocol, Literal


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

@abstractmethod
def foo(self) -> Literal["foo"]:
"""foo method"""

def foo_no_abstract(self) -> Literal["foo"]:
"""foo not abstract method"""


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


class FooBarProtocol(FooProtocol, BarProtocol, Protocol):
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"""FooBar Protocol"""

class BarParent(BarProtocol): # [abstract-method]
"""Doesn't subclass typing.Protocol directly"""

class IndirectProtocol(FooProtocol): # [abstract-method]
"""Doesn't subclass typing.Protocol directly"""

class AbcProtocol(FooProtocol, metaclass=ABCMeta):
"""Doesn't subclass typing.Protocol but uses metaclass directly"""

class FooBar(FooBarProtocol):
clavedeluna marked this conversation as resolved.
Show resolved Hide resolved
"""FooBar object"""

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

def foo(self) -> Literal["foo"]:
return "foo"
2 changes: 2 additions & 0 deletions tests/functional/p/protocol_classes_abstract.rc
@@ -0,0 +1,2 @@
[testoptions]
min_pyver=3.8
2 changes: 2 additions & 0 deletions tests/functional/p/protocol_classes_abstract.txt
@@ -0,0 +1,2 @@
abstract-method:30:0:30:15:BarParent:Method 'bar' is abstract in class 'BarProtocol' but is not overridden in child class 'BarParent':INFERENCE
abstract-method:33:0:33:22:IndirectProtocol:Method 'foo' is abstract in class 'FooProtocol' but is not overridden in child class 'IndirectProtocol':INFERENCE