Remove panicable unwrap calls

hedgetechllc · Nov 4, 2024 · ff68b7a · ff68b7a
1 parent cac39fa
commit ff68b7a
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 78 deletions.
diff --git a/musicxml/src/elements/lyric.rs b/musicxml/src/elements/lyric.rs
@@ -87,21 +87,23 @@ impl ContentDeserializer for TextLyric {
         "syllabic" => {
           if text_lyric.syllabic.is_none() {
             text_lyric.syllabic = Some(Syllabic::deserialize(el)?);
-          } else if !text_lyric.additional.is_empty() {
-            text_lyric.additional.last_mut().unwrap().syllabic = Some(Syllabic::deserialize(el)?);
+          } else if let Some(additional) = text_lyric.additional.last_mut() {
+            additional.syllabic = Some(Syllabic::deserialize(el)?);
           }
         }
         "text" => {
           if text_lyric.text.content.is_empty() {
             text_lyric.text = Text::deserialize(el)?;
-          } else if text_lyric.additional.is_empty() {
-            text_lyric.additional.push(AdditionalTextLyric {
-              elision: None,
-              syllabic: None,
-              text: Text::deserialize(el)?,
-            });
-          } else if text_lyric.additional.last().unwrap().text.content.is_empty() {
-            text_lyric.additional.last_mut().unwrap().text = Text::deserialize(el)?;
+          } else if let Some(additional) = text_lyric.additional.last_mut() {
+            if additional.text.content.is_empty() {
+              additional.text = Text::deserialize(el)?;
+            } else {
+              text_lyric.additional.push(AdditionalTextLyric {
+                elision: None,
+                syllabic: None,
+                text: Text::deserialize(el)?,
+              });
+            }
           } else {
             text_lyric.additional.push(AdditionalTextLyric {
               elision: None,

diff --git a/musicxml/src/elements/note.rs b/musicxml/src/elements/note.rs
@@ -224,7 +224,7 @@ pub struct GraceInfo {
 impl ContentDeserializer for GraceInfo {
   fn deserialize(elements: &[XmlElement]) -> Result<Self, String> {
     Ok(GraceInfo {
-      grace: Grace::deserialize(elements.first().unwrap())?,
+      grace: Grace::deserialize(elements.first().ok_or("Missing <grace> sub-element")?)?,
       info: if elements.iter().any(|el| el.name == "cue") {
         GraceType::Cue(GraceCueInfo::deserialize(elements)?)
       } else {
@@ -435,9 +435,9 @@ pub struct NoteContents {
 impl ContentDeserializer for NoteContents {
   fn deserialize(elements: &[XmlElement]) -> Result<Self, String> {
     let mut note_contents = NoteContents {
-      info: if elements.first().unwrap().name == "grace" {
+      info: if elements.first().ok_or("Missing <note> sub-element")?.name == "grace" {
         NoteType::Grace(GraceInfo::deserialize(elements)?)
-      } else if elements.first().unwrap().name == "cue" {
+      } else if elements.first().ok_or("Missing <note> sub-element")?.name == "cue" {
         NoteType::Cue(CueInfo::deserialize(elements)?)
       } else {
         NoteType::Normal(NormalInfo::deserialize(elements)?)

diff --git a/musicxml/src/elements/percussion.rs b/musicxml/src/elements/percussion.rs
@@ -102,7 +102,7 @@ pub struct Percussion {
 
 impl ElementDeserializer for Percussion {
   fn deserialize(element: &XmlElement) -> Result<Self, String> {
-    let el = element.elements.first().unwrap();
+    let el = element.elements.first().ok_or("No sub-element found in <percussion>")?;
     Ok(Percussion {
       attributes: PercussionAttributes::deserialize(&element.attributes)?,
       content: match el.name.as_str() {
@@ -126,59 +126,59 @@ impl ElementDeserializer for Percussion {
 impl ElementSerializer for Percussion {
   fn serialize(element: &Self) -> XmlElement {
     let name;
-    let mut xml_element = XmlElement {
+    let mut xml_element = match &element.content {
+      PercussionContents::Glass(content) => {
+        name = String::from("glass");
+        Glass::serialize(content)
+      }
+      PercussionContents::Metal(content) => {
+        name = String::from("metal");
+        Metal::serialize(content)
+      }
+      PercussionContents::Wood(content) => {
+        name = String::from("wood");
+        Wood::serialize(content)
+      }
+      PercussionContents::Pitched(content) => {
+        name = String::from("pitched");
+        Pitched::serialize(content)
+      }
+      PercussionContents::Membrane(content) => {
+        name = String::from("membrane");
+        Membrane::serialize(content)
+      }
+      PercussionContents::Effect(content) => {
+        name = String::from("effect");
+        Effect::serialize(content)
+      }
+      PercussionContents::Timpani(content) => {
+        name = String::from("timpani");
+        Timpani::serialize(content)
+      }
+      PercussionContents::Beater(content) => {
+        name = String::from("beater");
+        Beater::serialize(content)
+      }
+      PercussionContents::Stick(content) => {
+        name = String::from("stick");
+        Stick::serialize(content)
+      }
+      PercussionContents::StickLocation(content) => {
+        name = String::from("stick-location");
+        StickLocation::serialize(content)
+      }
+      PercussionContents::OtherPercussion(content) => {
+        name = String::from("other-percussion");
+        OtherPercussion::serialize(content)
+      }
+    };
+    xml_element.name = name;
+    XmlElement {
       name: String::new(),
       attributes: PercussionAttributes::serialize(&element.attributes),
-      elements: vec![match &element.content {
-        PercussionContents::Glass(content) => {
-          name = String::from("glass");
-          Glass::serialize(content)
-        }
-        PercussionContents::Metal(content) => {
-          name = String::from("metal");
-          Metal::serialize(content)
-        }
-        PercussionContents::Wood(content) => {
-          name = String::from("wood");
-          Wood::serialize(content)
-        }
-        PercussionContents::Pitched(content) => {
-          name = String::from("pitched");
-          Pitched::serialize(content)
-        }
-        PercussionContents::Membrane(content) => {
-          name = String::from("membrane");
-          Membrane::serialize(content)
-        }
-        PercussionContents::Effect(content) => {
-          name = String::from("effect");
-          Effect::serialize(content)
-        }
-        PercussionContents::Timpani(content) => {
-          name = String::from("timpani");
-          Timpani::serialize(content)
-        }
-        PercussionContents::Beater(content) => {
-          name = String::from("beater");
-          Beater::serialize(content)
-        }
-        PercussionContents::Stick(content) => {
-          name = String::from("stick");
-          Stick::serialize(content)
-        }
-        PercussionContents::StickLocation(content) => {
-          name = String::from("stick-location");
-          StickLocation::serialize(content)
-        }
-        PercussionContents::OtherPercussion(content) => {
-          name = String::from("other-percussion");
-          OtherPercussion::serialize(content)
-        }
-      }],
+      elements: vec![xml_element],
       text: String::new(),
-    };
-    xml_element.elements.last_mut().unwrap().name = name;
-    xml_element
+    }
   }
 }
 

diff --git a/musicxml/src/elements/time.rs b/musicxml/src/elements/time.rs
@@ -97,7 +97,7 @@ impl ContentDeserializer for TimeContents {
         "beats" => time_beats = Some(Beats::deserialize(element)?),
         "beat-type" => {
           beats.push(TimeBeatContents {
-            beats: time_beats.unwrap(),
+            beats: time_beats.ok_or("Missing required <beats> element in <time>")?,
             beat_type: BeatType::deserialize(element)?,
           });
           time_beats = None;

diff --git a/musicxml/src/parser/xml_parser.rs b/musicxml/src/parser/xml_parser.rs
@@ -123,18 +123,19 @@ pub fn parse_from_string(str: &str) -> Result<XmlElement, String> {
           None => return Err(String::from("Root tag cannot be self-closing")),
         },
         TagType::Closing(tag) => {
-          let mut element = open_tags.pop().unwrap();
-          element.text.truncate(element.text.trim().len());
-          if tag.name != element.name {
-            return Err(format!(
-              "Mismatched closing tag...expected '{}' but found '{}'",
-              element.name, tag.name
-            ));
-          }
-          if let Some(last_open_tag) = open_tags.last_mut() {
-            last_open_tag.elements.push(element);
-          } else {
-            return Ok(element);
+          if let Some(mut element) = open_tags.pop() {
+            element.text.truncate(element.text.trim().len());
+            if tag.name != element.name {
+              return Err(format!(
+                "Mismatched closing tag...expected '{}' but found '{}'",
+                element.name, tag.name
+              ));
+            }
+            if let Some(last_open_tag) = open_tags.last_mut() {
+              last_open_tag.elements.push(element);
+            } else {
+              return Ok(element);
+            }
           }
         }
         TagType::Done => break,

diff --git a/musicxml_macros/CHANGELOG.md b/musicxml_macros/CHANGELOG.md
@@ -1 +1,19 @@
-# Changelog
+# Changelog
+
+## v1.1.0
+
+### Added
+
+* Library has been made `no_std` compatible
+* `std` feature (enabled by default) allows direct file reading/writing
+* Public functionality to allow parsing to/from data buffer in addition to files
+* Internal zip file reader/writer with no external dependencies
+* Error documentation
+
+### Fixed
+
+* Removed dependency on `zip` crate
+* Switched from `libflate` to `miniz_oxide` for DEFLATE implementation
+* Fixed incorrect `32nd` duration parsing string
+* Removed all naked `unwrap()` calls
+* Lots of clippy cleanups