-
-
Notifications
You must be signed in to change notification settings - Fork 293
Add typing to NodeNG.statement
#1217
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 20 commits
d8347f0
f9fe68d
7c47926
b6373e3
19c5ed4
baef395
005a9ff
da1b989
d95d5ac
4a9af1e
ef019e6
1f6ec98
df9b49c
6026560
d07374c
2596ee6
e8d0ff2
73fb21d
6777dc9
1900532
11aefd2
2b6e7d0
cab6a88
78588ff
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 | ||||
---|---|---|---|---|---|---|
|
@@ -44,8 +44,10 @@ | |||||
import io | ||||||
import itertools | ||||||
import os | ||||||
import sys | ||||||
import typing | ||||||
from typing import List, Optional, TypeVar | ||||||
import warnings | ||||||
from typing import List, Optional, TypeVar, Union, overload | ||||||
|
||||||
from astroid import bases | ||||||
from astroid import decorators as decorators_mod | ||||||
|
@@ -65,13 +67,26 @@ | |||||
InconsistentMroError, | ||||||
InferenceError, | ||||||
MroError, | ||||||
StatementMissing, | ||||||
TooManyLevelsError, | ||||||
) | ||||||
from astroid.interpreter.dunder_lookup import lookup | ||||||
from astroid.interpreter.objectmodel import ClassModel, FunctionModel, ModuleModel | ||||||
from astroid.manager import AstroidManager | ||||||
from astroid.nodes import Arguments, Const, node_classes | ||||||
|
||||||
if sys.version_info >= (3, 6, 2): | ||||||
DanielNoord marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
from typing import NoReturn | ||||||
else: | ||||||
from typing_extensions import NoReturn | ||||||
|
||||||
|
||||||
if sys.version_info >= (3, 8): | ||||||
from typing import Literal | ||||||
else: | ||||||
from typing_extensions import Literal | ||||||
|
||||||
|
||||||
ITER_METHODS = ("__iter__", "__getitem__") | ||||||
EXCEPTION_BASE_CLASSES = frozenset({"Exception", "BaseException"}) | ||||||
objects = util.lazy_import("objects") | ||||||
|
@@ -637,12 +652,36 @@ def fully_defined(self): | |||||
""" | ||||||
return self.file is not None and self.file.endswith(".py") | ||||||
|
||||||
def statement(self): | ||||||
@overload | ||||||
def statement(self, future: Literal[None] = ...) -> "Module": | ||||||
... | ||||||
|
||||||
# pylint: disable-next=signature-differs | ||||||
# https://github.com/PyCQA/pylint/issues/5264 | ||||||
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
It seems like our luck didn't outran us yet. This actual now triggers 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. Is We might want to enable that in another PR. 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. It's enabled, but not a failure condition! I don't think we should change that. 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. Sure? Seems like something that will be easily missed in review 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 also think we should make it a failure condition so it's handled in the proper MR each 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.
It would be as easy as adding The issue is that some errors might be Python version specific. It could create a situation where only one, Additionally, this would prevent us from having the latest 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 last point will indeed be difficult to avoid. I'll try and think of something to fix that, but in the meantime let's let this rest! 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. Do you maybe want to track this as issue in
I completely agree! 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. Shouldn't this be tracked 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. 😂 Though if we enable it, we'll likely want to do it for |
||||||
@overload | ||||||
def statement(self, future: Literal[True]) -> NoReturn: | ||||||
DanielNoord marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
... | ||||||
|
||||||
def statement( | ||||||
self, future: Literal[None, True] = None | ||||||
) -> Union[NoReturn, "Module"]: | ||||||
"""The first parent node, including self, marked as statement node. | ||||||
|
||||||
:returns: The first parent statement. | ||||||
:rtype: NodeNG | ||||||
When called on a :class:`Module` with the future parameter this raises an error. | ||||||
|
||||||
TODO: Deprecate the future parameter and only raise StatementMissing | ||||||
|
||||||
:raises StatementMissing: If no self has no parent attribute and future is True | ||||||
""" | ||||||
if future: | ||||||
raise StatementMissing(target=self) | ||||||
warnings.warn( | ||||||
"In astroid 3.0.0 NodeNG.statement() will return either a nodes.Statement " | ||||||
"or raise a StatementMissing exception. nodes.Module will no longer be " | ||||||
"considered a statement. This behaviour can already be triggered " | ||||||
"by passing 'future=True' to a statement() call.", | ||||||
DeprecationWarning, | ||||||
) | ||||||
return self | ||||||
cdce8p marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
def previous_sibling(self): | ||||||
|
Uh oh!
There was an error while loading. Please reload this page.