refactor and document internals

src/error.rs

@@ -31,17 +31,17 @@ impl fmt::Display for InsertError {
             Self::Conflict { with } => {
                 write!(
                     f,
-                    "insertion failed due to conflict with previously registered route: {}",
+                    "Insertion failed due to conflict with previously registered route: {}",
                     with
                 )
             }
             Self::InvalidParamSegment => {
-                write!(f, "only one parameter is allowed per path segment")
+                write!(f, "Only one parameter is allowed per path segment")
             }
-            Self::InvalidParam => write!(f, "parameters must be registered with a valid name"),
+            Self::InvalidParam => write!(f, "Parameters must be registered with a valid name"),
             Self::InvalidCatchAll => write!(
                 f,
-                "catch-all parameters are only allowed at the end of a route"
+                "Catch-all parameters are only allowed at the end of a route"
             ),
         }
     }
@@ -50,42 +50,49 @@ impl fmt::Display for InsertError {
 impl std::error::Error for InsertError {}
 
 impl InsertError {
+    /// Returns an error for a route conflict with the given node.
+    ///
+    /// This method attempts to find the full conflicting route.
     pub(crate) fn conflict<T>(
         route: &UnescapedRoute,
         prefix: UnescapedRef<'_>,
         current: &Node<T>,
     ) -> Self {
         let mut route = route.clone();
 
-        // The new route would have had to replace the current node in the tree.
-        if prefix.inner() == current.prefix.inner() {
-            denormalize_params(&mut route, &current.param_remapping);
+        // The route is conflicting with the current node.
+        if prefix.unescaped() == current.prefix.unescaped() {
+            denormalize_params(&mut route, &current.remapping);
             return InsertError::Conflict {
-                with: String::from_utf8(route.into_inner()).unwrap(),
+                with: String::from_utf8(route.into_unescaped()).unwrap(),
             };
         }
 
+        // Remove the non-matching suffix from the route.
         route.truncate(route.len() - prefix.len());
 
+        // Add the conflicting prefix.
         if !route.ends_with(&current.prefix) {
             route.append(&current.prefix);
         }
 
+        // Add the prefixes of any conflicting children.
+        let mut child = current.children.first();
+        while let Some(node) = child {
+            route.append(&node.prefix);
+            child = node.children.first();
+        }
+
+        // Denormalize any route parameters.
         let mut last = current;
         while let Some(node) = last.children.first() {
             last = node;
         }
+        denormalize_params(&mut route, &last.remapping);
 
-        let mut current = current.children.first();
-        while let Some(node) = current {
-            route.append(&node.prefix);
-            current = node.children.first();
-        }
-
-        denormalize_params(&mut route, &last.param_remapping);
-
+        // Return the conflicting route.
         InsertError::Conflict {
-            with: String::from_utf8(route.into_inner()).unwrap(),
+            with: String::from_utf8(route.into_unescaped()).unwrap(),
         }
     }
 }
@@ -113,7 +120,7 @@ pub enum MatchError {
 
 impl fmt::Display for MatchError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "matching route not found")
+        write!(f, "Matching route not found")
     }
 }
 

src/escape.rs

@@ -1,10 +1,11 @@
 use std::{fmt, ops::Range};
 
-/// An uescaped route that keeps track of the position of escaped characters ('{{' or '}}').
+/// An unescaped route that keeps track of the position of escaped characters ('{{' or '}}').
 ///
 /// Note that this type dereferences to `&[u8]`.
 #[derive(Clone, Default)]
 pub struct UnescapedRoute {
+    // The raw unescaped route.
     inner: Vec<u8>,
     escaped: Vec<usize>,
 }
@@ -40,10 +41,10 @@ impl UnescapedRoute {
         range: Range<usize>,
         replace: Vec<u8>,
     ) -> impl Iterator<Item = u8> + '_ {
-        // ignore any escaped characters in the range being replaced
+        // Ignore any escaped characters in the range being replaced.
         self.escaped.retain(|x| !range.contains(x));
 
-        // update the escaped indices
+        // Update the escaped indices.
         let offset = (replace.len() as isize) - (range.len() as isize);
         for i in &mut self.escaped {
             if *i > range.end {
@@ -78,13 +79,13 @@ impl UnescapedRoute {
         }
     }
 
-    /// Returns a reference to the inner slice.
-    pub fn inner(&self) -> &[u8] {
+    /// Returns a reference to the unescaped slice.
+    pub fn unescaped(&self) -> &[u8] {
         &self.inner
     }
 
-    /// Returns the inner slice.
-    pub fn into_inner(self) -> Vec<u8> {
+    /// Returns the unescaped route.
+    pub fn into_unescaped(self) -> Vec<u8> {
         self.inner
     }
 }
@@ -131,7 +132,7 @@ impl<'a> UnescapedRef<'a> {
         }
     }
 
-    /// Returns true if the character at the given index was escaped.
+    /// Returns `true` if the character at the given index was escaped.
     pub fn is_escaped(&self, i: usize) -> bool {
         if let Some(i) = i.checked_add_signed(-self.offset) {
             return self.escaped.contains(&i);
@@ -158,8 +159,8 @@ impl<'a> UnescapedRef<'a> {
         }
     }
 
-    /// Returns a reference to the inner slice.
-    pub fn inner(&self) -> &[u8] {
+    /// Returns a reference to the unescaped slice.
+    pub fn unescaped(&self) -> &[u8] {
         self.inner
     }
 }

src/params.rs

@@ -3,6 +3,9 @@ use std::{fmt, iter, mem, slice};
 /// A single URL parameter, consisting of a key and a value.
 #[derive(PartialEq, Eq, Ord, PartialOrd, Default, Copy, Clone)]
 struct Param<'k, 'v> {
+    // Keys and values are stored as byte slices internally by the router
+    // to avoid UTF8 checks when slicing, but UTF8 is still respected,
+    // so these slices are valid strings.
     key: &'k [u8],
     value: &'v [u8],
 }
@@ -13,11 +16,12 @@ impl<'k, 'v> Param<'k, 'v> {
         value: b"",
     };
 
-    // this could be from_utf8_unchecked, but we'll keep this safe for now
+    // Returns the parameter key as a string.
     fn key_str(&self) -> &'k str {
         std::str::from_utf8(self.key).unwrap()
     }
 
+    // Returns the parameter value as a string.
     fn value_str(&self) -> &'v str {
         std::str::from_utf8(self.value).unwrap()
     }
@@ -31,12 +35,12 @@ impl<'k, 'v> Param<'k, 'v> {
 /// # router.insert("/users/{id}", true).unwrap();
 /// let matched = router.at("/users/1")?;
 ///
-/// // you can iterate through the keys and values
+/// // Iterate through the keys and values.
 /// for (key, value) in matched.params.iter() {
 ///     println!("key: {}, value: {}", key, value);
 /// }
 ///
-/// // or get a specific value by key
+/// // Get a specific value by name.
 /// let id = matched.params.get("id");
 /// assert_eq!(id, Some("1"));
 /// # Ok(())
@@ -47,9 +51,11 @@ pub struct Params<'k, 'v> {
     kind: ParamsKind<'k, 'v>,
 }
 
-// most routes have 1-3 dynamic parameters, so we can avoid a heap allocation in common cases.
+// Most routes have a small number of dynamic parameters, so we can avoid
+// heap allocations in the common case.
 const SMALL: usize = 3;
 
+// A list of parameters, optimized to avoid allocations when possible.
 #[derive(PartialEq, Eq, Ord, PartialOrd, Clone)]
 enum ParamsKind<'k, 'v> {
     Small([Param<'k, 'v>; SMALL], usize),
@@ -71,6 +77,7 @@ impl<'k, 'v> Params<'k, 'v> {
         }
     }
 
+    // Truncates the parameter list to the given length.
     pub(crate) fn truncate(&mut self, n: usize) {
         match &mut self.kind {
             ParamsKind::Small(_, len) => *len = n,
@@ -133,7 +140,7 @@ impl<'k, 'v> Params<'k, 'v> {
         }
     }
 
-    // Transform each key.
+    // Applies a transformation function to each key.
     pub(crate) fn for_each_key_mut(&mut self, f: impl Fn((usize, &mut &'k [u8]))) {
         match &mut self.kind {
             ParamsKind::Small(arr, len) => arr
@@ -191,6 +198,7 @@ impl<'ps, 'k, 'v> Iterator for ParamsIter<'ps, 'k, 'v> {
         }
     }
 }
+
 impl ExactSizeIterator for ParamsIter<'_, '_, '_> {
     fn len(&self) -> usize {
         match self.kind {

src/router.rs

@@ -23,7 +23,7 @@ impl<T> Router<T> {
         Self::default()
     }
 
-    /// Insert a route.
+    /// Insert a route into the router.
     ///
     /// # Examples
     ///
@@ -37,7 +37,7 @@ impl<T> Router<T> {
     /// # }
     /// ```
     pub fn insert(&mut self, route: impl Into<String>, value: T) -> Result<(), InsertError> {
-        self.root.insert(route, value)
+        self.root.insert(route.into(), value)
     }
 
     /// Tries to find a value in the router matching the given path.
@@ -58,7 +58,7 @@ impl<T> Router<T> {
     pub fn at<'m, 'p>(&'m self, path: &'p str) -> Result<Match<'m, 'p, &'m T>, MatchError> {
         match self.root.at(path.as_bytes()) {
             Ok((value, params)) => Ok(Match {
-                // SAFETY: We only expose &mut T through &mut self
+                // Safety: We only expose `&mut T` through `&mut self`
                 value: unsafe { &*value.get() },
                 params,
             }),
@@ -88,7 +88,7 @@ impl<T> Router<T> {
     ) -> Result<Match<'m, 'p, &'m mut T>, MatchError> {
         match self.root.at(path.as_bytes()) {
             Ok((value, params)) => Ok(Match {
-                // SAFETY: We have &mut self
+                // Safety: We have `&mut self`
                 value: unsafe { &mut *value.get() },
                 params,
             }),
@@ -98,7 +98,8 @@ impl<T> Router<T> {
 
     /// Remove a given route from the router.
     ///
-    /// Returns the value stored under the route if it was found. If the route was not found or invalid, `None` is returned.
+    /// Returns the value stored under the route if it was found.
+    /// If the route was not found or invalid, `None` is returned.
     ///
     /// # Examples
     ///
@@ -125,7 +126,7 @@ impl<T> Router<T> {
     /// assert_eq!(router.remove("/home/{id}/"), Some("Hello!"));
     /// ```
     pub fn remove(&mut self, path: impl Into<String>) -> Option<T> {
-        self.root.remove(path)
+        self.root.remove(path.into())
     }
 
     #[cfg(feature = "__test_helpers")]
@@ -140,6 +141,7 @@ impl<T> Router<T> {
 pub struct Match<'k, 'v, V> {
     /// The value stored under the matched node.
     pub value: V,
+
     /// The route parameters. See [parameters](crate#parameters) for more details.
     pub params: Params<'k, 'v>,
 }