From d51392fe8a8f5e471677e385b25811876b56e1fb Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Sun, 12 Apr 2020 18:29:40 -0400 Subject: [PATCH 1/4] Initial move to returning results from render calls. --- src/bin.rs | 6 ++++-- src/renderer/errors.rs | 42 ++++++++++++++++++++++++++++++++-------- src/renderer/mod.rs | 2 ++ src/renderer/renderer.rs | 38 ++++++++++++++++++++++-------------- src/renderer/walkable.rs | 3 ++- 5 files changed, 66 insertions(+), 25 deletions(-) diff --git a/src/bin.rs b/src/bin.rs index 0e9bd5f..d334aef 100644 --- a/src/bin.rs +++ b/src/bin.rs @@ -4,6 +4,7 @@ use renderer::compile_template; use renderer::CompiledTemplate; use renderer::ContextElement; use renderer::DustRenderer; +use renderer::RenderError; use renderer::Renderable; use renderer::Walkable; use std::env; @@ -85,7 +86,7 @@ impl Renderable for serde_json::Value { } impl Walkable for serde_json::Value { - fn walk(&self, segment: &str) -> &dyn ContextElement { + fn walk(&self, segment: &str) -> Result<&dyn ContextElement, RenderError> { match self { serde_json::Value::Null => panic!("Attempted to walk to {} on a null value", segment), serde_json::Value::Bool(_boolean) => { @@ -98,7 +99,8 @@ impl Walkable for serde_json::Value { panic!("Attempted to walk to {} on a string value", segment) } serde_json::Value::Array(_arr) => todo!("Arrays not supported yet"), - serde_json::Value::Object(obj) => obj.get(segment).unwrap(), + // TODO: Handle this error better + serde_json::Value::Object(obj) => Ok(obj.get(segment).unwrap()), } } } diff --git a/src/renderer/errors.rs b/src/renderer/errors.rs index b324484..57804e9 100644 --- a/src/renderer/errors.rs +++ b/src/renderer/errors.rs @@ -1,9 +1,19 @@ +use crate::renderer::walkable::ContextElement; use std::error; use std::fmt; -#[derive(Clone)] -pub struct RenderError { - pub message: String, +pub enum RenderError<'a> { + Generic(String), + /// For when walking is absolutely impossible + CantWalk { + segment: String, + elem: &'a dyn ContextElement, + }, + /// For when walking fails (example, a missing key on a map) + WontWalk { + segment: String, + elem: &'a dyn ContextElement, + }, } #[derive(Clone)] @@ -11,19 +21,35 @@ pub struct CompileError { pub message: String, } -impl fmt::Display for RenderError { +impl fmt::Display for RenderError<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Error rendering: {}", self.message) + match self { + RenderError::Generic(msg) => write!(f, "{}", msg), + RenderError::CantWalk { segment, elem } => { + write!(f, "Tried to walk to {} from {:?}", segment, elem) + } + RenderError::WontWalk { segment, elem } => { + write!(f, "Failed to walk to {} from {:?}", segment, elem) + } + } } } -impl fmt::Debug for RenderError { +impl fmt::Debug for RenderError<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Error rendering: {}", self.message) + match self { + RenderError::Generic(msg) => write!(f, "{}", msg), + RenderError::CantWalk { segment, elem } => { + write!(f, "Tried to walk to {} from {:?}", segment, elem) + } + RenderError::WontWalk { segment, elem } => { + write!(f, "Failed to walk to {} from {:?}", segment, elem) + } + } } } -impl error::Error for RenderError { +impl error::Error for RenderError<'_> { fn source(&self) -> Option<&(dyn error::Error + 'static)> { None } diff --git a/src/renderer/mod.rs b/src/renderer/mod.rs index c80f1a5..8b536f9 100644 --- a/src/renderer/mod.rs +++ b/src/renderer/mod.rs @@ -5,6 +5,8 @@ mod renderable; mod renderer; mod walkable; +pub use errors::CompileError; +pub use errors::RenderError; pub use renderable::Renderable; pub use renderer::compile_template; pub use renderer::CompiledTemplate; diff --git a/src/renderer/renderer.rs b/src/renderer/renderer.rs index ac664a8..b6e4b6e 100644 --- a/src/renderer/renderer.rs +++ b/src/renderer/renderer.rs @@ -9,6 +9,7 @@ use crate::renderer::renderable::Renderable; use crate::renderer::walkable::ContextElement; use crate::renderer::walkable::Walkable; use std::collections::HashMap; +use std::error::Error; use std::ops::Index; #[derive(Clone, Debug)] @@ -47,22 +48,23 @@ impl<'a> DustRenderer<'a> { .insert(template.name.clone(), &template.template); } - pub fn render(&self, name: &str, context: &C) -> Result + pub fn render(&self, name: &str, context: &'a C) -> Result> where C: ContextElement, { let main_template = match self.templates.get(name) { Some(tmpl) => tmpl, None => { - return Err(RenderError { - message: format!("No template named {} in context", name), - }); + return Err(RenderError::Generic(format!( + "No template named {} in context", + name + ))); } }; self.render_body(&main_template.contents, context) } - fn render_body(&self, body: &Body, context: &C) -> Result + fn render_body(&self, body: &Body, context: &'a C) -> Result> where C: ContextElement, { @@ -78,14 +80,14 @@ impl<'a> DustRenderer<'a> { Ok(output) } - fn render_tag(&self, tag: &DustTag, context: &C) -> Result + fn render_tag(&self, tag: &DustTag, context: &'a C) -> Result> where C: ContextElement, { match tag { DustTag::DTComment(comment) => (), DustTag::DTReference(reference) => { - let val = walk_path(context, &reference.path.keys); + let val = walk_path(context, &reference.path.keys)?; return Ok(val.render()); } _ => (), // TODO: Implement the rest @@ -94,14 +96,17 @@ impl<'a> DustRenderer<'a> { } } -fn walk_path<'a>(context: &'a dyn ContextElement, path: &Vec<&str>) -> &'a dyn ContextElement { +fn walk_path<'a>( + context: &'a dyn ContextElement, + path: &Vec<&str>, +) -> Result<&'a dyn ContextElement, RenderError<'a>> { let mut output = context; for elem in path.iter() { - output = output.walk(elem); + output = output.walk(elem)?; } - output + Ok(output) } #[cfg(test)] @@ -133,19 +138,24 @@ mod tests { } impl Walkable for HashMap<&str, I> { - fn walk(&self, segment: &str) -> &dyn ContextElement { - self.get(segment).unwrap() + fn walk(&self, segment: &str) -> Result<&dyn ContextElement, RenderError> { + Ok(self.get(segment).unwrap()) + // TODO: Handle error here better + // self.get(segment).ok_or(RenderError::WontWalk { + // segment: segment.to_string(), + // elem: self, + // }) } } impl Walkable for &str { - fn walk(&self, _segment: &str) -> &dyn ContextElement { + fn walk(&self, _segment: &str) -> Result<&dyn ContextElement, RenderError> { panic!("Tried to walk down a str"); } } impl Walkable for u32 { - fn walk(&self, _segment: &str) -> &dyn ContextElement { + fn walk(&self, _segment: &str) -> Result<&dyn ContextElement, RenderError> { panic!("Tried to walk down a str"); } } diff --git a/src/renderer/walkable.rs b/src/renderer/walkable.rs index b861deb..d2ae81f 100644 --- a/src/renderer/walkable.rs +++ b/src/renderer/walkable.rs @@ -1,8 +1,9 @@ use super::renderable::Renderable; +use crate::renderer::errors::RenderError; use std::fmt::Debug; pub trait ContextElement: Walkable + Renderable + Debug {} pub trait Walkable { - fn walk(&self, segment: &str) -> &dyn ContextElement; + fn walk(&self, segment: &str) -> Result<&dyn ContextElement, RenderError>; } From a647ed6d437248c6e910afc8e63bb5c6e4425e45 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Sun, 12 Apr 2020 18:31:27 -0400 Subject: [PATCH 2/4] fix tests --- src/renderer/renderer.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/renderer/renderer.rs b/src/renderer/renderer.rs index b6e4b6e..80cd62e 100644 --- a/src/renderer/renderer.rs +++ b/src/renderer/renderer.rs @@ -177,10 +177,15 @@ mod tests { .iter() .cloned() .collect(); - assert_eq!(walk_path(&context, &vec!["cat"]).render(), "kitty"); - assert_eq!(walk_path(&number_context, &vec!["tiger"]).render(), "3"); + assert_eq!(walk_path(&context, &vec!["cat"]).unwrap().render(), "kitty"); assert_eq!( - walk_path(&deep_context, &vec!["tiger", "food"]).render(), + walk_path(&number_context, &vec!["tiger"]).unwrap().render(), + "3" + ); + assert_eq!( + walk_path(&deep_context, &vec!["tiger", "food"]) + .unwrap() + .render(), "people" ); } From 9c7883358a0d31d91e68f9a24bd7470e087096a9 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Sun, 12 Apr 2020 20:52:30 -0400 Subject: [PATCH 3/4] Better error handling added to walkable --- src/bin.rs | 35 ++++++++++++++++++++++++----------- src/renderer/renderer.rs | 25 +++++++++++++++---------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/bin.rs b/src/bin.rs index d334aef..7bad3b5 100644 --- a/src/bin.rs +++ b/src/bin.rs @@ -88,19 +88,32 @@ impl Renderable for serde_json::Value { impl Walkable for serde_json::Value { fn walk(&self, segment: &str) -> Result<&dyn ContextElement, RenderError> { match self { - serde_json::Value::Null => panic!("Attempted to walk to {} on a null value", segment), - serde_json::Value::Bool(_boolean) => { - panic!("Attempted to walk to {} on a boolean value", segment) - } - serde_json::Value::Number(_num) => { - panic!("Attempted to walk to {} on a number value", segment) - } - serde_json::Value::String(_string) => { - panic!("Attempted to walk to {} on a string value", segment) - } + serde_json::Value::Null => Err(RenderError::CantWalk { + segment: segment.to_string(), + elem: self, + }), + serde_json::Value::Bool(_boolean) => Err(RenderError::CantWalk { + segment: segment.to_string(), + elem: self, + }), + serde_json::Value::Number(_num) => Err(RenderError::CantWalk { + segment: segment.to_string(), + elem: self, + }), + serde_json::Value::String(_string) => Err(RenderError::CantWalk { + segment: segment.to_string(), + elem: self, + }), serde_json::Value::Array(_arr) => todo!("Arrays not supported yet"), // TODO: Handle this error better - serde_json::Value::Object(obj) => Ok(obj.get(segment).unwrap()), + serde_json::Value::Object(obj) => { + obj.get(segment) + .map(|val| val as _) + .ok_or(RenderError::WontWalk { + segment: segment.to_string(), + elem: self, + }) + } } } } diff --git a/src/renderer/renderer.rs b/src/renderer/renderer.rs index 80cd62e..b53b56c 100644 --- a/src/renderer/renderer.rs +++ b/src/renderer/renderer.rs @@ -139,24 +139,29 @@ mod tests { impl Walkable for HashMap<&str, I> { fn walk(&self, segment: &str) -> Result<&dyn ContextElement, RenderError> { - Ok(self.get(segment).unwrap()) - // TODO: Handle error here better - // self.get(segment).ok_or(RenderError::WontWalk { - // segment: segment.to_string(), - // elem: self, - // }) + let child = self.get(segment).ok_or(RenderError::WontWalk { + segment: segment.to_string(), + elem: self, + })?; + Ok(child) } } impl Walkable for &str { - fn walk(&self, _segment: &str) -> Result<&dyn ContextElement, RenderError> { - panic!("Tried to walk down a str"); + fn walk(&self, segment: &str) -> Result<&dyn ContextElement, RenderError> { + Err(RenderError::CantWalk { + segment: segment.to_string(), + elem: self, + }) } } impl Walkable for u32 { - fn walk(&self, _segment: &str) -> Result<&dyn ContextElement, RenderError> { - panic!("Tried to walk down a str"); + fn walk(&self, segment: &str) -> Result<&dyn ContextElement, RenderError> { + Err(RenderError::CantWalk { + segment: segment.to_string(), + elem: self, + }) } } From 610adc8b72c788a773475ca5418fc2e647c767fe Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Sun, 12 Apr 2020 21:03:55 -0400 Subject: [PATCH 4/4] 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 { 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; } 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 ContextElement for HashMap<&str, I> {} impl Renderable for u32 { - fn render(&self) -> std::string::String { - self.to_string() + fn render(&self) -> Result { + Ok(self.to_string()) } } impl Renderable for &str { - fn render(&self) -> std::string::String { - self.to_string() + fn render(&self) -> Result { + Ok(self.to_string()) } } impl Renderable for HashMap<&str, I> { - fn render(&self) -> std::string::String { - panic!("Attempted to render a hashmap"); + fn render(&self) -> Result { + 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() ); } }