Merge pull request #40 from rust-scraper/noop-prepend-append-id

Do not produce invalid trees when no-op instances of append_id or prepend_id operations are performed.
rust-scraper · Dec 8, 2024 · b8adcd1 · b8adcd1
2 parents 8127d31 + 8aba49f
commit b8adcd1
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 30 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -557,24 +557,27 @@ impl<'a, T: 'a> NodeMut<'a, T> {
         );
 
         let last_child_id = self.node().children.map(|(_, id)| id);
-        {
-            let mut new_child = self.tree.get_mut(new_child_id).unwrap();
-            new_child.detach();
-            new_child.node().parent = Some(self.id);
-            new_child.node().prev_sibling = last_child_id;
-        }
 
-        if let Some(id) = last_child_id {
-            unsafe {
-                self.tree.node_mut(id).next_sibling = Some(new_child_id);
+        if last_child_id != Some(new_child_id) {
+            {
+                let mut new_child = self.tree.get_mut(new_child_id).unwrap();
+                new_child.detach();
+                new_child.node().parent = Some(self.id);
+                new_child.node().prev_sibling = last_child_id;
             }
-        }
 
-        {
-            self.node().children = match self.node().children {
-                Some((first_child_id, _)) => Some((first_child_id, new_child_id)),
-                None => Some((new_child_id, new_child_id)),
-            };
+            if let Some(id) = last_child_id {
+                unsafe {
+                    self.tree.node_mut(id).next_sibling = Some(new_child_id);
+                }
+            }
+
+            {
+                self.node().children = match self.node().children {
+                    Some((first_child_id, _)) => Some((first_child_id, new_child_id)),
+                    None => Some((new_child_id, new_child_id)),
+                };
+            }
         }
 
         unsafe { self.tree.get_unchecked_mut(new_child_id) }
@@ -593,24 +596,27 @@ impl<'a, T: 'a> NodeMut<'a, T> {
         );
 
         let first_child_id = self.node().children.map(|(id, _)| id);
-        {
-            let mut new_child = self.tree.get_mut(new_child_id).unwrap();
-            new_child.detach();
-            new_child.node().parent = Some(self.id);
-            new_child.node().next_sibling = first_child_id;
-        }
 
-        if let Some(id) = first_child_id {
-            unsafe {
-                self.tree.node_mut(id).prev_sibling = Some(new_child_id);
+        if first_child_id != Some(new_child_id) {
+            {
+                let mut new_child = self.tree.get_mut(new_child_id).unwrap();
+                new_child.detach();
+                new_child.node().parent = Some(self.id);
+                new_child.node().next_sibling = first_child_id;
             }
-        }
 
-        {
-            self.node().children = match self.node().children {
-                Some((_, last_child_id)) => Some((new_child_id, last_child_id)),
-                None => Some((new_child_id, new_child_id)),
-            };
+            if let Some(id) = first_child_id {
+                unsafe {
+                    self.tree.node_mut(id).prev_sibling = Some(new_child_id);
+                }
+            }
+
+            {
+                self.node().children = match self.node().children {
+                    Some((_, last_child_id)) => Some((new_child_id, last_child_id)),
+                    None => Some((new_child_id, new_child_id)),
+                };
+            }
         }
 
         unsafe { self.tree.get_unchecked_mut(new_child_id) }

diff --git a/tests/node_mut.rs b/tests/node_mut.rs
@@ -325,6 +325,20 @@ fn append_id_itself() {
     e.append_id(e.id());
 }
 
+#[test]
+fn append_id_noop() {
+    let mut tree = tree! {
+        'a' => {
+            'b' => { 'c', 'd' },
+            'e' => { 'f', 'g' },
+        }
+    };
+    let mut a = tree.root_mut();
+    let e_id = a.last_child().unwrap().id();
+    a.append_id(e_id);
+    assert_eq!(a.first_child().unwrap().next_sibling().unwrap().id(), e_id);
+}
+
 #[test]
 #[should_panic(expected = "Cannot prepend node as a child to itself")]
 fn prepend_id_itself() {
@@ -339,6 +353,20 @@ fn prepend_id_itself() {
     e.prepend_id(e.id());
 }
 
+#[test]
+fn prepend_id_noop() {
+    let mut tree = tree! {
+        'a' => {
+            'b' => { 'c', 'd' },
+            'e' => { 'f', 'g' },
+        }
+    };
+    let mut a = tree.root_mut();
+    let b_id = a.first_child().unwrap().id();
+    a.prepend_id(b_id);
+    assert_eq!(a.last_child().unwrap().prev_sibling().unwrap().id(), b_id);
+}
+
 #[test]
 fn reparent_from_id_append() {
     let mut tree = tree! {