Skip to content

Commit

Permalink
control setting nodes as local outside of the constructor (#2588)
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 authored Sep 30, 2024
1 parent 32cb29e commit 36094ed
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 96 deletions.
3 changes: 3 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ What's New in astroid 3.3.5?
============================
Release date: TBA

* Control setting local nodes outside of the supposed local's constructor.

Closes pylint-dev/astroid/issues/1490


What's New in astroid 3.3.4?
Expand Down
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
107 changes: 51 additions & 56 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,10 @@ 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)
elif filename != getattr(module, "__file__", None):
attach_dummy_node(node, name, member)
else:
object_build_function(node, member, name)
return object_build_methoddescriptor(node, member)
if filename == getattr(module, "__file__", None):
return object_build_function(node, member)
return build_dummy(member)


def _safe_has_attribute(obj, member: str) -> bool:
Expand Down Expand Up @@ -472,58 +466,57 @@ 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)
elif inspect.ismethoddescriptor(member):
object_build_methoddescriptor(node, member, name)
child: nodes.NodeNG = object_build_methoddescriptor(node, member)
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: nodes.NodeNG = 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: nodes.NodeNG = 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 +622,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 +646,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 36094ed

Please sign in to comment.