From 610adc8b72c788a773475ca5418fc2e647c767fe Mon Sep 17 00:00:00 2001
From: Tom Alexander <tom@fizz.buzz>
Date: Sun, 12 Apr 2020 21:03:55 -0400
Subject: [PATCH] Switch the Renderable trait over to the returning a
 RenderError

---
 src/bin.rs                 | 15 +++++++--------
 src/renderer/errors.rs     |  6 ++++++
 src/renderer/renderable.rs |  4 +++-
 src/renderer/renderer.rs   | 33 ++++++++++++++++++++-------------
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/bin.rs b/src/bin.rs
index 7bad3b5..9ec9dad 100644
--- a/src/bin.rs
+++ b/src/bin.rs
@@ -73,14 +73,14 @@ fn read_context_from_stdin() -> serde_json::Value {
 impl ContextElement for serde_json::Value {}
 
 impl Renderable for serde_json::Value {
-    fn render(&self) -> String {
+    fn render(&self) -> Result<String, RenderError> {
         match self {
-            serde_json::Value::Null => "".to_owned(),
-            serde_json::Value::Bool(boolean) => boolean.to_string(),
-            serde_json::Value::Number(num) => num.to_string(),
-            serde_json::Value::String(string) => string.to_string(),
-            serde_json::Value::Array(_arr) => panic!("Attempted to render an array"),
-            serde_json::Value::Object(_obj) => panic!("Attempted to render an object"),
+            serde_json::Value::Null => Ok("".to_owned()),
+            serde_json::Value::Bool(boolean) => Ok(boolean.to_string()),
+            serde_json::Value::Number(num) => Ok(num.to_string()),
+            serde_json::Value::String(string) => Ok(string.to_string()),
+            serde_json::Value::Array(_arr) => Err(RenderError::CantRender { elem: self }),
+            serde_json::Value::Object(_obj) => Err(RenderError::CantRender { elem: self }),
         }
     }
 }
@@ -105,7 +105,6 @@ impl Walkable for serde_json::Value {
                 elem: self,
             }),
             serde_json::Value::Array(_arr) => todo!("Arrays not supported yet"),
-            // TODO: Handle this error better
             serde_json::Value::Object(obj) => {
                 obj.get(segment)
                     .map(|val| val as _)
diff --git a/src/renderer/errors.rs b/src/renderer/errors.rs
index 57804e9..ba6b031 100644
--- a/src/renderer/errors.rs
+++ b/src/renderer/errors.rs
@@ -14,6 +14,10 @@ pub enum RenderError<'a> {
         segment: String,
         elem: &'a dyn ContextElement,
     },
+    /// Attempting to render and unrenderable type (for example, an object without any filters)
+    CantRender {
+        elem: &'a dyn ContextElement,
+    },
 }
 
 #[derive(Clone)]
@@ -31,6 +35,7 @@ impl fmt::Display for RenderError<'_> {
             RenderError::WontWalk { segment, elem } => {
                 write!(f, "Failed to walk to {} from {:?}", segment, elem)
             }
+            RenderError::CantRender { elem } => write!(f, "Cant render {:?}", elem),
         }
     }
 }
@@ -45,6 +50,7 @@ impl fmt::Debug for RenderError<'_> {
             RenderError::WontWalk { segment, elem } => {
                 write!(f, "Failed to walk to {} from {:?}", segment, elem)
             }
+            RenderError::CantRender { elem } => write!(f, "Cant render {:?}", elem),
         }
     }
 }
diff --git a/src/renderer/renderable.rs b/src/renderer/renderable.rs
index 8451c14..e9a2c87 100644
--- a/src/renderer/renderable.rs
+++ b/src/renderer/renderable.rs
@@ -1,3 +1,5 @@
+use crate::renderer::errors::RenderError;
+
 pub trait Renderable {
-    fn render(&self) -> String;
+    fn render(&self) -> Result<String, RenderError>;
 }
diff --git a/src/renderer/renderer.rs b/src/renderer/renderer.rs
index b53b56c..dcdbc8a 100644
--- a/src/renderer/renderer.rs
+++ b/src/renderer/renderer.rs
@@ -85,10 +85,10 @@ impl<'a> DustRenderer<'a> {
         C: ContextElement,
     {
         match tag {
-            DustTag::DTComment(comment) => (),
+            DustTag::DTComment(_comment) => (),
             DustTag::DTReference(reference) => {
                 let val = walk_path(context, &reference.path.keys)?;
-                return Ok(val.render());
+                return val.render();
             }
             _ => (), // TODO: Implement the rest
         }
@@ -120,20 +120,20 @@ mod tests {
         impl<I: ContextElement> ContextElement for HashMap<&str, I> {}
 
         impl Renderable for u32 {
-            fn render(&self) -> std::string::String {
-                self.to_string()
+            fn render(&self) -> Result<String, RenderError> {
+                Ok(self.to_string())
             }
         }
 
         impl Renderable for &str {
-            fn render(&self) -> std::string::String {
-                self.to_string()
+            fn render(&self) -> Result<String, RenderError> {
+                Ok(self.to_string())
             }
         }
 
         impl<I: ContextElement> Renderable for HashMap<&str, I> {
-            fn render(&self) -> std::string::String {
-                panic!("Attempted to render a hashmap");
+            fn render(&self) -> Result<String, RenderError> {
+                Err(RenderError::CantRender { elem: self })
             }
         }
 
@@ -182,16 +182,23 @@ mod tests {
         .iter()
         .cloned()
         .collect();
-        assert_eq!(walk_path(&context, &vec!["cat"]).unwrap().render(), "kitty");
         assert_eq!(
-            walk_path(&number_context, &vec!["tiger"]).unwrap().render(),
-            "3"
+            walk_path(&context, &vec!["cat"]).unwrap().render().unwrap(),
+            "kitty".to_owned()
+        );
+        assert_eq!(
+            walk_path(&number_context, &vec!["tiger"])
+                .unwrap()
+                .render()
+                .unwrap(),
+            "3".to_owned()
         );
         assert_eq!(
             walk_path(&deep_context, &vec!["tiger", "food"])
                 .unwrap()
-                .render(),
-            "people"
+                .render()
+                .unwrap(),
+            "people".to_owned()
         );
     }
 }