Skip to content

Refactor and add typing to NodeNG.frame() #1225

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

Merged
merged 5 commits into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions astroid/nodes/node_ng.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from astroid.nodes.const import OP_PRECEDENCE

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

# Types for 'NodeNG.nodes_of_class()'
T_Nodes = TypeVar("T_Nodes", bound="NodeNG")
Expand Down Expand Up @@ -258,18 +258,19 @@ def statement(self):
return self
return self.parent.statement()

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

A frame node is a :class:`Module`, :class:`FunctionDef`,
or :class:`ClassDef`.
:class:`ClassDef` or :class:`Lambda`.

:returns: The first parent frame node.
:rtype: Module or FunctionDef or ClassDef
"""
return self.parent.frame()

def scope(self) -> "LocalsDictNodeNG":
def scope(self) -> "nodes.LocalsDictNodeNG":
"""The first parent node defining a new scope.
These can be Module, FunctionDef, ClassDef, Lambda, or GeneratorExp nodes.

Expand Down
60 changes: 39 additions & 21 deletions astroid/nodes/scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,17 +230,6 @@ def qname(self):
return self.name
return f"{self.parent.frame().qname()}.{self.name}"

def frame(self):
"""The first parent frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
or :class:`ClassDef`.

:returns: The first parent frame node.
:rtype: Module or FunctionDef or ClassDef
"""
return self

def scope(self: T) -> T:
"""The first parent node defining a new scope.

Expand Down Expand Up @@ -826,20 +815,19 @@ def bool_value(self, context=None):
def get_children(self):
yield from self.body


class ComprehensionScope(LocalsDictNodeNG):
"""Scoping for different types of comprehensions."""

def frame(self):
"""The first parent frame node.
def frame(self: T) -> T:
"""The node's frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
or :class:`ClassDef`.
:class:`ClassDef` or :class:`Lambda`.

:returns: The first parent frame node.
:rtype: Module or FunctionDef or ClassDef
:returns: The node itself.
"""
return self.parent.frame()
return self


class ComprehensionScope(LocalsDictNodeNG):
"""Scoping for different types of comprehensions."""

scope_lookup = LocalsDictNodeNG._scope_lookup

Expand Down Expand Up @@ -1344,6 +1332,16 @@ def get_children(self):
yield self.args
yield self.body

def frame(self: T) -> T:
"""The node's frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
:class:`ClassDef` or :class:`Lambda`.

:returns: The node itself.
"""
return self


class FunctionDef(mixins.MultiLineBlockMixin, node_classes.Statement, Lambda):
"""Class representing an :class:`ast.FunctionDef`.
Expand Down Expand Up @@ -1839,6 +1837,16 @@ def scope_lookup(self, node, name, offset=0):
return self, [frame]
return super().scope_lookup(node, name, offset)

def frame(self: T) -> T:
"""The node's frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
:class:`ClassDef` or :class:`Lambda`.

:returns: The node itself.
"""
return self


class AsyncFunctionDef(FunctionDef):
"""Class representing an :class:`ast.FunctionDef` node.
Expand Down Expand Up @@ -3054,3 +3062,13 @@ def _get_assign_nodes(self):
child_node._get_assign_nodes() for child_node in self.body
)
return list(itertools.chain.from_iterable(children_assign_nodes))

def frame(self: T) -> T:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use ClassDef here if we duplicate the function for typing purpose ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could, but this will tell the typing engine the same. self gets set to T so T is ClassDef here. This works similar to scope() which also is able to distinguish between NodeNG.scope() and ClassDef.scope(). You can test the latter in pylint yourself already!

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the explanation :)

"""The node's frame node.

A frame node is a :class:`Module`, :class:`FunctionDef`,
:class:`ClassDef` or :class:`Lambda`.

:returns: The node itself.
"""
return self
51 changes: 51 additions & 0 deletions tests/unittest_scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@

from astroid import MANAGER, builder, nodes, objects, test_utils, util
from astroid.bases import BoundMethod, Generator, Instance, UnboundMethod
from astroid.const import PY38_PLUS
from astroid.exceptions import (
AttributeInferenceError,
DuplicateBasesError,
Expand Down Expand Up @@ -2289,5 +2290,55 @@ class First(object, object): #@
astroid["First"].slots()


class TestFrameNodes:
@pytest.mark.skipif(not PY38_PLUS, reason="needs assignment expressions")
@staticmethod
def test_frame_node():
"""Test if the frame of FunctionDef, ClassDef and Module is correctly set"""
module = builder.parse(
"""
def func():
var_1 = x
return var_1

class MyClass:

attribute = 1

def method():
pass

VAR = lambda y = (named_expr := "walrus"): print(y)
"""
)
function = module.body[0]
assert function.frame() == function
assert function.body[0].frame() == function

class_node = module.body[1]
assert class_node.frame() == class_node
assert class_node.body[0].frame() == class_node
assert class_node.body[1].frame() == class_node.body[1]

lambda_assignment = module.body[2].value
assert lambda_assignment.args.args[0].frame() == lambda_assignment

assert module.frame() == module

@staticmethod
def test_non_frame_node():
"""Test if the frame of non frame nodes is set correctly"""
module = builder.parse(
"""
VAR_ONE = 1

VAR_TWO = [x for x in range(1)]
"""
)
assert module.body[0].frame() == module

assert module.body[1].value.locals["x"][0].frame() == module


if __name__ == "__main__":
unittest.main()