Merge pull request #259 from linebender/identifiers-mimic-names

Make code for Identifier match Name

src/fontinfo.rs

@@ -1750,7 +1750,7 @@ mod tests {
                     Line::Angle { x: 1.0, y: 2.0, degrees: 0.0 },
                     Some(Name::new_raw(" [locked]")),
                     Some(Color::new(1.0, 1.0, 1.0, 1.0).unwrap()),
-                    Some(Identifier::new("abc").unwrap()),
+                    Some(Identifier::new_raw("abc")),
                     None
                 ),
             ])
@@ -1907,14 +1907,14 @@ mod tests {
                 Line::Horizontal(10.0),
                 None,
                 None,
-                Some(Identifier::new("test1").unwrap()),
+                Some(Identifier::new_raw("test1")),
                 None,
             ),
             Guideline::new(
                 Line::Vertical(20.0),
                 None,
                 None,
-                Some(Identifier::new("test2").unwrap()),
+                Some(Identifier::new_raw("test2")),
                 None,
             ),
         ]);
@@ -1925,14 +1925,14 @@ mod tests {
                 Line::Horizontal(10.0),
                 None,
                 None,
-                Some(Identifier::new("test1").unwrap()),
+                Some(Identifier::new_raw("test1")),
                 None,
             ),
             Guideline::new(
                 Line::Vertical(20.0),
                 None,
                 None,
-                Some(Identifier::new("test1").unwrap()),
+                Some(Identifier::new_raw("test1")),
                 None,
             ),
         ]);

src/glyph/builder.rs

@@ -203,7 +203,7 @@ mod tests {
     fn builder_basic() -> Result<(), ErrorKind> {
         let mut outline_builder = OutlineBuilder::new();
         outline_builder
-            .begin_path(Some(Identifier::new("abc").unwrap()))?
+            .begin_path(Some(Identifier::new_raw("abc")))?
             .add_point((173.0, 536.0), PointType::Line, false, None, None)?
             .add_point((85.0, 536.0), PointType::Line, false, None, None)?
             .add_point((85.0, 0.0), PointType::Line, false, None, None)?
@@ -212,13 +212,13 @@ mod tests {
                 PointType::Line,
                 false,
                 None,
-                Some(Identifier::new("def").unwrap()),
+                Some(Identifier::new_raw("def")),
             )?
             .end_path()?
             .add_component(
                 Name::new_raw("hallo"),
                 AffineTransform::default(),
-                Some(Identifier::new("xyz").unwrap()),
+                Some(Identifier::new_raw("xyz")),
             );
         let (contours, components) = outline_builder.finish()?;
 
@@ -235,11 +235,11 @@ mod tests {
                         PointType::Line,
                         false,
                         None,
-                        Some(Identifier::new("def").unwrap()),
+                        Some(Identifier::new_raw("def")),
                         None,
                     ),
                 ],
-                Some(Identifier::new("abc").unwrap()),
+                Some(Identifier::new_raw("abc")),
                 None,
             )]
         );
@@ -256,7 +256,7 @@ mod tests {
                     x_offset: 0.0,
                     y_offset: 0.0,
                 },
-                Some(Identifier::new("xyz").unwrap()),
+                Some(Identifier::new_raw("xyz")),
                 None,
             )]
         );
@@ -268,15 +268,15 @@ mod tests {
     #[should_panic(expected = "UnfinishedDrawing")]
     fn outline_builder_unfinished_drawing() {
         let mut outline_builder = OutlineBuilder::new();
-        outline_builder.begin_path(Some(Identifier::new("abc").unwrap())).unwrap();
+        outline_builder.begin_path(Some(Identifier::new_raw("abc"))).unwrap();
         outline_builder.finish().unwrap();
     }
 
     #[test]
     #[should_panic(expected = "UnfinishedDrawing")]
     fn outline_builder_unfinished_drawing2() {
         OutlineBuilder::new()
-            .begin_path(Some(Identifier::new("abc").unwrap()))
+            .begin_path(Some(Identifier::new_raw("abc")))
             .unwrap()
             .begin_path(None)
             .unwrap();

src/guideline.rs

@@ -186,7 +186,7 @@ mod tests {
             Line::Angle { x: 10.0, y: 20.0, degrees: 360.0 },
             Some(Name::new_raw("hello")),
             Some(Color::new(0.0, 0.5, 0.0, 0.5).unwrap()),
-            Some(Identifier::new("abcABC123").unwrap()),
+            Some(Identifier::new_raw("abcABC123")),
             None,
         );
         assert_tokens(

src/identifier.rs

@@ -1,5 +1,4 @@
 use std::hash::Hash;
-use std::str::FromStr;
 use std::sync::Arc;
 
 use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
@@ -19,45 +18,76 @@ use crate::error::ErrorKind;
 pub struct Identifier(Arc<str>);
 
 impl Identifier {
-    /// Create a new [`Identifier`] from a [`String`], if it is valid.
+    /// Create a new [`Identifier`] from a string, if it is valid.
     ///
     /// A valid identifier must have between 0 and 100 characters, and each
     /// character must be in the printable ASCII range, 0x20 to 0x7E.
-    pub fn new(s: impl Into<Arc<str>>) -> Result<Self, ErrorKind> {
-        let string = s.into();
-        if is_valid_identifier(&string) {
-            Ok(Identifier(string))
+    pub fn new(string: &str) -> Result<Self, ErrorKind> {
+        if is_valid_identifier(string) {
+            Ok(Identifier(string.into()))
         } else {
             Err(ErrorKind::BadIdentifier)
         }
     }
 
+    /// Creates a new `Identifier`, panicking if the given identifier is invalid.
+    #[cfg(test)]
+    pub(crate) fn new_raw(string: &str) -> Self {
+        assert!(is_valid_identifier(string));
+        Self(string.into())
+    }
+
     /// Create a new [`Identifier`] from a UUID v4 identifier.
     pub fn from_uuidv4() -> Self {
-        Self::new(uuid::Uuid::new_v4().to_string()).unwrap()
+        Self::new(uuid::Uuid::new_v4().to_string().as_ref()).unwrap()
     }
 
     /// Return the raw identifier, as a `&str`.
     pub fn as_str(&self) -> &str {
-        &self.0
+        self.as_ref()
     }
 }
 
-impl PartialEq<String> for Identifier {
-    fn eq(&self, other: &String) -> bool {
-        *self.0 == *other
+fn is_valid_identifier(s: &str) -> bool {
+    s.len() <= 100 && s.bytes().all(|b| (0x20..=0x7E).contains(&b))
+}
+
+impl AsRef<str> for Identifier {
+    fn as_ref(&self) -> &str {
+        self.0.as_ref()
     }
 }
 
-impl FromStr for Identifier {
-    type Err = ErrorKind;
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        Identifier::new(s)
+impl std::ops::Deref for Identifier {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0.as_ref()
     }
 }
 
-fn is_valid_identifier(s: &Arc<str>) -> bool {
-    s.len() <= 100 && s.bytes().all(|b| (0x20..=0x7E).contains(&b))
+// so that assert_eq! macros work
+impl<'a> PartialEq<&'a str> for Identifier {
+    fn eq(&self, other: &&'a str) -> bool {
+        self.0.as_ref() == *other
+    }
+}
+
+impl<'a> PartialEq<Identifier> for &'a str {
+    fn eq(&self, other: &Identifier) -> bool {
+        other == self
+    }
+}
+
+impl std::fmt::Display for Identifier {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        std::fmt::Display::fmt(&self.0, f)
+    }
+}
+
+impl std::borrow::Borrow<str> for Identifier {
+    fn borrow(&self) -> &str {
+        self.0.as_ref()
+    }
 }
 
 impl Serialize for Identifier {
@@ -79,7 +109,7 @@ impl<'de> Deserialize<'de> for Identifier {
         D: Deserializer<'de>,
     {
         let string = String::deserialize(deserializer)?;
-        Identifier::new(string).map_err(de::Error::custom)
+        Identifier::new(string.as_str()).map_err(de::Error::custom)
     }
 }