Skip to content

Commit

Permalink
control setting nodes as local outside of the constructor
Browse files Browse the repository at this point in the history
1. The main reason is that a node might be assigned to its parent via
   an «alias»:

Sometimes a class accesses a member by a different name than
   "__name__" of that member: in pypy3 `list.__mul__.__name__ ==
   "__rmul__"`.

As a result, in the example above we weren't able to find
   "list.__mul__", because it was recorded only as "list.__rmul__".

2. Sometimes we want to have a parent semantically, but we don't want
   to add it to the list of locals. For example, when inferring
   properties we are creating ad-hoc properties. We wouldn't want to
   add another symbol to the locals every time we do an
   inference. (actually, there's a very good question as to why we are
   doing those ad-hoc properties but that's out of scope)

it's a part of the campaign to get rid of non-module roots
  • Loading branch information
temyurchenko committed Sep 27, 2024
1 parent 62c5bad commit 2ae573b
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 94 deletions.
33 changes: 1 addition & 32 deletions astroid/nodes/scoped_nodes/scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,6 @@ def function_to_method(n, klass):
return n


def _attach_to_parent(node: NodeNG, name: str, parent: NodeNG):
frame = parent.frame()
frame.set_local(name, node)
if frame is parent:
frame._append_node(node)


class Module(LocalsDictNodeNG):
"""Class representing an :class:`ast.Module` node.
Expand Down Expand Up @@ -1173,9 +1166,6 @@ def __init__(
end_col_offset=end_col_offset,
parent=parent,
)
if parent and not isinstance(parent, Unknown):
frame = parent.frame()
frame.set_local(name, self)

def postinit(
self,
Expand Down Expand Up @@ -1528,33 +1518,15 @@ def _infer(
yield self
return InferenceErrorInfo(node=self, context=context)

# When inferring a property, we instantiate a new `objects.Property` object,
# which in turn, because it inherits from `FunctionDef`, sets itself in the locals
# of the wrapping frame. This means that every time we infer a property, the locals
# are mutated with a new instance of the property. To avoid this, we detect this
# scenario and avoid passing the `parent` argument to the constructor.
if not self.parent:
raise ParentMissingError(target=self)
parent_frame = self.parent.frame()
property_already_in_parent_locals = self.name in parent_frame.locals and any(
isinstance(val, objects.Property) for val in parent_frame.locals[self.name]
)
# We also don't want to pass parent if the definition is within a Try node
if isinstance(
self.parent,
(node_classes.Try, node_classes.If),
):
property_already_in_parent_locals = True

prop_func = objects.Property(
function=self,
name=self.name,
lineno=self.lineno,
parent=self.parent if not property_already_in_parent_locals else None,
parent=self.parent,
col_offset=self.col_offset,
)
if property_already_in_parent_locals:
prop_func.parent = self.parent
prop_func.postinit(body=[], args=self.args, doc_node=self.doc_node)
yield prop_func
return InferenceErrorInfo(node=self, context=context)
Expand Down Expand Up @@ -1941,9 +1913,6 @@ def __init__(
end_col_offset=end_col_offset,
parent=parent,
)
if parent and not isinstance(parent, Unknown):
_attach_to_parent(self, name, parent)

for local_name, node in self.implicit_locals():
self.add_local_node(node, local_name)

Expand Down
106 changes: 52 additions & 54 deletions astroid/raw_building.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,17 @@ def _add_dunder_class(func, parent: nodes.NodeNG, member) -> None:
func.instance_attrs["__class__"] = [ast_klass]


def build_dummy(runtime_object) -> nodes.EmptyNode:
enode = nodes.EmptyNode()
enode.object = runtime_object
return enode


def attach_dummy_node(node, name: str, runtime_object=_EMPTY_OBJECT_MARKER) -> None:
"""create a dummy node and register it in the locals of the given
node with the specified name
"""
enode = nodes.EmptyNode()
enode.object = runtime_object
_attach_local_node(node, enode, name)
_attach_local_node(node, build_dummy(runtime_object), name)


def attach_const_node(node, name: str, value) -> None:
Expand Down Expand Up @@ -262,11 +266,11 @@ def register_arguments(func: nodes.FunctionDef, args: list | None = None) -> Non


def object_build_class(
node: nodes.Module | nodes.ClassDef, member: type, localname: str
node: nodes.Module | nodes.ClassDef, member: type
) -> nodes.ClassDef:
"""create astroid for a living class object"""
basenames = [base.__name__ for base in member.__bases__]
return _base_class_object_build(node, member, basenames, localname=localname)
return _base_class_object_build(node, member, basenames)


def _get_args_info_from_callable(
Expand Down Expand Up @@ -304,8 +308,8 @@ def _get_args_info_from_callable(


def object_build_function(
node: nodes.Module | nodes.ClassDef, member: _FunctionTypes, localname: str
) -> None:
node: nodes.Module | nodes.ClassDef, member: _FunctionTypes
) -> nodes.FunctionDef:
"""create astroid for a living function object"""
(
args,
Expand All @@ -315,8 +319,8 @@ def object_build_function(
kwonly_defaults,
) = _get_args_info_from_callable(member)

func = build_function(
getattr(member, "__name__", None) or localname,
return build_function(
getattr(member, "__name__", "<no-name>"),
args,
posonlyargs,
defaults,
Expand All @@ -325,44 +329,37 @@ def object_build_function(
kwonlydefaults=kwonly_defaults,
)

node.add_local_node(func, localname)


def object_build_datadescriptor(
node: nodes.Module | nodes.ClassDef, member: type, name: str
node: nodes.Module | nodes.ClassDef, member: type
) -> nodes.ClassDef:
"""create astroid for a living data descriptor object"""
return _base_class_object_build(node, member, [], name)
return _base_class_object_build(node, member, [])


def object_build_methoddescriptor(
node: nodes.Module | nodes.ClassDef,
member: _FunctionTypes,
localname: str,
) -> None:
) -> nodes.FunctionDef:
"""create astroid for a living method descriptor object"""
# FIXME get arguments ?
func = build_function(
getattr(member, "__name__", None) or localname, doc=member.__doc__
)
node.add_local_node(func, localname)
name = getattr(member, "__name__", "<no-name>")
func = build_function(name, doc=member.__doc__)
_add_dunder_class(func, node, member)
return func


def _base_class_object_build(
node: nodes.Module | nodes.ClassDef,
member: type,
basenames: list[str],
name: str | None = None,
localname: str | None = None,
) -> nodes.ClassDef:
"""create astroid for a living class object, with a given set of base names
(e.g. ancestors)
"""
class_name = name or getattr(member, "__name__", None) or localname
assert isinstance(class_name, str)
name = getattr(member, "__name__", "<no-name>")
doc = member.__doc__ if isinstance(member.__doc__, str) else None
klass = build_class(class_name, node, basenames, doc)
klass = build_class(name, node, basenames, doc)
klass._newstyle = isinstance(member, type)
try:
# limit the instantiation trick since it's too dangerous
Expand All @@ -387,10 +384,9 @@ def _base_class_object_build(

def _build_from_function(
node: nodes.Module | nodes.ClassDef,
name: str,
member: _FunctionTypes,
module: types.ModuleType,
) -> None:
) -> nodes.FunctionDef | nodes.EmptyNode:
# verify this is not an imported function
try:
code = member.__code__ # type: ignore[union-attr]
Expand All @@ -400,12 +396,11 @@ def _build_from_function(
code = None
filename = getattr(code, "co_filename", None)
if filename is None:
assert isinstance(member, object)
object_build_methoddescriptor(node, member, name)
return object_build_methoddescriptor(node, member)
elif filename != getattr(module, "__file__", None):
attach_dummy_node(node, name, member)
return build_dummy(member)
else:
object_build_function(node, member, name)
return object_build_function(node, member)

Check notice on line 403 in astroid/raw_building.py

View workflow job for this annotation

GitHub Actions / Checks

R1705

Unnecessary "elif" after "return", remove the leading "el" from "elif"


def _safe_has_attribute(obj, member: str) -> bool:
Expand Down Expand Up @@ -472,58 +467,59 @@ def object_build(
if obj in self._done:
return None
self._done[obj] = node
for name in dir(obj):
for alias in dir(obj):
# inspect.ismethod() and inspect.isbuiltin() in PyPy return
# the opposite of what they do in CPython for __class_getitem__.
pypy__class_getitem__ = IS_PYPY and name == "__class_getitem__"
pypy__class_getitem__ = IS_PYPY and alias == "__class_getitem__"
try:
with warnings.catch_warnings():
warnings.simplefilter("ignore")
member = getattr(obj, name)
member = getattr(obj, alias)
except AttributeError:
# damned ExtensionClass.Base, I know you're there !
attach_dummy_node(node, name)
attach_dummy_node(node, alias)
continue
if inspect.ismethod(member) and not pypy__class_getitem__:
member = member.__func__
if inspect.isfunction(member):
_build_from_function(node, name, member, self._module)
child = _build_from_function(node, member, self._module)
elif inspect.isbuiltin(member) or pypy__class_getitem__:
if self.imported_member(node, member, name):
if self.imported_member(node, member, alias):
continue
object_build_methoddescriptor(node, member, name)
child = object_build_methoddescriptor(node, member)
elif inspect.isclass(member):
if self.imported_member(node, member, name):
if self.imported_member(node, member, alias):
continue
if member in self._done:
class_node = self._done[member]
assert isinstance(class_node, nodes.ClassDef)
if class_node not in node.locals.get(name, ()):
node.add_local_node(class_node, name)
child = self._done[member]
assert isinstance(child, nodes.ClassDef)
else:
class_node = object_build_class(node, member, name)
child = object_build_class(node, member)
# recursion
self.object_build(class_node, member)
if name == "__class__" and class_node.parent is None:
class_node.parent = self._done[self._module]
self.object_build(child, member)
if alias == "__class__" and child.parent is None:
child.parent = self._done[self._module]

Check warning on line 501 in astroid/raw_building.py

View check run for this annotation

Codecov / codecov/patch

astroid/raw_building.py#L501

Added line #L501 was not covered by tests
elif inspect.ismethoddescriptor(member):
object_build_methoddescriptor(node, member, name)
child = object_build_methoddescriptor(node, member)

Check notice on line 503 in astroid/raw_building.py

View workflow job for this annotation

GitHub Actions / Checks

R0204

Redefinition of child type from astroid.nodes.scoped_nodes.scoped_nodes.ClassDef to astroid.nodes.scoped_nodes.scoped_nodes.FunctionDef
elif inspect.isdatadescriptor(member):
object_build_datadescriptor(node, member, name)
child = object_build_datadescriptor(node, member)
elif isinstance(member, _CONSTANTS):
attach_const_node(node, name, member)
if alias in node.special_attributes:
continue
child = nodes.const_factory(member)
elif inspect.isroutine(member):
# This should be called for Jython, where some builtin
# methods aren't caught by isbuiltin branch.
_build_from_function(node, name, member, self._module)
child = _build_from_function(node, member, self._module)
elif _safe_has_attribute(member, "__all__"):
module = build_module(name)
_attach_local_node(node, module, name)
child = build_module(alias)
# recursion
self.object_build(module, member)
self.object_build(child, member)
else:
# create an empty node so that the name is actually defined
attach_dummy_node(node, name, member)
child = build_dummy(member)
if child not in node.locals.get(alias, ()):
node.add_local_node(child, alias)
return None

def imported_member(self, node, member, name: str) -> bool:
Expand Down Expand Up @@ -629,6 +625,7 @@ def _astroid_bootstrapping() -> None:
end_col_offset=0,
parent=astroid_builtin,
)
astroid_builtin.set_local(_GeneratorType.name, _GeneratorType)
generator_doc_node = (
nodes.Const(value=types.GeneratorType.__doc__)
if types.GeneratorType.__doc__
Expand All @@ -652,6 +649,7 @@ def _astroid_bootstrapping() -> None:
end_col_offset=0,
parent=astroid_builtin,
)
astroid_builtin.set_local(_AsyncGeneratorType.name, _AsyncGeneratorType)
async_generator_doc_node = (
nodes.Const(value=types.AsyncGeneratorType.__doc__)
if types.AsyncGeneratorType.__doc__
Expand Down
2 changes: 2 additions & 0 deletions astroid/rebuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,7 @@ def visit_classdef(
else []
),
)
parent.set_local(newnode.name, newnode)
return newnode

def visit_continue(self, node: ast.Continue, parent: NodeNG) -> nodes.Continue:
Expand Down Expand Up @@ -1112,6 +1113,7 @@ def _visit_functiondef(
),
)
self._global_names.pop()
parent.set_local(newnode.name, newnode)
return newnode

def visit_functiondef(
Expand Down
11 changes: 3 additions & 8 deletions tests/test_inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -6880,20 +6880,15 @@ class A:
@property
def a(self):
return 42
A()
"""
node = extract_node(code)
# Infer the class
cls = next(node.infer())
cls = extract_node(code)
(prop,) = cls.getattr("a")

# Try to infer the property function *multiple* times. `A.locals` should be modified only once
assert len(cls.locals["a"]) == 1
for _ in range(3):
prop.inferred()
a_locals = cls.locals["a"]
# [FunctionDef, Property]
assert len(a_locals) == 2
assert len(a_locals) == 1


def test_getattr_fails_on_empty_values() -> None:
Expand Down

0 comments on commit 2ae573b

Please sign in to comment.