Skip to content

Commit

Permalink
fix(error): Simplify Result::context API
Browse files Browse the repository at this point in the history
The current API is all or nothing `str` or `Fn -> String`.  This is
awkward for when you have a mixture between key/value.  This new API
let's you build up the state so you can mix and match them.

Related, all of the extension traits more consistently take `Cow`.

BREAKING CHANGE: Everything about the liquid ResultExt traits.
  • Loading branch information
epage committed Dec 9, 2018
1 parent bbd7114 commit 6e6f3b5
Show file tree
Hide file tree
Showing 12 changed files with 244 additions and 58 deletions.
16 changes: 8 additions & 8 deletions liquid-compiler/src/include.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,15 @@ impl Include for FilesystemInclude {
.root
.canonicalize()
.chain("Snippet does not exist")
.context_with(|| {
let key = "non-existent source".into();
let value = self.root.to_string_lossy().into();
(key, value)
})?;
.context_key("non-existent source")
.value_with(|| self.root.to_string_lossy().into_owned().into())?;
let mut path = root.clone();
path.extend(relative_path.split('/'));
let path = path
.canonicalize()
.chain("Snippet does not exist")
.context_with(|| ("non-existent path".into(), path.to_string_lossy().into()))?;
.context_key("non-existent path")
.value_with(|| path.to_string_lossy().into_owned().into())?;
if !path.starts_with(&root) {
return Err(Error::with_msg("Snippet is outside of source")
.context("source", format!("{}", root.display()))
Expand All @@ -81,11 +79,13 @@ impl Include for FilesystemInclude {

let mut file = File::open(&path)
.chain("Failed to open snippet")
.context_with(|| ("full path".into(), path.to_string_lossy().into()))?;
.context_key("full path")
.value_with(|| path.to_string_lossy().into_owned().into())?;
let mut content = String::new();
file.read_to_string(&mut content)
.chain("Failed to read snippet")
.context_with(|| ("full path".into(), path.to_string_lossy().into()))?;
.context_key("full path")
.value_with(|| path.to_string_lossy().into_owned().into())?;
Ok(content)
}
}
223 changes: 201 additions & 22 deletions liquid-error/src/result_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,85 +5,264 @@ use std::result;
use super::Result;
use super::Error;

type CowStr = borrow::Cow<'static, str>;

/// `Result` extension methods for adapting third party errors to `Error`.
pub trait ResultLiquidChainExt<T> {
/// Create an `Error` with `E` as the cause.
fn chain(self, msg: &'static str) -> Result<T>;
///
/// # Example
///
/// ```rust
/// use std::io;
/// use liquid_error::Result;
/// use liquid_error::ResultLiquidChainExt;
///
/// let error = Err(io::Error::new(io::ErrorKind::NotFound, "Oops"));
/// let error: Result<i32> = error.chain("Missing liquid partial");
/// ```
#[must_use]
fn chain<S: Into<CowStr>>(self, msg: S) -> Result<T>;

/// Create an `Error` with `E` as the cause.
///
/// # Example
///
/// ```rust
/// use std::io;
/// use liquid_error::Result;
/// use liquid_error::ResultLiquidChainExt;
///
/// let filename = "foo";
/// let error = Err(io::Error::new(io::ErrorKind::NotFound, "Oops"));
/// let error: Result<i32> = error
/// .chain_with(|| format!("Missing liquid partial: {}", filename).into());
/// ```
#[must_use]
fn chain_with<F>(self, msg: F) -> Result<T>
where
F: FnOnce() -> String;
F: FnOnce() -> CowStr;
}

impl<T, E> ResultLiquidChainExt<T> for result::Result<T, E>
where
E: error::Error + Send + Sync + 'static,
{
fn chain(self, msg: &'static str) -> Result<T> {
fn chain<S: Into<CowStr>>(self, msg: S) -> Result<T> {
self.map_err(|err| Error::with_msg(msg).cause(err))
}

fn chain_with<F>(self, msg: F) -> Result<T>
where
F: FnOnce() -> String,
F: FnOnce() -> CowStr,
{
self.map_err(|err| Error::with_msg(msg()).cause(err))
}
}

/// Add context to a `liquid_error::Error`.
pub trait ResultLiquidExt<T> {
pub trait ResultLiquidExt<T>
where Self: ::std::marker::Sized
{
/// Add a new stack frame to the `liquid_error::Error`.
///
/// # Example
///
/// ```rust
/// use liquid_error::Error;
/// use liquid_error::Result;
/// use liquid_error::ResultLiquidExt;
///
/// let error: Result<i32> = Err(Error::with_msg("Oops"));
/// let error = error.trace("Within forloop");
/// ```
#[must_use]
fn trace<S>(self, trace: S) -> Result<T>
where
S: Into<borrow::Cow<'static, str>>;
S: Into<CowStr>;

/// Add a new stack frame to the `liquid_error::Error`.
///
/// # Example
///
/// ```rust
/// use liquid_error::Error;
/// use liquid_error::Result;
/// use liquid_error::ResultLiquidExt;
///
/// let for_var = "foo";
/// let error: Result<i32> = Err(Error::with_msg("Oops"));
/// let error = error.trace_with(|| format!("Within forloop with {}", for_var).into());
/// ```
#[must_use]
fn trace_with<F>(self, trace: F) -> Result<T>
where
F: FnOnce() -> String;
F: FnOnce() -> CowStr;

/// Add state the current stack frame.
fn context<K, V>(self, key: K, value: V) -> Result<T>
///
/// # Example
///
/// ```rust
/// use liquid_error::Error;
/// use liquid_error::Result;
/// use liquid_error::ResultLiquidExt;
///
/// let for_var = "foo";
/// let error: Result<i32> = Err(Error::with_msg("Oops"));
/// let error = error
/// .context_key("foo")
/// .value("10");
/// let error = error
/// .context_key("foo")
/// .value_with(|| format!("{}", for_var).into());
/// ```
#[must_use]
fn context_key<S>(self, key: S) -> Key<T>
where
K: Into<borrow::Cow<'static, str>>,
V: Into<borrow::Cow<'static, str>>;
S: Into<CowStr>;

/// Add state the current stack frame.
fn context_with<F>(self, context: F) -> Result<T>
///
/// # Example
///
/// ```rust
/// use liquid_error::Error;
/// use liquid_error::Result;
/// use liquid_error::ResultLiquidExt;
///
/// let for_var = "foo";
/// let error: Result<i32> = Err(Error::with_msg("Oops"));
/// let error = error
/// .context_key_with(|| format!("{}", 10).into())
/// .value("10");
/// let error = error
/// .context_key_with(|| format!("{}", 10).into())
/// .value_with(|| format!("{}", for_var).into());
/// ```
#[must_use]
fn context_key_with<F>(self, key: F) -> FnKey<T, F>
where
F: FnOnce() -> (String, String);
F: FnOnce() -> CowStr;
}

impl<T> ResultLiquidExt<T> for Result<T> {
fn trace<S>(self, trace: S) -> Result<T>
where
S: Into<borrow::Cow<'static, str>>,
S: Into<CowStr>,
{
self.map_err(|err| err.trace(trace))
}

fn trace_with<F>(self, trace: F) -> Result<T>
where
F: FnOnce() -> String,
F: FnOnce() -> CowStr,
{
self.map_err(|err| err.trace(trace()))
}

fn context<K, V>(self, key: K, value: V) -> Result<T>
fn context_key<S>(self, key: S) -> Key<T>
where
S: Into<CowStr>
{
Key::new(self, key)
}

fn context_key_with<F>(self, key: F) -> FnKey<T, F>
where
F: FnOnce() -> CowStr {
FnKey::new(self, key)
}
}

/// Partially constructed context (missing value) for `Result<T>`.
#[allow(missing_debug_implementations)]
pub struct Key<T>
{
builder: Result<T>,
key: CowStr,
}

impl<T> Key<T>
{
/// Save off a key for a context that will be added to `builder`.
#[must_use]
pub fn new<S>(builder: Result<T>, key: S) -> Self
where
S: Into<CowStr>,
{
Self {
builder,
key: key.into()
}
}

/// Finish creating context and add it to `Result<T>`.
#[must_use]
pub fn value<S>(self, value: S) -> Result<T>
where
K: Into<borrow::Cow<'static, str>>,
V: Into<borrow::Cow<'static, str>>,
S: Into<CowStr>,
{
self.map_err(|err| err.context(key, value))
let builder = self.builder;
let key = self.key;
builder.map_err(|err| err.context(key, value.into()))
}

fn context_with<F>(self, context: F) -> Result<T>
/// Finish creating context and add it to `Result<T>`.
#[must_use]
pub fn value_with<F>(self, value: F) -> Result<T>
where
F: FnOnce() -> (String, String),
F: FnOnce() ->CowStr
{
let (key, value) = context();
self.map_err(|err| err.context(key, value))
let builder = self.builder;
let key = self.key;
builder.map_err(|err| err.context(key, value()))
}
}

/// Partially constructed context (missing value) for `Result<T>`.
#[allow(missing_debug_implementations)]
pub struct FnKey<T, F>
where
F: FnOnce() -> CowStr,
{
builder: Result<T>,
key: F
}

impl<T, F> FnKey<T, F>
where
F: FnOnce() -> CowStr
{
/// Save off a key for a context that will be added to `builder`.
#[must_use]
pub fn new(builder: Result<T>, key: F) -> Self
{
Self {
builder,
key,
}
}

/// Finish creating context and add it to `Result<T>`.
#[must_use]
pub fn value<S>(self, value: S) -> Result<T>
where
S: Into<CowStr>,
{
let builder = self.builder;
let key = self.key;
builder.map_err(|err| err.context((key)(), value.into()))
}

/// Finish creating context and add it to `Result<T>`.
#[must_use]
pub fn value_with<V>(self, value: V) -> Result<T>
where
V: FnOnce() -> CowStr
{
let builder = self.builder;
let key = self.key;
builder.map_err(|err| err.context((key)(), value()))
}
}
6 changes: 3 additions & 3 deletions liquid-interpreter/src/filter_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ impl FilterChain {
entry = f
.filter(&entry, &*arguments)
.chain("Filter error")
.context_with(|| ("filter".to_owned(), format!("{}", self)))
.context_with(|| ("input".to_owned(), format!("{}", &entry)))
.context_with(|| ("args".to_owned(), itertools::join(&arguments, ", ")))?;
.context_key("filter").value_with(|| format!("{}", self).into())
.context_key("input").value_with(|| format!("{}", &entry).into())
.context_key("args").value_with(|| itertools::join(&arguments, ", ").into())?;
}

Ok(entry)
Expand Down
6 changes: 4 additions & 2 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,13 @@ impl Parser {
fn parse_file_path(self, file: &path::Path) -> Result<Template> {
let mut f = File::open(file)
.chain("Cannot open file")
.context_with(|| ("path".into(), file.to_string_lossy().into()))?;
.context_key("path")
.value_with(|| file.to_string_lossy().into_owned().into())?;
let mut buf = String::new();
f.read_to_string(&mut buf)
.chain("Cannot read file")
.context_with(|| ("path".into(), file.to_string_lossy().into()))?;
.context_key("path")
.value_with(|| file.to_string_lossy().into_owned().into())?;

self.parse(&buf)
}
Expand Down
2 changes: 1 addition & 1 deletion src/tags/assign_tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl Assign {

impl Renderable for Assign {
fn render_to(&self, _writer: &mut Write, context: &mut Context) -> Result<()> {
let value = self.src.evaluate(context).trace_with(|| self.trace())?;
let value = self.src.evaluate(context).trace_with(|| self.trace().into())?;
context.stack_mut().set_global(self.dst.to_owned(), value);
Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions src/tags/capture_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl Renderable for Capture {
let mut captured = Vec::new();
self.template
.render_to(&mut captured, context)
.trace_with(|| self.trace())?;
.trace_with(|| self.trace().into())?;

let output = String::from_utf8(captured).expect("render only writes UTF-8");
context
Expand Down Expand Up @@ -55,7 +55,7 @@ pub fn capture_block(
let template = Template::new(
tokens
.parse_all(options)
.trace_with(|| format!("{{% capture {} %}}", &id))?,
.trace_with(|| format!("{{% capture {} %}}", &id).into())?,
);

tokens.assert_empty();
Expand Down

0 comments on commit 6e6f3b5

Please sign in to comment.