From 36094eddd69fe8c1862b6a6bd16024304e952b7a Mon Sep 17 00:00:00 2001 From: temyurchenko <44875844+temyurchenko@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:14:13 -0400 Subject: [PATCH] control setting nodes as local outside of the constructor (#2588) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ChangeLog | 3 + astroid/nodes/scoped_nodes/scoped_nodes.py | 33 +------ astroid/raw_building.py | 107 ++++++++++----------- astroid/rebuilder.py | 2 + tests/test_inference.py | 11 +-- 5 files changed, 60 insertions(+), 96 deletions(-) diff --git a/ChangeLog b/ChangeLog index 18f2c1607a..56d97ad391 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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? diff --git a/astroid/nodes/scoped_nodes/scoped_nodes.py b/astroid/nodes/scoped_nodes/scoped_nodes.py index 81c4d8dd20..176cb8dbd4 100644 --- a/astroid/nodes/scoped_nodes/scoped_nodes.py +++ b/astroid/nodes/scoped_nodes/scoped_nodes.py @@ -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. @@ -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, @@ -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) @@ -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) diff --git a/astroid/raw_building.py b/astroid/raw_building.py index 6343d51cc4..b4e812f386 100644 --- a/astroid/raw_building.py +++ b/astroid/raw_building.py @@ -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: @@ -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( @@ -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, @@ -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__", ""), args, posonlyargs, defaults, @@ -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__", "") + 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__", "") 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 @@ -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] @@ -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: @@ -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: @@ -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__ @@ -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__ diff --git a/astroid/rebuilder.py b/astroid/rebuilder.py index 8c6b74b327..4c77906e02 100644 --- a/astroid/rebuilder.py +++ b/astroid/rebuilder.py @@ -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: @@ -1112,6 +1113,7 @@ def _visit_functiondef( ), ) self._global_names.pop() + parent.set_local(newnode.name, newnode) return newnode def visit_functiondef( diff --git a/tests/test_inference.py b/tests/test_inference.py index acc8a2b1f8..8ddf04bc35 100644 --- a/tests/test_inference.py +++ b/tests/test_inference.py @@ -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: