-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Refactor and deprecate resolving of interface implementations in pyreverse
#6713
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
Changes from all commits
29ae03e
66a79f6
5dd961d
8a3dc44
dd75fba
5e2a150
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -281,10 +281,6 @@ Other Changes | |
|
||
Closes #6644 | ||
|
||
* ``pylint.pyreverse.ASTWalker`` has been removed, as it was only used internally by a single child class. | ||
|
||
Ref #6712 | ||
|
||
|
||
Deprecations | ||
============ | ||
|
@@ -387,6 +383,15 @@ Deprecations | |
|
||
Ref #5392 | ||
|
||
* ``pylint.pyreverse.ASTWalker`` has been removed, as it was only used internally by a single child class. | ||
|
||
Ref #6712 | ||
|
||
* ``pyreverse``: Resolving and displaying implemented interfaces that are defined by the ``__implements__`` | ||
attribute has been deprecated and will be removed in 3.0. | ||
|
||
Comment on lines
+390
to
+392
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done 😊 |
||
Ref #6713 | ||
|
||
* ``is_class_subscriptable_pep585_with_postponed_evaluation_enabled`` has been deprecated. | ||
Use ``is_postponed_evaluation_enabled(node) and is_node_in_type_annotation_context(node)`` | ||
instead. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
import collections | ||
import os | ||
import traceback | ||
import warnings | ||
from collections.abc import Generator | ||
from typing import Any, Callable, Optional | ||
|
||
|
@@ -24,11 +25,6 @@ | |
_WrapperFuncT = Callable[[Callable[[str], nodes.Module], str], Optional[nodes.Module]] | ||
|
||
|
||
def _iface_hdlr(_: nodes.NodeNG | Any) -> bool: | ||
"""Handler used by interfaces to handle suspicious interface nodes.""" | ||
return True | ||
|
||
|
||
def _astroid_wrapper( | ||
func: Callable[[str], nodes.Module], modname: str | ||
) -> nodes.Module | None: | ||
|
@@ -42,25 +38,21 @@ def _astroid_wrapper( | |
return None | ||
|
||
|
||
def interfaces( | ||
node: nodes.ClassDef, | ||
herited: bool = True, | ||
handler_func: Callable[[nodes.NodeNG | Any], bool] = _iface_hdlr, | ||
) -> Generator[Any, None, None]: | ||
def interfaces(node: nodes.ClassDef) -> Generator[Any, None, None]: | ||
"""Return an iterator on interfaces implemented by the given class node.""" | ||
try: | ||
implements = astroid.bases.Instance(node).getattr("__implements__")[0] | ||
except astroid.exceptions.NotFoundError: | ||
return | ||
if not herited and implements.frame(future=True) is not node: | ||
if implements.frame(future=True) is not node: | ||
return | ||
found = set() | ||
missing = False | ||
for iface in nodes.unpack_infer(implements): | ||
if iface is astroid.Uninferable: | ||
missing = True | ||
continue | ||
if iface not in found and handler_func(iface): | ||
if iface not in found: | ||
found.add(iface) | ||
yield iface | ||
if missing: | ||
|
@@ -135,13 +127,9 @@ class Linker(IdGeneratorMixIn, utils.LocalsVisitor): | |
list of implemented interface _objects_ (only on astroid.Class nodes) | ||
""" | ||
|
||
def __init__( | ||
self, project: Project, inherited_interfaces: bool = False, tag: bool = False | ||
) -> None: | ||
def __init__(self, project: Project, tag: bool = False) -> None: | ||
IdGeneratorMixIn.__init__(self) | ||
utils.LocalsVisitor.__init__(self) | ||
# take inherited interface in consideration or not | ||
self.inherited_interfaces = inherited_interfaces | ||
# tag nodes or not | ||
self.tag = tag | ||
# visited project | ||
|
@@ -196,8 +184,18 @@ def visit_classdef(self, node: nodes.ClassDef) -> None: | |
self.handle_assignattr_type(assignattr, node) | ||
# resolve implemented interface | ||
try: | ||
ifaces = interfaces(node, self.inherited_interfaces) | ||
node.implements = list(ifaces) if ifaces is not None else [] | ||
ifaces = interfaces(node) | ||
if ifaces is not None: | ||
node.implements = list(ifaces) | ||
DudeNr33 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# TODO: 3.0: Remove support for __implements__ | ||
warnings.warn( | ||
"pyreverse will drop support for resolving and displaying implemented interfaces in pylint 3.0. " | ||
"The implementation relies on the '__implements__' attribute proposed in PEP 245, which was rejected " | ||
"in 2006.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, 2006 😄 I was copy pasting javascript and basic for high school calculator at the time. |
||
DeprecationWarning, | ||
) | ||
else: | ||
node.implements = [] | ||
except astroid.InferenceError: | ||
node.implements = [] | ||
|
||
|
@@ -213,11 +211,6 @@ def visit_functiondef(self, node: nodes.FunctionDef) -> None: | |
if self.tag: | ||
node.uid = self.generate_id() | ||
|
||
link_project = visit_project | ||
link_module = visit_module | ||
link_class = visit_classdef | ||
link_function = visit_functiondef | ||
|
||
def visit_assignname(self, node: nodes.AssignName) -> None: | ||
"""Visit an astroid.AssignName node. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -110,7 +110,7 @@ class Concrete23(Concrete1): pass | |
("Concrete0", ["MyIFace"]), | ||
("Concrete1", ["MyIFace", "AnotherIFace"]), | ||
("Concrete2", ["MyIFace", "AnotherIFace"]), | ||
("Concrete23", ["MyIFace", "AnotherIFace"]), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason this test changed is that the unit test actually used the default value of |
||
("Concrete23", []), | ||
): | ||
klass = module[klass] | ||
assert [i.name for i in inspector.interfaces(klass)] == interfaces | ||
|
@@ -130,3 +130,18 @@ def test_project_node(project: Project) -> None: | |
"data.suppliermodule_test", | ||
] | ||
assert sorted(project.keys()) == expected | ||
|
||
|
||
def test_interface_deprecation(project: Project) -> None: | ||
linker = inspector.Linker(project) | ||
cls = astroid.extract_node( | ||
''' | ||
class IMachin: pass | ||
|
||
class Concrete: #@ | ||
"""docstring""" | ||
__implements__ = (IMachin,) | ||
''' | ||
) | ||
with pytest.warns(DeprecationWarning): | ||
linker.visit_classdef(cls) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now under "Deprecations" in order to be grouped with #6713, although this is technically not a deprecation but a hard removal. Let me know if that's OK for you to put it there.