From bb96989847da8673f6abf301582d655ddd70631b Mon Sep 17 00:00:00 2001 From: Dani Alcala <112832187+clavedeluna@users.noreply.github.com> Date: Wed, 30 Nov 2022 16:49:08 -0300 Subject: [PATCH] Define Protocol as abstract to prevent abstract-method FP (#7839) (cherry picked from commit 85e7d93ebbc47b480f1f32bac44633a2b34fc200) --- doc/whatsnew/fragments/7209.false_positive | 3 ++ pylint/checkers/classes/class_checker.py | 12 +---- pylint/checkers/utils.py | 21 +++++++-- .../functional/p/protocol_classes_abstract.py | 46 +++++++++++++++++++ .../functional/p/protocol_classes_abstract.rc | 2 + .../p/protocol_classes_abstract.txt | 2 + 6 files changed, 71 insertions(+), 15 deletions(-) create mode 100644 doc/whatsnew/fragments/7209.false_positive create mode 100644 tests/functional/p/protocol_classes_abstract.py create mode 100644 tests/functional/p/protocol_classes_abstract.rc create mode 100644 tests/functional/p/protocol_classes_abstract.txt diff --git a/doc/whatsnew/fragments/7209.false_positive b/doc/whatsnew/fragments/7209.false_positive new file mode 100644 index 0000000000..a1ba0c5d8a --- /dev/null +++ b/doc/whatsnew/fragments/7209.false_positive @@ -0,0 +1,3 @@ +Fixes false positive ``abstract-method`` on Protocol classes. + +Closes #7209 diff --git a/pylint/checkers/classes/class_checker.py b/pylint/checkers/classes/class_checker.py index eb157187e2..4e877458a8 100644 --- a/pylint/checkers/classes/class_checker.py +++ b/pylint/checkers/classes/class_checker.py @@ -2085,19 +2085,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( diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 0ccf1d883d..90339ad508 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -1151,6 +1151,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: @@ -1595,14 +1599,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 def is_call_of_name(node: nodes.NodeNG, name: str) -> bool: diff --git a/tests/functional/p/protocol_classes_abstract.py b/tests/functional/p/protocol_classes_abstract.py new file mode 100644 index 0000000000..ad8ec5cf5c --- /dev/null +++ b/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): + """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): + """FooBar object""" + + def bar(self) -> Literal["bar"]: + return "bar" + + def foo(self) -> Literal["foo"]: + return "foo" diff --git a/tests/functional/p/protocol_classes_abstract.rc b/tests/functional/p/protocol_classes_abstract.rc new file mode 100644 index 0000000000..85fc502b37 --- /dev/null +++ b/tests/functional/p/protocol_classes_abstract.rc @@ -0,0 +1,2 @@ +[testoptions] +min_pyver=3.8 diff --git a/tests/functional/p/protocol_classes_abstract.txt b/tests/functional/p/protocol_classes_abstract.txt new file mode 100644 index 0000000000..eea3756a03 --- /dev/null +++ b/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:UNDEFINED +abstract-method:33:0:33:22:IndirectProtocol:Method 'foo' is abstract in class 'FooProtocol' but is not overridden:UNDEFINED