Improve Errors (#152)
* Added checks for child_index out of bounds
* slightly better descriptions
* updated RELEASES.md
* Changed TaffyError back to Error
* split error type into two
* fixed error in git merge
* updated RELEASES.md file
* cargo fmt
* moved errors into error module, and gave them better names
* Split InvalidChild::InvalidNode into two
* updated RELEASES.md
* cargo fmt
* more changes to RELEASES.md
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
ab22fd7f9c0fe17e31e57b32f4ec6a04939353e4 · por TheDestroyer19 · 2022-06-11T18:19:44Z (hace 4 años) · padre 3bd95d61aver el árbol en este commit
diff --git a/RELEASES.md b/RELEASES.md
index 3ef21d4e1..fc4ed9d04 100644
--- a/RELEASES.md
+++ b/RELEASES.md
@@ -4,7 +4,7 @@
### 0.2.0 Added
-- Nothing yet
+- Added `taffy::error::InvalidChild` Error type
### 0.2.0 Changed
@@ -12,6 +12,8 @@
- renamed `taffy::forest::Forest.new-node(..)` `taffy::forest::Forest.new_with_children(..)`
- renamed `taffy::node::Taffy.new-node(..)` -> `taffy::node::Taffy.new_with_children(..)`
- renamed `taffy::style::Style` -> `taffy::style::FlexboxLayout` to more precicely indicate its purpose
+- renamed `taffy::Error` -> `taffy::error::InvalidNode`
+- `taffy::Taffy::remove_child_at_index`, `taffy::Taffy::replace_child_at_index`, and `taffy::Taffy::child_at_index` now return `taffy::InvalidChild::ChildIndexOutOfBounds` instead of panicing
### 0.2.0 Fixed
diff --git a/examples/basic.rs b/examples/basic.rs
index f5576a723..751477ee9 100644
--- a/examples/basic.rs
+++ b/examples/basic.rs
@@ -1,6 +1,6 @@
use taffy::prelude::*;
-fn main() -> Result<(), Error> {
+fn main() -> Result<(), taffy::error::InvalidNode> {
let mut taffy = Taffy::new();
let child = taffy.new_with_children(
diff --git a/examples/nested.rs b/examples/nested.rs
index ba185c028..53fb7e491 100644
--- a/examples/nested.rs
+++ b/examples/nested.rs
@@ -1,6 +1,6 @@
use taffy::prelude::*;
-fn main() -> Result<(), Error> {
+fn main() -> Result<(), taffy::error::InvalidNode> {
let mut taffy = Taffy::new();
// left
diff --git a/src/error.rs b/src/error.rs
new file mode 100644
index 000000000..30a2e8d94
--- /dev/null
+++ b/src/error.rs
@@ -0,0 +1,57 @@
+//! The Error types produced by Taffy.
+#[cfg(feature = "std")]
+use core::fmt::{Display, Formatter, Result};
+
+use crate::node::Node;
+
+/// The [`Node`] was not found in the [`Taffy`](crate::Taffy) instance
+#[derive(Debug)]
+pub struct InvalidNode(pub Node);
+
+#[cfg(feature = "std")]
+impl Display for InvalidNode {
+ fn fmt(&self, f: &mut Formatter) -> Result {
+ write!(f, "Node {:?} is not in the Taffy instance", self.0)
+ }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for InvalidNode {}
+
+/// An error that occurs while trying to access or modify a [`Node`]'s children by index.
+#[derive(Debug)]
+pub enum InvalidChild {
+ /// The parent [`Node`] does not have a child at `child_index`. It only has `child_count` children
+ ChildIndexOutOfBounds {
+ /// The parent node whose child was being looked up
+ parent: Node,
+ /// The index that was looked up
+ child_index: usize,
+ /// The total number of children the parent has
+ child_count: usize,
+ },
+ /// The parent [`Node`] was not found in the [`Taffy`](crate::Taffy) instance.
+ InvalidParentNode(Node),
+ /// The child [`Node`] was not found in the [`Taffy`](crate::Taffy) instance.
+ InvalidChildNode(Node),
+}
+
+#[cfg(feature = "std")]
+impl Display for InvalidChild {
+ fn fmt(&self, f: &mut Formatter) -> Result {
+ match self {
+ InvalidChild::ChildIndexOutOfBounds { parent, child_index, child_count } => write!(
+ f,
+ "Index (is {}) should be < child_count ({}) for parent node {:?}",
+ child_index, child_count, parent
+ ),
+ InvalidChild::InvalidParentNode(parent) => {
+ write!(f, "Parent Node {:?} is not in the Taffy instance", parent)
+ }
+ InvalidChild::InvalidChildNode(child) => write!(f, "Child Node {:?} is not in the Taffy instance", child),
+ }
+ }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for InvalidChild {}
diff --git a/src/lib.rs b/src/lib.rs
index 074a1dd07..b10b07041 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -11,6 +11,7 @@ extern crate alloc;
#[cfg(feature = "serde")]
extern crate serde;
+pub mod error;
pub mod geometry;
pub mod layout;
pub mod node;
@@ -25,31 +26,3 @@ mod indexmap;
mod sys;
pub use crate::node::Taffy;
-
-#[cfg(feature = "std")]
-use core::fmt::{Display, Formatter, Result};
-
-/// An error that can occur when performing layout
-#[derive(Debug)]
-pub enum Error {
- /// The [`Node`](node::Node) was invalid
- InvalidNode(node::Node),
-}
-
-#[cfg(feature = "std")]
-impl Display for Error {
- fn fmt(&self, f: &mut Formatter) -> Result {
- match *self {
- Error::InvalidNode(ref node) => write!(f, "Invalid node {:?}", node),
- }
- }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for Error {
- fn description(&self) -> &str {
- match *self {
- Error::InvalidNode(_) => "The node is not part of the Taffy instance",
- }
- }
-}
diff --git a/src/node.rs b/src/node.rs
index 97bfd1a40..ee56bc770 100644
--- a/src/node.rs
+++ b/src/node.rs
@@ -1,6 +1,7 @@
//! UI [`Node`] types and related data structures.
//!
//! Layouts are composed of multiple nodes, which live in a forest-like data structure.
+use crate::error;
use crate::forest::Forest;
use crate::geometry::Size;
use crate::layout::Layout;
@@ -9,7 +10,6 @@ use crate::style::FlexboxLayout;
#[cfg(any(feature = "std", feature = "alloc"))]
use crate::sys::Box;
use crate::sys::{new_map_with_capacity, ChildrenVec, Map, Vec};
-use crate::Error;
use core::sync::atomic::{AtomicUsize, Ordering};
/// A function that can be applied to a `Size<Number>` to obtain a `Size<f32>`
@@ -87,15 +87,15 @@ impl Taffy {
}
/// Returns the `NodeId` of the provided node within the forest
- fn find_node(&self, node: Node) -> Result<NodeId, Error> {
+ fn find_node(&self, node: Node) -> Result<NodeId, error::InvalidNode> {
match self.nodes_to_ids.get(&node) {
Some(id) => Ok(*id),
- None => Err(Error::InvalidNode(node)),
+ None => Err(error::InvalidNode(node)),
}
}
/// Adds a new leaf node, which does not have any children
- pub fn new_leaf(&mut self, style: FlexboxLayout, measure: MeasureFunc) -> Result<Node, Error> {
+ pub fn new_leaf(&mut self, style: FlexboxLayout, measure: MeasureFunc) -> Result<Node, error::InvalidNode> {
let node = self.allocate_node();
let id = self.forest.new_leaf(style, measure);
self.add_node(node, id);
@@ -103,10 +103,12 @@ impl Taffy {
}
/// Adds a new node, which may have any number of `children`
- pub fn new_with_children(&mut self, style: FlexboxLayout, children: &[Node]) -> Result<Node, Error> {
+ pub fn new_with_children(&mut self, style: FlexboxLayout, children: &[Node]) -> Result<Node, error::InvalidNode> {
let node = self.allocate_node();
- let children =
- children.iter().map(|child| self.find_node(*child)).collect::<Result<ChildrenVec<_>, Error>>()?;
+ let children = children
+ .iter()
+ .map(|child| self.find_node(*child))
+ .collect::<Result<ChildrenVec<_>, error::InvalidNode>>()?;
let id = self.forest.new_with_children(style, children);
self.add_node(node, id);
Ok(node)
@@ -124,7 +126,7 @@ impl Taffy {
/// Remove a specific [`Node`] from the tree
///
/// Its [`Id`] is marked as invalid. Returns the id of the node removed.
- pub fn remove(&mut self, node: Node) -> Result<usize, Error> {
+ pub fn remove(&mut self, node: Node) -> Result<usize, error::InvalidNode> {
let id = self.find_node(node)?;
self.nodes_to_ids.remove(&node);
@@ -140,7 +142,7 @@ impl Taffy {
}
/// Sets the [`MeasureFunc`] of the associated node
- pub fn set_measure(&mut self, node: Node, measure: Option<MeasureFunc>) -> Result<(), Error> {
+ pub fn set_measure(&mut self, node: Node, measure: Option<MeasureFunc>) -> Result<(), error::InvalidNode> {
let id = self.find_node(node)?;
self.forest.nodes[id].measure = measure;
self.forest.mark_dirty(id);
@@ -148,7 +150,7 @@ impl Taffy {
}
/// Adds a `child` [`Node`] under the supplied `parent`
- pub fn add_child(&mut self, parent: Node, child: Node) -> Result<(), Error> {
+ pub fn add_child(&mut self, parent: Node, child: Node) -> Result<(), error::InvalidNode> {
let node_id = self.find_node(parent)?;
let child_id = self.find_node(child)?;
@@ -157,7 +159,7 @@ impl Taffy {
}
/// Directly sets the `children` of the supplied `parent`
- pub fn set_children(&mut self, parent: Node, children: &[Node]) -> Result<(), Error> {
+ pub fn set_children(&mut self, parent: Node, children: &[Node]) -> Result<(), error::InvalidNode> {
let node_id = self.find_node(parent)?;
let children_id = children.iter().map(|child| self.find_node(*child)).collect::<Result<ChildrenVec<_>, _>>()?;
@@ -179,7 +181,7 @@ impl Taffy {
/// Removes the `child` of the parent `node`
///
/// The child is not removed from the forest entirely, it is simply no longer attached to its previous parent.
- pub fn remove_child(&mut self, parent: Node, child: Node) -> Result<Node, Error> {
+ pub fn remove_child(&mut self, parent: Node, child: Node) -> Result<Node, error::InvalidNode> {
let node_id = self.find_node(parent)?;
let child_id = self.find_node(child)?;
@@ -190,9 +192,13 @@ impl Taffy {
/// Removes the child at the given `index` from the `parent`
///
/// The child is not removed from the forest entirely, it is simply no longer attached to its previous parent.
- pub fn remove_child_at_index(&mut self, parent: Node, child_index: usize) -> Result<Node, Error> {
- let node_id = self.find_node(parent)?;
- // TODO: index check
+ pub fn remove_child_at_index(&mut self, parent: Node, child_index: usize) -> Result<Node, error::InvalidChild> {
+ let node_id = self.find_node(parent).map_err(|e| error::InvalidChild::InvalidParentNode(e.0))?;
+
+ let child_count = self.forest.children[node_id].len();
+ if child_index >= child_count {
+ return Err(error::InvalidChild::ChildIndexOutOfBounds { parent, child_index, child_count });
+ }
let prev_id = self.forest.remove_child_at_index(node_id, child_index);
Ok(self.ids_to_nodes[&prev_id])
@@ -201,10 +207,19 @@ impl Taffy {
/// Replaces the child at the given `child_index` from the `parent` node with the new `child` node
///
/// The child is not removed from the forest entirely, it is simply no longer attached to its previous parent.
- pub fn replace_child_at_index(&mut self, parent: Node, child_index: usize, new_child: Node) -> Result<Node, Error> {
- let node_id = self.find_node(parent)?;
- let child_id = self.find_node(new_child)?;
+ pub fn replace_child_at_index(
+ &mut self,
+ parent: Node,
+ child_index: usize,
+ new_child: Node,
+ ) -> Result<Node, error::InvalidChild> {
+ let node_id = self.find_node(parent).map_err(|e| error::InvalidChild::InvalidParentNode(e.0))?;
+ let child_id = self.find_node(new_child).map_err(|e| error::InvalidChild::InvalidChildNode(e.0))?;
// TODO: index check
+ let child_count = self.forest.children[node_id].len();
+ if child_index >= child_count {
+ return Err(error::InvalidChild::ChildIndexOutOfBounds { parent, child_index, child_count });
+ }
self.forest.parents[child_id].push(node_id);
let old_child = core::mem::replace(&mut self.forest.children[node_id][child_index], child_id);
@@ -216,25 +231,31 @@ impl Taffy {
}
/// Returns the child [`Node`] of the parent `node` at the provided `child_index`
- pub fn child_at_index(&self, parent: Node, child_index: usize) -> Result<Node, Error> {
- let id = self.find_node(parent)?;
+ pub fn child_at_index(&self, parent: Node, child_index: usize) -> Result<Node, error::InvalidChild> {
+ let id = self.find_node(parent).map_err(|e| error::InvalidChild::InvalidParentNode(e.0))?;
+
+ let child_count = self.forest.children[id].len();
+ if child_index >= child_count {
+ return Err(error::InvalidChild::ChildIndexOutOfBounds { parent, child_index, child_count });
+ }
+
Ok(self.ids_to_nodes[&self.forest.children[id][child_index]])
}
/// Returns the number of children of the `parent` [`Node`]
- pub fn child_count(&self, parent: Node) -> Result<usize, Error> {
+ pub fn child_count(&self, parent: Node) -> Result<usize, error::InvalidNode> {
let id = self.find_node(parent)?;
Ok(self.forest.children[id].len())
}
/// Returns a list of children that belong to the [`Parent`]
- pub fn children(&self, parent: Node) -> Result<Vec<Node>, Error> {
+ pub fn children(&self, parent: Node) -> Result<Vec<Node>, error::InvalidNode> {
let id = self.find_node(parent)?;
Ok(self.forest.children[id].iter().map(|child| self.ids_to_nodes[child]).collect())
}
/// Sets the [`Style`] of the provided `node`
- pub fn set_style(&mut self, node: Node, style: FlexboxLayout) -> Result<(), Error> {
+ pub fn set_style(&mut self, node: Node, style: FlexboxLayout) -> Result<(), error::InvalidNode> {
let id = self.find_node(node)?;
self.forest.nodes[id].style = style;
self.forest.mark_dirty(id);
@@ -242,32 +263,32 @@ impl Taffy {
}
/// Gets the [`Style`] of the provided `node`
- pub fn style(&self, node: Node) -> Result<&FlexboxLayout, Error> {
+ pub fn style(&self, node: Node) -> Result<&FlexboxLayout, error::InvalidNode> {
let id = self.find_node(node)?;
Ok(&self.forest.nodes[id].style)
}
/// Return this node layout relative to its parent
- pub fn layout(&self, node: Node) -> Result<&Layout, Error> {
+ pub fn layout(&self, node: Node) -> Result<&Layout, error::InvalidNode> {
let id = self.find_node(node)?;
Ok(&self.forest.nodes[id].layout)
}
/// Marks the layout computation of this node and its children as outdated
- pub fn mark_dirty(&mut self, node: Node) -> Result<(), Error> {
+ pub fn mark_dirty(&mut self, node: Node) -> Result<(), error::InvalidNode> {
let id = self.find_node(node)?;
self.forest.mark_dirty(id);
Ok(())
}
/// Indicates whether the layout of this node (and its children) need to be recomputed
- pub fn dirty(&self, node: Node) -> Result<bool, Error> {
+ pub fn dirty(&self, node: Node) -> Result<bool, error::InvalidNode> {
let id = self.find_node(node)?;
Ok(self.forest.nodes[id].is_dirty)
}
/// Updates the stored layout of the provided `node` and its children
- pub fn compute_layout(&mut self, node: Node, size: Size<Number>) -> Result<(), Error> {
+ pub fn compute_layout(&mut self, node: Node, size: Size<Number>) -> Result<(), error::InvalidNode> {
let id = self.find_node(node)?;
self.forest.compute_layout(id, size);
Ok(())
diff --git a/src/prelude.rs b/src/prelude.rs
index 297daa9d4..107baab24 100644
--- a/src/prelude.rs
+++ b/src/prelude.rs
@@ -9,5 +9,4 @@ pub use crate::{
AlignContent, AlignItems, AlignSelf, Dimension, Display, FlexDirection, FlexWrap, FlexboxLayout,
JustifyContent, PositionType,
},
- Error,
};