-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Finish typing of pylint.pyreverse.utils
#6549
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 1 commit
235b09e
943440b
d8d2301
41943a4
60707b1
5fb91a1
d0136b4
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 | ||||
---|---|---|---|---|---|---|
|
@@ -11,10 +11,17 @@ | |||||
import shutil | ||||||
import subprocess | ||||||
import sys | ||||||
from collections.abc import Callable | ||||||
from typing import TYPE_CHECKING, Any | ||||||
|
||||||
import astroid | ||||||
from astroid import nodes | ||||||
|
||||||
if TYPE_CHECKING: | ||||||
from pylint.pyreverse.diadefslib import DiaDefGenerator | ||||||
from pylint.pyreverse.inspector import Linker | ||||||
|
||||||
|
||||||
RCFILE = ".pyreverserc" | ||||||
|
||||||
|
||||||
|
@@ -46,7 +53,7 @@ def insert_default_options() -> None: | |||||
PROTECTED = re.compile(r"^_\w*$") | ||||||
|
||||||
|
||||||
def get_visibility(name): | ||||||
def get_visibility(name: str) -> str: | ||||||
"""Return the visibility from a name: public, protected, private or special.""" | ||||||
if SPECIAL.match(name): | ||||||
visibility = "special" | ||||||
|
@@ -60,37 +67,18 @@ def get_visibility(name): | |||||
return visibility | ||||||
|
||||||
|
||||||
ABSTRACT = re.compile(r"^.*Abstract.*") | ||||||
FINAL = re.compile(r"^[^\W\da-z]*$") | ||||||
|
||||||
|
||||||
def is_abstract(node): | ||||||
"""Return true if the given class node correspond to an abstract class | ||||||
definition. | ||||||
""" | ||||||
return ABSTRACT.match(node.name) | ||||||
|
||||||
|
||||||
def is_final(node): | ||||||
"""Return true if the given class/function node correspond to final | ||||||
definition. | ||||||
""" | ||||||
return FINAL.match(node.name) | ||||||
|
||||||
|
||||||
def is_interface(node): | ||||||
def is_interface(node: nodes.ClassDef) -> bool: | ||||||
# bw compatibility | ||||||
return node.type == "interface" | ||||||
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. So, here, mypy thinks we return "Any" ?! 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. Yes, no clue why. Maybe because the 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. Thank you, that explains why this happens here. def get_annotation_label(ann: nodes.Name | nodes.NodeNG) -> str:
if isinstance(ann, nodes.Name) and ann.name is not None:
return ann.name. # <-- l.213
if isinstance(ann, nodes.NodeNG):
return ann.as_string() # <-- l.215
return "" Using 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. Although we have already added some annotations to astroid, mypy will not use them until we add a 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. |
||||||
|
||||||
|
||||||
def is_exception(node): | ||||||
def is_exception(node: nodes.ClassDef) -> bool: | ||||||
# bw compatibility | ||||||
return node.type == "exception" | ||||||
|
||||||
|
||||||
# Helpers ##################################################################### | ||||||
|
||||||
_CONSTRUCTOR = 1 | ||||||
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. Dead code |
||||||
_SPECIAL = 2 | ||||||
_PROTECTED = 4 | ||||||
_PRIVATE = 8 | ||||||
|
@@ -111,7 +99,7 @@ def is_exception(node): | |||||
class FilterMixIn: | ||||||
"""Filter nodes according to a mode and nodes' visibility.""" | ||||||
|
||||||
def __init__(self, mode): | ||||||
def __init__(self, mode: str) -> None: | ||||||
"""Init filter modes.""" | ||||||
__mode = 0 | ||||||
for nummod in mode.split("+"): | ||||||
|
@@ -121,7 +109,7 @@ def __init__(self, mode): | |||||
print(f"Unknown filter mode {ex}", file=sys.stderr) | ||||||
self.__mode = __mode | ||||||
|
||||||
def show_attr(self, node): | ||||||
def show_attr(self, node: nodes.NodeNG | str) -> bool: | ||||||
"""Return true if the node should be treated.""" | ||||||
visibility = get_visibility(getattr(node, "name", node)) | ||||||
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.
Suggested change
Unrelated, but this is quite a funny hack 😄 |
||||||
return not self.__mode & VIS_MOD[visibility] | ||||||
|
@@ -137,11 +125,17 @@ class ASTWalker: | |||||
the node in lower case | ||||||
""" | ||||||
|
||||||
def __init__(self, handler): | ||||||
def __init__(self, handler: DiaDefGenerator | Linker | LocalsVisitor) -> None: | ||||||
self.handler = handler | ||||||
self._cache = {} | ||||||
|
||||||
def walk(self, node, _done=None): | ||||||
self._cache: dict[ | ||||||
type[nodes.NodeNG], | ||||||
tuple[ | ||||||
Callable[[nodes.NodeNG], Any] | None, | ||||||
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. Don't we know what these 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. As mentioned above: most of the time 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. I'd be in favour of using the narrowed down type if we can. We can always change it after a refactor later. |
||||||
Callable[[nodes.NodeNG], Any] | None, | ||||||
], | ||||||
] = {} | ||||||
|
||||||
def walk(self, node: nodes.NodeNG, _done: set[nodes.NodeNG] | None = None) -> None: | ||||||
"""Walk on the tree from <node>, getting callbacks from handler.""" | ||||||
if _done is None: | ||||||
_done = set() | ||||||
|
@@ -155,7 +149,11 @@ def walk(self, node, _done=None): | |||||
self.leave(node) | ||||||
assert node.parent is not node | ||||||
|
||||||
def get_callbacks(self, node): | ||||||
def get_callbacks( | ||||||
self, node: nodes.NodeNG | ||||||
) -> tuple[ | ||||||
Callable[[nodes.NodeNG], Any] | None, Callable[[nodes.NodeNG], Any] | None | ||||||
]: | ||||||
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 must be a better way to make a shorthand notation for it (maybe by creating a 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. You can use: 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. For the callable parameter type, take a look at my comment here as it's not completely correct (but probably the most practical): -- 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. Thank you two for the suggestions! I somehow thought just declaring a variable with the type might not work, but I should have probably just tried it. 😄 @cdce8p in most cases, yes - but |
||||||
"""Get callbacks from handler for the visited node.""" | ||||||
klass = node.__class__ | ||||||
methods = self._cache.get(klass) | ||||||
|
@@ -173,13 +171,13 @@ def get_callbacks(self, node): | |||||
e_method, l_method = methods | ||||||
return e_method, l_method | ||||||
|
||||||
def visit(self, node): | ||||||
def visit(self, node: nodes.NodeNG) -> None: | ||||||
"""Walk on the tree from <node>, getting callbacks from handler.""" | ||||||
method = self.get_callbacks(node)[0] | ||||||
if method is not None: | ||||||
method(node) | ||||||
|
||||||
def leave(self, node): | ||||||
def leave(self, node: nodes.NodeNG) -> None: | ||||||
"""Walk on the tree from <node>, getting callbacks from handler.""" | ||||||
method = self.get_callbacks(node)[1] | ||||||
if method is not None: | ||||||
|
@@ -189,11 +187,11 @@ def leave(self, node): | |||||
class LocalsVisitor(ASTWalker): | ||||||
"""Visit a project by traversing the locals dictionary.""" | ||||||
|
||||||
def __init__(self): | ||||||
def __init__(self) -> None: | ||||||
super().__init__(self) | ||||||
self._visited = set() | ||||||
self._visited: set[nodes.NodeNG] = set() | ||||||
|
||||||
def visit(self, node): | ||||||
def visit(self, node: nodes.NodeNG) -> Any: | ||||||
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. Is this truly 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. With the updated typing, should this be If that's the case, the visit method is probably not type compatible with the base implementation in 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. Good catch. I can update both methods to use 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. On second thought: Currently the 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. That should work. Alternatively, |
||||||
"""Launch the visit starting from the given node.""" | ||||||
if node in self._visited: | ||||||
return None | ||||||
|
@@ -253,7 +251,7 @@ def get_annotation( | |||||
return ann | ||||||
|
||||||
|
||||||
def infer_node(node: nodes.AssignAttr | nodes.AssignName) -> set: | ||||||
def infer_node(node: nodes.AssignAttr | nodes.AssignName) -> set[Any]: | ||||||
"""Return a set containing the node annotation if it exists | ||||||
otherwise return a set of the inferred types using the NodeNG.infer method. | ||||||
""" | ||||||
|
@@ -269,7 +267,7 @@ def infer_node(node: nodes.AssignAttr | nodes.AssignName) -> set: | |||||
return {ann} if ann else set() | ||||||
|
||||||
|
||||||
def check_graphviz_availability(): | ||||||
def check_graphviz_availability() -> None: | ||||||
"""Check if the ``dot`` command is available on the machine. | ||||||
|
||||||
This is needed if image output is desired and ``dot`` is used to convert | ||||||
|
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.
Dead code -> removed instead of adding typing