Skip to content
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

Fix frame() error on inferred node #1263

Merged
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
fff4db4
Fix frame() error on inferred node
tushar-deepsource Nov 20, 2021
2cab065
Update node_ng.py
tushar-deepsource Nov 20, 2021
8952c64
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 20, 2021
4ea13b2
Update node_ng.py
tushar-deepsource Nov 20, 2021
42ce9f4
Add FrameMissing exception
tushar-deepsource Nov 21, 2021
4629c93
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 21, 2021
9b2c483
Fix flake8 lint
tushar-deepsource Nov 21, 2021
27c1707
Fix pylint error
tushar-deepsource Nov 21, 2021
1b59f00
Remove unnecessary isinstance
tushar-deepsource Dec 14, 2021
54bcb59
Add types and remove exception type
tushar-deepsource Dec 20, 2021
7bee391
Add visit_unknown
tushar-deepsource Nov 21, 2021
9e55d84
Formatting
tushar-deepsource Dec 20, 2021
9da479c
Upgrade pylint to 2.12.2 (#1297)
Pierre-Sassoulas Dec 15, 2021
9fb55c5
Fix flake8 lint
tushar-deepsource Nov 21, 2021
dac07c6
Add types and remove exception type
tushar-deepsource Dec 20, 2021
3163665
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 20, 2021
08014ed
Remove unrelated code
tushar-deepsource Dec 20, 2021
67281cb
Hack to trigger ci
tushar-deepsource Dec 20, 2021
3bee4f5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 20, 2021
db901ce
Remove unused import
tushar-deepsource Dec 20, 2021
a01e510
something
tushar-deepsource Dec 20, 2021
410445b
Move import to TYPE_CHECKING block
tushar-deepsource Dec 20, 2021
1d6dc24
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 20, 2021
a3b2db9
Unused import
tushar-deepsource Dec 20, 2021
4a1a4ad
ci
tushar-deepsource Dec 20, 2021
b9f0ee0
Merge remote-tracking branch 'upstream/main' into tushar-deepsource-p…
tushar-deepsource Dec 24, 2021
42b787e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 24, 2021
9be8a89
Fix imports
tushar-deepsource Dec 24, 2021
08bcd08
Apply suggestions from code review
tushar-deepsource Dec 24, 2021
79bb6c2
Update astroid/nodes/node_ng.py
tushar-deepsource Dec 24, 2021
76faa99
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 24, 2021
1f1f7e7
Docstring
tushar-deepsource Dec 24, 2021
c68db00
Update astroid/nodes/node_ng.py
tushar-deepsource Dec 24, 2021
8bbba41
Add changelog entry
tushar-deepsource Dec 28, 2021
7ad182c
Move changelog
tushar-deepsource Dec 28, 2021
1fe3c11
whitespace
tushar-deepsource Dec 28, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ What's New in astroid 2.10.0?
=============================
Release date: TBA


* ``NodeNG.frame()`` and ``NodeNG.statement()`` will start raising ``ParentMissingError``
instead of ``AttributeError`` in astroid 3.0. This behaviour can already be triggered
by passing ``future=True`` to a ``frame()`` or ``statement()`` call.

What's New in astroid 2.9.1?
============================
Expand Down
5 changes: 4 additions & 1 deletion astroid/nodes/node_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
from typing_extensions import Literal

if TYPE_CHECKING:
from astroid import nodes
from astroid.nodes import LocalsDictNodeNG


Expand Down Expand Up @@ -4795,7 +4796,9 @@ def postinit(self, target: NodeNG, value: NodeNG) -> None:
See astroid/protocols.py for actual implementation.
"""

def frame(self):
def frame(
self, *, future: Literal[None, True] = None
) -> Union["nodes.FunctionDef", "nodes.Module", "nodes.ClassDef", "nodes.Lambda"]:
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"""The first parent frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
Expand Down
16 changes: 14 additions & 2 deletions astroid/nodes/node_ng.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def statement(
return self.parent.statement(future=future)

def frame(
self,
self, *, future: Literal[None, True] = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not

Suggested change
self, *, future: Literal[None, True] = None
self, *, future: bool = False

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea behind it was to only allow two specific call options:

  • node.frame()
  • node.frame(future=True)

It doesn't really make sense to add future=None or even future=False if you don't support the future case.

--
After your comment I noticed that we probably need to add overloads here to make that happen.

@overload
def frame(self) -> Union[<all types>]:
    ...
@overload
def frame(self, *, future: Literal[True]) -> Union[<all types>]:
    ...

This needs to be added to all frame definitions as overloads aren't inherited.

The overloads for statement() also need to be fixed, but I can do that in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably also need a pylint: disable comment, as there is a false-positive with the arguments-differ message and overloads: pylint-dev/pylint#5264

    # pylint: disable-next=arguments-differ
    # https://github.com/PyCQA/pylint/issues/5264

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, the PR to fix the node.statement() overload typing: #1308

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the overload idea doesn't actually work as I though. Sorry for the confusion. The default with None is needed for the parent calls.

...
return self.parent.frame(future=future)

I've already opened another PR to fix the node.statement calls #1317.

) -> Union["nodes.FunctionDef", "nodes.Module", "nodes.ClassDef", "nodes.Lambda"]:
"""The first parent frame node.

Expand All @@ -316,7 +316,19 @@ def frame(

:returns: The first parent frame node.
"""
return self.parent.frame()
if self.parent is None:
if future:
raise ParentMissingError(target=self)
warnings.warn(
"In astroid 3.0.0 NodeNG.frame() will return either a Frame node, "
"or raise ParentMissingError. AttributeError will no longer be raised. "
"This behaviour can already be triggered "
"by passing 'future=True' to a frame() call.",
DeprecationWarning,
)
raise AttributeError(f"{self} object has no attribute 'parent'")

return self.parent.frame(future=future)

def scope(self) -> "nodes.LocalsDictNodeNG":
"""The first parent node defining a new scope.
Expand Down
8 changes: 4 additions & 4 deletions astroid/nodes/scoped_nodes/scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ def bool_value(self, context=None):
def get_children(self):
yield from self.body

def frame(self: T) -> T:
def frame(self: T, *, future: Literal[None, True] = None) -> T:
"""The node's frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
Expand Down Expand Up @@ -1474,7 +1474,7 @@ def get_children(self):
yield self.args
yield self.body

def frame(self: T) -> T:
def frame(self: T, *, future: Literal[None, True] = None) -> T:
"""The node's frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
Expand Down Expand Up @@ -2004,7 +2004,7 @@ def scope_lookup(self, node, name, offset=0):
return self, [frame]
return super().scope_lookup(node, name, offset)

def frame(self: T) -> T:
def frame(self: T, *, future: Literal[None, True] = None) -> T:
"""The node's frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
Expand Down Expand Up @@ -3251,7 +3251,7 @@ def _get_assign_nodes(self):
)
return list(itertools.chain.from_iterable(children_assign_nodes))

def frame(self: T) -> T:
def frame(self: T, *, future: Literal[None, True] = None) -> T:
"""The node's frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
Expand Down
8 changes: 8 additions & 0 deletions tests/unittest_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
AstroidBuildingError,
AstroidSyntaxError,
AttributeInferenceError,
ParentMissingError,
StatementMissing,
)
from astroid.nodes.node_classes import (
Expand Down Expand Up @@ -641,6 +642,13 @@ def _test(self, value: Any) -> None:
with self.assertRaises(StatementMissing):
node.statement(future=True)

with self.assertRaises(AttributeError):
with pytest.warns(DeprecationWarning) as records:
node.frame()
assert len(records) == 1
with self.assertRaises(ParentMissingError):
node.frame(future=True)

def test_none(self) -> None:
self._test(None)

Expand Down