refactor(dag): ExportFormat implements FromStr instead of shadowing it

crates/aingle_cortex/src/rest/dag.rs

@@ -387,12 +387,10 @@ pub async fn get_dag_export(
 ) -> Result<axum::response::Response> {
     use axum::response::IntoResponse;
 
-    let format = aingle_graph::dag::ExportFormat::from_str(&query.format).ok_or_else(|| {
-        Error::InvalidInput(format!(
-            "Unknown format '{}'. Use: dot, mermaid, json",
-            query.format
-        ))
-    })?;
+    let format: aingle_graph::dag::ExportFormat = query
+        .format
+        .parse()
+        .map_err(|e: aingle_graph::dag::UnknownExportFormat| Error::InvalidInput(e.to_string()))?;
 
     let graph = state.graph.read().await;
     let dag_graph = graph

crates/aingle_graph/src/dag/export.rs

@@ -40,24 +40,41 @@ pub enum ExportFormat {
     Json,
 }
 
-impl ExportFormat {
-    /// Parse from string (case-insensitive).
-    ///
-    /// Shadows `std::str::FromStr::from_str`, which clippy rightly flags: a
-    /// caller writing `ExportFormat::from_str` cannot tell which one they get,
-    /// and this one returns `Option` where the trait returns `Result`.
-    ///
-    /// The fix is to implement `FromStr` properly with an error type and drop
-    /// this inherent method. It is cheap — there is exactly one caller outside
-    /// this module's own tests — but it changes a public signature, so it is a
-    /// deliberate change with its own review and not a line in a lint sweep.
-    #[allow(clippy::should_implement_trait)]
-    pub fn from_str(s: &str) -> Option<Self> {
+/// The string offered to [`ExportFormat`]'s `FromStr` named no known format.
+///
+/// Carries the offending input and knows the accepted spellings, so a caller
+/// does not have to keep its own copy of the list — which is how a REST handler
+/// ends up advertising formats the parser no longer accepts.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct UnknownExportFormat(pub String);
+
+impl std::fmt::Display for UnknownExportFormat {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "unknown export format '{}'. Use: dot (or graphviz), mermaid (or md), json",
+            self.0
+        )
+    }
+}
+
+impl std::error::Error for UnknownExportFormat {}
+
+/// Parse from string (case-insensitive).
+///
+/// This was an inherent `from_str` returning `Option`, which shadowed
+/// `std::str::FromStr::from_str` and returned a different shape from it: a
+/// caller reading `ExportFormat::from_str(..)` could not tell which one they
+/// were getting, and got no reason for the failure either way.
+impl std::str::FromStr for ExportFormat {
+    type Err = UnknownExportFormat;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s.to_lowercase().as_str() {
-            "dot" | "graphviz" => Some(Self::Dot),
-            "mermaid" | "md" => Some(Self::Mermaid),
-            "json" => Some(Self::Json),
-            _ => None,
+            "dot" | "graphviz" => Ok(Self::Dot),
+            "mermaid" | "md" => Ok(Self::Mermaid),
+            "json" => Ok(Self::Json),
+            _ => Err(UnknownExportFormat(s.to_string())),
         }
     }
 }
@@ -319,15 +336,28 @@ mod tests {
 
     #[test]
     fn test_export_format_parsing() {
-        assert_eq!(ExportFormat::from_str("dot"), Some(ExportFormat::Dot));
-        assert_eq!(ExportFormat::from_str("DOT"), Some(ExportFormat::Dot));
-        assert_eq!(ExportFormat::from_str("graphviz"), Some(ExportFormat::Dot));
+        assert_eq!("dot".parse(), Ok(ExportFormat::Dot));
+        assert_eq!("DOT".parse(), Ok(ExportFormat::Dot));
+        assert_eq!("graphviz".parse(), Ok(ExportFormat::Dot));
+        assert_eq!("mermaid".parse(), Ok(ExportFormat::Mermaid));
+        assert_eq!("md".parse(), Ok(ExportFormat::Mermaid));
+        assert_eq!("json".parse(), Ok(ExportFormat::Json));
         assert_eq!(
-            ExportFormat::from_str("mermaid"),
-            Some(ExportFormat::Mermaid)
+            "xml".parse::<ExportFormat>(),
+            Err(UnknownExportFormat("xml".to_string()))
         );
-        assert_eq!(ExportFormat::from_str("json"), Some(ExportFormat::Json));
-        assert_eq!(ExportFormat::from_str("xml"), None);
+    }
+
+    #[test]
+    fn an_unknown_format_says_what_it_would_have_accepted() {
+        // The list used to live in the REST handler, where it could drift away
+        // from the parser without anything noticing.
+        let e = "xml".parse::<ExportFormat>().unwrap_err();
+        let msg = e.to_string();
+        assert!(msg.contains("xml"), "{msg}");
+        for accepted in ["dot", "graphviz", "mermaid", "md", "json"] {
+            assert!(msg.contains(accepted), "{accepted} missing from: {msg}");
+        }
     }
 
     #[test]

crates/aingle_graph/src/dag/mod.rs

@@ -33,7 +33,7 @@ pub use action::{
 pub use backend::SledDagBackend;
 pub use backend::{DagBackend, MemoryDagBackend};
 pub use canonical::{CanonicalAction, CANONICAL_SPEC, HASH_ALG, SIGNATURE_ALG, SIGNED_MESSAGE};
-pub use export::{DagGraph, ExportFormat};
+pub use export::{DagGraph, ExportFormat, UnknownExportFormat};
 pub use pruning::{PruneResult, RetentionPolicy};
 #[cfg(feature = "dag-sign")]
 pub use signing::{DagSigningKey, DagVerifyingKey, VerifyResult};