Do not append child nodes to parents body

src/databricks/labs/ucx/source_code/python/python_ast.py

@@ -213,30 +213,31 @@ def __repr__(self):
     def attach_child_tree(self, tree: Tree) -> None:
         """Attach a child tree.
 
-        Attaching a child tree is a **stateful** operation for both the parent and child tree. After attaching a child
-        tree, a tree can be traversed starting from the parent or child tree. From both starting points all nodes in
-        both trees can be reached, though, the order of nodes will be different as that is relative to the starting
-        point.
+        1. Make parent tree of the nodes in the child tree
+        2. Extend parents globals with child globals
+
+        Attaching a child tree is a **stateful** operation for the child tree. After attaching a child
+        tree, the tree can be traversed starting from the child tree as a child knows its parent. However, the tree can
+        not be traversed from the parent tree as that node object does not contain a list with children trees.
         """
         if not isinstance(tree.node, Module):
             raise NotImplementedError(f"Cannot attach child tree: {type(tree.node).__name__}")
         tree_module: Module = cast(Module, tree.node)
-        self.attach_nodes(tree_module.body)
+        self.attach_child_nodes(tree_module.body)
         self.extend_globals(tree_module.globals)
 
-    def attach_nodes(self, nodes: list[NodeNG]) -> None:
-        """Attach nodes.
+    def attach_child_nodes(self, nodes: list[NodeNG]) -> None:
+        """Attach child nodes.
 
-        Attaching nodes is a **stateful** operation for both this tree's node, the parent node, and the child nodes.
-        After attaching the nodes, the parent node has the nodes in its body and the child nodes have this tree's node
-        as parent node.
+        Attaching a child tree is a **stateful** operation for the child tree. After attaching a child
+        tree, the tree can be traversed starting from the child tree as a child knows its parent. However, the tree can
+        not be traversed from the parent tree as that node object does not contain a list with children trees.
         """
         if not isinstance(self.node, Module):
             raise NotImplementedError(f"Cannot attach nodes to: {type(self.node).__name__}")
         self_module: Module = cast(Module, self.node)
         for node in nodes:
             node.parent = self_module
-            self_module.body.append(node)
 
     def extend_globals(self, globs: dict[str, list[NodeNG]]) -> None:
         """Extend globals by extending the global values for each global key.

tests/unit/source_code/python/test_python_ast.py

@@ -158,8 +158,7 @@ def test_ignores_magic_marker_in_multiline_comment() -> None:
     assert True
 
 
-def test_tree_attach_child_tree_infers_value() -> None:
-    """Attaching trees allows traversing from both parent and child."""
+def test_tree_attach_child_tree_allows_to_infer_value() -> None:
     inferred_string = "Hello John!"
     parent_source, child_source = "a = 'John'", 'b = f"Hello {a}!"'
     parent_maybe_tree = Tree.maybe_normalized_parse(parent_source)
@@ -170,11 +169,6 @@ def test_tree_attach_child_tree_infers_value() -> None:
 
     parent_maybe_tree.tree.attach_child_tree(child_maybe_tree.tree)
 
-    nodes = parent_maybe_tree.tree.locate(Assign, [])
-    tree = Tree(nodes[1].value)  # Starting from the parent, we are looking for the last assign
-    strings = [value.as_string() for value in InferredValue.infer_from_node(tree.node)]
-    assert strings == [inferred_string]
-
     nodes = child_maybe_tree.tree.locate(Assign, [])
     tree = Tree(nodes[0].value)  # Starting from child, we are looking for the first assign
     strings = [value.as_string() for value in InferredValue.infer_from_node(tree.node)]
@@ -322,26 +316,16 @@ def test_is_builtin(source, name, is_builtin) -> None:
     assert False  # could not locate call
 
 
-def test_tree_attach_nodes_sets_parent() -> None:
+def test_tree_attach_child_nodes_sets_parent() -> None:
     node = astroid.extract_node("b = a + 2")
     maybe_tree = Tree.maybe_normalized_parse("a = 1")
     assert maybe_tree.tree, maybe_tree.failure
 
-    maybe_tree.tree.attach_nodes([node])
+    maybe_tree.tree.attach_child_nodes([node])
 
     assert node.parent == maybe_tree.tree.node
 
 
-def test_tree_attach_nodes_adds_node_to_body() -> None:
-    node = astroid.extract_node("b = a + 2")
-    maybe_tree = Tree.maybe_normalized_parse("a = 1")
-    assert maybe_tree.tree, maybe_tree.failure
-
-    maybe_tree.tree.attach_nodes([node])
-
-    assert maybe_tree.tree.node.body[-1] == node
-
-
 def test_tree_extend_globals_adds_assign_name_to_tree() -> None:
     maybe_tree = Tree.maybe_normalized_parse("a = 1")
     assert maybe_tree.tree, maybe_tree.failure
@@ -383,9 +367,9 @@ def test_tree_attach_child_tree_raises_not_implemented_error_for_constant_node()
         Tree(Const("xyz")).attach_child_tree(Tree(Const("xyz")))
 
 
-def test_tree_attach_nodes_raises_not_implemented_error_for_constant_node() -> None:
+def test_tree_attach_child_nodes_raises_not_implemented_error_for_constant_node() -> None:
     with pytest.raises(NotImplementedError, match="Cannot attach nodes to: .*"):
-        Tree(Const("xyz")).attach_nodes([])
+        Tree(Const("xyz")).attach_child_nodes([])
 
 
 def test_extend_globals_raises_not_implemented_error_for_constant_node() -> None: