Report an error instead of panicking on invalid map types

cargo-typify/src/lib.rs

@@ -7,8 +7,8 @@
 use std::path::PathBuf;
 
 use clap::{ArgGroup, Args};
-use color_eyre::eyre::{Context, Result};
-use typify::{CrateVers, TypeSpace, TypeSpaceSettings, UnknownPolicy};
+use color_eyre::eyre::{eyre, Context, Result};
+use typify::{CrateVers, MapType, TypeSpace, TypeSpaceSettings, UnknownPolicy};
 
 /// A CLI for the `typify` crate that converts JSON Schema files to Rust code.
 #[derive(Args)]
@@ -165,7 +165,11 @@ pub fn convert(args: &CliArgs) -> Result<String> {
     }
 
     if let Some(map_type) = &args.map_type {
-        settings.with_map_type(map_type.as_str());
+        let map_type = map_type
+            .parse::<MapType>()
+            .map_err(|msg| eyre!(msg))
+            .wrap_err("Invalid map type")?;
+        settings.with_map_type(map_type);
     }
 
     if let Some(unknown_crates) = &args.unknown_crates {

cargo-typify/tests/integration.rs

@@ -192,3 +192,24 @@ fn test_btree_map() {
 
     assert_contents("tests/outputs/custom_btree_map.rs", &actual);
 }
+
+#[test]
+fn test_invalid_map_type() {
+    let input = concat!(env!("CARGO_MANIFEST_DIR"), "/../example.json");
+
+    let output = assert_cmd::cargo::cargo_bin_cmd!()
+        .args([
+            "typify",
+            input,
+            "--map-type",
+            "not a valid!!type",
+            "--output",
+            "-",
+        ])
+        .output()
+        .unwrap();
+
+    assert!(!output.status.success());
+    let stderr = String::from_utf8(output.stderr).unwrap();
+    assert!(stderr.contains("invalid map type"), "stderr: {stderr}");
+}

typify-impl/src/lib.rs

@@ -241,12 +241,28 @@ pub struct MapType(pub syn::Type);
 
 impl MapType {
     /// Create a new MapType from a [`str`].
+    ///
+    /// # Panics
+    ///
+    /// Panics if `s` cannot be parsed as a Rust type. Prefer
+    /// [`str::parse`] (via the [`FromStr`](std::str::FromStr)
+    /// implementation) to handle invalid input without panicking.
     pub fn new(s: &str) -> Self {
         let map_type = syn::parse_str::<syn::Type>(s).expect("valid ident");
         Self(map_type)
     }
 }
 
+impl std::str::FromStr for MapType {
+    type Err = String;
+
+    fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
+        let map_type = syn::parse_str::<syn::Type>(s)
+            .map_err(|err| format!("invalid map type {s:?}: {err}"))?;
+        Ok(Self(map_type))
+    }
+}
+
 impl Default for MapType {
     fn default() -> Self {
         Self::new("::std::collections::HashMap")
@@ -270,18 +286,28 @@ impl<'de> serde::Deserialize<'de> for MapType {
     where
         D: serde::Deserializer<'de>,
     {
-        let s = <&str>::deserialize(deserializer)?;
-        Ok(Self::new(s))
+        let s = String::deserialize(deserializer)?;
+        s.parse().map_err(serde::de::Error::custom)
     }
 }
 
 impl From<String> for MapType {
+    /// # Panics
+    ///
+    /// Panics if `s` cannot be parsed as a Rust type. Prefer
+    /// [`str::parse`] (via the [`FromStr`](std::str::FromStr)
+    /// implementation) to handle invalid input without panicking.
     fn from(s: String) -> Self {
         Self::new(&s)
     }
 }
 
 impl From<&str> for MapType {
+    /// # Panics
+    ///
+    /// Panics if `s` cannot be parsed as a Rust type. Prefer
+    /// [`str::parse`] (via the [`FromStr`](std::str::FromStr)
+    /// implementation) to handle invalid input without panicking.
     fn from(s: &str) -> Self {
         Self::new(s)
     }
@@ -1224,9 +1250,34 @@ mod tests {
         output::OutputSpace,
         test_util::validate_output,
         type_entry::{TypeEntryEnum, VariantDetails},
-        Name, TypeEntryDetails, TypeSpace, TypeSpaceSettings,
+        MapType, Name, TypeEntryDetails, TypeSpace, TypeSpaceSettings,
     };
 
+    #[test]
+    fn test_map_type_from_str() {
+        let map_type = "::std::collections::BTreeMap".parse::<MapType>().unwrap();
+        assert_eq!(map_type.to_string(), ":: std :: collections :: BTreeMap");
+
+        "not a valid!!type".parse::<MapType>().unwrap_err();
+        "".parse::<MapType>().unwrap_err();
+    }
+
+    #[test]
+    fn test_map_type_deserialize() {
+        let map_type: MapType =
+            serde_json::from_value(json!("::std::collections::BTreeMap")).unwrap();
+        assert_eq!(map_type.to_string(), ":: std :: collections :: BTreeMap");
+
+        // Strings with escape sequences require owned deserialization; make
+        // sure that works.
+        let map_type: MapType =
+            serde_json::from_str("\"::std::collections::\\u0042TreeMap\"").unwrap();
+        assert_eq!(map_type.to_string(), ":: std :: collections :: BTreeMap");
+
+        // ... and invalid types must produce an error rather than a panic.
+        serde_json::from_value::<MapType>(json!("not a valid!!type")).unwrap_err();
+    }
+
     #[allow(dead_code)]
     #[derive(Serialize, JsonSchema)]
     struct Blah {

typify/src/lib.rs

@@ -151,6 +151,7 @@
 pub use typify_impl::accept_as_ident;
 pub use typify_impl::CrateVers;
 pub use typify_impl::Error;
+pub use typify_impl::MapType;
 pub use typify_impl::Type;
 pub use typify_impl::TypeDetails;
 pub use typify_impl::TypeEnum;