From 08e18fc76b60b2deaa7e73ee50c2fa64fecb9d07 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 19 Nov 2018 11:32:23 -0700 Subject: [PATCH] refactor: Embed variable requirements BREAKING CHANGE: This brought back a custom `Path` but still broke it compared to the original. --- liquid-compiler/src/token.rs | 9 ++- liquid-interpreter/src/globals.rs | 7 +- liquid-interpreter/src/variable.rs | 66 ++++++++--------- liquid-value/src/path.rs | 109 ++++++++++++++++++++++++++++- 4 files changed, 144 insertions(+), 47 deletions(-) diff --git a/liquid-compiler/src/token.rs b/liquid-compiler/src/token.rs index 9a5cbce7b..58c0964e5 100644 --- a/liquid-compiler/src/token.rs +++ b/liquid-compiler/src/token.rs @@ -3,6 +3,7 @@ use std::fmt; use liquid_interpreter::Expression; use liquid_interpreter::Variable; use liquid_value::Value; +use liquid_value::Scalar; use super::error::Result; use super::parser::unexpected_token_error; @@ -79,11 +80,9 @@ impl Token { Token::StringLiteral(ref s) => Ok(Expression::with_literal(s.to_owned())), Token::BooleanLiteral(b) => Ok(Expression::with_literal(b)), Token::Identifier(ref id) => { - let mut var = Variable::default(); - var.extend( - id.split('.') - .map(|s| Expression::with_literal(s.to_owned())), - ); + let mut path = id.split('.').map(|s| Scalar::new(s.to_owned())); + let mut var = Variable::with_literal(path.next().expect("there should always be at least one")); + var.extend(path); Ok(Expression::Variable(var)) } ref x => Err(unexpected_token_error( diff --git a/liquid-interpreter/src/globals.rs b/liquid-interpreter/src/globals.rs index 6a9d1f695..de8df26e5 100644 --- a/liquid-interpreter/src/globals.rs +++ b/liquid-interpreter/src/globals.rs @@ -62,18 +62,19 @@ impl Globals for Object { let subpath = &path[0..subpath_end]; if let Some(parent) = self.try_get_variable(subpath) { let subpath = itertools::join(subpath.iter(), "."); - let index = &path[subpath_end]; + let requested = &path[subpath_end]; let available = itertools::join(parent.keys(), ", "); return Err(Error::with_msg("Unknown index") .context("variable", format!("{}", subpath)) - .context("requested index", format!("{}", index)) + .context("requested index", format!("{}", requested)) .context("available indexes", format!("{}", available))); } } + let requested = path.get(0).expect("`Path` guarantees at least one element").to_str().into_owned(); let available = itertools::join(self.keys(), ", "); return Err(Error::with_msg("Unknown variable") - .context("requested variable", path[0].to_str().into_owned()) + .context("requested variable", requested) .context("available variables", available)); } } diff --git a/liquid-interpreter/src/variable.rs b/liquid-interpreter/src/variable.rs index a866442bb..2afa68d36 100644 --- a/liquid-interpreter/src/variable.rs +++ b/liquid-interpreter/src/variable.rs @@ -10,80 +10,70 @@ use super::Expression; use super::Renderable; /// A `Value` reference. -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct Variable { - path: Vec, + variable: Scalar, + indexes: Vec, } impl Variable { /// Create a `Value` reference. pub fn with_literal>(value: S) -> Self { - let expr = Expression::with_literal(value); - let path = vec![expr]; - Self { path } + Self { variable: value.into(), indexes: Default::default() } } /// Append a literal. pub fn push_literal>(mut self, value: S) -> Self { - self.path.push(Expression::with_literal(value)); + self.indexes.push(Expression::with_literal(value)); self } /// Convert to a `Path`. pub fn try_evaluate(&self, context: &Context) -> Option { - let path: Option = self - .path - .iter() - .map(|e| e.try_evaluate(context)) - .map(|v| { - let v = v?; - v.into_scalar() - }).collect(); - path + let mut path = Path::with_index(self.variable.clone()); + path.reserve(self.indexes.len()); + for expr in &self.indexes { + let v = expr.try_evaluate(context)?; + let s = v.into_scalar()?; + path.push(s); + } + Some(path) } /// Convert to a `Path`. pub fn evaluate(&self, context: &Context) -> Result { - let path: Result = self - .path - .iter() - .map(|e| e.evaluate(context)) - .map(|v| { - let v = v?; - let s = v - .as_scalar() - .ok_or_else(|| Error::with_msg(format!("Expected scalar, found `{}`", v)))? - .clone(); - Ok(s) - }).collect(); - path + let mut path = Path::with_index(self.variable.clone()); + path.reserve(self.indexes.len()); + for expr in &self.indexes { + let v = expr.evaluate(context)?; + let s = v.as_scalar() + .ok_or_else(|| Error::with_msg(format!("Expected scalar, found `{}`", v)))? + .to_owned(); + path.push(s); + } + Ok(path) } } impl Extend for Variable { fn extend>(&mut self, iter: T) { let path = iter.into_iter().map(Expression::with_literal); - self.path.extend(path); + self.indexes.extend(path); } } impl Extend for Variable { fn extend>(&mut self, iter: T) { let path = iter.into_iter(); - self.path.extend(path); + self.indexes.extend(path); } } impl fmt::Display for Variable { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut iter = self.path.iter(); - let head = iter.next(); - match head { - Some(head) => write!(f, "{}", head)?, - None => return Ok(()), - } - for index in iter { - write!(f, "[\"{}\"]", index)?; + write!(f, "{}", self.variable)?; + for index in self.indexes.iter() { + write!(f, "[{}]", index)?; } Ok(()) } diff --git a/liquid-value/src/path.rs b/liquid-value/src/path.rs index a2c8be714..3f75811ba 100644 --- a/liquid-value/src/path.rs +++ b/liquid-value/src/path.rs @@ -1,7 +1,114 @@ +use std::fmt; +use std::slice; + +use itertools; + use super::Scalar; /// Path to a value in an `Object`. -pub type Path = Vec; +/// +/// There is guaranteed always at least one element. +#[derive(Clone, Debug, PartialEq)] +pub struct Path(Vec); + +impl Path { + /// Create a `Value` reference. + pub fn with_index>(value: I) -> Self { + let indexes = vec![value.into()]; + Path(indexes) + } + + /// Append an index. + pub fn push>(&mut self, value: I) { + self.0.push(value.into()); + } + + /// Reserves capacity for at least `additional` more elements to be inserted + /// in the given `Path`. The `Path` may reserve more space to avoid + /// frequent reallocations. After calling `reserve`, capacity will be + /// greater than or equal to `self.len() + additional`. Does nothing if + /// capacity is already sufficient. + pub fn reserve(&mut self, additional: usize) { + self.0.reserve(additional); + } + + /// Access the `Value` reference. + pub fn iter(&self) -> PathIter { + PathIter(self.0.iter()) + } + + /// Extracts a slice containing the entire vector. + #[inline] + pub fn as_slice(&self) -> &[Scalar] { + self.0.as_slice() + } +} + +impl Extend for Path { + fn extend>(&mut self, iter: T) { + self.0.extend(iter); + } +} + +impl ::std::ops::Deref for Path { + type Target = [Scalar]; + + #[inline] + fn deref( &self ) -> &Self::Target { + &self.0 + } +} + +impl ::std::borrow::Borrow<[Scalar]> for Path { + #[inline] + fn borrow(&self) -> &[Scalar] { + self + } +} + +impl AsRef<[Scalar]> for Path { + #[inline] + fn as_ref(&self) -> &[Scalar] { + self + } +} + +impl fmt::Display for Path { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let data = itertools::join(self.iter(), "."); + write!(f, "{}", data) + } +} + +/// Iterate over indexes in a `Value`'s `Path`. +#[derive(Debug)] +pub struct PathIter<'i>(slice::Iter<'i, Scalar>); + +impl<'i> Iterator for PathIter<'i> { + type Item = &'i Scalar; + + #[inline] + fn next(&mut self) -> Option<&'i Scalar> { + self.0.next() + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + self.0.size_hint() + } + + #[inline] + fn count(self) -> usize { + self.0.count() + } +} + +impl<'i> ExactSizeIterator for PathIter<'i>{ + #[inline] + fn len(&self) -> usize { + self.0.len() + } +} /// Path to a value in an `Object`. pub type PathRef<'s> = &'s [Scalar];