fix(error): Clean up error API

BREAKING CHANGE: Error API changed.
cobalt-org · Sep 26, 2018 · 6a95004 · 6a95004
1 parent 3d18b71
commit 6a95004
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 63 deletions.
diff --git a/liquid-compiler/src/include.rs b/liquid-compiler/src/include.rs
@@ -39,7 +39,7 @@ impl NullInclude {
 
 impl Include for NullInclude {
     fn include(&self, relative_path: &str) -> Result<String> {
-        Err(Error::with_msg("File does not exist").context("path", &relative_path.to_owned()))
+        Err(Error::with_msg("File does not exist").context("path", relative_path.to_owned()))
     }
 }
 
@@ -75,8 +75,8 @@ impl Include for FilesystemInclude {
             .context_with(|| ("non-existent path".into(), path.to_string_lossy().into()))?;
         if !path.starts_with(&root) {
             return Err(Error::with_msg("Snippet is outside of source")
-                .context("source", &root.to_string_lossy())
-                .context("full path", &path.to_string_lossy()));
+                .context("source", format!("{}", root.display()))
+                .context("full path", format!("{}", path.display())));
         }
 
         let mut file = File::open(&path)

diff --git a/liquid-compiler/src/lexer.rs b/liquid-compiler/src/lexer.rs
@@ -172,7 +172,7 @@ pub fn granularize(block: &str) -> Result<Vec<Token>> {
                 Token::Dot
             }
             x if IDENTIFIER.is_match(x) => Token::Identifier(x.to_owned()),
-            x => return Err(Error::with_msg("Invalid identifier").context("identifier", &x)),
+            x => return Err(Error::with_msg("Invalid identifier").context("identifier", format!("{}", x))),
         });
         if let Some(v) = push_more {
             result.extend(v);

diff --git a/liquid-compiler/src/parser.rs b/liquid-compiler/src/parser.rs
@@ -223,7 +223,7 @@ fn parse_tag(
             options.blocks[x.as_str()].parse(x, &tokens[1..], &children, options)
         }
 
-        ref x => Err(Error::with_msg("Tag is not supported").context("tag", x)),
+        ref x => Err(Error::with_msg("Tag is not supported").context("tag", format!("{}", x))),
     }
 }
 

diff --git a/liquid-error/src/error.rs b/liquid-error/src/error.rs
@@ -37,40 +37,42 @@ pub trait ResultLiquidExt<T> {
     /// Add a new stack frame to the `liquid_error::Error`.
     fn trace_with<F>(self, trace: F) -> Result<T>
     where
-        F: FnOnce() -> Trace;
+        F: FnOnce() -> String;
 
     /// Add state the current stack frame.
-    fn context<S>(self, key: &'static str, value: &S) -> Result<T>
+    fn context<K, V>(self, key: K, value: V) -> Result<T>
     where
-        S: ToString;
+        K: Into<borrow::Cow<'static, str>>,
+        V: Into<borrow::Cow<'static, str>>;
 
     /// Add state the current stack frame.
     fn context_with<F>(self, context: F) -> Result<T>
     where
-        F: FnOnce() -> (borrow::Cow<'static, str>, String);
+        F: FnOnce() -> (String, String);
 }
 
 impl<T> ResultLiquidExt<T> for Result<T> {
     fn trace_with<F>(self, trace: F) -> Result<T>
     where
-        F: FnOnce() -> Trace,
+        F: FnOnce() -> String,
     {
         self.map_err(|err| err.trace(trace()))
     }
 
-    fn context<S>(self, key: &'static str, value: &S) -> Result<T>
+    fn context<K, V>(self, key: K, value: V) -> Result<T>
     where
-        S: ToString,
+        K: Into<borrow::Cow<'static, str>>,
+        V: Into<borrow::Cow<'static, str>>
     {
         self.map_err(|err| err.context(key, value))
     }
 
     fn context_with<F>(self, context: F) -> Result<T>
     where
-        F: FnOnce() -> (borrow::Cow<'static, str>, String),
+        F: FnOnce() -> (String, String),
     {
         let (key, value) = context();
-        self.map_err(|err| err.context(key, &value))
+        self.map_err(|err| err.context(key, value))
     }
 }
 
@@ -107,27 +109,35 @@ impl Error {
     }
 
     /// Add a new call to the user-visible backtrace
-    pub fn trace<T: Into<Trace>>(self, trace: T) -> Self {
+    pub fn trace<T>(self, trace: T) -> Self
+    where
+        T: Into<borrow::Cow<'static, str>>,
+    {
         self.trace_trace(trace.into())
     }
 
-    fn trace_trace(mut self, trace: Trace) -> Self {
+    fn trace_trace(mut self, trace: borrow::Cow<'static, str>) -> Self {
+        let trace = Trace::new(trace);
         self.inner.user_backtrace.push(trace);
         self
     }
 
     /// Add context to the last traced call.
     ///
-    /// Example context: Value that parameters from ehe `trace` evaluate to.
-    pub fn context<C, S>(self, key: C, value: &S) -> Self
+    /// Example context: Value that parameters from the `trace` evaluate to.
+    pub fn context<K, V>(self, key: K, value: V) -> Self
     where
-        C: Into<borrow::Cow<'static, str>>,
-        S: ToString,
+        K: Into<borrow::Cow<'static, str>>,
+        V: Into<borrow::Cow<'static, str>>,
     {
-        self.context_cow_string(key.into(), value.to_string())
+        self.context_cow_string(key.into(), value.into())
     }
 
-    fn context_cow_string(mut self, key: borrow::Cow<'static, str>, value: String) -> Self {
+    fn context_cow_string(
+        mut self,
+        key: borrow::Cow<'static, str>,
+        value: borrow::Cow<'static, str>,
+    ) -> Self {
         self.inner
             .user_backtrace
             .last_mut()
@@ -187,54 +197,39 @@ impl error::Error for Error {
 
 /// User-visible call trace
 #[derive(Clone, PartialEq, Eq, Debug, Default)]
-pub struct Trace {
-    trace: Option<String>,
-    context: Vec<(borrow::Cow<'static, str>, String)>,
+struct Trace {
+    trace: Option<borrow::Cow<'static, str>>,
+    context: Vec<(borrow::Cow<'static, str>, borrow::Cow<'static, str>)>,
 }
 
 impl Trace {
-    /// User-visible call trace.
-    pub fn new(trace: String) -> Self {
+    fn new(trace: borrow::Cow<'static, str>) -> Self {
         Self {
             trace: Some(trace),
             context: vec![],
         }
     }
 
-    /// Add context to the traced call.
-    ///
-    /// Example context: Value that parameters from ehe `trace` evaluate to.
-    pub fn context(mut self, key: borrow::Cow<'static, str>, value: String) -> Self {
-        self.context.push((key, value));
-        self
-    }
-
-    pub(self) fn empty() -> Self {
+    fn empty() -> Self {
         Self {
             trace: None,
             context: vec![],
         }
     }
 
-    pub(self) fn append_context(&mut self, key: borrow::Cow<'static, str>, value: String) {
+    fn append_context(&mut self, key: borrow::Cow<'static, str>, value: borrow::Cow<'static, str>) {
         self.context.push((key, value));
     }
 
-    pub(self) fn get_trace(&self) -> Option<&str> {
+    fn get_trace(&self) -> Option<&str> {
         self.trace.as_ref().map(|s| s.as_ref())
     }
 
-    pub(self) fn get_context(&self) -> &[(borrow::Cow<'static, str>, String)] {
+    fn get_context(&self) -> &[(borrow::Cow<'static, str>, borrow::Cow<'static, str>)] {
         self.context.as_ref()
     }
 }
 
-impl From<String> for Trace {
-    fn from(trace: String) -> Self {
-        Self::new(trace)
-    }
-}
-
 #[derive(Debug)]
 enum ErrorCause {
     Generic(GenericError),

diff --git a/liquid-interpreter/src/context.rs b/liquid-interpreter/src/context.rs
@@ -88,8 +88,8 @@ impl<'a, 'g> CycleState<'a, 'g> {
         if index >= values.len() {
             return Err(Error::with_msg(
                 "cycle index out of bounds, most likely from mismatched cycles",
-            ).context("index", &index)
-                .context("count", &values.len()));
+            ).context("index", format!("{}", index))
+                .context("count", format!("{}", values.len())));
         }
 
         let val = values[index].evaluate(self.context)?;
@@ -159,17 +159,17 @@ impl<'g> Stack<'g> {
             .next()
             .ok_or_else(|| Error::with_msg("No index provided"))?;
         let key = key.as_key().ok_or_else(|| {
-            Error::with_msg("Root index must be an object key").context("index", &key)
+            Error::with_msg("Root index must be an object key").context("index", format!("{}", key))
         })?;
         let value = self
             .get_val(key)
-            .ok_or_else(|| Error::with_msg("Invalid index").context("index", &key))?;
+            .ok_or_else(|| Error::with_msg("Invalid index").context("index", format!("{}", key)))?;
 
         indexes.fold(Ok(value), |value, index| {
             let value = value?;
             let child = value.get(index);
             let child =
-                child.ok_or_else(|| Error::with_msg("Invalid index").context("index", &key))?;
+                child.ok_or_else(|| Error::with_msg("Invalid index").context("index", format!("{}", key)))?;
             Ok(child)
         })
     }

diff --git a/liquid-interpreter/src/output.rs b/liquid-interpreter/src/output.rs
@@ -59,7 +59,7 @@ impl FilterChain {
         // apply all specified filters
         for filter in &self.filters {
             let f = context.get_filter(&filter.name).ok_or_else(|| {
-                Error::with_msg("Unsupported filter").context("filter", &filter.name)
+                Error::with_msg("Unsupported filter").context("filter", filter.name.clone())
             })?;
 
             let arguments: Result<Vec<Value>> = filter
@@ -71,8 +71,8 @@ impl FilterChain {
             entry = f
                 .filter(&entry, &*arguments)
                 .chain("Filter error")
-                .context("input", &entry)
-                .context_with(|| ("args".into(), itertools::join(&arguments, ", ")))?;
+                .context_with(|| ("input".to_owned(), format!("{}", &entry)))
+                .context_with(|| ("args".to_owned(), itertools::join(&arguments, ", ")))?;
         }
 
         Ok(entry)

diff --git a/src/filters/mod.rs b/src/filters/mod.rs
@@ -22,21 +22,21 @@ use unicode_segmentation::UnicodeSegmentation;
 
 use interpreter::FilterResult;
 
-pub fn invalid_input<'s, S: Into<Cow<'s, str>>>(cause: S) -> liquid_error::Error {
-    liquid_error::Error::with_msg("Invalid input").context("cause", &cause.into())
+pub fn invalid_input<S: Into<Cow<'static, str>>>(cause: S) -> liquid_error::Error {
+    liquid_error::Error::with_msg("Invalid input").context("cause", cause)
 }
 
-pub fn invalid_argument_count<'s, S: Into<Cow<'s, str>>>(cause: S) -> liquid_error::Error {
-    liquid_error::Error::with_msg("Invalid number of arguments").context("cause", &cause.into())
+pub fn invalid_argument_count<S: Into<Cow<'static, str>>>(cause: S) -> liquid_error::Error {
+    liquid_error::Error::with_msg("Invalid number of arguments").context("cause", cause)
 }
 
-pub fn invalid_argument<'s, S: Into<Cow<'s, str>>>(
+pub fn invalid_argument<S: Into<Cow<'static, str>>>(
     position: usize,
     cause: S,
 ) -> liquid_error::Error {
     liquid_error::Error::with_msg("Invalid argument")
-        .context("position", &position)
-        .context("cause", &cause.into())
+        .context("position", format!("{}", position))
+        .context("cause", cause)
 }
 
 // Helper functions for the filters.
@@ -46,14 +46,14 @@ fn check_args_len(
     optional: usize,
 ) -> Result<(), liquid_error::Error> {
     if args.len() < required {
-        return Err(invalid_argument_count(&format!(
+        return Err(invalid_argument_count(format!(
             "expected at least {}, {} given",
             required,
             args.len()
         )));
     }
     if required + optional < args.len() {
-        return Err(invalid_argument_count(&format!(
+        return Err(invalid_argument_count(format!(
             "expected at most {}, {} given",
             required + optional,
             args.len()

diff --git a/src/tags/for_block.rs b/src/tags/for_block.rs
@@ -150,7 +150,7 @@ impl Renderable for For {
                             .render_to(writer, &mut scope)
                             .trace_with(|| self.trace().into())
                             .context_with(|| (self.var_name.clone().into(), v.to_string()))
-                            .context("index", &(i + 1))?;
+                            .context_with(|| ("index".to_owned(), format!("{}", i + 1)))?;
 
                         // given that we're at the end of the loop body
                         // already, dealing with a `continue` signal is just