From a61a828956eba052a34788886bb31f67db14f80f Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Thu, 24 Nov 2022 09:17:23 -0300 Subject: [PATCH 1/4] make protocols be defined as abstract --- doc/whatsnew/fragments/7209.false_positive | 3 ++ pylint/checkers/utils.py | 4 +++ .../functional/p/protocol_classes_abstract.py | 36 +++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 doc/whatsnew/fragments/7209.false_positive create mode 100644 tests/functional/p/protocol_classes_abstract.py 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/utils.py b/pylint/checkers/utils.py index a2a0c1b378..65df5096d0 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -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: diff --git a/tests/functional/p/protocol_classes_abstract.py b/tests/functional/p/protocol_classes_abstract.py new file mode 100644 index 0000000000..95f74246e7 --- /dev/null +++ b/tests/functional/p/protocol_classes_abstract.py @@ -0,0 +1,36 @@ +"""Test that classes inheriting from protocols should not warn about abstract-method.""" +# pylint: disable=too-few-public-methods,disallowed-name,invalid-name +from abc import abstractmethod +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 FooBar(FooBarProtocol): + """FooBar object""" + + def bar(self) -> Literal["bar"]: + return "bar" + + def foo(self) -> Literal["foo"]: + return "foo" From deaba1aefbb626f8d1c8a7fdfc21ad09ae9601e6 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Fri, 25 Nov 2022 08:00:05 -0300 Subject: [PATCH 2/4] add test and rc file --- tests/functional/p/protocol_classes_abstract.py | 8 +++++++- tests/functional/p/protocol_classes_abstract.rc | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 tests/functional/p/protocol_classes_abstract.rc diff --git a/tests/functional/p/protocol_classes_abstract.py b/tests/functional/p/protocol_classes_abstract.py index 95f74246e7..f417d9729f 100644 --- a/tests/functional/p/protocol_classes_abstract.py +++ b/tests/functional/p/protocol_classes_abstract.py @@ -1,5 +1,7 @@ """Test that classes inheriting from protocols should not warn about abstract-method.""" + # pylint: disable=too-few-public-methods,disallowed-name,invalid-name + from abc import abstractmethod from typing import Protocol, Literal @@ -14,6 +16,7 @@ def foo(self) -> Literal["foo"]: def foo_no_abstract(self) -> Literal["foo"]: """foo not abstract method""" + class BarProtocol(Protocol): """Bar Protocol""" @abstractmethod @@ -21,11 +24,14 @@ def bar(self) -> Literal["bar"]: """bar method""" - class FooBarProtocol(FooProtocol, BarProtocol, Protocol): """FooBar Protocol""" +class IndirectProtocol(FooProtocol): + """Doesn't subclass typing.Protocol directly""" + + class FooBar(FooBarProtocol): """FooBar object""" 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 From a74c8bfffff143067890057a9dba8325220cbd05 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Mon, 28 Nov 2022 18:46:21 -0300 Subject: [PATCH 3/4] define Protocol cls as must inherit directly from Protocol --- pylint/checkers/classes/class_checker.py | 12 +----------- pylint/checkers/utils.py | 17 +++++++++++++---- tests/functional/p/protocol_classes_abstract.py | 8 +++++--- .../functional/p/protocol_classes_abstract.txt | 1 + 4 files changed, 20 insertions(+), 18 deletions(-) create mode 100644 tests/functional/p/protocol_classes_abstract.txt diff --git a/pylint/checkers/classes/class_checker.py b/pylint/checkers/classes/class_checker.py index e2806ef433..67acd25af5 100644 --- a/pylint/checkers/classes/class_checker.py +++ b/pylint/checkers/classes/class_checker.py @@ -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( diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 65df5096d0..585c3f894b 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -1657,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 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 index f417d9729f..7b7b89f42d 100644 --- a/tests/functional/p/protocol_classes_abstract.py +++ b/tests/functional/p/protocol_classes_abstract.py @@ -1,8 +1,8 @@ -"""Test that classes inheriting from protocols should not warn about abstract-method.""" +"""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 +from abc import abstractmethod, ABCMeta from typing import Protocol, Literal @@ -28,9 +28,11 @@ class FooBarProtocol(FooProtocol, BarProtocol, Protocol): """FooBar Protocol""" -class IndirectProtocol(FooProtocol): +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""" diff --git a/tests/functional/p/protocol_classes_abstract.txt b/tests/functional/p/protocol_classes_abstract.txt new file mode 100644 index 0000000000..70e9b16b3c --- /dev/null +++ b/tests/functional/p/protocol_classes_abstract.txt @@ -0,0 +1 @@ +abstract-method:31:0:31:22:IndirectProtocol:Method 'foo' is abstract in class 'FooProtocol' but is not overridden in child class 'IndirectProtocol':INFERENCE From a7b119d11e62b5e20b80a0e3aa2e7794a3af73e2 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Wed, 30 Nov 2022 07:50:49 -0300 Subject: [PATCH 4/4] add one more test case --- tests/functional/p/protocol_classes_abstract.py | 2 ++ tests/functional/p/protocol_classes_abstract.txt | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/functional/p/protocol_classes_abstract.py b/tests/functional/p/protocol_classes_abstract.py index 7b7b89f42d..ad8ec5cf5c 100644 --- a/tests/functional/p/protocol_classes_abstract.py +++ b/tests/functional/p/protocol_classes_abstract.py @@ -27,6 +27,8 @@ def bar(self) -> Literal["bar"]: 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""" diff --git a/tests/functional/p/protocol_classes_abstract.txt b/tests/functional/p/protocol_classes_abstract.txt index 70e9b16b3c..9b099ceb7f 100644 --- a/tests/functional/p/protocol_classes_abstract.txt +++ b/tests/functional/p/protocol_classes_abstract.txt @@ -1 +1,2 @@ -abstract-method:31:0:31:22:IndirectProtocol:Method 'foo' is abstract in class 'FooProtocol' but is not overridden in child class 'IndirectProtocol':INFERENCE +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