From 72e73f4a39cf3d23e1a3fb1509bc2651d3f0bbb8 Mon Sep 17 00:00:00 2001 From: Amir <13120817+amirshukayev@users.noreply.github.com> Date: Thu, 11 Sep 2025 02:50:54 -0400 Subject: [PATCH 1/3] Fix reparent methods to update parent for all siblings Previously only first and last siblings had their parent updated, leaving middle siblings with stale parent references. --- src/lib.rs | 18 ++++++++----- tests/node_mut.rs | 64 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3b42f0f..58955c2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -843,9 +843,12 @@ impl<'a, T: 'a> NodeMut<'a, T> { } }; - unsafe { - self.tree.node_mut(new_child_ids.0).parent = Some(self.id); - self.tree.node_mut(new_child_ids.1).parent = Some(self.id); + // Update parent for all siblings in the chain + let mut child_id = new_child_ids.0; + loop { + unsafe { self.tree.node_mut(child_id).parent = Some(self.id); } + if child_id == new_child_ids.1 { break; } + child_id = unsafe { self.tree.node_mut(child_id).next_sibling.unwrap() }; } if self.node().children.is_none() { @@ -882,9 +885,12 @@ impl<'a, T: 'a> NodeMut<'a, T> { } }; - unsafe { - self.tree.node_mut(new_child_ids.0).parent = Some(self.id); - self.tree.node_mut(new_child_ids.1).parent = Some(self.id); + // Update parent for all siblings in the chain + let mut child_id = new_child_ids.0; + loop { + unsafe { self.tree.node_mut(child_id).parent = Some(self.id); } + if child_id == new_child_ids.1 { break; } + child_id = unsafe { self.tree.node_mut(child_id).next_sibling.unwrap() }; } if self.node().children.is_none() { diff --git a/tests/node_mut.rs b/tests/node_mut.rs index a93d30b..e862754 100644 --- a/tests/node_mut.rs +++ b/tests/node_mut.rs @@ -492,6 +492,70 @@ fn reparent_from_id_prepend() { assert_eq!(Some(d), f.prev_sibling()); } +#[test] +fn reparent_from_id_append_multiple_siblings() { + // Test reparenting when source has 3+ children to ensure all siblings get proper parent update + let mut tree = tree! { + 'a' => { + 'b' => { 'x' }, // destination node + 'c' => { 'd', 'e', 'f' }, // source node with 3 children + } + }; + + let c_id = tree.root().last_child().unwrap().id(); + let b_id = tree.root().first_child().unwrap().id(); + + // Reparent children from 'c' to 'b' + tree.root_mut() + .first_child() + .unwrap() + .reparent_from_id_append(c_id); + + let b = tree.get(b_id).unwrap(); + let x = b.first_child().unwrap(); // original child of b + let d = x.next_sibling().unwrap(); // first reparented child + let e = d.next_sibling().unwrap(); // middle reparented child + let f = e.next_sibling().unwrap(); // last reparented child + + // All children should now have 'b' as their parent + assert_eq!(Some(b), x.parent()); + assert_eq!(Some(b), d.parent()); + assert_eq!(Some(b), e.parent()); + assert_eq!(Some(b), f.parent()); +} + +#[test] +fn reparent_from_id_prepend_multiple_siblings() { + // Test reparenting when source has 3+ children to ensure all siblings get proper parent update + let mut tree = tree! { + 'a' => { + 'b' => { 'x' }, // destination node + 'c' => { 'd', 'e', 'f' }, // source node with 3 children + } + }; + + let c_id = tree.root().last_child().unwrap().id(); + let b_id = tree.root().first_child().unwrap().id(); + + // Reparent children from 'c' to 'b' + tree.root_mut() + .first_child() + .unwrap() + .reparent_from_id_prepend(c_id); + + let b = tree.get(b_id).unwrap(); + let d = b.first_child().unwrap(); // first reparented child + let e = d.next_sibling().unwrap(); // middle reparented child + let f = e.next_sibling().unwrap(); // last reparented child + let x = f.next_sibling().unwrap(); // original child of b + + // All children should now have 'b' as their parent + assert_eq!(Some(b), d.parent()); + assert_eq!(Some(b), e.parent()); + assert_eq!(Some(b), f.parent()); + assert_eq!(Some(b), x.parent()); +} + #[test] fn into() { let mut tree = tree!('a'); From 525bcde0692c6cd657b6e96c0fcc12c607f4a953 Mon Sep 17 00:00:00 2001 From: Amir <13120817+amirshukayev@users.noreply.github.com> Date: Thu, 11 Sep 2025 11:01:56 -0400 Subject: [PATCH 2/3] Cleanup + `cargo fmt` Cleaned up loop to skip 2nd access to `tree.node_mut`, and rust fmt the unit tests. --- src/lib.rs | 20 ++++++++++++-------- tests/node_mut.rs | 10 +++++----- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 58955c2..580a60c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -843,12 +843,14 @@ impl<'a, T: 'a> NodeMut<'a, T> { } }; - // Update parent for all siblings in the chain let mut child_id = new_child_ids.0; loop { - unsafe { self.tree.node_mut(child_id).parent = Some(self.id); } - if child_id == new_child_ids.1 { break; } - child_id = unsafe { self.tree.node_mut(child_id).next_sibling.unwrap() }; + let child = unsafe { self.tree.node_mut(child_id) }; + child.parent = Some(self.id); + child_id = match child.next_sibling { + Some(id) => id, + None => break, + }; } if self.node().children.is_none() { @@ -885,12 +887,14 @@ impl<'a, T: 'a> NodeMut<'a, T> { } }; - // Update parent for all siblings in the chain let mut child_id = new_child_ids.0; loop { - unsafe { self.tree.node_mut(child_id).parent = Some(self.id); } - if child_id == new_child_ids.1 { break; } - child_id = unsafe { self.tree.node_mut(child_id).next_sibling.unwrap() }; + let child = unsafe { self.tree.node_mut(child_id) }; + child.parent = Some(self.id); + child_id = match child.next_sibling { + Some(id) => id, + None => break, + }; } if self.node().children.is_none() { diff --git a/tests/node_mut.rs b/tests/node_mut.rs index e862754..177e9a7 100644 --- a/tests/node_mut.rs +++ b/tests/node_mut.rs @@ -512,10 +512,10 @@ fn reparent_from_id_append_multiple_siblings() { .reparent_from_id_append(c_id); let b = tree.get(b_id).unwrap(); - let x = b.first_child().unwrap(); // original child of b - let d = x.next_sibling().unwrap(); // first reparented child - let e = d.next_sibling().unwrap(); // middle reparented child - let f = e.next_sibling().unwrap(); // last reparented child + let x = b.first_child().unwrap(); // original child of b + let d = x.next_sibling().unwrap(); // first reparented child + let e = d.next_sibling().unwrap(); // middle reparented child + let f = e.next_sibling().unwrap(); // last reparented child // All children should now have 'b' as their parent assert_eq!(Some(b), x.parent()); @@ -544,7 +544,7 @@ fn reparent_from_id_prepend_multiple_siblings() { .reparent_from_id_prepend(c_id); let b = tree.get(b_id).unwrap(); - let d = b.first_child().unwrap(); // first reparented child + let d = b.first_child().unwrap(); // first reparented child let e = d.next_sibling().unwrap(); // middle reparented child let f = e.next_sibling().unwrap(); // last reparented child let x = f.next_sibling().unwrap(); // original child of b From af715710af0801420212445ffe4734f47b9c6dcc Mon Sep 17 00:00:00 2001 From: Amir <13120817+amirshukayev@users.noreply.github.com> Date: Thu, 11 Sep 2025 11:18:47 -0400 Subject: [PATCH 3/3] fix mismatched_lifetime_syntaxes lint from Rust 1.89 --- src/iter.rs | 6 +++--- src/lib.rs | 48 ++++++++++++++++++++++++------------------------ 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/iter.rs b/src/iter.rs index a40777c..fe591ef 100644 --- a/src/iter.rs +++ b/src/iter.rs @@ -114,17 +114,17 @@ impl IntoIterator for Tree { impl Tree { /// Returns an iterator over values in insert order. - pub fn values(&self) -> Values { + pub fn values(&self) -> Values<'_, T> { Values(self.vec.iter()) } /// Returns a mutable iterator over values in insert order. - pub fn values_mut(&mut self) -> ValuesMut { + pub fn values_mut(&mut self) -> ValuesMut<'_, T> { ValuesMut(self.vec.iter_mut()) } /// Returns an iterator over nodes in insert order. - pub fn nodes(&self) -> Nodes { + pub fn nodes(&self) -> Nodes<'_, T> { Nodes { tree: self, iter: 0..self.vec.len(), diff --git a/src/lib.rs b/src/lib.rs index 580a60c..79cc9d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -178,7 +178,7 @@ impl Tree { } /// Returns a reference to the specified node. - pub fn get(&self, id: NodeId) -> Option> { + pub fn get(&self, id: NodeId) -> Option> { self.vec.get(id.to_index()).map(|node| NodeRef { id, node, @@ -187,7 +187,7 @@ impl Tree { } /// Returns a mutator of the specified node. - pub fn get_mut(&mut self, id: NodeId) -> Option> { + pub fn get_mut(&mut self, id: NodeId) -> Option> { let exists = self.vec.get(id.to_index()).map(|_| ()); exists.map(move |_| NodeMut { id, tree: self }) } @@ -203,7 +203,7 @@ impl Tree { /// Returns a reference to the specified node. /// # Safety /// The caller must ensure that `id` is a valid node ID. - pub unsafe fn get_unchecked(&self, id: NodeId) -> NodeRef { + pub unsafe fn get_unchecked(&self, id: NodeId) -> NodeRef<'_, T> { NodeRef { id, node: self.node(id), @@ -214,22 +214,22 @@ impl Tree { /// Returns a mutator of the specified node. /// # Safety /// The caller must ensure that `id` is a valid node ID. - pub unsafe fn get_unchecked_mut(&mut self, id: NodeId) -> NodeMut { + pub unsafe fn get_unchecked_mut(&mut self, id: NodeId) -> NodeMut<'_, T> { NodeMut { id, tree: self } } /// Returns a reference to the root node. - pub fn root(&self) -> NodeRef { + pub fn root(&self) -> NodeRef<'_, T> { unsafe { self.get_unchecked(NodeId::from_index(0)) } } /// Returns a mutator of the root node. - pub fn root_mut(&mut self) -> NodeMut { + pub fn root_mut(&mut self) -> NodeMut<'_, T> { unsafe { self.get_unchecked_mut(NodeId::from_index(0)) } } /// Creates an orphan node. - pub fn orphan(&mut self, value: T) -> NodeMut { + pub fn orphan(&mut self, value: T) -> NodeMut<'_, T> { let id = unsafe { NodeId::from_index(self.vec.len()) }; self.vec.push(Node::new(value)); unsafe { self.get_unchecked_mut(id) } @@ -238,7 +238,7 @@ impl Tree { /// Merge with another tree as orphan, returning the new root of tree being merged. // Allowing this for compactness. #[allow(clippy::option_map_unit_fn)] - pub fn extend_tree(&mut self, mut other_tree: Tree) -> NodeMut { + pub fn extend_tree(&mut self, mut other_tree: Tree) -> NodeMut<'_, T> { let offset = self.vec.len(); let offset_id = |id: NodeId| -> NodeId { let old_index = id.to_index(); @@ -374,7 +374,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { unsafe { self.tree.get_unchecked(self.id) } } - fn axis(&mut self, f: F) -> Option> + fn axis(&mut self, f: F) -> Option> where F: FnOnce(&mut Node) -> Option, { @@ -394,7 +394,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { } /// Returns the parent of this node. - pub fn parent(&mut self) -> Option> { + pub fn parent(&mut self) -> Option> { self.axis(|node| node.parent) } @@ -407,7 +407,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { } /// Returns the previous sibling of this node. - pub fn prev_sibling(&mut self) -> Option> { + pub fn prev_sibling(&mut self) -> Option> { self.axis(|node| node.prev_sibling) } @@ -420,7 +420,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { } /// Returns the next sibling of this node. - pub fn next_sibling(&mut self) -> Option> { + pub fn next_sibling(&mut self) -> Option> { self.axis(|node| node.next_sibling) } @@ -433,7 +433,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { } /// Returns the first child of this node. - pub fn first_child(&mut self) -> Option> { + pub fn first_child(&mut self) -> Option> { self.axis(|node| node.children.map(|(id, _)| id)) } @@ -446,7 +446,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { } /// Returns the last child of this node. - pub fn last_child(&mut self) -> Option> { + pub fn last_child(&mut self) -> Option> { self.axis(|node| node.children.map(|(_, id)| id)) } @@ -579,25 +579,25 @@ impl<'a, T: 'a> NodeMut<'a, T> { } /// Appends a new child to this node. - pub fn append(&mut self, value: T) -> NodeMut { + pub fn append(&mut self, value: T) -> NodeMut<'_, T> { let id = self.tree.orphan(value).id; self.append_id(id) } /// Prepends a new child to this node. - pub fn prepend(&mut self, value: T) -> NodeMut { + pub fn prepend(&mut self, value: T) -> NodeMut<'_, T> { let id = self.tree.orphan(value).id; self.prepend_id(id) } /// Appends a subtree, return the root of the merged subtree. - pub fn append_subtree(&mut self, subtree: Tree) -> NodeMut { + pub fn append_subtree(&mut self, subtree: Tree) -> NodeMut<'_, T> { let root_id = self.tree.extend_tree(subtree).id; self.append_id(root_id) } /// Prepends a subtree, return the root of the merged subtree. - pub fn prepend_subtree(&mut self, subtree: Tree) -> NodeMut { + pub fn prepend_subtree(&mut self, subtree: Tree) -> NodeMut<'_, T> { let root_id = self.tree.extend_tree(subtree).id; self.prepend_id(root_id) } @@ -607,7 +607,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { /// # Panics /// /// Panics if this node is an orphan. - pub fn insert_before(&mut self, value: T) -> NodeMut { + pub fn insert_before(&mut self, value: T) -> NodeMut<'_, T> { let id = self.tree.orphan(value).id; self.insert_id_before(id) } @@ -617,7 +617,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { /// # Panics /// /// Panics if this node is an orphan. - pub fn insert_after(&mut self, value: T) -> NodeMut { + pub fn insert_after(&mut self, value: T) -> NodeMut<'_, T> { let id = self.tree.orphan(value).id; self.insert_id_after(id) } @@ -664,7 +664,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { /// # Panics /// /// Panics if `new_child_id` is not valid. - pub fn append_id(&mut self, new_child_id: NodeId) -> NodeMut { + pub fn append_id(&mut self, new_child_id: NodeId) -> NodeMut<'_, T> { assert_ne!( self.id(), new_child_id, @@ -703,7 +703,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { /// # Panics /// /// Panics if `new_child_id` is not valid. - pub fn prepend_id(&mut self, new_child_id: NodeId) -> NodeMut { + pub fn prepend_id(&mut self, new_child_id: NodeId) -> NodeMut<'_, T> { assert_ne!( self.id(), new_child_id, @@ -743,7 +743,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { /// /// - Panics if `new_sibling_id` is not valid. /// - Panics if this node is an orphan. - pub fn insert_id_before(&mut self, new_sibling_id: NodeId) -> NodeMut { + pub fn insert_id_before(&mut self, new_sibling_id: NodeId) -> NodeMut<'_, T> { assert_ne!( self.id(), new_sibling_id, @@ -786,7 +786,7 @@ impl<'a, T: 'a> NodeMut<'a, T> { /// /// - Panics if `new_sibling_id` is not valid. /// - Panics if this node is an orphan. - pub fn insert_id_after(&mut self, new_sibling_id: NodeId) -> NodeMut { + pub fn insert_id_after(&mut self, new_sibling_id: NodeId) -> NodeMut<'_, T> { assert_ne!( self.id(), new_sibling_id,